From 11824cea5edcc165618e8b5c8ce67e30b6b65c03 Mon Sep 17 00:00:00 2001
From: Varun Nuthalapati <nuthalapativarun@gmail.com>
Date: Sun, 5 Apr 2026 21:45:11 -0700
Subject: [PATCH 1/9] feat(drive): add +download helper for downloading Drive
 files
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 .changeset/drive-download-helper.md           |  22 ++
 .../google-workspace-cli/src/helpers/drive.rs | 230 +++++++++++++++++-
 2 files changed, 247 insertions(+), 5 deletions(-)
 create mode 100644 .changeset/drive-download-helper.md

diff --git a/.changeset/drive-download-helper.md b/.changeset/drive-download-helper.md
new file mode 100644
index 00000000..bf77e768
--- /dev/null
+++ b/.changeset/drive-download-helper.md
@@ -0,0 +1,22 @@
+---
+"@googleworkspace/cli": minor
+---
+
+Add `drive +download` helper for downloading Drive files to a local path
+
+The new `+download` command is a multi-step helper that:
+1. Fetches file metadata (name, MIME type) to determine how to download
+2. For Google Workspace native files (Docs, Sheets, Slides) uses `files.export`
+   with the caller-supplied `--mime-type` (e.g. `application/pdf`, `text/csv`)
+3. For all other files uses `files.get?alt=media`
+4. Writes the response bytes to a local path validated against path traversal
+
+This complements the existing `+upload` helper and follows all helper
+guidelines: it performs multi-step orchestration that the raw Discovery
+API cannot express as a single call.
+
+```
+gws drive +download --file FILE_ID
+gws drive +download --file FILE_ID --output report.pdf
+gws drive +download --file FILE_ID --mime-type application/pdf
+```
diff --git a/crates/google-workspace-cli/src/helpers/drive.rs b/crates/google-workspace-cli/src/helpers/drive.rs
index 68662ec6..1b058c3c 100644
--- a/crates/google-workspace-cli/src/helpers/drive.rs
+++ b/crates/google-workspace-cli/src/helpers/drive.rs
@@ -63,6 +63,47 @@ TIPS:
   Filename is inferred from the local path unless --name is given.",
                 ),
         );
+        cmd = cmd.subcommand(
+            Command::new("+download")
+                .about("[Helper] Download a Drive file to a local path")
+                .arg(
+                    Arg::new("file")
+                        .long("file")
+                        .help("Drive file ID")
+                        .required(true)
+                        .value_name("ID"),
+                )
+                .arg(
+                    Arg::new("output")
+                        .long("output")
+                        .help("Output file path (defaults to the file's name in Drive)")
+                        .value_name("PATH"),
+                )
+                .arg(
+                    Arg::new("mime-type")
+                        .long("mime-type")
+                        .help(
+                            "Export MIME type for Google Workspace native files \
+                             (e.g. application/pdf, text/csv, \
+                             application/vnd.openxmlformats-officedocument.wordprocessingml.document). \
+                             Required for Docs/Sheets/Slides; ignored for binary files.",
+                        )
+                        .value_name("TYPE"),
+                )
+                .after_help(
+                    "\
+EXAMPLES:
+  gws drive +download --file FILE_ID
+  gws drive +download --file FILE_ID --output report.pdf
+  gws drive +download --file FILE_ID --mime-type application/pdf
+  gws drive +download --file FILE_ID --mime-type text/csv --output data.csv
+
+TIPS:
+  For Google Docs/Sheets/Slides, provide --mime-type to choose the export format.
+  For binary files (PDFs, images, etc.), --mime-type is not needed.
+  Output path must be relative to the current directory.",
+                ),
+        );
         cmd
     }
 
@@ -91,7 +132,7 @@ TIPS:
                 })?;
 
                 // Build metadata
-                let metadata = build_metadata(&filename, parent_id.map(|s| s.as_str()));
+                let metadata = build_metadata(&filename, parent_id.map(|s| s.as_str()))?;
 
                 let body_str = metadata.to_string();
 
@@ -125,11 +166,162 @@ TIPS:
 
                 return Ok(true);
             }
+
+            if let Some(matches) = matches.subcommand_matches("+download") {
+                handle_download(matches).await?;
+                return Ok(true);
+            }
+
             Ok(false)
         })
     }
 }
 
+async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
+    let file_id =
+        crate::validate::validate_resource_name(matches.get_one::<String>("file").unwrap())?;
+    let output_arg = matches.get_one::<String>("output");
+    let export_mime = matches.get_one::<String>("mime-type");
+
+    // Validate export mime-type for dangerous characters if provided
+    if let Some(mime) = export_mime {
+        crate::validate::reject_dangerous_chars(mime, "--mime-type")?;
+    }
+
+    let scope = "https://www.googleapis.com/auth/drive.readonly";
+    let token = auth::get_token(&[scope])
+        .await
+        .map_err(|e| GwsError::Auth(format!("Drive auth failed: {e}")))?;
+
+    let client = crate::client::build_client()?;
+
+    // 1. Fetch file metadata to get name and MIME type
+    let metadata_url = format!(
+        "https://www.googleapis.com/drive/v3/files/{}",
+        crate::validate::encode_path_segment(file_id),
+    );
+    let meta_resp = crate::client::send_with_retry(|| {
+        client
+            .get(&metadata_url)
+            .query(&[("fields", "name,mimeType")])
+            .bearer_auth(&token)
+    })
+    .await
+    .map_err(|e| GwsError::Other(anyhow::anyhow!("Failed to fetch file metadata: {e}")))?;
+
+    if !meta_resp.status().is_success() {
+        let status = meta_resp.status().as_u16();
+        let body = meta_resp.text().await.unwrap_or_default();
+        return Err(GwsError::Api {
+            code: status.into(),
+            message: body,
+            reason: "files_get_metadata_failed".to_string(),
+            enable_url: None,
+        });
+    }
+
+    let meta: Value = meta_resp
+        .json()
+        .await
+        .map_err(|e| GwsError::Other(anyhow::anyhow!("Failed to parse file metadata: {e}")))?;
+
+    let drive_name = meta
+        .get("name")
+        .and_then(|v| v.as_str())
+        .unwrap_or("download");
+    let mime_type = meta
+        .get("mimeType")
+        .and_then(|v| v.as_str())
+        .unwrap_or("");
+
+    // 2. Resolve and validate output path
+    let out_str = output_arg.map(|s| s.as_str()).unwrap_or(drive_name);
+    let out_path = crate::validate::validate_safe_file_path(out_str, "--output")?;
+
+    // 3. Fetch file content — native Google Workspace files require export,
+    //    everything else uses alt=media.
+    let is_google_native = mime_type.starts_with("application/vnd.google-apps.");
+    let bytes = if is_google_native {
+        let mime = export_mime.ok_or_else(|| {
+            GwsError::Validation(format!(
+                "The file is a Google Workspace native file ({mime_type}). \
+                 Provide --mime-type to choose an export format, e.g. \
+                 --mime-type application/pdf or --mime-type text/csv"
+            ))
+        })?;
+        let export_url = format!(
+            "https://www.googleapis.com/drive/v3/files/{}/export",
+            crate::validate::encode_path_segment(file_id),
+        );
+        let resp = crate::client::send_with_retry(|| {
+            client
+                .get(&export_url)
+                .query(&[("mimeType", mime.as_str())])
+                .bearer_auth(&token)
+        })
+        .await
+        .map_err(|e| GwsError::Other(anyhow::anyhow!("Drive export request failed: {e}")))?;
+
+        if !resp.status().is_success() {
+            let status = resp.status().as_u16();
+            let body = resp.text().await.unwrap_or_default();
+            return Err(GwsError::Api {
+                code: status.into(),
+                message: body,
+                reason: "files_export_failed".to_string(),
+                enable_url: None,
+            });
+        }
+        resp.bytes()
+            .await
+            .map_err(|e| GwsError::Other(anyhow::anyhow!("Failed to read export response: {e}")))?
+    } else {
+        let resp = crate::client::send_with_retry(|| {
+            client
+                .get(&metadata_url)
+                .query(&[("alt", "media")])
+                .bearer_auth(&token)
+        })
+        .await
+        .map_err(|e| GwsError::Other(anyhow::anyhow!("Drive download request failed: {e}")))?;
+
+        if !resp.status().is_success() {
+            let status = resp.status().as_u16();
+            let body = resp.text().await.unwrap_or_default();
+            return Err(GwsError::Api {
+                code: status.into(),
+                message: body,
+                reason: "files_get_media_failed".to_string(),
+                enable_url: None,
+            });
+        }
+        resp.bytes()
+            .await
+            .map_err(|e| GwsError::Other(anyhow::anyhow!("Failed to read download response: {e}")))?
+    };
+
+    // 4. Write to the output file
+    std::fs::write(&out_path, &bytes).map_err(|e| {
+        GwsError::Other(anyhow::anyhow!(
+            "Failed to write '{}': {e}",
+            out_path.display()
+        ))
+    })?;
+
+    // 5. Print result as JSON (consistent with other helper output)
+    println!(
+        "{}",
+        serde_json::to_string_pretty(&json!({
+            "file": out_path.display().to_string(),
+            "bytes": bytes.len(),
+            "mimeType": mime_type,
+        }))
+        .unwrap_or_default()
+    );
+
+    Ok(())
+}
+
 fn determine_filename(file_path: &str, name_arg: Option<&str>) -> Result<String, GwsError> {
     if let Some(n) = name_arg {
         Ok(n.to_string())
@@ -142,16 +334,17 @@ fn determine_filename(file_path: &str, name_arg: Option<&str>) -> Result<String,
     }
 }
 
