clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-05-24 04:54:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: require approval for ripgrep subprocess options

Browse Source
This commit is contained in:
Adrian Bravo
2026-05-06 22:15:33 -07:00
parent 678bd3583a
commit 00308093a6
5 changed files with 255 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
codex-rs/core/src/exec_policy.rs
View File

@@ -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::<Vec<_>>();
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()))

73
codex-rs/core/src/exec_policy_tests.rs
View File

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

1
codex-rs/exec/tests/suite/mod.rs
View File

@@ -8,5 +8,6 @@ mod originator;
mod output_schema;
mod prompt_stdin;
mod resume;
mod ripgrep_pre;
mod sandbox;
mod server_error_exit;

103
codex-rs/exec/tests/suite/ripgrep_pre.rs Normal file
View File

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

33
codex-rs/shell-command/src/command_safety/is_dangerous_command.rs
View File

@@ -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 <cmd> simply do the check for <cmd>
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"]),
] {