Compare commits

...

2 Commits

Author SHA1 Message Date
Michael Bolin
efc5556781 core: add host_executable integration coverage 2026-02-28 09:40:00 -08:00
Michael Bolin
f38077e765 core: resolve host_executable() rules during preflight 2026-02-28 09:01:03 -08:00
4 changed files with 390 additions and 7 deletions

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(),
)
}

View File

@@ -8,6 +8,7 @@ use codex_protocol::config_types::Settings;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::user_input::UserInput;
use core_test_support::responses::ev_assistant_message;
@@ -19,6 +20,7 @@ use core_test_support::responses::sse;
use core_test_support::responses::start_mock_server;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use pretty_assertions::assert_eq;
use serde_json::Value;
use serde_json::json;
use std::fs;
@@ -62,6 +64,33 @@ async fn submit_user_turn(
Ok(())
}
async fn expect_exec_approval(
test: &core_test_support::test_codex::TestCodex,
expected_command: &str,
) -> codex_protocol::protocol::ExecApprovalRequestEvent {
let event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
)
})
.await;
match event {
EventMsg::ExecApprovalRequest(approval) => {
let last_arg = approval
.command
.last()
.map(String::as_str)
.unwrap_or_default();
assert_eq!(last_arg, expected_command);
approval
}
EventMsg::TurnComplete(_) => panic!("expected approval request before completion"),
other => panic!("unexpected event: {other:?}"),
}
}
fn assert_no_matched_rules_invariant(output_item: &Value) {
let Some(output) = output_item.get("output").and_then(Value::as_str) else {
panic!("function_call_output should include string output payload: {output_item:?}");
@@ -161,6 +190,192 @@ async fn execpolicy_blocks_shell_invocation() -> Result<()> {
Ok(())
}
#[tokio::test]
#[cfg(unix)]
async fn shell_command_absolute_path_uses_host_executable_rules() -> Result<()> {
let git_path = "/tmp/codex-host-executable-shell/git";
let command = format!("{git_path} status");
let mut builder = test_codex().with_config(move |config| {
let policy_path = config.codex_home.join("rules").join("policy.rules");
fs::create_dir_all(
policy_path
.parent()
.expect("policy directory must have a parent"),
)
.expect("create policy directory");
fs::write(
&policy_path,
format!(
r#"host_executable(name = "git", paths = ["{git_path}"])
prefix_rule(pattern = ["git", "status"], decision = "prompt")"#
),
)
.expect("write policy file");
});
let server = start_mock_server().await;
let test = builder.build(&server).await?;
let call_id = "shell-host-executable-prompt";
let args = json!({
"command": command,
"timeout_ms": 1_000,
});
mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-shell-host-exec-1"),
ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?),
ev_completed("resp-shell-host-exec-1"),
]),
)
.await;
let results_mock = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-shell-host-exec-1", "done"),
ev_completed("resp-shell-host-exec-2"),
]),
)
.await;
submit_user_turn(
&test,
"run absolute git path through shell_command",
AskForApproval::OnRequest,
SandboxPolicy::DangerFullAccess,
None,
)
.await?;
let approval = expect_exec_approval(&test, &command).await;
let reason = approval.reason.as_deref().unwrap_or_default();
assert!(
reason.contains("requires approval by policy"),
"unexpected approval reason: {reason}",
);
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
})
.await?;
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
let output_item = results_mock.single_request().function_call_output(call_id);
let output = output_item
.get("output")
.and_then(Value::as_str)
.unwrap_or_default();
assert!(
output.contains("rejected by user"),
"unexpected output: {output}",
);
Ok(())
}
#[tokio::test]
#[cfg(unix)]
async fn unified_exec_absolute_path_uses_host_executable_rules() -> Result<()> {
let git_path = "/tmp/codex-host-executable-unified/git";
let command = format!("{git_path} status");
let mut builder = test_codex().with_config(move |config| {
config.features.enable(Feature::UnifiedExec);
let policy_path = config.codex_home.join("rules").join("policy.rules");
fs::create_dir_all(
policy_path
.parent()
.expect("policy directory must have a parent"),
)
.expect("create policy directory");
fs::write(
&policy_path,
format!(
r#"host_executable(name = "git", paths = ["{git_path}"])
prefix_rule(pattern = ["git", "status"], decision = "prompt")"#
),
)
.expect("write policy file");
});
let server = start_mock_server().await;
let test = builder.build(&server).await?;
let call_id = "unified-host-executable-prompt";
let args = json!({
"cmd": command,
"shell": "/bin/bash",
"yield_time_ms": 1_000,
});
mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-unified-host-exec-1"),
ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?),
ev_completed("resp-unified-host-exec-1"),
]),
)
.await;
let results_mock = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-unified-host-exec-1", "done"),
ev_completed("resp-unified-host-exec-2"),
]),
)
.await;
submit_user_turn(
&test,
"run absolute git path through exec_command",
AskForApproval::OnRequest,
SandboxPolicy::DangerFullAccess,
None,
)
.await?;
let approval = expect_exec_approval(&test, &command).await;
let reason = approval.reason.as_deref().unwrap_or_default();
assert!(
reason.contains("requires approval by policy"),
"unexpected approval reason: {reason}",
);
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
})
.await?;
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
let output_item = results_mock.single_request().function_call_output(call_id);
let output = output_item
.get("output")
.and_then(Value::as_str)
.unwrap_or_default();
assert!(
output.contains("rejected by user"),
"unexpected output: {output}",
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn shell_command_empty_script_with_collaboration_mode_does_not_panic() -> Result<()> {
let server = start_mock_server().await;

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;
}

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"])));