From ddde50c611e4800cb805f243ed3c50bbafe7d011 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 22 Apr 2026 11:06:34 -0700 Subject: [PATCH 1/2] arg0: keep dispatch aliases alive during async main (#18999) ## Why The Ubuntu GNU remote Cargo run has been regularly failing sandboxed `suite::remote_env` filesystem tests with `No such file or directory`, while the same cases pass under Bazel. The Cargo remote-env setup starts `target/debug/codex exec-server` inside Docker via `scripts/test-remote-env.sh`. That CLI builds `codex-linux-sandbox` and other arg0 helper aliases in a temporary directory, then passes those alias paths into the exec-server runtime. `arg0_dispatch_or_else` constructed `Arg0DispatchPaths` from that temporary alias guard, but then awaited the async CLI entry point without otherwise keeping the guard live. That allowed the guard to be dropped while the exec-server was still running, removing the helper alias directory. Later sandboxed filesystem calls tried to spawn the now-deleted `codex-linux-sandbox` path and surfaced as `ENOENT`. The relevant distinction I found is that `core/tests/common` stores the result of `arg0_dispatch()` in a process-lifetime `OnceLock>` for test binaries. The Cargo remote-env setup exercises a real `codex exec-server` process instead, so it depends on the normal CLI lifetime behavior fixed here. ## What Changed - Keep the arg0 tempdir guard alive until `main_fn(paths).await` completes. - Keep the helper on the real `arg0_dispatch()` shape, where alias setup can fail and return `None` in production. - Add a regression test that uses an explicit guard, yields once, and verifies the generated helper alias path still exists while the async entry point is running. ## Verification - `cargo test -p codex-arg0` - `just argument-comment-lint -p codex-arg0` - `just fix -p codex-arg0` --- codex-rs/arg0/src/lib.rs | 94 +++++++++++++++++++++++++++++++++------- 1 file changed, 79 insertions(+), 15 deletions(-) diff --git a/codex-rs/arg0/src/lib.rs b/codex-rs/arg0/src/lib.rs index 38f88452af..75fefce5cc 100644 --- a/codex-rs/arg0/src/lib.rs +++ b/codex-rs/arg0/src/lib.rs @@ -185,22 +185,39 @@ where // Regular invocation – create a Tokio runtime and execute the provided // async entry-point. let runtime = build_runtime()?; - runtime.block_on(async move { - let current_exe = std::env::current_exe().ok(); - let paths = Arg0DispatchPaths { - codex_self_exe: current_exe.clone(), - codex_linux_sandbox_exe: if cfg!(target_os = "linux") { - linux_sandbox_exe_path(path_entry_guard.as_ref(), current_exe) - } else { - None - }, - main_execve_wrapper_exe: path_entry_guard - .as_ref() - .and_then(|path_entry| path_entry.paths().main_execve_wrapper_exe.clone()), - }; + runtime.block_on(run_main_with_arg0_guard( + path_entry_guard, + std::env::current_exe().ok(), + main_fn, + )) +} - main_fn(paths).await - }) +async fn run_main_with_arg0_guard( + path_entry_guard: Option, + current_exe: Option, + main_fn: F, +) -> anyhow::Result<()> +where + F: FnOnce(Arg0DispatchPaths) -> Fut, + Fut: Future>, +{ + let paths = Arg0DispatchPaths { + codex_self_exe: current_exe.clone(), + codex_linux_sandbox_exe: if cfg!(target_os = "linux") { + linux_sandbox_exe_path(path_entry_guard.as_ref(), current_exe) + } else { + None + }, + main_execve_wrapper_exe: path_entry_guard + .as_ref() + .and_then(|path_entry| path_entry.paths().main_execve_wrapper_exe.clone()), + }; + + let result = main_fn(paths).await; + // Keep the arg0 tempdir guard alive until the async entry point finishes; + // runtime paths above can point at aliases inside that directory. + drop(path_entry_guard); + result } fn linux_sandbox_exe_path( @@ -442,6 +459,10 @@ mod tests { use super::LOCK_FILENAME; use super::janitor_cleanup; use super::linux_sandbox_exe_path; + #[cfg(unix)] + use super::run_main_with_arg0_guard; + #[cfg(unix)] + use anyhow::ensure; use std::fs; use std::fs::File; use std::path::Path; @@ -480,6 +501,49 @@ mod tests { Ok(()) } + #[cfg(unix)] + #[test] + fn run_main_with_arg0_guard_keeps_aliases_alive_until_main_returns() -> anyhow::Result<()> { + let temp_dir = TempDir::new()?; + let alias_path = temp_dir.path().join("codex-helper-alias"); + fs::write(&alias_path, b"")?; + let lock_file = create_lock(temp_dir.path())?; + let path_entry = Arg0PathEntryGuard::new( + temp_dir, + lock_file, + Arg0DispatchPaths { + codex_self_exe: Some(PathBuf::from("/usr/bin/codex")), + codex_linux_sandbox_exe: Some(alias_path.clone()), + main_execve_wrapper_exe: Some(alias_path), + }, + ); + + super::build_runtime()?.block_on(run_main_with_arg0_guard( + /*path_entry_guard*/ Some(path_entry), + Some(PathBuf::from("/usr/bin/codex")), + |paths| async move { + let alias_path = paths + .codex_linux_sandbox_exe + .or(paths.main_execve_wrapper_exe) + .expect("unix dispatch should create at least one alias path"); + ensure!( + alias_path.exists(), + "alias path disappeared before main future was polled: {}", + alias_path.display() + ); + + tokio::task::yield_now().await; + + ensure!( + alias_path.exists(), + "alias path disappeared while main future was running: {}", + alias_path.display() + ); + Ok(()) + }, + )) + } + #[test] fn janitor_skips_dirs_without_lock_file() -> std::io::Result<()> { let root = tempfile::tempdir()?; From 1506cb173c6856ed759b9ebc9cdeccab21ed3c6a Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 22 Apr 2026 11:33:08 -0700 Subject: [PATCH 2/2] exec-server: expose arg0 alias root to fs sandbox ## Why The post-merge `rust-ci-full` run for #18999 still failed the Ubuntu remote `suite::remote_env` sandboxed filesystem tests. That run checked out merge commit `ddde50c611e4800cb805f243ed3c50bbafe7d011`, so the arg0 guard lifetime fix was present. The remaining gap is that the remote exec-server can pass an arg0 alias path such as `codex-linux-sandbox` as a runtime helper, but the sandboxed filesystem helper only added the real Codex binary parent as a readable runtime root. When bubblewrap re-enters Codex through the alias path, the alias directory also has to be visible inside the sandbox. ## What Changed - Track all helper runtime read roots instead of a single root. - Add both the real Codex executable parent and the `codex-linux-sandbox` alias parent to sandbox readable roots. - Add unit coverage for the alias-parent root. ## Verification - `cargo test -p codex-exec-server` - `just argument-comment-lint -p codex-exec-server` - `just fix -p codex-exec-server` --- codex-rs/exec-server/src/fs_sandbox.rs | 95 ++++++++++++++++++-------- 1 file changed, 68 insertions(+), 27 deletions(-) diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index 5f347e6e1c..9972f6ebb2 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -52,12 +52,12 @@ impl FileSystemSandboxRunner { ) -> Result { let cwd = sandbox_cwd(sandbox)?; let mut file_system_policy = sandbox.permissions.file_system_sandbox_policy(); - let helper_read_root = if sandbox.use_legacy_landlock { - None + let helper_read_roots = if sandbox.use_legacy_landlock { + Vec::new() } else { - helper_read_root(&self.runtime_paths) + helper_read_roots(&self.runtime_paths) }; - add_helper_runtime_permissions(&mut file_system_policy, helper_read_root, cwd.as_path()); + add_helper_runtime_permissions(&mut file_system_policy, &helper_read_roots, cwd.as_path()); normalize_file_system_policy_root_aliases(&mut file_system_policy); let network_policy = NetworkSandboxPolicy::Restricted; let sandbox_policy = @@ -150,16 +150,24 @@ fn file_system_policy_has_cwd_dependent_entries( }) } -fn helper_read_root(runtime_paths: &ExecServerRuntimePaths) -> Option { - runtime_paths - .codex_self_exe - .parent() - .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) +fn helper_read_roots(runtime_paths: &ExecServerRuntimePaths) -> Vec { + let mut roots = Vec::new(); + for path in std::iter::once(runtime_paths.codex_self_exe.as_path()) + .chain(runtime_paths.codex_linux_sandbox_exe.as_deref().into_iter()) + { + if let Some(parent) = path.parent() + && let Ok(root) = AbsolutePathBuf::from_absolute_path(parent) + && !roots.contains(&root) + { + roots.push(root); + } + } + roots } fn add_helper_runtime_permissions( file_system_policy: &mut FileSystemSandboxPolicy, - helper_read_root: Option, + helper_read_roots: &[AbsolutePathBuf], cwd: &std::path::Path, ) { if !file_system_policy.has_full_disk_read_access() { @@ -174,19 +182,18 @@ fn add_helper_runtime_permissions( } } - let Some(helper_read_root) = helper_read_root else { - return; - }; - if file_system_policy.can_read_path_with_cwd(helper_read_root.as_path(), cwd) { - return; - } + for helper_read_root in helper_read_roots { + if file_system_policy.can_read_path_with_cwd(helper_read_root.as_path(), cwd) { + continue; + } - file_system_policy.entries.push(FileSystemSandboxEntry { - path: FileSystemPath::Path { - path: helper_read_root, - }, - access: FileSystemAccessMode::Read, - }); + file_system_policy.entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: helper_read_root.clone(), + }, + access: FileSystemAccessMode::Read, + }); + } } fn compatibility_sandbox_policy( @@ -371,7 +378,7 @@ mod tests { use super::helper_env; use super::helper_env_from_vars; use super::helper_env_key_is_allowed; - use super::helper_read_root; + use super::helper_read_roots; use super::sandbox_cwd; #[test] @@ -388,7 +395,7 @@ mod tests { let mut policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path()); - add_helper_runtime_permissions(&mut policy, /*helper_read_root*/ None, cwd.as_path()); + add_helper_runtime_permissions(&mut policy, /*helper_read_roots*/ &[], cwd.as_path()); assert!(policy.include_platform_defaults()); } @@ -410,7 +417,7 @@ mod tests { let mut policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path()); - add_helper_runtime_permissions(&mut policy, /*helper_read_root*/ None, cwd.as_path()); + add_helper_runtime_permissions(&mut policy, /*helper_read_roots*/ &[], cwd.as_path()); assert!(policy.include_platform_defaults()); } @@ -449,7 +456,7 @@ mod tests { add_helper_runtime_permissions( &mut policy, - helper_read_root(&runtime_paths), + &helper_read_roots(&runtime_paths), cwd.as_path(), ); @@ -611,10 +618,44 @@ mod tests { add_helper_runtime_permissions( &mut policy, - helper_read_root(&runtime_paths), + &helper_read_roots(&runtime_paths), cwd.as_path(), ); assert!(policy.can_read_path_with_cwd(readable.as_path(), cwd.as_path())); } + + #[test] + fn helper_permissions_include_linux_sandbox_alias_parent() { + let root = tempfile::tempdir().expect("temp dir"); + let codex_self_exe = root.path().join("bin").join("codex"); + let codex_linux_sandbox_exe = root.path().join("aliases").join("codex-linux-sandbox"); + let runtime_paths = + ExecServerRuntimePaths::new(codex_self_exe, Some(codex_linux_sandbox_exe)) + .expect("runtime paths"); + let cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path()) + .expect("absolute cwd"); + let sandbox_policy = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: Vec::new(), + }, + network_access: false, + }; + let mut policy = + FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path()); + let codex_parent = AbsolutePathBuf::from_absolute_path(root.path().join("bin")) + .expect("absolute codex parent"); + let alias_parent = AbsolutePathBuf::from_absolute_path(root.path().join("aliases")) + .expect("absolute alias parent"); + + add_helper_runtime_permissions( + &mut policy, + &helper_read_roots(&runtime_paths), + cwd.as_path(), + ); + + assert!(policy.can_read_path_with_cwd(codex_parent.as_path(), cwd.as_path())); + assert!(policy.can_read_path_with_cwd(alias_parent.as_path(), cwd.as_path())); + } }