diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index 8c424c541c..c400712b55 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -2051,6 +2051,57 @@ fn vec_str(items: &[&str]) -> Vec { items.iter().map(std::string::ToString::to_string).collect() } +#[tokio::test] +async fn ripgrep_pre_processor_requires_approval_in_sandboxed_exec() { + for command in [ + vec_str(&["rg", "--pre=./pre.sh", "needle", "input.txt"]), + vec_str(&["/bin/zsh", "-lc", r"rg --pre\=./pre.sh needle input.txt"]), + ] { + assert_exec_approval_requirement_for_command( + ExecApprovalRequirementScenario { + policy_src: None, + command, + 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::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_is_forbidden_when_exec_cannot_ask() { + let command = vec_str(&["rg", "--pre=./pre.sh", "needle", "input.txt"]); + 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: "`rg '--pre=./pre.sh' needle input.txt` rejected: blocked by policy" + .to_string(), + }, + ) + .await; +} + /// Note this test behaves differently on Windows because it exercises an /// `if cfg!(windows)` code path in render_decision_for_unmatched_command(). #[tokio::test] 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 fef98e8362..bcd92bf04f 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 @@ -1,4 +1,6 @@ use crate::bash::parse_shell_lc_plain_commands; +use crate::command_safety::ripgrep::RipgrepArgCase; +use crate::command_safety::ripgrep::ripgrep_command_can_execute_arbitrary_command; use std::path::Path; #[cfg(windows)] #[path = "windows_dangerous_commands.rs"] @@ -143,14 +145,20 @@ pub(crate) fn find_git_subcommand<'a>( } fn is_dangerous_to_call_with_exec(command: &[String]) -> bool { - let cmd0 = command.first().map(String::as_str); + let cmd0 = command + .first() + .and_then(|command| executable_name_lookup_key(command)); - match cmd0 { + match cmd0.as_deref() { Some("rm") => matches!(command.get(1).map(String::as_str), Some("-f" | "-rf")), // for sudo simply do the check for Some("sudo") => is_dangerous_to_call_with_exec(&command[1..]), + Some("rg") => { + ripgrep_command_can_execute_arbitrary_command(command, RipgrepArgCase::Sensitive) + } + // ── anything else ───────────────────────────────────────────────── _ => false, } @@ -174,6 +182,34 @@ mod tests { assert!(command_might_be_dangerous(&vec_str(&["rm", "-f", "/"]))); } + #[test] + fn ripgrep_pre_processor_is_dangerous() { + for command in [ + 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", "-lc", r"rg --pre\=./pre.sh needle input.txt"]), + vec_str(&["/bin/zsh", "-lc", "rg --pre=./pre.sh needle input.txt"]), + ] { + assert!( + command_might_be_dangerous(&command), + "expected {command:?} to be dangerous", + ); + } + } + + #[test] + fn ripgrep_search_zip_is_not_dangerous() { + assert!(!command_might_be_dangerous(&vec_str(&[ + "rg", + "--search-zip", + "needle", + ]))); + assert!(!command_might_be_dangerous(&vec_str(&[ + "rg", "-z", "needle", + ]))); + } + #[test] fn direct_powershell_words_reuse_windows_dangerous_detection() { let command = vec_str(&["Remove-Item", "test", "-Force"]); diff --git a/codex-rs/shell-command/src/command_safety/ripgrep.rs b/codex-rs/shell-command/src/command_safety/ripgrep.rs index 0a2bb2fab1..bcb685c1f5 100644 --- a/codex-rs/shell-command/src/command_safety/ripgrep.rs +++ b/codex-rs/shell-command/src/command_safety/ripgrep.rs @@ -14,23 +14,42 @@ pub(crate) fn is_safe_ripgrep_command(command: &[String], arg_case: RipgrepArgCa .any(|arg| is_unsafe_ripgrep_arg(arg, arg_case)) } +pub(crate) fn ripgrep_command_can_execute_arbitrary_command( + command: &[String], + arg_case: RipgrepArgCase, +) -> bool { + command.iter().skip(1).map(String::as_str).any(|arg| { + let normalized = normalized_long_arg(arg, arg_case); + ripgrep_arg_can_execute_arbitrary_command(normalized.as_ref()) + }) +} + fn is_unsafe_ripgrep_arg(arg: &str, arg_case: RipgrepArgCase) -> bool { let normalized = normalized_long_arg(arg, arg_case); + if ripgrep_arg_can_execute_arbitrary_command(normalized.as_ref()) { + return true; + } + match normalized.as_ref() { + // Calls out to other decompression tools, so do not auto-approve + // out of an abundance of caution. + "--search-zip" => true, + _ => { + normalized.starts_with("--search-zip=") + || ripgrep_short_options_contain_search_zip(arg, arg_case) + } + } +} + +fn ripgrep_arg_can_execute_arbitrary_command(normalized_arg: &str) -> bool { + matches!( + normalized_arg, // Takes an arbitrary command that is executed for each match. "--pre" // Takes a command that can be used to obtain the local hostname. | "--hostname-bin" - // Calls out to other decompression tools, so do not auto-approve - // out of an abundance of caution. - | "--search-zip" => true, - _ => { - normalized.starts_with("--pre=") - || normalized.starts_with("--hostname-bin=") - || normalized.starts_with("--search-zip=") - || ripgrep_short_options_contain_search_zip(arg, arg_case) - } - } + ) || normalized_arg.starts_with("--pre=") + || normalized_arg.starts_with("--hostname-bin=") } fn normalized_long_arg(arg: &str, arg_case: RipgrepArgCase) -> Cow<'_, str> {