core/core-agent-ide
8
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

copy command-runner to CODEX_HOME so sandbox users can always execute it (#13413)

Browse source 
• Keep Windows sandbox runner launches working from packaged installs by
running the helper from a user-owned runtime location.

On some Windows installs, the packaged helper location is difficult to
use reliably for sandboxed runner launches even though the binaries are
present. This change works around that by copying codex-
command-runner.exe into CODEX_HOME/.sandbox-bin/, reusing that copy
across launches, and falling back to the existing packaged-path lookup
if anything goes wrong.

The runtime copy lives in a dedicated directory with tighter ACLs than
.sandbox: sandbox users can read and execute the runner there, but they
cannot modify it. This keeps the workaround focused on the
command runner, leaves the setup helper on its trusted packaged path,
and adds logging so it is clear which runner path was selected at
launch.
This commit is contained in:
iceweasel-oai 2026-03-03 17:31:37 -08:00 • committed by GitHub
parent 52521a5e40
commit 639a5f6c48
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 448 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
codex-rs/windows-sandbox-rs/Cargo.toml
View file

@ -29,6 +29,7 @@ codex-utils-string = { workspace = true }
dunce = "1.0"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
tempfile = "3"
windows = { version = "0.58", features = [
"Win32_Foundation",
"Win32_NetworkManagement_WindowsFirewall",
@ -79,7 +80,6 @@ version = "0.52"
[dev-dependencies]
pretty_assertions = { workspace = true }
tempfile = "3"
[build-dependencies]
winres = "0.1"

36
codex-rs/windows-sandbox-rs/src/elevated_impl.rs
View file

@ -6,6 +6,8 @@ mod windows_impl {
use crate::env::ensure_non_interactive_pager;
use crate::env::inherit_path_env;
use crate::env::normalize_null_device_env;
use crate::helper_materialization::resolve_helper_for_launch;
use crate::helper_materialization::HelperExecutable;
use crate::identity::require_logon_sandbox_creds;
use crate::logging::log_failure;
use crate::logging::log_note;
@ -110,17 +112,9 @@ mod windows_impl {
}
}
/// Locates `codex-command-runner.exe` next to the current binary.
fn find_runner_exe() -> PathBuf {
if let Ok(exe) = std::env::current_exe() {
if let Some(dir) = exe.parent() {
let candidate = dir.join("codex-command-runner.exe");
if candidate.exists() {
return candidate;
}
}
}
PathBuf::from("codex-command-runner.exe")
/// Resolves the command runner path, preferring CODEX_HOME/.sandbox/bin.
fn find_runner_exe(codex_home: &Path, log_dir: Option<&Path>) -> PathBuf {
resolve_helper_for_launch(HelperExecutable::CommandRunner, codex_home, log_dir)
}
/// Generates a unique named-pipe path used to communicate with the runner process.
@ -286,7 +280,7 @@ mod windows_impl {
)?;
// Launch runner as sandbox user via CreateProcessWithLogonW.
let runner_exe = find_runner_exe();
let runner_exe = find_runner_exe(codex_home, logs_base_dir);
let runner_cmdline = runner_exe
.to_str()
.map(|s| s.to_string())
@ -340,6 +334,16 @@ mod windows_impl {
// Suppress WER/UI popups from the runner process so we can collect exit codes.
let _ = unsafe { SetErrorMode(0x0001 | 0x0002) }; // SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX
log_note(
&format!(
"runner launch: exe={} cmdline={} cwd={}",
runner_exe.display(),
runner_full_cmd,
cwd.display()
),
logs_base_dir,
);
// Ensure command line buffer is mutable and includes the exe as argv[0].
let spawn_res = unsafe {
CreateProcessWithLogonW(
@ -362,6 +366,14 @@ mod windows_impl {
};
if spawn_res == 0 {
let err = unsafe { GetLastError() } as i32;
log_note(
&format!(
"runner launch failed before process start: exe={} cmdline={} error={err}",
runner_exe.display(),
runner_full_cmd
),
logs_base_dir,
);
return Err(anyhow::anyhow!("CreateProcessWithLogonW failed: {}", err));
}

351
codex-rs/windows-sandbox-rs/src/helper_materialization.rs Normal file
View file

@ -0,0 +1,351 @@
use anyhow::anyhow;
use anyhow::Context;
use anyhow::Result;
use std::collections::HashMap;
use std::fs;
use std::io::Write;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Mutex;
use std::sync::OnceLock;
use tempfile::NamedTempFile;
use crate::logging::log_note;
use crate::sandbox_bin_dir;
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub(crate) enum HelperExecutable {
CommandRunner,
}
impl HelperExecutable {
fn file_name(self) -> &'static str {
match self {
Self::CommandRunner => "codex-command-runner.exe",
}
}
fn label(self) -> &'static str {
match self {
Self::CommandRunner => "command-runner",
}
}
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
enum CopyOutcome {
Reused,
ReCopied,
}
static HELPER_PATH_CACHE: OnceLock<Mutex<HashMap<String, PathBuf>>> = OnceLock::new();
pub(crate) fn helper_bin_dir(codex_home: &Path) -> PathBuf {
sandbox_bin_dir(codex_home)
}
pub(crate) fn legacy_lookup(kind: HelperExecutable) -> PathBuf {
if let Ok(exe) = std::env::current_exe() {
if let Some(dir) = exe.parent() {
let candidate = dir.join(kind.file_name());
if candidate.exists() {
return candidate;
}
}
}
PathBuf::from(kind.file_name())
}
pub(crate) fn resolve_helper_for_launch(
kind: HelperExecutable,
codex_home: &Path,
log_dir: Option<&Path>,
) -> PathBuf {
match copy_helper_if_needed(kind, codex_home, log_dir) {
Ok(path) => {
log_note(
&format!(
"helper launch resolution: using copied {} path {}",
kind.label(),
path.display()
),
log_dir,
);
path
}
Err(err) => {
let fallback = legacy_lookup(kind);
log_note(
&format!(
"helper copy failed for {}: {err:#}; falling back to legacy path {}",
kind.label(),
fallback.display()
),
log_dir,
);
fallback
}
}
}
pub(crate) fn copy_helper_if_needed(
kind: HelperExecutable,
codex_home: &Path,
log_dir: Option<&Path>,
) -> Result<PathBuf> {
let cache_key = format!("{}|{}", kind.file_name(), codex_home.display());
if let Some(path) = cached_helper_path(&cache_key) {
log_note(
&format!(
"helper copy: using in-memory cache for {} -> {}",
kind.label(),
path.display()
),
log_dir,
);
return Ok(path);
}
let source = sibling_source_path(kind)?;
let destination = helper_bin_dir(codex_home).join(kind.file_name());
log_note(
&format!(
"helper copy: validating {} source={} destination={}",
kind.label(),
source.display(),
destination.display()
),
log_dir,
);
let outcome = copy_from_source_if_needed(&source, &destination)?;
let action = match outcome {
CopyOutcome::Reused => "reused",
CopyOutcome::ReCopied => "recopied",
};
log_note(
&format!(
"helper copy: {} {} source={} destination={}",
action,
kind.label(),
source.display(),
destination.display()
),
log_dir,
);
store_helper_path(cache_key, destination.clone());
Ok(destination)
}
fn cached_helper_path(cache_key: &str) -> Option<PathBuf> {
let cache = HELPER_PATH_CACHE.get_or_init(|| Mutex::new(HashMap::new()));
let guard = cache.lock().ok()?;
guard.get(cache_key).cloned()
}
fn store_helper_path(cache_key: String, path: PathBuf) {
let cache = HELPER_PATH_CACHE.get_or_init(|| Mutex::new(HashMap::new()));
if let Ok(mut guard) = cache.lock() {
guard.insert(cache_key, path);
}
}
fn sibling_source_path(kind: HelperExecutable) -> Result<PathBuf> {
let exe = std::env::current_exe().context("resolve current executable for helper lookup")?;
let dir = exe
.parent()
.ok_or_else(|| anyhow!("current executable has no parent directory"))?;
let candidate = dir.join(kind.file_name());
if candidate.exists() {
Ok(candidate)
} else {
Err(anyhow!(
"helper not found next to current executable: {}",
candidate.display()
))
}
}
fn copy_from_source_if_needed(source: &Path, destination: &Path) -> Result<CopyOutcome> {
if destination_is_fresh(source, destination)? {
return Ok(CopyOutcome::Reused);
}
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 {}",
destination_dir.display()
)
})?;
let temp_path = NamedTempFile::new_in(destination_dir)
.with_context(|| {
format!(
"create temporary helper file in {}",
destination_dir.display()
)
})?
.into_temp_path();
let temp_path_buf = temp_path.to_path_buf();
let mut source_file = fs::File::open(source)
.with_context(|| format!("open helper source for read {}", source.display()))?;
let mut temp_file = fs::OpenOptions::new()
.write(true)
.truncate(true)
.open(&temp_path_buf)
.with_context(|| format!("open temporary helper file {}", temp_path_buf.display()))?;
// Write into a temp file created inside `.sandbox-bin` so the copied helper keeps the
// destination directory's inherited ACLs instead of reusing the source file's descriptor.
std::io::copy(&mut source_file, &mut temp_file).with_context(|| {
format!(
"copy helper from {} to {}",
source.display(),
temp_path_buf.display()
)
})?;
temp_file
.flush()
.with_context(|| format!("flush temporary helper file {}", temp_path_buf.display()))?;
drop(temp_file);
if destination.exists() {
fs::remove_file(destination).with_context(|| {
format!("remove stale helper destination {}", destination.display())
})?;
}
match fs::rename(&temp_path_buf, destination) {
Ok(()) => Ok(CopyOutcome::ReCopied),
Err(rename_err) => {
if destination_is_fresh(source, destination)? {
Ok(CopyOutcome::Reused)
} else {
Err(rename_err).with_context(|| {
format!(
"rename helper temp file {} to {}",
temp_path_buf.display(),
destination.display()
)
})
}
}
}
}
fn destination_is_fresh(source: &Path, destination: &Path) -> Result<bool> {
let source_meta = fs::metadata(source)
.with_context(|| format!("read helper source metadata {}", source.display()))?;
let destination_meta = match fs::metadata(destination) {
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()));
}
};
if source_meta.len() != destination_meta.len() {
return Ok(false);
}
let source_modified = source_meta
.modified()
.with_context(|| format!("read helper source mtime {}", source.display()))?;
let destination_modified = destination_meta
.modified()
.with_context(|| format!("read helper destination mtime {}", destination.display()))?;
Ok(destination_modified >= source_modified)
}
#[cfg(test)]
mod tests {
use super::destination_is_fresh;
use super::helper_bin_dir;
use super::copy_from_source_if_needed;
use super::CopyOutcome;
use pretty_assertions::assert_eq;
use std::fs;
use std::path::Path;
use std::path::PathBuf;
use tempfile::TempDir;
#[test]
fn copy_from_source_if_needed_copies_missing_destination() {
let tmp = TempDir::new().expect("tempdir");
let source = tmp.path().join("source.exe");
let destination = tmp.path().join("bin").join("helper.exe");
fs::write(&source, b"runner-v1").expect("write source");
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"));
}
#[test]
fn destination_is_fresh_uses_size_and_mtime() {
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"));
fs::write(&destination, b"same-size").expect("rewrite destination");
assert!(destination_is_fresh(&source, &destination).expect("fresh metadata"));
}
#[test]
fn copy_from_source_if_needed_reuses_fresh_destination() {
let tmp = TempDir::new().expect("tempdir");
let source = tmp.path().join("source.exe");
let destination = tmp.path().join("bin").join("helper.exe");
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");
assert_eq!(CopyOutcome::Reused, outcome);
assert_eq!(b"runner-v1".as_slice(), fs::read(&destination).expect("read destination"));
}
#[test]
fn helper_bin_dir_is_under_sandbox_bin() {
let codex_home = Path::new(r"C:\Users\example\.codex");
assert_eq!(
PathBuf::from(r"C:\Users\example\.codex\.sandbox-bin"),
helper_bin_dir(codex_home)
);
}
#[test]
fn copy_runner_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 runner_source = source_dir.join("codex-command-runner.exe");
let runner_destination = helper_bin_dir(&codex_home).join("codex-command-runner.exe");
fs::write(&runner_source, b"runner").expect("runner");
let runner_outcome =
copy_from_source_if_needed(&runner_source, &runner_destination).expect("runner copy");
assert_eq!(CopyOutcome::ReCopied, runner_outcome);
assert_eq!(
b"runner".as_slice(),
fs::read(&runner_destination).expect("read runner")
);
}
}

