mirror of
https://github.com/openai/codex.git
synced 2026-10-26 18:38:47 +00:00
Apply MCP matchers to plugin servers
This commit is contained in:
@@ -4466,15 +4466,20 @@ fn filter_mcp_servers_by_allowlist_blocks_all_when_empty() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_plugin_mcp_servers_by_allowlist_enforces_plugin_and_identity_rules() {
|
||||
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"),
|
||||
@@ -4502,13 +4507,52 @@ fn filter_plugin_mcp_servers_by_allowlist_enforces_plugin_and_identity_rules() {
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
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));
|
||||
filter_plugin_mcp_servers_by_requirements(
|
||||
"sample@test",
|
||||
&mut servers,
|
||||
Some(&requirements),
|
||||
Some(&matchers),
|
||||
);
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
@@ -4522,6 +4566,10 @@ fn filter_plugin_mcp_servers_by_allowlist_enforces_plugin_and_identity_rules() {
|
||||
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)),
|
||||
])
|
||||
);
|
||||
@@ -4548,7 +4596,12 @@ fn filter_plugin_mcp_servers_by_allowlist_blocks_unlisted_plugin() {
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_plugin_mcp_servers_by_requirements("sample@test", &mut servers, Some(&requirements));
|
||||
filter_plugin_mcp_servers_by_requirements(
|
||||
"sample@test",
|
||||
&mut servers,
|
||||
Some(&requirements),
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
servers
|
||||
@@ -4568,6 +4621,65 @@ fn filter_plugin_mcp_servers_by_allowlist_blocks_unlisted_plugin() {
|
||||
);
|
||||
}
|
||||
|
||||
#[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("forge", &["approved"]),
|
||||
),
|
||||
(
|
||||
MISMATCHED_SERVER.to_string(),
|
||||
stdio_mcp_with_args("forge", &["rejected"]),
|
||||
),
|
||||
(
|
||||
UNLISTED_SERVER.to_string(),
|
||||
stdio_mcp_with_args("forge", &["approved"]),
|
||||
),
|
||||
]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let matcher = McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
command: "forge".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()?;
|
||||
|
||||
@@ -1489,24 +1489,22 @@ impl Config {
|
||||
plugin_id: &str,
|
||||
mcp_servers: &mut HashMap<String, McpServerConfig>,
|
||||
) {
|
||||
let requirements = self.config_layer_stack.requirements();
|
||||
filter_plugin_mcp_servers_by_requirements(
|
||||
plugin_id,
|
||||
mcp_servers,
|
||||
self.config_layer_stack.requirements().plugins.as_ref(),
|
||||
requirements.plugins.as_ref(),
|
||||
requirements.mcp_server_matchers.as_ref(),
|
||||
);
|
||||
let empty_mcp_allowlist = self
|
||||
.config_layer_stack
|
||||
.requirements()
|
||||
let empty_mcp_allowlist = requirements
|
||||
.mcp_servers
|
||||
.as_ref()
|
||||
.filter(|requirements| requirements.value.is_empty());
|
||||
let empty_mcp_matchers = self
|
||||
.config_layer_stack
|
||||
.requirements()
|
||||
.mcp_server_matchers
|
||||
.as_ref()
|
||||
.filter(|requirements| requirements.value.is_empty());
|
||||
filter_mcp_servers_by_requirements(mcp_servers, empty_mcp_allowlist, empty_mcp_matchers);
|
||||
filter_mcp_servers_by_requirements(
|
||||
mcp_servers,
|
||||
empty_mcp_allowlist,
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
}
|
||||
|
||||
pub async fn to_mcp_config(
|
||||
@@ -1915,20 +1913,38 @@ 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>>>,
|
||||
) {
|
||||
let Some(requirements) = plugin_requirements else {
|
||||
if plugin_requirements.is_none() && mcp_matchers.is_none() {
|
||||
return;
|
||||
};
|
||||
let source = requirements.source.clone();
|
||||
let plugin_mcp_requirements = requirements
|
||||
.value
|
||||
.get(plugin_config_name)
|
||||
}
|
||||
|
||||
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 = plugin_mcp_requirements
|
||||
.and_then(|mcp_requirements| mcp_requirements.get(name))
|
||||
.is_some_and(|requirement| mcp_server_matches_requirement(requirement, server));
|
||||
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 {
|
||||
|
||||
Reference in New Issue
Block a user