From 91d9cad26954243ea066b350543690c24ccb97b3 Mon Sep 17 00:00:00 2001 From: Felix Xia Date: Tue, 23 Jun 2026 15:16:15 +0100 Subject: [PATCH] Add managed MCP server matchers --- codex-rs/Cargo.lock | 1 + codex-rs/config/Cargo.toml | 1 + codex-rs/config/src/config_requirements.rs | 106 ++++++++ codex-rs/config/src/constraint.rs | 9 + codex-rs/config/src/lib.rs | 5 + codex-rs/config/src/mcp_requirements.rs | 95 ++++++++ codex-rs/config/src/mcp_requirements_tests.rs | 116 +++++++++ .../config/src/requirements_layers/stack.rs | 2 + .../src/requirements_layers/stack_tests.rs | 40 +++ codex-rs/core/src/config/config_tests.rs | 227 +++++++++++++++++- codex-rs/core/src/config/mod.rs | 56 ++++- 11 files changed, 642 insertions(+), 16 deletions(-) create mode 100644 codex-rs/config/src/mcp_requirements.rs create mode 100644 codex-rs/config/src/mcp_requirements_tests.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 65d581db6f..c7de9d6f8d 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2556,6 +2556,7 @@ dependencies = [ "multimap", "pretty_assertions", "prost 0.14.3", + "regex-lite", "schemars 0.8.22", "serde", "serde_ignored", diff --git a/codex-rs/config/Cargo.toml b/codex-rs/config/Cargo.toml index 54ad61f94d..fc96d034af 100644 --- a/codex-rs/config/Cargo.toml +++ b/codex-rs/config/Cargo.toml @@ -31,6 +31,7 @@ gethostname = { workspace = true } indexmap = { workspace = true, features = ["serde"] } multimap = { workspace = true } prost = "0.14.3" +regex-lite = { workspace = true } schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_ignored = { workspace = true } diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index ac51734536..239f763fb5 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -18,6 +18,7 @@ use super::requirements_exec_policy::RequirementsExecPolicyToml; use crate::Constrained; use crate::ConstraintError; use crate::ManagedHooksRequirementsToml; +use crate::McpServerMatcher; use crate::mcp_types::AppToolApproval; use crate::permissions_toml::PermissionProfileToml; use crate::types::WindowsSandboxModeToml; @@ -154,6 +155,7 @@ pub struct ConfigRequirements { pub feature_requirements: Option>, pub managed_hooks: Option>, pub mcp_servers: Option>>, + pub mcp_server_matchers: Option>>, pub plugins: Option>>, pub exec_policy: Option>, pub enforce_residency: ConstrainedWithSource>, @@ -195,6 +197,7 @@ impl Default for ConfigRequirements { feature_requirements: None, managed_hooks: None, mcp_servers: None, + mcp_server_matchers: None, plugins: None, exec_policy: None, enforce_residency: ConstrainedWithSource::new( @@ -841,6 +844,7 @@ pub struct ConfigRequirementsToml { pub feature_requirements: Option, pub hooks: Option, pub mcp_servers: Option>, + pub mcp_server_matchers: Option>, pub plugins: Option>, pub apps: Option, pub rules: Option, @@ -895,6 +899,7 @@ pub struct ConfigRequirementsWithSources { pub feature_requirements: Option>, pub hooks: Option>, pub mcp_servers: Option>>, + pub mcp_server_matchers: Option>>, pub plugins: Option>>, pub apps: Option>, pub rules: Option>, @@ -938,6 +943,7 @@ impl ConfigRequirementsWithSources { feature_requirements: _, hooks: _, mcp_servers: _, + mcp_server_matchers: _, plugins: _, apps: _, rules: _, @@ -974,6 +980,7 @@ impl ConfigRequirementsWithSources { feature_requirements, hooks, mcp_servers, + mcp_server_matchers, plugins, rules, enforce_residency, @@ -1008,6 +1015,7 @@ impl ConfigRequirementsWithSources { feature_requirements, hooks, mcp_servers, + mcp_server_matchers, plugins, apps, rules, @@ -1032,6 +1040,7 @@ impl ConfigRequirementsWithSources { feature_requirements: feature_requirements.map(|sourced| sourced.value), hooks: hooks.map(|sourced| sourced.value), mcp_servers: mcp_servers.map(|sourced| sourced.value), + mcp_server_matchers: mcp_server_matchers.map(|sourced| sourced.value), plugins: plugins.map(|sourced| sourced.value), apps: apps.map(|sourced| sourced.value), rules: rules.map(|sourced| sourced.value), @@ -1134,6 +1143,7 @@ impl ConfigRequirementsToml { .as_ref() .is_none_or(ManagedHooksRequirementsToml::is_empty) && self.mcp_servers.is_none() + && self.mcp_server_matchers.is_none() && self .plugins .as_ref() @@ -1175,6 +1185,7 @@ impl TryFrom for ConfigRequirements { feature_requirements, hooks, mcp_servers, + mcp_server_matchers, plugins, apps: _apps, rules, @@ -1184,6 +1195,22 @@ impl TryFrom for ConfigRequirements { guardian_policy_config, } = toml; + if let Some(Sourced { + value: matchers, + source, + }) = &mcp_server_matchers + { + for (server_name, matcher) in matchers { + matcher + .validate() + .map_err(|reason| ConstraintError::McpServerMatcherParse { + server_name: server_name.clone(), + requirement_source: source.clone(), + reason, + })?; + } + } + let approval_policy = match allowed_approval_policies { Some(Sourced { value: policies, @@ -1455,6 +1482,7 @@ impl TryFrom for ConfigRequirements { feature_requirements, managed_hooks, mcp_servers, + mcp_server_matchers, plugins, exec_policy, enforce_residency, @@ -1492,6 +1520,8 @@ pub fn sandbox_mode_requirement_for_permission_profile( mod tests { use super::*; use crate::HookEventsToml; + use crate::McpServerCommandMatcher; + use crate::McpServerValueMatcher; use anyhow::Result; use codex_execpolicy::Decision; use codex_execpolicy::Evaluation; @@ -1548,6 +1578,7 @@ mod tests { feature_requirements, hooks, mcp_servers, + mcp_server_matchers, plugins, apps, rules, @@ -1581,6 +1612,8 @@ mod tests { .map(|value| Sourced::new(value, RequirementSource::Unknown)), hooks: hooks.map(|value| Sourced::new(value, RequirementSource::Unknown)), mcp_servers: mcp_servers.map(|value| Sourced::new(value, RequirementSource::Unknown)), + mcp_server_matchers: mcp_server_matchers + .map(|value| Sourced::new(value, RequirementSource::Unknown)), plugins: plugins.map(|value| Sourced::new(value, RequirementSource::Unknown)), apps: apps.map(|value| Sourced::new(value, RequirementSource::Unknown)), rules: rules.map(|value| Sourced::new(value, RequirementSource::Unknown)), @@ -1784,6 +1817,7 @@ mod tests { feature_requirements: Some(feature_requirements.clone()), hooks: None, mcp_servers: None, + mcp_server_matchers: None, plugins: None, apps: None, rules: None, @@ -1833,6 +1867,7 @@ mod tests { )), hooks: None, mcp_servers: None, + mcp_server_matchers: None, plugins: None, apps: None, rules: None, @@ -1879,6 +1914,7 @@ mod tests { feature_requirements: None, hooks: None, mcp_servers: None, + mcp_server_matchers: None, plugins: None, apps: None, rules: None, @@ -1933,6 +1969,7 @@ mod tests { feature_requirements: None, hooks: None, mcp_servers: None, + mcp_server_matchers: None, plugins: None, apps: None, rules: None, @@ -3445,6 +3482,75 @@ command = "python3 /enterprise/hooks/pre.py" Ok(()) } + #[test] + fn deserialize_mcp_server_matchers() -> Result<()> { + let toml_str = r#" + [mcp_server_matchers.bloomberg_forge] + command = "forge" + args = [ + { match = "exact", value = "mcp" }, + { match = "exact", value = "proxy" }, + { match = "exact", value = "--server" }, + { match = "regex", expression = '^https://[A-Za-z0-9-]+\.mcp\.internal\.example\.com(?::443)?(?:/.*)?$' }, + ] + "#; + let requirements: ConfigRequirements = + with_unknown_source(from_str(toml_str)?).try_into()?; + + assert_eq!( + requirements.mcp_server_matchers, + Some(Sourced::new( + BTreeMap::from([( + "bloomberg_forge".to_string(), + McpServerMatcher::Command(McpServerCommandMatcher { + command: "forge".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(), + }, + ], + }), + )]), + RequirementSource::Unknown, + )) + ); + Ok(()) + } + + #[test] + fn invalid_mcp_server_matcher_regex_reports_the_server_name_and_source() -> Result<()> { + let toml_str = r#" + [mcp_server_matchers.broken_rule] + url = { match = "regex", expression = "[" } + "#; + + let err = ConfigRequirements::try_from(with_unknown_source(from_str(toml_str)?)) + .expect_err("invalid matcher regex should fail requirements normalization"); + let ConstraintError::McpServerMatcherParse { + server_name, + requirement_source, + reason, + } = err + else { + panic!("unexpected error: {err:?}"); + }; + + assert_eq!(server_name, "broken_rule"); + assert_eq!(requirement_source, RequirementSource::Unknown); + assert!(reason.contains("invalid regex `[`"), "{reason}"); + Ok(()) + } + #[test] fn deserialize_plugin_mcp_server_requirements() -> Result<()> { let toml_str = r#" diff --git a/codex-rs/config/src/constraint.rs b/codex-rs/config/src/constraint.rs index 64b604cbc2..98250e1ee7 100644 --- a/codex-rs/config/src/constraint.rs +++ b/codex-rs/config/src/constraint.rs @@ -24,6 +24,15 @@ pub enum ConstraintError { requirement_source: RequirementSource, reason: String, }, + + #[error( + "invalid matcher for MCP server `{server_name}` in requirements (set by {requirement_source}): {reason}" + )] + McpServerMatcherParse { + server_name: String, + requirement_source: RequirementSource, + reason: String, + }, } impl ConstraintError { diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index ed26757715..fab0d9f5a8 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -12,6 +12,7 @@ mod key_aliases; pub mod loader; mod marketplace_edit; mod mcp_edit; +mod mcp_requirements; mod mcp_types; mod merge; mod overrides; @@ -108,6 +109,10 @@ pub use marketplace_edit::remove_user_marketplace; pub use marketplace_edit::remove_user_marketplace_config; 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::McpServerUrlMatcher; +pub use mcp_requirements::McpServerValueMatcher; pub use mcp_types::AppToolApproval; pub use mcp_types::DEFAULT_MCP_SERVER_ENVIRONMENT_ID; pub use mcp_types::McpServerConfig; diff --git a/codex-rs/config/src/mcp_requirements.rs b/codex-rs/config/src/mcp_requirements.rs new file mode 100644 index 0000000000..132e5edf55 --- /dev/null +++ b/codex-rs/config/src/mcp_requirements.rs @@ -0,0 +1,95 @@ +use crate::mcp_types::McpServerConfig; +use crate::mcp_types::McpServerTransportConfig; +use regex_lite::Regex; +use serde::Deserialize; + +/// String matching operations available to managed MCP server matchers. +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(tag = "match", rename_all = "snake_case", deny_unknown_fields)] +pub enum McpServerValueMatcher { + Exact { value: String }, + Prefix { value: String }, + Regex { expression: String }, +} + +impl McpServerValueMatcher { + fn validate(&self) -> Result<(), String> { + let Self::Regex { expression } = self else { + return Ok(()); + }; + Regex::new(expression) + .map(|_| ()) + .map_err(|err| format!("invalid regex `{expression}`: {err}")) + } + + fn matches(&self, candidate: &str) -> bool { + match self { + Self::Exact { value } => candidate == value, + Self::Prefix { value } => candidate.starts_with(value), + Self::Regex { expression } => Regex::new(expression) + .ok() + .and_then(|regex| regex.find(candidate)) + .is_some_and(|matched| matched.start() == 0 && matched.end() == candidate.len()), + } + } +} + +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub struct McpServerCommandMatcher { + pub command: String, + pub args: Vec, +} + +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub struct McpServerUrlMatcher { + pub url: McpServerValueMatcher, +} + +/// A managed matcher for either a stdio command invocation or a direct MCP URL. +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(untagged)] +pub enum McpServerMatcher { + Command(McpServerCommandMatcher), + Url(McpServerUrlMatcher), +} + +impl McpServerMatcher { + pub(crate) fn validate(&self) -> Result<(), String> { + match self { + Self::Command(matcher) => { + for (index, arg) in matcher.args.iter().enumerate() { + arg.validate().map_err(|err| { + format!("invalid argument matcher at index {index}: {err}") + })?; + } + Ok(()) + } + Self::Url(matcher) => matcher.url.validate(), + } + } + + pub fn matches(&self, server: &McpServerConfig) -> bool { + match (self, &server.transport) { + (Self::Command(matcher), McpServerTransportConfig::Stdio { command, args, .. }) => { + matcher.command == *command + && matcher.args.len() == args.len() + && matcher + .args + .iter() + .zip(args) + .all(|(matcher, arg)| matcher.matches(arg)) + } + (Self::Url(matcher), McpServerTransportConfig::StreamableHttp { url, .. }) => { + matcher.url.matches(url) + } + (Self::Command(_), McpServerTransportConfig::StreamableHttp { .. }) + | (Self::Url(_), McpServerTransportConfig::Stdio { .. }) => false, + } + } +} + +#[cfg(test)] +#[path = "mcp_requirements_tests.rs"] +mod tests; diff --git a/codex-rs/config/src/mcp_requirements_tests.rs b/codex-rs/config/src/mcp_requirements_tests.rs new file mode 100644 index 0000000000..beed543877 --- /dev/null +++ b/codex-rs/config/src/mcp_requirements_tests.rs @@ -0,0 +1,116 @@ +use super::*; +use crate::mcp_types::McpServerConfig; +use pretty_assertions::assert_eq; +use std::collections::HashMap; + +fn stdio_server(command: &str, args: &[&str]) -> McpServerConfig { + McpServerConfig { + transport: McpServerTransportConfig::Stdio { + command: command.to_string(), + args: args.iter().map(ToString::to_string).collect(), + env: None, + env_vars: Vec::new(), + cwd: 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(), + } +} + +#[test] +fn command_matcher_matches_exact_positional_arguments() { + let matcher = McpServerMatcher::Command(McpServerCommandMatcher { + command: "forge".to_string(), + args: vec![ + McpServerValueMatcher::Exact { + value: "mcp".to_string(), + }, + McpServerValueMatcher::Regex { + expression: r"https://[a-z]+\.example\.com".to_string(), + }, + ], + }); + + assert!(matcher.matches(&stdio_server( + "forge", + &["mcp", "https://pricing.example.com"] + ))); + assert!(!matcher.matches(&stdio_server( + "forge", + &["https://pricing.example.com", "mcp"] + ))); + assert!(!matcher.matches(&stdio_server( + "forge", + &["mcp", "https://pricing.example.com", "--verbose"] + ))); + assert!(!matcher.matches(&stdio_server( + "/usr/local/bin/forge", + &["mcp", "https://pricing.example.com"] + ))); +} + +#[test] +fn regex_matcher_requires_a_full_value_match() { + let matcher = McpServerValueMatcher::Regex { + expression: "mcp".to_string(), + }; + + assert!(matcher.matches("mcp")); + assert!(!matcher.matches("mcp-proxy")); + assert!(!matcher.matches("prefix-mcp")); +} + +#[test] +fn matcher_deserializes_command_and_url_shapes() { + let command: McpServerMatcher = toml::from_str( + r#" +command = "forge" +args = [ + { match = "exact", value = "mcp" }, + { match = "regex", expression = '^https://[a-z]+\.example\.com$' }, +] +"#, + ) + .expect("command matcher"); + let url: McpServerMatcher = toml::from_str( + r#" +url = { match = "prefix", value = "https://mcp.example.com/" } +"#, + ) + .expect("URL matcher"); + + assert_eq!( + command, + McpServerMatcher::Command(McpServerCommandMatcher { + command: "forge".to_string(), + args: vec![ + McpServerValueMatcher::Exact { + value: "mcp".to_string(), + }, + McpServerValueMatcher::Regex { + expression: r"^https://[a-z]+\.example\.com$".to_string(), + }, + ], + }) + ); + assert_eq!( + url, + McpServerMatcher::Url(McpServerUrlMatcher { + url: McpServerValueMatcher::Prefix { + value: "https://mcp.example.com/".to_string(), + }, + }) + ); +} diff --git a/codex-rs/config/src/requirements_layers/stack.rs b/codex-rs/config/src/requirements_layers/stack.rs index a7d8245ece..0a5208ea80 100644 --- a/codex-rs/config/src/requirements_layers/stack.rs +++ b/codex-rs/config/src/requirements_layers/stack.rs @@ -221,6 +221,7 @@ fn populate_merged_regular_fields_with_sources( feature_requirements, hooks: _, mcp_servers, + mcp_server_matchers, plugins, apps, rules: _, @@ -249,6 +250,7 @@ fn populate_merged_regular_fields_with_sources( set_sourced!(windows, &["windows"]); set_sourced!(feature_requirements, &["features", "feature_requirements"]); set_sourced!(mcp_servers, &["mcp_servers"]); + set_sourced!(mcp_server_matchers, &["mcp_server_matchers"]); set_sourced!(plugins, &["plugins"]); set_sourced!(apps, &["apps"]); set_sourced!(enforce_residency, &["enforce_residency"]); diff --git a/codex-rs/config/src/requirements_layers/stack_tests.rs b/codex-rs/config/src/requirements_layers/stack_tests.rs index fafd0523e8..469eb7c8fc 100644 --- a/codex-rs/config/src/requirements_layers/stack_tests.rs +++ b/codex-rs/config/src/requirements_layers/stack_tests.rs @@ -389,6 +389,46 @@ command = "high-mcp" ); } +#[test] +fn mcp_server_matchers_use_regular_toml_merge() { + let composed = compose(vec![ + layer( + "req_low", + "Low", + r#" +[mcp_server_matchers.shared] +url = { match = "prefix", value = "https://low.example.com/" } + +[mcp_server_matchers.low] +url = { match = "exact", value = "https://low.example.com/mcp" } +"#, + ), + layer( + "req_high", + "High", + r#" +[mcp_server_matchers.shared] +url = { match = "prefix", value = "https://high.example.com/" } +"#, + ), + ]) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements( + r#" +[mcp_server_matchers.low] +url = { match = "exact", value = "https://low.example.com/mcp" } + +[mcp_server_matchers.shared] +url = { match = "prefix", value = "https://high.example.com/" } +"# + ) + ); +} + #[test] fn network_maps_use_regular_toml_merge() { let composed = compose(vec![ diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 67cb0932de..6aceb7349b 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -5,6 +5,9 @@ use assert_matches::assert_matches; use codex_config::CONFIG_TOML_FILE; use codex_config::ConfigLayerEntry; use codex_config::ConfigLayerStack; +use codex_config::McpServerCommandMatcher; +use codex_config::McpServerMatcher; +use codex_config::McpServerValueMatcher; use codex_config::ProfileV2Name; use codex_config::RequirementSource; use codex_config::Sourced; @@ -112,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, @@ -4172,7 +4179,11 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() { ]), source.clone(), ); - filter_mcp_servers_by_requirements(&mut servers, Some(&requirements)); + filter_mcp_servers_by_requirements( + &mut servers, + Some(&requirements), + /*mcp_matchers*/ None, + ); let reason = Some(McpServerDisabledReason::Requirements { source }); assert_eq!( @@ -4203,7 +4214,11 @@ fn filter_mcp_servers_by_allowlist_allows_all_when_unset() { ("server-b".to_string(), http_mcp("https://example.com/b")), ]); - filter_mcp_servers_by_requirements(&mut servers, /*mcp_requirements*/ None); + filter_mcp_servers_by_requirements( + &mut servers, + /*mcp_requirements*/ None, + /*mcp_matchers*/ None, + ); assert_eq!( servers @@ -4220,6 +4235,205 @@ fn filter_mcp_servers_by_allowlist_allows_all_when_unset() { ); } +#[test] +fn filter_mcp_servers_by_matchers_enforces_command_and_positional_args() { + let mut servers = HashMap::from([ + ( + "bloomberg_forge".to_string(), + stdio_mcp_with_args( + "forge", + &[ + "mcp", + "proxy", + "--server", + "https://pricing.mcp.internal.example.com", + ], + ), + ), + ( + "unlisted".to_string(), + stdio_mcp_with_args( + "forge", + &[ + "mcp", + "proxy", + "--server", + "https://pricing.mcp.internal.example.com", + ], + ), + ), + ( + "wrong-order".to_string(), + stdio_mcp_with_args( + "forge", + &[ + "proxy", + "mcp", + "--server", + "https://pricing.mcp.internal.example.com", + ], + ), + ), + ( + "trailing-arg".to_string(), + stdio_mcp_with_args( + "forge", + &[ + "mcp", + "proxy", + "--server", + "https://pricing.mcp.internal.example.com", + "--verbose", + ], + ), + ), + ( + "wrong-host".to_string(), + stdio_mcp_with_args( + "forge", + &["mcp", "proxy", "--server", "https://mcp.example.com"], + ), + ), + ]); + let source = RequirementSource::LegacyManagedConfigTomlFromMdm; + let matcher = McpServerMatcher::Command(McpServerCommandMatcher { + command: "forge".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([ + ("bloomberg_forge".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::from([ + ("bloomberg_forge".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("forge", &["approved"]), + ), + ( + EXACT_ONLY_MATCHES.to_string(), + stdio_mcp_with_args("forge", &["rejected"]), + ), + ( + MATCHER_ONLY_MATCHES.to_string(), + stdio_mcp_with_args("forge", &["approved"]), + ), + ]); + let source = RequirementSource::LegacyManagedConfigTomlFromMdm; + let requirements = Sourced::new( + BTreeMap::from([ + ( + BOTH_MATCH.to_string(), + McpServerRequirement { + identity: McpServerIdentity::Command { + command: "forge".to_string(), + }, + }, + ), + ( + EXACT_ONLY_MATCHES.to_string(), + McpServerRequirement { + identity: McpServerIdentity::Command { + command: "forge".to_string(), + }, + }, + ), + ( + MATCHER_ONLY_MATCHES.to_string(), + McpServerRequirement { + identity: McpServerIdentity::Command { + command: "different-command".to_string(), + }, + }, + ), + ]), + source.clone(), + ); + let matcher = McpServerMatcher::Command(McpServerCommandMatcher { + command: "forge".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::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([ @@ -4229,7 +4443,11 @@ fn filter_mcp_servers_by_allowlist_blocks_all_when_empty() { let source = RequirementSource::LegacyManagedConfigTomlFromMdm; let requirements = Sourced::new(BTreeMap::new(), source.clone()); - filter_mcp_servers_by_requirements(&mut servers, Some(&requirements)); + filter_mcp_servers_by_requirements( + &mut servers, + Some(&requirements), + /*mcp_matchers*/ None, + ); let reason = Some(McpServerDisabledReason::Requirements { source }); assert_eq!( @@ -8678,6 +8896,7 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset() feature_requirements: None, hooks: None, mcp_servers: None, + mcp_server_matchers: None, plugins: None, apps: None, rules: None, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 81d2870c11..5ed06d6fef 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -15,9 +15,11 @@ 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; @@ -1498,7 +1500,13 @@ impl Config { .mcp_servers .as_ref() .filter(|requirements| requirements.value.is_empty()); - filter_mcp_servers_by_requirements(mcp_servers, empty_mcp_allowlist); + 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); } pub async fn to_mcp_config( @@ -1868,17 +1876,30 @@ fn load_model_catalog( fn filter_mcp_servers_by_requirements( mcp_servers: &mut HashMap, mcp_requirements: Option<&Sourced>>, + mcp_matchers: Option<&Sourced>>, ) { - let Some(allowlist) = mcp_requirements else { + if mcp_requirements.is_none() && mcp_matchers.is_none() { return; - }; + } - let source = allowlist.source.clone(); + 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 allowed = allowlist - .value - .get(name) - .is_some_and(|requirement| mcp_server_matches_requirement(requirement, server)); + 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 { @@ -1922,14 +1943,20 @@ fn filter_plugin_mcp_servers_by_requirements( fn constrain_mcp_servers( mcp_servers: HashMap, mcp_requirements: Option<&Sourced>>, + mcp_matchers: Option<&Sourced>>, ) -> ConstraintResult>> { - if mcp_requirements.is_none() { + 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()); + filter_mcp_servers_by_requirements( + &mut servers, + mcp_requirements.as_ref(), + mcp_matchers.as_ref(), + ); servers }) } @@ -2939,6 +2966,7 @@ impl Config { feature_requirements, managed_hooks: _, mcp_servers, + mcp_server_matchers, plugins: _, exec_policy: _, enforce_residency, @@ -3694,8 +3722,12 @@ impl Config { &mut startup_warnings, )?; - let mcp_servers = constrain_mcp_servers(cfg.mcp_servers.clone(), mcp_servers.as_ref()) - .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(