From 8c0e98327aaa372ea8d522c8fdecdf77d33cf004 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 27 Jan 2026 15:21:14 -0800 Subject: [PATCH 1/5] Use path2 arg0 temp dirs with advisory-lock janitor --- codex-rs/arg0/src/lib.rs | 94 ++++++++++++++++++++++++++++++-- codex-rs/core/tests/suite/mod.rs | 29 +++++++++- 2 files changed, 117 insertions(+), 6 deletions(-) diff --git a/codex-rs/arg0/src/lib.rs b/codex-rs/arg0/src/lib.rs index bf2f7afb7cc8..324b67f67b9a 100644 --- a/codex-rs/arg0/src/lib.rs +++ b/codex-rs/arg0/src/lib.rs @@ -1,3 +1,4 @@ +use std::fs::File; use std::future::Future; use std::path::Path; use std::path::PathBuf; @@ -10,8 +11,34 @@ use tempfile::TempDir; const LINUX_SANDBOX_ARG0: &str = "codex-linux-sandbox"; const APPLY_PATCH_ARG0: &str = "apply_patch"; const MISSPELLED_APPLY_PATCH_ARG0: &str = "applypatch"; +const LOCK_FILENAME: &str = ".lock"; -pub fn arg0_dispatch() -> Option { +/// Keeps the per-session PATH entry alive and locked for the process lifetime. +pub struct Arg0PathEntryGuard { + temp_dir: TempDir, + lock_file: File, +} + +impl Arg0PathEntryGuard { + fn new(temp_dir: TempDir, lock_file: File) -> Self { + Self { + temp_dir, + lock_file, + } + } + + #[allow(dead_code)] + pub fn path(&self) -> &Path { + self.temp_dir.path() + } + + #[allow(dead_code)] + pub fn lock_file(&self) -> &File { + &self.lock_file + } +} + +pub fn arg0_dispatch() -> Option { // Determine if we were invoked via the special alias. let mut args = std::env::args_os(); let argv0 = args.next().unwrap_or_default(); @@ -149,7 +176,7 @@ where /// /// IMPORTANT: This function modifies the PATH environment variable, so it MUST /// be called before multiple threads are spawned. -pub fn prepend_path_entry_for_codex_aliases() -> std::io::Result { +pub fn prepend_path_entry_for_codex_aliases() -> std::io::Result { let codex_home = codex_core::config::find_codex_home()?; #[cfg(not(debug_assertions))] { @@ -167,7 +194,7 @@ pub fn prepend_path_entry_for_codex_aliases() -> std::io::Result { std::fs::create_dir_all(&codex_home)?; // Use a CODEX_HOME-scoped temp root to avoid cluttering the top-level directory. - let temp_root = codex_home.join("tmp").join("path"); + let temp_root = codex_home.join("tmp").join("path2"); std::fs::create_dir_all(&temp_root)?; #[cfg(unix)] { @@ -177,11 +204,25 @@ pub fn prepend_path_entry_for_codex_aliases() -> std::io::Result { std::fs::set_permissions(&temp_root, std::fs::Permissions::from_mode(0o700))?; } + // Best-effort cleanup of stale per-session dirs. Ignore failures so startup proceeds. + if let Err(err) = janitor_cleanup(&temp_root) { + eprintln!("WARNING: failed to clean up stale arg0 temp dirs: {err}"); + } + let temp_dir = tempfile::Builder::new() .prefix("codex-arg0") .tempdir_in(&temp_root)?; let path = temp_dir.path(); + let lock_path = path.join(LOCK_FILENAME); + let lock_file = File::options() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&lock_path)?; + lock_file.try_lock()?; + for filename in &[ APPLY_PATCH_ARG0, MISSPELLED_APPLY_PATCH_ARG0, @@ -231,5 +272,50 @@ pub fn prepend_path_entry_for_codex_aliases() -> std::io::Result { std::env::set_var("PATH", updated_path_env_var); } - Ok(temp_dir) + Ok(Arg0PathEntryGuard::new(temp_dir, lock_file)) +} + +fn janitor_cleanup(temp_root: &Path) -> std::io::Result<()> { + let entries = match std::fs::read_dir(temp_root) { + Ok(entries) => entries, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(()), + Err(err) => return Err(err), + }; + + for entry in entries.flatten() { + let path = entry.path(); + if !path.is_dir() { + continue; + } + + // Skip the directory if locking fails or the lock is currently held. + let Some(_lock_file) = try_lock_dir(&path)? else { + continue; + }; + + std::fs::remove_dir_all(&path)?; + } + + Ok(()) +} + +fn try_lock_dir(dir: &Path) -> std::io::Result> { + let lock_path = dir.join(LOCK_FILENAME); + let lock_file = match File::options() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&lock_path) + { + Ok(file) => file, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(err) => return Err(err), + }; + + match lock_file.try_lock() { + Ok(()) => Ok(Some(lock_file)), + Err(std::fs::TryLockError::WouldBlock) => Ok(None), + Err(err) => Err(err.into()), + } } diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index f34ca09b40d6..0fa2d5be8dca 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -1,16 +1,41 @@ // Aggregates all former standalone integration tests as modules. +use codex_arg0::Arg0PathEntryGuard; use codex_arg0::arg0_dispatch; use ctor::ctor; use tempfile::TempDir; +struct TestCodexAliasesGuard { + _codex_home: TempDir, + _arg0: Arg0PathEntryGuard, +} + +const CODEX_HOME_ENV_VAR: &str = "CODEX_HOME"; + // This code runs before any other tests are run. // It allows the test binary to behave like codex and dispatch to apply_patch and codex-linux-sandbox // based on the arg0. // NOTE: this doesn't work on ARM #[ctor] -pub static CODEX_ALIASES_TEMP_DIR: TempDir = unsafe { +pub static CODEX_ALIASES_TEMP_DIR: TestCodexAliasesGuard = unsafe { #[allow(clippy::unwrap_used)] - arg0_dispatch().unwrap() + let codex_home = tempfile::Builder::new() + .prefix("codex-core-tests") + .tempdir() + .unwrap(); + unsafe { + std::env::set_var(CODEX_HOME_ENV_VAR, codex_home.path()); + } + + #[allow(clippy::unwrap_used)] + let arg0 = arg0_dispatch().unwrap(); + unsafe { + std::env::remove_var(CODEX_HOME_ENV_VAR); + } + + TestCodexAliasesGuard { + _codex_home: codex_home, + _arg0: arg0, + } }; #[cfg(not(target_os = "windows"))] From 337c4accba97015d55941330cd24bd542673ddb5 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 27 Jan 2026 15:39:54 -0800 Subject: [PATCH 2/5] Avoid janitor cleanup of in-progress arg0 dirs --- codex-rs/arg0/src/lib.rs | 66 +++++++++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 7 deletions(-) diff --git a/codex-rs/arg0/src/lib.rs b/codex-rs/arg0/src/lib.rs index 324b67f67b9a..a362a799545c 100644 --- a/codex-rs/arg0/src/lib.rs +++ b/codex-rs/arg0/src/lib.rs @@ -301,13 +301,7 @@ fn janitor_cleanup(temp_root: &Path) -> std::io::Result<()> { fn try_lock_dir(dir: &Path) -> std::io::Result> { let lock_path = dir.join(LOCK_FILENAME); - let lock_file = match File::options() - .read(true) - .write(true) - .create(true) - .truncate(false) - .open(&lock_path) - { + let lock_file = match File::options().read(true).write(true).open(&lock_path) { Ok(file) => file, Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None), Err(err) => return Err(err), @@ -319,3 +313,61 @@ fn try_lock_dir(dir: &Path) -> std::io::Result> { Err(err) => Err(err.into()), } } + +#[cfg(test)] +mod tests { + use super::LOCK_FILENAME; + use super::janitor_cleanup; + use std::fs; + use std::fs::File; + use std::path::Path; + + fn create_lock(dir: &Path) -> std::io::Result { + let lock_path = dir.join(LOCK_FILENAME); + File::options() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(lock_path) + } + + #[test] + fn janitor_skips_dirs_without_lock_file() -> std::io::Result<()> { + let root = tempfile::tempdir()?; + let dir = root.path().join("no-lock"); + fs::create_dir(&dir)?; + + janitor_cleanup(root.path())?; + + assert!(dir.exists()); + Ok(()) + } + + #[test] + fn janitor_skips_dirs_with_held_lock() -> std::io::Result<()> { + let root = tempfile::tempdir()?; + let dir = root.path().join("locked"); + fs::create_dir(&dir)?; + let lock_file = create_lock(&dir)?; + lock_file.try_lock()?; + + janitor_cleanup(root.path())?; + + assert!(dir.exists()); + Ok(()) + } + + #[test] + fn janitor_removes_dirs_with_unlocked_lock() -> std::io::Result<()> { + let root = tempfile::tempdir()?; + let dir = root.path().join("stale"); + fs::create_dir(&dir)?; + create_lock(&dir)?; + + janitor_cleanup(root.path())?; + + assert!(!dir.exists()); + Ok(()) + } +} From ed08884b00d7f9e07951e20354c2bbd18559778b Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 27 Jan 2026 18:12:55 -0800 Subject: [PATCH 3/5] Restore CODEX_HOME after arg0 test setup --- codex-rs/core/tests/suite/mod.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 0fa2d5be8dca..f2dea43fe584 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -1,4 +1,6 @@ // Aggregates all former standalone integration tests as modules. +use std::ffi::OsString; + use codex_arg0::Arg0PathEntryGuard; use codex_arg0::arg0_dispatch; use ctor::ctor; @@ -7,6 +9,7 @@ use tempfile::TempDir; struct TestCodexAliasesGuard { _codex_home: TempDir, _arg0: Arg0PathEntryGuard, + _previous_codex_home: Option, } const CODEX_HOME_ENV_VAR: &str = "CODEX_HOME"; @@ -22,19 +25,26 @@ pub static CODEX_ALIASES_TEMP_DIR: TestCodexAliasesGuard = unsafe { .prefix("codex-core-tests") .tempdir() .unwrap(); + let previous_codex_home = std::env::var_os(CODEX_HOME_ENV_VAR); unsafe { std::env::set_var(CODEX_HOME_ENV_VAR, codex_home.path()); } #[allow(clippy::unwrap_used)] let arg0 = arg0_dispatch().unwrap(); - unsafe { - std::env::remove_var(CODEX_HOME_ENV_VAR); + match previous_codex_home.as_ref() { + Some(value) => unsafe { + std::env::set_var(CODEX_HOME_ENV_VAR, value); + }, + None => unsafe { + std::env::remove_var(CODEX_HOME_ENV_VAR); + }, } TestCodexAliasesGuard { _codex_home: codex_home, _arg0: arg0, + _previous_codex_home: previous_codex_home, } }; From 0489ee4fe69bc41734043ef32af4815bd60a96a6 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Mon, 2 Feb 2026 23:37:08 -0800 Subject: [PATCH 4/5] fix(arg0): harden janitor cleanup and test env setup --- codex-rs/arg0/src/lib.rs | 25 ++++++++++--------------- codex-rs/core/tests/suite/mod.rs | 6 ++++++ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/codex-rs/arg0/src/lib.rs b/codex-rs/arg0/src/lib.rs index a362a799545c..1b8d923c5e6c 100644 --- a/codex-rs/arg0/src/lib.rs +++ b/codex-rs/arg0/src/lib.rs @@ -15,27 +15,17 @@ const LOCK_FILENAME: &str = ".lock"; /// Keeps the per-session PATH entry alive and locked for the process lifetime. pub struct Arg0PathEntryGuard { - temp_dir: TempDir, - lock_file: File, + _temp_dir: TempDir, + _lock_file: File, } impl Arg0PathEntryGuard { fn new(temp_dir: TempDir, lock_file: File) -> Self { Self { - temp_dir, - lock_file, + _temp_dir: temp_dir, + _lock_file: lock_file, } } - - #[allow(dead_code)] - pub fn path(&self) -> &Path { - self.temp_dir.path() - } - - #[allow(dead_code)] - pub fn lock_file(&self) -> &File { - &self.lock_file - } } pub fn arg0_dispatch() -> Option { @@ -293,7 +283,12 @@ fn janitor_cleanup(temp_root: &Path) -> std::io::Result<()> { continue; }; - std::fs::remove_dir_all(&path)?; + match std::fs::remove_dir_all(&path) { + Ok(()) => {} + // Expected TOCTOU race: directory can disappear after read_dir/lock checks. + Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue, + Err(err) => return Err(err), + } } Ok(()) diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index f2dea43fe584..47a0829898b5 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -26,12 +26,18 @@ pub static CODEX_ALIASES_TEMP_DIR: TestCodexAliasesGuard = unsafe { .tempdir() .unwrap(); let previous_codex_home = std::env::var_os(CODEX_HOME_ENV_VAR); + // arg0_dispatch() creates helper links under CODEX_HOME/tmp. Point it at a + // test-owned temp dir so startup never mutates the developer's real ~/.codex. + // + // Safety: #[ctor] runs before tests start, so no test threads exist yet. unsafe { std::env::set_var(CODEX_HOME_ENV_VAR, codex_home.path()); } #[allow(clippy::unwrap_used)] let arg0 = arg0_dispatch().unwrap(); + // Restore the process environment immediately so later tests observe the + // same CODEX_HOME state they started with. match previous_codex_home.as_ref() { Some(value) => unsafe { std::env::set_var(CODEX_HOME_ENV_VAR, value); From 2ca7ee3016d46771c23ab8f096a9ab7e8bd0da9f Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 3 Feb 2026 12:07:20 -0800 Subject: [PATCH 5/5] refactor(arg0): rename temp root path2 to arg0 --- codex-rs/arg0/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/arg0/src/lib.rs b/codex-rs/arg0/src/lib.rs index 1b8d923c5e6c..9c455ddbba28 100644 --- a/codex-rs/arg0/src/lib.rs +++ b/codex-rs/arg0/src/lib.rs @@ -184,7 +184,7 @@ pub fn prepend_path_entry_for_codex_aliases() -> std::io::Result