diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index c4bf9ed5a1..90ba97210c 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -17,6 +17,7 @@ use crate::shell::ShellType; use crate::tools::runtimes::build_sandbox_command; use crate::tools::runtimes::exec_env_for_sandbox_permissions; use crate::tools::runtimes::prepend_zsh_fork_bin_to_path; +use crate::tools::sandboxing::ExecApprovalRequirement; use crate::tools::sandboxing::PermissionRequestPayload; use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::ToolCtx; @@ -100,6 +101,19 @@ fn approval_sandbox_permissions( } } +fn parent_approved_sandbox_override( + sandbox_permissions: SandboxPermissions, + additional_permissions_preapproved: bool, + exec_approval_requirement: &ExecApprovalRequirement, +) -> bool { + sandbox_permissions.requests_sandbox_override() + && (additional_permissions_preapproved + || matches!( + exec_approval_requirement, + ExecApprovalRequirement::NeedsApproval { .. } + )) +} + pub(super) async fn try_run_zsh_fork( req: &ShellRequest, attempt: &SandboxAttempt<'_>, @@ -228,6 +242,11 @@ pub(super) async fn try_run_zsh_fork( sandbox_permissions: req.sandbox_permissions, approval_sandbox_permissions, prompt_permissions: req.additional_permissions.clone(), + parent_sandbox_override_approved: parent_approved_sandbox_override( + req.sandbox_permissions, + req.additional_permissions_preapproved, + &req.exec_approval_requirement, + ), stopwatch: stopwatch.clone(), }; @@ -304,6 +323,11 @@ pub(crate) async fn prepare_unified_exec_zsh_fork( req.additional_permissions_preapproved, ), prompt_permissions: req.additional_permissions.clone(), + parent_sandbox_override_approved: parent_approved_sandbox_override( + req.sandbox_permissions, + req.additional_permissions_preapproved, + &req.exec_approval_requirement, + ), stopwatch: Stopwatch::unlimited(), }; @@ -336,6 +360,7 @@ struct CoreShellActionProvider { sandbox_permissions: SandboxPermissions, approval_sandbox_permissions: SandboxPermissions, prompt_permissions: Option, + parent_sandbox_override_approved: bool, stopwatch: Stopwatch, } @@ -637,6 +662,17 @@ impl EscalationPolicy for CoreShellActionProvider { let needs_unsandboxed_escalation = unsandboxed_allowed && (self.sandbox_permissions.requires_escalated_permissions() || decision_driven_by_policy); + // The parent shell/unified-exec approval already covered unmatched + // prompts caused by its sandbox override. Explicit exec-policy rules + // still keep their own decisions. + let decision = if self.parent_sandbox_override_approved + && evaluation.decision == Decision::Prompt + && !decision_driven_by_policy + { + Decision::Allow + } else { + evaluation.decision + }; let needs_escalation = self.sandbox_permissions.uses_additional_permissions() || needs_unsandboxed_escalation; @@ -656,7 +692,7 @@ impl EscalationPolicy for CoreShellActionProvider { ), }; self.process_decision( - evaluation.decision, + decision, needs_escalation, program, argv, diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index 87e0854176..84066e0928 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -153,6 +153,42 @@ fn approval_sandbox_permissions_only_downgrades_preapproved_additional_permissio ); } +#[test] +fn parent_approved_sandbox_override_tracks_parent_approval_sources() { + assert!(super::parent_approved_sandbox_override( + SandboxPermissions::RequireEscalated, + /*additional_permissions_preapproved*/ false, + &crate::tools::sandboxing::ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + }, + )); + assert!(super::parent_approved_sandbox_override( + SandboxPermissions::WithAdditionalPermissions, + /*additional_permissions_preapproved*/ true, + &crate::tools::sandboxing::ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + }, + )); + assert!(!super::parent_approved_sandbox_override( + SandboxPermissions::UseDefault, + /*additional_permissions_preapproved*/ false, + &crate::tools::sandboxing::ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + }, + )); + assert!(!super::parent_approved_sandbox_override( + SandboxPermissions::RequireEscalated, + /*additional_permissions_preapproved*/ false, + &crate::tools::sandboxing::ExecApprovalRequirement::Skip { + bypass_sandbox: true, + proposed_execpolicy_amendment: None, + }, + )); +} + #[test] fn extract_shell_script_preserves_login_flag() { assert_eq!( @@ -428,6 +464,7 @@ async fn preapproved_additional_permissions_escalate_intercepted_exec() -> anyho sandbox_permissions: SandboxPermissions::WithAdditionalPermissions, approval_sandbox_permissions: SandboxPermissions::UseDefault, prompt_permissions: Some(requested_permissions), + parent_sandbox_override_approved: true, stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)), }; @@ -449,6 +486,93 @@ async fn preapproved_additional_permissions_escalate_intercepted_exec() -> anyho Ok(()) } +#[tokio::test] +async fn parent_require_escalated_approval_escalates_intercepted_exec() -> anyhow::Result<()> { + let (session, turn_context) = make_session_and_context().await; + let workdir = test_sandbox_cwd(); + let provider = CoreShellActionProvider { + policy: Arc::new(RwLock::new(codex_execpolicy::Policy::empty())), + session: Arc::new(session), + turn: Arc::new(turn_context), + call_id: "parent-require-escalated".to_string(), + tool_name: GuardianCommandSource::Shell, + approval_policy: AskForApproval::OnRequest, + permission_profile: PermissionProfile::workspace_write(), + file_system_sandbox_policy: read_only_file_system_sandbox_policy(), + sandbox_policy_cwd: workdir.clone(), + sandbox_permissions: SandboxPermissions::RequireEscalated, + approval_sandbox_permissions: SandboxPermissions::RequireEscalated, + prompt_permissions: None, + parent_sandbox_override_approved: true, + stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)), + }; + + let action = codex_shell_escalation::EscalationPolicy::determine_action( + &provider, + &AbsolutePathBuf::from_absolute_path("/usr/bin/curl")?, + &["curl".to_string(), "example.com".to_string()], + &workdir, + ) + .await?; + + assert_eq!( + action, + codex_shell_escalation::EscalationDecision::Escalate(EscalationExecution::Unsandboxed) + ); + + Ok(()) +} + +#[tokio::test] +async fn parent_approval_does_not_override_intercepted_exec_policy_prompt() -> anyhow::Result<()> { + let (session, turn_context) = make_session_and_context().await; + let mut parser = PolicyParser::new(); + parser.parse( + "test.rules", + r#"prefix_rule(pattern = ["curl"], decision = "prompt")"#, + )?; + let workdir = test_sandbox_cwd(); + let provider = CoreShellActionProvider { + policy: Arc::new(RwLock::new(parser.build())), + session: Arc::new(session), + turn: Arc::new(turn_context), + call_id: "parent-policy-prompt".to_string(), + tool_name: GuardianCommandSource::Shell, + approval_policy: AskForApproval::Granular(GranularApprovalConfig { + sandbox_approval: true, + rules: false, + skill_approval: true, + request_permissions: true, + mcp_elicitations: true, + }), + permission_profile: PermissionProfile::workspace_write(), + file_system_sandbox_policy: read_only_file_system_sandbox_policy(), + sandbox_policy_cwd: workdir.clone(), + sandbox_permissions: SandboxPermissions::RequireEscalated, + approval_sandbox_permissions: SandboxPermissions::RequireEscalated, + prompt_permissions: None, + parent_sandbox_override_approved: true, + stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)), + }; + + let action = codex_shell_escalation::EscalationPolicy::determine_action( + &provider, + &AbsolutePathBuf::from_absolute_path("/usr/bin/curl")?, + &["curl".to_string(), "example.com".to_string()], + &workdir, + ) + .await?; + + assert_eq!( + action, + codex_shell_escalation::EscalationDecision::Deny { + reason: Some("Execution forbidden by policy".to_string()), + } + ); + + Ok(()) +} + #[tokio::test(flavor = "current_thread")] async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Result<()> { let (session, mut turn_context) = make_session_and_context().await; @@ -561,6 +685,7 @@ async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Resul sandbox_permissions: SandboxPermissions::RequireEscalated, approval_sandbox_permissions: SandboxPermissions::RequireEscalated, prompt_permissions: None, + parent_sandbox_override_approved: false, stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)), }; @@ -775,6 +900,7 @@ prefix_rule(pattern = ["{cat_path_literal}"], decision = "allow") sandbox_permissions: SandboxPermissions::UseDefault, approval_sandbox_permissions: SandboxPermissions::UseDefault, prompt_permissions: None, + parent_sandbox_override_approved: false, stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)), }; @@ -818,6 +944,7 @@ async fn denied_reads_keep_granular_sandbox_rejection_for_escalation() -> anyhow sandbox_permissions: SandboxPermissions::RequireEscalated, approval_sandbox_permissions: SandboxPermissions::RequireEscalated, prompt_permissions: None, + parent_sandbox_override_approved: false, stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)), }; diff --git a/codex-rs/core/tests/common/zsh_fork.rs b/codex-rs/core/tests/common/zsh_fork.rs index e58ebd81dd..5abfc753b9 100644 --- a/codex-rs/core/tests/common/zsh_fork.rs +++ b/codex-rs/core/tests/common/zsh_fork.rs @@ -94,6 +94,33 @@ where builder.build(server).await } +pub async fn build_unified_exec_zsh_fork_test( + server: &wiremock::MockServer, + runtime: ZshForkRuntime, + approval_policy: AskForApproval, + permission_profile: PermissionProfile, + pre_build_hook: F, +) -> Result +where + F: FnOnce(&Path) + Send + 'static, +{ + let mut builder = test_codex() + .with_pre_build_hook(pre_build_hook) + .with_config(move |config| { + runtime.apply_to_config(config, approval_policy, permission_profile); + config.use_experimental_unified_exec_tool = true; + config + .features + .enable(Feature::UnifiedExec) + .expect("test config should allow feature update"); + config + .features + .enable(Feature::UnifiedExecZshFork) + .expect("test config should allow feature update"); + }); + builder.build(server).await +} + fn find_test_zsh_path() -> Result> { let repo_root = codex_utils_cargo_bin::repo_root()?; let dotslash_zsh = repo_root.join("codex-rs/app-server/tests/suite/zsh"); diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index aceac7320e..61d1037e00 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -43,6 +43,7 @@ use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; use core_test_support::wait_for_event_with_timeout; +use core_test_support::zsh_fork::build_unified_exec_zsh_fork_test; use core_test_support::zsh_fork::build_zsh_fork_test; use core_test_support::zsh_fork::restrictive_workspace_write_profile; use core_test_support::zsh_fork::zsh_fork_runtime; @@ -2800,6 +2801,122 @@ async fn matched_prefix_rule_runs_unsandboxed_under_zsh_fork() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[cfg(unix)] +async fn unified_exec_zsh_fork_parent_approval_escalates_intercepted_exec() -> Result<()> { + skip_if_no_network!(Ok(())); + + let Some(runtime) = zsh_fork_runtime("unified-exec zsh-fork parent approval test")? else { + return Ok(()); + }; + + let approval_policy = AskForApproval::OnRequest; + let permission_profile = restrictive_workspace_write_profile(); + let outside_dir = tempfile::tempdir_in(std::env::current_dir()?)?; + let outside_path = outside_dir + .path() + .join("unified-exec-zsh-fork-parent-approval.txt"); + let command = format!("touch {outside_path:?}"); + + let server = start_mock_server().await; + let outside_path_for_hook = outside_path.clone(); + let test = build_unified_exec_zsh_fork_test( + &server, + runtime, + approval_policy, + permission_profile, + move |_home| { + let _ = fs::remove_file(&outside_path_for_hook); + }, + ) + .await?; + + let call_id = "uexec-zsh-fork-parent-approval"; + let event = exec_command_event( + call_id, + &command, + Some(30_000), + SandboxPermissions::RequireEscalated, + Some("write outside the workspace for the test"), + )?; + let _ = mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-uexec-zsh-fork-parent-approval-1"), + event, + ev_completed("resp-uexec-zsh-fork-parent-approval-1"), + ]), + ) + .await; + let results = mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-uexec-zsh-fork-parent-approval-1", "done"), + ev_completed("resp-uexec-zsh-fork-parent-approval-2"), + ]), + ) + .await; + + let session_model = test.session_configured.model.clone(); + let (sandbox_policy, permission_profile) = turn_permission_fields( + test.session_configured.permission_profile.clone(), + test.cwd.path(), + ); + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "run approved unified exec through zsh fork".into(), + text_elements: Vec::new(), + }], + environments: None, + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: codex_protocol::protocol::ThreadSettingsOverrides { + cwd: Some(test.cwd.path().to_path_buf()), + approval_policy: Some(approval_policy), + approvals_reviewer: Some(ApprovalsReviewer::User), + sandbox_policy: Some(sandbox_policy), + permission_profile, + collaboration_mode: Some(codex_protocol::config_types::CollaborationMode { + mode: codex_protocol::config_types::ModeKind::Default, + settings: codex_protocol::config_types::Settings { + model: session_model, + reasoning_effort: None, + developer_instructions: None, + }, + }), + ..Default::default() + }, + }) + .await?; + + let approval = expect_exec_approval(&test, &command).await; + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::Approved, + }) + .await?; + wait_for_completion_without_approval(&test).await; + + let result = parse_result(&results.single_request().function_call_output(call_id)); + assert_eq!( + result.exit_code.unwrap_or(0), + 0, + "approved unified exec zsh-fork command should complete: {}", + result.stdout + ); + assert!( + outside_path.exists(), + "expected parent approval to let the intercepted touch run unsandboxed; output: {}", + result.stdout + ); + + Ok(()) +} + #[tokio::test(flavor = "current_thread")] #[cfg(unix)] async fn invalid_requested_prefix_rule_falls_back_for_compound_command() -> Result<()> {