From 82b22ee434bd5a12f99d165a6c6cd3dabc2ccc77 Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Thu, 9 Apr 2026 13:07:04 -0700 Subject: [PATCH] windows sandbox: keep real user able to refresh write-root ACLs (cherry picked from commit 0207e7d5a444c70cd6ec439406ef36835c87c618) --- .../windows-sandbox-rs/src/setup_main_win.rs | 42 +++++++++++++++++-- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs index ca3fc1e444..7e28a47b76 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs @@ -15,6 +15,7 @@ use codex_windows_sandbox::SetupFailure; use codex_windows_sandbox::add_deny_write_ace; use codex_windows_sandbox::canonicalize_path; use codex_windows_sandbox::convert_string_sid_to_sid; +use codex_windows_sandbox::ensure_allow_mask_aces; use codex_windows_sandbox::ensure_allow_mask_aces_with_inheritance; use codex_windows_sandbox::ensure_allow_write_aces; use codex_windows_sandbox::extract_setup_failure; @@ -66,6 +67,8 @@ use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_READ; use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_WRITE; const DENY_ACCESS: i32 = 3; +const WRITE_DAC_ACCESS: u32 = 0x0004_0000; +const WRITE_OWNER_ACCESS: u32 = 0x0008_0000; mod read_acl_mutex; mod sandbox_users; @@ -558,6 +561,13 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( format!("convert sandbox users group SID to PSID failed: {err}"), )) })?; + let real_user_sid = resolve_sid(&payload.real_user)?; + let real_user_psid = sid_bytes_to_psid(&real_user_sid).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperSidResolveFailed, + format!("convert real user SID to PSID failed: {err}"), + )) + })?; let caps = load_or_create_cap_sids(&payload.codex_home).map_err(|err| { anyhow::Error::new(SetupFailure::new( @@ -650,8 +660,10 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( let cap_sid_str = caps.workspace; let sandbox_group_sid_str = string_from_sid_bytes(&sandbox_group_sid).map_err(anyhow::Error::msg)?; + let real_user_sid_str = string_from_sid_bytes(&real_user_sid).map_err(anyhow::Error::msg)?; let write_mask = FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE | FILE_DELETE_CHILD; + let real_user_maintenance_mask = write_mask | WRITE_DAC_ACCESS | WRITE_OWNER_ACCESS; let mut grant_tasks: Vec = Vec::new(); let mut seen_deny_paths: HashSet = HashSet::new(); @@ -684,9 +696,15 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( for (label, psid) in [ ("sandbox_group", sandbox_group_psid), (cap_label, cap_psid_for_root), + ("real_user", real_user_psid), ] { + let desired_mask = if label == "real_user" { + real_user_maintenance_mask + } else { + write_mask + }; let has = - match path_mask_allows(root, &[psid], write_mask, /*require_all_bits*/ true) { + match path_mask_allows(root, &[psid], desired_mask, /*require_all_bits*/ true) { Ok(h) => h, Err(e) => { refresh_errors.push(format!( @@ -713,7 +731,7 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( log_line( log, &format!( - "granting write ACE to {} for sandbox group and capability SID", + "granting write ACE to {} for sandbox group, capability SID, and real user", root.display() ), )?; @@ -730,6 +748,7 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( } else { vec![sandbox_group_sid_str.clone(), cap_sid_str.clone()] }; + let real_user_sid_str = real_user_sid_str.clone(); let tx = tx.clone(); scope.spawn(move || { // Convert SID strings to psids locally in this thread. @@ -743,7 +762,21 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( } } - let res = unsafe { ensure_allow_write_aces(&root, &psids) }; + let res = (|| { + unsafe { + ensure_allow_write_aces(&root, &psids)?; + } + let real_user_psid = + unsafe { convert_string_sid_to_sid(&real_user_sid_str) } + .ok_or_else(|| anyhow::anyhow!("convert real user SID failed"))?; + let real_user_res = unsafe { + ensure_allow_mask_aces(&root, &[real_user_psid], real_user_maintenance_mask) + }; + unsafe { + LocalFree(real_user_psid as HLOCAL); + } + real_user_res + })(); for psid in psids { unsafe { @@ -892,6 +925,9 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( if !sandbox_group_psid.is_null() { LocalFree(sandbox_group_psid as HLOCAL); } + if !real_user_psid.is_null() { + LocalFree(real_user_psid as HLOCAL); + } if !cap_psid.is_null() { LocalFree(cap_psid as HLOCAL); }