mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Support explicit provider model catalog URLs (#46561)
## Why Providers need to serve Codex model metadata independently of their inference endpoint. A custom inference `base_url` alone should not opt API-key sessions into remote catalog discovery. ## What changed - Add `model_catalog_url` to provider configuration and thread-config serialization. Fetch the full catalog URL with provider authentication, headers, query parameters, and `client_version`, while preserving inference routing. - Require an explicit catalog URL for API-key discovery with a custom base URL. Keep default OpenAI discovery on the Codex backend, and apply `api_key_model_discovery` gating to provider API keys and cached catalogs. - Include the catalog URL in cache identity so different catalogs do not share cached metadata. - Limit explicitly configured catalog responses to 1 MiB, reject redirects, and suppress URL and response diagnostics that could expose credentials. ## Testing Add coverage for catalog configuration round-trips, query encoding, authentication and header reuse, cache identity and discovery gating, oversized responses, and redirect rejection without credential forwarding. Add an integration test verifying that catalog model metadata and instructions reach conversation requests. GitOrigin-RevId: 7f6ac5eec257278685e1867a6bcebae6514a365a
This commit is contained in:
@@ -594,6 +594,7 @@ mod thread_processor_behavior_tests {
|
||||
let session_provider = ModelProviderInfo {
|
||||
name: "session".to_string(),
|
||||
base_url: Some("http://127.0.0.1:8061/api/codex".to_string()),
|
||||
model_catalog_url: None,
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
|
||||
@@ -39,14 +39,16 @@ use wiremock::matchers::path;
|
||||
const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10);
|
||||
const INVALID_REQUEST_ERROR_CODE: i64 = -32600;
|
||||
|
||||
#[test_case(None, false; "default off")]
|
||||
#[test_case(None, true; "app rollout")]
|
||||
#[test_case(Some(false), true; "user opt out")]
|
||||
#[test_case(Some(true), false; "user opt in")]
|
||||
#[test_case(None, false, true; "default off")]
|
||||
#[test_case(None, true, true; "app rollout")]
|
||||
#[test_case(Some(false), true, true; "user opt out")]
|
||||
#[test_case(Some(true), false, true; "user opt in")]
|
||||
#[test_case(Some(true), false, false; "base URL without catalog opt in")]
|
||||
#[tokio::test]
|
||||
async fn api_key_model_discovery_startup_enablement_respects_user_config(
|
||||
user_enablement: Option<bool>,
|
||||
app_enablement: bool,
|
||||
catalog_opt_in: bool,
|
||||
) -> Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
let mut remote_model = codex_models_manager::bundled_models_response()?
|
||||
@@ -69,9 +71,24 @@ async fn api_key_model_discovery_startup_enablement_respects_user_config(
|
||||
let feature_config = user_enablement
|
||||
.map(|enabled| format!("[features]\napi_key_model_discovery = {enabled}\n"))
|
||||
.unwrap_or_default();
|
||||
let catalog_config = if catalog_opt_in {
|
||||
format!("model_catalog_url = \"{server_uri}/v1/models\"\n")
|
||||
} else {
|
||||
String::new()
|
||||
};
|
||||
std::fs::write(
|
||||
codex_home.path().join("config.toml"),
|
||||
format!("openai_base_url = \"{server_uri}/v1\"\n{feature_config}"),
|
||||
format!(
|
||||
r#"
|
||||
model_provider = "catalog-test"
|
||||
{feature_config}
|
||||
[model_providers.catalog-test]
|
||||
name = "OpenAI"
|
||||
base_url = "{server_uri}/v1"
|
||||
requires_openai_auth = true
|
||||
{catalog_config}
|
||||
"#
|
||||
),
|
||||
)?;
|
||||
login_with_api_key(
|
||||
codex_home.path(),
|
||||
@@ -113,7 +130,7 @@ async fn api_key_model_discovery_startup_enablement_respects_user_config(
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
let enabled = user_enablement.unwrap_or(app_enablement);
|
||||
let enabled = user_enablement.unwrap_or(app_enablement) && catalog_opt_in;
|
||||
let expected = if enabled { &remote } else { &bundled };
|
||||
assert_eq!(
|
||||
response,
|
||||
@@ -135,7 +152,8 @@ async fn api_key_model_discovery_startup_enablement_respects_user_config(
|
||||
.received_requests()
|
||||
.await
|
||||
.expect("request recording is enabled")
|
||||
.is_empty()
|
||||
.iter()
|
||||
.all(|request| request.url.path() != "/v1/models")
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
@@ -344,9 +362,14 @@ async fn list_models_uses_remote_catalog_as_source_of_truth(
|
||||
model = "mock-model"
|
||||
approval_policy = "never"
|
||||
sandbox_mode = "read-only"
|
||||
openai_base_url = "{server_uri}/v1"
|
||||
model_provider = "catalog-test"
|
||||
[features]
|
||||
api_key_model_discovery = true
|
||||
[model_providers.catalog-test]
|
||||
name = "OpenAI"
|
||||
base_url = "{server_uri}/v1"
|
||||
requires_openai_auth = true
|
||||
model_catalog_url = "{server_uri}/v1/models"
|
||||
"#
|
||||
),
|
||||
)?;
|
||||
|
||||
@@ -11,6 +11,7 @@ use http::HeaderMap;
|
||||
use http::Method;
|
||||
use http::header::ETAG;
|
||||
use std::sync::Arc;
|
||||
use url::Url;
|
||||
|
||||
pub struct ModelsClient<T: HttpTransport> {
|
||||
session: EndpointSession<T>,
|
||||
@@ -44,17 +45,36 @@ impl<T: HttpTransport> ModelsClient<T> {
|
||||
request.url
|
||||
}
|
||||
|
||||
/// Builds a full catalog URL, preserving provider routing parameters and client version.
|
||||
pub fn catalog_request_url(
|
||||
provider: &Provider,
|
||||
catalog_url: &str,
|
||||
client_version: &str,
|
||||
) -> Result<String, ApiError> {
|
||||
let invalid = |message: &str| ApiError::InvalidRequest {
|
||||
message: message.to_string(),
|
||||
};
|
||||
let mut url = Url::parse(catalog_url)
|
||||
.map_err(|_| invalid("model_catalog_url must be an absolute URL"))?;
|
||||
{
|
||||
let mut query = url.query_pairs_mut();
|
||||
if let Some(params) = &provider.query_params {
|
||||
query.extend_pairs(params);
|
||||
}
|
||||
query.append_pair("client_version", client_version);
|
||||
}
|
||||
Ok(url.into())
|
||||
}
|
||||
|
||||
/// Fetches and decodes a catalog, optionally bounding response bytes before decoding.
|
||||
pub async fn list_models(
|
||||
&self,
|
||||
request_url: String,
|
||||
extra_headers: HeaderMap,
|
||||
response_body_limit_bytes: Option<usize>,
|
||||
) -> Result<(Vec<ModelInfo>, Option<String>), ApiError> {
|
||||
let (body, header_etag) = self
|
||||
.list_models_raw(
|
||||
request_url,
|
||||
extra_headers,
|
||||
/*response_body_limit_bytes*/ None,
|
||||
)
|
||||
.list_models_raw(request_url, extra_headers, response_body_limit_bytes)
|
||||
.await?;
|
||||
let ModelsResponse { models } =
|
||||
serde_json::from_slice::<ModelsResponse>(&body).map_err(|e| {
|
||||
@@ -207,6 +227,39 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn catalog_request_url_preserves_routing_and_encodes_queries() {
|
||||
let mut provider = provider("https://gateway.example/v1");
|
||||
provider.query_params = Some(std::collections::HashMap::from([(
|
||||
"api-version".to_string(),
|
||||
"2026 09".to_string(),
|
||||
)]));
|
||||
let url = ModelsClient::<CapturingTransport>::catalog_request_url(
|
||||
&provider,
|
||||
"https://catalog.example/codex/models?deployment=one",
|
||||
"1.2.3",
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
url,
|
||||
"https://catalog.example/codex/models?deployment=one&api-version=2026+09&client_version=1.2.3"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn catalog_request_url_rejects_relative_urls() {
|
||||
let provider = provider("https://gateway.example/v1");
|
||||
for catalog_url in ["/codex/models", "codex/models"] {
|
||||
let error = ModelsClient::<CapturingTransport>::catalog_request_url(
|
||||
&provider,
|
||||
catalog_url,
|
||||
"1.2.3",
|
||||
)
|
||||
.unwrap_err();
|
||||
assert!(matches!(error, ApiError::InvalidRequest { .. }));
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn response_body_limit_survives_auth_retry_without_limiting_other_models_requests() {
|
||||
let transport = CapturingTransport::default();
|
||||
@@ -217,7 +270,7 @@ mod tests {
|
||||
let client = ModelsClient::new(transport.clone(), provider, auth.clone());
|
||||
|
||||
client
|
||||
.list_models_raw(
|
||||
.list_models(
|
||||
request_url.clone(),
|
||||
HeaderMap::new(),
|
||||
/*response_body_limit_bytes*/ Some(64),
|
||||
@@ -233,7 +286,11 @@ mod tests {
|
||||
}
|
||||
|
||||
let (models, _) = client
|
||||
.list_models(request_url, HeaderMap::new())
|
||||
.list_models(
|
||||
request_url,
|
||||
HeaderMap::new(),
|
||||
/*response_body_limit_bytes*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("ordinary request on the same client should remain unbounded");
|
||||
assert!(models.is_empty());
|
||||
@@ -268,7 +325,11 @@ mod tests {
|
||||
let client = ModelsClient::new(transport.clone(), provider, Arc::new(DummyAuth));
|
||||
|
||||
let (models, _) = client
|
||||
.list_models(request_url, HeaderMap::new())
|
||||
.list_models(
|
||||
request_url,
|
||||
HeaderMap::new(),
|
||||
/*response_body_limit_bytes*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("request should succeed");
|
||||
|
||||
@@ -327,7 +388,11 @@ mod tests {
|
||||
let client = ModelsClient::new(transport, provider, Arc::new(DummyAuth));
|
||||
|
||||
let (models, _) = client
|
||||
.list_models(request_url, HeaderMap::new())
|
||||
.list_models(
|
||||
request_url,
|
||||
HeaderMap::new(),
|
||||
/*response_body_limit_bytes*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("request should succeed");
|
||||
|
||||
@@ -352,7 +417,11 @@ mod tests {
|
||||
let client = ModelsClient::new(transport, provider, Arc::new(DummyAuth));
|
||||
|
||||
let (models, etag) = client
|
||||
.list_models(request_url, HeaderMap::new())
|
||||
.list_models(
|
||||
request_url,
|
||||
HeaderMap::new(),
|
||||
/*response_body_limit_bytes*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("request should succeed");
|
||||
|
||||
|
||||
@@ -81,7 +81,11 @@ async fn invalid_models_response_reports_bounded_decode_metadata() {
|
||||
let client = ModelsClient::new(transport, provider, Arc::new(DummyAuth));
|
||||
|
||||
let error = client
|
||||
.list_models(request_url, HeaderMap::new())
|
||||
.list_models(
|
||||
request_url,
|
||||
HeaderMap::new(),
|
||||
/*response_body_limit_bytes*/ None,
|
||||
)
|
||||
.await
|
||||
.expect_err("invalid catalog should fail to decode");
|
||||
|
||||
@@ -187,7 +191,11 @@ async fn models_client_hits_models_endpoint() {
|
||||
let client = ModelsClient::new(transport, provider, Arc::new(DummyAuth));
|
||||
|
||||
let (models, _) = client
|
||||
.list_models(request_url, HeaderMap::new())
|
||||
.list_models(
|
||||
request_url,
|
||||
HeaderMap::new(),
|
||||
/*response_body_limit_bytes*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("models request should succeed");
|
||||
|
||||
|
||||
@@ -298,6 +298,7 @@ mod tests {
|
||||
ModelProviderInfo {
|
||||
name: name.to_string(),
|
||||
base_url: Some("http://127.0.0.1:8061/api/codex".to_string()),
|
||||
model_catalog_url: None,
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
|
||||
@@ -49,6 +49,7 @@ message ModelProvider {
|
||||
bool requires_openai_auth = 16;
|
||||
bool supports_websockets = 17;
|
||||
bool supports_standalone_web_search = 18;
|
||||
optional string model_catalog_url = 19;
|
||||
}
|
||||
|
||||
message StringMap {
|
||||
|
||||
@@ -77,6 +77,8 @@ pub struct ModelProvider {
|
||||
pub supports_websockets: bool,
|
||||
#[prost(bool, tag = "18")]
|
||||
pub supports_standalone_web_search: bool,
|
||||
#[prost(string, optional, tag = "19")]
|
||||
pub model_catalog_url: ::core::option::Option<::prost::alloc::string::String>,
|
||||
}
|
||||
#[derive(Clone, PartialEq, ::prost::Message)]
|
||||
pub struct StringMap {
|
||||
|
||||
@@ -173,6 +173,7 @@ fn model_provider_from_proto(
|
||||
let info = ModelProviderInfo {
|
||||
name: provider.name,
|
||||
base_url: provider.base_url,
|
||||
model_catalog_url: provider.model_catalog_url.map(Into::into),
|
||||
env_key: provider.env_key,
|
||||
env_key_instructions: provider.env_key_instructions,
|
||||
experimental_bearer_token: provider.experimental_bearer_token.map(Into::into),
|
||||
@@ -205,6 +206,7 @@ fn model_provider_to_proto(
|
||||
let ModelProviderInfo {
|
||||
name,
|
||||
base_url,
|
||||
model_catalog_url,
|
||||
env_key,
|
||||
env_key_instructions,
|
||||
experimental_bearer_token,
|
||||
@@ -228,6 +230,7 @@ fn model_provider_to_proto(
|
||||
id: id.into(),
|
||||
name,
|
||||
base_url,
|
||||
model_catalog_url: model_catalog_url.map(RedactedString::into_inner),
|
||||
env_key,
|
||||
env_key_instructions,
|
||||
experimental_bearer_token: experimental_bearer_token.map(RedactedString::into_inner),
|
||||
@@ -473,6 +476,9 @@ mod tests {
|
||||
id: "local".to_string(),
|
||||
name: "Local".to_string(),
|
||||
base_url: Some("http://127.0.0.1:8061/api/codex".to_string()),
|
||||
model_catalog_url: Some(
|
||||
"http://127.0.0.1:8061/api/codex/models".to_string(),
|
||||
),
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
@@ -543,6 +549,7 @@ mod tests {
|
||||
ModelProviderInfo {
|
||||
name: "Local".to_string(),
|
||||
base_url: Some("http://127.0.0.1:8061/api/codex".to_string()),
|
||||
model_catalog_url: Some("http://127.0.0.1:8061/api/codex/models".into()),
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
|
||||
@@ -2549,6 +2549,10 @@
|
||||
"description": "Additional HTTP headers to include in requests to this provider where the (key, value) pairs are the header name and value.",
|
||||
"type": "object"
|
||||
},
|
||||
"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.",
|
||||
"type": "string"
|
||||
},
|
||||
"name": {
|
||||
"default": "",
|
||||
"description": "Friendly display name.",
|
||||
|
||||
@@ -74,6 +74,7 @@ async fn responses_stream_includes_subagent_header_on_review() {
|
||||
let provider = ModelProviderInfo {
|
||||
name: "mock".into(),
|
||||
base_url: Some(format!("{}/v1", server.uri())),
|
||||
model_catalog_url: None,
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
@@ -214,6 +215,7 @@ async fn responses_stream_includes_subagent_header_on_other() {
|
||||
let provider = ModelProviderInfo {
|
||||
name: "mock".into(),
|
||||
base_url: Some(format!("{}/v1", server.uri())),
|
||||
model_catalog_url: None,
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
@@ -335,6 +337,7 @@ async fn responses_respects_model_info_overrides_from_config() {
|
||||
let provider = ModelProviderInfo {
|
||||
name: "mock".into(),
|
||||
base_url: Some(format!("{}/v1", server.uri())),
|
||||
model_catalog_url: None,
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
|
||||
@@ -1604,6 +1604,7 @@ async fn send_provider_auth_request(server: &MockServer, auth: ModelProviderAuth
|
||||
let provider = ModelProviderInfo {
|
||||
name: "corp".into(),
|
||||
base_url: Some(format!("{}/v1", server.uri())),
|
||||
model_catalog_url: None,
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
@@ -3105,6 +3106,7 @@ async fn azure_responses_request_does_not_store_and_preserves_prefixed_item_ids(
|
||||
let provider = ModelProviderInfo {
|
||||
name: "azure".into(),
|
||||
base_url: Some(format!("{}/openai", server.uri())),
|
||||
model_catalog_url: None,
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
@@ -3733,6 +3735,7 @@ async fn azure_overrides_assign_properties_used_for_responses_url() {
|
||||
let provider = ModelProviderInfo {
|
||||
name: "custom".to_string(),
|
||||
base_url: Some(format!("{}/openai", server.uri())),
|
||||
model_catalog_url: None,
|
||||
// Reuse the existing environment variable to avoid using unsafe code
|
||||
env_key: Some(EXISTING_ENV_VAR_WITH_NON_EMPTY_VALUE.to_string()),
|
||||
experimental_bearer_token: None,
|
||||
@@ -3818,6 +3821,7 @@ async fn env_var_overrides_loaded_auth() {
|
||||
let provider = ModelProviderInfo {
|
||||
name: ModelProviderInfo::create_openai_provider(/*base_url*/ None).name,
|
||||
base_url: Some(format!("{}/openai", server.uri())),
|
||||
model_catalog_url: None,
|
||||
// Reuse the existing environment variable to avoid using unsafe code
|
||||
env_key: Some(EXISTING_ENV_VAR_WITH_NON_EMPTY_VALUE.to_string()),
|
||||
query_params: Some(std::collections::HashMap::from([(
|
||||
|
||||
@@ -2721,6 +2721,7 @@ fn websocket_provider_with_connect_timeout(
|
||||
ModelProviderInfo {
|
||||
name: "mock-ws".into(),
|
||||
base_url: Some(format!("{}/v1", server.uri())),
|
||||
model_catalog_url: None,
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
|
||||
@@ -310,6 +310,11 @@ async fn auth_and_provider_switches_do_not_reuse_chatgpt_catalog() -> Result<()>
|
||||
.with_home(home.clone())
|
||||
.with_auth(CodexAuth::from_api_key("api-key"))
|
||||
.with_config(|config| {
|
||||
config.model_provider.model_catalog_url = config
|
||||
.model_provider
|
||||
.base_url
|
||||
.as_ref()
|
||||
.map(|base_url| format!("{base_url}/models").into());
|
||||
config
|
||||
.features
|
||||
.enable(codex_features::Feature::ApiKeyModelDiscovery)
|
||||
|
||||
@@ -933,6 +933,11 @@ async fn remote_models_apply_legacy_instructions(auth: CodexAuth) -> Result<()>
|
||||
.features
|
||||
.enable(Feature::ApiKeyModelDiscovery)
|
||||
.expect("enable API-key model discovery");
|
||||
config.model_provider.model_catalog_url = config
|
||||
.model_provider
|
||||
.base_url
|
||||
.as_ref()
|
||||
.map(|base_url| format!("{base_url}/models").into());
|
||||
config.update_plan_enabled = true;
|
||||
config.model = Some("gpt-5.2".to_string());
|
||||
});
|
||||
@@ -1478,3 +1483,64 @@ fn test_remote_model_with_policy(
|
||||
experimental_supported_tools: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn model_catalog_url_supplies_conversation_model_and_instructions() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
let server = MockServer::start().await;
|
||||
let instructions = "Use the gateway's reviewed conversation instructions.";
|
||||
let mut model = codex_models_manager::model_info::model_info_from_slug("gateway-conversation");
|
||||
model.visibility = ModelVisibility::List;
|
||||
model.supported_in_api = true;
|
||||
model.support_verbosity = false;
|
||||
model.used_fallback_model_metadata = false;
|
||||
model
|
||||
.model_messages
|
||||
.as_mut()
|
||||
.expect("fallback instructions")
|
||||
.instructions_template = Some(instructions.to_string());
|
||||
let catalog = ModelsResponse {
|
||||
models: vec![model],
|
||||
};
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/codex/models"))
|
||||
.respond_with(ResponseTemplate::new(/*s*/ 200).set_body_json(&catalog))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
let response = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
|
||||
)
|
||||
.await;
|
||||
let catalog_url = format!("{}/codex/models", server.uri());
|
||||
let test = test_codex()
|
||||
.with_auth(CodexAuth::from_api_key("gateway-api-key"))
|
||||
.with_model("gateway-conversation")
|
||||
.with_config(move |config| {
|
||||
config.model_provider.name = "Gateway".to_string();
|
||||
config.model_provider.model_catalog_url = Some(catalog_url.into());
|
||||
config
|
||||
.features
|
||||
.enable(Feature::ApiKeyModelDiscovery)
|
||||
.expect("enable API-key discovery");
|
||||
config.model_verbosity = Some(codex_protocol::config_types::Verbosity::High);
|
||||
})
|
||||
.build_with_auto_env(&server)
|
||||
.await?;
|
||||
let received = test
|
||||
.thread_manager
|
||||
.get_models_manager()
|
||||
.raw_model_catalog(
|
||||
RefreshStrategy::OnlineIfUncached,
|
||||
codex_core::test_support::default_http_client_factory(),
|
||||
)
|
||||
.await;
|
||||
assert_eq!(received.models, catalog.models);
|
||||
test.submit_turn("hello").await?;
|
||||
let request = response.single_request().body_json();
|
||||
assert_eq!(request["model"], "gateway-conversation");
|
||||
assert_eq!(request["instructions"], instructions);
|
||||
assert_eq!(request["text"].get("verbosity"), None);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -66,6 +66,7 @@ async fn continue_after_stream_error() {
|
||||
let provider = ModelProviderInfo {
|
||||
name: "mock-openai".into(),
|
||||
base_url: Some(format!("{}/v1", server.uri())),
|
||||
model_catalog_url: None,
|
||||
env_key: Some("PATH".into()),
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
|
||||
@@ -48,6 +48,7 @@ async fn retries_on_early_close() {
|
||||
let model_provider = ModelProviderInfo {
|
||||
name: "openai".into(),
|
||||
base_url: Some(format!("{}/v1", server.uri())),
|
||||
model_catalog_url: None,
|
||||
// Environment variable that should exist in the test environment.
|
||||
// ModelClient will return an error if the environment variable for the
|
||||
// provider is not set.
|
||||
|
||||
@@ -22,6 +22,7 @@ use codex_api::TransportError;
|
||||
use codex_api::build_session_headers;
|
||||
use codex_http_client::ClientRouteClass;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_login::default_client::ClientRedirectPolicy;
|
||||
use codex_login::default_client::add_originator_header;
|
||||
use codex_login::default_client::create_client_for_route_async;
|
||||
use codex_login::default_client::default_headers;
|
||||
@@ -216,6 +217,7 @@ impl ConnectionPool {
|
||||
self.config.http_client_factory.clone(),
|
||||
url,
|
||||
ClientRouteClass::Api,
|
||||
ClientRedirectPolicy::Default,
|
||||
)
|
||||
.await
|
||||
.map_err(|error| {
|
||||
|
||||
@@ -40,6 +40,12 @@ impl HttpClient {
|
||||
Self::from_parts(inner, RequestLogging::Disabled)
|
||||
}
|
||||
|
||||
/// Suppresses URL and response-header diagnostics while preserving this client's routing.
|
||||
pub fn without_request_logging(mut self) -> Self {
|
||||
self.request_logging = RequestLogging::Disabled;
|
||||
self
|
||||
}
|
||||
|
||||
pub(crate) fn from_parts(inner: reqwest::Client, request_logging: RequestLogging) -> Self {
|
||||
Self {
|
||||
inner,
|
||||
|
||||
@@ -85,6 +85,7 @@ impl ReqwestTransport {
|
||||
}
|
||||
|
||||
fn map_error(err: reqwest::Error) -> TransportError {
|
||||
let err = err.without_url();
|
||||
if err.is_connect() {
|
||||
TransportError::Connection(err.without_url())
|
||||
} else if err.is_timeout() {
|
||||
|
||||
@@ -18,10 +18,9 @@ async fn enabled_request_logging_emits_transport_url_and_body() {
|
||||
|
||||
#[tokio::test]
|
||||
async fn disabled_request_logging_suppresses_transport_url_and_body() {
|
||||
let logs = capture_transport_logs(HttpClient::new_without_request_logging(
|
||||
test_reqwest_client(),
|
||||
))
|
||||
.await;
|
||||
let logs =
|
||||
capture_transport_logs(HttpClient::new(test_reqwest_client()).without_request_logging())
|
||||
.await;
|
||||
|
||||
assert!(logs.contains("log capture sentinel"));
|
||||
assert!(!logs.contains("url-secret"));
|
||||
|
||||
@@ -273,6 +273,7 @@ pub async fn create_client_for_route_async(
|
||||
http_client_factory: HttpClientFactory,
|
||||
request_url: String,
|
||||
route_class: ClientRouteClass,
|
||||
redirect_policy: ClientRedirectPolicy,
|
||||
) -> std::io::Result<HttpClient> {
|
||||
let permit = ROUTE_AWARE_CLIENT_BUILD_PERMIT
|
||||
.acquire()
|
||||
@@ -284,7 +285,7 @@ pub async fn create_client_for_route_async(
|
||||
&http_client_factory,
|
||||
&request_url,
|
||||
route_class,
|
||||
ClientRedirectPolicy::Default,
|
||||
redirect_policy,
|
||||
)
|
||||
.map_err(std::io::Error::from)
|
||||
})
|
||||
|
||||
@@ -61,6 +61,7 @@ mod tests {
|
||||
let provider = ModelProviderInfo {
|
||||
name: "Custom".to_string(),
|
||||
base_url: None,
|
||||
model_catalog_url: None,
|
||||
env_key: Some("sk-should-not-leak".to_string()),
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
|
||||
@@ -137,6 +137,9 @@ pub struct ModelProviderInfo {
|
||||
pub name: String,
|
||||
/// Base URL for the provider's OpenAI-compatible API.
|
||||
pub base_url: Option<String>,
|
||||
/// Optional full URL for a Codex-native model catalog. When unset, OpenAI discovery
|
||||
/// uses the Codex backend unless `base_url` overrides the inference endpoint.
|
||||
pub model_catalog_url: Option<RedactedString>,
|
||||
/// Environment variable that stores the user's API key for this provider.
|
||||
pub env_key: Option<String>,
|
||||
|
||||
@@ -510,6 +513,7 @@ other non-default provider fields are not supported"
|
||||
ModelProviderInfo {
|
||||
name: OPENAI_PROVIDER_NAME.into(),
|
||||
base_url,
|
||||
model_catalog_url: None,
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
@@ -554,6 +558,7 @@ other non-default provider fields are not supported"
|
||||
// this is unset. A configured value is therefore unambiguously an
|
||||
// endpoint override.
|
||||
base_url: None,
|
||||
model_catalog_url: None,
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
@@ -734,6 +739,7 @@ pub fn create_oss_provider_with_base_url(base_url: &str, wire_api: WireApi) -> M
|
||||
ModelProviderInfo {
|
||||
name: "gpt-oss".into(),
|
||||
base_url: Some(base_url.into()),
|
||||
model_catalog_url: None,
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
|
||||
@@ -62,6 +62,7 @@ base_url = "http://localhost:11434/v1"
|
||||
let expected_provider = ModelProviderInfo {
|
||||
name: "Ollama".into(),
|
||||
base_url: Some("http://localhost:11434/v1".into()),
|
||||
model_catalog_url: None,
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
@@ -96,6 +97,7 @@ query_params = { api-version = "2025-04-01-preview" }
|
||||
let expected_provider = ModelProviderInfo {
|
||||
name: "Azure".into(),
|
||||
base_url: Some("https://xxxxx.openai.azure.com/openai".into()),
|
||||
model_catalog_url: None,
|
||||
env_key: Some("AZURE_OPENAI_API_KEY".into()),
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
@@ -134,6 +136,7 @@ supports_standalone_web_search = true
|
||||
let expected_provider = ModelProviderInfo {
|
||||
name: "Example".into(),
|
||||
base_url: Some("https://example.com".into()),
|
||||
model_catalog_url: None,
|
||||
env_key: Some("API_KEY".into()),
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
@@ -317,6 +320,7 @@ fn test_create_amazon_bedrock_provider() {
|
||||
ModelProviderInfo {
|
||||
name: "Amazon Bedrock".to_string(),
|
||||
base_url: None,
|
||||
model_catalog_url: None,
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
@@ -814,3 +818,34 @@ refresh_interval_ms = 0
|
||||
assert_eq!(auth.refresh_interval_ms, 0);
|
||||
assert_eq!(auth.refresh_interval(), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn model_catalog_url_deserializes_without_changing_inference_routing() {
|
||||
let provider: ModelProviderInfo = toml::from_str(
|
||||
r#"
|
||||
name = "Gateway"
|
||||
base_url = "https://gateway.example/v1"
|
||||
model_catalog_url = "https://gateway.example/codex/catalog?token=catalog-secret"
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
assert!(!format!("{provider:?}").contains("catalog-secret"));
|
||||
assert_eq!(
|
||||
provider,
|
||||
ModelProviderInfo {
|
||||
name: "Gateway".to_string(),
|
||||
base_url: Some("https://gateway.example/v1".to_string()),
|
||||
model_catalog_url: Some(
|
||||
"https://gateway.example/codex/catalog?token=catalog-secret".into()
|
||||
),
|
||||
..ModelProviderInfo::default()
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
provider
|
||||
.to_api_provider(Some(AuthMode::ApiKey))
|
||||
.unwrap()
|
||||
.base_url,
|
||||
"https://gateway.example/v1"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -20,6 +20,7 @@ use codex_login::AuthManager;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_login::GatewayAuthManager;
|
||||
use codex_login::collect_auth_env_telemetry;
|
||||
use codex_login::default_client::ClientRedirectPolicy;
|
||||
use codex_login::default_client::create_client_for_route_async;
|
||||
use codex_model_provider_info::CHATGPT_CODEX_BASE_URL;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
@@ -41,6 +42,8 @@ use crate::combined_auth::compose_auth;
|
||||
|
||||
const MODELS_REFRESH_TIMEOUT: Duration = Duration::from_secs(5);
|
||||
const MODELS_ENDPOINT: &str = "/models";
|
||||
// Bound downloads from explicitly configured catalogs before decoding or caching them.
|
||||
const MAX_MODEL_CATALOG_BYTES: usize = 1024 * 1024;
|
||||
|
||||
/// Provider-owned OpenAI-compatible `/models` endpoint.
|
||||
#[derive(Debug)]
|
||||
@@ -57,11 +60,16 @@ impl OpenAiModelsEndpoint {
|
||||
auth_manager: Option<Arc<AuthManager>>,
|
||||
gateway_auth_manager: Option<Result<Arc<GatewayAuthManager>, String>>,
|
||||
) -> Self {
|
||||
let redirect_policy = if provider_info.model_catalog_url.is_some() {
|
||||
ClientRedirectPolicy::Reject
|
||||
} else {
|
||||
ClientRedirectPolicy::Default
|
||||
};
|
||||
Self {
|
||||
provider_info,
|
||||
auth_manager,
|
||||
gateway_auth_manager,
|
||||
transport_builder: Arc::new(RouteAwareModelsTransportBuilder),
|
||||
transport_builder: Arc::new(RouteAwareModelsTransportBuilder { redirect_policy }),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -90,9 +98,10 @@ impl OpenAiModelsEndpoint {
|
||||
let identity = crate::models_identity::identity(&self.provider_info, auth.as_ref())?;
|
||||
let auth_mode = auth.as_ref().map(CodexAuth::auth_mode);
|
||||
let mut api_provider = self.provider_info.to_api_provider(auth_mode)?;
|
||||
if auth.as_ref().is_some_and(CodexAuth::is_api_key_auth)
|
||||
if (auth.as_ref().is_some_and(CodexAuth::is_api_key_auth) || self.has_provider_api_key())
|
||||
&& self.supports_api_key_models()
|
||||
&& self.provider_info.base_url.is_none()
|
||||
&& self.provider_info.model_catalog_url.is_none()
|
||||
{
|
||||
// Codex metadata is served by the Codex backend, not the public /v1/models API.
|
||||
api_provider.base_url = CHATGPT_CODEX_BASE_URL.to_string();
|
||||
@@ -104,8 +113,15 @@ impl OpenAiModelsEndpoint {
|
||||
)
|
||||
.await?;
|
||||
let api_auth = resolved.auth;
|
||||
let request_url =
|
||||
ModelsClient::<ReqwestTransport>::request_url(&api_provider, client_version);
|
||||
let request_url = match self.provider_info.model_catalog_url.as_deref() {
|
||||
Some(catalog_url) => ModelsClient::<ReqwestTransport>::catalog_request_url(
|
||||
&api_provider,
|
||||
catalog_url,
|
||||
client_version,
|
||||
)
|
||||
.map_err(map_api_error)?,
|
||||
None => ModelsClient::<ReqwestTransport>::request_url(&api_provider, client_version),
|
||||
};
|
||||
let auth_telemetry = auth_header_telemetry(api_auth.as_ref());
|
||||
let agent_identity_telemetry = if let Some(CodexAuth::AgentIdentity(auth)) = auth.as_ref() {
|
||||
Some(agent_identity_telemetry(auth))
|
||||
@@ -113,6 +129,7 @@ impl OpenAiModelsEndpoint {
|
||||
None
|
||||
};
|
||||
let request_telemetry: Arc<dyn RequestTelemetry> = Arc::new(ModelsRequestTelemetry {
|
||||
include_response_debug: self.provider_info.model_catalog_url.is_none(),
|
||||
auth_mode: auth_mode.map(|mode| TelemetryAuthMode::from(mode).to_string()),
|
||||
auth_header_attached: auth_telemetry.attached,
|
||||
auth_header_name: auth_telemetry.name,
|
||||
@@ -126,10 +143,30 @@ impl OpenAiModelsEndpoint {
|
||||
.await?;
|
||||
let client = ModelsClient::new(transport, api_provider, api_auth)
|
||||
.with_telemetry(Some(request_telemetry));
|
||||
let response_body_limit_bytes = self
|
||||
.provider_info
|
||||
.model_catalog_url
|
||||
.as_ref()
|
||||
.map(|_| MAX_MODEL_CATALOG_BYTES);
|
||||
client
|
||||
.list_models(request_url, HeaderMap::new())
|
||||
.list_models(request_url, HeaderMap::new(), response_body_limit_bytes)
|
||||
.await
|
||||
.map_err(map_api_error)
|
||||
.map_err(|mut error| {
|
||||
if self.provider_info.model_catalog_url.is_some()
|
||||
&& let codex_api::ApiError::Transport(TransportError::Http {
|
||||
url,
|
||||
headers,
|
||||
body,
|
||||
..
|
||||
}) = &mut error
|
||||
{
|
||||
// Provider diagnostics may echo URL credentials or other secrets.
|
||||
*url = None;
|
||||
*headers = None;
|
||||
*body = None;
|
||||
}
|
||||
map_api_error(error)
|
||||
})
|
||||
})
|
||||
.await
|
||||
.map_err(|_| CodexErr::RequestTimeout)??;
|
||||
@@ -151,7 +188,13 @@ impl OpenAiModelsEndpoint {
|
||||
|
||||
impl ModelsEndpointClient for OpenAiModelsEndpoint {
|
||||
fn supports_api_key_models(&self) -> bool {
|
||||
self.provider_info.is_openai()
|
||||
self.provider_info.model_catalog_url.is_some()
|
||||
|| (self.provider_info.is_openai() && self.provider_info.base_url.is_none())
|
||||
}
|
||||
|
||||
fn has_provider_api_key(&self) -> bool {
|
||||
self.provider_info.env_key.is_some()
|
||||
|| self.provider_info.experimental_bearer_token.is_some()
|
||||
}
|
||||
|
||||
fn identity(&self) -> Option<String> {
|
||||
@@ -198,7 +241,9 @@ trait ModelsTransportBuilder: fmt::Debug + Send + Sync {
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct RouteAwareModelsTransportBuilder;
|
||||
struct RouteAwareModelsTransportBuilder {
|
||||
redirect_policy: ClientRedirectPolicy,
|
||||
}
|
||||
|
||||
impl ModelsTransportBuilder for RouteAwareModelsTransportBuilder {
|
||||
fn build(
|
||||
@@ -206,16 +251,27 @@ impl ModelsTransportBuilder for RouteAwareModelsTransportBuilder {
|
||||
http_client_factory: HttpClientFactory,
|
||||
request_url: String,
|
||||
) -> ModelsTransportFuture<'_> {
|
||||
let redirect_policy = self.redirect_policy;
|
||||
Box::pin(async move {
|
||||
create_client_for_route_async(http_client_factory, request_url, ClientRouteClass::Api)
|
||||
.await
|
||||
.map(ReqwestTransport::from_http_client)
|
||||
let client = create_client_for_route_async(
|
||||
http_client_factory,
|
||||
request_url,
|
||||
ClientRouteClass::Api,
|
||||
redirect_policy,
|
||||
)
|
||||
.await?;
|
||||
let client = match redirect_policy {
|
||||
ClientRedirectPolicy::Default => client,
|
||||
ClientRedirectPolicy::Reject => client.without_request_logging(),
|
||||
};
|
||||
Ok(ReqwestTransport::from_http_client(client))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct ModelsRequestTelemetry {
|
||||
include_response_debug: bool,
|
||||
auth_mode: Option<String>,
|
||||
auth_header_attached: bool,
|
||||
auth_header_name: Option<&'static str>,
|
||||
@@ -234,6 +290,7 @@ impl RequestTelemetry for ModelsRequestTelemetry {
|
||||
let success = status.is_some_and(|code| code.is_success()) && error.is_none();
|
||||
let error_message = error.map(telemetry_transport_error_message);
|
||||
let response_debug = error
|
||||
.filter(|_| self.include_response_debug)
|
||||
.map(extract_response_debug_context)
|
||||
.unwrap_or_default();
|
||||
let status = status.map(|status| status.as_u16());
|
||||
@@ -331,6 +388,8 @@ mod tests {
|
||||
use codex_models_manager::manager::RefreshStrategy;
|
||||
use codex_protocol::auth::AuthMode;
|
||||
use codex_protocol::config_types::ModelProviderAuthInfo;
|
||||
use codex_protocol::error::CodexErrorDetails;
|
||||
use codex_protocol::openai_models::ModelVisibility;
|
||||
use codex_protocol::openai_models::ModelsResponse;
|
||||
use pretty_assertions::assert_eq;
|
||||
use wiremock::Mock;
|
||||
@@ -390,7 +449,7 @@ mod tests {
|
||||
(
|
||||
"OpenAI",
|
||||
Some("https://example.com/codex"),
|
||||
Some("https://example.com/codex/models"),
|
||||
None,
|
||||
"https://example.com/codex",
|
||||
),
|
||||
(
|
||||
@@ -434,6 +493,41 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn provider_api_key_without_endpoint_overrides_uses_codex_backend() {
|
||||
for auth in [
|
||||
None,
|
||||
Some(CodexAuth::create_dummy_chatgpt_auth_for_testing()),
|
||||
] {
|
||||
let capture = Arc::new(CaptureModelsUrl(Mutex::new(/*t*/ None)));
|
||||
let auth_manager = auth.map(AuthManager::from_auth_for_testing);
|
||||
let endpoint = Arc::new(OpenAiModelsEndpoint {
|
||||
provider_info: ModelProviderInfo {
|
||||
experimental_bearer_token: Some("provider-key".into()),
|
||||
..ModelProviderInfo::create_openai_provider(/*base_url*/ None)
|
||||
},
|
||||
auth_manager: auth_manager.clone(),
|
||||
gateway_auth_manager: None,
|
||||
transport_builder: capture.clone(),
|
||||
});
|
||||
let manager = OpenAiModelsManager::new_without_cache(endpoint, auth_manager);
|
||||
manager.set_api_key_model_discovery_enabled(/*enabled*/ true);
|
||||
manager
|
||||
.raw_model_catalog(
|
||||
RefreshStrategy::Online,
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
)
|
||||
.await;
|
||||
assert_eq!(
|
||||
*capture.0.lock().unwrap(),
|
||||
Some(format!(
|
||||
"{CHATGPT_CODEX_BASE_URL}/models?client_version={}",
|
||||
codex_models_manager::client_version_to_whole(),
|
||||
))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn provider_info_with_command_auth() -> ModelProviderInfo {
|
||||
ModelProviderInfo {
|
||||
auth: Some(ModelProviderAuthInfo {
|
||||
@@ -671,4 +765,153 @@ mod tests {
|
||||
vec!["Bearer token-2", "Bearer token-5", "Bearer token-9"]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn explicit_catalog_reuses_auth_headers_and_query_parameters() {
|
||||
for auth in [
|
||||
CodexAuth::create_dummy_chatgpt_auth_for_testing(),
|
||||
CodexAuth::from_api_key("test-key"),
|
||||
] {
|
||||
let server = MockServer::start().await;
|
||||
let mut model =
|
||||
codex_models_manager::model_info::model_info_from_slug("provider-model");
|
||||
model.visibility = ModelVisibility::List;
|
||||
model.supported_in_api = true;
|
||||
model.used_fallback_model_metadata = false;
|
||||
let expected = ModelsResponse {
|
||||
models: vec![model],
|
||||
};
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/codex/models"))
|
||||
.and(header(
|
||||
"authorization",
|
||||
format!("Bearer {}", auth.get_token().unwrap()),
|
||||
))
|
||||
.and(header("x-provider-header", "preserved"))
|
||||
.and(query_param("deployment", "one"))
|
||||
.and(query_param("api-version", "2026-09"))
|
||||
.respond_with(ResponseTemplate::new(/*s*/ 200).set_body_json(&expected))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
let auth_manager = AuthManager::from_auth_for_testing(auth);
|
||||
// The parallel residency test must not change this catalog's cache identity.
|
||||
let provider = ModelProviderInfo {
|
||||
model_catalog_url: Some(
|
||||
format!("{}/codex/models?deployment=one", server.uri()).into(),
|
||||
),
|
||||
query_params: Some(std::collections::HashMap::from([(
|
||||
"api-version".to_string(),
|
||||
"2026-09".into(),
|
||||
)])),
|
||||
http_headers: Some(std::collections::HashMap::from([
|
||||
("x-provider-header".to_string(), "preserved".into()),
|
||||
(
|
||||
codex_login::default_client::RESIDENCY_HEADER_NAME.to_string(),
|
||||
"us".into(),
|
||||
),
|
||||
])),
|
||||
..ModelProviderInfo::create_openai_provider(Some(format!("{}/v1", server.uri())))
|
||||
};
|
||||
let home = tempfile::tempdir().unwrap();
|
||||
let endpoint = Arc::new(OpenAiModelsEndpoint::new(
|
||||
provider,
|
||||
Some(auth_manager.clone()),
|
||||
/*gateway_auth_manager*/ None,
|
||||
));
|
||||
let manager = OpenAiModelsManager::new(
|
||||
home.path().to_path_buf(),
|
||||
endpoint,
|
||||
Some(auth_manager.clone()),
|
||||
);
|
||||
// SIWC does not require the API-key rollout flag.
|
||||
manager.set_api_key_model_discovery_enabled(
|
||||
auth_manager.auth_mode() == Some(codex_protocol::auth::AuthMode::ApiKey),
|
||||
);
|
||||
for _ in 0..2 {
|
||||
assert_eq!(
|
||||
manager
|
||||
.raw_model_catalog(
|
||||
RefreshStrategy::OnlineIfUncached,
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault)
|
||||
)
|
||||
.await,
|
||||
expected
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn explicit_catalog_rejects_oversized_response() {
|
||||
let server = MockServer::start().await;
|
||||
let mut body = br#"{"models":[]}"#.to_vec();
|
||||
body.resize(MAX_MODEL_CATALOG_BYTES + 1, b' ');
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/codex/models"))
|
||||
.respond_with(ResponseTemplate::new(/*s*/ 200).set_body_bytes(body))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
let endpoint = OpenAiModelsEndpoint::new(
|
||||
ModelProviderInfo {
|
||||
model_catalog_url: Some(format!("{}/codex/models", server.uri()).into()),
|
||||
..ModelProviderInfo::default()
|
||||
},
|
||||
/*auth_manager*/ None,
|
||||
/*gateway_auth_manager*/ None,
|
||||
);
|
||||
let error = endpoint
|
||||
.list_models(
|
||||
"1.2.3",
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
)
|
||||
.await
|
||||
.expect_err("oversized catalog should be rejected");
|
||||
let CodexErrorDetails::InvalidRequest(message) = error.details() else {
|
||||
panic!("expected a response-size error, got {error:?}");
|
||||
};
|
||||
assert_eq!(
|
||||
message,
|
||||
&format!("response body exceeds the {MAX_MODEL_CATALOG_BYTES} byte limit")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn explicit_catalog_rejects_redirects_without_forwarding_provider_credentials() {
|
||||
let server = MockServer::start().await;
|
||||
let destination = MockServer::start().await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/codex/models"))
|
||||
.respond_with(
|
||||
ResponseTemplate::new(/*s*/ 302)
|
||||
.insert_header("location", format!("{}/codex/models", destination.uri()))
|
||||
.set_body_string("catalog-secret"),
|
||||
)
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
let endpoint = OpenAiModelsEndpoint::new(
|
||||
ModelProviderInfo {
|
||||
base_url: Some(server.uri()),
|
||||
model_catalog_url: Some(
|
||||
format!("{}/codex/models?token=catalog-secret", server.uri()).into(),
|
||||
),
|
||||
experimental_bearer_token: Some("provider-key".into()),
|
||||
..ModelProviderInfo::default()
|
||||
},
|
||||
/*auth_manager*/ None,
|
||||
/*gateway_auth_manager*/ None,
|
||||
);
|
||||
let error = endpoint
|
||||
.list_models(
|
||||
"1.2.3",
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
)
|
||||
.await
|
||||
.expect_err("redirect should be rejected");
|
||||
assert!(!error.to_string().contains("catalog-secret"));
|
||||
assert!(!format!("{error:?}").contains("catalog-secret"));
|
||||
assert_eq!(destination.received_requests().await.unwrap().len(), 0);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,7 +14,7 @@ pub(crate) fn identity(
|
||||
provider_info: &ModelProviderInfo,
|
||||
auth: Option<&CodexAuth>,
|
||||
) -> CoreResult<String> {
|
||||
let mut provider = provider_info.to_api_provider(auth.map(CodexAuth::auth_mode))?;
|
||||
let mut api_provider = provider_info.to_api_provider(auth.map(CodexAuth::auth_mode))?;
|
||||
let mut digest = Sha256::new();
|
||||
// Length-prefix every field to avoid ambiguity between adjacent values.
|
||||
let mut field = |value: &[u8]| {
|
||||
@@ -22,9 +22,15 @@ pub(crate) fn identity(
|
||||
digest.update(value);
|
||||
};
|
||||
field(b"models-cache-v1");
|
||||
field(provider.name.as_bytes());
|
||||
field(provider.base_url.as_bytes());
|
||||
let mut query: Vec<_> = provider.query_params.iter().flatten().collect();
|
||||
field(api_provider.name.as_bytes());
|
||||
field(api_provider.base_url.as_bytes());
|
||||
// Preserve existing cache identities when no explicit catalog is configured.
|
||||
// The API provider resolves base_url defaults; model_catalog_url stays on ModelProviderInfo.
|
||||
if let Some(catalog_url) = &provider_info.model_catalog_url {
|
||||
field(b"model_catalog_url");
|
||||
field(catalog_url.as_bytes());
|
||||
}
|
||||
let mut query: Vec<_> = api_provider.query_params.iter().flatten().collect();
|
||||
query.sort();
|
||||
field(&(query.len() as u64).to_le_bytes());
|
||||
for (name, value) in query {
|
||||
@@ -63,9 +69,9 @@ pub(crate) fn identity(
|
||||
// different account, so only credentials with stable owner metadata can be reused.
|
||||
if !has_stable_account || explicit_bearer {
|
||||
let headers = resolve_provider_auth(auth, provider_info)?.to_auth_headers();
|
||||
provider.headers.extend(headers);
|
||||
api_provider.headers.extend(headers);
|
||||
}
|
||||
let mut headers: Vec<_> = provider.headers.iter().collect();
|
||||
let mut headers: Vec<_> = api_provider.headers.iter().collect();
|
||||
headers.sort_by(|(a, av), (b, bv)| {
|
||||
a.as_str()
|
||||
.cmp(b.as_str())
|
||||
|
||||
@@ -87,3 +87,17 @@ fn cache_identity_distinguishes_auth_requirements_and_unknown_plans() {
|
||||
provider.requires_openai_auth = false;
|
||||
assert_ne!(key, identity(&provider, Some(&first)).unwrap());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cache_identity_tracks_catalog_url() {
|
||||
let auth = CodexAuth::from_api_key("test-key");
|
||||
let mut provider =
|
||||
ModelProviderInfo::create_openai_provider(Some("https://gateway.example/v1".into()));
|
||||
let bundled = identity(&provider, Some(&auth)).unwrap();
|
||||
provider.model_catalog_url = Some("https://gateway.example/catalog-one".into());
|
||||
let first = identity(&provider, Some(&auth)).unwrap();
|
||||
provider.model_catalog_url = Some("https://gateway.example/catalog-two".into());
|
||||
let second = identity(&provider, Some(&auth)).unwrap();
|
||||
assert_ne!(bundled, first);
|
||||
assert_ne!(first, second);
|
||||
}
|
||||
|
||||
@@ -679,6 +679,7 @@ mod tests {
|
||||
ModelProviderInfo {
|
||||
name: "mock".into(),
|
||||
base_url: Some(base_url),
|
||||
model_catalog_url: None,
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
@@ -1380,33 +1381,50 @@ printf '%s\n' '{"AccessKeyId":"exported","SecretAccessKey":"secret"}'
|
||||
models: remote_models.clone(),
|
||||
}),
|
||||
)
|
||||
.expect(1)
|
||||
.expect(2)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let mut provider_info = provider_for(server.uri());
|
||||
provider_info.experimental_bearer_token = Some("provider-token".into());
|
||||
let provider = create_model_provider(
|
||||
provider_info,
|
||||
Some(AuthManager::from_auth_for_testing(
|
||||
CodexAuth::create_dummy_chatgpt_auth_for_testing(),
|
||||
)),
|
||||
);
|
||||
provider_info.model_catalog_url = Some(format!("{}/models", server.uri()).into());
|
||||
provider_info.http_headers = Some(std::collections::HashMap::from([(
|
||||
codex_login::default_client::RESIDENCY_HEADER_NAME.to_string(),
|
||||
"us".into(),
|
||||
)]));
|
||||
for auth in [
|
||||
None,
|
||||
Some(CodexAuth::create_dummy_chatgpt_auth_for_testing()),
|
||||
] {
|
||||
// Disabled discovery must ignore the catalog cached by the enabled run.
|
||||
for enabled in [true, false] {
|
||||
let provider = create_model_provider(
|
||||
provider_info.clone(),
|
||||
auth.clone().map(AuthManager::from_auth_for_testing),
|
||||
);
|
||||
let manager =
|
||||
provider.models_manager(test_codex_home(), /*config_model_catalog*/ None);
|
||||
manager.set_api_key_model_discovery_enabled(enabled);
|
||||
let refresh_strategy = if enabled {
|
||||
RefreshStrategy::Online
|
||||
} else {
|
||||
RefreshStrategy::Offline
|
||||
};
|
||||
let catalog = manager
|
||||
.raw_model_catalog(
|
||||
refresh_strategy,
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
)
|
||||
.await;
|
||||
|
||||
let manager =
|
||||
provider.models_manager(test_codex_home(), /*config_model_catalog*/ None);
|
||||
let catalog = manager
|
||||
.raw_model_catalog(
|
||||
RefreshStrategy::Online,
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
catalog
|
||||
.models
|
||||
.iter()
|
||||
.any(|model| model.slug == "provider-model")
|
||||
);
|
||||
assert_eq!(
|
||||
catalog
|
||||
.models
|
||||
.iter()
|
||||
.any(|model| model.slug == "provider-model"),
|
||||
enabled
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -271,6 +271,7 @@ async fn models_cache_is_reused_only_for_matching_gateway_configuration() {
|
||||
home.path().to_path_buf(),
|
||||
);
|
||||
let mut info = ModelProviderInfo {
|
||||
model_catalog_url: Some(format!("{}/models", server.uri()).into()),
|
||||
http_headers: Some(std::collections::HashMap::from([(
|
||||
codex_login::default_client::RESIDENCY_HEADER_NAME.to_string(),
|
||||
"us".into(),
|
||||
@@ -354,6 +355,7 @@ async fn models_observe_gateway_rotation(instance: ProviderInstance) {
|
||||
home.path().to_path_buf(),
|
||||
);
|
||||
let info = ModelProviderInfo {
|
||||
model_catalog_url: Some(format!("{}/models", server.uri()).into()),
|
||||
gateway_oauth: Some(GatewayOAuthConfig {
|
||||
authorization_url: format!("{}/authorize", server.uri()),
|
||||
token_url: format!("{}/token", server.uri()),
|
||||
|
||||
@@ -54,6 +54,12 @@ pub trait ModelsEndpointClient: fmt::Debug + Send + Sync {
|
||||
false
|
||||
}
|
||||
|
||||
/// Returns whether explicit provider configuration supplies API-key authentication.
|
||||
/// This takes precedence over any unrelated first-party login used by the picker.
|
||||
fn has_provider_api_key(&self) -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
/// Fetches the latest remote model catalog and optional ETag.
|
||||
fn list_models<'a>(
|
||||
&'a self,
|
||||
@@ -476,9 +482,13 @@ impl OpenAiModelsManager {
|
||||
refresh_strategy: RefreshStrategy,
|
||||
http_client_factory: &HttpClientFactory,
|
||||
) -> CoreResult<()> {
|
||||
// Gate cache loading as well as requests; disabled sessions use bundled API-key models.
|
||||
if self.supports_api_key_discovery()
|
||||
&& !self.api_key_model_discovery_enabled.load(Ordering::SeqCst)
|
||||
// API-key discovery must be enabled and supported before reusing a remote catalog.
|
||||
// Otherwise even a matching cache from an earlier run would bypass bundled-only behavior.
|
||||
// Command-auth providers retain their existing discovery behavior.
|
||||
if self.uses_api_key_auth()
|
||||
&& !self.endpoint_client.has_command_auth()
|
||||
&& (!self.endpoint_client.supports_api_key_models()
|
||||
|| !self.api_key_model_discovery_enabled.load(Ordering::SeqCst))
|
||||
{
|
||||
return Ok(());
|
||||
}
|
||||
@@ -541,7 +551,12 @@ impl OpenAiModelsManager {
|
||||
fn supports_api_key_discovery(&self) -> bool {
|
||||
self.endpoint_client.supports_api_key_models()
|
||||
&& !self.endpoint_client.has_command_auth()
|
||||
&& self
|
||||
&& self.uses_api_key_auth()
|
||||
}
|
||||
|
||||
fn uses_api_key_auth(&self) -> bool {
|
||||
self.endpoint_client.has_provider_api_key()
|
||||
|| self
|
||||
.auth_manager
|
||||
.as_ref()
|
||||
.is_some_and(|auth_manager| auth_manager.auth_mode() == Some(AuthMode::ApiKey))
|
||||
|
||||
Reference in New Issue
Block a user