mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Skip plugin MCP filtering when no allowlists are configured (#35280)
## What changed - Leave plugin MCP servers unchanged when every plugin requirement omits `mcp_servers`. - Continue treating an explicitly empty `mcp_servers` allowlist as deny-all. ## Testing Added coverage for both absent and explicitly empty plugin MCP allowlists. GitOrigin-RevId: 80bb9aa6bbd779d669c573f63681dc46019b668c
This commit is contained in:
@@ -4655,6 +4655,63 @@ fn filter_mcp_servers_by_allowlist_blocks_all_when_empty() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_plugin_mcp_servers_without_allowlists_does_not_filter_any_plugin() {
|
||||
let original_servers = HashMap::from([
|
||||
("server-a".to_string(), stdio_mcp("cmd-a")),
|
||||
("server-b".to_string(), http_mcp("https://example.com/b")),
|
||||
]);
|
||||
let requirements = Sourced::new(
|
||||
BTreeMap::from([(
|
||||
"sites@openai-bundled".to_string(),
|
||||
codex_config::PluginRequirementsToml { mcp_servers: None },
|
||||
)]),
|
||||
RequirementSource::LegacyManagedConfigTomlFromMdm,
|
||||
);
|
||||
|
||||
for plugin_name in ["sites@openai-bundled", "sample@test"] {
|
||||
let mut servers = original_servers.clone();
|
||||
filter_plugin_mcp_servers_by_requirements(plugin_name, &mut servers, Some(&requirements));
|
||||
|
||||
assert_eq!(servers, original_servers);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_plugin_mcp_servers_by_empty_allowlist_blocks_all() {
|
||||
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::from([(
|
||||
"sample@test".to_string(),
|
||||
codex_config::PluginRequirementsToml {
|
||||
mcp_servers: Some(BTreeMap::new()),
|
||||
},
|
||||
)]),
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_plugin_mcp_servers_by_requirements("sample@test", &mut servers, Some(&requirements));
|
||||
|
||||
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_identity_rules() {
|
||||
const MATCHED_SERVER: &str = "matched-should-allow";
|
||||
|
||||
@@ -1995,6 +1995,13 @@ fn filter_plugin_mcp_servers_by_requirements(
|
||||
let Some(requirements) = plugin_requirements else {
|
||||
return;
|
||||
};
|
||||
if !requirements
|
||||
.value
|
||||
.values()
|
||||
.any(|plugin| plugin.mcp_servers.is_some())
|
||||
{
|
||||
return;
|
||||
}
|
||||
let source = requirements.source.clone();
|
||||
let plugin_mcp_requirements = requirements
|
||||
.value
|
||||
|
||||
Reference in New Issue
Block a user