From b2543af02b2db9214ada5741d3d673315b89a5d0 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Tue, 11 Aug 2026 17:15:05 +0000 Subject: [PATCH] Propagate custom CA settings to local MCP servers (#38040) ## Why Local stdio MCP servers may start in a different working directory from Codex, so inherited relative CA bundle paths can otherwise resolve to the wrong file. ## What changed - Forward the supported custom CA environment variables to local stdio MCP servers, convert inherited non-empty paths to absolute paths, and let explicit MCP environment overrides take precedence. - Match CA variable names and `PATH` case-insensitively where required, avoiding duplicate environment entries and preserving executable discovery on Windows. ## Testing - Add an end-to-end stdio MCP test covering relative inherited CA paths, empty values, explicit overrides, alternate key casing, and a different server working directory. - Exercise executable resolution with Windows-style `Path` casing. GitOrigin-RevId: ea1bc0ae63bc0dc780acb8363b2ed102120f282e --- codex-rs/Cargo.lock | 1 + codex-rs/rmcp-client/Cargo.toml | 1 + codex-rs/rmcp-client/src/program_resolver.rs | 16 ++- codex-rs/rmcp-client/src/utils.rs | 29 +++- .../tests/mcp_2026_stdio_discovery.rs | 126 ++++++++++++++++++ 5 files changed, 171 insertions(+), 2 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 33ef6e2272..2e04b753e5 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3972,6 +3972,7 @@ dependencies = [ "codex-exec-server", "codex-http-client", "codex-keyring-store", + "codex-network-proxy", "codex-protocol", "codex-secrets", "codex-utils-cargo-bin", diff --git a/codex-rs/rmcp-client/Cargo.toml b/codex-rs/rmcp-client/Cargo.toml index 43d735ef1b..8b734f4b53 100644 --- a/codex-rs/rmcp-client/Cargo.toml +++ b/codex-rs/rmcp-client/Cargo.toml @@ -20,6 +20,7 @@ codex-config = { workspace = true } codex-exec-server = { workspace = true } codex-http-client = { workspace = true } codex-keyring-store = { workspace = true } +codex-network-proxy = { workspace = true } codex-protocol = { workspace = true } codex-secrets = { workspace = true } codex-utils-path-uri = { workspace = true } diff --git a/codex-rs/rmcp-client/src/program_resolver.rs b/codex-rs/rmcp-client/src/program_resolver.rs index 395b330d7d..53db522166 100644 --- a/codex-rs/rmcp-client/src/program_resolver.rs +++ b/codex-rs/rmcp-client/src/program_resolver.rs @@ -44,7 +44,11 @@ pub fn resolve( cwd: &Path, ) -> std::io::Result { // Extract PATH from environment for search locations - let search_path = env.get(std::ffi::OsStr::new("PATH")); + let search_path = env.iter().find_map(|(name, value)| { + name.to_string_lossy() + .eq_ignore_ascii_case("PATH") + .then_some(value) + }); // Attempt resolution via which crate match which::which_in(&program, search_path, cwd) { @@ -139,6 +143,16 @@ mod tests { #[tokio::test] async fn test_resolved_program_executes_successfully() -> Result<()> { let env = TestExecutableEnv::new()?; + #[cfg(windows)] + let env = { + let mut env = env; + let path = env + .mcp_env + .remove(std::ffi::OsStr::new("PATH")) + .expect("test environment should include PATH"); + env.mcp_env.insert(OsString::from("Path"), path); + env + }; let program = OsString::from(&env.program_name); // Apply platform-specific resolution diff --git a/codex-rs/rmcp-client/src/utils.rs b/codex-rs/rmcp-client/src/utils.rs index f6267d38d9..8f1adc93c5 100644 --- a/codex-rs/rmcp-client/src/utils.rs +++ b/codex-rs/rmcp-client/src/utils.rs @@ -1,6 +1,7 @@ use anyhow::Result; use anyhow::anyhow; use codex_config::types::McpServerEnvVar; +use codex_network_proxy::CUSTOM_CA_ENV_KEYS; use codex_protocol::shell_environment::is_non_inheritable_env_var; use http::HeaderMap; use http::HeaderName; @@ -22,8 +23,34 @@ pub(crate) fn create_env_for_mcp_server( .copied() .chain(additional_env_vars) .filter_map(|var| env::var_os(var).map(|value| (OsString::from(var), value))) - .chain(extra_env.unwrap_or_default()) .collect(); + for name in CUSTOM_CA_ENV_KEYS { + let Some(value) = env::var_os(name) else { + continue; + }; + if value.is_empty() { + continue; + } + let value = std::path::absolute(value)?.into_os_string(); + #[cfg(windows)] + env.retain(|key, _| !key.to_string_lossy().eq_ignore_ascii_case(name)); + env.insert(OsString::from(name), value); + } + for (name, value) in extra_env.unwrap_or_default() { + if cfg!(windows) + || name.to_str().is_some_and(|name| { + CUSTOM_CA_ENV_KEYS + .iter() + .any(|ca_name| ca_name.eq_ignore_ascii_case(name)) + }) + { + env.retain(|key, _| { + !key.to_string_lossy() + .eq_ignore_ascii_case(&name.to_string_lossy()) + }); + } + env.insert(name, value); + } env.retain(|name, _| { name.to_str() .is_none_or(|name| !is_non_inheritable_env_var(name)) diff --git a/codex-rs/rmcp-client/tests/mcp_2026_stdio_discovery.rs b/codex-rs/rmcp-client/tests/mcp_2026_stdio_discovery.rs index b2ff92ef6a..8cf62a2697 100644 --- a/codex-rs/rmcp-client/tests/mcp_2026_stdio_discovery.rs +++ b/codex-rs/rmcp-client/tests/mcp_2026_stdio_discovery.rs @@ -1,9 +1,11 @@ use std::collections::HashMap; use std::ffi::OsString; +use std::process::Command; use std::sync::Arc; use std::time::Duration; use codex_exec_server::Environment; +use codex_network_proxy::CUSTOM_CA_ENV_KEYS; use codex_rmcp_client::ElicitationAction; use codex_rmcp_client::ElicitationResponse; use codex_rmcp_client::ExecutorStdioServerLauncher; @@ -12,12 +14,136 @@ use codex_rmcp_client::McpProtocolMode; use codex_rmcp_client::RmcpClient; use codex_rmcp_client::StdioServerLauncher; use futures::FutureExt; +use pretty_assertions::assert_eq; use rmcp::model::ClientCapabilities; use rmcp::model::Implementation; use rmcp::model::InitializeRequestParams; use rmcp::model::ProtocolVersion; use serde_json::json; +#[test] +fn local_stdio_inherits_ca_certificate_variables() -> anyhow::Result<()> { + let directories = tempfile::tempdir()?; + let source_dir = directories.path().join("source"); + let server_dir = directories.path().join("server"); + std::fs::create_dir_all(source_dir.join("certs"))?; + std::fs::create_dir_all(&server_dir)?; + + #[cfg(windows)] + let requests_ca_bundle = match source_dir.components().next() { + Some(std::path::Component::Prefix(prefix)) => match prefix.kind() { + std::path::Prefix::Disk(drive) | std::path::Prefix::VerbatimDisk(drive) => { + format!("{}:certs\\custom-ca.pem", char::from(drive)) + } + _ => "certs/custom-ca.pem".to_string(), + }, + _ => "certs/custom-ca.pem".to_string(), + }; + #[cfg(not(windows))] + let requests_ca_bundle = "certs/custom-ca.pem"; + + let output = Command::new(std::env::current_exe()?) + .arg("--exact") + .arg("local_stdio_inherits_ca_certificate_variables_child") + .arg("--ignored") + .current_dir(&source_dir) + .envs( + CUSTOM_CA_ENV_KEYS + .into_iter() + .map(|name| (name, "certs/custom-ca.pem")), + ) + .env("CODEX_CA_CERTIFICATE", "") + .env("REQUESTS_CA_BUNDLE", requests_ca_bundle) + .env("CODEX_MCP_TEST_SERVER_CWD", &server_dir) + .output()?; + + assert!( + output.status.success(), + "MCP subprocess inheritance test failed:\n{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "child process for local_stdio_inherits_ca_certificate_variables"] +async fn local_stdio_inherits_ca_certificate_variables_child() -> anyhow::Result<()> { + let server = codex_utils_cargo_bin::cargo_bin("test_stdio_server")?; + let source_dir = std::env::current_dir()?; + let server_dir = std::env::var("CODEX_MCP_TEST_SERVER_CWD")?; + let expected = source_dir.join("certs").join("custom-ca.pem"); + let npm_override = HashMap::from([( + OsString::from("NPM_CONFIG_CAFILE"), + OsString::from("explicit/custom-ca.pem"), + )]); + + for (overrides, env_name, expected) in [ + ( + None, + "SSL_CERT_FILE", + Some(expected.to_string_lossy().into_owned()), + ), + ( + None, + "REQUESTS_CA_BUNDLE", + Some(expected.to_string_lossy().into_owned()), + ), + (None, "CODEX_CA_CERTIFICATE", None), + ( + Some(npm_override.clone()), + "NPM_CONFIG_CAFILE", + Some("explicit/custom-ca.pem".to_string()), + ), + (Some(npm_override), "npm_config_cafile", None), + ] { + let client = RmcpClient::new_stdio_client( + server.clone().into(), + Vec::new(), + overrides, + &[], + Some(server_dir.clone()), + Arc::new(LocalStdioServerLauncher::new(source_dir.clone())), + ) + .await?; + client + .initialize( + InitializeRequestParams::new( + ClientCapabilities::default(), + Implementation::new("stdio-ca-inheritance-test", "1.0.0"), + ) + .with_protocol_version(ProtocolVersion::V_2025_06_18), + Some(Duration::from_secs(10)), + Box::new(|_, _| { + async { + Ok(ElicitationResponse { + action: ElicitationAction::Decline, + content: None, + meta: None, + }) + } + .boxed() + }), + ) + .await?; + let result = client + .call_tool( + "echo".to_string(), + Some(json!({ "message": "ca inheritance", "env_var": env_name })), + /*meta*/ None, + Some(Duration::from_secs(10)), + ) + .await?; + assert_eq!( + result.structured_content, + Some(json!({ "echo": "ECHOING: ca inheritance", "env": expected })) + ); + client.shutdown().await; + } + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn modern_local_and_executor_stdio_discover_metadata_identity_and_catalogs() -> anyhow::Result<()> {