fix(security): eliminate TOCTOU race condition in atomic writes

[dumko2001]

Description

This PR fixes a critical TOCTOU (Time-of-Check to Time-of-Use) race condition in fs_util::atomic_write and fs_util::atomic_write_async where temporary files were initially created with default system permissions before being tightened to 0600. This could allow a local attacker to read sensitive OAuth secrets or encrypted credentials during the brief window before permissions were applied.

Fixes #401

Changes:

• Switched to tempfile::NamedTempFile for synchronous atomic_write, ensuring 0600 permissions and O_EXCL flags are set atomically at creation time.
• Updated atomic_write_async to use tokio::fs::OpenOptions with .mode(0o600) for secure asynchronous writes.
• Removed redundant set_permissions calls in oauth_config.rs and credential_store.rs that are now handled natively by the atomic write utilities.
• Promoted tempfile to a regular dependency (was previously only in dev-dependencies).
• Added unit tests in fs_util.rs to verify that files are created with 0600 permissions on Unix.

Dry Run Output:
N/A (This change affects internal file persistence logic, not Discovery-based API requests).

Checklist:

• My code follows the AGENTS.md guidelines.
• I have run cargo fmt --all to format the code perfectly.
• I have run cargo clippy -- -D warnings and resolved all warnings.
• I have added tests that prove my fix is effective.
• I have provided a Changeset file (.changeset/fix-security-toctou.md).

[changeset-bot]

🦋 Changeset detected

Latest commit: b32659b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@googleworkspace/cli	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the security posture of file persistence operations by addressing a critical TOCTOU race condition. By ensuring that sensitive files, such as OAuth secrets and encrypted credentials, are created with strict permissions from the outset, it mitigates the risk of local attackers exploiting a brief window of vulnerability. The changes streamline file writing logic by centralizing secure temporary file handling, improving both robustness and maintainability.

Highlights

• Security Fix: Eliminated a critical TOCTOU (Time-of-Check to Time-of-Use) race condition in fs_util::atomic_write and fs_util::atomic_write_async.
• Secure File Creation: Synchronous atomic_write now uses tempfile::NamedTempFile to ensure temporary files are created with 0600 permissions and O_EXCL flags atomically.
• Asynchronous Security: atomic_write_async was updated to use tokio::fs::OpenOptions with create_new(true) and mode(0o600) for secure asynchronous writes, including randomized temporary filenames.
• Permission Redundancy Removed: Redundant set_permissions calls in oauth_config.rs and credential_store.rs were removed as permissions are now handled by the atomic write utilities.
• Dependency Update: The tempfile crate was promoted from a development dependency to a regular dependency.
• Test Coverage: Added unit tests to fs_util.rs to verify that files are created with 0600 permissions on Unix systems.

Changelog

• .changeset/fix-security-toctou.md
  • Added a new changeset entry detailing the security fix, including randomized filenames, O_EXCL flags, and strict 0600 permissions.
• Cargo.toml
  • Moved the tempfile dependency from [dev-dependencies] to [dependencies].
• src/credential_store.rs
  • Updated the save_key_file function to delegate file writing to crate::fs_util::atomic_write, removing the previous manual file creation and permission setting logic.
  • Removed the explicit set_permissions call for Unix systems within the save_encrypted function, relying on the atomic write utility for secure permissions.
• src/fs_util.rs
  • Refactored atomic_write to utilize tempfile::NamedTempFile::new_in for atomic, secure temporary file creation and persistence.
  • Rewrote atomic_write_async to use tokio::fs::OpenOptions with create_new(true) and mode(0o600) for secure asynchronous temporary file creation, incorporating random suffix generation and retry logic.
  • Added rand and tokio::io::AsyncWriteExt imports.
  • Introduced new unit tests to verify that files created by atomic_write and atomic_write_async have 0o600 permissions on Unix.
  • Modified the test for atomic_write to check for the absence of .tmp files using directory listing instead of a specific temporary filename.
• src/oauth_config.rs
  • Removed the explicit set_permissions call for Unix systems within the save_client_config function, relying on the atomic write utility for secure permissions.

Activity

• No human activity (comments, reviews, etc.) has occurred on this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Generative AI Prohibited Use Policy, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request effectively eliminates a critical TOCTOU race condition in atomic_write and atomic_write_async. The synchronous implementation correctly leverages tempfile::NamedTempFile to ensure atomic creation with secure permissions. The asynchronous version is also correctly implemented to generate a random temporary file, create it with O_EXCL and secure permissions, and then atomically rename it. The related changes, such as removing redundant permission-setting calls and updating dependencies, are appropriate. The addition of tests to verify file permissions is a valuable improvement. The changes are well-executed and significantly improve the security of file writing operations. I have not found any issues of high or critical severity.