Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions codex-rs/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions codex-rs/config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,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 }
Expand Down
197 changes: 181 additions & 16 deletions codex-rs/config/src/config_requirements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use super::requirements_exec_policy::RequirementsExecPolicyToml;
use crate::Constrained;
use crate::ConstraintError;
use crate::ManagedHooksRequirementsToml;
use crate::mcp_requirements::McpServerRequirement;
use crate::mcp_types::AppToolApproval;
use crate::permissions_toml::PermissionProfileToml;
use crate::types::WindowsSandboxModeToml;
Expand Down Expand Up @@ -217,18 +218,6 @@ impl ConfigRequirements {
}
}

#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
#[serde(untagged)]
pub enum McpServerIdentity {
Command { command: String },
Url { url: String },
}

#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
pub struct McpServerRequirement {
pub identity: McpServerIdentity,
}

#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)]
pub struct PluginRequirementsToml {
pub mcp_servers: Option<BTreeMap<String, McpServerRequirement>>,
Expand Down Expand Up @@ -1201,6 +1190,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;

Expand Down Expand Up @@ -1233,6 +1241,25 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
guardian_policy_config,
} = toml;

if let Some(requirements) = &mcp_servers {
validate_mcp_server_requirements(
&requirements.value,
&requirements.source,
/*plugin_name*/ 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),
)?;
}
}
}

let approval_policy = match allowed_approval_policies {
Some(Sourced {
value: policies,
Expand Down Expand Up @@ -1542,6 +1569,9 @@ pub fn sandbox_mode_requirement_for_permission_profile(
mod tests {
use super::*;
use crate::HookEventsToml;
use crate::McpServerCommandMatcher;
use crate::McpServerIdentity;
use crate::McpServerValueMatcher;
use anyhow::Result;
use codex_execpolicy::Decision;
use codex_execpolicy::Evaluation;
Expand Down Expand Up @@ -3454,6 +3484,9 @@ command = "python3 /enterprise/hooks/pre.py"
#[test]
fn deserialize_mcp_server_requirements() -> Result<()> {
let toml_str = r#"
[mcp_servers.docs]
description = "ignored legacy field"

[mcp_servers.docs.identity]
command = "codex-mcp"

Expand All @@ -3469,15 +3502,15 @@ command = "python3 /enterprise/hooks/pre.py"
BTreeMap::from([
(
"docs".to_string(),
McpServerRequirement {
McpServerRequirement::Identity {
identity: McpServerIdentity::Command {
command: "codex-mcp".to_string(),
},
},
),
(
"remote".to_string(),
McpServerRequirement {
McpServerRequirement::Identity {
identity: McpServerIdentity::Url {
url: "https://example.com/mcp".to_string(),
},
Expand All @@ -3490,6 +3523,74 @@ command = "python3 /enterprise/hooks/pre.py"
Ok(())
}

#[test]
fn deserialize_mcp_server_matcher_requirements() -> Result<()> {
let toml_str = r#"
[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)?(?:/.*)?$' },
] }
"#;
let requirements: ConfigRequirements =
with_unknown_source(from_str(toml_str)?).try_into()?;

assert_eq!(
requirements.mcp_servers,
Some(Sourced::new(
BTreeMap::from([(
"internal_mcp_proxy".to_string(),
McpServerRequirement::Command(McpServerCommandMatcher {
executable: "company-cli".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_requirement_regex_reports_the_server_name_and_source() -> Result<()> {
let toml_str = r#"
[mcp_servers.broken_rule.identity]
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::McpServerRequirementParse {
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#"
Expand All @@ -3511,7 +3612,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(),
},
Expand All @@ -3524,7 +3625,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(),
},
Expand All @@ -3539,6 +3640,70 @@ 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.identity]
command = { executable = "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 {
executable: "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.identity]
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#"
Expand Down
9 changes: 9 additions & 0 deletions codex-rs/config/src/constraint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ pub enum ConstraintError {
requirement_source: RequirementSource,
reason: String,
},

#[error(
"invalid requirement for MCP server `{server_name}` (set by {requirement_source}): {reason}"
)]
McpServerRequirementParse {
server_name: String,
requirement_source: RequirementSource,
reason: String,
},
}

impl ConstraintError {
Expand Down
7 changes: 5 additions & 2 deletions codex-rs/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -66,8 +67,6 @@ pub use config_requirements::FilesystemDenyReadPattern;
pub use config_requirements::MarketplaceAllowedSourceKind;
pub use config_requirements::MarketplaceAllowedSourceToml;
pub use config_requirements::MarketplaceRequirementsToml;
pub use config_requirements::McpServerIdentity;
pub use config_requirements::McpServerRequirement;
pub use config_requirements::NetworkConstraints;
pub use config_requirements::NetworkDomainPermissionToml;
pub use config_requirements::NetworkDomainPermissionsToml;
Expand Down Expand Up @@ -113,6 +112,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::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::McpServerConfig;
Expand Down
Loading
Loading