diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index d992fccc..06c13ff1 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -174,13 +174,14 @@ impl App { skip_world_writable_scan_once: false, }; - // On startup, if Auto mode (workspace-write) is active, warn about world-writable dirs on Windows. + // On startup, if Auto mode (workspace-write) or ReadOnly 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 { .. } + | codex_core::protocol::SandboxPolicy::ReadOnly ) && !app .config @@ -192,7 +193,7 @@ 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, false); + Self::spawn_world_writable_scan(cwd, env_map, logs_base_dir, tx); } } @@ -386,9 +387,18 @@ 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::OpenWorldWritableWarningConfirmation { + preset, + sample_paths, + extra_count, + failed_scan, + } => { + self.chat_widget.open_world_writable_warning_confirmation( + preset, + sample_paths, + extra_count, + failed_scan, + ); } AppEvent::OpenFeedbackNote { category, @@ -449,14 +459,15 @@ impl App { } AppEvent::UpdateSandboxPolicy(policy) => { #[cfg(target_os = "windows")] - let policy_is_workspace_write = matches!( + let policy_is_workspace_write_or_ro = matches!( policy, codex_core::protocol::SandboxPolicy::WorkspaceWrite { .. } + | codex_core::protocol::SandboxPolicy::ReadOnly ); self.chat_widget.set_sandbox_policy(policy); - // If sandbox policy becomes workspace-write, run the Windows world-writable scan. + // If sandbox policy becomes workspace-write or read-only, run the Windows world-writable scan. #[cfg(target_os = "windows")] { // One-shot suppression if the user just confirmed continue. @@ -466,7 +477,7 @@ impl App { } let should_check = codex_core::get_platform_sandbox().is_some() - && policy_is_workspace_write + && policy_is_workspace_write_or_ro && !self.chat_widget.world_writable_warning_hidden(); if should_check { let cwd = self.config.cwd.clone(); @@ -474,7 +485,7 @@ 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, false); + Self::spawn_world_writable_scan(cwd, env_map, logs_base_dir, tx); } } } @@ -628,28 +639,48 @@ impl App { env_map: std::collections::HashMap, logs_base_dir: PathBuf, tx: AppEventSender, - apply_preset_on_continue: bool, ) { + #[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 || { - if codex_windows_sandbox::preflight_audit_everyone_writable( + let result = codex_windows_sandbox::preflight_audit_everyone_writable( &cwd, &env_map, Some(logs_base_dir.as_path()), - ) - .is_err() + ); + if let Ok(ref paths) = result + && !paths.is_empty() { - 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), - }); - } + 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 { - tx.send(AppEvent::OpenWorldWritableWarningConfirmation { preset: None }); - } + 0 + }; + + 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, + failed_scan: true, + }); } }); } diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index f14951a9..7068cb79 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -79,6 +79,12 @@ pub(crate) enum AppEvent { #[cfg_attr(not(target_os = "windows"), allow(dead_code))] OpenWorldWritableWarningConfirmation { preset: Option, + /// Up to 3 sample world-writable directories to display in the warning. + sample_paths: Vec, + /// If there are more than `sample_paths`, this carries the remaining count. + extra_count: usize, + /// True when the scan failed (e.g. ACL query error) and protections could not be verified. + failed_scan: bool, }, /// Show Windows Subsystem for Linux setup instructions for auto mode. diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 0e0ff4c5..2a5eb276 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2045,9 +2045,48 @@ impl ChatWidget { && self.windows_world_writable_flagged() { let preset_clone = preset.clone(); + // Compute sample paths for the warning popup. + let mut env_map: std::collections::HashMap = + std::collections::HashMap::new(); + for (k, v) in std::env::vars() { + env_map.insert(k, v); + } + let (sample_paths, extra_count, failed_scan) = + match codex_windows_sandbox::preflight_audit_everyone_writable( + &self.config.cwd, + &env_map, + Some(self.config.codex_home.as_path()), + ) { + Ok(paths) if !paths.is_empty() => { + 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('/', "\\") + } + let as_strings: Vec = paths + .iter() + .map(|p| normalize_windows_path_for_display(p)) + .collect(); + let samples: Vec = + as_strings.iter().take(3).cloned().collect(); + let extra = if as_strings.len() > samples.len() { + as_strings.len() - samples.len() + } else { + 0 + }; + (samples, extra, false) + } + Err(_) => (Vec::new(), 0, true), + _ => (Vec::new(), 0, false), + }; vec![Box::new(move |tx| { tx.send(AppEvent::OpenWorldWritableWarningConfirmation { preset: Some(preset_clone.clone()), + sample_paths: sample_paths.clone(), + extra_count, + failed_scan, }); })] } else { @@ -2111,7 +2150,7 @@ impl ChatWidget { &env_map, Some(self.config.codex_home.as_path()), ) { - Ok(()) => false, + Ok(paths) => !paths.is_empty(), Err(_) => true, } } @@ -2184,31 +2223,66 @@ impl ChatWidget { pub(crate) fn open_world_writable_warning_confirmation( &mut self, preset: Option, + sample_paths: Vec, + extra_count: usize, + failed_scan: bool, ) { 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." + let mode_label = match self.config.sandbox_policy { + SandboxPolicy::WorkspaceWrite { .. } => "Auto mode", + SandboxPolicy::ReadOnly => "Read-Only mode", + _ => "Auto mode", + }; + let title_line = Line::from("Unprotected directories found").bold(); + let info_line = if failed_scan { + Line::from(vec![ + "We couldn't complete the world-writable scan, so protections cannot be verified. " + .into(), + format!("The Windows sandbox cannot guarantee protection in {mode_label}.") + .fg(Color::Red), + ]) + } else { + Line::from(vec![ + "Some important directories on this system are world-writable. ".into(), + format!( + "The Windows sandbox cannot protect writes to these locations in {mode_label}." + ) .fg(Color::Red), - ]); + ]) + }; header_children.push(Box::new(title_line)); header_children.push(Box::new( Paragraph::new(vec![info_line]).wrap(Wrap { trim: false }), )); + + if !sample_paths.is_empty() { + // Show up to three examples and optionally an "and X more" line. + let mut lines: Vec = Vec::new(); + lines.push(Line::from("Examples:").bold()); + for p in &sample_paths { + lines.push(Line::from(format!(" - {p}"))); + } + if extra_count > 0 { + lines.push(Line::from(format!("and {extra_count} more"))); + } + header_children.push(Box::new(Paragraph::new(lines).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); - })); + // Suppress the immediate re-scan only when a preset will be applied (i.e., via /approvals), + // to avoid duplicate warnings from the ensuing policy change. + if preset.is_some() { + 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)); } @@ -2222,36 +2296,21 @@ impl ChatWidget { 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()), + description: Some(format!("Apply {mode_label} for this session")), 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()), + description: Some(format!("Enable {mode_label} and remember this choice")), 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 { @@ -2266,6 +2325,9 @@ impl ChatWidget { pub(crate) fn open_world_writable_warning_confirmation( &mut self, _preset: Option, + _sample_paths: Vec, + _extra_count: usize, + _failed_scan: bool, ) { } diff --git a/codex-rs/windows-sandbox-rs/src/audit.rs b/codex-rs/windows-sandbox-rs/src/audit.rs index b0a55da0..4cd19dea 100644 --- a/codex-rs/windows-sandbox-rs/src/audit.rs +++ b/codex-rs/windows-sandbox-rs/src/audit.rs @@ -1,6 +1,5 @@ use crate::token::world_sid; use crate::winutil::to_wide; -use anyhow::anyhow; use anyhow::Result; use std::collections::HashSet; use std::ffi::c_void; @@ -177,7 +176,7 @@ pub fn audit_everyone_writable( cwd: &Path, env: &std::collections::HashMap, logs_base_dir: Option<&Path>, -) -> Result<()> { +) -> Result> { let start = Instant::now(); let mut flagged: Vec = Vec::new(); let mut seen: HashSet = HashSet::new(); @@ -265,14 +264,7 @@ pub fn audit_everyone_writable( ), logs_base_dir, ); - 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_err - )); + return Ok(flagged); } // Log success once if nothing flagged crate::logging::log_note( @@ -281,7 +273,7 @@ pub fn audit_everyone_writable( ), logs_base_dir, ); - Ok(()) + Ok(Vec::new()) } // Fast mask-based check: does the DACL contain any ACCESS_ALLOWED ACE for // Everyone that includes generic or specific write bits? Skips inherit-only diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 561ae2c3..955f2ca3 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -172,7 +172,7 @@ mod windows_impl { cwd: &Path, env_map: &HashMap, logs_base_dir: Option<&Path>, - ) -> Result<()> { + ) -> Result> { audit::audit_everyone_writable(cwd, env_map, logs_base_dir) } @@ -438,7 +438,7 @@ mod stub { _cwd: &Path, _env_map: &HashMap, _logs_base_dir: Option<&Path>, - ) -> Result<()> { + ) -> Result> { bail!("Windows sandbox is only available on Windows") }