Ignore inherited ACEs when refreshing Windows write roots (#34392)

## Why

An inherited `FILE_DELETE_CHILD` grant can make a write root look stale, but
`SET_ACCESS` cannot replace an ACE inherited from an ancestor. Treating that
grant as explicit causes unnecessary ACL refresh attempts that cannot converge.

## What changed

- Add explicit-ACE filtering to the Windows DACL permission checks.
- Refresh a write root only when `FILE_DELETE_CHILD` is present in an explicit
  allow ACE, while retaining effective-permission checks for required rights.
- Ignore inherited stale rights when deciding whether `SET_ACCESS` must repair
  an allow ACE.

## Testing

Add a Windows regression test covering a write root that inherits
`FILE_DELETE_CHILD`, verifying that repeated refresh checks leave its explicit
write ACE unchanged.

GitOrigin-RevId: d0df9429efcf299da3ff3c1bce92942684803293

codex-rs/windows-sandbox-rs/src/acl.rs

@@ -49,6 +49,7 @@ use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING;
 use windows_sys::Win32::Storage::FileSystem::READ_CONTROL;
 const SE_KERNEL_OBJECT: u32 = 6;
 const INHERIT_ONLY_ACE: u8 = 0x08;
+const INHERITED_ACE: u8 = 0x10;
 const ACCESS_ALLOWED_ACE_TYPE: u8 = 0;
 const ACCESS_DENIED_ACE_TYPE: u8 = 1;
 const GENERIC_READ_MASK: u32 = 0x8000_0000;
@@ -103,6 +104,28 @@ pub unsafe fn dacl_mask_allows(
     psids: &[*mut c_void],
     desired_mask: u32,
     require_all_bits: bool,
+) -> bool {
+    dacl_mask_allows_with_scope(
+        p_dacl,
+        psids,
+        desired_mask,
+        require_all_bits,
+        AceScope::Effective,
+    )
+}
+
+#[derive(Clone, Copy)]
+enum AceScope {
+    Effective,
+    Explicit,
+}
+
+unsafe fn dacl_mask_allows_with_scope(
+    p_dacl: *mut ACL,
+    psids: &[*mut c_void],
+    desired_mask: u32,
+    require_all_bits: bool,
+    scope: AceScope,
 ) -> bool {
     if p_dacl.is_null() {
         return false;
@@ -135,6 +158,11 @@ pub unsafe fn dacl_mask_allows(
         if (hdr.AceFlags & INHERIT_ONLY_ACE) != 0 {
             continue;
         }
+        // SET_ACCESS cannot replace an ACE inherited from an ancestor, so it cannot make
+        // an explicit-only repair converge when that inherited ACE contains stale rights.
+        if matches!(scope, AceScope::Explicit) && (hdr.AceFlags & INHERITED_ACE) != 0 {
+            continue;
+        }
         let base = p_ace as usize;
         let sid_ptr =
             (base + std::mem::size_of::<ACE_HEADER>() + std::mem::size_of::<u32>()) as *mut c_void;
@@ -166,10 +194,41 @@ pub fn path_mask_allows(
     psids: &[*mut c_void],
     desired_mask: u32,
     require_all_bits: bool,
+) -> Result<bool> {
+    path_mask_allows_with_scope(
+        path,
+        psids,
+        desired_mask,
+        require_all_bits,
+        AceScope::Effective,
+    )
+}
+
+/// Returns whether an explicit allow ACE for one of the provided SIDs grants any bit in `desired_mask`.
+pub fn path_mask_has_explicit_allow_ace(
+    path: &Path,
+    psids: &[*mut c_void],
+    desired_mask: u32,
+) -> Result<bool> {
+    path_mask_allows_with_scope(
+        path,
+        psids,
+        desired_mask,
+        /*require_all_bits*/ false,
+        AceScope::Explicit,
+    )
+}
+
+fn path_mask_allows_with_scope(
+    path: &Path,
+    psids: &[*mut c_void],
+    desired_mask: u32,
+    require_all_bits: bool,
+    scope: AceScope,
 ) -> Result<bool> {
     unsafe {
         let (p_dacl, sd) = fetch_dacl_handle(path)?;
-        let has = dacl_mask_allows(p_dacl, psids, desired_mask, require_all_bits);
+        let has = dacl_mask_allows_with_scope(p_dacl, psids, desired_mask, require_all_bits, scope);
         if !sd.is_null() {
             LocalFree(sd as HLOCAL);
         }
@@ -318,11 +377,12 @@ unsafe fn ensure_allow_mask_aces_with_inheritance_impl(
     let mut entries: Vec<EXPLICIT_ACCESS_W> = Vec::new();
     for sid in sids {
         if dacl_mask_allows(p_dacl, &[*sid], allow_mask, /*require_all_bits*/ true)
-            && !dacl_mask_allows(
+            && !dacl_mask_allows_with_scope(
                 p_dacl,
                 &[*sid],
                 disallow_mask,
                 /*require_all_bits*/ false,
+                AceScope::Explicit,
             )
         {
             continue;

codex-rs/windows-sandbox-rs/src/bin/setup_main/win.rs

@@ -22,6 +22,7 @@ use codex_windows_sandbox::is_command_cwd_root;
 use codex_windows_sandbox::log_note;
 use codex_windows_sandbox::log_writer;
 use codex_windows_sandbox::path_mask_allows;
+use codex_windows_sandbox::path_mask_has_explicit_allow_ace;
 use codex_windows_sandbox::sandbox_bin_dir;
 use codex_windows_sandbox::sandbox_dir;
 use codex_windows_sandbox::sandbox_secrets_dir;
@@ -169,12 +170,7 @@ fn write_root_needs_refresh(root: &Path, psid: *mut c_void) -> Result<bool> {
     )? {
         return Ok(true);
     }
-    path_mask_allows(
+    path_mask_has_explicit_allow_ace(root, &[psid], FILE_DELETE_CHILD)
-        root,
-        &[psid],
-        FILE_DELETE_CHILD,
-        /*require_all_bits*/ false,
-    )
 }
 
 fn spawn_read_acl_helper(payload: &Payload, _log: &mut dyn Write) -> Result<()> {
@@ -1060,6 +1056,7 @@ mod tests {
     use codex_windows_sandbox::ensure_allow_mask_aces;
     use codex_windows_sandbox::ensure_allow_write_aces;
     use codex_windows_sandbox::load_or_create_cap_sids;
+    use codex_windows_sandbox::path_mask_allows;
     use codex_windows_sandbox::workspace_write_cap_sid_for_root;
     use pretty_assertions::assert_eq;
     use serde_json::json;
@@ -1144,6 +1141,55 @@ mod tests {
         );
     }
 
+    #[test]
+    fn write_root_refresh_ignores_inherited_delete_child_grant() {
+        let temp = tempfile::tempdir().expect("tempdir");
+        let codex_home = temp.path().join("codex-home");
+        let parent = temp.path().join("parent");
+        let workspace = parent.join("workspace");
+        fs::create_dir_all(&codex_home).expect("create codex home");
+        fs::create_dir_all(&workspace).expect("create workspace");
+
+        let sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &workspace)
+            .expect("workspace sid");
+        let psid = unsafe { convert_string_sid_to_sid(&sid).expect("convert workspace sid") };
+        let seeded_explicit =
+            unsafe { ensure_allow_mask_aces(&workspace, &[psid], WRITE_ROOT_ALLOW_MASK) }
+                .expect("seed explicit write ACE");
+        let seeded_parent = unsafe {
+            ensure_allow_mask_aces(&parent, &[psid], WRITE_ROOT_ALLOW_MASK | FILE_DELETE_CHILD)
+        }
+        .expect("seed inherited stale write ACE");
+        let has_inherited_delete_child = path_mask_allows(
+            &workspace,
+            &[psid],
+            FILE_DELETE_CHILD,
+            /*require_all_bits*/ false,
+        )
+        .expect("check inherited stale write ACE");
+        let needs_refresh =
+            write_root_needs_refresh(&workspace, psid).expect("check inherited stale write ACE");
+        let first_refresh = unsafe { ensure_allow_write_aces(&workspace, &[psid]) }
+            .expect("first inherited write ACE refresh");
+        let second_refresh = unsafe { ensure_allow_write_aces(&workspace, &[psid]) }
+            .expect("second inherited write ACE refresh");
+        unsafe {
+            LocalFree(psid as HLOCAL);
+        }
+
+        assert_eq!(
+            (
+                seeded_explicit,
+                seeded_parent,
+                has_inherited_delete_child,
+                needs_refresh,
+                first_refresh,
+                second_refresh,
+            ),
+            (true, true, true, false, false, false)
+        );
+    }
+
     #[test]
     fn deny_path_under_active_root_uses_only_matching_root_sid() {
         let temp = tempfile::tempdir().expect("tempdir");

codex-rs/windows-sandbox-rs/src/lib.rs

@@ -148,6 +148,8 @@ pub use acl::fetch_dacl_handle;
 #[cfg(target_os = "windows")]
 pub use acl::path_mask_allows;
 #[cfg(target_os = "windows")]
+pub use acl::path_mask_has_explicit_allow_ace;
+#[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;