Reject NUL bytes in reviewed terminal input (#41354)

## Why

Manual terminal-input approvals shell-quote the proposed input, which cannot
preserve NUL bytes for an accurate review.

## What changed

Reject terminal input containing a NUL byte before creating an approval request
or sending any of the input to the terminal.

## Testing

Extend the unreviewable-input integration test to cover both oversized input
and NUL-containing input, and verify that rejected input is not executed.

GitOrigin-RevId: e738c59ba5570012663bf6168ad052be237ca226
This commit is contained in:
jif
2026-08-28 15:44:48 +00:00
committed by copyberry
parent 868c9edb0d
commit 39507eea53
2 changed files with 22 additions and 11 deletions
@@ -217,6 +217,12 @@ impl ProcessEntry {
if sandbox_permissions == SandboxPermissions::UseDefault && !strict_auto_review {
return Ok(None);
}
// Manual approvals shell-quote the input, which cannot preserve NUL bytes.
if input.contains('\0') {
return Err(approval_error(
"terminal input contains a NUL byte and cannot be reviewed safely",
));
}
let reason = permissions
.approval_reason(sandbox_permissions)
.map_err(approval_error)?;
@@ -24,8 +24,19 @@ use core_test_support::wait_for_event;
use pretty_assertions::assert_eq;
use serde_json::json;
// The oversized input fits in 8KB before JSON escaping. Neither rejected input
// may reach the shell, including the trailing assignment.
#[test_case::test_case(
format!("#{}\nREJECTED=1\n", "\"".repeat(4_100)),
"too large to review safely";
"oversized"
)]
#[test_case::test_case("\0REJECTED=1\n".to_string(), "NUL byte"; "nul")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn oversized_stdin_is_rejected_before_approval_or_execution() -> Result<()> {
async fn unreviewable_stdin_is_rejected_before_approval_or_execution(
rejected_input: String,
expected_error: &str,
) -> Result<()> {
skip_if_target_windows!(Ok(()), "uses a POSIX interactive shell");
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
@@ -37,9 +48,6 @@ async fn oversized_stdin_is_rejected_before_approval_or_execution() -> Result<()
.expect("enable stdin approvals");
});
let test = builder.build_with_auto_env(&server).await?;
// The raw input fits in 8KB, but its JSON escapes do not. None of the
// rejected input, including the trailing assignment, may reach the shell.
let oversized = format!("#{}\nREJECTED=1\n", "\"".repeat(4_100));
let mut sequence = Vec::new();
for (id, tool, args) in [
(
@@ -48,9 +56,9 @@ async fn oversized_stdin_is_rejected_before_approval_or_execution() -> Result<()
json!({"cmd":"/bin/bash --noprofile --norc", "tty":true, "yield_time_ms":200, "sandbox_permissions":"require_escalated"}),
),
(
"oversized",
"rejected",
"write_stdin",
json!({"session_id":1000, "chars":oversized, "yield_time_ms":1000}),
json!({"session_id":1000, "chars":rejected_input, "yield_time_ms":1000}),
),
(
"allowed",
@@ -107,11 +115,8 @@ async fn oversized_stdin_is_rejected_before_approval_or_execution() -> Result<()
);
let requests = responses.requests();
let last = requests.last().expect("final model request");
let rejected = last.function_call_output("oversized");
assert!(
rejected.to_string().contains("too large to review safely"),
"{rejected}"
);
let rejected = last.function_call_output("rejected");
assert!(rejected.to_string().contains(expected_error), "{rejected}");
let allowed = last.function_call_output("allowed");
assert!(allowed.to_string().contains("STDIN_REVIEW_OK"), "{allowed}");
test.codex.submit(Op::Shutdown).await?;