From f00c70f5d0e1715bbbcba99b0cc6e629bf83a4f1 Mon Sep 17 00:00:00 2001 From: jamy-OAI Date: Tue, 27 Oct 2026 21:40:16 +0000 Subject: [PATCH] Allow catalog URL overrides for the built-in OpenAI provider (#57567) ## What changed Allow `model_catalog_url` under `[model_providers.openai]` while preserving the built-in provider's identity, authentication, and inference routing through `openai_base_url`. Reject other non-default overrides and require an HTTPS catalog URL on an allowed ChatGPT host, without embedded credentials. Apply the same catalog override merging when checking managed provider requirements for existing threads, preserving each thread's inference endpoint even if user defaults change. Document the required Codex catalog format, credential forwarding, and existing discovery and access restrictions. ## Testing Add coverage for allowed and rejected catalog destinations, preservation of built-in provider settings, and managed requirement changes. Add integration coverage for API key and ChatGPT authentication that verifies separate catalog and inference routing. GitOrigin-RevId: 4d91ada518ebd2a09cbf6ded0ddf4b11c92b77dd --- codex-rs/app-server/src/config_manager.rs | 5 +- .../src/config_manager_provider_tests.rs | 27 ++++- codex-rs/config/src/config_toml.rs | 11 +- codex-rs/core/config.schema.json | 2 +- .../core/tests/suite/builtin_model_catalog.rs | 103 ++++++++++++++++++ codex-rs/core/tests/suite/mod.rs | 1 + codex-rs/model-provider-info/src/lib.rs | 35 +++++- .../src/model_provider_info_tests.rs | 88 +++++++++++++++ 8 files changed, 262 insertions(+), 10 deletions(-) create mode 100644 codex-rs/core/tests/suite/builtin_model_catalog.rs diff --git a/codex-rs/app-server/src/config_manager.rs b/codex-rs/app-server/src/config_manager.rs index 83b1f04f67..96e30332b7 100644 --- a/codex-rs/app-server/src/config_manager.rs +++ b/codex-rs/app-server/src/config_manager.rs @@ -17,6 +17,7 @@ use codex_login::default_client::set_default_client_residency_requirement; use codex_model_provider_info::AMAZON_BEDROCK_PROVIDER_ID; use codex_model_provider_info::AMAZON_BEDROCK_RUNTIME_PROVIDER_ID; use codex_model_provider_info::GCP_PROVIDER_ID; +use codex_model_provider_info::OPENAI_PROVIDER_ID; use codex_model_provider_info::built_in_model_providers; use codex_model_provider_info::merge_configured_model_providers; use codex_utils_absolute_path::AbsolutePathBuf; @@ -380,11 +381,13 @@ impl ConfigManager { AMAZON_BEDROCK_PROVIDER_ID | AMAZON_BEDROCK_RUNTIME_PROVIDER_ID | GCP_PROVIDER_ID + | OPENAI_PROVIDER_ID ) => { // Managed overrides must receive the same built-in defaults as startup config. + // OpenAI's inference URL belongs to the retained session, not the catalog override. let mut provider = merge_configured_model_providers( - built_in_model_providers(/*openai_base_url*/ None), + built_in_model_providers(current.model_provider.base_url.clone()), HashMap::from([(current.model_provider_id.clone(), provider)]), ) .map_err(|message| std::io::Error::new(std::io::ErrorKind::InvalidData, message))? diff --git a/codex-rs/app-server/src/config_manager_provider_tests.rs b/codex-rs/app-server/src/config_manager_provider_tests.rs index abd14e2210..a58138b3bf 100644 --- a/codex-rs/app-server/src/config_manager_provider_tests.rs +++ b/codex-rs/app-server/src/config_manager_provider_tests.rs @@ -14,20 +14,29 @@ use pretty_assertions::assert_eq; use tempfile::tempdir; use test_case::test_case; -#[test_case(AMAZON_BEDROCK_PROVIDER_ID, "aws.region = 'us-west-2'", "us-west-2", "us-east-1"; "bedrock")] -#[test_case(AMAZON_BEDROCK_RUNTIME_PROVIDER_ID, "aws.region = 'us-west-2'", "us-west-2", "us-east-1"; "bedrock_runtime")] -#[test_case(GCP_PROVIDER_ID, "project_id = 'example-project'\nlocation = 'us-central1'", "example-project", "other-project"; "gcp_project")] -#[test_case(GCP_PROVIDER_ID, "project_id = 'example-project'\nlocation = 'us-central1'", "us-central1", "us-east4"; "gcp_location")] -#[test_case(GCP_PROVIDER_ID, "project_id = 'example-project'\nauth.command = 'print-token'", "print-token", "other-token"; "gcp_auth")] +#[test_case(AMAZON_BEDROCK_PROVIDER_ID, "aws.region = 'us-west-2'", "us-west-2", "us-east-1", None; "bedrock")] +#[test_case(AMAZON_BEDROCK_RUNTIME_PROVIDER_ID, "aws.region = 'us-west-2'", "us-west-2", "us-east-1", None; "bedrock_runtime")] +#[test_case(GCP_PROVIDER_ID, "project_id = 'example-project'\nlocation = 'us-central1'", "example-project", "other-project", None; "gcp_project")] +#[test_case(GCP_PROVIDER_ID, "project_id = 'example-project'\nlocation = 'us-central1'", "us-central1", "us-east4", None; "gcp_location")] +#[test_case(GCP_PROVIDER_ID, "project_id = 'example-project'\nauth.command = 'print-token'", "print-token", "other-token", None; "gcp_auth")] +#[test_case("openai", "model_catalog_url = 'https://chatgpt.com/backend-api/codex/models'", "https://chatgpt.com/backend-api/codex/models", "https://chatgpt.com/backend-api/codex/other-models", None; "openai_catalog")] +#[test_case("openai", "model_catalog_url = 'https://chatgpt.com/backend-api/codex/models'", "https://chatgpt.com/backend-api/codex/models", "https://chatgpt.com/backend-api/codex/other-models", Some("https://us.api.openai.com/v1"); "openai_regional_catalog")] #[tokio::test] async fn provider_requirements_resolve_builtin_overrides( provider_id: &str, provider_config: &str, original: &str, replacement: &str, + openai_base_url: Option<&str>, ) -> Result<()> { let home = tempdir()?; let workspace = tempdir()?; + if let Some(base_url) = openai_base_url { + std::fs::write( + home.path().join("config.toml"), + format!("openai_base_url = '{base_url}'"), + )?; + } let requirements = format!( "model_provider = '{provider_id}'\n[model_providers.{provider_id}]\n{provider_config}" ); @@ -40,6 +49,14 @@ async fn provider_requirements_resolve_builtin_overrides( let current = manager .load_latest_config(Some(workspace.path().to_path_buf())) .await?; + if provider_id == "openai" { + assert_eq!(current.model_provider.base_url.as_deref(), openai_base_url); + // Retained threads keep their inference route even if user defaults later change. + std::fs::write( + home.path().join("config.toml"), + "openai_base_url = 'https://eu.api.openai.com/v1'", + )?; + } manager.check_thread_model_provider(¤t).await?; let changed = ConfigManager::new_for_tests( diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index dedf0aea6d..2578c59d9c 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -940,6 +940,14 @@ pub fn validate_reserved_model_provider_ids( let mut conflicts = model_providers .keys() .filter(|key| { + if key.as_str() == "openai" { + let provider = &model_providers[*key]; + return provider + != &ModelProviderInfo { + model_catalog_url: provider.model_catalog_url.clone(), + ..Default::default() + }; + } !matches!( key.as_str(), AMAZON_BEDROCK_PROVIDER_ID | AMAZON_BEDROCK_RUNTIME_PROVIDER_ID | GCP_PROVIDER_ID @@ -980,7 +988,8 @@ pub fn validate_model_providers( if !matches!( key.as_str(), AMAZON_BEDROCK_PROVIDER_ID | AMAZON_BEDROCK_RUNTIME_PROVIDER_ID | GCP_PROVIDER_ID - ) && provider.name.trim().is_empty() + ) && key != "openai" + && provider.name.trim().is_empty() { return Err(format!( "model_providers.{key}: provider name must not be empty" diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index c285c5e35a..1348a9ef9e 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -3113,7 +3113,7 @@ "type": "string" }, "model_catalog_url": { - "description": "Optional full URL for a Codex-native model catalog. When unset, OpenAI discovery uses the Codex backend unless `base_url` overrides the inference endpoint.", + "description": "Optional full URL for a Codex-native model catalog. When unset, OpenAI discovery uses the Codex backend unless `base_url` overrides the inference endpoint. The endpoint must serve the Codex catalog format, not the public `/v1/models` API.\n\nFor built-in OpenAI, set this under `[model_providers.openai]`; other non-default provider overrides are rejected. The URL must use HTTPS and an allowed ChatGPT host. Other hosts require a custom provider. Inference still uses `openai_base_url`.\n\nCatalog requests use the provider's credentials, including an API key or ChatGPT access token. Only configure an endpoint you trust with those credentials. Setting this URL does not grant catalog access or guarantee that the endpoint accepts a regional API key. Existing discovery gates and entitlement checks apply.", "type": "string" }, "name": { diff --git a/codex-rs/core/tests/suite/builtin_model_catalog.rs b/codex-rs/core/tests/suite/builtin_model_catalog.rs new file mode 100644 index 0000000000..a2d8f46229 --- /dev/null +++ b/codex-rs/core/tests/suite/builtin_model_catalog.rs @@ -0,0 +1,103 @@ +//! Built-in OpenAI catalog routing uses configured metadata without redirecting inference. +#![cfg(test)] + +use anyhow::Result; +use codex_core::config::ConfigBuilder; +use codex_login::CodexAuth; +use codex_models_manager::bundled_models_response; +use core_test_support::responses; +use core_test_support::test_codex::test_codex; +use pretty_assertions::assert_eq; +use std::sync::Arc; +use tempfile::TempDir; +use test_case::test_case; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::ResponseTemplate; +use wiremock::matchers::header; +use wiremock::matchers::method; +use wiremock::matchers::path; + +#[test_case(CodexAuth::from_api_key("catalog-routing-test-key"), "Bearer catalog-routing-test-key"; "api_key")] +#[test_case(CodexAuth::create_dummy_chatgpt_auth_for_testing(), "Bearer Access Token"; "chatgpt")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn builtin_openai_catalog_override_preserves_inference( + auth: CodexAuth, + authorization: &str, +) -> Result<()> { + let inference = responses::start_mock_server().await; + let catalog_server = MockServer::start().await; + let mut catalog = bundled_models_response()?; + catalog.models[0].display_name = "Explicit built-in catalog".into(); + Mock::given(method("GET")) + .and(path("/catalog")) + .and(header("authorization", authorization)) + .respond_with(ResponseTemplate::new(200).set_body_json(&catalog)) + .mount(&catalog_server) + .await; + let home = Arc::new(TempDir::new()?); + std::fs::write( + home.path().join("config.toml"), + format!( + r#"model_provider = "openai" +openai_base_url = "{}/v1" +[features] +api_key_model_discovery = true +[model_providers.openai] +model_catalog_url = "https://chatgpt.com/backend-api/codex/models" +"#, + inference.uri(), + ), + )?; + let mut loaded = ConfigBuilder::default() + .codex_home(home.path().to_path_buf()) + .build() + .await?; + assert_eq!( + loaded + .model_provider + .model_catalog_url + .as_deref() + .map(String::as_str), + Some("https://chatgpt.com/backend-api/codex/models"), + ); + // Config validation uses the production allowlist. Redirect only this test's + // runtime catalog client to a local server to capture credential routing. + loaded.model_provider.model_catalog_url = + Some(format!("{}/catalog", catalog_server.uri()).into()); + assert_eq!(loaded.model_provider_id, "openai"); + assert_eq!(loaded.model_provider.name, "OpenAI"); + let test = test_codex() + .with_home(home) + .with_auth(auth) + .with_config(move |config| { + config.model_provider = loaded.model_provider.clone(); + config.model_provider.supports_websockets = false; + config.features = loaded.features; + }) + .build_with_auto_env(&inference) + .await?; + let response = responses::mount_sse_once(&inference, responses::sse_completed("done")).await; + test.submit_turn("hello").await?; + assert_eq!( + response.single_request().header("authorization").as_deref(), + Some(authorization), + ); + assert_eq!( + test.thread_manager + .get_models_manager() + .get_remote_models() + .await, + catalog.models, + ); + assert!( + inference + .received_requests() + .await + .unwrap() + .iter() + .all(|request| request.url.path() != "/v1/models") + ); + assert_eq!(catalog_server.received_requests().await.unwrap().len(), 1); + Ok(()) +} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 9bb0c274f2..d78151c35b 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -98,6 +98,7 @@ mod guardian_cached_score; #[path = "guardian_checkpoint_migration_tests.rs"] mod guardian_checkpoint_migration; // Uses the same command-approval harness as guardian_review below. +mod builtin_model_catalog; mod canonical_plugin_connectors; mod critical_path_runtime; mod gateway_auth; diff --git a/codex-rs/model-provider-info/src/lib.rs b/codex-rs/model-provider-info/src/lib.rs index 78fc793c35..a03dfef900 100644 --- a/codex-rs/model-provider-info/src/lib.rs +++ b/codex-rs/model-provider-info/src/lib.rs @@ -152,6 +152,16 @@ pub struct ModelProviderInfo { pub base_url: Option, /// Optional full URL for a Codex-native model catalog. When unset, OpenAI discovery /// uses the Codex backend unless `base_url` overrides the inference endpoint. + /// The endpoint must serve the Codex catalog format, not the public `/v1/models` API. + /// + /// For built-in OpenAI, set this under `[model_providers.openai]`; other non-default + /// provider overrides are rejected. The URL must use HTTPS and an allowed ChatGPT + /// host. Other hosts require a custom provider. Inference still uses `openai_base_url`. + /// + /// Catalog requests use the provider's credentials, including an API key or ChatGPT + /// access token. Only configure an endpoint you trust with those credentials. + /// Setting this URL does not grant catalog access or guarantee that the endpoint + /// accepts a regional API key. Existing discovery gates and entitlement checks apply. pub model_catalog_url: Option, /// Environment variable that stores the user's API key for this provider. pub env_key: Option, @@ -768,14 +778,35 @@ pub fn built_in_model_providers( /// Merge configured providers into the built-in provider catalog. /// /// Configured providers extend the built-in set. Built-in providers are not -/// generally overridable, but the cloud-hosted OpenAI providers allow a limited +/// generally overridable. OpenAI allows a catalog URL override, while the +/// cloud-hosted OpenAI providers allow a limited /// set of endpoint, authentication, header, and cloud-specific overrides. pub fn merge_configured_model_providers( mut model_providers: HashMap, configured_model_providers: HashMap, ) -> Result, String> { for (key, mut provider) in configured_model_providers { - if matches!( + if key == "openai" { + // Keep the built-in identity, inference routing, and authentication. + // Only catalog routing is configurable through this provider entry. + if let Some(catalog_url) = provider.model_catalog_url.take() + && let Some(built_in_provider) = model_providers.get_mut(&key) + { + let allowed = catalog_url.parse::().ok().is_some_and(|uri| { + uri.scheme_str() == Some("https") + && uri.host().is_some_and(|host| { + codex_http_client::is_allowed_chatgpt_host(&host.to_ascii_lowercase()) + }) + && uri + .authority() + .is_some_and(|authority| !authority.as_str().contains('@')) + }); + if !allowed { + return Err("model_providers.openai.model_catalog_url must use an allowed ChatGPT host. If you need to use a 3rd party host, configure a custom model_provider".to_string()); + } + built_in_provider.model_catalog_url = Some(catalog_url); + } + } else if matches!( key.as_str(), AMAZON_BEDROCK_PROVIDER_ID | AMAZON_BEDROCK_RUNTIME_PROVIDER_ID ) { diff --git a/codex-rs/model-provider-info/src/model_provider_info_tests.rs b/codex-rs/model-provider-info/src/model_provider_info_tests.rs index 78f476af1a..0d3742f81e 100644 --- a/codex-rs/model-provider-info/src/model_provider_info_tests.rs +++ b/codex-rs/model-provider-info/src/model_provider_info_tests.rs @@ -993,3 +993,91 @@ model_catalog_url = "https://gateway.example/codex/catalog?token=catalog-secret" "https://gateway.example/v1" ); } + +#[test] +fn builtin_openai_catalog_override_preserves_inference_and_identity() { + for inference_url in [None, Some("https://us.api.openai.com/v1")] { + for catalog_url in [None, Some("https://chatgpt.com/backend-api/codex/models")] { + let configured: ModelProviderInfo = toml::from_str(&format!( + "name = \"Ignored name\"\nbase_url = \"https://ignored.example/v1\"\nrequires_openai_auth = false\n{}", + catalog_url.map(|url| format!("model_catalog_url = {url:?}")).unwrap_or_default() + )).unwrap(); + let builtins = built_in_model_providers(inference_url.map(str::to_owned)); + let mut expected = builtins.clone(); + expected.get_mut("openai").unwrap().model_catalog_url = catalog_url.map(Into::into); + assert_eq!( + merge_configured_model_providers( + builtins, + HashMap::from([("openai".to_string(), configured)]), + ) + .unwrap(), + expected, + ); + } + } +} + +#[test] +fn builtin_openai_catalog_rejects_untrusted_destinations() { + for url in [ + "http://chatgpt.com/backend-api/codex/models", + "https://chatgpt.com.evil.example/models", + "https://evilchatgpt.com/models", + "https://api.openai.com/v1/models", + "https://example.com/models?secret=do-not-log", + "https://user:secret@chatgpt.com/models", + "/models", + "not a URL", + ] { + let provider = ModelProviderInfo { + model_catalog_url: Some(url.into()), + ..Default::default() + }; + assert_eq!( + merge_configured_model_providers( + built_in_model_providers(/*openai_base_url*/ None), + HashMap::from([("openai".to_string(), provider.clone())]), + ) + .unwrap_err(), + "model_providers.openai.model_catalog_url must use an allowed ChatGPT host. If you need to use a 3rd party host, configure a custom model_provider", + ); + let mut expected = built_in_model_providers(/*openai_base_url*/ None); + expected.insert("custom".into(), provider.clone()); + assert_eq!( + merge_configured_model_providers( + built_in_model_providers(/*openai_base_url*/ None), + HashMap::from([("custom".into(), provider)]), + ) + .unwrap(), + expected, + ); + } +} + +#[test] +fn builtin_openai_catalog_accepts_existing_chatgpt_hosts() { + for host in [ + "chatgpt.com", + "gov.chatgpt.com", + "chat.openai.com", + "chatgpt-staging.com", + "api.chatgpt-staging.com", + "CHATGPT.COM", + ] { + let url = format!("https://{host}/backend-api/codex/models"); + let provider = ModelProviderInfo { + model_catalog_url: Some(url.clone().into()), + ..Default::default() + }; + let mut expected = built_in_model_providers(/*openai_base_url*/ None); + expected.get_mut("openai").unwrap().model_catalog_url = Some(url.into()); + assert_eq!( + merge_configured_model_providers( + built_in_model_providers(/*openai_base_url*/ None), + HashMap::from([("openai".into(), provider)]), + ) + .unwrap(), + expected + ); + } +}