diff --git a/codex-rs/analytics/src/client.rs b/codex-rs/analytics/src/client.rs index 15e6ddc866..26991de16d 100644 --- a/codex-rs/analytics/src/client.rs +++ b/codex-rs/analytics/src/client.rs @@ -1054,6 +1054,13 @@ async fn send_track_events_request( #[cfg(debug_assertions)] AnalyticsEventsDestination::CaptureFile { .. } => return, }; + let headers = match codex_model_provider::auth_provider_from_auth(auth).to_auth_headers() { + Ok(headers) => headers, + Err(error) => { + tracing::warn!("failed to authenticate events request: {error}"); + return; + } + }; let client = match codex_login::default_client::create_client_for_route_async( http_client_factory.clone(), url.clone(), @@ -1071,7 +1078,7 @@ async fn send_track_events_request( let mut request = client .post(url) .timeout(ANALYTICS_EVENTS_TIMEOUT) - .headers(codex_model_provider::auth_provider_from_auth(auth).to_auth_headers()) + .headers(headers) .header("Content-Type", "application/json") .json(&payload); if let Some(product_sku) = product_sku { diff --git a/codex-rs/app-server-transport/src/transport/remote_control/auth.rs b/codex-rs/app-server-transport/src/transport/remote_control/auth.rs index 9d274627e2..e0e7f9e9dd 100644 --- a/codex-rs/app-server-transport/src/transport/remote_control/auth.rs +++ b/codex-rs/app-server-transport/src/transport/remote_control/auth.rs @@ -72,7 +72,9 @@ pub(super) struct RemoteControlConnectionAuth { impl RemoteControlConnectionAuth { pub(super) fn request_headers(&self) -> io::Result { let mut headers = HeaderMap::new(); - self.auth_provider.add_auth_headers(&mut headers); + self.auth_provider + .add_auth_headers(&mut headers) + .map_err(io::Error::other)?; headers.insert( REMOTE_CONTROL_ACCOUNT_ID_HEADER, HeaderValue::from_str(&self.account_id).map_err(|err| { @@ -239,7 +241,7 @@ mod tests { } impl AuthProvider for TestAuthProvider { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { headers.insert( axum::http::header::AUTHORIZATION, HeaderValue::from_static("Bearer test-token"), @@ -248,6 +250,7 @@ mod tests { for account_id in &self.account_ids { headers.append("ChatGPT-Account-ID", HeaderValue::from_static(account_id)); } + Ok(()) } } diff --git a/codex-rs/app-server/src/message_processor_gateway_oauth_tests.rs b/codex-rs/app-server/src/message_processor_gateway_oauth_tests.rs index dd63e67183..2859336dbf 100644 --- a/codex-rs/app-server/src/message_processor_gateway_oauth_tests.rs +++ b/codex-rs/app-server/src/message_processor_gateway_oauth_tests.rs @@ -103,7 +103,7 @@ delivery = {{ kind = "header", name = "X-Gateway-Authorization" }} Some(Arc::clone(&auth_manager)), ); assert_eq!( - provider.api_auth().await?.to_auth_headers()["x-gateway-authorization"], + provider.api_auth().await?.to_auth_headers().unwrap()["x-gateway-authorization"], "Bearer old-access" ); let (processor, mut outgoing) = build_test_processor(config, auth_manager).await; @@ -182,7 +182,7 @@ delivery = {{ kind = "header", name = "X-Gateway-Authorization" }} ); assert_eq!(observer.resolve_access_token().await?, "new-access"); assert_eq!( - provider.api_auth().await?.to_auth_headers()["x-gateway-authorization"], + provider.api_auth().await?.to_auth_headers().unwrap()["x-gateway-authorization"], "Bearer new-access" ); processor.shutdown_threads().await; diff --git a/codex-rs/backend-client/src/client.rs b/codex-rs/backend-client/src/client.rs index d7f1862ccb..ed72e9ae5c 100644 --- a/codex-rs/backend-client/src/client.rs +++ b/codex-rs/backend-client/src/client.rs @@ -266,14 +266,14 @@ impl Client { self } - fn headers(&self) -> HeaderMap { + fn headers(&self) -> Result { let mut h = HeaderMap::new(); if let Some(ua) = &self.user_agent { h.insert(USER_AGENT, ua.clone()); } else { h.insert(USER_AGENT, HeaderValue::from_static("codex-cli")); } - self.auth_provider.add_auth_headers(&mut h); + self.auth_provider.add_auth_headers(&mut h)?; if let Some(acc) = &self.chatgpt_account_id && let Ok(name) = HeaderName::from_bytes(b"ChatGPT-Account-Id") && let Ok(hv) = HeaderValue::from_str(acc) @@ -285,7 +285,7 @@ impl Client { { h.insert(name, HeaderValue::from_static("true")); } - h + Ok(h) } fn request(&self, method: Method, url: &str) -> RequestBuilder { @@ -344,7 +344,7 @@ impl Client { &self, url: &str, ) -> std::result::Result<(String, String), RequestError> { - let request = self.request(Method::GET, url).headers(self.headers()); + let request = self.request(Method::GET, url).headers(self.headers()?); if !self.http.allows_system_proxy_fallback() { return self.exec_request_detailed(request, "GET", url).await; } @@ -371,7 +371,7 @@ impl Client { .http .clone() .with_outbound_proxy_policy(OutboundProxyPolicy::RespectSystemProxy); - let request = http.get(url).headers(self.headers()); + let request = http.get(url).headers(self.headers()?); self.exec_request_detailed(request, "GET", url).await } @@ -411,7 +411,7 @@ impl Client { pub async fn get_token_usage_profile(&self) -> Result { let url = self.token_usage_profile_url(); - let req = self.request(Method::GET, &url).headers(self.headers()); + let req = self.request(Method::GET, &url).headers(self.headers()?); let (body, ct) = self.exec_request(req, "GET", &url).await?; self.decode_json(&url, &ct, &body) } @@ -430,7 +430,7 @@ impl Client { let url = self.send_add_credits_nudge_email_url(); let req = self .request(Method::POST, &url) - .headers(self.headers()) + .headers(self.headers()?) .header(CONTENT_TYPE, HeaderValue::from_static("application/json")) .json(&SendAddCreditsNudgeEmailRequest { credit_type }); self.exec_request_detailed(req, "POST", &url).await?; @@ -445,7 +445,7 @@ impl Client { cursor: Option<&str>, ) -> Result { let url = self.list_tasks_url(limit, task_filter, environment_id, cursor)?; - let req = self.request(Method::GET, &url).headers(self.headers()); + let req = self.request(Method::GET, &url).headers(self.headers()?); let (body, ct) = self.exec_request(req, "GET", &url).await?; self.decode_json::(&url, &ct, &body) } @@ -497,7 +497,7 @@ impl Client { PathStyle::CodexApi => format!("{}/api/codex/tasks/{}", self.base_url, task_id), PathStyle::ChatGptApi => format!("{}/wham/tasks/{}", self.base_url, task_id), }; - let req = self.request(Method::GET, &url).headers(self.headers()); + let req = self.request(Method::GET, &url).headers(self.headers()?); let (body, ct) = self.exec_request(req, "GET", &url).await?; let parsed: CodeTaskDetailsResponse = self.decode_json(&url, &ct, &body)?; Ok((parsed, body, ct)) @@ -518,7 +518,7 @@ impl Client { self.base_url, task_id, turn_id ), }; - let req = self.request(Method::GET, &url).headers(self.headers()); + let req = self.request(Method::GET, &url).headers(self.headers()?); let (body, ct) = self.exec_request(req, "GET", &url).await?; self.decode_json::(&url, &ct, &body) } @@ -554,7 +554,7 @@ impl Client { let url = self.user_settings_url(); let req = self .request(Method::GET, &url) - .headers(self.headers()) + .headers(self.headers()?) .header( CACHE_CONTROL, HeaderValue::from_static("no-cache, no-store"), @@ -570,7 +570,7 @@ impl Client { let url = self.workspace_messages_url(); let req = self .request(Method::GET, &url) - .headers(self.headers()) + .headers(self.headers()?) .header(CACHE_CONTROL, HeaderValue::from_static("no-store")); let (body, ct) = self.exec_request_detailed(req, "GET", &url).await?; self.decode_json::(&url, &ct, &body) @@ -586,7 +586,7 @@ impl Client { }; let req = self .request(Method::POST, &url) - .headers(self.headers()) + .headers(self.headers()?) .header(CONTENT_TYPE, HeaderValue::from_static("application/json")) .json(&request_body); let (body, ct) = self.exec_request(req, "POST", &url).await?; @@ -1255,7 +1255,7 @@ mod tests { &auth, HttpClientFactory::new(codex_http_client::OutboundProxyPolicy::ReqwestDefault), ); - let headers = client.headers(); + let headers = client.headers().unwrap(); assert_eq!( [ diff --git a/codex-rs/backend-client/src/client/analytics.rs b/codex-rs/backend-client/src/client/analytics.rs index 3ef85ac99b..be92c63e25 100644 --- a/codex-rs/backend-client/src/client/analytics.rs +++ b/codex-rs/backend-client/src/client/analytics.rs @@ -96,7 +96,7 @@ impl Client { } let request = self .request(Method::GET, url.as_str()) - .headers(self.headers()); + .headers(self.headers()?); let (body, _) = self .exec_request_detailed(request, "GET", url.as_str()) .await?; diff --git a/codex-rs/backend-client/src/client/chatgpt_turn_cost.rs b/codex-rs/backend-client/src/client/chatgpt_turn_cost.rs index a0398ef5c0..2379d00ff3 100644 --- a/codex-rs/backend-client/src/client/chatgpt_turn_cost.rs +++ b/codex-rs/backend-client/src/client/chatgpt_turn_cost.rs @@ -57,7 +57,7 @@ impl Client { }; let request = self .request(Method::POST, &url) - .headers(self.headers()) + .headers(self.headers()?) .json(&TurnCostsRequest { threads: threads .iter() diff --git a/codex-rs/backend-client/src/client/plan_history.rs b/codex-rs/backend-client/src/client/plan_history.rs index 875c450168..4700123a25 100644 --- a/codex-rs/backend-client/src/client/plan_history.rs +++ b/codex-rs/backend-client/src/client/plan_history.rs @@ -66,7 +66,7 @@ impl Client { PathStyle::ChatGptApi => "wham", }; let url = format!("{}/{prefix}/usage/plan_limit_history?days=7", self.base_url); - let request = self.request(Method::GET, &url).headers(self.headers()); + let request = self.request(Method::GET, &url).headers(self.headers()?); let (body, _) = match self.exec_request_detailed(request, "GET", &url).await { Ok(response) => response, Err(error) if error.status().is_some_and(|status| status.as_u16() == 404) => { diff --git a/codex-rs/backend-client/src/client/profile.rs b/codex-rs/backend-client/src/client/profile.rs index 745efa1893..4117c8d5f7 100644 --- a/codex-rs/backend-client/src/client/profile.rs +++ b/codex-rs/backend-client/src/client/profile.rs @@ -61,7 +61,7 @@ impl Client { /// Read the full profile using the same route as account token usage. pub async fn get_account_profile(&self) -> Result { let url = self.token_usage_profile_url(); - let request = self.request(Method::GET, &url).headers(self.headers()); + let request = self.request(Method::GET, &url).headers(self.headers()?); let (body, _) = self.exec_request_detailed(request, "GET", &url).await?; serde_json::from_str(&body) .map_err(|_| RequestError::Other(anyhow::anyhow!("Invalid account profile response."))) diff --git a/codex-rs/backend-client/src/client/rate_limit_resets.rs b/codex-rs/backend-client/src/client/rate_limit_resets.rs index 07a6ecc5dc..03e7a30481 100644 --- a/codex-rs/backend-client/src/client/rate_limit_resets.rs +++ b/codex-rs/backend-client/src/client/rate_limit_resets.rs @@ -71,7 +71,7 @@ impl Client { supports_luna_reserve: bool, ) -> Result { let url = self.rate_limit_status_url(); - let mut req = self.request(Method::GET, &url).headers(self.headers()); + let mut req = self.request(Method::GET, &url).headers(self.headers()?); if supports_luna_reserve { req = req.header("x-openai-codex-luna-reserve", HeaderValue::from_static("1")); } @@ -81,7 +81,7 @@ impl Client { pub async fn list_rate_limit_reset_credits(&self) -> Result { let url = self.rate_limit_reset_credits_url(); - let req = self.request(Method::GET, &url).headers(self.headers()); + let req = self.request(Method::GET, &url).headers(self.headers()?); let (body, ct) = self.exec_request(req, "GET", &url).await?; self.decode_json(&url, &ct, &body) } @@ -111,7 +111,7 @@ impl Client { let url = self.consume_rate_limit_reset_credit_url(); let req = self .request(Method::POST, &url) - .headers(self.headers()) + .headers(self.headers()?) .header(CONTENT_TYPE, HeaderValue::from_static("application/json")) .json(&ConsumeRateLimitResetCreditRequest { redeem_request_id, diff --git a/codex-rs/backend-client/src/client/task_usage.rs b/codex-rs/backend-client/src/client/task_usage.rs index daf0790eeb..cbb38cbe0f 100644 --- a/codex-rs/backend-client/src/client/task_usage.rs +++ b/codex-rs/backend-client/src/client/task_usage.rs @@ -103,7 +103,7 @@ impl Client { } let request = self .request(Method::POST, &url) - .headers(self.headers()) + .headers(self.headers()?) .json(&Query { threads }); let (body, _) = self.exec_request_detailed(request, "POST", &url).await?; let response: TaskUsageResponse = serde_json::from_str(&body) diff --git a/codex-rs/backend-client/src/client/thread_usage.rs b/codex-rs/backend-client/src/client/thread_usage.rs index e7723b1e7b..fb899a837a 100644 --- a/codex-rs/backend-client/src/client/thread_usage.rs +++ b/codex-rs/backend-client/src/client/thread_usage.rs @@ -82,7 +82,7 @@ impl Client { let url = self.thread_usage_url(); let request = self .request(Method::POST, &url) - .headers(self.headers()) + .headers(self.headers()?) .header(CONTENT_TYPE, HeaderValue::from_static("application/json")) .json(&ThreadUsageQueryRequest { thread_ids }); let (body, _) = self.exec_request_detailed(request, "POST", &url).await?; diff --git a/codex-rs/backend-client/src/client/turn_usage.rs b/codex-rs/backend-client/src/client/turn_usage.rs index b3b5b27c12..fa8314e316 100644 --- a/codex-rs/backend-client/src/client/turn_usage.rs +++ b/codex-rs/backend-client/src/client/turn_usage.rs @@ -84,7 +84,7 @@ impl Client { provider_headers: &HeaderMap, ) -> Result, RequestError> { let mut headers = provider_headers.clone(); - headers.extend(self.headers()); + headers.extend(self.headers()?); let request = self .request(Method::POST, url) .headers(headers) diff --git a/codex-rs/chatgpt/src/chatgpt_client.rs b/codex-rs/chatgpt/src/chatgpt_client.rs index c56f8b9b27..ec6efb67fc 100644 --- a/codex-rs/chatgpt/src/chatgpt_client.rs +++ b/codex-rs/chatgpt/src/chatgpt_client.rs @@ -93,7 +93,7 @@ pub(crate) async fn chatgpt_get_request_with_timeout( let mut request = client .get(&url) .headers(default_headers()) - .headers(codex_model_provider::auth_provider_from_auth(&auth).to_auth_headers()) + .headers(codex_model_provider::auth_provider_from_auth(&auth).to_auth_headers()?) .header(OAI_PRODUCT_SKU_HEADER, CODEX_PRODUCT_SKU) .header("Content-Type", "application/json"); if let Some(timeout) = timeout { @@ -147,7 +147,7 @@ pub(crate) async fn chatgpt_post_request_with_timeout< let response = client .post(&url) .headers(default_headers()) - .headers(codex_model_provider::auth_provider_from_auth(auth).to_auth_headers()) + .headers(codex_model_provider::auth_provider_from_auth(auth).to_auth_headers()?) .header(OAI_PRODUCT_SKU_HEADER, product_sku) .header("Content-Type", "application/json") .timeout(timeout) diff --git a/codex-rs/cli/src/exec_server_auth.rs b/codex-rs/cli/src/exec_server_auth.rs index 96193ea9b5..ba74f98996 100644 --- a/codex-rs/cli/src/exec_server_auth.rs +++ b/codex-rs/cli/src/exec_server_auth.rs @@ -43,7 +43,9 @@ struct AwsSigV4AuthProvider { } impl AuthProvider for AwsSigV4AuthProvider { - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> Result<(), AuthError> { + Ok(()) + } fn apply_auth(&self, mut request: Request) -> codex_api::AuthProviderFuture<'_> { Box::pin(async move { diff --git a/codex-rs/cli/src/exec_server_credential.rs b/codex-rs/cli/src/exec_server_credential.rs index 72d034e9a5..6142a57e35 100644 --- a/codex-rs/cli/src/exec_server_credential.rs +++ b/codex-rs/cli/src/exec_server_credential.rs @@ -16,7 +16,6 @@ use std::time::SystemTime; use std::time::UNIX_EPOCH; use codex_api::AuthError; -use codex_api::AuthHeadersFuture; use codex_api::AuthProvider; use http::HeaderMap; use http::HeaderValue; @@ -107,26 +106,20 @@ impl ExecutorCredentialProvider { } impl AuthProvider for ExecutorCredentialProvider { - // Only the fallible API can prevent an unauthenticated registry request. - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} - - fn resolve_auth_headers(&self) -> AuthHeadersFuture<'_> { - Box::pin(async { - let token = self - .current - .borrow() - .header() - .map_err(|error| AuthError::Build(error.to_string()))?; - let token = token - .to_str() - .map_err(|_| AuthError::Build(CredentialError::InvalidFrame.to_string()))?; - let mut authorization = HeaderValue::from_str(&format!("Bearer {token}")) - .map_err(|_| AuthError::Build(CredentialError::InvalidFrame.to_string()))?; - authorization.set_sensitive(true); - let mut headers = HeaderMap::new(); - headers.insert(AUTHORIZATION, authorization); - Ok(headers) - }) + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), AuthError> { + let token = self + .current + .borrow() + .header() + .map_err(|error| AuthError::Build(error.to_string()))?; + let token = token + .to_str() + .map_err(|_| AuthError::Build(CredentialError::InvalidFrame.to_string()))?; + let mut authorization = HeaderValue::from_str(&format!("Bearer {token}")) + .map_err(|_| AuthError::Build(CredentialError::InvalidFrame.to_string()))?; + authorization.set_sensitive(true); + headers.insert(AUTHORIZATION, authorization); + Ok(()) } } diff --git a/codex-rs/cli/src/exec_server_credential_tests.rs b/codex-rs/cli/src/exec_server_credential_tests.rs index 8d5e7956e6..1772985cb3 100644 --- a/codex-rs/cli/src/exec_server_credential_tests.rs +++ b/codex-rs/cli/src/exec_server_credential_tests.rs @@ -43,7 +43,7 @@ async fn waits_for_initial_rotation_replaces_token_and_eof_clears_auth() { Err(CredentialError::InitialTimeout) ); assert!(provider.resolve_auth_headers().await.is_err()); - assert!(provider.to_auth_headers().is_empty()); + assert!(provider.to_auth_headers().is_err()); writer .write_all(&frame("env-a", "first-scoped-token", expires)) .expect("write"); diff --git a/codex-rs/cloud-tasks/src/lib.rs b/codex-rs/cloud-tasks/src/lib.rs index 6643684166..b0975fe929 100644 --- a/codex-rs/cloud-tasks/src/lib.rs +++ b/codex-rs/cloud-tasks/src/lib.rs @@ -206,7 +206,7 @@ async fn resolve_environment_id(ctx: &BackendContext, requested: &str) -> anyhow return Err(anyhow!("environment id must not be empty")); } let normalized = util::normalize_base_url(&ctx.base_url); - let headers = util::build_chatgpt_headers().await; + let headers = util::build_chatgpt_headers().await?; let environments = crate::env_detect::list_environments(&ctx.environment_http, &normalized, &headers).await?; if environments.is_empty() { @@ -870,7 +870,13 @@ pub async fn run_main(cli: Cli, _codex_linux_sandbox_exe: Option) -> an tokio::spawn(async move { let base_url = util::normalize_base_url(&base_url); // Build headers: UA + ChatGPT auth if available - let headers = util::build_chatgpt_headers().await; + let headers = match util::build_chatgpt_headers().await { + Ok(headers) => headers, + Err(error) => { + let _ = tx.send(app::AppEvent::EnvironmentAutodetected(Err(error))); + return; + } + }; // Run autodetect. If it fails, we keep using "All". let res = crate::env_detect::autodetect_environment_id( @@ -2030,8 +2036,10 @@ fn spawn_environment_load( ) { tokio::spawn(async move { let base_url = util::normalize_base_url(&base_url); - let headers = util::build_chatgpt_headers().await; - let result = crate::env_detect::list_environments(&http, &base_url, &headers).await; + let result = match util::build_chatgpt_headers().await { + Ok(headers) => crate::env_detect::list_environments(&http, &base_url, &headers).await, + Err(error) => Err(error), + }; let _ = tx.send(app::AppEvent::EnvironmentsLoaded(result)); }); } diff --git a/codex-rs/cloud-tasks/src/util.rs b/codex-rs/cloud-tasks/src/util.rs index d041d8ce10..f5f770b197 100644 --- a/codex-rs/cloud-tasks/src/util.rs +++ b/codex-rs/cloud-tasks/src/util.rs @@ -110,7 +110,7 @@ pub async fn load_auth_manager( /// Build headers for ChatGPT-backed requests: `User-Agent`, optional `Authorization`, /// and optional `ChatGPT-Account-Id`. -pub async fn build_chatgpt_headers() -> HeaderMap { +pub async fn build_chatgpt_headers() -> anyhow::Result { use http::header::HeaderValue; use http::header::USER_AGENT; @@ -125,9 +125,9 @@ pub async fn build_chatgpt_headers() -> HeaderMap { && let Some(auth) = am.auth().await && auth.uses_codex_backend() { - headers.extend(codex_model_provider::auth_provider_from_auth(&auth).to_auth_headers()); + headers.extend(codex_model_provider::auth_provider_from_auth(&auth).to_auth_headers()?); } - headers + Ok(headers) } /// Construct a browser-friendly task URL for the given backend base URL. diff --git a/codex-rs/codex-api/src/auth.rs b/codex-rs/codex-api/src/auth.rs index 5cd8007af0..4e23badcff 100644 --- a/codex-rs/codex-api/src/auth.rs +++ b/codex-rs/codex-api/src/auth.rs @@ -1,5 +1,6 @@ use codex_client::Request; use codex_client::TransportError; +use codex_http_client::NetworkPolicyDenied; use http::HeaderMap; use std::future::Future; use std::pin::Pin; @@ -8,6 +9,8 @@ use std::sync::Arc; /// Error returned while applying authentication to an outbound request. #[derive(Debug, thiserror::Error)] pub enum AuthError { + #[error(transparent)] + Policy(#[from] NetworkPolicyDenied), #[error("request auth build error: {0}")] Build(String), #[error("transient auth error: {0}")] @@ -17,6 +20,7 @@ pub enum AuthError { impl From for TransportError { fn from(error: AuthError) -> Self { match error { + AuthError::Policy(denied) => TransportError::Policy(denied), AuthError::Build(message) => TransportError::Build(message), AuthError::Transient(message) => TransportError::Network(message), } @@ -31,14 +35,16 @@ pub trait AuthProvider: Send + Sync { /// Adds any auth headers that are available without request body access. /// /// Implementations should be cheap and non-blocking. This method is also - /// used by telemetry and non-HTTP request paths. - fn add_auth_headers(&self, headers: &mut HeaderMap); + /// used by telemetry and non-HTTP request paths. Providers that require async + /// work must return an error here and override `resolve_auth_headers`. + /// Callers must not send the request if this returns an error. + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), AuthError>; /// Returns any auth headers that are available without request body access. - fn to_auth_headers(&self) -> HeaderMap { + fn to_auth_headers(&self) -> Result { let mut headers = HeaderMap::new(); - self.add_auth_headers(&mut headers); - headers + self.add_auth_headers(&mut headers)?; + Ok(headers) } /// Resolves auth headers for an outbound request. @@ -47,7 +53,7 @@ pub trait AuthProvider: Send + Sync { /// credentials before returning. Header-only providers with static credentials can rely on the /// default implementation. fn resolve_auth_headers(&self) -> AuthHeadersFuture<'_> { - Box::pin(async { Ok(self.to_auth_headers()) }) + Box::pin(async { self.to_auth_headers() }) } /// Applies auth to a complete outbound request and returns the request to send. @@ -91,14 +97,14 @@ pub struct AuthHeaderTelemetry { pub name: Option<&'static str>, } -pub fn auth_header_telemetry(auth: &dyn AuthProvider) -> AuthHeaderTelemetry { +pub fn auth_header_telemetry(auth: &dyn AuthProvider) -> Result { let mut headers = HeaderMap::new(); - auth.add_auth_headers(&mut headers); + auth.add_auth_headers(&mut headers)?; let name = headers .contains_key(http::header::AUTHORIZATION) .then_some("authorization"); - AuthHeaderTelemetry { + Ok(AuthHeaderTelemetry { attached: name.is_some(), name, - } + }) } diff --git a/codex-rs/codex-api/src/endpoint/images.rs b/codex-rs/codex-api/src/endpoint/images.rs index 2217ce8961..ec4b3f8608 100644 --- a/codex-rs/codex-api/src/endpoint/images.rs +++ b/codex-rs/codex-api/src/endpoint/images.rs @@ -139,7 +139,9 @@ mod tests { struct DummyAuth; impl AuthProvider for DummyAuth { - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> Result<(), crate::AuthError> { + Ok(()) + } } #[derive(Clone)] diff --git a/codex-rs/codex-api/src/endpoint/memories.rs b/codex-rs/codex-api/src/endpoint/memories.rs index 122ca565f7..979f96bb64 100644 --- a/codex-rs/codex-api/src/endpoint/memories.rs +++ b/codex-rs/codex-api/src/endpoint/memories.rs @@ -102,7 +102,9 @@ mod tests { struct DummyAuth; impl AuthProvider for DummyAuth { - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> Result<(), crate::AuthError> { + Ok(()) + } } #[derive(Clone)] diff --git a/codex-rs/codex-api/src/endpoint/models.rs b/codex-rs/codex-api/src/endpoint/models.rs index c00bc0115e..1a15d05ddc 100644 --- a/codex-rs/codex-api/src/endpoint/models.rs +++ b/codex-rs/codex-api/src/endpoint/models.rs @@ -184,7 +184,9 @@ mod tests { struct DummyAuth; impl AuthProvider for DummyAuth { - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> Result<(), crate::AuthError> { + Ok(()) + } } #[derive(Default)] @@ -193,8 +195,9 @@ mod tests { } impl AuthProvider for RetryAuth { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), AuthError> { headers.insert(http::header::AUTHORIZATION, "Bearer test".parse().unwrap()); + Ok(()) } fn apply_auth(&self, mut request: Request) -> AuthProviderFuture<'_> { @@ -204,7 +207,7 @@ mod tests { if limits.len() == 1 { return Err(AuthError::Transient("retry authentication".to_string())); } - self.add_auth_headers(&mut request.headers); + self.add_auth_headers(&mut request.headers)?; Ok(request) }) } diff --git a/codex-rs/codex-api/src/endpoint/realtime_call.rs b/codex-rs/codex-api/src/endpoint/realtime_call.rs index b43e02d9b0..745d0b0405 100644 --- a/codex-rs/codex-api/src/endpoint/realtime_call.rs +++ b/codex-rs/codex-api/src/endpoint/realtime_call.rs @@ -360,11 +360,12 @@ mod tests { struct DummyAuth; impl AuthProvider for DummyAuth { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), crate::AuthError> { headers.insert( http::header::AUTHORIZATION, HeaderValue::from_static("Bearer test-token"), ); + Ok(()) } } diff --git a/codex-rs/codex-api/src/endpoint/responses_websocket.rs b/codex-rs/codex-api/src/endpoint/responses_websocket.rs index 77ed1ea60d..c59683d69d 100644 --- a/codex-rs/codex-api/src/endpoint/responses_websocket.rs +++ b/codex-rs/codex-api/src/endpoint/responses_websocket.rs @@ -466,7 +466,7 @@ impl ResponsesWebsocketClient { let mut headers = merge_request_headers(&self.provider.headers, extra_headers, default_headers); - self.auth.add_auth_headers(&mut headers); + self.auth.add_auth_headers(&mut headers)?; let connector = WebSocketConnector::new(http_client_factory) .map_err(|err| ApiError::Stream(format!("failed to configure websocket TLS: {err}")))?; diff --git a/codex-rs/codex-api/src/endpoint/responses_websocket/connector.rs b/codex-rs/codex-api/src/endpoint/responses_websocket/connector.rs index e305ca3a4a..86e02c1847 100644 --- a/codex-rs/codex-api/src/endpoint/responses_websocket/connector.rs +++ b/codex-rs/codex-api/src/endpoint/responses_websocket/connector.rs @@ -35,7 +35,7 @@ impl ResponsesWebsocketClient { .map_err(|err| ApiError::Stream(format!("failed to build websocket URL: {err}")))?; let mut headers = merge_request_headers(&self.provider.headers, extra_headers, default_headers); - self.auth.add_auth_headers(&mut headers); + self.auth.add_auth_headers(&mut headers)?; let (stream, _status, server_reasoning_included, server_model) = connect_websocket(ws_url, headers, connector, turn_state).await?; Ok(ResponsesWebsocketConnection::new( diff --git a/codex-rs/codex-api/src/endpoint/search.rs b/codex-rs/codex-api/src/endpoint/search.rs index 502e264b4b..ee47b906a7 100644 --- a/codex-rs/codex-api/src/endpoint/search.rs +++ b/codex-rs/codex-api/src/endpoint/search.rs @@ -83,7 +83,9 @@ mod tests { struct DummyAuth; impl AuthProvider for DummyAuth { - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> Result<(), crate::AuthError> { + Ok(()) + } } #[derive(Clone)] diff --git a/codex-rs/codex-api/src/error.rs b/codex-rs/codex-api/src/error.rs index 6d98cd4cb2..ad54b727ab 100644 --- a/codex-rs/codex-api/src/error.rs +++ b/codex-rs/codex-api/src/error.rs @@ -66,3 +66,9 @@ pub(crate) fn parse_flex_unavailable(error: &Value) -> Option { (error.get("code").and_then(Value::as_str) == Some("flex_unavailable")) .then_some(ApiError::FlexUnavailable) } + +impl From for ApiError { + fn from(error: crate::AuthError) -> Self { + Self::Transport(error.into()) + } +} diff --git a/codex-rs/codex-api/src/files.rs b/codex-rs/codex-api/src/files.rs index 2534516b60..0a37b2753a 100644 --- a/codex-rs/codex-api/src/files.rs +++ b/codex-rs/codex-api/src/files.rs @@ -46,6 +46,8 @@ pub struct UploadedOpenAiFile { pub enum OpenAiFileError { #[error("failed to open OpenAI file upload contents: {0}")] ReadContents(#[source] std::io::Error), + #[error(transparent)] + Auth(#[from] crate::AuthError), #[error( "file `{file_name}` is too large: {size_bytes} bytes exceeds the limit of {limit_bytes} bytes" )] @@ -152,7 +154,7 @@ where "file_size": file_size_bytes, "use_case": OPENAI_FILE_USE_CASE, }); - let request = authorized_request(client_pool, auth, Method::POST, &create_url); + let request = authorized_request(client_pool, auth, Method::POST, &create_url)?; let create_request = match hosted_upload { Some(context) => serde_json::json!({ "file_name": file_name.as_str(), @@ -363,7 +365,7 @@ where }; let finalize_started_at = Instant::now(); loop { - let finalize_response = authorized_request(client_pool, auth, Method::POST, &finalize_url) + let finalize_response = authorized_request(client_pool, auth, Method::POST, &finalize_url)? .json(&finalize_request) .send() .await @@ -435,14 +437,14 @@ fn authorized_request( auth: &dyn AuthProvider, method: Method, url: &str, -) -> RequestBuilder { +) -> Result { let mut headers = http::HeaderMap::new(); - auth.add_auth_headers(&mut headers); + auth.add_auth_headers(&mut headers)?; - client_pool + Ok(client_pool .request(method, url) .timeout(OPENAI_FILE_REQUEST_TIMEOUT) - .headers(headers) + .headers(headers)) } fn upload_response_header(response: &HttpResponse, header: &str) -> String { @@ -511,12 +513,13 @@ mod tests { } impl AuthProvider for ChatGptTestAuth { - fn add_auth_headers(&self, headers: &mut http::HeaderMap) { + fn add_auth_headers(&self, headers: &mut http::HeaderMap) -> Result<(), crate::AuthError> { headers.insert( http::header::AUTHORIZATION, HeaderValue::from_static("Bearer token"), ); headers.insert("ChatGPT-Account-ID", HeaderValue::from_static("account_id")); + Ok(()) } } diff --git a/codex-rs/codex-api/tests/clients.rs b/codex-rs/codex-api/tests/clients.rs index a7400d306e..4410c15d75 100644 --- a/codex-rs/codex-api/tests/clients.rs +++ b/codex-rs/codex-api/tests/clients.rs @@ -105,7 +105,9 @@ impl HttpTransport for RecordingTransport { struct NoAuth; impl AuthProvider for NoAuth { - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> Result<(), AuthError> { + Ok(()) + } } #[derive(Clone)] @@ -124,7 +126,7 @@ impl StaticAuth { } impl AuthProvider for StaticAuth { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), AuthError> { let token = &self.token; if let Ok(header) = HeaderValue::from_str(&format!("Bearer {token}")) { headers.insert(http::header::AUTHORIZATION, header); @@ -132,6 +134,7 @@ impl AuthProvider for StaticAuth { if let Ok(header) = HeaderValue::from_str(&self.account_id) { headers.insert("ChatGPT-Account-ID", header); } + Ok(()) } } @@ -199,19 +202,10 @@ struct FailsOnceAuth { } impl FailsOnceAuth { - fn transient() -> Self { + fn new(error: AuthError) -> Self { Self { attempts: Arc::new(Mutex::new(0)), - error: Arc::new(AuthError::Transient( - "sts temporarily unavailable".to_string(), - )), - } - } - - fn build() -> Self { - Self { - attempts: Arc::new(Mutex::new(0)), - error: Arc::new(AuthError::Build("invalid auth configuration".to_string())), + error: Arc::new(error), } } @@ -231,6 +225,7 @@ impl FailsOnceAuth { if *attempts == 1 { return match self.error.as_ref() { + AuthError::Policy(denied) => Err(AuthError::Policy(*denied)), AuthError::Build(message) => Err(AuthError::Build(message.clone())), AuthError::Transient(message) => Err(AuthError::Transient(message.clone())), }; @@ -241,7 +236,9 @@ impl FailsOnceAuth { } impl AuthProvider for FailsOnceAuth { - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> Result<(), AuthError> { + Ok(()) + } fn apply_auth(&self, request: Request) -> codex_api::AuthProviderFuture<'_> { Box::pin(FailsOnceAuth::apply_auth(self, request)) @@ -545,7 +542,7 @@ async fn streaming_client_retries_on_transport_error() -> Result<()> { async fn streaming_client_retries_on_transient_auth_error() -> Result<()> { let state = RecordingState::default(); let transport = RecordingTransport::new(state.clone()); - let auth = FailsOnceAuth::transient(); + let auth = FailsOnceAuth::new(AuthError::Transient("sts temporarily unavailable".into())); let mut provider = provider("openai"); provider.retry.max_attempts = 2; @@ -567,36 +564,39 @@ async fn streaming_client_retries_on_transient_auth_error() -> Result<()> { } #[tokio::test] -async fn streaming_client_does_not_retry_auth_build_error() -> Result<()> { - let state = RecordingState::default(); - let transport = RecordingTransport::new(state.clone()); - let auth = FailsOnceAuth::build(); - - let mut provider = provider("openai"); - provider.retry.max_attempts = 2; - - let client = ResponsesClient::new(transport, provider, Arc::new(auth.clone())); - let body = serde_json::json!({ "model": "gpt-test" }); - let result = client - .stream( - body, - HeaderMap::new(), - Compression::None, - /*turn_state*/ None, - ) - .await; - let err = result - .err() - .expect("auth build errors should fail without retry"); - - assert!(matches!( - err, - ApiError::Transport(TransportError::Build(message)) - if message == "invalid auth configuration" - )); - assert_eq!(auth.attempts(), 1); - assert_eq!(state.take_stream_requests().len(), 0); - Ok(()) +async fn streaming_client_does_not_retry_terminal_auth_errors() { + for error in [ + AuthError::Build("invalid auth configuration".into()), + AuthError::Policy(codex_http_client::NetworkPolicyDenied::Revoked), + ] { + let state = RecordingState::default(); + let transport = RecordingTransport::new(state.clone()); + let auth = FailsOnceAuth::new(error); + let mut provider = provider("openai"); + provider.retry.max_attempts = 2; + let client = ResponsesClient::new(transport, provider, Arc::new(auth.clone())); + let error = client + .stream( + serde_json::json!({ "model": "gpt-test" }), + HeaderMap::new(), + Compression::None, + /*turn_state*/ None, + ) + .await + .err() + .expect("terminal auth errors must fail without retry"); + match (error, auth.error.as_ref()) { + (ApiError::Transport(TransportError::Build(actual)), AuthError::Build(expected)) => { + assert_eq!(&actual, expected); + } + (ApiError::Transport(TransportError::Policy(actual)), AuthError::Policy(expected)) => { + assert_eq!(&actual, expected); + } + (actual, expected) => panic!("expected {expected:?}, got {actual:?}"), + } + assert_eq!(auth.attempts(), 1); + assert!(state.take_stream_requests().is_empty()); + } } #[tokio::test] @@ -688,3 +688,84 @@ async fn azure_store_sends_ids_and_headers() -> Result<()> { Ok(()) } + +struct RejectedHeaderAuth; + +impl AuthProvider for RejectedHeaderAuth { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), AuthError> { + headers.insert( + http::header::AUTHORIZATION, + HeaderValue::from_static("Bearer partial-auth"), + ); + Err(AuthError::Build("header authentication failed".to_string())) + } +} + +#[tokio::test] +async fn header_auth_error_prevents_sending_partial_authentication() { + let state = RecordingState::default(); + let client = ResponsesClient::new( + RecordingTransport::new(state.clone()), + provider("openai"), + Arc::new(RejectedHeaderAuth), + ); + let result = client + .stream( + serde_json::json!({"echo": true}), + HeaderMap::new(), + Compression::None, + /*turn_state*/ None, + ) + .await; + + assert!(matches!( + result, + Err(ApiError::Transport(TransportError::Build(_))) + )); + assert!(state.take_stream_requests().is_empty()); +} + +struct AsyncHeaderAuth; + +impl AuthProvider for AsyncHeaderAuth { + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { + Err(codex_api::AuthError::Build( + "authentication requires async header resolution".to_string(), + )) + } + + fn resolve_auth_headers(&self) -> codex_api::AuthHeadersFuture<'_> { + Box::pin(async { + Ok(HeaderMap::from_iter([( + http::header::AUTHORIZATION, + HeaderValue::from_static("Bearer async-auth"), + )])) + }) + } +} + +#[tokio::test] +async fn async_auth_provider_rejects_sync_resolution_and_authenticates_requests() -> Result<()> { + assert!(AsyncHeaderAuth.to_auth_headers().is_err()); + let state = RecordingState::default(); + let client = ResponsesClient::new( + RecordingTransport::new(state.clone()), + provider("openai"), + Arc::new(AsyncHeaderAuth), + ); + let _stream = client + .stream( + serde_json::json!({"echo": true}), + HeaderMap::new(), + Compression::None, + /*turn_state*/ None, + ) + .await?; + let requests = state.take_stream_requests(); + assert_eq!(requests.len(), 1); + assert_eq!( + requests[0].headers.get(http::header::AUTHORIZATION), + Some(&HeaderValue::from_static("Bearer async-auth")) + ); + Ok(()) +} diff --git a/codex-rs/codex-api/tests/models_integration.rs b/codex-rs/codex-api/tests/models_integration.rs index ed43d8aaed..b284a8db6f 100644 --- a/codex-rs/codex-api/tests/models_integration.rs +++ b/codex-rs/codex-api/tests/models_integration.rs @@ -28,7 +28,9 @@ use wiremock::matchers::path; struct DummyAuth; impl AuthProvider for DummyAuth { - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { + Ok(()) + } } fn provider(base_url: &str) -> Provider { diff --git a/codex-rs/codex-api/tests/sse_end_to_end.rs b/codex-rs/codex-api/tests/sse_end_to_end.rs index 5c1dcb8ee5..bf72eda9c0 100644 --- a/codex-rs/codex-api/tests/sse_end_to_end.rs +++ b/codex-rs/codex-api/tests/sse_end_to_end.rs @@ -53,7 +53,9 @@ impl HttpTransport for FixtureSseTransport { struct NoAuth; impl AuthProvider for NoAuth { - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { + Ok(()) + } } fn provider(name: &str) -> Provider { diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 7f69637bf5..b2f35edb68 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -4545,7 +4545,8 @@ fn hosted_actor_credentials_are_only_available_to_host_owned_mcp_servers() { let bootstrap_auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let mut actor_headers = Default::default(); codex_model_provider::auth_provider_from_auth(&bootstrap_auth) - .add_auth_headers(&mut actor_headers); + .add_auth_headers(&mut actor_headers) + .unwrap(); actor_headers.insert( "x-openai-actor-authorization", "hosted-actor-secret" @@ -4568,6 +4569,7 @@ fn hosted_actor_credentials_are_only_available_to_host_owned_mcp_servers() { assert_eq!( local_provider .to_auth_headers() + .unwrap() .get("x-openai-actor-authorization") .and_then(|value| value.to_str().ok()), Some("hosted-actor-secret") @@ -4654,7 +4656,8 @@ async fn executor_owned_chatgpt_mcp_accepts_only_safe_explicit_authorization() - let bootstrap_auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let mut actor_headers = Default::default(); codex_model_provider::auth_provider_from_auth(&bootstrap_auth) - .add_auth_headers(&mut actor_headers); + .add_auth_headers(&mut actor_headers) + .unwrap(); actor_headers.insert( "x-openai-actor-authorization", "hosted-actor-secret" diff --git a/codex-rs/codex-mcp/src/trusted_access.rs b/codex-rs/codex-mcp/src/trusted_access.rs index baed4690cb..4191b127b1 100644 --- a/codex-rs/codex-mcp/src/trusted_access.rs +++ b/codex-rs/codex-mcp/src/trusted_access.rs @@ -205,6 +205,7 @@ impl TrustedAccessContext { let headers = codex_model_provider::auth_provider_from_auth(&auth) .to_auth_headers() + .ok()? .iter() .map(|(name, value)| { Some(HttpHeader { diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index ef2d91ef1f..f94e942966 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -1043,7 +1043,7 @@ pub async fn fetch_recommended_plugins( config.http_request(Method::GET, &url), auth, &config.product_sku, - ) + )? .timeout(RECOMMENDED_PLUGINS_TIMEOUT); let response: RecommendedPluginsResponse = send_and_decode(request, &url).await?; Ok(recommended_plugins_mode(response)) @@ -1415,7 +1415,7 @@ pub async fn fetch_remote_plugin_skill_detail( config.http_request(Method::GET, &url), auth, &config.product_sku, - ); + )?; let response: RemotePluginSkillDetailResponse = send_and_decode(request, &url).await?; if response.plugin_id != plugin_id { return Err(RemotePluginCatalogError::UnexpectedPluginId { @@ -1626,7 +1626,7 @@ async fn install_remote_plugin_inner( config.http_request(Method::POST, &url), auth, &config.product_sku, - ); + )?; let request = if let Some(install_attempt_id) = install_attempt_id { request.json(&RemotePluginInstallRequest { install_attempt_id }) } else { @@ -1723,7 +1723,7 @@ pub async fn uninstall_remote_plugin( config.http_request(Method::POST, &url), auth, &config.product_sku, - ); + )?; let response: RemotePluginMutationResponse = send_and_decode(request, &url).await?; if response.id != remote_plugin_id { return Err(RemotePluginCatalogError::UnexpectedPluginId { @@ -2221,7 +2221,7 @@ async fn get_remote_plugin_list_page( config.http_request(Method::GET, &url), auth, &config.product_sku, - ); + )?; send_and_decode(request, &url).await } @@ -2243,7 +2243,7 @@ async fn get_remote_shared_workspace_plugins_page( config.http_request(Method::GET, &url), auth, &config.product_sku, - ); + )?; send_and_decode(request, &url).await } @@ -2279,7 +2279,7 @@ async fn get_remote_plugin_installed_page( config.http_request(Method::GET, &url), auth, &config.product_sku, - ); + )?; send_and_decode(request, &url).await } @@ -2318,7 +2318,7 @@ async fn fetch_plugin_detail_with_timeout( config.http_request(Method::GET, &url), auth, &config.product_sku, - ) + )? .timeout(timeout); send_and_decode(request, &url).await } @@ -2358,11 +2358,16 @@ fn authenticated_request( request: RequestBuilder, auth: &CodexAuth, product_sku: &str, -) -> RequestBuilder { - request +) -> Result { + Ok(request .timeout(REMOTE_PLUGIN_CATALOG_TIMEOUT) - .headers(codex_model_provider::auth_provider_from_auth(auth).to_auth_headers()) - .header(OAI_PRODUCT_SKU_HEADER, product_sku) + .headers( + codex_model_provider::auth_provider_from_auth(auth) + .to_auth_headers() + .map_err(std::io::Error::other) + .map_err(RemotePluginCatalogError::AuthToken)?, + ) + .header(OAI_PRODUCT_SKU_HEADER, product_sku)) } async fn send_and_decode Deserialize<'de>>( diff --git a/codex-rs/core-plugins/src/remote/search.rs b/codex-rs/core-plugins/src/remote/search.rs index f4a6021c28..71a56f9425 100644 --- a/codex-rs/core-plugins/src/remote/search.rs +++ b/codex-rs/core-plugins/src/remote/search.rs @@ -64,7 +64,7 @@ pub async fn search_remote_plugins( config.http_request(Method::GET, &url), auth, &config.product_sku, - ); + )?; let response: RemotePluginListResponse = send_and_decode(request, &url_for_error) .await .map_err(|error| match error { diff --git a/codex-rs/core-plugins/src/remote/share.rs b/codex-rs/core-plugins/src/remote/share.rs index 1e499aa9bb..f5aa54937a 100644 --- a/codex-rs/core-plugins/src/remote/share.rs +++ b/codex-rs/core-plugins/src/remote/share.rs @@ -289,7 +289,7 @@ pub async fn delete_remote_plugin_share( config.http_request(Method::DELETE, &url), auth, &config.product_sku, - ); + )?; send_and_expect_status(request, &url, &[StatusCode::NO_CONTENT]).await?; if let Err(err) = local_paths::remove_plugin_share_local_path(codex_home, remote_plugin_id) { warn!( @@ -326,7 +326,7 @@ pub async fn update_remote_plugin_share_targets( config.http_request(Method::PUT, &url), auth, &config.product_sku, - ) + )? .json(&RemotePluginShareUpdateTargetsRequest { discoverability, targets, @@ -401,7 +401,7 @@ async fn get_created_workspace_plugins_page( config.http_request(Method::GET, &url), auth, &config.product_sku, - ); + )?; send_and_decode(request, &url).await } @@ -418,7 +418,7 @@ async fn create_workspace_plugin_upload( config.http_request(Method::POST, &url), auth, &config.product_sku, - ) + )? .json(&RemoteWorkspacePluginUploadUrlRequest { filename, mime_type: "application/gzip", @@ -480,7 +480,7 @@ async fn finalize_workspace_plugin_upload( config.http_request(Method::POST, &url), auth, &config.product_sku, - ) + )? .json(&body); send_and_decode(request, &url).await } diff --git a/codex-rs/core-plugins/src/remote_legacy.rs b/codex-rs/core-plugins/src/remote_legacy.rs index 65f1c04efd..846634358b 100644 --- a/codex-rs/core-plugins/src/remote_legacy.rs +++ b/codex-rs/core-plugins/src/remote_legacy.rs @@ -95,6 +95,8 @@ impl RemotePluginMutationError { #[derive(Debug, thiserror::Error)] pub enum RemotePluginFetchError { + #[error("failed to authenticate remote featured plugin request: {0}")] + AuthToken(#[source] std::io::Error), #[error("invalid chatgpt base url for remote featured plugin request: {0}")] InvalidBaseUrl(#[source] url::ParseError), @@ -138,8 +140,12 @@ pub async fn fetch_remote_featured_plugin_ids( .timeout(REMOTE_FEATURED_PLUGIN_FETCH_TIMEOUT); if let Some(auth) = auth.filter(|auth| auth.uses_codex_backend()) { - request = - request.headers(codex_model_provider::auth_provider_from_auth(auth).to_auth_headers()); + request = request.headers( + codex_model_provider::auth_provider_from_auth(auth) + .to_auth_headers() + .map_err(std::io::Error::other) + .map_err(RemotePluginFetchError::AuthToken)?, + ); } let response = request @@ -202,7 +208,12 @@ async fn post_remote_plugin_mutation( let request = config .http_request(Method::POST, &url) .timeout(REMOTE_PLUGIN_MUTATION_TIMEOUT) - .headers(codex_model_provider::auth_provider_from_auth(auth).to_auth_headers()); + .headers( + codex_model_provider::auth_provider_from_auth(auth) + .to_auth_headers() + .map_err(std::io::Error::other) + .map_err(RemotePluginMutationError::AuthToken)?, + ); let response = request .send() diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index a63641a4d7..815ab10a7d 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -527,10 +527,12 @@ pub(crate) struct RealtimeWebrtcCallStart { /// API-key sessions send that API bearer. ChatGPT-auth sessions send their bearer plus account id; /// transceiver is responsible for accepting that same call-create identity on the direct /// `api.openai.com` sideband path. -fn sideband_websocket_auth_headers(api_auth: &dyn AuthProvider) -> ApiHeaderMap { +fn sideband_websocket_auth_headers( + api_auth: &dyn AuthProvider, +) -> std::result::Result { let mut headers = ApiHeaderMap::new(); - api_auth.add_auth_headers(&mut headers); - headers + api_auth.add_auth_headers(&mut headers)?; + Ok(headers) } impl ModelClient { @@ -771,9 +773,10 @@ impl ModelClient { extra_headers.insert(X_OAI_ATTESTATION_HEADER, header_value); } let mut sideband_headers = extra_headers.clone(); - sideband_headers.extend(sideband_websocket_auth_headers( - client_setup.api_auth.as_ref(), - )); + sideband_headers.extend( + sideband_websocket_auth_headers(client_setup.api_auth.as_ref()) + .map_err(|error| self.state.provider.map_api_error(error))?, + ); let api_provider = api_provider_override.unwrap_or(client_setup.api_provider); let transport = self.build_api_transport( &api_provider, @@ -801,9 +804,10 @@ impl ModelClient { if let Some(header_value) = self.generate_attestation_header_for().await { extra_headers.insert(X_OAI_ATTESTATION_HEADER, header_value); } - extra_headers.extend(sideband_websocket_auth_headers( - client_setup.api_auth.as_ref(), - )); + extra_headers.extend( + sideband_websocket_auth_headers(client_setup.api_auth.as_ref()) + .map_err(|error| self.state.provider.map_api_error(error))?, + ); Ok(extra_headers) } @@ -839,7 +843,8 @@ impl ModelClient { client_setup.api_auth.as_ref(), client_setup.agent_identity_telemetry.clone(), PendingUnauthorizedRetry::default(), - ), + ) + .map_err(|error| self.state.provider.map_api_error(error))?, RequestRouteTelemetry::for_endpoint(MEMORIES_SUMMARIZE_ENDPOINT), self.state.auth_env_telemetry.clone(), ); @@ -1619,7 +1624,8 @@ impl ModelClientSession { client_setup.api_auth.as_ref(), client_setup.agent_identity_telemetry.clone(), pending_retry, - ); + ) + .map_err(|error| provider.map_api_error(error))?; let responses_headers = self .client .responses_headers(client_setup.auth.as_ref(), &model_info.slug); @@ -1838,7 +1844,8 @@ impl ResponsesHttpExecutor<'_> { client_setup.api_auth.as_ref(), client_setup.agent_identity_telemetry.clone(), pending_retry, - ); + ) + .map_err(|error| self.client.state.provider.map_api_error(error))?; let (request_telemetry, sse_telemetry) = ModelClientSession::build_streaming_telemetry( session_telemetry, request_auth_context, @@ -2029,7 +2036,8 @@ impl ModelClientSession { client_setup.api_auth.as_ref(), client_setup.agent_identity_telemetry.clone(), pending_retry, - ); + ) + .map_err(|error| self.client.state.provider.map_api_error(error))?; let mut request = self.client.build_responses_request( prompt, model_info, @@ -2730,9 +2738,9 @@ impl AuthRequestTelemetryContext { api_auth: &dyn AuthProvider, agent_identity_telemetry: Option, retry: PendingUnauthorizedRetry, - ) -> Self { - let auth_telemetry = auth_header_telemetry(api_auth); - Self { + ) -> std::result::Result { + let auth_telemetry = auth_header_telemetry(api_auth)?; + Ok(Self { auth_mode: auth_mode.map(|mode| match mode { AuthMode::ApiKey | AuthMode::BedrockApiKey | AuthMode::BedrockAccessKeys => { "ApiKey" @@ -2749,7 +2757,7 @@ impl AuthRequestTelemetryContext { retry_after_unauthorized: retry.retry_after_unauthorized, recovery_mode: retry.recovery_mode, recovery_phase: retry.recovery_phase, - } + }) } fn agent_identity_telemetry(&self) -> Option<&AgentIdentityTelemetry> { diff --git a/codex-rs/core/src/client_tests.rs b/codex-rs/core/src/client_tests.rs index b9a23192a7..f929e2c969 100644 --- a/codex-rs/core/src/client_tests.rs +++ b/codex-rs/core/src/client_tests.rs @@ -345,7 +345,7 @@ async fn client_setup_accepts_command_credential_refresh() { let setup = client.current_client_setup(routing).await.unwrap(); let mut headers = http::HeaderMap::new(); - setup.api_auth.add_auth_headers(&mut headers); + setup.api_auth.add_auth_headers(&mut headers).unwrap(); assert_eq!( headers.get(http::header::AUTHORIZATION).unwrap(), "Bearer refreshed-token" @@ -408,7 +408,7 @@ async fn client_setup_rebuilds_chatgpt_refresh_but_rejects_account_switches() { if expected_calls == 2 { let setup = result.unwrap(); let mut headers = http::HeaderMap::new(); - setup.api_auth.add_auth_headers(&mut headers); + setup.api_auth.add_auth_headers(&mut headers).unwrap(); assert_eq!( headers.get(http::header::AUTHORIZATION).unwrap(), &format!("Bearer {refreshed_token}") @@ -1945,7 +1945,8 @@ fn auth_request_telemetry_context_tracks_attached_auth_and_retry_phase() { mode: "managed", phase: "refresh_token", }), - ); + ) + .unwrap(); assert_eq!(auth_context.auth_mode, Some("Chatgpt")); assert!(auth_context.auth_header_attached); @@ -1965,7 +1966,8 @@ fn auth_request_telemetry_context_tracks_agent_identity_ids() { task_id: "task-run-context".to_string(), }), PendingUnauthorizedRetry::default(), - ); + ) + .unwrap(); assert_eq!( auth_context.agent_identity_telemetry(), diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 74985f3eab..c98ae447cf 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -1622,7 +1622,9 @@ impl NoiseRendezvousConnectProvider for OfflineThenReadyNoiseConnectProvider { struct NoopRegistryAuthProvider; impl AuthProvider for NoopRegistryAuthProvider { - fn add_auth_headers(&self, _: &mut HeaderMap) {} + fn add_auth_headers(&self, _: &mut HeaderMap) -> Result<(), codex_api::AuthError> { + Ok(()) + } } async fn wait_for_response_request_count(response_mock: &ResponseMock, expected_count: usize) { diff --git a/codex-rs/exec-server/src/remote.rs b/codex-rs/exec-server/src/remote.rs index 90eea34806..ebb3fa60de 100644 --- a/codex-rs/exec-server/src/remote.rs +++ b/codex-rs/exec-server/src/remote.rs @@ -563,7 +563,7 @@ impl std::fmt::Debug for StaticBearerAuthProvider { } impl AuthProvider for StaticBearerAuthProvider { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { headers.insert(http::header::AUTHORIZATION, self.authorization.clone()); if let Some(chatgpt_account_id) = &self.chatgpt_account_id { headers.insert( @@ -571,6 +571,7 @@ impl AuthProvider for StaticBearerAuthProvider { chatgpt_account_id.clone(), ); } + Ok(()) } } @@ -1133,7 +1134,9 @@ mod tests { struct StaticRegistryAuthProvider; impl AuthProvider for StaticRegistryAuthProvider { - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { + Ok(()) + } fn resolve_auth_headers(&self) -> codex_api::AuthHeadersFuture<'_> { Box::pin(async { @@ -1158,11 +1161,12 @@ mod tests { struct ScopedExecutorAuth; impl AuthProvider for ScopedExecutorAuth { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { headers.insert( AUTHORIZATION, HeaderValue::from_static("Bearer synthetic-scoped-token"), ); + Ok(()) } } @@ -1171,8 +1175,12 @@ mod tests { struct InvalidExecutorAuth(HeaderMap); impl AuthProvider for InvalidExecutorAuth { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers( + &self, + headers: &mut HeaderMap, + ) -> Result<(), codex_api::AuthError> { headers.extend(self.0.clone()); + Ok(()) } } @@ -1433,11 +1441,15 @@ mod tests { const TOKEN: &str = "synthetic-scoped-registry-secret"; struct ExecutorAuth; impl AuthProvider for ExecutorAuth { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers( + &self, + headers: &mut HeaderMap, + ) -> Result<(), codex_api::AuthError> { let mut credential = HeaderValue::from_str(&format!("Bearer {TOKEN}")).expect("bearer"); credential.set_sensitive(true); headers.insert(AUTHORIZATION, credential); + Ok(()) } } let encoded_echo = serde_json::json!({ diff --git a/codex-rs/exec-server/src/remote/direct.rs b/codex-rs/exec-server/src/remote/direct.rs index 99b0a2aa78..f6155afe89 100644 --- a/codex-rs/exec-server/src/remote/direct.rs +++ b/codex-rs/exec-server/src/remote/direct.rs @@ -331,6 +331,7 @@ fn websocket_diagnostic_url(url: &str) -> String { fn direct_auth_error(error: AuthError) -> ExecServerError { let message = format!("failed to resolve environment registry authentication: {error}"); match error { + AuthError::Policy(denied) => ExecServerError::ApplicationNetworkPolicy(denied), AuthError::Build(_) => ExecServerError::EnvironmentRegistryAuth(message), AuthError::Transient(_) => ExecServerError::Disconnected(message), } diff --git a/codex-rs/exec-server/src/remote/direct_tests.rs b/codex-rs/exec-server/src/remote/direct_tests.rs index 9c49254824..fd354803df 100644 --- a/codex-rs/exec-server/src/remote/direct_tests.rs +++ b/codex-rs/exec-server/src/remote/direct_tests.rs @@ -39,11 +39,12 @@ use crate::RemoteEnvironmentTransport; struct StaticAuthProvider; impl AuthProvider for StaticAuthProvider { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { headers.insert( http::header::AUTHORIZATION, HeaderValue::from_static("AWS4-HMAC-SHA256 test-signature"), ); + Ok(()) } fn apply_auth( @@ -56,7 +57,7 @@ impl AuthProvider for StaticAuthProvider { assert!(request.headers.contains_key(http::header::HOST)); assert!(request.url.starts_with("http")); } - self.add_auth_headers(&mut request.headers); + self.add_auth_headers(&mut request.headers)?; Ok(request) }) } @@ -159,7 +160,9 @@ async fn direct_environment_accepts_large_file_writes() -> Result<()> { } impl AuthProvider for QueryAuthProvider { - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { + Ok(()) + } fn apply_auth( &self, @@ -178,11 +181,12 @@ impl AuthProvider for QueryAuthProvider { struct ScopedExecutorAuth; impl AuthProvider for ScopedExecutorAuth { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { headers.insert( http::header::AUTHORIZATION, HeaderValue::from_static("Bearer synthetic-scoped-token"), ); + Ok(()) } } diff --git a/codex-rs/exec-server/src/remote/noise_tests.rs b/codex-rs/exec-server/src/remote/noise_tests.rs index d6d5057336..27640300f5 100644 --- a/codex-rs/exec-server/src/remote/noise_tests.rs +++ b/codex-rs/exec-server/src/remote/noise_tests.rs @@ -35,11 +35,12 @@ const HARNESS_KEY_AUTHORIZATION: &str = "authorization-that-must-not-leak"; struct StaticRegistryAuthProvider; impl AuthProvider for StaticRegistryAuthProvider { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { let _ = headers.insert( http::header::AUTHORIZATION, HeaderValue::from_static("Bearer registry-token"), ); + Ok(()) } } diff --git a/codex-rs/exec-server/src/remote/registration_retry_tests.rs b/codex-rs/exec-server/src/remote/registration_retry_tests.rs index 1e9fdcd3a2..bae77ef078 100644 --- a/codex-rs/exec-server/src/remote/registration_retry_tests.rs +++ b/codex-rs/exec-server/src/remote/registration_retry_tests.rs @@ -43,7 +43,9 @@ struct RegistrationAuthProvider { } impl AuthProvider for RegistrationAuthProvider { - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { + Ok(()) + } fn resolve_auth_headers(&self) -> codex_api::AuthHeadersFuture<'_> { self.calls.fetch_add(1, Ordering::Relaxed); diff --git a/codex-rs/exec-server/tests/accepted_websocket.rs b/codex-rs/exec-server/tests/accepted_websocket.rs index 99eb447549..c02472bd10 100644 --- a/codex-rs/exec-server/tests/accepted_websocket.rs +++ b/codex-rs/exec-server/tests/accepted_websocket.rs @@ -77,11 +77,12 @@ const SESSION_ALREADY_ATTACHED_ERROR_CODE: i64 = -32010; struct DirectExecutorAuth; impl AuthProvider for DirectExecutorAuth { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { headers.insert( http::header::AUTHORIZATION, HeaderValue::from_static("AWS4-HMAC-SHA256 test-signature"), ); + Ok(()) } } diff --git a/codex-rs/exec-server/tests/common/relay.rs b/codex-rs/exec-server/tests/common/relay.rs index cca0f967a4..d10f06c145 100644 --- a/codex-rs/exec-server/tests/common/relay.rs +++ b/codex-rs/exec-server/tests/common/relay.rs @@ -38,11 +38,12 @@ pub(crate) const REGISTRY_TOKEN: &str = "registry-token"; struct StaticRegistryAuthProvider; impl AuthProvider for StaticRegistryAuthProvider { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { let _ = headers.insert( http::header::AUTHORIZATION, HeaderValue::from_static("Bearer registry-token"), ); + Ok(()) } } diff --git a/codex-rs/model-provider/src/amazon_bedrock/auth.rs b/codex-rs/model-provider/src/amazon_bedrock/auth.rs index 4fb04ff816..07a6612140 100644 --- a/codex-rs/model-provider/src/amazon_bedrock/auth.rs +++ b/codex-rs/model-provider/src/amazon_bedrock/auth.rs @@ -342,7 +342,9 @@ impl BedrockSigV4AuthProvider { } impl AuthProvider for BedrockSigV4AuthProvider { - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> std::result::Result<(), AuthError> { + Ok(()) + } fn apply_auth(&self, request: Request) -> codex_api::AuthProviderFuture<'_> { Box::pin(BedrockSigV4AuthProvider::apply_auth(self, request)) @@ -513,7 +515,7 @@ mod tests { }; let mut headers = http::HeaderMap::new(); - provider.add_auth_headers(&mut headers); + provider.add_auth_headers(&mut headers).unwrap(); assert_eq!(region, "us-west-2"); assert!( diff --git a/codex-rs/model-provider/src/amazon_bedrock/mod.rs b/codex-rs/model-provider/src/amazon_bedrock/mod.rs index 21ab79ce06..d18aa4bd56 100644 --- a/codex-rs/model-provider/src/amazon_bedrock/mod.rs +++ b/codex-rs/model-provider/src/amazon_bedrock/mod.rs @@ -609,6 +609,7 @@ mod tests { .await .expect("managed Bedrock auth should resolve") .to_auth_headers() + .unwrap() .get(http::header::AUTHORIZATION), Some(&HeaderValue::from_static("Bearer managed-bedrock-api-key")) ); @@ -655,6 +656,7 @@ mod tests { .await .expect("configured AWS profile auth should resolve") .to_auth_headers() + .unwrap() .get(http::header::AUTHORIZATION) .is_none() ); diff --git a/codex-rs/model-provider/src/auth.rs b/codex-rs/model-provider/src/auth.rs index f4d276465d..38430cf978 100644 --- a/codex-rs/model-provider/src/auth.rs +++ b/codex-rs/model-provider/src/auth.rs @@ -85,7 +85,7 @@ struct AgentIdentityAuthProvider { } impl AuthProvider for AgentIdentityAuthProvider { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), AuthError> { let record = self.auth.record(); let header_value = authorization_header_for_agent_task( AgentIdentityKey { @@ -109,6 +109,7 @@ impl AuthProvider for AgentIdentityAuthProvider { if self.auth.is_fedramp_account() { let _ = headers.insert("X-OpenAI-Fedramp", HeaderValue::from_static("true")); } + Ok(()) } } @@ -118,8 +119,9 @@ struct HeaderAuthProvider { } impl AuthProvider for HeaderAuthProvider { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), AuthError> { headers.extend(self.auth.headers().clone()); + Ok(()) } } @@ -146,11 +148,11 @@ impl AuthManagerAuthProvider { } impl AuthProvider for AuthManagerAuthProvider { - fn add_auth_headers(&self, headers: &mut HeaderMap) { - let Some(auth) = self.current_auth() else { - return; - }; - auth_provider_from_auth(&auth).add_auth_headers(headers); + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), AuthError> { + let auth = self.current_auth().ok_or_else(|| { + AuthError::Transient("managed authentication is unavailable".to_string()) + })?; + auth_provider_from_auth(&auth).add_auth_headers(headers) } fn resolve_auth_headers(&self) -> AuthHeadersFuture<'_> { @@ -163,7 +165,7 @@ impl AuthProvider for AuthManagerAuthProvider { .ok_or_else(|| { AuthError::Transient("managed authentication is unavailable".to_string()) })?; - Ok(auth_provider_from_auth(&auth).to_auth_headers()) + auth_provider_from_auth(&auth).to_auth_headers() }) } } @@ -174,7 +176,9 @@ impl AuthProvider for AuthManagerAuthProvider { struct UnauthenticatedAuthProvider; impl AuthProvider for UnauthenticatedAuthProvider { - fn add_auth_headers(&self, _headers: &mut HeaderMap) {} + fn add_auth_headers(&self, _headers: &mut HeaderMap) -> Result<(), AuthError> { + Ok(()) + } } pub fn unauthenticated_auth_provider() -> SharedAuthProvider { @@ -474,7 +478,7 @@ mod tests { create_oss_provider_with_base_url("http://localhost:11434/v1", WireApi::Responses); let auth = resolve_provider_auth(/*auth*/ None, &provider).expect("auth should resolve"); - assert!(auth.to_auth_headers().is_empty()); + assert!(auth.to_auth_headers().unwrap().is_empty()); } #[test] @@ -495,7 +499,7 @@ mod tests { let auth = resolve_provider_auth(Some(&ambient_auth), &provider).expect("auth should resolve"); - assert!(auth.to_auth_headers().is_empty()); + assert!(auth.to_auth_headers().unwrap().is_empty()); } #[test] @@ -510,7 +514,7 @@ mod tests { let auth = resolve_provider_auth(Some(&ambient_auth), &provider).expect("auth should resolve"); - assert!(auth.to_auth_headers().is_empty()); + assert!(auth.to_auth_headers().unwrap().is_empty()); } #[test] @@ -525,7 +529,8 @@ mod tests { let headers = resolve_provider_auth(Some(&ambient_auth), &provider) .expect("auth should resolve") - .to_auth_headers(); + .to_auth_headers() + .unwrap(); assert_eq!( headers.get(AUTHORIZATION), @@ -552,7 +557,8 @@ mod tests { let headers = resolve_provider_auth(Some(&command_auth), &provider) .expect("auth should resolve") - .to_auth_headers(); + .to_auth_headers() + .unwrap(); assert_eq!( headers.get(AUTHORIZATION), @@ -577,7 +583,7 @@ mod tests { let auth = resolve_provider_auth(Some(&ambient_auth), &provider).expect("auth should resolve"); - assert_eq!(auth.to_auth_headers(), expected); + assert_eq!(auth.to_auth_headers().unwrap(), expected); } #[test] @@ -590,7 +596,7 @@ mod tests { expected.insert("x-external-auth", HeaderValue::from_static("enabled")); let auth = CodexAuth::Headers(AuthHeaders::new(expected.clone())); - let actual = auth_provider_from_auth(&auth).to_auth_headers(); + let actual = auth_provider_from_auth(&auth).to_auth_headers().unwrap(); assert_eq!(actual, expected); } @@ -642,7 +648,7 @@ mod tests { let provider = auth_provider_from_auth_manager(Arc::clone(&auth_manager), &expected_auth); assert_eq!( - provider.to_auth_headers().get(AUTHORIZATION), + provider.to_auth_headers().unwrap().get(AUTHORIZATION), Some(&HeaderValue::from_static("Bearer header.e30.first")) ); @@ -673,7 +679,14 @@ mod tests { .expect("save switched-account auth"); auth_manager.reload().await; - assert!(provider.to_auth_headers().is_empty()); + assert!(matches!( + provider.to_auth_headers(), + Err(AuthError::Transient(_)) + )); + assert!(matches!( + provider.resolve_auth_headers().await, + Err(AuthError::Transient(_)) + )); } #[tokio::test] @@ -702,7 +715,7 @@ mod tests { task_id: "task-run-1".to_string(), }) ); - let headers = auth.auth.to_auth_headers(); + let headers = auth.auth.to_auth_headers().unwrap(); assert!( headers .get(http::header::AUTHORIZATION) @@ -716,7 +729,7 @@ mod tests { let auth = agent_identity_auth(/*chatgpt_account_is_fedramp*/ true).await; let provider = auth_provider_from_auth(&CodexAuth::AgentIdentity(auth)); - let headers = provider.to_auth_headers(); + let headers = provider.to_auth_headers().unwrap(); assert!( headers @@ -761,7 +774,7 @@ mod tests { .await .expect("fallback should resolve bearer auth"); - let headers = provider_auth.auth.to_auth_headers(); + let headers = provider_auth.auth.to_auth_headers().unwrap(); assert_eq!( headers .get(http::header::AUTHORIZATION) diff --git a/codex-rs/model-provider/src/bearer_auth_provider.rs b/codex-rs/model-provider/src/bearer_auth_provider.rs index a28e069227..616d122d45 100644 --- a/codex-rs/model-provider/src/bearer_auth_provider.rs +++ b/codex-rs/model-provider/src/bearer_auth_provider.rs @@ -29,7 +29,7 @@ impl BearerAuthProvider { } impl AuthProvider for BearerAuthProvider { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), codex_api::AuthError> { if let Some(token) = self.token.as_ref() && let Ok(header) = HeaderValue::from_str(&format!("Bearer {token}")) { @@ -43,6 +43,7 @@ impl AuthProvider for BearerAuthProvider { if self.is_fedramp_account { let _ = headers.insert("X-OpenAI-Fedramp", HeaderValue::from_static("true")); } + Ok(()) } } @@ -60,7 +61,7 @@ mod tests { }; assert_eq!( - codex_api::auth_header_telemetry(&auth), + codex_api::auth_header_telemetry(&auth).unwrap(), codex_api::AuthHeaderTelemetry { attached: true, name: Some("authorization"), @@ -73,7 +74,7 @@ mod tests { let auth = BearerAuthProvider::for_test(Some("access-token"), Some("workspace-123")); let mut headers = HeaderMap::new(); - auth.add_auth_headers(&mut headers); + auth.add_auth_headers(&mut headers).unwrap(); assert_eq!( headers @@ -98,7 +99,7 @@ mod tests { }; let mut headers = HeaderMap::new(); - auth.add_auth_headers(&mut headers); + auth.add_auth_headers(&mut headers).unwrap(); assert_eq!( headers diff --git a/codex-rs/model-provider/src/combined_auth.rs b/codex-rs/model-provider/src/combined_auth.rs index 513d82836c..24dd9c5c81 100644 --- a/codex-rs/model-provider/src/combined_auth.rs +++ b/codex-rs/model-provider/src/combined_auth.rs @@ -49,7 +49,12 @@ pub(crate) async fn compose_auth( )) })?; let (name, value) = gateway_header(config, &token)?; - if resolved.auth.to_auth_headers().contains_key(&name) { + if resolved + .auth + .to_auth_headers() + .map_err(|error| codex_api::map_api_error(error.into()))? + .contains_key(&name) + { return Err(CodexErr::InvalidRequest( "gateway OAuth conflicts with primary auth headers".into(), )); @@ -99,11 +104,12 @@ struct CombinedAuth { } impl AuthProvider for CombinedAuth { - fn add_auth_headers(&self, headers: &mut HeaderMap) { - self.primary.add_auth_headers(headers); + fn add_auth_headers(&self, headers: &mut HeaderMap) -> std::result::Result<(), AuthError> { + self.primary.add_auth_headers(headers)?; headers .entry(self.name.clone()) .or_insert(self.value.clone()); + Ok(()) } fn resolve_auth_headers(&self) -> AuthHeadersFuture<'_> { diff --git a/codex-rs/model-provider/src/models_endpoint.rs b/codex-rs/model-provider/src/models_endpoint.rs index 780ae934bb..9734a38abf 100644 --- a/codex-rs/model-provider/src/models_endpoint.rs +++ b/codex-rs/model-provider/src/models_endpoint.rs @@ -134,7 +134,8 @@ impl OpenAiModelsEndpoint { .map_err(map_api_error)?, None => ModelsClient::::request_url(&api_provider, client_version), }; - let auth_telemetry = auth_header_telemetry(api_auth.as_ref()); + let auth_telemetry = auth_header_telemetry(api_auth.as_ref()) + .map_err(|error| map_api_error(error.into()))?; let agent_identity_telemetry = if let Some(CodexAuth::AgentIdentity(auth)) = auth.as_ref() { Some(agent_identity_telemetry(auth)) } else { diff --git a/codex-rs/model-provider/src/models_identity.rs b/codex-rs/model-provider/src/models_identity.rs index 06cafacb86..713064a24c 100644 --- a/codex-rs/model-provider/src/models_identity.rs +++ b/codex-rs/model-provider/src/models_identity.rs @@ -76,7 +76,9 @@ pub(crate) fn identity( // different account, so reuse requires stable owner metadata. Explicit provider // keys take precedence over the header auth's account identity. if !has_stable_account || explicit_bearer { - let headers = resolve_provider_auth(auth, provider_info)?.to_auth_headers(); + let headers = resolve_provider_auth(auth, provider_info)? + .to_auth_headers() + .map_err(|error| codex_api::map_api_error(error.into()))?; api_provider.headers.extend(headers); } let mut headers: Vec<_> = api_provider.headers.iter().collect(); diff --git a/codex-rs/model-provider/src/provider.rs b/codex-rs/model-provider/src/provider.rs index a720bcad25..a5263a8523 100644 --- a/codex-rs/model-provider/src/provider.rs +++ b/codex-rs/model-provider/src/provider.rs @@ -714,7 +714,7 @@ mod tests { .await .expect("auth should resolve"); - assert!(auth.auth.to_auth_headers().is_empty()); + assert!(auth.auth.to_auth_headers().unwrap().is_empty()); } #[test] diff --git a/codex-rs/model-provider/src/shared_state_tests.rs b/codex-rs/model-provider/src/shared_state_tests.rs index e58edd0ef8..e150e31625 100644 --- a/codex-rs/model-provider/src/shared_state_tests.rs +++ b/codex-rs/model-provider/src/shared_state_tests.rs @@ -131,10 +131,10 @@ async fn gateway_credentials_accompany_primary_auth_in_models_and_responses() { HeaderValue::from_static(value), ); // WebSocket handshakes use the synchronous header interface. - assert_eq!(auth.to_auth_headers(), expected); + assert_eq!(auth.to_auth_headers().unwrap(), expected); assert_eq!(auth.resolve_auth_headers().await.unwrap(), expected); - assert_eq!(scoped.auth.to_auth_headers(), expected); - assert!(auth.to_auth_headers()[name].is_sensitive()); + assert_eq!(scoped.auth.to_auth_headers().unwrap(), expected); + assert!(auth.to_auth_headers().unwrap()[name].is_sensitive()); let api = info.to_api_provider(/*auth_mode*/ None).unwrap(); let transport = ReqwestTransport::from_http_client( create_client_for_route( @@ -219,7 +219,7 @@ async fn gateway_refresh_preserves_primary_auth_and_hides_issuer_errors() { let result = provider.api_auth().await; if succeeds { assert_eq!( - result.unwrap().to_auth_headers(), + result.unwrap().to_auth_headers().unwrap(), HeaderMap::from_iter([ ( http::header::AUTHORIZATION, @@ -407,7 +407,8 @@ async fn models_observe_gateway_rotation(instance: ProviderInstance) { .api_auth() .await .unwrap() - .to_auth_headers(), + .to_auth_headers() + .unwrap(), HeaderMap::from_iter([ ( http::header::AUTHORIZATION, @@ -442,7 +443,8 @@ async fn models_observe_gateway_rotation(instance: ProviderInstance) { .api_auth() .await .unwrap() - .to_auth_headers(), + .to_auth_headers() + .unwrap(), HeaderMap::from_iter([ ( http::header::AUTHORIZATION, diff --git a/codex-rs/rmcp-client/src/http_client_adapter.rs b/codex-rs/rmcp-client/src/http_client_adapter.rs index aa1239e0a4..d6c6b02c3c 100644 --- a/codex-rs/rmcp-client/src/http_client_adapter.rs +++ b/codex-rs/rmcp-client/src/http_client_adapter.rs @@ -102,6 +102,8 @@ impl Drop for EventStreamCancellation { #[derive(Debug, thiserror::Error)] pub(crate) enum StreamableHttpClientAdapterError { + #[error(transparent)] + Auth(#[from] codex_api::AuthError), #[error("streamable HTTP session expired with 404 Not Found")] SessionExpired404, #[error(transparent)] @@ -148,11 +150,24 @@ impl StreamableHttpClient for StreamableHttpClientAdapter { auth_token: Option, custom_headers: HashMap, ) -> std::result::Result> { + if let JsonRpcMessage::Notification(notification) = &message + && let ClientNotification::CancelledNotification(cancelled) = ¬ification.notification + && let Some(request_id) = cancelled.params.request_id.as_ref() + && let Some(cancellation) = self + .event_stream_cancellations + .lock() + .unwrap_or_else(PoisonError::into_inner) + .remove(request_id) + { + let _ = cancellation.send(()); + return Ok(StreamableHttpPostResponse::Accepted); + } + let (mcp_method, mcp_request_id) = client_jsonrpc_message_fields(&message); let has_session_id = session_id.is_some(); let mut headers = self.default_headers.clone(); headers.extend(custom_headers); - self.add_auth_headers(&mut headers); + self.add_auth_headers(&mut headers)?; insert_header( &mut headers, ACCEPT, @@ -223,18 +238,6 @@ impl StreamableHttpClient for StreamableHttpClientAdapter { let request_span = crate::trace_context::request_span(&body); let body = serde_json::to_vec(&body).map_err(StreamableHttpError::Deserialize)?; let has_authorization_header = headers.contains_key(AUTHORIZATION); - if let JsonRpcMessage::Notification(notification) = &message - && let ClientNotification::CancelledNotification(cancelled) = ¬ification.notification - && let Some(request_id) = cancelled.params.request_id.as_ref() - && let Some(cancellation) = self - .event_stream_cancellations - .lock() - .unwrap_or_else(PoisonError::into_inner) - .remove(request_id) - { - let _ = cancellation.send(()); - return Ok(StreamableHttpPostResponse::Accepted); - } let request = self .http_client @@ -461,7 +464,7 @@ impl StreamableHttpClient for StreamableHttpClientAdapter { ) -> std::result::Result<(), StreamableHttpError> { let mut headers = self.default_headers.clone(); headers.extend(custom_headers); - self.add_auth_headers(&mut headers); + self.add_auth_headers(&mut headers)?; if let Some(auth_token) = auth_token { insert_header( &mut headers, @@ -518,7 +521,7 @@ impl StreamableHttpClient for StreamableHttpClientAdapter { > { let mut headers = self.default_headers.clone(); headers.extend(custom_headers); - self.add_auth_headers(&mut headers); + self.add_auth_headers(&mut headers)?; insert_header( &mut headers, ACCEPT, @@ -603,10 +606,19 @@ impl StreamableHttpClient for StreamableHttpClientAdapter { } impl StreamableHttpClientAdapter { - fn add_auth_headers(&self, headers: &mut HeaderMap) { + fn add_auth_headers( + &self, + headers: &mut HeaderMap, + ) -> Result<(), StreamableHttpError> { if let Some(auth_provider) = &self.auth_provider { - headers.extend(auth_provider.to_auth_headers()); + headers.extend( + auth_provider + .to_auth_headers() + .map_err(StreamableHttpClientAdapterError::from) + .map_err(StreamableHttpError::Client)?, + ); } + Ok(()) } } diff --git a/codex-rs/rmcp-client/src/streamable_http_retry.rs b/codex-rs/rmcp-client/src/streamable_http_retry.rs index 66dda2cb59..db6aad4261 100644 --- a/codex-rs/rmcp-client/src/streamable_http_retry.rs +++ b/codex-rs/rmcp-client/src/streamable_http_retry.rs @@ -152,6 +152,9 @@ impl RmcpClient { error: &StreamableHttpError, ) -> bool { match error { + StreamableHttpError::Client(StreamableHttpClientAdapterError::Auth( + codex_api::AuthError::Transient(_), + )) => true, StreamableHttpError::Client(StreamableHttpClientAdapterError::HttpRequest( ExecServerError::HttpRequest(_), )) => true, @@ -172,6 +175,9 @@ impl RmcpClient { | StreamableHttpError::UnexpectedContentType(_) | StreamableHttpError::ServerDoesNotSupportSse | StreamableHttpError::Deserialize(_) + | StreamableHttpError::Client(StreamableHttpClientAdapterError::Auth( + codex_api::AuthError::Build(_), + )) | StreamableHttpError::Client(StreamableHttpClientAdapterError::SessionExpired404) | StreamableHttpError::Client(StreamableHttpClientAdapterError::Header(_)) => false, _ => false, diff --git a/codex-rs/rmcp-client/src/streamable_http_retry_tests.rs b/codex-rs/rmcp-client/src/streamable_http_retry_tests.rs index efda30dad6..92847d2a1f 100644 --- a/codex-rs/rmcp-client/src/streamable_http_retry_tests.rs +++ b/codex-rs/rmcp-client/src/streamable_http_retry_tests.rs @@ -29,7 +29,7 @@ fn retryable_initialize_error_includes_discovery_and_initialized_notification_co } #[test] -fn retryable_streamable_http_error_includes_remote_body_stream_failure() { +fn streamable_http_retry_classifies_transport_and_auth_errors() { let errors = [ StreamableHttpError::Client(StreamableHttpClientAdapterError::HttpRequest( ExecServerError::HttpRequest("error sending request for url".to_string()), @@ -53,11 +53,20 @@ fn retryable_streamable_http_error_includes_remote_body_stream_failure() { )), StreamableHttpError::UnexpectedServerResponse("HTTP 502: upstream failure".into()), StreamableHttpError::UnexpectedServerResponse("HTTP 400: bad request".into()), + StreamableHttpError::Client(StreamableHttpClientAdapterError::Auth( + codex_api::AuthError::Transient("credential refresh unavailable".to_string()), + )), + StreamableHttpError::Client(StreamableHttpClientAdapterError::Auth( + codex_api::AuthError::Build("invalid authentication".to_string()), + )), + StreamableHttpError::Client(StreamableHttpClientAdapterError::Auth( + codex_api::AuthError::Policy(codex_http_client::NetworkPolicyDenied::Revoked), + )), ]; assert_eq!( errors.map(|error| RmcpClient::is_retryable_streamable_http_error(&error)), - [true, true, true, false, true, false], + [true, true, true, false, true, false, true, false, false], ); } diff --git a/codex-rs/rmcp-client/tests/mcp_events.rs b/codex-rs/rmcp-client/tests/mcp_events.rs index 3b015a01d2..9d3e54e1ee 100644 --- a/codex-rs/rmcp-client/tests/mcp_events.rs +++ b/codex-rs/rmcp-client/tests/mcp_events.rs @@ -1,6 +1,9 @@ mod streamable_http_test_support; use std::convert::Infallible; +use std::sync::Arc; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; use std::time::Duration; use anyhow::Context as _; @@ -11,8 +14,16 @@ use axum::response::IntoResponse; use axum::response::sse::Event; use axum::response::sse::Sse; use axum::routing::post; +use codex_api::AuthError; +use codex_api::AuthProvider; +use codex_config::types::AuthKeyringBackendKind; +use codex_config::types::OAuthCredentialsStoreMode; +use codex_exec_server::Environment; +use codex_rmcp_client::RmcpClient; use futures::StreamExt as _; use futures::stream; +use http::HeaderMap; +use http::HeaderValue; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; @@ -22,6 +33,27 @@ use tokio::sync::mpsc; use tokio::time::timeout; use streamable_http_test_support::create_client; +use streamable_http_test_support::initialize_client; + +#[derive(Default)] +struct RevocableAuth { + unavailable: AtomicBool, +} + +impl AuthProvider for RevocableAuth { + fn add_auth_headers(&self, headers: &mut HeaderMap) -> Result<(), AuthError> { + if self.unavailable.load(Ordering::SeqCst) { + return Err(AuthError::Transient( + "authentication unavailable".to_string(), + )); + } + headers.insert( + http::header::AUTHORIZATION, + HeaderValue::from_static("Bearer test-bearer"), + ); + Ok(()) + } +} struct StreamClosed { event_name: String, @@ -127,7 +159,20 @@ async fn plugin_runtime_event_streams_are_isolated_and_cancel_locally() -> anyho let listener = TcpListener::bind("127.0.0.1:0").await?; let base_url = format!("http://{}", listener.local_addr()?); let server = tokio::spawn(async move { axum::serve(listener, router).await }); - let client = create_client(&base_url).await?; + let auth = Arc::new(RevocableAuth::default()); + let client = RmcpClient::new_streamable_http_client( + "test-streamable-http", + &format!("{base_url}/mcp"), + /*bearer_token*/ None, + /*http_headers*/ None, + /*env_http_headers*/ None, + OAuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), + Environment::default_for_tests().get_http_client(), + Some(auth.clone()), + ) + .await?; + initialize_client(&client).await?; let mut first = client .send_event_stream_request(Some(json!({ @@ -174,6 +219,8 @@ async fn plugin_runtime_event_streams_are_isolated_and_cancel_locally() -> anyho .context("cancelled stream did not close")?; assert_eq!(closed, "github.pull_request.opened"); + // Local cleanup must still work if authentication fails after the stream starts. + auth.unavailable.store(true, Ordering::SeqCst); second .handle .cancel(Some("event subscription closed".to_string())) diff --git a/codex-rs/tui/src/security_setup.rs b/codex-rs/tui/src/security_setup.rs index 9e48eda944..62e7a7c3d6 100644 --- a/codex-rs/tui/src/security_setup.rs +++ b/codex-rs/tui/src/security_setup.rs @@ -117,7 +117,11 @@ pub(crate) fn prefetch( "{}/wham/security-setup", config.chatgpt_base_url.trim_end_matches('/') )) - .headers(codex_model_provider::auth_provider_from_auth(&auth).to_auth_headers()) + .headers( + codex_model_provider::auth_provider_from_auth(&auth) + .to_auth_headers() + .ok()?, + ) .header( "User-Agent", codex_login::default_client::get_codex_user_agent(),