diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index 2566c21266..42df9aea9d 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -23,9 +23,11 @@ use crate::Constrained; use crate::ConstraintError; use crate::ManagedAuthPolicy; use crate::ManagedHooksRequirementsToml; +use crate::McpServerRequirement; +use crate::PluginRequirementsToml; use crate::RequirementsExecPolicy; use crate::config_toml::ConfigToml; -use crate::mcp_requirements::McpServerRequirement; +use crate::mcp_requirements::validate_mcp_server_requirement; use crate::mcp_types::AppToolApproval; use crate::permissions_toml::PermissionProfileToml; use crate::types::FeedbackConfigToml; @@ -286,11 +288,6 @@ impl ConfigRequirements { } } -#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)] -pub struct PluginRequirementsToml { - pub mcp_servers: Option>, -} - #[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)] #[serde(deny_unknown_fields)] pub struct MarketplaceRequirementsToml { @@ -326,12 +323,6 @@ pub enum MarketplaceAllowedSourceKind { Local, } -impl PluginRequirementsToml { - pub fn is_empty(&self) -> bool { - self.mcp_servers.as_ref().is_none_or(BTreeMap::is_empty) - } -} - #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq)] pub struct NetworkDomainPermissionsToml { #[serde(flatten)] @@ -1516,15 +1507,15 @@ fn validate_mcp_server_requirements( plugin_name: Option<&str>, ) -> Result<(), ConstraintError> { for (server_name, requirement) in requirements { - requirement - .validate() - .map_err(|reason| ConstraintError::McpServerRequirementParse { + validate_mcp_server_requirement(requirement).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(()) } diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index 5baef30168..d8ad0f49ec 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -52,6 +52,11 @@ pub use codex_execpolicy::RequirementsExecPolicy; pub use codex_protocol::config_types::ProfileV2Name; pub use codex_protocol::config_types::ProfileV2NameParseError; pub use codex_protocol::config_types::ToolExposureSurface; +pub use codex_protocol::mcp_policy::McpServerCommandMatcher; +pub use codex_protocol::mcp_policy::McpServerIdentity; +pub use codex_protocol::mcp_policy::McpServerRequirement; +pub use codex_protocol::mcp_policy::McpServerValueMatcher; +pub use codex_protocol::mcp_policy::PluginMcpRequirements as PluginRequirementsToml; pub use codex_utils_absolute_path::AbsolutePathBuf; pub use codex_utils_absolute_path::AbsolutePathBufGuard; pub use config_layer_source::ConfigLayer; @@ -83,7 +88,6 @@ pub use config_requirements::NetworkRequirementsToml; pub use config_requirements::NetworkUnixSocketPermissionToml; pub use config_requirements::NetworkUnixSocketPermissionsToml; pub use config_requirements::NewThreadModelDefaultsToml; -pub use config_requirements::PluginRequirementsToml; pub use config_requirements::RemoteSandboxConfigToml; pub use config_requirements::RequirementSource; pub use config_requirements::ResidencyRequirement; @@ -121,10 +125,6 @@ pub use marketplace_edit::record_user_marketplace; pub use marketplace_edit::remove_user_marketplace; pub use marketplace_edit::remove_user_marketplace_config; pub use mcp_edit::load_global_mcp_servers; -pub use mcp_requirements::McpServerCommandMatcher; -pub use mcp_requirements::McpServerIdentity; -pub use mcp_requirements::McpServerRequirement; -pub use mcp_requirements::McpServerValueMatcher; pub use mcp_types::AppToolApproval; pub use mcp_types::DEFAULT_MCP_SERVER_ENVIRONMENT_ID; pub use mcp_types::McpServerAuth; diff --git a/codex-rs/config/src/mcp_requirements.rs b/codex-rs/config/src/mcp_requirements.rs index 44ef2df2ca..acdcc354e4 100644 --- a/codex-rs/config/src/mcp_requirements.rs +++ b/codex-rs/config/src/mcp_requirements.rs @@ -1,131 +1,57 @@ +use crate::McpServerIdentity; +use crate::McpServerRequirement; +use crate::McpServerValueMatcher; use crate::mcp_types::McpServerConfig; use crate::mcp_types::McpServerTransportConfig; use regex_lite::Regex; -use serde::Deserialize; -#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] -#[serde(untagged)] -pub enum McpServerIdentity { - Command { command: String }, - Url { url: String }, +fn compile_full_regex(expression: &str) -> Result { + Regex::new(&format!(r"\A(?:{expression})\z")).map_err(|err| { + format!("regex `{expression}` cannot be used for full-value matching: {err}") + }) } -/// 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 }, +fn validate_value_matcher(matcher: &McpServerValueMatcher) -> Result<(), String> { + let McpServerValueMatcher::Regex { expression } = matcher else { + return Ok(()); + }; + + Regex::new(expression).map_err(|err| format!("invalid regex `{expression}`: {err}"))?; + compile_full_regex(expression).map(|_| ()) } -impl McpServerValueMatcher { - fn compile_full_regex(expression: &str) -> Result { - Regex::new(&format!(r"\A(?:{expression})\z")).map_err(|err| { - format!("regex `{expression}` cannot be used for full-value matching: {err}") - }) - } - - fn validate(&self) -> Result<(), String> { - let Self::Regex { expression } = self else { - return Ok(()); - }; - - Regex::new(expression).map_err(|err| format!("invalid regex `{expression}`: {err}"))?; - Self::compile_full_regex(expression).map(|_| ()) - } - - fn matches(&self, candidate: &str) -> bool { - match self { - Self::Exact { value } => candidate == value, - Self::Prefix { value } => candidate.starts_with(value), - Self::Regex { expression } => Self::compile_full_regex(expression) - .ok() - .is_some_and(|regex| regex.is_match(candidate)), - } +fn matches_value(matcher: &McpServerValueMatcher, candidate: &str) -> bool { + match matcher { + McpServerValueMatcher::Exact { value } => candidate == value, + McpServerValueMatcher::Prefix { value } => candidate.starts_with(value), + McpServerValueMatcher::Regex { expression } => compile_full_regex(expression) + .ok() + .is_some_and(|regex| regex.is_match(candidate)), } } -#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] -#[serde(deny_unknown_fields)] -pub struct McpServerCommandMatcher { - pub executable: String, - pub args: Vec, -} - -#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] -#[serde(deny_unknown_fields)] -struct RawMcpServerCommandIdentity { - command: McpServerCommandMatcher, -} - -#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] -#[serde(deny_unknown_fields)] -struct RawMcpServerUrlIdentity { - url: McpServerValueMatcher, -} - -/// A requirement for one named MCP server. -/// -/// The `Identity` variant preserves the released exact-match contract. The -/// command and URL variants are the normalized matcher-based forms accepted -/// under the `identity` key. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum McpServerRequirement { - Identity { identity: McpServerIdentity }, - Command(McpServerCommandMatcher), - Url(McpServerValueMatcher), -} - -#[derive(Deserialize)] -struct RawMcpServerRequirement { - identity: RawMcpServerIdentity, -} - -#[derive(Deserialize)] -#[serde(untagged)] -enum RawMcpServerIdentity { - Exact(McpServerIdentity), - Command(RawMcpServerCommandIdentity), - Url(RawMcpServerUrlIdentity), -} - -impl<'de> Deserialize<'de> for McpServerRequirement { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - let RawMcpServerRequirement { identity } = - RawMcpServerRequirement::deserialize(deserializer)?; - match identity { - RawMcpServerIdentity::Exact(identity) => Ok(Self::Identity { identity }), - RawMcpServerIdentity::Command(matcher) => Ok(Self::Command(matcher.command)), - RawMcpServerIdentity::Url(matcher) => Ok(Self::Url(matcher.url)), - } - } -} - -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| { - format!("invalid argument matcher at index {index}: {err}") - })?; - } - Ok(()) +pub(crate) fn validate_mcp_server_requirement( + requirement: &McpServerRequirement, +) -> Result<(), String> { + match requirement { + McpServerRequirement::Identity { .. } => Ok(()), + McpServerRequirement::Command(matcher) => { + for (index, arg) in matcher.args.iter().enumerate() { + validate_value_matcher(arg) + .map_err(|err| format!("invalid argument matcher at index {index}: {err}"))?; } - Self::Url(matcher) => matcher.validate(), + Ok(()) } + McpServerRequirement::Url(matcher) => validate_value_matcher(matcher), } +} - pub fn matches(&self, server: &McpServerConfig) -> bool { +impl McpServerConfig { + pub fn matches_requirement(&self, requirement: &McpServerRequirement) -> bool { // HTTP requirements intentionally authorize the complete server configuration by URL. - match (self, &server.transport) { + match (requirement, &self.transport) { ( - Self::Identity { + McpServerRequirement::Identity { identity: McpServerIdentity::Command { command: want_command, @@ -137,23 +63,27 @@ impl McpServerRequirement { }, ) => got_command == want_command, ( - Self::Identity { + McpServerRequirement::Identity { identity: McpServerIdentity::Url { url: want_url }, }, McpServerTransportConfig::StreamableHttp { url: got_url, .. }, ) => got_url == want_url, - (Self::Command(matcher), McpServerTransportConfig::Stdio { command, args, .. }) => { + ( + McpServerRequirement::Command(matcher), + McpServerTransportConfig::Stdio { command, args, .. }, + ) => { matcher.executable == *command && matcher.args.len() == args.len() && matcher .args .iter() .zip(args) - .all(|(matcher, arg)| matcher.matches(arg)) - } - (Self::Url(matcher), McpServerTransportConfig::StreamableHttp { url, .. }) => { - matcher.matches(url) + .all(|(matcher, arg)| matches_value(matcher, arg)) } + ( + McpServerRequirement::Url(matcher), + McpServerTransportConfig::StreamableHttp { url, .. }, + ) => matches_value(matcher, url), _ => false, } } diff --git a/codex-rs/config/src/mcp_requirements_tests.rs b/codex-rs/config/src/mcp_requirements_tests.rs index 7fcf14b477..8ce0c546e2 100644 --- a/codex-rs/config/src/mcp_requirements_tests.rs +++ b/codex-rs/config/src/mcp_requirements_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::McpServerCommandMatcher; use crate::mcp_types::McpServerConfig; use pretty_assertions::assert_eq; use std::collections::HashMap; @@ -45,22 +46,28 @@ fn command_matcher_matches_exact_positional_arguments() { ], }); - assert!(requirement.matches(&stdio_server( - "company-cli", - &["mcp", "https://pricing.example.com"] - ))); - assert!(!requirement.matches(&stdio_server( - "company-cli", - &["https://pricing.example.com", "mcp"] - ))); - assert!(!requirement.matches(&stdio_server( - "company-cli", - &["mcp", "https://pricing.example.com", "--verbose"] - ))); - assert!(!requirement.matches(&stdio_server( - "/usr/local/bin/company-cli", - &["mcp", "https://pricing.example.com"] - ))); + assert!( + stdio_server("company-cli", &["mcp", "https://pricing.example.com"]) + .matches_requirement(&requirement) + ); + assert!( + !stdio_server("company-cli", &["https://pricing.example.com", "mcp"]) + .matches_requirement(&requirement) + ); + assert!( + !stdio_server( + "company-cli", + &["mcp", "https://pricing.example.com", "--verbose"] + ) + .matches_requirement(&requirement) + ); + assert!( + !stdio_server( + "/usr/local/bin/company-cli", + &["mcp", "https://pricing.example.com"] + ) + .matches_requirement(&requirement) + ); } #[test] @@ -69,9 +76,9 @@ fn regex_matcher_requires_a_full_value_match() { expression: "mcp".to_string(), }; - assert!(matcher.matches("mcp")); - assert!(!matcher.matches("mcp-proxy")); - assert!(!matcher.matches("prefix-mcp")); + assert!(matches_value(&matcher, "mcp")); + assert!(!matches_value(&matcher, "mcp-proxy")); + assert!(!matches_value(&matcher, "prefix-mcp")); } #[test] @@ -80,7 +87,7 @@ fn regex_matcher_allows_a_later_alternative_to_match_the_full_value() { expression: r"https://api\.example\.com|https://api\.example\.com/mcp".to_string(), }; - assert!(matcher.matches("https://api.example.com/mcp")); + assert!(matches_value(&matcher, "https://api.example.com/mcp")); } #[test] @@ -89,8 +96,7 @@ fn regex_matcher_validation_rejects_expression_that_cannot_be_wrapped() { expression: "(?x)mcp # trailing comment".to_string(), }; - let err = matcher - .validate() + let err = validate_value_matcher(&matcher) .expect_err("expression should not be valid for full-value matching"); assert!( err.contains("cannot be used for full-value matching"), @@ -108,11 +114,11 @@ 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", &[]))); + assert!( + stdio_server("company-cli", &["any", "arguments", "remain", "allowed"]) + .matches_requirement(&requirement) + ); + assert!(!stdio_server("different-cli", &[]).matches_requirement(&requirement)); } #[test] diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index d7716076c8..f7985c8870 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2018,7 +2018,7 @@ fn filter_mcp_servers_by_requirements( let allowed = allowlist .value .get(name) - .is_some_and(|requirement| requirement.matches(server)); + .is_some_and(|requirement| server.matches_requirement(requirement)); if allowed { server.disabled_reason = None; } else { @@ -2054,7 +2054,7 @@ fn filter_plugin_mcp_servers_by_requirements( for (name, server) in mcp_servers.iter_mut() { let allowed = plugin_mcp_requirements .and_then(|mcp_requirements| mcp_requirements.get(name)) - .is_some_and(|requirement| requirement.matches(server)); + .is_some_and(|requirement| server.matches_requirement(requirement)); if allowed { server.disabled_reason = None; } else { diff --git a/codex-rs/protocol/src/lib.rs b/codex-rs/protocol/src/lib.rs index 5608f5d8ea..64993ec903 100644 --- a/codex-rs/protocol/src/lib.rs +++ b/codex-rs/protocol/src/lib.rs @@ -24,6 +24,7 @@ mod legacy_events; pub mod local_media; pub mod mcp; pub mod mcp_approval_meta; +pub mod mcp_policy; pub mod memory_citation; pub mod models; pub mod network_policy; diff --git a/codex-rs/protocol/src/mcp_policy.rs b/codex-rs/protocol/src/mcp_policy.rs new file mode 100644 index 0000000000..cfbaf94be2 --- /dev/null +++ b/codex-rs/protocol/src/mcp_policy.rs @@ -0,0 +1,89 @@ +use serde::Deserialize; +use std::collections::BTreeMap; + +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(untagged)] +pub enum McpServerIdentity { + Command { command: String }, + Url { url: String }, +} + +/// 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 }, +} + +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub struct McpServerCommandMatcher { + pub executable: String, + pub args: Vec, +} + +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +struct RawMcpServerCommandIdentity { + command: McpServerCommandMatcher, +} + +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +struct RawMcpServerUrlIdentity { + url: McpServerValueMatcher, +} + +/// A requirement for one named MCP server. +/// +/// The `Identity` variant preserves the released exact-match contract. The +/// command and URL variants are the normalized matcher-based forms accepted +/// under the `identity` key. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum McpServerRequirement { + Identity { identity: McpServerIdentity }, + Command(McpServerCommandMatcher), + Url(McpServerValueMatcher), +} + +#[derive(Deserialize)] +struct RawMcpServerRequirement { + identity: RawMcpServerIdentity, +} + +#[derive(Deserialize)] +#[serde(untagged)] +enum RawMcpServerIdentity { + Exact(McpServerIdentity), + Command(RawMcpServerCommandIdentity), + Url(RawMcpServerUrlIdentity), +} + +impl<'de> Deserialize<'de> for McpServerRequirement { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let RawMcpServerRequirement { identity } = + RawMcpServerRequirement::deserialize(deserializer)?; + match identity { + RawMcpServerIdentity::Exact(identity) => Ok(Self::Identity { identity }), + RawMcpServerIdentity::Command(matcher) => Ok(Self::Command(matcher.command)), + RawMcpServerIdentity::Url(matcher) => Ok(Self::Url(matcher.url)), + } + } +} + +/// Managed MCP server requirements for one plugin. +#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)] +pub struct PluginMcpRequirements { + pub mcp_servers: Option>, +} + +impl PluginMcpRequirements { + pub fn is_empty(&self) -> bool { + self.mcp_servers.as_ref().is_none_or(BTreeMap::is_empty) + } +}