diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 7c86dd3b6..b96a09f17 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -343,7 +343,8 @@ impl App { let env_map: std::collections::HashMap = std::env::vars().collect(); let tx = app.app_event_tx.clone(); let logs_base_dir = app.config.codex_home.clone(); - Self::spawn_world_writable_scan(cwd, env_map, logs_base_dir, tx); + let sandbox_policy = app.config.sandbox_policy.clone(); + Self::spawn_world_writable_scan(cwd, env_map, logs_base_dir, sandbox_policy, tx); } } @@ -717,7 +718,14 @@ impl App { std::env::vars().collect(); let tx = self.app_event_tx.clone(); let logs_base_dir = self.config.codex_home.clone(); - Self::spawn_world_writable_scan(cwd, env_map, logs_base_dir, tx); + let sandbox_policy = self.config.sandbox_policy.clone(); + Self::spawn_world_writable_scan( + cwd, + env_map, + logs_base_dir, + sandbox_policy, + tx, + ); } } } @@ -911,6 +919,7 @@ impl App { cwd: PathBuf, env_map: std::collections::HashMap, logs_base_dir: PathBuf, + sandbox_policy: codex_core::protocol::SandboxPolicy, tx: AppEventSender, ) { #[inline] @@ -920,8 +929,10 @@ impl App { } tokio::task::spawn_blocking(move || { let result = codex_windows_sandbox::preflight_audit_everyone_writable( + &logs_base_dir, &cwd, &env_map, + &sandbox_policy, Some(logs_base_dir.as_path()), ); if let Ok(ref paths) = result diff --git a/codex-rs/windows-sandbox-rs/src/acl.rs b/codex-rs/windows-sandbox-rs/src/acl.rs index 1ec75e672..f2e1e0948 100644 --- a/codex-rs/windows-sandbox-rs/src/acl.rs +++ b/codex-rs/windows-sandbox-rs/src/acl.rs @@ -31,11 +31,17 @@ use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_NORMAL; use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_EXECUTE; use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_READ; use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_WRITE; +use windows_sys::Win32::Storage::FileSystem::FILE_APPEND_DATA; +use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_ATTRIBUTES; +use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_DATA; +use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_EA; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE; use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING; const SE_KERNEL_OBJECT: u32 = 6; const INHERIT_ONLY_ACE: u8 = 0x08; +const GENERIC_WRITE_MASK: u32 = 0x4000_0000; +const DENY_ACCESS: i32 = 3; pub unsafe fn dacl_has_write_allow_for_sid(p_dacl: *mut ACL, psid: *mut c_void) -> bool { if p_dacl.is_null() { @@ -78,6 +84,49 @@ pub unsafe fn dacl_has_write_allow_for_sid(p_dacl: *mut ACL, psid: *mut c_void) false } +pub unsafe fn dacl_has_write_deny_for_sid(p_dacl: *mut ACL, psid: *mut c_void) -> bool { + if p_dacl.is_null() { + return false; + } + let mut info: ACL_SIZE_INFORMATION = std::mem::zeroed(); + let ok = GetAclInformation( + p_dacl as *const ACL, + &mut info as *mut _ as *mut c_void, + std::mem::size_of::() as u32, + AclSizeInformation, + ); + if ok == 0 { + return false; + } + let deny_write_mask = FILE_GENERIC_WRITE + | FILE_WRITE_DATA + | FILE_APPEND_DATA + | FILE_WRITE_EA + | FILE_WRITE_ATTRIBUTES + | GENERIC_WRITE_MASK; + for i in 0..info.AceCount { + let mut p_ace: *mut c_void = std::ptr::null_mut(); + if GetAce(p_dacl as *const ACL, i, &mut p_ace) == 0 { + continue; + } + let hdr = &*(p_ace as *const ACE_HEADER); + if hdr.AceType != 1 { + continue; // ACCESS_DENIED_ACE_TYPE + } + if (hdr.AceFlags & INHERIT_ONLY_ACE) != 0 { + continue; + } + let ace = &*(p_ace as *const ACCESS_ALLOWED_ACE); + let base = p_ace as usize; + let sid_ptr = + (base + std::mem::size_of::() + std::mem::size_of::()) as *mut c_void; + if EqualSid(sid_ptr, psid) != 0 && (ace.Mask & deny_write_mask) != 0 { + return true; + } + } + false +} + // Compute effective rights for a trustee SID against a DACL and decide if write is effectively allowed. // This accounts for deny ACEs and ordering; falls back to a conservative per-ACE scan if the API fails. #[allow(dead_code)] @@ -168,6 +217,67 @@ pub unsafe fn add_allow_ace(path: &Path, psid: *mut c_void) -> Result { Ok(added) } +pub unsafe fn add_deny_write_ace(path: &Path, psid: *mut c_void) -> Result { + let mut p_sd: *mut c_void = std::ptr::null_mut(); + let mut p_dacl: *mut ACL = std::ptr::null_mut(); + let code = GetNamedSecurityInfoW( + to_wide(path).as_ptr(), + 1, + DACL_SECURITY_INFORMATION, + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut p_dacl, + std::ptr::null_mut(), + &mut p_sd, + ); + if code != ERROR_SUCCESS { + return Err(anyhow!("GetNamedSecurityInfoW failed: {}", code)); + } + let mut added = false; + if !dacl_has_write_deny_for_sid(p_dacl, psid) { + let trustee = TRUSTEE_W { + pMultipleTrustee: std::ptr::null_mut(), + MultipleTrusteeOperation: 0, + TrusteeForm: TRUSTEE_IS_SID, + TrusteeType: TRUSTEE_IS_UNKNOWN, + ptstrName: psid as *mut u16, + }; + let mut explicit: EXPLICIT_ACCESS_W = std::mem::zeroed(); + explicit.grfAccessPermissions = FILE_GENERIC_WRITE + | FILE_WRITE_DATA + | FILE_APPEND_DATA + | FILE_WRITE_EA + | FILE_WRITE_ATTRIBUTES + | GENERIC_WRITE_MASK; + explicit.grfAccessMode = DENY_ACCESS; + explicit.grfInheritance = CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE; + explicit.Trustee = trustee; + let mut p_new_dacl: *mut ACL = std::ptr::null_mut(); + let code2 = SetEntriesInAclW(1, &explicit, p_dacl, &mut p_new_dacl); + if code2 == ERROR_SUCCESS { + let code3 = SetNamedSecurityInfoW( + to_wide(path).as_ptr() as *mut u16, + 1, + DACL_SECURITY_INFORMATION, + std::ptr::null_mut(), + std::ptr::null_mut(), + p_new_dacl, + std::ptr::null_mut(), + ); + if code3 == ERROR_SUCCESS { + added = true; + } + if !p_new_dacl.is_null() { + LocalFree(p_new_dacl as HLOCAL); + } + } + } + if !p_sd.is_null() { + LocalFree(p_sd as HLOCAL); + } + Ok(added) +} + pub unsafe fn revoke_ace(path: &Path, psid: *mut c_void) { let mut p_sd: *mut c_void = std::ptr::null_mut(); let mut p_dacl: *mut ACL = std::ptr::null_mut(); diff --git a/codex-rs/windows-sandbox-rs/src/audit.rs b/codex-rs/windows-sandbox-rs/src/audit.rs index cba8d0633..210fd58ee 100644 --- a/codex-rs/windows-sandbox-rs/src/audit.rs +++ b/codex-rs/windows-sandbox-rs/src/audit.rs @@ -1,4 +1,11 @@ +use crate::acl::add_deny_write_ace; +use crate::cap::cap_sid_file; +use crate::cap::load_or_create_cap_sids; +use crate::logging::log_note; +use crate::policy::SandboxPolicy; +use crate::token::convert_string_sid_to_sid; use crate::token::world_sid; +use anyhow::anyhow; use crate::winutil::to_wide; use anyhow::Result; use std::collections::HashMap; @@ -271,6 +278,13 @@ pub fn audit_everyone_writable( for p in &flagged { list.push_str(&format!("\n - {}", p.display())); } + crate::logging::log_note( + &format!( + "AUDIT: world-writable scan FAILED; cwd={cwd:?}; checked={checked}; duration_ms={elapsed_ms}; flagged:{}", + list + ), + logs_base_dir, + ); return Ok(flagged); } @@ -282,6 +296,70 @@ pub fn audit_everyone_writable( Ok(Vec::new()) } +pub fn apply_capability_denies_for_world_writable( + codex_home: &Path, + flagged: &[PathBuf], + sandbox_policy: &SandboxPolicy, + cwd: &Path, + logs_base_dir: Option<&Path>, +) -> Result<()> { + if flagged.is_empty() { + return Ok(()); + } + std::fs::create_dir_all(codex_home)?; + let cap_path = cap_sid_file(codex_home); + let caps = load_or_create_cap_sids(codex_home); + std::fs::write(&cap_path, serde_json::to_string(&caps)?)?; + let (active_sid, workspace_roots): (*mut c_void, Vec) = match sandbox_policy { + SandboxPolicy::WorkspaceWrite { writable_roots, .. } => { + let sid = unsafe { convert_string_sid_to_sid(&caps.workspace) } + .ok_or_else(|| anyhow!("ConvertStringSidToSidW failed for workspace capability"))?; + let mut roots: Vec = + vec![dunce::canonicalize(cwd).unwrap_or_else(|_| cwd.to_path_buf())]; + for root in writable_roots { + let candidate = if root.is_absolute() { + root.clone() + } else { + cwd.join(root) + }; + roots.push(dunce::canonicalize(&candidate).unwrap_or(candidate)); + } + (sid, roots) + } + SandboxPolicy::ReadOnly => ( + unsafe { convert_string_sid_to_sid(&caps.readonly) }.ok_or_else(|| { + anyhow!("ConvertStringSidToSidW failed for readonly capability") + })?, + Vec::new(), + ), + SandboxPolicy::DangerFullAccess => { + return Ok(()); + } + }; + for path in flagged { + if workspace_roots.iter().any(|root| path.starts_with(root)) { + continue; + } + let res = unsafe { add_deny_write_ace(path, active_sid) }; + match res { + Ok(true) => log_note( + &format!("AUDIT: applied capability deny ACE to {}", path.display()), + logs_base_dir, + ), + Ok(false) => {} + Err(err) => log_note( + &format!( + "AUDIT: failed to apply capability deny ACE to {}: {}", + path.display(), + err + ), + logs_base_dir, + ), + } + } + Ok(()) +} + fn normalize_windows_path_for_display(p: impl AsRef) -> String { let canon = dunce::canonicalize(p.as_ref()).unwrap_or_else(|_| p.as_ref().to_path_buf()); canon.display().to_string().replace('/', "\\") diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index a2b4b6a53..1720294f2 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -38,6 +38,7 @@ mod windows_impl { use super::env::normalize_null_device_env; use super::logging::debug_log; use super::logging::log_failure; + use super::logging::log_note; use super::logging::log_start; use super::logging::log_success; use super::policy::parse_policy; @@ -178,11 +179,29 @@ mod windows_impl { } pub fn preflight_audit_everyone_writable( + codex_home: &Path, cwd: &Path, env_map: &HashMap, + sandbox_policy: &SandboxPolicy, logs_base_dir: Option<&Path>, ) -> Result> { - audit::audit_everyone_writable(cwd, env_map, logs_base_dir) + let flagged = audit::audit_everyone_writable(cwd, env_map, logs_base_dir)?; + if flagged.is_empty() { + return Ok(flagged); + } + if let Err(err) = audit::apply_capability_denies_for_world_writable( + codex_home, + &flagged, + sandbox_policy, + cwd, + logs_base_dir, + ) { + log_note( + &format!("AUDIT: failed to apply capability deny ACEs: {}", err), + logs_base_dir, + ); + } + Ok(Vec::new()) } pub fn run_windows_sandbox_capture( @@ -434,6 +453,7 @@ mod windows_impl { mod stub { use anyhow::bail; use anyhow::Result; + use codex_protocol::protocol::SandboxPolicy; use std::collections::HashMap; use std::path::Path; @@ -446,8 +466,10 @@ mod stub { } pub fn preflight_audit_everyone_writable( + _codex_home: &Path, _cwd: &Path, _env_map: &HashMap, + _sandbox_policy: &SandboxPolicy, _logs_base_dir: Option<&Path>, ) -> Result> { bail!("Windows sandbox is only available on Windows")