diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 94a160fa..74291e55 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -446,6 +446,16 @@ jobs: continue-on-error: true timeout-minutes: 20 run: cargo test -p basal-host --lib --locked -- spawn_lock_tests --show-output 2>&1 | tee "$RUNNER_TEMP/windows-baseline/test-spawn-lock.log" + - name: Test the shared test kit on its own + id: test-testkit + continue-on-error: true + timeout-minutes: 30 + run: cargo test -p basal-testkit --locked --no-fail-fast 2>&1 | tee "$RUNNER_TEMP/windows-baseline/test-basal-testkit.log" + - name: Test the integration rig on its own + id: test-rig + continue-on-error: true + timeout-minutes: 30 + run: cargo test -p basal-rig --locked --no-fail-fast 2>&1 | tee "$RUNNER_TEMP/windows-baseline/test-basal-rig.log" - name: Check every workspace target id: check continue-on-error: true @@ -473,6 +483,11 @@ jobs: continue-on-error: true timeout-minutes: 30 run: cargo test -p basal-host --lib --locked --no-fail-fast 2>&1 | tee "$RUNNER_TEMP/windows-baseline/test-basal-host.log" + - name: Test the host integration regressions on their own + id: test-host-regressions + continue-on-error: true + timeout-minutes: 20 + run: cargo test -p basal-host --test builtin_host_regressions --locked --no-fail-fast 2>&1 | tee "$RUNNER_TEMP/windows-baseline/test-basal-host-regressions.log" - name: Summarize the baseline if: always() run: | @@ -484,6 +499,8 @@ jobs: echo "- cargo test -p basal-launch --features deviations --locked: ${{ steps.test-launch.outcome }}" echo "- cargo test -p basal-module -p basal-core --locked --no-fail-fast: ${{ steps.test-module.outcome }}" echo "- cargo test -p basal-host --lib --locked -- spawn_lock_tests: ${{ steps.test-spawn-lock.outcome }}" + echo "- cargo test -p basal-testkit --locked --no-fail-fast: ${{ steps.test-testkit.outcome }}" + echo "- cargo test -p basal-rig --locked --no-fail-fast: ${{ steps.test-rig.outcome }}" } >> "$GITHUB_STEP_SUMMARY" - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 if: always() diff --git a/Cargo.lock b/Cargo.lock index bcb449a4..dabab69a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -141,6 +141,7 @@ version = "0.1.0" dependencies = [ "async-trait", "basal-host", + "basal-launch", "basal-proto", "blake3", "rusqlite", @@ -149,6 +150,7 @@ dependencies = [ "subc-client-rs", "subc-protocol", "tokio", + "windows-sys 0.61.2", ] [[package]] @@ -157,6 +159,7 @@ version = "0.1.0" dependencies = [ "basal-core", "basal-host", + "basal-launch", "basal-proto", "blake3", "jiff", @@ -166,6 +169,7 @@ dependencies = [ "rustls", "serde_json", "subc-protocol", + "windows-sys 0.61.2", ] [[package]] diff --git a/crates/basal-host/tests/builtin_host_regressions.rs b/crates/basal-host/tests/builtin_host_regressions.rs index 1108d1b3..eb04fc00 100644 --- a/crates/basal-host/tests/builtin_host_regressions.rs +++ b/crates/basal-host/tests/builtin_host_regressions.rs @@ -2,6 +2,7 @@ use basal_host::builtins::{BuiltinHost, Grant, codes, envelope, fs, git, net}; use basal_host::{CallClass, CallRequest, Dispatched, Host}; use basal_proto::{CallKind, JsonText, Primitive, Settlement}; use serde_json::json; +#[cfg(unix)] use std::os::unix::fs::{PermissionsExt, symlink}; use std::path::PathBuf; use std::sync::atomic::{AtomicU64, Ordering}; @@ -31,7 +32,7 @@ impl Drop for Tree { #[test] fn escaped_800kb_file_is_a_typed_refusal_not_successful_null() { let tree = Tree::new(); - let path = tree.0.join("root/escaped"); + let path = tree.0.join("root").join("escaped"); std::fs::write(&path, vec![0; 800 * 1024]).unwrap(); let request = CallRequest { flow_id: "f".into(), @@ -63,6 +64,7 @@ fn escaped_800kb_file_is_a_typed_refusal_not_successful_null() { } #[test] +#[cfg(unix)] fn escaped_listing_exceeds_encoded_cap_as_a_typed_refusal() { let tree = Tree::new(); let dir = tree.0.join("root"); @@ -106,13 +108,16 @@ fn outside_missing_and_symlink_targets_have_identical_refusals() { let tree = Tree::new(); let existing = tree.0.join("secret"); std::fs::write(&existing, "secret").unwrap(); + #[cfg(unix)] let link = tree.0.join("root/link"); + #[cfg(unix)] symlink(&existing, &link).unwrap(); - let missing = tree.0.join("absent/child"); + let missing = tree.0.join("absent").join("child"); for purpose in [fs::Purpose::Read, fs::Purpose::Write] { let a = fs::resolve(existing.to_str().unwrap(), &tree.roots(), purpose).unwrap_err(); let b = fs::resolve(missing.to_str().unwrap(), &tree.roots(), purpose).unwrap_err(); assert_eq!(a, b); + #[cfg(unix)] if purpose == fs::Purpose::Read { assert_eq!( a, @@ -122,16 +127,20 @@ fn outside_missing_and_symlink_targets_have_identical_refusals() { assert_eq!(a.code, codes::DENIED); assert!(!a.message.contains(tree.0.to_str().unwrap())); } + let op = git::Op::RevParse { + reference: "HEAD".into(), + }; assert_eq!( - git::repo(existing.to_str().unwrap(), &tree.roots()).unwrap_err(), - git::repo(missing.to_str().unwrap(), &tree.roots()).unwrap_err() + git::run(existing.to_str().unwrap(), &tree.roots(), &op).unwrap_err(), + git::run(missing.to_str().unwrap(), &tree.roots(), &op).unwrap_err() ); } #[test] +#[cfg(unix)] fn atomic_write_drops_special_permission_bits() { let tree = Tree::new(); - let path = tree.0.join("root/file"); + let path = tree.0.join("root").join("file"); std::fs::write(&path, "old").unwrap(); std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o6755)).unwrap(); fs::write(path.to_str().unwrap(), &tree.roots(), "").unwrap(); @@ -179,6 +188,7 @@ fn method_override_and_compression_headers_are_denied() { } #[test] +#[cfg(unix)] fn git_reaps_descendants_that_keep_its_output_pipe_open() { use std::process::{Command, Stdio}; use std::time::Duration; @@ -219,13 +229,21 @@ fn git_reaps_descendants_that_keep_its_output_pipe_open() { fn tag_patterns_use_git_globs_including_hierarchical_names() { let tree = Tree::new(); let repo = tree.0.join("root"); + let global_config = tree.0.join("empty.gitconfig"); + std::fs::write(&global_config, []).unwrap(); let git_command = |args: &[&str]| { let result = std::process::Command::new("git") + .env_clear() + .envs(std::env::vars_os().filter(|(key, _)| { + ["PATH", "HOME", "SystemRoot", "USERPROFILE"] + .iter() + .any(|kept| key.to_string_lossy().eq_ignore_ascii_case(kept)) + })) .arg("-C") .arg(&repo) .args(args) .env("GIT_CONFIG_NOSYSTEM", "1") - .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_GLOBAL", &global_config) .env("GIT_AUTHOR_NAME", "Fixture") .env("GIT_AUTHOR_EMAIL", "fixture@example.invalid") .env("GIT_COMMITTER_NAME", "Fixture") diff --git a/crates/basal-rig/Cargo.toml b/crates/basal-rig/Cargo.toml index b7c40033..abf11e7f 100644 --- a/crates/basal-rig/Cargo.toml +++ b/crates/basal-rig/Cargo.toml @@ -31,6 +31,15 @@ subc-client-rs.workspace = true subc-protocol.workspace = true tokio = { workspace = true, features = ["sync", "time"] } +[target.'cfg(windows)'.dependencies] +basal-launch.workspace = true +rusqlite = { workspace = true, features = ["bundled"] } +windows-sys = { version = "0.61", features = [ + "Win32_Foundation", + "Win32_System_Diagnostics_ToolHelp", + "Win32_System_Threading", +] } + [dev-dependencies] basal-host.workspace = true basal-proto.workspace = true diff --git a/crates/basal-rig/src/bin/basal-rig-contract.rs b/crates/basal-rig/src/bin/basal-rig-contract.rs index 48e75709..ce40c924 100644 --- a/crates/basal-rig/src/bin/basal-rig-contract.rs +++ b/crates/basal-rig/src/bin/basal-rig-contract.rs @@ -50,6 +50,9 @@ mod ownership; mod package_cases; #[path = "basal-rig-contract/seeded.rs"] mod seeded; +#[cfg(windows)] +#[path = "basal-rig-contract/windows.rs"] +mod windows; const CORE: &str = "prefrontal-core"; const BASAL: &str = "basal"; @@ -1394,6 +1397,7 @@ async fn relay_refusals(rig: &Rig, flow: &Flow, agent: &Agent) -> Case { case } +#[cfg(unix)] fn basal_pid() -> Option { let out = std::process::Command::new("ps") .args(["-axo", "pid=,command="]) @@ -1411,6 +1415,11 @@ fn basal_pid() -> Option { }) } +#[cfg(windows)] +fn basal_pid() -> Option { + windows::basal_pid() +} + async fn crash(rig: &Rig, flow: &Flow, agent: &Agent, since: i64) -> Case { let mut case = Case::new(CRASH_CASE); // Arm the kill switch before approving, so the flow's first run cannot diff --git a/crates/basal-rig/src/bin/basal-rig-contract/seeded.rs b/crates/basal-rig/src/bin/basal-rig-contract/seeded.rs index 3432f37b..e0ed635b 100644 --- a/crates/basal-rig/src/bin/basal-rig-contract/seeded.rs +++ b/crates/basal-rig/src/bin/basal-rig-contract/seeded.rs @@ -9,13 +9,21 @@ pub(super) fn module_cli_path(home: &std::path::Path) -> Result { return Err("module control requires the isolated ckdev-flows home".into()); } - Ok(root.join("bin/ckdev-ck")) + #[cfg(unix)] + { + Ok(root.join("bin/ckdev-ck")) + } + #[cfg(windows)] + { + Ok(root.join("bin").join("ckdev-ck.exe")) + } } pub(super) fn ck(args: &[&str]) -> Result { let connection = std::env::var_os("SUBC_CONNECTION_FILE").ok_or("no isolated connection file")?; let home = PathBuf::from(std::env::var_os("HOME").ok_or("missing isolated HOME")?); + #[cfg(unix)] let output = std::process::Command::new(module_cli_path(&home)?) .arg("--subc") .arg(connection) @@ -23,6 +31,8 @@ pub(super) fn ck(args: &[&str]) -> Result { .args(args) .output() .map_err(|e| e.to_string())?; + #[cfg(windows)] + let output = super::windows::ck_output(module_cli_path(&home)?, connection, args)?; if !output.status.success() { return Err(format!( "ckdev-ck {args:?}: {} {}", @@ -195,6 +205,7 @@ pub(super) async fn stale( case } +#[cfg(unix)] pub(super) fn no_store_handles(path: &std::path::Path) -> Result<(), String> { let mut command = std::process::Command::new("lsof"); command.arg("-F").arg("p").arg(path); @@ -212,3 +223,33 @@ pub(super) fn no_store_handles(path: &std::path::Path) -> Result<(), String> { } Ok(()) } + +#[cfg(windows)] +pub(super) fn no_store_handles(path: &std::path::Path) -> Result<(), String> { + use std::os::windows::fs::OpenOptionsExt; + let mut files = vec![path.to_path_buf()]; + for suffix in ["-wal", "-shm"] { + let sidecar = PathBuf::from(format!("{}{suffix}", path.display())); + if sidecar.exists() { + files.push(sidecar); + } + } + // No-sharing opens refuse while any other reader or writer owns the file. + // Hold all three together so the observation covers the store and sidecars. + let _handles = files + .iter() + .map(|file| { + std::fs::OpenOptions::new() + .read(true) + .share_mode(0) + .open(file) + .map_err(|e| { + format!( + "cannot prove {} has no open store handles: {e}", + file.display() + ) + }) + }) + .collect::, _>>()?; + Ok(()) +} diff --git a/crates/basal-rig/src/bin/basal-rig-contract/tests.rs b/crates/basal-rig/src/bin/basal-rig-contract/tests.rs index c71dd230..f4869a99 100644 --- a/crates/basal-rig/src/bin/basal-rig-contract/tests.rs +++ b/crates/basal-rig/src/bin/basal-rig-contract/tests.rs @@ -129,15 +129,19 @@ fn seeded_fixture_requires_a_terminal_disabled_module_and_no_pid() { #[test] fn seeded_fixture_rejects_foreign_paths_and_symlinked_stores() { - let output = std::process::Command::new("mktemp") - .arg("-d") - .arg(std::env::temp_dir().join("basal-rig-fixture.XXXXXXXX")) - .output() - .unwrap(); - assert!(output.status.success()); - let root = PathBuf::from(String::from_utf8(output.stdout).unwrap().trim()) - .canonicalize() - .unwrap(); + static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + let root = loop { + let root = std::env::temp_dir().join(format!( + "basal-rig-fixture-{}-{}", + std::process::id(), + NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + )); + match std::fs::create_dir(&root) { + Ok(()) => break root.canonicalize().unwrap(), + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(error) => panic!("create isolated fixture: {error}"), + } + }; let rig = root.join("ckdev-flows"); let home = rig.join("home"); let store = rig.join("data/cortexkit/basal/store.db"); @@ -151,9 +155,12 @@ fn seeded_fixture_rejects_foreign_paths_and_symlinked_stores() { let foreign = root.join("production.db"); std::fs::write(&foreign, []).unwrap(); assert!(seeded::fixture_path_for(&home, &foreign, "basal").is_err()); - std::fs::remove_file(&store).unwrap(); - std::os::unix::fs::symlink(&foreign, &store).unwrap(); - assert!(seeded::fixture_path_for(&home, &store, "basal").is_err()); + #[cfg(unix)] + { + std::fs::remove_file(&store).unwrap(); + std::os::unix::fs::symlink(&foreign, &store).unwrap(); + assert!(seeded::fixture_path_for(&home, &store, "basal").is_err()); + } assert!(seeded::fixture_path_for(&home, &store, "other").is_err()); std::fs::remove_dir_all(&root).unwrap(); } @@ -191,10 +198,38 @@ fn authorship_fingerprint_allows_only_the_one_author_cell() { #[test] fn fixture_module_control_uses_only_the_absolute_isolated_cli() { + let root = std::env::temp_dir().join("rig").join("ckdev-flows"); + let home = root.join("home"); assert_eq!( - seeded::module_cli_path(std::path::Path::new("/rig/ckdev-flows/home")).unwrap(), - PathBuf::from("/rig/ckdev-flows/bin/ckdev-ck") + seeded::module_cli_path(&home).unwrap(), + root.join("bin").join(if cfg!(windows) { + "ckdev-ck.exe" + } else { + "ckdev-ck" + }) ); - assert!(seeded::module_cli_path(std::path::Path::new("/rig/home")).is_err()); + assert!(seeded::module_cli_path(&std::env::temp_dir().join("rig").join("home")).is_err()); assert!(seeded::module_cli_path(std::path::Path::new("ckdev-flows/home")).is_err()); } + +#[cfg(windows)] +#[test] +fn store_handle_probe_refuses_an_open_database_or_sidecar() { + let directory = + std::env::temp_dir().join(format!("basal-rig-handle-probe-{}", std::process::id())); + std::fs::create_dir(&directory).unwrap(); + let store = directory.join("store.db"); + std::fs::write(&store, []).unwrap(); + for suffix in ["", "-wal", "-shm"] { + let path = PathBuf::from(format!("{}{suffix}", store.display())); + std::fs::write(&path, []).unwrap(); + let held = std::fs::File::open(&path).unwrap(); + assert!( + seeded::no_store_handles(&store).is_err(), + "open {suffix:?} was not refused" + ); + drop(held); + assert!(seeded::no_store_handles(&store).is_ok()); + } + std::fs::remove_dir_all(directory).unwrap(); +} diff --git a/crates/basal-rig/src/bin/basal-rig-contract/windows.rs b/crates/basal-rig/src/bin/basal-rig-contract/windows.rs new file mode 100644 index 00000000..473c7203 --- /dev/null +++ b/crates/basal-rig/src/bin/basal-rig-contract/windows.rs @@ -0,0 +1,130 @@ +//! Native process observations and CLI capture for the isolated Windows rig. + +use std::ffi::OsString; +use std::io::Read; +use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle}; +use std::path::PathBuf; +use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE; +use windows_sys::Win32::System::Diagnostics::ToolHelp::{ + CreateToolhelp32Snapshot, PROCESSENTRY32W, Process32FirstW, Process32NextW, TH32CS_SNAPPROCESS, +}; +use windows_sys::Win32::System::Threading::{ + OpenProcess, PROCESS_QUERY_LIMITED_INFORMATION, QueryFullProcessImageNameW, +}; + +pub(super) fn basal_pid() -> Option { + let home = PathBuf::from(std::env::var_os("HOME")?); + let expected = module_image(&home)?; + pid_of_image(&expected) +} + +fn module_image(home: &std::path::Path) -> Option { + Some(home.parent()?.join("bin").join("ckdev-basal.exe")) +} + +fn pid_of_image(expected: &std::path::Path) -> Option { + // SAFETY: snapshot handles are newly owned unless the call returns INVALID_HANDLE_VALUE. + let snapshot = unsafe { CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0) }; + if snapshot == INVALID_HANDLE_VALUE { + return None; + } + let snapshot = unsafe { OwnedHandle::from_raw_handle(snapshot) }; + let mut row: PROCESSENTRY32W = unsafe { std::mem::zeroed() }; + row.dwSize = std::mem::size_of_val(&row) as u32; + let mut more = unsafe { Process32FirstW(snapshot.as_raw_handle(), &mut row) }; + while more != 0 { + let process = + unsafe { OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, 0, row.th32ProcessID) }; + if !process.is_null() { + let process = unsafe { OwnedHandle::from_raw_handle(process) }; + let mut path = vec![0u16; 32768]; + let mut size = path.len() as u32; + // SAFETY: size describes the writable UTF-16 buffer; the process is owned. + if unsafe { + QueryFullProcessImageNameW(process.as_raw_handle(), 0, path.as_mut_ptr(), &mut size) + } != 0 + { + use std::os::windows::ffi::OsStringExt; + let actual = PathBuf::from(OsString::from_wide(&path[..size as usize])); + if actual + .to_string_lossy() + .eq_ignore_ascii_case(&expected.to_string_lossy()) + { + return Some(row.th32ProcessID.to_string()); + } + } + } + more = unsafe { Process32NextW(snapshot.as_raw_handle(), &mut row) }; + } + None +} + +pub(super) fn ck_output( + program: PathBuf, + connection: OsString, + args: &[&str], +) -> Result { + use basal_launch::{PlainCommand, PlainStdio}; + use std::os::windows::process::ExitStatusExt; + let mut argv = vec!["--subc".into(), connection, "--json".into()]; + argv.extend(args.iter().map(|arg| OsString::from(*arg))); + let command = PlainCommand { + program, + args: argv, + env: std::env::vars_os().collect(), + cwd: None, + stdin: PlainStdio::Null, + stdout: PlainStdio::Piped, + stderr: PlainStdio::Piped, + }; + let mut child = basal_launch::spawn_plain(&command).map_err(|e| e.to_string())?; + let mut stdout = child.stdout.take().ok_or("no stdout pipe")?; + let mut stderr = child.stderr.take().ok_or("no stderr pipe")?; + let out = std::thread::spawn(move || { + let mut bytes = vec![]; + stdout.read_to_end(&mut bytes).map(|_| bytes) + }); + let err = std::thread::spawn(move || { + let mut bytes = vec![]; + stderr.read_to_end(&mut bytes).map(|_| bytes) + }); + let status = child.wait().map_err(|e| e.to_string())?; + // End descendants before joining pipe readers, even when the CLI exited normally. + child.kill().map_err(|e| e.to_string())?; + drop(child); + let stdout = out + .join() + .map_err(|_| "stdout reader panicked")? + .map_err(|e| e.to_string())?; + let stderr = err + .join() + .map_err(|_| "stderr reader panicked")? + .map_err(|e| e.to_string())?; + Ok(std::process::Output { + status: std::process::ExitStatus::from_raw(status), + stdout, + stderr, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn process_snapshot_finds_the_current_image_and_refuses_an_absent_image() { + let module = module_image(std::path::Path::new(r"C:\rig\home")).unwrap(); + // Path equality normalizes separators, but the native image-name + // comparison uses raw spelling, so assert the spelling itself here. + assert_eq!( + module.as_os_str(), + std::ffi::OsStr::new(r"C:\rig\bin\ckdev-basal.exe") + ); + let image = std::env::current_exe().unwrap(); + assert_eq!(pid_of_image(&image), Some(std::process::id().to_string())); + assert_eq!( + pid_of_image(&image.with_file_name("absent-rig-image.exe")), + None + ); + } +} diff --git a/crates/basal-rig/tests/stores.rs b/crates/basal-rig/tests/stores.rs index ce61deef..487c8301 100644 --- a/crates/basal-rig/tests/stores.rs +++ b/crates/basal-rig/tests/stores.rs @@ -7,13 +7,19 @@ use serde_json::{Value, json}; struct Scratch(PathBuf); impl Scratch { fn new() -> Self { - let output = std::process::Command::new("mktemp") - .arg("-d") - .arg(std::env::temp_dir().join("basal-rig-store.XXXXXXXX")) - .output() - .expect("create isolated store directory"); - assert!(output.status.success(), "{output:?}"); - let dir = PathBuf::from(String::from_utf8(output.stdout).unwrap().trim()); + static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + let dir = loop { + let dir = std::env::temp_dir().join(format!( + "basal-rig-store-{}-{}", + std::process::id(), + NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + )); + match std::fs::create_dir(&dir) { + Ok(()) => break dir, + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(error) => panic!("create isolated store directory: {error}"), + } + }; Self(dir.join("store.db")) } } diff --git a/crates/basal-testkit/Cargo.toml b/crates/basal-testkit/Cargo.toml index 355eb50e..f8112a22 100644 --- a/crates/basal-testkit/Cargo.toml +++ b/crates/basal-testkit/Cargo.toml @@ -19,6 +19,15 @@ libc.workspace = true rcgen.workspace = true rustls.workspace = true +[target.'cfg(windows)'.dependencies] +basal-launch.workspace = true +windows-sys = { version = "0.61", features = [ + "Win32_Foundation", + "Win32_Storage_FileSystem", + "Win32_System_ProcessStatus", + "Win32_System_Threading", +] } + [build-dependencies] serde_json.workspace = true diff --git a/crates/basal-testkit/src/bin/basal-test-parent.rs b/crates/basal-testkit/src/bin/basal-test-parent.rs index 44826df7..fb5371f4 100644 --- a/crates/basal-testkit/src/bin/basal-test-parent.rs +++ b/crates/basal-testkit/src/bin/basal-test-parent.rs @@ -1,5 +1,5 @@ //! A test parent process: basal-core's store and driver, real workers, and -//! the mock host, in one process the kill harness can `kill -9`. +//! the mock host, in one process the kill harness can terminate abruptly. //! //! ```text //! basal-test-parent --dir --worker @@ -13,7 +13,7 @@ //! `/mock.json`, synced before every reply, so effects survive the //! kill like a remote system's would. //! -//! With `--kill-at`, the process sends itself SIGKILL the moment it passes +//! With `--kill-at`, the process terminates without cleanup the moment it passes //! that boundary. Otherwise it prints one JSON line: the run's summary and //! every boundary it passed. @@ -69,6 +69,14 @@ fn run(args: Args) -> Result { let probe = Arc::new(match args.kill_at { Some(point) => Probe::act_at(point, |_, boundary| { eprintln!("basal-test-parent: killed at {boundary:?}"); + #[cfg(windows)] + unsafe { + // SAFETY: the pseudo-handle names this process; termination + // deliberately bypasses destructors to simulate a crash. + use windows_sys::Win32::System::Threading::{GetCurrentProcess, TerminateProcess}; + TerminateProcess(GetCurrentProcess(), basal_launch::KILL_EXIT_CODE); + } + #[cfg(unix)] // SAFETY: kill(2) on our own pid has no memory-safety // preconditions. unsafe { @@ -123,6 +131,21 @@ fn run(args: Args) -> Result { } fn main() -> ExitCode { + if let Some(calls) = std::env::var_os("BASAL_DISCOVERY_CALLS") { + // A native sentinel for the discovery test: if runtime discovery ever + // invokes its CARGO override, record the invocation before refusing it. + use std::io::Write; + writeln!( + std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(calls) + .expect("sentinel log"), + "build" + ) + .expect("record invocation"); + return ExitCode::FAILURE; + } let args = match parse() { Ok(a) => a, Err(e) => { diff --git a/crates/basal-testkit/src/bin/worker-bench.rs b/crates/basal-testkit/src/bin/worker-bench.rs index 195f980e..22b6e519 100644 --- a/crates/basal-testkit/src/bin/worker-bench.rs +++ b/crates/basal-testkit/src/bin/worker-bench.rs @@ -16,8 +16,8 @@ //! Every sample is kept in the output; summaries are the median and the //! nearest-rank 95th percentile. +use basal_testkit::command::Command; use std::path::{Path, PathBuf}; -use std::process::Command; use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::thread; diff --git a/crates/basal-testkit/src/binaries.rs b/crates/basal-testkit/src/binaries.rs index b0c0749e..08d1d98a 100644 --- a/crates/basal-testkit/src/binaries.rs +++ b/crates/basal-testkit/src/binaries.rs @@ -208,14 +208,21 @@ pub fn dev_binary(binary: impl AsRef) -> PathBuf { static COPIES: OnceLock>> = OnceLock::new(); let mut copies = COPIES.get_or_init(Mutex::default).lock().unwrap(); let placed = copies.entry(binary.to_path_buf()).or_insert_with(|| { - let worker = binary.with_file_name("ck-basal-worker"); - let module = binary.with_file_name("ck-basal"); + let suffix = if cfg!(windows) { ".exe" } else { "" }; + let worker_name = format!("ck-basal-worker{suffix}"); + let module_name = format!("ck-basal{suffix}"); + let worker = binary.with_file_name(&worker_name); + let module = binary.with_file_name(&module_name); let mut sources = vec![binary]; - if binary.file_name().is_some_and(|name| name == "ck-basal") && worker.is_file() { + if binary + .file_name() + .is_some_and(|name| name == module_name.as_str()) + && worker.is_file() + { sources.push(&worker); } else if binary .file_name() - .is_some_and(|name| name == "ck-basal-worker") + .is_some_and(|name| name == worker_name.as_str()) && module.is_file() { sources.insert(0, &module); @@ -225,6 +232,26 @@ pub fn dev_binary(binary: impl AsRef) -> PathBuf { placed.path(binary).expect("development executable path") } +/// Places a test worker and makes that directory launchable by the Windows +/// AppContainer. Production installation never uses this test-only ACL grant. +pub(crate) fn dev_worker_binary(binary: impl AsRef) -> PathBuf { + let binary = dev_binary(binary); + #[cfg(windows)] + { + use std::collections::HashSet; + static GRANTED: OnceLock>> = OnceLock::new(); + let directory = binary.parent().expect("worker directory").to_path_buf(); + let mut granted = GRANTED.get_or_init(Mutex::default).lock().unwrap(); + if !granted.contains(&directory) { + let package = basal_launch::create_or_open_profile().expect("test worker package SID"); + basal_launch::grant_test_binary_directory(&binary, &package) + .expect("test worker directory ACL"); + granted.insert(directory); + } + } + binary +} + #[cfg(test)] mod tests { use super::*; @@ -258,6 +285,7 @@ mod tests { #[test] fn module_and_worker_share_a_content_addressed_development_directory() { + #[cfg(unix)] use std::os::unix::fs::PermissionsExt; let source = SourceFiles::new(); @@ -270,6 +298,7 @@ mod tests { assert_eq!(path.file_name().unwrap(), name); assert_eq!(fs::read(&path).unwrap(), fs::read(original).unwrap()); assert_eq!(path.parent(), Some(placed.directory.as_path())); + #[cfg(unix)] assert_eq!( fs::metadata(&path).unwrap().permissions().mode() & 0o777, 0o755 @@ -279,6 +308,7 @@ mod tests { #[test] fn repeated_placement_reuses_the_same_file_and_inode() { + #[cfg(unix)] use std::os::unix::fs::MetadataExt; let source = SourceFiles::new(); @@ -286,10 +316,33 @@ mod tests { let first = DevBinaries::new(&[&binary]).unwrap().path(&binary).unwrap(); let second = DevBinaries::new(&[&binary]).unwrap().path(&binary).unwrap(); assert_eq!(first, second); + #[cfg(unix)] assert_eq!( fs::metadata(first).unwrap().ino(), fs::metadata(second).unwrap().ino() ); + #[cfg(windows)] + assert_eq!(file_identity(&first), file_identity(&second)); + } + + #[cfg(windows)] + fn file_identity(path: &Path) -> (u32, u32, u32) { + use std::os::windows::io::AsRawHandle; + use windows_sys::Win32::Storage::FileSystem::{ + BY_HANDLE_FILE_INFORMATION, GetFileInformationByHandle, + }; + let file = File::open(path).unwrap(); + let mut info: BY_HANDLE_FILE_INFORMATION = unsafe { std::mem::zeroed() }; + // SAFETY: file owns the handle and info is writable for the call. + assert_ne!( + unsafe { GetFileInformationByHandle(file.as_raw_handle(), &mut info) }, + 0 + ); + ( + info.dwVolumeSerialNumber, + info.nFileIndexHigh, + info.nFileIndexLow, + ) } #[test] diff --git a/crates/basal-testkit/src/channel.rs b/crates/basal-testkit/src/channel.rs index e182760b..e03a3c77 100644 --- a/crates/basal-testkit/src/channel.rs +++ b/crates/basal-testkit/src/channel.rs @@ -155,9 +155,9 @@ pub fn worker_binary() -> PathBuf { static PATH: OnceLock = OnceLock::new(); PATH.get_or_init(|| { if let Some(p) = std::env::var_os("BASAL_WORKER_BIN") { - return crate::dev_binary(PathBuf::from(p)); + return crate::binaries::dev_worker_binary(PathBuf::from(p)); } - crate::dev_binary(env!("BASAL_TEST_WORKER_BIN")) + crate::binaries::dev_worker_binary(env!("BASAL_TEST_WORKER_BIN")) }) .clone() } diff --git a/crates/basal-testkit/src/command.rs b/crates/basal-testkit/src/command.rs new file mode 100644 index 00000000..6f44f424 --- /dev/null +++ b/crates/basal-testkit/src/command.rs @@ -0,0 +1,153 @@ +//! Commands used by test parents, with an explicit environment on Windows. +//! +//! Keeping the environment here avoids trying to recover `env_clear` from a +//! standard Command, which does not expose whether inheritance was disabled. + +#[cfg(unix)] +pub use std::process::Command; + +#[cfg(windows)] +mod windows { + use std::collections::BTreeMap; + use std::ffi::{OsStr, OsString}; + use std::io; + use std::path::{Path, PathBuf}; + use std::process::{ExitStatus, Output}; + use std::time::Duration; + + pub struct Command { + program: OsString, + args: Vec, + env: BTreeMap, + cwd: Option, + } + + impl Command { + pub fn new(program: impl AsRef) -> Self { + Self { + program: program.as_ref().into(), + args: vec![], + env: std::env::vars_os().collect(), + cwd: None, + } + } + + pub fn arg(&mut self, arg: impl AsRef) -> &mut Self { + self.args.push(arg.as_ref().into()); + self + } + + pub fn args(&mut self, args: I) -> &mut Self + where + I: IntoIterator, + S: AsRef, + { + for arg in args { + self.arg(arg); + } + self + } + + pub fn env(&mut self, key: impl AsRef, value: impl AsRef) -> &mut Self { + self.env_remove(key.as_ref()); + self.env.insert(key.as_ref().into(), value.as_ref().into()); + self + } + + pub fn envs(&mut self, env: I) -> &mut Self + where + I: IntoIterator, + K: AsRef, + V: AsRef, + { + for (key, value) in env { + self.env(key, value); + } + self + } + + pub fn env_clear(&mut self) -> &mut Self { + self.env.clear(); + self + } + + pub fn env_remove(&mut self, key: impl AsRef) -> &mut Self { + // Windows environment keys are case-insensitive, unlike OsString's ordering. + let key = key.as_ref().to_string_lossy(); + self.env + .retain(|name, _| !name.to_string_lossy().eq_ignore_ascii_case(&key)); + self + } + + pub fn current_dir(&mut self, path: impl AsRef) -> &mut Self { + self.cwd = Some(path.as_ref().into()); + self + } + + pub fn output(&mut self) -> io::Result { + self.output_until(Duration::from_secs(950)) + } + pub fn status(&mut self) -> io::Result { + self.output().map(|output| output.status) + } + + pub fn output_until(&self, timeout: Duration) -> io::Result { + use basal_launch::{PlainCommand, PlainStdio}; + let program = resolve_program(&self.program, &self.env, self.cwd.as_deref())?; + let command = PlainCommand { + program, + args: self.args.clone(), + env: self + .env + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect(), + cwd: self.cwd.clone(), + stdin: PlainStdio::Null, + stdout: PlainStdio::Piped, + stderr: PlainStdio::Piped, + }; + super::super::process::windows::output_until(&command, timeout) + } + } + + fn resolve_program( + program: &OsStr, + env: &BTreeMap, + cwd: Option<&Path>, + ) -> io::Result { + let path = Path::new(program); + if path.is_absolute() { + return Ok(path.into()); + } + if path.components().count() > 1 { + return Ok(cwd + .map(PathBuf::from) + .unwrap_or(std::env::current_dir()?) + .join(path)); + } + let search = env + .iter() + .find(|(key, _)| key.to_string_lossy().eq_ignore_ascii_case("PATH")) + .map(|(_, value)| value) + .ok_or_else(|| io::Error::new(io::ErrorKind::NotFound, "command PATH is absent"))?; + for directory in std::env::split_paths(search) { + let candidate = directory.join(path); + let candidate = if candidate.extension().is_none() { + candidate.with_extension("exe") + } else { + candidate + }; + if candidate.is_file() { + return Ok(candidate); + } + } + Err(io::Error::new( + io::ErrorKind::NotFound, + format!("executable {} not found", path.display()), + )) + } +} + +#[cfg(windows)] +pub use windows::Command; diff --git a/crates/basal-testkit/src/git.rs b/crates/basal-testkit/src/git.rs index c6689857..2f726e06 100644 --- a/crates/basal-testkit/src/git.rs +++ b/crates/basal-testkit/src/git.rs @@ -1,5 +1,5 @@ +use crate::command::Command; use std::ffi::{OsStr, OsString}; -use std::process::Command; /// Starts git without inheriting caller configuration or environment. /// @@ -19,14 +19,19 @@ fn git_command_with_env(base_env: impl IntoIterator // would reach git, and the isolation test would see the difference. command.envs(base_env.iter().cloned()); command.env_clear(); - for key in [OsStr::new("PATH"), OsStr::new("HOME")] { + for key in [ + OsStr::new("PATH"), + OsStr::new("HOME"), + OsStr::new("SystemRoot"), + OsStr::new("USERPROFILE"), + ] { if let Some((_, value)) = base_env.iter().rev().find(|(name, _)| name == key) { command.env(key, value); } } command .env("GIT_CONFIG_NOSYSTEM", "1") - .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_GLOBAL", empty_global_config()) .env("GIT_TERMINAL_PROMPT", "0") .env("GIT_AUTHOR_NAME", "Basal Test") .env("GIT_AUTHOR_EMAIL", "basal-test@example.com") @@ -35,7 +40,17 @@ fn git_command_with_env(base_env: impl IntoIterator command } -#[cfg(test)] +fn empty_global_config() -> &'static std::path::Path { + static CONFIG: std::sync::OnceLock = std::sync::OnceLock::new(); + CONFIG.get_or_init(|| { + let dir = crate::harness::scratch("empty-git-config"); + let path = dir.join("global.gitconfig"); + std::fs::write(&path, []).expect("empty git global config"); + path + }) +} + +#[cfg(all(test, unix))] mod tests { use std::os::unix::fs::PermissionsExt; diff --git a/crates/basal-testkit/src/https.rs b/crates/basal-testkit/src/https.rs index e018f2bf..ac4c328b 100644 --- a/crates/basal-testkit/src/https.rs +++ b/crates/basal-testkit/src/https.rs @@ -99,6 +99,7 @@ pub struct TestServer { stop: Arc, thread: Option>, #[cfg(test)] + #[cfg(unix)] blocking_accept: bool, } @@ -121,7 +122,7 @@ impl TestServer { .expect("server certificate"); let config = Arc::new(config); let listener = TcpListener::bind("127.0.0.1:0").expect("bind"); - #[cfg(test)] + #[cfg(all(test, unix))] let blocking_accept = { use std::os::fd::AsRawFd; // SAFETY: listener owns this live socket; F_GETFL only reads flags. @@ -166,6 +167,7 @@ impl TestServer { stop, thread: Some(thread), #[cfg(test)] + #[cfg(unix)] blocking_accept, } } @@ -299,6 +301,7 @@ mod tests { let server = TestServer::start(|_| BTreeMap::new()); let connections = server.connections.clone(); let seen = server.seen.clone(); + #[cfg(unix)] assert!( server.blocking_accept, "the accept loop must block instead of polling" diff --git a/crates/basal-testkit/src/lib.rs b/crates/basal-testkit/src/lib.rs index 078e6484..b695f5c7 100644 --- a/crates/basal-testkit/src/lib.rs +++ b/crates/basal-testkit/src/lib.rs @@ -10,6 +10,7 @@ mod backoff; pub mod binaries; pub mod channel; +pub mod command; pub mod fuzz; pub mod git; pub mod harness; diff --git a/crates/basal-testkit/src/process.rs b/crates/basal-testkit/src/process.rs index 0ee4dff9..8b6bb3a3 100644 --- a/crates/basal-testkit/src/process.rs +++ b/crates/basal-testkit/src/process.rs @@ -1,9 +1,12 @@ //! A real `ck-basal-worker` child process, driven over its stdio frames. +use crate::command::Command; use std::fmt; use std::io::{self, Read, Write}; use std::path::Path; -use std::process::{Child, ChildStdin, Command, ExitStatus, Stdio}; +use std::process::ExitStatus; +#[cfg(unix)] +use std::process::{Child, ChildStdin, Stdio}; use std::sync::mpsc::{self, Receiver, RecvTimeoutError}; use std::sync::{Arc, Mutex}; use std::thread; @@ -44,6 +47,15 @@ impl std::error::Error for ParentError {} type Incoming = Result; +#[cfg(windows)] +pub(crate) mod windows; +#[cfg(windows)] +use windows::Child; +#[cfg(windows)] +pub use windows::terminate_process; +#[cfg(windows)] +type ChildStdin = std::fs::File; + /// A spawned worker. Killed on drop. pub struct WorkerProcess { child: Child, @@ -62,19 +74,24 @@ impl WorkerProcess { } pub fn spawn_with_args(binary: &Path, args: &[&str]) -> io::Result { - let binary = crate::dev_binary(binary); - let mut command = Command::new(binary); - command.args(args); - #[cfg(target_os = "linux")] - if !args.iter().any(|arg| arg.starts_with("--landlock=")) { - command.arg("--landlock=required"); - } - let mut child = command - .env_clear() - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn()?; + let binary = crate::binaries::dev_worker_binary(binary); + #[cfg(windows)] + let mut child = windows::spawn_worker(&binary, args)?; + #[cfg(unix)] + let mut child = { + let mut command = Command::new(binary); + command.args(args); + #[cfg(target_os = "linux")] + if !args.iter().any(|arg| arg.starts_with("--landlock=")) { + command.arg("--landlock=required"); + } + command + .env_clear() + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()? + }; let stdin = child.stdin.take(); let mut stdout = child .stdout @@ -251,7 +268,11 @@ pub fn rss_kib(pid: u32) -> Option { } Some(unsafe { info.assume_init() }.pti_resident_size / 1024) } - #[cfg(not(target_os = "macos"))] + #[cfg(windows)] + { + windows::rss_kib(pid) + } + #[cfg(all(unix, not(target_os = "macos")))] { let out = Command::new("ps") .args(["-o", "rss=", "-p", &pid.to_string()]) @@ -270,25 +291,31 @@ impl Drop for WorkerProcess { /// Probe the operating system without waiting for an exit. A successful /// process-exit wait can hide a missing reap in the cleanup path. pub fn assert_reaped(pid: u32) { - let mut status = 0; - // SAFETY: waitpid writes only to this status and the caller's child pid. - let result = unsafe { - libc::waitpid( - pid.try_into().expect("child pid"), - &mut status, - libc::WNOHANG, - ) - }; - assert_eq!(result, -1, "worker {pid} was not reaped"); - assert_eq!( - std::io::Error::last_os_error().raw_os_error(), - Some(libc::ECHILD) - ); + #[cfg(windows)] + windows::assert_reaped(pid); + #[cfg(unix)] + { + let mut status = 0; + // SAFETY: waitpid writes only to this status and the caller's child pid. + let result = unsafe { + libc::waitpid( + pid.try_into().expect("child pid"), + &mut status, + libc::WNOHANG, + ) + }; + assert_eq!(result, -1, "worker {pid} was not reaped"); + assert_eq!( + std::io::Error::last_os_error().raw_os_error(), + Some(libc::ECHILD) + ); + } } /// Capture a subprocess with a deadline, draining both pipes concurrently. /// Timeout kills the process group, so inherited pipes cannot keep a reader /// alive after the parent has been reaped. +#[cfg(unix)] pub fn output_until(command: &mut Command, timeout: Duration) -> io::Result { use std::os::unix::process::CommandExt; // SAFETY: setpgid is async-signal-safe and touches no Rust-owned memory. @@ -351,9 +378,15 @@ pub fn output_until(command: &mut Command, timeout: Duration) -> io::Result io::Result { + command.output_until(timeout) +} + #[cfg(test)] mod tests { use super::*; + #[cfg(unix)] #[test] fn subprocess_deadline_reaps_descendants_holding_output_pipes() { let error = output_until( diff --git a/crates/basal-testkit/src/process/windows.rs b/crates/basal-testkit/src/process/windows.rs new file mode 100644 index 00000000..01ee5206 --- /dev/null +++ b/crates/basal-testkit/src/process/windows.rs @@ -0,0 +1,204 @@ +//! Native worker ownership and process observations for the Windows test parent. + +use basal_launch::{ConfinedProcess, LaunchOptions}; +use std::collections::HashMap; +use std::fs::File; +use std::io; +use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle}; +use std::os::windows::process::ExitStatusExt; +use std::path::Path; +use std::process::ExitStatus; +use std::sync::{Arc, Mutex, OnceLock, Weak}; +use windows_sys::Win32::Foundation::WAIT_OBJECT_0; +use windows_sys::Win32::System::ProcessStatus::{K32GetProcessMemoryInfo, PROCESS_MEMORY_COUNTERS}; +use windows_sys::Win32::System::Threading::{ + OpenProcess, PROCESS_QUERY_INFORMATION, PROCESS_TERMINATE, PROCESS_VM_READ, TerminateProcess, + WaitForSingleObject, +}; + +struct ReapState { + process: Weak, + signaled: bool, +} + +fn processes() -> &'static Mutex> { + static PROCESSES: OnceLock>> = OnceLock::new(); + PROCESSES.get_or_init(Mutex::default) +} + +pub(super) struct Child { + process: Option>, + pid: u32, + status: Option, + pub stdin: Option, + pub stdout: Option, + pub stderr: Option, +} + +pub(super) fn spawn_worker(binary: &Path, args: &[&str]) -> io::Result { + let mut options = LaunchOptions::new(binary, basal_proto::FLOW_JOB_COMMIT_BYTES); + options.args = args.iter().map(|arg| (*arg).into()).collect(); + let mut process = basal_launch::launch(&options).map_err(io::Error::other)?; + let stdin = process.stdin.take(); + let stdout = process.stdout.take(); + let stderr = process.stderr.take(); + let process = Arc::new(process); + let pid = process.id(); + processes().lock().unwrap().insert( + pid, + ReapState { + process: Arc::downgrade(&process), + signaled: false, + }, + ); + Ok(Child { + process: Some(process), + pid, + status: None, + stdin, + stdout, + stderr, + }) +} + +impl Child { + pub fn id(&self) -> u32 { + self.pid + } + + pub fn kill(&self) -> io::Result<()> { + self.process + .as_ref() + .map_or(Ok(()), |process| process.kill()) + } + + fn release(&mut self, code: u32) -> ExitStatus { + let process = self.process.take().expect("owned process before release"); + // Observe the real handle before releasing it, rather than treating a + // cached exit code or the disappearance of a pid as evidence of exit. + let signaled = unsafe { WaitForSingleObject(process.as_raw_handle(), 0) } == WAIT_OBJECT_0; + processes() + .lock() + .unwrap() + .get_mut(&self.pid) + .unwrap() + .signaled = signaled; + drop(process); + let status = ExitStatus::from_raw(code); + self.status = Some(status); + status + } + + pub fn try_wait(&mut self) -> io::Result> { + if let Some(status) = self.status { + return Ok(Some(status)); + } + let code = self.process.as_ref().unwrap().try_wait()?; + Ok(code.map(|code| self.release(code))) + } + + pub fn wait(&mut self) -> io::Result { + if let Some(status) = self.status { + return Ok(status); + } + let code = self.process.as_ref().unwrap().wait()?; + Ok(self.release(code)) + } +} + +pub(super) fn assert_reaped(pid: u32) { + let state = processes() + .lock() + .unwrap() + .get(&pid) + .map(|state| (state.process.clone(), state.signaled)); + let (process, signaled) = state.expect("process was launched by the test kit"); + assert!(signaled, "worker {pid}'s process handle was not signaled"); + assert!( + process.upgrade().is_none(), + "worker {pid}'s process handle is still owned" + ); +} + +pub(super) fn rss_kib(pid: u32) -> Option { + // SAFETY: OpenProcess returns a new owned handle, or NULL on failure. + let handle = unsafe { OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, 0, pid) }; + if handle.is_null() { + return None; + } + let handle = unsafe { OwnedHandle::from_raw_handle(handle) }; + let mut counters: PROCESS_MEMORY_COUNTERS = unsafe { std::mem::zeroed() }; + let size = std::mem::size_of_val(&counters) as u32; + counters.cb = size; + // SAFETY: counters is writable for exactly the declared structure size. + if unsafe { K32GetProcessMemoryInfo(handle.as_raw_handle(), &mut counters, size) } == 0 { + return None; + } + Some(counters.WorkingSetSize as u64 / 1024) +} + +pub fn terminate_process(pid: u32) -> io::Result<()> { + // SAFETY: OpenProcess returns a fresh owned handle, or NULL on failure. + let handle = unsafe { OpenProcess(PROCESS_TERMINATE, 0, pid) }; + if handle.is_null() { + return Err(io::Error::last_os_error()); + } + let handle = unsafe { OwnedHandle::from_raw_handle(handle) }; + if unsafe { TerminateProcess(handle.as_raw_handle(), basal_launch::KILL_EXIT_CODE) } == 0 { + return Err(io::Error::last_os_error()); + } + Ok(()) +} + +pub(crate) fn output_until( + command: &basal_launch::PlainCommand, + timeout: std::time::Duration, +) -> io::Result { + use std::io::Read; + use std::time::{Duration, Instant}; + let mut child = basal_launch::spawn_plain(command).map_err(io::Error::other)?; + let mut stdout = child.stdout.take().expect("stdout pipe"); + let mut stderr = child.stderr.take().expect("stderr pipe"); + let out = std::thread::spawn(move || { + let mut bytes = Vec::new(); + stdout.read_to_end(&mut bytes).map(|_| bytes) + }); + let err = std::thread::spawn(move || { + let mut bytes = Vec::new(); + stderr.read_to_end(&mut bytes).map(|_| bytes) + }); + let deadline = Instant::now() + timeout; + let mut timed_out = false; + let status = loop { + match child.try_wait() { + Ok(Some(code)) => break Ok(ExitStatus::from_raw(code)), + Ok(None) if Instant::now() < deadline => std::thread::sleep(Duration::from_millis(10)), + result => { + timed_out = result.is_ok(); + child.kill()?; + break child.wait().map(ExitStatus::from_raw); + } + } + }; + // Even a normally exited parent can leave a descendant holding a pipe. + // End the entire job before joining readers, never only the root process. + child.kill()?; + drop(child); + let stdout = out + .join() + .map_err(|_| io::Error::other("stdout reader panicked"))??; + let stderr = err + .join() + .map_err(|_| io::Error::other("stderr reader panicked"))??; + if timed_out { + return Err(io::Error::new( + io::ErrorKind::TimedOut, + "subprocess deadline expired; job killed and process released", + )); + } + Ok(std::process::Output { + status: status?, + stdout, + stderr, + }) +} diff --git a/crates/basal-testkit/tests/bench_args.rs b/crates/basal-testkit/tests/bench_args.rs index b6657d69..70d214c9 100644 --- a/crates/basal-testkit/tests/bench_args.rs +++ b/crates/basal-testkit/tests/bench_args.rs @@ -1,4 +1,4 @@ -use std::process::Command; +use basal_testkit::command::Command; #[test] fn worker_benchmark_refuses_invalid_arguments_before_spawning() { diff --git a/crates/basal-testkit/tests/builtins_dispatch.rs b/crates/basal-testkit/tests/builtins_dispatch.rs index f227316c..d01f5e11 100644 --- a/crates/basal-testkit/tests/builtins_dispatch.rs +++ b/crates/basal-testkit/tests/builtins_dispatch.rs @@ -12,7 +12,7 @@ use std::time::Duration; use basal_core::{ Config, InstallError, InstallRequest, NoHooks, RunState, Runtime, Store, StoredClass, Warning, }; -use basal_host::builtins::{self, BuiltinHost}; +use basal_host::builtins::{self, BuiltinHost, codes}; use basal_host::{ CallClass, CallRequest, CompletionSink, Dispatched, Host, HostOutcome, InstallStatus, TransportError, @@ -217,7 +217,9 @@ fn calls_outside_the_manifest_are_refused_in_the_parent_and_journaled() { outside = files.outside.join("secret.txt").display().to_string(), escape = files .root - .join("../outside/secret.txt") + .join("..") + .join("outside") + .join("secret.txt") .display() .to_string(), root = files.root.display().to_string(), @@ -225,12 +227,19 @@ fn calls_outside_the_manifest_are_refused_in_the_parent_and_journaled() { let run_id = admit_with(&rt, &world, &script, &files.manifest()); let run = finish(&rt, &world, &run_id); assert_eq!(run.state, RunState::Succeeded, "{run:#?}"); + // Windows rejects raw relative components as malformed arguments; Unix + // resolves the spelling and refuses the resulting outside-root target. + let escape_code = if cfg!(windows) { + codes::INVALID_ARGUMENTS + } else { + codes::DENIED + }; assert_eq!( result(&run), json!({ "inside": "PRIVATE-FILE-TEXT", "outside": "denied", - "escape": "denied", + "escape": escape_code, "git": "denied", "host": "denied", "method": "denied", @@ -245,7 +254,11 @@ fn calls_outside_the_manifest_are_refused_in_the_parent_and_journaled() { assert_eq!(outcome.settlement, Settlement::Rejected, "{row:?}"); assert_eq!( code_of(&outcome.value).as_deref(), - Some("denied"), + Some(if row.position == 2 { + escape_code + } else { + codes::DENIED + }), "{row:?}" ); } @@ -265,7 +278,7 @@ fn calls_outside_the_manifest_are_refused_in_the_parent_and_journaled() { [ ("fs.read", "allowed"), ("fs.read", "denied"), - ("fs.read", "denied"), + ("fs.read", escape_code), ("git.log", "denied"), ("net.fetch", "denied"), ("net.fetch", "denied"), @@ -453,7 +466,7 @@ fn install_refuses_bad_roots_hosts_and_methods() { ), ( "dotdot root", - json!({ "fs": { "read": [format!("{root}/../outside")] } }), + json!({ "fs": { "read": [files.root.join("..").join("outside").display().to_string()] } }), ), ( "missing read root", diff --git a/crates/basal-testkit/tests/builtins_fs.rs b/crates/basal-testkit/tests/builtins_fs.rs index 7755d742..15ceec86 100644 --- a/crates/basal-testkit/tests/builtins_fs.rs +++ b/crates/basal-testkit/tests/builtins_fs.rs @@ -5,10 +5,13 @@ //! reads refuse text that is not UTF-8 or is over the cap; writes replace //! the whole file and never through a symlink. +#[cfg(unix)] use std::os::unix::fs::symlink; use std::path::PathBuf; -use basal_host::builtins::fs::{self, Purpose, Target}; +use basal_host::builtins::fs; +#[cfg(unix)] +use basal_host::builtins::fs::{Purpose, Target}; use basal_host::builtins::{BuiltinHost, Denial, Failure, Grant, codes, envelope}; use basal_proto::Primitive; use basal_testkit::harness::scratch; @@ -44,7 +47,16 @@ impl Tree { } fn p(&self, rel: &str) -> String { - self.root.join(rel).display().to_string() + let path = if rel.is_empty() { + self.root.clone() + } else { + self.root.join(rel) + }; + if cfg!(windows) { + path.display().to_string().replace('/', "\\") + } else { + path.display().to_string() + } } } @@ -69,14 +81,25 @@ fn reads_inside_a_root_and_refuses_a_dotdot_escape() { fs::read(&t.p("a.txt"), &roots, 1024).expect("read"), json!({ "text": "inside" }) ); + #[cfg(unix)] assert_eq!( fs::read(&t.p("sub/../a.txt"), &roots, 1024).expect("read"), json!({ "text": "inside" }) ); let escape = t.p("../outside/secret.txt"); - assert_eq!(code(fs::read(&escape, &roots, 1024)), codes::DENIED); - assert_eq!(code(fs::stat(&escape, &roots)), codes::DENIED); - assert_eq!(code(fs::list(&t.p(".."), &roots)), codes::DENIED); + let escape_code = if cfg!(windows) { + codes::INVALID_ARGUMENTS + } else { + codes::DENIED + }; + #[cfg(windows)] + assert_eq!( + code(fs::read(&t.p("sub/../a.txt"), &roots, 1024)), + codes::INVALID_ARGUMENTS + ); + assert_eq!(code(fs::read(&escape, &roots, 1024)), escape_code); + assert_eq!(code(fs::stat(&escape, &roots)), escape_code); + assert_eq!(code(fs::list(&t.p(".."), &roots)), escape_code); // A sibling whose name only starts with the root's is not under it. let sibling = t.base.join("rootx"); std::fs::create_dir_all(&sibling).expect("sibling"); @@ -97,6 +120,7 @@ fn reads_inside_a_root_and_refuses_a_dotdot_escape() { } #[test] +#[cfg(unix)] fn a_symlink_resolving_outside_the_root_is_refused() { let t = Tree::new("fs-symlink"); let roots = t.roots(); @@ -154,7 +178,9 @@ fn stat_of_a_missing_path_is_exists_false_only_when_its_parent_is_in_scope() { fn list_names_entries_and_their_kinds() { let t = Tree::new("fs-list"); let roots = t.roots(); + #[cfg(unix)] symlink(t.root.join("a.txt"), t.root.join("link")).expect("link"); + #[cfg(unix)] assert_eq!( fs::list(&t.p(""), &roots).expect("list"), json!([ @@ -163,6 +189,13 @@ fn list_names_entries_and_their_kinds() { { "name": "sub", "kind": "dir" }, ]) ); + #[cfg(windows)] + assert_eq!( + fs::list(&t.p(""), &roots).expect("list"), + json!([ + { "name": "a.txt", "kind": "file" }, { "name": "sub", "kind": "dir" } + ]) + ); for i in 0..=fs::MAX_LIST_ENTRIES { std::fs::write(t.root.join(format!("sub/f{i}")), "").expect("file"); } @@ -225,21 +258,37 @@ fn write_replaces_the_whole_file_inside_a_write_root_only() { .collect(); assert!(names.iter().all(|n| !n.contains(".basal-")), "{names:?}"); - // Outside the root, through `..`, through a directory symlink, or onto - // a symlink: refused, and nothing outside changes. - symlink(&t.outside, t.root.join("out-dir")).expect("dir link"); - symlink(t.outside.join("secret.txt"), t.root.join("out-link")).expect("link"); - for path in [ - t.p("../outside/new.txt"), - t.p("out-dir/new.txt"), - t.p("out-link"), - ] { - assert_eq!( - code(fs::write(&path, &roots, "pwned")), - codes::DENIED, - "{path}" - ); + // Raw relative components are refused on Windows before resolution. + #[cfg(windows)] + assert_eq!( + code(fs::write(&t.p("../outside/new.txt"), &roots, "pwned")), + codes::INVALID_ARGUMENTS + ); + // Unix permits symlink creation without an elevated test account. + #[cfg(unix)] + { + symlink(&t.outside, t.root.join("out-dir")).expect("dir link"); + symlink(t.outside.join("secret.txt"), t.root.join("out-link")).expect("link"); + for path in [ + t.p("../outside/new.txt"), + t.p("out-dir/new.txt"), + t.p("out-link"), + ] { + assert_eq!( + code(fs::write(&path, &roots, "pwned")), + codes::DENIED, + "{path}" + ); + } } + assert_eq!( + code(fs::write( + t.outside.join("new.txt").to_str().unwrap(), + &roots, + "pwned" + )), + codes::DENIED + ); assert_eq!( std::fs::read_to_string(t.outside.join("secret.txt")).expect("secret"), "secret" @@ -259,6 +308,7 @@ fn write_replaces_the_whole_file_inside_a_write_root_only() { /// component is then swapped for a symlink to a file outside. The open /// refuses to follow it. #[test] +#[cfg(unix)] fn a_symlink_swapped_into_the_last_component_after_the_check_is_not_followed() { let t = Tree::new("fs-swap-last"); let roots = t.roots(); @@ -278,6 +328,7 @@ fn a_symlink_swapped_into_the_last_component_after_the_check_is_not_followed() { /// directory outside after the check. The open succeeds, but the opened /// file's real path lies outside the root, so it is refused. #[test] +#[cfg(unix)] fn a_directory_swapped_for_a_symlink_after_the_check_is_caught_after_the_open() { let t = Tree::new("fs-swap-dir"); let roots = t.roots(); diff --git a/crates/basal-testkit/tests/builtins_git.rs b/crates/basal-testkit/tests/builtins_git.rs index 712cd8e2..dc1787c1 100644 --- a/crates/basal-testkit/tests/builtins_git.rs +++ b/crates/basal-testkit/tests/builtins_git.rs @@ -2,24 +2,34 @@ //! the answers, the approved-repository check, argument checks, and that a //! repository's own configuration cannot make a built-in run a program. -use basal_host::builtins::git::{self, Op, hardened_command, run_command}; +use basal_host::builtins::git::{self, Op}; +#[cfg(unix)] +use basal_host::builtins::git::{hardened_command, run_command}; use basal_host::builtins::{Denial, codes}; use basal_proto::Primitive; use basal_testkit::git::git_command; use basal_testkit::harness::scratch; use serde_json::{Value, json}; +#[cfg(unix)] use std::os::unix::process::ExitStatusExt; use std::path::{Path, PathBuf}; fn child_exit(status: &std::process::ExitStatus) -> String { - match status.code() { - Some(code) => format!("exit code {code}"), - None => format!( - "terminated by signal {}", - status - .signal() - .map_or_else(|| "unknown".to_owned(), |signal| signal.to_string()) - ), + #[cfg(windows)] + { + status.to_string() + } + #[cfg(unix)] + { + match status.code() { + Some(code) => format!("exit code {code}"), + None => format!( + "terminated by signal {}", + status + .signal() + .map_or_else(|| "unknown".to_owned(), |signal| signal.to_string()) + ), + } } } @@ -160,6 +170,7 @@ fn git_reads_only_approved_repositories() { }; let refused = git::run(&other.path(), &approved.repos(), &op).expect_err("refused"); assert_eq!(refused.code, codes::DENIED); + assert!(git::run(&approved.path(), &approved.repos(), &op).is_ok()); // A directory inside an approved repository is not itself approved. std::fs::create_dir_all(approved.dir.join("sub")).expect("sub"); let sub = approved.dir.join("sub").display().to_string(); @@ -169,10 +180,16 @@ fn git_reads_only_approved_repositories() { .code, codes::DENIED ); - // The same repository reached through `..` resolves to the approved - // path and is allowed. - let around = approved.dir.join("sub/..").display().to_string(); + // Unix canonicalizes relative components; Windows refuses them before + // opening a root, just as the Windows filesystem built-in does. + let around = approved.dir.join("sub").join("..").display().to_string(); + #[cfg(unix)] assert!(git::run(&around, &approved.repos(), &op).is_ok()); + #[cfg(windows)] + assert_eq!( + git::run(&around, &approved.repos(), &op).unwrap_err().code, + codes::DENIED + ); } #[test] @@ -201,6 +218,7 @@ fn arguments_that_git_could_read_as_options_are_refused() { } /// A script `name` in `dir` that appends its name to `marker` when run. +#[cfg(unix)] fn plant(dir: &Path, name: &str, marker: &Path) -> PathBuf { let path = dir.join(name); std::fs::write( @@ -220,6 +238,7 @@ fn plant(dir: &Path, name: &str, marker: &Path) -> PathBuf { /// while plain git in the same repository does run the fsmonitor program, /// which shows the plant works. #[test] +#[cfg(unix)] fn a_repository_config_cannot_make_a_built_in_run_a_program() { let repo = Repo::new("git-fsmonitor"); let marker = repo.base.join("marker"); diff --git a/crates/basal-testkit/tests/fs_write_temps.rs b/crates/basal-testkit/tests/fs_write_temps.rs index 8b0d3d95..12718cad 100644 --- a/crates/basal-testkit/tests/fs_write_temps.rs +++ b/crates/basal-testkit/tests/fs_write_temps.rs @@ -11,6 +11,7 @@ mod common; +#[cfg(unix)] use std::os::unix::fs::symlink; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex, mpsc}; @@ -170,7 +171,7 @@ impl Files { } fn out(&self) -> PathBuf { - self.root.join("sub/out.txt") + self.root.join("sub").join("out.txt") } fn secret(&self) -> PathBuf { @@ -323,11 +324,14 @@ fn a_cancelled_runs_write_that_dies_mid_flight_is_cleaned_up() { enum Tamper { /// Nothing: recovery removes it. None, + #[cfg(unix)] /// The file was replaced by a symlink to a file outside the root. SymlinkedTemp, + #[cfg(unix)] /// The directory was moved outside the root and a symlink to it put in /// its place. DirectoryMovedOutside, + #[cfg(unix)] /// The directory was moved elsewhere inside the root and a symlink to it /// put in its place. DirectoryMovedWithin, @@ -369,8 +373,11 @@ fn crash_mid_write(world: &World, files: &Files, manifest: &Value) -> (String, T fn a_crash_mid_write_is_cleaned_up_by_recovery_without_following_a_swapped_path() { for tamper in [ Tamper::None, + #[cfg(unix)] Tamper::SymlinkedTemp, + #[cfg(unix)] Tamper::DirectoryMovedOutside, + #[cfg(unix)] Tamper::DirectoryMovedWithin, ] { let world = World::new("fs-temps-crash"); @@ -380,19 +387,22 @@ fn a_crash_mid_write_is_cleaned_up_by_recovery_without_following_a_swapped_path( assert!(temp.is_file(), "{tamper:?}: {temp:?}"); let sub = files.root.join("sub"); // Where the recorded file is after tampering, if it is anywhere. - let left = match tamper { + let left: Option = match tamper { Tamper::None => None, + #[cfg(unix)] Tamper::SymlinkedTemp => { std::fs::remove_file(&temp).expect("remove"); symlink(files.secret(), &temp).expect("symlink"); Some(temp.clone()) } + #[cfg(unix)] Tamper::DirectoryMovedOutside => { let moved = files.outside.join("sub"); std::fs::rename(&sub, &moved).expect("move out"); symlink(&moved, &sub).expect("symlink"); Some(moved.join(&lease.temp)) } + #[cfg(unix)] Tamper::DirectoryMovedWithin => { let moved = files.root.join("moved"); std::fs::rename(&sub, &moved).expect("move within"); @@ -411,7 +421,10 @@ fn a_crash_mid_write_is_cleaned_up_by_recovery_without_following_a_swapped_path( Some(left) => { let meta = std::fs::symlink_metadata(&left) .unwrap_or_else(|e| panic!("{tamper:?}: {left:?} was removed: {e}")); + #[cfg(unix)] assert_eq!(meta.is_symlink(), tamper == Tamper::SymlinkedTemp); + #[cfg(windows)] + assert!(!meta.is_symlink()); } } assert_eq!( @@ -437,6 +450,7 @@ fn a_crash_mid_write_is_cleaned_up_by_recovery_without_following_a_swapped_path( /// directory still exists under its recorded path, but no longer lies in /// the root the write was granted, so its file is not removed. #[test] +#[cfg(unix)] fn a_crash_record_whose_directory_left_its_root_is_not_acted_on() { let world = World::new("fs-temps-retarget"); let files = Files::new("fs-temps-retarget-files"); @@ -495,6 +509,7 @@ fn legacy_temporary_files_are_removed_once_from_written_directories() { for name in keep { std::fs::write(sub.join(name), "keep").expect("decoy"); } + #[cfg(unix)] symlink(files.secret(), sub.join(".basal-55-6.tmp")).expect("symlink"); std::fs::write(files.root.join(".basal-7-8.tmp"), "unwritten dir").expect("root stray"); std::fs::write(files.outside.join(".basal-9-9.tmp"), "outside").expect("outside"); @@ -507,9 +522,11 @@ fn legacy_temporary_files_are_removed_once_from_written_directories() { assert!(!sub.join(".basal-123-4.tmp").exists()); let mut expected: Vec = keep.iter().map(|n| (*n).to_owned()).collect(); + #[cfg(unix)] expected.push(".basal-55-6.tmp".to_owned()); expected.sort(); assert_eq!(temps_in(&sub), expected); + #[cfg(unix)] assert!( std::fs::symlink_metadata(sub.join(".basal-55-6.tmp")) .expect("symlink kept") @@ -539,7 +556,12 @@ fn legacy_temporary_files_are_removed_once_from_written_directories() { fn remove_temp_removes_only_a_recorded_regular_file() { let files = Files::new("fs-temps-remove"); let sub = files.root.join("sub"); + #[cfg(unix)] let dir = std::fs::canonicalize(&sub).expect("real dir"); + #[cfg(windows)] + // Windows leases retain the write's drive-path spelling. Canonicalization + // could expand a short-name alias while the grant retains its original spelling. + let dir = sub.clone(); let roots = vec![files.root.display().to_string()]; let lease = |temp: &str| TempLease { call_key: "k".into(), diff --git a/crates/basal-testkit/tests/journal_kill.rs b/crates/basal-testkit/tests/journal_kill.rs index 310449ab..819af561 100644 --- a/crates/basal-testkit/tests/journal_kill.rs +++ b/crates/basal-testkit/tests/journal_kill.rs @@ -2,9 +2,10 @@ //! representative run, and separately of the worker. Every killed run must //! recover to the uncut run's final state and effect counts. +use basal_testkit::command::Command; +#[cfg(unix)] use std::os::unix::process::ExitStatusExt; use std::path::Path; -use std::process::Command; use std::sync::atomic::Ordering; use std::sync::{Arc, Mutex}; @@ -27,7 +28,10 @@ fn parent(dir: &Path, kill_at: Option<&Point>) -> (bool, Option, String) } let out = basal_testkit::process::output_until(&mut cmd, std::time::Duration::from_secs(950)) .expect("test parent must finish within its cleanup deadline"); + #[cfg(unix)] let killed = out.status.signal() == Some(libc::SIGKILL); + #[cfg(windows)] + let killed = out.status.code() == Some(basal_launch::KILL_EXIT_CODE as i32); let json = String::from_utf8_lossy(&out.stdout) .lines() .last() @@ -133,6 +137,10 @@ fn killing_the_worker_at_every_boundary_recovers_to_the_uncut_state() { let probe = Arc::new(Probe::act_at(point.clone(), move |_, _| { let pid = source.counts.live_pid.load(Ordering::SeqCst); if pid != 0 { + #[cfg(windows)] + basal_testkit::process::terminate_process(pid) + .expect("terminate live worker"); + #[cfg(unix)] // SAFETY: kill(2) has no memory-safety // preconditions; the pid is a worker this test // spawned and has not reaped. diff --git a/crates/basal-testkit/tests/windows_process.rs b/crates/basal-testkit/tests/windows_process.rs new file mode 100644 index 00000000..37e4a032 --- /dev/null +++ b/crates/basal-testkit/tests/windows_process.rs @@ -0,0 +1,161 @@ +#![cfg(windows)] + +use basal_testkit::command::Command; +use basal_testkit::harness::scratch; +use basal_testkit::process::{assert_reaped, output_until}; +use basal_testkit::{WorkerProcess, worker_binary}; +use std::path::Path; +use std::time::Duration; + +fn pipe_tree(dir: &Path, hold_parent: bool) -> Command { + let powershell = std::path::PathBuf::from(std::env::var_os("SystemRoot").unwrap()) + .join("System32") + .join("WindowsPowerShell") + .join("v1.0") + .join("powershell.exe"); + let pid_file = dir.join("descendant.pid"); + let quoted_path = pid_file.display().to_string().replace('\'', "''"); + // Start-Process without redirection inherits the parent's standard pipes. + // The child publishes its pid before waiting, so a missing job kill would + // leave both a live process and a pipe reader that cannot finish. + let script = format!( + "$p = Start-Process -PassThru -NoNewWindow -FilePath '{}' -ArgumentList '-NoProfile','-Command','\"[IO.File]::WriteAllText(''{}'', [string]$PID); Start-Sleep -Seconds 600\"'; while (!(Test-Path '{}')) {{ Start-Sleep -Milliseconds 10 }}; {}", + powershell.display(), + quoted_path, + quoted_path, + if hold_parent { + "Start-Sleep -Seconds 600" + } else { + "exit 0" + }, + ); + let mut command = Command::new(powershell); + command + .args(["-NoProfile", "-NonInteractive", "-Command"]) + .arg(script); + command +} + +fn assert_descendant_dead(dir: &Path) { + use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle}; + use windows_sys::Win32::Foundation::WAIT_OBJECT_0; + use windows_sys::Win32::System::Threading::{ + OpenProcess, PROCESS_QUERY_LIMITED_INFORMATION, WaitForSingleObject, + }; + let pid: u32 = std::fs::read_to_string(dir.join("descendant.pid")) + .expect("descendant readiness") + .trim() + .parse() + .unwrap(); + // SYNCHRONIZE is a standard access right, not a process-specific flag. + let handle = unsafe { OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION | 0x0010_0000, 0, pid) }; + if handle.is_null() { + assert_eq!( + std::io::Error::last_os_error().raw_os_error(), + Some(87), + "only an exited, released process may disappear" + ); + return; + } + let handle = unsafe { OwnedHandle::from_raw_handle(handle) }; + assert_eq!( + unsafe { WaitForSingleObject(handle.as_raw_handle(), 30_000) }, + WAIT_OBJECT_0, + "descendant survived job cleanup" + ); +} + +#[test] +fn subprocess_deadline_reaps_descendants_holding_output_pipes() { + let dir = scratch("windows-pipe-deadline"); + let mut command = pipe_tree(&dir, true); + let error = output_until(&mut command, Duration::from_secs(10)).unwrap_err(); + assert_eq!(error.kind(), std::io::ErrorKind::TimedOut); + assert_descendant_dead(&dir); + std::fs::remove_dir_all(dir).unwrap(); +} + +#[test] +fn subprocess_exit_reaps_descendants_holding_output_pipes() { + let dir = scratch("windows-pipe-exit"); + let output = output_until(&mut pipe_tree(&dir, false), Duration::from_secs(30)).unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert_descendant_dead(&dir); + std::fs::remove_dir_all(dir).unwrap(); +} + +#[test] +fn worker_memory_sampling_and_reaping_observe_native_process_state() { + let (mut worker, _) = WorkerProcess::start(&worker_binary(), Duration::from_secs(60)).unwrap(); + let pid = worker.pid(); + assert!(worker.rss_kib().is_some_and(|kib| kib > 0)); + assert!( + std::panic::catch_unwind(|| assert_reaped(pid)).is_err(), + "a running worker is not reaped" + ); + worker.close_stdin(); + assert!(worker.wait_exit(Duration::from_secs(60)).unwrap().success()); + assert_reaped(pid); + drop(worker); + assert_reaped(pid); +} + +#[test] +fn discovered_worker_launches_with_full_confinement_without_an_extra_acl_grant() { + use basal_proto::{ + Confinement, PROTOCOL_VERSION, ParentMessage, WorkerMessage, encode_parent_frame, + read_worker_message, + }; + use std::io::Write; + let binary = worker_binary(); + assert_eq!(binary, basal_testkit::channel::worker_binary()); + assert!( + binary + .file_name() + .unwrap() + .to_string_lossy() + .starts_with("ckdev-") + ); + let options = basal_launch::LaunchOptions::new(&binary, basal_proto::FLOW_JOB_COMMIT_BYTES); + let mut child = basal_launch::launch(&options).unwrap(); + child + .stdin + .as_mut() + .unwrap() + .write_all( + &encode_parent_frame(&ParentMessage::Hello { + protocol_version: PROTOCOL_VERSION, + }) + .unwrap(), + ) + .unwrap(); + let mut stdout = child.stdout.take().unwrap(); + let (tx, rx) = std::sync::mpsc::channel(); + let reader = std::thread::spawn(move || { + let _ = tx.send(read_worker_message(&mut stdout)); + }); + let frame = rx + .recv_timeout(Duration::from_secs(60)) + .expect("handshake before hang deadline") + .unwrap(); + let WorkerMessage::Welcome(welcome) = frame else { + panic!("expected welcome, got {frame:?}"); + }; + assert_eq!( + welcome.confinement, + Confinement::Windows { + lpac: true, + untrusted: true, + no_thread_token: true, + mitigations: true, + handle_table: true + } + ); + child.stdin.take(); + assert_eq!(child.wait().unwrap(), 0); + reader.join().unwrap(); +} diff --git a/crates/basal-testkit/tests/worker_discovery.rs b/crates/basal-testkit/tests/worker_discovery.rs index 53c37763..2175dc32 100644 --- a/crates/basal-testkit/tests/worker_discovery.rs +++ b/crates/basal-testkit/tests/worker_discovery.rs @@ -1,18 +1,27 @@ //! Cargo owns the build-time worker path; runtime discovery never invokes it. +use basal_testkit::command::Command; +#[cfg(unix)] use std::os::unix::fs::PermissionsExt; +#[cfg(unix)] use std::os::unix::process::ExitStatusExt; -use std::process::Command; fn child_exit(status: &std::process::ExitStatus) -> String { - match status.code() { - Some(code) => format!("exit code {code}"), - None => format!( - "terminated by signal {}", - status - .signal() - .map_or_else(|| "unknown".to_owned(), |signal| signal.to_string()) - ), + #[cfg(windows)] + { + status.to_string() + } + #[cfg(unix)] + { + match status.code() { + Some(code) => format!("exit code {code}"), + None => format!( + "terminated by signal {}", + status + .signal() + .map_or_else(|| "unknown".to_owned(), |signal| signal.to_string()) + ), + } } } @@ -49,13 +58,18 @@ fn every_test_process_uses_the_built_worker_without_invoking_cargo() { return; } let dir = basal_testkit::harness::scratch("worker-discovery"); + #[cfg(unix)] let cargo = dir.join("cargo-fixture"); + #[cfg(windows)] + let cargo = std::path::PathBuf::from(env!("CARGO_BIN_EXE_basal-test-parent")); let calls = dir.join("calls"); + #[cfg(unix)] std::fs::write( &cargo, "#!/bin/sh\necho build >> \"$BASAL_DISCOVERY_CALLS\"\nexit 1\n", ) .unwrap(); + #[cfg(unix)] std::fs::set_permissions(&cargo, std::fs::Permissions::from_mode(0o755)).unwrap(); for _ in 0..2 { let result = Command::new(std::env::current_exe().unwrap()) diff --git a/crates/basal-worker/Cargo.toml b/crates/basal-worker/Cargo.toml index 264dbdef..3ed997c6 100644 --- a/crates/basal-worker/Cargo.toml +++ b/crates/basal-worker/Cargo.toml @@ -52,10 +52,6 @@ deviations = [] [dev-dependencies] serde_json.workspace = true - -# basal-testkit does not build on Windows yet. The test files that use it are -# compiled only on other systems until it does. -[target.'cfg(not(windows))'.dev-dependencies] basal-testkit.workspace = true # On Windows every test spawn of the worker goes through the launcher, under diff --git a/crates/basal-worker/examples/wrapper_stack.rs b/crates/basal-worker/examples/wrapper_stack.rs index 93b6e79b..9efbbe00 100644 --- a/crates/basal-worker/examples/wrapper_stack.rs +++ b/crates/basal-worker/examples/wrapper_stack.rs @@ -3,8 +3,6 @@ //! Frozen globals reject new names before a call can run; scoped functions can //! recurse. Direct versus worker entry separates those engine costs from OS confinement. -// Windows: gated until basal-testkit builds there. -#![cfg_attr(windows, allow(dead_code, unused_imports))] use basal_proto::{ActivationRequest, ActivationResult, Budgets, JsonText, Profile}; use basal_worker::{engine, link::Channel}; use std::cell::RefCell; @@ -33,12 +31,6 @@ fn direct(request: &ActivationRequest) -> ActivationResult { ))); engine::run_activation(request, link) } -#[cfg(windows)] -fn main() { - panic!("wrapper_stack needs basal-testkit, which does not build on Windows yet"); -} - -#[cfg(not(windows))] fn main() { let mode = std::env::args().nth(1).unwrap_or_else(|| "direct".into()); println!( diff --git a/crates/basal-worker/tests/budgets.rs b/crates/basal-worker/tests/budgets.rs index ff966d17..167bef48 100644 --- a/crates/basal-worker/tests/budgets.rs +++ b/crates/basal-worker/tests/budgets.rs @@ -1,8 +1,5 @@ //! Per-activation limits: JS time, memory, stack, and stalled promises. -// Windows: gated until basal-testkit builds there. -#![cfg(not(windows))] - mod common; use std::time::Duration; diff --git a/crates/basal-worker/tests/clock.rs b/crates/basal-worker/tests/clock.rs index b4059f0c..227b452f 100644 --- a/crates/basal-worker/tests/clock.rs +++ b/crates/basal-worker/tests/clock.rs @@ -1,9 +1,6 @@ //! Clock reads and random samples are host calls: journaled on the first //! run, replayed from the journal afterwards. -// Windows: gated until basal-testkit builds there. -#![cfg(not(windows))] - mod common; use std::time::Duration; diff --git a/crates/basal-worker/tests/codemode.rs b/crates/basal-worker/tests/codemode.rs index 264c5489..b9d4cf27 100644 --- a/crates/basal-worker/tests/codemode.rs +++ b/crates/basal-worker/tests/codemode.rs @@ -1,8 +1,5 @@ //! Codemode's confined wire contract, without the flow parent's journal or hosts. -// Windows: gated until basal-testkit builds there. -#![cfg(not(windows))] - mod common; use std::time::Duration; diff --git a/crates/basal-worker/tests/fuzz.rs b/crates/basal-worker/tests/fuzz.rs index 0bce3d69..d9f9f654 100644 --- a/crates/basal-worker/tests/fuzz.rs +++ b/crates/basal-worker/tests/fuzz.rs @@ -2,9 +2,6 @@ //! the JS time budget. Every activation must end with a typed result and the //! worker must survive all of them. -// Windows: gated until basal-testkit builds there. -#![cfg(not(windows))] - mod common; use std::time::Duration; diff --git a/crates/basal-worker/tests/inherited_descriptors.rs b/crates/basal-worker/tests/inherited_descriptors.rs index 5800718d..c86f7e88 100644 --- a/crates/basal-worker/tests/inherited_descriptors.rs +++ b/crates/basal-worker/tests/inherited_descriptors.rs @@ -1,7 +1,5 @@ //! The real worker must discard inherited authority even in probe mode. - -// Windows: gated until basal-testkit builds there. -#![cfg(not(windows))] +#![cfg(unix)] mod common; diff --git a/crates/basal-worker/tests/ipc.rs b/crates/basal-worker/tests/ipc.rs index caaa5815..ed86ef1d 100644 --- a/crates/basal-worker/tests/ipc.rs +++ b/crates/basal-worker/tests/ipc.rs @@ -1,9 +1,6 @@ //! The channel: malformed, oversized and out-of-order frames are refused //! with typed errors, and none of them crashes the worker. -// Windows: gated until basal-testkit builds there. -#![cfg(not(windows))] - mod common; use std::time::Duration; @@ -88,6 +85,17 @@ fn handshake_reports_version_engine_and_confinement() { assert_eq!(welcome.protocol_version, PROTOCOL_VERSION); #[cfg(target_os = "macos")] assert_eq!(welcome.confinement, Confinement::Seatbelt); + #[cfg(windows)] + assert_eq!( + welcome.confinement, + Confinement::Windows { + lpac: true, + untrusted: true, + no_thread_token: true, + mitigations: true, + handle_table: true, + } + ); #[cfg(target_os = "linux")] assert!(matches!( welcome.confinement, diff --git a/crates/basal-worker/tests/lockdown.rs b/crates/basal-worker/tests/lockdown.rs index bfa3a769..e0a13165 100644 --- a/crates/basal-worker/tests/lockdown.rs +++ b/crates/basal-worker/tests/lockdown.rs @@ -2,9 +2,6 @@ //! operator-approved script that basal runs unattended; the lockdown keeps //! it to the globals listed below and the host calls basal journals. -// Windows: gated until basal-testkit builds there. -#![cfg(not(windows))] - mod common; use basal_proto::{CallKind, JsonText, Primitive}; diff --git a/crates/basal-worker/tests/profile.rs b/crates/basal-worker/tests/profile.rs index 927a32af..e2bc4c62 100644 --- a/crates/basal-worker/tests/profile.rs +++ b/crates/basal-worker/tests/profile.rs @@ -3,9 +3,6 @@ //! engine unit test `raw_bridge_cannot_issue_sh_in_flow_profile` also checks the //! Rust refusal when a call bypasses the script's API. -// Windows: gated until basal-testkit builds there. -#![cfg(not(windows))] - mod common; use basal_proto::{CallKind, Primitive, Profile}; diff --git a/crates/basal-worker/tests/replay.rs b/crates/basal-worker/tests/replay.rs index acb63e9c..cd9999e4 100644 --- a/crates/basal-worker/tests/replay.rs +++ b/crates/basal-worker/tests/replay.rs @@ -1,8 +1,5 @@ //! Replay of the recorded prefix inside the worker. -// Windows: gated until basal-testkit builds there. -#![cfg(not(windows))] - mod common; use basal_proto::{ diff --git a/crates/basal-worker/tests/sandbox_integrity.rs b/crates/basal-worker/tests/sandbox_integrity.rs index 04ae2532..2cb61731 100644 --- a/crates/basal-worker/tests/sandbox_integrity.rs +++ b/crates/basal-worker/tests/sandbox_integrity.rs @@ -1,8 +1,5 @@ //! Adversarial scripts at the sandbox's function and exception boundaries. -// Windows: gated until basal-testkit builds there. -#![cfg(not(windows))] - mod common; use basal_proto::{ActivationResult, Failure}; diff --git a/evidence/windows-build-failures.md b/evidence/windows-build-failures.md index e7dd9878..7a57a70f 100644 --- a/evidence/windows-build-failures.md +++ b/evidence/windows-build-failures.md @@ -155,6 +155,30 @@ the failures above are fixed. - **Tests:** every test in the workspace. Both test commands stopped while compiling `basal-host`. +## Test-kit and rig portability + +The original measured baseline above is retained. The following tests, or the +explicitly named fixture subcases of mixed tests, require POSIX semantics. The +remaining assertions in mixed tests still run on Windows. + +- `crates/basal-worker/tests/inherited_descriptors.rs::worker_closes_extra_inherited_descriptors_at_startup` — POSIX descriptor numbers, `dup2`, and close-on-exec flags; Windows inherited-handle checks run in the launcher and Windows confinement suites. +- `crates/basal-testkit/src/git.rs::tests::fixture_git_commands_ignore_injected_global_hooks` — executable `/bin/sh` hook fixture; Windows built-in config/hook isolation has native coverage in basal-host. +- `crates/basal-testkit/tests/builtins_git.rs::a_repository_config_cannot_make_a_built_in_run_a_program` — executable `/bin/sh` fsmonitor, pager, diff and hook fixtures; Windows built-in isolation has native coverage in basal-host. +- `crates/basal-testkit/tests/builtins_fs.rs::a_symlink_resolving_outside_the_root_is_refused` — symlink creation requiring Windows privilege, and Unix in-root-link traversal semantics. +- `crates/basal-testkit/tests/builtins_fs.rs::a_symlink_swapped_into_the_last_component_after_the_check_is_not_followed` — privilege-dependent symlink fixture. +- `crates/basal-testkit/tests/builtins_fs.rs::a_directory_swapped_for_a_symlink_after_the_check_is_caught_after_the_open` — privilege-dependent symlink fixture. +- `crates/basal-testkit/tests/builtins_fs.rs::list_names_entries_and_their_kinds` (symlink entry only) — privilege-dependent symlink creation; ordinary entries and the listing cap remain tested on Windows. +- `crates/basal-testkit/tests/builtins_fs.rs::write_replaces_the_whole_file_inside_a_write_root_only` (symlink paths only) — privilege-dependent symlink creation; writes, outside-root refusal, relative-component refusal, size cap and temp cleanup remain tested on Windows. +- `crates/basal-testkit/tests/fs_write_temps.rs::a_crash_mid_write_is_cleaned_up_by_recovery_without_following_a_swapped_path` (three symlink tamper variants only) — privilege-dependent symlink creation; untampered crash recovery runs on Windows. +- `crates/basal-testkit/tests/fs_write_temps.rs::a_crash_record_whose_directory_left_its_root_is_not_acted_on` — privilege-dependent symlink creation and retargeting a granted Unix symlink root. +- `crates/basal-testkit/tests/fs_write_temps.rs::legacy_temporary_files_are_removed_once_from_written_directories` (symlink temp only) — privilege-dependent symlink creation; regular temp cleanup, decoy names, outside files and once-per-store behavior remain tested on Windows. +- `crates/basal-rig/src/bin/basal-rig-contract/tests.rs::seeded_fixture_rejects_foreign_paths_and_symlinked_stores` (symlinked store only) — privilege-dependent symlink creation; foreign-path and module refusal assertions remain tested on Windows. +- `crates/basal-host/tests/builtin_host_regressions.rs::escaped_listing_exceeds_encoded_cap_as_a_typed_refusal` — Unix control-character filenames, which NTFS refuses; escaped-file dispatch limits remain tested on Windows. +- `crates/basal-host/tests/builtin_host_regressions.rs::atomic_write_drops_special_permission_bits` — POSIX setuid/setgid/mode bits; Windows replacement-DACL behavior has native basal-host coverage. +- `crates/basal-host/tests/builtin_host_regressions.rs::git_reaps_descendants_that_keep_its_output_pipe_open` — `/bin/sh`/`sleep` fixture and POSIX process-group cleanup; native Windows job/descendant tests run in basal-host and basal-testkit. +- `crates/basal-testkit/src/https.rs::tests::idle_https_accept_blocks_and_shutdown_wakes_it_without_a_request` (`fcntl` flag readback only) — POSIX socket flags; shutdown wakeup and absence of HTTPS requests remain tested on Windows. +- `crates/basal-testkit/src/process.rs::tests::subprocess_deadline_reaps_descendants_holding_output_pipes` — `/bin/sh`/`sleep` process-group fixture; `crates/basal-testkit/tests/windows_process.rs::subprocess_deadline_reaps_descendants_holding_output_pipes` tests the Windows job equivalent by the same test name. + ## Tests gated on Windows These tests are compiled out (`cfg(unix)`) or ignored (`cfg_attr(windows, diff --git a/mutations.toml b/mutations.toml index 65bd603a..4510d0ae 100644 --- a/mutations.toml +++ b/mutations.toml @@ -546,7 +546,7 @@ edits = [ { file = "Cargo.lock", old = " \"subc-client-rs\",\n \"subc-protocol\",\n \"subc-transport\",\n \"tokio\",\n \"tracing\",\n \"webpki-roots\",\n", new = " \"subc-client-rs\",\n \"subc-protocol 0.30.0\",\n \"subc-transport\",\n \"tokio\",\n \"tracing\",\n \"webpki-roots\",\n" }, { file = "Cargo.lock", old = " \"serde\",\n \"serde_json\",\n \"sha2\",\n \"subc-client-rs\",\n \"subc-os\",\n \"subc-protocol\",\n \"subc-transport\",\n \"tokio\",\n \"tracing\",\n", new = " \"serde\",\n \"serde_json\",\n \"sha2\",\n \"subc-client-rs\",\n \"subc-os\",\n \"subc-protocol 0.30.0\",\n \"subc-transport\",\n \"tokio\",\n \"tracing\",\n" }, { file = "Cargo.lock", old = " \"rcgen\",\n \"rusqlite\",\n \"rustls\",\n \"serde_json\",\n \"subc-protocol\",\n", new = " \"rcgen\",\n \"rusqlite\",\n \"rustls\",\n \"serde_json\",\n \"subc-protocol 0.30.0\",\n" }, - { file = "Cargo.lock", old = " \"rusqlite\",\n \"serde_json\",\n \"sha2\",\n \"subc-client-rs\",\n \"subc-protocol\",\n \"tokio\",\n]\n", new = " \"rusqlite\",\n \"serde_json\",\n \"sha2\",\n \"subc-client-rs\",\n \"subc-protocol 0.30.0\",\n \"tokio\",\n]\n" }, + { file = "Cargo.lock", old = " \"rusqlite\",\n \"serde_json\",\n \"sha2\",\n \"subc-client-rs\",\n \"subc-protocol\",\n \"tokio\",\n \"windows-sys 0.61.2\",\n]\n", new = " \"rusqlite\",\n \"serde_json\",\n \"sha2\",\n \"subc-client-rs\",\n \"subc-protocol 0.30.0\",\n \"tokio\",\n \"windows-sys 0.61.2\",\n]\n" }, { file = "Cargo.lock", old = " \"rquickjs\",\n \"seccompiler\",\n \"serde_json\",\n \"windows-sys 0.61.2\",\n]\n\n[[package]]\n", new = " \"rquickjs\",\n \"seccompiler\",\n \"serde_json\",\n \"subc-protocol 0.24.1\",\n \"windows-sys 0.61.2\",\n]\n\n[[package]]\n" }, { file = "Cargo.lock", old = " \"serde_json\",\n \"subc-control\",\n \"subc-os\",\n \"subc-protocol\",\n \"subc-transport\",\n \"tokio\",\n \"tokio-util\",\n", new = " \"serde_json\",\n \"subc-control\",\n \"subc-os\",\n \"subc-protocol 0.30.0\",\n \"subc-transport\",\n \"tokio\",\n \"tokio-util\",\n" }, { file = "Cargo.lock", old = "dependencies = [\n \"serde\",\n \"serde_json\",\n \"subc-protocol\",\n]\n", new = "dependencies = [\n \"serde\",\n \"serde_json\",\n \"subc-protocol 0.30.0\",\n]\n" },