From ffd3429d754991183433b4d41b67551e5656ba27 Mon Sep 17 00:00:00 2001 From: Henry Levy Date: Sat, 17 Oct 2026 23:15:18 +0000 Subject: [PATCH] Preserve model catalog caches across header credential rotation (#54946) ## Why Rotating header credentials should not discard the model catalog when a stable account ID is available. ## What changed - Use the account ID from `CodexAuth::Headers` instead of opaque auth headers in the catalog cache identity. Provider settings remain part of the key. - Retain credential-sensitive keys when the account ID is missing or the provider uses an explicit API key, bearer token, or command auth. - Document that hosts using header auth must isolate model managers and cache storage by user and catalog-varying request context, since an account ID can represent a shared workspace. ## Testing Update the auth-rotation test fixture to omit stable account metadata, preserving coverage for catalog invalidation when opaque credentials change. GitOrigin-RevId: f96a20c5baf76b9235e131140716731da4dbee81 --- .../core/tests/suite/models_cache_auth.rs | 12 +++++------ .../model-provider/src/models_identity.rs | 20 +++++++++++++------ codex-rs/model-provider/src/provider.rs | 8 ++++++++ codex-rs/models-manager/src/manager.rs | 3 ++- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/codex-rs/core/tests/suite/models_cache_auth.rs b/codex-rs/core/tests/suite/models_cache_auth.rs index 4521237045..a6200a8395 100644 --- a/codex-rs/core/tests/suite/models_cache_auth.rs +++ b/codex-rs/core/tests/suite/models_cache_auth.rs @@ -78,13 +78,11 @@ impl ExternalAuth for StallingAuth { } fn header_auth(token: &'static str) -> CodexAuth { - CodexAuth::Headers(AuthHeaders::new(HeaderMap::from_iter([ - (http::header::AUTHORIZATION, HeaderValue::from_static(token)), - ( - http::header::HeaderName::from_static("chatgpt-account-id"), - HeaderValue::from_static("same-account"), - ), - ]))) + // Without stable account metadata, credential rotation must invalidate the catalog. + CodexAuth::Headers(AuthHeaders::new(HeaderMap::from_iter([( + http::header::AUTHORIZATION, + HeaderValue::from_static(token), + )]))) } #[derive(Clone, Copy)] diff --git a/codex-rs/model-provider/src/models_identity.rs b/codex-rs/model-provider/src/models_identity.rs index 409ff83a07..06cafacb86 100644 --- a/codex-rs/model-provider/src/models_identity.rs +++ b/codex-rs/model-provider/src/models_identity.rs @@ -1,6 +1,7 @@ //! Model catalog cache identity follows provider routing, effective auth, and gateway OAuth config. //! Only a digest is persisted; access tokens for ChatGPT are excluded so token //! rotation does not discard a catalog for the same account, user, and plan. +//! Header auth with an account ID uses that identity instead of opaque auth headers. use codex_login::CodexAuth; use codex_model_provider_info::ModelProviderInfo; @@ -57,16 +58,23 @@ pub(crate) fn identity( field(&[u8::from(auth.is_fedramp_account())]); } let has_stable_account = auth.is_some_and(|auth| { - matches!( - auth, - CodexAuth::Chatgpt(_) | CodexAuth::ChatgptAuthTokens(_) | CodexAuth::AgentIdentity(_) - ) && auth.get_account_id().is_some() - && (auth.get_chatgpt_user_id().is_some() || auth.get_account_email().is_some()) + auth.get_account_id().is_some() + && if matches!(auth, CodexAuth::Headers(_)) { + !provider_info.has_command_auth() + } else { + matches!( + auth, + CodexAuth::Chatgpt(_) + | CodexAuth::ChatgptAuthTokens(_) + | CodexAuth::AgentIdentity(_) + ) && (auth.get_chatgpt_user_id().is_some() || auth.get_account_email().is_some()) + } }); let explicit_bearer = provider_info.api_key()?.is_some() || provider_info.experimental_bearer_token.is_some(); // Command auth also returns opaque API keys. A changed key may belong to a - // different account, so only credentials with stable owner metadata can be reused. + // different account, so reuse requires stable owner metadata. Explicit provider + // keys take precedence over the header auth's account identity. if !has_stable_account || explicit_bearer { let headers = resolve_provider_auth(auth, provider_info)?.to_auth_headers(); api_provider.headers.extend(headers); diff --git a/codex-rs/model-provider/src/provider.rs b/codex-rs/model-provider/src/provider.rs index f8f0790dde..a720bcad25 100644 --- a/codex-rs/model-provider/src/provider.rs +++ b/codex-rs/model-provider/src/provider.rs @@ -105,6 +105,14 @@ pub const DEFAULT_MEMORY_CONSOLIDATION_PREFERRED_MODEL: &str = "gpt-5.6-terra"; /// Implementations own provider-specific behavior for a model backend. The /// `ModelProviderInfo` returned by `info` is the serialized/configured provider /// metadata used by the default OpenAI-compatible implementation. +/// +/// OpenAI-compatible catalog caches use header auth's account ID instead of auth +/// headers when available. Missing IDs and explicit provider API key, bearer, or +/// command auth retain credential-sensitive keys. Provider settings remain in the key. +/// Hosts using header auth must isolate model managers and cache storage by user and +/// catalog-varying request context, such as country: an account ID may identify a +/// workspace shared by multiple users. Request authentication, credential refresh, +/// and permission checks are unchanged. pub trait ModelProvider: fmt::Debug + Send + Sync { /// Returns the configured provider metadata. fn info(&self) -> &ModelProviderInfo; diff --git a/codex-rs/models-manager/src/manager.rs b/codex-rs/models-manager/src/manager.rs index b1efc0fe7c..2a0b98091f 100644 --- a/codex-rs/models-manager/src/manager.rs +++ b/codex-rs/models-manager/src/manager.rs @@ -42,7 +42,8 @@ const DEFAULT_MODEL_CACHE_TTL: Duration = Duration::from_secs(300); pub trait ModelsEndpointClient: fmt::Debug + Send + Sync { /// Opaque identity of the current provider and credentials, without resolving auth. /// - /// Must change when account, email, plan, provider, or API credentials change. + /// Must distinguish provider routing and authenticated principals within the + /// manager and cache's caller-established scope. /// Return `None` when identity is unavailable; cached catalog reuse is then disabled. fn identity(&self) -> Option;