fix(security): validate --upload and --output file paths#374
fix(security): validate --upload and --output file paths#374AshwinRenjith wants to merge 4 commits intogoogleworkspace:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 6eaa5ad The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
googleworkspace-bot
added
area: http
area: core
Summary of ChangesHello, I'm Gemini Code Assist1! 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 improves the security posture of the application by implementing robust validation for user-provided file paths. It addresses a potential trust-boundary gap by ensuring that paths specified via Highlights
Changelog
Activity
Using Gemini Code AssistThe 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
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces important security validations for file paths provided via --upload and --output. However, the validation logic for output files is vulnerable to a bypass using broken symbolic links, which could allow an attacker to write files outside the current directory. Additionally, a critical Time-of-check to Time-of-use (TOCTOU) race condition vulnerability exists in the new validation logic, potentially allowing unintended file access. The new validation functions also perform blocking I/O within an async context, which may impact performance and stability.
|
/gemini review |
1 similar comment
|
/gemini review |
|
/gemini review |
Harden file path handling for untrusted CLI inputs by adding safe upload/output validators and enforcing them centrally in execute_method. Covers traversal, absolute paths, control characters, and symlink escapes before any file I/O. Adds regression tests and required changeset.
Address broken symlink prefix bypass in output path validation by using symlink-aware normalization and canonicalization. Also run path validators via spawn_blocking in execute_method to avoid blocking filesystem checks on async runtime threads. Adds regression coverage for broken symlink prefixes.
Disallow existing FIFO/special-file paths for --output so untrusted CLI inputs cannot redirect writes to non-regular sinks. Adds unix regression test for FIFO target rejection.
AshwinRenjith
force-pushed
the
ashwin/codex/pr1-path-validation-upload-output
branch
from
7ec989e to
6eaa5ad
Compare
|
/gemini review |
2 participants
Harden file path handling for untrusted CLI inputs by adding safe upload/output validators and enforcing them centrally in
execute_method.Covers traversal, absolute paths, control characters, and symlink escapes before any file I/O. Adds regression tests and required changeset.
Description
This PR fixes a filesystem trust-boundary gap where untrusted CLI path inputs (
--upload,--output) could be consumed directly for file I/O.Changes:
validate_safe_upload_file_pathandvalidate_safe_output_file_pathinsrc/validate.rs.src/executor.rs::execute_method, so all command paths (including helpers) are covered..changeset/hardening-upload-output-paths.md.Dry Run Output (
--outputsafe path):{ "body": null, "dry_run": true, "is_multipart_upload": false, "method": "GET", "query_params": {}, "url": "https://www.googleapis.com/drive/v3/files" }Dry Run Output (
--uploadsafe path):{ "body": null, "dry_run": true, "is_multipart_upload": true, "method": "POST", "query_params": {}, "url": "https://www.googleapis.com/upload/drive/v3/files" }Checklist:
AGENTS.mdguidelines (no generatedgoogle-*crates).cargo fmt --allto format the code perfectly.cargo clippy -- -D warningsand resolved all warnings.pnpx changeset) to document my changes.