From 60f0ab6cbfe464dd6fc2603e53604995423714db Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 23 Sep 2026 17:39:56 +0000 Subject: [PATCH] Use the shared process launcher for Unix shell snapshots (#47611) ## What changed Launch Unix shell snapshot commands through `codex_utils_pty::Command` with `ProcessMode::NewSession`, replacing the custom `pre_exec` hook. Pass the complete, scrubbed child environment and preserve the original command arguments so NUL bytes are rejected before execution. ## Testing Add a Unix regression test asserting that a snapshot script containing a NUL byte returns `InvalidInput` without creating the file it would otherwise write. GitOrigin-RevId: ef68a6c9f64d7a541498d3ecc4113fad8ff7ed14 --- codex-rs/core/src/shell_snapshot.rs | 35 +++++++++++++++++------ codex-rs/core/src/shell_snapshot_tests.rs | 29 +++++++++++++++++++ 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/codex-rs/core/src/shell_snapshot.rs b/codex-rs/core/src/shell_snapshot.rs index 4aecafa091..880474c65a 100644 --- a/codex-rs/core/src/shell_snapshot.rs +++ b/codex-rs/core/src/shell_snapshot.rs @@ -1053,20 +1053,39 @@ async fn run_script_with_timeout( handler.args(&args[1..]); handler.stdin(Stdio::null()); handler.current_dir(cwd); + #[cfg(unix)] + { + // The shared launcher needs the complete child environment. + handler.env_clear(); + handler.envs(std::env::vars_os()); + } if let Some(env) = prepared_env { handler.env_clear(); handler.envs(env); } codex_protocol::shell_environment::scrub_non_inheritable_env_vars(handler.as_std_mut()); #[cfg(unix)] - unsafe { - handler.pre_exec(|| { - codex_utils_pty::process_group::detach_from_tty()?; - Ok(()) - }); - } - handler.kill_on_drop(true); - let output = timeout(snapshot_timeout, handler.output()) + let output = { + let settings = handler.as_std(); + // Preserve original inputs: std replaces strings containing NUL bytes. + let mut command = codex_utils_pty::Command::new(&args[0]); + command + .args(&args[1..]) + .envs( + settings + .get_envs() + .filter_map(|(key, value)| value.map(|value| (key, value))), + ) + .current_dir(cwd) + .stdin(codex_utils_pty::ChildStdin::File( + std::fs::File::open("/dev/null")?.into(), + )) + .process_mode(codex_utils_pty::ProcessMode::NewSession); + async move { command.spawn()?.wait_with_output().await } + }; + #[cfg(not(unix))] + let output = handler.kill_on_drop(true).output(); + let output = timeout(snapshot_timeout, output) .await .map_err(|_| anyhow!("Snapshot command timed out for {shell_name}"))? .with_context(|| format!("Failed to execute {shell_name}"))?; diff --git a/codex-rs/core/src/shell_snapshot_tests.rs b/codex-rs/core/src/shell_snapshot_tests.rs index 45b2b9cdae..944a41f116 100644 --- a/codex-rs/core/src/shell_snapshot_tests.rs +++ b/codex-rs/core/src/shell_snapshot_tests.rs @@ -1906,3 +1906,32 @@ fn set_file_mtime(path: &Path, age: Duration) -> Result<()> { } Ok(()) } + +#[cfg(unix)] +#[tokio::test] +async fn snapshot_rejects_nul_script_before_execution() -> Result<()> { + let root = tempdir()?; + let shell = Shell { + shell_type: ShellType::Sh, + shell_path: PathBuf::from("/bin/sh"), + }; + let error = run_script_with_timeout( + &shell, + "printf ran > ran\0", + Duration::from_secs(5), + SnapshotShellMode::NonLogin, + &root.path().abs(), + /*credential_broker*/ None, + /*sandbox*/ None, + ) + .await + .expect_err("invalid snapshot script must fail to spawn"); + assert_eq!( + error + .downcast_ref::() + .map(std::io::Error::kind), + Some(std::io::ErrorKind::InvalidInput) + ); + assert!(!root.path().join("ran").exists()); + Ok(()) +}