mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
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
This commit is contained in:
Generated
+1
@@ -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",
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -44,7 +44,11 @@ pub fn resolve(
|
||||
cwd: &Path,
|
||||
) -> std::io::Result<OsString> {
|
||||
// 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
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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<()> {
|
||||
|
||||
Reference in New Issue
Block a user