diff --git a/codex-rs/windows-sandbox-rs/src/acl.rs b/codex-rs/windows-sandbox-rs/src/acl.rs index f351dba190..e69bb0901e 100644 --- a/codex-rs/windows-sandbox-rs/src/acl.rs +++ b/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> { + 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( diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index dd8f23d00f..41f6c0c43e 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/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; 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 e3f200ca71..bbd60f0726 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs +++ b/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 {