diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index c4121da2..e8ce0bfe 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1452,6 +1452,7 @@ dependencies = [ "codex-login", "codex-ollama", "codex-protocol", + "codex-windows-sandbox", "color-eyre", "crossterm", "diffy", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 36f31d2e..619dbb56 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -87,7 +87,7 @@ codex-utils-pty = { path = "utils/pty" } codex-utils-readiness = { path = "utils/readiness" } codex-utils-string = { path = "utils/string" } codex-utils-tokenizer = { path = "utils/tokenizer" } -codex-windows-sandbox = { path = "windows-sandbox" } +codex-windows-sandbox = { path = "windows-sandbox-rs" } core_test_support = { path = "core/tests/common" } mcp-types = { path = "mcp-types" } mcp_test_support = { path = "mcp-server/tests/common" } diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index c8e3bfc3..64974c24 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -23,6 +23,8 @@ pub enum ConfigEdit { }, /// Toggle the acknowledgement flag under `[notice]`. SetNoticeHideFullAccessWarning(bool), + /// Toggle the Windows world-writable directories warning acknowledgement flag. + SetNoticeHideWorldWritableWarning(bool), /// Toggle the Windows onboarding acknowledgement flag. SetWindowsWslSetupAcknowledged(bool), /// Replace the entire `[mcp_servers]` table. @@ -239,6 +241,11 @@ impl ConfigDocument { &[Notice::TABLE_KEY, "hide_full_access_warning"], value(*acknowledged), )), + ConfigEdit::SetNoticeHideWorldWritableWarning(acknowledged) => Ok(self.write_value( + Scope::Global, + &[Notice::TABLE_KEY, "hide_world_writable_warning"], + value(*acknowledged), + )), ConfigEdit::SetWindowsWslSetupAcknowledged(acknowledged) => Ok(self.write_value( Scope::Global, &["windows_wsl_setup_acknowledged"], @@ -473,6 +480,12 @@ impl ConfigEditsBuilder { self } + pub fn set_hide_world_writable_warning(mut self, acknowledged: bool) -> Self { + self.edits + .push(ConfigEdit::SetNoticeHideWorldWritableWarning(acknowledged)); + self + } + pub fn set_windows_wsl_setup_acknowledged(mut self, acknowledged: bool) -> Self { self.edits .push(ConfigEdit::SetWindowsWslSetupAcknowledged(acknowledged)); diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index fd93df8f..a5fd71d1 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -358,6 +358,8 @@ pub struct Tui { pub struct Notice { /// Tracks whether the user has acknowledged the full access warning prompt. pub hide_full_access_warning: Option, + /// Tracks whether the user has acknowledged the Windows world-writable directories warning. + pub hide_world_writable_warning: Option, } impl Notice { diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index c56aba0f..ac00e905 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -27,6 +27,7 @@ base64 = { workspace = true } chrono = { workspace = true, features = ["serde"] } clap = { workspace = true, features = ["derive"] } codex-ansi-escape = { workspace = true } +codex-app-server-protocol = { workspace = true } codex-arg0 = { workspace = true } codex-common = { workspace = true, features = [ "cli", @@ -34,17 +35,13 @@ codex-common = { workspace = true, features = [ "sandbox_summary", ] } codex-core = { workspace = true } +codex-feedback = { workspace = true } codex-file-search = { workspace = true } codex-login = { workspace = true } codex-ollama = { workspace = true } codex-protocol = { workspace = true } -codex-app-server-protocol = { workspace = true } -codex-feedback = { workspace = true } color-eyre = { workspace = true } -crossterm = { workspace = true, features = [ - "bracketed-paste", - "event-stream", -] } +crossterm = { workspace = true, features = ["bracketed-paste", "event-stream"] } diffy = { workspace = true } dirs = { workspace = true } dunce = { workspace = true } @@ -52,6 +49,7 @@ image = { workspace = true, features = ["jpeg", "png"] } itertools = { workspace = true } lazy_static = { workspace = true } mcp-types = { workspace = true } +opentelemetry-appender-tracing = { workspace = true } pathdiff = { workspace = true } pulldown-cmark = { workspace = true } rand = { workspace = true } @@ -71,8 +69,6 @@ strum_macros = { workspace = true } supports-color = { workspace = true } tempfile = { workspace = true } textwrap = { workspace = true } -tree-sitter-highlight = { workspace = true } -tree-sitter-bash = { workspace = true } tokio = { workspace = true, features = [ "io-std", "macros", @@ -85,11 +81,14 @@ toml = { workspace = true } tracing = { workspace = true, features = ["log"] } tracing-appender = { workspace = true } tracing-subscriber = { workspace = true, features = ["env-filter"] } -opentelemetry-appender-tracing = { workspace = true } +tree-sitter-bash = { workspace = true } +tree-sitter-highlight = { workspace = true } unicode-segmentation = { workspace = true } unicode-width = { workspace = true } url = { workspace = true } +codex-windows-sandbox = { workspace = true } + [target.'cfg(unix)'.dependencies] libc = { workspace = true } @@ -105,5 +104,5 @@ chrono = { workspace = true, features = ["serde"] } insta = { workspace = true } pretty_assertions = { workspace = true } rand = { workspace = true } -vt100 = { workspace = true } serial_test = { workspace = true } +vt100 = { workspace = true } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 1d2c9690..fbf3d2a5 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -79,6 +79,9 @@ pub(crate) struct App { pub(crate) feedback: codex_feedback::CodexFeedback, /// Set when the user confirms an update; propagated on exit. pub(crate) pending_update_action: Option, + + // One-shot suppression of the next world-writable scan after user confirmation. + skip_world_writable_scan_once: bool, } impl App { @@ -168,8 +171,30 @@ impl App { backtrack: BacktrackState::default(), feedback: feedback.clone(), pending_update_action: None, + skip_world_writable_scan_once: false, }; + // On startup, if Auto mode (workspace-write) is active, warn about world-writable dirs on Windows. + #[cfg(target_os = "windows")] + { + let should_check = codex_core::get_platform_sandbox().is_some() + && matches!( + app.config.sandbox_policy, + codex_core::protocol::SandboxPolicy::WorkspaceWrite { .. } + ) + && !app + .config + .notices + .hide_world_writable_warning + .unwrap_or(false); + if should_check { + let cwd = app.config.cwd.clone(); + let env_map: std::collections::HashMap = std::env::vars().collect(); + let tx = app.app_event_tx.clone(); + Self::spawn_world_writable_scan(cwd, env_map, tx, false); + } + } + #[cfg(not(debug_assertions))] if let Some(latest_version) = upgrade_version { app.handle_event( @@ -360,6 +385,10 @@ impl App { AppEvent::OpenFullAccessConfirmation { preset } => { self.chat_widget.open_full_access_confirmation(preset); } + AppEvent::OpenWorldWritableWarningConfirmation { preset } => { + self.chat_widget + .open_world_writable_warning_confirmation(preset); + } AppEvent::OpenFeedbackNote { category, include_logs, @@ -418,11 +447,45 @@ impl App { self.chat_widget.set_approval_policy(policy); } AppEvent::UpdateSandboxPolicy(policy) => { + #[cfg(target_os = "windows")] + let policy_is_workspace_write = matches!( + policy, + codex_core::protocol::SandboxPolicy::WorkspaceWrite { .. } + ); + self.chat_widget.set_sandbox_policy(policy); + + // If sandbox policy becomes workspace-write, run the Windows world-writable scan. + #[cfg(target_os = "windows")] + { + // One-shot suppression if the user just confirmed continue. + if self.skip_world_writable_scan_once { + self.skip_world_writable_scan_once = false; + return Ok(true); + } + + let should_check = codex_core::get_platform_sandbox().is_some() + && policy_is_workspace_write + && !self.chat_widget.world_writable_warning_hidden(); + if should_check { + let cwd = self.config.cwd.clone(); + let env_map: std::collections::HashMap = + std::env::vars().collect(); + let tx = self.app_event_tx.clone(); + Self::spawn_world_writable_scan(cwd, env_map, tx, false); + } + } + } + AppEvent::SkipNextWorldWritableScan => { + self.skip_world_writable_scan_once = true; } AppEvent::UpdateFullAccessWarningAcknowledged(ack) => { self.chat_widget.set_full_access_warning_acknowledged(ack); } + AppEvent::UpdateWorldWritableWarningAcknowledged(ack) => { + self.chat_widget + .set_world_writable_warning_acknowledged(ack); + } AppEvent::PersistFullAccessWarningAcknowledged => { if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) .set_hide_full_access_warning(true) @@ -438,6 +501,21 @@ impl App { )); } } + AppEvent::PersistWorldWritableWarningAcknowledged => { + if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) + .set_hide_world_writable_warning(true) + .apply() + .await + { + tracing::error!( + error = %err, + "failed to persist world-writable warning acknowledgement" + ); + self.chat_widget.add_error_message(format!( + "Failed to save Auto mode warning preference: {err}" + )); + } + } AppEvent::OpenApprovalsPopup => { self.chat_widget.open_approvals_popup(); } @@ -541,6 +619,31 @@ impl App { } }; } + + #[cfg(target_os = "windows")] + fn spawn_world_writable_scan( + cwd: PathBuf, + env_map: std::collections::HashMap, + tx: AppEventSender, + apply_preset_on_continue: bool, + ) { + tokio::task::spawn_blocking(move || { + if codex_windows_sandbox::preflight_audit_everyone_writable(&cwd, &env_map).is_err() { + if apply_preset_on_continue { + if let Some(preset) = codex_common::approval_presets::builtin_approval_presets() + .into_iter() + .find(|p| p.id == "auto") + { + tx.send(AppEvent::OpenWorldWritableWarningConfirmation { + preset: Some(preset), + }); + } + } else { + tx.send(AppEvent::OpenWorldWritableWarningConfirmation { preset: None }); + } + } + }); + } } #[cfg(test)] @@ -592,6 +695,7 @@ mod tests { backtrack: BacktrackState::default(), feedback: codex_feedback::CodexFeedback::new(), pending_update_action: None, + skip_world_writable_scan_once: false, } } diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 514e2347..f14951a9 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -72,7 +72,17 @@ pub(crate) enum AppEvent { preset: ApprovalPreset, }, + /// Open the Windows world-writable directories warning. + /// If `preset` is `Some`, the confirmation will apply the provided + /// approval/sandbox configuration on Continue; if `None`, it performs no + /// policy change and only acknowledges/dismisses the warning. + #[cfg_attr(not(target_os = "windows"), allow(dead_code))] + OpenWorldWritableWarningConfirmation { + preset: Option, + }, + /// Show Windows Subsystem for Linux setup instructions for auto mode. + #[cfg_attr(not(target_os = "windows"), allow(dead_code))] ShowWindowsAutoModeInstructions, /// Update the current approval policy in the running app and widget. @@ -84,9 +94,21 @@ pub(crate) enum AppEvent { /// Update whether the full access warning prompt has been acknowledged. UpdateFullAccessWarningAcknowledged(bool), + /// Update whether the world-writable directories warning has been acknowledged. + #[cfg_attr(not(target_os = "windows"), allow(dead_code))] + UpdateWorldWritableWarningAcknowledged(bool), + /// Persist the acknowledgement flag for the full access warning prompt. PersistFullAccessWarningAcknowledged, + /// Persist the acknowledgement flag for the world-writable directories warning. + #[cfg_attr(not(target_os = "windows"), allow(dead_code))] + PersistWorldWritableWarningAcknowledged, + + /// Skip the next world-writable scan (one-shot) after a user-confirmed continue. + #[cfg_attr(not(target_os = "windows"), allow(dead_code))] + SkipNextWorldWritableScan, + /// Re-open the approval presets popup. OpenApprovalsPopup, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 72b64fac..30f9b66f 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2030,13 +2030,34 @@ impl ChatWidget { preset: preset_clone.clone(), }); })] - } else if cfg!(target_os = "windows") - && preset.id == "auto" - && codex_core::get_platform_sandbox().is_none() - { - vec![Box::new(|tx| { - tx.send(AppEvent::ShowWindowsAutoModeInstructions); - })] + } else if preset.id == "auto" { + #[cfg(target_os = "windows")] + { + if codex_core::get_platform_sandbox().is_none() { + vec![Box::new(|tx| { + tx.send(AppEvent::ShowWindowsAutoModeInstructions); + })] + } else if !self + .config + .notices + .hide_world_writable_warning + .unwrap_or(false) + && self.windows_world_writable_flagged() + { + let preset_clone = preset.clone(); + vec![Box::new(move |tx| { + tx.send(AppEvent::OpenWorldWritableWarningConfirmation { + preset: Some(preset_clone.clone()), + }); + })] + } else { + Self::approval_preset_actions(preset.approval, preset.sandbox.clone()) + } + } + #[cfg(not(target_os = "windows"))] + { + Self::approval_preset_actions(preset.approval, preset.sandbox.clone()) + } } else { Self::approval_preset_actions(preset.approval, preset.sandbox.clone()) }; @@ -2078,6 +2099,19 @@ impl ChatWidget { })] } + #[cfg(target_os = "windows")] + fn windows_world_writable_flagged(&self) -> bool { + use std::collections::HashMap; + let mut env_map: HashMap = HashMap::new(); + for (k, v) in std::env::vars() { + env_map.insert(k, v); + } + match codex_windows_sandbox::preflight_audit_everyone_writable(&self.config.cwd, &env_map) { + Ok(()) => false, + Err(_) => true, + } + } + pub(crate) fn open_full_access_confirmation(&mut self, preset: ApprovalPreset) { let approval = preset.approval; let sandbox = preset.sandbox; @@ -2142,6 +2176,95 @@ impl ChatWidget { }); } + #[cfg(target_os = "windows")] + pub(crate) fn open_world_writable_warning_confirmation( + &mut self, + preset: Option, + ) { + let (approval, sandbox) = match &preset { + Some(p) => (Some(p.approval), Some(p.sandbox.clone())), + None => (None, None), + }; + let mut header_children: Vec> = Vec::new(); + let title_line = Line::from("Auto mode has unprotected directories").bold(); + let info_line = Line::from(vec![ + "Some important directories on this system are world-writable. ".into(), + "The Windows sandbox cannot protect writes to these locations in Auto mode." + .fg(Color::Red), + ]); + header_children.push(Box::new(title_line)); + header_children.push(Box::new( + Paragraph::new(vec![info_line]).wrap(Wrap { trim: false }), + )); + let header = ColumnRenderable::with(header_children); + + // Build actions ensuring acknowledgement happens before applying the new sandbox policy, + // so downstream policy-change hooks don't re-trigger the warning. + let mut accept_actions: Vec = Vec::new(); + // Suppress the immediate re-scan once after user confirms continue. + accept_actions.push(Box::new(|tx| { + tx.send(AppEvent::SkipNextWorldWritableScan); + })); + if let (Some(approval), Some(sandbox)) = (approval, sandbox.clone()) { + accept_actions.extend(Self::approval_preset_actions(approval, sandbox)); + } + + let mut accept_and_remember_actions: Vec = Vec::new(); + accept_and_remember_actions.push(Box::new(|tx| { + tx.send(AppEvent::UpdateWorldWritableWarningAcknowledged(true)); + tx.send(AppEvent::PersistWorldWritableWarningAcknowledged); + })); + if let (Some(approval), Some(sandbox)) = (approval, sandbox) { + accept_and_remember_actions.extend(Self::approval_preset_actions(approval, sandbox)); + } + + let deny_actions: Vec = if preset.is_some() { + vec![Box::new(|tx| { + tx.send(AppEvent::OpenApprovalsPopup); + })] + } else { + Vec::new() + }; + + let items = vec![ + SelectionItem { + name: "Continue".to_string(), + description: Some("Apply Auto mode for this session".to_string()), + actions: accept_actions, + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "Continue and don't warn again".to_string(), + description: Some("Enable Auto mode and remember this choice".to_string()), + actions: accept_and_remember_actions, + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "Cancel".to_string(), + description: Some("Go back without enabling Auto mode".to_string()), + actions: deny_actions, + dismiss_on_select: true, + ..Default::default() + }, + ]; + + self.bottom_pane.show_selection_view(SelectionViewParams { + footer_hint: Some(standard_popup_hint_line()), + items, + header: Box::new(header), + ..Default::default() + }); + } + + #[cfg(not(target_os = "windows"))] + pub(crate) fn open_world_writable_warning_confirmation( + &mut self, + _preset: Option, + ) { + } + #[cfg(target_os = "windows")] pub(crate) fn open_windows_auto_mode_instructions(&mut self) { use ratatui_macros::line; @@ -2193,6 +2316,18 @@ impl ChatWidget { self.config.notices.hide_full_access_warning = Some(acknowledged); } + pub(crate) fn set_world_writable_warning_acknowledged(&mut self, acknowledged: bool) { + self.config.notices.hide_world_writable_warning = Some(acknowledged); + } + + #[cfg_attr(not(target_os = "windows"), allow(dead_code))] + pub(crate) fn world_writable_warning_hidden(&self) -> bool { + self.config + .notices + .hide_world_writable_warning + .unwrap_or(false) + } + /// Set the reasoning effort in the widget's config copy. pub(crate) fn set_reasoning_effort(&mut self, effort: Option) { self.config.model_reasoning_effort = effort; diff --git a/codex-rs/windows-sandbox-rs/src/audit.rs b/codex-rs/windows-sandbox-rs/src/audit.rs index 3f8ae4ff..71dceaa5 100644 --- a/codex-rs/windows-sandbox-rs/src/audit.rs +++ b/codex-rs/windows-sandbox-rs/src/audit.rs @@ -1,4 +1,3 @@ -use crate::acl::dacl_effective_allows_write; use crate::token::world_sid; use crate::winutil::to_wide; use anyhow::anyhow; @@ -13,8 +12,31 @@ use windows_sys::Win32::Foundation::LocalFree; use windows_sys::Win32::Foundation::ERROR_SUCCESS; use windows_sys::Win32::Foundation::HLOCAL; use windows_sys::Win32::Security::Authorization::GetNamedSecurityInfoW; +use windows_sys::Win32::Security::Authorization::GetSecurityInfo; +use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE; +use windows_sys::Win32::Foundation::CloseHandle; +use windows_sys::Win32::Storage::FileSystem::CreateFileW; +use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_BACKUP_SEMANTICS; +use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_DELETE; +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; +use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_WRITE; +use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_DATA; +use windows_sys::Win32::Storage::FileSystem::FILE_APPEND_DATA; +use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_EA; +use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_ATTRIBUTES; +const GENERIC_ALL_MASK: u32 = 0x1000_0000; +const GENERIC_WRITE_MASK: u32 = 0x4000_0000; use windows_sys::Win32::Security::ACL; use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION; +use windows_sys::Win32::Security::ACL_SIZE_INFORMATION; +use windows_sys::Win32::Security::AclSizeInformation; +use windows_sys::Win32::Security::GetAclInformation; +use windows_sys::Win32::Security::GetAce; +use windows_sys::Win32::Security::ACCESS_ALLOWED_ACE; +use windows_sys::Win32::Security::ACE_HEADER; +use windows_sys::Win32::Security::EqualSid; fn unique_push(set: &mut HashSet, out: &mut Vec, p: PathBuf) { if let Ok(abs) = p.canonicalize() { @@ -27,30 +49,22 @@ fn unique_push(set: &mut HashSet, out: &mut Vec, p: PathBuf) { fn gather_candidates(cwd: &Path, env: &std::collections::HashMap) -> Vec { let mut set: HashSet = HashSet::new(); let mut out: Vec = Vec::new(); - // Core roots - for p in [ - PathBuf::from("C:/"), - PathBuf::from("C:/Windows"), - PathBuf::from("C:/ProgramData"), - ] { - unique_push(&mut set, &mut out, p); + // 1) CWD first (so immediate children get scanned early) + unique_push(&mut set, &mut out, cwd.to_path_buf()); + // 2) TEMP/TMP next (often small, quick to scan) + for k in ["TEMP", "TMP"] { + if let Some(v) = env.get(k).cloned().or_else(|| std::env::var(k).ok()) { + unique_push(&mut set, &mut out, PathBuf::from(v)); + } } - // User roots + // 3) User roots if let Some(up) = std::env::var_os("USERPROFILE") { unique_push(&mut set, &mut out, PathBuf::from(up)); } if let Some(pubp) = std::env::var_os("PUBLIC") { unique_push(&mut set, &mut out, PathBuf::from(pubp)); } - // CWD - unique_push(&mut set, &mut out, cwd.to_path_buf()); - // TEMP/TMP - for k in ["TEMP", "TMP"] { - if let Some(v) = env.get(k).cloned().or_else(|| std::env::var(k).ok()) { - unique_push(&mut set, &mut out, PathBuf::from(v)); - } - } - // PATH entries + // 4) PATH entries (best-effort) if let Some(path) = env .get("PATH") .cloned() @@ -62,31 +76,85 @@ fn gather_candidates(cwd: &Path, env: &std::collections::HashMap } } } + // 5) Core system roots last + for p in [ + PathBuf::from("C:/"), + PathBuf::from("C:/Windows"), + PathBuf::from("C:/ProgramData"), + ] { + unique_push(&mut set, &mut out, p); + } out } unsafe fn path_has_world_write_allow(path: &Path) -> Result { + // Prefer handle-based query (often faster than name-based), fallback to name-based on error 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, + + let mut try_named = false; + let wpath = to_wide(path); + let h = CreateFileW( + wpath.as_ptr(), + 0x00020000, // READ_CONTROL + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, std::ptr::null_mut(), - std::ptr::null_mut(), - &mut p_dacl, - std::ptr::null_mut(), - &mut p_sd, + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS, + 0, ); - if code != ERROR_SUCCESS { - if !p_sd.is_null() { - LocalFree(p_sd as HLOCAL); + if h == INVALID_HANDLE_VALUE { + try_named = true; + } else { + let code = GetSecurityInfo( + h, + 1, // SE_FILE_OBJECT + DACL_SECURITY_INFORMATION, + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut p_dacl, + std::ptr::null_mut(), + &mut p_sd, + ); + CloseHandle(h); + if code != ERROR_SUCCESS { + try_named = true; + if !p_sd.is_null() { + LocalFree(p_sd as HLOCAL); + p_sd = std::ptr::null_mut(); + p_dacl = std::ptr::null_mut(); + } } - return Ok(false); } + + if try_named { + let code = GetNamedSecurityInfoW( + wpath.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 { + if !p_sd.is_null() { + LocalFree(p_sd as HLOCAL); + } + return Ok(false); + } + } + let mut world = world_sid()?; let psid_world = world.as_mut_ptr() as *mut c_void; - let has = dacl_effective_allows_write(p_dacl, psid_world); + // Very fast mask-based check for world-writable grants (includes GENERIC_*). + if !dacl_quick_world_write_mask_allows(p_dacl, psid_world) { + if !p_sd.is_null() { LocalFree(p_sd as HLOCAL); } + return Ok(false); + } + // Quick detector flagged a write grant for Everyone: treat as writable. + let has = true; if !p_sd.is_null() { LocalFree(p_sd as HLOCAL); } @@ -100,18 +168,41 @@ pub fn audit_everyone_writable( let start = Instant::now(); let mut flagged: Vec = Vec::new(); let mut checked = 0usize; + // Fast path: check CWD immediate children first so workspace issues are caught early. + if let Ok(read) = std::fs::read_dir(cwd) { + for ent in read.flatten().take(250) { + if start.elapsed() > Duration::from_secs(5) || checked > 5000 { + break; + } + let ft = match ent.file_type() { + Ok(ft) => ft, + Err(_) => continue, + }; + if ft.is_symlink() || !ft.is_dir() { + continue; + } + let p = ent.path(); + checked += 1; + let has = unsafe { path_has_world_write_allow(&p)? }; + if has { + flagged.push(p); + } + } + } + // Continue with broader candidate sweep let candidates = gather_candidates(cwd, env); for root in candidates { if start.elapsed() > Duration::from_secs(5) || checked > 5000 { break; } checked += 1; - if unsafe { path_has_world_write_allow(&root)? } { + let has_root = unsafe { path_has_world_write_allow(&root)? }; + if has_root { flagged.push(root.clone()); } // one level down best-effort if let Ok(read) = std::fs::read_dir(&root) { - for ent in read.flatten().take(50) { + for ent in read.flatten().take(250) { let p = ent.path(); if start.elapsed() > Duration::from_secs(5) || checked > 5000 { break; @@ -126,22 +217,93 @@ pub fn audit_everyone_writable( } if ft.is_dir() { checked += 1; - if unsafe { path_has_world_write_allow(&p)? } { + let has_child = unsafe { path_has_world_write_allow(&p)? }; + if has_child { flagged.push(p); } } } } } + let elapsed_ms = start.elapsed().as_millis(); if !flagged.is_empty() { let mut list = String::new(); - for p in flagged { + for p in &flagged { list.push_str(&format!("\n - {}", p.display())); } + crate::logging::log_note( + &format!( + "AUDIT: world-writable scan FAILED; checked={checked}; duration_ms={elapsed_ms}; flagged:{}", + list + ), + Some(cwd), + ); + let mut list_err = String::new(); + for p in flagged { + list_err.push_str(&format!("\n - {}", p.display())); + } return Err(anyhow!( "Refusing to run: found directories writable by Everyone: {}", - list + list_err )); } + // Log success once if nothing flagged + crate::logging::log_note( + &format!( + "AUDIT: world-writable scan OK; checked={checked}; duration_ms={elapsed_ms}" + ), + Some(cwd), + ); Ok(()) } +// Fast mask-based check: does the DACL contain any ACCESS_ALLOWED ACE for +// Everyone that includes generic or specific write bits? Skips inherit-only +// ACEs (do not apply to the current object). +unsafe fn dacl_quick_world_write_mask_allows(p_dacl: *mut ACL, psid_world: *mut c_void) -> bool { + if p_dacl.is_null() { + return false; + } + const INHERIT_ONLY_ACE: u8 = 0x08; + 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; + } + for i in 0..(info.AceCount as usize) { + let mut p_ace: *mut c_void = std::ptr::null_mut(); + if GetAce(p_dacl as *const ACL, i as u32, &mut p_ace) == 0 { + continue; + } + let hdr = &*(p_ace as *const ACE_HEADER); + if hdr.AceType != 0 { // ACCESS_ALLOWED_ACE_TYPE + continue; + } + if (hdr.AceFlags & INHERIT_ONLY_ACE) != 0 { + continue; + } + let base = p_ace as usize; + let sid_ptr = (base + + std::mem::size_of::() + + std::mem::size_of::()) as *mut c_void; // skip header + mask + if EqualSid(sid_ptr, psid_world) != 0 { + let ace = &*(p_ace as *const ACCESS_ALLOWED_ACE); + let mask = ace.Mask; + let writey = FILE_GENERIC_WRITE + | FILE_WRITE_DATA + | FILE_APPEND_DATA + | FILE_WRITE_EA + | FILE_WRITE_ATTRIBUTES + | GENERIC_WRITE_MASK + | GENERIC_ALL_MASK; + if (mask & writey) != 0 { + return true; + } + } + } + false +} diff --git a/codex-rs/windows-sandbox-rs/src/logging.rs b/codex-rs/windows-sandbox-rs/src/logging.rs index 42d6d807..9887ce81 100644 --- a/codex-rs/windows-sandbox-rs/src/logging.rs +++ b/codex-rs/windows-sandbox-rs/src/logging.rs @@ -55,3 +55,8 @@ pub fn debug_log(msg: &str, base_dir: Option<&Path>) { eprintln!("{msg}"); } } + +// Unconditional note logging to sandbox_commands.rust.log +pub fn log_note(msg: &str, base_dir: Option<&Path>) { + append_line(msg, base_dir); +}