From 7c11b8074dbdbab2a3afedf30df7c55f0c6907d8 Mon Sep 17 00:00:00 2001 From: jif Date: Wed, 28 Oct 2026 14:57:56 +0000 Subject: [PATCH] Include terminal launch commands in Guardian stdin reviews (#57758) ## Why Guardian's `write_stdin` approval requests lacked the command used to launch the receiving terminal, leaving reviews without that context. ## What changed - Carry `original_command` from the terminal's launch command into the approval request and Guardian action JSON. This describes the original launch, not the terminal's current state. - Exclude `original_command` from the 8,000-byte stdin approval size check so launch context does not consume the budget for input, permission details, and the approval reason. Launch context remains subject to Guardian's normal request budget. ## Testing Extend serialization coverage to check `original_command` and the strict stdin review integration test to verify that a launch command longer than 8,000 bytes reaches Guardian intact. GitOrigin-RevId: cc765a6bf80d080baf94c959eafa6227b2889e1f --- codex-rs/core/src/guardian/mod.rs | 1 - codex-rs/core/src/guardian/tests.rs | 1 + codex-rs/core/src/tools/approvals.rs | 4 +++ .../core/src/unified_exec/process_manager.rs | 12 ++++++--- .../core/src/unified_exec/stdin_approval.rs | 1 + .../suite/unified_exec_stdin_approval.rs | 26 +++++++++++++------ .../guardian-reviewer/src/approval_request.rs | 2 ++ .../src/approval_request_json.rs | 3 +++ .../src/review_action_tests.rs | 4 ++- 9 files changed, 41 insertions(+), 13 deletions(-) diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index 86bf379f8b..27aa9d033b 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -195,7 +195,6 @@ impl From<&Arc> for GuardianReviewContext { #[cfg(test)] use codex_guardian_reviewer::guardian_output_schema; -pub(crate) use approval_request::format_guardian_action_pretty; #[cfg(test)] use approval_request::guardian_assessment_action; #[cfg(test)] diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index fed2b2860c..be96a54d5a 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -653,6 +653,7 @@ async fn approval_permissions_use_the_owning_environment() -> anyhow::Result<()> approval_id: "stdin-secondary".to_string(), environment_id: "secondary".to_string(), process_id: 42, + original_command: "bash".to_string(), input: "cat private/secret.txt\n".to_string(), cwd: cwd.clone().into(), tty: true, diff --git a/codex-rs/core/src/tools/approvals.rs b/codex-rs/core/src/tools/approvals.rs index 058382cf25..74082aaf49 100644 --- a/codex-rs/core/src/tools/approvals.rs +++ b/codex-rs/core/src/tools/approvals.rs @@ -82,6 +82,7 @@ pub(crate) enum ApprovalAction { approval_id: String, environment_id: String, process_id: i32, + original_command: String, input: String, cwd: PathUri, tty: bool, @@ -182,6 +183,7 @@ impl ApprovalAction { approval_id, environment_id, process_id, + original_command: _, input, cwd, tty, @@ -337,6 +339,7 @@ impl ApprovalAction { approval_id, environment_id, process_id, + original_command, input, cwd, tty, @@ -348,6 +351,7 @@ impl ApprovalAction { approval_id, environment_id, process_id, + original_command, input, cwd, tty, diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index c8b319c998..2a2eb2afe0 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -934,7 +934,7 @@ impl UnifiedExecProcessManager { "terminal input and permission details are too large to review safely; use a smaller input or start a new terminal with fewer grants".to_string(), )) }; - let reviewed = crate::guardian::format_guardian_action_pretty( + let mut reviewed = codex_guardian_reviewer::guardian_approval_request_to_json( &approval .clone() .into_guardian_request(/*exec_command_cwd_convention*/ None) @@ -943,8 +943,14 @@ impl UnifiedExecProcessManager { })?, ) .map_err(|_| unreviewable_input_error())?; - // Bound the entire serialized action plus its reason, including JSON - // escaping. Reject, never execute an unreviewed tail. + // Launch context uses Guardian's normal request budget, not the stdin limit. + if let Some(fields) = reviewed.as_object_mut() { + fields.remove("original_command"); + } + let reviewed = + serde_json::to_string_pretty(&reviewed).map_err(|_| unreviewable_input_error())?; + // Bound the input and permission details plus their reason, including + // JSON escaping. Reject, never execute an unreviewed tail. let oversized = reviewed.len().saturating_add(approval_reason.len()) > MAX_STDIN_APPROVAL_BYTES; let size_check_result = if oversized { diff --git a/codex-rs/core/src/unified_exec/stdin_approval.rs b/codex-rs/core/src/unified_exec/stdin_approval.rs index 35d3e363d2..9b28eee9f1 100644 --- a/codex-rs/core/src/unified_exec/stdin_approval.rs +++ b/codex-rs/core/src/unified_exec/stdin_approval.rs @@ -276,6 +276,7 @@ impl ProcessEntry { approval_id: context.call_id.clone(), environment_id: self.environment_id.clone(), process_id: self.process_id, + original_command: self.hook_command.clone(), input: input.to_string(), cwd: self.cwd.clone(), tty: self.tty, diff --git a/codex-rs/core/tests/suite/unified_exec_stdin_approval.rs b/codex-rs/core/tests/suite/unified_exec_stdin_approval.rs index ad945521e2..d9901dec25 100644 --- a/codex-rs/core/tests/suite/unified_exec_stdin_approval.rs +++ b/codex-rs/core/tests/suite/unified_exec_stdin_approval.rs @@ -1,5 +1,6 @@ //! Exercises retained terminal grants and strict stdin review through the agent API. +use anyhow::Context; use anyhow::Result; use codex_core::TurnInputRequest; use codex_core::config::Config; @@ -342,13 +343,14 @@ async fn strict_stdin_review_reaches_guardian_with_sandbox_prompts_disabled() -> extensions.approval_review_contributor(calls.clone()); let harness = harness(test_codex().with_extensions(Arc::new(extensions.build()))).await?; let test = harness.test(); + let original_command = format!("/bin/bash --noprofile --norc # {}", "x".repeat(8_000)); let opened = mount_sse_sequence( harness.server(), vec![ tool_response( "open", "exec_command", - json!({"cmd":"/bin/bash --noprofile --norc", "tty":true, "yield_time_ms":200}), + json!({"cmd":original_command, "tty":true, "yield_time_ms":200}), ), tool_response( "baseline", @@ -420,13 +422,21 @@ async fn strict_stdin_review_reaches_guardian_with_sandbox_prompts_disabled() -> *calls.0.lock().unwrap(), vec!["strict", "denied", "allowed"] ); - assert_eq!( - requests - .iter() - .filter(|request| request.body_contains_text("Planned action JSON:")) - .count(), - 2 - ); + let original_commands = requests + .iter() + .filter(|request| request.body_contains_text("Planned action JSON:")) + .map(|request| { + let text = request.message_input_texts("user").join("\n"); + let (_, action) = text + .rsplit_once("Planned action JSON:\n") + .context("planned stdin action")?; + let (action, _) = action + .split_once("\n>>> APPROVAL REQUEST END") + .context("end of planned stdin action")?; + Ok(serde_json::from_str::(action)?["original_command"].clone()) + }) + .collect::>>()?; + assert_eq!(original_commands, vec![json!(original_command); 2]); let output = requests.last().unwrap().function_call_output("allowed"); assert!(output.to_string().contains("STDIN_REVIEW_OK"), "{output}"); Ok(()) diff --git a/codex-rs/ext/guardian-reviewer/src/approval_request.rs b/codex-rs/ext/guardian-reviewer/src/approval_request.rs index 740b6e053f..076eb9b877 100644 --- a/codex-rs/ext/guardian-reviewer/src/approval_request.rs +++ b/codex-rs/ext/guardian-reviewer/src/approval_request.rs @@ -37,6 +37,8 @@ pub enum GuardianApprovalRequest { approval_id: String, environment_id: String, process_id: i32, + /// Command used to launch the receiving terminal, not its current state. + original_command: String, input: String, cwd: PathUri, tty: bool, diff --git a/codex-rs/ext/guardian-reviewer/src/approval_request_json.rs b/codex-rs/ext/guardian-reviewer/src/approval_request_json.rs index aaaca50591..893e16f15d 100644 --- a/codex-rs/ext/guardian-reviewer/src/approval_request_json.rs +++ b/codex-rs/ext/guardian-reviewer/src/approval_request_json.rs @@ -38,6 +38,7 @@ struct ApplyPatchApprovalAction<'a> { struct WriteStdinApprovalAction<'a> { tool: &'static str, session_id: i32, + original_command: &'a str, chars: &'a str, cwd: LegacyAppPathString, sandbox_permissions: SandboxPermissions, @@ -140,6 +141,7 @@ pub fn guardian_approval_request_to_json( ), GuardianApprovalRequest::WriteStdin { process_id, + original_command, input, cwd, tty, @@ -149,6 +151,7 @@ pub fn guardian_approval_request_to_json( } => serialize_guardian_action(WriteStdinApprovalAction { tool: "write_stdin", session_id: *process_id, + original_command, chars: input, cwd: cwd.clone().into(), sandbox_permissions: *sandbox_permissions, diff --git a/codex-rs/ext/guardian-reviewer/src/review_action_tests.rs b/codex-rs/ext/guardian-reviewer/src/review_action_tests.rs index 103a884eba..80cb909df5 100644 --- a/codex-rs/ext/guardian-reviewer/src/review_action_tests.rs +++ b/codex-rs/ext/guardian-reviewer/src/review_action_tests.rs @@ -16,7 +16,7 @@ use pretty_assertions::assert_eq; #[test_case::test_case(SandboxPermissions::UseDefault)] #[test_case::test_case(SandboxPermissions::RequireEscalated)] #[test_case::test_case(SandboxPermissions::WithAdditionalPermissions)] -fn guardian_write_stdin_preserves_input_and_foreign_cwd( +fn guardian_write_stdin_preserves_original_command_input_and_foreign_cwd( sandbox_permissions: SandboxPermissions, ) -> serde_json::Result<()> { let cwd = PathUri::parse("file:///C:/workspace").expect("valid executor cwd"); @@ -34,6 +34,7 @@ fn guardian_write_stdin_preserves_input_and_foreign_cwd( approval_id: "terminal-write".to_string(), environment_id: "windows-executor".to_string(), process_id: 1000, + original_command: "python helper.py --project 'example site'".to_string(), input: input.to_string(), cwd: cwd.clone(), tty: true, @@ -45,6 +46,7 @@ fn guardian_write_stdin_preserves_input_and_foreign_cwd( "tool": "write_stdin", "environment_id": "windows-executor", "session_id": 1000, + "original_command": "python helper.py --project 'example site'", "chars": input, "cwd": r"C:\workspace", "sandbox_permissions": sandbox_permissions,