diff --git a/codex-rs/execpolicy/README.md b/codex-rs/execpolicy/README.md index 30fc57184b..c20b92371d 100644 --- a/codex-rs/execpolicy/README.md +++ b/codex-rs/execpolicy/README.md @@ -2,11 +2,13 @@ ## Overview -- Policy engine and CLI built around `prefix_rule(pattern=[...], decision?, justification?, match?, not_match?)`. -- This release covers the prefix-rule subset of the execpolicy language; a richer language will follow. +- Policy engine and CLI built around two Starlark rule forms: + - `prefix_rule(pattern=[...], decision?, justification?, match?, not_match?)` + - `network_rule(host=..., protocol=..., decision=..., justification?)` - Tokens are matched in order; any `pattern` element may be a list to denote alternatives. `decision` defaults to `allow`; valid values: `allow`, `prompt`, `forbidden`. - `justification` is an optional human-readable rationale for why a rule exists. It can be provided for any `decision` and may be surfaced in different contexts (for example, in approval prompts or rejection messages). When `decision = "forbidden"` is used, include a recommended alternative in the `justification`, when appropriate (e.g., ``"Use `jj` instead of `git`."``). - `match` / `not_match` supply example invocations that are validated at load time (think of them as unit tests); examples can be token arrays or strings (strings are tokenized with `shlex`). +- `network_rule` entries are consumed by `codex-network-proxy` for per-host network decisions. They are exact-host (normalized) matches only, with `protocol` limited to `http|https` and `decision` limited to `allow|deny|ask`. - The CLI always prints the JSON serialization of the evaluation result. - The legacy rule matcher lives in `codex-execpolicy-legacy`. @@ -24,6 +26,17 @@ prefix_rule( ) ``` +- Network rules use Starlark syntax: + +```starlark +network_rule( + host = "api.example.com", # exact host match after normalization + protocol = "https", # http | https + decision = "allow", # allow | deny | ask + justification = "Allow API calls", +) +``` + ## CLI - From the Codex CLI, run `codex execpolicy check` subcommand with one or more policy files (for example `src/default.rules`) to check a command: diff --git a/codex-rs/execpolicy/src/amend.rs b/codex-rs/execpolicy/src/amend.rs index 9114d3a64f..c7e461ab23 100644 --- a/codex-rs/execpolicy/src/amend.rs +++ b/codex-rs/execpolicy/src/amend.rs @@ -22,6 +22,12 @@ pub enum AmendError { }, #[error("failed to format prefix tokens: {source}")] SerializePrefix { source: serde_json::Error }, + #[error("network rule host cannot be empty")] + EmptyNetworkHost, + #[error("network rule protocol must be http or https")] + InvalidNetworkProtocol, + #[error("network rule decision must be allow, deny, or ask")] + InvalidNetworkDecision, #[error("failed to open policy file {path}: {source}")] OpenPolicyFile { path: PathBuf, @@ -90,6 +96,53 @@ pub fn blocking_append_allow_prefix_rule( append_locked_line(policy_path, &rule) } +/// Append a `network_rule(...)` line to the policy file. +pub fn blocking_append_network_rule( + policy_path: &Path, + host: &str, + protocol: &str, + decision: &str, + justification: Option<&str>, +) -> Result<(), AmendError> { + let host = host.trim(); + if host.is_empty() { + return Err(AmendError::EmptyNetworkHost); + } + if !matches!(protocol, "http" | "https") { + return Err(AmendError::InvalidNetworkProtocol); + } + if !matches!(decision, "allow" | "deny" | "ask") { + return Err(AmendError::InvalidNetworkDecision); + } + let host = + serde_json::to_string(host).map_err(|source| AmendError::SerializePrefix { source })?; + let mut rule = + format!(r#"network_rule(host={host}, protocol="{protocol}", decision="{decision}""#); + if let Some(justification) = justification { + let justification = serde_json::to_string(justification) + .map_err(|source| AmendError::SerializePrefix { source })?; + rule.push_str(&format!(", justification={justification}")); + } + rule.push(')'); + + let dir = policy_path + .parent() + .ok_or_else(|| AmendError::MissingParent { + path: policy_path.to_path_buf(), + })?; + match std::fs::create_dir(dir) { + Ok(()) => {} + Err(ref source) if source.kind() == std::io::ErrorKind::AlreadyExists => {} + Err(source) => { + return Err(AmendError::CreatePolicyDir { + dir: dir.to_path_buf(), + source, + }); + } + } + append_locked_line(policy_path, &rule) +} + fn append_locked_line(policy_path: &Path, line: &str) -> Result<(), AmendError> { let mut file = OpenOptions::new() .create(true) @@ -222,4 +275,74 @@ prefix_rule(pattern=["echo", "Hello, world!"], decision="allow") "# ); } + + #[test] + fn appends_network_rule_and_creates_directories() { + let tmp = tempdir().expect("create temp dir"); + let policy_path = tmp.path().join("rules").join("default.rules"); + + blocking_append_network_rule( + &policy_path, + "api.example.com", + "https", + "allow", + Some("Allow API calls"), + ) + .expect("append network rule"); + + let contents = std::fs::read_to_string(&policy_path).expect("default.rules should exist"); + assert_eq!( + contents, + r#"network_rule(host="api.example.com", protocol="https", decision="allow", justification="Allow API calls") +"# + ); + } + + #[test] + fn appends_network_rule_without_duplicate_newline() { + let tmp = tempdir().expect("create temp dir"); + let policy_path = tmp.path().join("rules").join("default.rules"); + std::fs::create_dir_all( + policy_path + .parent() + .expect("policy path should have parent"), + ) + .expect("create policy dir"); + std::fs::write( + &policy_path, + r#"prefix_rule(pattern=["ls"], decision="allow") +"#, + ) + .expect("write seed rule"); + + blocking_append_network_rule(&policy_path, "api.example.com", "https", "allow", None) + .expect("append network rule"); + + let contents = std::fs::read_to_string(&policy_path).expect("read policy"); + assert_eq!( + contents, + r#"prefix_rule(pattern=["ls"], decision="allow") +network_rule(host="api.example.com", protocol="https", decision="allow") +"# + ); + } + + #[test] + fn rejects_invalid_network_rule_inputs() { + let tmp = tempdir().expect("create temp dir"); + let policy_path = tmp.path().join("rules").join("default.rules"); + + assert!(matches!( + blocking_append_network_rule(&policy_path, " ", "https", "allow", None), + Err(AmendError::EmptyNetworkHost) + )); + assert!(matches!( + blocking_append_network_rule(&policy_path, "api.example.com", "socks5", "allow", None), + Err(AmendError::InvalidNetworkProtocol) + )); + assert!(matches!( + blocking_append_network_rule(&policy_path, "api.example.com", "https", "prompt", None), + Err(AmendError::InvalidNetworkDecision) + )); + } } diff --git a/codex-rs/execpolicy/src/lib.rs b/codex-rs/execpolicy/src/lib.rs index a9453f28d3..2da6502ab6 100644 --- a/codex-rs/execpolicy/src/lib.rs +++ b/codex-rs/execpolicy/src/lib.rs @@ -8,6 +8,7 @@ pub mod rule; pub use amend::AmendError; pub use amend::blocking_append_allow_prefix_rule; +pub use amend::blocking_append_network_rule; pub use decision::Decision; pub use error::Error; pub use error::ErrorLocation; @@ -18,6 +19,9 @@ pub use execpolicycheck::ExecPolicyCheckCommand; pub use parser::PolicyParser; pub use policy::Evaluation; pub use policy::Policy; +pub use rule::NetworkRule; +pub use rule::NetworkRuleDecision; +pub use rule::NetworkRuleProtocol; pub use rule::Rule; pub use rule::RuleMatch; pub use rule::RuleRef; diff --git a/codex-rs/execpolicy/src/parser.rs b/codex-rs/execpolicy/src/parser.rs index 0ff0f4b34a..150951c9b9 100644 --- a/codex-rs/execpolicy/src/parser.rs +++ b/codex-rs/execpolicy/src/parser.rs @@ -18,6 +18,9 @@ use std::sync::Arc; use crate::decision::Decision; use crate::error::Error; use crate::error::Result; +use crate::rule::NetworkRule; +use crate::rule::NetworkRuleDecision; +use crate::rule::NetworkRuleProtocol; use crate::rule::PatternToken; use crate::rule::PrefixPattern; use crate::rule::PrefixRule; @@ -71,12 +74,14 @@ impl PolicyParser { #[derive(Debug, ProvidesStaticType)] struct PolicyBuilder { rules_by_program: MultiMap, + network_rules: Vec, } impl PolicyBuilder { fn new() -> Self { Self { rules_by_program: MultiMap::new(), + network_rules: Vec::new(), } } @@ -85,8 +90,12 @@ impl PolicyBuilder { .insert(rule.program().to_string(), rule); } + fn add_network_rule(&mut self, rule: NetworkRule) { + self.network_rules.push(rule); + } + fn build(self) -> crate::policy::Policy { - crate::policy::Policy::new(self.rules_by_program) + crate::policy::Policy::new(self.rules_by_program, self.network_rules) } } @@ -266,4 +275,35 @@ fn policy_builtins(builder: &mut GlobalsBuilder) { rules.into_iter().for_each(|rule| builder.add_rule(rule)); Ok(NoneType) } + + fn network_rule<'v>( + host: &'v str, + protocol: &'v str, + decision: &'v str, + justification: Option<&'v str>, + eval: &mut Evaluator<'v, '_, '_>, + ) -> anyhow::Result { + let host = host.trim(); + if host.is_empty() { + return Err(Error::InvalidRule("host cannot be empty".to_string()).into()); + } + + let justification = match justification { + Some(raw) if raw.trim().is_empty() => { + return Err(Error::InvalidRule("justification cannot be empty".to_string()).into()); + } + Some(raw) => Some(raw.to_string()), + None => None, + }; + + let rule = NetworkRule { + host: host.to_string(), + protocol: NetworkRuleProtocol::parse(protocol)?, + decision: NetworkRuleDecision::parse(decision)?, + justification, + }; + + policy_builder(eval).add_network_rule(rule); + Ok(NoneType) + } } diff --git a/codex-rs/execpolicy/src/policy.rs b/codex-rs/execpolicy/src/policy.rs index 0da0332d0b..6c03e55619 100644 --- a/codex-rs/execpolicy/src/policy.rs +++ b/codex-rs/execpolicy/src/policy.rs @@ -1,6 +1,7 @@ use crate::decision::Decision; use crate::error::Error; use crate::error::Result; +use crate::rule::NetworkRule; use crate::rule::PatternToken; use crate::rule::PrefixPattern; use crate::rule::PrefixRule; @@ -16,21 +17,32 @@ type HeuristicsFallback<'a> = Option<&'a dyn Fn(&[String]) -> Decision>; #[derive(Clone, Debug)] pub struct Policy { rules_by_program: MultiMap, + network_rules: Vec, } impl Policy { - pub fn new(rules_by_program: MultiMap) -> Self { - Self { rules_by_program } + pub fn new( + rules_by_program: MultiMap, + network_rules: Vec, + ) -> Self { + Self { + rules_by_program, + network_rules, + } } pub fn empty() -> Self { - Self::new(MultiMap::new()) + Self::new(MultiMap::new(), Vec::new()) } pub fn rules(&self) -> &MultiMap { &self.rules_by_program } + pub fn network_rules(&self) -> &[NetworkRule] { + &self.network_rules + } + pub fn get_allowed_prefixes(&self) -> Vec> { let mut prefixes = Vec::new(); @@ -77,6 +89,10 @@ impl Policy { Ok(()) } + pub fn add_network_rule(&mut self, rule: NetworkRule) { + self.network_rules.push(rule); + } + pub fn check(&self, cmd: &[String], heuristics_fallback: &F) -> Evaluation where F: Fn(&[String]) -> Decision, diff --git a/codex-rs/execpolicy/src/rule.rs b/codex-rs/execpolicy/src/rule.rs index b7c1a7cfde..a66975b520 100644 --- a/codex-rs/execpolicy/src/rule.rs +++ b/codex-rs/execpolicy/src/rule.rs @@ -92,6 +92,55 @@ pub struct PrefixRule { pub justification: Option, } +#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum NetworkRuleProtocol { + Http, + Https, +} + +impl NetworkRuleProtocol { + pub fn parse(raw: &str) -> Result { + match raw { + "http" => Ok(Self::Http), + "https" => Ok(Self::Https), + other => Err(Error::InvalidRule(format!( + "invalid network protocol: {other}" + ))), + } + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum NetworkRuleDecision { + Allow, + Deny, + Ask, +} + +impl NetworkRuleDecision { + pub fn parse(raw: &str) -> Result { + match raw { + "allow" => Ok(Self::Allow), + "deny" => Ok(Self::Deny), + "ask" => Ok(Self::Ask), + other => Err(Error::InvalidRule(format!( + "invalid network decision: {other}" + ))), + } + } +} + +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct NetworkRule { + pub host: String, + pub protocol: NetworkRuleProtocol, + pub decision: NetworkRuleDecision, + pub justification: Option, +} + pub trait Rule: Any + Debug + Send + Sync { fn program(&self) -> &str; diff --git a/codex-rs/execpolicy/tests/basic.rs b/codex-rs/execpolicy/tests/basic.rs index ed6cf3185e..2de1221134 100644 --- a/codex-rs/execpolicy/tests/basic.rs +++ b/codex-rs/execpolicy/tests/basic.rs @@ -6,6 +6,9 @@ use anyhow::Result; use codex_execpolicy::Decision; use codex_execpolicy::Error; use codex_execpolicy::Evaluation; +use codex_execpolicy::NetworkRule; +use codex_execpolicy::NetworkRuleDecision; +use codex_execpolicy::NetworkRuleProtocol; use codex_execpolicy::Policy; use codex_execpolicy::PolicyParser; use codex_execpolicy::RuleMatch; @@ -72,6 +75,114 @@ prefix_rule( Ok(()) } +#[test] +fn parses_network_rule() -> Result<()> { + let policy_src = r#" +network_rule( + host = "api.example.com", + protocol = "https", + decision = "allow", + justification = "Allow API calls", +) + "#; + + let mut parser = PolicyParser::new(); + parser.parse("test.rules", policy_src)?; + let policy = parser.build(); + + assert_eq!( + policy.network_rules(), + &[NetworkRule { + host: "api.example.com".to_string(), + protocol: NetworkRuleProtocol::Https, + decision: NetworkRuleDecision::Allow, + justification: Some("Allow API calls".to_string()), + }] + ); + Ok(()) +} + +#[test] +fn rejects_network_rule_with_empty_host() { + let policy_src = r#" +network_rule( + host = " ", + protocol = "https", + decision = "allow", +) + "#; + + let mut parser = PolicyParser::new(); + let err = parser + .parse("test.rules", policy_src) + .expect_err("expected parse error"); + assert!( + err.to_string() + .contains("invalid rule: host cannot be empty") + ); +} + +#[test] +fn rejects_network_rule_with_invalid_protocol() { + let policy_src = r#" +network_rule( + host = "api.example.com", + protocol = "socks5", + decision = "allow", +) + "#; + + let mut parser = PolicyParser::new(); + let err = parser + .parse("test.rules", policy_src) + .expect_err("expected parse error"); + assert!( + err.to_string() + .contains("invalid rule: invalid network protocol: socks5") + ); +} + +#[test] +fn rejects_network_rule_with_invalid_decision() { + let policy_src = r#" +network_rule( + host = "api.example.com", + protocol = "https", + decision = "prompt", +) + "#; + + let mut parser = PolicyParser::new(); + let err = parser + .parse("test.rules", policy_src) + .expect_err("expected parse error"); + assert!( + err.to_string() + .contains("invalid rule: invalid network decision: prompt") + ); +} + +#[test] +fn rejects_network_rule_with_empty_justification() { + let policy_src = r#" +network_rule( + host = "api.example.com", + protocol = "https", + decision = "allow", + justification = " ", +) + "#; + + let mut parser = PolicyParser::new(); + let err = parser + .parse("test.rules", policy_src) + .expect_err("expected parse error"); + assert!( + err.to_string() + .contains("invalid rule: justification cannot be empty") + ); +} + #[test] fn justification_is_attached_to_forbidden_matches() -> Result<()> { let policy_src = r#"