From 7ab45487dd1898894b675bae9ccb73e7cb4925ee Mon Sep 17 00:00:00 2001 From: zhao-oai Date: Mon, 17 Nov 2025 16:44:41 -0800 Subject: [PATCH] execpolicy2 extension (#6627) - enabling execpolicy2 parser to parse multiple policy files to build a combined `Policy` (useful if codex detects many `.codexpolicy` files) - adding functionality to `Policy` to allow evaluation of multiple cmds at once (useful when we have chained commands) --- codex-rs/execpolicy2/README.md | 7 +- codex-rs/execpolicy2/src/main.rs | 42 ++++--- codex-rs/execpolicy2/src/parser.rs | 32 +++-- codex-rs/execpolicy2/src/policy.rs | 22 ++++ codex-rs/execpolicy2/tests/basic.rs | 173 +++++++++++++++++++++++----- 5 files changed, 223 insertions(+), 53 deletions(-) diff --git a/codex-rs/execpolicy2/README.md b/codex-rs/execpolicy2/README.md index e433f5e2c..8cf5302fb 100644 --- a/codex-rs/execpolicy2/README.md +++ b/codex-rs/execpolicy2/README.md @@ -45,10 +45,15 @@ prefix_rule( - The effective `decision` is the strictest severity across all matches (`forbidden` > `prompt` > `allow`). ## CLI -- Provide a policy file (for example `src/default.codexpolicy`) to check a command: +- Provide one or more policy files (for example `src/default.codexpolicy`) to check a command: ```bash cargo run -p codex-execpolicy2 -- check --policy path/to/policy.codexpolicy git status ``` +- Pass multiple `--policy` flags to merge rules, evaluated in the order provided: +```bash +cargo run -p codex-execpolicy2 -- check --policy base.codexpolicy --policy overrides.codexpolicy git status +``` +- Output is newline-delimited JSON by default; pass `--pretty` for pretty-printed JSON if desired. - Example outcomes: - Match: `{"match": { ... "decision": "allow" ... }}` - No match: `"noMatch"` diff --git a/codex-rs/execpolicy2/src/main.rs b/codex-rs/execpolicy2/src/main.rs index c0786a320..f654c5f84 100644 --- a/codex-rs/execpolicy2/src/main.rs +++ b/codex-rs/execpolicy2/src/main.rs @@ -1,5 +1,4 @@ use std::fs; -use std::path::Path; use std::path::PathBuf; use anyhow::Context; @@ -13,8 +12,12 @@ use codex_execpolicy2::PolicyParser; enum Cli { /// Evaluate a command against a policy. Check { - #[arg(short, long, value_name = "PATH")] - policy: PathBuf, + #[arg(short, long = "policy", value_name = "PATH", required = true)] + policies: Vec, + + /// Pretty-print the JSON output. + #[arg(long)] + pretty: bool, /// Command tokens to check. #[arg( @@ -30,25 +33,34 @@ enum Cli { fn main() -> Result<()> { let cli = Cli::parse(); match cli { - Cli::Check { policy, command } => cmd_check(policy, command), + Cli::Check { + policies, + command, + pretty, + } => cmd_check(policies, command, pretty), } } -fn cmd_check(policy_path: PathBuf, args: Vec) -> Result<()> { - let policy = load_policy(&policy_path)?; +fn cmd_check(policy_paths: Vec, args: Vec, pretty: bool) -> Result<()> { + let policy = load_policies(&policy_paths)?; let eval = policy.check(&args); - let json = serde_json::to_string_pretty(&eval)?; + let json = if pretty { + serde_json::to_string_pretty(&eval)? + } else { + serde_json::to_string(&eval)? + }; println!("{json}"); Ok(()) } -fn load_policy(policy_path: &Path) -> Result { - let policy_file_contents = fs::read_to_string(policy_path) - .with_context(|| format!("failed to read policy at {}", policy_path.display()))?; - let policy_identifier = policy_path.to_string_lossy(); - Ok(PolicyParser::parse( - policy_identifier.as_ref(), - &policy_file_contents, - )?) +fn load_policies(policy_paths: &[PathBuf]) -> Result { + let mut parser = PolicyParser::new(); + for policy_path in policy_paths { + let policy_file_contents = fs::read_to_string(policy_path) + .with_context(|| format!("failed to read policy at {}", policy_path.display()))?; + let policy_identifier = policy_path.to_string_lossy().to_string(); + parser.parse(&policy_identifier, &policy_file_contents)?; + } + Ok(parser.build()) } diff --git a/codex-rs/execpolicy2/src/parser.rs b/codex-rs/execpolicy2/src/parser.rs index b978781af..d50549055 100644 --- a/codex-rs/execpolicy2/src/parser.rs +++ b/codex-rs/execpolicy2/src/parser.rs @@ -25,16 +25,26 @@ use crate::rule::RuleRef; use crate::rule::validate_match_examples; use crate::rule::validate_not_match_examples; -// todo: support parsing multiple policies -pub struct PolicyParser; +pub struct PolicyParser { + builder: RefCell, +} + +impl Default for PolicyParser { + fn default() -> Self { + Self::new() + } +} impl PolicyParser { + pub fn new() -> Self { + Self { + builder: RefCell::new(PolicyBuilder::new()), + } + } + /// Parses a policy, tagging parser errors with `policy_identifier` so failures include the /// identifier alongside line numbers. - pub fn parse( - policy_identifier: &str, - policy_file_contents: &str, - ) -> Result { + pub fn parse(&mut self, policy_identifier: &str, policy_file_contents: &str) -> Result<()> { let mut dialect = Dialect::Extended.clone(); dialect.enable_f_strings = true; let ast = AstModule::parse( @@ -45,14 +55,16 @@ impl PolicyParser { .map_err(Error::Starlark)?; let globals = GlobalsBuilder::standard().with(policy_builtins).build(); let module = Module::new(); - - let builder = RefCell::new(PolicyBuilder::new()); { let mut eval = Evaluator::new(&module); - eval.extra = Some(&builder); + eval.extra = Some(&self.builder); eval.eval_module(ast, &globals).map_err(Error::Starlark)?; } - Ok(builder.into_inner().build()) + Ok(()) + } + + pub fn build(self) -> crate::policy::Policy { + self.builder.into_inner().build() } } diff --git a/codex-rs/execpolicy2/src/policy.rs b/codex-rs/execpolicy2/src/policy.rs index d453404b2..12416b050 100644 --- a/codex-rs/execpolicy2/src/policy.rs +++ b/codex-rs/execpolicy2/src/policy.rs @@ -38,6 +38,28 @@ impl Policy { None => Evaluation::NoMatch, } } + + pub fn check_multiple(&self, commands: Commands) -> Evaluation + where + Commands: IntoIterator, + Commands::Item: AsRef<[String]>, + { + let matched_rules: Vec = commands + .into_iter() + .flat_map(|command| match self.check(command.as_ref()) { + Evaluation::Match { matched_rules, .. } => matched_rules, + Evaluation::NoMatch => Vec::new(), + }) + .collect(); + + match matched_rules.iter().map(RuleMatch::decision).max() { + Some(decision) => Evaluation::Match { + decision, + matched_rules, + }, + None => Evaluation::NoMatch, + } + } } #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] diff --git a/codex-rs/execpolicy2/tests/basic.rs b/codex-rs/execpolicy2/tests/basic.rs index 702ea6c0a..75921ecd5 100644 --- a/codex-rs/execpolicy2/tests/basic.rs +++ b/codex-rs/execpolicy2/tests/basic.rs @@ -41,7 +41,11 @@ prefix_rule( pattern = ["git", "status"], ) "#; - let policy = PolicyParser::parse("test.codexpolicy", policy_src).expect("parse policy"); + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); let cmd = tokens(&["git", "status"]); let evaluation = policy.check(&cmd); assert_eq!( @@ -56,6 +60,81 @@ prefix_rule( ); } +#[test] +fn parses_multiple_policy_files() { + let first_policy = r#" +prefix_rule( + pattern = ["git"], + decision = "prompt", +) + "#; + let second_policy = r#" +prefix_rule( + pattern = ["git", "commit"], + decision = "forbidden", +) + "#; + let mut parser = PolicyParser::new(); + parser + .parse("first.codexpolicy", first_policy) + .expect("parse policy"); + parser + .parse("second.codexpolicy", second_policy) + .expect("parse policy"); + let policy = parser.build(); + + let git_rules = rule_snapshots(policy.rules().get_vec("git").expect("git rules")); + assert_eq!( + vec![ + RuleSnapshot::Prefix(PrefixRule { + pattern: PrefixPattern { + first: Arc::from("git"), + rest: Vec::::new().into(), + }, + decision: Decision::Prompt, + }), + RuleSnapshot::Prefix(PrefixRule { + pattern: PrefixPattern { + first: Arc::from("git"), + rest: vec![PatternToken::Single("commit".to_string())].into(), + }, + decision: Decision::Forbidden, + }), + ], + git_rules + ); + + let status_eval = policy.check(&tokens(&["git", "status"])); + assert_eq!( + Evaluation::Match { + decision: Decision::Prompt, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: tokens(&["git"]), + decision: Decision::Prompt, + }], + }, + status_eval + ); + + let commit_eval = policy.check(&tokens(&["git", "commit", "-m", "hi"])); + assert_eq!( + Evaluation::Match { + decision: Decision::Forbidden, + matched_rules: vec![ + RuleMatch::PrefixRuleMatch { + matched_prefix: tokens(&["git"]), + decision: Decision::Prompt, + }, + RuleMatch::PrefixRuleMatch { + matched_prefix: tokens(&["git", "commit"]), + decision: Decision::Forbidden, + }, + ], + }, + commit_eval + ); +} + #[test] fn only_first_token_alias_expands_to_multiple_rules() { let policy_src = r#" @@ -63,7 +142,11 @@ prefix_rule( pattern = [["bash", "sh"], ["-c", "-l"]], ) "#; - let policy = PolicyParser::parse("test.codexpolicy", policy_src).expect("parse policy"); + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); let bash_rules = rule_snapshots(policy.rules().get_vec("bash").expect("bash rules")); let sh_rules = rule_snapshots(policy.rules().get_vec("sh").expect("sh rules")); @@ -120,7 +203,11 @@ prefix_rule( pattern = ["npm", ["i", "install"], ["--legacy-peer-deps", "--no-save"]], ) "#; - let policy = PolicyParser::parse("test.codexpolicy", policy_src).expect("parse policy"); + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); let rules = rule_snapshots(policy.rules().get_vec("npm").expect("npm rules")); assert_eq!( @@ -178,7 +265,11 @@ prefix_rule( ], ) "#; - let policy = PolicyParser::parse("test.codexpolicy", policy_src).expect("parse policy"); + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); let match_eval = policy.check(&tokens(&["git", "status"])); assert_eq!( Evaluation::Match { @@ -203,10 +294,6 @@ prefix_rule( #[test] fn strictest_decision_wins_across_matches() { let policy_src = r#" -prefix_rule( - pattern = ["git", "status"], - decision = "allow", -) prefix_rule( pattern = ["git"], decision = "prompt", @@ -216,25 +303,11 @@ prefix_rule( decision = "forbidden", ) "#; - let policy = PolicyParser::parse("test.codexpolicy", policy_src).expect("parse policy"); - - let status = policy.check(&tokens(&["git", "status"])); - assert_eq!( - Evaluation::Match { - decision: Decision::Prompt, - matched_rules: vec![ - RuleMatch::PrefixRuleMatch { - matched_prefix: tokens(&["git", "status"]), - decision: Decision::Allow, - }, - RuleMatch::PrefixRuleMatch { - matched_prefix: tokens(&["git"]), - decision: Decision::Prompt, - }, - ], - }, - status - ); + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); let commit = policy.check(&tokens(&["git", "commit", "-m", "hi"])); assert_eq!( @@ -254,3 +327,49 @@ prefix_rule( commit ); } + +#[test] +fn strictest_decision_across_multiple_commands() { + let policy_src = r#" +prefix_rule( + pattern = ["git"], + decision = "prompt", +) +prefix_rule( + pattern = ["git", "commit"], + decision = "forbidden", +) + "#; + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); + + let commands = vec![ + tokens(&["git", "status"]), + tokens(&["git", "commit", "-m", "hi"]), + ]; + + let evaluation = policy.check_multiple(&commands); + assert_eq!( + Evaluation::Match { + decision: Decision::Forbidden, + matched_rules: vec![ + RuleMatch::PrefixRuleMatch { + matched_prefix: tokens(&["git"]), + decision: Decision::Prompt, + }, + RuleMatch::PrefixRuleMatch { + matched_prefix: tokens(&["git"]), + decision: Decision::Prompt, + }, + RuleMatch::PrefixRuleMatch { + matched_prefix: tokens(&["git", "commit"]), + decision: Decision::Forbidden, + }, + ], + }, + evaluation + ); +}