fix: honor parent approvals for intercepted execs

2026-09-06 15:29:32 +00:00 · 2026-05-28 16:06:08 -07:00
parent 86b8ed790e
commit d592f5cdcb
2 changed files with 162 additions and 1 deletions
--- 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;
@@ -98,6 +99,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<'_>,
@@ -217,6 +231,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(),
    };

@@ -293,6 +312,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(),
    };

@@ -325,6 +349,7 @@ struct CoreShellActionProvider {
    sandbox_permissions: SandboxPermissions,
    approval_sandbox_permissions: SandboxPermissions,
    prompt_permissions: Option<AdditionalPermissionProfile>,
+    parent_sandbox_override_approved: bool,
    stopwatch: Stopwatch,
 }

@@ -616,6 +641,17 @@ impl EscalationPolicy for CoreShellActionProvider {
        // fallback function.
        let decision_driven_by_policy =
            Self::decision_driven_by_policy(&evaluation.matched_rules, evaluation.decision);
+        // 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.requests_sandbox_override() || decision_driven_by_policy;

@@ -633,7 +669,7 @@ impl EscalationPolicy for CoreShellActionProvider {
            ),
        };
        self.process_decision(
-            evaluation.decision,
+            decision,
            needs_escalation,
            program,
            argv,
--- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs
+++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs
@@ -136,6 +136,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!(
@@ -398,6 +434,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)),
    };

@@ -419,6 +456,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;
@@ -531,6 +655,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)),
    };