Conversation
0bfef31 to
93194bc
Compare
codex-rs/execpolicy/src/parser.rs
Outdated
| )) | ||
| })?; | ||
| let path = parse_literal_absolute_path(raw)?; | ||
| if path.as_path().file_name().and_then(|value| value.to_str()) != Some(name) { |
There was a problem hiding this comment.
This basename check makes the feature effectively unusable on Windows. Existing rules are typically written as git, but the actual resolved path is usually ...\\git.exe; host_executable(name = "git", paths = ["C:\\...\\git.exe"]) is rejected here, and runtime lookup later keys off the literal file_name() as well. If this API is meant to be cross-platform, we need Windows-specific normalization (file_stem()/PATHEXT-style handling) before comparing or looking up the executable name.
bolinfest
left a comment
There was a problem hiding this comment.
One other issue I noticed while reading through the new parser/matcher flow: match / not_match examples still validate a prefix_rule by calling rule.matches(example) on the raw prefix rules, so they never see the new host_executable() fallback. That means a rule like prefix_rule(pattern = ["git", "status"], match = ["/usr/bin/git status"]) together with host_executable(name = "git", paths = ["/usr/bin/git"]) will still be rejected at load time even though runtime matching in Policy::match_host_executable_rules() accepts it. Since these examples are documented as load-time unit tests, there is currently no way to validate the new absolute-path behavior.
bolinfest
left a comment
There was a problem hiding this comment.
Another issue in the core wiring: the new host-executable fallback is only enabled in evaluate_intercepted_exec_policy() and in execpolicy check --resolve-host-executables. The normal preflight path in ExecPolicyManager::create_exec_approval_requirement_for_command() still calls exec_policy.check_multiple(...) with default options, so absolute commands like /usr/bin/git status remain unmatched during approval derivation. For the generic shell tool and the classic shell_command backend, that preflight path is the only execpolicy evaluation, so the new host_executable() rules do not apply there at all; even on zsh-fork backends, you now get different policy decisions before and after interception.
Why
execpolicycurrently keysprefix_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 inunix_escalation.rsand other call sites without having to redesign the policy layer first.What Changed
host_executable(name = ..., paths = [...])to the execpolicy parser and validated it withAbsolutePathBufPolicyMatchOptionsand opt-in*_with_options()APIs that preserve existing behavior by defaulthost_executable()allowlists when presentgit.execan satisfyhost_executable(name = "git", ...)match/not_matchexample validation to exercise the host-executable resolution path instead of only raw prefix-rule matchingresolvedProgramonRuleMatchso callers can tell when a basename rule matched an absolute executable pathcore/src/exec_policy.rsexecpolicy/README.mdVerification
cargo test -p codex-execpolicyexecpolicy/tests/basic.rsfor parsing, precedence, empty allowlists, basename fallback, exact-match precedence, and host-executable-backedmatch/not_matchexamplescore/src/exec_policy.rsto verify requirements overlays preservehost_executable()metadatacargo test -p codex-core --lib, including source-rendering coverage for deferred validation errors