From 5662eb8b75cd6aa5e1acbdbf93549f85dee510a1 Mon Sep 17 00:00:00 2001 From: gt-oai Date: Fri, 30 Jan 2026 18:04:09 +0000 Subject: [PATCH] Load exec policy rules from requirements (#10190) `requirements.toml` should be able to specify rules which always run. My intention here was that these rules could only ever be restrictive, which means the decision can be "prompt" or "forbidden" but never "allow". A requirement of "you must always allow this command" didn't make sense to me, but happy to be gaveled otherwise. Rules already applies the most restrictive decision, so we can safely merge these with rules found in other config folders. --- codex-rs/app-server/src/config_api.rs | 1 + codex-rs/cloud-requirements/src/lib.rs | 1 + codex-rs/core/src/config/constraint.rs | 6 + codex-rs/core/src/config/mod.rs | 1 + .../src/config_loader/config_requirements.rs | 98 +++++++ codex-rs/core/src/config_loader/mod.rs | 1 - .../config_loader/requirements_exec_policy.rs | 82 ++++-- codex-rs/core/src/config_loader/tests.rs | 266 ++++++++++++++---- codex-rs/core/src/exec_policy.rs | 13 +- 9 files changed, 398 insertions(+), 71 deletions(-) diff --git a/codex-rs/app-server/src/config_api.rs b/codex-rs/app-server/src/config_api.rs index 5c924d1819..9207668486 100644 --- a/codex-rs/app-server/src/config_api.rs +++ b/codex-rs/app-server/src/config_api.rs @@ -136,6 +136,7 @@ mod tests { CoreSandboxModeRequirement::ExternalSandbox, ]), mcp_servers: None, + rules: None, }; let mapped = map_requirements_toml_to_api(requirements); diff --git a/codex-rs/cloud-requirements/src/lib.rs b/codex-rs/cloud-requirements/src/lib.rs index ccfa600319..37faed081b 100644 --- a/codex-rs/cloud-requirements/src/lib.rs +++ b/codex-rs/cloud-requirements/src/lib.rs @@ -317,6 +317,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, mcp_servers: None, + rules: None, }) ); } diff --git a/codex-rs/core/src/config/constraint.rs b/codex-rs/core/src/config/constraint.rs index fa431a6eb2..23b6c57c74 100644 --- a/codex-rs/core/src/config/constraint.rs +++ b/codex-rs/core/src/config/constraint.rs @@ -18,6 +18,12 @@ pub enum ConstraintError { #[error("field `{field_name}` cannot be empty")] EmptyField { field_name: String }, + + #[error("invalid rules in requirements (set by {requirement_source}): {reason}")] + ExecPolicyParse { + requirement_source: RequirementSource, + reason: String, + }, } impl ConstraintError { diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index fbd8999918..2e1c853620 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1512,6 +1512,7 @@ impl Config { approval_policy: mut constrained_approval_policy, sandbox_policy: mut constrained_sandbox_policy, mcp_servers, + exec_policy: _, } = requirements; constrained_approval_policy diff --git a/codex-rs/core/src/config_loader/config_requirements.rs b/codex-rs/core/src/config_loader/config_requirements.rs index 2c220049cf..6b9184a539 100644 --- a/codex-rs/core/src/config_loader/config_requirements.rs +++ b/codex-rs/core/src/config_loader/config_requirements.rs @@ -6,6 +6,8 @@ use serde::Deserialize; use std::collections::BTreeMap; use std::fmt; +use super::requirements_exec_policy::RequirementsExecPolicy; +use super::requirements_exec_policy::RequirementsExecPolicyToml; use crate::config::Constrained; use crate::config::ConstraintError; @@ -49,6 +51,7 @@ pub struct ConfigRequirements { pub approval_policy: Constrained, pub sandbox_policy: Constrained, pub mcp_servers: Option>>, + pub(crate) exec_policy: Option>, } impl Default for ConfigRequirements { @@ -57,6 +60,7 @@ impl Default for ConfigRequirements { approval_policy: Constrained::allow_any_from_default(), sandbox_policy: Constrained::allow_any(SandboxPolicy::ReadOnly), mcp_servers: None, + exec_policy: None, } } } @@ -79,6 +83,7 @@ pub struct ConfigRequirementsToml { pub allowed_approval_policies: Option>, pub allowed_sandbox_modes: Option>, pub mcp_servers: Option>, + pub rules: Option, } /// Value paired with the requirement source it came from, for better error @@ -108,6 +113,7 @@ pub struct ConfigRequirementsWithSources { pub allowed_approval_policies: Option>>, pub allowed_sandbox_modes: Option>>, pub mcp_servers: Option>>, + pub rules: Option>, } impl ConfigRequirementsWithSources { @@ -139,6 +145,7 @@ impl ConfigRequirementsWithSources { allowed_approval_policies, allowed_sandbox_modes, mcp_servers, + rules, } ); } @@ -148,11 +155,13 @@ impl ConfigRequirementsWithSources { allowed_approval_policies, allowed_sandbox_modes, mcp_servers, + rules, } = self; ConfigRequirementsToml { allowed_approval_policies: allowed_approval_policies.map(|sourced| sourced.value), allowed_sandbox_modes: allowed_sandbox_modes.map(|sourced| sourced.value), mcp_servers: mcp_servers.map(|sourced| sourced.value), + rules: rules.map(|sourced| sourced.value), } } } @@ -189,6 +198,7 @@ impl ConfigRequirementsToml { self.allowed_approval_policies.is_none() && self.allowed_sandbox_modes.is_none() && self.mcp_servers.is_none() + && self.rules.is_none() } } @@ -200,6 +210,7 @@ impl TryFrom for ConfigRequirements { allowed_approval_policies, allowed_sandbox_modes, mcp_servers, + rules, } = toml; let approval_policy: Constrained = match allowed_approval_policies { @@ -274,10 +285,24 @@ impl TryFrom for ConfigRequirements { } None => Constrained::allow_any(default_sandbox_policy), }; + let exec_policy = match rules { + Some(Sourced { value, source }) => { + let policy = value.to_requirements_policy().map_err(|err| { + ConstraintError::ExecPolicyParse { + requirement_source: source.clone(), + reason: err.to_string(), + } + })?; + Some(Sourced::new(policy, source)) + } + None => None, + }; + Ok(ConfigRequirements { approval_policy, sandbox_policy, mcp_servers, + exec_policy, }) } } @@ -286,16 +311,24 @@ impl TryFrom for ConfigRequirements { mod tests { use super::*; use anyhow::Result; + use codex_execpolicy::Decision; + use codex_execpolicy::Evaluation; + use codex_execpolicy::RuleMatch; use codex_protocol::protocol::NetworkAccess; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use toml::from_str; + fn tokens(cmd: &[&str]) -> Vec { + cmd.iter().map(std::string::ToString::to_string).collect() + } + fn with_unknown_source(toml: ConfigRequirementsToml) -> ConfigRequirementsWithSources { let ConfigRequirementsToml { allowed_approval_policies, allowed_sandbox_modes, mcp_servers, + rules, } = toml; ConfigRequirementsWithSources { allowed_approval_policies: allowed_approval_policies @@ -303,6 +336,7 @@ mod tests { allowed_sandbox_modes: allowed_sandbox_modes .map(|value| Sourced::new(value, RequirementSource::Unknown)), mcp_servers: mcp_servers.map(|value| Sourced::new(value, RequirementSource::Unknown)), + rules: rules.map(|value| Sourced::new(value, RequirementSource::Unknown)), } } @@ -323,6 +357,7 @@ mod tests { allowed_approval_policies: Some(allowed_approval_policies.clone()), allowed_sandbox_modes: Some(allowed_sandbox_modes.clone()), mcp_servers: None, + rules: None, }; target.merge_unset_fields(source.clone(), other); @@ -336,6 +371,7 @@ mod tests { )), allowed_sandbox_modes: Some(Sourced::new(allowed_sandbox_modes, source)), mcp_servers: None, + rules: None, } ); } @@ -364,6 +400,7 @@ mod tests { )), allowed_sandbox_modes: None, mcp_servers: None, + rules: None, } ); Ok(()) @@ -400,6 +437,7 @@ mod tests { )), allowed_sandbox_modes: None, mcp_servers: None, + rules: None, } ); Ok(()) @@ -626,4 +664,64 @@ mod tests { ); Ok(()) } + + #[test] + fn deserialize_exec_policy_requirements() -> Result<()> { + let toml_str = r#" + [rules] + prefix_rules = [ + { pattern = [{ token = "rm" }], decision = "forbidden" }, + ] + "#; + let config: ConfigRequirementsToml = from_str(toml_str)?; + let requirements: ConfigRequirements = with_unknown_source(config).try_into()?; + let policy = requirements.exec_policy.expect("exec policy").value; + + assert_eq!( + policy.as_ref().check(&tokens(&["rm", "-rf"]), &|_| { + panic!("rule should match so heuristic should not be called"); + }), + Evaluation { + decision: Decision::Forbidden, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: tokens(&["rm"]), + decision: Decision::Forbidden, + justification: None, + }], + } + ); + + Ok(()) + } + + #[test] + fn exec_policy_error_includes_requirement_source() -> Result<()> { + let toml_str = r#" + [rules] + prefix_rules = [ + { pattern = [{ token = "rm" }] }, + ] + "#; + let config: ConfigRequirementsToml = from_str(toml_str)?; + let requirements_toml_file = + AbsolutePathBuf::from_absolute_path("/etc/codex/requirements.toml")?; + let source_location = RequirementSource::SystemRequirementsToml { + file: requirements_toml_file, + }; + + let mut requirements_with_sources = ConfigRequirementsWithSources::default(); + requirements_with_sources.merge_unset_fields(source_location.clone(), config); + let err = ConfigRequirements::try_from(requirements_with_sources) + .expect_err("invalid exec policy"); + + assert_eq!( + err, + ConstraintError::ExecPolicyParse { + requirement_source: source_location, + reason: "rules prefix_rule at index 0 is missing a decision".to_string(), + } + ); + + Ok(()) + } } diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 0dd704cab5..eba89fdb3d 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -7,7 +7,6 @@ mod layer_io; mod macos; mod merge; mod overrides; -#[cfg(test)] mod requirements_exec_policy; mod state; diff --git a/codex-rs/core/src/config_loader/requirements_exec_policy.rs b/codex-rs/core/src/config_loader/requirements_exec_policy.rs index cbc1d7531e..74546fc426 100644 --- a/codex-rs/core/src/config_loader/requirements_exec_policy.rs +++ b/codex-rs/core/src/config_loader/requirements_exec_policy.rs @@ -9,16 +9,43 @@ use serde::Deserialize; use std::sync::Arc; use thiserror::Error; -/// TOML types for expressing exec policy requirements. -/// -/// These types are kept separate from `ConfigRequirementsToml` and are -/// converted into `codex-execpolicy` rules. -#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] -pub struct RequirementsExecPolicyTomlRoot { - pub exec_policy: RequirementsExecPolicyToml, +#[derive(Debug, Clone)] +pub(crate) struct RequirementsExecPolicy { + policy: Policy, } -/// TOML representation of `[exec_policy]` within `requirements.toml`. +impl RequirementsExecPolicy { + pub fn new(policy: Policy) -> Self { + Self { policy } + } +} + +impl PartialEq for RequirementsExecPolicy { + fn eq(&self, other: &Self) -> bool { + policy_fingerprint(&self.policy) == policy_fingerprint(&other.policy) + } +} + +impl Eq for RequirementsExecPolicy {} + +impl AsRef for RequirementsExecPolicy { + fn as_ref(&self) -> &Policy { + &self.policy + } +} + +fn policy_fingerprint(policy: &Policy) -> Vec { + let mut entries = Vec::new(); + for (program, rules) in policy.rules().iter_all() { + for rule in rules { + entries.push(format!("{program}:{rule:?}")); + } + } + entries.sort(); + entries +} + +/// TOML representation of `[rules]` within `requirements.toml`. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] pub struct RequirementsExecPolicyToml { pub prefix_rules: Vec, @@ -65,14 +92,14 @@ impl RequirementsExecPolicyDecisionToml { #[derive(Debug, Error)] pub enum RequirementsExecPolicyParseError { - #[error("exec policy prefix_rules cannot be empty")] + #[error("rules prefix_rules cannot be empty")] EmptyPrefixRules, - #[error("exec policy prefix_rule at index {rule_index} has an empty pattern")] + #[error("rules prefix_rule at index {rule_index} has an empty pattern")] EmptyPattern { rule_index: usize }, #[error( - "exec policy prefix_rule at index {rule_index} has an invalid pattern token at index {token_index}: {reason}" + "rules prefix_rule at index {rule_index} has an invalid pattern token at index {token_index}: {reason}" )] InvalidPatternToken { rule_index: usize, @@ -80,12 +107,20 @@ pub enum RequirementsExecPolicyParseError { reason: String, }, - #[error("exec policy prefix_rule at index {rule_index} has an empty justification")] + #[error("rules prefix_rule at index {rule_index} has an empty justification")] EmptyJustification { rule_index: usize }, + + #[error("rules prefix_rule at index {rule_index} is missing a decision")] + MissingDecision { rule_index: usize }, + + #[error( + "rules prefix_rule at index {rule_index} has decision 'allow', which is not permitted in requirements.toml: Codex merges these rules with other config and uses the most restrictive result (use 'prompt' or 'forbidden')" + )] + AllowDecisionNotAllowed { rule_index: usize }, } impl RequirementsExecPolicyToml { - /// Convert requirements TOML exec policy rules into the internal `.rules` + /// Convert requirements TOML rules into the internal `.rules` /// representation used by `codex-execpolicy`. pub fn to_policy(&self) -> Result { if self.prefix_rules.is_empty() { @@ -112,10 +147,17 @@ impl RequirementsExecPolicyToml { .map(|(token_index, token)| parse_pattern_token(token, rule_index, token_index)) .collect::, _>>()?; - let decision = rule - .decision - .map(RequirementsExecPolicyDecisionToml::as_decision) - .unwrap_or(Decision::Allow); + let decision = match rule.decision { + Some(RequirementsExecPolicyDecisionToml::Allow) => { + return Err(RequirementsExecPolicyParseError::AllowDecisionNotAllowed { + rule_index, + }); + } + Some(decision) => decision.as_decision(), + None => { + return Err(RequirementsExecPolicyParseError::MissingDecision { rule_index }); + } + }; let justification = rule.justification.clone(); let (first_token, remaining_tokens) = pattern_tokens @@ -139,6 +181,12 @@ impl RequirementsExecPolicyToml { Ok(Policy::new(rules_by_program)) } + + pub(crate) fn to_requirements_policy( + &self, + ) -> Result { + self.to_policy().map(RequirementsExecPolicy::new) + } } fn parse_pattern_token( diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index c213c0b2ea..38d104fc19 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -502,6 +502,7 @@ allowed_approval_policies = ["on-request"] allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, mcp_servers: None, + rules: None, }, ); load_requirements_toml(&mut config_requirements_toml, &requirements_file).await?; @@ -535,6 +536,7 @@ async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()> allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, mcp_servers: None, + rules: None, }; let expected = requirements.clone(); let cloud_requirements = CloudRequirementsLoader::new(async move { Some(requirements) }); @@ -1016,49 +1018,79 @@ async fn project_root_markers_supports_alternate_markers() -> std::io::Result<() } mod requirements_exec_policy_tests { + use super::super::config_requirements::ConfigRequirementsWithSources; use super::super::requirements_exec_policy::RequirementsExecPolicyDecisionToml; + use super::super::requirements_exec_policy::RequirementsExecPolicyParseError; use super::super::requirements_exec_policy::RequirementsExecPolicyPatternTokenToml; use super::super::requirements_exec_policy::RequirementsExecPolicyPrefixRuleToml; use super::super::requirements_exec_policy::RequirementsExecPolicyToml; - use super::super::requirements_exec_policy::RequirementsExecPolicyTomlRoot; + use crate::config_loader::ConfigLayerEntry; + use crate::config_loader::ConfigLayerStack; + use crate::config_loader::ConfigRequirements; + use crate::config_loader::ConfigRequirementsToml; + use crate::config_loader::RequirementSource; + use crate::exec_policy::load_exec_policy; + use codex_app_server_protocol::ConfigLayerSource; use codex_execpolicy::Decision; use codex_execpolicy::Evaluation; use codex_execpolicy::RuleMatch; + use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; + use std::path::Path; + use tempfile::tempdir; + use toml::Value as TomlValue; use toml::from_str; fn tokens(cmd: &[&str]) -> Vec { cmd.iter().map(std::string::ToString::to_string).collect() } - fn allow_all(_: &[String]) -> Decision { - Decision::Allow + fn panic_if_called(_: &[String]) -> Decision { + panic!("rule should match so heuristic should not be called"); + } + + fn config_stack_for_dot_codex_folder_with_requirements( + dot_codex_folder: &Path, + requirements: ConfigRequirements, + ) -> ConfigLayerStack { + let dot_codex_folder = AbsolutePathBuf::from_absolute_path(dot_codex_folder) + .expect("absolute dot_codex_folder"); + let layer = ConfigLayerEntry::new( + ConfigLayerSource::Project { dot_codex_folder }, + TomlValue::Table(Default::default()), + ); + ConfigLayerStack::new(vec![layer], requirements, ConfigRequirementsToml::default()) + .expect("ConfigLayerStack") + } + + fn requirements_from_toml(toml_str: &str) -> ConfigRequirements { + let config: ConfigRequirementsToml = from_str(toml_str).expect("parse requirements toml"); + let mut with_sources = ConfigRequirementsWithSources::default(); + with_sources.merge_unset_fields(RequirementSource::Unknown, config); + ConfigRequirements::try_from(with_sources).expect("requirements") } #[test] fn parses_single_prefix_rule_from_raw_toml() -> anyhow::Result<()> { let toml_str = r#" -[exec_policy] prefix_rules = [ { pattern = [{ token = "rm" }], decision = "forbidden" }, ] "#; - let parsed: RequirementsExecPolicyTomlRoot = from_str(toml_str)?; + let parsed: RequirementsExecPolicyToml = from_str(toml_str)?; assert_eq!( parsed, - RequirementsExecPolicyTomlRoot { - exec_policy: RequirementsExecPolicyToml { - prefix_rules: vec![RequirementsExecPolicyPrefixRuleToml { - pattern: vec![RequirementsExecPolicyPatternTokenToml { - token: Some("rm".to_string()), - any_of: None, - }], - decision: Some(RequirementsExecPolicyDecisionToml::Forbidden), - justification: None, + RequirementsExecPolicyToml { + prefix_rules: vec![RequirementsExecPolicyPrefixRuleToml { + pattern: vec![RequirementsExecPolicyPatternTokenToml { + token: Some("rm".to_string()), + any_of: None, }], - }, + decision: Some(RequirementsExecPolicyDecisionToml::Forbidden), + justification: None, + }], } ); @@ -1068,44 +1100,41 @@ prefix_rules = [ #[test] fn parses_multiple_prefix_rules_from_raw_toml() -> anyhow::Result<()> { let toml_str = r#" -[exec_policy] prefix_rules = [ { pattern = [{ token = "rm" }], decision = "forbidden" }, { pattern = [{ token = "git" }, { any_of = ["push", "commit"] }], decision = "prompt", justification = "review changes before push or commit" }, ] "#; - let parsed: RequirementsExecPolicyTomlRoot = from_str(toml_str)?; + let parsed: RequirementsExecPolicyToml = from_str(toml_str)?; assert_eq!( parsed, - RequirementsExecPolicyTomlRoot { - exec_policy: RequirementsExecPolicyToml { - prefix_rules: vec![ - RequirementsExecPolicyPrefixRuleToml { - pattern: vec![RequirementsExecPolicyPatternTokenToml { - token: Some("rm".to_string()), + RequirementsExecPolicyToml { + prefix_rules: vec![ + RequirementsExecPolicyPrefixRuleToml { + pattern: vec![RequirementsExecPolicyPatternTokenToml { + token: Some("rm".to_string()), + any_of: None, + }], + decision: Some(RequirementsExecPolicyDecisionToml::Forbidden), + justification: None, + }, + RequirementsExecPolicyPrefixRuleToml { + pattern: vec![ + RequirementsExecPolicyPatternTokenToml { + token: Some("git".to_string()), any_of: None, - }], - decision: Some(RequirementsExecPolicyDecisionToml::Forbidden), - justification: None, - }, - RequirementsExecPolicyPrefixRuleToml { - pattern: vec![ - RequirementsExecPolicyPatternTokenToml { - token: Some("git".to_string()), - any_of: None, - }, - RequirementsExecPolicyPatternTokenToml { - token: None, - any_of: Some(vec!["push".to_string(), "commit".to_string()]), - }, - ], - decision: Some(RequirementsExecPolicyDecisionToml::Prompt), - justification: Some("review changes before push or commit".to_string()), - }, - ], - }, + }, + RequirementsExecPolicyPatternTokenToml { + token: None, + any_of: Some(vec!["push".to_string(), "commit".to_string()]), + }, + ], + decision: Some(RequirementsExecPolicyDecisionToml::Prompt), + justification: Some("review changes before push or commit".to_string()), + }, + ], } ); @@ -1115,17 +1144,16 @@ prefix_rules = [ #[test] fn converts_rules_toml_into_internal_policy_representation() -> anyhow::Result<()> { let toml_str = r#" -[exec_policy] prefix_rules = [ { pattern = [{ token = "rm" }], decision = "forbidden" }, ] "#; - let parsed: RequirementsExecPolicyTomlRoot = from_str(toml_str)?; - let policy = parsed.exec_policy.to_policy()?; + let parsed: RequirementsExecPolicyToml = from_str(toml_str)?; + let policy = parsed.to_policy()?; assert_eq!( - policy.check(&tokens(&["rm", "-rf", "/tmp"]), &allow_all), + policy.check(&tokens(&["rm", "-rf", "/tmp"]), &panic_if_called), Evaluation { decision: Decision::Forbidden, matched_rules: vec![RuleMatch::PrefixRuleMatch { @@ -1142,16 +1170,15 @@ prefix_rules = [ #[test] fn head_any_of_expands_into_multiple_program_rules() -> anyhow::Result<()> { let toml_str = r#" -[exec_policy] prefix_rules = [ { pattern = [{ any_of = ["git", "hg"] }, { token = "status" }], decision = "prompt" }, ] "#; - let parsed: RequirementsExecPolicyTomlRoot = from_str(toml_str)?; - let policy = parsed.exec_policy.to_policy()?; + let parsed: RequirementsExecPolicyToml = from_str(toml_str)?; + let policy = parsed.to_policy()?; assert_eq!( - policy.check(&tokens(&["git", "status"]), &allow_all), + policy.check(&tokens(&["git", "status"]), &panic_if_called), Evaluation { decision: Decision::Prompt, matched_rules: vec![RuleMatch::PrefixRuleMatch { @@ -1162,7 +1189,7 @@ prefix_rules = [ } ); assert_eq!( - policy.check(&tokens(&["hg", "status"]), &allow_all), + policy.check(&tokens(&["hg", "status"]), &panic_if_called), Evaluation { decision: Decision::Prompt, matched_rules: vec![RuleMatch::PrefixRuleMatch { @@ -1175,4 +1202,139 @@ prefix_rules = [ Ok(()) } + + #[test] + fn missing_decision_is_rejected() -> anyhow::Result<()> { + let toml_str = r#" +prefix_rules = [ + { pattern = [{ token = "rm" }] }, +] +"#; + + let parsed: RequirementsExecPolicyToml = from_str(toml_str)?; + let err = parsed.to_policy().expect_err("missing decision"); + + assert!(matches!( + err, + RequirementsExecPolicyParseError::MissingDecision { rule_index: 0 } + )); + Ok(()) + } + + #[test] + fn allow_decision_is_rejected() -> anyhow::Result<()> { + let toml_str = r#" +prefix_rules = [ + { pattern = [{ token = "rm" }], decision = "allow" }, +] +"#; + + let parsed: RequirementsExecPolicyToml = from_str(toml_str)?; + let err = parsed.to_policy().expect_err("allow decision not allowed"); + + assert!(matches!( + err, + RequirementsExecPolicyParseError::AllowDecisionNotAllowed { rule_index: 0 } + )); + Ok(()) + } + + #[test] + fn empty_prefix_rules_is_rejected() -> anyhow::Result<()> { + let toml_str = r#" +prefix_rules = [] +"#; + + let parsed: RequirementsExecPolicyToml = from_str(toml_str)?; + let err = parsed.to_policy().expect_err("empty prefix rules"); + + assert!(matches!( + err, + RequirementsExecPolicyParseError::EmptyPrefixRules + )); + Ok(()) + } + + #[tokio::test] + async fn loads_requirements_exec_policy_without_rules_files() -> anyhow::Result<()> { + let temp_dir = tempdir()?; + let requirements = requirements_from_toml( + r#" + [rules] + prefix_rules = [ + { pattern = [{ token = "rm" }], decision = "forbidden" }, + ] + "#, + ); + let config_stack = + config_stack_for_dot_codex_folder_with_requirements(temp_dir.path(), requirements); + + let policy = load_exec_policy(&config_stack).await?; + + assert_eq!( + policy.check_multiple([vec!["rm".to_string()]].iter(), &panic_if_called), + Evaluation { + decision: Decision::Forbidden, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec!["rm".to_string()], + decision: Decision::Forbidden, + justification: None, + }], + } + ); + + Ok(()) + } + + #[tokio::test] + async fn merges_requirements_exec_policy_with_file_rules() -> anyhow::Result<()> { + let temp_dir = tempdir()?; + let policy_dir = temp_dir.path().join("rules"); + std::fs::create_dir_all(&policy_dir)?; + std::fs::write( + policy_dir.join("deny.rules"), + r#"prefix_rule(pattern=["rm"], decision="forbidden")"#, + )?; + + let requirements = requirements_from_toml( + r#" + [rules] + prefix_rules = [ + { pattern = [{ token = "git" }, { token = "push" }], decision = "prompt" }, + ] + "#, + ); + let config_stack = + config_stack_for_dot_codex_folder_with_requirements(temp_dir.path(), requirements); + + let policy = load_exec_policy(&config_stack).await?; + + assert_eq!( + policy.check_multiple([vec!["rm".to_string()]].iter(), &panic_if_called), + Evaluation { + decision: Decision::Forbidden, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec!["rm".to_string()], + decision: Decision::Forbidden, + justification: None, + }], + } + ); + assert_eq!( + policy.check_multiple( + [vec!["git".to_string(), "push".to_string()]].iter(), + &panic_if_called + ), + Evaluation { + decision: Decision::Prompt, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec!["git".to_string(), "push".to_string()], + decision: Decision::Prompt, + justification: None, + }], + } + ); + + Ok(()) + } } diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 07a9ebceba..c0cb7f8fcf 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -280,7 +280,18 @@ pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result