fix(permissions): preserve managed deny-read during escalation

codex-rs/core/src/exec_policy_tests.rs

@@ -1705,8 +1705,12 @@ async fn proposed_execpolicy_amendment_is_present_when_heuristics_allow() {
 async fn proposed_execpolicy_amendment_is_suppressed_when_policy_matches_allow() {
     assert_exec_approval_requirement_for_command(
         ExecApprovalRequirementScenario {
-            policy_src: Some(r#"prefix_rule(pattern=["echo"], decision="allow")"#.to_string()),
-            command: vec!["echo".to_string(), "safe".to_string()],
+            policy_src: Some(r#"prefix_rule(pattern=["python3"], decision="allow")"#.to_string()),
+            command: vec![
+                "python3".to_string(),
+                "-c".to_string(),
+                "print(1)".to_string(),
+            ],
             approval_policy: AskForApproval::OnRequest,
             sandbox_policy: SandboxPolicy::new_read_only_policy(),
             file_system_sandbox_policy: read_only_file_system_sandbox_policy(),

codex-rs/core/src/tools/orchestrator.rs

@@ -27,6 +27,7 @@ use crate::tools::sandboxing::ToolCtx;
 use crate::tools::sandboxing::ToolError;
 use crate::tools::sandboxing::ToolRuntime;
 use crate::tools::sandboxing::default_exec_approval_requirement;
+use crate::tools::sandboxing::sandbox_override_for_first_attempt;
 use codex_hooks::PermissionRequestDecision;
 use codex_otel::ToolDecisionSource;
 use codex_protocol::error::CodexErr;
@@ -149,7 +150,7 @@ impl ToolOrchestrator {
         let requirement = tool.exec_approval_requirement(req).unwrap_or_else(|| {
             default_exec_approval_requirement(approval_policy, &file_system_sandbox_policy)
         });
-        match requirement {
+        match &requirement {
             ExecApprovalRequirement::Skip { .. } => {
                 if strict_auto_review {
                     let guardian_review_id = Some(new_guardian_review_id());
@@ -184,7 +185,7 @@ impl ToolOrchestrator {
                 }
             }
             ExecApprovalRequirement::Forbidden { reason } => {
-                return Err(ToolError::Rejected(reason));
+                return Err(ToolError::Rejected(reason.clone()));
             }
             ExecApprovalRequirement::NeedsApproval { reason, .. } => {
                 let guardian_review_id = use_guardian.then(new_guardian_review_id);
@@ -193,7 +194,7 @@ impl ToolOrchestrator {
                     turn: &tool_ctx.turn,
                     call_id: &tool_ctx.call_id,
                     guardian_review_id: guardian_review_id.clone(),
-                    retry_reason: reason,
+                    retry_reason: reason.clone(),
                     network_approval_context: None,
                 };
                 let decision = Self::request_approval(
@@ -214,8 +215,13 @@ impl ToolOrchestrator {
         }
 
         // 2) First attempt under the selected sandbox.
+        let sandbox_override = sandbox_override_for_first_attempt(
+            tool.sandbox_permissions(req),
+            &requirement,
+            &file_system_sandbox_policy,
+        );
         let managed_network_active = turn_ctx.network.is_some();
-        let initial_sandbox = match tool.sandbox_mode_for_first_attempt(req) {
+        let initial_sandbox = match sandbox_override {
             SandboxOverride::BypassSandboxFirstAttempt => SandboxType::None,
             SandboxOverride::NoOverride => self.sandbox.select_initial(
                 &file_system_sandbox_policy,

codex-rs/core/src/tools/runtimes/shell.rs

@@ -28,13 +28,11 @@ use crate::tools::sandboxing::ApprovalCtx;
 use crate::tools::sandboxing::ExecApprovalRequirement;
 use crate::tools::sandboxing::PermissionRequestPayload;
 use crate::tools::sandboxing::SandboxAttempt;
-use crate::tools::sandboxing::SandboxOverride;
 use crate::tools::sandboxing::Sandboxable;
 use crate::tools::sandboxing::ToolCtx;
 use crate::tools::sandboxing::ToolError;
 use crate::tools::sandboxing::ToolRuntime;
 use crate::tools::sandboxing::managed_network_for_sandbox_permissions;
-use crate::tools::sandboxing::sandbox_override_for_first_attempt;
 use crate::tools::sandboxing::with_cached_approval;
 use codex_network_proxy::NetworkProxy;
 use codex_protocol::exec_output::ExecToolCallOutput;
@@ -210,8 +208,8 @@ impl Approvable<ShellRequest> for ShellRuntime {
         ))
     }
 
-    fn sandbox_mode_for_first_attempt(&self, req: &ShellRequest) -> SandboxOverride {
-        sandbox_override_for_first_attempt(req.sandbox_permissions, &req.exec_approval_requirement)
+    fn sandbox_permissions(&self, req: &ShellRequest) -> SandboxPermissions {
+        req.sandbox_permissions
     }
 }
 

codex-rs/core/src/tools/runtimes/unified_exec.rs

@@ -26,13 +26,11 @@ use crate::tools::sandboxing::ApprovalCtx;
 use crate::tools::sandboxing::ExecApprovalRequirement;
 use crate::tools::sandboxing::PermissionRequestPayload;
 use crate::tools::sandboxing::SandboxAttempt;
-use crate::tools::sandboxing::SandboxOverride;
 use crate::tools::sandboxing::Sandboxable;
 use crate::tools::sandboxing::ToolCtx;
 use crate::tools::sandboxing::ToolError;
 use crate::tools::sandboxing::ToolRuntime;
 use crate::tools::sandboxing::managed_network_for_sandbox_permissions;
-use crate::tools::sandboxing::sandbox_override_for_first_attempt;
 use crate::tools::sandboxing::with_cached_approval;
 use crate::unified_exec::NoopSpawnLifecycle;
 use crate::unified_exec::UnifiedExecError;
@@ -212,8 +210,8 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
         ))
     }
 
-    fn sandbox_mode_for_first_attempt(&self, req: &UnifiedExecRequest) -> SandboxOverride {
-        sandbox_override_for_first_attempt(req.sandbox_permissions, &req.exec_approval_requirement)
+    fn sandbox_permissions(&self, req: &UnifiedExecRequest) -> SandboxPermissions {
+        req.sandbox_permissions
     }
 }
 

codex-rs/core/src/tools/sandboxing.rs

@@ -248,18 +248,27 @@ pub(crate) enum SandboxOverride {
 pub(crate) fn sandbox_override_for_first_attempt(
     sandbox_permissions: SandboxPermissions,
     exec_approval_requirement: &ExecApprovalRequirement,
+    file_system_sandbox_policy: &FileSystemSandboxPolicy,
 ) -> SandboxOverride {
     // ExecPolicy `Allow` can intentionally imply full trust (Skip + bypass_sandbox=true),
     // which supersedes `with_additional_permissions` sandboxed execution hints.
-    if sandbox_permissions.requires_escalated_permissions()
-        || matches!(
-            exec_approval_requirement,
-            ExecApprovalRequirement::Skip {
-                bypass_sandbox: true,
-                ..
-            }
-        )
-    {
+    if matches!(
+        exec_approval_requirement,
+        ExecApprovalRequirement::Skip {
+            bypass_sandbox: true,
+            ..
+        }
+    ) {
+        return SandboxOverride::BypassSandboxFirstAttempt;
+    }
+
+    // Deny-read restrictions suppress explicit escalation because that path
+    // would otherwise discard the filesystem policy entirely.
+    if file_system_sandbox_policy.has_denied_read_restrictions() {
+        return SandboxOverride::NoOverride;
+    }
+
+    if sandbox_permissions.requires_escalated_permissions() {
         SandboxOverride::BypassSandboxFirstAttempt
     } else {
         SandboxOverride::NoOverride
@@ -289,11 +298,10 @@ pub(crate) trait Approvable<Req> {
     // requests touching a subset can be auto-approved.
     fn approval_keys(&self, req: &Req) -> Vec<Self::ApprovalKey>;
 
-    /// Some tools may request to skip the sandbox on the first attempt
-    /// (e.g., when the request explicitly asks for escalated permissions).
-    /// Defaults to `NoOverride`.
-    fn sandbox_mode_for_first_attempt(&self, _req: &Req) -> SandboxOverride {
-        SandboxOverride::NoOverride
+    /// Return per-request sandbox permissions for first-attempt sandbox
+    /// selection. Most tools use the ambient sandbox policy unchanged.
+    fn sandbox_permissions(&self, _req: &Req) -> SandboxPermissions {
+        SandboxPermissions::UseDefault
     }
 
     fn should_bypass_approval(&self, policy: AskForApproval, already_approved: bool) -> bool {

codex-rs/core/src/tools/sandboxing_tests.rs

@@ -1,6 +1,9 @@
 use super::*;
 use crate::sandboxing::SandboxPermissions;
 use crate::tools::hook_names::HookToolName;
+use codex_protocol::permissions::FileSystemAccessMode;
+use codex_protocol::permissions::FileSystemPath;
+use codex_protocol::permissions::FileSystemSandboxEntry;
 use codex_protocol::protocol::GranularApprovalConfig;
 use codex_protocol::protocol::NetworkAccess;
 use pretty_assertions::assert_eq;
@@ -120,6 +123,7 @@ fn additional_permissions_allow_bypass_sandbox_first_attempt_when_execpolicy_ski
                 bypass_sandbox: true,
                 proposed_execpolicy_amendment: None,
             },
+            &FileSystemSandboxPolicy::default(),
         ),
         SandboxOverride::BypassSandboxFirstAttempt
     );
@@ -134,7 +138,43 @@ fn guardian_bypasses_sandbox_for_explicit_escalation_on_first_attempt() {
                 bypass_sandbox: false,
                 proposed_execpolicy_amendment: None,
             },
+            &FileSystemSandboxPolicy::default(),
         ),
         SandboxOverride::BypassSandboxFirstAttempt
     );
 }
+
+#[test]
+fn deny_read_blocks_explicit_escalation_but_preserves_policy_bypass() {
+    let file_system_policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
+        path: FileSystemPath::GlobPattern {
+            pattern: "**/*.env".to_string(),
+        },
+        access: FileSystemAccessMode::None,
+    }]);
+
+    assert_eq!(
+        sandbox_override_for_first_attempt(
+            SandboxPermissions::RequireEscalated,
+            &ExecApprovalRequirement::Skip {
+                bypass_sandbox: false,
+                proposed_execpolicy_amendment: None,
+            },
+            &file_system_policy,
+        ),
+        SandboxOverride::NoOverride,
+        "explicit escalation would drop deny-read filesystem policy, so keep the first attempt sandboxed",
+    );
+    assert_eq!(
+        sandbox_override_for_first_attempt(
+            SandboxPermissions::WithAdditionalPermissions,
+            &ExecApprovalRequirement::Skip {
+                bypass_sandbox: true,
+                proposed_execpolicy_amendment: None,
+            },
+            &file_system_policy,
+        ),
+        SandboxOverride::BypassSandboxFirstAttempt,
+        "exec-policy allow rules intentionally bypass sandbox even when deny-read entries exist",
+    );
+}