diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index d288c0f668..ce9ebb90a8 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1188,6 +1188,19 @@ dependencies = [ "tempfile", ] +[[package]] +name = "codex-execpolicy2" +version = "0.0.0" +dependencies = [ + "anyhow", + "log", + "serde", + "serde_json", + "shlex", + "starlark", + "thiserror 2.0.17", +] + [[package]] name = "codex-feedback" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index c50c69aa3f..38bbd59449 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -38,6 +38,7 @@ members = [ "utils/readiness", "utils/string", "utils/tokenizer", + "execpolicy2", ] resolver = "2" @@ -63,6 +64,7 @@ codex-chatgpt = { path = "chatgpt" } codex-common = { path = "common" } codex-core = { path = "core" } codex-exec = { path = "exec" } +codex-execpolicy2 = { path = "execpolicy2" } codex-feedback = { path = "feedback" } codex-file-search = { path = "file-search" } codex-git = { path = "utils/git" } diff --git a/codex-rs/execpolicy2/Cargo.toml b/codex-rs/execpolicy2/Cargo.toml new file mode 100644 index 0000000000..6ce9111f2d --- /dev/null +++ b/codex-rs/execpolicy2/Cargo.toml @@ -0,0 +1,24 @@ +[package] +name = "codex-execpolicy2" +version = "0.0.0" +edition = "2024" +license = "Apache-2.0" +description = "Codex exec policy v2: prefix-based Starlark rules for command decisions." + +[lib] +name = "codex_execpolicy2" +path = "src/lib.rs" + +[[bin]] +name = "codex-execpolicy2" +path = "src/main.rs" + +[dependencies] +anyhow = { workspace = true } +log = { workspace = true } +serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } +shlex = { workspace = true } +starlark = { workspace = true } +thiserror = { workspace = true } + diff --git a/codex-rs/execpolicy2/src/command.rs b/codex-rs/execpolicy2/src/command.rs new file mode 100644 index 0000000000..da7c230a63 --- /dev/null +++ b/codex-rs/execpolicy2/src/command.rs @@ -0,0 +1,9 @@ +use crate::error::Error; +use crate::error::Result; + +pub fn tokenize_command(raw: &str) -> Result> { + shlex::split(raw).ok_or_else(|| Error::TokenizationFailed { + example: raw.to_string(), + reason: "invalid shell tokens".to_string(), + }) +} diff --git a/codex-rs/execpolicy2/src/decision.rs b/codex-rs/execpolicy2/src/decision.rs new file mode 100644 index 0000000000..10b0fac1a4 --- /dev/null +++ b/codex-rs/execpolicy2/src/decision.rs @@ -0,0 +1,33 @@ +use serde::Deserialize; +use serde::Serialize; + +use crate::error::Error; +use crate::error::Result; + +#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum Decision { + Allow, + Prompt, + Forbidden, +} + +impl Decision { + pub fn parse(raw: &str) -> Result { + match raw { + "allow" => Ok(Self::Allow), + "prompt" => Ok(Self::Prompt), + "forbidden" => Ok(Self::Forbidden), + other => Err(Error::InvalidDecision(other.to_string())), + } + } + + /// Returns true if `self` is stricter (less permissive) than `other`. + pub fn is_stricter_than(self, other: Self) -> bool { + matches!( + (self, other), + (Decision::Forbidden, Decision::Prompt | Decision::Allow) + | (Decision::Prompt, Decision::Allow) + ) + } +} diff --git a/codex-rs/execpolicy2/src/default.policy b/codex-rs/execpolicy2/src/default.policy new file mode 100644 index 0000000000..54dd006cec --- /dev/null +++ b/codex-rs/execpolicy2/src/default.policy @@ -0,0 +1,36 @@ +prefix_rule( + id = "git_status", + pattern = ["git", "status"], + match = [ + "git status", + "git status -- path/to/file", + ], + not_match = [ + "git statusx", + "git reset --hard", + ], +) + +prefix_rule( + id = "npm_install", + pattern = ["npm", ["i", "install"]], + decision = "prompt", + match = [ + "npm i", + "npm install", + "npm install lodash", + ], + not_match = [ + "npmx install", + "npm outdated", + ], +) + +prefix_rule( + id = "git_reset_hard", + pattern = ["git", "reset", "--hard"], + decision = "forbidden", + match = [ + "git reset --hard", + ], +) diff --git a/codex-rs/execpolicy2/src/error.rs b/codex-rs/execpolicy2/src/error.rs new file mode 100644 index 0000000000..e572ff29dd --- /dev/null +++ b/codex-rs/execpolicy2/src/error.rs @@ -0,0 +1,19 @@ +use thiserror::Error; + +pub type Result = std::result::Result; + +#[derive(Debug, Error)] +pub enum Error { + #[error("invalid decision: {0}")] + InvalidDecision(String), + #[error("invalid pattern element: {0}")] + InvalidPattern(String), + #[error("failed to tokenize example `{example}`: {reason}")] + TokenizationFailed { example: String, reason: String }, + #[error("expected example to match rule `{rule_id}`: {example}")] + ExampleDidNotMatch { rule_id: String, example: String }, + #[error("expected example to not match rule `{rule_id}`: {example}")] + ExampleDidMatch { rule_id: String, example: String }, + #[error("starlark error: {0}")] + Starlark(String), +} diff --git a/codex-rs/execpolicy2/src/lib.rs b/codex-rs/execpolicy2/src/lib.rs new file mode 100644 index 0000000000..476f79b7f2 --- /dev/null +++ b/codex-rs/execpolicy2/src/lib.rs @@ -0,0 +1,23 @@ +pub mod command; +pub mod decision; +pub mod error; +pub mod parser; +pub mod policy; +pub mod rule; + +pub use command::tokenize_command; +pub use decision::Decision; +pub use error::Error; +pub use error::Result; +pub use parser::PolicyParser; +pub use policy::Evaluation; +pub use policy::Policy; +pub use rule::Rule; +pub use rule::RuleMatch; + +/// Load the default bundled policy. +pub fn load_default_policy() -> Result { + let policy_src = include_str!("default.policy"); + let parser = PolicyParser::new("default.policy", policy_src); + parser.parse() +} diff --git a/codex-rs/execpolicy2/src/main.rs b/codex-rs/execpolicy2/src/main.rs new file mode 100644 index 0000000000..04030a7b36 --- /dev/null +++ b/codex-rs/execpolicy2/src/main.rs @@ -0,0 +1,85 @@ +use std::fs; +use std::path::Path; + +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use codex_execpolicy2::PolicyParser; +use codex_execpolicy2::load_default_policy; +use codex_execpolicy2::tokenize_command; + +fn main() -> Result<()> { + let mut args = std::env::args().skip(1); + let mut policy_path: Option = None; + + while let Some(arg) = args.next() { + if arg == "--policy" || arg == "-p" { + let path = args + .next() + .context("expected a policy path after --policy/-p")?; + policy_path = Some(path); + continue; + } + // First non-flag argument is the subcommand. + let subcommand = arg; + return run_subcommand(subcommand, policy_path, args.collect()); + } + + print_usage(); + bail!("missing subcommand") +} + +fn run_subcommand( + subcommand: String, + policy_path: Option, + args: Vec, +) -> Result<()> { + match subcommand.as_str() { + "check" => cmd_check(policy_path, args), + _ => { + print_usage(); + bail!("unknown subcommand: {subcommand}") + } + } +} + +fn cmd_check(policy_path: Option, args: Vec) -> Result<()> { + if args.is_empty() { + bail!("usage: codex-execpolicy2 check "); + } + let policy = load_policy(policy_path)?; + + let tokens = if args.len() == 1 { + tokenize_command(&args[0])? + } else { + args + }; + + match policy.evaluate(&tokens) { + Some(eval) => { + let json = serde_json::to_string_pretty(&eval)?; + println!("{json}"); + } + None => { + println!("no match"); + } + } + Ok(()) +} + +fn load_policy(policy_path: Option) -> Result { + if let Some(path) = policy_path { + let content = fs::read_to_string(&path) + .with_context(|| format!("failed to read policy at {}", Path::new(&path).display()))?; + let parser = PolicyParser::new(&path, &content); + return Ok(parser.parse()?); + } + Ok(load_default_policy()?) +} + +fn print_usage() { + eprintln!( + "usage: + codex-execpolicy2 [--policy path] check " + ); +} diff --git a/codex-rs/execpolicy2/src/parser.rs b/codex-rs/execpolicy2/src/parser.rs new file mode 100644 index 0000000000..1823136501 --- /dev/null +++ b/codex-rs/execpolicy2/src/parser.rs @@ -0,0 +1,205 @@ +use std::cell::RefCell; + +use starlark::any::ProvidesStaticType; +use starlark::environment::GlobalsBuilder; +use starlark::environment::Module; +use starlark::eval::Evaluator; +use starlark::starlark_module; +use starlark::syntax::AstModule; +use starlark::syntax::Dialect; +use starlark::values::Value; +use starlark::values::list::ListRef; +use starlark::values::list::UnpackList; +use starlark::values::none::NoneType; + +use crate::command::tokenize_command; +use crate::decision::Decision; +use crate::error::Error; +use crate::error::Result; +use crate::rule::Rule; + +pub struct PolicyParser { + policy_source: String, + unparsed_policy: String, +} + +impl PolicyParser { + pub fn new(policy_source: &str, unparsed_policy: &str) -> Self { + Self { + policy_source: policy_source.to_string(), + unparsed_policy: unparsed_policy.to_string(), + } + } + + pub fn parse(&self) -> Result { + let mut dialect = Dialect::Extended.clone(); + dialect.enable_f_strings = true; + let ast = AstModule::parse(&self.policy_source, self.unparsed_policy.clone(), &dialect) + .map_err(|e| Error::Starlark(e.to_string()))?; + let globals = GlobalsBuilder::standard().with(policy_builtins).build(); + let module = Module::new(); + + let builder = PolicyBuilder::new(); + { + let mut eval = Evaluator::new(&module); + eval.extra = Some(&builder); + eval.eval_module(ast, &globals) + .map_err(|e| Error::Starlark(e.to_string()))?; + } + Ok(builder.build()) + } +} + +#[derive(Debug, ProvidesStaticType)] +struct PolicyBuilder { + rules: RefCell>, + next_auto_id: RefCell, +} + +impl PolicyBuilder { + fn new() -> Self { + Self { + rules: RefCell::new(Vec::new()), + next_auto_id: RefCell::new(0), + } + } + + fn alloc_id(&self) -> String { + let mut next = self.next_auto_id.borrow_mut(); + let id = *next; + *next += 1; + format!("rule_{id}") + } + + fn add_rule(&self, rule: Rule) { + self.rules.borrow_mut().push(rule); + } + + fn build(&self) -> crate::policy::Policy { + crate::policy::Policy::new(self.rules.borrow().clone()) + } +} + +#[derive(Debug)] +enum PatternPart { + Single(String), + Alts(Vec), +} + +fn expand_pattern(parts: &[PatternPart]) -> Vec> { + let mut acc: Vec> = vec![Vec::new()]; + for part in parts { + let alts: Vec = match part { + PatternPart::Single(s) => vec![s.clone()], + PatternPart::Alts(v) => v.clone(), + }; + let mut next = Vec::new(); + for prefix in &acc { + for alt in &alts { + let mut combined = prefix.clone(); + combined.push(alt.clone()); + next.push(combined); + } + } + acc = next; + } + acc +} + +fn parse_pattern<'v>(pattern: UnpackList>) -> Result>> { + let mut parts = Vec::new(); + for item in pattern.items { + if let Some(s) = item.unpack_str() { + parts.push(PatternPart::Single(s.to_string())); + continue; + } + let mut alts = Vec::new(); + if let Some(list) = ListRef::from_value(item) { + for value in list.content() { + let s = value.unpack_str().ok_or_else(|| { + Error::InvalidPattern("pattern alternative must be a string".to_string()) + })?; + alts.push(s.to_string()); + } + } else { + return Err(Error::InvalidPattern( + "pattern element must be a string or list of strings".to_string(), + )); + } + if alts.is_empty() { + return Err(Error::InvalidPattern( + "pattern alternatives cannot be empty".to_string(), + )); + } + parts.push(PatternPart::Alts(alts)); + } + Ok(expand_pattern(&parts)) +} + +#[starlark_module] +fn policy_builtins(builder: &mut GlobalsBuilder) { + fn prefix_rule<'v>( + pattern: UnpackList>, + decision: Option<&'v str>, + r#match: Option>, + not_match: Option>, + id: Option<&'v str>, + eval: &mut Evaluator<'v, '_, '_>, + ) -> anyhow::Result { + let decision = match decision { + Some(raw) => Decision::parse(raw)?, + None => Decision::Allow, + }; + + let prefixes = parse_pattern(pattern)?; + + let positive_examples: Vec> = r#match + .map(|examples| { + examples + .items + .into_iter() + .map(tokenize_command) + .collect::>>() + }) + .transpose()? + .unwrap_or_default(); + let negative_examples: Vec> = not_match + .map(|examples| { + examples + .items + .into_iter() + .map(tokenize_command) + .collect::>>() + }) + .transpose()? + .unwrap_or_default(); + + let id = id.map(std::string::ToString::to_string).unwrap_or_else(|| { + #[expect(clippy::unwrap_used)] + let builder = eval + .extra + .as_ref() + .unwrap() + .downcast_ref::() + .unwrap(); + builder.alloc_id() + }); + + let rule = Rule { + id: id.clone(), + prefixes, + decision, + }; + rule.validate_examples(&positive_examples, &negative_examples)?; + + #[expect(clippy::unwrap_used)] + let builder = eval + .extra + .as_ref() + .unwrap() + .downcast_ref::() + .unwrap(); + builder.add_rule(rule); + Ok(NoneType) + } +} diff --git a/codex-rs/execpolicy2/src/policy.rs b/codex-rs/execpolicy2/src/policy.rs new file mode 100644 index 0000000000..8f9e105ddb --- /dev/null +++ b/codex-rs/execpolicy2/src/policy.rs @@ -0,0 +1,59 @@ +use crate::decision::Decision; +use crate::rule::Rule; +use crate::rule::RuleMatch; +use serde::Deserialize; +use serde::Serialize; + +#[derive(Clone, Debug)] +pub struct Policy { + rules: Vec, +} + +#[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, + } + } +} + +impl Policy { + pub fn new(rules: Vec) -> Self { + Self { rules } + } + + pub fn rules(&self) -> &[Rule] { + &self.rules + } + + pub fn evaluate(&self, cmd: &[String]) -> Option { + let mut best: 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), + Some(current) => { + if eval.decision.is_stricter_than(current.decision) { + Some(eval) + } else { + Some(current) + } + } + }; + } + } + best + } +} diff --git a/codex-rs/execpolicy2/src/rule.rs b/codex-rs/execpolicy2/src/rule.rs new file mode 100644 index 0000000000..0bbad97140 --- /dev/null +++ b/codex-rs/execpolicy2/src/rule.rs @@ -0,0 +1,66 @@ +use crate::decision::Decision; +use crate::error::Error; +use crate::error::Result; + +#[derive(Clone, Debug)] +pub struct Rule { + pub id: String, + pub prefixes: Vec>, + pub decision: Decision, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct RuleMatch { + pub rule_id: String, + pub matched_prefix: Vec, + pub remainder: Vec, + pub decision: Decision, +} + +impl Rule { + pub fn matches(&self, cmd: &[String]) -> Option { + for prefix in &self.prefixes { + if prefix.len() > cmd.len() { + continue; + } + if cmd + .iter() + .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, + }); + } + } + None + } + + pub fn validate_examples( + &self, + positive: &[Vec], + negative: &[Vec], + ) -> Result<()> { + for example in positive { + if self.matches(example).is_none() { + return Err(Error::ExampleDidNotMatch { + rule_id: self.id.clone(), + example: example.join(" "), + }); + } + } + for example in negative { + if self.matches(example).is_some() { + return Err(Error::ExampleDidMatch { + rule_id: self.id.clone(), + example: example.join(" "), + }); + } + } + Ok(()) + } +} diff --git a/codex-rs/execpolicy2/tests/basic.rs b/codex-rs/execpolicy2/tests/basic.rs new file mode 100644 index 0000000000..1aff08c000 --- /dev/null +++ b/codex-rs/execpolicy2/tests/basic.rs @@ -0,0 +1,90 @@ +use codex_execpolicy2::Decision; +use codex_execpolicy2::PolicyParser; +use codex_execpolicy2::tokenize_command; + +#[test] +fn matches_default_git_status() { + let policy = codex_execpolicy2::load_default_policy().expect("parse"); + 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"); +} + +#[test] +fn pattern_expands_alternatives() { + let policy_src = r#" +prefix_rule( + id = "npm_install", + pattern = ["npm", ["i", "install"]], +) + "#; + let parser = PolicyParser::new("test.policy", policy_src); + let policy = parser.parse().expect("parse policy"); + + 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 no_match = tokenize_command("npmx install").expect("tokenize"); + assert!(policy.evaluate(&no_match).is_none()); +} + +#[test] +fn match_and_not_match_examples_are_enforced() { + let policy_src = r#" +prefix_rule( + id = "git_status", + pattern = ["git", "status"], + match = ["git status"], + not_match = ["git reset --hard"], +) + "#; + let parser = PolicyParser::new("test.policy", policy_src); + let policy = parser.parse().expect("parse policy"); + assert!( + policy + .evaluate(&tokenize_command("git status").expect("tokenize")) + .is_some() + ); + assert!( + policy + .evaluate(&tokenize_command("git reset --hard").expect("tokenize")) + .is_none() + ); +} + +#[test] +fn strictest_decision_wins_across_matches() { + let policy_src = r#" +prefix_rule( + id = "allow_git_status", + pattern = ["git", "status"], + decision = "allow", +) +prefix_rule( + id = "prompt_git", + pattern = ["git"], + decision = "prompt", +) +prefix_rule( + id = "forbid_git_commit", + pattern = ["git", "commit"], + decision = "forbidden", +) + "#; + let parser = PolicyParser::new("test.policy", policy_src); + let policy = parser.parse().expect("parse policy"); + + 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"); + + 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"); +}