mirror of
https://github.com/openai/codex.git
synced 2026-09-09 15:58:47 +00:00
fix windows sandbox helper materialization fallback
This commit is contained in:
@@ -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",
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -48,7 +52,7 @@ 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());
|
||||
let candidate = legacy_lookup_candidate(kind, dir);
|
||||
if candidate.exists() {
|
||||
return candidate;
|
||||
}
|
||||
@@ -56,6 +60,13 @@ pub(crate) fn legacy_lookup(kind: HelperExecutable) -> PathBuf {
|
||||
PathBuf::from(kind.file_name())
|
||||
}
|
||||
|
||||
fn legacy_lookup_candidate(kind: HelperExecutable, exe_dir: &Path) -> PathBuf {
|
||||
if exe_dir_is_windows_apps(exe_dir) {
|
||||
return PathBuf::from(kind.file_name());
|
||||
}
|
||||
exe_dir.join(kind.file_name())
|
||||
}
|
||||
|
||||
pub(crate) fn resolve_helper_for_launch(
|
||||
kind: HelperExecutable,
|
||||
codex_home: &Path,
|
||||
@@ -287,14 +298,64 @@ 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);
|
||||
}
|
||||
|
||||
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 compare {}", source.display()))?;
|
||||
let mut destination_file = fs::File::open(destination).with_context(|| {
|
||||
format!(
|
||||
"open helper destination for compare {}",
|
||||
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 compare {}", source.display())
|
||||
})?;
|
||||
let destination_read = destination_file.read(&mut destination_buf).with_context(|| {
|
||||
format!(
|
||||
"read helper destination for compare {}",
|
||||
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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn exe_dir_is_windows_apps(path: &Path) -> bool {
|
||||
path.components().any(|component| {
|
||||
component
|
||||
.as_os_str()
|
||||
.to_string_lossy()
|
||||
.eq_ignore_ascii_case("WindowsApps")
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::destination_is_fresh;
|
||||
use super::exe_dir_is_windows_apps;
|
||||
use super::helper_bin_dir;
|
||||
use super::legacy_lookup_candidate;
|
||||
use super::copy_from_source_if_needed;
|
||||
use super::HelperExecutable;
|
||||
use super::CopyOutcome;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
@@ -317,18 +378,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"source-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"other--v1").expect("write destination");
|
||||
assert!(!destination_is_fresh(&source, &destination).expect("content drift"));
|
||||
|
||||
fs::write(&destination, b"same-size").expect("rewrite destination");
|
||||
assert!(destination_is_fresh(&source, &destination).expect("fresh metadata"));
|
||||
fs::write(&destination, b"source-v1").expect("rewrite destination");
|
||||
assert!(destination_is_fresh(&source, &destination).expect("fresh helper"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -376,4 +437,24 @@ mod tests {
|
||||
fs::read(&runner_destination).expect("read runner")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn windows_apps_dirs_do_not_use_legacy_sibling_lookup() {
|
||||
let candidate = legacy_lookup_candidate(
|
||||
HelperExecutable::CommandRunner,
|
||||
Path::new(r"C:\Program Files\WindowsApps\OpenAI.Codex"),
|
||||
);
|
||||
|
||||
assert_eq!(PathBuf::from("codex-command-runner.exe"), candidate);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exe_dir_detection_matches_windows_apps_component() {
|
||||
assert!(exe_dir_is_windows_apps(Path::new(
|
||||
r"C:\Program Files\WindowsApps\OpenAI.Codex"
|
||||
)));
|
||||
assert!(!exe_dir_is_windows_apps(Path::new(
|
||||
r"C:\Users\example\AppData\Local\Programs\Codex"
|
||||
)));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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::exe_dir_is_windows_apps;
|
||||
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 +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());
|
||||
@@ -334,6 +337,7 @@ 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()
|
||||
&& should_include_current_exe_parent_read_root(dir)
|
||||
{
|
||||
roots.push(dir.to_path_buf());
|
||||
}
|
||||
@@ -343,6 +347,10 @@ fn gather_helper_read_roots(codex_home: &Path) -> Vec<PathBuf> {
|
||||
roots
|
||||
}
|
||||
|
||||
fn should_include_current_exe_parent_read_root(exe_dir: &Path) -> bool {
|
||||
!exe_dir_is_windows_apps(exe_dir)
|
||||
}
|
||||
|
||||
fn gather_legacy_full_read_roots(
|
||||
command_cwd: &Path,
|
||||
policy: &SandboxPolicy,
|
||||
@@ -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,
|
||||
@@ -816,6 +820,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 +1014,16 @@ mod tests {
|
||||
assert!(roots.contains(&expected));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn windows_apps_parent_is_excluded_from_helper_read_roots() {
|
||||
assert!(!super::should_include_current_exe_parent_read_root(
|
||||
Path::new(r"C:\Program Files\WindowsApps\OpenAI.Codex")
|
||||
));
|
||||
assert!(super::should_include_current_exe_parent_read_root(
|
||||
Path::new(r"C:\Users\example\AppData\Local\Programs\Codex")
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restricted_read_roots_skip_platform_defaults_when_disabled() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
|
||||
Reference in New Issue
Block a user