From 486b1c4d9d60e7d7690b6db81df9fbc100a2a0e1 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Mon, 24 Nov 2025 09:51:58 -0800 Subject: [PATCH] consolidate world-writable-directories scanning. (#7234) clean up the code for scanning for world writable directories One path (selecting a sandbox mode from /approvals) was using an incorrect method that did not use the new method of creating deny aces to prevent writing to those directories. Now all paths are the same. --- codex-rs/tui/src/app.rs | 37 ++------------- codex-rs/tui/src/chatwidget.rs | 17 +++++-- codex-rs/windows-sandbox-rs/src/audit.rs | 52 ++++++++++----------- codex-rs/windows-sandbox-rs/src/lib.rs | 59 ++++-------------------- 4 files changed, 53 insertions(+), 112 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 9c1bb5a30..42dadd96c 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -941,47 +941,20 @@ impl App { sandbox_policy: codex_core::protocol::SandboxPolicy, tx: AppEventSender, ) { - #[inline] - fn normalize_windows_path_for_display(p: &std::path::Path) -> String { - let canon = dunce::canonicalize(p).unwrap_or_else(|_| p.to_path_buf()); - canon.display().to_string().replace('/', "\\") - } tokio::task::spawn_blocking(move || { - let result = codex_windows_sandbox::preflight_audit_everyone_writable( + let result = codex_windows_sandbox::apply_world_writable_scan_and_denies( &logs_base_dir, &cwd, &env_map, &sandbox_policy, Some(logs_base_dir.as_path()), ); - if let Ok(ref paths) = result - && !paths.is_empty() - { - let as_strings: Vec = paths - .iter() - .map(|p| normalize_windows_path_for_display(p)) - .collect(); - let sample_paths: Vec = as_strings.iter().take(3).cloned().collect(); - let extra_count = if as_strings.len() > sample_paths.len() { - as_strings.len() - sample_paths.len() - } else { - 0 - }; - + if result.is_err() { + // Scan failed: warn without examples. tx.send(AppEvent::OpenWorldWritableWarningConfirmation { preset: None, - sample_paths, - extra_count, - failed_scan: false, - }); - } else if result.is_err() { - // Scan failed: still warn, but with no examples and mark as failed. - let sample_paths: Vec = Vec::new(); - let extra_count = 0usize; - tx.send(AppEvent::OpenWorldWritableWarningConfirmation { - preset: None, - sample_paths, - extra_count, + sample_paths: Vec::new(), + extra_count: 0usize, failed_scan: true, }); } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 1e9fbd39f..ddba41443 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2380,11 +2380,18 @@ impl ChatWidget { { return None; } - let cwd = match std::env::current_dir() { - Ok(cwd) => cwd, - Err(_) => return Some((Vec::new(), 0, true)), - }; - codex_windows_sandbox::world_writable_warning_details(self.config.codex_home.as_path(), cwd) + let cwd = self.config.cwd.clone(); + let env_map: std::collections::HashMap = std::env::vars().collect(); + match codex_windows_sandbox::apply_world_writable_scan_and_denies( + self.config.codex_home.as_path(), + cwd.as_path(), + &env_map, + &self.config.sandbox_policy, + Some(self.config.codex_home.as_path()), + ) { + Ok(_) => None, + Err(_) => Some((Vec::new(), 0, true)), + } } #[cfg(not(target_os = "windows"))] diff --git a/codex-rs/windows-sandbox-rs/src/audit.rs b/codex-rs/windows-sandbox-rs/src/audit.rs index 210fd58ee..6234dbf26 100644 --- a/codex-rs/windows-sandbox-rs/src/audit.rs +++ b/codex-rs/windows-sandbox-rs/src/audit.rs @@ -8,7 +8,6 @@ use crate::token::world_sid; use anyhow::anyhow; use crate::winutil::to_wide; use anyhow::Result; -use std::collections::HashMap; use std::collections::HashSet; use std::ffi::c_void; use std::path::Path; @@ -296,6 +295,32 @@ pub fn audit_everyone_writable( Ok(Vec::new()) } +pub fn apply_world_writable_scan_and_denies( + codex_home: &Path, + cwd: &Path, + env_map: &std::collections::HashMap, + sandbox_policy: &SandboxPolicy, + logs_base_dir: Option<&Path>, +) -> Result<()> { + let flagged = audit_everyone_writable(cwd, env_map, logs_base_dir)?; + if flagged.is_empty() { + return Ok(()); + } + if let Err(err) = 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(()) +} + pub fn apply_capability_denies_for_world_writable( codex_home: &Path, flagged: &[PathBuf], @@ -359,31 +384,6 @@ pub fn apply_capability_denies_for_world_writable( } 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('/', "\\") -} - -pub fn world_writable_warning_details( - codex_home: impl AsRef, - cwd: impl AsRef, -) -> Option<(Vec, usize, bool)> { - let env_map: HashMap = std::env::vars().collect(); - match audit_everyone_writable(cwd.as_ref(), &env_map, Some(codex_home.as_ref())) { - Ok(paths) if paths.is_empty() => None, - Ok(paths) => { - let as_strings: Vec = paths - .iter() - .map(normalize_windows_path_for_display) - .collect(); - let sample_paths: Vec = as_strings.iter().take(3).cloned().collect(); - let extra_count = as_strings.len().saturating_sub(sample_paths.len()); - Some((sample_paths, extra_count, false)) - } - Err(_) => Some((Vec::new(), 0, true)), - } -} // Fast mask-based check: does the DACL contain any ACCESS_ALLOWED ACE for // Everyone that grants write after generic bits are expanded? Skips inherit-only // ACEs (do not apply to the current object). diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index e2277daad..2d591fced 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -7,21 +7,17 @@ macro_rules! windows_modules { windows_modules!(acl, allow, audit, cap, env, logging, policy, token, winutil); #[cfg(target_os = "windows")] -pub use audit::world_writable_warning_details; -#[cfg(target_os = "windows")] -pub use windows_impl::preflight_audit_everyone_writable; +pub use audit::apply_world_writable_scan_and_denies; #[cfg(target_os = "windows")] pub use windows_impl::run_windows_sandbox_capture; #[cfg(target_os = "windows")] pub use windows_impl::CaptureResult; #[cfg(not(target_os = "windows"))] -pub use stub::preflight_audit_everyone_writable; +pub use stub::apply_world_writable_scan_and_denies; #[cfg(not(target_os = "windows"))] pub use stub::run_windows_sandbox_capture; #[cfg(not(target_os = "windows"))] -pub use stub::world_writable_warning_details; -#[cfg(not(target_os = "windows"))] pub use stub::CaptureResult; #[cfg(target_os = "windows")] @@ -30,7 +26,6 @@ mod windows_impl { use super::acl::allow_null_device; use super::acl::revoke_ace; use super::allow::compute_allow_paths; - use super::audit; use super::cap::cap_sid_file; use super::cap::load_or_create_cap_sids; use super::env::apply_no_network_to_env; @@ -38,7 +33,6 @@ 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; @@ -182,32 +176,6 @@ mod windows_impl { pub timed_out: bool, } - pub fn preflight_audit_everyone_writable( - codex_home: &Path, - cwd: &Path, - env_map: &HashMap, - sandbox_policy: &SandboxPolicy, - logs_base_dir: Option<&Path>, - ) -> Result> { - 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( policy_json_or_preset: &str, sandbox_policy_cwd: &Path, @@ -502,16 +470,6 @@ mod stub { pub timed_out: bool, } - 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") - } - pub fn run_windows_sandbox_capture( _policy_json_or_preset: &str, _sandbox_policy_cwd: &Path, @@ -524,10 +482,13 @@ mod stub { bail!("Windows sandbox is only available on Windows") } - pub fn world_writable_warning_details( - _codex_home: impl AsRef, - _cwd: impl AsRef, - ) -> Option<(Vec, usize, bool)> { - None + pub fn apply_world_writable_scan_and_denies( + _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") } }