openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-01 19:33:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: add forbidden_reason arg to prefix_rule() in *.rules

Browse Source
This commit is contained in:
Michael Bolin
2026-01-05 11:20:46 -08:00
parent 1d8e2b4da8
commit f0ccf9fcb4
10 changed files with 265 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/cli/tests/execpolicy.rs
+58
View File
@@ -59,3 +59,61 @@ prefix_rule(
Ok(())
}
#[test]
fn execpolicy_check_includes_forbidden_reason_when_present()
-> Result<(), Box<dyn std::error::Error>> {
let codex_home = TempDir::new()?;
let policy_path = codex_home.path().join("rules").join("policy.rules");
fs::create_dir_all(
policy_path
.parent()
.expect("policy path should have a parent"),
)?;
fs::write(
&policy_path,
r#"
prefix_rule(
pattern = ["git", "push"],
decision = "forbidden",
forbidden_reason = "pushing is blocked in this repo",
)
"#,
)?;
let output = Command::new(codex_utils_cargo_bin::cargo_bin("codex")?)
.env("CODEX_HOME", codex_home.path())
.args([
"execpolicy",
"check",
"--rules",
policy_path
.to_str()
.expect("policy path should be valid UTF-8"),
"git",
"push",
"origin",
"main",
])
.output()?;
assert!(output.status.success());
let result: serde_json::Value = serde_json::from_slice(&output.stdout)?;
assert_eq!(
result,
json!({
"decision": "forbidden",
"matchedRules": [
{
"prefixRuleMatch": {
"matchedPrefix": ["git", "push"],
"decision": "forbidden",
"forbiddenReason": "pushing is blocked in this repo"
}
}
]
})
);
Ok(())
}
codex-rs/core/src/exec_policy.rs
+99 -7
View File
@@ -28,8 +28,8 @@ use crate::features::Feature;
use crate::features::Features;
use crate::sandboxing::SandboxPermissions;
use crate::tools::sandboxing::ExecApprovalRequirement;
use shlex::try_join as shlex_try_join;
const FORBIDDEN_REASON: &str = "execpolicy forbids this command";
const PROMPT_CONFLICT_REASON: &str =
"execpolicy requires approval for this command, but AskForApproval is set to Never";
const PROMPT_REASON: &str = "execpolicy requires approval for this command";
@@ -128,7 +128,7 @@ impl ExecPolicyManager {
match evaluation.decision {
Decision::Forbidden => ExecApprovalRequirement::Forbidden {
reason: FORBIDDEN_REASON.to_string(),
reason: derive_forbidden_reason(command, &evaluation),
},
Decision::Prompt => {
if matches!(approval_policy, AskForApproval::Never) {
@@ -310,6 +310,53 @@ fn derive_prompt_reason(evaluation: &Evaluation) -> Option<String> {
})
}
fn render_shlex_command(args: &[String]) -> String {
shlex_try_join(args.iter().map(String::as_str)).unwrap_or_else(|_| args.join(" "))
}
fn derive_forbidden_reason(command_args: &[String], evaluation: &Evaluation) -> String {
let command = render_shlex_command(command_args);
let forbidden_reason = evaluation
.matched_rules
.iter()
.filter_map(|rule_match| match rule_match {
RuleMatch::PrefixRuleMatch {
matched_prefix,
decision: Decision::Forbidden,
forbidden_reason: Some(reason),
} => Some((matched_prefix.len(), reason.as_str())),
_ => None,
})
.max_by_key(|(matched_prefix_len, _)| *matched_prefix_len)
.map(|(_, reason)| reason);
if let Some(forbidden_reason) = forbidden_reason {
return format!("{command} rejected: {forbidden_reason}");
}
let matched_prefix = evaluation
.matched_rules
.iter()
.filter_map(|rule_match| match rule_match {
RuleMatch::PrefixRuleMatch {
matched_prefix,
decision: Decision::Forbidden,
..
} => Some(matched_prefix),
_ => None,
})
.max_by_key(|matched_prefix| matched_prefix.len());
match matched_prefix {
Some(prefix) => {
let prefix = render_shlex_command(prefix);
format!("{command} rejected: policy forbids commands starting with `{prefix}`")
}
None => format!("{command} rejected: blocked by policy"),
}
}
async fn collect_policy_files(dir: impl AsRef<Path>) -> Result<Vec<PathBuf>, ExecPolicyError> {
let dir = dir.as_ref();
let mut read_dir = match fs::read_dir(dir).await {
@@ -450,7 +497,8 @@ mod tests {
decision: Decision::Forbidden,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: vec!["rm".to_string()],
decision: Decision::Forbidden
decision: Decision::Forbidden,
forbidden_reason: None,
}],
},
policy.check_multiple(command.iter(), &|_| Decision::Allow)
@@ -528,7 +576,8 @@ mod tests {
decision: Decision::Forbidden,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: vec!["rm".to_string()],
decision: Decision::Forbidden
decision: Decision::Forbidden,
forbidden_reason: None,
}],
},
policy.check_multiple([vec!["rm".to_string()]].iter(), &|_| Decision::Allow)
@@ -538,7 +587,8 @@ mod tests {
decision: Decision::Prompt,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: vec!["ls".to_string()],
decision: Decision::Prompt
decision: Decision::Prompt,
forbidden_reason: None,
}],
},
policy.check_multiple([vec!["ls".to_string()]].iter(), &|_| Decision::Allow)
@@ -560,7 +610,7 @@ prefix_rule(pattern=["rm"], decision="forbidden")
let forbidden_script = vec![
"bash".to_string(),
"-lc".to_string(),
"rm -rf /tmp".to_string(),
"rm -rf /some/important/folder".to_string(),
];
let manager = ExecPolicyManager::new(policy);
@@ -574,10 +624,52 @@ prefix_rule(pattern=["rm"], decision="forbidden")
)
.await;
let forbidden_script_rendered =
shlex::try_join(forbidden_script.iter().map(String::as_str)).expect("shlex join");
assert_eq!(
requirement,
ExecApprovalRequirement::Forbidden {
reason: FORBIDDEN_REASON.to_string()
reason: format!(
"{forbidden_script_rendered} rejected: policy forbids commands starting with `rm`"
)
}
);
}
#[tokio::test]
async fn forbidden_reason_is_included_in_forbidden_exec_approval_requirement() {
let policy_src = r#"
prefix_rule(
pattern=["rm"],
decision="forbidden",
forbidden_reason="destructive command",
)
"#;
let mut parser = PolicyParser::new();
parser
.parse("test.rules", policy_src)
.expect("parse policy");
let policy = Arc::new(parser.build());
let manager = ExecPolicyManager::new(policy);
let requirement = manager
.create_exec_approval_requirement_for_command(
&Features::with_defaults(),
&[
"rm".to_string(),
"-rf".to_string(),
"/some/important/folder".to_string(),
],
AskForApproval::OnRequest,
&SandboxPolicy::DangerFullAccess,
SandboxPermissions::UseDefault,
)
.await;
assert_eq!(
requirement,
ExecApprovalRequirement::Forbidden {
reason: "rm -rf /some/important/folder rejected: destructive command".to_string()
}
);
}
codex-rs/core/tests/suite/exec_policy.rs
+1 -1
View File
@@ -97,7 +97,7 @@ async fn execpolicy_blocks_shell_invocation() -> Result<()> {
assert!(
end.aggregated_output
.contains("execpolicy forbids this command"),
.contains("rejected: policy forbids commands starting with `echo`"),
"unexpected output: {}",
end.aggregated_output
);
codex-rs/execpolicy/README.md
+5 -2
View File
@@ -1,9 +1,10 @@
# codex-execpolicy
## Overview
- Policy engine and CLI built around `prefix_rule(pattern=[...], decision?, match?, not_match?)`.
- Policy engine and CLI built around `prefix_rule(pattern=[...], decision?, forbidden_reason?, match?, not_match?)`.
- This release covers the prefix-rule subset of the execpolicy language; a richer language will follow.
- Tokens are matched in order; any `pattern` element may be a list to denote alternatives. `decision` defaults to `allow`; valid values: `allow`, `prompt`, `forbidden`.
- When `decision = "forbidden"`, an optional `forbidden_reason` can be provided to explain why the command is blocked.
- `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`).
- The CLI always prints the JSON serialization of the evaluation result.
- The legacy rule matcher lives in `codex-execpolicy-legacy`.
@@ -14,6 +15,7 @@
prefix_rule(
pattern = ["cmd", ["alt1", "alt2"]], # ordered tokens; list entries denote alternatives
decision = "prompt", # allow | prompt | forbidden; defaults to allow
forbidden_reason = "explain why this is forbidden", # only valid when decision="forbidden"
match = [["cmd", "alt1"], "cmd alt2"], # examples that must match this rule
not_match = [["cmd", "oops"], "cmd alt3"], # examples that must not match this rule
)
@@ -40,7 +42,8 @@ cargo run -p codex-execpolicy -- check --rules path/to/policy.rules git status
{
"prefixRuleMatch": {
"matchedPrefix": ["<token>", "..."],
"decision": "allow|prompt|forbidden"
"decision": "allow|prompt|forbidden",
"forbiddenReason": "..."
}
}
],
codex-rs/execpolicy/examples/example.codexpolicy
+1
View File
@@ -4,6 +4,7 @@
prefix_rule(
pattern = ["git", "reset", "--hard"],
decision = "forbidden",
forbidden_reason = "destructive operation",
match = [
["git", "reset", "--hard"],
],
codex-rs/execpolicy/src/error.rs
+2
View File
@@ -11,6 +11,8 @@ pub enum Error {
InvalidPattern(String),
#[error("invalid example: {0}")]
InvalidExample(String),
#[error("invalid rule: {0}")]
InvalidRule(String),
#[error(
"expected every example to match at least one rule. rules: {rules:?}; unmatched examples: \
{examples:?}"
codex-rs/execpolicy/src/parser.rs
+19
View File
@@ -212,6 +212,7 @@ fn policy_builtins(builder: &mut GlobalsBuilder) {
decision: Option<&'v str>,
r#match: Option<UnpackList<Value<'v>>>,
not_match: Option<UnpackList<Value<'v>>>,
forbidden_reason: Option<&'v str>,
eval: &mut Evaluator<'v, '_, '_>,
) -> anyhow::Result<NoneType> {
let decision = match decision {
@@ -219,6 +220,23 @@ fn policy_builtins(builder: &mut GlobalsBuilder) {
None => Decision::Allow,
};
let forbidden_reason = match forbidden_reason {
Some(raw) if raw.trim().is_empty() => {
return Err(
Error::InvalidRule("forbidden_reason cannot be empty".to_string()).into(),
);
}
Some(raw) => Some(raw.to_string()),
None => None,
};
if forbidden_reason.is_some() && decision != Decision::Forbidden {
return Err(Error::InvalidRule(format!(
"forbidden_reason requires decision=\"forbidden\" (got {decision:?})"
))
.into());
}
let pattern_tokens = parse_pattern(pattern)?;
let matches: Vec<Vec<String>> =
@@ -246,6 +264,7 @@ fn policy_builtins(builder: &mut GlobalsBuilder) {
rest: rest.clone(),
},
decision,
forbidden_reason: forbidden_reason.clone(),
}) as RuleRef
})
.collect();
codex-rs/execpolicy/src/policy.rs
+1
View File
@@ -46,6 +46,7 @@ impl Policy {
.into(),
},
decision,
forbidden_reason: None,
});
self.rules_by_program.insert(first_token.clone(), rule);
codex-rs/execpolicy/src/rule.rs
+7
View File
@@ -63,6 +63,11 @@ pub enum RuleMatch {
#[serde(rename = "matchedPrefix")]
matched_prefix: Vec<String>,
decision: Decision,
/// Optional explanation for why a matching rule forbids this command.
///
/// Only present when provided in the policy and `decision == forbidden`.
#[serde(rename = "forbiddenReason", skip_serializing_if = "Option::is_none")]
forbidden_reason: Option<String>,
},
HeuristicsRuleMatch {
command: Vec<String>,
@@ -83,6 +88,7 @@ impl RuleMatch {
pub struct PrefixRule {
pub pattern: PrefixPattern,
pub decision: Decision,
pub forbidden_reason: Option<String>,
}
pub trait Rule: Any + Debug + Send + Sync {
@@ -104,6 +110,7 @@ impl Rule for PrefixRule {
.map(|matched_prefix| RuleMatch::PrefixRuleMatch {
matched_prefix,
decision: self.decision,
forbidden_reason: self.forbidden_reason.clone(),
})
}
}
codex-rs/execpolicy/tests/basic.rs
+72 -1
View File
@@ -64,6 +64,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git", "status"]),
decision: Decision::Allow,
forbidden_reason: None,
}],
},
evaluation
@@ -71,6 +72,56 @@ prefix_rule(
Ok(())
}
#[test]
fn forbidden_reason_is_attached_to_forbidden_matches() -> Result<()> {
let policy_src = r#"
prefix_rule(
pattern = ["rm"],
decision = "forbidden",
forbidden_reason = "destructive command",
)
"#;
let mut parser = PolicyParser::new();
parser.parse("test.rules", policy_src)?;
let policy = parser.build();
let evaluation = policy.check(
&tokens(&["rm", "-rf", "/some/important/folder"]),
&allow_all,
);
assert_eq!(
Evaluation {
decision: Decision::Forbidden,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["rm"]),
decision: Decision::Forbidden,
forbidden_reason: Some("destructive command".to_string()),
}],
},
evaluation
);
Ok(())
}
#[test]
fn forbidden_reason_requires_forbidden_decision() {
let policy_src = r#"
prefix_rule(
pattern = ["ls"],
decision = "allow",
forbidden_reason = "not allowed here",
)
"#;
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: forbidden_reason requires decision=\"forbidden\"")
);
}
#[test]
fn add_prefix_rule_extends_policy() -> Result<()> {
let mut policy = Policy::empty();
@@ -84,17 +135,19 @@ fn add_prefix_rule_extends_policy() -> Result<()> {
rest: vec![PatternToken::Single(String::from("-l"))].into(),
},
decision: Decision::Prompt,
forbidden_reason: None,
})],
rules
);
let evaluation = policy.check(&tokens(&["ls", "-l", "/tmp"]), &allow_all);
let evaluation = policy.check(&tokens(&["ls", "-l", "/some/important/folder"]), &allow_all);
assert_eq!(
Evaluation {
decision: Decision::Prompt,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["ls", "-l"]),
decision: Decision::Prompt,
forbidden_reason: None,
}],
},
evaluation
@@ -142,6 +195,7 @@ prefix_rule(
rest: Vec::<PatternToken>::new().into(),
},
decision: Decision::Prompt,
forbidden_reason: None,
}),
RuleSnapshot::Prefix(PrefixRule {
pattern: PrefixPattern {
@@ -149,6 +203,7 @@ prefix_rule(
rest: vec![PatternToken::Single("commit".to_string())].into(),
},
decision: Decision::Forbidden,
forbidden_reason: None,
}),
],
git_rules
@@ -161,6 +216,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git"]),
decision: Decision::Prompt,
forbidden_reason: None,
}],
},
status_eval
@@ -174,10 +230,12 @@ prefix_rule(
RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git"]),
decision: Decision::Prompt,
forbidden_reason: None,
},
RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git", "commit"]),
decision: Decision::Forbidden,
forbidden_reason: None,
},
],
},
@@ -211,6 +269,7 @@ prefix_rule(
rest: vec![PatternToken::Alts(vec!["-c".to_string(), "-l".to_string()])].into(),
},
decision: Decision::Allow,
forbidden_reason: None,
})],
bash_rules
);
@@ -221,6 +280,7 @@ prefix_rule(
rest: vec![PatternToken::Alts(vec!["-c".to_string(), "-l".to_string()])].into(),
},
decision: Decision::Allow,
forbidden_reason: None,
})],
sh_rules
);
@@ -232,6 +292,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["bash", "-c"]),
decision: Decision::Allow,
forbidden_reason: None,
}],
},
bash_eval
@@ -244,6 +305,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["sh", "-l"]),
decision: Decision::Allow,
forbidden_reason: None,
}],
},
sh_eval
@@ -277,6 +339,7 @@ prefix_rule(
.into(),
},
decision: Decision::Allow,
forbidden_reason: None,
})],
rules
);
@@ -288,6 +351,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["npm", "i", "--legacy-peer-deps"]),
decision: Decision::Allow,
forbidden_reason: None,
}],
},
npm_i
@@ -303,6 +367,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["npm", "install", "--no-save"]),
decision: Decision::Allow,
forbidden_reason: None,
}],
},
npm_install
@@ -332,6 +397,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git", "status"]),
decision: Decision::Allow,
forbidden_reason: None,
}],
},
match_eval
@@ -378,10 +444,12 @@ prefix_rule(
RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git"]),
decision: Decision::Prompt,
forbidden_reason: None,
},
RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git", "commit"]),
decision: Decision::Forbidden,
forbidden_reason: None,
},
],
},
@@ -419,14 +487,17 @@ prefix_rule(
RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git"]),
decision: Decision::Prompt,
forbidden_reason: None,
},
RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git"]),
decision: Decision::Prompt,
forbidden_reason: None,
},
RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git", "commit"]),
decision: Decision::Forbidden,
forbidden_reason: None,
},
],
},