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.
This commit is contained in:
iceweasel-oai 2025-11-24 09:51:58 -08:00 committed by GitHub
parent b2cddec3d7
commit 486b1c4d9d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 53 additions and 112 deletions

View file

@ -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<String> = paths
.iter()
.map(|p| normalize_windows_path_for_display(p))
.collect();
let sample_paths: Vec<String> = 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<String> = 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,
});
}

View file

@ -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<String, String> = 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"))]

View file

@ -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<String, String>,
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<Path>) -> 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<Path>,
cwd: impl AsRef<Path>,
) -> Option<(Vec<String>, usize, bool)> {
let env_map: HashMap<String, String> = 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<String> = paths
.iter()
.map(normalize_windows_path_for_display)
.collect();
let sample_paths: Vec<String> = 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).

View file

@ -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<String, String>,
sandbox_policy: &SandboxPolicy,
logs_base_dir: Option<&Path>,
) -> Result<Vec<PathBuf>> {
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<String, String>,
_sandbox_policy: &SandboxPolicy,
_logs_base_dir: Option<&Path>,
) -> Result<Vec<std::path::PathBuf>> {
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<Path>,
_cwd: impl AsRef<Path>,
) -> Option<(Vec<String>, usize, bool)> {
None
pub fn apply_world_writable_scan_and_denies(
_codex_home: &Path,
_cwd: &Path,
_env_map: &HashMap<String, String>,
_sandbox_policy: &SandboxPolicy,
_logs_base_dir: Option<&Path>,
) -> Result<()> {
bail!("Windows sandbox is only available on Windows")
}
}