mirror of
https://github.com/openai/codex.git
synced 2026-10-21 07:42:01 +00:00
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
This commit is contained in:
@@ -23,7 +23,9 @@ pub(super) fn chatgpt_auth_provider_for_server(
|
||||
server: &EffectiveMcpServer,
|
||||
chatgpt_auth_provider: Option<SharedAuthProvider>,
|
||||
) -> Option<SharedAuthProvider> {
|
||||
if !matches!(&server.config().auth, McpServerAuth::ChatGpt) {
|
||||
if !matches!(&server.config().auth, McpServerAuth::ChatGpt)
|
||||
|| !server.config().is_local_environment()
|
||||
{
|
||||
return None;
|
||||
}
|
||||
chatgpt_auth_provider
|
||||
|
||||
@@ -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::<McpServerConfig>(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");
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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<RmcpClient, StartupOutcomeError> {
|
||||
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();
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user