Compare commits

...

1 Commits

Author SHA1 Message Date
David Wiesen
bd8dbac27d fix(windows-sandbox): materialize setup helper 2026-04-06 09:26:47 -07:00
2 changed files with 115 additions and 21 deletions

View File

@@ -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<bool> {
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<bool> {
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"
)));
}
}

View File

@@ -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<PathBuf> {
.collect()
}
fn current_exe_parent_read_root(current_exe: &Path) -> Option<PathBuf> {
let dir = current_exe.parent()?;
(!is_windowsapps_path(dir)).then(|| dir.to_path_buf())
}
fn gather_helper_read_roots(codex_home: &Path) -> Vec<PathBuf> {
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<PathBuf>, 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");