From f3a90545aed3188ee1e5c119ec89a742de399cdc Mon Sep 17 00:00:00 2001 From: smurching-oai Date: Fri, 23 Oct 2026 00:46:50 +0000 Subject: [PATCH] Add managed configuration loaders for custom config servers (#56314) ## What changed - Add loaders that select `config_server` when configured and otherwise use the existing ChatGPT bundle source. Fetch custom bundles from `/codex/config/bundle` without redirects or request logging. - Support optional OAuth sessions without interactive login during bundle fetches, recover credentials after a `401`, and scope cached bundles to the endpoint and authorization identity. - Restrict bootstrap network access to configuration and OAuth endpoints using local administrator rules when available. - Return generic bundle validation errors so invalid TOML, requirements, and provider settings do not expose configuration contents. ## Testing Add tests for missing configuration-server sign-in, replacement OAuth credentials, endpoint-specific cache reuse, and validation errors that omit sensitive values while preserving cached bundles. GitOrigin-RevId: 56590a9e66340540666ca21590bef46c495e0e33 --- codex-rs/Cargo.lock | 2 + codex-rs/backend-client/src/client.rs | 34 ++- .../src/client/rate_limit_resets_tests.rs | 1 + codex-rs/cloud-config/Cargo.toml | 2 + codex-rs/cloud-config/src/backend.rs | 3 - codex-rs/cloud-config/src/bundle_loader.rs | 60 ++++ codex-rs/cloud-config/src/config_server.rs | 272 ++++++++++++++++++ .../cloud-config/src/config_server_tests.rs | 195 +++++++++++++ codex-rs/cloud-config/src/lib.rs | 6 + codex-rs/cloud-config/src/service_tests.rs | 16 +- codex-rs/cloud-config/src/validation.rs | 12 +- codex-rs/config/src/config_server.rs | 16 +- codex-rs/login/src/outbound_proxy.rs | 22 +- 13 files changed, 608 insertions(+), 33 deletions(-) create mode 100644 codex-rs/cloud-config/src/config_server.rs create mode 100644 codex-rs/cloud-config/src/config_server_tests.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 6bf43c6b6a..84057851d1 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2650,6 +2650,7 @@ dependencies = [ "codex-config", "codex-core", "codex-http-client", + "codex-keyring-store", "codex-login", "codex-otel", "codex-protocol", @@ -2662,6 +2663,7 @@ dependencies = [ "thiserror 2.0.18", "tokio", "tracing", + "wiremock", ] [[package]] diff --git a/codex-rs/backend-client/src/client.rs b/codex-rs/backend-client/src/client.rs index ed72e9ae5c..86e9433909 100644 --- a/codex-rs/backend-client/src/client.rs +++ b/codex-rs/backend-client/src/client.rs @@ -165,6 +165,7 @@ pub struct Client { chatgpt_account_id: Option, chatgpt_account_is_fedramp: bool, path_style: PathStyle, + config_server_base_url: Option, } impl fmt::Debug for Client { @@ -205,6 +206,22 @@ impl Client { Self::with_http(base_url.into(), http) } + pub fn for_config_endpoint( + base_url: impl Into, + http_client_factory: HttpClientFactory, + ) -> Self { + let base_url = base_url.into(); + let mut client = Self::with_http( + base_url.clone(), + RouteAwareClientPool::new_without_redirects_or_request_logging( + http_client_factory, + ClientRouteClass::Api, + ), + ); + client.config_server_base_url = Some(base_url); + client + } + fn with_http(mut base_url: String, http: RouteAwareClientPool) -> Self { // Normalize common ChatGPT hostnames to include /backend-api so we hit the WHAM paths. // Also trim trailing slashes for consistent URL building. @@ -226,6 +243,7 @@ impl Client { chatgpt_account_id: None, chatgpt_account_is_fedramp: false, path_style, + config_server_base_url: None, } } @@ -244,6 +262,12 @@ impl Client { self } + pub fn with_bearer_token(self, token: String) -> Self { + self.with_auth_provider(std::sync::Arc::new( + codex_model_provider::BearerAuthProvider::new(token), + )) + } + pub fn with_user_agent(mut self, ua: impl Into) -> Self { if let Ok(hv) = HeaderValue::from_str(&ua.into()) { self.user_agent = Some(hv); @@ -523,10 +547,9 @@ impl Client { self.decode_json::(&url, &ct, &body) } - /// Fetch the selected cloud-managed config bundle from codex-backend. - /// /// `GET /api/codex/config/bundle` (Codex API style) or - /// `GET /wham/config/bundle` (ChatGPT backend-api style). + /// `GET /wham/config/bundle` (ChatGPT backend-api style), or + /// `GET /codex/config/bundle` beneath a custom config server's base URL. pub async fn get_config_bundle( &self, ) -> std::result::Result { @@ -538,6 +561,10 @@ impl Client { /// Returns the normalized discovery endpoint used by this client. pub fn config_bundle_url(&self) -> String { + if let Some(base_url) = &self.config_server_base_url { + let base_url = base_url.strip_suffix('/').unwrap_or(base_url); + return format!("{base_url}/codex/config/bundle"); + } match self.path_style { PathStyle::CodexApi => format!("{}/api/codex/config/bundle", self.base_url), PathStyle::ChatGptApi => format!("{}/wham/config/bundle", self.base_url), @@ -1282,6 +1309,7 @@ mod tests { chatgpt_account_id: None, chatgpt_account_is_fedramp: false, path_style, + config_server_base_url: None, } } } diff --git a/codex-rs/backend-client/src/client/rate_limit_resets_tests.rs b/codex-rs/backend-client/src/client/rate_limit_resets_tests.rs index ad398641c9..4c4470fedf 100644 --- a/codex-rs/backend-client/src/client/rate_limit_resets_tests.rs +++ b/codex-rs/backend-client/src/client/rate_limit_resets_tests.rs @@ -183,5 +183,6 @@ fn test_client(base_url: &str, path_style: PathStyle) -> Client { chatgpt_account_id: None, chatgpt_account_is_fedramp: false, path_style, + config_server_base_url: None, } } diff --git a/codex-rs/cloud-config/Cargo.toml b/codex-rs/cloud-config/Cargo.toml index 2077502017..ad8a75212b 100644 --- a/codex-rs/cloud-config/Cargo.toml +++ b/codex-rs/cloud-config/Cargo.toml @@ -14,6 +14,7 @@ codex-async-utils = { workspace = true } codex-backend-client = { workspace = true } codex-config = { workspace = true } codex-http-client = { workspace = true } +codex-keyring-store = { workspace = true } codex-login = { workspace = true } codex-otel = { workspace = true } codex-protocol = { workspace = true } @@ -27,6 +28,7 @@ tokio = { workspace = true, features = ["fs", "rt", "sync", "time"] } tracing = { workspace = true } [dev-dependencies] +wiremock = { workspace = true } codex-agent-identity = { workspace = true } codex-core = { workspace = true } pretty_assertions = { workspace = true } diff --git a/codex-rs/cloud-config/src/backend.rs b/codex-rs/cloud-config/src/backend.rs index 3787229b43..68f337d56f 100644 --- a/codex-rs/cloud-config/src/backend.rs +++ b/codex-rs/cloud-config/src/backend.rs @@ -98,9 +98,6 @@ pub(crate) async fn fetch_bundle( client .get_config_bundle() .await - .inspect_err(|err| { - tracing::warn!(error = %err, "Failed to fetch cloud config bundle"); - }) .map(bundle_from_response) .map_err(bundle_request_error) } diff --git a/codex-rs/cloud-config/src/bundle_loader.rs b/codex-rs/cloud-config/src/bundle_loader.rs index 2016e092ab..997ae7d622 100644 --- a/codex-rs/cloud-config/src/bundle_loader.rs +++ b/codex-rs/cloud-config/src/bundle_loader.rs @@ -1,9 +1,11 @@ use crate::backend::BackendBundleClient; use crate::chatgpt::ChatGptSource; +use crate::config_server::config_server_http_client_factory; use crate::config_source::BundleSource; use crate::service::CLOUD_CONFIG_BUNDLE_TIMEOUT; use crate::service::CloudConfigBundleService; use codex_config::CloudConfigBundleLoader; +use codex_config::ConfigRequirementsToml; use codex_http_client::HttpClientFactory; use codex_login::AuthConfig; use codex_login::AuthManager; @@ -12,6 +14,64 @@ use std::sync::Arc; use tokio::task::AbortHandle; use tokio::task::JoinHandle; +pub fn managed_config_auth_manager( + auth: AuthConfig, + requirements: &ConfigRequirementsToml, +) -> std::io::Result> { + let Some(service_config) = requirements.config_server.as_ref() else { + return Ok(None); + }; + let factory = config_server_http_client_factory(service_config, &auth.auth_route_config); + crate::config_server_auth_manager(service_config, auth.codex_home, &factory) +} + +pub fn managed_config_bundle_loader( + auth_manager: Arc, + chatgpt_base_url: String, + codex_home: PathBuf, + factory: HttpClientFactory, + requirements: &ConfigRequirementsToml, +) -> CloudConfigBundleLoader { + if let Some(service_config) = requirements.config_server.as_ref() { + let factory = config_server_http_client_factory( + service_config, + &codex_login::AuthRouteConfig::from_http_client_factory(factory), + ); + return crate::config_server_bundle_loader(service_config.clone(), codex_home, factory); + } + let endpoint = codex_backend_client::Client::new(chatgpt_base_url.clone(), factory.clone()) + .config_bundle_url(); + let policy = factory + .network_policy() + .clone() + .restrict_to_endpoints(endpoint.parse().into_iter().collect()); + cloud_config_bundle_loader( + auth_manager, + chatgpt_base_url, + codex_home, + factory.with_network_policy(policy), + ) +} + +pub async fn managed_config_bundle_loader_for_storage( + auth_config: AuthConfig, + requirements: &ConfigRequirementsToml, + enable_codex_api_key_env: bool, +) -> std::io::Result { + if let Some(service_config) = requirements.config_server.as_ref() { + let factory = + config_server_http_client_factory(service_config, &auth_config.auth_route_config); + return Ok(crate::config_server_bundle_loader( + service_config.clone(), + auth_config.codex_home, + factory, + )); + } + let service = + cloud_config_bundle_service_for_storage(auth_config, enable_codex_api_key_env).await?; + Ok(cloud_config_bundle_loader_for_service(service).0) +} + struct CloudConfigBundleLoaderLifetime { service: Arc>, refresh_task: AbortHandle, diff --git a/codex-rs/cloud-config/src/config_server.rs b/codex-rs/cloud-config/src/config_server.rs new file mode 100644 index 0000000000..8a2422e9d5 --- /dev/null +++ b/codex-rs/cloud-config/src/config_server.rs @@ -0,0 +1,272 @@ +//! Fetch managed configuration from a custom config server. +//! Cache entries are scoped to the endpoint and the current OAuth sign-in. + +use crate::backend::BundleRequestError; +use crate::backend::RetryableFailureKind; +use crate::backend::fetch_bundle; +use crate::config_source::BundleSource; +use crate::service::CLOUD_CONFIG_BUNDLE_TIMEOUT; +use crate::service::CloudConfigBundleService; +use codex_backend_client::Client as BackendClient; +use codex_config::CloudConfigBundle; +use codex_config::CloudConfigBundleLoadError; +use codex_config::CloudConfigBundleLoadErrorCode; +use codex_config::CloudConfigBundleLoader; +use codex_config::ConfigServer; +use codex_http_client::HttpClientFactory; +use codex_keyring_store::DefaultKeyringStore; +use codex_login::AuthRouteConfig; +use codex_login::OAuthSessionConfig; +use codex_login::OAuthSessionError; +use codex_login::OAuthSessionManager; +use codex_login::OAuthSessionRequest; +use sha2::Digest; +use sha2::Sha256; +use std::path::PathBuf; +use std::sync::Arc; + +pub fn config_server_bundle_loader( + config: ConfigServer, + codex_home: PathBuf, + factory: HttpClientFactory, +) -> CloudConfigBundleLoader { + match ConfigServerSource::from_config_server(config.clone(), codex_home.clone(), factory) { + Ok(source) => { + let service = CloudConfigBundleService::from_source( + source, + codex_home, + CLOUD_CONFIG_BUNDLE_TIMEOUT, + ); + crate::bundle_loader::cloud_config_bundle_loader_for_service(service).0 + } + Err(error) => CloudConfigBundleLoader::new(std::future::ready(Err(error))), + } + .with_config_server(config) +} + +pub fn config_server_auth_manager( + service_config: &ConfigServer, + codex_home: PathBuf, + factory: &HttpClientFactory, +) -> std::io::Result> { + service_config + .validated_base_url() + .map_err(std::io::Error::other)?; + oauth_session_manager(service_config.oauth.as_deref(), codex_home, factory) +} + +pub(crate) fn config_server_http_client_factory( + config: &ConfigServer, + routes: &AuthRouteConfig, +) -> HttpClientFactory { + let endpoint = config.validated_base_url().ok().map(|base_url| { + BackendClient::for_config_endpoint(base_url, routes.http_client_factory().clone()) + .config_bundle_url() + }); + let endpoints = endpoint + .as_deref() + .into_iter() + .chain(config.oauth.as_ref().map(|oauth| oauth.token_url.as_str())) + .chain( + config + .oauth + .as_ref() + .and_then(|oauth| oauth.device_authorization_url.as_deref()), + ) + .filter_map(|endpoint| endpoint.parse().ok()); + routes.http_client_factory_for_bootstrap(endpoints) +} + +struct ConfigServerSource { + cache_namespace: String, + client: BackendClient, + auth: Option, +} + +fn oauth_session_manager( + oauth: Option<&OAuthSessionConfig>, + codex_home: PathBuf, + factory: &HttpClientFactory, +) -> std::io::Result> { + oauth + .map(|oauth| { + OAuthSessionManager::new( + oauth.clone(), + codex_home, + factory, + Arc::new(DefaultKeyringStore), + ) + .map(OAuthSessionManager::without_interactive_login) + }) + .transpose() +} + +impl ConfigServerSource { + fn from_config_server( + config: ConfigServer, + codex_home: PathBuf, + factory: HttpClientFactory, + ) -> Result { + let base_url = config.validated_base_url().map_err(|message| { + CloudConfigBundleLoadError::new( + CloudConfigBundleLoadErrorCode::Internal, + /*status_code*/ None, + message, + ) + })?; + Self::new( + base_url, + config.oauth.map(|oauth| *oauth), + codex_home, + factory, + ) + } + + fn new( + base_url: String, + oauth: Option, + codex_home: PathBuf, + factory: HttpClientFactory, + ) -> Result { + let auth = + oauth_session_manager(oauth.as_ref(), codex_home, &factory).map_err(auth_io_error)?; + let client = BackendClient::for_config_endpoint(base_url, factory); + // Reuse the OAuth session's issuer/client/audience/scope identity; the endpoint + // additionally separates policy authorities that share one OAuth grant. + let namespace = serde_json::to_vec(&( + client.config_bundle_url(), + auth.as_ref().map(OAuthSessionManager::credential_id), + )) + .map_err(|_| auth_failure())?; + Ok(Self { + client, + cache_namespace: format!("{:x}", Sha256::digest(namespace)), + auth, + }) + } +} + +impl BundleSource for ConfigServerSource { + type Auth = Option; + type FetchContext = Option; + fn begin_fetch(&self) -> Self::FetchContext { + self.auth.as_ref().map(OAuthSessionManager::begin_request) + } + + async fn auth(&self) -> Result, CloudConfigBundleLoadError> { + // Cache reads need the grant identity, not a refreshed access token. + Ok(Some(match &self.auth { + Some(auth) => Some(auth.authorization_id().await.map_err(auth_io_error)?), + None => None, + })) + } + + fn cache_namespace(&self) -> Option { + Some(self.cache_namespace.clone()) + } + + fn identity(&self, auth: &Self::Auth) -> (Option, Option) { + ( + Some(auth.clone().unwrap_or_else(|| "anonymous".into())), + self.cache_namespace(), + ) + } + + async fn get_bundle( + &self, + identity: &mut Self::Auth, + context: &mut Self::FetchContext, + ) -> Result { + let Some(request) = context else { + return fetch_bundle(&self.client).await; + }; + let credentials = request + .credentials() + .await + .map_err(|error| auth_request_error(error, request.is_recovering()))?; + if credentials.authorization_id.is_none() { + return Err(BundleRequestError::Fatal { + error: auth_failure(), + metric_reason: Some("auth"), + }); + } + *identity = credentials.authorization_id; + fetch_bundle( + &self + .client + .clone() + .with_bearer_token(credentials.access_token), + ) + .await + } + + async fn recover_auth( + &self, + _identity: &mut Self::Auth, + context: &mut Self::FetchContext, + status_code: Option, + recovery_attempt: usize, + ) -> Result { + let Some(auth) = context else { + return Ok(false); + }; + if status_code != Some(401) || recovery_attempt != 0 { + return Ok(false); + } + Ok(auth.reject_credentials()) + } +} + +fn auth_request_error(error: std::io::Error, recovering: bool) -> BundleRequestError { + if error + .get_ref() + .is_some_and(::is::) + || matches!( + error.kind(), + std::io::ErrorKind::PermissionDenied + | std::io::ErrorKind::InvalidInput + | std::io::ErrorKind::InvalidData + ) + { + BundleRequestError::Fatal { + error: auth_io_error(error), + metric_reason: Some("auth"), + } + } else { + BundleRequestError::Retryable(if recovering { + RetryableFailureKind::AuthRecovery { + status_code: Some(401), + } + } else { + RetryableFailureKind::Request { status_code: None } + }) + } +} + +fn auth_io_error(error: std::io::Error) -> CloudConfigBundleLoadError { + if error + .get_ref() + .and_then(|source| source.downcast_ref::()) + == Some(&OAuthSessionError::LoginRequired) + { + CloudConfigBundleLoadError::new( + CloudConfigBundleLoadErrorCode::Auth, + /*status_code*/ None, + "Sign in to the configuration server to load your organization's settings", + ) + } else { + auth_failure() + } +} + +fn auth_failure() -> CloudConfigBundleLoadError { + CloudConfigBundleLoadError::new( + CloudConfigBundleLoadErrorCode::Auth, + /*status_code*/ None, + "Could not authenticate with the configuration server", + ) +} + +#[cfg(test)] +#[path = "config_server_tests.rs"] +mod tests; diff --git a/codex-rs/cloud-config/src/config_server_tests.rs b/codex-rs/cloud-config/src/config_server_tests.rs new file mode 100644 index 0000000000..db987b266f --- /dev/null +++ b/codex-rs/cloud-config/src/config_server_tests.rs @@ -0,0 +1,195 @@ +#![cfg(test)] + +use super::*; +use crate::bundle_loader::cloud_config_bundle_loader_for_service; +use crate::service::CLOUD_CONFIG_BUNDLE_TIMEOUT; +use crate::service::CloudConfigBundleService; +use codex_config::CloudConfigBundleLoader; +use codex_config::ConfigServer; +use codex_config::test_support::CloudConfigBundleFixture; +use codex_keyring_store::tests::MockKeyringStore; +use codex_login::test_support::transport_default_auth_route_config; +use pretty_assertions::assert_eq; +use serde_json::json; +use std::sync::atomic::AtomicUsize; +use std::sync::atomic::Ordering; +use tempfile::tempdir; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::ResponseTemplate; +use wiremock::matchers::header; +use wiremock::matchers::method; +use wiremock::matchers::path; + +// HTTP fixtures exercise the same service while production enforces HTTPS at construction. +fn remote_loader(url: String, codex_home: PathBuf) -> CloudConfigBundleLoader { + let factory = transport_default_auth_route_config() + .http_client_factory() + .clone(); + let source = ConfigServerSource::new(url, /*oauth*/ None, codex_home.clone(), factory).unwrap(); + cloud_config_bundle_loader_for_service(CloudConfigBundleService::from_source( + source, + codex_home, + CLOUD_CONFIG_BUNDLE_TIMEOUT, + )) + .0 +} + +fn oauth_config(base_url: &str) -> OAuthSessionConfig { + OAuthSessionConfig { + authorization_url: format!("{base_url}/authorize"), + token_url: format!("{base_url}/token"), + device_authorization_url: None, + client_id: "company-client".into(), + resource: None, + scopes: vec![], + redirect_port: None, + } +} + +#[tokio::test] +async fn policy_fetch_requires_config_server_sign_in() { + let server = MockServer::start().await; + let home = tempdir().unwrap(); + let endpoint = format!("https://{}", server.address()); + let factory = transport_default_auth_route_config() + .http_client_factory() + .clone(); + let loader = config_server_bundle_loader( + ConfigServer { + base_url: format!("{endpoint}/settings"), + oauth: Some(Box::new(oauth_config(&endpoint))), + }, + home.path().to_path_buf(), + factory, + ); + let error = loader.get().await.unwrap_err(); + assert_eq!(error.code(), CloudConfigBundleLoadErrorCode::Auth); + assert_eq!( + error.to_string(), + "Sign in to the configuration server to load your organization's settings" + ); + assert!(server.received_requests().await.unwrap().is_empty()); +} + +#[tokio::test] +async fn policy_fetch_uses_replacement_config_server_credentials() { + let server = MockServer::start().await; + let sign_in = AtomicUsize::new(0); + Mock::given(path("/token")) + .respond_with(move |_: &wiremock::Request| { + ResponseTemplate::new(200).set_body_json(json!({ + "access_token": format!("account-{}", sign_in.fetch_add(1, Ordering::Relaxed)), + "token_type": "Bearer", "expires_in": 3600, + })) + }) + .mount(&server) + .await; + let policies = ["never", "on-request"].map(|approval| { + CloudConfigBundleFixture::enterprise_requirement(format!( + "allowed_approval_policies = ['{approval}']" + )) + .into_bundle() + }); + for (account, policy) in policies.iter().enumerate() { + Mock::given(path("/settings/codex/config/bundle")) + .and(header("authorization", format!("Bearer account-{account}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(policy)) + .mount(&server) + .await; + } + let home = tempdir().unwrap(); + let factory = transport_default_auth_route_config() + .http_client_factory() + .clone(); + let oauth = oauth_config(&server.uri()); + let keyring = Arc::new(MockKeyringStore::default()); + let manager = || { + OAuthSessionManager::new( + oauth.clone(), + home.path().to_path_buf(), + &factory, + keyring.clone(), + ) + .unwrap() + }; + let gateway = manager(); + gateway + .login_with_browser(std::future::pending(), complete_browser_authorization) + .await + .unwrap(); + let source = ConfigServerSource { + cache_namespace: "test".into(), + client: BackendClient::for_config_endpoint( + format!("{}/settings", server.uri()), + factory.clone(), + ), + auth: Some(manager().without_interactive_login()), + }; + let service = CloudConfigBundleService::from_source( + source, + home.path().to_path_buf(), + CLOUD_CONFIG_BUNDLE_TIMEOUT, + ); + assert_eq!( + service.load_startup_bundle_with_timeout().await.unwrap(), + Some(policies[0].clone()) + ); + + gateway + .login_with_browser(std::future::pending(), complete_browser_authorization) + .await + .unwrap(); + // The old access token is still valid, so this must detect the changed grant. + assert_eq!( + service.load_startup_bundle_with_timeout().await.unwrap(), + Some(policies[1].clone()) + ); +} + +fn complete_browser_authorization(url: &wiremock::http::Url) { + let query = url + .query_pairs() + .collect::>(); + let callback = format!( + "{}?code=browser-authorization-code&state={}", + query["redirect_uri"], query["state"] + ); + tokio::spawn(async move { + codex_login::auth::default_client::create_client_without_request_logging() + .get(callback) + .send() + .await + .expect("OAuth callback"); + }); +} + +#[tokio::test] +async fn cached_policy_is_reused_only_for_the_same_endpoint() { + let server = MockServer::start().await; + let home = tempdir().unwrap(); + let endpoint = format!("{}/settings", server.uri()); + let loader = || remote_loader(endpoint.clone(), home.path().to_path_buf()); + let bundle = + CloudConfigBundleFixture::enterprise_requirement("allowed_approval_policies = ['never']") + .into_bundle(); + Mock::given(method("GET")) + .and(path("/settings/codex/config/bundle")) + .respond_with(ResponseTemplate::new(/*s*/ 200).set_body_json(&bundle)) + .mount(&server) + .await; + let first = loader(); + assert_eq!(first.get().await.unwrap(), Some(bundle.clone())); + drop(first); + server.reset().await; + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(/*s*/ 403)) + .mount(&server) + .await; + assert_eq!(loader().get().await.unwrap(), Some(bundle)); + let different = remote_loader( + format!("{}/different", server.uri()), + home.path().to_path_buf(), + ); + assert_eq!(different.get().await.unwrap_err().status_code(), Some(403)); +} diff --git a/codex-rs/cloud-config/src/lib.rs b/codex-rs/cloud-config/src/lib.rs index 0eb289a0f2..0ede538cbd 100644 --- a/codex-rs/cloud-config/src/lib.rs +++ b/codex-rs/cloud-config/src/lib.rs @@ -7,6 +7,7 @@ mod backend; mod bundle_loader; mod cache; mod chatgpt; +mod config_server; mod config_source; mod metrics; mod service; @@ -15,3 +16,8 @@ mod validation; pub use bundle_loader::cloud_config_bundle_loader; pub use bundle_loader::cloud_config_bundle_loader_for_storage; pub use bundle_loader::cloud_config_bundle_loader_for_storage_without_cache; +pub use bundle_loader::managed_config_auth_manager; +pub use bundle_loader::managed_config_bundle_loader; +pub use bundle_loader::managed_config_bundle_loader_for_storage; +pub use config_server::config_server_auth_manager; +pub use config_server::config_server_bundle_loader; diff --git a/codex-rs/cloud-config/src/service_tests.rs b/codex-rs/cloud-config/src/service_tests.rs index 2a131a6dee..51a10ec17a 100644 --- a/codex-rs/cloud-config/src/service_tests.rs +++ b/codex-rs/cloud-config/src/service_tests.rs @@ -666,7 +666,10 @@ async fn get_bundle_skips_team_like_business_plans() { #[tokio::test] async fn get_bundle_rejects_invalid_remote_bundle_before_cache_write() { let codex_home = tempdir().expect("tempdir"); - let fetcher = Arc::new(StaticBundleClient::new(invalid_config_bundle())); + let mut invalid = invalid_config_bundle(); + invalid.config_toml.enterprise_managed[0].contents = + "secret_credential = [secret-token".to_string(); + let fetcher = Arc::new(StaticBundleClient::new(invalid)); let service = CloudConfigBundleService::new( auth_manager_with_plan("business").await, fetcher.clone(), @@ -681,6 +684,8 @@ async fn get_bundle_rejects_invalid_remote_bundle_before_cache_write() { assert_eq!(err.code(), CloudConfigBundleLoadErrorCode::InvalidBundle); assert!(err.to_string().contains("invalid cloud config bundle")); + assert!(!err.to_string().contains("secret")); + assert!(!format!("{err:?}").contains("secret")); assert_eq!(fetcher.request_count.load(Ordering::SeqCst), 1); assert!( !codex_home @@ -691,15 +696,16 @@ async fn get_bundle_rejects_invalid_remote_bundle_before_cache_write() { } #[tokio::test] -async fn invalid_cloud_provider_does_not_replace_cached_bundle() { +async fn invalid_requirements_do_not_replace_cached_bundle() { let codex_home = tempdir().expect("tempdir"); let cache = create_test_cache(codex_home.path()); let previous = test_bundle(); write_test_cache(&cache, previous.clone()).await; for contents in [ + "allowed_approval_policies = ['secret-policy']", "[model_providers.openai]\nname = 'Reserved'", - "[model_providers.gateway]\nname = ' '", + "[model_providers.secret-provider]\nname = ' '", "[model_providers.gateway]\nbase_url = 'https://gateway.example/v1'", "[model_providers.amazon-bedrock]\nname = 'Managed Bedrock'", "[model_providers.amazon-bedrock-runtime]\nname = 'Managed Bedrock Runtime'", @@ -719,8 +725,10 @@ async fn invalid_cloud_provider_does_not_replace_cached_bundle() { let error = service .validate_and_cache_remote_bundle(&auth, "refresh", /*attempt*/ 1, invalid) .await - .expect_err("invalid provider must fail before cache write"); + .expect_err("invalid requirements must fail before cache write"); assert_eq!(error.code(), CloudConfigBundleLoadErrorCode::InvalidBundle); + assert!(!error.to_string().contains("secret")); + assert!(!format!("{error:?}").contains("secret")); assert_eq!( cache .load(Some("user-12345"), Some("account-12345")) diff --git a/codex-rs/cloud-config/src/validation.rs b/codex-rs/cloud-config/src/validation.rs index b8304a679a..4718658ac4 100644 --- a/codex-rs/cloud-config/src/validation.rs +++ b/codex-rs/cloud-config/src/validation.rs @@ -11,11 +11,11 @@ pub(crate) fn validate_bundle( base_dir: &AbsolutePathBuf, ) -> Result<(), CloudConfigBundleLoadError> { let bundle_layers = - CloudConfigBundleLayers::from_bundle(bundle.clone(), base_dir).map_err(|err| { + CloudConfigBundleLayers::from_bundle(bundle.clone(), base_dir).map_err(|_| { CloudConfigBundleLoadError::new( CloudConfigBundleLoadErrorCode::InvalidBundle, /*status_code*/ None, - format!("invalid cloud config bundle: {err}"), + "invalid cloud config bundle: invalid TOML", ) })?; let CloudConfigBundleLayers { @@ -23,19 +23,19 @@ pub(crate) fn validate_bundle( enterprise_managed_requirements, } = bundle_layers; - let requirements = compose_requirements(enterprise_managed_requirements).map_err(|err| { + let requirements = compose_requirements(enterprise_managed_requirements).map_err(|_| { CloudConfigBundleLoadError::new( CloudConfigBundleLoadErrorCode::InvalidBundle, /*status_code*/ None, - format!("invalid cloud config bundle: {err}"), + "invalid cloud config bundle: invalid requirements", ) })?; if let Some(providers) = requirements.and_then(|requirements| requirements.model_providers) { - validate_model_providers(&providers).map_err(|err| { + validate_model_providers(&providers).map_err(|_| { CloudConfigBundleLoadError::new( CloudConfigBundleLoadErrorCode::InvalidBundle, /*status_code*/ None, - format!("invalid cloud config bundle: {err}"), + "invalid cloud config bundle: invalid model provider configuration", ) })?; } diff --git a/codex-rs/config/src/config_server.rs b/codex-rs/config/src/config_server.rs index 2a9159f03e..00c5213860 100644 --- a/codex-rs/config/src/config_server.rs +++ b/codex-rs/config/src/config_server.rs @@ -7,9 +7,6 @@ use std::fmt; pub use codex_model_provider_info::OAuthSessionConfig as ConfigServerOAuth; -// Deployment-specific prefixes belong in base_url. -const BUNDLE_PATH: &str = "codex/config/bundle"; - #[derive(Clone, PartialEq, Eq, Deserialize, Serialize)] #[serde(try_from = "ConfigServerInput")] pub struct ConfigServer { @@ -33,29 +30,24 @@ impl TryFrom for ConfigServer { base_url: input.base_url, oauth: input.oauth, }; - service.endpoint_url()?; + service.validated_base_url()?; Ok(service) } } impl ConfigServer { - /// Validates the trusted settings and appends the fixed bundle route to the base URL. - pub fn endpoint_url(&self) -> Result { + /// Validates the trusted settings and returns the normalized base URL. + pub fn validated_base_url(&self) -> Result { let validate_https_url = |value: &str| { codex_http_client::validate_endpoint_url(value, "config_server") .ok() .filter(|url| url.scheme() == "https") .ok_or("config_server URLs must use HTTPS without userinfo or fragments") }; - let mut endpoint = validate_https_url(&self.base_url)?; + let endpoint = validate_https_url(&self.base_url)?; if endpoint.query().is_some() { return Err("config_server.base_url must not contain a query".into()); } - endpoint - .path_segments_mut() - .map_err(|_| "config_server.base_url cannot be used as a base URL")? - .pop_if_empty() - .extend(BUNDLE_PATH.split('/')); if let Some(oauth) = &self.oauth { validate_https_url(&oauth.authorization_url)?; validate_https_url(&oauth.token_url)?; diff --git a/codex-rs/login/src/outbound_proxy.rs b/codex-rs/login/src/outbound_proxy.rs index 89fca57e08..be183a52e9 100644 --- a/codex-rs/login/src/outbound_proxy.rs +++ b/codex-rs/login/src/outbound_proxy.rs @@ -30,22 +30,34 @@ impl AuthRouteConfig { } /// Installs the configuration owner's local-only policy for auth discovery. - /// Only auth-owned endpoint constructors can access this factory. + /// Endpoint constructors access it through destination-restricted factories. pub fn with_local_bootstrap_factory(mut self, factory: HttpClientFactory) -> Self { self.local_bootstrap_factory = Some(factory); self } pub(crate) fn authentication_factory(&self, endpoint: &str) -> HttpClientFactory { - let Some(factory) = &self.local_bootstrap_factory else { + if self.local_bootstrap_factory.is_none() { return self.http_client_factory.clone(); - }; - let endpoints = endpoint.parse().into_iter().collect(); + } + self.http_client_factory_for_bootstrap(endpoint.parse::()) + } + + /// Uses local administrator network rules when available, limited to the supplied endpoints. + /// Fetched policy must not block the sign-in and configuration requests needed to refresh it. + pub fn http_client_factory_for_bootstrap( + &self, + endpoints: impl IntoIterator, + ) -> HttpClientFactory { + let factory = self + .local_bootstrap_factory + .as_ref() + .unwrap_or(&self.http_client_factory); factory.clone().with_network_policy( factory .network_policy() .clone() - .restrict_to_endpoints(endpoints), + .restrict_to_endpoints(endpoints.into_iter().collect()), ) }