Skip to content

Commit db541f4

Browse files
authored
[codex] Add managed MCP server matchers (#29648)
## Summary This PR extends the existing managed `mcp_servers` identity requirement so that one name-qualified rule can use either: - the released exact command or URL identity; - an exact stdio executable with an exact-length, ordered argument matcher list; or - a direct MCP URL matcher. Matcher-based rules stay under the released `identity` key and use the same `McpServerRequirement` abstraction and `mcp_servers.<server_name>` namespace. ## Behavior Policy activation and name qualification are unchanged: - If `mcp_servers` is absent, ordinary configured MCP servers remain unrestricted. - If `mcp_servers` is present, a server needs a matching same-name requirement. - `mcp_servers = {}` continues to deny every configured MCP server. - Existing exact identity requirements keep their released semantics. Plugin-bundled MCP servers use the same requirement shapes under `plugins.<plugin_name>.mcp_servers.<server_name>`. Top-level non-empty rules continue to govern only ordinary configured servers; plugin rules remain explicitly plugin-scoped. The existing globally empty `mcp_servers = {}` plugin kill switch is preserved. Requirements layers continue to use the existing regular TOML merge behavior. Atomic replacement of named MCP requirements is intentionally out of scope here and is tracked independently in #30118. ## Requirement contract The released exact identity contract remains valid: ```toml [mcp_servers.docs.identity] command = "codex-mcp" [mcp_servers.remote.identity] url = "https://example.com/mcp" ``` Command identities continue to check only `command`; they do not inspect arguments, `cwd`, `env`, or `env_vars`. A command matcher uses an exact executable plus an exact-length, ordered argument list. Each argument position supports `exact`, `prefix`, or full-value `regex` matching: ```toml [mcp_servers.internal_mcp_proxy.identity] command = { executable = "company-cli", 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)?(?:/.*)?$' }, ] } ``` Direct streamable HTTP MCP definitions can use the same value matcher types through `identity.url`: ```toml [mcp_servers.internal_http.identity] url = { match = "regex", expression = '^https://[A-Za-z0-9-]+\.mcp\.internal\.example\.com(?:/.*)?$', } ``` Plugin-bundled MCP matchers use the same contract inside the plugin-qualified allowlist: ```toml [plugins."sample@test".mcp_servers.internal_mcp_proxy.identity] command = { executable = "company-cli", args = [ { match = "exact", value = "mcp" }, { match = "exact", value = "proxy" }, ] } ``` Regexes are validated while managed requirements are loaded, and regex matching must cover the complete value. Command matchers constrain only the executable and arguments. ## Why Enterprise administrators need to allow MCP servers by executable and positional-argument shape, including fixed arguments plus constrained values such as internal MCP URLs passed to a proxy. ## Validation - `just fmt` - `git diff --check` - `just test -p codex-config` (198 passed) - `just test -p codex-core mcp_servers_by_matchers --lib` (2 passed)
1 parent e23e7cb commit db541f4

10 files changed

Lines changed: 771 additions & 56 deletions

File tree

codex-rs/Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

codex-rs/config/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ gethostname = { workspace = true }
3030
indexmap = { workspace = true, features = ["serde"] }
3131
multimap = { workspace = true }
3232
prost = "0.14.3"
33+
regex-lite = { workspace = true }
3334
schemars = { workspace = true }
3435
serde = { workspace = true, features = ["derive"] }
3536
serde_ignored = { workspace = true }

codex-rs/config/src/config_requirements.rs

Lines changed: 181 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use super::requirements_exec_policy::RequirementsExecPolicyToml;
1919
use crate::Constrained;
2020
use crate::ConstraintError;
2121
use crate::ManagedHooksRequirementsToml;
22+
use crate::mcp_requirements::McpServerRequirement;
2223
use crate::mcp_types::AppToolApproval;
2324
use crate::permissions_toml::PermissionProfileToml;
2425
use crate::types::WindowsSandboxModeToml;
@@ -217,18 +218,6 @@ impl ConfigRequirements {
217218
}
218219
}
219220

