mirror of
https://github.com/openai/codex.git
synced 2026-11-08 20:40:33 +00:00
Add managed MCP server matchers
This commit is contained in:
Generated
+1
@@ -2556,6 +2556,7 @@ dependencies = [
|
||||
"multimap",
|
||||
"pretty_assertions",
|
||||
"prost 0.14.3",
|
||||
"regex-lite",
|
||||
"schemars 0.8.22",
|
||||
"serde",
|
||||
"serde_ignored",
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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<Sourced<FeatureRequirementsToml>>,
|
||||
pub managed_hooks: Option<ConstrainedWithSource<ManagedHooksRequirementsToml>>,
|
||||
pub mcp_servers: Option<Sourced<BTreeMap<String, McpServerRequirement>>>,
|
||||
pub mcp_server_matchers: Option<Sourced<BTreeMap<String, McpServerMatcher>>>,
|
||||
pub plugins: Option<Sourced<BTreeMap<String, PluginRequirementsToml>>>,
|
||||
pub exec_policy: Option<Sourced<RequirementsExecPolicy>>,
|
||||
pub enforce_residency: ConstrainedWithSource<Option<ResidencyRequirement>>,
|
||||
@@ -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<FeatureRequirementsToml>,
|
||||
pub hooks: Option<ManagedHooksRequirementsToml>,
|
||||
pub mcp_servers: Option<BTreeMap<String, McpServerRequirement>>,
|
||||
pub mcp_server_matchers: Option<BTreeMap<String, McpServerMatcher>>,
|
||||
pub plugins: Option<BTreeMap<String, PluginRequirementsToml>>,
|
||||
pub apps: Option<AppsRequirementsToml>,
|
||||
pub rules: Option<RequirementsExecPolicyToml>,
|
||||
@@ -895,6 +899,7 @@ pub struct ConfigRequirementsWithSources {
|
||||
pub feature_requirements: Option<Sourced<FeatureRequirementsToml>>,
|
||||
pub hooks: Option<Sourced<ManagedHooksRequirementsToml>>,
|
||||
pub mcp_servers: Option<Sourced<BTreeMap<String, McpServerRequirement>>>,
|
||||
pub mcp_server_matchers: Option<Sourced<BTreeMap<String, McpServerMatcher>>>,
|
||||
pub plugins: Option<Sourced<BTreeMap<String, PluginRequirementsToml>>>,
|
||||
pub apps: Option<Sourced<AppsRequirementsToml>>,
|
||||
pub rules: Option<Sourced<RequirementsExecPolicyToml>>,
|
||||
@@ -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<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
feature_requirements,
|
||||
hooks,
|
||||
mcp_servers,
|
||||
mcp_server_matchers,
|
||||
plugins,
|
||||
apps: _apps,
|
||||
rules,
|
||||
@@ -1184,6 +1195,22 @@ impl TryFrom<ConfigRequirementsWithSources> 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<ConfigRequirementsWithSources> 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#"
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<McpServerValueMatcher>,
|
||||
}
|
||||
|
||||
#[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;
|
||||
@@ -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(),
|
||||
},
|
||||
})
|
||||
);
|
||||
}
|
||||
@@ -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"]);
|
||||
|
||||
@@ -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![
|
||||
|
||||
@@ -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<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
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<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([
|
||||
@@ -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,
|
||||
|
||||
@@ -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<String, McpServerConfig>,
|
||||
mcp_requirements: Option<&Sourced<BTreeMap<String, McpServerRequirement>>>,
|
||||
mcp_matchers: Option<&Sourced<BTreeMap<String, McpServerMatcher>>>,
|
||||
) {
|
||||
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<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() {
|
||||
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(
|
||||
|
||||
Reference in New Issue
Block a user