mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Revert "Move MCP requirement policy out of core"
This reverts commit 2bae9d57c6.
This commit is contained in:
@@ -111,7 +111,6 @@ pub use mcp_edit::ConfigEditsBuilder;
|
||||
pub use mcp_edit::load_global_mcp_servers;
|
||||
pub use mcp_requirements::McpServerCommandMatcher;
|
||||
pub use mcp_requirements::McpServerMatcher;
|
||||
pub use mcp_requirements::McpServerPolicy;
|
||||
pub use mcp_requirements::McpServerUrlMatcher;
|
||||
pub use mcp_requirements::McpServerValueMatcher;
|
||||
pub use mcp_types::AppToolApproval;
|
||||
|
||||
@@ -1,18 +1,7 @@
|
||||
use crate::ConfigRequirements;
|
||||
use crate::Constrained;
|
||||
use crate::ConstraintResult;
|
||||
use crate::McpServerIdentity;
|
||||
use crate::McpServerRequirement;
|
||||
use crate::PluginRequirementsToml;
|
||||
use crate::RequirementSource;
|
||||
use crate::Sourced;
|
||||
use crate::mcp_types::McpServerConfig;
|
||||
use crate::mcp_types::McpServerDisabledReason;
|
||||
use crate::mcp_types::McpServerTransportConfig;
|
||||
use regex_lite::Regex;
|
||||
use serde::Deserialize;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
|
||||
/// String matching operations available to managed MCP server matchers.
|
||||
#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
|
||||
@@ -103,177 +92,6 @@ impl McpServerMatcher {
|
||||
}
|
||||
}
|
||||
|
||||
/// Owned policy for applying exact and matcher requirements to configured MCP servers.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct McpServerPolicy {
|
||||
exact_requirements: Option<Sourced<BTreeMap<String, McpServerRequirement>>>,
|
||||
matchers: Option<Sourced<BTreeMap<String, McpServerMatcher>>>,
|
||||
}
|
||||
|
||||
impl McpServerPolicy {
|
||||
fn new(
|
||||
exact_requirements: Option<Sourced<BTreeMap<String, McpServerRequirement>>>,
|
||||
matchers: Option<Sourced<BTreeMap<String, McpServerMatcher>>>,
|
||||
) -> Self {
|
||||
Self {
|
||||
exact_requirements,
|
||||
matchers,
|
||||
}
|
||||
}
|
||||
|
||||
/// Constrains configured MCP servers using the exact and matcher policies.
|
||||
pub fn constrain(
|
||||
self,
|
||||
mcp_servers: HashMap<String, McpServerConfig>,
|
||||
) -> ConstraintResult<Constrained<HashMap<String, McpServerConfig>>> {
|
||||
if self.exact_requirements.is_none() && self.matchers.is_none() {
|
||||
return Ok(Constrained::allow_any(mcp_servers));
|
||||
}
|
||||
|
||||
Constrained::normalized(mcp_servers, move |mut servers| {
|
||||
self.apply_to_configured_servers(&mut servers);
|
||||
servers
|
||||
})
|
||||
}
|
||||
|
||||
fn apply_to_configured_servers(&self, mcp_servers: &mut HashMap<String, McpServerConfig>) {
|
||||
let source = RequirementSource::composite(
|
||||
self.exact_requirements
|
||||
.iter()
|
||||
.map(|requirements| requirements.source.clone())
|
||||
.chain(
|
||||
self.matchers
|
||||
.iter()
|
||||
.map(|requirements| requirements.source.clone()),
|
||||
),
|
||||
);
|
||||
for (name, server) in mcp_servers {
|
||||
let exact_requirement = self
|
||||
.exact_requirements
|
||||
.as_ref()
|
||||
.and_then(|requirements| requirements.value.get(name));
|
||||
let matcher = self
|
||||
.matchers
|
||||
.as_ref()
|
||||
.and_then(|matchers| matchers.value.get(name));
|
||||
let allowed = (exact_requirement.is_some() || matcher.is_some())
|
||||
&& exact_requirement
|
||||
.is_none_or(|requirement| mcp_server_matches_requirement(requirement, server))
|
||||
&& matcher.is_none_or(|matcher| matcher.matches(server));
|
||||
apply_requirement_result(server, allowed, &source);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ConfigRequirements {
|
||||
/// Returns the owned policy used to constrain ordinary configured MCP servers.
|
||||
pub fn mcp_server_policy(&self) -> McpServerPolicy {
|
||||
McpServerPolicy::new(self.mcp_servers.clone(), self.mcp_server_matchers.clone())
|
||||
}
|
||||
|
||||
/// Applies managed MCP requirements to servers supplied by one plugin.
|
||||
pub fn apply_to_plugin_mcp_servers(
|
||||
&self,
|
||||
plugin_config_name: &str,
|
||||
mcp_servers: &mut HashMap<String, McpServerConfig>,
|
||||
) {
|
||||
apply_plugin_requirements(
|
||||
plugin_config_name,
|
||||
mcp_servers,
|
||||
self.plugins.as_ref(),
|
||||
self.mcp_server_matchers.as_ref(),
|
||||
);
|
||||
|
||||
if let Some(empty_allowlist) = self
|
||||
.mcp_servers
|
||||
.as_ref()
|
||||
.filter(|requirements| requirements.value.is_empty())
|
||||
{
|
||||
for server in mcp_servers.values_mut() {
|
||||
apply_requirement_result(server, /*allowed*/ false, &empty_allowlist.source);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn apply_plugin_requirements(
|
||||
plugin_config_name: &str,
|
||||
mcp_servers: &mut HashMap<String, McpServerConfig>,
|
||||
plugin_requirements: Option<&Sourced<BTreeMap<String, PluginRequirementsToml>>>,
|
||||
matchers: Option<&Sourced<BTreeMap<String, McpServerMatcher>>>,
|
||||
) {
|
||||
if plugin_requirements.is_none() && matchers.is_none() {
|
||||
return;
|
||||
}
|
||||
|
||||
let source = RequirementSource::composite(
|
||||
plugin_requirements
|
||||
.iter()
|
||||
.map(|requirements| requirements.source.clone())
|
||||
.chain(
|
||||
matchers
|
||||
.iter()
|
||||
.map(|requirements| requirements.source.clone()),
|
||||
),
|
||||
);
|
||||
let plugin_mcp_requirements = plugin_requirements
|
||||
.and_then(|requirements| requirements.value.get(plugin_config_name))
|
||||
.and_then(|plugin| plugin.mcp_servers.as_ref());
|
||||
|
||||
for (name, server) in mcp_servers {
|
||||
let allowed_by_plugin_requirement = plugin_requirements.is_none()
|
||||
|| plugin_mcp_requirements
|
||||
.and_then(|requirements| requirements.get(name))
|
||||
.is_some_and(|requirement| mcp_server_matches_requirement(requirement, server));
|
||||
let allowed_by_matcher = matchers.is_none_or(|matchers| {
|
||||
matchers
|
||||
.value
|
||||
.get(name)
|
||||
.is_some_and(|matcher| matcher.matches(server))
|
||||
});
|
||||
apply_requirement_result(
|
||||
server,
|
||||
allowed_by_plugin_requirement && allowed_by_matcher,
|
||||
&source,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn apply_requirement_result(
|
||||
server: &mut McpServerConfig,
|
||||
allowed: bool,
|
||||
source: &RequirementSource,
|
||||
) {
|
||||
if allowed {
|
||||
server.disabled_reason = None;
|
||||
} else {
|
||||
server.enabled = false;
|
||||
server.disabled_reason = Some(McpServerDisabledReason::Requirements {
|
||||
source: source.clone(),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
fn mcp_server_matches_requirement(
|
||||
requirement: &McpServerRequirement,
|
||||
server: &McpServerConfig,
|
||||
) -> bool {
|
||||
match &requirement.identity {
|
||||
McpServerIdentity::Command {
|
||||
command: want_command,
|
||||
} => matches!(
|
||||
&server.transport,
|
||||
McpServerTransportConfig::Stdio { command: got_command, .. }
|
||||
if got_command == want_command
|
||||
),
|
||||
McpServerIdentity::Url { url: want_url } => matches!(
|
||||
&server.transport,
|
||||
McpServerTransportConfig::StreamableHttp { url: got_url, .. }
|
||||
if got_url == want_url
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "mcp_requirements_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
use super::*;
|
||||
use crate::mcp_types::McpServerConfig;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
|
||||
fn stdio_server(command: &str, args: &[&str]) -> McpServerConfig {
|
||||
@@ -30,625 +29,6 @@ fn stdio_server(command: &str, args: &[&str]) -> McpServerConfig {
|
||||
}
|
||||
}
|
||||
|
||||
fn stdio_mcp(command: &str) -> McpServerConfig {
|
||||
stdio_server(command, &[])
|
||||
}
|
||||
|
||||
fn stdio_mcp_with_args(command: &str, args: &[&str]) -> McpServerConfig {
|
||||
stdio_server(command, args)
|
||||
}
|
||||
|
||||
fn http_mcp(url: &str) -> McpServerConfig {
|
||||
McpServerConfig {
|
||||
transport: McpServerTransportConfig::StreamableHttp {
|
||||
url: url.to_string(),
|
||||
bearer_token_env_var: None,
|
||||
http_headers: None,
|
||||
env_http_headers: None,
|
||||
},
|
||||
environment_id: crate::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(),
|
||||
enabled: true,
|
||||
required: false,
|
||||
supports_parallel_tool_calls: false,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
oauth: None,
|
||||
oauth_resource: None,
|
||||
tools: HashMap::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn filter_mcp_servers_by_requirements(
|
||||
mcp_servers: &mut HashMap<String, McpServerConfig>,
|
||||
exact_requirements: Option<&Sourced<BTreeMap<String, McpServerRequirement>>>,
|
||||
matchers: Option<&Sourced<BTreeMap<String, McpServerMatcher>>>,
|
||||
) {
|
||||
let policy = McpServerPolicy::new(exact_requirements.cloned(), matchers.cloned());
|
||||
let constrained = policy
|
||||
.constrain(std::mem::take(mcp_servers))
|
||||
.expect("MCP policy should produce a valid constraint");
|
||||
*mcp_servers = constrained.get().clone();
|
||||
}
|
||||
|
||||
fn filter_plugin_mcp_servers_by_requirements(
|
||||
plugin_config_name: &str,
|
||||
mcp_servers: &mut HashMap<String, McpServerConfig>,
|
||||
plugin_requirements: Option<&Sourced<BTreeMap<String, PluginRequirementsToml>>>,
|
||||
matchers: Option<&Sourced<BTreeMap<String, McpServerMatcher>>>,
|
||||
) {
|
||||
ConfigRequirements {
|
||||
plugins: plugin_requirements.cloned(),
|
||||
mcp_server_matchers: matchers.cloned(),
|
||||
..Default::default()
|
||||
}
|
||||
.apply_to_plugin_mcp_servers(plugin_config_name, mcp_servers);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_mcp_servers_by_allowlist_enforces_identity_rules() {
|
||||
const MISMATCHED_COMMAND_SERVER: &str = "mismatched-command-should-disable";
|
||||
const MISMATCHED_URL_SERVER: &str = "mismatched-url-should-disable";
|
||||
const MATCHED_COMMAND_SERVER: &str = "matched-command-should-allow";
|
||||
const MATCHED_URL_SERVER: &str = "matched-url-should-allow";
|
||||
const DIFFERENT_NAME_SERVER: &str = "different-name-should-disable";
|
||||
|
||||
const GOOD_CMD: &str = "good-cmd";
|
||||
const GOOD_URL: &str = "https://example.com/good";
|
||||
|
||||
let mut servers = HashMap::from([
|
||||
(MISMATCHED_COMMAND_SERVER.to_string(), stdio_mcp("docs-cmd")),
|
||||
(
|
||||
MISMATCHED_URL_SERVER.to_string(),
|
||||
http_mcp("https://example.com/mcp"),
|
||||
),
|
||||
(MATCHED_COMMAND_SERVER.to_string(), stdio_mcp(GOOD_CMD)),
|
||||
(MATCHED_URL_SERVER.to_string(), http_mcp(GOOD_URL)),
|
||||
(DIFFERENT_NAME_SERVER.to_string(), stdio_mcp("same-cmd")),
|
||||
]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let requirements = Sourced::new(
|
||||
BTreeMap::from([
|
||||
(
|
||||
MISMATCHED_URL_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Url {
|
||||
url: "https://example.com/other".to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
MISMATCHED_COMMAND_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "other-cmd".to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
MATCHED_URL_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Url {
|
||||
url: GOOD_URL.to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
MATCHED_COMMAND_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: GOOD_CMD.to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
filter_mcp_servers_by_requirements(
|
||||
&mut servers,
|
||||
Some(&requirements),
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
(MISMATCHED_URL_SERVER.to_string(), (false, reason.clone())),
|
||||
(
|
||||
MISMATCHED_COMMAND_SERVER.to_string(),
|
||||
(false, reason.clone()),
|
||||
),
|
||||
(MATCHED_URL_SERVER.to_string(), (true, None)),
|
||||
(MATCHED_COMMAND_SERVER.to_string(), (true, None)),
|
||||
(DIFFERENT_NAME_SERVER.to_string(), (false, reason)),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_mcp_servers_by_allowlist_allows_all_when_unset() {
|
||||
let mut servers = HashMap::from([
|
||||
("server-a".to_string(), stdio_mcp("cmd-a")),
|
||||
("server-b".to_string(), http_mcp("https://example.com/b")),
|
||||
]);
|
||||
|
||||
filter_mcp_servers_by_requirements(
|
||||
&mut servers,
|
||||
/*mcp_requirements*/ None,
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
("server-a".to_string(), (true, None)),
|
||||
("server-b".to_string(), (true, None)),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_mcp_servers_by_matchers_enforces_command_and_positional_args() {
|
||||
let mut servers = HashMap::from([
|
||||
(
|
||||
"internal_mcp_proxy".to_string(),
|
||||
stdio_mcp_with_args(
|
||||
"company-cli",
|
||||
&[
|
||||
"mcp",
|
||||
"proxy",
|
||||
"--server",
|
||||
"https://pricing.mcp.internal.example.com",
|
||||
],
|
||||
),
|
||||
),
|
||||
(
|
||||
"unlisted".to_string(),
|
||||
stdio_mcp_with_args(
|
||||
"company-cli",
|
||||
&[
|
||||
"mcp",
|
||||
"proxy",
|
||||
"--server",
|
||||
"https://pricing.mcp.internal.example.com",
|
||||
],
|
||||
),
|
||||
),
|
||||
(
|
||||
"wrong-order".to_string(),
|
||||
stdio_mcp_with_args(
|
||||
"company-cli",
|
||||
&[
|
||||
"proxy",
|
||||
"mcp",
|
||||
"--server",
|
||||
"https://pricing.mcp.internal.example.com",
|
||||
],
|
||||
),
|
||||
),
|
||||
(
|
||||
"trailing-arg".to_string(),
|
||||
stdio_mcp_with_args(
|
||||
"company-cli",
|
||||
&[
|
||||
"mcp",
|
||||
"proxy",
|
||||
"--server",
|
||||
"https://pricing.mcp.internal.example.com",
|
||||
"--verbose",
|
||||
],
|
||||
),
|
||||
),
|
||||
(
|
||||
"wrong-host".to_string(),
|
||||
stdio_mcp_with_args(
|
||||
"company-cli",
|
||||
&["mcp", "proxy", "--server", "https://mcp.example.com"],
|
||||
),
|
||||
),
|
||||
]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let matcher = McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
command: "company-cli".to_string(),
|
||||
args: vec![
|
||||
McpServerValueMatcher::Exact {
|
||||
value: "mcp".to_string(),
|
||||
},
|
||||
McpServerValueMatcher::Exact {
|
||||
value: "proxy".to_string(),
|
||||
},
|
||||
McpServerValueMatcher::Exact {
|
||||
value: "--server".to_string(),
|
||||
},
|
||||
McpServerValueMatcher::Regex {
|
||||
expression:
|
||||
r"^https://[A-Za-z0-9-]+\.mcp\.internal\.example\.com(?::443)?(?:/.*)?$"
|
||||
.to_string(),
|
||||
},
|
||||
],
|
||||
});
|
||||
let matchers = Sourced::new(
|
||||
BTreeMap::from([
|
||||
("internal_mcp_proxy".to_string(), matcher.clone()),
|
||||
("wrong-order".to_string(), matcher.clone()),
|
||||
("trailing-arg".to_string(), matcher.clone()),
|
||||
("wrong-host".to_string(), matcher),
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_mcp_servers_by_requirements(
|
||||
&mut servers,
|
||||
/*mcp_requirements*/ None,
|
||||
Some(&matchers),
|
||||
);
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
("internal_mcp_proxy".to_string(), (true, None)),
|
||||
("unlisted".to_string(), (false, reason.clone())),
|
||||
("wrong-order".to_string(), (false, reason.clone())),
|
||||
("trailing-arg".to_string(), (false, reason.clone())),
|
||||
("wrong-host".to_string(), (false, reason)),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_mcp_servers_by_requirements_enforces_all_same_name_rules() {
|
||||
const BOTH_MATCH: &str = "both-match";
|
||||
const EXACT_ONLY_MATCHES: &str = "exact-only-matches";
|
||||
const MATCHER_ONLY_MATCHES: &str = "matcher-only-matches";
|
||||
|
||||
let mut servers = HashMap::from([
|
||||
(
|
||||
BOTH_MATCH.to_string(),
|
||||
stdio_mcp_with_args("company-cli", &["approved"]),
|
||||
),
|
||||
(
|
||||
EXACT_ONLY_MATCHES.to_string(),
|
||||
stdio_mcp_with_args("company-cli", &["rejected"]),
|
||||
),
|
||||
(
|
||||
MATCHER_ONLY_MATCHES.to_string(),
|
||||
stdio_mcp_with_args("company-cli", &["approved"]),
|
||||
),
|
||||
]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let requirements = Sourced::new(
|
||||
BTreeMap::from([
|
||||
(
|
||||
BOTH_MATCH.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "company-cli".to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
EXACT_ONLY_MATCHES.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "company-cli".to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
MATCHER_ONLY_MATCHES.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "different-command".to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
let matcher = McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
command: "company-cli".to_string(),
|
||||
args: vec![McpServerValueMatcher::Exact {
|
||||
value: "approved".to_string(),
|
||||
}],
|
||||
});
|
||||
let matchers = Sourced::new(
|
||||
BTreeMap::from([
|
||||
(BOTH_MATCH.to_string(), matcher.clone()),
|
||||
(EXACT_ONLY_MATCHES.to_string(), matcher.clone()),
|
||||
(MATCHER_ONLY_MATCHES.to_string(), matcher),
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_mcp_servers_by_requirements(&mut servers, Some(&requirements), Some(&matchers));
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
(BOTH_MATCH.to_string(), (true, None)),
|
||||
(EXACT_ONLY_MATCHES.to_string(), (false, reason.clone())),
|
||||
(MATCHER_ONLY_MATCHES.to_string(), (false, reason)),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_mcp_servers_by_allowlist_blocks_all_when_empty() {
|
||||
let mut servers = HashMap::from([
|
||||
("server-a".to_string(), stdio_mcp("cmd-a")),
|
||||
("server-b".to_string(), http_mcp("https://example.com/b")),
|
||||
]);
|
||||
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let requirements = Sourced::new(BTreeMap::new(), source.clone());
|
||||
filter_mcp_servers_by_requirements(
|
||||
&mut servers,
|
||||
Some(&requirements),
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
("server-a".to_string(), (false, reason.clone())),
|
||||
("server-b".to_string(), (false, reason)),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_plugin_mcp_servers_by_allowlist_enforces_plugin_and_matcher_rules() {
|
||||
const MATCHED_SERVER: &str = "matched-should-allow";
|
||||
const MISMATCHED_SERVER: &str = "mismatched-should-disable";
|
||||
const MATCHER_MISMATCHED_SERVER: &str = "matcher-mismatched-should-disable";
|
||||
const UNLISTED_SERVER: &str = "unlisted-should-disable";
|
||||
const GOOD_CMD: &str = "good-cmd";
|
||||
|
||||
let mut servers = HashMap::from([
|
||||
(MATCHED_SERVER.to_string(), stdio_mcp(GOOD_CMD)),
|
||||
(MISMATCHED_SERVER.to_string(), stdio_mcp("bad-cmd")),
|
||||
(
|
||||
MATCHER_MISMATCHED_SERVER.to_string(),
|
||||
stdio_mcp_with_args(GOOD_CMD, &["unexpected"]),
|
||||
),
|
||||
(
|
||||
UNLISTED_SERVER.to_string(),
|
||||
http_mcp("https://example.com/mcp"),
|
||||
),
|
||||
]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let requirements = Sourced::new(
|
||||
BTreeMap::from([(
|
||||
"sample@test".to_string(),
|
||||
PluginRequirementsToml {
|
||||
mcp_servers: Some(BTreeMap::from([
|
||||
(
|
||||
MATCHED_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: GOOD_CMD.to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
MISMATCHED_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: GOOD_CMD.to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
MATCHER_MISMATCHED_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: GOOD_CMD.to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
])),
|
||||
},
|
||||
)]),
|
||||
source.clone(),
|
||||
);
|
||||
let matchers = Sourced::new(
|
||||
BTreeMap::from([
|
||||
(
|
||||
MATCHED_SERVER.to_string(),
|
||||
McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
command: GOOD_CMD.to_string(),
|
||||
args: Vec::new(),
|
||||
}),
|
||||
),
|
||||
(
|
||||
MISMATCHED_SERVER.to_string(),
|
||||
McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
command: "bad-cmd".to_string(),
|
||||
args: Vec::new(),
|
||||
}),
|
||||
),
|
||||
(
|
||||
MATCHER_MISMATCHED_SERVER.to_string(),
|
||||
McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
command: GOOD_CMD.to_string(),
|
||||
args: Vec::new(),
|
||||
}),
|
||||
),
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_plugin_mcp_servers_by_requirements(
|
||||
"sample@test",
|
||||
&mut servers,
|
||||
Some(&requirements),
|
||||
Some(&matchers),
|
||||
);
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
(MATCHED_SERVER.to_string(), (true, None)),
|
||||
(MISMATCHED_SERVER.to_string(), (false, reason.clone())),
|
||||
(
|
||||
MATCHER_MISMATCHED_SERVER.to_string(),
|
||||
(false, reason.clone()),
|
||||
),
|
||||
(UNLISTED_SERVER.to_string(), (false, reason)),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_plugin_mcp_servers_by_allowlist_blocks_unlisted_plugin() {
|
||||
let mut servers = HashMap::from([("server-a".to_string(), stdio_mcp("cmd-a"))]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let requirements = Sourced::new(
|
||||
BTreeMap::from([(
|
||||
"other@test".to_string(),
|
||||
PluginRequirementsToml {
|
||||
mcp_servers: Some(BTreeMap::from([(
|
||||
"server-a".to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "cmd-a".to_string(),
|
||||
},
|
||||
},
|
||||
)])),
|
||||
},
|
||||
)]),
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_plugin_mcp_servers_by_requirements(
|
||||
"sample@test",
|
||||
&mut servers,
|
||||
Some(&requirements),
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([(
|
||||
"server-a".to_string(),
|
||||
(
|
||||
false,
|
||||
Some(McpServerDisabledReason::Requirements { source })
|
||||
)
|
||||
)])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_plugin_mcp_servers_by_matchers_enforces_name_and_invocation() {
|
||||
const MATCHED_SERVER: &str = "matched";
|
||||
const MISMATCHED_SERVER: &str = "mismatched";
|
||||
const UNLISTED_SERVER: &str = "unlisted";
|
||||
|
||||
let mut servers = HashMap::from([
|
||||
(
|
||||
MATCHED_SERVER.to_string(),
|
||||
stdio_mcp_with_args("company-cli", &["approved"]),
|
||||
),
|
||||
(
|
||||
MISMATCHED_SERVER.to_string(),
|
||||
stdio_mcp_with_args("company-cli", &["rejected"]),
|
||||
),
|
||||
(
|
||||
UNLISTED_SERVER.to_string(),
|
||||
stdio_mcp_with_args("company-cli", &["approved"]),
|
||||
),
|
||||
]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let matcher = McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
command: "company-cli".to_string(),
|
||||
args: vec![McpServerValueMatcher::Exact {
|
||||
value: "approved".to_string(),
|
||||
}],
|
||||
});
|
||||
let matchers = Sourced::new(
|
||||
BTreeMap::from([
|
||||
(MATCHED_SERVER.to_string(), matcher.clone()),
|
||||
(MISMATCHED_SERVER.to_string(), matcher),
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_plugin_mcp_servers_by_requirements(
|
||||
"sample@test",
|
||||
&mut servers,
|
||||
/*plugin_requirements*/ None,
|
||||
Some(&matchers),
|
||||
);
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
(MATCHED_SERVER.to_string(), (true, None)),
|
||||
(MISMATCHED_SERVER.to_string(), (false, reason.clone())),
|
||||
(UNLISTED_SERVER.to_string(), (false, reason)),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn command_matcher_matches_exact_positional_arguments() {
|
||||
let matcher = McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
|
||||
@@ -5,8 +5,9 @@ use assert_matches::assert_matches;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::ConfigLayerEntry;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::McpServerIdentity;
|
||||
use codex_config::McpServerRequirement;
|
||||
use codex_config::McpServerCommandMatcher;
|
||||
use codex_config::McpServerMatcher;
|
||||
use codex_config::McpServerValueMatcher;
|
||||
use codex_config::ProfileV2Name;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_config::Sourced;
|
||||
@@ -39,7 +40,6 @@ use codex_config::types::ApprovalsReviewer;
|
||||
use codex_config::types::BundledSkillsConfig;
|
||||
use codex_config::types::FeedbackConfigToml;
|
||||
use codex_config::types::HistoryPersistence;
|
||||
use codex_config::types::McpServerDisabledReason;
|
||||
use codex_config::types::McpServerEnvVar;
|
||||
use codex_config::types::McpServerOAuthConfig;
|
||||
use codex_config::types::McpServerToolConfig;
|
||||
@@ -115,10 +115,14 @@ use std::time::Duration;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn stdio_mcp(command: &str) -> McpServerConfig {
|
||||
stdio_mcp_with_args(command, &[])
|
||||
}
|
||||
|
||||
fn stdio_mcp_with_args(command: &str, args: &[&str]) -> McpServerConfig {
|
||||
McpServerConfig {
|
||||
transport: McpServerTransportConfig::Stdio {
|
||||
command: command.to_string(),
|
||||
args: Vec::new(),
|
||||
args: args.iter().map(ToString::to_string).collect(),
|
||||
env: None,
|
||||
env_vars: Vec::new(),
|
||||
cwd: None,
|
||||
@@ -4116,6 +4120,566 @@ exclude_slash_tmp = true
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_mcp_servers_by_allowlist_enforces_identity_rules() {
|
||||
const MISMATCHED_COMMAND_SERVER: &str = "mismatched-command-should-disable";
|
||||
const MISMATCHED_URL_SERVER: &str = "mismatched-url-should-disable";
|
||||
const MATCHED_COMMAND_SERVER: &str = "matched-command-should-allow";
|
||||
const MATCHED_URL_SERVER: &str = "matched-url-should-allow";
|
||||
const DIFFERENT_NAME_SERVER: &str = "different-name-should-disable";
|
||||
|
||||
const GOOD_CMD: &str = "good-cmd";
|
||||
const GOOD_URL: &str = "https://example.com/good";
|
||||
|
||||
let mut servers = HashMap::from([
|
||||
(MISMATCHED_COMMAND_SERVER.to_string(), stdio_mcp("docs-cmd")),
|
||||
(
|
||||
MISMATCHED_URL_SERVER.to_string(),
|
||||
http_mcp("https://example.com/mcp"),
|
||||
),
|
||||
(MATCHED_COMMAND_SERVER.to_string(), stdio_mcp(GOOD_CMD)),
|
||||
(MATCHED_URL_SERVER.to_string(), http_mcp(GOOD_URL)),
|
||||
(DIFFERENT_NAME_SERVER.to_string(), stdio_mcp("same-cmd")),
|
||||
]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let requirements = Sourced::new(
|
||||
BTreeMap::from([
|
||||
(
|
||||
MISMATCHED_URL_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Url {
|
||||
url: "https://example.com/other".to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
MISMATCHED_COMMAND_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "other-cmd".to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
MATCHED_URL_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Url {
|
||||
url: GOOD_URL.to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
MATCHED_COMMAND_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: GOOD_CMD.to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
filter_mcp_servers_by_requirements(
|
||||
&mut servers,
|
||||
Some(&requirements),
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
(MISMATCHED_URL_SERVER.to_string(), (false, reason.clone())),
|
||||
(
|
||||
MISMATCHED_COMMAND_SERVER.to_string(),
|
||||
(false, reason.clone()),
|
||||
),
|
||||
(MATCHED_URL_SERVER.to_string(), (true, None)),
|
||||
(MATCHED_COMMAND_SERVER.to_string(), (true, None)),
|
||||
(DIFFERENT_NAME_SERVER.to_string(), (false, reason)),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_mcp_servers_by_allowlist_allows_all_when_unset() {
|
||||
let mut servers = HashMap::from([
|
||||
("server-a".to_string(), stdio_mcp("cmd-a")),
|
||||
("server-b".to_string(), http_mcp("https://example.com/b")),
|
||||
]);
|
||||
|
||||
filter_mcp_servers_by_requirements(
|
||||
&mut servers,
|
||||
/*mcp_requirements*/ None,
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
("server-a".to_string(), (true, None)),
|
||||
("server-b".to_string(), (true, None)),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_mcp_servers_by_matchers_enforces_command_and_positional_args() {
|
||||
let mut servers = HashMap::from([
|
||||
(
|
||||
"internal_mcp_proxy".to_string(),
|
||||
stdio_mcp_with_args(
|
||||
"company-cli",
|
||||
&[
|
||||
"mcp",
|
||||
"proxy",
|
||||
"--server",
|
||||
"https://pricing.mcp.internal.example.com",
|
||||
],
|
||||
),
|
||||
),
|
||||
(
|
||||
"unlisted".to_string(),
|
||||
stdio_mcp_with_args(
|
||||
"company-cli",
|
||||
&[
|
||||
"mcp",
|
||||
"proxy",
|
||||
"--server",
|
||||
"https://pricing.mcp.internal.example.com",
|
||||
],
|
||||
),
|
||||
),
|
||||
(
|
||||
"wrong-order".to_string(),
|
||||
stdio_mcp_with_args(
|
||||
"company-cli",
|
||||
&[
|
||||
"proxy",
|
||||
"mcp",
|
||||
"--server",
|
||||
"https://pricing.mcp.internal.example.com",
|
||||
],
|
||||
),
|
||||
),
|
||||
(
|
||||
"trailing-arg".to_string(),
|
||||
stdio_mcp_with_args(
|
||||
"company-cli",
|
||||
&[
|
||||
"mcp",
|
||||
"proxy",
|
||||
"--server",
|
||||
"https://pricing.mcp.internal.example.com",
|
||||
"--verbose",
|
||||
],
|
||||
),
|
||||
),
|
||||
(
|
||||
"wrong-host".to_string(),
|
||||
stdio_mcp_with_args(
|
||||
"company-cli",
|
||||
&["mcp", "proxy", "--server", "https://mcp.example.com"],
|
||||
),
|
||||
),
|
||||
]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let matcher = McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
command: "company-cli".to_string(),
|
||||
args: vec![
|
||||
McpServerValueMatcher::Exact {
|
||||
value: "mcp".to_string(),
|
||||
},
|
||||
McpServerValueMatcher::Exact {
|
||||
value: "proxy".to_string(),
|
||||
},
|
||||
McpServerValueMatcher::Exact {
|
||||
value: "--server".to_string(),
|
||||
},
|
||||
McpServerValueMatcher::Regex {
|
||||
expression:
|
||||
r"^https://[A-Za-z0-9-]+\.mcp\.internal\.example\.com(?::443)?(?:/.*)?$"
|
||||
.to_string(),
|
||||
},
|
||||
],
|
||||
});
|
||||
let matchers = Sourced::new(
|
||||
BTreeMap::from([
|
||||
("internal_mcp_proxy".to_string(), matcher.clone()),
|
||||
("wrong-order".to_string(), matcher.clone()),
|
||||
("trailing-arg".to_string(), matcher.clone()),
|
||||
("wrong-host".to_string(), matcher),
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_mcp_servers_by_requirements(
|
||||
&mut servers,
|
||||
/*mcp_requirements*/ None,
|
||||
Some(&matchers),
|
||||
);
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
("internal_mcp_proxy".to_string(), (true, None)),
|
||||
("unlisted".to_string(), (false, reason.clone())),
|
||||
("wrong-order".to_string(), (false, reason.clone())),
|
||||
("trailing-arg".to_string(), (false, reason.clone())),
|
||||
("wrong-host".to_string(), (false, reason)),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_mcp_servers_by_requirements_enforces_all_same_name_rules() {
|
||||
const BOTH_MATCH: &str = "both-match";
|
||||
const EXACT_ONLY_MATCHES: &str = "exact-only-matches";
|
||||
const MATCHER_ONLY_MATCHES: &str = "matcher-only-matches";
|
||||
|
||||
let mut servers = HashMap::from([
|
||||
(
|
||||
BOTH_MATCH.to_string(),
|
||||
stdio_mcp_with_args("company-cli", &["approved"]),
|
||||
),
|
||||
(
|
||||
EXACT_ONLY_MATCHES.to_string(),
|
||||
stdio_mcp_with_args("company-cli", &["rejected"]),
|
||||
),
|
||||
(
|
||||
MATCHER_ONLY_MATCHES.to_string(),
|
||||
stdio_mcp_with_args("company-cli", &["approved"]),
|
||||
),
|
||||
]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let requirements = Sourced::new(
|
||||
BTreeMap::from([
|
||||
(
|
||||
BOTH_MATCH.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "company-cli".to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
EXACT_ONLY_MATCHES.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "company-cli".to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
MATCHER_ONLY_MATCHES.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "different-command".to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
let matcher = McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
command: "company-cli".to_string(),
|
||||
args: vec![McpServerValueMatcher::Exact {
|
||||
value: "approved".to_string(),
|
||||
}],
|
||||
});
|
||||
let matchers = Sourced::new(
|
||||
BTreeMap::from([
|
||||
(BOTH_MATCH.to_string(), matcher.clone()),
|
||||
(EXACT_ONLY_MATCHES.to_string(), matcher.clone()),
|
||||
(MATCHER_ONLY_MATCHES.to_string(), matcher),
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_mcp_servers_by_requirements(&mut servers, Some(&requirements), Some(&matchers));
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
(BOTH_MATCH.to_string(), (true, None)),
|
||||
(EXACT_ONLY_MATCHES.to_string(), (false, reason.clone())),
|
||||
(MATCHER_ONLY_MATCHES.to_string(), (false, reason)),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_mcp_servers_by_allowlist_blocks_all_when_empty() {
|
||||
let mut servers = HashMap::from([
|
||||
("server-a".to_string(), stdio_mcp("cmd-a")),
|
||||
("server-b".to_string(), http_mcp("https://example.com/b")),
|
||||
]);
|
||||
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let requirements = Sourced::new(BTreeMap::new(), source.clone());
|
||||
filter_mcp_servers_by_requirements(
|
||||
&mut servers,
|
||||
Some(&requirements),
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
("server-a".to_string(), (false, reason.clone())),
|
||||
("server-b".to_string(), (false, reason)),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_plugin_mcp_servers_by_allowlist_enforces_plugin_and_matcher_rules() {
|
||||
const MATCHED_SERVER: &str = "matched-should-allow";
|
||||
const MISMATCHED_SERVER: &str = "mismatched-should-disable";
|
||||
const MATCHER_MISMATCHED_SERVER: &str = "matcher-mismatched-should-disable";
|
||||
const UNLISTED_SERVER: &str = "unlisted-should-disable";
|
||||
const GOOD_CMD: &str = "good-cmd";
|
||||
|
||||
let mut servers = HashMap::from([
|
||||
(MATCHED_SERVER.to_string(), stdio_mcp(GOOD_CMD)),
|
||||
(MISMATCHED_SERVER.to_string(), stdio_mcp("bad-cmd")),
|
||||
(
|
||||
MATCHER_MISMATCHED_SERVER.to_string(),
|
||||
stdio_mcp_with_args(GOOD_CMD, &["unexpected"]),
|
||||
),
|
||||
(
|
||||
UNLISTED_SERVER.to_string(),
|
||||
http_mcp("https://example.com/mcp"),
|
||||
),
|
||||
]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let requirements = Sourced::new(
|
||||
BTreeMap::from([(
|
||||
"sample@test".to_string(),
|
||||
codex_config::PluginRequirementsToml {
|
||||
mcp_servers: Some(BTreeMap::from([
|
||||
(
|
||||
MATCHED_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: GOOD_CMD.to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
MISMATCHED_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: GOOD_CMD.to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
MATCHER_MISMATCHED_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: GOOD_CMD.to_string(),
|
||||
},
|
||||
},
|
||||
),
|
||||
])),
|
||||
},
|
||||
)]),
|
||||
source.clone(),
|
||||
);
|
||||
let matchers = Sourced::new(
|
||||
BTreeMap::from([
|
||||
(
|
||||
MATCHED_SERVER.to_string(),
|
||||
McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
command: GOOD_CMD.to_string(),
|
||||
args: Vec::new(),
|
||||
}),
|
||||
),
|
||||
(
|
||||
MISMATCHED_SERVER.to_string(),
|
||||
McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
command: "bad-cmd".to_string(),
|
||||
args: Vec::new(),
|
||||
}),
|
||||
),
|
||||
(
|
||||
MATCHER_MISMATCHED_SERVER.to_string(),
|
||||
McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
command: GOOD_CMD.to_string(),
|
||||
args: Vec::new(),
|
||||
}),
|
||||
),
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_plugin_mcp_servers_by_requirements(
|
||||
"sample@test",
|
||||
&mut servers,
|
||||
Some(&requirements),
|
||||
Some(&matchers),
|
||||
);
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
(MATCHED_SERVER.to_string(), (true, None)),
|
||||
(MISMATCHED_SERVER.to_string(), (false, reason.clone())),
|
||||
(
|
||||
MATCHER_MISMATCHED_SERVER.to_string(),
|
||||
(false, reason.clone()),
|
||||
),
|
||||
(UNLISTED_SERVER.to_string(), (false, reason)),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_plugin_mcp_servers_by_allowlist_blocks_unlisted_plugin() {
|
||||
let mut servers = HashMap::from([("server-a".to_string(), stdio_mcp("cmd-a"))]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let requirements = Sourced::new(
|
||||
BTreeMap::from([(
|
||||
"other@test".to_string(),
|
||||
codex_config::PluginRequirementsToml {
|
||||
mcp_servers: Some(BTreeMap::from([(
|
||||
"server-a".to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "cmd-a".to_string(),
|
||||
},
|
||||
},
|
||||
)])),
|
||||
},
|
||||
)]),
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_plugin_mcp_servers_by_requirements(
|
||||
"sample@test",
|
||||
&mut servers,
|
||||
Some(&requirements),
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([(
|
||||
"server-a".to_string(),
|
||||
(
|
||||
false,
|
||||
Some(McpServerDisabledReason::Requirements { source })
|
||||
)
|
||||
)])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_plugin_mcp_servers_by_matchers_enforces_name_and_invocation() {
|
||||
const MATCHED_SERVER: &str = "matched";
|
||||
const MISMATCHED_SERVER: &str = "mismatched";
|
||||
const UNLISTED_SERVER: &str = "unlisted";
|
||||
|
||||
let mut servers = HashMap::from([
|
||||
(
|
||||
MATCHED_SERVER.to_string(),
|
||||
stdio_mcp_with_args("company-cli", &["approved"]),
|
||||
),
|
||||
(
|
||||
MISMATCHED_SERVER.to_string(),
|
||||
stdio_mcp_with_args("company-cli", &["rejected"]),
|
||||
),
|
||||
(
|
||||
UNLISTED_SERVER.to_string(),
|
||||
stdio_mcp_with_args("company-cli", &["approved"]),
|
||||
),
|
||||
]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let matcher = McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
command: "company-cli".to_string(),
|
||||
args: vec![McpServerValueMatcher::Exact {
|
||||
value: "approved".to_string(),
|
||||
}],
|
||||
});
|
||||
let matchers = Sourced::new(
|
||||
BTreeMap::from([
|
||||
(MATCHED_SERVER.to_string(), matcher.clone()),
|
||||
(MISMATCHED_SERVER.to_string(), matcher),
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_plugin_mcp_servers_by_requirements(
|
||||
"sample@test",
|
||||
&mut servers,
|
||||
/*plugin_requirements*/ None,
|
||||
Some(&matchers),
|
||||
);
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
(MATCHED_SERVER.to_string(), (true, None)),
|
||||
(MISMATCHED_SERVER.to_string(), (false, reason.clone())),
|
||||
(UNLISTED_SERVER.to_string(), (false, reason)),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
+158
-11
@@ -14,7 +14,12 @@ use codex_config::ConfigRequirements;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_config::ConstrainedWithSource;
|
||||
use codex_config::FeatureRequirementsToml;
|
||||
use codex_config::McpServerIdentity;
|
||||
use codex_config::McpServerMatcher;
|
||||
use codex_config::McpServerRequirement;
|
||||
use codex_config::PluginRequirementsToml;
|
||||
use codex_config::ProfileV2Name;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_config::ResidencyRequirement;
|
||||
use codex_config::SandboxModeRequirement;
|
||||
use codex_config::Sourced;
|
||||
@@ -36,6 +41,8 @@ use codex_config::types::AuthCredentialsStoreMode;
|
||||
use codex_config::types::AuthKeyringBackendKind;
|
||||
use codex_config::types::History;
|
||||
use codex_config::types::McpServerConfig;
|
||||
use codex_config::types::McpServerDisabledReason;
|
||||
use codex_config::types::McpServerTransportConfig;
|
||||
use codex_config::types::MemoriesConfig;
|
||||
use codex_config::types::ModelAvailabilityNuxConfig;
|
||||
use codex_config::types::Notice;
|
||||
@@ -1482,9 +1489,22 @@ impl Config {
|
||||
plugin_id: &str,
|
||||
mcp_servers: &mut HashMap<String, McpServerConfig>,
|
||||
) {
|
||||
self.config_layer_stack
|
||||
.requirements()
|
||||
.apply_to_plugin_mcp_servers(plugin_id, mcp_servers);
|
||||
let requirements = self.config_layer_stack.requirements();
|
||||
filter_plugin_mcp_servers_by_requirements(
|
||||
plugin_id,
|
||||
mcp_servers,
|
||||
requirements.plugins.as_ref(),
|
||||
requirements.mcp_server_matchers.as_ref(),
|
||||
);
|
||||
let empty_mcp_allowlist = requirements
|
||||
.mcp_servers
|
||||
.as_ref()
|
||||
.filter(|requirements| requirements.value.is_empty());
|
||||
filter_mcp_servers_by_requirements(
|
||||
mcp_servers,
|
||||
empty_mcp_allowlist,
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
}
|
||||
|
||||
pub async fn to_mcp_config(
|
||||
@@ -1851,6 +1871,112 @@ fn load_model_catalog(
|
||||
.transpose()
|
||||
}
|
||||
|
||||
fn filter_mcp_servers_by_requirements(
|
||||
mcp_servers: &mut HashMap<String, McpServerConfig>,
|
||||
mcp_requirements: Option<&Sourced<BTreeMap<String, McpServerRequirement>>>,
|
||||
mcp_matchers: Option<&Sourced<BTreeMap<String, McpServerMatcher>>>,
|
||||
) {
|
||||
if mcp_requirements.is_none() && mcp_matchers.is_none() {
|
||||
return;
|
||||
}
|
||||
|
||||
let source = RequirementSource::composite(
|
||||
mcp_requirements
|
||||
.iter()
|
||||
.map(|requirements| requirements.source.clone())
|
||||
.chain(
|
||||
mcp_matchers
|
||||
.iter()
|
||||
.map(|requirements| requirements.source.clone()),
|
||||
),
|
||||
);
|
||||
for (name, server) in mcp_servers.iter_mut() {
|
||||
let identity_requirement =
|
||||
mcp_requirements.and_then(|requirements| requirements.value.get(name));
|
||||
let matcher = mcp_matchers.and_then(|matchers| matchers.value.get(name));
|
||||
let allowed = (identity_requirement.is_some() || matcher.is_some())
|
||||
&& identity_requirement
|
||||
.is_none_or(|requirement| mcp_server_matches_requirement(requirement, server))
|
||||
&& matcher.is_none_or(|matcher| matcher.matches(server));
|
||||
if allowed {
|
||||
server.disabled_reason = None;
|
||||
} else {
|
||||
server.enabled = false;
|
||||
server.disabled_reason = Some(McpServerDisabledReason::Requirements {
|
||||
source: source.clone(),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn filter_plugin_mcp_servers_by_requirements(
|
||||
plugin_config_name: &str,
|
||||
mcp_servers: &mut HashMap<String, McpServerConfig>,
|
||||
plugin_requirements: Option<&Sourced<BTreeMap<String, PluginRequirementsToml>>>,
|
||||
mcp_matchers: Option<&Sourced<BTreeMap<String, McpServerMatcher>>>,
|
||||
) {
|
||||
if plugin_requirements.is_none() && mcp_matchers.is_none() {
|
||||
return;
|
||||
}
|
||||
|
||||
let source = RequirementSource::composite(
|
||||
plugin_requirements
|
||||
.iter()
|
||||
.map(|requirements| requirements.source.clone())
|
||||
.chain(
|
||||
mcp_matchers
|
||||
.iter()
|
||||
.map(|requirements| requirements.source.clone()),
|
||||
),
|
||||
);
|
||||
let plugin_mcp_requirements = plugin_requirements
|
||||
.and_then(|requirements| requirements.value.get(plugin_config_name))
|
||||
.and_then(|plugin| plugin.mcp_servers.as_ref());
|
||||
|
||||
for (name, server) in mcp_servers.iter_mut() {
|
||||
let allowed_by_plugin_requirement = plugin_requirements.is_none()
|
||||
|| plugin_mcp_requirements
|
||||
.and_then(|mcp_requirements| mcp_requirements.get(name))
|
||||
.is_some_and(|requirement| mcp_server_matches_requirement(requirement, server));
|
||||
let allowed_by_matcher = mcp_matchers.is_none_or(|matchers| {
|
||||
matchers
|
||||
.value
|
||||
.get(name)
|
||||
.is_some_and(|matcher| matcher.matches(server))
|
||||
});
|
||||
let allowed = allowed_by_plugin_requirement && allowed_by_matcher;
|
||||
if allowed {
|
||||
server.disabled_reason = None;
|
||||
} else {
|
||||
server.enabled = false;
|
||||
server.disabled_reason = Some(McpServerDisabledReason::Requirements {
|
||||
source: source.clone(),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn constrain_mcp_servers(
|
||||
mcp_servers: HashMap<String, McpServerConfig>,
|
||||
mcp_requirements: Option<&Sourced<BTreeMap<String, McpServerRequirement>>>,
|
||||
mcp_matchers: Option<&Sourced<BTreeMap<String, McpServerMatcher>>>,
|
||||
) -> ConstraintResult<Constrained<HashMap<String, McpServerConfig>>> {
|
||||
if mcp_requirements.is_none() && mcp_matchers.is_none() {
|
||||
return Ok(Constrained::allow_any(mcp_servers));
|
||||
}
|
||||
|
||||
let mcp_requirements = mcp_requirements.cloned();
|
||||
let mcp_matchers = mcp_matchers.cloned();
|
||||
Constrained::normalized(mcp_servers, move |mut servers| {
|
||||
filter_mcp_servers_by_requirements(
|
||||
&mut servers,
|
||||
mcp_requirements.as_ref(),
|
||||
mcp_matchers.as_ref(),
|
||||
);
|
||||
servers
|
||||
})
|
||||
}
|
||||
|
||||
fn apply_requirement_constrained_value<T>(
|
||||
field_name: &'static str,
|
||||
configured_value: T,
|
||||
@@ -1887,6 +2013,26 @@ where
|
||||
Ok(false)
|
||||
}
|
||||
|
||||
fn mcp_server_matches_requirement(
|
||||
requirement: &McpServerRequirement,
|
||||
server: &McpServerConfig,
|
||||
) -> bool {
|
||||
match &requirement.identity {
|
||||
McpServerIdentity::Command {
|
||||
command: want_command,
|
||||
} => matches!(
|
||||
&server.transport,
|
||||
McpServerTransportConfig::Stdio { command: got_command, .. }
|
||||
if got_command == want_command
|
||||
),
|
||||
McpServerIdentity::Url { url: want_url } => matches!(
|
||||
&server.transport,
|
||||
McpServerTransportConfig::StreamableHttp { url: got_url, .. }
|
||||
if got_url == want_url
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn load_global_mcp_servers(
|
||||
codex_home: &Path,
|
||||
) -> std::io::Result<BTreeMap<String, McpServerConfig>> {
|
||||
@@ -2823,8 +2969,6 @@ impl Config {
|
||||
resolve_orchestrator_feature_enabled(orchestrator.and_then(|value| value.mcp.as_ref()));
|
||||
// Ensure that every field of ConfigRequirements is applied to the final
|
||||
// Config.
|
||||
let requirements = config_layer_stack.requirements().clone();
|
||||
let mcp_server_policy = requirements.mcp_server_policy();
|
||||
let ConfigRequirements {
|
||||
approval_policy: mut constrained_approval_policy,
|
||||
approvals_reviewer: mut constrained_approvals_reviewer,
|
||||
@@ -2837,15 +2981,15 @@ impl Config {
|
||||
computer_use: _,
|
||||
feature_requirements,
|
||||
managed_hooks: _,
|
||||
mcp_servers: _,
|
||||
mcp_server_matchers: _,
|
||||
mcp_servers,
|
||||
mcp_server_matchers,
|
||||
plugins: _,
|
||||
exec_policy: _,
|
||||
enforce_residency,
|
||||
network: network_requirements,
|
||||
filesystem: filesystem_requirements,
|
||||
guardian_policy_config_source: _,
|
||||
} = requirements;
|
||||
} = config_layer_stack.requirements().clone();
|
||||
|
||||
let mut startup_warnings = config_layer_stack
|
||||
.startup_warnings()
|
||||
@@ -3594,9 +3738,12 @@ impl Config {
|
||||
&mut startup_warnings,
|
||||
)?;
|
||||
|
||||
let mcp_servers = mcp_server_policy
|
||||
.constrain(cfg.mcp_servers.clone())
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{e}")))?;
|
||||
let mcp_servers = constrain_mcp_servers(
|
||||
cfg.mcp_servers.clone(),
|
||||
mcp_servers.as_ref(),
|
||||
mcp_server_matchers.as_ref(),
|
||||
)
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{e}")))?;
|
||||
|
||||
let network_permission_profile = constrained_permission_profile.get().clone();
|
||||
let network = build_network_proxy_spec(
|
||||
|
||||
Reference in New Issue
Block a user