From 0226fa6df98d41f61dc412903fce91b7ec45e657 Mon Sep 17 00:00:00 2001 From: Alex Kotliarskyi Date: Fri, 16 Jan 2026 16:35:39 -0800 Subject: [PATCH] Detach non-tty child spawns --- codex-rs/core/src/exec.rs | 2 ++ codex-rs/core/src/landlock.rs | 1 + codex-rs/core/src/seatbelt.rs | 1 + codex-rs/core/src/spawn.rs | 12 +++++-- .../core/src/unified_exec/process_manager.rs | 29 +++++++++++----- codex-rs/utils/pty/src/pipe.rs | 33 +++++++++++++++++-- codex-rs/utils/pty/src/process_group.rs | 22 +++++++++++++ 7 files changed, 86 insertions(+), 14 deletions(-) diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 2a91873238..b803432730 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -549,6 +549,7 @@ async fn exec( )) })?; let arg0_ref = arg0.as_deref(); + let detach_from_tty = matches!(sandbox, SandboxType::None); let child = spawn_child_async( PathBuf::from(program), args.into(), @@ -557,6 +558,7 @@ async fn exec( sandbox_policy, StdioPolicy::RedirectForShellTool, env, + detach_from_tty, ) .await?; consume_truncated_output(child, expiration, stdout_stream).await diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index 340aebff2c..5944ef27fb 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -35,6 +35,7 @@ where sandbox_policy, stdio_policy, env, + false, ) .await } diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index a15ebb177b..4a68fdc803 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -39,6 +39,7 @@ pub async fn spawn_command_under_seatbelt( sandbox_policy, stdio_policy, env, + false, ) .await } diff --git a/codex-rs/core/src/spawn.rs b/codex-rs/core/src/spawn.rs index c1a8d4457a..cce49940f1 100644 --- a/codex-rs/core/src/spawn.rs +++ b/codex-rs/core/src/spawn.rs @@ -35,6 +35,7 @@ pub enum StdioPolicy { /// For now, we take `SandboxPolicy` as a parameter to spawn_child() because /// we need to determine whether to set the /// `CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR` environment variable. +#[allow(clippy::too_many_arguments)] pub(crate) async fn spawn_child_async( program: PathBuf, args: Vec, @@ -43,6 +44,7 @@ pub(crate) async fn spawn_child_async( sandbox_policy: &SandboxPolicy, stdio_policy: StdioPolicy, env: HashMap, + detach_from_tty: bool, ) -> std::io::Result { trace!( "spawn_child_async: {program:?} {args:?} {arg0:?} {cwd:?} {sandbox_policy:?} {stdio_policy:?} {env:?}" @@ -66,12 +68,16 @@ pub(crate) async fn spawn_child_async( #[cfg(unix)] unsafe { - let set_process_group = matches!(stdio_policy, StdioPolicy::RedirectForShellTool); + let isolate_process_group = matches!(stdio_policy, StdioPolicy::RedirectForShellTool); #[cfg(target_os = "linux")] let parent_pid = libc::getpid(); cmd.pre_exec(move || { - if set_process_group { - codex_utils_pty::process_group::set_process_group()?; + if isolate_process_group { + if detach_from_tty { + codex_utils_pty::process_group::detach_from_tty()?; + } else { + codex_utils_pty::process_group::set_process_group()?; + } } // This relies on prctl(2), so it only works on Linux. diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 3230e75b15..923aabd7f6 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -10,6 +10,7 @@ use tokio::time::Duration; use tokio::time::Instant; use tokio_util::sync::CancellationToken; +use crate::exec::SandboxType; use crate::exec_env::create_env; use crate::protocol::ExecCommandSource; use crate::sandboxing::ExecEnv; @@ -470,14 +471,26 @@ impl UnifiedExecProcessManager { ) .await } else { - codex_utils_pty::pipe::spawn_process_no_stdin( - program, - args, - env.cwd.as_path(), - &env.env, - &env.arg0, - ) - .await + let detach_from_tty = matches!(env.sandbox, SandboxType::None); + if detach_from_tty { + codex_utils_pty::pipe::spawn_process_no_stdin_detached( + program, + args, + env.cwd.as_path(), + &env.env, + &env.arg0, + ) + .await + } else { + codex_utils_pty::pipe::spawn_process_no_stdin( + program, + args, + env.cwd.as_path(), + &env.env, + &env.arg0, + ) + .await + } }; let spawned = spawn_result.map_err(|err| UnifiedExecError::create_process(err.to_string()))?; diff --git a/codex-rs/utils/pty/src/pipe.rs b/codex-rs/utils/pty/src/pipe.rs index 5d9eb0232d..037b3e5c66 100644 --- a/codex-rs/utils/pty/src/pipe.rs +++ b/codex-rs/utils/pty/src/pipe.rs @@ -103,6 +103,7 @@ async fn spawn_process_with_stdin_mode( env: &HashMap, arg0: &Option, stdin_mode: PipeStdinMode, + detach_from_tty: bool, ) -> Result { if program.is_empty() { anyhow::bail!("missing program for pipe spawn"); @@ -118,7 +119,11 @@ async fn spawn_process_with_stdin_mode( #[cfg(unix)] unsafe { command.pre_exec(move || { - crate::process_group::set_process_group()?; + if detach_from_tty { + crate::process_group::detach_from_tty()?; + } else { + crate::process_group::set_process_group()?; + } #[cfg(target_os = "linux")] crate::process_group::set_parent_death_signal(parent_pid)?; Ok(()) @@ -253,7 +258,7 @@ pub async fn spawn_process( env: &HashMap, arg0: &Option, ) -> Result { - spawn_process_with_stdin_mode(program, args, cwd, env, arg0, PipeStdinMode::Piped).await + spawn_process_with_stdin_mode(program, args, cwd, env, arg0, PipeStdinMode::Piped, false).await } /// Spawn a process using regular pipes, but close stdin immediately. @@ -264,5 +269,27 @@ pub async fn spawn_process_no_stdin( env: &HashMap, arg0: &Option, ) -> Result { - spawn_process_with_stdin_mode(program, args, cwd, env, arg0, PipeStdinMode::Null).await + spawn_process_with_stdin_mode(program, args, cwd, env, arg0, PipeStdinMode::Null, false).await +} + +/// Spawn a process using regular pipes, but detach it from the controlling TTY. +pub async fn spawn_process_detached( + program: &str, + args: &[String], + cwd: &Path, + env: &HashMap, + arg0: &Option, +) -> Result { + spawn_process_with_stdin_mode(program, args, cwd, env, arg0, PipeStdinMode::Piped, true).await +} + +/// Spawn a process using regular pipes, close stdin immediately, and detach it from the TTY. +pub async fn spawn_process_no_stdin_detached( + program: &str, + args: &[String], + cwd: &Path, + env: &HashMap, + arg0: &Option, +) -> Result { + spawn_process_with_stdin_mode(program, args, cwd, env, arg0, PipeStdinMode::Null, true).await } diff --git a/codex-rs/utils/pty/src/process_group.rs b/codex-rs/utils/pty/src/process_group.rs index ae77a36be0..dadff29f9e 100644 --- a/codex-rs/utils/pty/src/process_group.rs +++ b/codex-rs/utils/pty/src/process_group.rs @@ -4,6 +4,8 @@ //! command can be cleaned up reliably: //! - `set_process_group` is called in `pre_exec` so the child starts its own //! process group. +//! - `detach_from_tty` starts a new session so non-interactive children do not +//! inherit the controlling TTY. //! - `kill_process_group_by_pid` targets the whole group (children/grandchildren) //! - `kill_process_group` targets a known process group ID directly //! instead of a single PID. @@ -42,6 +44,26 @@ pub fn set_parent_death_signal(_parent_pid: i32) -> io::Result<()> { Ok(()) } +#[cfg(unix)] +/// Detach from the controlling TTY by starting a new session. +pub fn detach_from_tty() -> io::Result<()> { + let result = unsafe { libc::setsid() }; + if result == -1 { + let err = io::Error::last_os_error(); + if err.raw_os_error() == Some(libc::EPERM) { + return set_process_group(); + } + return Err(err); + } + Ok(()) +} + +#[cfg(not(unix))] +/// No-op on non-Unix platforms. +pub fn detach_from_tty() -> io::Result<()> { + Ok(()) +} + #[cfg(unix)] /// Put the calling process into its own process group. ///