mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
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
This commit is contained in:
1 parent
2df6705423
commit
40b7560169
4 files changed
+270
-3
No files matched your search
@@ -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<String, McpServerConfig>,
|
||||||
|
) {
|
||||||
|
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
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -61,6 +61,9 @@ use tempfile::TempDir;
|
|||||||
use tracing::instrument;
|
use tracing::instrument;
|
||||||
use tracing::warn;
|
use tracing::warn;
|
||||||
|
|
||||||
|
#[path = "agent_plugin_mcp_overlay.rs"]
|
||||||
|
mod agent_plugin_mcp_overlay;
|
||||||
|
|
||||||
const DEFAULT_SKILLS_DIR_NAME: &str = "skills";
|
const DEFAULT_SKILLS_DIR_NAME: &str = "skills";
|
||||||
const DEFAULT_HOOKS_CONFIG_FILE: &str = "hooks/hooks.json";
|
const DEFAULT_HOOKS_CONFIG_FILE: &str = "hooks/hooks.json";
|
||||||
const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.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
|
mcp_servers
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -47,6 +47,123 @@ async fn agent_plugin_overlay_apps_are_not_runtime_active() {
|
|||||||
assert!(load_plugin_apps(&plugin_root).await.is_empty());
|
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)]
|
#[cfg(unix)]
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn agent_plugin_mcp_rejects_config_symlink_outside_plugin_root() {
|
async fn agent_plugin_mcp_rejects_config_symlink_outside_plugin_root() {
|
||||||
|
|||||||
@@ -685,11 +685,13 @@ async fn legacy_plugin_skill_prompt_remains_complete() -> Result<()> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[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(()));
|
skip_if_no_network!(Ok(()));
|
||||||
let server = start_mock_server().await;
|
let server = start_mock_server().await;
|
||||||
let search_call_id = "search-agent-echo";
|
let search_call_id = "search-agent-echo";
|
||||||
let tool_call_id = "call-agent-echo";
|
let tool_call_id = "call-agent-echo";
|
||||||
|
let overlay_call_id = "call-agent-overlay-env";
|
||||||
let mock = mount_sse_sequence(
|
let mock = mount_sse_sequence(
|
||||||
&server,
|
&server,
|
||||||
vec![
|
vec![
|
||||||
@@ -710,9 +712,19 @@ async fn agent_plugin_root_mcp_stdio_tool_round_trip_expands_reserved_paths() ->
|
|||||||
]),
|
]),
|
||||||
sse(vec![
|
sse(vec![
|
||||||
ev_response_created("resp-3"),
|
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"),
|
ev_completed("resp-3"),
|
||||||
]),
|
]),
|
||||||
|
sse(vec![
|
||||||
|
ev_response_created("resp-4"),
|
||||||
|
ev_assistant_message("msg-1", "done"),
|
||||||
|
ev_completed("resp-4"),
|
||||||
|
]),
|
||||||
],
|
],
|
||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
@@ -736,7 +748,10 @@ async fn agent_plugin_root_mcp_stdio_tool_round_trip_expands_reserved_paths() ->
|
|||||||
"agent": {
|
"agent": {
|
||||||
"type": "stdio",
|
"type": "stdio",
|
||||||
"command": format!("./{stdio_server_name}"),
|
"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"),
|
plugin_root.join("mcp.json"),
|
||||||
serde_json::to_vec_pretty(&mcp_config)?,
|
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 mut builder = test_codex().with_home(Arc::clone(&codex_home));
|
||||||
let test_codex = builder.build_with_remote_and_local_env(&server).await?;
|
let test_codex = builder.build_with_remote_and_local_env(&server).await?;
|
||||||
wait_for_mcp_server(&test_codex.codex, "agent").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(_))
|
matches!(event, EventMsg::McpToolCallEnd(_))
|
||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
|
let overlay_end = wait_for_event(&test_codex.codex, |event| {
|
||||||
|
matches!(event, EventMsg::McpToolCallEnd(_))
|
||||||
|
})
|
||||||
|
.await;
|
||||||
wait_for_event(&test_codex.codex, |event| {
|
wait_for_event(&test_codex.codex, |event| {
|
||||||
matches!(event, EventMsg::TurnComplete(_))
|
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),
|
.and_then(serde_json::Value::as_str),
|
||||||
Some(expected_env.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 requests = mock.requests();
|
||||||
let search_output = requests[1].tool_search_output(search_call_id);
|
let search_output = requests[1].tool_search_output(search_call_id);
|
||||||
assert!(namespace_child_tool(&search_output, "mcp__agent", "echo").is_some());
|
assert!(namespace_child_tool(&search_output, "mcp__agent", "echo").is_some());
|
||||||
assert!(requests[2].function_call_output(tool_call_id).is_object());
|
assert!(requests[2].function_call_output(tool_call_id).is_object());
|
||||||
|
assert!(
|
||||||
|
requests[3]
|
||||||
|
.function_call_output(overlay_call_id)
|
||||||
|
.is_object()
|
||||||
|
);
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user