diff --git a/codex-rs/app-server/tests/suite/v2/gateway_oauth_tests.rs b/codex-rs/app-server/tests/suite/v2/gateway_oauth_tests.rs index 4193b65c20..1f389b50b6 100644 --- a/codex-rs/app-server/tests/suite/v2/gateway_oauth_tests.rs +++ b/codex-rs/app-server/tests/suite/v2/gateway_oauth_tests.rs @@ -54,6 +54,7 @@ delivery = {{ kind = "header", name = "X-Gateway-Authorization" }} async fn gateway_server(home: &std::path::Path) -> Result { let mut server = TestAppServer::builder() .with_codex_home(home) + .with_env_overrides(&[("OPENAI_API_KEY", None), ("CODEX_API_KEY", None)]) .build() .await?; server @@ -238,10 +239,7 @@ async fn model_list_leaves_oauth_gateway_after_account_change() -> Result<()> { Ok(()) } -#[tokio::test] -async fn file_credentials_reuse_gateway_login_after_restart() -> Result<()> { - let home = TempDir::new()?; - let oauth = MockServer::start().await; +async fn login_to_gateway(server: &mut TestAppServer, oauth: &MockServer) -> Result<()> { Mock::given(method("POST")) .and(path("/token")) .and(body_string_contains("grant_type=authorization_code")) @@ -249,18 +247,12 @@ async fn file_credentials_reuse_gateway_login_after_restart() -> Result<()> { "access_token": "gateway-access", "refresh_token": "gateway-refresh", "expires_in": 3600, }))) .expect(/*r*/ 1) - .mount(&oauth) + .mount(oauth) .await; - // Exercise the managed provider definition used by headless installations. - std::fs::write( - home.path().join("requirements.toml"), - format!("{}\ncredentials_store = 'file'\n", gateway_config(&oauth)), - )?; - let mut server = gateway_server(home.path()).await?; let login = server .send_raw_request("account/gatewayOAuth/login", /*params*/ None) .await?; - let url = authorization_url(&mut server).await?; + let url = authorization_url(server).await?; let query = url .query_pairs() .collect::>(); @@ -275,6 +267,73 @@ async fn file_credentials_reuse_gateway_login_after_restart() -> Result<()> { .await? .error_for_status()?; let _: GatewayOAuthLoginResponse = server.read_response(login).await?; + Ok(()) +} + +#[tokio::test] +async fn gateway_oauth_without_api_key_discovers_explicit_catalog_by_default() -> Result<()> { + let home = TempDir::new()?; + let gateway = MockServer::start().await; + let url = gateway.uri(); + std::fs::write( + home.path().join("config.toml"), + format!( + r#" +model_provider = "gateway" +[model_providers.gateway] +name = "Test gateway" +base_url = "{url}/v1" +model_catalog_url = "{url}/catalog" +[model_providers.gateway.gateway_oauth] +authorization_url = "{url}/authorize" +token_url = "{url}/token" +client_id = "catalog-test" +credentials_store = "file" +"# + ), + )?; + let model = codex_protocol::openai_models::ModelInfo { + visibility: codex_protocol::openai_models::ModelVisibility::List, + ..codex_models_manager::model_info::model_info_from_slug("gateway-coder") + }; + Mock::given(method("GET")) + .and(path("/catalog")) + .and(wiremock::matchers::header( + "authorization", + "Bearer gateway-access", + )) + .respond_with(ResponseTemplate::new(/*s*/ 200).set_body_json( + codex_protocol::openai_models::ModelsResponse { + models: vec![model], + }, + )) + .expect(1..) + .mount(&gateway) + .await; + let mut server = gateway_server(home.path()).await?; + login_to_gateway(&mut server, &gateway).await?; + let response: codex_app_server_protocol::ModelListResponse = server + .request(|request_id| ClientRequest::ModelList { + request_id, + params: Default::default(), + }) + .await?; + let ids: Vec<_> = response.data.into_iter().map(|model| model.id).collect(); + assert_eq!(ids, ["gateway-coder"]); + Ok(()) +} + +#[tokio::test] +async fn file_credentials_reuse_gateway_login_after_restart() -> Result<()> { + let home = TempDir::new()?; + let oauth = MockServer::start().await; + // Exercise the managed provider definition used by headless installations. + std::fs::write( + home.path().join("requirements.toml"), + format!("{}\ncredentials_store = 'file'\n", gateway_config(&oauth)), + )?; + let mut server = gateway_server(home.path()).await?; + login_to_gateway(&mut server, &oauth).await?; server.shutdown_gracefully().await?; let path = home.path().join("secrets/gateway_oauth.json"); diff --git a/codex-rs/model-provider/src/gateway_auth.rs b/codex-rs/model-provider/src/gateway_auth.rs index 81ce4e0243..4f7dfbdbc3 100644 --- a/codex-rs/model-provider/src/gateway_auth.rs +++ b/codex-rs/model-provider/src/gateway_auth.rs @@ -65,6 +65,15 @@ impl GatewayAuthManager { } } + pub(crate) fn without_interactive_login(self: &Arc) -> Arc { + let GatewayCredentialSource::OAuth(session) = &self.0 else { + return Arc::clone(self); + }; + Arc::new(Self(GatewayCredentialSource::OAuth(Arc::new( + session.as_ref().clone().without_interactive_login(), + )))) + } + pub async fn resolve_access_token(&self) -> Result> { match &self.0 { GatewayCredentialSource::Export(export) => export.token().await, diff --git a/codex-rs/model-provider/src/models_endpoint.rs b/codex-rs/model-provider/src/models_endpoint.rs index 072444db65..2900c337e9 100644 --- a/codex-rs/model-provider/src/models_endpoint.rs +++ b/codex-rs/model-provider/src/models_endpoint.rs @@ -50,6 +50,25 @@ const MODELS_REFRESH_TIMEOUT: Duration = Duration::from_secs(5); const MODELS_ENDPOINT: &str = "/models"; const FEDRAMP_CODEX_BASE_URL: &str = "https://gov.chatgpt.com/backend-api/codex"; +// Catalog requests use the model provider's credentials. Before enabling a fetch +// with gateway OAuth alone, check that the catalog URL has the same origin as the gateway. +fn supports_gateway_catalog(info: &ModelProviderInfo) -> bool { + if info.is_openai() || info.gateway_oauth.is_none() { + return false; + } + let (Some(base), Some(catalog)) = (info.base_url.as_deref(), info.model_catalog_url.as_deref()) + else { + return false; + }; + match ( + codex_http_client::validate_endpoint_url(base, "base_url"), + codex_http_client::validate_endpoint_url(catalog, "model_catalog_url"), + ) { + (Ok(base), Ok(catalog)) => base.origin() == catalog.origin(), + _ => false, + } +} + /// Provider-owned OpenAI-compatible `/models` endpoint. #[derive(Debug)] pub(crate) struct OpenAiModelsEndpoint { @@ -64,8 +83,14 @@ impl OpenAiModelsEndpoint { pub(crate) fn new( provider_info: ModelProviderInfo, auth_manager: Option>, - gateway_auth_manager: Option, String>>, + mut gateway_auth_manager: Option, String>>, ) -> Self { + // Fetch with an existing gateway session; a catalog refresh must not open a browser. + if supports_gateway_catalog(&provider_info) + && let Some(Ok(manager)) = &mut gateway_auth_manager + { + *manager = manager.without_interactive_login(); + } let redirect_policy = if provider_info.model_catalog_url.is_some() || provider_info .gateway_oauth @@ -299,6 +324,16 @@ impl ModelsEndpointClient for OpenAiModelsEndpoint { || self.provider_info.experimental_bearer_token.is_some() } + fn supports_gateway_catalog(&self) -> bool { + supports_gateway_catalog(&self.provider_info) + } + + fn uses_api_key_auth(&self, login: Option) -> bool { + self.has_provider_api_key() + || (login == Some(codex_protocol::auth::AuthMode::ApiKey) + && (!self.supports_gateway_catalog() || self.provider_info.requires_openai_auth)) + } + fn identity(&self) -> Option { let auth = self .auth_manager diff --git a/codex-rs/model-provider/src/shared_state_tests.rs b/codex-rs/model-provider/src/shared_state_tests.rs index 04b69780a0..f1988d0bc1 100644 --- a/codex-rs/model-provider/src/shared_state_tests.rs +++ b/codex-rs/model-provider/src/shared_state_tests.rs @@ -689,7 +689,9 @@ async fn gateway_login_required_invalidates_the_last_successful_catalog() { home.path().to_path_buf(), ); let info = ModelProviderInfo { - model_catalog_url: Some(format!("{}/models", server.uri()).into()), + name: "Gateway".to_string(), + base_url: Some(server.uri()), + model_catalog_url: Some(format!("{}/catalog", server.uri()).into()), gateway_oauth: Some(GatewayOAuthConfig::OAuth(GatewayOAuthSettings { authorization_url: format!("{}/authorize", server.uri()), device_authorization_url: None, @@ -699,12 +701,9 @@ async fn gateway_login_required_invalidates_the_last_successful_catalog() { scopes: vec![], redirect_port: None, credentials_store: OAuthCredentialsStore::File, - delivery: GatewayOAuthDelivery::Header { - name: "x-gateway-auth".into(), - scheme: "Bearer".into(), - }, + delivery: GatewayOAuthDelivery::default(), })), - ..ModelProviderInfo::create_openai_provider(Some(server.uri())) + ..Default::default() }; let login_control = codex_login::GatewayLoginControl::for_runtime(&primary.runtime_config()); login_control.require_explicit_login(); @@ -718,11 +717,13 @@ async fn gateway_login_required_invalidates_the_last_successful_catalog() { home.path().to_path_buf(), /*config_model_catalog*/ None, ); - models.set_api_key_model_discovery_enabled(/*enabled*/ true); + // Only the gateway token should enable this fetch; the saved API key is unrelated. + models.set_api_key_model_discovery_enabled(/*enabled*/ false); let factory = HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault); let catalog = codex_models_manager::bundled_models_response().unwrap(); Mock::given(method("GET")) - .and(path("/models")) + .and(path("/catalog")) + .and(wiremock::matchers::header("authorization", "Bearer first")) .respond_with(ResponseTemplate::new(200).set_body_json(&catalog)) .expect(1) .mount(&server) @@ -758,7 +759,7 @@ async fn gateway_login_required_invalidates_the_last_successful_catalog() { home.path().to_path_buf(), /*config_model_catalog*/ None, ); - restarted.set_api_key_model_discovery_enabled(/*enabled*/ true); + restarted.set_api_key_model_discovery_enabled(/*enabled*/ false); assert_eq!( restarted .raw_model_catalog(RefreshStrategy::Offline, factory) diff --git a/codex-rs/models-manager/src/manager.rs b/codex-rs/models-manager/src/manager.rs index a8aafa125d..cdc890e540 100644 --- a/codex-rs/models-manager/src/manager.rs +++ b/codex-rs/models-manager/src/manager.rs @@ -79,6 +79,16 @@ pub trait ModelsEndpointClient: fmt::Debug + Send + Sync { false } + /// Whether this custom provider can serve a Codex catalog using its gateway credentials. + fn supports_gateway_catalog(&self) -> bool { + false + } + + /// Determines whether this provider uses API key auth to fetch the model catalog. + fn uses_api_key_auth(&self, login: Option) -> bool { + self.has_provider_api_key() || login == Some(AuthMode::ApiKey) + } + /// Fetches the latest remote model catalog and optional ETag. fn list_models<'a>( &'a self, @@ -936,15 +946,16 @@ impl OpenAiModelsManager { } fn uses_api_key_auth(&self) -> bool { - self.endpoint_client.has_provider_api_key() - || self - .auth_manager + self.endpoint_client.uses_api_key_auth( + self.auth_manager .as_ref() - .is_some_and(|auth_manager| auth_manager.auth_mode() == Some(AuthMode::ApiKey)) + .and_then(|manager| manager.auth_mode()), + ) } async fn should_refresh_models(&self) -> bool { - self.endpoint_client.uses_codex_backend().await + self.endpoint_client.supports_gateway_catalog() + || self.endpoint_client.uses_codex_backend().await || self.endpoint_client.has_command_auth() || self.supports_api_key_discovery() }