From 9b33613db62526359686349bf717f93532807849 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Tue, 21 Jul 2026 22:01:40 +0000 Subject: [PATCH] Terminate Windows process trees with job objects (#34624) ## Why Terminating a Windows execution session must also stop child processes, while a normal root-process exit must continue to allow background descendants to run. ## What changed - Assign Windows pipe, ConPTY, and sandbox processes to job objects and terminate the job when a session is cancelled, times out, or is explicitly stopped. - Preserve descendants when the root process exits normally. - Attach ConPTY and sandbox processes to their jobs atomically at creation; keep root-process termination as a fallback where job setup is unavailable. ## Testing Added Windows coverage for descendant termination and preservation across pipe, ConPTY, capture, cancellation, and legacy sandbox execution paths. GitOrigin-RevId: 8f831f2fc4caaa7b79ce842a3ed7192bd02dd3b4 --- codex-rs/utils/pty/Cargo.toml | 1 + codex-rs/utils/pty/src/lib.rs | 2 + codex-rs/utils/pty/src/pipe.rs | 69 ++++- codex-rs/utils/pty/src/pipe_tests.rs | 19 ++ codex-rs/utils/pty/src/win/job.rs | 122 ++++++++ codex-rs/utils/pty/src/win/mod.rs | 58 ++-- codex-rs/utils/pty/src/win/procthreadattr.rs | 44 ++- codex-rs/utils/pty/src/win/psuedocon.rs | 11 +- codex-rs/utils/pty/src/windows_tests.rs | 142 +++++++++ .../src/bin/command_runner/win.rs | 116 ++++---- codex-rs/windows-sandbox-rs/src/conpty/mod.rs | 15 +- codex-rs/windows-sandbox-rs/src/lib.rs | 29 +- .../src/proc_thread_attr.rs | 63 ++-- codex-rs/windows-sandbox-rs/src/process.rs | 157 +++++----- .../src/unified_exec/backends/legacy.rs | 65 +++-- .../src/unified_exec/tests.rs | 274 +++++++++++++++++- 16 files changed, 946 insertions(+), 241 deletions(-) create mode 100644 codex-rs/utils/pty/src/pipe_tests.rs create mode 100644 codex-rs/utils/pty/src/win/job.rs diff --git a/codex-rs/utils/pty/Cargo.toml b/codex-rs/utils/pty/Cargo.toml index f38e8f7a63b8..d03962edc71a 100644 --- a/codex-rs/utils/pty/Cargo.toml +++ b/codex-rs/utils/pty/Cargo.toml @@ -22,6 +22,7 @@ log = { workspace = true } shared_library = "0.1.9" winapi = { version = "0.3.9", features = [ "handleapi", + "jobapi2", "minwinbase", "processthreadsapi", "synchapi", diff --git a/codex-rs/utils/pty/src/lib.rs b/codex-rs/utils/pty/src/lib.rs index ff0d1684432e..24c667a0ac0c 100644 --- a/codex-rs/utils/pty/src/lib.rs +++ b/codex-rs/utils/pty/src/lib.rs @@ -38,6 +38,8 @@ pub use pty::conpty_supported; /// Spawn a process attached to a PTY for interactive use. pub use pty::spawn_process as spawn_pty_process; #[cfg(windows)] +pub use win::JobObject; +#[cfg(windows)] pub use win::PsuedoCon; #[cfg(windows)] pub use win::conpty::RawConPty; diff --git a/codex-rs/utils/pty/src/pipe.rs b/codex-rs/utils/pty/src/pipe.rs index d862493b99eb..bc84d2baebb8 100644 --- a/codex-rs/utils/pty/src/pipe.rs +++ b/codex-rs/utils/pty/src/pipe.rs @@ -26,9 +26,15 @@ use crate::process::exit_code_from_status; #[cfg(target_os = "linux")] use libc; +#[cfg(windows)] +enum WindowsChildTerminator { + Job(Arc), + Process(u32), +} + struct PipeChildTerminator { #[cfg(windows)] - pid: u32, + windows: WindowsChildTerminator, #[cfg(unix)] process_group_id: u32, } @@ -58,7 +64,10 @@ impl ChildTerminator for PipeChildTerminator { #[cfg(windows)] { - kill_process(self.pid) + match &self.windows { + WindowsChildTerminator::Job(job) => job.terminate(), + WindowsChildTerminator::Process(pid) => kill_process(*pid), + } } #[cfg(not(any(unix, windows)))] @@ -109,6 +118,8 @@ enum PipeStdinMode { Null, } +/// On Windows, process-tree containment is best-effort because Tokio returns +/// only after the root process starts, so job assignment cannot be atomic. async fn spawn_process_with_stdin_mode( program: &str, args: &[String], @@ -165,12 +176,37 @@ async fn spawn_process_with_stdin_mode( command.stdout(Stdio::piped()); command.stderr(Stdio::piped()); + #[cfg(windows)] + let job = crate::win::JobObject::create().map(Arc::new); let mut child = command.spawn()?; - let pid = child + #[cfg(windows)] + let windows_terminator = { + // Accept the small race: a descendant created between spawn and + // assignment is not guaranteed to join the job and can escape termination. + let pid = child + .id() + .ok_or_else(|| io::Error::other("missing child pid"))?; + let assigned_job = job.and_then(|job| { + let process_handle = child + .raw_handle() + .ok_or_else(|| io::Error::other("missing child process handle"))?; + job.assign_process(process_handle)?; + Ok(job) + }); + match assigned_job { + Ok(job) => WindowsChildTerminator::Job(job), + Err(err) => { + log::warn!( + "Windows pipe process tree containment unavailable for pid {pid}: {err}" + ); + WindowsChildTerminator::Process(pid) + } + } + }; + #[cfg(unix)] + let process_group_id = child .id() .ok_or_else(|| io::Error::other("missing child pid"))?; - #[cfg(unix)] - let process_group_id = pid; let stdin = child.stdin.take(); let stdout = child.stdout.take(); @@ -225,9 +261,24 @@ async fn spawn_process_with_stdin_mode( let wait_exit_status = Arc::clone(&exit_status); let exit_code = Arc::new(StdMutex::new(None)); let wait_exit_code = Arc::clone(&exit_code); + #[cfg(windows)] + let wait_job = match &windows_terminator { + WindowsChildTerminator::Job(job) => Some(Arc::clone(job)), + WindowsChildTerminator::Process(_) => None, + }; let wait_handle: JoinHandle<()> = tokio::spawn(async move { let code = match child.wait().await { - Ok(status) => exit_code_from_status(status), + Ok(status) => { + #[cfg(windows)] + if let Some(job) = wait_job + && let Err(err) = job.preserve_descendants() + { + log::warn!( + "Windows pipe failed to preserve descendants after root exit: {err}" + ); + } + exit_code_from_status(status) + } Err(_) => -1, }; wait_exit_status.store(true, std::sync::atomic::Ordering::SeqCst); @@ -241,7 +292,7 @@ async fn spawn_process_with_stdin_mode( writer_tx, Box::new(PipeChildTerminator { #[cfg(windows)] - pid, + windows: windows_terminator, #[cfg(unix)] process_group_id, }), @@ -306,3 +357,7 @@ pub async fn spawn_process_no_stdin( ) .await } + +#[cfg(all(test, windows))] +#[path = "pipe_tests.rs"] +mod tests; diff --git a/codex-rs/utils/pty/src/pipe_tests.rs b/codex-rs/utils/pty/src/pipe_tests.rs new file mode 100644 index 000000000000..185ca656c45b --- /dev/null +++ b/codex-rs/utils/pty/src/pipe_tests.rs @@ -0,0 +1,19 @@ +use super::*; + +#[test] +fn process_fallback_terminates_root() -> anyhow::Result<()> { + let mut child = std::process::Command::new("ping.exe") + .args(["-n", "60", "127.0.0.1"]) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn()?; + let mut terminator = PipeChildTerminator { + windows: WindowsChildTerminator::Process(child.id()), + }; + + terminator.kill()?; + + assert!(!child.wait()?.success()); + Ok(()) +} diff --git a/codex-rs/utils/pty/src/win/job.rs b/codex-rs/utils/pty/src/win/job.rs new file mode 100644 index 000000000000..8073360573ba --- /dev/null +++ b/codex-rs/utils/pty/src/win/job.rs @@ -0,0 +1,122 @@ +use filedescriptor::OwnedHandle; +use std::io; +use std::os::windows::io::AsRawHandle; +use std::os::windows::io::FromRawHandle; +use std::os::windows::io::RawHandle; +use std::sync::Mutex; +use winapi::um::jobapi2::AssignProcessToJobObject; +use winapi::um::jobapi2::CreateJobObjectW; +use winapi::um::jobapi2::SetInformationJobObject; +use winapi::um::jobapi2::TerminateJobObject; +use winapi::um::winnt::JOB_OBJECT_LIMIT_BREAKAWAY_OK; +use winapi::um::winnt::JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; +use winapi::um::winnt::JOBOBJECT_EXTENDED_LIMIT_INFORMATION; +use winapi::um::winnt::JobObjectExtendedLimitInformation; + +/// Owns a Windows Job Object used to terminate a spawned process tree. +#[derive(Debug)] +pub struct JobObject { + handle: OwnedHandle, + // A mutex makes the state check, Job Object API call, and state update + // atomic with respect to concurrent preserve and terminate requests. + preserve_descendants: Mutex, +} + +impl JobObject { + /// Creates a Job Object configured to terminate all members when its last handle closes. + pub fn create() -> io::Result { + let handle = unsafe { CreateJobObjectW(std::ptr::null_mut(), std::ptr::null()) }; + if handle.is_null() { + return Err(io::Error::last_os_error()); + } + let handle = unsafe { OwnedHandle::from_raw_handle(handle.cast()) }; + + Self::set_limit_flags( + &handle, + JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE | JOB_OBJECT_LIMIT_BREAKAWAY_OK, + )?; + + Ok(Self { + handle, + preserve_descendants: Mutex::new(false), + }) + } + + fn set_limit_flags(handle: &OwnedHandle, flags: u32) -> io::Result<()> { + let mut limits: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = unsafe { std::mem::zeroed() }; + limits.BasicLimitInformation.LimitFlags = flags; + let configured = unsafe { + SetInformationJobObject( + handle.as_raw_handle().cast(), + JobObjectExtendedLimitInformation, + std::ptr::addr_of_mut!(limits).cast(), + std::mem::size_of::() as u32, + ) + }; + if configured == 0 { + return Err(io::Error::last_os_error()); + } + Ok(()) + } + + /// Assigns a running process to this job. + /// + /// Assignment is not retroactive: descendants created before this call + /// completes are not guaranteed to become members of the job. + pub(crate) fn assign_process(&self, process_handle: RawHandle) -> io::Result<()> { + let assigned = unsafe { + AssignProcessToJobObject(self.handle.as_raw_handle().cast(), process_handle.cast()) + }; + if assigned == 0 { + Err(io::Error::last_os_error()) + } else { + Ok(()) + } + } + + /// Allows contained descendants to keep running after the root exits normally. + /// + /// This disables both explicit job termination and kill-on-close for this + /// object. Calls race safely with [`Self::terminate`]: whichever operation + /// acquires the state lock first determines whether the process tree is + /// preserved or terminated. + pub fn preserve_descendants(&self) -> io::Result<()> { + let mut preserve_descendants = self + .preserve_descendants + .lock() + .map_err(|_| io::Error::other("job state lock poisoned"))?; + if *preserve_descendants { + return Ok(()); + } + + Self::set_limit_flags(&self.handle, JOB_OBJECT_LIMIT_BREAKAWAY_OK)?; + *preserve_descendants = true; + Ok(()) + } + + /// Terminates every process currently assigned to the job. + pub fn terminate(&self) -> io::Result<()> { + let preserve_descendants = self + .preserve_descendants + .lock() + .map_err(|_| io::Error::other("job state lock poisoned"))?; + if *preserve_descendants { + return Ok(()); + } + + let terminated = unsafe { + TerminateJobObject(self.handle.as_raw_handle().cast(), /*uExitCode*/ 1) + }; + if terminated == 0 { + Err(io::Error::last_os_error()) + } else { + Ok(()) + } + } +} + +impl AsRawHandle for JobObject { + fn as_raw_handle(&self) -> RawHandle { + self.handle.as_raw_handle() + } +} diff --git a/codex-rs/utils/pty/src/win/mod.rs b/codex-rs/utils/pty/src/win/mod.rs index cfc53cb51e2b..05063cf66a69 100644 --- a/codex-rs/utils/pty/src/win/mod.rs +++ b/codex-rs/utils/pty/src/win/mod.rs @@ -19,12 +19,8 @@ // SOFTWARE. // Local modifications: -// - Fix Codex bug #13945 in the Windows PTY kill path. The vendored code treated -// `TerminateProcess`'s nonzero success return as failure and `0` as success, -// which inverts kill outcomes for both `WinChild::do_kill` and -// `WinChildKiller::kill`. -// - This bug still exists in the original WezTerm source as of 2026-03-08, so -// this is an intentional divergence from upstream. +// - Place spawned processes in a Job Object so kill operations terminate the +// full process tree, while normal root exit preserves background descendants. use anyhow::Context as _; use filedescriptor::OwnedHandle; @@ -35,6 +31,7 @@ use std::io::Error as IoError; use std::io::Result as IoResult; use std::os::windows::io::AsRawHandle; use std::pin::Pin; +use std::sync::Arc; use std::sync::Mutex; use std::task::Context; use std::task::Poll; @@ -45,19 +42,29 @@ use winapi::um::synchapi::WaitForSingleObject; use winapi::um::winbase::INFINITE; pub(crate) mod conpty; +mod job; mod procthreadattr; mod psuedocon; pub use conpty::ConPtySystem; +pub use job::JobObject; pub use psuedocon::PsuedoCon; pub use psuedocon::conpty_supported; #[derive(Debug)] pub struct WinChild { proc: Mutex, + job: Arc, } impl WinChild { + pub(crate) fn new(proc: OwnedHandle, job: Arc) -> Self { + Self { + proc: Mutex::new(proc), + job, + } + } + fn is_complete(&mut self) -> IoResult> { let mut status: DWORD = 0; let proc = self.proc.lock().unwrap().try_clone().unwrap(); @@ -66,6 +73,7 @@ impl WinChild { if status == STILL_ACTIVE { Ok(None) } else { + self.preserve_descendants(); Ok(Some(ExitStatus::with_exit_code(status))) } } else { @@ -74,48 +82,45 @@ impl WinChild { } fn do_kill(&mut self) -> IoResult<()> { - let proc = self.proc.lock().unwrap().try_clone().unwrap(); - let res = unsafe { TerminateProcess(proc.as_raw_handle() as _, 1) }; - // Codex bug #13945: Win32 returns nonzero on success, so only `0` is an error. - if res == 0 { - Err(IoError::last_os_error()) - } else { - Ok(()) + self.job.terminate() + } + + fn preserve_descendants(&self) { + if let Err(err) = self.job.preserve_descendants() { + log::warn!("ConPTY failed to preserve descendants after root exit: {err}"); } } } impl ChildKiller for WinChild { fn kill(&mut self) -> IoResult<()> { - self.do_kill().ok(); + if let Err(err) = self.do_kill() { + log::warn!("ConPTY failed to terminate process tree: {err}"); + } Ok(()) } fn clone_killer(&self) -> Box { - let proc = self.proc.lock().unwrap().try_clone().unwrap(); - Box::new(WinChildKiller { proc }) + Box::new(WinChildKiller { + job: Arc::clone(&self.job), + }) } } #[derive(Debug)] pub struct WinChildKiller { - proc: OwnedHandle, + job: Arc, } impl ChildKiller for WinChildKiller { fn kill(&mut self) -> IoResult<()> { - let res = unsafe { TerminateProcess(self.proc.as_raw_handle() as _, 1) }; - // Codex bug #13945: Win32 returns nonzero on success, so only `0` is an error. - if res == 0 { - Err(IoError::last_os_error()) - } else { - Ok(()) - } + self.job.terminate() } fn clone_killer(&self) -> Box { - let proc = self.proc.try_clone().unwrap(); - Box::new(WinChildKiller { proc }) + Box::new(WinChildKiller { + job: Arc::clone(&self.job), + }) } } @@ -135,6 +140,7 @@ impl Child for WinChild { let mut status: DWORD = 0; let res = unsafe { GetExitCodeProcess(proc.as_raw_handle() as _, &mut status) }; if res != 0 { + self.preserve_descendants(); Ok(ExitStatus::with_exit_code(status)) } else { Err(IoError::last_os_error()) diff --git a/codex-rs/utils/pty/src/win/procthreadattr.rs b/codex-rs/utils/pty/src/win/procthreadattr.rs index c7cf68fed9aa..6b4726afccc3 100644 --- a/codex-rs/utils/pty/src/win/procthreadattr.rs +++ b/codex-rs/utils/pty/src/win/procthreadattr.rs @@ -21,16 +21,20 @@ use super::psuedocon::HPCON; use anyhow::Error; use anyhow::ensure; +use std::ffi::c_void; use std::io::Error as IoError; use std::mem; use std::ptr; use winapi::shared::minwindef::DWORD; use winapi::um::processthreadsapi::*; +use winapi::um::winnt::HANDLE; +const PROC_THREAD_ATTRIBUTE_JOB_LIST: usize = 0x0002000D; const PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE: usize = 0x00020016; pub struct ProcThreadAttributeList { data: Vec, + job_list: Vec, } impl ProcThreadAttributeList { @@ -56,7 +60,10 @@ impl ProcThreadAttributeList { "InitializeProcThreadAttributeList failed: {}", IoError::last_os_error() ); - Ok(Self { data }) + Ok(Self { + data, + job_list: Vec::new(), + }) } pub fn as_mut_ptr(&mut self) -> LPPROC_THREAD_ATTRIBUTE_LIST { @@ -64,13 +71,42 @@ impl ProcThreadAttributeList { } pub fn set_pty(&mut self, con: HPCON) -> Result<(), Error> { + // SAFETY: `con` is the Windows-defined value and size for this attribute. + unsafe { + self.update( + PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, + con, + mem::size_of::(), + ) + } + } + + pub fn set_job(&mut self, job: HANDLE) -> Result<(), Error> { + // Atomic job attachment is intentionally required for native ConPTY + // spawns. If Windows cannot honor the job list (for example, because a + // parent job forbids nesting), fail the spawn rather than briefly run + // an uncontained process tree. + self.job_list = vec![job]; + let value = self.job_list.as_mut_ptr().cast(); + let size = std::mem::size_of_val(self.job_list.as_slice()); + // SAFETY: `value` points to `self.job_list`, which remains alive while + // the attribute list can reference it, and `size` covers that slice. + unsafe { self.update(PROC_THREAD_ATTRIBUTE_JOB_LIST, value, size) } + } + + unsafe fn update( + &mut self, + attribute: usize, + value: *mut c_void, + size: usize, + ) -> Result<(), Error> { let res = unsafe { UpdateProcThreadAttribute( self.as_mut_ptr(), 0, - PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, - con, - mem::size_of::(), + attribute, + value, + size, ptr::null_mut(), ptr::null_mut(), ) diff --git a/codex-rs/utils/pty/src/win/psuedocon.rs b/codex-rs/utils/pty/src/win/psuedocon.rs index f235c70278f5..0727c58a7062 100644 --- a/codex-rs/utils/pty/src/win/psuedocon.rs +++ b/codex-rs/utils/pty/src/win/psuedocon.rs @@ -20,6 +20,7 @@ // SOFTWARE. use super::WinChild; +use crate::win::job::JobObject; use crate::win::procthreadattr::ProcThreadAttributeList; use anyhow::Error; use anyhow::bail; @@ -40,7 +41,7 @@ use std::os::windows::io::AsRawHandle; use std::os::windows::io::FromRawHandle; use std::path::Path; use std::ptr; -use std::sync::Mutex; +use std::sync::Arc; use winapi::shared::minwindef::DWORD; use winapi::shared::ntdef::NTSTATUS; use winapi::shared::ntstatus::STATUS_SUCCESS; @@ -173,6 +174,7 @@ impl PsuedoCon { } pub fn spawn_command(&self, cmd: CommandBuilder) -> anyhow::Result { + let job = Arc::new(JobObject::create()?); let mut si: STARTUPINFOEXW = unsafe { mem::zeroed() }; si.StartupInfo.cb = mem::size_of::() as u32; si.StartupInfo.dwFlags = STARTF_USESTDHANDLES; @@ -180,8 +182,9 @@ impl PsuedoCon { si.StartupInfo.hStdOutput = INVALID_HANDLE_VALUE; si.StartupInfo.hStdError = INVALID_HANDLE_VALUE; - let mut attrs = ProcThreadAttributeList::with_capacity(/*num_attributes*/ 1)?; + let mut attrs = ProcThreadAttributeList::with_capacity(/*num_attributes*/ 2)?; attrs.set_pty(self.con)?; + attrs.set_job(job.as_raw_handle().cast())?; si.lpAttributeList = attrs.as_mut_ptr(); let mut pi: PROCESS_INFORMATION = unsafe { mem::zeroed() }; @@ -221,9 +224,7 @@ impl PsuedoCon { let _main_thread = unsafe { OwnedHandle::from_raw_handle(pi.hThread as _) }; let proc = unsafe { OwnedHandle::from_raw_handle(pi.hProcess as _) }; - Ok(WinChild { - proc: Mutex::new(proc), - }) + Ok(WinChild::new(proc, job)) } } diff --git a/codex-rs/utils/pty/src/windows_tests.rs b/codex-rs/utils/pty/src/windows_tests.rs index 17829cb85a43..06e7fd0a51d6 100644 --- a/codex-rs/utils/pty/src/windows_tests.rs +++ b/codex-rs/utils/pty/src/windows_tests.rs @@ -3,9 +3,11 @@ use super::combine_spawned_output; use super::find_python; use super::wait_for_output_contains; use crate::TerminalSize; +use crate::spawn_pipe_process_no_stdin; use crate::spawn_pty_process; use std::collections::HashMap; use std::path::Path; +use std::time::Duration; const READY_MARKER: &str = "__CODEX_CHILD_READY__"; const VALUE_MARKER: &str = "__CODEX_CHILD_VALUE__"; @@ -39,6 +41,146 @@ fn utf8_hex(value: &str) -> String { .join("") } +async fn wait_for_path(path: &Path, timeout: Duration) -> bool { + let deadline = tokio::time::Instant::now() + timeout; + loop { + if path.exists() { + return true; + } + let remaining = deadline.saturating_duration_since(tokio::time::Instant::now()); + if remaining.is_zero() { + return false; + } + tokio::time::sleep(remaining.min(Duration::from_millis(25))).await; + } +} + +async fn assert_terminate_kills_descendant( + backend: &str, + python: &str, + env: &HashMap, +) -> anyhow::Result<()> { + let marker = std::env::temp_dir().join(format!( + "codex-job-descendant-{backend}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH)? + .as_nanos() + )); + let child_code = format!( + "import pathlib,time; print('{READY_MARKER}',flush=True); time.sleep(1); pathlib.Path(bytes.fromhex('{}').decode()).write_text('survived')", + utf8_hex(&marker.to_string_lossy()) + ); + // Exercise descendants created after the best-effort pipe assignment, + // without making the test depend on winning the intentionally accepted race. + let code = format!( + "import subprocess,sys,time; time.sleep(0.5); code=bytes.fromhex('{}').decode(); subprocess.Popen([sys.executable,'-u','-c',code]); time.sleep(60)", + utf8_hex(&child_code) + ); + let args = vec!["-u".to_string(), "-c".to_string(), code]; + let spawned = if backend == "pipe" { + spawn_pipe_process_no_stdin(python, &args, Path::new("."), env, /*arg0*/ &None).await? + } else { + spawn_pty_process( + python, + &args, + Path::new("."), + env, + /*arg0*/ &None, + TerminalSize::default(), + ) + .await? + }; + let (session, mut output_rx, exit_rx) = combine_spawned_output(spawned); + wait_for_output_contains(&mut output_rx, READY_MARKER, /*timeout_ms*/ 10_000).await?; + session.request_terminate(); + let (_, exit_code) = collect_output_until_exit(output_rx, exit_rx, /*timeout_ms*/ 10_000).await; + assert_ne!( + exit_code, -1, + "{backend} root did not exit after termination" + ); + tokio::time::sleep(Duration::from_secs(2)).await; + let survived = marker.exists(); + if survived { + std::fs::remove_file(&marker)?; + } + assert!(!survived, "{backend} descendant survived termination"); + Ok(()) +} + +async fn assert_normal_exit_preserves_descendant( + backend: &str, + python: &str, + env: &HashMap, +) -> anyhow::Result<()> { + let marker_base = std::env::temp_dir().join(format!( + "codex-job-natural-exit-{backend}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH)? + .as_nanos() + )); + let ready_marker = marker_base.with_extension("ready"); + let survival_marker = marker_base.with_extension("survived"); + let child_code = format!( + "import pathlib,time; pathlib.Path(bytes.fromhex('{}').decode()).write_text('ready'); time.sleep(1); pathlib.Path(bytes.fromhex('{}').decode()).write_text('survived')", + utf8_hex(&ready_marker.to_string_lossy()), + utf8_hex(&survival_marker.to_string_lossy()) + ); + let code = format!( + "import pathlib,subprocess,sys,time; code=bytes.fromhex('{}').decode(); ready=pathlib.Path(bytes.fromhex('{}').decode()); subprocess.Popen([sys.executable,'-u','-c',code],stdin=subprocess.DEVNULL,stdout=subprocess.DEVNULL,stderr=subprocess.DEVNULL,creationflags=subprocess.DETACHED_PROCESS|subprocess.CREATE_NEW_PROCESS_GROUP); deadline=time.time()+10\nwhile not ready.exists() and time.time() anyhow::Result<()> +{ + let Some(python) = find_python() else { + eprintln!("python not found; skipping Windows process-tree termination test"); + return Ok(()); + }; + let env: HashMap = std::env::vars().collect(); + assert_terminate_kills_descendant("pipe", &python, &env).await?; + assert_terminate_kills_descendant("ConPTY", &python, &env).await +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn normal_exit_preserves_descendants_for_pipe_and_conpty() -> anyhow::Result<()> { + let Some(python) = find_python() else { + eprintln!("python not found; skipping Windows process-tree natural-exit test"); + return Ok(()); + }; + let env: HashMap = std::env::vars().collect(); + assert_normal_exit_preserves_descendant("pipe", &python, &env).await?; + assert_normal_exit_preserves_descendant("ConPTY", &python, &env).await +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn conpty_delivers_input_to_foreground_children() -> anyhow::Result<()> { let Some(python) = find_python() else { diff --git a/codex-rs/windows-sandbox-rs/src/bin/command_runner/win.rs b/codex-rs/windows-sandbox-rs/src/bin/command_runner/win.rs index b38b35f307a5..41d1e5c8ca2d 100644 --- a/codex-rs/windows-sandbox-rs/src/bin/command_runner/win.rs +++ b/codex-rs/windows-sandbox-rs/src/bin/command_runner/win.rs @@ -13,6 +13,7 @@ mod cwd_junction; use anyhow::Context; use anyhow::Result; +use codex_utils_pty::JobObject; use codex_windows_sandbox::ConsoleMode; use codex_windows_sandbox::ErrorPayload; use codex_windows_sandbox::ErrorStage; @@ -63,12 +64,6 @@ use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_WRITE; use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING; use windows_sys::Win32::System::Console::COORD; use windows_sys::Win32::System::Console::ResizePseudoConsole; -use windows_sys::Win32::System::JobObjects::AssignProcessToJobObject; -use windows_sys::Win32::System::JobObjects::CreateJobObjectW; -use windows_sys::Win32::System::JobObjects::JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; -use windows_sys::Win32::System::JobObjects::JOBOBJECT_EXTENDED_LIMIT_INFORMATION; -use windows_sys::Win32::System::JobObjects::JobObjectExtendedLimitInformation; -use windows_sys::Win32::System::JobObjects::SetInformationJobObject; use windows_sys::Win32::System::Threading::GetExitCodeProcess; use windows_sys::Win32::System::Threading::GetProcessId; use windows_sys::Win32::System::Threading::INFINITE; @@ -82,11 +77,13 @@ use windows_sys::Win32::System::Threading::WaitForSingleObject; // a dependency cycle. const FS_HELPER_ARG: &str = "--codex-run-as-fs-helper"; const READ_ACL_MUTEX_NAME: &str = "Local\\CodexSandboxReadAcl"; +const TERMINATION_WAIT_MS: u32 = 5_000; const WAIT_TIMEOUT: u32 = 0x0000_0102; struct IpcSpawnedProcess { log_dir: PathBuf, pi: PROCESS_INFORMATION, + job: Arc, stdout_handle: HANDLE, stderr_handle: HANDLE, stdin_handle: Option, @@ -130,25 +127,6 @@ impl Drop for OwnedWinHandle { } } -unsafe fn create_job_kill_on_close() -> Result { - let h_job = OwnedWinHandle::new(CreateJobObjectW(std::ptr::null_mut(), std::ptr::null())); - if h_job.raw() == 0 { - return Err(anyhow::anyhow!("CreateJobObjectW failed")); - } - let mut limits: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = std::mem::zeroed(); - limits.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; - let ok = SetInformationJobObject( - h_job.raw(), - JobObjectExtendedLimitInformation, - &mut limits as *mut _ as *mut _, - std::mem::size_of::() as u32, - ); - if ok == 0 { - return Err(anyhow::anyhow!("SetInformationJobObject failed")); - } - Ok(h_job.into_raw()) -} - /// Open a named pipe created by the parent process. fn open_pipe(name: &str, access: u32) -> Result { let path = to_wide(name); @@ -321,7 +299,7 @@ fn spawn_ipc_process(req: &SpawnRequest) -> Result { let mut conpty_owner = None; let mut hpc_handle: Option = None; let mut pipe_handles = None; - let (pi, stdout_handle, stderr_handle, stdin_handle) = if req.tty { + let (pi, job, stdout_handle, stderr_handle, stdin_handle) = if req.tty { let (pi, mut conpty) = codex_windows_sandbox::spawn_conpty_process_as_user( h_token.raw(), &req.command, @@ -330,6 +308,9 @@ fn spawn_ipc_process(req: &SpawnRequest) -> Result { req.use_private_desktop, Some(log_dir.as_path()), )?; + let job = conpty + .job() + .context("spawned ConPTY is missing its process job")?; hpc_handle = conpty.raw_handle(); let input_write = conpty.take_input_write(); let output_read = conpty.take_output_read(); @@ -344,6 +325,7 @@ fn spawn_ipc_process(req: &SpawnRequest) -> Result { }; ( pi, + job, output_read, windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE, stdin_handle, @@ -375,12 +357,14 @@ fn spawn_ipc_process(req: &SpawnRequest) -> Result { .stderr_read .unwrap_or(windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE); let stdin_handle = spawned_pipes.stdin_write; + let job = spawned_pipes.job(); pipe_handles = Some(spawned_pipes); - (pi, stdout_handle, stderr_handle, stdin_handle) + (pi, job, stdout_handle, stderr_handle, stdin_handle) }; Ok(IpcSpawnedProcess { log_dir, pi, + job, stdout_handle, stderr_handle, stdin_handle, @@ -418,12 +402,30 @@ fn spawn_output_reader( }) } +fn terminate_job_or_process(job: &JobObject, process: HANDLE, log_dir: Option<&Path>) { + if let Err(job_err) = job.terminate() { + log_note( + &format!("runner failed to terminate process tree: {job_err}"), + log_dir, + ); + if unsafe { TerminateProcess(process, 1) } == 0 { + log_note( + &format!("runner failed to terminate root process: {}", unsafe { + GetLastError() + }), + log_dir, + ); + } + } +} + /// Read stdin/terminate frames and forward to the child process. fn spawn_input_loop( mut reader: File, stdin_handle: Option, hpc_handle: Arc>>, - process_handle: Arc>>, + job: Arc, + process: HANDLE, log_dir: Option, ) -> std::thread::JoinHandle<()> { std::thread::spawn(move || { @@ -516,13 +518,7 @@ fn spawn_input_loop( } } Message::Terminate { .. } => { - if let Ok(guard) = process_handle.lock() - && let Some(handle) = guard.as_ref() - { - unsafe { - let _ = TerminateProcess(*handle, 1); - } - } + terminate_job_or_process(&job, process, log_dir.as_deref()); } Message::SpawnRequest { .. } => {} Message::SpawnReady { .. } => {} @@ -597,19 +593,11 @@ pub fn main() -> Result<()> { let pi = ipc_spawn.pi; let stdout_handle = ipc_spawn.stdout_handle; let stderr_handle = ipc_spawn.stderr_handle; + let job = Arc::clone(&ipc_spawn.job); let mut conpty_owner = ipc_spawn.conpty_owner; let stdin_handle = ipc_spawn.stdin_handle; let hpc_handle = Arc::new(StdMutex::new(ipc_spawn.hpc_handle)); - let h_job = unsafe { create_job_kill_on_close().ok() }; - if let Some(job) = h_job { - unsafe { - let _ = AssignProcessToJobObject(job, pi.hProcess); - } - } - - let process_handle = Arc::new(StdMutex::new(Some(pi.hProcess))); - let msg = FramedMessage { version: IPC_PROTOCOL_VERSION, message: Message::SpawnReady { @@ -653,18 +641,39 @@ pub fn main() -> Result<()> { pipe_read, stdin_handle, Arc::clone(&hpc_handle), - Arc::clone(&process_handle), + Arc::clone(&job), + pi.hProcess, log_dir_owned, ); let timeout = req.timeout_ms.map(|ms| ms as u32).unwrap_or(INFINITE); let wait_res = unsafe { WaitForSingleObject(pi.hProcess, timeout) }; let timed_out = wait_res == WAIT_TIMEOUT; + let child_stopped = if timed_out { + terminate_job_or_process(&job, pi.hProcess, log_dir); + let termination_wait = unsafe { WaitForSingleObject(pi.hProcess, TERMINATION_WAIT_MS) }; + if termination_wait == WAIT_TIMEOUT { + log_note( + "runner root process did not exit after termination", + log_dir, + ); + false + } else { + true + } + } else { + if let Err(err) = job.preserve_descendants() { + log_note( + &format!("runner failed to preserve descendants after root exit: {err}"), + log_dir, + ); + } + true + }; let exit_code: i32; unsafe { if timed_out { - let _ = TerminateProcess(pi.hProcess, 1); exit_code = 128 + 64; } else { let mut raw_exit: u32 = 1; @@ -677,9 +686,6 @@ pub fn main() -> Result<()> { if pi.hProcess != 0 { CloseHandle(pi.hProcess); } - if let Some(job) = h_job { - CloseHandle(job); - } } if let Ok(mut guard) = hpc_handle.lock() { @@ -687,9 +693,15 @@ pub fn main() -> Result<()> { } drop(conpty_owner.take()); - let _ = out_thread.join(); - if let Some(thread) = err_thread { - let _ = thread.join(); + if child_stopped { + if out_thread.join().is_err() { + log_note("runner stdout reader thread panicked", log_dir); + } + if let Some(thread) = err_thread + && thread.join().is_err() + { + log_note("runner stderr reader thread panicked", log_dir); + } } let exit_msg = FramedMessage { diff --git a/codex-rs/windows-sandbox-rs/src/conpty/mod.rs b/codex-rs/windows-sandbox-rs/src/conpty/mod.rs index 09dc52b572a7..475a6c0f54ec 100644 --- a/codex-rs/windows-sandbox-rs/src/conpty/mod.rs +++ b/codex-rs/windows-sandbox-rs/src/conpty/mod.rs @@ -13,12 +13,15 @@ use crate::winutil::quote_windows_arg; use crate::winutil::to_wide; use anyhow::Context; use anyhow::Result; +use codex_utils_pty::JobObject; use codex_utils_pty::PsuedoCon; use codex_utils_pty::RawConPty; use std::collections::HashMap; use std::ffi::c_void; +use std::os::windows::io::AsRawHandle; use std::os::windows::io::IntoRawHandle; use std::path::Path; +use std::sync::Arc; use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::HANDLE; @@ -37,6 +40,7 @@ pub struct ConptyInstance { pseudoconsole: Option, input_write: HANDLE, output_read: HANDLE, + job: Option>, _desktop: Option, } @@ -68,6 +72,11 @@ impl ConptyInstance { pub fn take_output_read(&mut self) -> HANDLE { std::mem::replace(&mut self.output_read, 0) } + + /// Returns the Job Object containing the spawned process, if this instance owns one. + pub fn job(&self) -> Option> { + self.job.as_ref().map(Arc::clone) + } } /// Create a ConPTY with backing pipes. @@ -83,6 +92,7 @@ pub fn create_conpty(cols: i16, rows: i16) -> Result { pseudoconsole: Some(pseudoconsole), input_write: input_write.into_raw_handle() as HANDLE, output_read: output_read.into_raw_handle() as HANDLE, + job: None, _desktop: None, }) } @@ -114,6 +124,7 @@ pub fn spawn_conpty_process_as_user( si.StartupInfo.hStdError = INVALID_HANDLE_VALUE; let desktop = LaunchDesktop::prepare(use_private_desktop, logs_base_dir)?; si.StartupInfo.lpDesktop = desktop.startup_info_desktop(); + let job = Arc::new(JobObject::create().context("create process job")?); let raw = RawConPty::new(/*cols*/ 80, /*rows*/ 24)?; let (pseudoconsole, input_write, output_read) = raw.into_handles(); @@ -122,10 +133,12 @@ pub fn spawn_conpty_process_as_user( pseudoconsole: Some(pseudoconsole), input_write: input_write.into_raw_handle() as HANDLE, output_read: output_read.into_raw_handle() as HANDLE, + job: Some(Arc::clone(&job)), _desktop: Some(desktop), }; - let mut attrs = ProcThreadAttributeList::new(/*attr_count*/ 1)?; + let mut attrs = ProcThreadAttributeList::new(/*attr_count*/ 2)?; attrs.set_pseudoconsole(hpc)?; + attrs.set_job(job.as_raw_handle() as HANDLE)?; si.lpAttributeList = attrs.as_mut_ptr(); let mut pi: PROCESS_INFORMATION = unsafe { std::mem::zeroed() }; diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 7365e3882452..3f68aeab9ea5 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -364,6 +364,7 @@ pub use stub::run_windows_sandbox_legacy_preflight; mod windows_impl { use super::WindowsSandboxCancellationToken; use super::logging::log_failure; + use super::logging::log_note; use super::logging::log_success; use super::process::ConsoleMode; use super::process::create_process_as_user; @@ -383,6 +384,7 @@ mod windows_impl { use std::io; use std::path::Path; use std::ptr; + use std::sync::Arc; use std::time::Duration; use std::time::Instant; use windows_sys::Win32::Foundation::CloseHandle; @@ -603,6 +605,7 @@ mod windows_impl { } }; let pi = created.process_info; + let job = Arc::clone(&created.job); let _desktop = created; unsafe { @@ -666,10 +669,30 @@ mod windows_impl { unsafe { GetExitCodeProcess(pi.hProcess, &mut exit_code_u32); } - } else { - unsafe { - windows_sys::Win32::System::Threading::TerminateProcess(pi.hProcess, 1); + } + if timed_out || cancelled { + if let Err(job_err) = job.terminate() { + log_note( + &format!("capture failed to terminate process tree: {job_err}"), + logs_base_dir, + ); + let root_result = unsafe { + windows_sys::Win32::System::Threading::TerminateProcess(pi.hProcess, 1) + }; + if root_result == 0 { + log_note( + &format!("capture failed to terminate root process: {}", unsafe { + GetLastError() + }), + logs_base_dir, + ); + } } + } else if let Err(err) = job.preserve_descendants() { + log_note( + &format!("capture failed to preserve descendants after root exit: {err}"), + logs_base_dir, + ); } unsafe { diff --git a/codex-rs/windows-sandbox-rs/src/proc_thread_attr.rs b/codex-rs/windows-sandbox-rs/src/proc_thread_attr.rs index b81469983935..38110a69239e 100644 --- a/codex-rs/windows-sandbox-rs/src/proc_thread_attr.rs +++ b/codex-rs/windows-sandbox-rs/src/proc_thread_attr.rs @@ -8,11 +8,13 @@ use windows_sys::Win32::System::Threading::LPPROC_THREAD_ATTRIBUTE_LIST; use windows_sys::Win32::System::Threading::UpdateProcThreadAttribute; const PROC_THREAD_ATTRIBUTE_HANDLE_LIST: usize = 0x0002_0002; +const PROC_THREAD_ATTRIBUTE_JOB_LIST: usize = 0x0002_000D; const PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE: usize = 0x0002_0016; pub struct ProcThreadAttributeList { buffer: Vec, - handle_list: Option>, + handle_list: Vec, + job_list: Vec, } impl ProcThreadAttributeList { @@ -36,7 +38,8 @@ impl ProcThreadAttributeList { } Ok(Self { buffer, - handle_list: None, + handle_list: Vec::new(), + job_list: Vec::new(), }) } @@ -45,39 +48,51 @@ impl ProcThreadAttributeList { } pub fn set_pseudoconsole(&mut self, hpc: isize) -> io::Result<()> { - let list = self.as_mut_ptr(); - let ok = unsafe { - UpdateProcThreadAttribute( - list, - 0, + // SAFETY: `hpc` is the Windows-defined value and size for this attribute. + unsafe { + self.update( PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, hpc as *mut c_void, std::mem::size_of::(), - std::ptr::null_mut(), - std::ptr::null_mut(), ) - }; - if ok == 0 { - return Err(io::Error::from_raw_os_error(unsafe { - GetLastError() as i32 - })); } - Ok(()) } pub fn set_handle_list(&mut self, handles: Vec) -> io::Result<()> { - self.handle_list = Some(handles); - let list = self.as_mut_ptr(); - let Some(handle_list) = self.handle_list.as_mut() else { - return Err(io::Error::other("handle list missing after initialization")); - }; + self.handle_list = handles; + let value = self.handle_list.as_mut_ptr().cast(); + let size = std::mem::size_of_val(self.handle_list.as_slice()); + // SAFETY: `value` points to `self.handle_list`, which remains alive + // while the attribute list can reference it, and `size` covers that slice. + unsafe { self.update(PROC_THREAD_ATTRIBUTE_HANDLE_LIST, value, size) } + } + + pub fn set_job(&mut self, job: HANDLE) -> io::Result<()> { + // Sandboxed processes must enter the job atomically. If Windows cannot + // honor the job list (for example, because a parent job forbids + // nesting), fail the spawn rather than briefly run an uncontained + // sandbox process tree. + self.job_list = vec![job]; + let value = self.job_list.as_mut_ptr().cast(); + let size = std::mem::size_of_val(self.job_list.as_slice()); + // SAFETY: `value` points to `self.job_list`, which remains alive while + // the attribute list can reference it, and `size` covers that slice. + unsafe { self.update(PROC_THREAD_ATTRIBUTE_JOB_LIST, value, size) } + } + + unsafe fn update( + &mut self, + attribute: usize, + value: *mut c_void, + size: usize, + ) -> io::Result<()> { let ok = unsafe { UpdateProcThreadAttribute( - list, + self.as_mut_ptr(), 0, - PROC_THREAD_ATTRIBUTE_HANDLE_LIST, - handle_list.as_mut_ptr().cast(), - std::mem::size_of_val(handle_list.as_slice()), + attribute, + value, + size, std::ptr::null_mut(), std::ptr::null_mut(), ) diff --git a/codex-rs/windows-sandbox-rs/src/process.rs b/codex-rs/windows-sandbox-rs/src/process.rs index d2aeac5383f8..c2f873f0e150 100644 --- a/codex-rs/windows-sandbox-rs/src/process.rs +++ b/codex-rs/windows-sandbox-rs/src/process.rs @@ -7,10 +7,13 @@ use crate::winutil::to_wide; use anyhow::Context; use anyhow::Result; use anyhow::anyhow; +use codex_utils_pty::JobObject; use std::collections::HashMap; use std::ffi::c_void; +use std::os::windows::io::AsRawHandle; use std::path::Path; use std::ptr; +use std::sync::Arc; use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::HANDLE; @@ -35,6 +38,7 @@ use windows_sys::Win32::System::Threading::STARTUPINFOW; pub struct CreatedProcess { pub process_info: PROCESS_INFORMATION, pub startup_info: STARTUPINFOW, + pub(crate) job: Arc, _desktop: LaunchDesktop, } @@ -99,17 +103,28 @@ pub unsafe fn create_process_as_user( let mut cmdline: Vec = to_wide(&cmdline_str); let env_block = make_env_block(env_map); let desktop = LaunchDesktop::prepare(use_private_desktop, logs_base_dir)?; + let job = Arc::new(JobObject::create().context("create process job")?); let mut pi: PROCESS_INFORMATION = std::mem::zeroed(); let cwd_wide = to_wide(cwd); let env_block_len = env_block.len(); + let console_flags = match (&stdio, console_mode) { + (Some(_), ConsoleMode::NoWindow) => CREATE_NO_WINDOW, + (Some(_), ConsoleMode::Inherit) + | (None, ConsoleMode::Inherit) + | (None, ConsoleMode::NoWindow) => 0, + }; + let attr_count = if stdio.is_some() { 2 } else { 1 }; + let mut attrs = ProcThreadAttributeList::new(attr_count)?; + attrs.set_job(job.as_raw_handle() as HANDLE)?; + + let mut si: STARTUPINFOEXW = std::mem::zeroed(); + si.StartupInfo.cb = std::mem::size_of::() as u32; + // Some processes (e.g., PowerShell) can fail with STATUS_DLL_INIT_FAILED + // if lpDesktop is not set when launching with a restricted token. + // Point explicitly at the interactive desktop or a private desktop. + si.StartupInfo.lpDesktop = desktop.startup_info_desktop(); match stdio { Some((stdin_h, stdout_h, stderr_h)) => { - let mut si: STARTUPINFOEXW = std::mem::zeroed(); - si.StartupInfo.cb = std::mem::size_of::() as u32; - // Some processes (e.g., PowerShell) can fail with STATUS_DLL_INIT_FAILED - // if lpDesktop is not set when launching with a restricted token. - // Point explicitly at the interactive desktop or a private desktop. - si.StartupInfo.lpDesktop = desktop.startup_info_desktop(); si.StartupInfo.dwFlags |= STARTF_USESTDHANDLES; si.StartupInfo.hStdInput = stdin_h; si.StartupInfo.hStdOutput = stdout_h; @@ -126,92 +141,50 @@ pub unsafe fn create_process_as_user( )); } } - let mut attrs = ProcThreadAttributeList::new(/*attr_count*/ 1)?; attrs.set_handle_list(inherited_handles)?; - si.lpAttributeList = attrs.as_mut_ptr(); - - let creation_flags = CREATE_UNICODE_ENVIRONMENT - | EXTENDED_STARTUPINFO_PRESENT - | match console_mode { - ConsoleMode::Inherit => 0, - ConsoleMode::NoWindow => CREATE_NO_WINDOW, - }; - let ok = CreateProcessAsUserW( - h_token, - std::ptr::null(), - cmdline.as_mut_ptr(), - std::ptr::null_mut(), - std::ptr::null_mut(), - 1, - creation_flags, - env_block.as_ptr() as *mut c_void, - cwd_wide.as_ptr(), - &si.StartupInfo, - &mut pi, - ); - if ok == 0 { - let err = GetLastError() as i32; - let msg = format!( - "CreateProcessAsUserW failed: {} ({}) | cwd={} | cmd={} | env_u16_len={} | si_flags={} | creation_flags={}", - err, - format_last_error(err), - cwd.display(), - cmdline_str, - env_block_len, - si.StartupInfo.dwFlags, - creation_flags, - ); - logging::debug_log(&msg, logs_base_dir); - return Err(std::io::Error::from_raw_os_error(err)).context(msg); - } - Ok(CreatedProcess { - process_info: pi, - startup_info: si.StartupInfo, - _desktop: desktop, - }) } None => { - let mut si: STARTUPINFOW = std::mem::zeroed(); - si.cb = std::mem::size_of::() as u32; - si.lpDesktop = desktop.startup_info_desktop(); - ensure_inheritable_stdio(&mut si)?; - - let creation_flags = CREATE_UNICODE_ENVIRONMENT; - let ok = CreateProcessAsUserW( - h_token, - std::ptr::null(), - cmdline.as_mut_ptr(), - std::ptr::null_mut(), - std::ptr::null_mut(), - 1, - creation_flags, - env_block.as_ptr() as *mut c_void, - cwd_wide.as_ptr(), - &si, - &mut pi, - ); - if ok == 0 { - let err = GetLastError() as i32; - let msg = format!( - "CreateProcessAsUserW failed: {} ({}) | cwd={} | cmd={} | env_u16_len={} | si_flags={} | creation_flags={}", - err, - format_last_error(err), - cwd.display(), - cmdline_str, - env_block_len, - si.dwFlags, - creation_flags, - ); - logging::debug_log(&msg, logs_base_dir); - return Err(std::io::Error::from_raw_os_error(err)).context(msg); - } - Ok(CreatedProcess { - process_info: pi, - startup_info: si, - _desktop: desktop, - }) + ensure_inheritable_stdio(&mut si.StartupInfo)?; } } + si.lpAttributeList = attrs.as_mut_ptr(); + + let creation_flags = CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT | console_flags; + let ok = CreateProcessAsUserW( + h_token, + std::ptr::null(), + cmdline.as_mut_ptr(), + std::ptr::null_mut(), + std::ptr::null_mut(), + 1, + creation_flags, + env_block.as_ptr() as *mut c_void, + cwd_wide.as_ptr(), + &si.StartupInfo, + &mut pi, + ); + if ok == 0 { + let err = GetLastError() as i32; + let msg = format!( + "CreateProcessAsUserW failed: {} ({}) | cwd={} | cmd={} | env_u16_len={} | si_flags={} | creation_flags={}", + err, + format_last_error(err), + cwd.display(), + cmdline_str, + env_block_len, + si.StartupInfo.dwFlags, + creation_flags, + ); + logging::debug_log(&msg, logs_base_dir); + return Err(std::io::Error::from_raw_os_error(err)).context(msg); + } + + Ok(CreatedProcess { + process_info: pi, + startup_info: si.StartupInfo, + job, + _desktop: desktop, + }) } /// Controls whether the child's stdin handle is kept open for writing. @@ -232,12 +205,20 @@ pub enum StderrMode { #[allow(dead_code)] pub struct PipeSpawnHandles { pub process: PROCESS_INFORMATION, + job: Arc, pub stdin_write: Option, pub stdout_read: HANDLE, pub stderr_read: Option, pub(crate) desktop: LaunchDesktop, } +impl PipeSpawnHandles { + /// Returns the Job Object containing the spawned process. + pub fn job(&self) -> Arc { + Arc::clone(&self.job) + } +} + /// Spawns a process with anonymous pipes and returns the relevant handles. #[allow(clippy::too_many_arguments)] pub fn spawn_process_with_pipes( @@ -313,6 +294,7 @@ pub fn spawn_process_with_pipes( }; let CreatedProcess { process_info: pi, + job, _desktop: desktop, .. } = created; @@ -330,6 +312,7 @@ pub fn spawn_process_with_pipes( Ok(PipeSpawnHandles { process: pi, + job, stdin_write: match stdin_mode { StdinMode::Open => Some(in_w), StdinMode::Closed => None, diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs index a09f949135b5..f1b690d7dc53 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs @@ -3,6 +3,7 @@ use crate::conpty::ConptyInstance; use crate::conpty::spawn_conpty_process_as_user; use crate::desktop::LaunchDesktop; use crate::logging::log_failure; +use crate::logging::log_note; use crate::logging::log_success; use crate::process::ConsoleMode; use crate::process::StderrMode; @@ -19,6 +20,7 @@ use crate::spawn_prep::prepare_legacy_spawn_context; use anyhow::Result; use codex_protocol::models::PermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_pty::JobObject; use codex_utils_pty::ProcessDriver; use codex_utils_pty::SpawnedProcess; use codex_utils_pty::TerminalSize; @@ -48,6 +50,7 @@ const WAIT_TIMEOUT: u32 = 0x0000_0102; struct LegacyProcessHandles { process: PROCESS_INFORMATION, + job: Arc, output_join: std::thread::JoinHandle<()>, writer_handle: tokio::task::JoinHandle<()>, hpc: Option, @@ -70,7 +73,7 @@ fn spawn_legacy_process( writer_rx: mpsc::Receiver>, logs_base_dir: Option<&Path>, ) -> Result { - let (pi, output_join, writer_handle, hpc, conpty_owner, desktop) = if tty { + let (pi, job, output_join, writer_handle, hpc, conpty_owner, desktop) = if tty { let (pi, mut conpty) = spawn_conpty_process_as_user( h_token, command, @@ -79,6 +82,9 @@ fn spawn_legacy_process( use_private_desktop, logs_base_dir, )?; + let job = conpty + .job() + .ok_or_else(|| anyhow::anyhow!("spawned ConPTY is missing its process job"))?; let hpc = conpty.raw_handle(); let output_join = spawn_output_reader(conpty.take_output_read(), stdout_tx); let writer_handle = spawn_input_writer( @@ -86,7 +92,7 @@ fn spawn_legacy_process( writer_rx, /*normalize_newlines*/ true, ); - (pi, output_join, writer_handle, hpc, Some(conpty), None) + (pi, job, output_join, writer_handle, hpc, Some(conpty), None) } else { let pipe_handles = spawn_process_with_pipes( h_token, @@ -122,6 +128,7 @@ fn spawn_legacy_process( ); ( pipe_handles.process, + pipe_handles.job(), output_join, writer_handle, None, @@ -131,6 +138,7 @@ fn spawn_legacy_process( }; Ok(LegacyProcessHandles { process: pi, + job, output_join, writer_handle, hpc, @@ -177,6 +185,31 @@ fn spawn_input_writer( }) } +fn terminate_job_or_process( + job: &JobObject, + process_handle: &Arc>>, + logs_base_dir: Option<&Path>, +) { + if let Err(job_err) = job.terminate() { + log_note( + &format!("legacy spawn failed to terminate process tree: {job_err}"), + logs_base_dir, + ); + if let Ok(guard) = process_handle.lock() + && let Some(handle) = guard.as_ref() + && unsafe { TerminateProcess(*handle, 1) } == 0 + { + log_note( + &format!( + "legacy spawn failed to terminate root process: {}", + unsafe { GetLastError() } + ), + logs_base_dir, + ); + } + } +} + fn write_all_handle(handle: HANDLE, mut bytes: &[u8]) -> Result<()> { while !bytes.is_empty() { let mut written = 0u32; @@ -348,6 +381,7 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy( let LegacyProcessHandles { process: pi, + job, output_join, writer_handle, hpc, @@ -379,20 +413,21 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy( let process_handle = Arc::new(StdMutex::new(Some(pi.hProcess))); let wait_handle = Arc::clone(&process_handle); + let job_for_wait = Arc::clone(&job); let command_for_wait = command.clone(); let hpc_for_wait = hpc_handle.clone(); + let wait_logs_base_dir = common.logs_base_dir.clone(); std::thread::spawn(move || { let _desktop = desktop; let timeout = timeout_ms.map(|ms| ms as u32).unwrap_or(INFINITE); let wait_res = unsafe { WaitForSingleObject(pi.hProcess, timeout) }; if wait_res == WAIT_TIMEOUT { - unsafe { - if let Ok(guard) = wait_handle.lock() - && let Some(handle) = guard.as_ref() - { - let _ = TerminateProcess(*handle, 1); - } - } + terminate_job_or_process(&job_for_wait, &wait_handle, wait_logs_base_dir.as_deref()); + } else if let Err(err) = job_for_wait.preserve_descendants() { + log_note( + &format!("legacy spawn failed to preserve descendants after root exit: {err}"), + wait_logs_base_dir.as_deref(), + ); } if let Some(hpc) = hpc_for_wait && let Ok(mut guard) = hpc.lock() @@ -410,21 +445,17 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy( wait_handle, pi.hThread, output_join, - common.logs_base_dir.as_deref(), + wait_logs_base_dir.as_deref(), command_for_wait, ); }); let terminator = { + let job = Arc::clone(&job); let process_handle = Arc::clone(&process_handle); + let logs_base_dir = common.logs_base_dir; Some(Box::new(move || { - if let Ok(guard) = process_handle.lock() - && let Some(handle) = guard.as_ref() - { - unsafe { - let _ = TerminateProcess(*handle, 1); - } - } + terminate_job_or_process(&job, &process_handle, logs_base_dir.as_deref()); }) as Box) }; diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/tests.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/tests.rs index 12159ec9912d..7a9a988656ca 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/tests.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/tests.rs @@ -6,6 +6,8 @@ use crate::ipc_framed::Message; use crate::ipc_framed::decode_bytes; use crate::ipc_framed::read_frame; use crate::run_windows_sandbox_capture; +use base64::Engine; +use base64::engine::general_purpose::STANDARD as BASE64; use codex_protocol::models::PermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_pty::ProcessDriver; @@ -15,12 +17,14 @@ use std::fs; use std::fs::OpenOptions; use std::io::Seek; use std::io::SeekFrom; +use std::os::windows::io::AsRawHandle; +use std::os::windows::io::FromRawHandle; +use std::os::windows::io::OwnedHandle; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::sync::Mutex; use std::sync::MutexGuard; -use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering; use std::time::Duration; @@ -31,6 +35,12 @@ use tokio::sync::broadcast; use tokio::sync::mpsc; use tokio::sync::oneshot; use tokio::time::timeout; +use windows_sys::Win32::Foundation::WAIT_FAILED; +use windows_sys::Win32::Foundation::WAIT_OBJECT_0; +use windows_sys::Win32::Foundation::WAIT_TIMEOUT; +use windows_sys::Win32::System::Threading::OpenProcess; +use windows_sys::Win32::System::Threading::PROCESS_SYNCHRONIZE; +use windows_sys::Win32::System::Threading::WaitForSingleObject; static TEST_HOME_COUNTER: AtomicU64 = AtomicU64::new(0); static LEGACY_PROCESS_TEST_LOCK: Mutex<()> = Mutex::new(()); @@ -83,6 +93,66 @@ fn workspace_roots_for(root: &Path) -> Vec { vec![AbsolutePathBuf::from_absolute_path(root).expect("absolute workspace root")] } +fn powershell_literal(path: &Path) -> String { + path.to_string_lossy().replace('\'', "''") +} + +fn start_powershell_child( + pwsh: &Path, + stdio_dir: &Path, + child_command: &str, + parent_tail: &str, +) -> String { + let encoded = BASE64.encode( + child_command + .encode_utf16() + .flat_map(u16::to_le_bytes) + .collect::>(), + ); + format!( + "Start-Process -WindowStyle Hidden -FilePath '{}' -ArgumentList '-NoProfile','-EncodedCommand','{encoded}' -RedirectStandardOutput '{}' -RedirectStandardError '{}'; {parent_tail}", + powershell_literal(pwsh), + powershell_literal(&stdio_dir.join("descendant.stdout")), + powershell_literal(&stdio_dir.join("descendant.stderr")), + ) +} + +fn wait_for_path(path: &Path, timeout: Duration) -> bool { + let deadline = Instant::now() + timeout; + while Instant::now() < deadline { + if path.exists() { + return true; + } + std::thread::sleep(Duration::from_millis(25)); + } + path.exists() +} + +fn open_process_for_wait(pid: u32) -> std::io::Result { + let handle = unsafe { OpenProcess(PROCESS_SYNCHRONIZE, 0, pid) }; + if handle == 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(unsafe { OwnedHandle::from_raw_handle(handle as _) }) +} + +fn wait_for_process_exit(process: &OwnedHandle, timeout: Duration) -> std::io::Result<()> { + let timeout_ms = u32::try_from(timeout.as_millis()) + .map_err(|_| std::io::Error::other("process wait timeout exceeds u32"))?; + let result = unsafe { WaitForSingleObject(process.as_raw_handle() as _, timeout_ms) }; + match result { + WAIT_OBJECT_0 => Ok(()), + WAIT_TIMEOUT => Err(std::io::Error::new( + std::io::ErrorKind::TimedOut, + "timed out waiting for process to exit", + )), + WAIT_FAILED => Err(std::io::Error::last_os_error()), + result => Err(std::io::Error::other(format!( + "unexpected process wait result: {result}" + ))), + } +} + fn wait_for_frame_count(frames_path: &Path, expected_frames: usize) -> Vec { let deadline = Instant::now() + Duration::from_secs(2); loop { @@ -416,7 +486,7 @@ fn runner_resizer_sends_resize_frame() { } #[test] -fn legacy_capture_powershell_emits_output() { +fn legacy_capture_emits_output_and_preserves_descendant_after_normal_exit() { let Some(pwsh) = pwsh_path() else { return; }; @@ -424,6 +494,23 @@ fn legacy_capture_powershell_emits_output() { let cwd = sandbox_cwd(); let codex_home = sandbox_home("legacy-capture-pwsh"); println!("capture pwsh codex_home={}", codex_home.path().display()); + let ready_marker = codex_home.path().join("descendant-started"); + let release_marker = codex_home.path().join("release-descendant"); + let survival_marker = codex_home.path().join("descendant-survived"); + let descendant_command = format!( + "$deadline=(Get-Date).AddSeconds(30); Set-Content -LiteralPath '{}' -Value $PID; while (-not (Test-Path -LiteralPath '{}')) {{ if ((Get-Date) -ge $deadline) {{ exit 3 }}; Start-Sleep -Milliseconds 25 }}; Set-Content -LiteralPath '{}' -Value survived", + powershell_literal(&ready_marker), + powershell_literal(&release_marker), + powershell_literal(&survival_marker), + ); + let parent_tail = format!( + "while (-not (Test-Path -LiteralPath '{}')) {{ Start-Sleep -Milliseconds 25 }}", + powershell_literal(&ready_marker), + ); + let parent_command = format!( + "Write-Output LEGACY-CAPTURE-DIRECT; {}", + start_powershell_child(&pwsh, codex_home.path(), &descendant_command, &parent_tail,), + ); let permission_profile = PermissionProfile::workspace_write(); let result = run_windows_sandbox_capture( &permission_profile, @@ -433,7 +520,7 @@ fn legacy_capture_powershell_emits_output() { pwsh.display().to_string(), "-NoProfile".to_string(), "-Command".to_string(), - "Write-Output LEGACY-CAPTURE-DIRECT".to_string(), + parent_command, ], cwd.as_path(), HashMap::new(), @@ -442,6 +529,15 @@ fn legacy_capture_powershell_emits_output() { /*use_private_desktop*/ true, ) .expect("run legacy capture powershell"); + let descendant_pid = fs::read_to_string(&ready_marker) + .expect("read descendant pid") + .trim() + .parse() + .expect("parse descendant pid"); + let descendant_process = open_process_for_wait(descendant_pid); + fs::write(&release_marker, "release").expect("release descendant after root exit"); + let descendant_process = descendant_process.expect("open descendant after normal capture exit"); + println!("capture pwsh exit_code={}", result.exit_code); println!("capture pwsh timed_out={}", result.timed_out); let stdout = String::from_utf8_lossy(&result.stdout); @@ -452,6 +548,12 @@ fn legacy_capture_powershell_emits_output() { stdout.contains("LEGACY-CAPTURE-DIRECT"), "stdout={stdout:?}" ); + assert!( + wait_for_path(&survival_marker, Duration::from_secs(10)), + "sandbox descendant did not survive normal capture exit" + ); + wait_for_process_exit(&descendant_process, Duration::from_secs(10)) + .expect("sandbox descendant did not exit after release"); } #[test] @@ -565,7 +667,7 @@ fn legacy_workspace_write_delete_is_limited_to_writable_roots() { } #[test] -fn legacy_capture_cancellation_is_not_reported_as_timeout() { +fn legacy_capture_cancellation_terminates_descendants_without_timeout() { let Some(pwsh) = pwsh_path() else { eprintln!("skipping cancellation regression test: PowerShell 7 is not installed"); return; @@ -573,18 +675,40 @@ fn legacy_capture_cancellation_is_not_reported_as_timeout() { let _guard = legacy_process_test_guard(); let cwd = sandbox_cwd(); let codex_home = sandbox_home("legacy-capture-cancel"); - let permission_profile = PermissionProfile::workspace_write(); - let cancelled = Arc::new(AtomicBool::new(false)); - let cancelled_for_token = Arc::clone(&cancelled); - let cancellation = - WindowsSandboxCancellationToken::new(move || cancelled_for_token.load(Ordering::SeqCst)); - let cancelled_for_thread = Arc::clone(&cancelled); - let cancel_thread = std::thread::spawn(move || { - std::thread::sleep(Duration::from_millis(200)); - cancelled_for_thread.store(true, Ordering::SeqCst); + let descendant_marker = codex_home.path().join("descendant-survived"); + let ready_marker = codex_home.path().join("descendant-started"); + let descendant_command = format!( + "Set-Content -LiteralPath '{}' -Value $PID; Start-Sleep -Seconds 1; Set-Content -LiteralPath '{}' -Value survived", + powershell_literal(&ready_marker), + powershell_literal(&descendant_marker), + ); + let parent_command = start_powershell_child( + &pwsh, + codex_home.path(), + &descendant_command, + "Start-Sleep -Seconds 30", + ); + let descendant_process = Arc::new(Mutex::new(None)); + let descendant_process_for_cancellation = Arc::clone(&descendant_process); + let cancellation = WindowsSandboxCancellationToken::new(move || { + let Ok(pid) = fs::read_to_string(&ready_marker).and_then(|pid| { + pid.trim() + .parse() + .map_err(|err| std::io::Error::other(format!("invalid descendant pid: {err}"))) + }) else { + return false; + }; + let Ok(process) = open_process_for_wait(pid) else { + return false; + }; + *descendant_process_for_cancellation + .lock() + .expect("descendant process lock poisoned") = Some(process); + true }); let started_at = Instant::now(); + let permission_profile = PermissionProfile::workspace_write(); let result = run_windows_sandbox_capture( &permission_profile, workspace_roots_for(cwd.as_path()).as_slice(), @@ -593,7 +717,7 @@ fn legacy_capture_cancellation_is_not_reported_as_timeout() { pwsh.display().to_string(), "-NoProfile".to_string(), "-Command".to_string(), - "Start-Sleep -Seconds 30".to_string(), + parent_command, ], cwd.as_path(), HashMap::new(), @@ -602,7 +726,6 @@ fn legacy_capture_cancellation_is_not_reported_as_timeout() { /*use_private_desktop*/ true, ) .expect("run legacy capture powershell with cancellation"); - cancel_thread.join().expect("cancel thread should finish"); assert!( started_at.elapsed() < Duration::from_secs(10), @@ -613,6 +736,127 @@ fn legacy_capture_cancellation_is_not_reported_as_timeout() { "cancellation should not be reported as a timeout" ); assert_ne!(result.exit_code, 0); + let descendant_process = descendant_process + .lock() + .expect("descendant process lock poisoned") + .take() + .expect("cancellation did not capture descendant process"); + wait_for_process_exit(&descendant_process, Duration::from_secs(10)) + .expect("sandbox descendant did not exit after cancellation"); + assert!( + !descendant_marker.exists(), + "sandbox descendant survived cancellation" + ); +} + +#[derive(Clone, Copy, Debug)] +enum LegacyTtyDescendantLifecycle { + Terminate, + Preserve, +} + +async fn assert_legacy_tty_descendant_lifecycle( + pwsh: &Path, + lifecycle: LegacyTtyDescendantLifecycle, +) { + let cwd = sandbox_cwd(); + let codex_home = sandbox_home(match lifecycle { + LegacyTtyDescendantLifecycle::Terminate => "legacy-tty-descendant-terminate", + LegacyTtyDescendantLifecycle::Preserve => "legacy-tty-descendant-preserve", + }); + let ready_marker = codex_home.path().join("descendant-started"); + let release_marker = codex_home.path().join("release-descendant"); + let survival_marker = codex_home.path().join("descendant-survived"); + let child_tail = match lifecycle { + LegacyTtyDescendantLifecycle::Terminate => "Start-Sleep -Seconds 30".to_string(), + LegacyTtyDescendantLifecycle::Preserve => format!( + "$deadline=(Get-Date).AddSeconds(30); while (-not (Test-Path -LiteralPath '{}')) {{ if ((Get-Date) -ge $deadline) {{ exit 3 }}; Start-Sleep -Milliseconds 25 }}; Set-Content -LiteralPath '{}' -Value survived", + powershell_literal(&release_marker), + powershell_literal(&survival_marker), + ), + }; + let child_command = format!( + "Set-Content -LiteralPath '{}' -Value $PID; {child_tail}", + powershell_literal(&ready_marker), + ); + let parent_tail = match lifecycle { + LegacyTtyDescendantLifecycle::Terminate => "Start-Sleep -Seconds 30".to_string(), + LegacyTtyDescendantLifecycle::Preserve => format!( + "while (-not (Test-Path -LiteralPath '{}')) {{ Start-Sleep -Milliseconds 25 }}", + powershell_literal(&ready_marker), + ), + }; + let parent_command = + start_powershell_child(pwsh, codex_home.path(), &child_command, &parent_tail); + let permission_profile = PermissionProfile::workspace_write(); + let spawned = spawn_windows_sandbox_session_legacy( + &permission_profile, + workspace_roots_for(cwd.as_path()).as_slice(), + codex_home.path(), + vec![ + pwsh.display().to_string(), + "-NoProfile".to_string(), + "-Command".to_string(), + parent_command, + ], + cwd.as_path(), + HashMap::new(), + Some(30_000), + &[], + &[], + /*tty*/ true, + /*stdin_open*/ false, + /*use_private_desktop*/ true, + ) + .await + .expect("spawn legacy sandbox ConPTY lifecycle test"); + assert!( + wait_for_path(&ready_marker, Duration::from_secs(10)), + "{lifecycle:?} descendant did not start" + ); + let descendant_pid = fs::read_to_string(&ready_marker) + .expect("read descendant pid") + .trim() + .parse() + .expect("parse descendant pid"); + let descendant_process = open_process_for_wait(descendant_pid); + + if matches!(lifecycle, LegacyTtyDescendantLifecycle::Terminate) { + spawned.session.request_terminate(); + } + let (_, exit_code) = + collect_stdout_and_exit(spawned, codex_home.path(), Duration::from_secs(15)).await; + if matches!(lifecycle, LegacyTtyDescendantLifecycle::Preserve) { + fs::write(&release_marker, "release").expect("release preserved descendant"); + } + let descendant_process = descendant_process.expect("open sandbox ConPTY descendant"); + + match lifecycle { + LegacyTtyDescendantLifecycle::Terminate => assert_ne!(exit_code, 0), + LegacyTtyDescendantLifecycle::Preserve => { + assert_eq!(exit_code, 0); + assert!( + wait_for_path(&survival_marker, Duration::from_secs(10)), + "sandbox ConPTY descendant did not survive normal exit" + ); + } + } + wait_for_process_exit(&descendant_process, Duration::from_secs(10)) + .expect("sandbox ConPTY descendant did not exit"); +} + +#[test] +fn legacy_tty_job_terminates_and_preserves_descendants() { + let Some(pwsh) = pwsh_path() else { + eprintln!("skipping sandbox ConPTY lifecycle test: PowerShell 7 is not installed"); + return; + }; + let _guard = legacy_process_test_guard(); + current_thread_runtime().block_on(async move { + assert_legacy_tty_descendant_lifecycle(&pwsh, LegacyTtyDescendantLifecycle::Terminate) + .await; + assert_legacy_tty_descendant_lifecycle(&pwsh, LegacyTtyDescendantLifecycle::Preserve).await; + }); } #[test]