From 899374d06394778f0880ac4a66ca11dbb262c0ee Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Fri, 17 Apr 2026 08:04:44 -0700 Subject: [PATCH] fix(windows-sandbox): repair existing descendant ACLs --- .../windows-sandbox-rs/src/setup_main_win.rs | 129 +++++++++++++++++- 1 file changed, 126 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 6619f60c42..312fa8a1da 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs @@ -15,7 +15,6 @@ 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_with_inheritance; -use codex_windows_sandbox::ensure_allow_write_aces; use codex_windows_sandbox::extract_setup_failure; use codex_windows_sandbox::hide_newly_created_users; use codex_windows_sandbox::is_command_cwd_root; @@ -66,6 +65,7 @@ use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_READ; use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_WRITE; const DENY_ACCESS: i32 = 3; +const WORKSPACE_PROTECTED_DIRS: [&str; 3] = [".git", ".codex", ".agents"]; mod read_acl_mutex; mod sandbox_users; @@ -247,6 +247,104 @@ fn read_mask_allows_or_log( } } +fn should_skip_workspace_descendant(path: &Path, command_cwd: &Path) -> bool { + let Ok(relative) = path.strip_prefix(command_cwd) else { + return false; + }; + let Some(first) = relative.iter().next() else { + return false; + }; + WORKSPACE_PROTECTED_DIRS + .iter() + .any(|protected| first.to_string_lossy().eq_ignore_ascii_case(protected)) +} + +fn collect_existing_write_descendants( + root: &Path, + command_cwd: &Path, + log: &mut File, + refresh_errors: &mut Vec, +) -> Result> { + let mut descendants = Vec::new(); + let mut dirs = vec![root.to_path_buf()]; + + while let Some(dir) = dirs.pop() { + let entries = match std::fs::read_dir(&dir) { + Ok(entries) => entries, + Err(err) => { + refresh_errors.push(format!( + "enumerate write descendants failed on {}: {}", + dir.display(), + err + )); + log_line( + log, + &format!( + "enumerate write descendants failed on {}: {}; continuing", + dir.display(), + err + ), + )?; + continue; + } + }; + + for entry in entries { + let entry = match entry { + Ok(entry) => entry, + Err(err) => { + refresh_errors.push(format!( + "enumerate write descendants entry failed under {}: {}", + dir.display(), + err + )); + log_line( + log, + &format!( + "enumerate write descendants entry failed under {}: {}; continuing", + dir.display(), + err + ), + )?; + continue; + } + }; + + let path = entry.path(); + if should_skip_workspace_descendant(&path, command_cwd) { + continue; + } + + let file_type = match entry.file_type() { + Ok(file_type) => file_type, + Err(err) => { + refresh_errors.push(format!( + "inspect write descendant type failed on {}: {}", + path.display(), + err + )); + log_line( + log, + &format!( + "inspect write descendant type failed on {}: {}; continuing", + path.display(), + err + ), + )?; + continue; + } + }; + + descendants.push(path.clone()); + if file_type.is_dir() && !file_type.is_symlink() { + dirs.push(path); + } + } + } + + Ok(descendants) +} + fn lock_sandbox_dir( dir: &Path, real_user: &str, @@ -643,6 +741,7 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( let write_mask = FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE | FILE_DELETE_CHILD; let mut grant_tasks: Vec = Vec::new(); + let mut seen_grant_paths: HashSet = HashSet::new(); let mut seen_deny_paths: HashSet = HashSet::new(); let mut seen_write_roots: HashSet = HashSet::new(); @@ -707,7 +806,17 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( root.display() ), )?; - grant_tasks.push(root.clone()); + if seen_grant_paths.insert(root.clone()) { + grant_tasks.push(root.clone()); + } + } + + for descendant in + collect_existing_write_descendants(root, &canonical_command_cwd, log, refresh_errors)? + { + if seen_grant_paths.insert(descendant.clone()) { + grant_tasks.push(descendant); + } } } @@ -733,7 +842,21 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( } } - let res = unsafe { ensure_allow_write_aces(&root, &psids) }; + let inheritance = match std::fs::metadata(&root) { + Ok(metadata) if metadata.is_dir() => CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE, + Ok(_) => 0, + Err(err) => { + let _ = tx.send(( + root.clone(), + Err(anyhow::anyhow!("metadata lookup failed: {err}")), + )); + return; + } + }; + + let res = unsafe { + ensure_allow_mask_aces_with_inheritance(&root, &psids, write_mask, inheritance) + }; for psid in psids { unsafe {