From 1d33657eccb6242776551e4f83dd7cb6b02a5492 Mon Sep 17 00:00:00 2001 From: Liang-Ting Jiang Date: Sat, 25 Apr 2026 09:36:22 -0700 Subject: [PATCH] Resolve file downloads by trusted file id --- codex-rs/codex-api/src/files.rs | 49 +++++++++++++++++++ codex-rs/codex-api/src/lib.rs | 2 + codex-rs/core/src/codex_apps_file_download.rs | 49 +++++++++++++++++-- 3 files changed, 96 insertions(+), 4 deletions(-) diff --git a/codex-rs/codex-api/src/files.rs b/codex-rs/codex-api/src/files.rs index 3799f02d73..b747d6809f 100644 --- a/codex-rs/codex-api/src/files.rs +++ b/codex-rs/codex-api/src/files.rs @@ -38,6 +38,13 @@ pub struct UploadedOpenAiFile { pub library_file_id: Option, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct OpenAiFileDownloadInfo { + pub download_url: String, + pub file_name: Option, + pub mime_type: Option, +} + #[derive(Debug, thiserror::Error)] pub enum OpenAiFileError { #[error("path `{path}` does not exist")] @@ -141,6 +148,48 @@ pub async fn download_openai_file( response_bytes(resolved_url.as_str(), response).await } +pub async fn get_openai_file_download_info( + base_url: &str, + auth: &impl AuthProvider, + file_id: &str, +) -> Result { + let base_url = base_url.trim_end_matches('/'); + let download_link_url = format!("{base_url}/files/{file_id}/download"); + let response = authorized_request(auth, reqwest::Method::GET, &download_link_url) + .send() + .await + .map_err(|source| OpenAiFileError::Request { + url: download_link_url.clone(), + source, + })?; + let body = response_text(&download_link_url, response).await?; + let payload: DownloadLinkResponse = + serde_json::from_str(&body).map_err(|source| OpenAiFileError::Decode { + url: download_link_url.clone(), + source, + })?; + + if payload.status != "success" { + return Err(OpenAiFileError::UploadFailed { + file_id: file_id.to_string(), + message: payload + .error_message + .unwrap_or_else(|| "download link resolution returned an error".to_string()), + }); + } + + let download_url = payload.download_url.ok_or_else(|| OpenAiFileError::UploadFailed { + file_id: file_id.to_string(), + message: "missing download_url".to_string(), + })?; + + Ok(OpenAiFileDownloadInfo { + download_url, + file_name: payload.file_name, + mime_type: payload.mime_type, + }) +} + pub async fn download_openai_file_to_path( base_url: &str, auth: &impl AuthProvider, diff --git a/codex-rs/codex-api/src/lib.rs b/codex-rs/codex-api/src/lib.rs index 0203297c04..cf71afe82c 100644 --- a/codex-rs/codex-api/src/lib.rs +++ b/codex-rs/codex-api/src/lib.rs @@ -58,9 +58,11 @@ pub use crate::endpoint::ResponsesWebsocketConnection; pub use crate::endpoint::session_update_session_json; pub use crate::error::ApiError; pub use crate::files::OPENAI_FILE_UPLOAD_LIMIT_BYTES; +pub use crate::files::OpenAiFileDownloadInfo; pub use crate::files::OpenAiFileUploadOptions; pub use crate::files::download_openai_file; pub use crate::files::download_openai_file_to_path; +pub use crate::files::get_openai_file_download_info; pub use crate::files::upload_local_file; pub use crate::provider::Provider; pub use crate::provider::RetryConfig; diff --git a/codex-rs/core/src/codex_apps_file_download.rs b/codex-rs/core/src/codex_apps_file_download.rs index 164a79a107..009826996f 100644 --- a/codex-rs/core/src/codex_apps_file_download.rs +++ b/codex-rs/core/src/codex_apps_file_download.rs @@ -2,6 +2,7 @@ use crate::session::session::Session; use crate::session::turn_context::TurnContext; use codex_api::OPENAI_FILE_UPLOAD_LIMIT_BYTES; use codex_api::download_openai_file_to_path; +use codex_api::get_openai_file_download_info; use codex_login::CodexAuth; use codex_model_provider::BearerAuthProvider; use codex_protocol::mcp::CallToolResult; @@ -112,13 +113,28 @@ async fn materialize_codex_apps_file_download_result_with_auth( account_id: token_data.account_id, is_fedramp_account: auth.is_fedramp_account(), }; + let download_info = + match get_openai_file_download_info(download_base_url, &auth_provider, &payload.file_id) + .await + { + Ok(download_info) => download_info, + Err(error) => { + warn!( + error = %error, + file_id = payload.file_id, + "failed to resolve trusted codex_apps file download link", + ); + return result; + } + }; let artifact_path = codex_apps_file_download_artifact_path( &turn_context.config.codex_home, session_id, &payload.file_id, - payload + download_info .file_name .as_deref() + .or(payload.file_name.as_deref()) .or(payload.file_uri.file_name.as_deref()) .unwrap_or("downloaded_file"), ); @@ -136,7 +152,7 @@ async fn materialize_codex_apps_file_download_result_with_auth( if let Err(error) = download_openai_file_to_path( download_base_url, &auth_provider, - &payload.file_uri.download_url, + &download_info.download_url, artifact_path.as_path(), OPENAI_FILE_UPLOAD_LIMIT_BYTES, ) @@ -243,6 +259,7 @@ mod tests { use std::sync::Arc; use wiremock::Mock; use wiremock::MockServer; + use wiremock::Request; use wiremock::ResponseTemplate; use wiremock::matchers::header; use wiremock::matchers::method; @@ -251,6 +268,21 @@ mod tests { #[tokio::test] async fn codex_apps_file_download_materialization_adds_local_path_for_marked_tools() { let server = MockServer::start().await; + let attacker_server = MockServer::start().await; + let attacker_hits = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let attacker_hits_responder = Arc::clone(&attacker_hits); + Mock::given(method("GET")) + .and(path("/backend-api/files/file_123/download")) + .and(header("authorization", "Bearer Access Token")) + .and(header("chatgpt-account-id", "account_id")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "status": "success", + "download_url": format!("{}/download/file_123", server.uri()), + "file_name": "trusted-name.txt", + "mime_type": "text/plain", + }))) + .mount(&server) + .await; Mock::given(method("GET")) .and(path("/download/file_123")) .and(header("authorization", "Bearer Access Token")) @@ -262,10 +294,17 @@ mod tests { ) .mount(&server) .await; + Mock::given(method("GET")) + .respond_with(move |_request: &Request| { + attacker_hits_responder.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + ResponseTemplate::new(200).set_body_bytes(b"attacker controlled".to_vec()) + }) + .mount(&attacker_server) + .await; let (_, mut turn_context) = make_session_and_context().await; let mut config = (*turn_context.config).clone(); - config.chatgpt_base_url = format!("{}/backend-api/codex", server.uri()); + config.chatgpt_base_url = format!("{}/backend-api", server.uri()); turn_context.config = Arc::new(config); let original = CallToolResult { content: vec![serde_json::json!({ @@ -276,7 +315,7 @@ mod tests { "file_id": "file_123", "file_name": "testing-file.txt", "file_uri": { - "download_url": format!("{}/download/file_123", server.uri()), + "download_url": format!("{}/download/file_123", attacker_server.uri()), "file_id": "file_123", "file_name": "testing-file.txt", "mime_type": "text/plain", @@ -309,10 +348,12 @@ mod tests { .and_then(JsonValue::as_str) .expect("local_path in structured content"); assert!(local_path.contains("codex_apps_downloads")); + assert!(local_path.ends_with("trusted-name.txt")); let saved = tokio::fs::read(local_path) .await .expect("saved local file should exist"); assert_eq!(saved, b"downloaded contents".to_vec()); + assert_eq!(attacker_hits.load(std::sync::atomic::Ordering::SeqCst), 0); assert!(result.content.iter().any(|block| { block.get("type").and_then(JsonValue::as_str) == Some("text") && block