openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-20 12:47:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(windows-sandbox): materialize setup helper

Browse Source
This commit is contained in:
David Wiesen
2026-04-06 09:18:23 -07:00
parent 95845cf6ce
commit 052007b421
2 changed files with 246 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

189
codex-rs/windows-sandbox-rs/src/helper_materialization.rs
View File

@@ -1,6 +1,6 @@
use anyhow::anyhow;
use anyhow::Context;
use anyhow::Result;
use anyhow::anyhow;
use std::collections::HashMap;
use std::fs;
use std::io::Write;
@@ -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-helper",
}
}
}
@@ -44,18 +47,35 @@ pub(crate) fn helper_bin_dir(codex_home: &Path) -> PathBuf {
sandbox_bin_dir(codex_home)
}
pub(crate) fn path_is_under_windows_apps(path: &Path) -> bool {
path.components().any(|component| {
component
.as_os_str()
.to_string_lossy()
.eq_ignore_ascii_case("WindowsApps")
})
}
pub(crate) fn legacy_lookup(kind: HelperExecutable) -> PathBuf {
if let Ok(exe) = std::env::current_exe()
&& let Some(dir) = exe.parent()
{
let candidate = dir.join(kind.file_name());
if candidate.exists() {
return candidate;
}
if let Some(candidate) = legacy_lookup_from_current_exe(std::env::current_exe().ok(), kind) {
return candidate;
}
PathBuf::from(kind.file_name())
}
fn legacy_lookup_from_current_exe(
current_exe: Option<PathBuf>,
kind: HelperExecutable,
) -> Option<PathBuf> {
let exe = current_exe?;
let dir = exe.parent()?;
if path_is_under_windows_apps(dir) {
return None;
}
let candidate = dir.join(kind.file_name());
candidate.exists().then_some(candidate)
}
pub(crate) fn resolve_helper_for_launch(
kind: HelperExecutable,
codex_home: &Path,
@@ -88,10 +108,7 @@ pub(crate) fn resolve_helper_for_launch(
}
}
pub fn resolve_current_exe_for_launch(
codex_home: &Path,
fallback_executable: &str,
) -> PathBuf {
pub fn resolve_current_exe_for_launch(codex_home: &Path, fallback_executable: &str) -> PathBuf {
let source = match std::env::current_exe() {
Ok(path) => path,
Err(_) => return PathBuf::from(fallback_executable),
@@ -182,6 +199,12 @@ fn sibling_source_path(kind: HelperExecutable) -> Result<PathBuf> {
let dir = exe
.parent()
.ok_or_else(|| anyhow!("current executable has no parent directory"))?;
if path_is_under_windows_apps(dir) {
return Err(anyhow!(
"helper lookup next to current executable is disabled for WindowsApps installs: {}",
dir.display()
));
}
let candidate = dir.join(kind.file_name());
if candidate.exists() {
Ok(candidate)
@@ -198,9 +221,12 @@ fn copy_from_source_if_needed(source: &Path, destination: &Path) -> Result<CopyO
return Ok(CopyOutcome::Reused);
}
let destination_dir = destination
.parent()
.ok_or_else(|| anyhow!("helper destination has no parent: {}", destination.display()))?;
let destination_dir = destination.parent().ok_or_else(|| {
anyhow!(
"helper destination has no parent: {}",
destination.display()
)
})?;
fs::create_dir_all(destination_dir).with_context(|| {
format!(
"create helper destination directory {}",
@@ -271,8 +297,9 @@ fn destination_is_fresh(source: &Path, destination: &Path) -> Result<bool> {
Ok(meta) => meta,
Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(false),
Err(err) => {
return Err(err)
.with_context(|| format!("read helper destination metadata {}", destination.display()));
return Err(err).with_context(|| {
format!("read helper destination metadata {}", destination.display())
});
}
};
@@ -287,19 +314,41 @@ 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);
}
if source_files_match(source, destination)? {
return Ok(true);
}
Ok(false)
}
fn source_files_match(source: &Path, destination: &Path) -> Result<bool> {
let source_bytes = fs::read(source)
.with_context(|| format!("read helper source bytes {}", source.display()))?;
let destination_bytes = fs::read(destination)
.with_context(|| format!("read helper destination bytes {}", destination.display()))?;
Ok(source_bytes == destination_bytes)
}
#[cfg(test)]
mod tests {
use super::CopyOutcome;
use super::HelperExecutable;
use super::copy_from_source_if_needed;
use super::destination_is_fresh;
use super::helper_bin_dir;
use super::copy_from_source_if_needed;
use super::CopyOutcome;
use super::legacy_lookup_from_current_exe;
use super::path_is_under_windows_apps;
use pretty_assertions::assert_eq;
use std::fs;
use std::fs::File;
use std::fs::FileTimes;
use std::path::Path;
use std::path::PathBuf;
use std::time::SystemTime;
use tempfile::TempDir;
#[test]
@@ -313,24 +362,50 @@ mod tests {
let outcome = copy_from_source_if_needed(&source, &destination).expect("copy helper");
assert_eq!(CopyOutcome::ReCopied, outcome);
assert_eq!(b"runner-v1".as_slice(), fs::read(&destination).expect("read destination"));
assert_eq!(
b"runner-v1".as_slice(),
fs::read(&destination).expect("read destination")
);
}
#[test]
fn destination_is_fresh_uses_size_and_mtime() {
fn destination_is_fresh_requires_matching_contents_when_metadata_matches() {
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(&destination, b"dest-v1!!").expect("write destination");
std::thread::sleep(std::time::Duration::from_secs(1));
fs::write(&source, b"same-size").expect("write source");
fs::write(&source, b"source-v1").expect("write source");
assert!(!destination_is_fresh(&source, &destination).expect("stale metadata"));
fs::write(&destination, b"same-size").expect("rewrite destination");
fs::write(&destination, b"source-v1").expect("rewrite destination");
assert!(destination_is_fresh(&source, &destination).expect("fresh metadata"));
}
#[test]
fn destination_is_fresh_rejects_same_size_same_mtime_content_drift() {
let tmp = TempDir::new().expect("tempdir");
let source = tmp.path().join("source.exe");
let destination = tmp.path().join("destination.exe");
fs::write(&source, b"runner-v1").expect("write source");
fs::write(&destination, b"runner-v2").expect("write destination");
let modified = fs::metadata(&source)
.expect("source metadata")
.modified()
.unwrap_or_else(|_| SystemTime::now());
File::options()
.write(true)
.open(&destination)
.expect("open destination")
.set_times(FileTimes::new().set_modified(modified))
.expect("align destination mtime");
assert!(!destination_is_fresh(&source, &destination).expect("detect drift"));
}
#[test]
fn copy_from_source_if_needed_reuses_fresh_destination() {
let tmp = TempDir::new().expect("tempdir");
@@ -340,11 +415,13 @@ mod tests {
fs::write(&source, b"runner-v1").expect("write source");
copy_from_source_if_needed(&source, &destination).expect("initial copy");
let outcome =
copy_from_source_if_needed(&source, &destination).expect("revalidate helper");
let outcome = copy_from_source_if_needed(&source, &destination).expect("revalidate helper");
assert_eq!(CopyOutcome::Reused, outcome);
assert_eq!(b"runner-v1".as_slice(), fs::read(&destination).expect("read destination"));
assert_eq!(
b"runner-v1".as_slice(),
fs::read(&destination).expect("read destination")
);
}
#[test]
@@ -376,4 +453,60 @@ mod tests {
fs::read(&runner_destination).expect("read runner")
);
}
#[test]
fn copy_setup_into_shared_bin_dir() {
let tmp = TempDir::new().expect("tempdir");
let codex_home = tmp.path().join("codex-home");
let source_dir = tmp.path().join("sibling-source");
fs::create_dir_all(&source_dir).expect("create source dir");
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(&setup_source, b"setup").expect("setup");
let setup_outcome =
copy_from_source_if_needed(&setup_source, &setup_destination).expect("setup copy");
assert_eq!(CopyOutcome::ReCopied, setup_outcome);
assert_eq!(
b"setup".as_slice(),
fs::read(&setup_destination).expect("read setup")
);
}
#[test]
fn path_is_under_windows_apps_matches_component() {
assert!(path_is_under_windows_apps(Path::new(
r"C:\Program Files\WindowsApps\OpenAI.Codex\codex.exe"
)));
assert!(!path_is_under_windows_apps(Path::new(
r"C:\Program Files\OpenAI\Codex\codex.exe"
)));
}
#[test]
fn legacy_lookup_skips_windows_apps_siblings() {
let tmp = TempDir::new().expect("tempdir");
let normal_dir = tmp.path().join("OpenAI");
fs::create_dir_all(&normal_dir).expect("create normal dir");
let normal_helper = normal_dir.join("codex-command-runner.exe");
fs::write(&normal_helper, b"runner").expect("write helper");
let lookup = legacy_lookup_from_current_exe(
Some(normal_dir.join("codex.exe")),
HelperExecutable::CommandRunner,
);
assert_eq!(Some(normal_helper), lookup);
let windows_apps_dir = tmp.path().join("WindowsApps").join("OpenAI.Codex");
fs::create_dir_all(&windows_apps_dir).expect("create WindowsApps dir");
let windows_apps_helper = windows_apps_dir.join("codex-command-runner.exe");
fs::write(&windows_apps_helper, b"runner").expect("write helper in WindowsApps");
let lookup = legacy_lookup_from_current_exe(
Some(windows_apps_dir.join("codex.exe")),
HelperExecutable::CommandRunner,
);
assert_eq!(None, lookup);
}
}

99
codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs
View File

@@ -12,7 +12,9 @@ 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::resolve_helper_for_launch;
use crate::logging::log_note;
use crate::path_normalization::canonical_path_key;
use crate::policy::SandboxPolicy;
@@ -172,7 +174,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,10 +332,14 @@ fn profile_read_roots(user_profile: &Path) -> Vec<PathBuf> {
.collect()
}
fn gather_helper_read_roots(codex_home: &Path) -> Vec<PathBuf> {
fn gather_helper_read_roots_for_current_exe(
current_exe: Option<PathBuf>,
codex_home: &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_windows_apps_path(dir)
{
roots.push(dir.to_path_buf());
}
@@ -343,6 +349,10 @@ fn gather_helper_read_roots(codex_home: &Path) -> Vec<PathBuf> {
roots
}
fn gather_helper_read_roots(codex_home: &Path) -> Vec<PathBuf> {
gather_helper_read_roots_for_current_exe(std::env::current_exe().ok(), codex_home)
}
fn gather_legacy_full_read_roots(
command_cwd: &Path,
policy: &SandboxPolicy,
@@ -569,16 +579,18 @@ 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 {
let log_dir = sandbox_dir(codex_home);
resolve_helper_for_launch(HelperExecutable::Setup, codex_home, Some(&log_dir))
}
fn is_windows_apps_path(path: &Path) -> bool {
path.components().any(|component| {
component
.as_os_str()
.to_string_lossy()
.eq_ignore_ascii_case("WindowsApps")
})
}
fn report_helper_failure(
@@ -611,7 +623,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,8 +814,11 @@ 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;
use super::gather_helper_read_roots_for_current_exe;
use super::gather_legacy_full_read_roots;
use super::gather_read_roots;
use super::is_windows_apps_path;
use super::loopback_proxy_port_from_url;
use super::offline_proxy_settings_from_env;
use super::profile_read_roots;
@@ -816,6 +831,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 +1025,61 @@ mod tests {
assert!(roots.contains(&expected));
}
#[test]
fn helper_read_roots_always_include_materialized_helper_dir() {
let tmp = TempDir::new().expect("tempdir");
let codex_home = tmp.path().join("codex-home");
let roots = gather_helper_read_roots(&codex_home);
let expected =
dunce::canonicalize(helper_bin_dir(&codex_home)).expect("canonical helper dir");
assert!(roots.contains(&expected));
}
#[test]
fn helper_read_roots_skip_windows_apps_parent() {
let tmp = TempDir::new().expect("tempdir");
let codex_home = tmp.path().join("codex-home");
let exe =
PathBuf::from(r"C:\Program Files\WindowsApps\OpenAI.Codex_1.2.3.4_x64__abc\codex.exe");
let roots = gather_helper_read_roots_for_current_exe(Some(exe), &codex_home);
let expected =
dunce::canonicalize(helper_bin_dir(&codex_home)).expect("canonical helper dir");
assert_eq!(vec![expected], roots);
}
#[test]
fn helper_read_roots_include_non_windows_apps_parent() {
let tmp = TempDir::new().expect("tempdir");
let codex_home = tmp.path().join("codex-home");
let install_dir = tmp.path().join("OpenAI").join("Codex");
fs::create_dir_all(&install_dir).expect("create install dir");
let exe = install_dir.join("codex.exe");
let roots = gather_helper_read_roots_for_current_exe(Some(exe), &codex_home);
let install_dir = dunce::canonicalize(&install_dir).expect("canonical install dir");
let expected =
dunce::canonicalize(helper_bin_dir(&codex_home)).expect("canonical helper dir");
assert!(roots.contains(&install_dir));
assert!(roots.contains(&expected));
}
#[test]
fn windows_apps_paths_are_detected_case_insensitively() {
assert!(is_windows_apps_path(Path::new(
r"C:\Program Files\WindowsApps\OpenAI.Codex\codex.exe"
)));
assert!(is_windows_apps_path(Path::new(
r"c:\program files\windowsapps\OpenAI.Codex"
)));
assert!(!is_windows_apps_path(Path::new(
r"C:\Users\example\.codex\.sandbox-bin"
)));
}
#[test]
fn restricted_read_roots_skip_platform_defaults_when_disabled() {
let tmp = TempDir::new().expect("tempdir");