diff --git a/codex-rs/windows-sandbox-rs/src/conpty/mod.rs b/codex-rs/windows-sandbox-rs/src/conpty/mod.rs index 09dc52b572a7..4ba0eace42f8 100644 --- a/codex-rs/windows-sandbox-rs/src/conpty/mod.rs +++ b/codex-rs/windows-sandbox-rs/src/conpty/mod.rs @@ -13,16 +13,21 @@ use crate::winutil::quote_windows_arg; use crate::winutil::to_wide; use anyhow::Context; use anyhow::Result; +use codex_utils_pty::JobProcess; +use codex_utils_pty::KillOnCloseJob; use codex_utils_pty::PsuedoCon; use codex_utils_pty::RawConPty; +use codex_utils_pty::SuspendedProcess; use std::collections::HashMap; use std::ffi::c_void; use std::os::windows::io::IntoRawHandle; +use std::os::windows::io::RawHandle; use std::path::Path; use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::HANDLE; use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE; +use windows_sys::Win32::System::Threading::CREATE_SUSPENDED; use windows_sys::Win32::System::Threading::CREATE_UNICODE_ENVIRONMENT; use windows_sys::Win32::System::Threading::CreateProcessAsUserW; use windows_sys::Win32::System::Threading::EXTENDED_STARTUPINFO_PRESENT; @@ -98,6 +103,57 @@ pub fn spawn_conpty_process_as_user( env_map: &HashMap, use_private_desktop: bool, logs_base_dir: Option<&Path>, +) -> Result<(PROCESS_INFORMATION, ConptyInstance)> { + spawn_conpty_process_as_user_with_extra_flags( + h_token, + argv, + cwd, + env_map, + use_private_desktop, + logs_base_dir, + /*extra_creation_flags*/ 0, + ) +} + +/// Spawns a suspended ConPTY process, attaches it to a kill-on-close job, and resumes it. +pub fn spawn_job_conpty_process_as_user( + h_token: HANDLE, + argv: &[String], + cwd: &Path, + env_map: &HashMap, + use_private_desktop: bool, + logs_base_dir: Option<&Path>, +) -> Result<(JobProcess, ConptyInstance)> { + let job = KillOnCloseJob::new()?; + let (process_info, conpty) = spawn_conpty_process_as_user_with_extra_flags( + h_token, + argv, + cwd, + env_map, + use_private_desktop, + logs_base_dir, + CREATE_SUSPENDED, + )?; + let suspended = unsafe { + SuspendedProcess::from_raw_handles( + process_info.hProcess as RawHandle, + process_info.hThread as RawHandle, + process_info.dwProcessId, + ) + }; + let process = suspended.assign_and_resume(job)?; + Ok((process, conpty)) +} + +#[allow(clippy::too_many_arguments)] +fn spawn_conpty_process_as_user_with_extra_flags( + h_token: HANDLE, + argv: &[String], + cwd: &Path, + env_map: &HashMap, + use_private_desktop: bool, + logs_base_dir: Option<&Path>, + extra_creation_flags: u32, ) -> Result<(PROCESS_INFORMATION, ConptyInstance)> { let cmdline_str = argv .iter() @@ -137,7 +193,7 @@ pub fn spawn_conpty_process_as_user( std::ptr::null_mut(), std::ptr::null_mut(), 0, - EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT, + EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT | extra_creation_flags, env_block.as_ptr() as *mut c_void, to_wide(cwd).as_ptr(), &si.StartupInfo, diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 2a394f11ffc5..f89e88ae5424 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -164,6 +164,8 @@ pub use conpty::ConptyInstance; #[cfg(target_os = "windows")] pub use conpty::spawn_conpty_process_as_user; #[cfg(target_os = "windows")] +pub use conpty::spawn_job_conpty_process_as_user; +#[cfg(target_os = "windows")] pub use deny_read_acl::apply_deny_read_acls; #[cfg(target_os = "windows")] pub use deny_read_acl::plan_deny_read_acl_paths; @@ -235,6 +237,8 @@ pub use logging::log_writer; #[cfg(target_os = "windows")] pub use path_normalization::canonicalize_path; #[cfg(target_os = "windows")] +pub use process::JobPipeSpawnHandles; +#[cfg(target_os = "windows")] pub use process::PipeSpawnHandles; #[cfg(target_os = "windows")] pub use process::StderrMode; @@ -245,6 +249,8 @@ pub use process::create_process_as_user; #[cfg(target_os = "windows")] pub use process::read_handle_loop; #[cfg(target_os = "windows")] +pub use process::spawn_job_process_with_pipes; +#[cfg(target_os = "windows")] pub use process::spawn_process_with_pipes; #[cfg(target_os = "windows")] pub use resolved_permissions::ResolvedWindowsSandboxPermissions; diff --git a/codex-rs/windows-sandbox-rs/src/process.rs b/codex-rs/windows-sandbox-rs/src/process.rs index 86859baf107d..9c6b205e876c 100644 --- a/codex-rs/windows-sandbox-rs/src/process.rs +++ b/codex-rs/windows-sandbox-rs/src/process.rs @@ -7,8 +7,12 @@ use crate::winutil::to_wide; use anyhow::Context; use anyhow::Result; use anyhow::anyhow; +use codex_utils_pty::JobProcess; +use codex_utils_pty::KillOnCloseJob; +use codex_utils_pty::SuspendedProcess; use std::collections::HashMap; use std::ffi::c_void; +use std::os::windows::io::RawHandle; use std::path::Path; use std::ptr; use windows_sys::Win32::Foundation::CloseHandle; @@ -23,6 +27,7 @@ use windows_sys::Win32::System::Console::STD_ERROR_HANDLE; use windows_sys::Win32::System::Console::STD_INPUT_HANDLE; use windows_sys::Win32::System::Console::STD_OUTPUT_HANDLE; use windows_sys::Win32::System::Pipes::CreatePipe; +use windows_sys::Win32::System::Threading::CREATE_SUSPENDED; use windows_sys::Win32::System::Threading::CREATE_UNICODE_ENVIRONMENT; use windows_sys::Win32::System::Threading::CreateProcessAsUserW; use windows_sys::Win32::System::Threading::EXTENDED_STARTUPINFO_PRESENT; @@ -84,6 +89,29 @@ pub unsafe fn create_process_as_user( logs_base_dir: Option<&Path>, stdio: Option<(HANDLE, HANDLE, HANDLE)>, use_private_desktop: bool, +) -> Result { + create_process_as_user_with_extra_flags( + h_token, + argv, + cwd, + env_map, + logs_base_dir, + stdio, + use_private_desktop, + /*extra_creation_flags*/ 0, + ) +} + +#[allow(clippy::too_many_arguments)] +unsafe fn create_process_as_user_with_extra_flags( + h_token: HANDLE, + argv: &[String], + cwd: &Path, + env_map: &HashMap, + logs_base_dir: Option<&Path>, + stdio: Option<(HANDLE, HANDLE, HANDLE)>, + use_private_desktop: bool, + extra_creation_flags: u32, ) -> Result { let cmdline_str = argv_to_command_line(argv); let mut cmdline: Vec = to_wide(&cmdline_str); @@ -120,7 +148,8 @@ pub unsafe fn create_process_as_user( attrs.set_handle_list(inherited_handles)?; si.lpAttributeList = attrs.as_mut_ptr(); - let creation_flags = CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT; + let creation_flags = + CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT | extra_creation_flags; let ok = CreateProcessAsUserW( h_token, std::ptr::null(), @@ -161,7 +190,7 @@ pub unsafe fn create_process_as_user( si.lpDesktop = desktop.startup_info_desktop(); ensure_inheritable_stdio(&mut si)?; - let creation_flags = CREATE_UNICODE_ENVIRONMENT; + let creation_flags = CREATE_UNICODE_ENVIRONMENT | extra_creation_flags; let ok = CreateProcessAsUserW( h_token, std::ptr::null(), @@ -223,6 +252,23 @@ pub struct PipeSpawnHandles { pub(crate) desktop: LaunchDesktop, } +/// Handles returned by a job-contained pipe spawn used by live Windows sessions. +pub struct JobPipeSpawnHandles { + pub process: JobProcess, + pub stdin_write: Option, + pub stdout_read: HANDLE, + pub stderr_read: Option, + pub desktop: LaunchDesktop, +} + +struct PipeSpawnHandlesInner

