diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index a44ffbe3ab..9f12ad4a1e 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -24,6 +24,7 @@ use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemSandboxKind; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::protocol::AskForApproval; +use codex_shell_command::is_dangerous_command::command_can_execute_arbitrary_command; use codex_shell_command::is_dangerous_command::command_might_be_dangerous; use codex_shell_command::is_safe_command::is_known_safe_command; use thiserror::Error; @@ -309,11 +310,45 @@ impl ExecPolicyManager { let match_options = MatchOptions { resolve_host_executables: true, }; - let evaluation = exec_policy.check_multiple_with_options( + let mut evaluation = exec_policy.check_multiple_with_options( commands.iter(), &exec_policy_fallback, &match_options, ); + if evaluation.decision == Decision::Allow { + let mut arbitrary_command_matches = commands + .iter() + .filter_map(|command| { + if !command_can_execute_arbitrary_command(command) + || arbitrary_command_execution_is_explicitly_allowed( + command, + &evaluation.matched_rules, + ) + { + return None; + } + + let decision = exec_policy_fallback(command); + (decision != Decision::Allow).then(|| RuleMatch::HeuristicsRuleMatch { + command: command.clone(), + decision, + }) + }) + .collect::>(); + if !arbitrary_command_matches.is_empty() { + evaluation.decision = if arbitrary_command_matches + .iter() + .any(|rule_match| rule_match.decision() == Decision::Forbidden) + { + Decision::Forbidden + } else { + Decision::Prompt + }; + evaluation + .matched_rules + .append(&mut arbitrary_command_matches); + } + } let requested_amendment = if auto_amendment_allowed { derive_requested_execpolicy_amendment_from_prefix_rule( @@ -475,6 +510,24 @@ impl ExecPolicyManager { } } +fn arbitrary_command_execution_is_explicitly_allowed( + command: &[String], + matched_rules: &[RuleMatch], +) -> bool { + matched_rules.iter().any(|rule_match| { + let RuleMatch::PrefixRuleMatch { + matched_prefix, + decision: Decision::Allow, + .. + } = rule_match + else { + return false; + }; + + command.starts_with(matched_prefix) && command_can_execute_arbitrary_command(matched_prefix) + }) +} + impl Default for ExecPolicyManager { fn default() -> Self { Self::new(Arc::new(Policy::empty())) diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index 232ca09d69..31882428dc 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -2062,20 +2062,79 @@ async fn ripgrep_pre_processor_requires_approval_in_sandboxed_exec() { #[tokio::test] async fn ripgrep_pre_processor_is_forbidden_when_exec_cannot_ask() { - let command = vec_str(&["rg", "--pre=./pre.sh", "needle", "input.txt"]); + for (command, reason) in [ + ( + vec_str(&["rg", "--pre=./pre.sh", "needle", "input.txt"]), + "`rg '--pre=./pre.sh' needle input.txt` rejected: blocked by policy", + ), + ( + vec_str(&["/bin/zsh", "-c", r"rg --pre\=./pre.sh needle input.txt"]), + "`/bin/zsh -c \"rg --pre\\\\=./pre.sh needle input.txt\"` rejected: blocked by policy", + ), + ] { + assert_exec_approval_requirement_for_command( + ExecApprovalRequirementScenario { + policy_src: None, + command, + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + file_system_sandbox_policy: workspace_write_file_system_sandbox_policy(), + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: None, + }, + ExecApprovalRequirement::Forbidden { + reason: reason.to_string(), + }, + ) + .await; + } +} + +#[tokio::test] +async fn ripgrep_pre_processor_requires_approval_despite_broad_allow_rule() { assert_exec_approval_requirement_for_command( ExecApprovalRequirementScenario { - policy_src: None, - command, - approval_policy: AskForApproval::Never, + policy_src: Some( + r#"prefix_rule(pattern=["rg"], decision="allow", justification="read-only search")"# + .to_string(), + ), + command: vec_str(&["/bin/zsh", "-c", r"rg --pre\=./pre.sh needle input.txt"]), + approval_policy: AskForApproval::OnRequest, sandbox_policy: SandboxPolicy::new_workspace_write_policy(), file_system_sandbox_policy: workspace_write_file_system_sandbox_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }, - ExecApprovalRequirement::Forbidden { - reason: "`rg '--pre=./pre.sh' needle input.txt` rejected: blocked by policy" - .to_string(), + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec_str(&[ + "rg", + "--pre=./pre.sh", + "needle", + "input.txt", + ]))), + }, + ) + .await; +} + +#[tokio::test] +async fn ripgrep_pre_processor_honors_exact_allow_rule() { + assert_exec_approval_requirement_for_command( + ExecApprovalRequirementScenario { + policy_src: Some( + r#"prefix_rule(pattern=["rg", "--pre=./pre.sh"], decision="allow")"#.to_string(), + ), + command: vec_str(&["/bin/zsh", "-c", r"rg --pre\=./pre.sh needle input.txt"]), + approval_policy: AskForApproval::OnRequest, + sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + file_system_sandbox_policy: workspace_write_file_system_sandbox_policy(), + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: None, + }, + ExecApprovalRequirement::Skip { + bypass_sandbox: true, + proposed_execpolicy_amendment: None, }, ) .await; diff --git a/codex-rs/exec/tests/suite/mod.rs b/codex-rs/exec/tests/suite/mod.rs index c6fa0f9fde..dac1c73242 100644 --- a/codex-rs/exec/tests/suite/mod.rs +++ b/codex-rs/exec/tests/suite/mod.rs @@ -8,5 +8,6 @@ mod originator; mod output_schema; mod prompt_stdin; mod resume; +mod ripgrep_pre; mod sandbox; mod server_error_exit; diff --git a/codex-rs/exec/tests/suite/ripgrep_pre.rs b/codex-rs/exec/tests/suite/ripgrep_pre.rs new file mode 100644 index 0000000000..f1fb56cd71 --- /dev/null +++ b/codex-rs/exec/tests/suite/ripgrep_pre.rs @@ -0,0 +1,103 @@ +#![cfg(not(target_os = "windows"))] +#![allow(clippy::expect_used, clippy::unwrap_used)] + +use core_test_support::responses; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex_exec::test_codex_exec; +use serde_json::json; +use std::os::unix::fs::PermissionsExt; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn blocks_escaped_ripgrep_preprocessor() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let test = test_codex_exec(); + let marker = test.cwd_path().join("preprocessor-ran"); + std::fs::write(test.cwd_path().join("input.txt"), "needle\n")?; + + let preprocessor = test.cwd_path().join("pre.sh"); + std::fs::write( + &preprocessor, + format!( + "#!/bin/sh\nprintf ran > {:?}\ncat\n", + marker.to_string_lossy() + ), + )?; + let mut permissions = std::fs::metadata(&preprocessor)?.permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&preprocessor, permissions)?; + + let call_id = "rg-pre-poc"; + let args = json!({ + "command": r"rg --pre\=./pre.sh needle input.txt", + "login": false, + "timeout_ms": 1_000, + }); + let server = responses::start_mock_server().await; + let provider_override = format!( + "model_providers.mock={{ name = \"mock\", base_url = \"{}/v1\", env_key = \"PATH\", wire_api = \"responses\" }}", + server.uri() + ); + let resp_mock = responses::mount_sse_sequence( + &server, + vec![ + responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_function_call( + call_id, + "shell_command", + &serde_json::to_string(&args)?, + ), + responses::ev_completed("resp-1"), + ]), + responses::sse(vec![ + responses::ev_response_created("resp-2"), + responses::ev_assistant_message("msg-1", "done"), + responses::ev_completed("resp-2"), + ]), + ], + ) + .await; + + let mut cmd = test.cmd(); + let output = cmd + .env_remove("CODEX_API_KEY") + .env_remove("OPENAI_API_KEY") + .arg("-c") + .arg(&provider_override) + .arg("-c") + .arg("model_provider=\"mock\"") + .arg("--skip-git-repo-check") + .arg("-s") + .arg("workspace-write") + .arg("run the ripgrep preprocessor proof of concept") + .output()?; + + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !output.status.success(), + "escaped rg --pre should be blocked, but codex-exec succeeded\nStatus: {}\nStdout:\n{}\nStderr:\n{}", + output.status, + stdout, + stderr + ); + assert!( + stderr.contains("rejected") + || stderr.contains("approval") + || stderr.contains("unacceptable risk"), + "blocked run should report approval rejection\nStatus: {}\nStdout:\n{}\nStderr:\n{}", + output.status, + stdout, + stderr + ); + if let Some(tool_output) = resp_mock.function_call_output_text(call_id) { + assert!( + !tool_output.contains("Exit code: 0"), + "blocked command should not return a successful shell result: {tool_output}" + ); + } + assert!(!marker.exists(), "ripgrep --pre helper should not execute"); + + Ok(()) +} diff --git a/codex-rs/shell-command/src/command_safety/is_dangerous_command.rs b/codex-rs/shell-command/src/command_safety/is_dangerous_command.rs index bcd92bf04f..15a9396805 100644 --- a/codex-rs/shell-command/src/command_safety/is_dangerous_command.rs +++ b/codex-rs/shell-command/src/command_safety/is_dangerous_command.rs @@ -30,6 +30,22 @@ pub fn command_might_be_dangerous(command: &[String]) -> bool { false } +pub fn command_can_execute_arbitrary_command(command: &[String]) -> bool { + if direct_command_can_execute_arbitrary_command(command) { + return true; + } + + if let Some(all_commands) = parse_shell_lc_plain_commands(command) + && all_commands + .iter() + .any(|cmd| direct_command_can_execute_arbitrary_command(cmd)) + { + return true; + } + + false +} + /// Returns whether already-tokenized PowerShell words should be treated as /// dangerous by the Windows unmatched-command heuristics. pub fn is_dangerous_powershell_words(command: &[String]) -> bool { @@ -155,11 +171,23 @@ fn is_dangerous_to_call_with_exec(command: &[String]) -> bool { // for sudo simply do the check for Some("sudo") => is_dangerous_to_call_with_exec(&command[1..]), + Some("rg") => direct_command_can_execute_arbitrary_command(command), + + // ── anything else ───────────────────────────────────────────────── + _ => false, + } +} + +fn direct_command_can_execute_arbitrary_command(command: &[String]) -> bool { + let cmd0 = command + .first() + .and_then(|command| executable_name_lookup_key(command)); + + match cmd0.as_deref() { + Some("sudo") => direct_command_can_execute_arbitrary_command(&command[1..]), Some("rg") => { ripgrep_command_can_execute_arbitrary_command(command, RipgrepArgCase::Sensitive) } - - // ── anything else ───────────────────────────────────────────────── _ => false, } } @@ -188,6 +216,7 @@ mod tests { vec_str(&["rg", "--pre", "./pre.sh", "needle", "input.txt"]), vec_str(&["rg", "--pre=./pre.sh", "needle", "input.txt"]), vec_str(&["/usr/bin/rg", "--hostname-bin=./hostname.sh", "needle"]), + vec_str(&["zsh", "-c", r"rg --pre\=./pre.sh needle input.txt"]), vec_str(&["zsh", "-lc", r"rg --pre\=./pre.sh needle input.txt"]), vec_str(&["/bin/zsh", "-lc", "rg --pre=./pre.sh needle input.txt"]), ] {