mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Merge MCP matchers into server requirements
This commit is contained in:
@@ -18,7 +18,8 @@ use super::requirements_exec_policy::RequirementsExecPolicyToml;
|
||||
use crate::Constrained;
|
||||
use crate::ConstraintError;
|
||||
use crate::ManagedHooksRequirementsToml;
|
||||
use crate::McpServerMatcher;
|
||||
use crate::McpServerCommandMatcher;
|
||||
use crate::McpServerUrlMatcher;
|
||||
use crate::mcp_types::AppToolApproval;
|
||||
use crate::permissions_toml::PermissionProfileToml;
|
||||
use crate::types::WindowsSandboxModeToml;
|
||||
@@ -155,7 +156,6 @@ 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>>,
|
||||
@@ -197,7 +197,6 @@ 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(
|
||||
@@ -224,9 +223,17 @@ pub enum McpServerIdentity {
|
||||
Url { url: String },
|
||||
}
|
||||
|
||||
/// A requirement for one named MCP server.
|
||||
///
|
||||
/// The `Identity` variant preserves the released exact-match contract. The
|
||||
/// command and URL variants add matcher-based requirements under the same
|
||||
/// `mcp_servers` namespace.
|
||||
#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
|
||||
pub struct McpServerRequirement {
|
||||
pub identity: McpServerIdentity,
|
||||
#[serde(untagged, deny_unknown_fields)]
|
||||
pub enum McpServerRequirement {
|
||||
Identity { identity: McpServerIdentity },
|
||||
Command(McpServerCommandMatcher),
|
||||
Url(McpServerUrlMatcher),
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)]
|
||||
@@ -844,7 +851,6 @@ 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>,
|
||||
@@ -899,7 +905,6 @@ 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>>,
|
||||
@@ -943,7 +948,6 @@ impl ConfigRequirementsWithSources {
|
||||
feature_requirements: _,
|
||||
hooks: _,
|
||||
mcp_servers: _,
|
||||
mcp_server_matchers: _,
|
||||
plugins: _,
|
||||
apps: _,
|
||||
rules: _,
|
||||
@@ -980,7 +984,6 @@ impl ConfigRequirementsWithSources {
|
||||
feature_requirements,
|
||||
hooks,
|
||||
mcp_servers,
|
||||
mcp_server_matchers,
|
||||
plugins,
|
||||
rules,
|
||||
enforce_residency,
|
||||
@@ -1015,7 +1018,6 @@ impl ConfigRequirementsWithSources {
|
||||
feature_requirements,
|
||||
hooks,
|
||||
mcp_servers,
|
||||
mcp_server_matchers,
|
||||
plugins,
|
||||
apps,
|
||||
rules,
|
||||
@@ -1040,7 +1042,6 @@ 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),
|
||||
@@ -1143,7 +1144,6 @@ 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()
|
||||
@@ -1163,6 +1163,25 @@ impl ConfigRequirementsToml {
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_mcp_server_requirements(
|
||||
requirements: &BTreeMap<String, McpServerRequirement>,
|
||||
source: &RequirementSource,
|
||||
plugin_name: Option<&str>,
|
||||
) -> Result<(), ConstraintError> {
|
||||
for (server_name, requirement) in requirements {
|
||||
requirement
|
||||
.validate()
|
||||
.map_err(|reason| ConstraintError::McpServerRequirementParse {
|
||||
server_name: plugin_name
|
||||
.map(|plugin_name| format!("{plugin_name}/{server_name}"))
|
||||
.unwrap_or_else(|| server_name.clone()),
|
||||
requirement_source: source.clone(),
|
||||
reason,
|
||||
})?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
type Error = ConstraintError;
|
||||
|
||||
@@ -1185,7 +1204,6 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
feature_requirements,
|
||||
hooks,
|
||||
mcp_servers,
|
||||
mcp_server_matchers,
|
||||
plugins,
|
||||
apps: _apps,
|
||||
rules,
|
||||
@@ -1195,19 +1213,18 @@ 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,
|
||||
})?;
|
||||
if let Some(requirements) = &mcp_servers {
|
||||
validate_mcp_server_requirements(&requirements.value, &requirements.source, None)?;
|
||||
}
|
||||
if let Some(plugin_requirements) = &plugins {
|
||||
for (plugin_name, plugin) in &plugin_requirements.value {
|
||||
if let Some(requirements) = &plugin.mcp_servers {
|
||||
validate_mcp_server_requirements(
|
||||
requirements,
|
||||
&plugin_requirements.source,
|
||||
Some(plugin_name),
|
||||
)?;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1482,7 +1499,6 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
feature_requirements,
|
||||
managed_hooks,
|
||||
mcp_servers,
|
||||
mcp_server_matchers,
|
||||
plugins,
|
||||
exec_policy,
|
||||
enforce_residency,
|
||||
@@ -1578,7 +1594,6 @@ mod tests {
|
||||
feature_requirements,
|
||||
hooks,
|
||||
mcp_servers,
|
||||
mcp_server_matchers,
|
||||
plugins,
|
||||
apps,
|
||||
rules,
|
||||
@@ -1612,8 +1627,6 @@ 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)),
|
||||
@@ -1817,7 +1830,6 @@ mod tests {
|
||||
feature_requirements: Some(feature_requirements.clone()),
|
||||
hooks: None,
|
||||
mcp_servers: None,
|
||||
mcp_server_matchers: None,
|
||||
plugins: None,
|
||||
apps: None,
|
||||
rules: None,
|
||||
@@ -1867,7 +1879,6 @@ mod tests {
|
||||
)),
|
||||
hooks: None,
|
||||
mcp_servers: None,
|
||||
mcp_server_matchers: None,
|
||||
plugins: None,
|
||||
apps: None,
|
||||
rules: None,
|
||||
@@ -1914,7 +1925,6 @@ mod tests {
|
||||
feature_requirements: None,
|
||||
hooks: None,
|
||||
mcp_servers: None,
|
||||
mcp_server_matchers: None,
|
||||
plugins: None,
|
||||
apps: None,
|
||||
rules: None,
|
||||
@@ -1969,7 +1979,6 @@ mod tests {
|
||||
feature_requirements: None,
|
||||
hooks: None,
|
||||
mcp_servers: None,
|
||||
mcp_server_matchers: None,
|
||||
plugins: None,
|
||||
apps: None,
|
||||
rules: None,
|
||||
@@ -3461,7 +3470,7 @@ command = "python3 /enterprise/hooks/pre.py"
|
||||
BTreeMap::from([
|
||||
(
|
||||
"docs".to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "codex-mcp".to_string(),
|
||||
},
|
||||
@@ -3469,7 +3478,7 @@ command = "python3 /enterprise/hooks/pre.py"
|
||||
),
|
||||
(
|
||||
"remote".to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Url {
|
||||
url: "https://example.com/mcp".to_string(),
|
||||
},
|
||||
@@ -3483,9 +3492,9 @@ command = "python3 /enterprise/hooks/pre.py"
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserialize_mcp_server_matchers() -> Result<()> {
|
||||
fn deserialize_mcp_server_matcher_requirements() -> Result<()> {
|
||||
let toml_str = r#"
|
||||
[mcp_server_matchers.internal_mcp_proxy]
|
||||
[mcp_servers.internal_mcp_proxy]
|
||||
command = "company-cli"
|
||||
args = [
|
||||
{ match = "exact", value = "mcp" },
|
||||
@@ -3498,11 +3507,11 @@ command = "python3 /enterprise/hooks/pre.py"
|
||||
with_unknown_source(from_str(toml_str)?).try_into()?;
|
||||
|
||||
assert_eq!(
|
||||
requirements.mcp_server_matchers,
|
||||
requirements.mcp_servers,
|
||||
Some(Sourced::new(
|
||||
BTreeMap::from([(
|
||||
"internal_mcp_proxy".to_string(),
|
||||
McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
McpServerRequirement::Command(McpServerCommandMatcher {
|
||||
command: "company-cli".to_string(),
|
||||
args: vec![
|
||||
McpServerValueMatcher::Exact {
|
||||
@@ -3528,15 +3537,15 @@ command = "python3 /enterprise/hooks/pre.py"
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_mcp_server_matcher_regex_reports_the_server_name_and_source() -> Result<()> {
|
||||
fn invalid_mcp_server_requirement_regex_reports_the_server_name_and_source() -> Result<()> {
|
||||
let toml_str = r#"
|
||||
[mcp_server_matchers.broken_rule]
|
||||
[mcp_servers.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 {
|
||||
let ConstraintError::McpServerRequirementParse {
|
||||
server_name,
|
||||
requirement_source,
|
||||
reason,
|
||||
@@ -3572,7 +3581,7 @@ command = "python3 /enterprise/hooks/pre.py"
|
||||
PluginRequirementsToml {
|
||||
mcp_servers: Some(BTreeMap::from([(
|
||||
"remote".to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Url {
|
||||
url: "https://example.com/mcp".to_string(),
|
||||
},
|
||||
@@ -3585,7 +3594,7 @@ command = "python3 /enterprise/hooks/pre.py"
|
||||
PluginRequirementsToml {
|
||||
mcp_servers: Some(BTreeMap::from([(
|
||||
"sample".to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "sample-mcp".to_string(),
|
||||
},
|
||||
@@ -3600,6 +3609,71 @@ command = "python3 /enterprise/hooks/pre.py"
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserialize_plugin_mcp_server_matcher_requirement() -> Result<()> {
|
||||
let toml_str = r#"
|
||||
[plugins."sample@test".mcp_servers.internal_proxy]
|
||||
command = "company-cli"
|
||||
args = [
|
||||
{ match = "exact", value = "mcp" },
|
||||
{ match = "regex", expression = '^https://[a-z]+\.example\.com$' },
|
||||
]
|
||||
"#;
|
||||
let requirements: ConfigRequirements =
|
||||
with_unknown_source(from_str(toml_str)?).try_into()?;
|
||||
|
||||
assert_eq!(
|
||||
requirements.plugins,
|
||||
Some(Sourced::new(
|
||||
BTreeMap::from([(
|
||||
"sample@test".to_string(),
|
||||
PluginRequirementsToml {
|
||||
mcp_servers: Some(BTreeMap::from([(
|
||||
"internal_proxy".to_string(),
|
||||
McpServerRequirement::Command(McpServerCommandMatcher {
|
||||
command: "company-cli".to_string(),
|
||||
args: vec![
|
||||
McpServerValueMatcher::Exact {
|
||||
value: "mcp".to_string(),
|
||||
},
|
||||
McpServerValueMatcher::Regex {
|
||||
expression: r"^https://[a-z]+\.example\.com$".to_string(),
|
||||
},
|
||||
],
|
||||
}),
|
||||
)])),
|
||||
},
|
||||
)]),
|
||||
RequirementSource::Unknown,
|
||||
))
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_plugin_mcp_server_regex_reports_plugin_and_server_name() -> Result<()> {
|
||||
let toml_str = r#"
|
||||
[plugins."sample@test".mcp_servers.broken_rule]
|
||||
url = { match = "regex", expression = "[" }
|
||||
"#;
|
||||
|
||||
let err = ConfigRequirements::try_from(with_unknown_source(from_str(toml_str)?))
|
||||
.expect_err("invalid plugin MCP regex should fail requirements normalization");
|
||||
let ConstraintError::McpServerRequirementParse {
|
||||
server_name,
|
||||
requirement_source,
|
||||
reason,
|
||||
} = err
|
||||
else {
|
||||
panic!("unexpected error: {err:?}");
|
||||
};
|
||||
|
||||
assert_eq!(server_name, "sample@test/broken_rule");
|
||||
assert_eq!(requirement_source, RequirementSource::Unknown);
|
||||
assert!(reason.contains("invalid regex `[`"), "{reason}");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserialize_exec_policy_requirements() -> Result<()> {
|
||||
let toml_str = r#"
|
||||
|
||||
@@ -26,9 +26,9 @@ pub enum ConstraintError {
|
||||
},
|
||||
|
||||
#[error(
|
||||
"invalid matcher for MCP server `{server_name}` in requirements (set by {requirement_source}): {reason}"
|
||||
"invalid requirement for MCP server `{server_name}` (set by {requirement_source}): {reason}"
|
||||
)]
|
||||
McpServerMatcherParse {
|
||||
McpServerRequirementParse {
|
||||
server_name: String,
|
||||
requirement_source: RequirementSource,
|
||||
reason: String,
|
||||
|
||||
@@ -110,7 +110,6 @@ 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;
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
use crate::McpServerIdentity;
|
||||
use crate::McpServerRequirement;
|
||||
use crate::mcp_types::McpServerConfig;
|
||||
use crate::mcp_types::McpServerTransportConfig;
|
||||
use regex_lite::Regex;
|
||||
@@ -49,17 +51,10 @@ 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 {
|
||||
impl McpServerRequirement {
|
||||
pub(crate) fn validate(&self) -> Result<(), String> {
|
||||
match self {
|
||||
Self::Identity { .. } => Ok(()),
|
||||
Self::Command(matcher) => {
|
||||
for (index, arg) in matcher.args.iter().enumerate() {
|
||||
arg.validate().map_err(|err| {
|
||||
@@ -74,6 +69,24 @@ impl McpServerMatcher {
|
||||
|
||||
pub fn matches(&self, server: &McpServerConfig) -> bool {
|
||||
match (self, &server.transport) {
|
||||
(
|
||||
Self::Identity {
|
||||
identity:
|
||||
McpServerIdentity::Command {
|
||||
command: want_command,
|
||||
},
|
||||
},
|
||||
McpServerTransportConfig::Stdio {
|
||||
command: got_command,
|
||||
..
|
||||
},
|
||||
) => got_command == want_command,
|
||||
(
|
||||
Self::Identity {
|
||||
identity: McpServerIdentity::Url { url: want_url },
|
||||
},
|
||||
McpServerTransportConfig::StreamableHttp { url: got_url, .. },
|
||||
) => got_url == want_url,
|
||||
(Self::Command(matcher), McpServerTransportConfig::Stdio { command, args, .. }) => {
|
||||
matcher.command == *command
|
||||
&& matcher.args.len() == args.len()
|
||||
@@ -86,8 +99,7 @@ impl McpServerMatcher {
|
||||
(Self::Url(matcher), McpServerTransportConfig::StreamableHttp { url, .. }) => {
|
||||
matcher.url.matches(url)
|
||||
}
|
||||
(Self::Command(_), McpServerTransportConfig::StreamableHttp { .. })
|
||||
| (Self::Url(_), McpServerTransportConfig::Stdio { .. }) => false,
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -31,7 +31,7 @@ fn stdio_server(command: &str, args: &[&str]) -> McpServerConfig {
|
||||
|
||||
#[test]
|
||||
fn command_matcher_matches_exact_positional_arguments() {
|
||||
let matcher = McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
let requirement = McpServerRequirement::Command(McpServerCommandMatcher {
|
||||
command: "company-cli".to_string(),
|
||||
args: vec![
|
||||
McpServerValueMatcher::Exact {
|
||||
@@ -43,19 +43,19 @@ fn command_matcher_matches_exact_positional_arguments() {
|
||||
],
|
||||
});
|
||||
|
||||
assert!(matcher.matches(&stdio_server(
|
||||
assert!(requirement.matches(&stdio_server(
|
||||
"company-cli",
|
||||
&["mcp", "https://pricing.example.com"]
|
||||
)));
|
||||
assert!(!matcher.matches(&stdio_server(
|
||||
assert!(!requirement.matches(&stdio_server(
|
||||
"company-cli",
|
||||
&["https://pricing.example.com", "mcp"]
|
||||
)));
|
||||
assert!(!matcher.matches(&stdio_server(
|
||||
assert!(!requirement.matches(&stdio_server(
|
||||
"company-cli",
|
||||
&["mcp", "https://pricing.example.com", "--verbose"]
|
||||
)));
|
||||
assert!(!matcher.matches(&stdio_server(
|
||||
assert!(!requirement.matches(&stdio_server(
|
||||
"/usr/local/bin/company-cli",
|
||||
&["mcp", "https://pricing.example.com"]
|
||||
)));
|
||||
@@ -82,8 +82,25 @@ fn regex_matcher_allows_a_later_alternative_to_match_the_full_value() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn matcher_deserializes_command_and_url_shapes() {
|
||||
let command: McpServerMatcher = toml::from_str(
|
||||
fn legacy_command_identity_keeps_ignoring_arguments() {
|
||||
let requirement: McpServerRequirement = toml::from_str(
|
||||
r#"
|
||||
[identity]
|
||||
command = "company-cli"
|
||||
"#,
|
||||
)
|
||||
.expect("legacy command identity");
|
||||
|
||||
assert!(requirement.matches(&stdio_server(
|
||||
"company-cli",
|
||||
&["any", "arguments", "remain", "allowed"]
|
||||
)));
|
||||
assert!(!requirement.matches(&stdio_server("different-cli", &[])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn requirement_deserializes_command_and_url_matcher_shapes() {
|
||||
let command: McpServerRequirement = toml::from_str(
|
||||
r#"
|
||||
command = "company-cli"
|
||||
args = [
|
||||
@@ -93,7 +110,7 @@ args = [
|
||||
"#,
|
||||
)
|
||||
.expect("command matcher");
|
||||
let url: McpServerMatcher = toml::from_str(
|
||||
let url: McpServerRequirement = toml::from_str(
|
||||
r#"
|
||||
url = { match = "prefix", value = "https://mcp.example.com/" }
|
||||
"#,
|
||||
@@ -102,7 +119,7 @@ url = { match = "prefix", value = "https://mcp.example.com/" }
|
||||
|
||||
assert_eq!(
|
||||
command,
|
||||
McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
McpServerRequirement::Command(McpServerCommandMatcher {
|
||||
command: "company-cli".to_string(),
|
||||
args: vec![
|
||||
McpServerValueMatcher::Exact {
|
||||
@@ -116,7 +133,7 @@ url = { match = "prefix", value = "https://mcp.example.com/" }
|
||||
);
|
||||
assert_eq!(
|
||||
url,
|
||||
McpServerMatcher::Url(McpServerUrlMatcher {
|
||||
McpServerRequirement::Url(McpServerUrlMatcher {
|
||||
url: McpServerValueMatcher::Prefix {
|
||||
value: "https://mcp.example.com/".to_string(),
|
||||
},
|
||||
|
||||
@@ -10,6 +10,8 @@
|
||||
//! - `rules.prefix_rules` append high-priority rules first.
|
||||
//! - `hooks` append high-priority event groups first while failing closed on
|
||||
//! active managed-dir conflicts.
|
||||
//! - Incompatible definitions of the same named MCP server fail composition
|
||||
//! instead of silently combining legacy identities with matcher rules.
|
||||
//! - `permissions.filesystem.deny_read` is a high-priority-first union across
|
||||
//! layers.
|
||||
|
||||
@@ -19,6 +21,7 @@ use crate::RequirementSource;
|
||||
use crate::Sourced;
|
||||
use crate::merge::merge_toml_values;
|
||||
use std::cell::OnceCell;
|
||||
use std::collections::BTreeMap;
|
||||
use std::io;
|
||||
use thiserror::Error;
|
||||
use toml::Value as TomlValue;
|
||||
@@ -148,6 +151,7 @@ impl RequirementsLayerStack {
|
||||
hook_directory_field,
|
||||
} = self;
|
||||
|
||||
validate_mcp_server_requirement_shapes(&layers)?;
|
||||
let mut merged_toml = TomlValue::Table(toml::map::Map::new());
|
||||
for layer in &layers {
|
||||
merge_toml_values(&mut merged_toml, &layer.regular_toml);
|
||||
@@ -187,6 +191,122 @@ impl RequirementsLayerStack {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum McpServerRequirementShape {
|
||||
LegacyCommandIdentity,
|
||||
LegacyUrlIdentity,
|
||||
CommandMatcher,
|
||||
ExactUrlMatcher,
|
||||
PrefixUrlMatcher,
|
||||
RegexUrlMatcher,
|
||||
}
|
||||
|
||||
impl std::fmt::Display for McpServerRequirementShape {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
match self {
|
||||
Self::LegacyCommandIdentity => f.write_str("legacy command identity"),
|
||||
Self::LegacyUrlIdentity => f.write_str("legacy URL identity"),
|
||||
Self::CommandMatcher => f.write_str("command matcher"),
|
||||
Self::ExactUrlMatcher => f.write_str("exact URL matcher"),
|
||||
Self::PrefixUrlMatcher => f.write_str("prefix URL matcher"),
|
||||
Self::RegexUrlMatcher => f.write_str("regex URL matcher"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_mcp_server_requirement_shapes(
|
||||
layers: &[ComposableRequirementsLayer],
|
||||
) -> Result<(), RequirementsCompositionError> {
|
||||
let mut seen = BTreeMap::<String, (McpServerRequirementShape, RequirementSource)>::new();
|
||||
for layer in layers {
|
||||
if let Some(requirements) = table_at_path(&layer.regular_toml, &["mcp_servers"]) {
|
||||
validate_mcp_server_requirement_table(
|
||||
requirements,
|
||||
"mcp_servers",
|
||||
&layer.source,
|
||||
&mut seen,
|
||||
)?;
|
||||
}
|
||||
|
||||
let Some(plugins) = table_at_path(&layer.regular_toml, &["plugins"]) else {
|
||||
continue;
|
||||
};
|
||||
for plugin_name in plugins.keys() {
|
||||
let Some(requirements) = table_at_path(
|
||||
&layer.regular_toml,
|
||||
&["plugins", plugin_name, "mcp_servers"],
|
||||
) else {
|
||||
continue;
|
||||
};
|
||||
validate_mcp_server_requirement_table(
|
||||
requirements,
|
||||
&format!("plugins.\"{plugin_name}\".mcp_servers"),
|
||||
&layer.source,
|
||||
&mut seen,
|
||||
)?;
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_mcp_server_requirement_table(
|
||||
requirements: &toml::Table,
|
||||
table_path: &str,
|
||||
source: &RequirementSource,
|
||||
seen: &mut BTreeMap<String, (McpServerRequirementShape, RequirementSource)>,
|
||||
) -> Result<(), RequirementsCompositionError> {
|
||||
for (server_name, requirement) in requirements {
|
||||
let Some(shape) = mcp_server_requirement_shape(requirement) else {
|
||||
continue;
|
||||
};
|
||||
let field = format!("{table_path}.{server_name}");
|
||||
if let Some((existing_shape, existing_source)) = seen.get(&field)
|
||||
&& *existing_shape != shape
|
||||
{
|
||||
return Err(composition_conflict(
|
||||
field,
|
||||
existing_source.clone(),
|
||||
source.clone(),
|
||||
format!(
|
||||
"cannot combine {existing_shape} with {shape}; define this MCP server using one requirement form"
|
||||
),
|
||||
));
|
||||
}
|
||||
seen.insert(field, (shape, source.clone()));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn mcp_server_requirement_shape(value: &TomlValue) -> Option<McpServerRequirementShape> {
|
||||
let requirement = value.as_table()?;
|
||||
if let Some(identity) = requirement.get("identity").and_then(TomlValue::as_table) {
|
||||
if identity.contains_key("command") {
|
||||
return Some(McpServerRequirementShape::LegacyCommandIdentity);
|
||||
}
|
||||
if identity.contains_key("url") {
|
||||
return Some(McpServerRequirementShape::LegacyUrlIdentity);
|
||||
}
|
||||
return None;
|
||||
}
|
||||
if requirement.contains_key("command") {
|
||||
return Some(McpServerRequirementShape::CommandMatcher);
|
||||
}
|
||||
let matcher = requirement.get("url")?.as_table()?;
|
||||
match matcher.get("match")?.as_str()? {
|
||||
"exact" => Some(McpServerRequirementShape::ExactUrlMatcher),
|
||||
"prefix" => Some(McpServerRequirementShape::PrefixUrlMatcher),
|
||||
"regex" => Some(McpServerRequirementShape::RegexUrlMatcher),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn table_at_path<'a>(mut value: &'a TomlValue, path: &[&str]) -> Option<&'a toml::Table> {
|
||||
for key in path {
|
||||
value = value.as_table()?.get(*key)?;
|
||||
}
|
||||
value.as_table()
|
||||
}
|
||||
|
||||
fn populate_merged_regular_fields_with_sources(
|
||||
output: &mut ConfigRequirementsWithSources,
|
||||
requirements: ConfigRequirementsToml,
|
||||
@@ -221,7 +341,6 @@ fn populate_merged_regular_fields_with_sources(
|
||||
feature_requirements,
|
||||
hooks: _,
|
||||
mcp_servers,
|
||||
mcp_server_matchers,
|
||||
plugins,
|
||||
apps,
|
||||
rules: _,
|
||||
@@ -250,7 +369,6 @@ 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"]);
|
||||
|
||||
@@ -390,43 +390,65 @@ command = "high-mcp"
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_server_matchers_use_regular_toml_merge() {
|
||||
let composed = compose(vec![
|
||||
fn mcp_requirements_reject_mixed_forms_across_layers() {
|
||||
let err = 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" }
|
||||
[mcp_servers.shared.identity]
|
||||
command = "low-mcp"
|
||||
"#,
|
||||
),
|
||||
layer(
|
||||
"req_high",
|
||||
"High",
|
||||
r#"
|
||||
[mcp_server_matchers.shared]
|
||||
url = { match = "prefix", value = "https://high.example.com/" }
|
||||
[mcp_servers.shared]
|
||||
command = "company-cli"
|
||||
args = [{ match = "exact", value = "mcp" }]
|
||||
"#,
|
||||
),
|
||||
])
|
||||
.expect("compose requirements")
|
||||
.expect("requirements present");
|
||||
.expect_err("mixed MCP requirement forms should fail composition");
|
||||
|
||||
assert_eq!(
|
||||
composed,
|
||||
expected_requirements(
|
||||
assert!(err.to_string().contains("mcp_servers.shared"));
|
||||
assert!(err.to_string().contains("legacy command identity"));
|
||||
assert!(err.to_string().contains("command matcher"));
|
||||
assert!(err.to_string().contains("High (req_high)"));
|
||||
assert!(err.to_string().contains("Low (req_low)"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plugin_mcp_requirements_reject_mixed_forms_across_layers() {
|
||||
let err = compose(vec![
|
||||
layer(
|
||||
"req_low",
|
||||
"Low",
|
||||
r#"
|
||||
[mcp_server_matchers.low]
|
||||
url = { match = "exact", value = "https://low.example.com/mcp" }
|
||||
[plugins."sample@test".mcp_servers.shared.identity]
|
||||
command = "low-plugin-mcp"
|
||||
"#,
|
||||
),
|
||||
layer(
|
||||
"req_high",
|
||||
"High",
|
||||
r#"
|
||||
[plugins."sample@test".mcp_servers.shared]
|
||||
url = { match = "regex", expression = '^https://high\.example\.com/mcp$' }
|
||||
"#,
|
||||
),
|
||||
])
|
||||
.expect_err("mixed plugin MCP requirement forms should fail composition");
|
||||
|
||||
[mcp_server_matchers.shared]
|
||||
url = { match = "prefix", value = "https://high.example.com/" }
|
||||
"#
|
||||
)
|
||||
assert!(
|
||||
err.to_string()
|
||||
.contains("plugins.\"sample@test\".mcp_servers.shared")
|
||||
);
|
||||
assert!(err.to_string().contains("legacy command identity"));
|
||||
assert!(err.to_string().contains("regex URL matcher"));
|
||||
assert!(err.to_string().contains("High (req_high)"));
|
||||
assert!(err.to_string().contains("Low (req_low)"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -6,7 +6,8 @@ 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::McpServerIdentity;
|
||||
use codex_config::McpServerRequirement;
|
||||
use codex_config::McpServerValueMatcher;
|
||||
use codex_config::ProfileV2Name;
|
||||
use codex_config::RequirementSource;
|
||||
@@ -4146,7 +4147,7 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() {
|
||||
BTreeMap::from([
|
||||
(
|
||||
MISMATCHED_URL_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Url {
|
||||
url: "https://example.com/other".to_string(),
|
||||
},
|
||||
@@ -4154,7 +4155,7 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() {
|
||||
),
|
||||
(
|
||||
MISMATCHED_COMMAND_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "other-cmd".to_string(),
|
||||
},
|
||||
@@ -4162,7 +4163,7 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() {
|
||||
),
|
||||
(
|
||||
MATCHED_URL_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Url {
|
||||
url: GOOD_URL.to_string(),
|
||||
},
|
||||
@@ -4170,7 +4171,7 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() {
|
||||
),
|
||||
(
|
||||
MATCHED_COMMAND_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: GOOD_CMD.to_string(),
|
||||
},
|
||||
@@ -4179,11 +4180,7 @@ fn filter_mcp_servers_by_allowlist_enforces_identity_rules() {
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
filter_mcp_servers_by_requirements(
|
||||
&mut servers,
|
||||
Some(&requirements),
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
filter_mcp_servers_by_requirements(&mut servers, Some(&requirements));
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
@@ -4214,11 +4211,7 @@ 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,
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
filter_mcp_servers_by_requirements(&mut servers, /*mcp_requirements*/ None);
|
||||
|
||||
assert_eq!(
|
||||
servers
|
||||
@@ -4296,7 +4289,7 @@ fn filter_mcp_servers_by_matchers_enforces_command_and_positional_args() {
|
||||
),
|
||||
]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let matcher = McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
let requirement = McpServerRequirement::Command(McpServerCommandMatcher {
|
||||
command: "company-cli".to_string(),
|
||||
args: vec![
|
||||
McpServerValueMatcher::Exact {
|
||||
@@ -4315,21 +4308,17 @@ fn filter_mcp_servers_by_matchers_enforces_command_and_positional_args() {
|
||||
},
|
||||
],
|
||||
});
|
||||
let matchers = Sourced::new(
|
||||
let requirements = 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),
|
||||
("internal_mcp_proxy".to_string(), requirement.clone()),
|
||||
("wrong-order".to_string(), requirement.clone()),
|
||||
("trailing-arg".to_string(), requirement.clone()),
|
||||
("wrong-host".to_string(), requirement),
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_mcp_servers_by_requirements(
|
||||
&mut servers,
|
||||
/*mcp_requirements*/ None,
|
||||
Some(&matchers),
|
||||
);
|
||||
filter_mcp_servers_by_requirements(&mut servers, Some(&requirements));
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
@@ -4350,90 +4339,6 @@ fn filter_mcp_servers_by_matchers_enforces_command_and_positional_args() {
|
||||
);
|
||||
}
|
||||
|
||||
#[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([
|
||||
@@ -4443,11 +4348,7 @@ 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),
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
filter_mcp_servers_by_requirements(&mut servers, Some(&requirements));
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
@@ -4493,66 +4394,31 @@ fn filter_plugin_mcp_servers_by_allowlist_enforces_plugin_and_matcher_rules() {
|
||||
mcp_servers: Some(BTreeMap::from([
|
||||
(
|
||||
MATCHED_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: GOOD_CMD.to_string(),
|
||||
},
|
||||
},
|
||||
McpServerRequirement::Command(McpServerCommandMatcher {
|
||||
command: GOOD_CMD.to_string(),
|
||||
args: Vec::new(),
|
||||
}),
|
||||
),
|
||||
(
|
||||
MISMATCHED_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: GOOD_CMD.to_string(),
|
||||
},
|
||||
},
|
||||
McpServerRequirement::Command(McpServerCommandMatcher {
|
||||
command: GOOD_CMD.to_string(),
|
||||
args: Vec::new(),
|
||||
}),
|
||||
),
|
||||
(
|
||||
MATCHER_MISMATCHED_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: GOOD_CMD.to_string(),
|
||||
},
|
||||
},
|
||||
McpServerRequirement::Command(McpServerCommandMatcher {
|
||||
command: GOOD_CMD.to_string(),
|
||||
args: Vec::new(),
|
||||
}),
|
||||
),
|
||||
])),
|
||||
},
|
||||
)]),
|
||||
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),
|
||||
);
|
||||
filter_plugin_mcp_servers_by_requirements("sample@test", &mut servers, Some(&requirements));
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
@@ -4585,7 +4451,7 @@ fn filter_plugin_mcp_servers_by_allowlist_blocks_unlisted_plugin() {
|
||||
codex_config::PluginRequirementsToml {
|
||||
mcp_servers: Some(BTreeMap::from([(
|
||||
"server-a".to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "cmd-a".to_string(),
|
||||
},
|
||||
@@ -4596,12 +4462,7 @@ fn filter_plugin_mcp_servers_by_allowlist_blocks_unlisted_plugin() {
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_plugin_mcp_servers_by_requirements(
|
||||
"sample@test",
|
||||
&mut servers,
|
||||
Some(&requirements),
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
filter_plugin_mcp_servers_by_requirements("sample@test", &mut servers, Some(&requirements));
|
||||
|
||||
assert_eq!(
|
||||
servers
|
||||
@@ -4642,26 +4503,26 @@ fn filter_plugin_mcp_servers_by_matchers_enforces_name_and_invocation() {
|
||||
),
|
||||
]);
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let matcher = McpServerMatcher::Command(McpServerCommandMatcher {
|
||||
let requirement = McpServerRequirement::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),
|
||||
]),
|
||||
let requirements = Sourced::new(
|
||||
BTreeMap::from([(
|
||||
"sample@test".to_string(),
|
||||
codex_config::PluginRequirementsToml {
|
||||
mcp_servers: Some(BTreeMap::from([
|
||||
(MATCHED_SERVER.to_string(), requirement.clone()),
|
||||
(MISMATCHED_SERVER.to_string(), requirement),
|
||||
])),
|
||||
},
|
||||
)]),
|
||||
source.clone(),
|
||||
);
|
||||
|
||||
filter_plugin_mcp_servers_by_requirements(
|
||||
"sample@test",
|
||||
&mut servers,
|
||||
/*plugin_requirements*/ None,
|
||||
Some(&matchers),
|
||||
);
|
||||
filter_plugin_mcp_servers_by_requirements("sample@test", &mut servers, Some(&requirements));
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
@@ -4689,7 +4550,7 @@ async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io::
|
||||
let mcp_requirements = BTreeMap::from([
|
||||
(
|
||||
"session_overrides_user".to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "session-command".to_string(),
|
||||
},
|
||||
@@ -4697,7 +4558,7 @@ async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io::
|
||||
),
|
||||
(
|
||||
"managed_overrides_session".to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "managed-command".to_string(),
|
||||
},
|
||||
@@ -4705,7 +4566,7 @@ async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io::
|
||||
),
|
||||
(
|
||||
"fresh_global".to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "fresh-global-command".to_string(),
|
||||
},
|
||||
@@ -4713,7 +4574,7 @@ async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io::
|
||||
),
|
||||
(
|
||||
"fresh_project".to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "fresh-project-command".to_string(),
|
||||
},
|
||||
@@ -9008,7 +8869,6 @@ 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,
|
||||
|
||||
@@ -14,12 +14,9 @@ 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;
|
||||
@@ -42,7 +39,6 @@ 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;
|
||||
@@ -1489,22 +1485,18 @@ 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,
|
||||
requirements.plugins.as_ref(),
|
||||
requirements.mcp_server_matchers.as_ref(),
|
||||
self.config_layer_stack.requirements().plugins.as_ref(),
|
||||
);
|
||||
let empty_mcp_allowlist = requirements
|
||||
let empty_mcp_allowlist = self
|
||||
.config_layer_stack
|
||||
.requirements()
|
||||
.mcp_servers
|
||||
.as_ref()
|
||||
.filter(|requirements| requirements.value.is_empty());
|
||||
filter_mcp_servers_by_requirements(
|
||||
mcp_servers,
|
||||
empty_mcp_allowlist,
|
||||
/*mcp_matchers*/ None,
|
||||
);
|
||||
filter_mcp_servers_by_requirements(mcp_servers, empty_mcp_allowlist);
|
||||
}
|
||||
|
||||
pub async fn to_mcp_config(
|
||||
@@ -1874,30 +1866,17 @@ 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>>>,
|
||||
) {
|
||||
if mcp_requirements.is_none() && mcp_matchers.is_none() {
|
||||
let Some(allowlist) = mcp_requirements else {
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
let source = RequirementSource::composite(
|
||||
mcp_requirements
|
||||
.iter()
|
||||
.map(|requirements| requirements.source.clone())
|
||||
.chain(
|
||||
mcp_matchers
|
||||
.iter()
|
||||
.map(|requirements| requirements.source.clone()),
|
||||
),
|
||||
);
|
||||
let source = allowlist.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));
|
||||
let allowed = allowlist
|
||||
.value
|
||||
.get(name)
|
||||
.is_some_and(|requirement| requirement.matches(server));
|
||||
if allowed {
|
||||
server.disabled_reason = None;
|
||||
} else {
|
||||
@@ -1913,38 +1892,20 @@ 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() {
|
||||
let Some(requirements) = plugin_requirements else {
|
||||
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))
|
||||
};
|
||||
let source = requirements.source.clone();
|
||||
let plugin_mcp_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;
|
||||
let allowed = plugin_mcp_requirements
|
||||
.and_then(|mcp_requirements| mcp_requirements.get(name))
|
||||
.is_some_and(|requirement| requirement.matches(server));
|
||||
if allowed {
|
||||
server.disabled_reason = None;
|
||||
} else {
|
||||
@@ -1959,20 +1920,14 @@ 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() && mcp_matchers.is_none() {
|
||||
if mcp_requirements.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(),
|
||||
);
|
||||
filter_mcp_servers_by_requirements(&mut servers, mcp_requirements.as_ref());
|
||||
servers
|
||||
})
|
||||
}
|
||||
@@ -2013,26 +1968,6 @@ 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>> {
|
||||
@@ -2982,7 +2917,6 @@ impl Config {
|
||||
feature_requirements,
|
||||
managed_hooks: _,
|
||||
mcp_servers,
|
||||
mcp_server_matchers,
|
||||
plugins: _,
|
||||
exec_policy: _,
|
||||
enforce_residency,
|
||||
@@ -3738,12 +3672,8 @@ impl Config {
|
||||
&mut startup_warnings,
|
||||
)?;
|
||||
|
||||
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 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 network_permission_profile = constrained_permission_profile.get().clone();
|
||||
let network = build_network_proxy_spec(
|
||||
|
||||
@@ -699,7 +699,7 @@ mod tests {
|
||||
mcp_servers: Some(Sourced::new(
|
||||
BTreeMap::from([(
|
||||
"docs".to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "codex-mcp".to_string(),
|
||||
},
|
||||
@@ -778,13 +778,12 @@ mod tests {
|
||||
hooks: None,
|
||||
mcp_servers: Some(BTreeMap::from([(
|
||||
"docs".to_string(),
|
||||
McpServerRequirement {
|
||||
McpServerRequirement::Identity {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "codex-mcp".to_string(),
|
||||
},
|
||||
},
|
||||
)])),
|
||||
mcp_server_matchers: None,
|
||||
plugins: None,
|
||||
apps: None,
|
||||
rules: None,
|
||||
@@ -1142,7 +1141,6 @@ approval_policy = "never"
|
||||
feature_requirements: None,
|
||||
hooks: None,
|
||||
mcp_servers: None,
|
||||
mcp_server_matchers: None,
|
||||
plugins: None,
|
||||
apps: None,
|
||||
rules: None,
|
||||
|
||||
Reference in New Issue
Block a user