diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index a1b54e6bcc..c59d29117f 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -1968,7 +1968,17 @@ "type": "string" }, "delivery": { - "$ref": "#/definitions/GatewayOAuthDelivery" + "allOf": [ + { + "$ref": "#/definitions/GatewayOAuthDelivery" + } + ], + "default": { + "kind": "header", + "name": "Authorization", + "scheme": "Bearer" + }, + "description": "Omit to send the gateway token via the Authorization header" }, "redirect_port": { "format": "uint16", @@ -1992,7 +2002,6 @@ "required": [ "authorization_url", "client_id", - "delivery", "token_url" ], "type": "object" diff --git a/codex-rs/core/tests/suite/gateway_auth.rs b/codex-rs/core/tests/suite/gateway_auth.rs index 8b40dbfaba..dcd56e256b 100644 --- a/codex-rs/core/tests/suite/gateway_auth.rs +++ b/codex-rs/core/tests/suite/gateway_auth.rs @@ -12,13 +12,12 @@ use codex_model_provider::AgentIdentitySessionFallback; use codex_model_provider::ProviderAuthScope; use codex_model_provider::create_model_provider; use codex_model_provider::test_support::seed_gateway_auth; -use codex_model_provider_info::GatewayOAuthConfig; -use codex_model_provider_info::GatewayOAuthDelivery; use codex_model_provider_info::ModelProviderInfo; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::SessionSource; use codex_protocol::user_input::UserInput; use core_test_support::responses; +use core_test_support::test_codex::TestCodex; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use pretty_assertions::assert_eq; @@ -31,20 +30,56 @@ use wiremock::matchers::method; use wiremock::matchers::path; #[tokio::test] -async fn sampling_uses_gateway_and_primary_auth() -> Result<()> { - sampling_requires_gateway_and_primary_auth(/*expected_error*/ None).await +async fn inference_sends_chatgpt_and_gateway_credentials_to_the_configured_headers() -> Result<()> { + sample_with_gateway( + "requires_openai_auth = true", + "delivery = { kind = 'header', name = 'x-gateway-auth' }", + CodexAuth::from_external_chatgpt_tokens( + "header.e30.sig", + "test-account", + /*chatgpt_plan_type*/ None, + )?, + &[ + ("authorization", "Bearer header.e30.sig"), + ("x-gateway-auth", "Bearer gateway-token"), + ("chatgpt-account-id", "test-account"), + ], + /*expected_error*/ None, + ) + .await } #[tokio::test] async fn gateway_token_failure_blocks_sampling() -> Result<()> { - sampling_requires_gateway_and_primary_auth(Some( - "Gateway OAuth authentication failed; check the gateway configuration and credential store.", - )).await + sample_with_gateway( + "requires_openai_auth = true", + "delivery = { kind = 'header', name = 'x-gateway-auth' }", + CodexAuth::from_api_key("provider-token"), + &[], + Some("Gateway OAuth authentication failed; check the gateway configuration and credential store."), + ).await } -async fn sampling_requires_gateway_and_primary_auth(expected_error: Option<&str>) -> Result<()> { +#[tokio::test] +async fn omitted_delivery_sends_only_gateway_auth_even_when_signed_in() -> Result<()> { + sample_with_gateway( + "requires_openai_auth = false", + "", + CodexAuth::create_dummy_chatgpt_auth_for_testing(), + &[("authorization", "Bearer gateway-token")], + /*expected_error*/ None, + ) + .await +} + +async fn sample_with_gateway( + provider_settings: &str, + gateway_settings: &str, + primary_auth: CodexAuth, + expected_headers: &[(&str, &str)], + expected_error: Option<&str>, +) -> Result<()> { let server = MockServer::start().await; - let home = Arc::new(TempDir::new()?); let response = if expected_error.is_none() { Some( responses::mount_sse_once( @@ -66,38 +101,94 @@ async fn sampling_requires_gateway_and_primary_auth(expected_error: Option<&str> .await; None }; - let mut provider = - ModelProviderInfo::create_openai_provider(Some(format!("{}/v1", server.uri()))); - provider.supports_websockets = false; - provider.gateway_oauth = Some(GatewayOAuthConfig { - authorization_url: format!("{}/authorize", server.uri()), - token_url: format!("{}/token", server.uri()), - client_id: "client".into(), - resource: None, - scopes: vec![], - redirect_port: None, - delivery: GatewayOAuthDelivery::Header { - name: "x-gateway-auth".into(), - scheme: "Bearer".into(), - }, - }); + let expires_at = if expected_error.is_some() { + 0 + } else { + i64::MAX + }; + let test = gateway_codex( + &server, + provider_settings, + gateway_settings, + primary_auth, + expires_at, + ) + .await?; + assert_eq!(run_gateway_turn(&test).await?.as_deref(), expected_error); + if let Some(response) = response { + let request = response.single_request(); + for name in [ + "authorization", + "x-gateway-auth", + "cookie", + "chatgpt-account-id", + ] { + let expected = expected_headers + .iter() + .find(|(key, _)| *key == name) + .map(|(_, value)| *value); + assert_eq!( + request.header(name).as_deref(), + expected, + "{gateway_settings}: {name}" + ); + } + } else { + let requests = server + .received_requests() + .await + .expect("recorded gateway requests"); + assert!(!requests.is_empty()); + assert!( + requests + .iter() + .all(|request| request.url.path() == "/token") + ); + } + Ok(()) +} + +async fn gateway_codex( + server: &MockServer, + provider_settings: &str, + gateway_settings: &str, + primary_auth: CodexAuth, + expires_at: i64, +) -> Result { + let home = Arc::new(TempDir::new()?); + let provider: ModelProviderInfo = toml::from_str(&format!( + r#" +name = "gateway" +base_url = "{base}/v1" +{provider_settings} +[gateway_oauth] +authorization_url = "{base}/authorize" +token_url = "{base}/token" +client_id = "client" +{gateway_settings} +"#, + base = server.uri() + ))?; let primary = AuthManager::from_auth_for_testing_with_home( - CodexAuth::from_api_key("primary-token"), + primary_auth.clone(), home.path().to_path_buf(), ); let _gateway = seed_gateway_auth( &provider, &primary, - json!({"access_token": "gateway-token", "refresh_token": "refresh-token", "expires_at": if expected_error.is_some() { 0 } else { i64::MAX }}), + json!({"access_token": "gateway-token", "refresh_token": "refresh-token", "expires_at": expires_at}), ); - let test = test_codex() + test_codex() .with_home(home) - .with_auth(CodexAuth::from_api_key("primary-token")) + .with_auth(primary_auth) .with_config(move |config| { config.model_provider = provider; }) - .build_with_auto_env(&server) - .await?; + .build_with_auto_env(server) + .await +} + +async fn run_gateway_turn(test: &TestCodex) -> Result> { test.codex .start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text { text: "hello".into(), @@ -112,32 +203,7 @@ async fn sampling_requires_gateway_and_primary_auth(expected_error: Option<&str> matches!(event, EventMsg::TurnComplete(_)) }) .await; - assert_eq!(error_message.as_deref(), expected_error); - if let Some(response) = response { - let request = response.single_request(); - assert_eq!( - ( - request.header("authorization"), - request.header("x-gateway-auth") - ), - ( - Some("Bearer primary-token".to_string()), - Some("Bearer gateway-token".to_string()) - ), - ); - } else { - let requests = server - .received_requests() - .await - .expect("recorded gateway requests"); - assert!(!requests.is_empty()); - assert!( - requests - .iter() - .all(|request| request.url.path() == "/token") - ); - } - Ok(()) + Ok(error_message) } #[tokio::test] diff --git a/codex-rs/model-provider-info/src/gateway_oauth.rs b/codex-rs/model-provider-info/src/gateway_oauth.rs index d09961a1d3..b611037c66 100644 --- a/codex-rs/model-provider-info/src/gateway_oauth.rs +++ b/codex-rs/model-provider-info/src/gateway_oauth.rs @@ -36,6 +36,8 @@ pub struct GatewayOAuthConfig { #[serde(default)] pub scopes: Vec, pub redirect_port: Option, + /// Omit to send the gateway token via the Authorization header + #[serde(default)] pub delivery: GatewayOAuthDelivery, } @@ -60,11 +62,55 @@ pub enum GatewayOAuthDelivery { }, } +impl Default for GatewayOAuthDelivery { + fn default() -> Self { + Self::Header { + name: "Authorization".into(), + scheme: bearer_scheme(), + } + } +} + +impl GatewayOAuthDelivery { + fn is_authorization_bearer(&self) -> bool { + matches!(self, Self::Header { name, scheme } if name.eq_ignore_ascii_case("authorization") && scheme.eq_ignore_ascii_case("bearer")) + } + + fn secondary_header(&self) -> Result { + match self { + Self::Header { name, scheme } => { + let header = HeaderName::from_bytes(name.as_bytes()) + .map_err(|_| "needs a valid HTTP header name, such as X-Gateway-Auth")?; + if RESERVED_GATEWAY_OAUTH_HEADERS.contains(&header.as_str()) + || RESERVED_GATEWAY_OAUTH_HEADER_PREFIXES + .iter() + .any(|prefix| header.as_str().starts_with(prefix)) + { + return Err( + "uses a reserved header. Choose a custom header or cookie accepted by your gateway", + ); + } + if !is_token(scheme) { + return Err("needs an HTTP authentication scheme such as Bearer"); + } + Ok(header) + } + Self::Cookie { name } if is_token(name) => Ok(http::header::COOKIE), + Self::Cookie { .. } => Err("needs a valid cookie name, such as gateway_session"), + } + } +} + fn bearer_scheme() -> String { "Bearer".to_owned() } impl GatewayOAuthConfig { + /// Send the gateway bearer only to the configured endpoint. + pub fn requires_gateway_origin(&self) -> bool { + self.delivery.is_authorization_bearer() + } + pub(crate) fn validate(&self, provider: &ModelProviderInfo) -> Result<(), String> { if provider.aws.is_some() || provider.is_amazon_bedrock() { return Err("provider gateway_oauth cannot be combined with AWS authentication".into()); @@ -75,34 +121,44 @@ impl GatewayOAuthConfig { .base_url .as_deref() .ok_or("gateway_oauth requires base_url")?; - validate_endpoint_url(base_url, "base_url")?; + let gateway = validate_endpoint_url(base_url, "base_url")?; + if self.requires_gateway_origin() + && let Some(catalog) = provider.model_catalog_url.as_deref() + && validate_endpoint_url(catalog, "model_catalog_url")?.origin() != gateway.origin() + { + return Err( + "Codex would send gateway credentials to a different model catalog. Set model_catalog_url \ + to a URL with the same scheme, host, and port as base_url, or remove model_catalog_url \ + to use the gateway's catalog." + .into(), + ); + } if self.client_id.trim().is_empty() || self.redirect_port == Some(0) { return Err( "gateway_oauth requires a nonempty client_id and a nonzero redirect_port".into(), ); } - let header = match &self.delivery { - GatewayOAuthDelivery::Header { name, scheme } => { - let header = HeaderName::from_bytes(name.as_bytes()) - .map_err(|_| "invalid gateway_oauth header name")?; - if !is_token(scheme) - || RESERVED_GATEWAY_OAUTH_HEADERS.contains(&header.as_str()) - || RESERVED_GATEWAY_OAUTH_HEADER_PREFIXES - .iter() - .any(|prefix| header.as_str().starts_with(prefix)) - { - return Err( - "invalid or reserved gateway_oauth delivery header or scheme".into(), - ); - } - header + let gateway_header = if self.delivery.is_authorization_bearer() { + if provider.is_gcp() { + return Err("Google Cloud uses the Authorization header for its own token. Remove gateway_oauth to use the built-in Google Cloud provider".into()); } - GatewayOAuthDelivery::Cookie { name } => { - if !is_token(name) { - return Err("invalid gateway_oauth cookie name".into()); - } - http::header::COOKIE + if provider.requires_openai_auth + || provider.env_key.is_some() + || provider.experimental_bearer_token.is_some() + || provider.has_command_auth() + { + return Err( + "Header conflict. Codex is configured to send both the model provider credential \ + and the gateway token in the Authorization header. Set gateway_oauth.delivery \ + to a different header or cookie that your gateway accepts for its token." + .into(), + ); } + http::header::AUTHORIZATION + } else { + self.delivery + .secondary_header() + .map_err(|reason| format!("gateway_oauth.delivery {reason}"))? }; if provider .http_headers @@ -114,11 +170,11 @@ impl GatewayOAuthConfig { .iter() .flat_map(|headers| headers.keys()), ) - .any(|name| name.eq_ignore_ascii_case(header.as_str())) + .any(|name| name.eq_ignore_ascii_case(gateway_header.as_str())) { - return Err( - "gateway_oauth delivery conflicts with a configured provider header".into(), - ); + return Err(format!( + "The {gateway_header} header is set in both gateway_oauth.delivery and the provider's http_headers or env_http_headers. Remove the duplicate header from the provider or choose a different gateway delivery." + )); } Ok(()) } diff --git a/codex-rs/model-provider-info/src/gateway_oauth_tests.rs b/codex-rs/model-provider-info/src/gateway_oauth_tests.rs index f5ea122f90..f0165b251a 100644 --- a/codex-rs/model-provider-info/src/gateway_oauth_tests.rs +++ b/codex-rs/model-provider-info/src/gateway_oauth_tests.rs @@ -52,7 +52,6 @@ fn rejects_unsafe_endpoints_and_delivery_collisions() { assert!(!error.contains(url)); } for name in [ - "Authorization", "HOST", "x-openai-actor-authorization", "content-length", @@ -77,3 +76,45 @@ fn rejects_unsafe_endpoints_and_delivery_collisions() { )])); assert!(cookie.validate().is_err()); } + +#[test] +fn gateway_bearer_requires_the_model_catalog_to_share_its_origin() { + let other = "https://gateway.example.test:8443/catalog"; + let mut info = provider(""); + info.model_catalog_url = Some(other.into()); + assert!( + info.validate() + .unwrap_err() + .contains("same scheme, host, and port as base_url") + ); + + info.model_catalog_url = Some("https://gateway.example.test:443/custom/catalog".into()); + assert_eq!(info.validate(), Ok(())); + + let mut legacy = provider("delivery = { kind = 'header', name = 'x-gateway-auth' }"); + legacy.model_catalog_url = Some(other.into()); + assert_eq!(legacy.validate(), Ok(())); +} + +#[test] +fn omitted_delivery_rejects_an_existing_provider_bearer() { + let mut info = provider(""); + assert_eq!(info.validate(), Ok(())); + info.requires_openai_auth = true; + assert!( + info.validate() + .unwrap_err() + .contains("gateway_oauth.delivery") + ); + info.requires_openai_auth = false; + info.env_key = Some("PROVIDER_API_KEY".into()); + assert!( + info.validate() + .unwrap_err() + .contains("gateway_oauth.delivery") + ); + + let mut explicit = provider("delivery = { kind = 'header', name = 'x-gateway-auth' }"); + explicit.env_key = info.env_key; + assert_eq!(explicit.validate(), Ok(())); +} diff --git a/codex-rs/model-provider/src/combined_auth.rs b/codex-rs/model-provider/src/combined_auth.rs index da79c4ebea..e4518a8922 100644 --- a/codex-rs/model-provider/src/combined_auth.rs +++ b/codex-rs/model-provider/src/combined_auth.rs @@ -63,9 +63,9 @@ pub(crate) async fn compose_auth( .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(), - )); + return Err(CodexErr::InvalidRequest(format!( + "The model provider already sets the {name} header. Choose a different header or cookie in gateway_oauth.delivery for the gateway token." + ))); } resolved.auth = Arc::new(CombinedAuth { primary: resolved.auth, @@ -111,26 +111,29 @@ struct CombinedAuth { value: HeaderValue, } +impl CombinedAuth { + fn insert_gateway_header(&self, headers: &mut HeaderMap) -> std::result::Result<(), AuthError> { + if headers.contains_key(&self.name) { + return Err(AuthError::Build(format!( + "The request already sets the {} header. Choose a different header or cookie in gateway_oauth.delivery for the gateway token.", + self.name + ))); + } + headers.insert(self.name.clone(), self.value.clone()); + Ok(()) + } +} + impl AuthProvider for CombinedAuth { 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(()) + self.insert_gateway_header(headers) } fn apply_auth(&self, request: codex_http_client::Request) -> AuthProviderFuture<'_> { Box::pin(async move { let mut request = self.primary.apply_auth(request).await?; - if request.headers.contains_key(&self.name) { - return Err(AuthError::Build( - "gateway OAuth conflicts with request headers".into(), - )); - } - request - .headers - .insert(self.name.clone(), self.value.clone()); + self.insert_gateway_header(&mut request.headers)?; Ok(request) }) } @@ -138,12 +141,7 @@ impl AuthProvider for CombinedAuth { fn apply_auth_headers(&self, headers: HeaderMap) -> AuthHeadersFuture<'_> { Box::pin(async move { let mut headers = self.primary.apply_auth_headers(headers).await?; - if headers.contains_key(&self.name) { - return Err(AuthError::Build( - "gateway OAuth conflicts with request headers".into(), - )); - } - headers.insert(self.name.clone(), self.value.clone()); + self.insert_gateway_header(&mut headers)?; Ok(headers) }) }