fix: honor parent approvals for intercepted execs

2026-09-05 15:18:41 +00:00 · 2026-06-05 09:42:39 -07:00
parent 4f00e2ade6
commit 595ff156ec
2 changed files with 217 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;
@@ -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<AdditionalPermissionProfile>,
+    parent_sandbox_override_approved: bool,
    stopwatch: Stopwatch,
 }

@@ -639,6 +664,19 @@ impl EscalationPolicy for CoreShellActionProvider {
            SandboxPermissions::RequireEscalated => unsandboxed_allowed,
            SandboxPermissions::WithAdditionalPermissions => true,
        };
+        // The parent shell/unified-exec approval already covered unmatched
+        // prompts caused by its sandbox override. Explicit exec-policy rules
+        // still keep their own decisions. Guardian-routed turns keep the
+        // prompt so Guardian can review each intercepted execve independently.
+        let decision = if self.parent_sandbox_override_approved
+            && !routes_approval_to_guardian(&self.turn)
+            && evaluation.decision == Decision::Prompt
+            && !decision_driven_by_policy
+        {
+            Decision::Allow
+        } else {
+            evaluation.decision
+        };

        let decision_source = if decision_driven_by_policy {
            DecisionSource::PrefixRule
@@ -656,7 +694,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
@@ -11,6 +11,7 @@ use crate::config::Constrained;
 use crate::sandboxing::SandboxPermissions;
 use crate::session::tests::make_session_and_context;
 use anyhow::Context;
+use codex_config::types::ApprovalsReviewer;
 use codex_execpolicy::Decision;
 use codex_execpolicy::Evaluation;
 use codex_execpolicy::PolicyParser;
@@ -153,6 +154,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 +465,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 +487,143 @@ 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_bypass_guardian_routed_exec_prompt() -> anyhow::Result<()> {
+    let (session, mut turn_context) = make_session_and_context().await;
+    let approval_policy = AskForApproval::Granular(GranularApprovalConfig {
+        sandbox_approval: false,
+        rules: true,
+        skill_approval: true,
+        request_permissions: true,
+        mcp_elicitations: true,
+    });
+    turn_context.approval_policy = Constrained::allow_any(approval_policy);
+    let mut config = (*turn_context.config).clone();
+    config.approvals_reviewer = ApprovalsReviewer::AutoReview;
+    turn_context.config = Arc::new(config);
+    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-guardian-routed".to_string(),
+        tool_name: GuardianCommandSource::Shell,
+        approval_policy,
+        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]
+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 +736,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 +951,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 +995,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)),
    };