From 34d7de873f70bc05dc2052701e4beab0be394da7 Mon Sep 17 00:00:00 2001 From: Eva Wong Date: Mon, 4 May 2026 10:10:38 -0700 Subject: [PATCH] Grant Windows legacy Git read roots --- codex-rs/windows-sandbox-rs/src/lib.rs | 33 +++++++++++++++ codex-rs/windows-sandbox-rs/src/spawn_prep.rs | 40 +++++++++++++++---- .../src/unified_exec/backends/legacy.rs | 1 + 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 744aeac643..33c154fa88 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -267,6 +267,8 @@ mod windows_impl { use super::acl::add_deny_write_ace; use super::acl::allow_named_pipe_device; use super::acl::allow_null_device; + use super::acl::ensure_allow_mask_aces; + use super::acl::ensure_allow_mask_aces_with_inheritance; use super::acl::revoke_ace; use super::allow::AllowDenyPaths; use super::allow::compute_allow_paths; @@ -279,6 +281,8 @@ mod windows_impl { use super::process::create_process_as_user; use super::protected_metadata::prepare_protected_metadata_targets; use super::sandbox_utils::ensure_codex_home_exists; + use super::spawn_prep::legacy_session_direct_read_paths; + use super::spawn_prep::legacy_session_executable_read_roots; use super::spawn_prep::prepare_legacy_spawn_context; use super::token::convert_string_sid_to_sid; use super::token::create_workspace_write_token_with_caps_from; @@ -295,6 +299,8 @@ mod windows_impl { use windows_sys::Win32::Foundation::HANDLE; use windows_sys::Win32::Foundation::HANDLE_FLAG_INHERIT; use windows_sys::Win32::Foundation::SetHandleInformation; + use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_EXECUTE; + use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_READ; use windows_sys::Win32::System::Pipes::CreatePipe; use windows_sys::Win32::System::Threading::GetExitCodeProcess; use windows_sys::Win32::System::Threading::INFINITE; @@ -443,6 +449,8 @@ mod windows_impl { let persist_aces = is_workspace_write; let AllowDenyPaths { allow, mut deny } = compute_allow_paths(&policy, sandbox_policy_cwd, ¤t_dir, &env_map); + let read_roots = legacy_session_executable_read_roots(&env_map, &command); + let direct_read_paths = legacy_session_direct_read_paths(&env_map); let protected_metadata_guard = prepare_protected_metadata_targets(protected_metadata_targets); for path in protected_metadata_guard.deny_paths() { @@ -455,7 +463,32 @@ mod windows_impl { } let canonical_cwd = canonicalize_path(¤t_dir); let mut guards: Vec<(PathBuf, *mut c_void)> = Vec::new(); + let read_execute_mask = FILE_GENERIC_READ | FILE_GENERIC_EXECUTE; unsafe { + let read_execute_sids: Vec<*mut c_void> = match psid_workspace { + Some(psid) => vec![psid_generic, psid], + None => vec![psid_generic], + }; + for p in &read_roots { + if let Ok(added) = ensure_allow_mask_aces(p, &read_execute_sids, read_execute_mask) + && added + && !persist_aces + { + guards.push((p.clone(), psid_generic)); + } + } + for p in &direct_read_paths { + if let Ok(added) = ensure_allow_mask_aces_with_inheritance( + p, + &read_execute_sids, + read_execute_mask, + /*inheritance*/ 0, + ) && added + && !persist_aces + { + guards.push((p.clone(), psid_generic)); + } + } for p in &allow { let psid = if is_workspace_write && is_command_cwd_root(p, &canonical_cwd) { psid_workspace.unwrap_or(psid_generic) diff --git a/codex-rs/windows-sandbox-rs/src/spawn_prep.rs b/codex-rs/windows-sandbox-rs/src/spawn_prep.rs index 49cc19c825..1b8d7b5bc6 100644 --- a/codex-rs/windows-sandbox-rs/src/spawn_prep.rs +++ b/codex-rs/windows-sandbox-rs/src/spawn_prep.rs @@ -2,6 +2,8 @@ use crate::acl::add_allow_ace; use crate::acl::add_deny_write_ace; use crate::acl::allow_named_pipe_device; use crate::acl::allow_null_device; +use crate::acl::ensure_allow_mask_aces; +use crate::acl::ensure_allow_mask_aces_with_inheritance; use crate::allow::AllowDenyPaths; use crate::allow::compute_allow_paths; use crate::cap::load_or_create_cap_sids; @@ -36,6 +38,8 @@ use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::Foundation::HANDLE; use windows_sys::Win32::Foundation::HLOCAL; use windows_sys::Win32::Foundation::LocalFree; +use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_EXECUTE; +use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_READ; pub(crate) struct SpawnContext { pub(crate) policy: SandboxPolicy, @@ -220,6 +224,7 @@ pub(crate) fn apply_legacy_session_acl_rules( sandbox_policy_cwd: &Path, current_dir: &Path, env_map: &HashMap, + command: &[String], psid_generic: &LocalSid, psid_workspace: Option<&LocalSid>, persist_aces: bool, @@ -229,8 +234,35 @@ pub(crate) fn apply_legacy_session_acl_rules( compute_allow_paths(policy, sandbox_policy_cwd, current_dir, env_map); deny.extend(additional_deny_paths.iter().cloned()); let mut guards: Vec = Vec::new(); + let read_roots = legacy_session_executable_read_roots(env_map, command); + let direct_read_paths = legacy_session_direct_read_paths(env_map); + let read_execute_mask = FILE_GENERIC_READ | FILE_GENERIC_EXECUTE; let canonical_cwd = canonicalize_path(current_dir); unsafe { + let read_execute_sids: Vec<*mut std::ffi::c_void> = match psid_workspace { + Some(psid_workspace) => vec![psid_generic.as_ptr(), psid_workspace.as_ptr()], + None => vec![psid_generic.as_ptr()], + }; + for p in &read_roots { + if let Ok(added) = ensure_allow_mask_aces(p, &read_execute_sids, read_execute_mask) + && added + && !persist_aces + { + guards.push(p.clone()); + } + } + for p in &direct_read_paths { + if let Ok(added) = ensure_allow_mask_aces_with_inheritance( + p, + &read_execute_sids, + read_execute_mask, + /*inheritance*/ 0, + ) && added + && !persist_aces + { + guards.push(p.clone()); + } + } for p in &allow { let psid = if matches!(policy, SandboxPolicy::WorkspaceWrite { .. }) && is_command_cwd_root(p, &canonical_cwd) @@ -265,7 +297,6 @@ pub(crate) fn apply_legacy_session_acl_rules( guards } -#[allow(dead_code)] pub(crate) fn legacy_session_executable_read_roots( env_map: &HashMap, command: &[String], @@ -296,7 +327,6 @@ pub(crate) fn legacy_session_executable_read_roots( canonical_existing_deduped(roots) } -#[allow(dead_code)] pub(crate) fn legacy_session_direct_read_paths(env_map: &HashMap) -> Vec { let mut paths = Vec::new(); @@ -308,7 +338,6 @@ pub(crate) fn legacy_session_direct_read_paths(env_map: &HashMap canonical_existing_deduped(paths) } -#[allow(dead_code)] fn add_git_for_windows_support_roots( env_map: &HashMap, tool_root: &Path, @@ -326,7 +355,6 @@ fn add_git_for_windows_support_roots( } } -#[allow(dead_code)] fn legacy_session_home_dirs(env_map: &HashMap) -> Vec { let mut homes = Vec::new(); @@ -346,12 +374,10 @@ fn legacy_session_home_dirs(env_map: &HashMap) -> Vec { canonical_existing_deduped(homes) } -#[allow(dead_code)] fn env_path(env_map: &HashMap, name: &str) -> Option { env_value(env_map, name).map(PathBuf::from) } -#[allow(dead_code)] fn env_value(env_map: &HashMap, name: &str) -> Option { env_map .iter() @@ -359,7 +385,6 @@ fn env_value(env_map: &HashMap, name: &str) -> Option { .map(|(_, value)| value.clone()) } -#[allow(dead_code)] fn canonical_existing_deduped(paths: Vec) -> Vec { let mut deduped = Vec::new(); for path in paths { @@ -374,7 +399,6 @@ fn canonical_existing_deduped(paths: Vec) -> Vec { deduped } -#[allow(dead_code)] fn windows_tool_root_for_path_dir(path: &Path) -> Option { let name = path.file_name()?.to_string_lossy(); if !name.eq_ignore_ascii_case("cmd") && !name.eq_ignore_ascii_case("bin") { 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 f9e2ba4e68..c9a982b695 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 @@ -337,6 +337,7 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy( sandbox_policy_cwd, &common.current_dir, &env_map, + &command, &security.psid_generic, security.psid_workspace.as_ref(), persist_aces,