From 6a97ac0f9aa9834690315cb4fe1d62138da56926 Mon Sep 17 00:00:00 2001 From: Zhang GH Date: Fri, 18 Sep 2026 23:00:57 +0800 Subject: [PATCH 1/2] fix(cli): isolate Windows daemon startup from caller jobs Use an explicit standard-handle inheritance list for daemon startup and the Windows update helper. Require and verify breakaway before resuming a background daemon, with an actionable error for restrictive host Jobs. Share direct startup and its readiness deadline across explicit and automatic entry points, retain child ownership through startup, and reap Unix children after handoff. Add native Windows EOF, Job lifetime, concurrency and failure regressions to CI, and document persistent host setup. Refs #268 --- .github/workflows/ci.yml | 3 + crates/bsk-cli/Cargo.toml | 1 + crates/bsk-cli/src/cli/daemon.rs | 2 +- crates/bsk-cli/src/cli/ensure_daemon.rs | 43 +- crates/bsk-cli/src/cli/update/windows.rs | 173 +----- crates/bsk-cli/src/daemon/start.rs | 187 +++++-- crates/bsk-cli/src/daemon/start/windows.rs | 87 +++ crates/bsk-cli/src/lib.rs | 3 + crates/bsk-cli/src/windows_process.rs | 238 +++++++++ crates/bsk-cli/tests/windows_daemon_start.rs | 535 +++++++++++++++++++ docs/sandboxed-agents.md | 24 + 11 files changed, 1045 insertions(+), 251 deletions(-) create mode 100644 crates/bsk-cli/src/daemon/start/windows.rs create mode 100644 crates/bsk-cli/src/windows_process.rs create mode 100644 crates/bsk-cli/tests/windows_daemon_start.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b7e044d6..6da8829e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,6 +55,9 @@ jobs: - name: Run Windows audit and browser interaction policy tests run: cargo test -p bsk --test audit_ws --test interaction_preferences --locked + - name: Run Windows daemon startup and Job lifetime tests + run: cargo test -p bsk --test windows_daemon_start --locked -- --test-threads=1 + - name: Run Windows process liveness tests run: cargo test -p bsk --lib --locked daemon::lockfile::tests diff --git a/crates/bsk-cli/Cargo.toml b/crates/bsk-cli/Cargo.toml index 565a7424..a7ec2d7e 100644 --- a/crates/bsk-cli/Cargo.toml +++ b/crates/bsk-cli/Cargo.toml @@ -70,6 +70,7 @@ libc = "0.2" windows-sys = { version = "0.59", features = [ "Win32_Foundation", "Win32_System_Threading", + "Win32_System_JobObjects", "Win32_System_Pipes", "Win32_Security", "Win32_Storage_FileSystem", diff --git a/crates/bsk-cli/src/cli/daemon.rs b/crates/bsk-cli/src/cli/daemon.rs index 85990255..f171a39a 100644 --- a/crates/bsk-cli/src/cli/daemon.rs +++ b/crates/bsk-cli/src/cli/daemon.rs @@ -81,7 +81,7 @@ pub struct StartArgs { #[arg(long, value_name = "PORT")] pub port: Option, - /// Run in the foreground (do not double-fork). Useful for development. + /// Run in the foreground, owned by the current terminal or supervisor. #[arg(long)] pub foreground: bool, diff --git a/crates/bsk-cli/src/cli/ensure_daemon.rs b/crates/bsk-cli/src/cli/ensure_daemon.rs index e95929fd..6390f72b 100644 --- a/crates/bsk-cli/src/cli/ensure_daemon.rs +++ b/crates/bsk-cli/src/cli/ensure_daemon.rs @@ -4,19 +4,19 @@ //! Flow (per design §3.1): //! 1. Verify the daemon over IPC and return its discovery info. //! 2. Only if no endpoint is listening and auto-start is enabled, spawn -//! `bsk daemon start` (the same binary), inheriting -//! `BSK_HOME` if set, and poll for verified IPC readiness until +//! the daemon directly through the shared background startup path, +//! inheriting `BSK_HOME` if set, and poll for verified IPC readiness until //! [`SPAWN_DEADLINE`] elapses. //! 3. If polling times out, return an error with hints. -use std::path::PathBuf; -use std::process::{Command, Stdio}; -use std::time::Duration; +use std::time::{Duration, Instant}; use anyhow::{Context, Result, ensure}; +use crate::cli::daemon::StartArgs; use crate::daemon::info::DaemonInfo; use crate::daemon::probe::{self, PROBE_TIMEOUT, Probe}; +use crate::daemon::start::start_background; /// Maximum time to wait for an auto-spawned daemon to become ready. pub const SPAWN_DEADLINE: Duration = Duration::from_millis(3_000); @@ -33,39 +33,10 @@ pub(crate) const AUTO_START_DISABLED_HINT: &str = "automatic daemon startup is d /// Return verified discovery info, starting a daemon only when its discovery /// file or IPC listener is absent and auto-start is enabled. pub fn ensure_daemon() -> Result { + let deadline = Instant::now() + SPAWN_DEADLINE; if let Probe::Ready(daemon) = probe::probe(PROBE_TIMEOUT)? { return Ok(daemon.info); } ensure!(auto_start_enabled(), AUTO_START_DISABLED_HINT); - spawn_daemon()?; - probe::wait_for_ready(SPAWN_DEADLINE) - .map(|daemon| daemon.info) - .with_context(|| "auto-spawned daemon failed to become ready in time") -} - -fn spawn_daemon() -> Result<()> { - let exe = bsk_executable()?; - let mut cmd = Command::new(exe); - cmd.arg("daemon") - .arg("start") - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::piped()); - // The child re-uses inherited env (BSK_HOME etc), so tests that set - // a temp home work transparently. - let output = cmd - .output() - .context("spawn `bsk daemon start` for auto-spawn")?; - if !output.status.success() { - return Err(anyhow::anyhow!( - "`bsk daemon start` exited with status {:?}: {}", - output.status, - String::from_utf8_lossy(&output.stderr).trim() - )); - } - Ok(()) -} - -fn bsk_executable() -> Result { - std::env::current_exe().context("locate current executable for auto-spawn") + start_background(&StartArgs::default(), deadline).context("automatic daemon startup failed") } diff --git a/crates/bsk-cli/src/cli/update/windows.rs b/crates/bsk-cli/src/cli/update/windows.rs index 6b4ca9ed..764d8299 100644 --- a/crates/bsk-cli/src/cli/update/windows.rs +++ b/crates/bsk-cli/src/cli/update/windows.rs @@ -1,76 +1,15 @@ -//! Launch the update helper with only its stdio handles inherited. -//! -//! std::process::Command on Windows also inherits other inheritable handles. -//! A daemon's listening socket must not survive in the helper: it would keep -//! the port occupied after the daemon exits and prevent its replacement starting. +//! The update helper inherits only its dedicated stdio handles. Keep its +//! existing Job policy: daemon startup separately requires verified breakaway. use std::ffi::OsStr; use std::fs::File; use std::io; -use std::os::windows::ffi::OsStrExt; -use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle}; -use std::os::windows::process::ExitStatusExt; use std::path::Path; -use std::process::ExitStatus; -use windows_sys::Win32::Foundation::{ - HANDLE, HANDLE_FLAG_INHERIT, SetHandleInformation, WAIT_OBJECT_0, WAIT_TIMEOUT, -}; -use windows_sys::Win32::System::Threading::{ - CREATE_NEW_PROCESS_GROUP, CREATE_NO_WINDOW, CREATE_UNICODE_ENVIRONMENT, CreateProcessW, - DeleteProcThreadAttributeList, EXTENDED_STARTUPINFO_PRESENT, GetExitCodeProcess, - InitializeProcThreadAttributeList, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, PROCESS_INFORMATION, - STARTF_USESTDHANDLES, STARTUPINFOEXW, TerminateProcess, UpdateProcThreadAttribute, - WaitForSingleObject, -}; +use windows_sys::Win32::System::Threading::{CREATE_NEW_PROCESS_GROUP, CREATE_NO_WINDOW}; -pub(super) struct Helper(OwnedHandle); - -impl Helper { - pub(super) fn try_wait(&mut self) -> io::Result> { - // SAFETY: the owned process handle remains valid for both calls. - match unsafe { WaitForSingleObject(self.0.as_raw_handle(), 0) } { - WAIT_OBJECT_0 => { - let mut code = 0; - if unsafe { GetExitCodeProcess(self.0.as_raw_handle(), &mut code) } == 0 { - return Err(io::Error::last_os_error()); - } - Ok(Some(ExitStatus::from_raw(code))) - } - WAIT_TIMEOUT => Ok(None), - _ => Err(io::Error::last_os_error()), - } - } - - pub(super) fn kill(&mut self) -> io::Result<()> { - // SAFETY: this handle belongs to the helper we created. - if unsafe { TerminateProcess(self.0.as_raw_handle(), 1) } == 0 { - return Err(io::Error::last_os_error()); - } - Ok(()) - } - - pub(super) fn wait(&mut self) -> io::Result<()> { - // Only used after kill(); do not leave a live helper on startup failure. - if unsafe { WaitForSingleObject(self.0.as_raw_handle(), 5000) } != WAIT_OBJECT_0 { - return Err(io::Error::new( - io::ErrorKind::TimedOut, - "update helper did not exit", - )); - } - Ok(()) - } -} - -// The opaque attribute list needs pointer-aligned storage and explicit teardown. -struct AttributeList(Vec); - -impl Drop for AttributeList { - fn drop(&mut self) { - // SAFETY: this wrapper is constructed only after successful initialization. - unsafe { DeleteProcThreadAttributeList(self.0.as_mut_ptr().cast()) }; - } -} +use crate::windows_process; +pub(super) use crate::windows_process::Process as Helper; pub(super) fn spawn( script: &Path, @@ -81,12 +20,10 @@ pub(super) fn spawn( ) -> io::Result { let root = std::env::var_os("SystemRoot") .ok_or_else(|| io::Error::new(io::ErrorKind::NotFound, "SystemRoot is not set"))?; - let application = wide(Path::new(&root).join("System32/cmd.exe").as_os_str()); + let application = Path::new(&root).join("System32/cmd.exe"); // /S /C strips one outer pair of quotes. Environment expansion keeps paths // Unicode and avoids CRT escaping, batch-file encoding, and CALL expansion. - let mut command = wide(OsStr::new( - "cmd.exe /D /V:OFF /S /C \"\"%BSK_UPDATE_SCRIPT%\"\"", - )); + let command = OsStr::new("cmd.exe /D /V:OFF /S /C \"\"%BSK_UPDATE_SCRIPT%\"\""); let overrides = [ ("BSK_UPDATE_SCRIPT", script.as_os_str()), ("BSK_UPDATE_SOURCE", source.as_os_str()), @@ -109,95 +46,13 @@ pub(super) fn spawn( .into_iter() .map(|(key, value)| (key.into(), value.to_owned())), ); - env.sort_by_key(|(key, _)| key.to_string_lossy().to_uppercase()); - let mut environment = Vec::new(); - for (key, value) in env { - environment.extend(key.encode_wide()); - environment.push(b'=' as u16); - environment.extend(value.encode_wide()); - environment.push(0); - } - environment.push(0); - let input = File::open("NUL")?; let log = File::create(log_path)?; - let handles: [HANDLE; 2] = [input.as_raw_handle(), log.as_raw_handle()]; - for &handle in &handles { - // SAFETY: these are dedicated helper stdio handles, held until spawn - // finishes. The explicit handle list excludes every other parent handle. - if unsafe { SetHandleInformation(handle, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT) } == 0 { - return Err(io::Error::last_os_error()); - } - } - - let mut size = 0; - // SAFETY: the first call queries the allocation size, as required by Win32. - unsafe { InitializeProcThreadAttributeList(std::ptr::null_mut(), 1, 0, &mut size) }; - if size == 0 { - return Err(io::Error::last_os_error()); - } - let mut storage = vec![0usize; size.div_ceil(std::mem::size_of::())]; - // SAFETY: storage has the requested size and pointer alignment. - if unsafe { InitializeProcThreadAttributeList(storage.as_mut_ptr().cast(), 1, 0, &mut size) } - == 0 - { - return Err(io::Error::last_os_error()); - } - let mut attributes = AttributeList(storage); - // SAFETY: the handle array and attribute storage outlive CreateProcessW. - if unsafe { - UpdateProcThreadAttribute( - attributes.0.as_mut_ptr().cast(), - 0, - PROC_THREAD_ATTRIBUTE_HANDLE_LIST as usize, - handles.as_ptr().cast(), - std::mem::size_of_val(&handles), - std::ptr::null_mut(), - std::ptr::null(), - ) - } == 0 - { - return Err(io::Error::last_os_error()); - } - // SAFETY: these Win32 structs permit zero initialization; required fields - // are populated below before the process is created. - let mut startup: STARTUPINFOEXW = unsafe { std::mem::zeroed() }; - startup.StartupInfo.cb = std::mem::size_of::() as u32; - startup.StartupInfo.dwFlags = STARTF_USESTDHANDLES; - startup.StartupInfo.hStdInput = handles[0]; - startup.StartupInfo.hStdOutput = handles[1]; - startup.StartupInfo.hStdError = handles[1]; - startup.lpAttributeList = attributes.0.as_mut_ptr().cast(); - let mut process: PROCESS_INFORMATION = unsafe { std::mem::zeroed() }; - // SAFETY: strings are NUL-terminated UTF-16, the environment is double-NUL - // terminated, and all buffers/handles remain alive for the duration of spawn. - if unsafe { - CreateProcessW( - application.as_ptr(), - command.as_mut_ptr(), - std::ptr::null(), - std::ptr::null(), - 1, - CREATE_NO_WINDOW - | CREATE_NEW_PROCESS_GROUP - | CREATE_UNICODE_ENVIRONMENT - | EXTENDED_STARTUPINFO_PRESENT, - environment.as_ptr().cast(), - std::ptr::null(), - &startup.StartupInfo, - &mut process, - ) - } == 0 - { - return Err(io::Error::last_os_error()); - } - // SAFETY: successful CreateProcessW transfers these two handles to us. - let _thread = unsafe { OwnedHandle::from_raw_handle(process.hThread) }; - Ok(Helper(unsafe { - OwnedHandle::from_raw_handle(process.hProcess) - })) -} - -fn wide(value: &OsStr) -> Vec { - value.encode_wide().chain(Some(0)).collect() + windows_process::spawn( + application.as_os_str(), + command, + &env, + [&input, &log, &log], + CREATE_NO_WINDOW | CREATE_NEW_PROCESS_GROUP, + ) } diff --git a/crates/bsk-cli/src/daemon/start.rs b/crates/bsk-cli/src/daemon/start.rs index dd422c48..fd42384e 100644 --- a/crates/bsk-cli/src/daemon/start.rs +++ b/crates/bsk-cli/src/daemon/start.rs @@ -4,7 +4,7 @@ //! * `--foreground` — run the daemon loop in the current process and //! inherit stdio. Used by tests and `--foreground` users. //! * default — fork-and-detach a child copy of the same binary, wait -//! for it to write a valid `daemon.json`, then return success. +//! for verified IPC readiness within the startup deadline, then return success. //! //! Detachment uses a hidden `BSK_DAEMONIZED=1` env handoff: when the //! parent spawns the child it sets the env var; the child sees it on @@ -22,6 +22,7 @@ use bsk_protocol::StatusResult; use tracing::{debug, info, warn}; use crate::cli::daemon::StartArgs; +use crate::cli::ensure_daemon::SPAWN_DEADLINE; use crate::daemon::{ browsers::{BROWSER_LIVENESS_TICK, BROWSER_LIVENESS_TIMEOUT, EXTENSION_CONNECT_WAIT}, info as daemon_info, ipc, lockfile, paths, @@ -31,6 +32,14 @@ use crate::daemon::{ ws, }; +#[cfg(windows)] +mod windows; + +#[cfg(windows)] +type DaemonChild = crate::windows_process::Process; +#[cfg(not(windows))] +type DaemonChild = std::process::Child; + /// Internal env-var contract: the parent sets this on the spawned child /// to indicate "you are the daemon, detach yourself and run". pub(crate) const DAEMONIZED_ENV: &str = "BSK_DAEMONIZED"; @@ -130,6 +139,7 @@ pub fn run_start(args: StartArgs) -> Result<()> { return run_foreground(cfg); } + let deadline = Instant::now() + SPAWN_DEADLINE; if let Probe::Ready(daemon) = probe::probe(PROBE_TIMEOUT)? { let status = daemon.status; validate_existing_start(&args, &status)?; @@ -141,17 +151,77 @@ pub fn run_start(args: StartArgs) -> Result<()> { return Ok(()); } - // Parent: spawn ourselves detached and wait for ready. - spawn_detached(&args)?; - let daemon = probe::wait_for_ready(Duration::from_secs(3))?; + start_background(&args, deadline)?; + Ok(()) +} + +/// Shared explicit/automatic startup. Keep ownership of the child until IPC +/// readiness is verified, without an intermediate launcher or captured pipe. +pub(crate) fn start_background( + args: &StartArgs, + deadline: Instant, +) -> Result { + anyhow::ensure!( + Instant::now() < deadline, + "daemon startup deadline exceeded" + ); + let exe = std::env::current_exe().context("locate daemon executable")?; + let mut child = spawn_detached_at(&exe, args, None)?; + let result = probe::wait_for_ready(deadline.saturating_duration_since(Instant::now())); + let daemon = match result { + Ok(daemon) => daemon, + Err(err) => { + let exit = child.try_wait().ok().flatten(); + stop_starting_child(&mut child); + return Err(err.context(match exit { + Some(status) => format!("daemon child exited during startup: {status}"), + None => "daemon child failed to become ready".into(), + })); + } + }; + if daemon.info.pid != child.id() { + // Another concurrent starter won the lock. Never stop that daemon. + stop_starting_child(&mut child); + } if let Some(port) = args.port.filter(|port| *port != 0) { - anyhow::ensure!( - daemon.status.ws_port == port, - "daemon started on ws port {}, expected {port}", - daemon.status.ws_port - ); + if daemon.status.ws_port != port { + if daemon.info.pid == child.id() { + stop_starting_child(&mut child); + } + anyhow::bail!( + "daemon started on ws port {}, expected {port}", + daemon.status.ws_port + ); + } } - Ok(()) + disown_daemon(child); + Ok(daemon.info) +} + +// Automatic startup now makes the daemon a direct child of a business CLI. +// That CLI may keep running after an idle exit or update, so reap exited Unix +// children without making the command wait for the daemon's lifetime. +#[cfg(unix)] +fn disown_daemon(mut child: DaemonChild) { + std::thread::spawn(move || { + let _ = child.wait(); + }); +} + +#[cfg(not(unix))] +fn disown_daemon(child: DaemonChild) { + drop(child); +} + +fn stop_starting_child(child: &mut DaemonChild) { + if matches!(child.try_wait(), Ok(Some(_))) { + return; + } + if let Err(err) = child.kill() { + warn!(error = %err, "failed to stop daemon child after startup failure"); + return; + } + let _ = child.wait(); } /// `bsk daemon stop` entrypoint. @@ -258,6 +328,13 @@ fn wait_for_stopped(expected: &daemon_info::DaemonInfo, timeout: Duration) -> Re pub fn run_foreground(cfg: DaemonConfig) -> Result<()> { paths::ensure_bsk_home()?; let _log_guard = init_tracing(); + #[cfg(windows)] + info!( + pid = std::process::id(), + background = is_daemonized_child(), + in_job = ?crate::windows_process::current_process_in_job(), + "Windows daemon process started" + ); let lock = lockfile::acquire().context("acquire daemon lock")?; info!(?lock, "daemon lock acquired"); @@ -718,7 +795,7 @@ pub(crate) fn spawn_update_check_task( // runs when it was captured. if let Some(exe) = &exe_path { match restart_start_args(&state.config).and_then(|args| { - spawn_detached_at(exe, &args, Some(std::process::id())) + spawn_detached_at(exe, &args, Some(std::process::id())).map(drop) }) { Ok(()) => { info!( @@ -921,8 +998,8 @@ fn detach_stdio() -> Result<()> { #[cfg(windows)] fn detach_stdio() -> Result<()> { - // On Windows the parent spawned us with DETACHED_PROCESS so stdio - // is already detached. Nothing extra to do here. + // The parent supplied dedicated NUL handles with an explicit inheritance + // list and verified Job breakaway before resuming us. Ok(()) } @@ -931,26 +1008,25 @@ fn detach_stdio() -> Result<()> { Ok(()) } -#[cfg(unix)] -fn spawn_detached(args: &StartArgs) -> Result<()> { - let exe = std::env::current_exe().context("current_exe")?; - spawn_detached_at(&exe, args, None) -} - /// Spawn a detached daemon child running the binary at `exe`. When /// `predecessor_pid` is set, the child first waits for that process to /// exit ([`DAEMON_REPLACEMENT_WAIT_ENV`]) — used by the auto-update /// self-restart, where the on-disk binary has already been replaced, so /// the child runs the new version. #[cfg(unix)] -fn spawn_detached_at(exe: &Path, args: &StartArgs, predecessor_pid: Option) -> Result<()> { +fn spawn_detached_at( + exe: &Path, + args: &StartArgs, + predecessor_pid: Option, +) -> Result { use std::os::unix::process::CommandExt; let mut cmd = std::process::Command::new(exe); apply_start_args(&mut cmd, args); cmd.stdin(std::process::Stdio::null()) .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()) - .env(DAEMONIZED_ENV, "1"); + .env(DAEMONIZED_ENV, "1") + .env_remove(DAEMON_REPLACEMENT_WAIT_ENV); if let Some(pid) = predecessor_pid { cmd.env(DAEMON_REPLACEMENT_WAIT_ENV, pid.to_string()); } @@ -961,46 +1037,24 @@ fn spawn_detached_at(exe: &Path, args: &StartArgs, predecessor_pid: Option) Ok(()) }); } - let child = cmd.spawn().context("spawn detached daemon child")?; - drop(child); // parent immediately disowns; child runs independently - Ok(()) -} - -#[cfg(windows)] -fn spawn_detached(args: &StartArgs) -> Result<()> { - let exe = std::env::current_exe().context("current_exe")?; - spawn_detached_at(&exe, args, None) + cmd.spawn().context("spawn detached daemon child") } -/// Windows counterpart of the unix [`spawn_detached_at`]; see its docs. #[cfg(windows)] -fn spawn_detached_at(exe: &Path, args: &StartArgs, predecessor_pid: Option) -> Result<()> { - use std::os::windows::process::CommandExt; - const DETACHED_PROCESS: u32 = 0x0000_0008; - const CREATE_NEW_PROCESS_GROUP: u32 = 0x0000_0200; - let mut cmd = std::process::Command::new(exe); - apply_start_args(&mut cmd, args); - cmd.stdin(std::process::Stdio::null()) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .env(DAEMONIZED_ENV, "1"); - if let Some(pid) = predecessor_pid { - cmd.env(DAEMON_REPLACEMENT_WAIT_ENV, pid.to_string()); - } - cmd.creation_flags(DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP); - let _child = cmd.spawn().context("spawn detached daemon child")?; - Ok(()) -} - -#[cfg(not(any(unix, windows)))] -fn spawn_detached(_args: &StartArgs) -> Result<()> { - Err(anyhow::anyhow!( - "detached daemon spawn is not supported on this platform" - )) +fn spawn_detached_at( + exe: &Path, + args: &StartArgs, + predecessor_pid: Option, +) -> Result { + windows::spawn(exe, args, predecessor_pid) } #[cfg(not(any(unix, windows)))] -fn spawn_detached_at(_exe: &Path, _args: &StartArgs, _predecessor_pid: Option) -> Result<()> { +fn spawn_detached_at( + _exe: &Path, + _args: &StartArgs, + _predecessor_pid: Option, +) -> Result { Err(anyhow::anyhow!( "detached daemon spawn is not supported on this platform" )) @@ -1096,6 +1150,29 @@ fn send_kill(_pid: u32) -> Result<()> { mod tests { use super::*; + #[cfg(unix)] + #[test] + fn disowned_child_is_reaped_while_the_launcher_stays_alive() { + let child = std::process::Command::new("/bin/sh") + .args(["-c", "exit 0"]) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .unwrap(); + let pid = child.id(); + disown_daemon(child); + let deadline = Instant::now() + Duration::from_secs(3); + // kill(pid, 0) also sees zombies; disappearance proves wait() reaped it. + while lockfile::pid_alive(pid) { + assert!( + Instant::now() < deadline, + "disowned child remained a zombie" + ); + std::thread::sleep(Duration::from_millis(10)); + } + } + #[test] fn stopping_rejects_replacement_metadata_with_or_without_a_held_lock() { crate::daemon::test_support::isolated( diff --git a/crates/bsk-cli/src/daemon/start/windows.rs b/crates/bsk-cli/src/daemon/start/windows.rs new file mode 100644 index 00000000..c32afed6 --- /dev/null +++ b/crates/bsk-cli/src/daemon/start/windows.rs @@ -0,0 +1,87 @@ +//! Windows daemon startup: isolate handles and verify Job breakaway before +//! allowing the child to acquire the daemon lock or publish discovery metadata. + +use std::ffi::{OsStr, OsString}; +use std::fs::File; +use std::os::windows::ffi::{OsStrExt, OsStringExt}; +use std::path::Path; + +use anyhow::{Context, Result}; +use windows_sys::Win32::System::Threading::{ + CREATE_BREAKAWAY_FROM_JOB, CREATE_NEW_PROCESS_GROUP, CREATE_SUSPENDED, DETACHED_PROCESS, +}; + +use super::{DAEMON_REPLACEMENT_WAIT_ENV, DAEMONIZED_ENV, StartArgs, apply_start_args}; +use crate::windows_process::{self, Process}; + +const DETACH_HINT: &str = "cannot start an independent Windows daemon; the host may prohibit Job Object breakaway. \ + Run `bsk daemon start --foreground` in a persistent host task outside the per-command Job, \ + or start the daemon from an independent terminal, using the same BSK_HOME and OS user. \ + Then use BSK_AUTO_START=0 in the agent"; + +pub(super) fn spawn(exe: &Path, args: &StartArgs, predecessor_pid: Option) -> Result { + let mut command = std::process::Command::new(exe); + apply_start_args(&mut command, args); + let command_line = command_line(std::iter::once(exe.as_os_str()).chain(command.get_args())); + let mut env: Vec<_> = std::env::vars_os() + .filter(|(key, _)| { + let key = key.to_string_lossy(); + !key.eq_ignore_ascii_case(DAEMONIZED_ENV) + && !key.eq_ignore_ascii_case(DAEMON_REPLACEMENT_WAIT_ENV) + }) + .collect(); + env.push((DAEMONIZED_ENV.into(), "1".into())); + if let Some(pid) = predecessor_pid { + env.push((DAEMON_REPLACEMENT_WAIT_ENV.into(), pid.to_string().into())); + } + let input = File::open("NUL").context("open daemon stdin")?; + let output = File::options() + .write(true) + .open("NUL") + .context("open daemon output")?; + let mut child = windows_process::spawn( + exe.as_os_str(), + &command_line, + &env, + [&input, &output, &output], + DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP | CREATE_BREAKAWAY_FROM_JOB | CREATE_SUSPENDED, + ) + .context(DETACH_HINT)?; + if let Err(err) = child.resume_outside_job() { + // A suspended child must never be left behind if validation/resume fails. + let _ = child.kill(); + let _ = child.wait(); + return Err(err).context(DETACH_HINT); + } + Ok(child) +} + +/// Quote argv directly for the Windows CRT, without invoking a shell. Preserve +/// UTF-16 paths, embedded quotes and backslashes before a closing quote. +fn command_line<'a>(args: impl Iterator) -> OsString { + let mut result = Vec::new(); + for arg in args { + if !result.is_empty() { + result.push(b' ' as u16); + } + result.push(b'"' as u16); + let mut slashes = 0; + for ch in arg.encode_wide() { + if ch == b'\\' as u16 { + slashes += 1; + continue; + } + let count = if ch == b'"' as u16 { + slashes * 2 + 1 + } else { + slashes + }; + result.extend(std::iter::repeat_n(b'\\' as u16, count)); + result.push(ch); + slashes = 0; + } + result.extend(std::iter::repeat_n(b'\\' as u16, slashes * 2)); + result.push(b'"' as u16); + } + OsString::from_wide(&result) +} diff --git a/crates/bsk-cli/src/lib.rs b/crates/bsk-cli/src/lib.rs index 1b2917ab..0b9e2388 100644 --- a/crates/bsk-cli/src/lib.rs +++ b/crates/bsk-cli/src/lib.rs @@ -8,4 +8,7 @@ pub mod rpc_reason { } pub mod skill_install; +#[cfg(windows)] +mod windows_process; + pub use cli::{Cli, Command}; diff --git a/crates/bsk-cli/src/windows_process.rs b/crates/bsk-cli/src/windows_process.rs new file mode 100644 index 00000000..4d12b59b --- /dev/null +++ b/crates/bsk-cli/src/windows_process.rs @@ -0,0 +1,238 @@ +//! Native process creation with an explicit handle inheritance list. +//! +//! Redirecting stdio alone does not stop other inheritable handles (including +//! a caller's output pipes or a daemon's listening socket) reaching a child. + +use std::ffi::{OsStr, OsString}; +use std::fs::File; +use std::io; +use std::os::windows::ffi::OsStrExt; +use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle}; +use std::os::windows::process::ExitStatusExt; +use std::process::ExitStatus; + +use windows_sys::Win32::Foundation::{ + DUPLICATE_SAME_ACCESS, DuplicateHandle, HANDLE, WAIT_OBJECT_0, WAIT_TIMEOUT, +}; +use windows_sys::Win32::System::JobObjects::IsProcessInJob; +use windows_sys::Win32::System::Threading::{ + CREATE_UNICODE_ENVIRONMENT, CreateProcessW, DeleteProcThreadAttributeList, + EXTENDED_STARTUPINFO_PRESENT, GetCurrentProcess, GetExitCodeProcess, + InitializeProcThreadAttributeList, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, PROCESS_INFORMATION, + ResumeThread, STARTF_USESTDHANDLES, STARTUPINFOEXW, TerminateProcess, + UpdateProcThreadAttribute, WaitForSingleObject, +}; + +pub(crate) struct Process { + handle: OwnedHandle, + thread: OwnedHandle, + pid: u32, +} + +impl Process { + pub(crate) fn id(&self) -> u32 { + self.pid + } + + pub(crate) fn try_wait(&mut self) -> io::Result> { + // SAFETY: the owned process handle remains valid for both calls. + match unsafe { WaitForSingleObject(self.handle.as_raw_handle(), 0) } { + WAIT_OBJECT_0 => { + let mut code = 0; + if unsafe { GetExitCodeProcess(self.handle.as_raw_handle(), &mut code) } == 0 { + return Err(io::Error::last_os_error()); + } + Ok(Some(ExitStatus::from_raw(code))) + } + WAIT_TIMEOUT => Ok(None), + _ => Err(io::Error::last_os_error()), + } + } + + pub(crate) fn kill(&mut self) -> io::Result<()> { + // SAFETY: this is the process we created, not a reopened/reused PID. + if unsafe { TerminateProcess(self.handle.as_raw_handle(), 1) } == 0 { + return Err(io::Error::last_os_error()); + } + Ok(()) + } + + pub(crate) fn wait(&mut self) -> io::Result<()> { + // Only used for cleanup after kill; never wait indefinitely on a child. + if unsafe { WaitForSingleObject(self.handle.as_raw_handle(), 5000) } != WAIT_OBJECT_0 { + return Err(io::Error::new( + io::ErrorKind::TimedOut, + "child did not exit", + )); + } + Ok(()) + } + + /// Called only for a child created suspended. A nested Job may allow only + /// partial breakaway, so successful CreateProcessW is not sufficient. + pub(crate) fn resume_outside_job(&self) -> io::Result<()> { + if process_in_job(self.handle.as_raw_handle())? { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "daemon is still associated with a host Job Object after breakaway", + )); + } + // SAFETY: this is the primary thread returned by CreateProcessW. + if unsafe { ResumeThread(self.thread.as_raw_handle()) } == u32::MAX { + return Err(io::Error::last_os_error()); + } + Ok(()) + } +} + +// The opaque attribute list needs pointer-aligned storage and explicit teardown. +struct AttributeList(Vec); + +impl Drop for AttributeList { + fn drop(&mut self) { + // SAFETY: constructed only after successful initialization. + unsafe { DeleteProcThreadAttributeList(self.0.as_mut_ptr().cast()) }; + } +} + +pub(crate) fn spawn( + application: &OsStr, + command_line: &OsStr, + environment: &[(OsString, OsString)], + stdio: [&File; 3], + flags: u32, +) -> io::Result { + let application = wide(application)?; + let mut command = wide(command_line)?; + let mut environment = environment.to_vec(); + environment.sort_by_key(|(key, _)| key.to_string_lossy().to_uppercase()); + let mut block = Vec::new(); + for (key, value) in environment { + let mut entry = key; + entry.push("="); + entry.push(value); + block.extend(wide(&entry)?); + } + if block.is_empty() { + block.push(0); + } + block.push(0); + + // Duplicate dedicated stdio handles rather than changing inheritability + // on caller-owned handles. All temporary copies close on every return path. + let inherited = stdio.map(duplicate_inheritable); + let inherited = inherited.into_iter().collect::>>()?; + let handles: Vec = inherited.iter().map(AsRawHandle::as_raw_handle).collect(); + + let mut size = 0; + // SAFETY: the first call queries the allocation size, as required by Win32. + unsafe { InitializeProcThreadAttributeList(std::ptr::null_mut(), 1, 0, &mut size) }; + if size == 0 { + return Err(io::Error::last_os_error()); + } + let mut storage = vec![0usize; size.div_ceil(std::mem::size_of::())]; + // SAFETY: storage has the requested size and pointer alignment. + if unsafe { InitializeProcThreadAttributeList(storage.as_mut_ptr().cast(), 1, 0, &mut size) } + == 0 + { + return Err(io::Error::last_os_error()); + } + let mut attributes = AttributeList(storage); + // SAFETY: the handle array and attribute storage outlive CreateProcessW. + if unsafe { + UpdateProcThreadAttribute( + attributes.0.as_mut_ptr().cast(), + 0, + PROC_THREAD_ATTRIBUTE_HANDLE_LIST as usize, + handles.as_ptr().cast(), + std::mem::size_of_val(handles.as_slice()), + std::ptr::null_mut(), + std::ptr::null(), + ) + } == 0 + { + return Err(io::Error::last_os_error()); + } + // SAFETY: these structs permit zero initialization; required fields follow. + let mut startup: STARTUPINFOEXW = unsafe { std::mem::zeroed() }; + startup.StartupInfo.cb = std::mem::size_of::() as u32; + startup.StartupInfo.dwFlags = STARTF_USESTDHANDLES; + startup.StartupInfo.hStdInput = handles[0]; + startup.StartupInfo.hStdOutput = handles[1]; + startup.StartupInfo.hStdError = handles[2]; + startup.lpAttributeList = attributes.0.as_mut_ptr().cast(); + let mut process: PROCESS_INFORMATION = unsafe { std::mem::zeroed() }; + // SAFETY: UTF-16 strings and environment are terminated, and every supplied + // buffer and inherited handle remains alive until CreateProcessW returns. + if unsafe { + CreateProcessW( + application.as_ptr(), + command.as_mut_ptr(), + std::ptr::null(), + std::ptr::null(), + 1, + flags | CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT, + block.as_ptr().cast(), + std::ptr::null(), + &startup.StartupInfo, + &mut process, + ) + } == 0 + { + return Err(io::Error::last_os_error()); + } + // SAFETY: successful CreateProcessW transfers these handles to us. + Ok(Process { + handle: unsafe { OwnedHandle::from_raw_handle(process.hProcess) }, + thread: unsafe { OwnedHandle::from_raw_handle(process.hThread) }, + pid: process.dwProcessId, + }) +} + +fn duplicate_inheritable(file: &File) -> io::Result { + let mut handle = std::ptr::null_mut(); + // SAFETY: duplicate a live file into this process with the same access. + if unsafe { + DuplicateHandle( + GetCurrentProcess(), + file.as_raw_handle(), + GetCurrentProcess(), + &mut handle, + 0, + 1, + DUPLICATE_SAME_ACCESS, + ) + } == 0 + { + return Err(io::Error::last_os_error()); + } + // SAFETY: DuplicateHandle returned a new owned handle. + Ok(unsafe { OwnedHandle::from_raw_handle(handle) }) +} + +fn wide(value: &OsStr) -> io::Result> { + let mut value: Vec = value.encode_wide().collect(); + if value.contains(&0) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "interior NUL in process argument", + )); + } + value.push(0); + Ok(value) +} + +/// Startup diagnostics also cover foreground daemons owned by a supervisor. +pub(crate) fn current_process_in_job() -> io::Result { + // SAFETY: the pseudo handle is valid for the current process. + process_in_job(unsafe { GetCurrentProcess() }) +} + +fn process_in_job(process: HANDLE) -> io::Result { + let mut in_job = 0; + // SAFETY: a null job queries membership in any Job; the output is valid. + if unsafe { IsProcessInJob(process, std::ptr::null_mut(), &mut in_job) } == 0 { + return Err(io::Error::last_os_error()); + } + Ok(in_job != 0) +} diff --git a/crates/bsk-cli/tests/windows_daemon_start.rs b/crates/bsk-cli/tests/windows_daemon_start.rs new file mode 100644 index 00000000..816b6c45 --- /dev/null +++ b/crates/bsk-cli/tests/windows_daemon_start.rs @@ -0,0 +1,535 @@ +//! Native Windows startup regression: process exit, pipe EOF and Job lifetime +//! are independent assertions. Every process uses a private BSK_HOME. +#![cfg(windows)] + +use std::fs::{self, File}; +use std::io::{Read, Write}; +use std::net::TcpListener; +use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle}; +use std::os::windows::process::CommandExt; +use std::path::PathBuf; +use std::process::{Child, Command, Output, Stdio}; +use std::sync::mpsc::{self, Receiver}; +use std::thread; +use std::time::{Duration, Instant}; + +use bsk::daemon::info::DaemonInfo; +use windows_sys::Win32::Foundation::{HANDLE, WAIT_OBJECT_0, WAIT_TIMEOUT}; +use windows_sys::Win32::System::JobObjects::{ + AssignProcessToJobObject, CreateJobObjectW, IsProcessInJob, JOB_OBJECT_LIMIT_BREAKAWAY_OK, + JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, JOB_OBJECT_LIMIT_SILENT_BREAKAWAY_OK, + JOBOBJECT_BASIC_ACCOUNTING_INFORMATION, JOBOBJECT_EXTENDED_LIMIT_INFORMATION, + JobObjectBasicAccountingInformation, JobObjectExtendedLimitInformation, + QueryInformationJobObject, SetInformationJobObject, TerminateJobObject, +}; +use windows_sys::Win32::System::Threading::{ + CREATE_BREAKAWAY_FROM_JOB, CREATE_NO_WINDOW, OpenProcess, PROCESS_QUERY_LIMITED_INFORMATION, + PROCESS_SYNCHRONIZE, PROCESS_TERMINATE, TerminateProcess, WaitForSingleObject, +}; + +const BUDGET: Duration = Duration::from_secs(8); + +struct Job(OwnedHandle); + +impl Job { + fn new(breakaway: u32) -> Self { + // SAFETY: no name or inheritable security descriptor; this test owns it. + let handle = unsafe { CreateJobObjectW(std::ptr::null(), std::ptr::null()) }; + assert!(!handle.is_null(), "{}", std::io::Error::last_os_error()); + let job = Self(unsafe { OwnedHandle::from_raw_handle(handle) }); + let mut limits: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = unsafe { std::mem::zeroed() }; + limits.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE | breakaway; + assert_ne!( + unsafe { + SetInformationJobObject( + job.0.as_raw_handle(), + JobObjectExtendedLimitInformation, + (&limits as *const JOBOBJECT_EXTENDED_LIMIT_INFORMATION).cast(), + std::mem::size_of_val(&limits) as u32, + ) + }, + 0, + "{}", + std::io::Error::last_os_error() + ); + job + } + + fn assign(&self, child: &Child) { + assert_ne!( + unsafe { AssignProcessToJobObject(self.0.as_raw_handle(), child.as_raw_handle()) }, + 0, + "{}", + std::io::Error::last_os_error() + ); + } + + fn assert_empty(&self) { + // Job accounting can lag a process's signaled exit handle briefly. + // Poll with a deadline so a leaked suspended daemon still fails. + let deadline = Instant::now() + Duration::from_secs(2); + loop { + let mut info: JOBOBJECT_BASIC_ACCOUNTING_INFORMATION = unsafe { std::mem::zeroed() }; + assert_ne!( + unsafe { + QueryInformationJobObject( + self.0.as_raw_handle(), + JobObjectBasicAccountingInformation, + (&mut info as *mut JOBOBJECT_BASIC_ACCOUNTING_INFORMATION).cast(), + std::mem::size_of_val(&info) as u32, + std::ptr::null_mut(), + ) + }, + 0 + ); + if info.ActiveProcesses == 0 { + return; + } + assert!( + Instant::now() < deadline, + "startup left {} process(es) in the host Job", + info.ActiveProcesses + ); + thread::sleep(Duration::from_millis(10)); + } + } + + fn terminate(&self) { + assert_ne!(unsafe { TerminateJobObject(self.0.as_raw_handle(), 99) }, 0); + } +} + +fn in_job(process: HANDLE, job: HANDLE) -> bool { + let mut result = 0; + assert_ne!(unsafe { IsProcessInJob(process, job, &mut result) }, 0); + result != 0 +} + +fn open_process(pid: u32) -> OwnedHandle { + let handle = unsafe { + OpenProcess( + PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_TERMINATE | PROCESS_SYNCHRONIZE, + 0, + pid, + ) + }; + assert!( + !handle.is_null(), + "open process {pid}: {}", + std::io::Error::last_os_error() + ); + unsafe { OwnedHandle::from_raw_handle(handle) } +} + +fn assert_alive(process: &OwnedHandle) { + assert_eq!( + unsafe { WaitForSingleObject(process.as_raw_handle(), 0) }, + WAIT_TIMEOUT + ); +} + +fn drain(mut pipe: impl Read + Send + 'static) -> Receiver> { + let (tx, rx) = mpsc::channel(); + thread::spawn(move || { + let mut bytes = Vec::new(); + pipe.read_to_end(&mut bytes).unwrap(); + let _ = tx.send(bytes); + }); + rx +} + +struct Captured { + child: Child, + stdout: Receiver>, + stderr: Receiver>, + started: Instant, +} + +impl Captured { + fn finish(&mut self) -> Output { + let deadline = self.started + BUDGET; + let status = loop { + if let Some(status) = self.child.try_wait().unwrap() { + break status; + } + assert!( + Instant::now() < deadline, + "launcher did not exit within {BUDGET:?}" + ); + thread::sleep(Duration::from_millis(10)); + }; + // Unlike wait_with_output(), these assertions distinguish launcher exit + // from a daemon retaining either captured pipe's write end. + let stdout = self + .stdout + .recv_timeout(deadline.saturating_duration_since(Instant::now())) + .expect("launcher exited but stdout never reached EOF"); + let stderr = self + .stderr + .recv_timeout(deadline.saturating_duration_since(Instant::now())) + .expect("launcher exited but stderr never reached EOF"); + Output { + status, + stdout, + stderr, + } + } +} + +impl Drop for Captured { + fn drop(&mut self) { + if self.child.try_wait().ok().flatten().is_none() { + let _ = self.child.kill(); + let _ = self.child.wait(); + } + } +} + +struct Fixture { + _temp: tempfile::TempDir, + home: PathBuf, + exe: PathBuf, + auto_start: bool, +} + +impl Fixture { + fn new() -> Self { + let temp = tempfile::tempdir().unwrap(); + let home = temp.path().join("中文 daemon home & %PATH% !"); + fs::create_dir(&home).unwrap(); + Self { + _temp: temp, + home, + exe: env!("CARGO_BIN_EXE_bsk").into(), + auto_start: true, + } + } + + fn launch(&self, args: &[&str], jobs: &[&Job]) -> Captured { + let mut command = Command::new(std::env::current_exe().unwrap()); + command + .args([ + "--exact", + "launcher_process", + "--ignored", + "--nocapture", + "--quiet", + ]) + .env("BSK_START_TEST_EXE", &self.exe) + .env("BSK_START_TEST_ARGS", serde_json::to_string(args).unwrap()) + .env("BSK_HOME", &self.home) + .env("BSK_AUTO_UPDATE", "off") + .env("BSK_UPDATE_MANIFEST_URL", "http://127.0.0.1:1/unreachable") + .env("BSK_BROWSER_WAIT_MS", "0") + .env("BSK_DOCTOR_BROWSER_WAIT_MS", "0") + .env("RUST_LOG", "error") + .env("BSK_AUTO_START", if self.auto_start { "1" } else { "0" }) + .env_remove("BSK_DAEMONIZED") + .env_remove("BSK_DAEMON_REPLACES_PID") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + // Isolate our fixture from a breakaway-capable CI runner's Job. + // Test Jobs below are assigned explicitly before the gate opens. + .creation_flags(CREATE_NO_WINDOW | CREATE_BREAKAWAY_FROM_JOB); + let started = Instant::now(); + let mut child = command + .spawn() + .expect("test host must allow a process outside its own Job"); + for job in jobs { + job.assign(&child); + } + let stdout = drain(child.stdout.take().unwrap()); + let stderr = drain(child.stderr.take().unwrap()); + child.stdin.take().unwrap().write_all(b"go\n").unwrap(); + Captured { + child, + stdout, + stderr, + started, + } + } + + fn run(&self, args: &[&str], jobs: &[&Job]) -> Output { + self.launch(args, jobs).finish() + } + + fn info(&self) -> DaemonInfo { + serde_json::from_slice(&fs::read(self.home.join("daemon.json")).unwrap()).unwrap() + } + + fn wait_for_info(&self) -> DaemonInfo { + let deadline = Instant::now() + BUDGET; + loop { + if let Ok(bytes) = fs::read(self.home.join("daemon.json")) { + if let Ok(info) = serde_json::from_slice(&bytes) { + return info; + } + } + assert!( + Instant::now() < deadline, + "daemon did not publish discovery" + ); + thread::sleep(Duration::from_millis(10)); + } + } + + fn assert_stopped(&self) { + assert!( + !self.home.join("daemon.json").exists(), + "failed startup published a daemon" + ); + if let Ok(lock) = File::open(self.home.join("daemon.lock")) { + fs2::FileExt::try_lock_exclusive(&lock).expect("startup left a locked daemon"); + } + } +} + +impl Drop for Fixture { + fn drop(&mut self) { + // Only this fixture can publish into this fresh private home. Keep a + // process handle through termination, so cleanup cannot target a new PID. + if let Ok(bytes) = fs::read(self.home.join("daemon.json")) { + if let Ok(info) = serde_json::from_slice::(&bytes) { + let handle = + unsafe { OpenProcess(PROCESS_TERMINATE | PROCESS_SYNCHRONIZE, 0, info.pid) }; + if !handle.is_null() { + let handle = unsafe { OwnedHandle::from_raw_handle(handle) }; + unsafe { + TerminateProcess(handle.as_raw_handle(), 0); + WaitForSingleObject(handle.as_raw_handle(), 5000); + } + } + } + } + } +} + +fn success(output: Output) { + assert!( + output.status.success(), + "stdout: {}\nstderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + +fn refused(output: Output) { + assert!(!output.status.success()); + let text = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!(text.contains("Job Object"), "{text}"); + assert!(text.contains("persistent host task"), "{text}"); + assert!(text.contains("BSK_AUTO_START=0"), "{text}"); +} + +// Re-exec the test binary as a gated launcher so assignment to a Job happens +// before bsk can create a daemon. No shell or separately compiled fixture needed. +#[test] +#[ignore = "subprocess entry point"] +fn launcher_process() { + let Some(exe) = std::env::var_os("BSK_START_TEST_EXE") else { + return; + }; + let mut gate = String::new(); + std::io::stdin().read_line(&mut gate).unwrap(); + assert_eq!(gate, "go\n"); + let args: Vec = + serde_json::from_str(&std::env::var("BSK_START_TEST_ARGS").unwrap()).unwrap(); + let status = Command::new(exe) + .args(args) + .stdin(Stdio::null()) + .status() + .unwrap(); + std::process::exit(status.code().unwrap_or(1)); +} + +#[test] +fn detached_start_closes_both_pipes_and_reuses_the_daemon() { + let mut fixture = Fixture::new(); + let exe_dir = fixture._temp.path().join("中文 bin space & %PATH% !"); + fs::create_dir(&exe_dir).unwrap(); + fixture.exe = exe_dir.join("bsk.exe"); + fs::copy(env!("CARGO_BIN_EXE_bsk"), &fixture.exe).unwrap(); + success(fixture.run(&["daemon", "start", "--port", "0"], &[])); + let original = fixture.info(); + let process = open_process(original.pid); + assert!(!in_job(process.as_raw_handle(), std::ptr::null_mut())); + for _ in 0..3 { + success(fixture.run(&["status", "--json"], &[])); + success(fixture.run(&["daemon", "start"], &[])); + assert_eq!(fixture.info().pid, original.pid); + assert_alive(&process); + } +} + +#[test] +fn detached_daemon_survives_job_close_and_termination() { + for flags in [ + JOB_OBJECT_LIMIT_BREAKAWAY_OK, + JOB_OBJECT_LIMIT_SILENT_BREAKAWAY_OK, + ] { + for terminate in [false, true] { + let fixture = Fixture::new(); + let job = Job::new(flags); + success(fixture.run(&["daemon", "start", "--port", "0"], &[&job])); + let process = open_process(fixture.info().pid); + assert!(!in_job(process.as_raw_handle(), job.0.as_raw_handle())); + assert!(!in_job(process.as_raw_handle(), std::ptr::null_mut())); + if terminate { + job.terminate(); + } + drop(job); + assert_alive(&process); + success(fixture.run(&["status", "--json"], &[])); + } + } +} + +#[test] +fn restrictive_job_refuses_explicit_and_automatic_start_without_a_daemon() { + for args in [ + vec!["daemon", "start", "--port", "0"], + vec!["status", "--json"], + vec!["doctor", "--json"], + vec!["browsers", "--json"], + ] { + let fixture = Fixture::new(); + let job = Job::new(0); + refused(fixture.run(&args, &[&job])); + fixture.assert_stopped(); + job.assert_empty(); + } +} + +#[test] +fn nested_jobs_require_breakaway_from_every_level() { + for outer_flags in [0, JOB_OBJECT_LIMIT_BREAKAWAY_OK] { + let fixture = Fixture::new(); + let outer = Job::new(outer_flags); + let inner = Job::new(JOB_OBJECT_LIMIT_BREAKAWAY_OK); + let output = fixture.run(&["daemon", "start", "--port", "0"], &[&outer, &inner]); + if outer_flags == 0 { + refused(output); + fixture.assert_stopped(); + inner.assert_empty(); + outer.assert_empty(); + } else { + success(output); + let process = open_process(fixture.info().pid); + assert!(!in_job(process.as_raw_handle(), std::ptr::null_mut())); + drop(inner); + drop(outer); + assert_alive(&process); + success(fixture.run(&["status", "--json"], &[])); + } + } +} + +#[test] +fn restrictive_job_reuses_an_existing_independent_daemon() { + let fixture = Fixture::new(); + success(fixture.run(&["daemon", "start", "--port", "0"], &[])); + let original = fixture.info(); + let process = open_process(original.pid); + let job = Job::new(0); + for args in [vec!["daemon", "start"], vec!["status", "--json"]] { + success(fixture.run(&args, &[&job])); + assert_eq!(fixture.info().pid, original.pid); + } + drop(job); + assert_alive(&process); +} + +#[test] +fn foreground_stays_owned_by_the_host_job() { + let fixture = Fixture::new(); + let job = Job::new(0); + let mut launcher = fixture.launch(&["daemon", "start", "--foreground", "--port", "0"], &[&job]); + let process = open_process(fixture.wait_for_info().pid); + assert!(in_job(process.as_raw_handle(), job.0.as_raw_handle())); + assert_alive(&process); + drop(job); + assert_eq!( + unsafe { WaitForSingleObject(process.as_raw_handle(), 5000) }, + WAIT_OBJECT_0 + ); + // KILL_ON_JOB_CLOSE does not promise a nonzero process exit code. + // Termination and EOF, verified independently, are the contract here. + let _ = launcher.finish(); +} + +#[test] +fn failed_start_is_bounded_and_releases_the_daemon_lock() { + let fixture = Fixture::new(); + let occupied = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = occupied.local_addr().unwrap().port().to_string(); + let output = fixture.run(&["daemon", "start", "--port", &port], &[]); + assert!(!output.status.success()); + assert!(String::from_utf8_lossy(&output.stderr).contains("exited during startup")); + fixture.assert_stopped(); + drop(occupied); + success(fixture.run(&["daemon", "start", "--port", &port], &[])); +} + +#[test] +fn concurrent_starters_share_a_single_ready_daemon() { + let fixture = Fixture::new(); + let reservation = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = reservation.local_addr().unwrap().port().to_string(); + drop(reservation); + let mut launchers: Vec<_> = (0..6) + .map(|_| fixture.launch(&["daemon", "start", "--port", &port], &[])) + .collect(); + for launcher in &mut launchers { + success(launcher.finish()); + } + let info = fixture.info(); + assert_eq!(info.ws_port, port.parse::().unwrap()); + success(fixture.run(&["status", "--json"], &[])); + assert_eq!(fixture.info().pid, info.pid); +} + +#[test] +fn automatic_start_returns_eof_and_reuses_the_same_daemon() { + // Auto-start intentionally uses the production default port. Never stop an + // unrelated developer daemon to free it; CI must supply a free port. + match TcpListener::bind("127.0.0.1:52800") { + Ok(listener) => drop(listener), + Err(err) if err.kind() == std::io::ErrorKind::AddrInUse => { + assert!( + std::env::var_os("CI").is_none(), + "default port unavailable: {err}" + ); + eprintln!("skipping auto-start success case: default port unavailable: {err}"); + return; + } + Err(err) => panic!("check default port: {err}"), + } + let fixture = Fixture::new(); + success(fixture.run(&["status", "--json"], &[])); + let original = fixture.info(); + success(fixture.run(&["status", "--json"], &[])); + assert_eq!(fixture.info().pid, original.pid); + let process = open_process(original.pid); + assert!(!in_job(process.as_raw_handle(), std::ptr::null_mut())); +} + +#[test] +fn disabled_auto_start_preserves_absence_but_allows_explicit_start_and_reuse() { + let mut fixture = Fixture::new(); + fixture.auto_start = false; + let output = fixture.run(&["status", "--json"], &[]); + assert!(!output.status.success()); + assert!(String::from_utf8_lossy(&output.stdout).contains("BSK_AUTO_START=0")); + fixture.assert_stopped(); + assert!(!fixture.home.join("daemon.lock").exists()); + success(fixture.run(&["daemon", "start", "--port", "0"], &[])); + let original = fixture.info(); + let job = Job::new(0); + success(fixture.run(&["status", "--json"], &[&job])); + assert_eq!(fixture.info().pid, original.pid); +} diff --git a/docs/sandboxed-agents.md b/docs/sandboxed-agents.md index a45b12a6..fd92490f 100644 --- a/docs/sandboxed-agents.md +++ b/docs/sandboxed-agents.md @@ -10,6 +10,30 @@ The same setup applies to Windows agents whose shell tasks terminate child proce Ordinary local use still auto-starts the daemon. No sandbox detection, service installation or global change to home-directory resolution is required. +## Windows background startup + +On Windows, background startup inherits only dedicated standard handles and +requests breakaway from the launching process's Job Object. The daemon is +resumed only after verifying it belongs to no Job, including outer Jobs in a +nested hierarchy. The command returns success after verifying local IPC; its +captured stdout and stderr can reach EOF independently of the daemon lifetime. + +A host that prohibits breakaway cannot launch an independent background daemon +through this path. `bsk daemon start` and implicit startup return a bounded +error with setup instructions instead of retrying as a host-owned background +process. An already reachable daemon is still reused, even from a restrictive +Job. Use the persistent host setup below when breakaway is unavailable. + +`--foreground` deliberately remains owned by its host task. Run that task +outside the per-command Job and keep it alive; the flag does not bypass Job +termination. Breakaway is also not a guarantee against an explicit process-tree +kill or host shutdown. Windows Job termination does not give a daemon an +opportunity to log a shutdown reason. + +Query commands retain their existing automatic-start behavior. Set +`BSK_AUTO_START=0` for probes that must not start a daemon, regardless of whether +stdout is a terminal or a pipe. + ## 1. Reuse or choose the daemon directory For an existing daemon, reuse its `BSK_HOME` and OS user, or its default directory From 58eb44b3062f8e723d86f254a46275c78bcdfd5e Mon Sep 17 00:00:00 2001 From: Zhang GH Date: Sun, 20 Sep 2026 00:38:54 +0800 Subject: [PATCH 2/2] fix(cli): preserve shared daemons after launcher timeout Do not terminate a running child when its launcher cannot confirm startup. A different client may already be using it, or publication may race the launcher's deadline. Retain cleanup only before a Windows child resumes. Add deterministic lifecycle regressions for reuse before timeout, delayed publication and port mismatch. Run Windows success-path and updater tests in a same-user WMI test host that verifies it is outside all Jobs, and keep restricted-host rejection tests in the normal CI runner. Refs #268 --- .github/workflows/ci.yml | 29 ++- crates/bsk-cli/src/daemon/start.rs | 220 ++++++++++++++++--- crates/bsk-cli/src/daemon/start/windows.rs | 2 +- crates/bsk-cli/src/windows_process.rs | 6 - crates/bsk-cli/tests/windows_daemon_start.rs | 15 +- docs/sandboxed-agents.md | 5 + scripts/test-windows-daemon.ps1 | 144 ++++++++++++ 7 files changed, 373 insertions(+), 48 deletions(-) create mode 100644 scripts/test-windows-daemon.ps1 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6da8829e..8d21d4f3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,8 +55,8 @@ jobs: - name: Run Windows audit and browser interaction policy tests run: cargo test -p bsk --test audit_ws --test interaction_preferences --locked - - name: Run Windows daemon startup and Job lifetime tests - run: cargo test -p bsk --test windows_daemon_start --locked -- --test-threads=1 + - name: Verify background startup refusal inside the runner Job + run: cargo test -p bsk --test windows_daemon_start --locked restrictive_job_refuses_explicit_and_automatic_start_without_a_daemon -- --exact - name: Run Windows process liveness tests run: cargo test -p bsk --lib --locked daemon::lockfile::tests @@ -64,8 +64,8 @@ jobs: - name: Run Windows update tests run: cargo test -p bsk --lib --locked cli::update::tests - - name: Run Windows update and cancellation process tests - run: cargo test -p bsk --test windows_update --test windows_parent_cancel --locked + - name: Run Windows cancellation process tests + run: cargo test -p bsk --test windows_parent_cancel --locked - name: Run installer tests (Windows PowerShell 5.1) shell: powershell @@ -75,6 +75,27 @@ jobs: shell: pwsh run: ./scripts/install-windows.test.ps1 + windows-daemon-lifecycle: + name: Windows independent daemon and update lifecycle + runs-on: windows-latest + timeout-minutes: 20 + + steps: + - uses: actions/checkout@v6 + - uses: dtolnay/rust-toolchain@stable + + - name: Run lifecycle regressions in a verified independent host + shell: powershell + run: ./scripts/test-windows-daemon.ps1 + + - name: Upload lifecycle test logs + if: always() + uses: actions/upload-artifact@v7 + with: + name: windows-daemon-lifecycle + path: target/windows-daemon-validation/ + if-no-files-found: warn + frontend: name: Frontend lint, typecheck, tests, build runs-on: ubuntu-latest diff --git a/crates/bsk-cli/src/daemon/start.rs b/crates/bsk-cli/src/daemon/start.rs index fd42384e..0e4591d4 100644 --- a/crates/bsk-cli/src/daemon/start.rs +++ b/crates/bsk-cli/src/daemon/start.rs @@ -155,8 +155,8 @@ pub fn run_start(args: StartArgs) -> Result<()> { Ok(()) } -/// Shared explicit/automatic startup. Keep ownership of the child until IPC -/// readiness is verified, without an intermediate launcher or captured pipe. +/// Shared explicit/automatic startup, without an intermediate launcher or +/// captured pipe. The deadline limits this caller's wait, not daemon lifetime. pub(crate) fn start_background( args: &StartArgs, deadline: Instant, @@ -166,35 +166,41 @@ pub(crate) fn start_background( "daemon startup deadline exceeded" ); let exe = std::env::current_exe().context("locate daemon executable")?; - let mut child = spawn_detached_at(&exe, args, None)?; + let child = spawn_detached_at(&exe, args, None)?; + wait_for_background(child, args, deadline) +} + +fn wait_for_background( + mut child: DaemonChild, + args: &StartArgs, + deadline: Instant, +) -> Result { let result = probe::wait_for_ready(deadline.saturating_duration_since(Instant::now())); let daemon = match result { Ok(daemon) => daemon, Err(err) => { let exit = child.try_wait().ok().flatten(); - stop_starting_child(&mut child); + // A paused or delayed launcher can time out after another client + // has already reused its daemon. Even absent discovery is not safe + // cancellation authority: publication can race any final probe. + disown_daemon(child); return Err(err.context(match exit { Some(status) => format!("daemon child exited during startup: {status}"), None => "daemon child failed to become ready".into(), })); } }; - if daemon.info.pid != child.id() { - // Another concurrent starter won the lock. Never stop that daemon. - stop_starting_child(&mut child); - } + // A concurrent starter may have won the daemon lock. Losing children + // exit on that lock themselves; neither a caller error nor a snapshot of + // another daemon authorizes killing a child that can become shared. + disown_daemon(child); if let Some(port) = args.port.filter(|port| *port != 0) { - if daemon.status.ws_port != port { - if daemon.info.pid == child.id() { - stop_starting_child(&mut child); - } - anyhow::bail!( - "daemon started on ws port {}, expected {port}", - daemon.status.ws_port - ); - } + anyhow::ensure!( + daemon.status.ws_port == port, + "daemon started on ws port {}, expected {port}", + daemon.status.ws_port + ); } - disown_daemon(child); Ok(daemon.info) } @@ -213,17 +219,6 @@ fn disown_daemon(child: DaemonChild) { drop(child); } -fn stop_starting_child(child: &mut DaemonChild) { - if matches!(child.try_wait(), Ok(Some(_))) { - return; - } - if let Err(err) = child.kill() { - warn!(error = %err, "failed to stop daemon child after startup failure"); - return; - } - let _ = child.wait(); -} - /// `bsk daemon stop` entrypoint. pub fn run_stop() -> Result<()> { stop_if_running().map(|_| ()) @@ -1150,6 +1145,173 @@ fn send_kill(_pid: u32) -> Result<()> { mod tests { use super::*; + // A gated real daemon makes the launcher/other-client interleaving + // deterministic, without test hooks or timing knobs in the shipped CLI. + #[test] + #[ignore = "subprocess entry point"] + fn lifecycle_daemon_process() { + let home = paths::bsk_home().unwrap(); + let deadline = Instant::now() + Duration::from_secs(10); + while !home.join("resume-child").exists() { + assert!( + Instant::now() < deadline, + "test did not release daemon gate" + ); + std::thread::sleep(Duration::from_millis(5)); + } + let mut config = DaemonConfig::new(0); + config.daemon_idle = Duration::from_secs(10); + run_foreground(config).unwrap(); + } + + fn gated_daemon() -> DaemonChild { + let mut command = std::process::Command::new(std::env::current_exe().unwrap()); + command.args([ + "--exact", + "daemon::start::tests::lifecycle_daemon_process", + "--ignored", + "--nocapture", + ]); + let home = paths::bsk_home().unwrap(); + #[cfg(not(windows))] + { + command + .env("HOME", home) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .unwrap() + } + #[cfg(windows)] + { + let mut env: Vec<_> = std::env::vars_os() + .filter(|(key, _)| { + let key = key.to_string_lossy(); + !key.eq_ignore_ascii_case("HOME") && !key.eq_ignore_ascii_case("USERPROFILE") + }) + .collect(); + env.push(("HOME".into(), home.as_os_str().to_owned())); + env.push(("USERPROFILE".into(), home.into_os_string())); + let input = std::fs::File::open("NUL").unwrap(); + let output = std::fs::File::options().write(true).open("NUL").unwrap(); + crate::windows_process::spawn( + command.get_program(), + &windows::command_line( + std::iter::once(command.get_program()).chain(command.get_args()), + ), + &env, + [&input, &output, &output], + windows_sys::Win32::System::Threading::CREATE_NO_WINDOW, + ) + .unwrap() + } + } + + struct StopTestDaemon; + + impl Drop for StopTestDaemon { + fn drop(&mut self) { + // Also release the gated fixture on an assertion failure. Its idle + // timeout bounds its lifetime if it cannot be reached for cleanup. + release_test_daemon(); + let _ = probe::wait_for_ready(Duration::from_secs(1)); + let _ = run_stop(); + } + } + + fn release_test_daemon() { + std::fs::write(paths::bsk_home().unwrap().join("resume-child"), []).unwrap(); + } + + #[test] + fn launcher_timeout_preserves_daemon_reused_by_another_client() { + crate::daemon::test_support::isolated( + concat!( + module_path!(), + "::launcher_timeout_preserves_daemon_reused_by_another_client" + ), + || { + let _cleanup = StopTestDaemon; + let child = gated_daemon(); + // Launcher A has spawned the child but is not allowed to continue + // its readiness wait until client B has successfully reused it. + release_test_daemon(); + let ready = probe::wait_for_ready(Duration::from_secs(5)).unwrap(); + let pid = ready.info.pid; + drop(ready); + run_start(StartArgs::default()).unwrap(); + // Resume A with an expired budget, exactly as after SIGSTOP/SIGCONT. + let error = + wait_for_background(child, &StartArgs::default(), Instant::now()).unwrap_err(); + assert!(format!("{error:#}").contains("failed to become ready")); + let Probe::Ready(after) = probe::probe(Duration::from_secs(1)).unwrap() else { + panic!("launcher timeout killed the daemon already reused by B"); + }; + assert_eq!(after.status.pid, pid); + }, + ); + } + + #[test] + fn launcher_timeout_before_publication_allows_child_to_finish() { + crate::daemon::test_support::isolated( + concat!( + module_path!(), + "::launcher_timeout_before_publication_allows_child_to_finish" + ), + || { + let _cleanup = StopTestDaemon; + let child = gated_daemon(); + assert!(!paths::info_path().unwrap().exists()); + assert!(wait_for_background(child, &StartArgs::default(), Instant::now()).is_err()); + // Absence of discovery at the deadline is not cancellation authority: + // the daemon can publish immediately afterwards and become shared. + release_test_daemon(); + let ready = probe::wait_for_ready(Duration::from_secs(5)).unwrap(); + let pid = ready.status.pid; + drop(ready); + run_start(StartArgs::default()).unwrap(); + let Probe::Ready(after) = probe::probe(Duration::from_secs(1)).unwrap() else { + panic!("daemon must be reusable after the launcher's timeout"); + }; + assert_eq!(after.status.pid, pid); + }, + ); + } + + #[test] + fn launcher_port_mismatch_preserves_shared_daemon() { + crate::daemon::test_support::isolated( + concat!( + module_path!(), + "::launcher_port_mismatch_preserves_shared_daemon" + ), + || { + let _cleanup = StopTestDaemon; + let child = gated_daemon(); + release_test_daemon(); + let ready = probe::wait_for_ready(Duration::from_secs(5)).unwrap(); + let pid = ready.status.pid; + let wrong_port = (ready.status.ws_port % u16::MAX) + 1; + drop(ready); + run_start(StartArgs::default()).unwrap(); + let args = StartArgs { + port: Some(wrong_port), + ..Default::default() + }; + let error = + wait_for_background(child, &args, Instant::now() + Duration::from_secs(3)) + .unwrap_err(); + assert!(format!("{error:#}").contains("expected")); + let Probe::Ready(after) = probe::probe(Duration::from_secs(1)).unwrap() else { + panic!("a caller's port mismatch killed the shared daemon"); + }; + assert_eq!(after.status.pid, pid); + }, + ); + } + #[cfg(unix)] #[test] fn disowned_child_is_reaped_while_the_launcher_stays_alive() { diff --git a/crates/bsk-cli/src/daemon/start/windows.rs b/crates/bsk-cli/src/daemon/start/windows.rs index c32afed6..ab19935f 100644 --- a/crates/bsk-cli/src/daemon/start/windows.rs +++ b/crates/bsk-cli/src/daemon/start/windows.rs @@ -58,7 +58,7 @@ pub(super) fn spawn(exe: &Path, args: &StartArgs, predecessor_pid: Option) /// Quote argv directly for the Windows CRT, without invoking a shell. Preserve /// UTF-16 paths, embedded quotes and backslashes before a closing quote. -fn command_line<'a>(args: impl Iterator) -> OsString { +pub(super) fn command_line<'a>(args: impl Iterator) -> OsString { let mut result = Vec::new(); for arg in args { if !result.is_empty() { diff --git a/crates/bsk-cli/src/windows_process.rs b/crates/bsk-cli/src/windows_process.rs index 4d12b59b..06fd612c 100644 --- a/crates/bsk-cli/src/windows_process.rs +++ b/crates/bsk-cli/src/windows_process.rs @@ -26,14 +26,9 @@ use windows_sys::Win32::System::Threading::{ pub(crate) struct Process { handle: OwnedHandle, thread: OwnedHandle, - pid: u32, } impl Process { - pub(crate) fn id(&self) -> u32 { - self.pid - } - pub(crate) fn try_wait(&mut self) -> io::Result> { // SAFETY: the owned process handle remains valid for both calls. match unsafe { WaitForSingleObject(self.handle.as_raw_handle(), 0) } { @@ -185,7 +180,6 @@ pub(crate) fn spawn( Ok(Process { handle: unsafe { OwnedHandle::from_raw_handle(process.hProcess) }, thread: unsafe { OwnedHandle::from_raw_handle(process.hThread) }, - pid: process.dwProcessId, }) } diff --git a/crates/bsk-cli/tests/windows_daemon_start.rs b/crates/bsk-cli/tests/windows_daemon_start.rs index 816b6c45..2fd60a21 100644 --- a/crates/bsk-cli/tests/windows_daemon_start.rs +++ b/crates/bsk-cli/tests/windows_daemon_start.rs @@ -23,8 +23,8 @@ use windows_sys::Win32::System::JobObjects::{ QueryInformationJobObject, SetInformationJobObject, TerminateJobObject, }; use windows_sys::Win32::System::Threading::{ - CREATE_BREAKAWAY_FROM_JOB, CREATE_NO_WINDOW, OpenProcess, PROCESS_QUERY_LIMITED_INFORMATION, - PROCESS_SYNCHRONIZE, PROCESS_TERMINATE, TerminateProcess, WaitForSingleObject, + CREATE_NO_WINDOW, OpenProcess, PROCESS_QUERY_LIMITED_INFORMATION, PROCESS_SYNCHRONIZE, + PROCESS_TERMINATE, TerminateProcess, WaitForSingleObject, }; const BUDGET: Duration = Duration::from_secs(8); @@ -229,13 +229,12 @@ impl Fixture { .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) - // Isolate our fixture from a breakaway-capable CI runner's Job. - // Test Jobs below are assigned explicitly before the gate opens. - .creation_flags(CREATE_NO_WINDOW | CREATE_BREAKAWAY_FROM_JOB); + // Preserve the test host's Job policy. The independent-host CI + // suite supplies a verified Job-free host; rejection tests also + // run directly inside the ordinary runner's restrictive Job. + .creation_flags(CREATE_NO_WINDOW); let started = Instant::now(); - let mut child = command - .spawn() - .expect("test host must allow a process outside its own Job"); + let mut child = command.spawn().expect("create gated test launcher"); for job in jobs { job.assign(&child); } diff --git a/docs/sandboxed-agents.md b/docs/sandboxed-agents.md index fd92490f..5bb5154b 100644 --- a/docs/sandboxed-agents.md +++ b/docs/sandboxed-agents.md @@ -34,6 +34,11 @@ Query commands retain their existing automatic-start behavior. Set `BSK_AUTO_START=0` for probes that must not start a daemon, regardless of whether stdout is a terminal or a pipe. +The startup deadline limits how long the initiating command waits. It does not +cancel a running daemon: another caller may already be using that service, or +it may finish publishing immediately after the deadline. Initialization errors +exit in the daemon itself; use `bsk daemon stop` for an explicit shutdown. + ## 1. Reuse or choose the daemon directory For an existing daemon, reuse its `BSK_HOME` and OS user, or its default directory diff --git a/scripts/test-windows-daemon.ps1 b/scripts/test-windows-daemon.ps1 new file mode 100644 index 00000000..52e6e667 --- /dev/null +++ b/scripts/test-windows-daemon.ps1 @@ -0,0 +1,144 @@ +#Requires -Version 5.1 +[CmdletBinding()] +param( + [ValidateRange(1, 5)][int]$Repeat = 1, + [switch]$Worker, + [string]$ManifestPath +) +$ErrorActionPreference = 'Stop' + +# This is a test host, never a production daemon launcher. WMI starts it outside +# the caller's Job; verify both identity and Job membership instead of assuming +# the CI runner permits CREATE_BREAKAWAY_FROM_JOB. +# https://learn.microsoft.com/windows/win32/procthread/job-objects +if ($Worker) { + $manifest = Get-Content -LiteralPath $ManifestPath -Raw | ConvertFrom-Json + $runDir = Split-Path -Parent $ManifestPath + $results = @() + $failure = $null + try { + Add-Type -TypeDefinition @" +using System; +using System.Runtime.InteropServices; +public static class DaemonTestHost { + [DllImport("kernel32.dll")] public static extern IntPtr GetCurrentProcess(); + [DllImport("kernel32.dll", SetLastError = true)] + public static extern bool IsProcessInJob(IntPtr process, IntPtr job, out bool inJob); +} +"@ + $sid = [Security.Principal.WindowsIdentity]::GetCurrent().User.Value + if ($sid -ne $manifest.userSid) { throw 'Independent test host changed OS user' } + $inJob = $false + if (-not [DaemonTestHost]::IsProcessInJob([DaemonTestHost]::GetCurrentProcess(), [IntPtr]::Zero, [ref]$inJob)) { + throw 'Could not query test host Job membership' + } + if ($inJob) { throw 'Independent test host is still in a Job; no success-path tests were run' } + "userSid=$sid; inJob=$inJob" | Set-Content -LiteralPath "$runDir/host.txt" + Set-Location -LiteralPath $manifest.workspace + $env:CI = 'true' # Default-port cold startup must execute, never skip. + if ($manifest.runnerTrackingId) { $env:RUNNER_TRACKING_ID = $manifest.runnerTrackingId } + for ($round = 1; $round -le $manifest.repeat; $round++) { + foreach ($suite in $manifest.suites) { + $name = "$($suite.name)-$round" + $process = New-Object Diagnostics.Process + $process.StartInfo.FileName = $suite.executable + $process.StartInfo.Arguments = $suite.arguments -join ' ' + $process.StartInfo.UseShellExecute = $false + $process.StartInfo.CreateNoWindow = $true + $process.StartInfo.RedirectStandardOutput = $true + $process.StartInfo.RedirectStandardError = $true + $exitCode = -1 + $errorText = $null + $watch = [Diagnostics.Stopwatch]::StartNew() + try { + if (-not $process.Start()) { throw 'Test process did not start' } + $stdout = $process.StandardOutput.ReadToEndAsync() + $stderr = $process.StandardError.ReadToEndAsync() + if (-not $process.WaitForExit(180000)) { throw 'Test process exceeded 180 seconds' } + $exitCode = $process.ExitCode + if (-not $stdout.Wait(5000) -or -not $stderr.Wait(5000)) { throw 'Test process exited without pipe EOF' } + $stdout.Result | Set-Content -LiteralPath "$runDir/$name.stdout.log" -Encoding utf8 + $stderr.Result | Set-Content -LiteralPath "$runDir/$name.stderr.log" -Encoding utf8 + } catch { + $exitCode = -1 + $errorText = $_.ToString() + } finally { + try { + if (-not $process.HasExited) { $process.Kill(); $null = $process.WaitForExit(5000) } + } catch { } + $process.Dispose() + } + $results += [pscustomobject]@{ name=$name; exitCode=$exitCode; seconds=$watch.Elapsed.TotalSeconds; error=$errorText } + # Keep running independent suites after a failure so updater + # coverage cannot be hidden by a startup regression. + } + } + } catch { $failure = $_.ToString() } + $report = [pscustomobject]@{ error=$failure; results=$results } + $report | ConvertTo-Json -Depth 6 | Set-Content -LiteralPath "$runDir/result.tmp" -Encoding utf8 + Move-Item -LiteralPath "$runDir/result.tmp" -Destination "$runDir/result.json" + if ($failure -or @($results | Where-Object exitCode -ne 0).Count) { exit 1 } + exit 0 +} + +$workspace = (Resolve-Path -LiteralPath (Join-Path $PSScriptRoot '..')).Path +$runDir = Join-Path $workspace ('target/windows-daemon-validation/' + [Guid]::NewGuid().ToString('N')) +$null = New-Item -ItemType Directory -Path $runDir -Force +Push-Location -LiteralPath $workspace +try { + # Build in the normal runner environment; the WMI host needs no inherited + # Cargo PATH, credentials or toolchain environment. Use Cargo's exact paths. + $artifacts = @(& cargo test -p bsk --lib --test windows_daemon_start --test windows_update --no-run --locked --message-format=json-render-diagnostics) + if ($LASTEXITCODE -ne 0) { throw 'Could not build Windows lifecycle tests' } + $tests = @{} + foreach ($line in $artifacts) { + $artifact = $line | ConvertFrom-Json + if ($artifact.reason -eq 'compiler-artifact' -and $artifact.profile.test -and $artifact.executable) { + if ($artifact.target.name -eq 'bsk' -and $artifact.target.kind -notcontains 'lib') { continue } + $tests[$artifact.target.name] = $artifact.executable + } + } + $suites = @( + @{ name='startup'; executable=$tests['windows_daemon_start']; arguments=@('--test-threads=1','--nocapture') }, + @{ name='launcher-lifetime'; executable=$tests['bsk']; arguments=@('daemon::start::tests','--test-threads=1','--nocapture') }, + @{ name='update'; executable=$tests['windows_update']; arguments=@('--test-threads=1','--nocapture') } + ) + foreach ($suite in $suites) { + if (-not $suite.executable -or -not (Test-Path -LiteralPath $suite.executable)) { throw "Missing executable for $($suite.name)" } + } + $manifest = @{ + workspace=$workspace; repeat=$Repeat; suites=$suites + userSid=[Security.Principal.WindowsIdentity]::GetCurrent().User.Value + runnerTrackingId=$env:RUNNER_TRACKING_ID + } + $manifestPath = Join-Path $runDir 'manifest.json' + $manifest | ConvertTo-Json -Depth 6 | Set-Content -LiteralPath $manifestPath -Encoding utf8 + $powershell = Join-Path $env:SystemRoot 'System32/WindowsPowerShell/v1.0/powershell.exe' + $command = '"{0}" -NoProfile -NonInteractive -WindowStyle Hidden -ExecutionPolicy Bypass -File "{1}" -Worker -ManifestPath "{2}"' -f $powershell,$PSCommandPath,$manifestPath + $startup = New-CimInstance -ClassName Win32_ProcessStartup -ClientOnly -Property @{ ShowWindow=[uint16]0 } + $created = Invoke-CimMethod -ClassName Win32_Process -MethodName Create -Arguments @{ CommandLine=$command; ProcessStartupInformation=$startup; CurrentDirectory=$workspace } + if ($created.ReturnValue -ne 0) { throw "WMI test host creation failed: $($created.ReturnValue)" } + $workerProcess = $null + try { + $workerProcess = [Diagnostics.Process]::GetProcessById($created.ProcessId) + $null = $workerProcess.Handle # Retain identity for timeout cleanup. + $deadline = [DateTime]::UtcNow.AddSeconds(600 * $Repeat) + while (-not $workerProcess.WaitForExit(1000)) { + if ([DateTime]::UtcNow -ge $deadline) { throw 'Independent test host timed out' } + } + if (-not (Test-Path -LiteralPath "$runDir/result.json")) { throw 'Independent test host exited without a report' } + if (Test-Path -LiteralPath "$runDir/host.txt") { Get-Content -LiteralPath "$runDir/host.txt" } + Get-ChildItem -LiteralPath $runDir -Filter '*.log' | ForEach-Object { Write-Output $_.Name; Get-Content -LiteralPath $_.FullName } + $report = Get-Content -LiteralPath "$runDir/result.json" -Raw | ConvertFrom-Json + $report.results | Format-Table -AutoSize | Out-Host + if ($report.error) { throw $report.error } + if (@($report.results).Count -ne (3 * $Repeat)) { throw 'Not all lifecycle suites executed' } + if (@($report.results | Where-Object exitCode -ne 0).Count) { throw "Lifecycle regression failed; logs: $runDir" } + Write-Output "All lifecycle suites passed; logs: $runDir" + } finally { + if ($workerProcess) { + if (-not $workerProcess.HasExited) { $workerProcess.Kill(); $null = $workerProcess.WaitForExit(5000) } + $workerProcess.Dispose() + } + } +} finally { Pop-Location }