diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index b5d2a1440..88510d371 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3660,6 +3660,7 @@ dependencies = [ "codex-exec-server", "codex-keyring-store", "codex-protocol", + "codex-secrets", "codex-utils-cargo-bin", "codex-utils-home-dir", "codex-utils-pty", diff --git a/codex-rs/app-server/src/mcp_refresh.rs b/codex-rs/app-server/src/mcp_refresh.rs index f8a1857fd..1517b354d 100644 --- a/codex-rs/app-server/src/mcp_refresh.rs +++ b/codex-rs/app-server/src/mcp_refresh.rs @@ -71,6 +71,8 @@ async fn build_refresh_config( config.mcp_oauth_credentials_store_mode, ) .map_err(io::Error::other)?, + auth_keyring_backend_kind: serde_json::to_value(config.auth_keyring_backend_kind()) + .map_err(io::Error::other)?, }) } @@ -104,6 +106,7 @@ mod tests { use codex_config::ThreadConfigLoadErrorCode; use codex_config::ThreadConfigLoader; use codex_config::ThreadConfigSource; + use codex_config::types::AuthKeyringBackendKind; use codex_core::config::ConfigOverrides; use codex_core::init_state_db; use codex_core::thread_store_from_config; @@ -142,6 +145,38 @@ mod tests { Ok(()) } + #[tokio::test] + async fn refresh_config_uses_latest_auth_keyring_backend() -> anyhow::Result<()> { + let (temp_dir, thread_manager, config_manager, _loader) = refresh_test_state().await?; + std::fs::write( + temp_dir.path().join(codex_config::CONFIG_TOML_FILE), + "[features]\nsecret_auth_storage = true\n", + )?; + + let mut good_thread = None; + for thread_id in thread_manager.list_thread_ids().await { + let thread = thread_manager.get_thread(thread_id).await?; + let thread_config = thread.config().await; + if thread_config.cwd.ends_with("good") { + good_thread = Some(thread); + break; + } + } + let thread = good_thread.expect("good test thread should exist"); + + let refresh_config = build_refresh_config(thread.as_ref(), &config_manager).await?; + let backend = serde_json::from_value::( + refresh_config.auth_keyring_backend_kind, + )?; + + assert_eq!( + thread.config().await.auth_keyring_backend_kind(), + AuthKeyringBackendKind::Direct + ); + assert_eq!(backend, AuthKeyringBackendKind::Secrets); + Ok(()) + } + async fn refresh_test_state() -> anyhow::Result<( TempDir, Arc, @@ -153,6 +188,10 @@ mod tests { let bad_cwd = temp_dir.path().join("bad"); std::fs::create_dir_all(&good_cwd)?; std::fs::create_dir_all(&bad_cwd)?; + std::fs::write( + temp_dir.path().join(codex_config::CONFIG_TOML_FILE), + "[features]\nsecret_auth_storage = false\n", + )?; let initial_config_manager = ConfigManager::without_managed_config_for_tests(temp_dir.path().to_path_buf()); diff --git a/codex-rs/app-server/src/request_processors/mcp_processor.rs b/codex-rs/app-server/src/request_processors/mcp_processor.rs index 855236df3..08d464598 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -161,6 +161,7 @@ impl McpRequestProcessor { &name, &url, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), http_headers, env_http_headers, &resolved_scopes.scopes, diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index cd4fccdca..cdc9bdd74 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -1701,6 +1701,7 @@ impl PluginRequestProcessor { ); let store_mode = config.mcp_oauth_credentials_store_mode; + let keyring_backend_kind = config.auth_keyring_backend_kind(); let callback_port = config.mcp_oauth_callback_port; let callback_url = config.mcp_oauth_callback_url.clone(); let outgoing = Arc::clone(&self.outgoing); @@ -1712,6 +1713,7 @@ impl PluginRequestProcessor { &name, &oauth_config.url, store_mode, + keyring_backend_kind, oauth_config.http_headers.clone(), oauth_config.env_http_headers.clone(), &resolved_scopes.scopes, @@ -1728,6 +1730,7 @@ impl PluginRequestProcessor { &name, &oauth_config.url, store_mode, + keyring_backend_kind, oauth_config.http_headers, oauth_config.env_http_headers, &[], diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 010378265..1466f1039 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -211,6 +211,7 @@ async fn perform_oauth_login_retry_without_scopes( name: &str, url: &str, store_mode: codex_config::types::OAuthCredentialsStoreMode, + keyring_backend_kind: codex_config::types::AuthKeyringBackendKind, http_headers: Option>, env_http_headers: Option>, resolved_scopes: &ResolvedMcpOAuthScopes, @@ -223,6 +224,7 @@ async fn perform_oauth_login_retry_without_scopes( name, url, store_mode, + keyring_backend_kind, http_headers.clone(), env_http_headers.clone(), &resolved_scopes.scopes, @@ -240,6 +242,7 @@ async fn perform_oauth_login_retry_without_scopes( name, url, store_mode, + keyring_backend_kind, http_headers, env_http_headers, &[], @@ -384,6 +387,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re &name, &oauth_config.url, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), oauth_config.http_headers, oauth_config.env_http_headers, &resolved_scopes, @@ -478,6 +482,7 @@ async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs) &name, &url, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), http_headers, env_http_headers, &resolved_scopes, @@ -514,7 +519,12 @@ async fn run_logout(config_overrides: &CliConfigOverrides, logout_args: LogoutAr _ => bail!("OAuth logout is only supported for streamable_http transports."), }; - match delete_oauth_tokens(&name, &url, config.mcp_oauth_credentials_store_mode) { + match delete_oauth_tokens( + &name, + &url, + config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), + ) { Ok(true) => println!("Removed OAuth credentials for '{name}'."), Ok(false) => println!("No OAuth credentials stored for '{name}'."), Err(err) => return Err(anyhow!("failed to delete OAuth credentials: {err}")), @@ -541,6 +551,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> let auth_statuses = compute_auth_statuses( effective_mcp_servers.iter(), config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), /*auth*/ None, ) .await; diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index 9d1be43d4..b7e318a39 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -42,6 +42,7 @@ use anyhow::anyhow; use async_channel::Sender; use codex_config::Constrained; use codex_config::McpServerTransportConfig; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_login::CodexAuth; use codex_protocol::mcp::CallToolResult; @@ -119,6 +120,7 @@ impl McpConnectionManager { pub async fn new( mcp_servers: &HashMap, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, auth_entries: HashMap, approval_policy: &Constrained, submit_id: String, @@ -198,6 +200,7 @@ impl McpConnectionManager { server_name.clone(), server, store_mode, + keyring_backend_kind, cancel_token.clone(), tx_event.clone(), elicitation_requests.clone(), diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index ea405ced9..508598d34 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -20,6 +20,7 @@ use crate::tools::normalize_tools_for_model_with_prefix; use crate::tools::tool_with_model_visible_input_schema; use codex_config::Constrained; use codex_config::McpServerConfig; +use codex_config::types::AuthKeyringBackendKind; use codex_exec_server::EnvironmentManager; use codex_protocol::ToolName; use codex_protocol::mcp::McpServerInfo; @@ -1213,6 +1214,7 @@ async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() { let manager = McpConnectionManager::new( &mcp_servers, OAuthCredentialsStoreMode::default(), + AuthKeyringBackendKind::default(), HashMap::new(), &approval_policy, String::new(), diff --git a/codex-rs/codex-mcp/src/mcp/auth.rs b/codex-rs/codex-mcp/src/mcp/auth.rs index 12f832f9e..ec67f07a8 100644 --- a/codex-rs/codex-mcp/src/mcp/auth.rs +++ b/codex-rs/codex-mcp/src/mcp/auth.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use anyhow::Result; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_login::CodexAuth; use codex_protocol::protocol::McpAuthStatus; @@ -130,6 +131,7 @@ pub fn should_retry_without_scopes(scopes: &ResolvedMcpOAuthScopes, error: &anyh pub async fn compute_auth_statuses<'a, I>( servers: I, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, auth: Option<&CodexAuth>, ) -> HashMap where @@ -152,7 +154,15 @@ where async move { let auth_status = match config.as_ref() { Some(config) => { - match compute_auth_status(&name, config, store_mode, has_runtime_auth).await { + match compute_auth_status( + &name, + config, + store_mode, + keyring_backend_kind, + has_runtime_auth, + ) + .await + { Ok(status) => status, Err(error) => { warn!( @@ -179,6 +189,7 @@ async fn compute_auth_status( server_name: &str, config: &McpServerConfig, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, has_runtime_auth: bool, ) -> Result { if !config.enabled { @@ -204,6 +215,7 @@ async fn compute_auth_status( http_headers.clone(), env_http_headers.clone(), store_mode, + keyring_backend_kind, ) .await } diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index eca8a28c5..1a0f900a2 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -21,6 +21,7 @@ use codex_config::Constrained; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; use codex_config::types::AppToolApproval; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_login::CodexAuth; use codex_plugin::PluginCapabilitySummary; @@ -114,6 +115,8 @@ pub struct McpConfig { pub codex_home: PathBuf, /// Preferred credential store for MCP OAuth tokens. pub mcp_oauth_credentials_store_mode: OAuthCredentialsStoreMode, + /// Backend used when MCP OAuth storage is configured for keyring-backed persistence. + pub auth_keyring_backend_kind: AuthKeyringBackendKind, /// Optional fixed localhost callback port for MCP OAuth login. pub mcp_oauth_callback_port: Option, /// Optional OAuth redirect URI override for MCP login. @@ -262,6 +265,7 @@ pub async fn read_mcp_resource( let auth_statuses = compute_auth_statuses( mcp_servers.iter(), config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind, auth, ) .await; @@ -271,6 +275,7 @@ pub async fn read_mcp_resource( let manager = McpConnectionManager::new( &mcp_servers, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind, auth_statuses, &config.approval_policy, String::new(), @@ -330,6 +335,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( let auth_status_entries = compute_auth_statuses( mcp_servers.iter(), config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind, auth, ) .await; @@ -343,6 +349,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( let mcp_connection_manager = McpConnectionManager::new( &mcp_servers, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind, auth_status_entries.clone(), &config.approval_policy, submit_id, diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 111c810a1..ecaccd484 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -2,6 +2,7 @@ use super::*; use crate::McpServerRegistration; use codex_config::Constrained; use codex_config::types::AppToolApproval; +use codex_config::types::AuthKeyringBackendKind; use codex_login::CodexAuth; use codex_plugin::AppConnectorId; use codex_plugin::PluginCapabilitySummary; @@ -20,6 +21,7 @@ fn test_mcp_config(codex_home: PathBuf) -> McpConfig { apps_mcp_product_sku: None, codex_home, mcp_oauth_credentials_store_mode: OAuthCredentialsStoreMode::default(), + auth_keyring_backend_kind: AuthKeyringBackendKind::default(), mcp_oauth_callback_port: None, mcp_oauth_callback_url: None, skill_mcp_dependency_install_enabled: true, diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index 78078720e..f4fc8b807 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -45,6 +45,7 @@ use codex_async_utils::CancelErr; use codex_async_utils::OrCancelExt; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_exec_server::HttpClient; use codex_exec_server::ReqwestHttpClient; @@ -141,6 +142,7 @@ impl AsyncManagedClient { server_name: String, server: EffectiveMcpServer, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, cancel_token: CancellationToken, tx_event: Sender, elicitation_requests: ElicitationRequestManager, @@ -179,6 +181,7 @@ impl AsyncManagedClient { &server_name, server.clone(), store_mode, + keyring_backend_kind, runtime_context, runtime_auth_provider, ) @@ -577,6 +580,7 @@ async fn make_rmcp_client( server_name: &str, server: EffectiveMcpServer, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, runtime_context: McpRuntimeContext, runtime_auth_provider: Option, ) -> Result { @@ -648,6 +652,7 @@ async fn make_rmcp_client( http_headers, env_http_headers, store_mode, + keyring_backend_kind, http_client, runtime_auth_provider, ) diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index d3905f8c9..82fbe2c93 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1443,6 +1443,7 @@ impl Config { apps_mcp_product_sku: self.apps_mcp_product_sku.clone(), codex_home: self.codex_home.to_path_buf(), mcp_oauth_credentials_store_mode: self.mcp_oauth_credentials_store_mode, + auth_keyring_backend_kind: self.auth_keyring_backend_kind(), mcp_oauth_callback_port: self.mcp_oauth_callback_port, mcp_oauth_callback_url: self.mcp_oauth_callback_url.clone(), skill_mcp_dependency_install_enabled: self diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index f21912fdb..cbd738d93 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -280,6 +280,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager( let auth_status_entries = compute_auth_statuses( mcp_servers.iter(), config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), auth.as_ref(), ) .await; @@ -291,6 +292,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager( let mcp_connection_manager = McpConnectionManager::new( &mcp_servers, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), auth_status_entries, &config.permissions.approval_policy, INITIAL_SUBMIT_ID.to_owned(), diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index 2c5507a01..32e95b80e 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -152,6 +152,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( &name, &oauth_config.url, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), oauth_config.http_headers.clone(), oauth_config.env_http_headers.clone(), &resolved_scopes.scopes, @@ -168,6 +169,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( &name, &oauth_config.url, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), oauth_config.http_headers, oauth_config.env_http_headers, &[], @@ -202,6 +204,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( turn_context, refresh_servers, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), elicitation_reviewer, ) .await; diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 723948b7b..b413704cd 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -1260,6 +1260,7 @@ async fn install_host_owned_codex_apps_manager(session: &Session, turn_context: let manager = codex_mcp::McpConnectionManager::new( &HashMap::new(), turn_context.config.mcp_oauth_credentials_store_mode, + turn_context.config.auth_keyring_backend_kind(), HashMap::new(), &turn_context.approval_policy, turn_context.sub_id.clone(), diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index eacbbcee9..c033511a7 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -299,6 +299,7 @@ impl Session { turn_context: &TurnContext, mcp_servers: HashMap, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, elicitation_reviewer: Option, ) { let auth = self.services.auth_manager.auth().await; @@ -309,8 +310,13 @@ impl Session { effective_mcp_servers_from_configured(mcp_servers, &mcp_config, auth.as_ref()); let host_owned_codex_apps_enabled = host_owned_codex_apps_enabled(&mcp_config, auth.as_ref()); - let auth_statuses = - compute_auth_statuses(mcp_servers.iter(), store_mode, auth.as_ref()).await; + let auth_statuses = compute_auth_statuses( + mcp_servers.iter(), + store_mode, + keyring_backend_kind, + auth.as_ref(), + ) + .await; let mcp_runtime_context = match turn_context.environments.primary() { Some(turn_environment) => McpRuntimeContext::new( Arc::clone(&self.services.environment_manager), @@ -332,6 +338,7 @@ impl Session { let refreshed_manager = McpConnectionManager::new( &mcp_servers, store_mode, + keyring_backend_kind, auth_statuses, &turn_context.approval_policy, turn_context.sub_id.clone(), @@ -371,6 +378,7 @@ impl Session { let McpServerRefreshConfig { mcp_servers, mcp_oauth_credentials_store_mode, + auth_keyring_backend_kind, } = refresh_config; let mcp_servers = @@ -390,9 +398,23 @@ impl Session { return; } }; + let keyring_backend_kind = + match serde_json::from_value::(auth_keyring_backend_kind) { + Ok(kind) => kind, + Err(err) => { + warn!("failed to parse MCP auth keyring backend refresh config: {err}"); + return; + } + }; - self.refresh_mcp_servers_inner(turn_context, mcp_servers, store_mode, elicitation_reviewer) - .await; + self.refresh_mcp_servers_inner( + turn_context, + mcp_servers, + store_mode, + keyring_backend_kind, + elicitation_reviewer, + ) + .await; } pub(crate) async fn refresh_mcp_servers_now( @@ -400,10 +422,17 @@ impl Session { turn_context: &TurnContext, mcp_servers: HashMap, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, elicitation_reviewer: Option, ) { - self.refresh_mcp_servers_inner(turn_context, mcp_servers, store_mode, elicitation_reviewer) - .await; + self.refresh_mcp_servers_inner( + turn_context, + mcp_servers, + store_mode, + keyring_backend_kind, + elicitation_reviewer, + ) + .await; } #[cfg(test)] diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index b1013965d..cc40c6d47 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -50,6 +50,7 @@ use codex_analytics::SubAgentThreadStartedInput; use codex_analytics::TurnCodexErrorFact; use codex_app_server_protocol::McpServerElicitationRequest; use codex_app_server_protocol::McpServerElicitationRequestParams; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_exec_server::Environment; use codex_exec_server::EnvironmentManager; diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 1601a6654..af4bda3b4 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -614,6 +614,7 @@ impl Session { let auth_statuses = compute_auth_statuses( mcp_servers.iter(), config_for_mcp.mcp_oauth_credentials_store_mode, + config_for_mcp.auth_keyring_backend_kind(), auth.as_ref(), ) .await; @@ -1145,6 +1146,7 @@ impl Session { let mcp_connection_manager = McpConnectionManager::new( &mcp_servers, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), auth_statuses, &session_configuration.approval_policy, INITIAL_SUBMIT_ID.to_owned(), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index a04290c5d..1250eeb58 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -7106,9 +7106,12 @@ async fn refresh_mcp_servers_is_deferred_until_next_turn() { let mcp_oauth_credentials_store_mode = serde_json::to_value(OAuthCredentialsStoreMode::Auto).expect("serialize store mode"); + let auth_keyring_backend_kind = + serde_json::to_value(AuthKeyringBackendKind::Secrets).expect("serialize keyring backend"); let refresh_config = McpServerRefreshConfig { mcp_servers: json!({}), mcp_oauth_credentials_store_mode, + auth_keyring_backend_kind, }; { let mut guard = session.pending_mcp_server_refresh_config.lock().await; diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index f3b9a8ff3..27c76f0e9 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -181,6 +181,7 @@ pub struct McpServerRefreshConfig { pub mcp_servers: Value, /// OAuth credential store mode to use with this server snapshot. pub mcp_oauth_credentials_store_mode: Value, + pub auth_keyring_backend_kind: Value, } #[derive(Debug, Clone, PartialEq)] diff --git a/codex-rs/rmcp-client/Cargo.toml b/codex-rs/rmcp-client/Cargo.toml index 1006ad74e..c355e0ee6 100644 --- a/codex-rs/rmcp-client/Cargo.toml +++ b/codex-rs/rmcp-client/Cargo.toml @@ -21,6 +21,7 @@ codex-config = { workspace = true } codex-exec-server = { workspace = true } codex-keyring-store = { workspace = true } codex-protocol = { workspace = true } +codex-secrets = { workspace = true } codex-utils-pty = { workspace = true } codex-utils-home-dir = { workspace = true } bytes = { workspace = true } diff --git a/codex-rs/rmcp-client/src/auth_status.rs b/codex-rs/rmcp-client/src/auth_status.rs index bef9edc6c..cdb0a69bb 100644 --- a/codex-rs/rmcp-client/src/auth_status.rs +++ b/codex-rs/rmcp-client/src/auth_status.rs @@ -16,6 +16,7 @@ use crate::oauth::StoredOAuthTokenStatus; use crate::oauth::oauth_token_status; use crate::utils::apply_default_headers; use crate::utils::build_default_headers; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; const DISCOVERY_TIMEOUT: Duration = Duration::from_secs(5); @@ -35,6 +36,7 @@ pub async fn determine_streamable_http_auth_status( http_headers: Option>, env_http_headers: Option>, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Result { if bearer_token_env_var.is_some() { return Ok(McpAuthStatus::BearerToken); @@ -45,7 +47,7 @@ pub async fn determine_streamable_http_auth_status( return Ok(McpAuthStatus::BearerToken); } - match oauth_token_status(server_name, url, store_mode)? { + match oauth_token_status(server_name, url, store_mode, keyring_backend_kind)? { StoredOAuthTokenStatus::Usable => return Ok(McpAuthStatus::OAuth), StoredOAuthTokenStatus::AuthorizationRequired => { return Ok(McpAuthStatus::NotLoggedIn); @@ -288,6 +290,7 @@ mod tests { )])), /*env_http_headers*/ None, OAuthCredentialsStoreMode::Keyring, + AuthKeyringBackendKind::default(), ) .await .expect("status should compute"); @@ -309,6 +312,7 @@ mod tests { "CODEX_RMCP_CLIENT_AUTH_STATUS_TEST_TOKEN".to_string(), )])), OAuthCredentialsStoreMode::Keyring, + AuthKeyringBackendKind::default(), ) .await .expect("status should compute"); diff --git a/codex-rs/rmcp-client/src/oauth.rs b/codex-rs/rmcp-client/src/oauth.rs index 574266027..d0005d20a 100644 --- a/codex-rs/rmcp-client/src/oauth.rs +++ b/codex-rs/rmcp-client/src/oauth.rs @@ -19,7 +19,13 @@ use anyhow::Context; use anyhow::Error; use anyhow::Result; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; +use codex_secrets::LocalSecretsNamespace; +use codex_secrets::SecretName; +use codex_secrets::SecretScope; +use codex_secrets::SecretsBackendKind; +use codex_secrets::SecretsManager; use oauth2::AccessToken; use oauth2::RefreshToken; use oauth2::Scope; @@ -51,6 +57,7 @@ use tokio::sync::Mutex; use codex_utils_home_dir::find_codex_home; const KEYRING_SERVICE: &str = "Codex MCP Credentials"; +const MCP_OAUTH_SECRET_PREFIX: &str = "MCP_OAUTH"; const REFRESH_SKEW_MILLIS: u64 = 30_000; #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] @@ -87,15 +94,19 @@ pub(crate) fn load_oauth_tokens( server_name: &str, url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Result> { let keyring_store = DefaultKeyringStore; match store_mode { - OAuthCredentialsStoreMode::Auto => { - load_oauth_tokens_from_keyring_with_fallback_to_file(&keyring_store, server_name, url) - } + OAuthCredentialsStoreMode::Auto => load_oauth_tokens_from_keyring_with_fallback_to_file( + &keyring_store, + keyring_backend_kind, + server_name, + url, + ), OAuthCredentialsStoreMode::File => load_oauth_tokens_from_file(server_name, url), OAuthCredentialsStoreMode::Keyring => { - load_oauth_tokens_from_keyring(&keyring_store, server_name, url) + load_oauth_tokens_from_keyring(&keyring_store, keyring_backend_kind, server_name, url) .with_context(|| "failed to read OAuth tokens from keyring".to_string()) } } @@ -105,9 +116,10 @@ pub(crate) fn oauth_token_status( server_name: &str, url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Result { Ok( - match load_oauth_tokens(server_name, url, store_mode)?.as_ref() { + match load_oauth_tokens(server_name, url, store_mode, keyring_backend_kind)?.as_ref() { None => StoredOAuthTokenStatus::Missing, Some(tokens) if oauth_tokens_are_usable(tokens) => StoredOAuthTokenStatus::Usable, Some(_) => StoredOAuthTokenStatus::AuthorizationRequired, @@ -152,12 +164,13 @@ fn refresh_expires_in_from_timestamp(tokens: &mut StoredOAuthTokens) { } } -fn load_oauth_tokens_from_keyring_with_fallback_to_file( +fn load_oauth_tokens_from_keyring_with_fallback_to_file( keyring_store: &K, + keyring_backend_kind: AuthKeyringBackendKind, server_name: &str, url: &str, ) -> Result> { - match load_oauth_tokens_from_keyring(keyring_store, server_name, url) { + match load_oauth_tokens_from_keyring(keyring_store, keyring_backend_kind, server_name, url) { Ok(Some(tokens)) => Ok(Some(tokens)), Ok(None) => load_oauth_tokens_from_file(server_name, url), Err(error) => { @@ -168,7 +181,23 @@ fn load_oauth_tokens_from_keyring_with_fallback_to_file( } } -fn load_oauth_tokens_from_keyring( +fn load_oauth_tokens_from_keyring( + keyring_store: &K, + keyring_backend_kind: AuthKeyringBackendKind, + server_name: &str, + url: &str, +) -> Result> { + match keyring_backend_kind { + AuthKeyringBackendKind::Direct => { + load_oauth_tokens_from_direct_keyring(keyring_store, server_name, url) + } + AuthKeyringBackendKind::Secrets => { + load_oauth_tokens_from_secrets_keyring(keyring_store, server_name, url) + } + } +} + +fn load_oauth_tokens_from_direct_keyring( keyring_store: &K, server_name: &str, url: &str, @@ -186,26 +215,74 @@ fn load_oauth_tokens_from_keyring( } } +fn load_oauth_tokens_from_secrets_keyring( + keyring_store: &K, + server_name: &str, + url: &str, +) -> Result> { + let codex_home = find_codex_home()?; + let manager = SecretsManager::new_with_keyring_store_and_namespace( + codex_home.to_path_buf(), + SecretsBackendKind::Local, + Arc::new(keyring_store.clone()), + LocalSecretsNamespace::McpOAuth, + ); + let secret_name = compute_secret_name(server_name, url)?; + match manager + .get(&SecretScope::Global, &secret_name) + .context("failed to load MCP OAuth tokens from encrypted storage")? + { + Some(serialized) => { + let mut tokens: StoredOAuthTokens = serde_json::from_str(&serialized) + .context("failed to deserialize OAuth tokens from encrypted storage")?; + refresh_expires_in_from_timestamp(&mut tokens); + Ok(Some(tokens)) + } + None => Ok(None), + } +} + pub fn save_oauth_tokens( server_name: &str, tokens: &StoredOAuthTokens, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Result<()> { let keyring_store = DefaultKeyringStore; match store_mode { OAuthCredentialsStoreMode::Auto => save_oauth_tokens_with_keyring_with_fallback_to_file( &keyring_store, + keyring_backend_kind, server_name, tokens, ), OAuthCredentialsStoreMode::File => save_oauth_tokens_to_file(tokens), - OAuthCredentialsStoreMode::Keyring => { - save_oauth_tokens_with_keyring(&keyring_store, server_name, tokens) + OAuthCredentialsStoreMode::Keyring => save_oauth_tokens_with_keyring( + &keyring_store, + keyring_backend_kind, + server_name, + tokens, + ), + } +} + +fn save_oauth_tokens_with_keyring( + keyring_store: &K, + keyring_backend_kind: AuthKeyringBackendKind, + server_name: &str, + tokens: &StoredOAuthTokens, +) -> Result<()> { + match keyring_backend_kind { + AuthKeyringBackendKind::Direct => { + save_oauth_tokens_to_direct_keyring(keyring_store, server_name, tokens) + } + AuthKeyringBackendKind::Secrets => { + save_oauth_tokens_to_secrets_keyring(keyring_store, server_name, tokens) } } } -fn save_oauth_tokens_with_keyring( +fn save_oauth_tokens_to_direct_keyring( keyring_store: &K, server_name: &str, tokens: &StoredOAuthTokens, @@ -231,12 +308,38 @@ fn save_oauth_tokens_with_keyring( } } -fn save_oauth_tokens_with_keyring_with_fallback_to_file( +fn save_oauth_tokens_to_secrets_keyring( keyring_store: &K, server_name: &str, tokens: &StoredOAuthTokens, ) -> Result<()> { - match save_oauth_tokens_with_keyring(keyring_store, server_name, tokens) { + let serialized = serde_json::to_string(tokens).context("failed to serialize OAuth tokens")?; + let codex_home = find_codex_home()?; + let manager = SecretsManager::new_with_keyring_store_and_namespace( + codex_home.to_path_buf(), + SecretsBackendKind::Local, + Arc::new(keyring_store.clone()), + LocalSecretsNamespace::McpOAuth, + ); + let secret_name = compute_secret_name(server_name, &tokens.url)?; + manager + .set(&SecretScope::Global, &secret_name, &serialized) + .context("failed to write OAuth tokens to encrypted storage")?; + + let key = compute_store_key(server_name, &tokens.url)?; + if let Err(error) = delete_oauth_tokens_from_file(&key) { + warn!("failed to remove OAuth tokens from fallback storage: {error:?}"); + } + Ok(()) +} + +fn save_oauth_tokens_with_keyring_with_fallback_to_file( + keyring_store: &K, + keyring_backend_kind: AuthKeyringBackendKind, + server_name: &str, + tokens: &StoredOAuthTokens, +) -> Result<()> { + match save_oauth_tokens_with_keyring(keyring_store, keyring_backend_kind, server_name, tokens) { Ok(()) => Ok(()), Err(error) => { let message = error.to_string(); @@ -251,28 +354,36 @@ pub fn delete_oauth_tokens( server_name: &str, url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Result { let keyring_store = DefaultKeyringStore; - delete_oauth_tokens_from_keyring_and_file(&keyring_store, store_mode, server_name, url) + delete_oauth_tokens_from_keyring_and_file( + &keyring_store, + store_mode, + keyring_backend_kind, + server_name, + url, + ) } -fn delete_oauth_tokens_from_keyring_and_file( +fn delete_oauth_tokens_from_keyring_and_file( keyring_store: &K, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, server_name: &str, url: &str, ) -> Result { let key = compute_store_key(server_name, url)?; - let keyring_result = keyring_store.delete(KEYRING_SERVICE, &key); + let keyring_result = + delete_oauth_tokens_from_keyring(keyring_store, keyring_backend_kind, server_name, url); let keyring_removed = match keyring_result { Ok(removed) => removed, Err(error) => { - let message = error.message(); + let message = error.to_string(); warn!("failed to delete OAuth tokens from keyring: {message}"); match store_mode { OAuthCredentialsStoreMode::Auto | OAuthCredentialsStoreMode::Keyring => { - return Err(error.into_error()) - .context("failed to delete OAuth tokens from keyring"); + return Err(error).context("failed to delete OAuth tokens from keyring"); } OAuthCredentialsStoreMode::File => false, } @@ -283,6 +394,56 @@ fn delete_oauth_tokens_from_keyring_and_file( Ok(keyring_removed || file_removed) } +fn delete_oauth_tokens_from_keyring( + keyring_store: &K, + keyring_backend_kind: AuthKeyringBackendKind, + server_name: &str, + url: &str, +) -> Result { + match keyring_backend_kind { + AuthKeyringBackendKind::Direct => { + delete_oauth_tokens_from_direct_keyring(keyring_store, server_name, url) + } + AuthKeyringBackendKind::Secrets => { + let direct_removed = + delete_oauth_tokens_from_direct_keyring(keyring_store, server_name, url)?; + let secrets_removed = + delete_oauth_tokens_from_secrets_keyring(keyring_store, server_name, url)?; + Ok(direct_removed || secrets_removed) + } + } +} + +fn delete_oauth_tokens_from_direct_keyring( + keyring_store: &K, + server_name: &str, + url: &str, +) -> Result { + let key = compute_store_key(server_name, url)?; + keyring_store + .delete(KEYRING_SERVICE, &key) + .map_err(|error| Error::new(error.into_error())) +} + +fn delete_oauth_tokens_from_secrets_keyring( + keyring_store: &K, + server_name: &str, + url: &str, +) -> Result { + let codex_home = find_codex_home()?; + let manager = SecretsManager::new_with_keyring_store_and_namespace( + codex_home.to_path_buf(), + SecretsBackendKind::Local, + Arc::new(keyring_store.clone()), + LocalSecretsNamespace::McpOAuth, + ); + let secret_name = compute_secret_name(server_name, url)?; + let secrets_removed = manager + .delete(&SecretScope::Global, &secret_name) + .context("failed to delete OAuth tokens from encrypted storage")?; + Ok(secrets_removed) +} + #[derive(Clone)] pub(crate) struct OAuthPersistor { inner: Arc, @@ -293,6 +454,7 @@ struct OAuthPersistorInner { url: String, authorization_manager: Arc>, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, last_credentials: Mutex>, } @@ -302,6 +464,7 @@ impl OAuthPersistor { url: String, authorization_manager: Arc>, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, initial_credentials: Option, ) -> Self { Self { @@ -310,6 +473,7 @@ impl OAuthPersistor { url, authorization_manager, store_mode, + keyring_backend_kind, last_credentials: Mutex::new(initial_credentials), }), } @@ -349,7 +513,12 @@ impl OAuthPersistor { expires_at, }; if last_credentials.as_ref() != Some(&stored) { - save_oauth_tokens(&self.inner.server_name, &stored, self.inner.store_mode)?; + save_oauth_tokens( + &self.inner.server_name, + &stored, + self.inner.store_mode, + self.inner.keyring_backend_kind, + )?; *last_credentials = Some(stored); } } @@ -360,6 +529,7 @@ impl OAuthPersistor { &self.inner.server_name, &self.inner.url, self.inner.store_mode, + self.inner.keyring_backend_kind, ) { warn!( @@ -561,6 +731,21 @@ fn compute_store_key(server_name: &str, server_url: &str) -> Result { Ok(format!("{server_name}|{truncated}")) } +/// Derive a valid secret-store name from the MCP OAuth store key. +/// +/// `compute_store_key` intentionally includes readable identity components and +/// a pipe separator, but `SecretName` only allows `A-Z`, `0-9`, and `_`. +/// Re-hashing keeps the secret key deterministic while satisfying that +/// restricted alphabet. +fn compute_secret_name(server_name: &str, server_url: &str) -> Result { + let key = compute_store_key(server_name, server_url)?; + let mut hasher = Sha256::new(); + hasher.update(key.as_bytes()); + let digest = hasher.finalize(); + let hex = format!("{digest:X}"); + SecretName::new(&format!("{MCP_OAUTH_SECRET_PREFIX}_{}", &hex[..32])) +} + fn fallback_file_path() -> Result { Ok(find_codex_home()?.join(FALLBACK_FILENAME).to_path_buf()) } @@ -629,8 +814,10 @@ fn sha_256_prefix(value: &Value) -> Result { mod tests { use super::*; use anyhow::Result; + use codex_secrets::compute_keyring_account; use keyring::Error as KeyringError; use pretty_assertions::assert_eq; + use std::sync::Arc; use std::sync::Mutex; use std::sync::MutexGuard; use std::sync::OnceLock; @@ -660,6 +847,10 @@ mod tests { _dir: dir, } } + + fn path(&self) -> &std::path::Path { + self._dir.path() + } } impl Drop for TempCodexHome { @@ -680,9 +871,13 @@ mod tests { let key = super::compute_store_key(&tokens.server_name, &tokens.url)?; store.save(KEYRING_SERVICE, &key, &serialized)?; - let loaded = - super::load_oauth_tokens_from_keyring(&store, &tokens.server_name, &tokens.url)? - .expect("tokens should load from keyring"); + let loaded = super::load_oauth_tokens_from_keyring( + &store, + AuthKeyringBackendKind::Direct, + &tokens.server_name, + &tokens.url, + )? + .expect("tokens should load from keyring"); assert_tokens_match_without_expiry(&loaded, &expected); Ok(()) } @@ -698,6 +893,7 @@ mod tests { let loaded = super::load_oauth_tokens_from_keyring_with_fallback_to_file( &store, + AuthKeyringBackendKind::Direct, &tokens.server_name, &tokens.url, )? @@ -719,6 +915,7 @@ mod tests { let loaded = super::load_oauth_tokens_from_keyring_with_fallback_to_file( &store, + AuthKeyringBackendKind::Direct, &tokens.server_name, &tokens.url, )? @@ -738,6 +935,7 @@ mod tests { super::save_oauth_tokens_with_keyring_with_fallback_to_file( &store, + AuthKeyringBackendKind::Direct, &tokens.server_name, &tokens, )?; @@ -759,6 +957,7 @@ mod tests { super::save_oauth_tokens_with_keyring_with_fallback_to_file( &store, + AuthKeyringBackendKind::Direct, &tokens.server_name, &tokens, )?; @@ -779,6 +978,149 @@ mod tests { Ok(()) } + #[test] + fn save_oauth_tokens_with_secrets_backend_writes_encrypted_storage() -> Result<()> { + let env = TempCodexHome::new(); + let store = MockKeyringStore::default(); + let tokens = sample_tokens(); + let key = super::compute_store_key(&tokens.server_name, &tokens.url)?; + let serialized = serde_json::to_string(&tokens)?; + store.save(KEYRING_SERVICE, &key, &serialized)?; + super::save_oauth_tokens_to_file(&tokens)?; + + super::save_oauth_tokens_with_keyring_with_fallback_to_file( + &store, + AuthKeyringBackendKind::Secrets, + &tokens.server_name, + &tokens, + )?; + + let manager = SecretsManager::new_with_keyring_store_and_namespace( + env.path().to_path_buf(), + SecretsBackendKind::Local, + Arc::new(store.clone()), + LocalSecretsNamespace::McpOAuth, + ); + let secret_name = super::compute_secret_name(&tokens.server_name, &tokens.url)?; + let stored = manager + .get(&SecretScope::Global, &secret_name)? + .expect("tokens should be saved to encrypted storage"); + assert_eq!(serde_json::from_str::(&stored)?, tokens); + assert_eq!(store.saved_value(&key), Some(serialized)); + assert!(env.path().join("secrets").join("mcp_oauth.age").exists()); + assert!(!env.path().join("secrets").join("local.age").exists()); + assert!(!super::fallback_file_path()?.exists()); + Ok(()) + } + + #[test] + fn load_oauth_tokens_with_secrets_backend_reads_encrypted_storage() -> Result<()> { + let _env = TempCodexHome::new(); + let store = MockKeyringStore::default(); + let tokens = sample_tokens(); + let expected = tokens.clone(); + + super::save_oauth_tokens_with_keyring( + &store, + AuthKeyringBackendKind::Secrets, + &tokens.server_name, + &tokens, + )?; + + let loaded = super::load_oauth_tokens_from_keyring( + &store, + AuthKeyringBackendKind::Secrets, + &tokens.server_name, + &tokens.url, + )? + .expect("tokens should load from encrypted storage"); + assert_tokens_match_without_expiry(&loaded, &expected); + Ok(()) + } + + #[test] + fn load_oauth_tokens_with_secrets_backend_ignores_direct_entry() -> Result<()> { + let _env = TempCodexHome::new(); + let store = MockKeyringStore::default(); + let tokens = sample_tokens(); + let key = super::compute_store_key(&tokens.server_name, &tokens.url)?; + let serialized = serde_json::to_string(&tokens)?; + store.save(KEYRING_SERVICE, &key, &serialized)?; + + let loaded = super::load_oauth_tokens_from_keyring( + &store, + AuthKeyringBackendKind::Secrets, + &tokens.server_name, + &tokens.url, + )?; + + assert!(loaded.is_none()); + Ok(()) + } + + #[test] + fn save_oauth_tokens_with_secrets_backend_falls_back_to_file_when_keyring_fails() -> Result<()> + { + let env = TempCodexHome::new(); + let store = MockKeyringStore::default(); + store.set_error( + &compute_keyring_account(env.path()), + KeyringError::Invalid("error".into(), "save".into()), + ); + let tokens = sample_tokens(); + + super::save_oauth_tokens_with_keyring_with_fallback_to_file( + &store, + AuthKeyringBackendKind::Secrets, + &tokens.server_name, + &tokens, + )?; + + let saved = super::read_fallback_file()?.expect("fallback file should load"); + let key = super::compute_store_key(&tokens.server_name, &tokens.url)?; + assert!(saved.contains_key(&key)); + Ok(()) + } + + #[test] + fn delete_oauth_tokens_with_secrets_backend_removes_secrets_and_file() -> Result<()> { + let env = TempCodexHome::new(); + let store = MockKeyringStore::default(); + let tokens = sample_tokens(); + let serialized = serde_json::to_string(&tokens)?; + let key = super::compute_store_key(&tokens.server_name, &tokens.url)?; + store.save(KEYRING_SERVICE, &key, &serialized)?; + super::save_oauth_tokens_with_keyring( + &store, + AuthKeyringBackendKind::Secrets, + &tokens.server_name, + &tokens, + )?; + store.save(KEYRING_SERVICE, &key, &serialized)?; + super::save_oauth_tokens_to_file(&tokens)?; + + let removed = super::delete_oauth_tokens_from_keyring_and_file( + &store, + OAuthCredentialsStoreMode::Auto, + AuthKeyringBackendKind::Secrets, + &tokens.server_name, + &tokens.url, + )?; + + let manager = SecretsManager::new_with_keyring_store_and_namespace( + env.path().to_path_buf(), + SecretsBackendKind::Local, + Arc::new(store.clone()), + LocalSecretsNamespace::McpOAuth, + ); + let secret_name = super::compute_secret_name(&tokens.server_name, &tokens.url)?; + assert!(removed); + assert!(manager.get(&SecretScope::Global, &secret_name)?.is_none()); + assert!(store.saved_value(&key).is_none()); + assert!(!super::fallback_file_path()?.exists()); + Ok(()) + } + #[test] fn delete_oauth_tokens_removes_all_storage() -> Result<()> { let _env = TempCodexHome::new(); @@ -792,6 +1134,7 @@ mod tests { let removed = super::delete_oauth_tokens_from_keyring_and_file( &store, OAuthCredentialsStoreMode::Auto, + AuthKeyringBackendKind::Direct, &tokens.server_name, &tokens.url, )?; @@ -814,6 +1157,7 @@ mod tests { let removed = super::delete_oauth_tokens_from_keyring_and_file( &store, OAuthCredentialsStoreMode::Auto, + AuthKeyringBackendKind::Direct, &tokens.server_name, &tokens.url, )?; @@ -835,6 +1179,7 @@ mod tests { let result = super::delete_oauth_tokens_from_keyring_and_file( &store, OAuthCredentialsStoreMode::Auto, + AuthKeyringBackendKind::Direct, &tokens.server_name, &tokens.url, ); diff --git a/codex-rs/rmcp-client/src/perform_oauth_login.rs b/codex-rs/rmcp-client/src/perform_oauth_login.rs index a416b3d9b..8c72f320b 100644 --- a/codex-rs/rmcp-client/src/perform_oauth_login.rs +++ b/codex-rs/rmcp-client/src/perform_oauth_login.rs @@ -29,6 +29,7 @@ use crate::oauth::compute_expires_at_millis; use crate::save_oauth_tokens; use crate::utils::apply_default_headers; use crate::utils::build_default_headers; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; struct OauthHeaders { @@ -81,6 +82,7 @@ pub async fn perform_oauth_login( server_name: &str, server_url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, http_headers: Option>, env_http_headers: Option>, scopes: &[String], @@ -93,6 +95,7 @@ pub async fn perform_oauth_login( server_name, server_url, store_mode, + keyring_backend_kind, http_headers, env_http_headers, scopes, @@ -110,6 +113,7 @@ pub async fn perform_oauth_login_silent( server_name: &str, server_url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, http_headers: Option>, env_http_headers: Option>, scopes: &[String], @@ -122,6 +126,7 @@ pub async fn perform_oauth_login_silent( server_name, server_url, store_mode, + keyring_backend_kind, http_headers, env_http_headers, scopes, @@ -139,6 +144,7 @@ async fn perform_oauth_login_with_browser_output( server_name: &str, server_url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, http_headers: Option>, env_http_headers: Option>, scopes: &[String], @@ -156,6 +162,7 @@ async fn perform_oauth_login_with_browser_output( server_name, server_url, store_mode, + keyring_backend_kind, headers, scopes, oauth_client_id, @@ -175,6 +182,7 @@ pub async fn perform_oauth_login_return_url( server_name: &str, server_url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, http_headers: Option>, env_http_headers: Option>, scopes: &[String], @@ -192,6 +200,7 @@ pub async fn perform_oauth_login_return_url( server_name, server_url, store_mode, + keyring_backend_kind, headers, scopes, oauth_client_id, @@ -350,6 +359,7 @@ struct OauthLoginFlow { server_name: String, server_url: String, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, launch_browser: bool, timeout: Duration, } @@ -445,6 +455,7 @@ impl OauthLoginFlow { server_name: &str, server_url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, headers: OauthHeaders, scopes: &[String], oauth_client_id: Option<&str>, @@ -508,6 +519,7 @@ impl OauthLoginFlow { server_name: server_name.to_string(), server_url: server_url.to_string(), store_mode, + keyring_backend_kind, launch_browser, timeout, }) @@ -571,7 +583,12 @@ impl OauthLoginFlow { token_response: WrappedOAuthTokenResponse(credentials), expires_at, }; - save_oauth_tokens(&self.server_name, &stored, self.store_mode)?; + save_oauth_tokens( + &self.server_name, + &stored, + self.store_mode, + self.keyring_backend_kind, + )?; Ok(()) } diff --git a/codex-rs/rmcp-client/src/rmcp_client.rs b/codex-rs/rmcp-client/src/rmcp_client.rs index a28403e67..e9b014862 100644 --- a/codex-rs/rmcp-client/src/rmcp_client.rs +++ b/codex-rs/rmcp-client/src/rmcp_client.rs @@ -13,6 +13,7 @@ use anyhow::Result; use anyhow::anyhow; use codex_api::SharedAuthProvider; use codex_client::maybe_build_rustls_client_config_with_custom_ca; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::McpServerEnvVar; use codex_exec_server::HttpClient; use futures::FutureExt; @@ -124,6 +125,7 @@ enum TransportRecipe { http_headers: Option>, env_http_headers: Option>, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, http_client: Arc, auth_provider: Option, }, @@ -371,6 +373,7 @@ impl RmcpClient { http_headers: Option>, env_http_headers: Option>, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, http_client: Arc, auth_provider: Option, ) -> Result { @@ -381,6 +384,7 @@ impl RmcpClient { http_headers, env_http_headers, store_mode, + keyring_backend_kind, http_client, auth_provider, }; @@ -757,6 +761,7 @@ impl RmcpClient { http_headers, env_http_headers, store_mode, + keyring_backend_kind, http_client, auth_provider, } => { @@ -767,7 +772,7 @@ impl RmcpClient { && auth_provider.is_none() && !default_headers.contains_key(AUTHORIZATION) { - match load_oauth_tokens(server_name, url, *store_mode) { + match load_oauth_tokens(server_name, url, *store_mode, *keyring_backend_kind) { Ok(tokens) => tokens, Err(err) => { warn!("failed to read tokens for server `{server_name}`: {err}"); @@ -784,6 +789,7 @@ impl RmcpClient { url, initial_tokens.clone(), *store_mode, + *keyring_backend_kind, default_headers.clone(), Arc::clone(http_client), ) @@ -1127,6 +1133,7 @@ async fn create_oauth_transport_and_runtime( url: &str, initial_tokens: StoredOAuthTokens, credentials_store: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, default_headers: HeaderMap, http_client: Arc, ) -> Result<( @@ -1175,6 +1182,7 @@ async fn create_oauth_transport_and_runtime( url.to_string(), auth_manager, credentials_store, + keyring_backend_kind, Some(initial_tokens), ); diff --git a/codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs b/codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs index d7d8cf423..866daa67a 100644 --- a/codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs +++ b/codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs @@ -4,6 +4,7 @@ use std::time::Duration; use std::time::SystemTime; use std::time::UNIX_EPOCH; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_exec_server::Environment; use codex_rmcp_client::McpAuthStatus; @@ -157,7 +158,12 @@ async fn persisted_credentials_auth_status_child() -> anyhow::Result<()> { token_response: WrappedOAuthTokenResponse(response), expires_at: Some(0), }; - save_oauth_tokens(SERVER_NAME, &tokens, OAuthCredentialsStoreMode::File)?; + save_oauth_tokens( + SERVER_NAME, + &tokens, + OAuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), + )?; let status = auth_status(UNREFRESHABLE_SERVER_URL).await?; assert_eq!(status, McpAuthStatus::NotLoggedIn); @@ -178,7 +184,12 @@ async fn persisted_credentials_auth_status_child() -> anyhow::Result<()> { token_response: WrappedOAuthTokenResponse(response), expires_at: Some(now.saturating_add(/*rhs*/ 60_000)), }; - save_oauth_tokens(SERVER_NAME, &tokens, OAuthCredentialsStoreMode::File)?; + save_oauth_tokens( + SERVER_NAME, + &tokens, + OAuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), + )?; let status = auth_status(UNEXPIRED_SERVER_URL).await?; assert_eq!(status, McpAuthStatus::OAuth); @@ -196,7 +207,12 @@ async fn persisted_credentials_auth_status_child() -> anyhow::Result<()> { token_response: WrappedOAuthTokenResponse(response), expires_at: Some(0), }; - save_oauth_tokens(SERVER_NAME, &tokens, OAuthCredentialsStoreMode::File)?; + save_oauth_tokens( + SERVER_NAME, + &tokens, + OAuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), + )?; let status = auth_status(REFRESHABLE_SERVER_URL).await?; assert_eq!(status, McpAuthStatus::OAuth); @@ -211,6 +227,7 @@ async fn auth_status(server_url: &str) -> anyhow::Result { /*http_headers*/ None, /*env_http_headers*/ None, OAuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), ) .await } @@ -236,7 +253,12 @@ async fn oauth_startup_child() -> anyhow::Result<()> { token_response: WrappedOAuthTokenResponse(response), expires_at: Some(0), }; - save_oauth_tokens(SERVER_NAME, &tokens, OAuthCredentialsStoreMode::File)?; + save_oauth_tokens( + SERVER_NAME, + &tokens, + OAuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), + )?; // This mirrors create_client's transport and initialization setup, except // it omits the direct bearer token. Supplying that token would bypass the @@ -248,6 +270,7 @@ async fn oauth_startup_child() -> anyhow::Result<()> { /*http_headers*/ None, /*env_http_headers*/ None, OAuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), Environment::default_for_tests().get_http_client(), /*auth_provider*/ None, ) diff --git a/codex-rs/rmcp-client/tests/streamable_http_test_support.rs b/codex-rs/rmcp-client/tests/streamable_http_test_support.rs index 6b5df6e29..87a52f55d 100644 --- a/codex-rs/rmcp-client/tests/streamable_http_test_support.rs +++ b/codex-rs/rmcp-client/tests/streamable_http_test_support.rs @@ -17,6 +17,7 @@ use std::time::Duration; use std::time::Instant; use anyhow::Context as _; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_exec_server::Environment; use codex_exec_server::ExecServerClient; @@ -93,6 +94,7 @@ pub(crate) async fn create_client_with_http_client( /*http_headers*/ None, /*env_http_headers*/ None, OAuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), http_client, /*auth_provider*/ None, ) @@ -136,6 +138,7 @@ pub(crate) async fn create_remote_client( /*http_headers*/ None, /*env_http_headers*/ None, OAuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), Arc::new(http_client), /*auth_provider*/ None, )