mirror of
https://github.com/openai/codex.git
synced 2026-05-22 20:14:17 +00:00
windows-sandbox: share bundled helper lookup (#23735)
## Summary Follow-up to #23636 review feedback: the Windows sandbox had two copies of the same bundled-helper lookup order, one for `codex-command-runner.exe` in `helper_materialization.rs` and one for `codex-windows-sandbox-setup.exe` in `setup.rs`. This PR centralizes that lookup in `helper_materialization::bundled_executable_path_for_exe()` and has setup reuse it for `codex-windows-sandbox-setup.exe`. The lookup behavior is unchanged: direct sibling first, package-root `codex-resources/` when running from `bin/`, then legacy sibling `codex-resources/`. ## Test plan - `cargo test -p codex-windows-sandbox` ## Notes I also attempted `cargo check -p codex-windows-sandbox --target x86_64-pc-windows-gnullvm`, but this local host is missing `x86_64-w64-mingw32-clang`.
This commit is contained in:
@@ -16,8 +16,8 @@ use crate::logging::log_note;
|
||||
use crate::sandbox_bin_dir;
|
||||
|
||||
const DEV_BUILD_VERSION_SENTINEL: &str = "0.0.0";
|
||||
const BIN_DIRNAME: &str = "bin";
|
||||
const RESOURCES_DIRNAME: &str = "codex-resources";
|
||||
pub(crate) const BIN_DIRNAME: &str = "bin";
|
||||
pub(crate) const RESOURCES_DIRNAME: &str = "codex-resources";
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
|
||||
pub(crate) enum HelperExecutable {
|
||||
@@ -52,7 +52,7 @@ 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(candidate) = source_path_for_exe(&exe, kind.file_name())
|
||||
&& let Some(candidate) = bundled_executable_path_for_exe(&exe, kind.file_name())
|
||||
{
|
||||
return candidate;
|
||||
}
|
||||
@@ -179,7 +179,7 @@ fn store_helper_path(cache_key: String, path: PathBuf) {
|
||||
|
||||
fn sibling_source_path(kind: HelperExecutable) -> Result<PathBuf> {
|
||||
let exe = std::env::current_exe().context("resolve current executable for helper lookup")?;
|
||||
source_path_for_exe(&exe, kind.file_name()).ok_or_else(|| {
|
||||
bundled_executable_path_for_exe(&exe, kind.file_name()).ok_or_else(|| {
|
||||
anyhow!(
|
||||
"helper not found next to current executable or under {RESOURCES_DIRNAME}: {}",
|
||||
exe.display()
|
||||
@@ -187,7 +187,7 @@ fn sibling_source_path(kind: HelperExecutable) -> Result<PathBuf> {
|
||||
})
|
||||
}
|
||||
|
||||
fn source_path_for_exe(exe: &Path, file_name: &str) -> Option<PathBuf> {
|
||||
pub(crate) fn bundled_executable_path_for_exe(exe: &Path, file_name: &str) -> Option<PathBuf> {
|
||||
let dir = exe.parent()?;
|
||||
let direct_candidate = dir.join(file_name);
|
||||
if direct_candidate.is_file() {
|
||||
@@ -361,13 +361,13 @@ mod tests {
|
||||
use super::DEV_BUILD_VERSION_SENTINEL;
|
||||
use super::HelperExecutable;
|
||||
use super::RESOURCES_DIRNAME;
|
||||
use super::bundled_executable_path_for_exe;
|
||||
use super::copy_from_source_if_needed;
|
||||
use super::destination_is_fresh;
|
||||
use super::dev_build_suffix;
|
||||
use super::helper_bin_dir;
|
||||
use super::helper_version_suffix;
|
||||
use super::materialized_file_name;
|
||||
use super::source_path_for_exe;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
@@ -469,8 +469,9 @@ mod tests {
|
||||
fs::write(&exe, b"codex").expect("write exe");
|
||||
fs::write(&helper, b"runner").expect("write helper");
|
||||
|
||||
let resolved = source_path_for_exe(&exe, /*file_name*/ "codex-command-runner.exe")
|
||||
.expect("helper path");
|
||||
let resolved =
|
||||
bundled_executable_path_for_exe(&exe, /*file_name*/ "codex-command-runner.exe")
|
||||
.expect("helper path");
|
||||
|
||||
assert_eq!(resolved, helper);
|
||||
}
|
||||
@@ -488,8 +489,9 @@ mod tests {
|
||||
fs::write(&exe, b"codex").expect("write exe");
|
||||
fs::write(&helper, b"runner").expect("write helper");
|
||||
|
||||
let resolved = source_path_for_exe(&exe, /*file_name*/ "codex-command-runner.exe")
|
||||
.expect("helper path");
|
||||
let resolved =
|
||||
bundled_executable_path_for_exe(&exe, /*file_name*/ "codex-command-runner.exe")
|
||||
.expect("helper path");
|
||||
|
||||
assert_eq!(resolved, helper);
|
||||
}
|
||||
@@ -510,8 +512,9 @@ mod tests {
|
||||
fs::write(&package_helper, b"package runner").expect("write package helper");
|
||||
fs::write(&bin_helper, b"bin runner").expect("write bin helper");
|
||||
|
||||
let resolved = source_path_for_exe(&exe, /*file_name*/ "codex-command-runner.exe")
|
||||
.expect("helper path");
|
||||
let resolved =
|
||||
bundled_executable_path_for_exe(&exe, /*file_name*/ "codex-command-runner.exe")
|
||||
.expect("helper path");
|
||||
|
||||
assert_eq!(resolved, package_helper);
|
||||
}
|
||||
@@ -529,8 +532,9 @@ mod tests {
|
||||
fs::write(&sibling_helper, b"sibling runner").expect("write sibling helper");
|
||||
fs::write(&resource_helper, b"resource runner").expect("write resource helper");
|
||||
|
||||
let resolved = source_path_for_exe(&exe, /*file_name*/ "codex-command-runner.exe")
|
||||
.expect("helper path");
|
||||
let resolved =
|
||||
bundled_executable_path_for_exe(&exe, /*file_name*/ "codex-command-runner.exe")
|
||||
.expect("helper path");
|
||||
|
||||
assert_eq!(resolved, sibling_helper);
|
||||
}
|
||||
|
||||
@@ -3,7 +3,6 @@ use serde::Serialize;
|
||||
use std::collections::BTreeSet;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::ffi::OsStr;
|
||||
use std::ffi::c_void;
|
||||
use std::os::windows::process::CommandExt;
|
||||
use std::path::Path;
|
||||
@@ -13,6 +12,7 @@ use std::process::Stdio;
|
||||
|
||||
use crate::allow::AllowDenyPaths;
|
||||
use crate::allow::compute_allow_paths;
|
||||
use crate::helper_materialization::bundled_executable_path_for_exe;
|
||||
use crate::helper_materialization::helper_bin_dir;
|
||||
use crate::logging::log_note;
|
||||
use crate::path_normalization::canonical_path_key;
|
||||
@@ -40,11 +40,10 @@ use windows_sys::Win32::Security::SECURITY_NT_AUTHORITY;
|
||||
pub const SETUP_VERSION: u32 = 5;
|
||||
pub const OFFLINE_USERNAME: &str = "CodexSandboxOffline";
|
||||
pub const ONLINE_USERNAME: &str = "CodexSandboxOnline";
|
||||
const BIN_DIRNAME: &str = "bin";
|
||||
const ERROR_CANCELLED: u32 = 1223;
|
||||
const SECURITY_BUILTIN_DOMAIN_RID: u32 = 0x0000_0020;
|
||||
const DOMAIN_ALIAS_RID_ADMINS: u32 = 0x0000_0220;
|
||||
const RESOURCES_DIRNAME: &str = "codex-resources";
|
||||
const SETUP_EXE_FILENAME: &str = "codex-windows-sandbox-setup.exe";
|
||||
const USERPROFILE_ROOT_EXCLUSIONS: &[&str] = &[
|
||||
".ssh",
|
||||
".tsh",
|
||||
@@ -586,34 +585,11 @@ fn find_setup_exe() -> PathBuf {
|
||||
{
|
||||
return setup_exe;
|
||||
}
|
||||
PathBuf::from("codex-windows-sandbox-setup.exe")
|
||||
PathBuf::from(SETUP_EXE_FILENAME)
|
||||
}
|
||||
|
||||
fn find_setup_exe_for_current_exe(exe: &Path) -> Option<PathBuf> {
|
||||
let dir = exe.parent()?;
|
||||
let candidate = dir.join("codex-windows-sandbox-setup.exe");
|
||||
if candidate.is_file() {
|
||||
return Some(candidate);
|
||||
}
|
||||
|
||||
if dir.file_name() == Some(OsStr::new(BIN_DIRNAME))
|
||||
&& let Some(package_dir) = dir.parent()
|
||||
{
|
||||
let package_resource_candidate = package_dir
|
||||
.join(RESOURCES_DIRNAME)
|
||||
.join("codex-windows-sandbox-setup.exe");
|
||||
if package_resource_candidate.is_file() {
|
||||
return Some(package_resource_candidate);
|
||||
}
|
||||
}
|
||||
|
||||
// Older standalone installs keep Windows helper binaries under
|
||||
// `codex-resources/` next to `codex.exe`, so elevation still probes that
|
||||
// sibling folder before falling back to PATH.
|
||||
let resource_candidate = dir
|
||||
.join(RESOURCES_DIRNAME)
|
||||
.join("codex-windows-sandbox-setup.exe");
|
||||
resource_candidate.is_file().then_some(resource_candidate)
|
||||
bundled_executable_path_for_exe(exe, SETUP_EXE_FILENAME)
|
||||
}
|
||||
|
||||
fn report_helper_failure(
|
||||
@@ -976,8 +952,6 @@ fn filter_sensitive_write_roots(mut roots: Vec<PathBuf>, codex_home: &Path) -> V
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::BIN_DIRNAME;
|
||||
use super::RESOURCES_DIRNAME;
|
||||
use super::WINDOWS_PLATFORM_DEFAULT_READ_ROOTS;
|
||||
use super::build_payload_roots;
|
||||
use super::find_setup_exe_for_current_exe;
|
||||
@@ -987,6 +961,8 @@ mod tests {
|
||||
use super::offline_proxy_settings_from_env;
|
||||
use super::profile_read_roots;
|
||||
use super::proxy_ports_from_env;
|
||||
use crate::helper_materialization::BIN_DIRNAME;
|
||||
use crate::helper_materialization::RESOURCES_DIRNAME;
|
||||
use crate::helper_materialization::helper_bin_dir;
|
||||
use crate::policy::SandboxPolicy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
Reference in New Issue
Block a user