220-
#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
221-
#[serde(untagged)]
222-
pub enum McpServerIdentity {
223-
Command { command: String },
224-
Url { url: String },
225-
}
226-
227-
#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
228-
pub struct McpServerRequirement {
229-
pub identity: McpServerIdentity,
230-
}
231-
232221
#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)]
233222
pub struct PluginRequirementsToml {
234223
pub mcp_servers: Option<BTreeMap<String, McpServerRequirement>>,
@@ -1201,6 +1190,25 @@ impl ConfigRequirementsToml {
12011190
}
12021191
}
12031192

1193+
fn validate_mcp_server_requirements(
1194+
requirements: &BTreeMap<String, McpServerRequirement>,
1195+
source: &RequirementSource,
1196+
plugin_name: Option<&str>,
1197+
) -> Result<(), ConstraintError> {
1198+
for (server_name, requirement) in requirements {
1199+
requirement
1200+
.validate()
1201+
.map_err(|reason| ConstraintError::McpServerRequirementParse {
1202+
server_name: plugin_name
1203+
.map(|plugin_name| format!("{plugin_name}/{server_name}"))
1204+
.unwrap_or_else(|| server_name.clone()),
1205+
requirement_source: source.clone(),
1206+
reason,
1207+
})?;
1208+
}
1209+
Ok(())
1210+
}
1211+
12041212
impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
12051213
type Error = ConstraintError;
12061214

@@ -1233,6 +1241,25 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
12331241
guardian_policy_config,
12341242
} = toml;
12351243

