diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index 45efa6e746..0a00e4c3ad 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -319,14 +319,23 @@ async fn run_guardian_review( options: GuardianReviewOptions, ) -> ReviewDecision { let turn = Arc::clone(context.turn()); - // Required models must use Guardian, but an enabled V2 monitor can satisfy the review. + let requires_synchronous_review = reasons.retry.is_some() + || matches!( + &request, + GuardianApprovalRequest::ExecCommand { + sandbox_permissions, + .. + } if sandbox_permissions.requires_escalated_permissions() + ); + // Guardian V2 may satisfy ordinary reviews, including required-model reviews, but broader + // permission requests and retries must run Guardian synchronously. if (!turn .config .config_layer_stack .requirements() .auto_review_required_for_model(&turn.model_info.slug) || turn.config.features.enabled(Feature::GuardianV2)) - && reasons.retry.is_none() + && !requires_synchronous_review && options .external_cancel .as_ref() diff --git a/codex-rs/core/tests/suite/auto_review.rs b/codex-rs/core/tests/suite/auto_review.rs index bfcf827bb5..245473083b 100644 --- a/codex-rs/core/tests/suite/auto_review.rs +++ b/codex-rs/core/tests/suite/auto_review.rs @@ -80,6 +80,23 @@ impl ApprovalReviewContributor for ApprovedReviewContributor { } } +struct EscalationApprovingReviewContributor; + +impl ApprovalReviewContributor for EscalationApprovingReviewContributor { + fn contribute<'a>( + &'a self, + _session_store: &'a ExtensionData, + _thread_store: &'a ExtensionData, + prompt: &'a str, + _extension_metrics: Option>, + ) -> ExtensionFuture<'a, Option> { + Box::pin(async move { + assert!(prompt.contains("\"sandbox_permissions\":\"require_escalated\"")); + Some(ReviewDecision::Approved) + }) + } +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn approval_review_contributor_skips_existing_guardian_model_call() -> Result<()> { skip_if_no_network!(Ok(())); @@ -160,6 +177,103 @@ async fn approval_review_contributor_skips_existing_guardian_model_call() -> Res Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn require_escalated_bypasses_extension_approval_and_runs_guardian() -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_wine_exec!(Ok(()), "exec_command requires host-native paths"); + + let server = MockServer::start().await; + let call_id = "require-escalated-command"; + let justification = "run outside the sandbox after a blocked attempt"; + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-parent-1"), + ev_function_call( + call_id, + "exec_command", + &json!({ + "cmd": "pwd", + "sandbox_permissions": "require_escalated", + "justification": justification, + }) + .to_string(), + ), + ev_completed("resp-parent-1"), + ]), + sse(vec![ + ev_response_created("resp-guardian"), + ev_assistant_message( + "msg-guardian", + &json!({ + "risk_level": "high", + "user_authorization": "low", + "outcome": "deny", + "rationale": "The unsandboxed command is not authorized.", + }) + .to_string(), + ), + ev_completed("resp-guardian"), + ]), + sse(vec![ + ev_response_created("resp-parent-2"), + ev_assistant_message("msg-parent", "done"), + ev_completed("resp-parent-2"), + ]), + ], + ) + .await; + + let mut extensions = ExtensionRegistryBuilder::new(); + extensions.approval_review_contributor(Arc::new(EscalationApprovingReviewContributor)); + let mut builder = test_codex() + .with_extensions(Arc::new(extensions.build())) + .with_config(|config| { + config.approvals_reviewer = ApprovalsReviewer::AutoReview; + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + config + .permissions + .set_permission_profile(PermissionProfile::read_only()) + .expect("set read-only permission profile"); + }); + let test = builder.build_with_auto_env(&server).await?; + test.codex + .start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text { + text: "retry the blocked command outside the sandbox".into(), + text_elements: Vec::new(), + }])) + .await?; + + loop { + match wait_for_event(&test.codex, |_| true).await { + EventMsg::ExecApprovalRequest(event) => { + panic!("escalated command should not prompt the user: {event:?}") + } + EventMsg::TurnComplete(_) => break, + _ => {} + } + } + + let requests = responses.requests(); + assert_eq!(requests.len(), 3); + let guardian_request = requests + .iter() + .find(|request| { + request.body_json()["client_metadata"]["x-openai-subagent"].as_str() == Some("guardian") + }) + .context("expected full Guardian review for require_escalated")?; + assert!(guardian_request.body_contains_text(justification)); + assert!( + responses + .function_call_output_text(call_id) + .context("expected exec_command output")? + .contains("not authorized") + ); + + Ok(()) +} + #[derive(Clone, Copy)] enum GuardianV2DisableSource { Config, diff --git a/codex-rs/core/tests/suite/guardian_review.rs b/codex-rs/core/tests/suite/guardian_review.rs index f021e719d2..0398d2cd9d 100644 --- a/codex-rs/core/tests/suite/guardian_review.rs +++ b/codex-rs/core/tests/suite/guardian_review.rs @@ -1171,15 +1171,21 @@ async fn guardian_timeout_rejects_tool_call_with_acting_model_instructions( }); }) .with_config(|config| { + let rules_dir = config.codex_home.join("rules"); + fs::create_dir_all(&rules_dir).expect("create execution policy directory"); + fs::write( + rules_dir.join("default.rules"), + r#"prefix_rule(pattern=["touch"], decision="prompt")"#, + ) + .expect("write execution policy rule"); config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); config.approvals_reviewer = ApprovalsReviewer::AutoReview; }); let test = builder.build_with_auto_env(&server).await?; let output_file = test.cwd.path().join("guardian-timed-out.txt"); let tool_args = json!({ - "cmd": format!("printf should-not-run > {}", output_file.display()), - "sandbox_permissions": SandboxPermissions::RequireEscalated, - "justification": "Exercise Guardian timeout routing.", + "cmd": format!("touch {}", output_file.display()), + "sandbox_permissions": SandboxPermissions::UseDefault, }); let responses = mount_sse_sequence( &server,