feat(drive): add +download helper for downloading Drive files#704
feat(drive): add +download helper for downloading Drive files#704nuthalapativarun wants to merge 10 commits intogoogleworkspace:mainfrom
Conversation
Adds a multi-step +download command that determines how to fetch a file based on its MIME type: - Google Workspace native files (Docs/Sheets/Slides) → files.export with a caller-supplied --mime-type (e.g. application/pdf, text/csv) - Binary/other files → files.get?alt=media The output path is validated with validate_safe_file_path() to reject path traversal and control characters. The file ID is validated with validate_resource_name() before being embedded in URLs. Complements the existing +upload helper; justified as a multi-step orchestration helper per the helper guidelines in AGENTS.md.
- Sanitize API error body strings with sanitize_for_terminal() before embedding them in GwsError::Api to prevent terminal escape injection - Add --dry-run support: after metadata fetch and path resolution, print what would be downloaded and return without network/disk I/O - Stream response bytes to file via tokio::fs::File + bytes_stream() instead of resp.bytes().await to avoid OOM on large Drive files
- Parse Google API error JSON properly (reason, message, enable_url) so that specialised error hints in error.rs (e.g. accessNotConfigured) still fire - Sanitize Drive filename: replace '/' chars to prevent unintended subdirectories - Use u64 for byte_count to avoid overflow on 32-bit platforms with >4GB files - Stream to a .tmp file and rename on success; delete partial file on any error - Report actual output MIME type in JSON result (export format for native files)
…tion, rename cleanup - Sanitize backslash in addition to slash in Drive filename to prevent unintended subdirectory creation on Windows - Validate --mime-type requirement for native Google Workspace files before the dry-run block so dry-run output is accurate and errors surface early - Clean up temp file if tokio::fs::rename fails to avoid orphaned .tmp files
… rename, dedup error parsing - Short-circuit dry-run before any auth or network I/O (consistent with +upload pattern) so unauthenticated users can verify syntax offline - Remove local parse_google_api_error; add pub executor::api_error_from_response so error parsing logic lives in one place and accessNotConfigured hints fire consistently from all callers - Remove existing destination file before rename for cross-platform consistency: tokio::fs::rename overwrites on Unix but fails if dest exists on Windows; TOCTOU limitation is documented as a known constraint
…response and +download - Use "httpError" (not "unknown") as fallback reason in api_error_from_response to match the existing handle_error_response fallback in executor.rs - Use snake_case keys in all JSON output (dry_run, file_id, export_mime_type, saved_file, mime_type) to match the project convention used in executor.rs - Expand filename sanitization to cover all Windows-reserved chars (:*?"<>|), control characters (Cc), and dangerous Unicode (Cf/bidi/zero-width) to prevent file creation failures and terminal injection on all platforms
…on all platforms tokio::fs::rename uses MOVEFILE_REPLACE_EXISTING on Windows (since Rust 1.26), so the destination file is always overwritten atomically. The remove_file + rename sequence added previously was wrong: it widens the failure window so that if rename fails the user loses the original file at the destination as well.
…name
- Sanitize `message` and `reason` fields extracted from Google API JSON
error responses to prevent terminal escape sequence injection from
malicious or compromised API responses.
- Add a random 64-bit hex suffix to the download temp filename
(`.{name}.{rand}.tmp`) to prevent symlink attacks in world-writable
output directories.
…ints - Pass `RestDescription` to `handle_download` so it resolves method paths, OAuth scopes (union of files.get + files.export), and the API base URL from the Discovery Document instead of hardcoded strings. This correctly respects custom root_url values (proxies, VPC-SC, alternative environments). - Add `x-goog-user-project` header to all manual requests in `handle_download` for correct billing/quota attribution with service accounts, mirroring the existing behaviour in `executor::execute_method`. - Add `auth_method` parameter to `api_error_from_response` and include the 401/403 login hint (matching `handle_error_response`) so the public function is complete and correct for all callers.
🦋 Changeset detectedLatest commit: 9d50117 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 introduces a new 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 the +download helper command for the Google Drive service, enabling users to download both binary and native Google Workspace files. The implementation includes metadata fetching, path traversal validation, and atomic file writing via temporary files. Additionally, a new utility for parsing Google API error responses was added to the executor. Feedback focuses on ensuring JSON output keys follow camelCase conventions for consistency with the Discovery API and improving the robustness of URL construction by handling potential missing trailing slashes in the base URL.
Three issues from review:
1. dry-run JSON used snake_case keys (export_mime_type, file_id); changed
to camelCase (exportMimeType, fileId) to match Discovery API conventions
and other helpers.
2. success JSON used mime_type and saved_file; changed to mimeType and
savedFile for the same reason.
3. Both metadata_url and export_url concatenated base_url directly without
guarding against a missing trailing slash. Discovery Documents usually
include it in servicePath, but proxies or custom root_url overrides may
not. Apply trim_end_matches('/') + '/' prefix on the path segment so the
URL is always well-formed.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new +download helper command for Google Drive, allowing users to download both binary files and Google Workspace native files with automatic export format handling. The implementation includes robust input validation to prevent path traversal, filename sanitization, and atomic file writing via temporary files. Additionally, a new api_error_from_response utility was added to the executor to improve error reporting for manual HTTP requests. I have no feedback to provide as there were no review comments to assess.
Description
Adds a
drive +downloadhelper that downloads a Drive file to a local path. This is a multi-step orchestration helper that cannot be expressed as a single Discovery API call:files.get?fields=name,mimeTypeto determine the file's name and typeapplication/vnd.google-apps.*) →files.export?mimeType=<type>, requiring a caller-supplied--mime-typefiles.get?alt=mediaThis complements the existing
+uploadhelper and satisfies the helper justification criteria in AGENTS.md (multi-step orchestration, format translation for native files).Input validation:
--file(Drive file ID):validate_resource_name()— rejects traversal/injection--output(local path):validate_safe_file_path()— rejects path traversal outside CWD--mime-type:reject_dangerous_chars()— rejects control charactersUsage:
Dry Run Output:
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.