Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-retry-after-and-mime-injection.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@googleworkspace/cli": patch
---

fix(security): cap Retry-After sleep to 60s and sanitize mimeType in multipart uploads
50 changes: 45 additions & 5 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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;
}
Expand All @@ -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::*;
Expand All @@ -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);
}
}
34 changes: 32 additions & 2 deletions src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -737,12 +737,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
Expand Down Expand Up @@ -1218,6 +1223,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 {
Expand Down
Loading