feat(drive): add +download, +export, and +move helpers#664
feat(drive): add +download, +export, and +move helpers#664Bot-Dev-RPA wants to merge 1 commit intogoogleworkspace:mainfrom
Conversation
🦋 Changeset detectedLatest commit: e8f62f5 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 |
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 expands the functionality of the Google Drive CLI by introducing three new helper commands that simplify common multi-step operations. These additions improve user experience by automating metadata fetching, format conversion, and parent folder management, while also ensuring robust security through path validation and API-specific guards. Highlights
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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces new Drive helper commands: +download for binary files, +export for Google Workspace documents, and +move for relocating files between folders. It also refactors the Drive helper into a modular structure. Feedback was provided regarding potential terminal escape sequence injection vulnerabilities in error messages within the download and export modules, where untrusted metadata from the Drive API is displayed without sanitization.
5905b35 to
b505df7
Compare
|
Addressed both review comments in b505df7: Terminal escape injection: Added Both comments were valid -- thanks for catching them. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces new Drive helper commands (+download, +export, and +move) and refactors the existing +upload command into a modular structure. The +download command handles binary file retrieval, while +export converts Google Workspace documents into various local formats. The +move command facilitates moving files between folders. Feedback highlights a path traversal vulnerability in the shared path resolution utility, logic errors in handling file extensions during export, and the inclusion of an unsupported markdown export format which would lead to API errors.
Add three new Drive helper commands that provide multi-step orchestration beyond what Discovery commands offer: - +download: fetches file metadata for the real filename, guards against Google Workspace docs (suggests +export), then streams binary content to disk - +export: maps friendly format names (pdf, docx, xlsx, md, etc.) to MIME types, fetches metadata, guards against non-Workspace files (suggests +download), then streams exported content to disk - +move: fetches current parent folder(s), filters destination from removal list to avoid API conflicts, then patches with addParents/removeParents in a single update Security: - Final resolved paths are validated with validate_safe_file_path() after incorporating untrusted remote filenames from the Drive API - encode_path_segment() used for all file IDs in URL paths - MIME type guards prevent misuse in both directions Also refactors the existing +upload helper from a single drive.rs file into a drive/ module directory, matching the gmail/ and events/ module patterns.
b505df7 to
e8f62f5
Compare
|
Addressed all review comments in e8f62f5: resolve_output_path path traversal (critical): Now uses Export directory output missing extension: Fixed. Terminal escape injection: Already fixed in previous push -- text/markdown not supported (false positive): The Drive API |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces new Google Drive helper commands—+download, +export, and +move—and refactors the existing +upload command into a modular structure. The review feedback identifies several critical issues: manual API requests are missing the x-goog-user-project header required for quota attribution, and the path resolution logic is duplicated and unreliable in its directory detection. Additionally, the reviewer recommends improving manual error handling to provide a better user experience and align with the project's core executor standards.
|
Addressed the valid findings from the latest review in e8f62f5. Responding to the remaining comments: x-goog-user-project header (mod.rs:236, move_file.rs:85): No existing helper in the codebase sets this header -- grep for Error handling JSON parsing (mod.rs:249): Our error handling matches the existing helper pattern exactly. See Export path duplication (export.rs:99): This is intentional, not duplication. The export case differs from is_dir() check (mod.rs:308): Reasonable suggestion but introduces TOCTOU. No existing helper in the codebase uses text/markdown (export.rs:159): Already confirmed -- Happy to address any further feedback from maintainers. |
Summary
+download: Fetches file metadata for the real filename, guards against Google Workspace docs (suggests+export), then streams binary content to disk+export: Maps friendly format names (pdf, docx, xlsx, md, etc.) to MIME types, fetches metadata, guards against non-Workspace files (suggests+download), then streams exported content to disk+move: Fetches current parent folder(s), filters destination from removal list to avoid API conflicts, then patches withaddParents/removeParentsAlso refactors the existing
+uploadhelper from a singledrive.rsfile into adrive/module directory, matching thegmail/andevents/module patterns.Justification
All three helpers pass the litmus test -- they provide multi-step orchestration that Discovery commands cannot:
+downloadalt=mediadownload.binwith no real filename+export/export+moveaddParents/removeParentsSecurity
validate_safe_file_path()is called on the final resolved path after incorporating the untrusted remote filename from the Drive API (not just the raw--outputflag)encode_path_segment()on all file IDs in URL paths+downloadrejects Workspace docs,+exportrejects regular files)+movefilters destination folder fromremoveParentsto avoid Drive API conflict when a file is already in the target foldersend_with_retry()for all API callsTest plan
cargo clippy -- -D warningsclean+downloadon a Doc returns validation error suggesting+export+exporton a PDF returns validation error suggesting+download--dry-runon all helpers outputs correct JSON--output ../../.ssh/keys) rejected--format mp4) rejected with supported list