From 40b7560169c7274147a47f9b0c75db89fe016d34 Mon Sep 17 00:00:00 2001 From: jif Date: Mon, 24 Aug 2026 08:37:23 +0000 Subject: [PATCH] Forward local env vars to Agent Plugin MCP servers (#40363) ## What changed - Apply local `env_vars` declarations from `.codex-plugin/plugin.json` to matching stdio servers loaded from an Agent Plugin manifest. - Replace matching `${NAME}` entries in the portable server environment with host environment forwarding while preserving the portable command, arguments, and unrelated servers. - Ignore remote-sourced environment variables and support both inline and file-based Codex MCP declarations. ## Testing - Cover matching-server filtering, local and remote sources, inline declarations, and an end-to-end `PATH` forwarding round trip. GitOrigin-RevId: 843e1232a084fa5f571fe56e07266995b87fd53d --- .../src/agent_plugin_mcp_overlay.rs | 99 +++++++++++++++ codex-rs/core-plugins/src/loader.rs | 7 ++ codex-rs/core-plugins/src/loader_tests.rs | 117 ++++++++++++++++++ codex-rs/core/tests/suite/plugins.rs | 50 +++++++- 4 files changed, 270 insertions(+), 3 deletions(-) create mode 100644 codex-rs/core-plugins/src/agent_plugin_mcp_overlay.rs diff --git a/codex-rs/core-plugins/src/agent_plugin_mcp_overlay.rs b/codex-rs/core-plugins/src/agent_plugin_mcp_overlay.rs new file mode 100644 index 0000000000..d6ddbf97af --- /dev/null +++ b/codex-rs/core-plugins/src/agent_plugin_mcp_overlay.rs @@ -0,0 +1,99 @@ +use super::load_mcp_servers_from_file; +use super::load_mcp_servers_from_manifest_object; +use super::plugin_mcp_config_paths; +use crate::manifest::PluginManifestFormat; +use crate::manifest::PluginManifestMcpServers; +use crate::manifest::parse_plugin_manifest; +use codex_config::types::McpServerConfig; +use codex_config::types::McpServerTransportConfig; +use std::collections::HashMap; +use std::path::Path; +use tracing::warn; + +pub(super) async fn apply_codex_env_overlay( + plugin_root: &Path, + agent_servers: &mut HashMap, +) { + if agent_servers.is_empty() { + return; + } + + let overlay_path = plugin_root.join(".codex-plugin/plugin.json"); + let Ok(contents) = tokio::fs::read_to_string(&overlay_path).await else { + return; + }; + let overlay_manifest = match parse_plugin_manifest(plugin_root, &overlay_path, &contents) { + Ok(manifest) => manifest, + Err(err) => { + warn!( + path = %overlay_path.display(), + "failed to parse Codex Agent Plugin MCP overlay: {err}" + ); + return; + } + }; + + let overlay_servers = match &overlay_manifest.paths.mcp_servers { + Some(PluginManifestMcpServers::Object(servers)) => { + load_mcp_servers_from_manifest_object(plugin_root, servers).mcp_servers + } + Some(PluginManifestMcpServers::Path(_)) | None => { + let mut servers = HashMap::new(); + for path in plugin_mcp_config_paths(plugin_root, &overlay_manifest.paths) { + servers.extend( + load_mcp_servers_from_file( + plugin_root, + /*plugin_data_root*/ None, + PluginManifestFormat::Legacy, + &path, + ) + .await + .mcp_servers, + ); + } + servers + } + }; + + for (name, overlay_server) in overlay_servers { + let Some(agent_server) = agent_servers.get_mut(&name) else { + continue; + }; + let ( + McpServerTransportConfig::Stdio { env, env_vars, .. }, + McpServerTransportConfig::Stdio { + env_vars: mut overlay_env_vars, + .. + }, + ) = (&mut agent_server.transport, overlay_server.transport) + else { + continue; + }; + + overlay_env_vars.retain(|variable| !variable.is_remote_source()); + + if let Some(env) = env { + env.retain(|name, value| { + let Some(reference) = value + .strip_prefix("${") + .and_then(|reference| reference.strip_suffix('}')) + else { + return true; + }; + !overlay_env_vars.iter().any(|variable| { + environment_names_match(name, variable.name()) + && environment_names_match(reference, variable.name()) + }) + }); + } + env_vars.extend(overlay_env_vars); + } +} + +fn environment_names_match(left: &str, right: &str) -> bool { + if cfg!(windows) { + left.eq_ignore_ascii_case(right) + } else { + left == right + } +} diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 175564f539..dce8f82ef6 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -61,6 +61,9 @@ use tempfile::TempDir; use tracing::instrument; use tracing::warn; +#[path = "agent_plugin_mcp_overlay.rs"] +mod agent_plugin_mcp_overlay; + const DEFAULT_SKILLS_DIR_NAME: &str = "skills"; const DEFAULT_HOOKS_CONFIG_FILE: &str = "hooks/hooks.json"; const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json"; @@ -1560,6 +1563,10 @@ pub(crate) async fn load_plugin_mcp_servers_from_manifest_with_format( } } + if manifest_format == PluginManifestFormat::AgentPlugin { + agent_plugin_mcp_overlay::apply_codex_env_overlay(plugin_root, &mut mcp_servers).await; + } + mcp_servers } diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index efc2f27ae0..897c840432 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -47,6 +47,123 @@ async fn agent_plugin_overlay_apps_are_not_runtime_active() { assert!(load_plugin_apps(&plugin_root).await.is_empty()); } +#[tokio::test] +async fn agent_plugin_codex_mcp_overlay_only_forwards_matching_stdio_server_env_vars() { + let temp_dir = TempDir::new().expect("tempdir"); + let plugin_root = temp_dir.path().join("plugin"); + write_file( + &plugin_root.join("plugin.json"), + &format!( + r#"{{"$schema":"{AGENT_PLUGIN_SCHEMA_URI}","name":"plugin","extensions":{{"com.openai":{{"interface":{{"displayName":"Portable"}}}}}}}}"# + ), + ); + write_file( + &plugin_root.join("mcp.json"), + r#"{ + "$schema": "https://agent-plugins.org/schemas/1.0.0/mcp.schema.json", + "mcpServers": { + "shared": { + "type": "stdio", + "command": "portable-server", + "args": ["portable"], + "env": { + "DB_PASSWORD": "${DB_PASSWORD}", + "API_TOKEN": "portable-token", + "REMOTE_ONLY": "${REMOTE_ONLY}", + "UNLISTED": "${UNLISTED}" + } + }, + "portable-only": {"type": "stdio", "command": "portable-only"} + } +}"#, + ); + + let mut expected = load_plugin_mcp_servers(&plugin_root, /*auth_mode*/ None).await; + let Some(McpServerConfig { + transport: McpServerTransportConfig::Stdio { env, env_vars, .. }, + .. + }) = expected.get_mut("shared") + else { + panic!("expected portable stdio server"); + }; + env.as_mut() + .expect("portable environment") + .remove("DB_PASSWORD"); + env_vars.extend(["DB_PASSWORD".into(), "API_TOKEN".into()]); + + write_file( + &plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"legacy-plugin"}"#, + ); + write_file( + &plugin_root.join(".mcp.json"), + r#"{ + "mcpServers": { + "shared": { + "command": "legacy-server", + "args": ["legacy"], + "env_vars": ["DB_PASSWORD", "API_TOKEN", {"name": "REMOTE_ONLY", "source": "remote"}] + }, + "legacy-only": {"command": "legacy-only", "env_vars": ["UNLISTED"]} + } +}"#, + ); + + assert_eq!( + load_plugin_mcp_servers(&plugin_root, /*auth_mode*/ None).await, + expected + ); +} + +#[tokio::test] +async fn agent_plugin_codex_mcp_overlay_supports_inline_legacy_servers() { + let temp_dir = TempDir::new().expect("tempdir"); + let plugin_root = temp_dir.path().join("plugin"); + write_file( + &plugin_root.join("plugin.json"), + &format!(r#"{{"$schema":"{AGENT_PLUGIN_SCHEMA_URI}","name":"plugin"}}"#), + ); + write_file( + &plugin_root.join("mcp.json"), + r#"{ + "$schema": "https://agent-plugins.org/schemas/1.0.0/mcp.schema.json", + "mcpServers": { + "shared": { + "type": "stdio", + "command": "portable-server", + "env": {"TOKEN": "${TOKEN}"} + } + } +}"#, + ); + + let mut expected = load_plugin_mcp_servers(&plugin_root, /*auth_mode*/ None).await; + let Some(McpServerConfig { + transport: McpServerTransportConfig::Stdio { env, env_vars, .. }, + .. + }) = expected.get_mut("shared") + else { + panic!("expected portable stdio server"); + }; + env.as_mut().expect("portable environment").remove("TOKEN"); + env_vars.push("TOKEN".into()); + + write_file( + &plugin_root.join(".codex-plugin/plugin.json"), + r#"{ + "name": "legacy-plugin", + "mcpServers": { + "shared": {"command": "legacy-server", "env_vars": ["TOKEN"]} + } +}"#, + ); + + assert_eq!( + load_plugin_mcp_servers(&plugin_root, /*auth_mode*/ None).await, + expected + ); +} + #[cfg(unix)] #[tokio::test] async fn agent_plugin_mcp_rejects_config_symlink_outside_plugin_root() { diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs index e41326239f..04b502888b 100644 --- a/codex-rs/core/tests/suite/plugins.rs +++ b/codex-rs/core/tests/suite/plugins.rs @@ -685,11 +685,13 @@ async fn legacy_plugin_skill_prompt_remains_complete() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn agent_plugin_root_mcp_stdio_tool_round_trip_expands_reserved_paths() -> Result<()> { +async fn agent_plugin_root_mcp_stdio_tool_round_trip_expands_reserved_paths_and_codex_env_overlay() +-> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; let search_call_id = "search-agent-echo"; let tool_call_id = "call-agent-echo"; + let overlay_call_id = "call-agent-overlay-env"; let mock = mount_sse_sequence( &server, vec![ @@ -710,9 +712,19 @@ async fn agent_plugin_root_mcp_stdio_tool_round_trip_expands_reserved_paths() -> ]), sse(vec![ ev_response_created("resp-3"), - ev_assistant_message("msg-1", "done"), + ev_function_call_with_namespace( + overlay_call_id, + "mcp__agent", + "echo", + r#"{"message":"ping","env_var":"PATH"}"#, + ), ev_completed("resp-3"), ]), + sse(vec![ + ev_response_created("resp-4"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-4"), + ]), ], ) .await; @@ -736,7 +748,10 @@ async fn agent_plugin_root_mcp_stdio_tool_round_trip_expands_reserved_paths() -> "agent": { "type": "stdio", "command": format!("./{stdio_server_name}"), - "env": {"MCP_TEST_VALUE": "${PLUGIN_ROOT}|${PLUGIN_DATA}"} + "env": { + "MCP_TEST_VALUE": "${PLUGIN_ROOT}|${PLUGIN_DATA}", + "PATH": "${PATH}" + } } } }); @@ -744,6 +759,11 @@ async fn agent_plugin_root_mcp_stdio_tool_round_trip_expands_reserved_paths() -> plugin_root.join("mcp.json"), serde_json::to_vec_pretty(&mcp_config)?, )?; + std::fs::create_dir_all(plugin_root.join(".codex-plugin"))?; + std::fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"acme.tools","mcpServers":{"agent":{"command":"ignored","env_vars":["PATH"]}}}"#, + )?; let mut builder = test_codex().with_home(Arc::clone(&codex_home)); let test_codex = builder.build_with_remote_and_local_env(&server).await?; wait_for_mcp_server(&test_codex.codex, "agent").await?; @@ -770,6 +790,10 @@ async fn agent_plugin_root_mcp_stdio_tool_round_trip_expands_reserved_paths() -> matches!(event, EventMsg::McpToolCallEnd(_)) }) .await; + let overlay_end = wait_for_event(&test_codex.codex, |event| { + matches!(event, EventMsg::McpToolCallEnd(_)) + }) + .await; wait_for_event(&test_codex.codex, |event| { matches!(event, EventMsg::TurnComplete(_)) }) @@ -787,10 +811,30 @@ async fn agent_plugin_root_mcp_stdio_tool_round_trip_expands_reserved_paths() -> .and_then(serde_json::Value::as_str), Some(expected_env.as_str()) ); + let EventMsg::McpToolCallEnd(overlay_end) = overlay_end else { + unreachable!("wait_for_event matched an MCP tool end") + }; + let overlay_result = overlay_end + .result + .as_ref() + .expect("Agent Plugin overlay MCP tool result"); + assert_eq!( + overlay_result + .structured_content + .as_ref() + .and_then(|content| content.get("env")) + .and_then(serde_json::Value::as_str), + Some(std::env::var("PATH")?.as_str()) + ); let requests = mock.requests(); let search_output = requests[1].tool_search_output(search_call_id); assert!(namespace_child_tool(&search_output, "mcp__agent", "echo").is_some()); assert!(requests[2].function_call_output(tool_call_id).is_object()); + assert!( + requests[3] + .function_call_output(overlay_call_id) + .is_object() + ); Ok(()) }