From bf4d3f51ea70ce70ab7fabce7a66f328fef49e57 Mon Sep 17 00:00:00 2001 From: jif Date: Fri, 31 Jul 2026 10:37:04 +0000 Subject: [PATCH] Restrict hosted MCP credentials to local environments (#36306) ## Why Executor-owned MCP servers must not receive hosted ChatGPT actor credentials or credentials from the host's OAuth store. ## What changed - Provide the ChatGPT auth provider and consult stored OAuth credentials only for MCP servers in the local environment. - Require non-local servers configured with `auth = "chatgpt"` to supply a non-empty, valid static `Authorization` header. Reject environment-backed authorization so host secrets are not resolved for an executor-owned server. - Report unsupported authentication and fail startup before connecting when an executor-owned server has no acceptable explicit authorization. ## Testing Add coverage for local credential forwarding, non-local credential isolation, static header validation, auth status, OAuth lookup avoidance, and startup rejection. GitOrigin-RevId: e18f1878f260c93d9babd390a5a7eac86172299b --- .../src/connection_manager/startup.rs | 4 +- .../codex-mcp/src/connection_manager_tests.rs | 254 ++++++++++++++++++ codex-rs/codex-mcp/src/mcp/auth.rs | 9 + codex-rs/codex-mcp/src/rmcp_client.rs | 10 + codex-rs/codex-mcp/src/server.rs | 35 ++- 5 files changed, 310 insertions(+), 2 deletions(-) diff --git a/codex-rs/codex-mcp/src/connection_manager/startup.rs b/codex-rs/codex-mcp/src/connection_manager/startup.rs index 3518975ff0..b06401d83e 100644 --- a/codex-rs/codex-mcp/src/connection_manager/startup.rs +++ b/codex-rs/codex-mcp/src/connection_manager/startup.rs @@ -23,7 +23,9 @@ pub(super) fn chatgpt_auth_provider_for_server( server: &EffectiveMcpServer, chatgpt_auth_provider: Option, ) -> Option { - if !matches!(&server.config().auth, McpServerAuth::ChatGpt) { + if !matches!(&server.config().auth, McpServerAuth::ChatGpt) + || !server.config().is_local_environment() + { return None; } chatgpt_auth_provider diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index e78e05668d..c7a4c1f602 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -22,6 +22,7 @@ use crate::tools::filter_tools; use crate::tools::normalize_tools_for_model_with_prefix; use codex_config::AppToolApproval; use codex_config::Constrained; +use codex_config::McpServerAuth; use codex_config::McpServerConfig; use codex_config::McpServerEnvVar; use codex_config::McpServerToolConfig; @@ -32,6 +33,7 @@ use codex_connectors::ConnectorRuntimeContextKey; use codex_connectors::ConnectorRuntimeFetchSource; use codex_connectors::ConnectorRuntimeManager; use codex_exec_server_test_support::environment_manager_without_environments; +use codex_login::AuthHeaders; use codex_login::CodexAuth; use codex_protocol::ToolName; use codex_protocol::mcp::McpServerInfo; @@ -2748,6 +2750,258 @@ fn server_metadata_preserves_tool_approval_policy() { ); } +#[test] +fn hosted_actor_credentials_are_only_available_to_host_owned_mcp_servers() { + let bootstrap_auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let mut actor_headers = Default::default(); + codex_model_provider::auth_provider_from_auth(&bootstrap_auth) + .add_auth_headers(&mut actor_headers); + actor_headers.insert( + "x-openai-actor-authorization", + "hosted-actor-secret" + .parse() + .expect("valid actor authorization header"), + ); + let hosted_auth = CodexAuth::Headers(AuthHeaders::new(actor_headers)); + let provider = codex_model_provider::auth_provider_from_auth(&hosted_auth); + let mut local_config = crate::codex_apps_mcp_server_config( + "https://chatgpt.com", + /*apps_mcp_product_sku*/ None, + /*originator*/ None, + ); + local_config.auth = McpServerAuth::ChatGpt; + + let local_server = EffectiveMcpServer::configured(local_config.clone()); + let local_provider = + chatgpt_auth_provider_for_server(&local_server, Some(Arc::clone(&provider))) + .expect("host-owned Codex Apps must retain hosted authentication"); + assert_eq!( + local_provider + .to_auth_headers() + .get("x-openai-actor-authorization") + .and_then(|value| value.to_str().ok()), + Some("hosted-actor-secret") + ); + + let mut remote_config = local_config; + remote_config.environment_id = "customer-executor".to_string(); + let remote_server = EffectiveMcpServer::configured(remote_config); + assert!( + chatgpt_auth_provider_for_server(&remote_server, Some(provider)).is_none(), + "customer-owned executors must never receive hosted actor credentials" + ); +} + +#[tokio::test] +async fn executor_owned_chatgpt_mcp_accepts_only_safe_explicit_authorization() -> anyhow::Result<()> +{ + let codex_home = tempdir()?; + let environment_manager = Arc::new(environment_manager_without_environments()); + environment_manager.upsert_environment( + "customer-executor".to_string(), + "ws://127.0.0.1:1".to_string(), + /*connect_timeout*/ None, + )?; + let runtime_context = + McpRuntimeContext::new(Arc::clone(&environment_manager), PathBuf::from("/tmp")); + let bootstrap_auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let mut actor_headers = Default::default(); + codex_model_provider::auth_provider_from_auth(&bootstrap_auth) + .add_auth_headers(&mut actor_headers); + actor_headers.insert( + "x-openai-actor-authorization", + "hosted-actor-secret" + .parse() + .expect("valid actor authorization header"), + ); + let hosted_auth = CodexAuth::Headers(AuthHeaders::new(actor_headers)); + let runtime_config = crate::mcp::tests::test_mcp_config(codex_home.path().to_path_buf()); + let cases = [ + ("missing", None, None, None, false), + ("empty", Some(("Authorization", "")), None, None, false), + ( + "whitespace", + Some(("Authorization", " \t ")), + None, + None, + false, + ), + ( + "invalid newline", + Some(("Authorization", "Bearer executor\r\nsecret")), + None, + None, + false, + ), + ( + "invalid NUL", + Some(("Authorization", "Bearer executor\0secret")), + None, + None, + false, + ), + ( + "invalid DEL", + Some(("Authorization", "Bearer executor\u{007f}secret")), + None, + None, + false, + ), + ( + "environment header", + Some(("Authorization", "Bearer executor-secret")), + None, + Some(("aUtHoRiZaTiOn", "CODEX_TEST_HOSTED_SECRET")), + false, + ), + ( + "environment bearer", + Some(("Authorization", "Bearer executor-secret")), + Some("CODEX_TEST_HOSTED_SECRET"), + None, + false, + ), + ( + "mixed-case static header", + Some(("aUtHoRiZaTiOn", "Bearer executor-secret")), + None, + None, + true, + ), + ]; + + for (case, static_header, bearer_env_var, env_header, allows_executor_auth) in cases { + let mut server_json = serde_json::json!({ + "url": "https://chatgpt.com/backend-api/ps/mcp", + "auth": "chatgpt", + "environment_id": "customer-executor", + }); + if let Some((name, value)) = static_header { + server_json["http_headers"] = serde_json::json!({ name: value }); + } + if let Some(name) = bearer_env_var { + server_json["bearer_token_env_var"] = serde_json::json!(name); + } + if let Some((name, value)) = env_header { + server_json["env_http_headers"] = serde_json::json!({ name: value }); + } + let server_config = serde_json::from_value::(server_json)?; + let mcp_servers = crate::effective_mcp_servers_from_configured( + HashMap::from([("fake-first-party".to_string(), server_config)]), + &runtime_config, + Some(&hosted_auth), + ); + assert!(matches!( + mcp_servers["fake-first-party"].config().auth, + McpServerAuth::ChatGpt + )); + let remote_server = &mcp_servers["fake-first-party"]; + assert!( + chatgpt_auth_provider_for_server( + remote_server, + Some(codex_model_provider::auth_provider_from_auth(&hosted_auth)), + ) + .is_none(), + "{case}: executor-owned servers must never receive hosted actor credentials" + ); + let resolved_environment = + runtime_context.resolve_server_environment("fake-first-party", remote_server.config()); + let connection_identity = |keyring_backend_kind| { + McpServerConnectionIdentity::new( + "fake-first-party", + remote_server, + OAuthCredentialsStoreMode::File, + keyring_backend_kind, + &resolved_environment, + &runtime_context, + /*runtime_auth_provider*/ None, + Some(&hosted_auth), + /*codex_apps_cache_identity*/ None, + ElicitationCapability::default(), + /*supports_openai_form_elicitation*/ false, + ) + }; + let direct_keyring_identity = connection_identity(AuthKeyringBackendKind::Direct); + let secrets_keyring_identity = connection_identity(AuthKeyringBackendKind::Secrets); + assert!( + direct_keyring_identity.has_same_connection_config(&secrets_keyring_identity), + "{case}: executor-owned servers must not inspect orchestrator OAuth stores" + ); + assert!( + direct_keyring_identity + .oauth_credentials() + .expect("executor-owned ChatGPT authentication must skip OAuth lookup") + .is_none(), + "{case}: executor-owned servers must not retain hosted OAuth credentials" + ); + let auth_statuses = crate::compute_auth_statuses( + mcp_servers.iter(), + OAuthCredentialsStoreMode::default(), + AuthKeyringBackendKind::default(), + Some(&hosted_auth), + &runtime_context, + ) + .await; + let expected_auth_state = if allows_executor_auth { + McpAuthState::BearerToken + } else { + McpAuthState::Unsupported + }; + assert_eq!( + auth_statuses["fake-first-party"].auth_state, expected_auth_state, + "{case}: auth status must only accept safe executor-owned authorization" + ); + + let manager = McpConnectionSet::new( + /*previous*/ None, + McpPublicationGate::already_published(), + McpRuntimeInput { + config: Arc::new(runtime_config.clone()), + plugins_available: false, + ready_selected_capability_roots: Vec::new(), + mcp_servers, + submit_id: "security-test".to_string(), + tx_event: None, + startup_cancellation_token: CancellationToken::new(), + runtime_context: runtime_context.clone(), + codex_apps_tools_cache: ConnectorRuntimeManager::default(), + tool_catalog_cache: McpToolCatalogCache::default(), + codex_apps_tools_cache_key: ConnectorRuntimeContextKey::personal( + /*account_id*/ None, /*chatgpt_user_id*/ None, + ), + supports_openai_form_elicitation: false, + auth: Some(hosted_auth.clone()), + codex_apps_auth_manager: None, + elicitation_reviewer: None, + elicitation_lifecycle: None, + }, + ElicitationRequestRouter::default(), + ) + .await; + let error = match manager.test_client("fake-first-party").client().await { + Ok(_) => panic!("{case}: the unreachable fake executor must not connect"), + Err(error) => error, + }; + let StartupOutcomeError::Failed { error, .. } = error else { + panic!("{case}: executor-owned authentication must fail rather than be cancelled"); + }; + if allows_executor_auth { + assert!( + error.contains("127.0.0.1:1"), + "{case}: safe explicit credentials should reach the executor: {error}" + ); + } else { + assert_eq!( + error, + "executor-owned MCP server `fake-first-party` cannot use hosted ChatGPT authentication; configure executor-owned credentials instead", + "{case}: unsafe credentials must fail before contacting the executor" + ); + } + } + + Ok(()) +} + #[tokio::test] async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() { let codex_home = tempdir().expect("tempdir"); diff --git a/codex-rs/codex-mcp/src/mcp/auth.rs b/codex-rs/codex-mcp/src/mcp/auth.rs index 0cb1f867d4..091d91d2ea 100644 --- a/codex-rs/codex-mcp/src/mcp/auth.rs +++ b/codex-rs/codex-mcp/src/mcp/auth.rs @@ -20,6 +20,7 @@ use tracing::warn; use crate::runtime::McpRuntimeContext; use crate::server::EffectiveMcpServer; +use crate::server::has_explicit_http_authorization; #[derive(Debug, Clone)] pub struct McpOAuthLoginConfig { @@ -216,6 +217,14 @@ async fn compute_auth_status( return Ok(McpAuthState::Unsupported); } + if matches!(config.auth, McpServerAuth::ChatGpt) && !config.is_local_environment() { + return Ok(if has_explicit_http_authorization(config) { + McpAuthState::BearerToken + } else { + McpAuthState::Unsupported + }); + } + if has_runtime_auth { return Ok(McpAuthState::BearerToken); } diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index 5a2a8b134e..74519672f9 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -30,6 +30,7 @@ use crate::pagination::collect_paginated; use crate::runtime::McpRuntimeContext; use crate::runtime::emit_duration; use crate::server::EffectiveMcpServer; +use crate::server::has_explicit_http_authorization; use crate::tool_catalog_cache::McpToolCatalogCacheContext; use crate::tool_catalog_cache::McpToolCatalogFetchTicket; use crate::tools::ToolInfo; @@ -39,6 +40,7 @@ use async_channel::Sender; use codex_api::SharedAuthProvider; use codex_async_utils::CancelErr; use codex_async_utils::OrCancelExt; +use codex_config::McpServerAuth; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; use codex_config::types::AuthKeyringBackendKind; @@ -995,6 +997,14 @@ async fn make_rmcp_client( protocol_mode: McpProtocolMode, ) -> Result { let config = server.config().clone(); + if matches!(config.auth, McpServerAuth::ChatGpt) + && !config.is_local_environment() + && !has_explicit_http_authorization(&config) + { + return Err(StartupOutcomeError::from(anyhow!( + "executor-owned MCP server `{server_name}` cannot use hosted ChatGPT authentication; configure executor-owned credentials instead" + ))); + } let resolved_environment = resolved_environment.map_err(|err| StartupOutcomeError::from(anyhow!(err)))?; let is_local_environment = config.is_local_environment(); diff --git a/codex-rs/codex-mcp/src/server.rs b/codex-rs/codex-mcp/src/server.rs index 0811d80435..f4e05166f6 100644 --- a/codex-rs/codex-mcp/src/server.rs +++ b/codex-rs/codex-mcp/src/server.rs @@ -6,6 +6,7 @@ use std::sync::Arc; use crate::runtime::McpRuntimeContext; use codex_api::SharedAuthProvider; use codex_config::AppToolApproval; +use codex_config::McpServerAuth; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; use codex_config::types::AuthKeyringBackendKind; @@ -42,6 +43,36 @@ impl EffectiveMcpServer { } } +pub(crate) fn has_explicit_http_authorization(config: &McpServerConfig) -> bool { + let McpServerTransportConfig::StreamableHttp { + bearer_token_env_var, + http_headers, + env_http_headers, + .. + } = &config.transport + else { + return false; + }; + + if bearer_token_env_var.is_some() + || env_http_headers + .as_ref() + .is_some_and(|headers| !headers.is_empty()) + { + return false; + } + + http_headers.as_ref().is_some_and(|headers| { + headers.iter().any(|(name, value)| { + name.eq_ignore_ascii_case("authorization") + && !value.trim().is_empty() + && value + .bytes() + .all(|byte| byte == b'\t' || (byte >= b' ' && byte != 0x7f)) + }) + }) +} + /// Inputs that determine the identity of a live MCP connection. /// /// Tool policy and presentation metadata intentionally do not appear here: @@ -78,7 +109,9 @@ impl McpServerConnectionIdentity { supports_openai_form_elicitation: bool, ) -> Self { let config = server.config(); - let stored_oauth_url = if runtime_auth_provider.is_none() { + let stored_oauth_url = if runtime_auth_provider.is_none() + && (!matches!(config.auth, McpServerAuth::ChatGpt) || config.is_local_environment()) + { match &config.transport { McpServerTransportConfig::StreamableHttp { url,