clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-27 08:05:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

execpolicy: add host_executable() path mappings (#12964)

Browse Source 
## Why

`execpolicy` currently keys `prefix_rule()` matching off the literal
first token. That works for rules like `["/usr/bin/git"]`, but it means
shared basename rules such as `["git"]` do not help when a caller passes
an absolute executable path like `/usr/bin/git`.

This PR lays the groundwork for basename-aware matching without changing
existing callers yet. It adds typed host-executable metadata and an
opt-in resolution path in `codex-execpolicy`, so a follow-up PR can
adopt the new behavior in `unix_escalation.rs` and other call sites
without having to redesign the policy layer first.

## What Changed

- added `host_executable(name = ..., paths = [...])` to the execpolicy
parser and validated it with `AbsolutePathBuf`
- stored host executable mappings separately from prefix rules inside
`Policy`
- added `MatchOptions` and opt-in `*_with_options()` APIs that preserve
existing behavior by default
- implemented exact-first matching with optional basename fallback,
gated by `host_executable()` allowlists when present
- normalized executable names for cross-platform matching so Windows
paths like `git.exe` can satisfy `host_executable(name = "git", ...)`
- updated `match` / `not_match` example validation to exercise the
host-executable resolution path instead of only raw prefix-rule matching
- preserved source locations for deferred example-validation errors so
policy load failures still point at the right file and line
- surfaced `resolvedProgram` on `RuleMatch` so callers can tell when a
basename rule matched an absolute executable path
- preserved host executable metadata when requirements policies overlay
file-based policies in `core/src/exec_policy.rs`
- documented the new rule shape and CLI behavior in
`execpolicy/README.md`

## Verification

- `cargo test -p codex-execpolicy`
- added coverage in `execpolicy/tests/basic.rs` for parsing, precedence,
empty allowlists, basename fallback, exact-match precedence, and
host-executable-backed `match` / `not_match` examples
- added a regression test in `core/src/exec_policy.rs` to verify
requirements overlays preserve `host_executable()` metadata
- verified `cargo test -p codex-core --lib`, including source-rendering
coverage for deferred validation errors
This commit is contained in:
Michael Bolin
2026-02-27 12:59:24 -08:00
committed by GitHub
parent 6e0f1e9469
commit b148d98e0e
14 changed files with 900 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

349
codex-rs/execpolicy/tests/basic.rs
View File

@@ -1,5 +1,6 @@
use std::any::Any;
use std::fs;
use std::path::PathBuf;
use std::sync::Arc;
use anyhow::Context;
@@ -7,6 +8,7 @@ use anyhow::Result;
use codex_execpolicy::Decision;
use codex_execpolicy::Error;
use codex_execpolicy::Evaluation;
use codex_execpolicy::MatchOptions;
use codex_execpolicy::NetworkRuleProtocol;
use codex_execpolicy::Policy;
use codex_execpolicy::PolicyParser;
@@ -16,6 +18,7 @@ use codex_execpolicy::blocking_append_allow_prefix_rule;
use codex_execpolicy::rule::PatternToken;
use codex_execpolicy::rule::PrefixPattern;
use codex_execpolicy::rule::PrefixRule;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use tempfile::tempdir;
@@ -31,6 +34,35 @@ fn prompt_all(_: &[String]) -> Decision {
Decision::Prompt
}
fn absolute_path(path: &str) -> AbsolutePathBuf {
AbsolutePathBuf::try_from(path.to_string())
.unwrap_or_else(|error| panic!("expected absolute path `{path}`: {error}"))
}
fn host_absolute_path(segments: &[&str]) -> String {
let mut path = if cfg!(windows) {
PathBuf::from(r"C:\")
} else {
PathBuf::from("/")
};
for segment in segments {
path.push(segment);
}
path.to_string_lossy().into_owned()
}
fn host_executable_name(name: &str) -> String {
if cfg!(windows) {
format!("{name}.exe")
} else {
name.to_string()
}
}
fn starlark_string(value: &str) -> String {
value.replace('\\', "\\\\").replace('"', "\\\"")
}
#[derive(Clone, Debug, Eq, PartialEq)]
enum RuleSnapshot {
Prefix(PrefixRule),
@@ -125,6 +157,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git", "status"]),
decision: Decision::Allow,
resolved_program: None,
justification: None,
}],
},
@@ -156,6 +189,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["rm"]),
decision: Decision::Forbidden,
resolved_program: None,
justification: Some("destructive command".to_string()),
}],
},
@@ -184,6 +218,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["ls"]),
decision: Decision::Allow,
resolved_program: None,
justification: Some("safe and commonly used".to_string()),
}],
},
@@ -236,6 +271,7 @@ fn add_prefix_rule_extends_policy() -> Result<()> {
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["ls", "-l"]),
decision: Decision::Prompt,
resolved_program: None,
justification: None,
}],
},
@@ -305,6 +341,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git"]),
decision: Decision::Prompt,
resolved_program: None,
justification: None,
}],
},
@@ -319,11 +356,13 @@ prefix_rule(
RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git"]),
decision: Decision::Prompt,
resolved_program: None,
justification: None,
},
RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git", "commit"]),
decision: Decision::Forbidden,
resolved_program: None,
justification: None,
},
],
@@ -381,6 +420,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["bash", "-c"]),
decision: Decision::Allow,
resolved_program: None,
justification: None,
}],
},
@@ -394,6 +434,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["sh", "-l"]),
decision: Decision::Allow,
resolved_program: None,
justification: None,
}],
},
@@ -440,6 +481,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["npm", "i", "--legacy-peer-deps"]),
decision: Decision::Allow,
resolved_program: None,
justification: None,
}],
},
@@ -456,6 +498,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["npm", "install", "--no-save"]),
decision: Decision::Allow,
resolved_program: None,
justification: None,
}],
},
@@ -486,6 +529,7 @@ prefix_rule(
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git", "status"]),
decision: Decision::Allow,
resolved_program: None,
justification: None,
}],
},
@@ -533,11 +577,13 @@ prefix_rule(
RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git"]),
decision: Decision::Prompt,
resolved_program: None,
justification: None,
},
RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git", "commit"]),
decision: Decision::Forbidden,
resolved_program: None,
justification: None,
},
],
@@ -576,16 +622,19 @@ prefix_rule(
RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git"]),
decision: Decision::Prompt,
resolved_program: None,
justification: None,
},
RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git"]),
decision: Decision::Prompt,
resolved_program: None,
justification: None,
},
RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git", "commit"]),
decision: Decision::Forbidden,
resolved_program: None,
justification: None,
},
],
@@ -612,3 +661,303 @@ fn heuristics_match_is_returned_when_no_policy_matches() {
evaluation
);
}
#[test]
fn parses_host_executable_paths() -> Result<()> {
let homebrew_git = host_absolute_path(&["opt", "homebrew", "bin", "git"]);
let usr_git = host_absolute_path(&["usr", "bin", "git"]);
let homebrew_git_literal = starlark_string(&homebrew_git);
let usr_git_literal = starlark_string(&usr_git);
let policy_src = format!(
r#"
host_executable(
name = "git",
paths = [
"{homebrew_git_literal}",
"{usr_git_literal}",
"{usr_git_literal}",
],
)
"#
);
let mut parser = PolicyParser::new();
parser.parse("test.rules", &policy_src)?;
let policy = parser.build();
assert_eq!(
policy
.host_executables()
.get("git")
.expect("missing git host executable")
.as_ref(),
[absolute_path(&homebrew_git), absolute_path(&usr_git)]
);
Ok(())
}
#[test]
fn host_executable_rejects_non_absolute_path() {
let policy_src = r#"
host_executable(name = "git", paths = ["git"])
"#;
let mut parser = PolicyParser::new();
let err = parser
.parse("test.rules", policy_src)
.expect_err("expected parse error");
assert!(
err.to_string()
.contains("host_executable paths must be absolute")
);
}
#[test]
fn host_executable_rejects_name_with_path_separator() {
let git_path = host_absolute_path(&["usr", "bin", "git"]);
let git_path_literal = starlark_string(&git_path);
let policy_src =
format!(r#"host_executable(name = "{git_path_literal}", paths = ["{git_path_literal}"])"#);
let mut parser = PolicyParser::new();
let err = parser
.parse("test.rules", &policy_src)
.expect_err("expected parse error");
assert!(
err.to_string()
.contains("host_executable name must be a bare executable name")
);
}
#[test]
fn host_executable_rejects_path_with_wrong_basename() {
let rg_path = host_absolute_path(&["usr", "bin", "rg"]);
let rg_path_literal = starlark_string(&rg_path);
let policy_src = format!(r#"host_executable(name = "git", paths = ["{rg_path_literal}"])"#);
let mut parser = PolicyParser::new();
let err = parser
.parse("test.rules", &policy_src)
.expect_err("expected parse error");
assert!(err.to_string().contains("must have basename `git`"));
}
#[test]
fn host_executable_last_definition_wins() -> Result<()> {
let usr_git = host_absolute_path(&["usr", "bin", "git"]);
let homebrew_git = host_absolute_path(&["opt", "homebrew", "bin", "git"]);
let usr_git_literal = starlark_string(&usr_git);
let homebrew_git_literal = starlark_string(&homebrew_git);
let mut parser = PolicyParser::new();
parser.parse(
"shared.rules",
&format!(r#"host_executable(name = "git", paths = ["{usr_git_literal}"])"#),
)?;
parser.parse(
"user.rules",
&format!(r#"host_executable(name = "git", paths = ["{homebrew_git_literal}"])"#),
)?;
let policy = parser.build();
assert_eq!(
policy
.host_executables()
.get("git")
.expect("missing git host executable")
.as_ref(),
[absolute_path(&homebrew_git)]
);
Ok(())
}
#[test]
fn host_executable_resolution_uses_basename_rule_when_allowed() -> Result<()> {
let git_name = host_executable_name("git");
let git_path = host_absolute_path(&["usr", "bin", &git_name]);
let git_path_literal = starlark_string(&git_path);
let policy_src = format!(
r#"
prefix_rule(pattern = ["git", "status"], decision = "prompt")
host_executable(name = "git", paths = ["{git_path_literal}"])
"#
);
let mut parser = PolicyParser::new();
parser.parse("test.rules", &policy_src)?;
let policy = parser.build();
let evaluation = policy.check_with_options(
&[git_path.clone(), "status".to_string()],
&allow_all,
&MatchOptions {
resolve_host_executables: true,
},
);
assert_eq!(
evaluation,
Evaluation {
decision: Decision::Prompt,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git", "status"]),
decision: Decision::Prompt,
resolved_program: Some(absolute_path(&git_path)),
justification: None,
}],
}
);
Ok(())
}
#[test]
fn prefix_rule_examples_honor_host_executable_resolution() -> Result<()> {
let allowed_git_name = host_executable_name("git");
let allowed_git = host_absolute_path(&["usr", "bin", &allowed_git_name]);
let other_git = host_absolute_path(&["opt", "homebrew", "bin", &allowed_git_name]);
let allowed_git_literal = starlark_string(&allowed_git);
let other_git_literal = starlark_string(&other_git);
let policy_src = format!(
r#"
prefix_rule(
pattern = ["git", "status"],
match = [["{allowed_git_literal}", "status"]],
not_match = [["{other_git_literal}", "status"]],
)
host_executable(name = "git", paths = ["{allowed_git_literal}"])
"#
);
let mut parser = PolicyParser::new();
parser.parse("test.rules", &policy_src)?;
Ok(())
}
#[test]
fn host_executable_resolution_respects_explicit_empty_allowlist() -> Result<()> {
let policy_src = r#"
prefix_rule(pattern = ["git"], decision = "prompt")
host_executable(name = "git", paths = [])
"#;
let mut parser = PolicyParser::new();
parser.parse("test.rules", policy_src)?;
let policy = parser.build();
let git_path = host_absolute_path(&["usr", "bin", "git"]);
let evaluation = policy.check_with_options(
&[git_path.clone(), "status".to_string()],
&allow_all,
&MatchOptions {
resolve_host_executables: true,
},
);
assert_eq!(
evaluation,
Evaluation {
decision: Decision::Allow,
matched_rules: vec![RuleMatch::HeuristicsRuleMatch {
command: vec![git_path, "status".to_string()],
decision: Decision::Allow,
}],
}
);
Ok(())
}
#[test]
fn host_executable_resolution_ignores_path_not_in_allowlist() -> Result<()> {
let allowed_git = host_absolute_path(&["usr", "bin", "git"]);
let other_git = host_absolute_path(&["opt", "homebrew", "bin", "git"]);
let allowed_git_literal = starlark_string(&allowed_git);
let policy_src = format!(
r#"
prefix_rule(pattern = ["git"], decision = "prompt")
host_executable(name = "git", paths = ["{allowed_git_literal}"])
"#
);
let mut parser = PolicyParser::new();
parser.parse("test.rules", &policy_src)?;
let policy = parser.build();
let evaluation = policy.check_with_options(
&[other_git.clone(), "status".to_string()],
&allow_all,
&MatchOptions {
resolve_host_executables: true,
},
);
assert_eq!(
evaluation,
Evaluation {
decision: Decision::Allow,
matched_rules: vec![RuleMatch::HeuristicsRuleMatch {
command: vec![other_git, "status".to_string()],
decision: Decision::Allow,
}],
}
);
Ok(())
}
#[test]
fn host_executable_resolution_falls_back_without_mapping() -> Result<()> {
let policy_src = r#"
prefix_rule(pattern = ["git"], decision = "prompt")
"#;
let mut parser = PolicyParser::new();
parser.parse("test.rules", policy_src)?;
let policy = parser.build();
let git_path = host_absolute_path(&["usr", "bin", "git"]);
let evaluation = policy.check_with_options(
&[git_path.clone(), "status".to_string()],
&allow_all,
&MatchOptions {
resolve_host_executables: true,
},
);
assert_eq!(
evaluation,
Evaluation {
decision: Decision::Prompt,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git"]),
decision: Decision::Prompt,
resolved_program: Some(absolute_path(&git_path)),
justification: None,
}],
}
);
Ok(())
}
#[test]
fn host_executable_resolution_does_not_override_exact_match() -> Result<()> {
let git_path = host_absolute_path(&["usr", "bin", "git"]);
let git_path_literal = starlark_string(&git_path);
let policy_src = format!(
r#"
prefix_rule(pattern = ["{git_path_literal}"], decision = "allow")
prefix_rule(pattern = ["git"], decision = "prompt")
host_executable(name = "git", paths = ["{git_path_literal}"])
"#
);
let mut parser = PolicyParser::new();
parser.parse("test.rules", &policy_src)?;
let policy = parser.build();
let evaluation = policy.check_with_options(
&[git_path.clone(), "status".to_string()],
&allow_all,
&MatchOptions {
resolve_host_executables: true,
},
);
assert_eq!(
evaluation,
Evaluation {
decision: Decision::Allow,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: vec![git_path],
decision: Decision::Allow,
resolved_program: None,
justification: None,
}],
}
);
Ok(())
}