agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

core: resolve host_executable() rules during preflight (#13065)

Browse Source 
## Why

[#12964](https://github.com/openai/codex/pull/12964) added
`host_executable()` support to `codex-execpolicy`, and
[#13046](https://github.com/openai/codex/pull/13046) adopted it in the
zsh-fork interception path.

The remaining gap was the preflight execpolicy check in
`core/src/exec_policy.rs`. That path derives approval requirements
before execution for `shell`, `shell_command`, and `unified_exec`, but
it was still using the default exact-token matcher.

As a result, a command that already included an absolute executable
path, such as `/usr/bin/git status`, could still miss a basename rule
like `prefix_rule(pattern = ["git"], ...)` during preflight even when
the policy also defined a matching `host_executable(name = "git", ...)`
entry.

This PR brings the same opt-in `host_executable()` resolution to the
preflight approval path when an absolute program path is already present
in the parsed command.

## What Changed

- updated
`ExecPolicyManager::create_exec_approval_requirement_for_command()` in
`core/src/exec_policy.rs` to use `check_multiple_with_options(...)` with
`MatchOptions { resolve_host_executables: true }`
- kept the existing shell parsing flow for approval derivation, but now
allow basename rules to match absolute executable paths during preflight
when `host_executable()` permits it
- updated requested-prefix amendment evaluation to use the same
host-executable-aware matching mode, so suggested `prefix_rule()`
amendments are checked consistently for absolute-path commands
- added preflight coverage for:
- absolute-path commands that should match basename rules through
`host_executable()`
- absolute-path commands whose paths are not in the allowed
`host_executable()` mapping
  - requested prefix-rule amendments for absolute-path commands

## Verification

- `just fix -p codex-core`
- `cargo test -p codex-core --lib exec_policy::tests::`
This commit is contained in:
Michael Bolin
2026-02-28 09:25:30 -08:00
committed by GitHub
Unverified
parent 74e5150b1e
commit 6a673e7339
3 changed files with 175 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/exec_policy.rs
+133 -2
View File
@@ -13,6 +13,7 @@ use codex_execpolicy::AmendError;
use codex_execpolicy::Decision;
use codex_execpolicy::Error as ExecPolicyRuleError;
use codex_execpolicy::Evaluation;
use codex_execpolicy::MatchOptions;
use codex_execpolicy::NetworkRuleProtocol;
use codex_execpolicy::Policy;
use codex_execpolicy::PolicyParser;
@@ -221,7 +222,14 @@ impl ExecPolicyManager {
used_complex_parsing,
)
};
let evaluation = exec_policy.check_multiple(commands.iter(), &exec_policy_fallback);
let match_options = MatchOptions {
resolve_host_executables: true,
};
let evaluation = exec_policy.check_multiple_with_options(
commands.iter(),
&exec_policy_fallback,
&match_options,
);
let requested_amendment = derive_requested_execpolicy_amendment_from_prefix_rule(
prefix_rule.as_ref(),
@@ -229,6 +237,7 @@ impl ExecPolicyManager {
exec_policy.as_ref(),
&commands,
&exec_policy_fallback,
&match_options,
);
match evaluation.decision {
@@ -630,6 +639,7 @@ fn derive_requested_execpolicy_amendment_from_prefix_rule(
exec_policy: &Policy,
commands: &[Vec<String>],
exec_policy_fallback: &impl Fn(&[String]) -> Decision,
match_options: &MatchOptions,
) -> Option<ExecPolicyAmendment> {
let prefix_rule = prefix_rule?;
if prefix_rule.is_empty() {
@@ -656,6 +666,7 @@ fn derive_requested_execpolicy_amendment_from_prefix_rule(
&amendment.command,
commands,
exec_policy_fallback,
match_options,
) {
Some(amendment)
} else {
@@ -668,6 +679,7 @@ fn prefix_rule_would_approve_all_commands(
prefix_rule: &[String],
commands: &[Vec<String>],
exec_policy_fallback: &impl Fn(&[String]) -> Decision,
match_options: &MatchOptions,
) -> bool {
let mut policy_with_prefix_rule = exec_policy.clone();
if policy_with_prefix_rule
@@ -679,7 +691,7 @@ fn prefix_rule_would_approve_all_commands(
commands.iter().all(|command| {
policy_with_prefix_rule
.check(command, exec_policy_fallback)
.check_with_options(command, exec_policy_fallback, match_options)
.decision
== Decision::Allow
})
@@ -849,6 +861,15 @@ mod tests {
path.to_string_lossy().into_owned()
}
fn host_program_path(name: &str) -> String {
let executable_name = if cfg!(windows) {
format!("{name}.exe")
} else {
name.to_string()
};
host_absolute_path(&["usr", "bin", &executable_name])
}
fn starlark_string(value: &str) -> String {
value.replace('\\', "\\\\").replace('"', "\\\"")
}
@@ -1398,6 +1419,115 @@ prefix_rule(
);
}
#[tokio::test]
async fn absolute_path_exec_approval_requirement_matches_host_executable_rules() {
let git_path = host_program_path("git");
let git_path_literal = starlark_string(&git_path);
let policy_src = format!(
r#"
host_executable(name = "git", paths = ["{git_path_literal}"])
prefix_rule(pattern=["git"], decision="allow")
"#
);
let mut parser = PolicyParser::new();
parser
.parse("test.rules", &policy_src)
.expect("parse policy");
let manager = ExecPolicyManager::new(Arc::new(parser.build()));
let command = vec![git_path, "status".to_string()];
let requirement = manager
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &command,
approval_policy: AskForApproval::UnlessTrusted,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
sandbox_permissions: SandboxPermissions::UseDefault,
prefix_rule: None,
})
.await;
assert_eq!(
requirement,
ExecApprovalRequirement::Skip {
bypass_sandbox: true,
proposed_execpolicy_amendment: None,
}
);
}
#[tokio::test]
async fn absolute_path_exec_approval_requirement_ignores_disallowed_host_executable_paths() {
let allowed_git_path = host_program_path("git");
let disallowed_git_path = host_absolute_path(&[
"opt",
"homebrew",
"bin",
if cfg!(windows) { "git.exe" } else { "git" },
]);
let allowed_git_path_literal = starlark_string(&allowed_git_path);
let policy_src = format!(
r#"
host_executable(name = "git", paths = ["{allowed_git_path_literal}"])
prefix_rule(pattern=["git"], decision="prompt")
"#
);
let mut parser = PolicyParser::new();
parser
.parse("test.rules", &policy_src)
.expect("parse policy");
let manager = ExecPolicyManager::new(Arc::new(parser.build()));
let command = vec![disallowed_git_path, "status".to_string()];
let requirement = manager
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &command,
approval_policy: AskForApproval::UnlessTrusted,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
sandbox_permissions: SandboxPermissions::UseDefault,
prefix_rule: None,
})
.await;
assert_eq!(
requirement,
ExecApprovalRequirement::Skip {
bypass_sandbox: false,
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
}
);
}
#[tokio::test]
async fn requested_prefix_rule_can_approve_absolute_path_commands() {
let command = vec![
host_program_path("cargo"),
"install".to_string(),
"cargo-insta".to_string(),
];
let manager = ExecPolicyManager::default();
let requirement = manager
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &command,
approval_policy: AskForApproval::UnlessTrusted,
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
sandbox_permissions: SandboxPermissions::UseDefault,
prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]),
})
.await;
assert_eq!(
requirement,
ExecApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
"cargo".to_string(),
"install".to_string(),
])),
}
);
}
#[tokio::test]
async fn exec_approval_requirement_respects_approval_policy() {
let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#;
@@ -1952,6 +2082,7 @@ prefix_rule(
&Policy::empty(),
&commands,
&|_: &[String]| Decision::Allow,
&MatchOptions::default(),
)
}
codex-rs/shell-command/src/command_safety/is_dangerous_command.rs
+28 -1
View File
@@ -1,4 +1,5 @@
use crate::bash::parse_shell_lc_plain_commands;
use std::path::Path;
#[cfg(windows)]
#[path = "windows_dangerous_commands.rs"]
mod windows_dangerous_commands;
@@ -52,6 +53,32 @@ fn is_git_global_option_with_inline_value(arg: &str) -> bool {
) || ((arg.starts_with("-C") || arg.starts_with("-c")) && arg.len() > 2)
}
pub(crate) fn executable_name_lookup_key(raw: &str) -> Option<String> {
#[cfg(windows)]
{
Path::new(raw)
.file_name()
.and_then(|name| name.to_str())
.map(|name| {
let name = name.to_ascii_lowercase();
for suffix in [".exe", ".cmd", ".bat", ".com"] {
if let Some(stripped) = name.strip_suffix(suffix) {
return stripped.to_string();
}
}
name
})
}
#[cfg(not(windows))]
{
Path::new(raw)
.file_name()
.and_then(|name| name.to_str())
.map(std::borrow::ToOwned::to_owned)
}
}
/// Find the first matching git subcommand, skipping known global options that
/// may appear before it (e.g., `-C`, `-c`, `--git-dir`).
///
@@ -61,7 +88,7 @@ pub(crate) fn find_git_subcommand<'a>(
subcommands: &[&str],
) -> Option<(usize, &'a str)> {
let cmd0 = command.first().map(String::as_str)?;
if !cmd0.ends_with("git") {
if executable_name_lookup_key(cmd0).as_deref() != Some("git") {
return None;
}
codex-rs/shell-command/src/command_safety/is_safe_command.rs
+14 -4
View File
@@ -1,4 +1,5 @@
use crate::bash::parse_shell_lc_plain_commands;
use crate::command_safety::is_dangerous_command::executable_name_lookup_key;
// Find the first matching git subcommand, skipping known global options that
// may appear before it (e.g., `-C`, `-c`, `--git-dir`).
// Implemented in `is_dangerous_command` and shared here.
@@ -47,10 +48,7 @@ fn is_safe_to_call_with_exec(command: &[String]) -> bool {
return false;
};
match std::path::Path::new(&cmd0)
.file_name()
.and_then(|osstr| osstr.to_str())
{
match executable_name_lookup_key(cmd0).as_deref() {
Some(cmd) if cfg!(target_os = "linux") && matches!(cmd, "numfmt" | "tac") => true,
#[rustfmt::skip]
@@ -495,6 +493,18 @@ mod tests {
])));
}
#[test]
fn windows_git_full_path_is_safe() {
if !cfg!(windows) {
return;
}
assert!(is_known_safe_command(&vec_str(&[
r"C:\Program Files\Git\cmd\git.exe",
"status",
])));
}
#[test]
fn bash_lc_safe_examples() {
assert!(is_known_safe_command(&vec_str(&["bash", "-lc", "ls"])));