mirror of
https://github.com/openai/codex.git
synced 2026-09-07 15:40:00 +00:00
fix: enforce ripgrep pre approval in exec policy
This commit is contained in:
@@ -1954,6 +1954,57 @@ fn vec_str(items: &[&str]) -> Vec<String> {
|
||||
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]
|
||||
|
||||
@@ -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 <cmd> simply do the check for <cmd>
|
||||
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"]);
|
||||
|
||||
@@ -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> {
|
||||
|
||||
Reference in New Issue
Block a user