fix: materialize windows sandbox setup helper

codex-rs/windows-sandbox-rs/src/helper_materialization.rs

@@ -16,18 +16,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",
         }
     }
 }
@@ -45,9 +48,19 @@ pub(crate) fn helper_bin_dir(codex_home: &Path) -> PathBuf {
 }
 
 pub(crate) fn legacy_lookup(kind: HelperExecutable) -> PathBuf {
-    if let Ok(exe) = std::env::current_exe()
-        && let Some(dir) = exe.parent()
+    let current_exe = std::env::current_exe().ok();
+    legacy_lookup_with_current_exe(kind, current_exe.as_deref())
+}
+
+fn legacy_lookup_with_current_exe(kind: HelperExecutable, current_exe: Option<&Path>) -> PathBuf {
+    if let Some(exe_path) = current_exe
+        && let Some(dir) = exe_path.parent()
     {
+        // WindowsApps package directories can be non-executable for spawned helpers.
+        // Prefer PATH-based fallback over sibling execution in that case.
+        if is_windowsapps_package_dir(dir) {
+            return PathBuf::from(kind.file_name());
+        }
         let candidate = dir.join(kind.file_name());
         if candidate.exists() {
             return candidate;
@@ -56,6 +69,11 @@ pub(crate) fn legacy_lookup(kind: HelperExecutable) -> PathBuf {
     PathBuf::from(kind.file_name())
 }
 
+pub(crate) fn is_windowsapps_package_dir(path: &Path) -> bool {
+    let lower = path.to_string_lossy().to_ascii_lowercase();
+    lower.contains(r"\program files\windowsapps\")
+}
+
 pub(crate) fn resolve_helper_for_launch(
     kind: HelperExecutable,
     codex_home: &Path,
@@ -75,11 +93,24 @@ pub(crate) fn resolve_helper_for_launch(
         }
         Err(err) => {
             let fallback = legacy_lookup(kind);
+            let current_exe = std::env::current_exe().ok();
+            let fallback_reason = if current_exe
+                .as_deref()
+                .and_then(Path::parent)
+                .is_some_and(is_windowsapps_package_dir)
+            {
+                format!(
+                    "falling back to PATH lookup {} (skipping WindowsApps sibling fallback)",
+                    fallback.display()
+                )
+            } else {
+                format!("falling back to legacy path {}", fallback.display())
+            };
             log_note(
                 &format!(
-                    "helper copy failed for {}: {err:#}; falling back to legacy path {}",
+                    "helper copy failed for {}: {err:#}; {}",
                     kind.label(),
-                    fallback.display()
+                    fallback_reason
                 ),
                 log_dir,
             );
@@ -287,14 +318,51 @@ fn destination_is_fresh(source: &Path, destination: &Path) -> Result<bool> {
         .modified()
         .with_context(|| format!("read helper destination mtime {}", destination.display()))?;
 
-    Ok(destination_modified >= source_modified)
+    if destination_modified < source_modified {
+        return Ok(false);
+    }
+
+    source_and_destination_match(source, destination)
+}
+
+fn source_and_destination_match(source: &Path, destination: &Path) -> Result<bool> {
+    let mut source_file = fs::File::open(source)
+        .with_context(|| format!("open helper source for compare {}", source.display()))?;
+    let mut destination_file = fs::File::open(destination)
+        .with_context(|| format!("open helper destination for compare {}", destination.display()))?;
+    let mut source_buffer = [0_u8; 8192];
+    let mut destination_buffer = [0_u8; 8192];
+
+    loop {
+        let source_len = std::io::Read::read(&mut source_file, &mut source_buffer)
+            .with_context(|| format!("read helper source bytes for compare {}", source.display()))?;
+        let destination_len = std::io::Read::read(&mut destination_file, &mut destination_buffer)
+            .with_context(|| {
+                format!(
+                    "read helper destination bytes for compare {}",
+                    destination.display()
+                )
+            })?;
+
+        if source_len != destination_len {
+            return Ok(false);
+        }
+        if source_len == 0 {
+            return Ok(true);
+        }
+        if source_buffer[..source_len] != destination_buffer[..destination_len] {
+            return Ok(false);
+        }
+    }
 }
 
 #[cfg(test)]
 mod tests {
+    use super::HelperExecutable;
+    use super::legacy_lookup_with_current_exe;
     use super::destination_is_fresh;
-    use super::helper_bin_dir;
     use super::copy_from_source_if_needed;
+    use super::helper_bin_dir;
     use super::CopyOutcome;
     use pretty_assertions::assert_eq;
     use std::fs;
@@ -317,18 +385,40 @@ mod tests {
     }
 
     #[test]
-    fn destination_is_fresh_uses_size_and_mtime() {
+    fn destination_is_fresh_detects_content_drift_with_same_size() {
         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");
-        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"));
+        std::thread::sleep(std::time::Duration::from_secs(1));
+        fs::write(&destination, b"DIFF-size").expect("write destination");
 
-        fs::write(&destination, b"same-size").expect("rewrite destination");
-        assert!(destination_is_fresh(&source, &destination).expect("fresh metadata"));
+        assert!(!destination_is_fresh(&source, &destination).expect("compare drifted content"));
+    }
+
+    #[test]
+    fn destination_is_fresh_when_size_mtime_and_content_match() {
+        let tmp = TempDir::new().expect("tempdir");
+        let source = tmp.path().join("source.exe");
+        let destination = tmp.path().join("destination.exe");
+
+        fs::write(&source, b"same-size").expect("write source");
+        std::thread::sleep(std::time::Duration::from_secs(1));
+        fs::write(&destination, b"same-size").expect("write destination");
+
+        assert!(destination_is_fresh(&source, &destination).expect("fresh matching content"));
+    }
+
+    #[test]
+    fn legacy_lookup_skips_windowsapps_sibling_candidates() {
+        let current_exe =
+            Path::new(r"C:\Program Files\WindowsApps\OpenAI.Codex_26.0.0_x64__2p2nqsd0c76g0\codex.exe");
+
+        let fallback =
+            legacy_lookup_with_current_exe(HelperExecutable::CommandRunner, Some(current_exe));
+
+        assert_eq!(PathBuf::from("codex-command-runner.exe"), fallback);
     }
 
     #[test]

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::copy_helper_if_needed;
 use crate::helper_materialization::helper_bin_dir;
+use crate::helper_materialization::is_windowsapps_package_dir;
 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());
@@ -331,9 +334,18 @@ fn profile_read_roots(user_profile: &Path) -> Vec<PathBuf> {
 }
 
 fn gather_helper_read_roots(codex_home: &Path) -> Vec<PathBuf> {
+    let current_exe = std::env::current_exe().ok();
+    gather_helper_read_roots_with_current_exe(codex_home, current_exe.as_deref())
+}
+
+fn gather_helper_read_roots_with_current_exe(
+    codex_home: &Path,
+    current_exe: Option<&Path>,
+) -> Vec<PathBuf> {
     let mut roots = Vec::new();
-    if let Ok(exe) = std::env::current_exe()
+    if let Some(exe) = current_exe
         && let Some(dir) = exe.parent()
+        && !is_windowsapps_package_dir(dir)
     {
         roots.push(dir.to_path_buf());
     }
@@ -569,16 +581,17 @@ 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) -> Result<PathBuf> {
+    let log_dir = sandbox_dir(codex_home);
+    copy_helper_if_needed(HelperExecutable::Setup, codex_home, Some(&log_dir)).map_err(|err| {
+        failure(
+            SetupErrorCode::OrchestratorHelperLaunchFailed,
+            format!(
+                "failed to resolve setup helper from materialized copy in {}: {err:#}",
+                helper_bin_dir(codex_home).display()
+            ),
+        )
+    })
 }
 
 fn report_helper_failure(
@@ -611,7 +624,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 +815,7 @@ fn filter_sensitive_write_roots(mut roots: Vec<PathBuf>, codex_home: &Path) -> V
 #[cfg(test)]
 mod tests {
     use super::WINDOWS_PLATFORM_DEFAULT_READ_ROOTS;
+    use super::gather_helper_read_roots_with_current_exe;
     use super::gather_legacy_full_read_roots;
     use super::gather_read_roots;
     use super::loopback_proxy_port_from_url;
@@ -1009,6 +1023,34 @@ mod tests {
         assert!(roots.contains(&expected));
     }
 
+    #[test]
+    fn helper_read_roots_skip_windowsapps_executable_directory() {
+        let tmp = TempDir::new().expect("tempdir");
+        let codex_home = tmp.path().join("codex-home");
+        let current_exe = PathBuf::from(
+            r"C:\Program Files\WindowsApps\OpenAI.Codex_26.0.0_x64__2p2nqsd0c76g0\codex.exe",
+        );
+
+        let roots = gather_helper_read_roots_with_current_exe(&codex_home, Some(&current_exe));
+        let windowsapps_parent = current_exe.parent().expect("current exe parent");
+        let expected_helper_dir = helper_bin_dir(&codex_home);
+
+        assert!(!roots.iter().any(|root| root == windowsapps_parent));
+        assert!(roots.iter().any(|root| root == &expected_helper_dir));
+    }
+
+    #[test]
+    fn helper_read_roots_include_non_windowsapps_executable_directory() {
+        let tmp = TempDir::new().expect("tempdir");
+        let codex_home = tmp.path().join("codex-home");
+        let current_exe = PathBuf::from(r"C:\Users\example\AppData\Local\OpenAI\Codex\codex.exe");
+
+        let roots = gather_helper_read_roots_with_current_exe(&codex_home, Some(&current_exe));
+        let expected_exe_parent = current_exe.parent().expect("current exe parent");
+
+        assert!(roots.iter().any(|root| root == expected_exe_parent));
+    }
+
     #[test]
     fn restricted_read_roots_skip_platform_defaults_when_disabled() {
         let tmp = TempDir::new().expect("tempdir");