fix(security): cap Retry-After sleep and sanitize mimeType in uploads

.changeset/fix-retry-after-and-mime-injection.md

@@ -0,0 +1,5 @@
+---
+"@googleworkspace/cli": patch
+---
+
+fix(security): cap Retry-After sleep to 60s and sanitize mimeType in multipart uploads

src/client.rs

@@ -20,6 +20,9 @@ pub fn build_client() -> Result<reqwest::Client, crate::error::GwsError> {
 }
 
 const MAX_RETRIES: u32 = 3;
+/// Maximum seconds to sleep on a 429 Retry-After header. Prevents a hostile
+/// or misconfigured server from hanging the process indefinitely.
+const MAX_RETRY_DELAY_SECS: u64 = 60;
 
 /// Send an HTTP request with automatic retry on 429 (rate limit) responses.
 /// Respects the `Retry-After` header; falls back to exponential backoff (1s, 2s, 4s).
@@ -33,13 +36,11 @@ pub async fn send_with_retry(
             return Ok(resp);
         }
 
-        // Parse Retry-After header (seconds), fall back to exponential backoff
-        let retry_after = resp
+        let header_value = resp
             .headers()
             .get("retry-after")
-            .and_then(|v| v.to_str().ok())
-            .and_then(|s| s.parse::<u64>().ok())
-            .unwrap_or(1 << attempt); // 1, 2, 4 seconds
+            .and_then(|v| v.to_str().ok());
+        let retry_after = compute_retry_delay(header_value, attempt);
 
         tokio::time::sleep(std::time::Duration::from_secs(retry_after)).await;
     }
@@ -48,6 +49,16 @@ pub async fn send_with_retry(
     build_request().send().await
 }
 
+/// Compute the retry delay from a Retry-After header value and attempt number.
+/// Falls back to exponential backoff (1, 2, 4s) when the header is absent or
+/// unparseable. Always caps the result at MAX_RETRY_DELAY_SECS.
+fn compute_retry_delay(header_value: Option<&str>, attempt: u32) -> u64 {
+    header_value
+        .and_then(|s| s.parse::<u64>().ok())
+        .unwrap_or(2u64.saturating_pow(attempt))
+        .min(MAX_RETRY_DELAY_SECS)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -56,4 +67,33 @@ mod tests {
     fn build_client_succeeds() {
         assert!(build_client().is_ok());
     }
+
+    #[test]
+    fn retry_delay_caps_large_header_value() {
+        assert_eq!(compute_retry_delay(Some("999999"), 0), MAX_RETRY_DELAY_SECS);
+    }
+
+    #[test]
+    fn retry_delay_passes_through_small_header_value() {
+        assert_eq!(compute_retry_delay(Some("5"), 0), 5);
+    }
+
+    #[test]
+    fn retry_delay_falls_back_to_exponential_on_missing_header() {
+        assert_eq!(compute_retry_delay(None, 0), 1); // 2^0
+        assert_eq!(compute_retry_delay(None, 1), 2); // 2^1
+        assert_eq!(compute_retry_delay(None, 2), 4); // 2^2
+    }
+
+    #[test]
+    fn retry_delay_falls_back_on_unparseable_header() {
+        assert_eq!(compute_retry_delay(Some("not-a-number"), 1), 2);
+        assert_eq!(compute_retry_delay(Some(""), 0), 1);
+    }
+
+    #[test]
+    fn retry_delay_caps_at_boundary() {
+        assert_eq!(compute_retry_delay(Some("60"), 0), 60);
+        assert_eq!(compute_retry_delay(Some("61"), 0), MAX_RETRY_DELAY_SECS);
+    }
 }

src/executor.rs

@@ -763,12 +763,17 @@ fn build_multipart_body(
 ) -> Result<(Vec<u8>, String), GwsError> {
     let boundary = format!("gws_boundary_{:016x}", rand::random::<u64>());
 
-    // Determine the media MIME type from the metadata's mimeType field, or fall back
-    let media_mime = metadata
+    // Determine the media MIME type from the metadata's mimeType field, or fall back.
+    // Strip CR/LF to prevent MIME header injection via user-controlled mimeType.
+    let media_mime_raw = metadata
         .as_ref()
         .and_then(|m| m.get("mimeType"))
         .and_then(|v| v.as_str())
         .unwrap_or("application/octet-stream");
+    let media_mime: String = media_mime_raw
+        .chars()
+        .filter(|c| !c.is_control())
+        .collect();
 
     // Build multipart/related body
     let metadata_json = metadata
@@ -1246,6 +1251,31 @@ mod tests {
         assert!(body_str.contains("Binary data"));
     }
 
+    #[tokio::test]
+    async fn test_build_multipart_body_sanitizes_mime_injection() {
+        // A malicious mimeType with CRLF should be stripped to prevent
+        // MIME header injection in the multipart body.
+        let metadata = Some(json!({
+            "name": "evil.txt",
+            "mimeType": "text/plain\r\nX-Injected: malicious"
+        }));
+        let content = b"payload";
+
+        let (body, _) = build_multipart_body(&metadata, content).unwrap();
+        let body_str = String::from_utf8(body).unwrap();
+
+        // After stripping CR/LF, the Content-Type line should NOT have a
+        // line break that would create a separate injected header.
+        // The CRLF between "text/plain" and "X-Injected" is removed,
+        // so the value is concatenated into a single line.
+        assert!(
+            !body_str.contains("Content-Type: text/plain\r\nX-Injected"),
+            "CRLF injection must not produce a separate header line"
+        );
+        // Verify the sanitized mime type appears as one continuous string
+        assert!(body_str.contains("Content-Type: text/plainX-Injected: malicious"));
+    }
+
     #[test]
     fn test_build_url_basic() {
         let doc = RestDescription {