Diagnose sandbox-owned paths during Windows ACL refresh

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

@@ -14,6 +14,7 @@ use windows_sys::Win32::Security::Authorization::GetSecurityInfo;
 use windows_sys::Win32::Security::Authorization::SetEntriesInAclW;
 use windows_sys::Win32::Security::Authorization::SetNamedSecurityInfoW;
 use windows_sys::Win32::Security::Authorization::SetSecurityInfo;
+use windows_sys::Win32::Security::Authorization::ConvertSidToStringSidW;
 use windows_sys::Win32::Security::Authorization::EXPLICIT_ACCESS_W;
 use windows_sys::Win32::Security::Authorization::TRUSTEE_IS_SID;
 use windows_sys::Win32::Security::Authorization::TRUSTEE_IS_UNKNOWN;
@@ -28,6 +29,7 @@ use windows_sys::Win32::Security::ACL;
 use windows_sys::Win32::Security::ACL_SIZE_INFORMATION;
 use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION;
 use windows_sys::Win32::Security::GENERIC_MAPPING;
+use windows_sys::Win32::Security::OWNER_SECURITY_INFORMATION;
 use windows_sys::Win32::Storage::FileSystem::CreateFileW;
 use windows_sys::Win32::Storage::FileSystem::FILE_ALL_ACCESS;
 use windows_sys::Win32::Storage::FileSystem::FILE_APPEND_DATA;
@@ -92,6 +94,52 @@ pub unsafe fn fetch_dacl_handle(path: &Path) -> Result<(*mut ACL, *mut c_void)>
     Ok((p_dacl, p_sd))
 }
 
+pub fn path_owner_sid_string(path: &Path) -> Result<Option<String>> {
+    unsafe {
+        let mut owner: *mut c_void = std::ptr::null_mut();
+        let mut p_sd: *mut c_void = std::ptr::null_mut();
+        let code = GetNamedSecurityInfoW(
+            to_wide(path).as_ptr() as *mut u16,
+            1, // SE_FILE_OBJECT
+            OWNER_SECURITY_INFORMATION,
+            &mut owner,
+            std::ptr::null_mut(),
+            std::ptr::null_mut(),
+            std::ptr::null_mut(),
+            &mut p_sd,
+        );
+        if code != ERROR_SUCCESS {
+            return Err(anyhow!("GetNamedSecurityInfoW owner failed: {code}"));
+        }
+        if owner.is_null() {
+            if !p_sd.is_null() {
+                LocalFree(p_sd as HLOCAL);
+            }
+            return Ok(None);
+        }
+
+        let mut sid_string_ptr: *mut u16 = std::ptr::null_mut();
+        let ok = ConvertSidToStringSidW(owner, &mut sid_string_ptr);
+        if ok == 0 || sid_string_ptr.is_null() {
+            if !p_sd.is_null() {
+                LocalFree(p_sd as HLOCAL);
+            }
+            return Err(anyhow!("ConvertSidToStringSidW owner failed"));
+        }
+        let mut len = 0;
+        while *sid_string_ptr.add(len) != 0 {
+            len += 1;
+        }
+        let sid =
+            String::from_utf16_lossy(std::slice::from_raw_parts(sid_string_ptr, len as usize));
+        LocalFree(sid_string_ptr as HLOCAL);
+        if !p_sd.is_null() {
+            LocalFree(p_sd as HLOCAL);
+        }
+        Ok(Some(sid))
+    }
+}
+
 /// Fast mask-based check: does an ACE for provided SIDs grant the desired mask? Skips inherit-only.
 /// When `require_all_bits` is true, all bits in `desired_mask` must be present; otherwise any bit suffices.
 pub unsafe fn dacl_mask_allows(

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

@@ -62,6 +62,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_owner_sid_string;
+#[cfg(target_os = "windows")]
 pub use audit::apply_world_writable_scan_and_denies;
 #[cfg(target_os = "windows")]
 pub use cap::load_or_create_cap_sids;

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

@@ -21,6 +21,7 @@ use codex_windows_sandbox::is_command_cwd_root;
 use codex_windows_sandbox::load_or_create_cap_sids;
 use codex_windows_sandbox::log_note;
 use codex_windows_sandbox::path_mask_allows;
+use codex_windows_sandbox::path_owner_sid_string;
 use codex_windows_sandbox::protect_workspace_agents_dir;
 use codex_windows_sandbox::protect_workspace_codex_dir;
 use codex_windows_sandbox::sandbox_bin_dir;
@@ -114,6 +115,27 @@ fn log_line(log: &mut File, msg: &str) -> Result<()> {
     Ok(())
 }
 
+fn add_write_ace_owner_context(
+    path: &Path,
+    err: anyhow::Error,
+    offline_sandbox_sid: &str,
+    offline_sandbox_username: &str,
+) -> anyhow::Error {
+    let owner = match path_owner_sid_string(path) {
+        Ok(Some(owner)) => owner,
+        Ok(None) => return err.context("path has no owner SID"),
+        Err(owner_err) => return err.context(format!("owner lookup failed: {owner_err}")),
+    };
+
+    if owner.eq_ignore_ascii_case(offline_sandbox_sid) {
+        return err.context(format!(
+            "path owner is {offline_sandbox_username}; repair ownership of this workspace path as the normal Windows user"
+        ));
+    }
+
+    err.context(format!("path owner SID is {owner}"))
+}
+
 fn spawn_read_acl_helper(payload: &Payload, _log: &mut File) -> Result<()> {
     let mut read_payload = payload.clone();
     read_payload.mode = SetupMode::ReadAclsOnly;
@@ -718,6 +740,8 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
                 vec![sandbox_group_sid_str.clone(), cap_sid_str.clone()]
             };
             let tx = tx.clone();
+            let offline_sid_str = offline_sid_str.clone();
+            let offline_username = payload.offline_username.clone();
             scope.spawn(move || {
                 // Convert SID strings to psids locally in this thread.
                 let mut psids: Vec<*mut c_void> = Vec::new();
@@ -730,7 +754,9 @@ 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) }.map_err(|err| {
+                    add_write_ace_owner_context(&root, err, &offline_sid_str, &offline_username)
+                });
 
                 for psid in psids {
                     unsafe {