-fn build_metadata(filename: &str, parent_id: Option<&str>) -> Value {
+fn build_metadata(filename: &str, parent_id: Option<&str>) -> Result<Value, GwsError> {
     let mut metadata = json!({
         "name": filename
     });
 
     if let Some(parent) = parent_id {
+        crate::validate::validate_resource_name(parent)?;
         metadata["parents"] = json!([parent]);
     }
 
-    metadata
+    Ok(metadata)
 }
 
 #[cfg(test)]
@@ -182,15 +375,42 @@ mod tests {
 
     #[test]
     fn test_build_metadata_no_parent() {
-        let meta = build_metadata("file.txt", None);
+        let meta = build_metadata("file.txt", None).unwrap();
         assert_eq!(meta["name"], "file.txt");
         assert!(meta.get("parents").is_none());
     }
 
     #[test]
     fn test_build_metadata_with_parent() {
-        let meta = build_metadata("file.txt", Some("folder123"));
+        let meta = build_metadata("file.txt", Some("folder123")).unwrap();
         assert_eq!(meta["name"], "file.txt");
         assert_eq!(meta["parents"][0], "folder123");
     }
+
+    #[test]
+    fn test_build_metadata_rejects_traversal_parent_id() {
+        assert!(
+            build_metadata("file.txt", Some("../../.ssh/id_rsa")).is_err(),
+            "path traversal in --parent must be rejected"
+        );
+    }
+
+    #[test]
+    fn test_build_metadata_rejects_query_injection_parent_id() {
+        assert!(
+            build_metadata("file.txt", Some("folder?evil=1")).is_err(),
+            "'?' in --parent must be rejected"
+        );
+    }
+
+    #[test]
+    fn test_download_command_injected() {
+        let helper = DriveHelper;
+        let cmd = Command::new("test");
+        let doc = crate::discovery::RestDescription::default();
+        let cmd = helper.inject_commands(cmd, &doc);
+        let names: Vec<_> = cmd.get_subcommands().map(|s| s.get_name()).collect();
+        assert!(names.contains(&"+upload"));
+        assert!(names.contains(&"+download"));
+    }
 }

From 6b7b809ea318f09c11b7b11d7f12490a6d8f3a1f Mon Sep 17 00:00:00 2001
From: Varun Nuthalapati <nuthalapativarun@gmail.com>
Date: Sun, 5 Apr 2026 21:45:11 -0700
Subject: [PATCH 2/9] fixup: sanitize API errors, add --dry-run, stream
 response to file

- 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
---
 .../google-workspace-cli/src/helpers/drive.rs | 86 +++++++++++++------
 1 file changed, 61 insertions(+), 25 deletions(-)

diff --git a/crates/google-workspace-cli/src/helpers/drive.rs b/crates/google-workspace-cli/src/helpers/drive.rs
index 1b058c3c..d5689675 100644
--- a/crates/google-workspace-cli/src/helpers/drive.rs
+++ b/crates/google-workspace-cli/src/helpers/drive.rs
@@ -178,10 +178,14 @@ TIPS:
 }
 
 async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
+    use futures_util::StreamExt;
+    use tokio::io::AsyncWriteExt;
+
     let file_id =
         crate::validate::validate_resource_name(matches.get_one::<String>("file").unwrap())?;
     let output_arg = matches.get_one::<String>("output");
     let export_mime = matches.get_one::<String>("mime-type");
