From a86cbebf526c5ca4167751249f77d478f4ebba8a Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 10:54:33 -0800 Subject: [PATCH] sandboxing: preserve denied paths when widening permissions --- codex-rs/core/src/exec.rs | 24 ++++++++++-------------- codex-rs/core/src/sandboxing/mod.rs | 26 ++++++-------------------- 2 files changed, 16 insertions(+), 34 deletions(-) diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index cd4da20616..487c653de1 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -32,6 +32,7 @@ use crate::sandboxing::CommandSpec; use crate::sandboxing::ExecRequest; use crate::sandboxing::SandboxManager; use crate::sandboxing::SandboxPermissions; +use crate::sandboxing::should_require_platform_sandbox; use crate::spawn::SpawnChildRequest; use crate::spawn::StdioPolicy; use crate::spawn::spawn_child_async; @@ -165,22 +166,17 @@ pub async fn process_exec_tool_call( ) -> Result { let windows_sandbox_level = params.windows_sandbox_level; let enforce_managed_network = params.network.is_some(); - let sandbox_type = match file_system_sandbox_policy.kind { - FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => { - if enforce_managed_network { - get_platform_sandbox( - windows_sandbox_level - != codex_protocol::config_types::WindowsSandboxLevel::Disabled, - ) - .unwrap_or(SandboxType::None) - } else { - SandboxType::None - } - } - _ => get_platform_sandbox( + let sandbox_type = if should_require_platform_sandbox( + file_system_sandbox_policy, + network_sandbox_policy, + enforce_managed_network, + ) { + get_platform_sandbox( windows_sandbox_level != codex_protocol::config_types::WindowsSandboxLevel::Disabled, ) - .unwrap_or(SandboxType::None), + .unwrap_or(SandboxType::None) + } else { + SandboxType::None }; tracing::debug!("Sandbox type: {sandbox_type:?}"); diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 724034502b..8541852f44 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -20,7 +20,6 @@ use crate::protocol::FileSystemPath; use crate::protocol::FileSystemSandboxEntry; use crate::protocol::FileSystemSandboxKind; use crate::protocol::FileSystemSandboxPolicy; -use crate::protocol::FileSystemSpecialPathKind; use crate::protocol::NetworkSandboxPolicy; use crate::protocol::SandboxPolicy; #[cfg(target_os = "macos")] @@ -215,7 +214,6 @@ fn additional_permission_roots( ) } -#[cfg_attr(not(test), allow(dead_code))] fn merge_file_system_policy_with_additional_permissions( file_system_policy: &FileSystemSandboxPolicy, extra_reads: Vec, @@ -249,7 +247,6 @@ fn merge_file_system_policy_with_additional_permissions( } } } - fn merge_read_only_access_with_additional_reads( read_only_access: &ReadOnlyAccess, extra_reads: Vec, @@ -369,19 +366,10 @@ pub(crate) fn should_require_platform_sandbox( } match file_system_policy.kind { - FileSystemSandboxKind::Restricted => !file_system_policy.entries.iter().any(|entry| { - entry.access == FileSystemAccessMode::Write - && matches!( - &entry.path, - FileSystemPath::Special { value } - if value.kind == FileSystemSpecialPathKind::Root - && value.subpath.is_none() - ) - }), + FileSystemSandboxKind::Restricted => !file_system_policy.has_full_disk_write_access(), FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => false, } } - #[derive(Default)] pub struct SandboxManager; @@ -461,13 +449,11 @@ impl SandboxManager { if extra_reads.is_empty() && extra_writes.is_empty() { file_system_policy.clone() } else { - match file_system_policy.kind { - FileSystemSandboxKind::Restricted => { - FileSystemSandboxPolicy::from(&effective_policy) - } - FileSystemSandboxKind::Unrestricted - | FileSystemSandboxKind::ExternalSandbox => file_system_policy.clone(), - } + merge_file_system_policy_with_additional_permissions( + file_system_policy, + extra_reads, + extra_writes, + ) }; let network_sandbox_policy = NetworkSandboxPolicy::from(&effective_policy); (file_system_sandbox_policy, network_sandbox_policy)