mirror of
https://github.com/openai/codex.git
synced 2026-10-17 17:08:55 +00:00
Isolate arg0 helper directories by operating system (#53412)
## Why Windows and WSL can share `CODEX_HOME` without sharing a lock namespace, allowing cleanup to remove another platform's live helpers. Older clients can also clean helpers stored under the shared `tmp/arg0` root. ## What changed Create helpers under `CODEX_HOME/tmp/arg0-<os>` and limit stale-directory cleanup to that operating system's root. Keep permission discovery compatible with both the current platform's root and legacy `tmp/arg0` directories, while ignoring other platforms' aliases. ## Testing Add regression coverage for cleanup isolation, live helper preservation during legacy cleanup, and selection of the first session alias directory from either supported root. GitOrigin-RevId: f34292748b5b1d3c25629604d849db4b133dfa3a
This commit is contained in:
@@ -324,9 +324,21 @@ fn prepare_path_entry_for_codex_aliases(
|
||||
}
|
||||
}
|
||||
|
||||
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("arg0");
|
||||
prepare_aliases_in_home(&codex_home, existing_path)
|
||||
}
|
||||
|
||||
// The production entry point validates the home before reaching this filesystem seam.
|
||||
fn prepare_aliases_in_home(
|
||||
codex_home: &Path,
|
||||
existing_path: Option<OsString>,
|
||||
) -> std::io::Result<(Arg0PathEntryGuard, OsString)> {
|
||||
std::fs::create_dir_all(codex_home)?;
|
||||
// Windows and WSL can share CODEX_HOME without sharing a lock namespace.
|
||||
// Keep each OS outside the legacy `tmp/arg0` root so older clients cannot
|
||||
// delete these helpers while their owning process is still running.
|
||||
let temp_root = codex_home
|
||||
.join("tmp")
|
||||
.join(format!("arg0-{}", std::env::consts::OS));
|
||||
std::fs::create_dir_all(&temp_root)?;
|
||||
#[cfg(unix)]
|
||||
{
|
||||
@@ -725,6 +737,49 @@ mod tests {
|
||||
))
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn helper_cleanup_is_isolated_from_other_platforms_and_legacy_clients() -> std::io::Result<()> {
|
||||
let home = tempfile::tempdir()?;
|
||||
let temporary = home.path().join("tmp");
|
||||
let own_root = temporary.join(format!("arg0-{}", std::env::consts::OS));
|
||||
let foreign_os = if cfg!(windows) { "linux" } else { "windows" };
|
||||
let foreign_root = temporary.join(format!("arg0-{foreign_os}"));
|
||||
let legacy_root = temporary.join("arg0");
|
||||
// Unlocked foreign directories model a live peer whose lock is invisible
|
||||
// to this kernel. Only stale directories in our own namespace may go.
|
||||
for root in [&own_root, &foreign_root, &legacy_root] {
|
||||
let stale = root.join("codex-arg0-existing");
|
||||
fs::create_dir_all(&stale)?;
|
||||
create_lock(&stale)?;
|
||||
fs::write(stale.join("helper"), b"keep foreign helpers")?;
|
||||
}
|
||||
|
||||
let (guard, _) = super::prepare_aliases_in_home(home.path(), /*existing_path*/ None)?;
|
||||
assert!(!own_root.join("codex-arg0-existing").exists());
|
||||
assert_eq!(
|
||||
[&foreign_root, &legacy_root]
|
||||
.map(|root| fs::read(root.join("codex-arg0-existing/helper")))
|
||||
.into_iter()
|
||||
.collect::<std::io::Result<Vec<_>>>()?,
|
||||
vec![b"keep foreign helpers".to_vec(); 2],
|
||||
);
|
||||
|
||||
// Older versions clean only this root. Run that cleanup while a new
|
||||
// helper is live, then confirm the advertised helper remains usable.
|
||||
janitor_cleanup(&legacy_root)?;
|
||||
let helper = guard._temp_dir.path().join(if cfg!(windows) {
|
||||
"apply_patch.bat"
|
||||
} else {
|
||||
"apply_patch"
|
||||
});
|
||||
assert!(helper.is_file());
|
||||
assert_eq!(
|
||||
fs::read(foreign_root.join("codex-arg0-existing/helper"))?,
|
||||
b"keep foreign helpers",
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn janitor_skips_dirs_without_lock_file() -> std::io::Result<()> {
|
||||
let root = tempfile::tempdir()?;
|
||||
|
||||
@@ -54,10 +54,14 @@ pub(super) fn active_arg0_helper_dir(
|
||||
codex_home: &Path,
|
||||
path_env: &OsStr,
|
||||
) -> Option<AbsolutePathBuf> {
|
||||
let arg0_root = codex_home.join("tmp").join("arg0");
|
||||
let arg0_root = codex_home
|
||||
.join("tmp")
|
||||
.join(format!("arg0-{}", std::env::consts::OS));
|
||||
let legacy_root = codex_home.join("tmp").join("arg0");
|
||||
std::env::split_paths(path_env)
|
||||
.find(|path| {
|
||||
path.parent() == Some(arg0_root.as_path())
|
||||
(path.parent() == Some(arg0_root.as_path())
|
||||
|| path.parent() == Some(legacy_root.as_path()))
|
||||
&& path
|
||||
.file_name()
|
||||
.is_some_and(|name| name.to_string_lossy().starts_with("codex-arg0"))
|
||||
|
||||
@@ -22,19 +22,32 @@ use tempfile::TempDir;
|
||||
#[test]
|
||||
fn active_arg0_helper_dir_selects_only_the_first_session_alias_directory() -> anyhow::Result<()> {
|
||||
let home = TempDir::new()?;
|
||||
let arg0_root = home.path().join("tmp/arg0");
|
||||
let active = arg0_root.join("codex-arg0-active");
|
||||
let paths = std::env::join_paths([
|
||||
home.path().join("bin"),
|
||||
arg0_root.clone(),
|
||||
arg0_root.join("unrelated"),
|
||||
active.clone(),
|
||||
arg0_root.join("codex-arg0-parent"),
|
||||
])?;
|
||||
assert_eq!(
|
||||
active_arg0_helper_dir(home.path(), &paths),
|
||||
Some(AbsolutePathBuf::from_absolute_path(active)?)
|
||||
);
|
||||
let platform_root = home
|
||||
.path()
|
||||
.join("tmp")
|
||||
.join(format!("arg0-{}", std::env::consts::OS));
|
||||
let legacy_root = home.path().join("tmp/arg0");
|
||||
let foreign_os = if cfg!(windows) { "linux" } else { "windows" };
|
||||
let foreign_alias = home
|
||||
.path()
|
||||
.join("tmp")
|
||||
.join(format!("arg0-{foreign_os}"))
|
||||
.join("codex-arg0-foreign");
|
||||
for arg0_root in [&platform_root, &legacy_root] {
|
||||
let active = arg0_root.join("codex-arg0-active");
|
||||
let paths = std::env::join_paths([
|
||||
home.path().join("bin"),
|
||||
foreign_alias.clone(),
|
||||
arg0_root.clone(),
|
||||
arg0_root.join("unrelated"),
|
||||
active.clone(),
|
||||
arg0_root.join("codex-arg0-parent"),
|
||||
])?;
|
||||
assert_eq!(
|
||||
active_arg0_helper_dir(home.path(), &paths),
|
||||
Some(AbsolutePathBuf::from_absolute_path(active)?)
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user