From 87b211709e4e6c0bfd50ed3bd513e2a734958dfe Mon Sep 17 00:00:00 2001 From: zhao-oai Date: Fri, 21 Nov 2025 18:03:23 -0500 Subject: [PATCH] bypass sandbox for policy approved commands (#7110) allowing cmds greenlit by execpolicy to bypass sandbox + minor refactor for a world where we have execpolicy rules with specific sandbox requirements --- codex-rs/core/src/exec_policy.rs | 8 +++++-- codex-rs/core/src/tools/orchestrator.rs | 16 +++++++------ codex-rs/core/src/tools/runtimes/shell.rs | 16 +++++++++++-- .../core/src/tools/runtimes/unified_exec.rs | 16 +++++++++++-- codex-rs/core/src/tools/sandboxing.rs | 24 ++++++++++++++----- 5 files changed, 61 insertions(+), 19 deletions(-) diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 2a5d3904e..15e591648 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -107,7 +107,9 @@ fn evaluate_with_policy( }) } } - Decision::Allow => Some(ApprovalRequirement::Skip), + Decision::Allow => Some(ApprovalRequirement::Skip { + bypass_sandbox: true, + }), }, Evaluation::NoMatch { .. } => None, } @@ -132,7 +134,9 @@ pub(crate) fn create_approval_requirement_for_command( ) { ApprovalRequirement::NeedsApproval { reason: None } } else { - ApprovalRequirement::Skip + ApprovalRequirement::Skip { + bypass_sandbox: false, + } } } diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 7e8e152f6..de23d510b 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -14,6 +14,7 @@ use crate::tools::sandboxing::ApprovalCtx; use crate::tools::sandboxing::ApprovalRequirement; use crate::tools::sandboxing::ProvidesSandboxRetryData; use crate::tools::sandboxing::SandboxAttempt; +use crate::tools::sandboxing::SandboxOverride; use crate::tools::sandboxing::ToolCtx; use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::ToolRuntime; @@ -57,7 +58,7 @@ impl ToolOrchestrator { default_approval_requirement(approval_policy, &turn_ctx.sandbox_policy) }); match requirement { - ApprovalRequirement::Skip => { + ApprovalRequirement::Skip { .. } => { otel.tool_decision(otel_tn, otel_ci, ReviewDecision::Approved, otel_cfg); } ApprovalRequirement::Forbidden { reason } => { @@ -100,12 +101,13 @@ impl ToolOrchestrator { } // 2) First attempt under the selected sandbox. - let mut initial_sandbox = self - .sandbox - .select_initial(&turn_ctx.sandbox_policy, tool.sandbox_preference()); - if tool.wants_escalated_first_attempt(req) { - initial_sandbox = crate::exec::SandboxType::None; - } + let initial_sandbox = match tool.sandbox_mode_for_first_attempt(req) { + SandboxOverride::BypassSandboxFirstAttempt => crate::exec::SandboxType::None, + SandboxOverride::NoOverride => self + .sandbox + .select_initial(&turn_ctx.sandbox_policy, tool.sandbox_preference()), + }; + // Platform-specific flag gating is handled by SandboxManager::select_initial // via crate::safety::get_platform_sandbox(). let initial_attempt = SandboxAttempt { diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index b46f72b48..56c72a827 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -12,6 +12,7 @@ use crate::tools::sandboxing::ApprovalCtx; use crate::tools::sandboxing::ApprovalRequirement; use crate::tools::sandboxing::ProvidesSandboxRetryData; use crate::tools::sandboxing::SandboxAttempt; +use crate::tools::sandboxing::SandboxOverride; use crate::tools::sandboxing::SandboxRetryData; use crate::tools::sandboxing::Sandboxable; use crate::tools::sandboxing::SandboxablePreference; @@ -117,8 +118,19 @@ impl Approvable for ShellRuntime { Some(req.approval_requirement.clone()) } - fn wants_escalated_first_attempt(&self, req: &ShellRequest) -> bool { - req.with_escalated_permissions.unwrap_or(false) + fn sandbox_mode_for_first_attempt(&self, req: &ShellRequest) -> SandboxOverride { + if req.with_escalated_permissions.unwrap_or(false) + || matches!( + req.approval_requirement, + ApprovalRequirement::Skip { + bypass_sandbox: true + } + ) + { + SandboxOverride::BypassSandboxFirstAttempt + } else { + SandboxOverride::NoOverride + } } } diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 3f0362259..0f306e6ff 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -13,6 +13,7 @@ use crate::tools::sandboxing::ApprovalCtx; use crate::tools::sandboxing::ApprovalRequirement; use crate::tools::sandboxing::ProvidesSandboxRetryData; use crate::tools::sandboxing::SandboxAttempt; +use crate::tools::sandboxing::SandboxOverride; use crate::tools::sandboxing::SandboxRetryData; use crate::tools::sandboxing::Sandboxable; use crate::tools::sandboxing::SandboxablePreference; @@ -135,8 +136,19 @@ impl Approvable for UnifiedExecRuntime<'_> { Some(req.approval_requirement.clone()) } - fn wants_escalated_first_attempt(&self, req: &UnifiedExecRequest) -> bool { - req.with_escalated_permissions.unwrap_or(false) + fn sandbox_mode_for_first_attempt(&self, req: &UnifiedExecRequest) -> SandboxOverride { + if req.with_escalated_permissions.unwrap_or(false) + || matches!( + req.approval_requirement, + ApprovalRequirement::Skip { + bypass_sandbox: true + } + ) + { + SandboxOverride::BypassSandboxFirstAttempt + } else { + SandboxOverride::NoOverride + } } } diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index f9e3e20ea..df10db952 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -89,8 +89,12 @@ pub(crate) struct ApprovalCtx<'a> { // Specifies what tool orchestrator should do with a given tool call. #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) enum ApprovalRequirement { - /// No approval required for this tool call - Skip, + /// No approval required for this tool call. + Skip { + /// The first attempt should skip sandboxing (e.g., when explicitly + /// greenlit by policy). + bypass_sandbox: bool, + }, /// Approval required for this tool call NeedsApproval { reason: Option }, /// Execution forbidden for this tool call @@ -113,10 +117,18 @@ pub(crate) fn default_approval_requirement( if needs_approval { ApprovalRequirement::NeedsApproval { reason: None } } else { - ApprovalRequirement::Skip + ApprovalRequirement::Skip { + bypass_sandbox: false, + } } } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum SandboxOverride { + NoOverride, + BypassSandboxFirstAttempt, +} + pub(crate) trait Approvable { type ApprovalKey: Hash + Eq + Clone + Debug + Serialize; @@ -124,9 +136,9 @@ pub(crate) trait Approvable { /// Some tools may request to skip the sandbox on the first attempt /// (e.g., when the request explicitly asks for escalated permissions). - /// Defaults to `false`. - fn wants_escalated_first_attempt(&self, _req: &Req) -> bool { - false + /// Defaults to `NoOverride`. + fn sandbox_mode_for_first_attempt(&self, _req: &Req) -> SandboxOverride { + SandboxOverride::NoOverride } fn should_bypass_approval(&self, policy: AskForApproval, already_approved: bool) -> bool {