1244+
if let Some(requirements) = &mcp_servers {
1245+
validate_mcp_server_requirements(
1246+
&requirements.value,
1247+
&requirements.source,
1248+
/*plugin_name*/ None,
1249+
)?;
1250+
}
1251+
if let Some(plugin_requirements) = &plugins {
1252+
for (plugin_name, plugin) in &plugin_requirements.value {
1253+
if let Some(requirements) = &plugin.mcp_servers {
1254+
validate_mcp_server_requirements(
1255+
requirements,
1256+
&plugin_requirements.source,
1257+
Some(plugin_name),
1258+
)?;
1259+
}
1260+
}
1261+
}
1262+
12361263
let approval_policy = match allowed_approval_policies {
12371264
Some(Sourced {
12381265
value: policies,
@@ -1542,6 +1569,9 @@ pub fn sandbox_mode_requirement_for_permission_profile(
15421569
mod tests {
15431570
use super::*;
15441571
use crate::HookEventsToml;
1572+
use crate::McpServerCommandMatcher;
1573+
use crate::McpServerIdentity;
1574+
use crate::McpServerValueMatcher;
15451575
use anyhow::Result;
15461576
use codex_execpolicy::Decision;
15471577
use codex_execpolicy::Evaluation;
@@ -3454,6 +3484,9 @@ command = "python3 /enterprise/hooks/pre.py"
34543484
#[test]
34553485
fn deserialize_mcp_server_requirements() -> Result<()> {
34563486
let toml_str = r#"
3487+
[mcp_servers.docs]
3488+
description = "ignored legacy field"
3489+
34573490
[mcp_servers.docs.identity]
34583491
command = "codex-mcp"
34593492
@@ -3469,15 +3502,15 @@ command = "python3 /enterprise/hooks/pre.py"
34693502
BTreeMap::from([
34703503
(
34713504
"docs".to_string(),
3472-
McpServerRequirement {
3505+
McpServerRequirement::Identity {
34733506
identity: McpServerIdentity::Command {
34743507
command: "codex-mcp".to_string(),
34753508
},
34763509
},
34773510
),
34783511
(
34793512
"remote".to_string(),
3480-
McpServerRequirement {
3513+
McpServerRequirement::Identity {
34813514
identity: McpServerIdentity::Url {
34823515
url: "https://example.com/mcp".to_string(),
34833516
},
@@ -3490,6 +3523,74 @@ command = "python3 /enterprise/hooks/pre.py"
34903523
Ok(())
34913524
}
34923525

3526+
#[test]
3527+
fn deserialize_mcp_server_matcher_requirements() -> Result<()> {
3528+
let toml_str = r#"
3529+
[mcp_servers.internal_mcp_proxy.identity]
3530+
command = { executable = "company-cli", args = [
3531+
{ match = "exact", value = "mcp" },
3532+
{ match = "exact", value = "proxy" },
3533+
{ match = "exact", value = "--server" },
3534+
{ match = "regex", expression = '^https://[A-Za-z0-9-]+\.mcp\.internal\.example\.com(?::443)?(?:/.*)?$' },
3535+
] }
3536+
"#;
3537+
let requirements: ConfigRequirements =
3538+
with_unknown_source(from_str(toml_str)?).try_into()?;
3539+
3540+
assert_eq!(
3541+
requirements.mcp_servers,
3542+
Some(Sourced::new(
3543+
BTreeMap::from([(
3544+
"internal_mcp_proxy".to_string(),
3545+
McpServerRequirement::Command(McpServerCommandMatcher {
3546+
executable: "company-cli".to_string(),
3547+
args: vec![
3548+
McpServerValueMatcher::Exact {
3549+
value: "mcp".to_string(),
3550+
},
3551+
McpServerValueMatcher::Exact {
3552+
value: "proxy".to_string(),
3553+
},
3554+
McpServerValueMatcher::Exact {
3555+
value: "--server".to_string(),
3556+
},
3557+
McpServerValueMatcher::Regex {
3558+
expression: r"^https://[A-Za-z0-9-]+\.mcp\.internal\.example\.com(?::443)?(?:/.*)?$"
3559+
.to_string(),
3560+
},
3561+
],
3562+
}),
3563+
)]),
3564+
RequirementSource::Unknown,
3565+
))
3566+
);
3567+
Ok(())
3568+
}
3569+
3570+
#[test]
3571+
fn invalid_mcp_server_requirement_regex_reports_the_server_name_and_source() -> Result<()> {
3572+
let toml_str = r#"
3573+
[mcp_servers.broken_rule.identity]
3574+
url = { match = "regex", expression = "[" }
3575+
"#;
3576+
3577+
let err = ConfigRequirements::try_from(with_unknown_source(from_str(toml_str)?))
3578+
.expect_err("invalid matcher regex should fail requirements normalization");
3579+
let ConstraintError::McpServerRequirementParse {
3580+
server_name,
3581+
requirement_source,
3582+
reason,
3583+
} = err
3584+
else {
3585+
panic!("unexpected error: {err:?}");
3586+
};
3587+
3588+
assert_eq!(server_name, "broken_rule");
3589+
assert_eq!(requirement_source, RequirementSource::Unknown);
3590+
assert!(reason.contains("invalid regex `[`"), "{reason}");
3591+
Ok(())
3592+
}
3593+
34933594
#[test]
34943595
fn deserialize_plugin_mcp_server_requirements() -> Result<()> {
34953596
let toml_str = r#"
@@ -3511,7 +3612,7 @@ command = "python3 /enterprise/hooks/pre.py"
35113612
PluginRequirementsToml {
35123613
mcp_servers: Some(BTreeMap::from([(
35133614
"remote".to_string(),
3514-
McpServerRequirement {
3615+
McpServerRequirement::Identity {
35153616
identity: McpServerIdentity::Url {
35163617
url: "https://example.com/mcp".to_string(),
35173618
},
@@ -3524,7 +3625,7 @@ command = "python3 /enterprise/hooks/pre.py"
35243625
PluginRequirementsToml {
35253626
mcp_servers: Some(BTreeMap::from([(
35263627
"sample".to_string(),
3527-
McpServerRequirement {
3628+
McpServerRequirement::Identity {
35283629
identity: McpServerIdentity::Command {
35293630
command: "sample-mcp".to_string(),
35303631
},
@@ -3539,6 +3640,70 @@ command = "python3 /enterprise/hooks/pre.py"
35393640
Ok(())
35403641
}
35413642

3643+
#[test]
3644+
fn deserialize_plugin_mcp_server_matcher_requirement() -> Result<()> {
3645+
let toml_str = r#"
3646+
[plugins."sample@test".mcp_servers.internal_proxy.identity]
3647+
command = { executable = "company-cli", args = [
3648+
{ match = "exact", value = "mcp" },
3649+
{ match = "regex", expression = '^https://[a-z]+\.example\.com$' },
3650+
] }
3651+
"#;
3652+
let requirements: ConfigRequirements =
3653+
with_unknown_source(from_str(toml_str)?).try_into()?;
3654+
3655+
assert_eq!(
3656+
requirements.plugins,
3657+
Some(Sourced::new(
3658+
BTreeMap::from([(
3659+
"sample@test".to_string(),
3660+
PluginRequirementsToml {
3661+
mcp_servers: Some(BTreeMap::from([(
3662+
"internal_proxy".to_string(),
3663+
McpServerRequirement::Command(McpServerCommandMatcher {
3664+
executable: "company-cli".to_string(),
3665+
args: vec![
3666+
McpServerValueMatcher::Exact {
3667+
value: "mcp".to_string(),
3668+
},
3669+
McpServerValueMatcher::Regex {
3670+
expression: r"^https://[a-z]+\.example\.com$".to_string(),
3671+
},
3672+
],
3673+
}),
3674+
)])),
3675+
},
3676+
)]),
3677+
RequirementSource::Unknown,
3678+
))
3679+
);
3680+
Ok(())
3681+
}
3682+
3683+
#[test]
3684+
fn invalid_plugin_mcp_server_regex_reports_plugin_and_server_name() -> Result<()> {
3685+
let toml_str = r#"
3686+
[plugins."sample@test".mcp_servers.broken_rule.identity]
3687+
url = { match = "regex", expression = "[" }
3688+
"#;
3689+
3690+
let err = ConfigRequirements::try_from(with_unknown_source(from_str(toml_str)?))
3691+
.expect_err("invalid plugin MCP regex should fail requirements normalization");
3692+
let ConstraintError::McpServerRequirementParse {
3693+
server_name,
3694+
requirement_source,
3695+
reason,
3696+
} = err
3697+
else {
3698+
panic!("unexpected error: {err:?}");
3699+
};
3700+
3701+
assert_eq!(server_name, "sample@test/broken_rule");
3702+
assert_eq!(requirement_source, RequirementSource::Unknown);
3703+
assert!(reason.contains("invalid regex `[`"), "{reason}");
3704+
Ok(())
3705+
}
3706+
35423707
#[test]
35433708
fn deserialize_exec_policy_requirements() -> Result<()> {
35443709
let toml_str = r#"

codex-rs/config/src/constraint.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@ pub enum ConstraintError {
2424
requirement_source: RequirementSource,
2525
reason: String,
2626
},
27+
28+
#[error(
29+
"invalid requirement for MCP server `{server_name}` (set by {requirement_source}): {reason}"
30+
)]
31+
McpServerRequirementParse {
32+
server_name: String,
33+
requirement_source: RequirementSource,
34+
reason: String,
35+
},
2736
}
2837

2938
impl ConstraintError {

codex-rs/config/src/lib.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ mod key_aliases;
1212
pub mod loader;
1313
mod marketplace_edit;
1414
mod mcp_edit;
15+
mod mcp_requirements;
1516
mod mcp_types;
1617
mod merge;
1718
mod overrides;
@@ -66,8 +67,6 @@ pub use config_requirements::FilesystemDenyReadPattern;
6667
pub use config_requirements::MarketplaceAllowedSourceKind;
6768
pub use config_requirements::MarketplaceAllowedSourceToml;
6869
pub use config_requirements::MarketplaceRequirementsToml;
69-
pub use config_requirements::McpServerIdentity;
70-
pub use config_requirements::McpServerRequirement;
7170
pub use config_requirements::NetworkConstraints;
7271
pub use config_requirements::NetworkDomainPermissionToml;
7372
pub use config_requirements::NetworkDomainPermissionsToml;
@@ -113,6 +112,10 @@ pub use marketplace_edit::remove_user_marketplace;
113112
pub use marketplace_edit::remove_user_marketplace_config;
114113
pub use mcp_edit::ConfigEditsBuilder;
115114
pub use mcp_edit::load_global_mcp_servers;
115+
pub use mcp_requirements::McpServerCommandMatcher;
116+
pub use mcp_requirements::McpServerIdentity;
117+
pub use mcp_requirements::McpServerRequirement;
118+
pub use mcp_requirements::McpServerValueMatcher;
116119
pub use mcp_types::AppToolApproval;
117120
pub use mcp_types::DEFAULT_MCP_SERVER_ENVIRONMENT_ID;
118121
pub use mcp_types::McpServerAuth;

0 commit comments

Comments
 (0)