From 896ee672cc323f5a1e761f72a09cf0d809580c4a Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 20 May 2026 14:52:38 -0700 Subject: [PATCH] windows-sandbox: feed setup from resolved permissions (#23167) ## Why This is the next step in the Windows sandbox migration away from the legacy `SandboxPolicy` abstraction. #22923 moved write-root and token decisions onto `ResolvedWindowsSandboxPermissions`, but setup and identity still accepted `SandboxPolicy` and converted internally. This PR pushes that conversion outward so the setup path consumes the resolved Windows permission view directly. ## What Changed - Changed `SandboxSetupRequest` to carry `ResolvedWindowsSandboxPermissions` instead of `SandboxPolicy` plus policy cwd. - Updated setup refresh/elevation and identity credential preparation to use resolved permissions for read roots, write roots, network identity, and deny-write payload planning. - Removed the production `allow.rs` legacy wrapper; allow-path computation now takes resolved permissions directly. - Added a permissions-based world-writable audit entry point while keeping the existing legacy wrapper for compatibility. - Updated legacy ACL setup and the core Windows setup bridge to construct resolved permissions at the boundary. - Hardened the Windows sandbox integration test helper staging so Bazel retries can reuse an already-staged helper if a prior sandbox helper process still has the executable open. ## Verification - `cargo test -p codex-windows-sandbox` - `cargo test -p codex-core --test all --no-run` - `just fix -p codex-windows-sandbox` - `just fix -p codex-core` - Attempted `cargo check -p codex-windows-sandbox --target x86_64-pc-windows-gnullvm`, but the local machine is missing `x86_64-w64-mingw32-clang`; Windows CI should cover that target. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/23167). * #23715 * #23714 * __->__ #23167 --- codex-rs/core/src/windows_sandbox.rs | 7 +- codex-rs/core/tests/suite/windows_sandbox.rs | 17 +- codex-rs/windows-sandbox-rs/src/allow.rs | 23 ++- codex-rs/windows-sandbox-rs/src/audit.rs | 35 ++-- .../windows-sandbox-rs/src/elevated_impl.rs | 3 +- codex-rs/windows-sandbox-rs/src/identity.rs | 18 +- codex-rs/windows-sandbox-rs/src/lib.rs | 15 +- .../src/resolved_permissions.rs | 28 ++- codex-rs/windows-sandbox-rs/src/setup.rs | 160 +++++++++++------- codex-rs/windows-sandbox-rs/src/spawn_prep.rs | 9 +- .../src/unified_exec/backends/legacy.rs | 3 +- 11 files changed, 201 insertions(+), 117 deletions(-) diff --git a/codex-rs/core/src/windows_sandbox.rs b/codex-rs/core/src/windows_sandbox.rs index a094545bab..2c87c885ad 100644 --- a/codex-rs/core/src/windows_sandbox.rs +++ b/codex-rs/core/src/windows_sandbox.rs @@ -179,10 +179,13 @@ pub fn run_elevated_setup( env_map: &HashMap, codex_home: &Path, ) -> anyhow::Result<()> { + let permissions = + codex_windows_sandbox::ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd( + policy, policy_cwd, + ); codex_windows_sandbox::run_elevated_setup( codex_windows_sandbox::SandboxSetupRequest { - policy, - policy_cwd, + permissions: &permissions, command_cwd, env_map, codex_home, diff --git a/codex-rs/core/tests/suite/windows_sandbox.rs b/codex-rs/core/tests/suite/windows_sandbox.rs index 155c701d42..7b91a4f3cd 100644 --- a/codex-rs/core/tests/suite/windows_sandbox.rs +++ b/codex-rs/core/tests/suite/windows_sandbox.rs @@ -93,7 +93,22 @@ fn stage_windows_sandbox_helpers() -> anyhow::Result<()> { for helper_name in ["codex-windows-sandbox-setup", "codex-command-runner"] { let helper = codex_utils_cargo_bin::cargo_bin(helper_name)?; let file_name = Path::new(helper_name).with_extension("exe"); - std::fs::copy(helper, resources_dir.join(file_name))?; + let destination = resources_dir.join(file_name); + if let Err(err) = std::fs::copy(&helper, &destination) { + // A sandbox helper can briefly remain alive after the sandboxed + // command exits. Bazel may retry the test while that process still + // has the staged executable open, so keep the already-staged copy. + if err.kind() == std::io::ErrorKind::PermissionDenied && destination.exists() { + continue; + } + return Err(err).with_context(|| { + format!( + "stage Windows sandbox helper {} at {}", + helper.display(), + destination.display() + ) + }); + } } Ok(()) } diff --git a/codex-rs/windows-sandbox-rs/src/allow.rs b/codex-rs/windows-sandbox-rs/src/allow.rs index b13081cdb5..541a86fc1c 100644 --- a/codex-rs/windows-sandbox-rs/src/allow.rs +++ b/codex-rs/windows-sandbox-rs/src/allow.rs @@ -1,4 +1,3 @@ -use crate::policy::SandboxPolicy; use crate::resolved_permissions::ResolvedWindowsSandboxPermissions; use dunce::canonicalize; use std::collections::HashMap; @@ -12,17 +11,6 @@ pub struct AllowDenyPaths { pub deny: HashSet, } -pub(crate) fn compute_allow_paths( - policy: &SandboxPolicy, - policy_cwd: &Path, - command_cwd: &Path, - env_map: &HashMap, -) -> AllowDenyPaths { - let permissions = - ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd(policy, policy_cwd); - compute_allow_paths_for_permissions(&permissions, command_cwd, env_map) -} - pub(crate) fn compute_allow_paths_for_permissions( permissions: &ResolvedWindowsSandboxPermissions, command_cwd: &Path, @@ -61,6 +49,17 @@ mod tests { use std::fs; use tempfile::TempDir; + fn compute_allow_paths( + policy: &SandboxPolicy, + policy_cwd: &Path, + command_cwd: &Path, + env_map: &HashMap, + ) -> AllowDenyPaths { + let permissions = + ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd(policy, policy_cwd); + compute_allow_paths_for_permissions(&permissions, command_cwd, env_map) + } + #[test] fn includes_additional_writable_roots() { let tmp = TempDir::new().expect("tempdir"); diff --git a/codex-rs/windows-sandbox-rs/src/audit.rs b/codex-rs/windows-sandbox-rs/src/audit.rs index cab0a06adf..0d6f4b5744 100644 --- a/codex-rs/windows-sandbox-rs/src/audit.rs +++ b/codex-rs/windows-sandbox-rs/src/audit.rs @@ -224,15 +224,33 @@ pub fn apply_world_writable_scan_and_denies( env_map: &std::collections::HashMap, sandbox_policy: &SandboxPolicy, logs_base_dir: Option<&Path>, +) -> Result<()> { + let permissions = + ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd(sandbox_policy, cwd); + apply_world_writable_scan_and_denies_for_permissions( + codex_home, + cwd, + env_map, + &permissions, + logs_base_dir, + ) +} + +pub fn apply_world_writable_scan_and_denies_for_permissions( + codex_home: &Path, + cwd: &Path, + env_map: &std::collections::HashMap, + permissions: &ResolvedWindowsSandboxPermissions, + logs_base_dir: Option<&Path>, ) -> Result<()> { let flagged = audit_everyone_writable(cwd, env_map, logs_base_dir)?; if flagged.is_empty() { return Ok(()); } - if let Err(err) = apply_capability_denies_for_world_writable( + if let Err(err) = apply_capability_denies_for_world_writable_for_permissions( codex_home, &flagged, - sandbox_policy, + permissions, cwd, env_map, logs_base_dir, @@ -245,10 +263,10 @@ pub fn apply_world_writable_scan_and_denies( Ok(()) } -pub fn apply_capability_denies_for_world_writable( +fn apply_capability_denies_for_world_writable_for_permissions( codex_home: &Path, flagged: &[PathBuf], - sandbox_policy: &SandboxPolicy, + permissions: &ResolvedWindowsSandboxPermissions, cwd: &Path, env_map: &std::collections::HashMap, logs_base_dir: Option<&Path>, @@ -260,18 +278,13 @@ pub fn apply_capability_denies_for_world_writable( let cap_path = cap_sid_file(codex_home); let caps = load_or_create_cap_sids(codex_home)?; std::fs::write(&cap_path, serde_json::to_string(&caps)?)?; - if matches!( - sandbox_policy, - SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } - ) { + if !permissions.is_enforceable_by_windows_sandbox() { return Ok(()); } - let permissions = - ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd(sandbox_policy, cwd); let (active_sids, workspace_roots): (Vec, Vec) = if permissions.uses_write_capabilities_for_cwd(cwd, env_map) { let roots = effective_write_roots_for_permissions( - &permissions, + permissions, cwd, env_map, codex_home, diff --git a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs index b012fd8af5..dd343a22ce 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -97,8 +97,7 @@ mod windows_impl { let logs_base_dir: Option<&Path> = Some(sandbox_base.as_path()); log_start(&command, logs_base_dir); let sandbox_creds = require_logon_sandbox_creds( - &policy, - sandbox_policy_cwd, + &permissions, cwd, &env_map, codex_home, diff --git a/codex-rs/windows-sandbox-rs/src/identity.rs b/codex-rs/windows-sandbox-rs/src/identity.rs index 30bf7ef493..37238190d0 100644 --- a/codex-rs/windows-sandbox-rs/src/identity.rs +++ b/codex-rs/windows-sandbox-rs/src/identity.rs @@ -1,6 +1,5 @@ use crate::dpapi; use crate::logging::debug_log; -use crate::policy::SandboxPolicy; use crate::resolved_permissions::ResolvedWindowsSandboxPermissions; use crate::setup::SandboxNetworkIdentity; use crate::setup::SandboxUserRecord; @@ -132,8 +131,7 @@ fn select_identity( #[allow(clippy::too_many_arguments)] pub fn require_logon_sandbox_creds( - policy: &SandboxPolicy, - policy_cwd: &Path, + permissions: &ResolvedWindowsSandboxPermissions, command_cwd: &Path, env_map: &HashMap, codex_home: &Path, @@ -144,16 +142,14 @@ pub fn require_logon_sandbox_creds( deny_write_paths_override: &[PathBuf], proxy_enforced: bool, ) -> Result { - let permissions = - ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd(policy, policy_cwd); let sandbox_dir = crate::setup::sandbox_dir(codex_home); let needed_read = read_roots_override .map(<[PathBuf]>::to_vec) - .unwrap_or_else(|| gather_read_roots(command_cwd, policy, codex_home)); + .unwrap_or_else(|| gather_read_roots(command_cwd, permissions, env_map, codex_home)); let needed_write = write_roots_override .map(<[PathBuf]>::to_vec) - .unwrap_or_else(|| gather_write_roots_for_permissions(&permissions, command_cwd, env_map)); - let network_identity = SandboxNetworkIdentity::from_permissions(&permissions, proxy_enforced); + .unwrap_or_else(|| gather_write_roots_for_permissions(permissions, command_cwd, env_map)); + let network_identity = SandboxNetworkIdentity::from_permissions(permissions, proxy_enforced); let desired_offline_proxy_settings = offline_proxy_settings_from_env(env_map, network_identity); // NOTE: Do not add CODEX_HOME/.sandbox to `needed_write`; it must remain non-writable by the // restricted capability token. The setup helper's `lock_sandbox_dir` is responsible for @@ -194,8 +190,7 @@ pub fn require_logon_sandbox_creds( } run_elevated_setup( crate::setup::SandboxSetupRequest { - policy, - policy_cwd, + permissions, command_cwd, env_map, codex_home, @@ -214,8 +209,7 @@ pub fn require_logon_sandbox_creds( // Always refresh ACLs (non-elevated) for current roots via the setup binary. run_setup_refresh_with_overrides( crate::setup::SandboxSetupRequest { - policy, - policy_cwd, + permissions, command_cwd, env_map, codex_home, diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 0276d61616..e3365a2c8f 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -108,6 +108,8 @@ pub use acl::path_mask_allows; #[cfg(target_os = "windows")] pub use audit::apply_world_writable_scan_and_denies; #[cfg(target_os = "windows")] +pub use audit::apply_world_writable_scan_and_denies_for_permissions; +#[cfg(target_os = "windows")] pub use cap::load_or_create_cap_sids; #[cfg(target_os = "windows")] pub use cap::workspace_cap_sid_for_cwd; @@ -199,6 +201,8 @@ pub use process::read_handle_loop; #[cfg(target_os = "windows")] pub use process::spawn_process_with_pipes; #[cfg(target_os = "windows")] +pub use resolved_permissions::ResolvedWindowsSandboxPermissions; +#[cfg(target_os = "windows")] pub use resolved_permissions::WindowsSandboxTokenMode; #[cfg(target_os = "windows")] pub use resolved_permissions::token_mode_for_permission_profile; @@ -291,6 +295,7 @@ mod windows_impl { use super::logging::log_success; use super::policy::SandboxPolicy; use super::process::create_process_as_user; + use super::resolved_permissions::ResolvedWindowsSandboxPermissions; use super::sandbox_utils::ensure_codex_home_exists; use super::spawn_prep::LegacyAclSids; use super::spawn_prep::SpawnPrepOptions; @@ -412,6 +417,7 @@ mod windows_impl { }, )?; let policy = common.policy; + let permissions = common.permissions; let current_dir = common.current_dir; let logs_base_dir = common.logs_base_dir.as_deref(); let uses_write_capabilities = common.uses_write_capabilities; @@ -435,8 +441,7 @@ mod windows_impl { let security = prepare_legacy_session_security(&policy, codex_home, cwd, capability_roots)?; allow_null_device_for_workspace_write(uses_write_capabilities); apply_legacy_session_acl_rules( - &policy, - sandbox_policy_cwd, + &permissions, codex_home, ¤t_dir, &env_map, @@ -602,8 +607,10 @@ mod windows_impl { ); let write_root_sids = root_capability_sids(codex_home, cwd, capability_roots)?; apply_legacy_session_acl_rules( - sandbox_policy, - sandbox_policy_cwd, + &ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd( + sandbox_policy, + sandbox_policy_cwd, + ), codex_home, ¤t_dir, env_map, diff --git a/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs b/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs index 6a43c99d8f..ba5a4ca615 100644 --- a/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs +++ b/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs @@ -17,7 +17,7 @@ use std::path::PathBuf; /// Windows-specific path conventions, not the user/config-facing /// `PermissionProfile` enum itself. #[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct ResolvedWindowsSandboxPermissions { +pub struct ResolvedWindowsSandboxPermissions { file_system: FileSystemSandboxPolicy, network: NetworkSandboxPolicy, } @@ -56,7 +56,7 @@ pub fn token_mode_for_permission_profile( } impl ResolvedWindowsSandboxPermissions { - pub(crate) fn from_legacy_policy_for_cwd(policy: &SandboxPolicy, cwd: &Path) -> Self { + pub fn from_legacy_policy_for_cwd(policy: &SandboxPolicy, cwd: &Path) -> Self { Self { file_system: FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(policy, cwd) .materialize_project_roots_with_cwd(cwd), @@ -64,9 +64,7 @@ impl ResolvedWindowsSandboxPermissions { } } - pub(crate) fn try_from_permission_profile( - permission_profile: &PermissionProfile, - ) -> Result { + pub fn try_from_permission_profile(permission_profile: &PermissionProfile) -> Result { if !matches!(permission_profile, PermissionProfile::Managed { .. }) { anyhow::bail!( "only managed permission profiles can be enforced by the Windows sandbox" @@ -92,6 +90,26 @@ impl ResolvedWindowsSandboxPermissions { self.network } + pub(crate) fn is_enforceable_by_windows_sandbox(&self) -> bool { + matches!(self.file_system.kind, FileSystemSandboxKind::Restricted) + } + + pub(crate) fn has_full_disk_read_access(&self) -> bool { + self.file_system.has_full_disk_read_access() + } + + pub(crate) fn include_platform_defaults(&self) -> bool { + self.file_system.include_platform_defaults() + } + + pub(crate) fn readable_roots_for_cwd(&self, cwd: &Path) -> Vec { + self.file_system + .get_readable_roots_with_cwd(cwd) + .into_iter() + .map(AbsolutePathBuf::into_path_buf) + .collect() + } + pub(crate) fn uses_write_capabilities_for_cwd( &self, cwd: &Path, diff --git a/codex-rs/windows-sandbox-rs/src/setup.rs b/codex-rs/windows-sandbox-rs/src/setup.rs index d53aacd957..b77a874160 100644 --- a/codex-rs/windows-sandbox-rs/src/setup.rs +++ b/codex-rs/windows-sandbox-rs/src/setup.rs @@ -11,7 +11,7 @@ use std::process::Command; use std::process::Stdio; use crate::allow::AllowDenyPaths; -use crate::allow::compute_allow_paths; +use crate::allow::compute_allow_paths_for_permissions; use crate::helper_materialization::bundled_executable_path_for_exe; use crate::helper_materialization::helper_bin_dir; use crate::logging::log_note; @@ -87,8 +87,7 @@ pub fn sandbox_users_path(codex_home: &Path) -> PathBuf { } pub struct SandboxSetupRequest<'a> { - pub policy: &'a SandboxPolicy, - pub policy_cwd: &'a Path, + pub permissions: &'a ResolvedWindowsSandboxPermissions, pub command_cwd: &'a Path, pub env_map: &'a HashMap, pub codex_home: &'a Path, @@ -112,10 +111,17 @@ pub fn run_setup_refresh( codex_home: &Path, proxy_enforced: bool, ) -> Result<()> { + if matches!( + policy, + SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } + ) { + return Ok(()); + } + let permissions = + ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd(policy, policy_cwd); run_setup_refresh_inner( SandboxSetupRequest { - policy, - policy_cwd, + permissions: &permissions, command_cwd, env_map, codex_home, @@ -141,12 +147,19 @@ pub fn run_setup_refresh_with_extra_read_roots( extra_read_roots: Vec, proxy_enforced: bool, ) -> Result<()> { - let mut read_roots = gather_read_roots(command_cwd, policy, codex_home); + if matches!( + policy, + SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } + ) { + return Ok(()); + } + let permissions = + ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd(policy, policy_cwd); + let mut read_roots = gather_read_roots(command_cwd, &permissions, env_map, codex_home); read_roots.extend(extra_read_roots); run_setup_refresh_inner( SandboxSetupRequest { - policy, - policy_cwd, + permissions: &permissions, command_cwd, env_map, codex_home, @@ -166,22 +179,14 @@ fn run_setup_refresh_inner( request: SandboxSetupRequest<'_>, overrides: SetupRootOverrides, ) -> Result<()> { - // Skip in danger-full-access. - if matches!( - request.policy, - SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } - ) { - return Ok(()); + if !request.permissions.is_enforceable_by_windows_sandbox() { + anyhow::bail!("unsupported filesystem permissions for Windows sandbox setup"); } let (read_roots, write_roots) = build_payload_roots(&request, &overrides); let deny_read_paths = build_payload_deny_read_paths(overrides.deny_read_paths); let deny_write_paths = build_payload_deny_write_paths(&request, overrides.deny_write_paths); - let permissions = ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd( - request.policy, - request.policy_cwd, - ); let network_identity = - SandboxNetworkIdentity::from_permissions(&permissions, request.proxy_enforced); + SandboxNetworkIdentity::from_permissions(request.permissions, request.proxy_enforced); let offline_proxy_settings = offline_proxy_settings_from_env(request.env_map, network_identity); let payload = ElevationPayload { version: SETUP_VERSION, @@ -365,9 +370,10 @@ fn gather_helper_read_roots(codex_home: &Path) -> Vec { vec![helper_dir] } -fn gather_legacy_full_read_roots( +fn gather_full_read_roots_for_permissions( command_cwd: &Path, - policy: &SandboxPolicy, + permissions: &ResolvedWindowsSandboxPermissions, + env_map: &HashMap, codex_home: &Path, ) -> Vec { let mut roots = gather_helper_read_roots(codex_home); @@ -380,20 +386,40 @@ fn gather_legacy_full_read_roots( roots.extend(profile_read_roots(Path::new(&up))); } roots.push(command_cwd.to_path_buf()); - if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = policy { - for root in writable_roots { - roots.push(root.to_path_buf()); - } - } + roots.extend( + permissions + .writable_roots_for_cwd(command_cwd, env_map) + .into_iter() + .map(|root| root.root), + ); canonical_existing(&roots) } pub(crate) fn gather_read_roots( command_cwd: &Path, - policy: &SandboxPolicy, + permissions: &ResolvedWindowsSandboxPermissions, + env_map: &HashMap, codex_home: &Path, ) -> Vec { - gather_legacy_full_read_roots(command_cwd, policy, codex_home) + if permissions.has_full_disk_read_access() { + return gather_full_read_roots_for_permissions( + command_cwd, + permissions, + env_map, + codex_home, + ); + } + + let mut roots = gather_helper_read_roots(codex_home); + if permissions.include_platform_defaults() { + roots.extend( + WINDOWS_PLATFORM_DEFAULT_READ_ROOTS + .iter() + .map(PathBuf::from), + ); + } + roots.extend(permissions.readable_roots_for_cwd(command_cwd)); + canonical_existing(&roots) } pub(crate) fn gather_write_roots_for_permissions( @@ -417,17 +443,14 @@ pub(crate) fn gather_write_roots_for_permissions( } pub(crate) fn effective_write_roots_for_setup( - policy: &SandboxPolicy, - policy_cwd: &Path, + permissions: &ResolvedWindowsSandboxPermissions, command_cwd: &Path, env_map: &HashMap, codex_home: &Path, write_roots_override: Option<&[PathBuf]>, ) -> Vec { - let permissions = - ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd(policy, policy_cwd); effective_write_roots_for_permissions( - &permissions, + permissions, command_cwd, env_map, codex_home, @@ -753,6 +776,9 @@ pub fn run_elevated_setup( request: SandboxSetupRequest<'_>, overrides: SetupRootOverrides, ) -> Result<()> { + if !request.permissions.is_enforceable_by_windows_sandbox() { + anyhow::bail!("unsupported filesystem permissions for Windows sandbox setup"); + } // Ensure the shared sandbox directory exists before we send it to the elevated helper. let sbx_dir = sandbox_dir(request.codex_home); std::fs::create_dir_all(&sbx_dir).map_err(|err| { @@ -764,12 +790,8 @@ pub fn run_elevated_setup( let (read_roots, write_roots) = build_payload_roots(&request, &overrides); let deny_read_paths = build_payload_deny_read_paths(overrides.deny_read_paths); let deny_write_paths = build_payload_deny_write_paths(&request, overrides.deny_write_paths); - let permissions = ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd( - request.policy, - request.policy_cwd, - ); let network_identity = - SandboxNetworkIdentity::from_permissions(&permissions, request.proxy_enforced); + SandboxNetworkIdentity::from_permissions(request.permissions, request.proxy_enforced); let offline_proxy_settings = offline_proxy_settings_from_env(request.env_map, network_identity); let payload = ElevationPayload { version: SETUP_VERSION, @@ -801,8 +823,7 @@ fn build_payload_roots( overrides: &SetupRootOverrides, ) -> (Vec, Vec) { let write_roots = effective_write_roots_for_setup( - request.policy, - request.policy_cwd, + request.permissions, request.command_cwd, request.env_map, request.codex_home, @@ -822,7 +843,12 @@ fn build_payload_roots( read_roots.extend(roots.iter().cloned()); canonical_existing(&read_roots) } else { - gather_read_roots(request.command_cwd, request.policy, request.codex_home) + gather_read_roots( + request.command_cwd, + request.permissions, + request.env_map, + request.codex_home, + ) }; read_roots = expand_user_profile_root(read_roots); read_roots = filter_user_profile_root(read_roots); @@ -837,9 +863,8 @@ fn build_payload_deny_write_paths( request: &SandboxSetupRequest<'_>, explicit_deny_write_paths: Option>, ) -> Vec { - let allow_deny_paths: AllowDenyPaths = compute_allow_paths( - request.policy, - request.policy_cwd, + let allow_deny_paths: AllowDenyPaths = compute_allow_paths_for_permissions( + request.permissions, request.command_cwd, request.env_map, ); @@ -986,7 +1011,7 @@ mod tests { use super::WINDOWS_PLATFORM_DEFAULT_READ_ROOTS; use super::build_payload_roots; use super::find_setup_exe_for_current_exe; - use super::gather_legacy_full_read_roots; + use super::gather_full_read_roots_for_permissions; use super::gather_read_roots; use super::loopback_proxy_port_from_url; use super::offline_proxy_settings_from_env; @@ -996,6 +1021,7 @@ mod tests { use crate::helper_materialization::RESOURCES_DIRNAME; use crate::helper_materialization::helper_bin_dir; use crate::policy::SandboxPolicy; + use crate::resolved_permissions::ResolvedWindowsSandboxPermissions; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::collections::HashMap; @@ -1011,6 +1037,13 @@ mod tests { .collect() } + fn permissions_for( + policy: &SandboxPolicy, + policy_cwd: &std::path::Path, + ) -> ResolvedWindowsSandboxPermissions { + ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd(policy, policy_cwd) + } + #[test] fn loopback_proxy_url_parsing_supports_common_forms() { assert_eq!( @@ -1329,8 +1362,9 @@ mod tests { let command_cwd = tmp.path().join("workspace"); fs::create_dir_all(&command_cwd).expect("create workspace"); let policy = SandboxPolicy::new_read_only_policy(); + let permissions = permissions_for(&policy, &command_cwd); - let roots = gather_read_roots(&command_cwd, &policy, &codex_home); + let roots = gather_read_roots(&command_cwd, &permissions, &HashMap::new(), &codex_home); let expected = dunce::canonicalize(helper_bin_dir(&codex_home)).expect("canonical helper dir"); @@ -1354,8 +1388,9 @@ mod tests { exclude_tmpdir_env_var: true, exclude_slash_tmp: true, }; + let permissions = permissions_for(&policy, &command_cwd); - let roots = gather_read_roots(&command_cwd, &policy, &codex_home); + let roots = gather_read_roots(&command_cwd, &permissions, &HashMap::new(), &codex_home); let expected_writable = dunce::canonicalize(&writable_root).expect("canonical writable root"); @@ -1375,11 +1410,11 @@ mod tests { let policy = SandboxPolicy::ReadOnly { network_access: false, }; + let permissions = permissions_for(&policy, &policy_cwd); let (read_roots, write_roots) = build_payload_roots( &super::SandboxSetupRequest { - policy: &policy, - policy_cwd: &policy_cwd, + permissions: &permissions, command_cwd: &command_cwd, env_map: &HashMap::new(), codex_home: &codex_home, @@ -1423,11 +1458,11 @@ mod tests { let policy = SandboxPolicy::ReadOnly { network_access: false, }; + let permissions = permissions_for(&policy, &policy_cwd); let (read_roots, write_roots) = build_payload_roots( &super::SandboxSetupRequest { - policy: &policy, - policy_cwd: &policy_cwd, + permissions: &permissions, command_cwd: &command_cwd, env_map: &HashMap::new(), codex_home: &codex_home, @@ -1475,6 +1510,7 @@ mod tests { exclude_tmpdir_env_var: true, exclude_slash_tmp: true, }; + let permissions = permissions_for(&policy, &command_cwd); let override_roots = vec![ command_cwd.clone(), extra_root.clone(), @@ -1482,8 +1518,7 @@ mod tests { sandbox_root.clone(), ]; let request = super::SandboxSetupRequest { - policy: &policy, - policy_cwd: &command_cwd, + permissions: &permissions, command_cwd: &command_cwd, env_map: &HashMap::new(), codex_home: &codex_home, @@ -1498,8 +1533,7 @@ mod tests { }; let effective_write_roots = super::effective_write_roots_for_setup( - &policy, - &command_cwd, + &permissions, &command_cwd, &HashMap::new(), &codex_home, @@ -1533,10 +1567,10 @@ mod tests { exclude_tmpdir_env_var: true, exclude_slash_tmp: true, }; + let permissions = permissions_for(&policy, &policy_cwd); let effective_write_roots = super::effective_write_roots_for_setup( - &policy, - &policy_cwd, + &permissions, &command_cwd, &HashMap::new(), &codex_home, @@ -1569,9 +1603,9 @@ mod tests { exclude_tmpdir_env_var: true, exclude_slash_tmp: true, }; + let permissions = permissions_for(&policy, &command_cwd); let request = super::SandboxSetupRequest { - policy: &policy, - policy_cwd: &command_cwd, + permissions: &permissions, command_cwd: &command_cwd, env_map: &HashMap::new(), codex_home: &codex_home, @@ -1600,8 +1634,14 @@ mod tests { let command_cwd = tmp.path().join("workspace"); fs::create_dir_all(&command_cwd).expect("create workspace"); let policy = SandboxPolicy::new_read_only_policy(); + let permissions = permissions_for(&policy, &command_cwd); - let roots = gather_legacy_full_read_roots(&command_cwd, &policy, &codex_home); + let roots = gather_full_read_roots_for_permissions( + &command_cwd, + &permissions, + &HashMap::new(), + &codex_home, + ); assert!( canonical_windows_platform_default_roots() diff --git a/codex-rs/windows-sandbox-rs/src/spawn_prep.rs b/codex-rs/windows-sandbox-rs/src/spawn_prep.rs index efe7a7c7cc..f4f0779742 100644 --- a/codex-rs/windows-sandbox-rs/src/spawn_prep.rs +++ b/codex-rs/windows-sandbox-rs/src/spawn_prep.rs @@ -2,7 +2,6 @@ use crate::acl::add_allow_ace; use crate::acl::add_deny_write_ace; use crate::acl::allow_null_device; use crate::allow::AllowDenyPaths; -use crate::allow::compute_allow_paths; use crate::allow::compute_allow_paths_for_permissions; use crate::cap::load_or_create_cap_sids; use crate::cap::workspace_write_cap_sid_for_root; @@ -283,8 +282,7 @@ pub(crate) fn allow_null_device_for_workspace_write(is_workspace_write: bool) { #[allow(clippy::too_many_arguments)] pub(crate) fn apply_legacy_session_acl_rules( - policy: &SandboxPolicy, - sandbox_policy_cwd: &Path, + permissions: &ResolvedWindowsSandboxPermissions, codex_home: &Path, current_dir: &Path, env_map: &HashMap, @@ -293,7 +291,7 @@ pub(crate) fn apply_legacy_session_acl_rules( acl_sids: LegacyAclSids<'_>, ) -> Result<()> { let AllowDenyPaths { allow, mut deny } = - compute_allow_paths(policy, sandbox_policy_cwd, current_dir, env_map); + compute_allow_paths_for_permissions(permissions, current_dir, env_map); unsafe { for path in additional_deny_write_paths { // Explicit carveouts must exist before the command starts so the @@ -417,8 +415,7 @@ pub(crate) fn prepare_elevated_spawn_context( write_roots_override }; let sandbox_creds = require_logon_sandbox_creds( - &common.policy, - sandbox_policy_cwd, + &common.permissions, cwd, env_map, codex_home, diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs index 7d4ccc0404..4db782a601 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs @@ -318,8 +318,7 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy( allow_null_device_for_workspace_write(common.uses_write_capabilities); apply_legacy_session_acl_rules( - &common.policy, - sandbox_policy_cwd, + &common.permissions, codex_home, &common.current_dir, &env_map,