{ + process: P, + stdin_write: Option, + stdout_read: HANDLE, + stderr_read: Option, + desktop: LaunchDesktop, +} + /// Spawns a process with anonymous pipes and returns the relevant handles. #[allow(clippy::too_many_arguments)] pub fn spawn_process_with_pipes( @@ -235,6 +281,86 @@ pub fn spawn_process_with_pipes( use_private_desktop: bool, logs_base_dir: Option<&Path>, ) -> Result { + let handles = spawn_process_with_pipes_inner(stdin_mode, stderr_mode, |stdio| unsafe { + let created = create_process_as_user( + h_token, + argv, + cwd, + env_map, + logs_base_dir, + Some(stdio), + use_private_desktop, + )?; + let CreatedProcess { + process_info, + _desktop: desktop, + .. + } = created; + Ok((process_info, desktop)) + })?; + Ok(PipeSpawnHandles { + process: handles.process, + stdin_write: handles.stdin_write, + stdout_read: handles.stdout_read, + stderr_read: handles.stderr_read, + desktop: handles.desktop, + }) +} + +/// Spawns a suspended process, attaches it to a kill-on-close job, and then resumes it. +/// +/// This is the pipe entry point for live Windows sessions. Capture-only callers continue to use +/// [`spawn_process_with_pipes`], whose process-start behavior is unchanged. +#[allow(clippy::too_many_arguments)] +pub fn spawn_job_process_with_pipes( + h_token: HANDLE, + argv: &[String], + cwd: &Path, + env_map: &HashMap, + stdin_mode: StdinMode, + stderr_mode: StderrMode, + use_private_desktop: bool, + logs_base_dir: Option<&Path>, +) -> Result { + let handles = spawn_process_with_pipes_inner(stdin_mode, stderr_mode, |stdio| unsafe { + let job = KillOnCloseJob::new()?; + let created = create_process_as_user_with_extra_flags( + h_token, + argv, + cwd, + env_map, + logs_base_dir, + Some(stdio), + use_private_desktop, + CREATE_SUSPENDED, + )?; + let CreatedProcess { + process_info, + _desktop: desktop, + .. + } = created; + let suspended = SuspendedProcess::from_raw_handles( + process_info.hProcess as RawHandle, + process_info.hThread as RawHandle, + process_info.dwProcessId, + ); + let process = suspended.assign_and_resume(job)?; + Ok((process, desktop)) + })?; + Ok(JobPipeSpawnHandles { + process: handles.process, + stdin_write: handles.stdin_write, + stdout_read: handles.stdout_read, + stderr_read: handles.stderr_read, + desktop: handles.desktop, + }) +} + +fn spawn_process_with_pipes_inner

