From 43db77a97bffb3a0b0e8ef51ca447d1e86a4a72f Mon Sep 17 00:00:00 2001 From: kevin zhao Date: Thu, 13 Nov 2025 16:58:32 -0500 Subject: [PATCH] feat: implementing parse_many --- codex-rs/execpolicy2/README.md | 6 ++- codex-rs/execpolicy2/src/main.rs | 35 ++++++++------ codex-rs/execpolicy2/src/parser.rs | 29 ++++++++---- codex-rs/execpolicy2/tests/basic.rs | 73 +++++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 24 deletions(-) diff --git a/codex-rs/execpolicy2/README.md b/codex-rs/execpolicy2/README.md index e433f5e2cc..af1df5157d 100644 --- a/codex-rs/execpolicy2/README.md +++ b/codex-rs/execpolicy2/README.md @@ -45,10 +45,14 @@ 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 +``` - 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 c0786a320c..544c072bb7 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,8 @@ use codex_execpolicy2::PolicyParser; enum Cli { /// Evaluate a command against a policy. Check { - #[arg(short, long, value_name = "PATH")] - policy: PathBuf, + #[arg(short, long, value_name = "PATH", required = true)] + policies: Vec, /// Command tokens to check. #[arg( @@ -30,12 +29,12 @@ enum Cli { fn main() -> Result<()> { let cli = Cli::parse(); match cli { - Cli::Check { policy, command } => cmd_check(policy, command), + Cli::Check { policies, command } => cmd_check(policies, command), } } -fn cmd_check(policy_path: PathBuf, args: Vec) -> Result<()> { - let policy = load_policy(&policy_path)?; +fn cmd_check(policies: Vec, args: Vec) -> Result<()> { + let policy = load_policies(&policies)?; let eval = policy.check(&args); let json = serde_json::to_string_pretty(&eval)?; @@ -43,12 +42,20 @@ fn cmd_check(policy_path: PathBuf, args: Vec) -> Result<()> { 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 loaded_policies: Vec<(String, String)> = policy_paths + .iter() + .map(|policy_path| { + 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(); + Ok((policy_identifier, policy_file_contents)) + }) + .collect::>() + .context("failed to load policy files")?; + Ok(PolicyParser::parse_many(loaded_policies.iter().map( + |(policy_identifier, policy_file_contents)| { + (policy_identifier.as_str(), policy_file_contents.as_str()) + }, + ))?) } diff --git a/codex-rs/execpolicy2/src/parser.rs b/codex-rs/execpolicy2/src/parser.rs index b978781af2..5e7a078ef2 100644 --- a/codex-rs/execpolicy2/src/parser.rs +++ b/codex-rs/execpolicy2/src/parser.rs @@ -25,7 +25,6 @@ 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; impl PolicyParser { @@ -35,19 +34,31 @@ impl PolicyParser { policy_identifier: &str, policy_file_contents: &str, ) -> Result { + Self::parse_many([(policy_identifier, policy_file_contents)]) + } + + /// Parses multiple policy files and merges the resulting rules. + pub fn parse_many( + policies: Policies, + ) -> Result + where + Policies: IntoIterator, + Identifier: AsRef, + Contents: AsRef, + { let mut dialect = Dialect::Extended.clone(); dialect.enable_f_strings = true; - let ast = AstModule::parse( - policy_identifier, - policy_file_contents.to_string(), - &dialect, - ) - .map_err(Error::Starlark)?; let globals = GlobalsBuilder::standard().with(policy_builtins).build(); - let module = Module::new(); let builder = RefCell::new(PolicyBuilder::new()); - { + for (policy_identifier, policy_file_contents) in policies { + let ast = AstModule::parse( + policy_identifier.as_ref(), + policy_file_contents.as_ref().to_string(), + &dialect, + ) + .map_err(Error::Starlark)?; + let module = Module::new(); let mut eval = Evaluator::new(&module); eval.extra = Some(&builder); eval.eval_module(ast, &globals).map_err(Error::Starlark)?; diff --git a/codex-rs/execpolicy2/tests/basic.rs b/codex-rs/execpolicy2/tests/basic.rs index 8506af05eb..67fd650c17 100644 --- a/codex-rs/execpolicy2/tests/basic.rs +++ b/codex-rs/execpolicy2/tests/basic.rs @@ -56,6 +56,79 @@ 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 policy = PolicyParser::parse_many([ + ("first.codexpolicy", first_policy), + ("second.codexpolicy", second_policy), + ]) + .expect("parse policy"); + + 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#"