+    let dry_run = matches.get_flag("dry-run");
 
     // Validate export mime-type for dangerous characters if provided
     if let Some(mime) = export_mime {
@@ -214,7 +218,7 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         let body = meta_resp.text().await.unwrap_or_default();
         return Err(GwsError::Api {
             code: status.into(),
-            message: body,
+            message: crate::output::sanitize_for_terminal(&body),
             reason: "files_get_metadata_failed".to_string(),
             enable_url: None,
         });
@@ -238,10 +242,27 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
     let out_str = output_arg.map(|s| s.as_str()).unwrap_or(drive_name);
     let out_path = crate::validate::validate_safe_file_path(out_str, "--output")?;
 
-    // 3. Fetch file content — native Google Workspace files require export,
-    //    everything else uses alt=media.
+    // 3. Dry-run: print what would be done and exit without network or disk I/O
+    if dry_run {
+        println!(
+            "{}",
+            serde_json::to_string_pretty(&json!({
+                "dryRun": true,
+                "fileId": file_id,
+                "driveName": drive_name,
+                "mimeType": mime_type,
+                "output": out_path.display().to_string(),
+                "exportMimeType": export_mime,
+            }))
+            .unwrap_or_default()
+        );
+        return Ok(());
+    }
+
+    // 4. Fetch file content and stream it to disk — native Google Workspace
+    //    files require export; everything else uses alt=media.
     let is_google_native = mime_type.starts_with("application/vnd.google-apps.");
-    let bytes = if is_google_native {
+    let resp = if is_google_native {
         let mime = export_mime.ok_or_else(|| {
             GwsError::Validation(format!(
                 "The file is a Google Workspace native file ({mime_type}). \
@@ -253,7 +274,7 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
             "https://www.googleapis.com/drive/v3/files/{}/export",
             crate::validate::encode_path_segment(file_id),
         );
-        let resp = crate::client::send_with_retry(|| {
+        let r = crate::client::send_with_retry(|| {
             client
                 .get(&export_url)
                 .query(&[("mimeType", mime.as_str())])
@@ -262,21 +283,19 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         .await
         .map_err(|e| GwsError::Other(anyhow::anyhow!("Drive export request failed: {e}")))?;
 
-        if !resp.status().is_success() {
-            let status = resp.status().as_u16();
-            let body = resp.text().await.unwrap_or_default();
+        if !r.status().is_success() {
+            let status = r.status().as_u16();
+            let body = r.text().await.unwrap_or_default();
             return Err(GwsError::Api {
                 code: status.into(),
-                message: body,
+                message: crate::output::sanitize_for_terminal(&body),
                 reason: "files_export_failed".to_string(),
                 enable_url: None,
             });
         }
-        resp.bytes()
-            .await
-            .map_err(|e| GwsError::Other(anyhow::anyhow!("Failed to read export response: {e}")))?
+        r
     } else {
-        let resp = crate::client::send_with_retry(|| {
+        let r = crate::client::send_with_retry(|| {
             client
                 .get(&metadata_url)
                 .query(&[("alt", "media")])
@@ -285,35 +304,52 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         .await
         .map_err(|e| GwsError::Other(anyhow::anyhow!("Drive download request failed: {e}")))?;
 
-        if !resp.status().is_success() {
-            let status = resp.status().as_u16();
-            let body = resp.text().await.unwrap_or_default();
+        if !r.status().is_success() {
+            let status = r.status().as_u16();
+            let body = r.text().await.unwrap_or_default();
             return Err(GwsError::Api {
                 code: status.into(),
-                message: body,
+                message: crate::output::sanitize_for_terminal(&body),
                 reason: "files_get_media_failed".to_string(),
                 enable_url: None,
             });
         }
-        resp.bytes()
-            .await
-            .map_err(|e| GwsError::Other(anyhow::anyhow!("Failed to read download response: {e}")))?
+        r
     };
 
-    // 4. Write to the output file
-    std::fs::write(&out_path, &bytes).map_err(|e| {
+    // 5. Stream response body to file to avoid OOM on large downloads
+    let mut file = tokio::fs::File::create(&out_path).await.map_err(|e| {
+        GwsError::Other(anyhow::anyhow!(
+            "Failed to create '{}': {e}",
+            out_path.display()
+        ))
+    })?;
+    let mut byte_count = 0usize;
+    let mut stream = resp.bytes_stream();
+    while let Some(chunk) = stream.next().await {
+        let chunk =
+            chunk.map_err(|e| GwsError::Other(anyhow::anyhow!("Download stream error: {e}")))?;
+        byte_count += chunk.len();
+        file.write_all(&chunk).await.map_err(|e| {
+            GwsError::Other(anyhow::anyhow!(
+                "Failed to write to '{}': {e}",
+                out_path.display()
+            ))
+        })?;
+    }
+    file.flush().await.map_err(|e| {
         GwsError::Other(anyhow::anyhow!(
-            "Failed to write '{}': {e}",
+            "Failed to flush '{}': {e}",
             out_path.display()
         ))
     })?;
 
-    // 5. Print result as JSON (consistent with other helper output)
+    // 6. Print result as JSON (consistent with other helper output)
     println!(
         "{}",
         serde_json::to_string_pretty(&json!({
             "file": out_path.display().to_string(),
-            "bytes": bytes.len(),
+            "bytes": byte_count,
             "mimeType": mime_type,
         }))
         .unwrap_or_default()

From 6a09110c5bb529ea7ea47393d2f2811bf6b3c43d Mon Sep 17 00:00:00 2001
From: Varun Nuthalapati <nuthalapativarun@gmail.com>
Date: Sun, 5 Apr 2026 22:15:34 -0700
Subject: [PATCH 3/9] fixup: address review feedback on +download helper

- 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)
---
 .../google-workspace-cli/src/helpers/drive.rs | 160 +++++++++++++-----
 1 file changed, 114 insertions(+), 46 deletions(-)

diff --git a/crates/google-workspace-cli/src/helpers/drive.rs b/crates/google-workspace-cli/src/helpers/drive.rs
index d5689675..8eaf5c5f 100644
--- a/crates/google-workspace-cli/src/helpers/drive.rs
+++ b/crates/google-workspace-cli/src/helpers/drive.rs
@@ -184,11 +184,11 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
     let file_id =
         crate::validate::validate_resource_name(matches.get_one::<String>("file").unwrap())?;
     let output_arg = matches.get_one::<String>("output");
-    let export_mime = matches.get_one::<String>("mime-type");
+    let export_mime: Option<String> = matches.get_one::<String>("mime-type").cloned();
     let dry_run = matches.get_flag("dry-run");
 
     // Validate export mime-type for dangerous characters if provided
-    if let Some(mime) = export_mime {
+    if let Some(mime) = &export_mime {
         crate::validate::reject_dangerous_chars(mime, "--mime-type")?;
     }
 
@@ -214,14 +214,9 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
     .map_err(|e| GwsError::Other(anyhow::anyhow!("Failed to fetch file metadata: {e}")))?;
 
     if !meta_resp.status().is_success() {
-        let status = meta_resp.status().as_u16();
+        let status = meta_resp.status();
         let body = meta_resp.text().await.unwrap_or_default();
-        return Err(GwsError::Api {
-            code: status.into(),
-            message: crate::output::sanitize_for_terminal(&body),
-            reason: "files_get_metadata_failed".to_string(),
-            enable_url: None,
-        });
+        return Err(parse_google_api_error(status, &body));
     }
 
     let meta: Value = meta_resp
@@ -238,10 +233,26 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         .and_then(|v| v.as_str())
         .unwrap_or("");
 
+    let is_google_native = mime_type.starts_with("application/vnd.google-apps.");
+
+    // Sanitize drive filename: Drive allows '/' in names which would create unintended
+    // subdirectories when used as a local filename. Replace with '_'.
+    let safe_name: String = drive_name
+        .chars()
+        .map(|c| if c == '/' { '_' } else { c })
+        .collect();
+
     // 2. Resolve and validate output path
-    let out_str = output_arg.map(|s| s.as_str()).unwrap_or(drive_name);
+    let out_str = output_arg.map(|s| s.as_str()).unwrap_or(&safe_name);
     let out_path = crate::validate::validate_safe_file_path(out_str, "--output")?;
 
+    // Actual MIME type of the file on disk (export format for native files, original otherwise)
+    let output_mime = if is_google_native {
+        export_mime.as_deref().unwrap_or(mime_type).to_string()
+    } else {
+        mime_type.to_string()
+    };
+
     // 3. Dry-run: print what would be done and exit without network or disk I/O
     if dry_run {
         println!(
@@ -250,7 +261,7 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
                 "dryRun": true,
                 "fileId": file_id,
                 "driveName": drive_name,
-                "mimeType": mime_type,
+                "mimeType": output_mime,
                 "output": out_path.display().to_string(),
                 "exportMimeType": export_mime,
             }))
@@ -259,11 +270,10 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         return Ok(());
     }
 
-    // 4. Fetch file content and stream it to disk — native Google Workspace
-    //    files require export; everything else uses alt=media.
-    let is_google_native = mime_type.starts_with("application/vnd.google-apps.");
+    // 4. Fetch file content — native Google Workspace files require export;
+    //    everything else uses alt=media.
     let resp = if is_google_native {
-        let mime = export_mime.ok_or_else(|| {
+        let mime = export_mime.as_deref().ok_or_else(|| {
             GwsError::Validation(format!(
                 "The file is a Google Workspace native file ({mime_type}). \
                  Provide --mime-type to choose an export format, e.g. \
@@ -277,21 +287,16 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         let r = crate::client::send_with_retry(|| {
             client
                 .get(&export_url)
-                .query(&[("mimeType", mime.as_str())])
+                .query(&[("mimeType", mime)])
                 .bearer_auth(&token)
         })
         .await
         .map_err(|e| GwsError::Other(anyhow::anyhow!("Drive export request failed: {e}")))?;
 
         if !r.status().is_success() {
-            let status = r.status().as_u16();
+            let status = r.status();
             let body = r.text().await.unwrap_or_default();
-            return Err(GwsError::Api {
-                code: status.into(),
-                message: crate::output::sanitize_for_terminal(&body),
-                reason: "files_export_failed".to_string(),
-                enable_url: None,
-            });
+            return Err(parse_google_api_error(status, &body));
         }
         r
     } else {
@@ -305,41 +310,58 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         .map_err(|e| GwsError::Other(anyhow::anyhow!("Drive download request failed: {e}")))?;
 
         if !r.status().is_success() {
-            let status = r.status().as_u16();
+            let status = r.status();
             let body = r.text().await.unwrap_or_default();
-            return Err(GwsError::Api {
-                code: status.into(),
-                message: crate::output::sanitize_for_terminal(&body),
-                reason: "files_get_media_failed".to_string(),
-                enable_url: None,
-            });
+            return Err(parse_google_api_error(status, &body));
         }
         r
     };
 
-    // 5. Stream response body to file to avoid OOM on large downloads
-    let mut file = tokio::fs::File::create(&out_path).await.map_err(|e| {
+    // 5. Stream to a temp file first; rename on success to avoid leaving partial files on disk.
+    let tmp_path = out_path.with_file_name(format!(
+        ".{}.tmp",
+        out_path.file_name().unwrap_or_default().to_string_lossy()
+    ));
+    let mut file = tokio::fs::File::create(&tmp_path).await.map_err(|e| {
         GwsError::Other(anyhow::anyhow!(
-            "Failed to create '{}': {e}",
-            out_path.display()
+            "Failed to create temp file '{}': {e}",
+            tmp_path.display()
         ))
     })?;
-    let mut byte_count = 0usize;
+    let mut byte_count = 0u64;
     let mut stream = resp.bytes_stream();
     while let Some(chunk) = stream.next().await {
-        let chunk =
-            chunk.map_err(|e| GwsError::Other(anyhow::anyhow!("Download stream error: {e}")))?;
-        byte_count += chunk.len();
-        file.write_all(&chunk).await.map_err(|e| {
-            GwsError::Other(anyhow::anyhow!(
+        let chunk = match chunk {
+            Ok(c) => c,
+            Err(e) => {
+                drop(file);
+                let _ = tokio::fs::remove_file(&tmp_path).await;
+                return Err(GwsError::Other(anyhow::anyhow!("Download stream error: {e}")));
+            }
+        };
+        byte_count += chunk.len() as u64;
+        if let Err(e) = file.write_all(&chunk).await {
+            drop(file);
+            let _ = tokio::fs::remove_file(&tmp_path).await;
+            return Err(GwsError::Other(anyhow::anyhow!(
                 "Failed to write to '{}': {e}",
-                out_path.display()
-            ))
-        })?;
+                tmp_path.display()
+            )));
+        }
     }
-    file.flush().await.map_err(|e| {
-        GwsError::Other(anyhow::anyhow!(
+    if let Err(e) = file.flush().await {
+        drop(file);
+        let _ = tokio::fs::remove_file(&tmp_path).await;
+        return Err(GwsError::Other(anyhow::anyhow!(
             "Failed to flush '{}': {e}",
+            tmp_path.display()
+        )));
+    }
+    drop(file);
+    tokio::fs::rename(&tmp_path, &out_path).await.map_err(|e| {
+        GwsError::Other(anyhow::anyhow!(
+            "Failed to finalize download ('{}' -> '{}'): {e}",
+            tmp_path.display(),
             out_path.display()
         ))
     })?;
@@ -350,7 +372,7 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         serde_json::to_string_pretty(&json!({
             "file": out_path.display().to_string(),
             "bytes": byte_count,
-            "mimeType": mime_type,
+            "mimeType": output_mime,
         }))
         .unwrap_or_default()
     );
@@ -358,6 +380,52 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
     Ok(())
 }
 
+/// Parse a Google API error response body into a [`GwsError::Api`], extracting
+/// the real `reason`, `message`, and `enable_url` from the JSON payload so that
+/// specialised error handling in `error.rs` (e.g. `accessNotConfigured` hints)
+/// continues to work correctly.
+fn parse_google_api_error(status: reqwest::StatusCode, body: &str) -> GwsError {
+    if let Ok(error_json) = serde_json::from_str::<Value>(body) {
+        if let Some(err_obj) = error_json.get("error") {
+            let code = err_obj
+                .get("code")
+                .and_then(|c| c.as_u64())
+                .unwrap_or(status.as_u16() as u64) as u16;
+            let message = err_obj
+                .get("message")
+                .and_then(|m| m.as_str())
+                .unwrap_or("Unknown error")
+                .to_string();
+            let reason = err_obj
+                .get("errors")
+                .and_then(|e| e.as_array())
+                .and_then(|arr| arr.first())
+                .and_then(|e| e.get("reason"))
+                .and_then(|r| r.as_str())
+                .or_else(|| err_obj.get("reason").and_then(|r| r.as_str()))
+                .unwrap_or("unknown")
+                .to_string();
+            let enable_url = if reason == "accessNotConfigured" {
+                executor::extract_enable_url(&message)
+            } else {
+                None
+            };
+            return GwsError::Api {
+                code,
+                message,
+                reason,
+                enable_url,
+            };
+        }
+    }
+    GwsError::Api {
+        code: status.as_u16(),
+        message: crate::output::sanitize_for_terminal(body),
+        reason: "unknown".to_string(),
+        enable_url: None,
+    }
+}
+
 fn determine_filename(file_path: &str, name_arg: Option<&str>) -> Result<String, GwsError> {
     if let Some(n) = name_arg {
         Ok(n.to_string())

From c24625983de04ec2e155eacc43f2553bbd9a5426 Mon Sep 17 00:00:00 2001
From: Varun Nuthalapati <nuthalapativarun@gmail.com>
Date: Sun, 5 Apr 2026 22:23:46 -0700
Subject: [PATCH 4/9] fixup: address review feedback on +download - backslash,
 early validation, 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
---
 .../google-workspace-cli/src/helpers/drive.rs | 36 +++++++++++--------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/crates/google-workspace-cli/src/helpers/drive.rs b/crates/google-workspace-cli/src/helpers/drive.rs
index 8eaf5c5f..d1cc365a 100644
--- a/crates/google-workspace-cli/src/helpers/drive.rs
+++ b/crates/google-workspace-cli/src/helpers/drive.rs
@@ -235,17 +235,29 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
 
     let is_google_native = mime_type.starts_with("application/vnd.google-apps.");
 
-    // Sanitize drive filename: Drive allows '/' in names which would create unintended
-    // subdirectories when used as a local filename. Replace with '_'.
+    // Sanitize drive filename: Drive allows '/' and '\' in names which would create unintended
+    // subdirectories on Unix and Windows respectively. Replace both with '_'.
+    // Note: TOCTOU race conditions on path components are a known limitation here;
+    // full mitigation via openat(O_NOFOLLOW) is out of scope for this change.
     let safe_name: String = drive_name
         .chars()
-        .map(|c| if c == '/' { '_' } else { c })
+        .map(|c| if c == '/' || c == '\\' { '_' } else { c })
         .collect();
 
     // 2. Resolve and validate output path
     let out_str = output_arg.map(|s| s.as_str()).unwrap_or(&safe_name);
     let out_path = crate::validate::validate_safe_file_path(out_str, "--output")?;
 
+    // For native Google Workspace files, --mime-type is required. Validate early so that
+    // dry-run output is accurate and the error surfaces before any network or disk I/O.
+    if is_google_native && export_mime.is_none() {
+        return Err(GwsError::Validation(format!(
+            "The file is a Google Workspace native file ({mime_type}). \
+             Provide --mime-type to choose an export format, e.g. \
+             --mime-type application/pdf or --mime-type text/csv"
+        )));
+    }
+
     // Actual MIME type of the file on disk (export format for native files, original otherwise)
     let output_mime = if is_google_native {
         export_mime.as_deref().unwrap_or(mime_type).to_string()
@@ -273,13 +285,8 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
     // 4. Fetch file content — native Google Workspace files require export;
     //    everything else uses alt=media.
     let resp = if is_google_native {
-        let mime = export_mime.as_deref().ok_or_else(|| {
-            GwsError::Validation(format!(
-                "The file is a Google Workspace native file ({mime_type}). \
-                 Provide --mime-type to choose an export format, e.g. \
-                 --mime-type application/pdf or --mime-type text/csv"
-            ))
-        })?;
+        // Safety: export_mime is validated as Some above for native files.
+        let mime = export_mime.as_deref().unwrap();
         let export_url = format!(
             "https://www.googleapis.com/drive/v3/files/{}/export",
             crate::validate::encode_path_segment(file_id),
@@ -358,13 +365,14 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         )));
     }
     drop(file);
-    tokio::fs::rename(&tmp_path, &out_path).await.map_err(|e| {
-        GwsError::Other(anyhow::anyhow!(
+    if let Err(e) = tokio::fs::rename(&tmp_path, &out_path).await {
+        let _ = tokio::fs::remove_file(&tmp_path).await;
+        return Err(GwsError::Other(anyhow::anyhow!(
             "Failed to finalize download ('{}' -> '{}'): {e}",
             tmp_path.display(),
             out_path.display()
-        ))
-    })?;
+        )));
+    }
 
     // 6. Print result as JSON (consistent with other helper output)
     println!(

From 08f98614ce9931b595366782018474161fd63ba9 Mon Sep 17 00:00:00 2001
From: Varun Nuthalapati <nuthalapativarun@gmail.com>
Date: Mon, 6 Apr 2026 07:56:43 -0700
Subject: [PATCH 5/9] fixup: address review feedback on +download - dry-run,
 cross-platform 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
---
 crates/google-workspace-cli/src/executor.rs   |  48 ++++++++
 .../google-workspace-cli/src/helpers/drive.rs | 115 ++++++------------
 2 files changed, 85 insertions(+), 78 deletions(-)

diff --git a/crates/google-workspace-cli/src/executor.rs b/crates/google-workspace-cli/src/executor.rs
index 46f31ac4..edd3c2cc 100644
--- a/crates/google-workspace-cli/src/executor.rs
+++ b/crates/google-workspace-cli/src/executor.rs
@@ -812,6 +812,54 @@ fn handle_error_response<T>(
     })
 }
 
+/// Parse a Google API error response body into a [`GwsError`].
+///
+/// Extracts the real `reason`, `message`, and `enable_url` from the JSON
+/// payload so that specialised error handling (e.g. `accessNotConfigured`
+/// hints) works correctly in callers that perform their own HTTP requests
+/// outside of [`execute_method`].
+pub fn api_error_from_response(status: reqwest::StatusCode, body: &str) -> GwsError {
+    if let Ok(error_json) = serde_json::from_str::<Value>(body) {
+        if let Some(err_obj) = error_json.get("error") {
+            let code = err_obj
+                .get("code")
+                .and_then(|c| c.as_u64())
+                .unwrap_or(status.as_u16() as u64) as u16;
+            let message = err_obj
+                .get("message")
+                .and_then(|m| m.as_str())
+                .unwrap_or("Unknown error")
+                .to_string();
+            let reason = err_obj
+                .get("errors")
+                .and_then(|e| e.as_array())
+                .and_then(|arr| arr.first())
+                .and_then(|e| e.get("reason"))
+                .and_then(|r| r.as_str())
+                .or_else(|| err_obj.get("reason").and_then(|r| r.as_str()))
+                .unwrap_or("unknown")
+                .to_string();
+            let enable_url = if reason == "accessNotConfigured" {
+                extract_enable_url(&message)
+            } else {
+                None
+            };
+            return GwsError::Api {
+                code,
+                message,
+                reason,
+                enable_url,
+            };
+        }
+    }
+    GwsError::Api {
+        code: status.as_u16(),
+        message: crate::output::sanitize_for_terminal(body),
+        reason: "unknown".to_string(),
+        enable_url: None,
+    }
+}
+
 /// Resolves the MIME type for the uploaded media content.
 ///
 /// Priority:
diff --git a/crates/google-workspace-cli/src/helpers/drive.rs b/crates/google-workspace-cli/src/helpers/drive.rs
index d1cc365a..6981ee1d 100644
--- a/crates/google-workspace-cli/src/helpers/drive.rs
+++ b/crates/google-workspace-cli/src/helpers/drive.rs
@@ -192,6 +192,23 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         crate::validate::reject_dangerous_chars(mime, "--mime-type")?;
     }
 
+    // 1. Dry-run: short-circuit before any auth or network I/O, consistent with
+    //    how +upload handles --dry-run (auth is attempted optionally, then skipped).
+    if dry_run {
+        let out_display = output_arg.map(|s| s.as_str()).unwrap_or("<Drive filename>");
+        println!(
+            "{}",
+            serde_json::to_string_pretty(&json!({
+                "dryRun": true,
+                "fileId": file_id,
+                "output": out_display,
+                "exportMimeType": export_mime,
+            }))
+            .unwrap_or_default()
+        );
+        return Ok(());
+    }
+
     let scope = "https://www.googleapis.com/auth/drive.readonly";
     let token = auth::get_token(&[scope])
         .await
@@ -199,7 +216,7 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
 
     let client = crate::client::build_client()?;
 
-    // 1. Fetch file metadata to get name and MIME type
+    // 2. Fetch file metadata to get name and MIME type
     let metadata_url = format!(
         "https://www.googleapis.com/drive/v3/files/{}",
         crate::validate::encode_path_segment(file_id),
@@ -216,7 +233,7 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
     if !meta_resp.status().is_success() {
         let status = meta_resp.status();
         let body = meta_resp.text().await.unwrap_or_default();
-        return Err(parse_google_api_error(status, &body));
+        return Err(executor::api_error_from_response(status, &body));
     }
 
     let meta: Value = meta_resp
@@ -235,6 +252,15 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
 
     let is_google_native = mime_type.starts_with("application/vnd.google-apps.");
 
+    // For native Google Workspace files, --mime-type is required.
+    if is_google_native && export_mime.is_none() {
+        return Err(GwsError::Validation(format!(
+            "The file is a Google Workspace native file ({mime_type}). \
+             Provide --mime-type to choose an export format, e.g. \
+             --mime-type application/pdf or --mime-type text/csv"
+        )));
+    }
+
     // Sanitize drive filename: Drive allows '/' and '\' in names which would create unintended
     // subdirectories on Unix and Windows respectively. Replace both with '_'.
     // Note: TOCTOU race conditions on path components are a known limitation here;
@@ -244,20 +270,10 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         .map(|c| if c == '/' || c == '\\' { '_' } else { c })
         .collect();
 
-    // 2. Resolve and validate output path
+    // 3. Resolve and validate output path
     let out_str = output_arg.map(|s| s.as_str()).unwrap_or(&safe_name);
     let out_path = crate::validate::validate_safe_file_path(out_str, "--output")?;
 
-    // For native Google Workspace files, --mime-type is required. Validate early so that
-    // dry-run output is accurate and the error surfaces before any network or disk I/O.
-    if is_google_native && export_mime.is_none() {
-        return Err(GwsError::Validation(format!(
-            "The file is a Google Workspace native file ({mime_type}). \
-             Provide --mime-type to choose an export format, e.g. \
-             --mime-type application/pdf or --mime-type text/csv"
-        )));
-    }
-
     // Actual MIME type of the file on disk (export format for native files, original otherwise)
     let output_mime = if is_google_native {
         export_mime.as_deref().unwrap_or(mime_type).to_string()
@@ -265,23 +281,6 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         mime_type.to_string()
     };
 
-    // 3. Dry-run: print what would be done and exit without network or disk I/O
-    if dry_run {
-        println!(
-            "{}",
-            serde_json::to_string_pretty(&json!({
-                "dryRun": true,
-                "fileId": file_id,
-                "driveName": drive_name,
-                "mimeType": output_mime,
-                "output": out_path.display().to_string(),
-                "exportMimeType": export_mime,
-            }))
-            .unwrap_or_default()
-        );
-        return Ok(());
-    }
-
     // 4. Fetch file content — native Google Workspace files require export;
     //    everything else uses alt=media.
     let resp = if is_google_native {
@@ -303,7 +302,7 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         if !r.status().is_success() {
             let status = r.status();
             let body = r.text().await.unwrap_or_default();
-            return Err(parse_google_api_error(status, &body));
+            return Err(executor::api_error_from_response(status, &body));
         }
         r
     } else {
@@ -319,7 +318,7 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         if !r.status().is_success() {
             let status = r.status();
             let body = r.text().await.unwrap_or_default();
-            return Err(parse_google_api_error(status, &body));
+            return Err(executor::api_error_from_response(status, &body));
         }
         r
     };
@@ -365,6 +364,12 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         )));
     }
     drop(file);
+    // Remove existing destination first for cross-platform consistency:
+    // tokio::fs::rename overwrites atomically on Unix but fails if the
+    // destination already exists on Windows. The remove + rename sequence
+    // introduces a small TOCTOU window; full mitigation via openat(O_NOFOLLOW)
+    // is considered out of scope for this change.
+    let _ = tokio::fs::remove_file(&out_path).await;
     if let Err(e) = tokio::fs::rename(&tmp_path, &out_path).await {
         let _ = tokio::fs::remove_file(&tmp_path).await;
         return Err(GwsError::Other(anyhow::anyhow!(
@@ -388,52 +393,6 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
     Ok(())
 }
 
-/// Parse a Google API error response body into a [`GwsError::Api`], extracting
-/// the real `reason`, `message`, and `enable_url` from the JSON payload so that
-/// specialised error handling in `error.rs` (e.g. `accessNotConfigured` hints)
-/// continues to work correctly.
-fn parse_google_api_error(status: reqwest::StatusCode, body: &str) -> GwsError {
-    if let Ok(error_json) = serde_json::from_str::<Value>(body) {
-        if let Some(err_obj) = error_json.get("error") {
-            let code = err_obj
-                .get("code")
-                .and_then(|c| c.as_u64())
-                .unwrap_or(status.as_u16() as u64) as u16;
-            let message = err_obj
-                .get("message")
-                .and_then(|m| m.as_str())
-                .unwrap_or("Unknown error")
-                .to_string();
-            let reason = err_obj
-                .get("errors")
-                .and_then(|e| e.as_array())
-                .and_then(|arr| arr.first())
-                .and_then(|e| e.get("reason"))
-                .and_then(|r| r.as_str())
-                .or_else(|| err_obj.get("reason").and_then(|r| r.as_str()))
-                .unwrap_or("unknown")
-                .to_string();
-            let enable_url = if reason == "accessNotConfigured" {
-                executor::extract_enable_url(&message)
-            } else {
-                None
-            };
-            return GwsError::Api {
-                code,
-                message,
-                reason,
-                enable_url,
-            };
-        }
-    }
-    GwsError::Api {
-        code: status.as_u16(),
-        message: crate::output::sanitize_for_terminal(body),
-        reason: "unknown".to_string(),
-        enable_url: None,
-    }
-}
-
 fn determine_filename(file_path: &str, name_arg: Option<&str>) -> Result<String, GwsError> {
     if let Some(n) = name_arg {
         Ok(n.to_string())

From 4f35687dca0162e9b3867c4aadd570cb8fd9a289 Mon Sep 17 00:00:00 2001
From: Varun Nuthalapati <nuthalapativarun@gmail.com>
Date: Mon, 6 Apr 2026 08:36:49 -0700
Subject: [PATCH 6/9] fixup: address review feedback - consistency fixes in
 api_error_from_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
---
 crates/google-workspace-cli/src/executor.rs   |  2 +-
 .../google-workspace-cli/src/helpers/drive.rs | 31 +++++++++++++------
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/crates/google-workspace-cli/src/executor.rs b/crates/google-workspace-cli/src/executor.rs
index edd3c2cc..b0ca03fa 100644
--- a/crates/google-workspace-cli/src/executor.rs
+++ b/crates/google-workspace-cli/src/executor.rs
@@ -855,7 +855,7 @@ pub fn api_error_from_response(status: reqwest::StatusCode, body: &str) -> GwsEr
     GwsError::Api {
         code: status.as_u16(),
         message: crate::output::sanitize_for_terminal(body),
-        reason: "unknown".to_string(),
+        reason: "httpError".to_string(),
         enable_url: None,
     }
 }
diff --git a/crates/google-workspace-cli/src/helpers/drive.rs b/crates/google-workspace-cli/src/helpers/drive.rs
index 6981ee1d..cf72cb10 100644
--- a/crates/google-workspace-cli/src/helpers/drive.rs
+++ b/crates/google-workspace-cli/src/helpers/drive.rs
@@ -199,10 +199,10 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         println!(
             "{}",
             serde_json::to_string_pretty(&json!({
-                "dryRun": true,
-                "fileId": file_id,
+                "dry_run": true,
+                "file_id": file_id,
                 "output": out_display,
-                "exportMimeType": export_mime,
+                "export_mime_type": export_mime,
             }))
             .unwrap_or_default()
         );
@@ -261,13 +261,26 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         )));
     }
 
-    // Sanitize drive filename: Drive allows '/' and '\' in names which would create unintended
-    // subdirectories on Unix and Windows respectively. Replace both with '_'.
-    // Note: TOCTOU race conditions on path components are a known limitation here;
+    // Sanitize drive filename for use as a local path component:
+    // - Replace Unix/Windows path separators ('/', '\') to prevent subdirectory traversal.
+    // - Replace Windows-reserved characters (':', '*', '?', '"', '<', '>', '|') that
+    //   cause file creation failures on Windows.
+    // - Strip control characters (Cc category) and dangerous Unicode (Cf/bidi/zero-width)
+    //   that could cause terminal injection or misleading output.
+    // Note: TOCTOU race conditions on path components are a known limitation;
     // full mitigation via openat(O_NOFOLLOW) is out of scope for this change.
     let safe_name: String = drive_name
         .chars()
-        .map(|c| if c == '/' || c == '\\' { '_' } else { c })
+        .map(|c| {
+            if matches!(c, '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|')
+                || c.is_control()
+                || crate::validate::is_dangerous_unicode(c)
+            {
+                '_'
+            } else {
+                c
+            }
+        })
         .collect();
 
     // 3. Resolve and validate output path
@@ -383,9 +396,9 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
     println!(
         "{}",
         serde_json::to_string_pretty(&json!({
-            "file": out_path.display().to_string(),
+            "saved_file": out_path.display().to_string(),
             "bytes": byte_count,
-            "mimeType": output_mime,
+            "mime_type": output_mime,
         }))
         .unwrap_or_default()
     );

From 499e065edd6d9d86a8c0706e016ecf03cf2e65bc Mon Sep 17 00:00:00 2001
From: Varun Nuthalapati <nuthalapativarun@gmail.com>
Date: Mon, 6 Apr 2026 09:07:00 -0700
Subject: [PATCH 7/9] fixup: drop remove_file before rename - tokio::fs::rename
 overwrites 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.
---
 crates/google-workspace-cli/src/helpers/drive.rs | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/crates/google-workspace-cli/src/helpers/drive.rs b/crates/google-workspace-cli/src/helpers/drive.rs
index cf72cb10..136cc723 100644
--- a/crates/google-workspace-cli/src/helpers/drive.rs
+++ b/crates/google-workspace-cli/src/helpers/drive.rs
@@ -377,12 +377,10 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         )));
     }
     drop(file);
-    // Remove existing destination first for cross-platform consistency:
-    // tokio::fs::rename overwrites atomically on Unix but fails if the
-    // destination already exists on Windows. The remove + rename sequence
-    // introduces a small TOCTOU window; full mitigation via openat(O_NOFOLLOW)
-    // is considered out of scope for this change.
-    let _ = tokio::fs::remove_file(&out_path).await;
+    // tokio::fs::rename overwrites the destination atomically on Unix, and on
+    // Windows it uses MOVEFILE_REPLACE_EXISTING (supported since Rust 1.26), so
+    // a separate remove_file step is unnecessary and would only widen the window
+    // where neither the old nor new file exists at the target path.
     if let Err(e) = tokio::fs::rename(&tmp_path, &out_path).await {
         let _ = tokio::fs::remove_file(&tmp_path).await;
         return Err(GwsError::Other(anyhow::anyhow!(

From fa1e32fa7f83d0e0876df103112c0c86286be14b Mon Sep 17 00:00:00 2001
From: Varun Nuthalapati <nuthalapativarun@gmail.com>
Date: Mon, 6 Apr 2026 09:24:36 -0700
Subject: [PATCH 8/9] fix(drive,executor): sanitize API error fields and
 randomize tmp filename

- 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.
---
 crates/google-workspace-cli/src/executor.rs      | 8 ++++----
 crates/google-workspace-cli/src/helpers/drive.rs | 6 ++++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/crates/google-workspace-cli/src/executor.rs b/crates/google-workspace-cli/src/executor.rs
index b0ca03fa..c711e438 100644
--- a/crates/google-workspace-cli/src/executor.rs
+++ b/crates/google-workspace-cli/src/executor.rs
@@ -828,8 +828,8 @@ pub fn api_error_from_response(status: reqwest::StatusCode, body: &str) -> GwsEr
             let message = err_obj
                 .get("message")
                 .and_then(|m| m.as_str())
-                .unwrap_or("Unknown error")
-                .to_string();
+                .map(sanitize_for_terminal)
+                .unwrap_or_else(|| "Unknown error".to_string());
             let reason = err_obj
                 .get("errors")
                 .and_then(|e| e.as_array())
@@ -837,8 +837,8 @@ pub fn api_error_from_response(status: reqwest::StatusCode, body: &str) -> GwsEr
                 .and_then(|e| e.get("reason"))
                 .and_then(|r| r.as_str())
                 .or_else(|| err_obj.get("reason").and_then(|r| r.as_str()))
-                .unwrap_or("unknown")
-                .to_string();
+                .map(sanitize_for_terminal)
+                .unwrap_or_else(|| "unknown".to_string());
             let enable_url = if reason == "accessNotConfigured" {
                 extract_enable_url(&message)
             } else {
diff --git a/crates/google-workspace-cli/src/helpers/drive.rs b/crates/google-workspace-cli/src/helpers/drive.rs
index 136cc723..f3f565e1 100644
--- a/crates/google-workspace-cli/src/helpers/drive.rs
+++ b/crates/google-workspace-cli/src/helpers/drive.rs
@@ -337,9 +337,11 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
     };
 
     // 5. Stream to a temp file first; rename on success to avoid leaving partial files on disk.
+    // Include a random suffix to prevent symlink attacks in world-writable directories.
     let tmp_path = out_path.with_file_name(format!(
-        ".{}.tmp",
-        out_path.file_name().unwrap_or_default().to_string_lossy()
+        ".{}.{:016x}.tmp",
+        out_path.file_name().unwrap_or_default().to_string_lossy(),
+        rand::random::<u64>()
     ));
     let mut file = tokio::fs::File::create(&tmp_path).await.map_err(|e| {
         GwsError::Other(anyhow::anyhow!(

From 06f53d317f706d6c3c323fc7d0345e7197d613a3 Mon Sep 17 00:00:00 2001
From: Varun Nuthalapati <nuthalapativarun@gmail.com>
Date: Mon, 6 Apr 2026 16:36:27 -0700
Subject: [PATCH 9/9] fix(drive,executor): use Discovery doc for URLs/scopes
 and add auth hints

- 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.
---
 crates/google-workspace-cli/src/executor.rs   | 19 +++-
 .../google-workspace-cli/src/helpers/drive.rs | 91 ++++++++++++++-----
 2 files changed, 88 insertions(+), 22 deletions(-)

diff --git a/crates/google-workspace-cli/src/executor.rs b/crates/google-workspace-cli/src/executor.rs
index c711e438..30ebc083 100644
--- a/crates/google-workspace-cli/src/executor.rs
+++ b/crates/google-workspace-cli/src/executor.rs
@@ -818,7 +818,24 @@ fn handle_error_response<T>(
 /// payload so that specialised error handling (e.g. `accessNotConfigured`
 /// hints) works correctly in callers that perform their own HTTP requests
 /// outside of [`execute_method`].
-pub fn api_error_from_response(status: reqwest::StatusCode, body: &str) -> GwsError {
+///
+/// When `auth_method` is [`AuthMethod::None`] and the status is 401 or 403,
+/// a helpful login hint is returned — mirroring the behaviour of the internal
+/// `handle_error_response` helper.
+pub fn api_error_from_response(
+    status: reqwest::StatusCode,
+    body: &str,
+    auth_method: &AuthMethod,
+) -> GwsError {
+    // Mirror handle_error_response: give a helpful login hint when no auth was provided.
+    if (status.as_u16() == 401 || status.as_u16() == 403) && *auth_method == AuthMethod::None {
+        return GwsError::Auth(
+            "Access denied. No credentials provided. Run `gws auth login` or set \
+             GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE to an OAuth credentials JSON file."
+                .to_string(),
+        );
+    }
+
     if let Ok(error_json) = serde_json::from_str::<Value>(body) {
         if let Some(err_obj) = error_json.get("error") {
             let code = err_obj
diff --git a/crates/google-workspace-cli/src/helpers/drive.rs b/crates/google-workspace-cli/src/helpers/drive.rs
index f3f565e1..ebb239f0 100644
--- a/crates/google-workspace-cli/src/helpers/drive.rs
+++ b/crates/google-workspace-cli/src/helpers/drive.rs
@@ -168,7 +168,7 @@ TIPS:
             }
 
             if let Some(matches) = matches.subcommand_matches("+download") {
-                handle_download(matches).await?;
+                handle_download(doc, matches).await?;
                 return Ok(true);
             }
 
@@ -177,7 +177,10 @@ TIPS:
     }
 }
 
-async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
+async fn handle_download(
+    doc: &crate::discovery::RestDescription,
+    matches: &ArgMatches,
+) -> Result<(), GwsError> {
     use futures_util::StreamExt;
     use tokio::io::AsyncWriteExt;
 
@@ -209,23 +212,55 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         return Ok(());
     }
 
-    let scope = "https://www.googleapis.com/auth/drive.readonly";
-    let token = auth::get_token(&[scope])
+    // Resolve methods and scopes from the Discovery Document so that the correct
+    // OAuth scopes are used and any custom root_url (proxy, VPC-SC, etc.) is respected.
+    let files_res = doc
+        .resources
+        .get("files")
+        .ok_or_else(|| GwsError::Discovery("Resource 'files' not found".to_string()))?;
+    let get_method = files_res
+        .methods
+        .get("get")
+        .ok_or_else(|| GwsError::Discovery("Method 'files.get' not found".to_string()))?;
+    let export_method = files_res
+        .methods
+        .get("export")
+        .ok_or_else(|| GwsError::Discovery("Method 'files.export' not found".to_string()))?;
+
+    // Union scopes from files.get and files.export to cover both download paths.
+    let mut scope_set: std::collections::HashSet<&str> = std::collections::HashSet::new();
+    for s in &get_method.scopes {
+        scope_set.insert(s.as_str());
+    }
+    for s in &export_method.scopes {
+        scope_set.insert(s.as_str());
+    }
+    let scopes: Vec<&str> = scope_set.into_iter().collect();
+
+    let token = auth::get_token(&scopes)
         .await
         .map_err(|e| GwsError::Auth(format!("Drive auth failed: {e}")))?;
 
+    // Resolve base URL from the Discovery Document (respects custom root_url / proxy configs).
+    let base_url = doc
+        .base_url
+        .clone()
+        .unwrap_or_else(|| format!("{}{}", doc.root_url, doc.service_path));
+
     let client = crate::client::build_client()?;
 
     // 2. Fetch file metadata to get name and MIME type
-    let metadata_url = format!(
-        "https://www.googleapis.com/drive/v3/files/{}",
-        crate::validate::encode_path_segment(file_id),
-    );
+    let encoded_id = crate::validate::encode_path_segment(file_id);
+    let metadata_url = format!("{}files/{}", base_url, encoded_id);
     let meta_resp = crate::client::send_with_retry(|| {
-        client
+        let mut req = client
             .get(&metadata_url)
             .query(&[("fields", "name,mimeType")])
-            .bearer_auth(&token)
+            .bearer_auth(&token);
+        if let Some(qp) = crate::auth::get_quota_project() {
+            req = req.header("x-goog-user-project", qp);
+        }
+        req
     })
     .await
     .map_err(|e| GwsError::Other(anyhow::anyhow!("Failed to fetch file metadata: {e}")))?;
@@ -233,7 +268,7 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
     if !meta_resp.status().is_success() {
         let status = meta_resp.status();
         let body = meta_resp.text().await.unwrap_or_default();
-        return Err(executor::api_error_from_response(status, &body));
+        return Err(executor::api_error_from_response(status, &body, &executor::AuthMethod::OAuth));
     }
 
     let meta: Value = meta_resp
@@ -299,15 +334,17 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
     let resp = if is_google_native {
         // Safety: export_mime is validated as Some above for native files.
         let mime = export_mime.as_deref().unwrap();
-        let export_url = format!(
-            "https://www.googleapis.com/drive/v3/files/{}/export",
-            crate::validate::encode_path_segment(file_id),
-        );
+        // Build export URL from Discovery Document base URL (respects custom root_url).
+        let export_url = format!("{}files/{}/export", base_url, encoded_id);
         let r = crate::client::send_with_retry(|| {
-            client
+            let mut req = client
                 .get(&export_url)
                 .query(&[("mimeType", mime)])
-                .bearer_auth(&token)
+                .bearer_auth(&token);
+            if let Some(qp) = crate::auth::get_quota_project() {
+                req = req.header("x-goog-user-project", qp);
+            }
+            req
         })
         .await
         .map_err(|e| GwsError::Other(anyhow::anyhow!("Drive export request failed: {e}")))?;
@@ -315,15 +352,23 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         if !r.status().is_success() {
             let status = r.status();
             let body = r.text().await.unwrap_or_default();
-            return Err(executor::api_error_from_response(status, &body));
+            return Err(executor::api_error_from_response(
+                status,
+                &body,
+                &executor::AuthMethod::OAuth,
+            ));
         }
         r
     } else {
         let r = crate::client::send_with_retry(|| {
-            client
+            let mut req = client
                 .get(&metadata_url)
                 .query(&[("alt", "media")])
-                .bearer_auth(&token)
+                .bearer_auth(&token);
+            if let Some(qp) = crate::auth::get_quota_project() {
+                req = req.header("x-goog-user-project", qp);
+            }
+            req
         })
         .await
         .map_err(|e| GwsError::Other(anyhow::anyhow!("Drive download request failed: {e}")))?;
@@ -331,7 +376,11 @@ async fn handle_download(matches: &ArgMatches) -> Result<(), GwsError> {
         if !r.status().is_success() {
             let status = r.status();
             let body = r.text().await.unwrap_or_default();
-            return Err(executor::api_error_from_response(status, &body));
+            return Err(executor::api_error_from_response(
+                status,
+                &body,
+                &executor::AuthMethod::OAuth,
+            ));
         }
         r
     };