( + stdin_mode: StdinMode, + stderr_mode: StderrMode, + spawn: impl FnOnce((HANDLE, HANDLE, HANDLE)) -> Result<(P, LaunchDesktop)>, +) -> Result> { let mut in_r: HANDLE = 0; let mut in_w: HANDLE = 0; let mut out_r: HANDLE = 0; @@ -266,18 +392,7 @@ pub fn spawn_process_with_pipes( StderrMode::Separate => err_w, }; - let stdio = Some((in_r, out_w, stderr_handle)); - let spawn_result = unsafe { - create_process_as_user( - h_token, - argv, - cwd, - env_map, - logs_base_dir, - stdio, - use_private_desktop, - ) - }; + let spawn_result = spawn((in_r, out_w, stderr_handle)); let created = match spawn_result { Ok(v) => v, Err(err) => { @@ -294,11 +409,7 @@ pub fn spawn_process_with_pipes( return Err(err); } }; - let CreatedProcess { - process_info: pi, - _desktop: desktop, - .. - } = created; + let (process, desktop) = created; unsafe { CloseHandle(in_r); @@ -311,8 +422,8 @@ pub fn spawn_process_with_pipes( } } - Ok(PipeSpawnHandles { - process: pi, + Ok(PipeSpawnHandlesInner { + process, 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 f3c8ada8c97f..03308ee9928e 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 @@ -1,13 +1,13 @@ use super::windows_common::finish_driver_spawn; use crate::conpty::ConptyInstance; -use crate::conpty::spawn_conpty_process_as_user; +use crate::conpty::spawn_job_conpty_process_as_user; use crate::desktop::LaunchDesktop; use crate::logging::log_failure; use crate::logging::log_success; use crate::process::StderrMode; use crate::process::StdinMode; use crate::process::read_handle_loop; -use crate::process::spawn_process_with_pipes; +use crate::process::spawn_job_process_with_pipes; use crate::spawn_prep::LegacyAclSids; use crate::spawn_prep::SpawnPrepOptions; use crate::spawn_prep::allow_null_device_for_workspace_write; @@ -18,6 +18,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::JobProcess; use codex_utils_pty::ProcessDriver; use codex_utils_pty::SpawnedProcess; use codex_utils_pty::TerminalSize; @@ -39,14 +40,12 @@ use windows_sys::Win32::System::Console::COORD; use windows_sys::Win32::System::Console::ResizePseudoConsole; use windows_sys::Win32::System::Threading::GetExitCodeProcess; use windows_sys::Win32::System::Threading::INFINITE; -use windows_sys::Win32::System::Threading::PROCESS_INFORMATION; -use windows_sys::Win32::System::Threading::TerminateProcess; use windows_sys::Win32::System::Threading::WaitForSingleObject; const WAIT_TIMEOUT: u32 = 0x0000_0102; struct LegacyProcessHandles { - process: PROCESS_INFORMATION, + process: JobProcess, output_join: std::thread::JoinHandle<()>, writer_handle: tokio::task::JoinHandle<()>, hpc: Option, @@ -69,8 +68,8 @@ 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, mut conpty) = spawn_conpty_process_as_user( + let (process, output_join, writer_handle, hpc, conpty_owner, desktop) = if tty { + let (process, mut conpty) = spawn_job_conpty_process_as_user( h_token, command, cwd, @@ -85,9 +84,9 @@ fn spawn_legacy_process( writer_rx, /*normalize_newlines*/ true, ); - (pi, output_join, writer_handle, hpc, Some(conpty), None) + (process, output_join, writer_handle, hpc, Some(conpty), None) } else { - let pipe_handles = spawn_process_with_pipes( + let pipe_handles = spawn_job_process_with_pipes( h_token, command, cwd, @@ -128,7 +127,7 @@ fn spawn_legacy_process( ) }; Ok(LegacyProcessHandles { - process: pi, + process, output_join, writer_handle, hpc, @@ -202,21 +201,17 @@ fn write_all_handle(handle: HANDLE, mut bytes: &[u8]) -> Result<()> { #[allow(clippy::too_many_arguments)] fn finalize_exit( exit_tx: oneshot::Sender, - process_handle: Arc>>, - thread_handle: HANDLE, + process: JobProcess, output_join: std::thread::JoinHandle<()>, logs_base_dir: Option<&Path>, command: Vec, ) { + let process_handle = process.as_raw_handle() as HANDLE; let exit_code = { let mut raw_exit = 1u32; - if let Ok(guard) = process_handle.lock() - && let Some(handle) = guard.as_ref() - { - unsafe { - WaitForSingleObject(*handle, INFINITE); - GetExitCodeProcess(*handle, &mut raw_exit); - } + unsafe { + WaitForSingleObject(process_handle, INFINITE); + GetExitCodeProcess(process_handle, &mut raw_exit); } raw_exit as i32 }; @@ -224,17 +219,6 @@ fn finalize_exit( let _ = output_join.join(); let _ = exit_tx.send(exit_code); - unsafe { - if thread_handle != 0 && thread_handle != INVALID_HANDLE_VALUE { - CloseHandle(thread_handle); - } - if let Ok(mut guard) = process_handle.lock() - && let Some(handle) = guard.take() - { - CloseHandle(handle); - } - } - if exit_code == 0 { log_success(&command, logs_base_dir); } else { @@ -345,7 +329,7 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy( let (exit_tx, exit_rx) = oneshot::channel::(); let LegacyProcessHandles { - process: pi, + process, output_join, writer_handle, hpc, @@ -375,22 +359,21 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy( }; let hpc_handle = hpc.map(|hpc| Arc::new(StdMutex::new(Some(hpc)))); - let process_handle = Arc::new(StdMutex::new(Some(pi.hProcess))); - let wait_handle = Arc::clone(&process_handle); + let process_handle = process.as_raw_handle() as HANDLE; + let job = process.controller(); + let terminator_job = job.clone(); let command_for_wait = command.clone(); let hpc_for_wait = hpc_handle.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) }; + let wait_res = unsafe { WaitForSingleObject(process_handle, timeout) }; if wait_res == WAIT_TIMEOUT { - unsafe { - if let Ok(guard) = wait_handle.lock() - && let Some(handle) = guard.as_ref() - { - let _ = TerminateProcess(*handle, 1); - } - } + let _ = job.terminate_and_close(1); + } else { + // The shell is the session root. Closing its job here prevents descendants from + // outliving the session or keeping output pipes open while the readers are joined. + let _ = job.close(); } if let Some(hpc) = hpc_for_wait && let Ok(mut guard) = hpc.lock() @@ -405,26 +388,16 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy( } finalize_exit( exit_tx, - wait_handle, - pi.hThread, + process, output_join, common.logs_base_dir.as_deref(), command_for_wait, ); }); - let terminator = { - let process_handle = Arc::clone(&process_handle); - Some(Box::new(move || { - if let Ok(guard) = process_handle.lock() - && let Some(handle) = guard.as_ref() - { - unsafe { - let _ = TerminateProcess(*handle, 1); - } - } - }) as Box) - }; + let terminator = Some(Box::new(move || { + let _ = terminator_job.terminate_and_close(1); + }) as Box); let driver = ProcessDriver { writer_tx, diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/job_test_support.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/job_test_support.rs new file mode 100644 index 000000000000..77f744d09200 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/job_test_support.rs @@ -0,0 +1,154 @@ +#![cfg(target_os = "windows")] + +use base64::Engine; +use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; +use pretty_assertions::assert_eq; +use std::fs; +use std::path::Path; +use std::path::PathBuf; +use std::sync::Mutex; +use std::sync::MutexGuard; +use std::time::Duration; +use std::time::Instant; +use tempfile::TempDir; + +static WINDOWS_PROCESS_TEST_LOCK: Mutex<()> = Mutex::new(()); + +pub(super) fn windows_process_test_guard() -> MutexGuard<'static, ()> { + WINDOWS_PROCESS_TEST_LOCK + .lock() + .expect("Windows sandbox process test lock poisoned") +} + +pub(super) fn windows_powershell_path() -> PathBuf { + let system_root = std::env::var_os("SystemRoot").expect("SystemRoot should be set on Windows"); + let path = PathBuf::from(system_root).join("System32\\WindowsPowerShell\\v1.0\\powershell.exe"); + assert!( + path.is_file(), + "Windows PowerShell is required for job lifecycle tests: {}", + path.display() + ); + path +} + +fn powershell_single_quoted(value: &Path) -> String { + value.display().to_string().replace('\'', "''") +} + +fn powershell_encoded_command(script: &str) -> String { + let bytes = script + .encode_utf16() + .flat_map(u16::to_le_bytes) + .collect::>(); + BASE64_STANDARD.encode(bytes) +} + +pub(super) struct GrandchildFixture { + _test_dir: TempDir, + ticks_path: PathBuf, + ready_path: PathBuf, + pub(super) command: Vec, +} + +pub(super) fn grandchild_fixture( + cwd: &Path, + powershell: &Path, + root_tail: &str, +) -> GrandchildFixture { + let test_dir = tempfile::tempdir_in(cwd).expect("create grandchild test directory"); + let ticks_path = test_dir.path().join("ticks.txt"); + let ready_path = test_dir.path().join("ready.txt"); + let child_script = format!( + "while ($true) {{ [IO.File]::AppendAllText('{}', 'x'); Start-Sleep -Milliseconds 25 }}", + powershell_single_quoted(&ticks_path) + ); + let root_script = format!( + "$child = Start-Process -PassThru -FilePath '{}' -ArgumentList @('-NoProfile', '-EncodedCommand', '{}'); [IO.File]::WriteAllText('{}', [string]$child.Id); {root_tail}", + powershell_single_quoted(powershell), + powershell_encoded_command(&child_script), + powershell_single_quoted(&ready_path), + ); + GrandchildFixture { + _test_dir: test_dir, + ticks_path, + ready_path, + command: vec![ + powershell.display().to_string(), + "-NoProfile".to_string(), + "-EncodedCommand".to_string(), + powershell_encoded_command(&root_script), + ], + } +} + +pub(super) fn wait_for_grandchild(fixture: &GrandchildFixture) { + let deadline = Instant::now() + Duration::from_secs(10); + while (!fixture.ready_path.exists() + || fs::metadata(&fixture.ticks_path) + .map(|meta| meta.len()) + .unwrap_or(0) + < 3) + && Instant::now() < deadline + { + std::thread::sleep(Duration::from_millis(25)); + } + assert!( + fixture.ready_path.exists(), + "root did not report child startup" + ); + assert!( + fs::metadata(&fixture.ticks_path) + .map(|meta| meta.len()) + .unwrap_or(0) + >= 3, + "grandchild did not write ticks" + ); +} + +pub(super) fn assert_grandchild_stopped(fixture: &GrandchildFixture) { + let length_after_exit = fs::metadata(&fixture.ticks_path) + .expect("ticks file after root exit") + .len(); + std::thread::sleep(Duration::from_millis(300)); + assert_eq!( + fs::metadata(&fixture.ticks_path) + .expect("ticks file after stability wait") + .len(), + length_after_exit, + "grandchild kept writing after the session job closed" + ); +} + +#[derive(Clone, Copy)] +pub(super) enum SessionMode { + Pipe, + Tty, +} + +impl SessionMode { + pub(super) fn tty(self) -> bool { + matches!(self, Self::Tty) + } + + pub(super) fn label(self) -> &'static str { + match self { + Self::Pipe => "pipe", + Self::Tty => "tty", + } + } +} + +#[derive(Clone, Copy)] +pub(super) enum SessionEnding { + ExplicitTermination, + RootExit, +} + +impl SessionEnding { + pub(super) fn root_tail(self) -> &'static str { + match self { + Self::ExplicitTermination => "Start-Sleep -Seconds 30", + Self::RootExit => "Start-Sleep -Milliseconds 500", + } + } +} diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/restricted_job_tests.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/restricted_job_tests.rs new file mode 100644 index 000000000000..4605202741ef --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/restricted_job_tests.rs @@ -0,0 +1,100 @@ +#![cfg(target_os = "windows")] + +use super::current_thread_runtime; +use super::job_test_support::SessionEnding; +use super::job_test_support::SessionMode; +use super::job_test_support::assert_grandchild_stopped; +use super::job_test_support::grandchild_fixture; +use super::job_test_support::wait_for_grandchild; +use super::job_test_support::windows_powershell_path; +use super::job_test_support::windows_process_test_guard; +use super::sandbox_cwd; +use super::sandbox_home; +use super::workspace_roots_for; +use crate::unified_exec::spawn_windows_sandbox_session_legacy; +use codex_protocol::models::PermissionProfile; +use std::collections::HashMap; +use std::time::Duration; +use tokio::time::timeout; + +fn assert_restricted_session_stops_grandchild(mode: SessionMode, ending: SessionEnding) { + let _guard = windows_process_test_guard(); + let runtime = current_thread_runtime(); + runtime.block_on(async move { + let cwd = sandbox_cwd(); + let powershell = windows_powershell_path(); + let fixture = grandchild_fixture(&cwd, &powershell, ending.root_tail()); + let codex_home = sandbox_home(&format!( + "restricted-grandchild-{}-{}", + mode.label(), + match ending { + SessionEnding::ExplicitTermination => "terminate", + SessionEnding::RootExit => "root-exit", + } + )); + 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(), + fixture.command.clone(), + cwd.as_path(), + HashMap::new(), + Some(30_000), + &[], + &[], + /*tty*/ mode.tty(), + /*stdin_open*/ mode.tty(), + /*use_private_desktop*/ mode.tty(), + ) + .await + .expect("spawn restricted-token grandchild session"); + + wait_for_grandchild(&fixture); + + let codex_utils_pty::SpawnedProcess { + session, + stdout_rx: _stdout_rx, + stderr_rx: _stderr_rx, + exit_rx, + } = spawned; + if matches!(ending, SessionEnding::ExplicitTermination) { + session.request_terminate(); + } + let exit_code = timeout(Duration::from_secs(10), exit_rx) + .await + .expect("timed out waiting for restricted-token root exit") + .unwrap_or(-1); + match ending { + SessionEnding::ExplicitTermination => assert_ne!(exit_code, 0), + SessionEnding::RootExit => assert_eq!(exit_code, 0), + } + assert_grandchild_stopped(&fixture); + }); +} + +#[test] +fn restricted_non_tty_termination_stops_grandchild() { + assert_restricted_session_stops_grandchild( + SessionMode::Pipe, + SessionEnding::ExplicitTermination, + ); +} + +#[test] +fn restricted_tty_termination_stops_grandchild() { + assert_restricted_session_stops_grandchild( + SessionMode::Tty, + SessionEnding::ExplicitTermination, + ); +} + +#[test] +fn restricted_non_tty_root_exit_stops_grandchild() { + assert_restricted_session_stops_grandchild(SessionMode::Pipe, SessionEnding::RootExit); +} + +#[test] +fn restricted_tty_root_exit_stops_grandchild() { + assert_restricted_session_stops_grandchild(SessionMode::Tty, SessionEnding::RootExit); +} 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 96f5c39e9233..d250001035fa 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/tests.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/tests.rs @@ -9,6 +9,7 @@ use crate::run_windows_sandbox_capture; use codex_protocol::models::PermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_pty::ProcessDriver; +use job_test_support::windows_process_test_guard; use pretty_assertions::assert_eq; use std::collections::HashMap; use std::fs; @@ -18,8 +19,6 @@ use std::io::SeekFrom; 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; @@ -33,13 +32,12 @@ use tokio::sync::oneshot; use tokio::time::timeout; static TEST_HOME_COUNTER: AtomicU64 = AtomicU64::new(0); -static LEGACY_PROCESS_TEST_LOCK: Mutex<()> = Mutex::new(()); -fn legacy_process_test_guard() -> MutexGuard<'static, ()> { - LEGACY_PROCESS_TEST_LOCK - .lock() - .expect("legacy Windows sandbox process test lock poisoned") -} +#[path = "job_test_support.rs"] +mod job_test_support; + +#[path = "restricted_job_tests.rs"] +mod restricted_job_tests; fn current_thread_runtime() -> tokio::runtime::Runtime { Builder::new_current_thread() @@ -151,7 +149,7 @@ async fn collect_stdout_and_exit( #[test] fn legacy_non_tty_cmd_emits_output() { - let _guard = legacy_process_test_guard(); + let _guard = windows_process_test_guard(); let runtime = current_thread_runtime(); runtime.block_on(async move { let cwd = sandbox_cwd(); @@ -190,7 +188,7 @@ fn legacy_non_tty_cmd_emits_output() { #[test] fn legacy_non_tty_cmd_rejects_deny_read_overrides() { - let _guard = legacy_process_test_guard(); + let _guard = windows_process_test_guard(); let runtime = current_thread_runtime(); runtime.block_on(async move { let cwd = sandbox_cwd(); @@ -232,7 +230,7 @@ fn legacy_non_tty_powershell_emits_output() { let Some(pwsh) = pwsh_path() else { return; }; - let _guard = legacy_process_test_guard(); + let _guard = windows_process_test_guard(); let runtime = current_thread_runtime(); runtime.block_on(async move { let cwd = sandbox_cwd(); @@ -420,7 +418,7 @@ fn legacy_capture_powershell_emits_output() { let Some(pwsh) = pwsh_path() else { return; }; - let _guard = legacy_process_test_guard(); + let _guard = windows_process_test_guard(); let cwd = sandbox_cwd(); let codex_home = sandbox_home("legacy-capture-pwsh"); println!("capture pwsh codex_home={}", codex_home.path().display()); @@ -460,7 +458,7 @@ fn legacy_capture_cancellation_is_not_reported_as_timeout() { eprintln!("skipping cancellation regression test: PowerShell 7 is not installed"); return; }; - let _guard = legacy_process_test_guard(); + let _guard = windows_process_test_guard(); let cwd = sandbox_cwd(); let codex_home = sandbox_home("legacy-capture-cancel"); let permission_profile = PermissionProfile::workspace_write(); @@ -510,7 +508,7 @@ fn legacy_tty_powershell_emits_output_and_accepts_input() { let Some(pwsh) = pwsh_path() else { return; }; - let _guard = legacy_process_test_guard(); + let _guard = windows_process_test_guard(); let runtime = current_thread_runtime(); runtime.block_on(async move { let cwd = sandbox_cwd();