From cc2f80d5157ccf7540589de764318315c4443948 Mon Sep 17 00:00:00 2001 From: alexsong-oai Date: Fri, 30 Oct 2026 02:20:17 +0000 Subject: [PATCH] Add file-backed storage for gateway OAuth credentials (#58219) ## What changed Allow gateway OAuth configuration to select `credentials_store = "file"` to persist credentials in plaintext at `$CODEX_HOME/secrets/gateway_oauth.json`. Keep `keyring` as the default. Read, update, and delete file-backed credentials by credential ID, using atomic writes and the existing cross-process lock. Preserve the serialized provider identity when using the default store. ## Testing Add an app-server test that completes gateway OAuth login with file storage, verifies Unix file permissions are `0600`, and checks that login remains successful after a server restart. GitOrigin-RevId: dc1d17922bd80bf65034cbc5aa3d97959e962dcc --- codex-rs/Cargo.lock | 1 + .../tests/suite/v2/gateway_oauth_tests.rs | 55 +++++++++++ .../cloud-config/src/config_server_tests.rs | 1 + codex-rs/core/config.schema.json | 26 +++++ .../suite/client_websockets_runtime_auth.rs | 1 + codex-rs/login/Cargo.toml | 1 + codex-rs/login/src/oauth_session.rs | 21 ++-- codex-rs/login/src/oauth_session_storage.rs | 99 +++++++++++++++---- codex-rs/login/src/oauth_session_tests.rs | 2 + .../model-provider-info/src/gateway_oauth.rs | 13 +++ codex-rs/model-provider-info/src/lib.rs | 1 + .../model-provider-info/src/oauth_session.rs | 14 +++ codex-rs/model-provider/src/shared_state.rs | 1 + .../src/shared_state_test_support.rs | 1 + .../model-provider/src/shared_state_tests.rs | 8 ++ 15 files changed, 211 insertions(+), 34 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index a98d142497..954ae00151 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3965,6 +3965,7 @@ dependencies = [ "codex-secrets", "codex-terminal-detection", "codex-utils-cargo-bin", + "codex-utils-path", "codex-utils-process", "codex-utils-template", "codex-workload-identity", diff --git a/codex-rs/app-server/tests/suite/v2/gateway_oauth_tests.rs b/codex-rs/app-server/tests/suite/v2/gateway_oauth_tests.rs index 1b9de1cd96..ac278cbf1d 100644 --- a/codex-rs/app-server/tests/suite/v2/gateway_oauth_tests.rs +++ b/codex-rs/app-server/tests/suite/v2/gateway_oauth_tests.rs @@ -7,6 +7,7 @@ use codex_app_server_protocol::ClientInfo; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::GatewayOAuthCancelResponse; use codex_app_server_protocol::GatewayOAuthChangedNotification; +use codex_app_server_protocol::GatewayOAuthLoginResponse; use codex_app_server_protocol::GatewayOAuthReadResponse; use codex_app_server_protocol::GatewayOAuthStatus; use codex_app_server_protocol::InitializeCapabilities; @@ -172,6 +173,60 @@ async fn model_list_leaves_oauth_gateway_after_account_change() -> Result<()> { Ok(()) } +#[tokio::test] +async fn file_credentials_reuse_gateway_login_after_restart() -> Result<()> { + let home = TempDir::new()?; + let oauth = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/token")) + .and(body_string_contains("grant_type=authorization_code")) + .respond_with(ResponseTemplate::new(/*s*/ 200).set_body_json(serde_json::json!({ + "access_token": "gateway-access", "refresh_token": "gateway-refresh", "expires_in": 3600, + }))) + .expect(/*r*/ 1) + .mount(&oauth) + .await; + // Exercise the managed provider definition used by headless installations. + std::fs::write( + home.path().join("requirements.toml"), + format!("{}\ncredentials_store = 'file'\n", gateway_config(&oauth)), + )?; + let mut server = gateway_server(home.path()).await?; + let login = server + .send_raw_request("account/gatewayOAuth/login", /*params*/ None) + .await?; + let url = authorization_url(&mut server).await?; + let query = url + .query_pairs() + .collect::>(); + let mut callback = url::Url::parse(query.get("redirect_uri").context("callback URI")?)?; + callback + .query_pairs_mut() + .append_pair("code", "accepted") + .append_pair("state", query.get("state").context("OAuth state")?); + codex_login::default_client::create_client_without_request_logging() + .get(callback.as_str()) + .send() + .await? + .error_for_status()?; + let _: GatewayOAuthLoginResponse = server.read_response(login).await?; + server.shutdown_gracefully().await?; + + let path = home.path().join("secrets/gateway_oauth.json"); + assert!(path.is_file()); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!(std::fs::metadata(path)?.permissions().mode() & 0o777, 0o600); + } + let mut restarted = gateway_server(home.path()).await?; + assert_eq!( + read_gateway(&mut restarted).await?.status, + Some(GatewayOAuthStatus::Succeeded) + ); + Ok(()) +} + #[tokio::test] async fn token_exchange_failure_completes_login_and_updates_readiness() -> Result<()> { let home = TempDir::new()?; diff --git a/codex-rs/cloud-config/src/config_server_tests.rs b/codex-rs/cloud-config/src/config_server_tests.rs index d07566d286..2a09f7261e 100644 --- a/codex-rs/cloud-config/src/config_server_tests.rs +++ b/codex-rs/cloud-config/src/config_server_tests.rs @@ -44,6 +44,7 @@ fn oauth_config(base_url: &str) -> OAuthSessionConfig { resource: None, scopes: vec![], redirect_port: None, + credentials_store: Default::default(), } } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 3947d5da2f..98a91b71fc 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -1991,6 +1991,14 @@ "client_id": { "type": "string" }, + "credentials_store": { + "allOf": [ + { + "$ref": "#/definitions/OAuthCredentialsStore" + } + ], + "description": "Where Codex stores this gateway's OAuth credentials. Defaults to the keyring-backed store." + }, "delivery": { "allOf": [ { @@ -3722,6 +3730,24 @@ } ] }, + "OAuthCredentialsStore": { + "oneOf": [ + { + "description": "Encrypt credentials using a passphrase kept in the OS keyring.", + "enum": [ + "keyring" + ], + "type": "string" + }, + { + "description": "Store credentials in plaintext in `$CODEX_HOME/secrets/gateway_oauth.json`.", + "enum": [ + "file" + ], + "type": "string" + } + ] + }, "OAuthCredentialsStoreMode": { "description": "Determine where Codex should store and read MCP credentials.", "oneOf": [ diff --git a/codex-rs/core/tests/suite/client_websockets_runtime_auth.rs b/codex-rs/core/tests/suite/client_websockets_runtime_auth.rs index 5eb1ea55ed..5d8408b629 100644 --- a/codex-rs/core/tests/suite/client_websockets_runtime_auth.rs +++ b/codex-rs/core/tests/suite/client_websockets_runtime_auth.rs @@ -96,6 +96,7 @@ fn gateway_config(server: &MockServer, delivery: GatewayOAuthDelivery) -> Gatewa resource: None, scopes: vec![], redirect_port: None, + credentials_store: Default::default(), delivery, } } diff --git a/codex-rs/login/Cargo.toml b/codex-rs/login/Cargo.toml index 50de4dc83a..3eba4b495b 100644 --- a/codex-rs/login/Cargo.toml +++ b/codex-rs/login/Cargo.toml @@ -21,6 +21,7 @@ codex-otel = { workspace = true } codex-protocol = { workspace = true } codex-secrets = { workspace = true } codex-terminal-detection = { workspace = true } +codex-utils-path = { workspace = true } codex-utils-template = { workspace = true } codex-workload-identity = { workspace = true } http = { workspace = true } diff --git a/codex-rs/login/src/oauth_session.rs b/codex-rs/login/src/oauth_session.rs index c62fa6135b..3e39d336e0 100644 --- a/codex-rs/login/src/oauth_session.rs +++ b/codex-rs/login/src/oauth_session.rs @@ -125,7 +125,7 @@ struct OAuthSessionCache { } impl OAuthSessionManager { - /// Creates a configured OAuth session using the caller's encrypted store and HTTP policy. + /// Creates an OAuth session using the configured credential store and the caller's HTTP policy. /// Token grants never follow redirects or include primary-provider credentials or request logs. pub fn new( config: OAuthSessionConfig, @@ -146,9 +146,13 @@ impl OAuthSessionManager { control: OAuthLoginControl::for_host(&codex_home, http_client_factory), allow_interactive_login: true, state: Arc::new(OAuthSessionState { + storage: OAuthSessionStorage::new( + codex_home.clone(), + config.credentials_store, + keyring, + ), config, status_tx: login::status_sender_for_host(&codex_home, http_client_factory), - storage: OAuthSessionStorage::new(codex_home.clone(), keyring), codex_home, http_client, cached_token: Arc::new(Mutex::new(OAuthSessionCache::default())), @@ -425,20 +429,11 @@ impl OAuthSessionManager { } fn load_token(&self) -> io::Result> { - self.state - .storage - .load(&self.credential_id())? - .map(|value| { - serde_json::from_str(&value) - .map_err(|_| io::Error::other("stored provider OAuth credentials are invalid")) - }) - .transpose() + self.state.storage.load(&self.credential_id()) } fn save_token(&self, token: &StoredToken) -> io::Result<()> { - let value = serde_json::to_string(token) - .map_err(|_| io::Error::other("failed to encode provider OAuth credentials"))?; - self.state.storage.save(&self.credential_id(), &value) + self.state.storage.save(&self.credential_id(), token) } async fn authorize_with_browser( diff --git a/codex-rs/login/src/oauth_session_storage.rs b/codex-rs/login/src/oauth_session_storage.rs index 41b34ec4f6..1eacc71d6f 100644 --- a/codex-rs/login/src/oauth_session_storage.rs +++ b/codex-rs/login/src/oauth_session_storage.rs @@ -1,6 +1,7 @@ -//! Persists configured OAuth grants and serializes exchanges in one encrypted store. +//! Persists configured OAuth grants and serializes exchanges across processes. //! Gateway storage paths and namespace are retained so existing credentials remain usable. +use std::collections::BTreeMap; use std::fs::File; use std::fs::OpenOptions; use std::fs::TryLockError; @@ -11,16 +12,20 @@ use std::sync::Arc; use std::time::Duration; use codex_keyring_store::KeyringStore; +use codex_model_provider_info::OAuthCredentialsStore; use codex_secrets::LocalSecretsNamespace; use codex_secrets::SecretName; use codex_secrets::SecretScope; use codex_secrets::SecretsBackendKind; use codex_secrets::SecretsManager; +use codex_utils_path::write_atomically; + +use super::StoredToken; pub(super) async fn lock_credentials(codex_home: &Path) -> io::Result { let directory = codex_home.join("secrets"); std::fs::create_dir_all(&directory)?; - // Configurations have separate credential entries but rewrite the same encrypted file. + // Configurations have separate credential entries but rewrite the same store file. // Keep one stable sidecar locked from the initial read through token exchange and save. let path = directory.join("gateway_oauth.lock"); let file = OpenOptions::new() @@ -50,37 +55,89 @@ pub(super) async fn lock_credentials(codex_home: &Path) -> io::Result { Ok(file) } -pub(super) struct OAuthSessionStorage(SecretsManager); +pub(super) enum OAuthSessionStorage { + Keyring(SecretsManager), + File(PathBuf), +} impl OAuthSessionStorage { - pub(super) fn new(codex_home: PathBuf, keyring: Arc) -> Self { - Self(SecretsManager::new_with_keyring_store_and_namespace( - codex_home, - SecretsBackendKind::Local, - keyring, - LocalSecretsNamespace::GatewayOAuth, - )) + pub(super) fn new( + codex_home: PathBuf, + mode: OAuthCredentialsStore, + keyring: Arc, + ) -> Self { + match mode { + OAuthCredentialsStore::Keyring => { + Self::Keyring(SecretsManager::new_with_keyring_store_and_namespace( + codex_home, + SecretsBackendKind::Local, + keyring, + LocalSecretsNamespace::GatewayOAuth, + )) + } + OAuthCredentialsStore::File => { + Self::File(codex_home.join("secrets").join("gateway_oauth.json")) + } + } } - pub(super) fn load(&self, credential_id: &str) -> io::Result> { - self.0 - .get(&SecretScope::Global, &secret_name(credential_id)?) - .map_err(|_| io::Error::other("failed to load provider OAuth credentials")) + pub(super) fn load(&self, credential_id: &str) -> io::Result> { + match self { + Self::Keyring(secrets) => secrets + .get(&SecretScope::Global, &secret_name(credential_id)?) + .map_err(|_| io::Error::other("failed to load provider OAuth credentials"))? + .map(|value| serde_json::from_str(&value).map_err(invalid_credentials)) + .transpose(), + Self::File(path) => Ok(read_tokens(path)?.remove(credential_id)), + } } - pub(super) fn save(&self, credential_id: &str, value: &str) -> io::Result<()> { - self.0 - .set(&SecretScope::Global, &secret_name(credential_id)?, value) - .map_err(|_| io::Error::other("failed to save provider OAuth credentials")) + pub(super) fn save(&self, credential_id: &str, token: &StoredToken) -> io::Result<()> { + match self { + Self::Keyring(secrets) => { + let value = serde_json::to_string(token)?; + secrets + .set(&SecretScope::Global, &secret_name(credential_id)?, &value) + .map_err(|_| io::Error::other("failed to save provider OAuth credentials")) + } + Self::File(path) => { + let mut tokens = read_tokens(path)?; + tokens.insert(credential_id.to_owned(), token.clone()); + write_atomically(path, &serde_json::to_string(&tokens)?) + } + } } pub(super) fn delete(&self, credential_id: &str) -> io::Result { - self.0 - .delete(&SecretScope::Global, &secret_name(credential_id)?) - .map_err(|_| io::Error::other("failed to remove provider OAuth credentials")) + match self { + Self::Keyring(secrets) => secrets + .delete(&SecretScope::Global, &secret_name(credential_id)?) + .map_err(|_| io::Error::other("failed to remove provider OAuth credentials")), + Self::File(path) => { + let mut tokens = read_tokens(path)?; + let removed = tokens.remove(credential_id).is_some(); + if removed { + write_atomically(path, &serde_json::to_string(&tokens)?)?; + } + Ok(removed) + } + } } } +fn read_tokens(path: &Path) -> io::Result> { + match std::fs::read(path) { + Ok(bytes) => serde_json::from_slice(&bytes).map_err(invalid_credentials), + Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(BTreeMap::new()), + Err(err) => Err(err), + } +} + +fn invalid_credentials(_: serde_json::Error) -> io::Error { + // Deserialization errors can contain values from the credential file. + io::Error::other("stored provider OAuth credentials are invalid") +} + fn secret_name(credential_id: &str) -> io::Result { let digest = credential_id .strip_prefix("provider-oauth|") diff --git a/codex-rs/login/src/oauth_session_tests.rs b/codex-rs/login/src/oauth_session_tests.rs index 0c16ba7656..b08270640c 100644 --- a/codex-rs/login/src/oauth_session_tests.rs +++ b/codex-rs/login/src/oauth_session_tests.rs @@ -57,6 +57,7 @@ fn config(server: &MockServer) -> OAuthSessionConfig { resource: Some("https://gateway.example.test/codex".to_string()), scopes: vec!["openid".to_string(), "gateway.inference".to_string()], redirect_port: None, + credentials_store: Default::default(), } } @@ -69,6 +70,7 @@ fn loopback_config() -> OAuthSessionConfig { resource: None, scopes: Vec::new(), redirect_port: None, + credentials_store: Default::default(), } } diff --git a/codex-rs/model-provider-info/src/gateway_oauth.rs b/codex-rs/model-provider-info/src/gateway_oauth.rs index b611037c66..c320599847 100644 --- a/codex-rs/model-provider-info/src/gateway_oauth.rs +++ b/codex-rs/model-provider-info/src/gateway_oauth.rs @@ -10,6 +10,7 @@ use serde::Deserialize; use serde::Serialize; use crate::ModelProviderInfo; +use crate::OAuthCredentialsStore; // Reserve authentication, routing/framing, and internal protocol headers. const RESERVED_GATEWAY_OAUTH_HEADERS: &[&str] = &[ @@ -36,11 +37,23 @@ pub struct GatewayOAuthConfig { #[serde(default)] pub scopes: Vec, pub redirect_port: Option, + /// Where Codex stores this gateway's OAuth credentials. Defaults to the keyring-backed store. + #[serde(default, skip_serializing_if = "is_keyring")] + pub credentials_store: OAuthCredentialsStore, /// Omit to send the gateway token via the Authorization header #[serde(default)] pub delivery: GatewayOAuthDelivery, } +// Preserve the serialized provider and model cache identity for the default store. +#[expect( + clippy::trivially_copy_pass_by_ref, + reason = "Serde passes a reference to skip_serializing_if predicates" +)] +fn is_keyring(store: &OAuthCredentialsStore) -> bool { + *store == OAuthCredentialsStore::Keyring +} + impl fmt::Debug for GatewayOAuthConfig { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("GatewayOAuthConfig") diff --git a/codex-rs/model-provider-info/src/lib.rs b/codex-rs/model-provider-info/src/lib.rs index 64417fc3ee..9640849163 100644 --- a/codex-rs/model-provider-info/src/lib.rs +++ b/codex-rs/model-provider-info/src/lib.rs @@ -41,6 +41,7 @@ pub use capabilities::ModelProviderCapabilities; pub use capabilities::RemoteCompactionSupport; pub use gateway_oauth::GatewayOAuthConfig; pub use gateway_oauth::GatewayOAuthDelivery; +pub use oauth_session::OAuthCredentialsStore; pub use oauth_session::OAuthSessionConfig; pub const RESIDENCY_HEADER_NAME: &str = "x-openai-internal-codex-residency"; diff --git a/codex-rs/model-provider-info/src/oauth_session.rs b/codex-rs/model-provider-info/src/oauth_session.rs index cd4bfb2ee6..eccafe5023 100644 --- a/codex-rs/model-provider-info/src/oauth_session.rs +++ b/codex-rs/model-provider-info/src/oauth_session.rs @@ -1,5 +1,6 @@ //! Public-client OAuth settings shared by managed configuration and service sign-in. +use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; use std::fmt; @@ -17,6 +18,19 @@ pub struct OAuthSessionConfig { #[serde(default)] pub scopes: Vec, pub redirect_port: Option, + /// Selected by a gateway provider. Managed config-server OAuth uses the default store. + #[serde(skip)] + pub credentials_store: OAuthCredentialsStore, +} + +#[derive(Clone, Copy, Debug, Default, Deserialize, Serialize, PartialEq, Eq, JsonSchema)] +#[serde(rename_all = "lowercase")] +pub enum OAuthCredentialsStore { + /// Encrypt credentials using a passphrase kept in the OS keyring. + #[default] + Keyring, + /// Store credentials in plaintext in `$CODEX_HOME/secrets/gateway_oauth.json`. + File, } impl fmt::Debug for OAuthSessionConfig { diff --git a/codex-rs/model-provider/src/shared_state.rs b/codex-rs/model-provider/src/shared_state.rs index e6f90597a8..aabbe84583 100644 --- a/codex-rs/model-provider/src/shared_state.rs +++ b/codex-rs/model-provider/src/shared_state.rs @@ -46,6 +46,7 @@ impl ModelProviderSharedState { resource: config.resource.clone(), scopes: config.scopes.clone(), redirect_port: config.redirect_port, + credentials_store: config.credentials_store, }; let mut managers = self .gateway_managers diff --git a/codex-rs/model-provider/src/shared_state_test_support.rs b/codex-rs/model-provider/src/shared_state_test_support.rs index d7a60e063e..38bc88b709 100644 --- a/codex-rs/model-provider/src/shared_state_test_support.rs +++ b/codex-rs/model-provider/src/shared_state_test_support.rs @@ -79,6 +79,7 @@ pub fn seed_gateway_auth( resource: config.resource, scopes: config.scopes, redirect_port: config.redirect_port, + credentials_store: config.credentials_store, }; // Repeated fixtures sharing a Codex home must use the same encryption key. let keyring = Arc::new(TestKeyring(Mutex::new(Some( diff --git a/codex-rs/model-provider/src/shared_state_tests.rs b/codex-rs/model-provider/src/shared_state_tests.rs index df75e49c83..7d06dfea34 100644 --- a/codex-rs/model-provider/src/shared_state_tests.rs +++ b/codex-rs/model-provider/src/shared_state_tests.rs @@ -74,6 +74,7 @@ async fn gateway_credentials_accompany_primary_auth_in_models_and_responses() { resource: None, scopes: vec![], redirect_port: None, + credentials_store: Default::default(), delivery: delivery.clone(), }; let info = ModelProviderInfo { @@ -210,6 +211,7 @@ async fn gateway_preserves_runtime_auth_and_rejects_header_collisions() { resource: None, scopes: vec![], redirect_port: None, + credentials_store: Default::default(), delivery: GatewayOAuthDelivery::Header { name: "x-gateway-auth".into(), scheme: "Bearer".into(), @@ -311,6 +313,7 @@ async fn gateway_refresh_preserves_primary_auth_and_hides_issuer_errors() { resource: None, scopes: vec![], redirect_port: None, + credentials_store: Default::default(), delivery: GatewayOAuthDelivery::Header { name: "x-gateway-auth".into(), scheme: "Bearer".into(), @@ -405,6 +408,7 @@ async fn models_cache_is_reused_only_for_matching_gateway_configuration() { resource: None, scopes: vec![], redirect_port: None, + credentials_store: Default::default(), delivery: GatewayOAuthDelivery::Header { name: "x-gateway-auth".into(), scheme: "Bearer".into(), @@ -485,6 +489,7 @@ async fn models_observe_gateway_rotation(instance: ProviderInstance) { resource: None, scopes: vec![], redirect_port: None, + credentials_store: Default::default(), delivery: GatewayOAuthDelivery::Header { name: "x-gateway-auth".into(), scheme: "Bearer".into(), @@ -599,6 +604,7 @@ async fn gateway_setup_errors_are_reported_when_authentication_is_requested() { resource: None, scopes: vec![], redirect_port: None, + credentials_store: Default::default(), delivery: GatewayOAuthDelivery::Header { name: "x-gateway-auth".into(), scheme: "Bearer".into(), @@ -666,6 +672,7 @@ async fn gateway_login_required_invalidates_the_last_successful_catalog() { resource: None, scopes: vec![], redirect_port: None, + credentials_store: Default::default(), delivery: GatewayOAuthDelivery::Header { name: "x-gateway-auth".into(), scheme: "Bearer".into(), @@ -750,6 +757,7 @@ async fn gateway_login_states_stop_request_retries() { resource: None, scopes: vec![], redirect_port: None, + credentials_store: Default::default(), delivery: GatewayOAuthDelivery::Header { name: "x-gateway-auth".into(), scheme: "Bearer".into(),