2
codex-rs/windows-sandbox-rs/src/identity.rs
View file

@ -130,7 +130,7 @@ pub fn require_logon_sandbox_creds(
codex_home: &Path,
) -> Result<SandboxCreds> {
let sandbox_dir = crate::setup::sandbox_dir(codex_home);
let needed_read = gather_read_roots(command_cwd, policy);
let needed_read = gather_read_roots(command_cwd, policy, codex_home);
let needed_write = gather_write_roots(policy, policy_cwd, command_cwd, env_map);
// NOTE: Do not add CODEX_HOME/.sandbox to `needed_write`; it must remain non-writable by the
// restricted capability token. The setup helper's `lock_sandbox_dir` is responsible for

3
codex-rs/windows-sandbox-rs/src/lib.rs
View file

@ -11,6 +11,7 @@ windows_modules!(
cap,
dpapi,
env,
helper_materialization,
hide_users,
identity,
logging,
@ -85,6 +86,8 @@ pub use setup::run_setup_refresh;
#[cfg(target_os = "windows")]
pub use setup::run_setup_refresh_with_extra_read_roots;
#[cfg(target_os = "windows")]
pub use setup::sandbox_bin_dir;
#[cfg(target_os = "windows")]
pub use setup::sandbox_dir;
#[cfg(target_os = "windows")]
pub use setup::sandbox_secrets_dir;

34
codex-rs/windows-sandbox-rs/src/setup_main_win.rs
View file

@ -18,6 +18,7 @@ use codex_windows_sandbox::log_note;
use codex_windows_sandbox::path_mask_allows;
use codex_windows_sandbox::protect_workspace_agents_dir;
use codex_windows_sandbox::protect_workspace_codex_dir;
use codex_windows_sandbox::sandbox_bin_dir;
use codex_windows_sandbox::sandbox_dir;
use codex_windows_sandbox::sandbox_secrets_dir;
use codex_windows_sandbox::string_from_sid_bytes;
@ -245,6 +246,8 @@ fn lock_sandbox_dir(
real_user: &str,
sandbox_group_sid: &[u8],
sandbox_group_access_mode: i32,
sandbox_group_mask: u32,
real_user_mask: u32,
_log: &mut File,
) -> Result<()> {
std::fs::create_dir_all(dir)?;
@ -254,7 +257,7 @@ fn lock_sandbox_dir(
let entries = [
(
sandbox_group_sid.to_vec(),
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE,
sandbox_group_mask,
sandbox_group_access_mode,
),
(
@ -267,11 +270,7 @@ fn lock_sandbox_dir(
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE,
GRANT_ACCESS,
),
(
real_sid,
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE,
GRANT_ACCESS,
),
(real_sid, real_user_mask, GRANT_ACCESS),
];
unsafe {
let mut eas: Vec<EXPLICIT_ACCESS_W> = Vec::new();
@ -740,6 +739,25 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
}
});
lock_sandbox_dir(
&sandbox_bin_dir(&payload.codex_home),
&payload.real_user,
&sandbox_group_sid,
GRANT_ACCESS,
FILE_GENERIC_READ | FILE_GENERIC_EXECUTE,
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE,
log,
)
.map_err(|err| {
anyhow::Error::new(SetupFailure::new(
SetupErrorCode::HelperSandboxLockFailed,
format!(
"lock sandbox bin dir {} failed: {err}",
sandbox_bin_dir(&payload.codex_home).display()
),
))
})?;
if refresh_only {
log_line(
log,
@ -756,6 +774,8 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
&payload.real_user,
&sandbox_group_sid,
GRANT_ACCESS,
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE,
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE,
log,
)
.map_err(|err| {
@ -772,6 +792,8 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
&payload.real_user,
&sandbox_group_sid,
DENY_ACCESS,
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE,
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE,
log,
)
.map_err(|err| {

45
codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs
View file

@ -11,6 +11,7 @@ use std::process::Stdio;
use crate::allow::compute_allow_paths;
use crate::allow::AllowDenyPaths;
use crate::helper_materialization::helper_bin_dir;
use crate::logging::log_note;
use crate::path_normalization::canonical_path_key;
use crate::policy::SandboxPolicy;
@ -55,6 +56,10 @@ pub fn sandbox_dir(codex_home: &Path) -> PathBuf {
codex_home.join(".sandbox")
}
pub fn sandbox_bin_dir(codex_home: &Path) -> PathBuf {
codex_home.join(".sandbox-bin")
}
pub fn sandbox_secrets_dir(codex_home: &Path) -> PathBuf {
codex_home.join(".sandbox-secrets")
}
@ -93,7 +98,7 @@ pub fn run_setup_refresh_with_extra_read_roots(
codex_home: &Path,
extra_read_roots: Vec<PathBuf>,
) -> Result<()> {
let mut read_roots = gather_read_roots(command_cwd, policy);
let mut read_roots = gather_read_roots(command_cwd, policy, codex_home);
read_roots.extend(extra_read_roots);
run_setup_refresh_inner(
policy,
@ -276,13 +281,20 @@ fn profile_read_roots(user_profile: &Path) -> Vec<PathBuf> {
.collect()
}
pub(crate) fn gather_read_roots(command_cwd: &Path, policy: &SandboxPolicy) -> Vec<PathBuf> {
pub(crate) fn gather_read_roots(
command_cwd: &Path,
policy: &SandboxPolicy,
codex_home: &Path,
) -> Vec<PathBuf> {
let mut roots: Vec<PathBuf> = Vec::new();
if let Ok(exe) = std::env::current_exe() {
if let Some(dir) = exe.parent() {
roots.push(dir.to_path_buf());
}
}
let helper_dir = helper_bin_dir(codex_home);
let _ = std::fs::create_dir_all(&helper_dir);
roots.push(helper_dir);
for p in [
PathBuf::from(r"C:\Windows"),
PathBuf::from(r"C:\Program Files"),
@ -583,7 +595,7 @@ fn build_payload_roots(
let mut read_roots = if let Some(roots) = read_roots_override {
canonical_existing(&roots)
} else {
gather_read_roots(command_cwd, policy)
gather_read_roots(command_cwd, policy, codex_home)
};
let write_root_set: HashSet<PathBuf> = write_roots.iter().cloned().collect();
read_roots.retain(|root| !write_root_set.contains(root));
@ -591,11 +603,14 @@ fn build_payload_roots(
}
fn filter_sensitive_write_roots(mut roots: Vec<PathBuf>, codex_home: &Path) -> Vec<PathBuf> {
// Never grant capability write access to CODEX_HOME or anything under CODEX_HOME/.sandbox.
// These locations contain sandbox control/state and must remain tamper-resistant.
// Never grant capability write access to CODEX_HOME or anything under CODEX_HOME/.sandbox,
// CODEX_HOME/.sandbox-bin, or CODEX_HOME/.sandbox-secrets. These locations contain sandbox
// control/state and helper binaries and must remain tamper-resistant.
let codex_home_key = canonical_path_key(codex_home);
let sbx_dir_key = canonical_path_key(&sandbox_dir(codex_home));
let sbx_dir_prefix = format!("{}/", sbx_dir_key.trim_end_matches('/'));
let sbx_bin_dir_key = canonical_path_key(&sandbox_bin_dir(codex_home));
let sbx_bin_dir_prefix = format!("{}/", sbx_bin_dir_key.trim_end_matches('/'));
let secrets_dir_key = canonical_path_key(&sandbox_secrets_dir(codex_home));
let secrets_dir_prefix = format!("{}/", secrets_dir_key.trim_end_matches('/'));
@ -604,6 +619,8 @@ fn filter_sensitive_write_roots(mut roots: Vec<PathBuf>, codex_home: &Path) -> V
key != codex_home_key
&& key != sbx_dir_key
&& !key.starts_with(&sbx_dir_prefix)
&& key != sbx_bin_dir_key
&& !key.starts_with(&sbx_bin_dir_prefix)
&& key != secrets_dir_key
&& !key.starts_with(&secrets_dir_prefix)
});
@ -612,7 +629,10 @@ fn filter_sensitive_write_roots(mut roots: Vec<PathBuf>, codex_home: &Path) -> V
#[cfg(test)]
mod tests {
use super::gather_read_roots;
use super::profile_read_roots;
use crate::helper_materialization::helper_bin_dir;
use crate::policy::SandboxPolicy;
use pretty_assertions::assert_eq;
use std::collections::HashSet;
use std::fs;
@ -649,4 +669,19 @@ mod tests {
assert_eq!(vec![missing_profile], roots);
}
#[test]
fn gather_read_roots_includes_helper_bin_dir() {
let tmp = TempDir::new().expect("tempdir");
let codex_home = tmp.path().join("codex-home");
let command_cwd = tmp.path().join("workspace");
fs::create_dir_all(&command_cwd).expect("create workspace");
let policy = SandboxPolicy::new_read_only_policy();
let roots = gather_read_roots(&command_cwd, &policy, &codex_home);
let expected =
dunce::canonicalize(helper_bin_dir(&codex_home)).expect("canonical helper dir");
assert!(roots.contains(&expected));
}
}