diff --git a/codex-rs/execpolicy2/src/policy.rs b/codex-rs/execpolicy2/src/policy.rs index 8f9e105ddb..bc0765f5e9 100644 --- a/codex-rs/execpolicy2/src/policy.rs +++ b/codex-rs/execpolicy2/src/policy.rs @@ -1,6 +1,5 @@ use crate::decision::Decision; use crate::rule::Rule; -use crate::rule::RuleMatch; use serde::Deserialize; use serde::Serialize; @@ -11,21 +10,8 @@ pub struct Policy { #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] pub struct Evaluation { - pub rule_id: String, pub decision: Decision, - pub matched_prefix: Vec, - pub remainder: Vec, -} - -impl From for Evaluation { - fn from(value: RuleMatch) -> Self { - Self { - rule_id: value.rule_id, - decision: value.decision, - matched_prefix: value.matched_prefix, - remainder: value.remainder, - } - } + pub matched_rules: Vec, } impl Policy { @@ -38,22 +24,27 @@ impl Policy { } pub fn evaluate(&self, cmd: &[String]) -> Option { - let mut best: Option = None; + let mut matched_rules: Vec = Vec::new(); + let mut best_decision: Option = None; for rule in &self.rules { if let Some(matched) = rule.matches(cmd) { - let eval = Evaluation::from(matched); - best = match best { - None => Some(eval), + let decision = match best_decision { + None => matched.decision, Some(current) => { - if eval.decision.is_stricter_than(current.decision) { - Some(eval) + if matched.decision.is_stricter_than(current) { + matched.decision } else { - Some(current) + current } } }; + best_decision = Some(decision); + matched_rules.push(matched); } } - best + best_decision.map(|decision| Evaluation { + decision, + matched_rules, + }) } } diff --git a/codex-rs/execpolicy2/src/rule.rs b/codex-rs/execpolicy2/src/rule.rs index 0bbad97140..50b2601863 100644 --- a/codex-rs/execpolicy2/src/rule.rs +++ b/codex-rs/execpolicy2/src/rule.rs @@ -1,6 +1,8 @@ use crate::decision::Decision; use crate::error::Error; use crate::error::Result; +use serde::Deserialize; +use serde::Serialize; #[derive(Clone, Debug)] pub struct Rule { @@ -9,11 +11,10 @@ pub struct Rule { pub decision: Decision, } -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] pub struct RuleMatch { pub rule_id: String, pub matched_prefix: Vec, - pub remainder: Vec, pub decision: Decision, } @@ -28,11 +29,9 @@ impl Rule { .zip(prefix) .all(|(cmd_tok, prefix_tok)| cmd_tok == prefix_tok) { - let remainder = cmd[prefix.len()..].to_vec(); return Some(RuleMatch { rule_id: self.id.clone(), matched_prefix: prefix.clone(), - remainder, decision: self.decision, }); } diff --git a/codex-rs/execpolicy2/tests/basic.rs b/codex-rs/execpolicy2/tests/basic.rs index 1aff08c000..7b69a93d15 100644 --- a/codex-rs/execpolicy2/tests/basic.rs +++ b/codex-rs/execpolicy2/tests/basic.rs @@ -1,5 +1,6 @@ use codex_execpolicy2::Decision; use codex_execpolicy2::PolicyParser; +use codex_execpolicy2::RuleMatch; use codex_execpolicy2::tokenize_command; #[test] @@ -8,7 +9,14 @@ fn matches_default_git_status() { let cmd = tokenize_command("git status").expect("tokenize"); let eval = policy.evaluate(&cmd).expect("match"); assert_eq!(eval.decision, Decision::Allow); - assert_eq!(eval.rule_id, "git_status"); + assert_eq!( + eval.matched_rules, + vec![RuleMatch { + rule_id: "git_status".to_string(), + matched_prefix: vec!["git".to_string(), "status".to_string()], + decision: Decision::Allow, + }] + ); } #[test] @@ -25,7 +33,19 @@ prefix_rule( for cmd in ["npm i", "npm install"] { let tokens = tokenize_command(cmd).expect("tokenize"); let eval = policy.evaluate(&tokens).expect("match"); - assert_eq!(eval.rule_id, "npm_install"); + let matched_prefix = if cmd.ends_with(" i") { + vec!["npm".to_string(), "i".to_string()] + } else { + vec!["npm".to_string(), "install".to_string()] + }; + assert_eq!( + eval.matched_rules, + vec![RuleMatch { + rule_id: "npm_install".to_string(), + matched_prefix, + decision: Decision::Allow, + }] + ); } let no_match = tokenize_command("npmx install").expect("tokenize"); @@ -81,10 +101,61 @@ prefix_rule( let status = tokenize_command("git status").expect("tokenize"); let status_eval = policy.evaluate(&status).expect("match"); assert_eq!(status_eval.decision, Decision::Prompt); - assert_eq!(status_eval.rule_id, "prompt_git"); + assert_eq!( + status_eval.matched_rules, + vec![ + RuleMatch { + rule_id: "allow_git_status".to_string(), + matched_prefix: vec!["git".to_string(), "status".to_string()], + decision: Decision::Allow, + }, + RuleMatch { + rule_id: "prompt_git".to_string(), + matched_prefix: vec!["git".to_string()], + decision: Decision::Prompt, + } + ] + ); let commit = tokenize_command("git commit -m hi").expect("tokenize"); let commit_eval = policy.evaluate(&commit).expect("match"); assert_eq!(commit_eval.decision, Decision::Forbidden); - assert_eq!(commit_eval.rule_id, "forbid_git_commit"); + assert_eq!( + commit_eval.matched_rules, + vec![ + RuleMatch { + rule_id: "prompt_git".to_string(), + matched_prefix: vec!["git".to_string()], + decision: Decision::Prompt, + }, + RuleMatch { + rule_id: "forbid_git_commit".to_string(), + matched_prefix: vec!["git".to_string(), "commit".to_string()], + decision: Decision::Forbidden, + } + ] + ); +} + +#[test] +fn unnamed_rule_uses_source_as_name() { + let policy_src = r#" +prefix_rule( + id = "unnamed_rule", + pattern = ["echo"], +) + "#; + let parser = PolicyParser::new("test.policy", policy_src); + let policy = parser.parse().expect("parse policy"); + let eval = policy + .evaluate(&tokenize_command("echo hi").expect("tokenize")) + .expect("match"); + assert_eq!( + eval.matched_rules, + vec![RuleMatch { + rule_id: "unnamed_rule".to_string(), + matched_prefix: vec!["echo".to_string()], + decision: Decision::Allow, + }] + ); }