mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Fail closed on deeply nested command wrappers (#39122)
## Why Dangerous-command inspection has a bounded wrapper depth. Returning no match after that limit could let a nested dangerous payload escape policy detection. ## What changed - Classify commands beyond the wrapper-depth limit as dangerous. - Cover the boundary and overflow cases in the command-safety unit tests. - Verify that exec policy rejects a deeply `env`-wrapped forced `rm` before it can delete a sentinel file when approvals are disabled. GitOrigin-RevId: a6cdbbb7bdc6274a62745f668189fabaaf46745f
This commit is contained in:
@@ -290,6 +290,68 @@ async fn granular_complex_forced_rm_requests_approval_when_allowed() -> Result<(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn deeply_nested_forced_rm_is_rejected_before_execution_when_approvals_are_disabled()
|
||||
-> Result<()> {
|
||||
skip_if_target_windows!(Ok(()), "uses a POSIX shell command fixture");
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex();
|
||||
let test = builder.build_with_auto_env(&server).await?;
|
||||
let sentinel = test.config.cwd.join("forced-rm-sentinel");
|
||||
fs::write(&sentinel, "must not be deleted")?;
|
||||
let call_id = "deeply-nested-forced-rm";
|
||||
let args = json!({
|
||||
"command": "env env env env env env env env env rm -rf forced-rm-sentinel",
|
||||
"timeout_ms": 1_000,
|
||||
});
|
||||
|
||||
mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-deeply-nested-forced-rm-1"),
|
||||
ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?),
|
||||
ev_completed("resp-deeply-nested-forced-rm-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let results_mock = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-deeply-nested-forced-rm", "done"),
|
||||
ev_completed("resp-deeply-nested-forced-rm-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_user_turn(
|
||||
&test,
|
||||
"run the deeply nested forced rm",
|
||||
AskForApproval::Never,
|
||||
PermissionProfile::Disabled,
|
||||
/*collaboration_mode*/ None,
|
||||
)
|
||||
.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)
|
||||
.expect("function call output should include a string output payload");
|
||||
assert!(
|
||||
output.contains("rejected: blocked by policy"),
|
||||
"unexpected output: {output}"
|
||||
);
|
||||
assert!(sentinel.exists(), "the rejected command must not execute");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[tokio::test]
|
||||
async fn unified_exec_disabled_windows_sandbox_rejects_managed_read_only_command() -> Result<()> {
|
||||
|
||||
@@ -25,7 +25,7 @@ fn dangerous_command_match_with_depth(
|
||||
wrapper_depth: usize,
|
||||
) -> Option<DangerousCommandMatch> {
|
||||
if wrapper_depth > MAX_DANGEROUS_COMMAND_WRAPPER_DEPTH {
|
||||
return None;
|
||||
return Some(DangerousCommandMatch::Other);
|
||||
}
|
||||
|
||||
if let Some(dangerous_match) = dangerous_command_match_for_exec(command, wrapper_depth) {
|
||||
@@ -288,6 +288,27 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deeply_nested_command_wrappers_fail_closed() {
|
||||
for (depth, expected) in [
|
||||
(
|
||||
MAX_DANGEROUS_COMMAND_WRAPPER_DEPTH,
|
||||
DangerousCommandMatch::ForcedRm,
|
||||
),
|
||||
(
|
||||
MAX_DANGEROUS_COMMAND_WRAPPER_DEPTH + 1,
|
||||
DangerousCommandMatch::Other,
|
||||
),
|
||||
] {
|
||||
let command = std::iter::repeat_n("env", depth)
|
||||
.chain(["rm", "-rf", "/tmp/example"])
|
||||
.map(str::to_owned)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
assert_eq!(dangerous_command_match(&command), Some(expected));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn forced_rm_in_complex_shell_syntax_is_dangerous() {
|
||||
for script in [
|
||||
|
||||
Reference in New Issue
Block a user