diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b7e044d6..8d21d4f3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,14 +55,17 @@ jobs: - name: Run Windows audit and browser interaction policy tests run: cargo test -p bsk --test audit_ws --test interaction_preferences --locked + - 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 - 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 @@ -72,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/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..0e4591d4 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,9 +151,49 @@ 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, 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, +) -> Result { + anyhow::ensure!( + Instant::now() < deadline, + "daemon startup deadline exceeded" + ); + let exe = std::env::current_exe().context("locate daemon executable")?; + 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(); + // 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(), + })); + } + }; + // 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) { anyhow::ensure!( daemon.status.ws_port == port, @@ -151,7 +201,22 @@ pub fn run_start(args: StartArgs) -> Result<()> { daemon.status.ws_port ); } - Ok(()) + 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); } /// `bsk daemon stop` entrypoint. @@ -258,6 +323,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 +790,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 +993,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 +1003,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 +1032,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(()) +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(_args: &StartArgs) -> Result<()> { - Err(anyhow::anyhow!( - "detached daemon spawn is not supported on this platform" - )) -} - -#[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 +1145,196 @@ 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() { + 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..ab19935f --- /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. +pub(super) 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..06fd612c --- /dev/null +++ b/crates/bsk-cli/src/windows_process.rs @@ -0,0 +1,232 @@ +//! 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, +} + +impl Process { + 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) }, + }) +} + +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..2fd60a21 --- /dev/null +++ b/crates/bsk-cli/tests/windows_daemon_start.rs @@ -0,0 +1,534 @@ +//! 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_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()) + // 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("create gated test launcher"); + 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..5bb5154b 100644 --- a/docs/sandboxed-agents.md +++ b/docs/sandboxed-agents.md @@ -10,6 +10,35 @@ 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. + +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 }