diff --git a/codex-rs/windows-sandbox-rs/src/helper_materialization.rs b/codex-rs/windows-sandbox-rs/src/helper_materialization.rs index 375199f5cc..225618c3c3 100644 --- a/codex-rs/windows-sandbox-rs/src/helper_materialization.rs +++ b/codex-rs/windows-sandbox-rs/src/helper_materialization.rs @@ -3,6 +3,7 @@ use anyhow::Context; use anyhow::Result; use std::collections::HashMap; use std::fs; +use std::io::Read; use std::io::Write; use std::path::Path; use std::path::PathBuf; @@ -16,18 +17,21 @@ use crate::sandbox_bin_dir; #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub(crate) enum HelperExecutable { CommandRunner, + Setup, } impl HelperExecutable { fn file_name(self) -> &'static str { match self { Self::CommandRunner => "codex-command-runner.exe", + Self::Setup => "codex-windows-sandbox-setup.exe", } } fn label(self) -> &'static str { match self { Self::CommandRunner => "command-runner", + Self::Setup => "setup", } } } @@ -49,13 +53,22 @@ pub(crate) fn legacy_lookup(kind: HelperExecutable) -> PathBuf { && let Some(dir) = exe.parent() { let candidate = dir.join(kind.file_name()); - if candidate.exists() { + if !is_windowsapps_path(dir) && candidate.exists() { return candidate; } } PathBuf::from(kind.file_name()) } +pub(crate) fn is_windowsapps_path(path: &Path) -> bool { + path.components().any(|component| { + component + .as_os_str() + .to_string_lossy() + .eq_ignore_ascii_case("WindowsApps") + }) +} + pub(crate) fn resolve_helper_for_launch( kind: HelperExecutable, codex_home: &Path, @@ -286,12 +299,50 @@ fn destination_is_fresh(source: &Path, destination: &Path) -> Result { let destination_modified = destination_meta .modified() .with_context(|| format!("read helper destination mtime {}", destination.display()))?; + if destination_modified < source_modified { + return Ok(false); + } - Ok(destination_modified >= source_modified) + files_match(source, destination) +} + +fn files_match(source: &Path, destination: &Path) -> Result { + let mut source_file = fs::File::open(source) + .with_context(|| format!("open helper source for comparison {}", source.display()))?; + let mut destination_file = fs::File::open(destination).with_context(|| { + format!( + "open helper destination for comparison {}", + destination.display() + ) + })?; + + let mut source_buf = [0_u8; 8192]; + let mut destination_buf = [0_u8; 8192]; + loop { + let source_read = source_file + .read(&mut source_buf) + .with_context(|| format!("read helper source for comparison {}", source.display()))?; + let destination_read = destination_file.read(&mut destination_buf).with_context(|| { + format!( + "read helper destination for comparison {}", + destination.display() + ) + })?; + if source_read != destination_read { + return Ok(false); + } + if source_read == 0 { + return Ok(true); + } + if source_buf[..source_read] != destination_buf[..destination_read] { + return Ok(false); + } + } } #[cfg(test)] mod tests { + use super::is_windowsapps_path; use super::destination_is_fresh; use super::helper_bin_dir; use super::copy_from_source_if_needed; @@ -317,17 +368,18 @@ mod tests { } #[test] - fn destination_is_fresh_uses_size_and_mtime() { + fn destination_is_fresh_requires_matching_contents() { let tmp = TempDir::new().expect("tempdir"); let source = tmp.path().join("source.exe"); let destination = tmp.path().join("destination.exe"); - fs::write(&destination, b"same-size").expect("write destination"); + fs::write(&source, b"runner-v1").expect("write source"); std::thread::sleep(std::time::Duration::from_secs(1)); - fs::write(&source, b"same-size").expect("write source"); - assert!(!destination_is_fresh(&source, &destination).expect("stale metadata")); + fs::write(&destination, b"stale--v1").expect("write destination"); - fs::write(&destination, b"same-size").expect("rewrite destination"); + assert!(!destination_is_fresh(&source, &destination).expect("freshness check")); + + fs::write(&destination, b"runner-v1").expect("rewrite destination"); assert!(destination_is_fresh(&source, &destination).expect("fresh metadata")); } @@ -365,15 +417,33 @@ mod tests { fs::create_dir_all(&source_dir).expect("create source dir"); let runner_source = source_dir.join("codex-command-runner.exe"); let runner_destination = helper_bin_dir(&codex_home).join("codex-command-runner.exe"); + let setup_source = source_dir.join("codex-windows-sandbox-setup.exe"); + let setup_destination = + helper_bin_dir(&codex_home).join("codex-windows-sandbox-setup.exe"); fs::write(&runner_source, b"runner").expect("runner"); + fs::write(&setup_source, b"setup").expect("setup"); let runner_outcome = copy_from_source_if_needed(&runner_source, &runner_destination).expect("runner copy"); + let setup_outcome = + copy_from_source_if_needed(&setup_source, &setup_destination).expect("setup copy"); assert_eq!(CopyOutcome::ReCopied, runner_outcome); + assert_eq!(CopyOutcome::ReCopied, setup_outcome); assert_eq!( b"runner".as_slice(), fs::read(&runner_destination).expect("read runner") ); + assert_eq!(b"setup".as_slice(), fs::read(&setup_destination).expect("read setup")); + } + + #[test] + fn windowsapps_detection_matches_any_path_segment() { + assert!(is_windowsapps_path(Path::new( + r"C:\Program Files\WindowsApps\OpenAI.Codex_1.0.0.0_x64__abc" + ))); + assert!(!is_windowsapps_path(Path::new( + r"C:\Users\example\AppData\Local\Programs\OpenAI" + ))); } } diff --git a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs index f6583e0de4..483511c64b 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -12,7 +12,10 @@ use std::process::Stdio; use crate::allow::AllowDenyPaths; use crate::allow::compute_allow_paths; +use crate::helper_materialization::HelperExecutable; use crate::helper_materialization::helper_bin_dir; +use crate::helper_materialization::is_windowsapps_path; +use crate::helper_materialization::resolve_helper_for_launch; use crate::logging::log_note; use crate::path_normalization::canonical_path_key; use crate::policy::SandboxPolicy; @@ -172,7 +175,7 @@ fn run_setup_refresh_inner( }; let json = serde_json::to_vec(&payload)?; let b64 = BASE64_STANDARD.encode(json); - let exe = find_setup_exe(); + let exe = find_setup_exe(request.codex_home); // Refresh should never request elevation; ensure verb isn't set and we don't trigger UAC. let mut cmd = Command::new(&exe); cmd.arg(&b64).stdout(Stdio::null()).stderr(Stdio::null()); @@ -330,12 +333,17 @@ fn profile_read_roots(user_profile: &Path) -> Vec { .collect() } +fn current_exe_parent_read_root(current_exe: &Path) -> Option { + let dir = current_exe.parent()?; + (!is_windowsapps_path(dir)).then(|| dir.to_path_buf()) +} + fn gather_helper_read_roots(codex_home: &Path) -> Vec { let mut roots = Vec::new(); if let Ok(exe) = std::env::current_exe() - && let Some(dir) = exe.parent() + && let Some(dir) = current_exe_parent_read_root(&exe) { - roots.push(dir.to_path_buf()); + roots.push(dir); } let helper_dir = helper_bin_dir(codex_home); let _ = std::fs::create_dir_all(&helper_dir); @@ -569,16 +577,12 @@ fn quote_arg(arg: &str) -> String { out } -fn find_setup_exe() -> PathBuf { - if let Ok(exe) = std::env::current_exe() - && let Some(dir) = exe.parent() - { - let candidate = dir.join("codex-windows-sandbox-setup.exe"); - if candidate.exists() { - return candidate; - } - } - PathBuf::from("codex-windows-sandbox-setup.exe") +fn find_setup_exe(codex_home: &Path) -> PathBuf { + resolve_helper_for_launch( + HelperExecutable::Setup, + codex_home, + Some(&sandbox_dir(codex_home)), + ) } fn report_helper_failure( @@ -611,7 +615,7 @@ fn run_setup_exe( use windows_sys::Win32::UI::Shell::SEE_MASK_NOCLOSEPROCESS; use windows_sys::Win32::UI::Shell::SHELLEXECUTEINFOW; use windows_sys::Win32::UI::Shell::ShellExecuteExW; - let exe = find_setup_exe(); + let exe = find_setup_exe(codex_home); let payload_json = serde_json::to_string(payload).map_err(|err| { failure( SetupErrorCode::OrchestratorPayloadSerializeFailed, @@ -802,6 +806,7 @@ fn filter_sensitive_write_roots(mut roots: Vec, codex_home: &Path) -> V #[cfg(test)] mod tests { use super::WINDOWS_PLATFORM_DEFAULT_READ_ROOTS; + use super::current_exe_parent_read_root; use super::gather_legacy_full_read_roots; use super::gather_read_roots; use super::loopback_proxy_port_from_url; @@ -816,6 +821,7 @@ mod tests { use std::collections::HashMap; use std::collections::HashSet; use std::fs; + use std::path::Path; use std::path::PathBuf; use tempfile::TempDir; @@ -1009,6 +1015,24 @@ mod tests { assert!(roots.contains(&expected)); } + #[test] + fn helper_read_roots_skip_windowsapps_parent_dir() { + assert_eq!( + None, + current_exe_parent_read_root(Path::new( + r"C:\Program Files\WindowsApps\OpenAI.Codex_1.0.0.0_x64__abc\codex.exe" + )) + ); + assert_eq!( + Some(PathBuf::from( + r"C:\Users\example\AppData\Local\Programs\OpenAI" + )), + current_exe_parent_read_root(Path::new( + r"C:\Users\example\AppData\Local\Programs\OpenAI\codex.exe" + )) + ); + } + #[test] fn restricted_read_roots_skip_platform_defaults_when_disabled() { let tmp = TempDir::new().expect("tempdir");