mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Apply user MCP policy to selected executor plugins (#39079)
## What changed - Resolve MCP server policy directly from the effective user configuration for selected executor-plugin roots. - Apply server enablement, tool allow/deny lists, and default and per-tool approval modes while preserving stricter plugin-declared restrictions. - Cover policy merging and opaque selected-root IDs in the executor-plugin MCP integration tests. GitOrigin-RevId: dce3021969ba71e642de52312449cab6277515c3
This commit is contained in:
1 parent
911335eed3
commit
8ef139667f
3 files changed
+182
-1
No files matched your search
@@ -1397,6 +1397,73 @@ pub async fn load_configured_plugin_mcp_servers(
|
|||||||
load_plugin_mcp_servers_with_policy(plugin_root, auth_mode, plugin_policy).await
|
load_plugin_mcp_servers_with_policy(plugin_root, auth_mode, plugin_policy).await
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Resolves effective per-plugin MCP policies without validating opaque selected-root IDs.
|
||||||
|
pub fn configured_plugin_mcp_server_policies(
|
||||||
|
config_layer_stack: &ConfigLayerStack,
|
||||||
|
) -> HashMap<String, HashMap<String, PluginMcpServerConfig>> {
|
||||||
|
config_layer_stack
|
||||||
|
.effective_user_config()
|
||||||
|
.map(|config| configured_plugins_from_user_config_value(&config))
|
||||||
|
.unwrap_or_default()
|
||||||
|
.into_iter()
|
||||||
|
.map(|(plugin_id, plugin)| (plugin_id, plugin.mcp_servers))
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Applies user policy without widening the selected plugin's declared restrictions.
|
||||||
|
pub fn apply_configured_plugin_mcp_server_policies(
|
||||||
|
policies: &HashMap<String, PluginMcpServerConfig>,
|
||||||
|
servers: &mut HashMap<String, McpServerConfig>,
|
||||||
|
) {
|
||||||
|
for (name, server) in servers {
|
||||||
|
if let Some(policy) = policies.get(name) {
|
||||||
|
let declared_approval_mode = server.default_tools_approval_mode.unwrap_or_default();
|
||||||
|
server.enabled &= policy.enabled;
|
||||||
|
|
||||||
|
if let Some(approval_mode) = policy.default_tools_approval_mode {
|
||||||
|
server.default_tools_approval_mode =
|
||||||
|
Some(declared_approval_mode.restrict_to(approval_mode));
|
||||||
|
}
|
||||||
|
if let Some(enabled_tools) = &policy.enabled_tools {
|
||||||
|
match &mut server.enabled_tools {
|
||||||
|
Some(declared_tools) => {
|
||||||
|
declared_tools.retain(|tool| enabled_tools.contains(tool));
|
||||||
|
}
|
||||||
|
None => server.enabled_tools = Some(enabled_tools.clone()),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if let Some(disabled_tools) = &policy.disabled_tools {
|
||||||
|
let declared_tools = server.disabled_tools.get_or_insert_default();
|
||||||
|
for tool in disabled_tools {
|
||||||
|
if !declared_tools.contains(tool) {
|
||||||
|
declared_tools.push(tool.clone());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for (tool_name, tool_policy) in &policy.tools {
|
||||||
|
if tool_policy.approval_mode.is_some() {
|
||||||
|
server.tools.entry(tool_name.clone()).or_default();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for (tool_name, tool_config) in &mut server.tools {
|
||||||
|
if let Some(approval_mode) = policy
|
||||||
|
.tools
|
||||||
|
.get(tool_name)
|
||||||
|
.and_then(|tool_policy| tool_policy.approval_mode)
|
||||||
|
.or(policy.default_tools_approval_mode)
|
||||||
|
{
|
||||||
|
tool_config.approval_mode = Some(
|
||||||
|
tool_config
|
||||||
|
.approval_mode
|
||||||
|
.unwrap_or(declared_approval_mode)
|
||||||
|
.restrict_to(approval_mode),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async fn load_plugin_mcp_servers_with_policy(
|
async fn load_plugin_mcp_servers_with_policy(
|
||||||
plugin_root: &Path,
|
plugin_root: &Path,
|
||||||
auth_mode: Option<AuthMode>,
|
auth_mode: Option<AuthMode>,
|
||||||
|
|||||||
@@ -1,6 +1,9 @@
|
|||||||
|
use codex_config::types::PluginMcpServerConfig;
|
||||||
use codex_connectors_extension::ExecutorPluginConnectorProvider;
|
use codex_connectors_extension::ExecutorPluginConnectorProvider;
|
||||||
use codex_core::config::Config;
|
use codex_core::config::Config;
|
||||||
use codex_core_plugins::ExecutorPluginProvider;
|
use codex_core_plugins::ExecutorPluginProvider;
|
||||||
|
use codex_core_plugins::loader::apply_configured_plugin_mcp_server_policies;
|
||||||
|
use codex_core_plugins::loader::configured_plugin_mcp_server_policies;
|
||||||
use codex_exec_server::EnvironmentManager;
|
use codex_exec_server::EnvironmentManager;
|
||||||
use codex_extension_api::ExtensionFuture;
|
use codex_extension_api::ExtensionFuture;
|
||||||
use codex_extension_api::McpServerContribution;
|
use codex_extension_api::McpServerContribution;
|
||||||
@@ -154,6 +157,8 @@ impl McpServerContributor<Config> for SelectedExecutorPluginMcpContributor {
|
|||||||
let Some(selected_roots) = context.ready_selected_capability_roots() else {
|
let Some(selected_roots) = context.ready_selected_capability_roots() else {
|
||||||
return Vec::new();
|
return Vec::new();
|
||||||
};
|
};
|
||||||
|
let plugin_policies =
|
||||||
|
configured_plugin_mcp_server_policies(&context.config().config_layer_stack);
|
||||||
let mut contributions = Vec::new();
|
let mut contributions = Vec::new();
|
||||||
|
|
||||||
if let Some(snapshot) = context.executor_capability_discovery() {
|
if let Some(snapshot) = context.executor_capability_discovery() {
|
||||||
@@ -176,6 +181,7 @@ impl McpServerContributor<Config> for SelectedExecutorPluginMcpContributor {
|
|||||||
};
|
};
|
||||||
contributions.extend(project_metadata(
|
contributions.extend(project_metadata(
|
||||||
context.config(),
|
context.config(),
|
||||||
|
plugin_policies.get(&plugin.plugin_id),
|
||||||
selection_order,
|
selection_order,
|
||||||
plugin,
|
plugin,
|
||||||
));
|
));
|
||||||
@@ -188,6 +194,7 @@ impl McpServerContributor<Config> for SelectedExecutorPluginMcpContributor {
|
|||||||
};
|
};
|
||||||
contributions.extend(project_metadata(
|
contributions.extend(project_metadata(
|
||||||
context.config(),
|
context.config(),
|
||||||
|
plugin_policies.get(&plugin.plugin_id),
|
||||||
selection_order,
|
selection_order,
|
||||||
plugin,
|
plugin,
|
||||||
));
|
));
|
||||||
@@ -201,10 +208,14 @@ impl McpServerContributor<Config> for SelectedExecutorPluginMcpContributor {
|
|||||||
|
|
||||||
fn project_metadata(
|
fn project_metadata(
|
||||||
config: &Config,
|
config: &Config,
|
||||||
|
plugin_policy: Option<&HashMap<String, PluginMcpServerConfig>>,
|
||||||
selection_order: usize,
|
selection_order: usize,
|
||||||
plugin: SelectedPluginMetadata,
|
plugin: SelectedPluginMetadata,
|
||||||
) -> Vec<McpServerContribution> {
|
) -> Vec<McpServerContribution> {
|
||||||
let mut servers = plugin.servers.iter().cloned().collect::<HashMap<_, _>>();
|
let mut servers = plugin.servers.iter().cloned().collect::<HashMap<_, _>>();
|
||||||
|
if let Some(plugin_policy) = plugin_policy {
|
||||||
|
apply_configured_plugin_mcp_server_policies(plugin_policy, &mut servers);
|
||||||
|
}
|
||||||
config.apply_plugin_mcp_server_requirements(&plugin.plugin_id, &mut servers);
|
config.apply_plugin_mcp_server_requirements(&plugin.plugin_id, &mut servers);
|
||||||
let mut servers = servers.into_iter().collect::<Vec<_>>();
|
let mut servers = servers.into_iter().collect::<Vec<_>>();
|
||||||
servers.sort_unstable_by(|left, right| left.0.cmp(&right.0));
|
servers.sort_unstable_by(|left, right| left.0.cmp(&right.0));
|
||||||
|
|||||||
@@ -1,3 +1,5 @@
|
|||||||
|
use codex_config::AppToolApproval;
|
||||||
|
use codex_config::McpServerToolConfig;
|
||||||
use codex_config::test_support::CloudConfigBundleFixture;
|
use codex_config::test_support::CloudConfigBundleFixture;
|
||||||
use codex_core::config::Config;
|
use codex_core::config::Config;
|
||||||
use codex_core::config::ConfigBuilder;
|
use codex_core::config::ConfigBuilder;
|
||||||
@@ -14,6 +16,7 @@ use codex_protocol::capabilities::CapabilityRootLocation;
|
|||||||
use codex_protocol::capabilities::SelectedCapabilityRoot;
|
use codex_protocol::capabilities::SelectedCapabilityRoot;
|
||||||
use codex_utils_path_uri::PathUri;
|
use codex_utils_path_uri::PathUri;
|
||||||
use pretty_assertions::assert_eq;
|
use pretty_assertions::assert_eq;
|
||||||
|
use std::collections::HashMap;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
|
|
||||||
type TestResult = Result<(), Box<dyn std::error::Error>>;
|
type TestResult = Result<(), Box<dyn std::error::Error>>;
|
||||||
@@ -53,6 +56,10 @@ async fn selected_plugin_servers_use_managed_requirements_for_the_selected_root_
|
|||||||
}
|
}
|
||||||
}"#,
|
}"#,
|
||||||
)?;
|
)?;
|
||||||
|
std::fs::write(
|
||||||
|
codex_home.path().join("config.toml"),
|
||||||
|
"[plugins.\"selected-root\".mcp_servers.mismatched]\nenabled = true\n[plugins.\"selected-root\".mcp_servers.unlisted]\nenabled = true",
|
||||||
|
)?;
|
||||||
let config = ConfigBuilder::default()
|
let config = ConfigBuilder::default()
|
||||||
.codex_home(codex_home.path().to_path_buf())
|
.codex_home(codex_home.path().to_path_buf())
|
||||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||||
@@ -160,7 +167,46 @@ async fn high_level_discovery_matches_the_existing_plugin_provider() -> TestResu
|
|||||||
)?;
|
)?;
|
||||||
std::fs::write(
|
std::fs::write(
|
||||||
plugin_root.path().join("servers.json"),
|
plugin_root.path().join("servers.json"),
|
||||||
r#"{"mcpServers":{"first":{"command":"first"},"second":{"command":"second"}}}"#,
|
r#"{
|
||||||
|
"mcpServers": {
|
||||||
|
"first": {
|
||||||
|
"command": "first",
|
||||||
|
"default_tools_approval_mode": "writes",
|
||||||
|
"enabled_tools": ["read", "deploy", "trusted", "package-only"],
|
||||||
|
"disabled_tools": ["package-denied"],
|
||||||
|
"tools": {
|
||||||
|
"read": {"approval_mode": "prompt"},
|
||||||
|
"deploy": {"approval_mode": "approve"},
|
||||||
|
"trusted": {"approval_mode": "approve"}
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"second": {
|
||||||
|
"command": "second",
|
||||||
|
"enabled": false,
|
||||||
|
"default_tools_approval_mode": "prompt"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}"#,
|
||||||
|
)?;
|
||||||
|
std::fs::write(
|
||||||
|
codex_home.path().join("config.toml"),
|
||||||
|
r#"
|
||||||
|
[plugins."selected-root".mcp_servers.first]
|
||||||
|
enabled = false
|
||||||
|
default_tools_approval_mode = "prompt"
|
||||||
|
enabled_tools = ["read", "deploy", "trusted", "host-only"]
|
||||||
|
disabled_tools = ["write"]
|
||||||
|
|
||||||
|
[plugins."selected-root".mcp_servers.first.tools.read]
|
||||||
|
approval_mode = "approve"
|
||||||
|
|
||||||
|
[plugins."selected-root".mcp_servers.first.tools.trusted]
|
||||||
|
approval_mode = "approve"
|
||||||
|
|
||||||
|
[plugins."selected-root".mcp_servers.second]
|
||||||
|
enabled = true
|
||||||
|
default_tools_approval_mode = "auto"
|
||||||
|
"#,
|
||||||
)?;
|
)?;
|
||||||
let mut config = ConfigBuilder::default()
|
let mut config = ConfigBuilder::default()
|
||||||
.codex_home(codex_home.path().to_path_buf())
|
.codex_home(codex_home.path().to_path_buf())
|
||||||
@@ -168,6 +214,63 @@ async fn high_level_discovery_matches_the_existing_plugin_provider() -> TestResu
|
|||||||
.build()
|
.build()
|
||||||
.await?;
|
.await?;
|
||||||
let existing = selected_plugin_contributions(&config, plugin_root.path()).await?;
|
let existing = selected_plugin_contributions(&config, plugin_root.path()).await?;
|
||||||
|
let mut servers = raw_selected_plugin_contributions(&config, plugin_root.path())
|
||||||
|
.await?
|
||||||
|
.into_iter()
|
||||||
|
.filter_map(|contribution| match contribution {
|
||||||
|
McpServerContribution::SelectedPlugin { name, config, .. } => Some((name, config)),
|
||||||
|
_ => None,
|
||||||
|
})
|
||||||
|
.collect::<HashMap<_, _>>();
|
||||||
|
let server = servers
|
||||||
|
.remove("first")
|
||||||
|
.expect("disabled selected server remains registered");
|
||||||
|
let declared_disabled_server = servers
|
||||||
|
.remove("second")
|
||||||
|
.expect("package-disabled server remains registered");
|
||||||
|
assert_eq!(
|
||||||
|
(
|
||||||
|
server.enabled,
|
||||||
|
server.default_tools_approval_mode,
|
||||||
|
server.enabled_tools,
|
||||||
|
server.disabled_tools,
|
||||||
|
server.tools,
|
||||||
|
declared_disabled_server.enabled,
|
||||||
|
declared_disabled_server.default_tools_approval_mode,
|
||||||
|
),
|
||||||
|
(
|
||||||
|
false,
|
||||||
|
Some(AppToolApproval::Prompt),
|
||||||
|
Some(vec![
|
||||||
|
"read".to_string(),
|
||||||
|
"deploy".to_string(),
|
||||||
|
"trusted".to_string(),
|
||||||
|
]),
|
||||||
|
Some(vec!["package-denied".to_string(), "write".to_string()]),
|
||||||
|
HashMap::from([
|
||||||
|
(
|
||||||
|
"read".to_string(),
|
||||||
|
McpServerToolConfig {
|
||||||
|
approval_mode: Some(AppToolApproval::Prompt),
|
||||||
|
},
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"deploy".to_string(),
|
||||||
|
McpServerToolConfig {
|
||||||
|
approval_mode: Some(AppToolApproval::Prompt),
|
||||||
|
},
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"trusted".to_string(),
|
||||||
|
McpServerToolConfig {
|
||||||
|
approval_mode: Some(AppToolApproval::Approve),
|
||||||
|
},
|
||||||
|
),
|
||||||
|
]),
|
||||||
|
false,
|
||||||
|
Some(AppToolApproval::Prompt),
|
||||||
|
)
|
||||||
|
);
|
||||||
config
|
config
|
||||||
.features
|
.features
|
||||||
.enable(Feature::ExecutorCapabilityDiscovery)
|
.enable(Feature::ExecutorCapabilityDiscovery)
|
||||||
|
|||||||
Reference in new issue
Block a user