diff --git a/.github/workflows/windows-process-host.yml b/.github/workflows/windows-process-host.yml new file mode 100644 index 000000000..4f2b70b18 --- /dev/null +++ b/.github/workflows/windows-process-host.yml @@ -0,0 +1,115 @@ +name: Windows process host + +on: + push: + branches: [main, stream-*] + paths: + - 'crates/astrid-audit/**' + - 'crates/astrid-capsule/**' + - 'crates/astrid-core/**' + - 'crates/astrid-kernel/**' + - 'crates/astrid-mcp/**' + - 'crates/astrid-workspace/**' + - 'Cargo.toml' + - 'Cargo.lock' + - '.github/workflows/windows-process-host.yml' + pull_request: + branches: [main] + paths: + - 'crates/astrid-audit/**' + - 'crates/astrid-capsule/**' + - 'crates/astrid-core/**' + - 'crates/astrid-kernel/**' + - 'crates/astrid-mcp/**' + - 'crates/astrid-workspace/**' + - 'Cargo.toml' + - 'Cargo.lock' + - '.github/workflows/windows-process-host.yml' + +permissions: + contents: read + +env: + CARGO_TERM_COLOR: always + RUST_BACKTRACE: 1 + +jobs: + native: + name: Native ${{ matrix.arch }} + runs-on: ${{ matrix.runner }} + timeout-minutes: 45 + strategy: + fail-fast: false + matrix: + include: + - arch: x86_64 + runner: windows-2025 + host: x86_64-pc-windows-msvc + - arch: aarch64 + runner: windows-11-arm + host: aarch64-pc-windows-msvc + steps: + - name: Harden Runner + uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4 + with: + egress-policy: audit + + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + submodules: recursive + + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + with: + toolchain: '1.95.0' + components: clippy + + - name: Cache Rust artifacts + uses: Swatinem/rust-cache@42dc69e1aa15d09112580998cf2ef0119e2e91ae # v2 + with: + key: windows-process-${{ matrix.host }} + cache-on-failure: true + + - name: Assert native architecture + shell: pwsh + env: + EXPECTED_HOST: ${{ matrix.host }} + run: | + $actual = (rustc -vV | Select-String '^host: ').Line.Substring(6) + if ($actual -ne $env:EXPECTED_HOST) { + throw "Expected native Rust host $env:EXPECTED_HOST, got $actual" + } + + - name: Check process-host crates + run: >- + cargo check --locked + -p astrid-audit + -p astrid-capsule + -p astrid-core + -p astrid-kernel + -p astrid-mcp + -p astrid-workspace + + - name: Clippy process-host crates + run: >- + cargo clippy --locked + -p astrid-audit + -p astrid-capsule + -p astrid-core + -p astrid-kernel + -p astrid-mcp + -p astrid-workspace + --all-targets + --all-features + --no-deps + -- + -D warnings + + - name: Test Windows process behavior + run: cargo test --locked -p astrid-capsule windows_ -- --nocapture + + - name: Test Windows sandbox policy + run: cargo test --locked -p astrid-workspace windows_ -- --nocapture + + - name: Test native MCP denial audit + run: cargo test --locked -p astrid-mcp windows_ -- --nocapture diff --git a/CHANGELOG.md b/CHANGELOG.md index d99f0a0be..10df7ce3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,18 @@ Changelog tracking starts with 0.2.0. Prior versions were not tracked. cancellable busy-instance retries support probe-then-connect and concurrent clients. Native x86_64 and ARM64 Windows jobs exercise binding, concurrent instances, shutdown, and reconnect behavior. Closes #1349. +- **The native process host has a deterministic Windows backend.** Capsule + arguments cross the OS boundary as distinct `CreateProcessW` arguments + without an implicit command shell; executable resolution, case-insensitive + environment handling, working directories, stdin/stdout/stderr, exit status, + cancellation, and descendant cleanup are pinned by native x86_64 and ARM64 + tests. Each child tree is owned by a kill-on-close Windows Job Object, so + cancellation and root-first exit cannot strand descendants or chase a reused + PID. Explicit sandbox policy `off` permits trusted native execution while + `required` fails closed before exec; unsupported signals and untrusted native + MCP starts produce signed denial records. Host-local bind support follows the + local-transport backend from #1349 rather than adding a process-host-specific + transport. Closes #1351. - **Linux amd64 now has a distro-neutral OCI build target.** The image packages exact immutable GitHub release bytes only after their tagged release-workflow signatures and manifest digests verify, runs the persistent daemon as a diff --git a/Cargo.lock b/Cargo.lock index 60e842e3c..b83513727 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -500,6 +500,7 @@ dependencies = [ "wasmparser 0.253.0", "wasmtime", "wasmtime-wasi", + "windows-sys 0.61.2", ] [[package]] diff --git a/crates/astrid-audit/src/entry.rs b/crates/astrid-audit/src/entry.rs index 5ef27fcca..30a1c47cd 100644 --- a/crates/astrid-audit/src/entry.rs +++ b/crates/astrid-audit/src/entry.rs @@ -467,6 +467,17 @@ pub enum AuditAction { #[serde(default, skip_serializing_if = "Option::is_none")] device_key_id: Option, }, + + /// Signal requested for an existing child-process tree. + /// + /// Keep new variants at the end of this public enum so existing implicit + /// discriminants remain stable for downstream consumers. + ProcessSignal { + /// Command or non-reversible process descriptor. + process: String, + /// Stable signal name. + signal: String, + }, } impl AuditAction { @@ -543,6 +554,9 @@ impl AuditAction { Self::ProcessSpawn { command } => { format!("Spawned process {command}") }, + Self::ProcessSignal { process, signal } => { + format!("Signalled process {process} with {signal}") + }, Self::CapabilityCreated { resource, .. } => { format!("Created capability for {resource}") }, diff --git a/crates/astrid-capsule/Cargo.toml b/crates/astrid-capsule/Cargo.toml index ddae8ba85..b81bf6e32 100644 --- a/crates/astrid-capsule/Cargo.toml +++ b/crates/astrid-capsule/Cargo.toml @@ -63,6 +63,15 @@ uuid = { workspace = true } [target.'cfg(unix)'.dependencies] nix = { workspace = true } +[target.'cfg(windows)'.dependencies] +windows-sys = { workspace = true, features = [ + "Win32_Foundation", + "Win32_Security", + "Win32_System_Diagnostics_ToolHelp", + "Win32_System_JobObjects", + "Win32_System_Threading", +] } + [target.'cfg(not(all(target_arch = "wasm32", target_os = "unknown")))'.dependencies] astrid-mcp = { workspace = true } # `astrid-vfs` is native-only (it uses `cap-std` and tokio's filesystem diff --git a/crates/astrid-capsule/src/audit_sink.rs b/crates/astrid-capsule/src/audit_sink.rs index 478763ffd..5a001ee90 100644 --- a/crates/astrid-capsule/src/audit_sink.rs +++ b/crates/astrid-capsule/src/audit_sink.rs @@ -103,4 +103,20 @@ pub trait HostAuditSink: Send + Sync { event: HostAuditEvent<'_>, outcome: HostAuditOutcome<'_>, ); + + /// Record a process-signal request without extending the public, + /// exhaustively matched [`HostAuditEvent`] enum. + /// + /// The provided no-op keeps existing external sink implementations source + /// compatible. The kernel sink overrides this method and persists a typed, + /// signed process-signal action. + fn record_process_signal( + &self, + principal: &astrid_core::PrincipalId, + process: &str, + signal: &str, + outcome: HostAuditOutcome<'_>, + ) { + let _ = (principal, process, signal, outcome); + } } diff --git a/crates/astrid-capsule/src/engine/wasm/host/audit_sink_tests.rs b/crates/astrid-capsule/src/engine/wasm/host/audit_sink_tests.rs index 2e9c942d9..96eff2d44 100644 --- a/crates/astrid-capsule/src/engine/wasm/host/audit_sink_tests.rs +++ b/crates/astrid-capsule/src/engine/wasm/host/audit_sink_tests.rs @@ -11,9 +11,15 @@ use std::sync::{Arc, Mutex}; use astrid_core::PrincipalId; use crate::audit_sink::{HostAuditEvent, HostAuditOutcome, HostAuditSink}; +#[cfg(windows)] +use crate::engine::wasm::bindings::astrid::process1_1_0::host as process_host; use crate::engine::wasm::host_state::HostState; use crate::engine::wasm::test_fixtures::minimal_host_state; +#[cfg(windows)] +#[path = "audit_sink_tests/windows_signal.rs"] +mod windows_signal; + /// An owned, comparable snapshot of a reported event. #[derive(Debug, Clone, PartialEq, Eq)] enum CapturedEvent { @@ -23,6 +29,7 @@ enum CapturedEvent { NetConnect(String, u16), NetBind(String), ProcessSpawn(String), + ProcessSignal(String, String), } impl CapturedEvent { @@ -75,6 +82,20 @@ impl HostAuditSink for RecordingSink { CapturedOutcome::from(outcome), )); } + + fn record_process_signal( + &self, + principal: &PrincipalId, + process: &str, + signal: &str, + outcome: HostAuditOutcome<'_>, + ) { + self.records.lock().expect("sink mutex").push(( + principal.clone(), + CapturedEvent::ProcessSignal(process.to_owned(), signal.to_owned()), + CapturedOutcome::from(outcome), + )); + } } impl RecordingSink { @@ -178,7 +199,7 @@ async fn audit_net_reports_bind_denied() { super::net::record_net_denied( &state, HostAuditEvent::NetBind { - addr: "unix:cli-socket", + addr: "local:cli-control", }, "no net_bind capability", ); @@ -188,7 +209,7 @@ async fn audit_net_reports_bind_denied() { assert_eq!(records[0].0, alice); assert_eq!( records[0].1, - CapturedEvent::NetBind("unix:cli-socket".into()) + CapturedEvent::NetBind("local:cli-control".into()) ); assert!( matches!(records[0].2, CapturedOutcome::Denied(_)), @@ -197,6 +218,25 @@ async fn audit_net_reports_bind_denied() { ); } +#[cfg(windows)] +#[tokio::test] +async fn windows_host_local_bind_denial_is_audited() { + let (mut state, sink) = state_with_sink(tokio::runtime::Handle::current()); + let result = crate::engine::wasm::bindings::astrid::net::host::Host::bind_unix(&mut state); + + assert!(matches!( + result, + Err(crate::engine::wasm::bindings::astrid::net::host::ErrorCode::CapabilityDenied) + )); + let records = sink.snapshot(); + assert_eq!(records.len(), 1, "denied bind must report exactly once"); + assert_eq!( + records[0].1, + CapturedEvent::NetBind("local:cli-control".into()) + ); + assert!(matches!(records[0].2, CapturedOutcome::Denied(_))); +} + #[tokio::test] async fn audit_process_reports_spawn() { let (state, sink) = state_with_sink(tokio::runtime::Handle::current()); @@ -257,6 +297,74 @@ async fn audit_process_reports_spawn_variants() { ); } +#[tokio::test] +async fn audit_process_signal_closed_is_a_system_failure() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ErrorCode, ProcessSignal}; + + let (state, sink) = state_with_sink(tokio::runtime::Handle::current()); + super::process::audit_process_signal( + &state, + "already-exited", + ProcessSignal::Term, + &Err(ErrorCode::Closed), + ); + + let records = sink.snapshot(); + assert_eq!(records.len(), 1); + assert_eq!( + records[0].1, + CapturedEvent::ProcessSignal("already-exited".into(), "term".into()) + ); + assert_eq!( + records[0].2, + CapturedOutcome::Failed("ErrorCode::Closed".into()) + ); +} + +#[tokio::test] +async fn audit_process_signal_unsupported_persistence_is_a_system_failure() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ErrorCode, ProcessSignal}; + + let (state, sink) = state_with_sink(tokio::runtime::Handle::current()); + super::process::audit_process_signal( + &state, + "unsupported-process", + ProcessSignal::Term, + &Err(ErrorCode::PersistUnsupported), + ); + + let records = sink.snapshot(); + assert_eq!(records.len(), 1); + assert_eq!( + records[0].2, + CapturedOutcome::Failed("ErrorCode::PersistUnsupported".into()) + ); +} + +#[tokio::test] +async fn audit_process_signal_capability_denial_remains_denied() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ErrorCode, ProcessSignal}; + + let (state, sink) = state_with_sink(tokio::runtime::Handle::current()); + super::process::audit_process_signal( + &state, + "protected-process", + ProcessSignal::Term, + &Err(ErrorCode::CapabilityDenied), + ); + + let records = sink.snapshot(); + assert_eq!(records.len(), 1); + assert_eq!( + records[0].1, + CapturedEvent::ProcessSignal("protected-process".into(), "term".into()) + ); + assert_eq!( + records[0].2, + CapturedOutcome::Denied("ErrorCode::CapabilityDenied".into()) + ); +} + #[tokio::test] async fn audit_fs_reports_denied() { // A security-gate denial must reach the sink as `Denied` — today the @@ -328,3 +436,600 @@ async fn connect_tcp_denial_lands_on_the_chain() { records[0].2 ); } + +#[cfg(windows)] +fn windows_probe_request( + mode: &str, + mut env: Vec, +) -> process_host::SpawnRequest { + use process_host::EnvVar; + + env.push(EnvVar { + key: "ASTRID_WINDOWS_PROCESS_PROBE".to_string(), + value: mode.to_string(), + }); + process_host::SpawnRequest { + cmd: std::env::current_exe() + .expect("current test executable") + .to_string_lossy() + .into_owned(), + args: vec![ + "windows_process_probe_child".to_string(), + "--nocapture".to_string(), + ], + stdin: None, + env, + cwd: None, + limits: None, + label: None, + keep_stdin_open: None, + overflow: None, + log_ring_bytes: None, + max_lifetime_ms: None, + idle_timeout_ms: None, + exit_retention_ms: None, + file_injections: Vec::new(), + } +} + +#[cfg(windows)] +fn windows_touch_request(sentinel: &std::path::Path) -> process_host::SpawnRequest { + windows_probe_request( + "touch", + vec![process_host::EnvVar { + key: "ASTRID_SENTINEL".to_string(), + value: sentinel.to_string_lossy().into_owned(), + }], + ) +} + +#[cfg(windows)] +fn authenticate_windows_process_state(state: &mut HostState) { + state.caller_context = Some( + astrid_events::ipc::IpcMessage::new( + astrid_events::ipc::Topic::from_raw("test.windows.process"), + astrid_events::ipc::IpcPayload::RawJson(serde_json::json!({})), + uuid::Uuid::new_v4(), + ) + .with_principal("alice"), + ); +} + +#[cfg(windows)] +async fn windows_wait_for_file(path: &std::path::Path) { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + while !path.is_file() && tokio::time::Instant::now() < deadline { + tokio::time::sleep(std::time::Duration::from_millis(25)).await; + } + assert!(path.is_file(), "timed out waiting for {}", path.display()); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_host_spawn_off_executes_and_audits() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::Host as _; + + let temp = tempfile::tempdir().expect("workspace"); + let sentinel = temp.path().join("off-executed"); + let (mut state, sink) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Off); + + let result = state + .spawn(windows_touch_request(&sentinel)) + .expect("explicit off policy should execute a trusted process"); + assert_eq!(result.exit.exit_code, Some(0)); + assert_eq!( + std::fs::read(&sentinel).expect("probe sentinel"), + b"executed" + ); + + let records = sink.snapshot(); + assert_eq!(records.len(), 1, "spawn must report exactly once"); + assert!(matches!( + records[0], + (_, CapturedEvent::ProcessSpawn(_), CapturedOutcome::Allowed) + )); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_host_spawn_preserves_cwd_env_stdin_output_and_exit() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::Host as _; + + let temp = tempfile::tempdir().expect("workspace"); + let child_cwd = temp.path().join("child"); + std::fs::create_dir(&child_cwd).expect("child cwd"); + let (mut state, _) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Off); + + let mut request = windows_probe_request( + "host-stdio", + vec![ + process_host::EnvVar { + key: "ASTRID_EXPECTED_CWD".to_string(), + value: child_cwd.to_string_lossy().into_owned(), + }, + process_host::EnvVar { + key: "ASTRID_WINDOWS_EDGE".to_string(), + value: "unicode-\u{2603}-quote\"-slash\\".to_string(), + }, + ], + ); + request.cwd = Some("child".to_string()); + request.stdin = Some("host stdin \u{2603} \" \\".as_bytes().to_vec()); + + let result = state.spawn(request).expect("foreground stdio spawn"); + assert_eq!(result.exit.exit_code, Some(37)); + assert!(result.stdout.contains("host-stdout"), "{:?}", result.stdout); + assert!(result.stderr.contains("host-stderr"), "{:?}", result.stderr); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_host_rejects_case_colliding_environment_before_exec() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ErrorCode, Host as _}; + + let temp = tempfile::tempdir().expect("workspace"); + let sentinel = temp.path().join("collision-must-not-execute"); + let (mut state, _) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Off); + let mut request = windows_touch_request(&sentinel); + request.env.extend([ + process_host::EnvVar { + key: "ASTRID_EDGE".to_string(), + value: "first".to_string(), + }, + process_host::EnvVar { + key: "astrid_edge".to_string(), + value: "second".to_string(), + }, + ]); + + assert!(matches!(state.spawn(request), Err(ErrorCode::InvalidInput))); + assert!(!sentinel.exists(), "case-colliding env reached exec"); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_host_rejects_batch_files_before_exec() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ErrorCode, Host as _}; + + let temp = tempfile::tempdir().expect("workspace"); + let sentinel = temp.path().join("batch-must-not-execute"); + let batch = temp.path().join("probe.cmd"); + std::fs::write( + &batch, + format!("@echo executed>\"{}\"\r\n", sentinel.display()), + ) + .expect("batch fixture"); + let (mut state, _) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Off); + let mut request = windows_touch_request(&sentinel); + request.cmd = batch.to_string_lossy().into_owned(); + + assert!(matches!(state.spawn(request), Err(ErrorCode::InvalidInput))); + assert!(!sentinel.exists(), "batch file reached exec"); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_host_foreground_root_exit_cleans_descendants() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::Host as _; + + let temp = tempfile::tempdir().expect("workspace"); + let heartbeat = temp.path().join("foreground-heartbeat"); + let (mut state, _) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Off); + + let request = windows_probe_request( + "tree-root-exit", + vec![process_host::EnvVar { + key: "ASTRID_HEARTBEAT".to_string(), + value: heartbeat.to_string_lossy().into_owned(), + }], + ); + let result = state.spawn(request).expect("foreground spawn"); + assert_eq!(result.exit.exit_code, Some(0)); + tokio::time::sleep(std::time::Duration::from_millis(150)).await; + let stopped = std::fs::read_to_string(&heartbeat).expect("heartbeat after root exit"); + tokio::time::sleep(std::time::Duration::from_millis(250)).await; + assert_eq!( + stopped, + std::fs::read_to_string(&heartbeat).expect("final heartbeat"), + "foreground descendant survived root cleanup" + ); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_host_background_off_executes_and_waits() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::{Host as _, ProcessHandle}; + use wasmtime::component::Resource; + + let temp = tempfile::tempdir().expect("workspace"); + let sentinel = temp.path().join("background-executed"); + let (mut state, sink) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Off); + + let handle = state + .spawn_background(windows_touch_request(&sentinel)) + .expect("background spawn"); + let rep = handle.rep(); + let exit = ::wait( + &mut state, + Resource::::new_borrow(rep), + Some(10_000), + ) + .expect("wait for background probe"); + assert_eq!(exit.exit_code, Some(0)); + assert_eq!( + std::fs::read(&sentinel).expect("probe sentinel"), + b"executed" + ); + ::drop(&mut state, handle) + .expect("drop process handle"); + + let records = sink.snapshot(); + assert_eq!(records.len(), 1, "background spawn must audit once"); + assert!(matches!(records[0].2, CapturedOutcome::Allowed)); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_host_background_root_exit_cleans_descendants() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::{Host as _, ProcessHandle}; + use wasmtime::component::Resource; + + let temp = tempfile::tempdir().expect("workspace"); + let heartbeat = temp.path().join("background-heartbeat"); + let (mut state, _) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Off); + + let request = windows_probe_request( + "tree-root-exit", + vec![process_host::EnvVar { + key: "ASTRID_HEARTBEAT".to_string(), + value: heartbeat.to_string_lossy().into_owned(), + }], + ); + let handle = state.spawn_background(request).expect("background spawn"); + let exit = ::wait( + &mut state, + Resource::::new_borrow(handle.rep()), + Some(10_000), + ) + .expect("background root exit"); + assert_eq!(exit.exit_code, Some(0)); + + tokio::time::sleep(std::time::Duration::from_millis(150)).await; + let stopped = std::fs::read_to_string(&heartbeat).expect("heartbeat after root exit"); + tokio::time::sleep(std::time::Duration::from_millis(250)).await; + assert_eq!( + stopped, + std::fs::read_to_string(&heartbeat).expect("final heartbeat"), + "background descendant survived root cleanup" + ); + ::drop(&mut state, handle) + .expect("drop process handle"); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_live_handle_kill_reports_true_and_cleans_descendants() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::{Host as _, ProcessHandle}; + use wasmtime::component::Resource; + + let temp = tempfile::tempdir().expect("workspace"); + let heartbeat = temp.path().join("kill-heartbeat"); + let (mut state, _) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Off); + + let request = windows_probe_request( + "tree-root-immediate", + vec![process_host::EnvVar { + key: "ASTRID_HEARTBEAT".to_string(), + value: heartbeat.to_string_lossy().into_owned(), + }], + ); + let handle = state.spawn_background(request).expect("background spawn"); + windows_wait_for_file(&heartbeat).await; + let killed = ::kill( + &mut state, + Resource::::new_borrow(handle.rep()), + ) + .expect("kill live Job"); + assert!( + killed.killed, + "successful live Job termination must report true" + ); + + tokio::time::sleep(std::time::Duration::from_millis(150)).await; + let stopped = std::fs::read_to_string(&heartbeat).expect("heartbeat after kill"); + tokio::time::sleep(std::time::Duration::from_millis(250)).await; + assert_eq!( + stopped, + std::fs::read_to_string(&heartbeat).expect("final heartbeat"), + "live handle kill left a descendant running" + ); + ::drop(&mut state, handle) + .expect("drop process handle"); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_signal_audit_never_persists_guest_arguments() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ + Host as _, ProcessHandle, ProcessSignal, + }; + use wasmtime::component::Resource; + + const SECRET_ARG: &str = "guest-secret-argument-must-not-be-audited"; + let temp = tempfile::tempdir().expect("workspace"); + let heartbeat = temp.path().join("signal-audit-heartbeat"); + let (mut state, sink) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Off); + + let mut request = windows_probe_request( + "tree-root-immediate", + vec![process_host::EnvVar { + key: "ASTRID_HEARTBEAT".to_string(), + value: heartbeat.to_string_lossy().into_owned(), + }], + ); + request + .args + .extend(["--skip".to_string(), SECRET_ARG.to_string()]); + let executable = request.cmd.clone(); + let handle = state.spawn_background(request).expect("background spawn"); + windows_wait_for_file(&heartbeat).await; + + process_host::HostProcessHandle::signal( + &mut state, + Resource::::new_borrow(handle.rep()), + ProcessSignal::Term, + ) + .expect("terminate process tree"); + ::drop(&mut state, handle) + .expect("drop process handle"); + + let records = sink.snapshot(); + assert!(records.iter().all(|(_, event, _)| match event { + CapturedEvent::ProcessSignal(process, _) => { + process == &executable && !process.contains(SECRET_ARG) + }, + _ => true, + })); + assert!(records.iter().any(|(_, event, _)| matches!( + event, + CapturedEvent::ProcessSignal(process, signal) + if process == &executable && signal == "term" + ))); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_host_spawn_required_denies_before_exec_and_audits() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ErrorCode, Host as _}; + + let temp = tempfile::tempdir().expect("workspace"); + let sentinel = temp.path().join("must-not-execute"); + let (mut state, sink) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Required); + + let result = state.spawn(windows_touch_request(&sentinel)); + assert!(matches!(result, Err(ErrorCode::CapabilityDenied))); + assert!(!sentinel.exists(), "required policy executed the child"); + + let records = sink.snapshot(); + assert_eq!(records.len(), 1, "denied spawn must report exactly once"); + assert!(matches!( + &records[0], + ( + _, + CapturedEvent::ProcessSpawn(_), + CapturedOutcome::Denied(reason) + ) if reason.contains("sandbox") + )); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_host_background_required_denies_before_exec_and_audits() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ErrorCode, Host as _}; + + let temp = tempfile::tempdir().expect("workspace"); + let sentinel = temp.path().join("background-must-not-execute"); + let (mut state, sink) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Required); + + let result = state.spawn_background(windows_touch_request(&sentinel)); + assert!(matches!(result, Err(ErrorCode::CapabilityDenied))); + assert!(!sentinel.exists(), "required policy executed the child"); + + let records = sink.snapshot(); + assert_eq!(records.len(), 1, "denied spawn must report exactly once"); + assert!(matches!( + &records[0], + ( + _, + CapturedEvent::ProcessSpawn(_), + CapturedOutcome::Denied(reason) + ) if reason.contains("sandbox") + )); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_host_persistent_required_denies_before_exec_and_audits() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ErrorCode, Host as _}; + + let temp = tempfile::tempdir().expect("workspace"); + let sentinel = temp.path().join("persistent-must-not-execute"); + let (mut state, sink) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.capability_names.push("allow_persistent".to_string()); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Required); + authenticate_windows_process_state(&mut state); + + let result = state.spawn_persistent(windows_touch_request(&sentinel)); + assert!(matches!(result, Err(ErrorCode::CapabilityDenied))); + assert!(!sentinel.exists(), "required policy executed the child"); + + let records = sink.snapshot(); + assert_eq!(records.len(), 1, "denied spawn must report exactly once"); + assert!(matches!( + &records[0], + ( + _, + CapturedEvent::ProcessSpawn(_), + CapturedOutcome::Denied(reason) + ) if reason.contains("sandbox") + )); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_host_persistent_root_exit_cleans_descendants() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::Host as _; + + let temp = tempfile::tempdir().expect("workspace"); + let heartbeat = temp.path().join("persistent-heartbeat"); + let (mut state, _) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.capability_names.push("allow_persistent".to_string()); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Off); + authenticate_windows_process_state(&mut state); + + let request = windows_probe_request( + "tree-root-exit", + vec![process_host::EnvVar { + key: "ASTRID_HEARTBEAT".to_string(), + value: heartbeat.to_string_lossy().into_owned(), + }], + ); + let id = state.spawn_persistent(request).expect("persistent spawn"); + let exit = state + .wait(id.clone(), 10_000) + .expect("persistent root exit"); + assert_eq!(exit.exit_code, Some(0)); + + tokio::time::sleep(std::time::Duration::from_millis(150)).await; + let stopped = std::fs::read_to_string(&heartbeat).expect("heartbeat after root exit"); + tokio::time::sleep(std::time::Duration::from_millis(250)).await; + assert_eq!( + stopped, + std::fs::read_to_string(&heartbeat).expect("final heartbeat"), + "persistent descendant survived root cleanup" + ); + state.release_process(id).expect("release persistent entry"); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_host_persistent_sweep_cleans_live_descendants() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::Host as _; + + let temp = tempfile::tempdir().expect("workspace"); + let heartbeat = temp.path().join("sweep-heartbeat"); + let leaf_pid = temp.path().join("sweep-leaf-pid"); + let (mut state, _) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.capability_names.push("allow_persistent".to_string()); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Off); + authenticate_windows_process_state(&mut state); + + let mut request = windows_probe_request( + "tree-root", + vec![ + process_host::EnvVar { + key: "ASTRID_HEARTBEAT".to_string(), + value: heartbeat.to_string_lossy().into_owned(), + }, + process_host::EnvVar { + key: "ASTRID_LEAF_PID".to_string(), + value: leaf_pid.to_string_lossy().into_owned(), + }, + ], + ); + request.max_lifetime_ms = Some(50); + let _id = state.spawn_persistent(request).expect("persistent spawn"); + windows_wait_for_file(&heartbeat).await; + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + assert_eq!(state.persistent_processes.reap_sweep(), 1); + + tokio::time::sleep(std::time::Duration::from_millis(150)).await; + let stopped = std::fs::read_to_string(&heartbeat).expect("heartbeat after sweep"); + tokio::time::sleep(std::time::Duration::from_millis(250)).await; + assert_eq!( + stopped, + std::fs::read_to_string(&heartbeat).expect("final heartbeat"), + "swept persistent descendant survived cleanup" + ); +} + +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread")] +async fn windows_host_persistent_shutdown_cleans_live_descendants() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::Host as _; + + let temp = tempfile::tempdir().expect("workspace"); + let heartbeat = temp.path().join("shutdown-heartbeat"); + let leaf_pid = temp.path().join("shutdown-leaf-pid"); + let (mut state, _) = state_with_sink(tokio::runtime::Handle::current()); + state.workspace_root = temp.path().to_path_buf(); + state.security = Some(Arc::new(crate::security::AllowAllGate)); + state.capability_names.push("allow_persistent".to_string()); + state.process_sandbox_policy = Some(astrid_workspace::SandboxPolicy::Off); + authenticate_windows_process_state(&mut state); + + let request = windows_probe_request( + "tree-root", + vec![ + process_host::EnvVar { + key: "ASTRID_HEARTBEAT".to_string(), + value: heartbeat.to_string_lossy().into_owned(), + }, + process_host::EnvVar { + key: "ASTRID_LEAF_PID".to_string(), + value: leaf_pid.to_string_lossy().into_owned(), + }, + ], + ); + let _id = state.spawn_persistent(request).expect("persistent spawn"); + windows_wait_for_file(&heartbeat).await; + state.persistent_processes.shutdown(); + + tokio::time::sleep(std::time::Duration::from_millis(150)).await; + let stopped = std::fs::read_to_string(&heartbeat).expect("heartbeat after shutdown"); + tokio::time::sleep(std::time::Duration::from_millis(250)).await; + assert_eq!( + stopped, + std::fs::read_to_string(&heartbeat).expect("final heartbeat"), + "shutdown persistent descendant survived cleanup" + ); +} diff --git a/crates/astrid-capsule/src/engine/wasm/host/audit_sink_tests/windows_signal.rs b/crates/astrid-capsule/src/engine/wasm/host/audit_sink_tests/windows_signal.rs new file mode 100644 index 000000000..b8c4c475f --- /dev/null +++ b/crates/astrid-capsule/src/engine/wasm/host/audit_sink_tests/windows_signal.rs @@ -0,0 +1,72 @@ +//! Windows signal-audit denial regressions. + +use super::*; + +#[tokio::test] +async fn windows_closed_handle_unsupported_signal_is_denied_once() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ + ErrorCode, HostProcessHandle as _, ProcessHandle, ProcessSignal, + }; + use wasmtime::component::Resource; + + let (mut state, sink) = state_with_sink(tokio::runtime::Handle::current()); + for signal in [ + ProcessSignal::Hup, + ProcessSignal::Usr1, + ProcessSignal::Usr2, + ProcessSignal::Int, + ProcessSignal::Stop, + ProcessSignal::Cont, + ] { + let result = state.signal(Resource::::new_borrow(u32::MAX), signal); + assert!(matches!(result, Err(ErrorCode::CapabilityDenied))); + } + + let records = sink.snapshot(); + assert_eq!( + records.len(), + 6, + "each signal denial must report exactly once" + ); + assert!(records.iter().all(|(_, event, outcome)| matches!( + (event, outcome), + ( + CapturedEvent::ProcessSignal(process, _), + CapturedOutcome::Denied(_) + ) if process == "process-handle" + ))); +} + +#[tokio::test] +async fn windows_missing_persistent_unsupported_signal_is_denied_once() { + use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ + ErrorCode, Host as _, ProcessSignal, + }; + + let (mut state, sink) = state_with_sink(tokio::runtime::Handle::current()); + for signal in [ + ProcessSignal::Hup, + ProcessSignal::Usr1, + ProcessSignal::Usr2, + ProcessSignal::Int, + ProcessSignal::Stop, + ProcessSignal::Cont, + ] { + let result = state.signal("already-exited-or-foreign".to_string(), signal); + assert!(matches!(result, Err(ErrorCode::CapabilityDenied))); + } + + let records = sink.snapshot(); + assert_eq!( + records.len(), + 6, + "each signal denial must report exactly once" + ); + assert!(records.iter().all(|(_, event, outcome)| matches!( + (event, outcome), + ( + CapturedEvent::ProcessSignal(process, _), + CapturedOutcome::Denied(_) + ) if process.starts_with("persistent:") + ))); +} diff --git a/crates/astrid-capsule/src/engine/wasm/host/net/mod.rs b/crates/astrid-capsule/src/engine/wasm/host/net/mod.rs index 9f80c3b0f..4233757ee 100644 --- a/crates/astrid-capsule/src/engine/wasm/host/net/mod.rs +++ b/crates/astrid-capsule/src/engine/wasm/host/net/mod.rs @@ -251,10 +251,15 @@ where impl net::Host for HostState { fn bind_unix(&mut self) -> Result, ErrorCode> { - // Stable descriptor for the pre-provisioned CLI control socket — a - // Unix-domain listener has no host:port, so this names the bind on - // the audit chain. - let bind_addr = "unix:cli-socket"; + // Stable transport-neutral descriptor for the pre-provisioned local + // control listener. The frozen WIT method remains `bind-unix`, while + // the backend may be a Unix socket or Windows named pipe. + let bind_addr = "local:cli-control"; + if !astrid_core::local_transport::backend_available() { + let reason = "host-local listener backend is unavailable on this platform"; + record_net_denied(self, HostAuditEvent::NetBind { addr: bind_addr }, reason); + return Err(ErrorCode::CapabilityDenied); + } if let Some(ref gate) = self.security { let capsule_id = self.capsule_id.as_str().to_owned(); let gate = gate.clone(); diff --git a/crates/astrid-capsule/src/engine/wasm/host/process/audit.rs b/crates/astrid-capsule/src/engine/wasm/host/process/audit.rs index 8a9d22d94..08bf48dfc 100644 --- a/crates/astrid-capsule/src/engine/wasm/host/process/audit.rs +++ b/crates/astrid-capsule/src/engine/wasm/host/process/audit.rs @@ -8,6 +8,7 @@ //! onto the kernel's signed audit chain (the sensitive exec seam). use crate::audit_sink::{HostAuditEvent, HostAuditOutcome}; +use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ErrorCode, ProcessSignal}; use crate::engine::wasm::host_state::HostState; /// True for the sensitive exec seams — `spawn`, `spawn-background`, @@ -227,3 +228,46 @@ pub(crate) fn audit_process_id( ), } } + +pub(crate) fn process_signal_name(signal: ProcessSignal) -> &'static str { + match signal { + ProcessSignal::Term => "term", + ProcessSignal::Hup => "hup", + ProcessSignal::Usr1 => "usr1", + ProcessSignal::Usr2 => "usr2", + ProcessSignal::Int => "int", + ProcessSignal::Stop => "stop", + ProcessSignal::Cont => "cont", + } +} + +/// Record exactly one signed signal outcome plus the existing trace envelope. +pub(crate) fn audit_process_signal( + state: &HostState, + process: &str, + signal: ProcessSignal, + result: &Result<(), ErrorCode>, +) { + audit_process(state, "astrid:process/host.signal", process, result); + let Some(sink) = state.audit_sink.as_ref() else { + return; + }; + let reason; + let outcome = match result { + Ok(()) => HostAuditOutcome::Allowed, + Err(ErrorCode::CapabilityDenied | ErrorCode::NoSuchProcess) => { + reason = format!("{:?}", result.as_ref().expect_err("matched error")); + HostAuditOutcome::Denied(&reason) + }, + Err(error) => { + reason = format!("{error:?}"); + HostAuditOutcome::Failed(&reason) + }, + }; + sink.record_process_signal( + &state.effective_principal(), + process, + process_signal_name(signal), + outcome, + ); +} diff --git a/crates/astrid-capsule/src/engine/wasm/host/process/context.rs b/crates/astrid-capsule/src/engine/wasm/host/process/context.rs index 62869d09b..7edec0257 100644 --- a/crates/astrid-capsule/src/engine/wasm/host/process/context.rs +++ b/crates/astrid-capsule/src/engine/wasm/host/process/context.rs @@ -20,6 +20,17 @@ pub(super) fn prepare_spawn_context( state: &HostState, request: &SpawnRequest, ) -> Result { + #[cfg(windows)] + const PASSTHROUGH: &[&str] = &[ + "PATH", + "PATHEXT", + "SystemRoot", + "LANG", + "LC_ALL", + "LC_CTYPE", + "TZ", + ]; + #[cfg(not(windows))] const PASSTHROUGH: &[&str] = &["PATH", "LANG", "LC_ALL", "LC_CTYPE", "TZ"]; const MAX_ENV_VARS: usize = 256; const MAX_ENV_VALUE_BYTES: usize = 64 * 1024; @@ -31,7 +42,7 @@ pub(super) fn prepare_spawn_context( let mut env = BTreeMap::new(); for key in PASSTHROUGH { if let Ok(value) = std::env::var(key) { - env.insert((*key).to_string(), value); + env.insert(canonical_env_key(key), value); } } @@ -40,10 +51,11 @@ pub(super) fn prepare_spawn_context( let mut write_paths = Vec::new(); for item in &request.env { let key = item.key.as_str(); + let canonical_key = canonical_env_key(key); if !valid_env_key(key) || item.value.contains('\0') || item.value.len() > MAX_ENV_VALUE_BYTES - || !supplied.insert(key.to_string()) + || !supplied.insert(canonical_key.clone()) || reserved_process_env(key) { return Err(ErrorCode::InvalidInput); @@ -57,7 +69,7 @@ pub(super) fn prepare_spawn_context( } else { item.value.clone() }; - env.insert(key.to_string(), value); + env.insert(canonical_key, value); } let cwd = match request.cwd.as_deref() { @@ -129,9 +141,31 @@ pub(super) fn valid_env_key(key: &str) -> bool { && chars.all(|ch| ch == '_' || ch.is_ascii_alphanumeric()) } +pub(super) fn canonical_env_key(key: &str) -> String { + #[cfg(windows)] + { + key.to_ascii_uppercase() + } + #[cfg(not(windows))] + { + key.to_string() + } +} + /// Environment variables that can redirect command resolution, inject code /// before the approved executable starts, or impersonate a host-issued session. +#[cfg(windows)] +pub(super) fn reserved_process_env(key: &str) -> bool { + let key = key.to_ascii_uppercase(); + matches!(key.as_str(), "PATHEXT" | "COMSPEC" | "SYSTEMROOT") || reserved_process_env_exact(&key) +} + +#[cfg(not(windows))] pub(super) fn reserved_process_env(key: &str) -> bool { + reserved_process_env_exact(key) +} + +fn reserved_process_env_exact(key: &str) -> bool { key == "PATH" || key == "ASTRID_SESSION_TOKEN" || key == "BASH_ENV" @@ -192,6 +226,29 @@ mod tests { assert!(!reserved_process_env("ANTHROPIC_API_KEY")); } + #[cfg(unix)] + #[test] + fn unix_reserved_environment_keys_remain_case_sensitive() { + assert!(!reserved_process_env("Path")); + assert!(!reserved_process_env("ld_preload")); + assert!(!reserved_process_env("PATHEXT")); + assert!(!reserved_process_env("COMSPEC")); + assert!(!reserved_process_env("SYSTEMROOT")); + } + + #[cfg(windows)] + #[test] + fn windows_environment_keys_are_case_insensitive() { + assert_eq!(canonical_env_key("Path"), "PATH"); + assert_eq!(canonical_env_key("systemRoot"), "SYSTEMROOT"); + assert_eq!(canonical_env_key("Mixed_Case"), "MIXED_CASE"); + assert!(reserved_process_env("Path")); + assert!(reserved_process_env("PATHEXT")); + assert!(reserved_process_env("ComSpec")); + assert!(reserved_process_env("SystemRoot")); + assert!(reserved_process_env("ld_preload")); + } + #[test] fn workspace_cwd_rejects_absolute_and_parent_escapes() { let root = tempfile::tempdir().expect("root"); diff --git a/crates/astrid-capsule/src/engine/wasm/host/process/handle.rs b/crates/astrid-capsule/src/engine/wasm/host/process/handle.rs index d11ea6962..8eaa714ae 100644 --- a/crates/astrid-capsule/src/engine/wasm/host/process/handle.rs +++ b/crates/astrid-capsule/src/engine/wasm/host/process/handle.rs @@ -1,15 +1,13 @@ //! `HostProcessHandle` impl — methods on the `Resource`. //! -//! Live: `read-logs`, `wait`, `kill`, `os-pid`. +//! Live: `read-logs`, `wait`, `kill`, `signal`, `os-pid`. //! Stubbed (return Unknown / CapabilityDenied): `write-stdin`, -//! `close-stdin`, `signal`, `wait-with-output`, `subscribe-exit`, -//! `subscribe-logs`. These need: +//! `close-stdin`, `wait-with-output`, `subscribe-exit`, `subscribe-logs`. +//! These need: //! - stdin pipe storage in ManagedProcess (write-stdin / close-stdin) -//! - real signal mapping (signal — currently only the SIGKILL fast path -//! is exposed via kill()) //! - pollable wiring (subscribe-*) //! - wait_with_output requires re-architecting around the streaming -//! reader threads (the captured output is already drained piecemeal +//! reader tasks (the captured output is already drained piecemeal //! into the ring buffer; reassembling a one-shot final output across //! the wait/drain race is non-trivial) //! @@ -35,14 +33,21 @@ impl HostProcessHandle for HostState { // `tokio::process::Child::try_wait` is non-blocking and // returns the exit status if the child has exited. Same // semantics as std::process::Child::try_wait. + let mut exited = false; let (running, exit_code) = if let Some(child) = proc.child.as_mut() { match child.try_wait() { Ok(Some(status)) => { + #[cfg(windows)] + proc.tree.terminate(super::platform::Termination::Force)?; + exited = true; proc.child.take(); (false, status.code()) }, Ok(None) => (true, None), Err(_) => { + #[cfg(windows)] + proc.tree.terminate(super::platform::Termination::Force)?; + exited = true; proc.child.take(); (false, Some(-1)) }, @@ -50,6 +55,9 @@ impl HostProcessHandle for HostState { } else { (false, None) }; + if exited { + self.process_tracker.unregister_tree(&proc.tree); + } let stdout = drain_buffer(&proc.stdout_buf); let stderr = drain_buffer(&proc.stderr_buf); @@ -90,42 +98,30 @@ impl HostProcessHandle for HostState { self_: Resource, sig: ProcessSignal, ) -> Result<(), ErrorCode> { - #[cfg(unix)] - { - let proc = self - .resource_table - .get::(&Resource::new_borrow(self_.rep())) - .map_err(|_| ErrorCode::Closed)?; - // `tokio::process::Child::id()` returns `Option` — - // `None` once the child has been polled and reaped, which - // we treat as Closed here. The std variant returned `u32` - // unconditionally. - let pid = proc - .child - .as_ref() - .and_then(tokio::process::Child::id) - .ok_or(ErrorCode::Closed)?; - let nix_sig = match sig { - ProcessSignal::Term => nix::sys::signal::Signal::SIGTERM, - ProcessSignal::Hup => nix::sys::signal::Signal::SIGHUP, - ProcessSignal::Usr1 => nix::sys::signal::Signal::SIGUSR1, - ProcessSignal::Usr2 => nix::sys::signal::Signal::SIGUSR2, - ProcessSignal::Int => nix::sys::signal::Signal::SIGINT, - ProcessSignal::Stop => nix::sys::signal::Signal::SIGSTOP, - ProcessSignal::Cont => nix::sys::signal::Signal::SIGCONT, - }; - let raw = i32::try_from(pid).map_err(|_| ErrorCode::InvalidInput)?; - nix::sys::signal::kill(nix::unistd::Pid::from_raw(raw), nix_sig) - .map_err(|e| ErrorCode::Unknown(format!("kill({sig:?}): {e}")))?; - Ok(()) + if !super::platform::signal_supported(sig) { + let result = Err(ErrorCode::CapabilityDenied); + super::audit::audit_process_signal(self, "process-handle", sig, &result); + return result; } - #[cfg(not(unix))] + let (command, result) = match self + .resource_table + .get::(&Resource::new_borrow(self_.rep())) { - let _ = (self_, sig); - Err(ErrorCode::Unknown( - "ProcessHandle.signal: not supported on this platform".to_string(), - )) - } + Ok(proc) => { + // `tokio::process::Child::id()` returns `None` once the + // child has exited and been reaped. + let result = proc + .child + .as_ref() + .and_then(tokio::process::Child::id) + .ok_or(ErrorCode::Closed) + .and_then(|_| super::platform::signal_root_process(&proc.tree, sig)); + (proc.audit_descriptor.clone(), result) + }, + Err(_) => ("closed-process-handle".to_string(), Err(ErrorCode::Closed)), + }; + super::audit::audit_process_signal(self, &command, sig, &result); + result } fn kill(&mut self, self_: Resource) -> Result { @@ -134,12 +130,13 @@ impl HostProcessHandle for HostState { .get_mut::(&Resource::new_borrow(self_.rep())) .map_err(|_| ErrorCode::Closed)?; let (killed, exit_code) = match proc.child.take() { - Some(mut child) => { - let code = kill_and_reap(&mut child); - (true, code) + Some(mut child) => kill_and_reap(&mut child, &proc.tree)?, + None => { + proc.tree.terminate(super::platform::Termination::Force)?; + (false, None) }, - None => (false, None), }; + self.process_tracker.unregister_tree(&proc.tree); let stdout = drain_buffer(&proc.stdout_buf); let stderr = drain_buffer(&proc.stderr_buf); Ok(KillResult { @@ -176,7 +173,6 @@ impl HostProcessHandle for HostState { Some(c) => c, None => return Err(ErrorCode::Closed), }; - let result = crate::engine::wasm::host::util::bounded_block_on_cancellable( &rt, &sem, @@ -206,7 +202,10 @@ impl HostProcessHandle for HostState { // that the OS no longer knows about. let succeeded = matches!(result, Some(Ok(_))); if succeeded { + #[cfg(windows)] + proc.tree.terminate(super::platform::Termination::Force)?; proc.child.take(); + self.process_tracker.unregister_tree(&proc.tree); } match result { @@ -249,7 +248,7 @@ impl HostProcessHandle for HostState { fn subscribe_logs(&mut self, _self_: Resource) -> Resource { // Same pattern — guests poll, then `read-logs` drains whatever - // the reader thread has buffered (or returns empty if nothing + // the reader task has buffered (or returns empty if nothing // is available yet, which is honest non-blocking semantics). super::super::stubs::always_ready_pollable(&mut self.resource_table) } @@ -263,9 +262,7 @@ impl HostProcessHandle for HostState { .resource_table .delete::(Resource::new_own(rep.rep())) { - if let Some(pid) = managed.child.as_ref().and_then(tokio::process::Child::id) { - self.process_tracker.unregister(pid); - } + self.process_tracker.unregister_tree(&managed.tree); self.process_count_total = self.process_count_total.saturating_sub(1); if let Some(count) = self.process_count_by_principal.get_mut(&managed.creator) { *count = count.saturating_sub(1); diff --git a/crates/astrid-capsule/src/engine/wasm/host/process/managed.rs b/crates/astrid-capsule/src/engine/wasm/host/process/managed.rs index 8c333b0be..f3836ec15 100644 --- a/crates/astrid-capsule/src/engine/wasm/host/process/managed.rs +++ b/crates/astrid-capsule/src/engine/wasm/host/process/managed.rs @@ -2,12 +2,31 @@ //! drains stdout/stderr into bounded ring buffers and reaps the child //! on Drop. -use std::collections::VecDeque; +use std::collections::{BTreeMap, VecDeque}; use std::process::{Command, Stdio}; use std::sync::{Arc, Mutex}; use astrid_workspace::SandboxCommand; use tokio::io::AsyncReadExt; +use tokio::sync::Semaphore; +use tokio_util::sync::CancellationToken; + +use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ErrorCode, SpawnRequest}; + +#[derive(Debug)] +pub(super) enum PrepareCommandError { + Invalid, + SandboxDenied(String), +} + +#[derive(Clone, Copy)] +pub(super) struct SandboxInputs<'a> { + pub(super) workspace_root: &'a std::path::Path, + pub(super) injections: &'a [astrid_workspace::RoInjection], + pub(super) inject_env: &'a [(String, String)], + pub(super) extra_masks: &'a [std::path::PathBuf], + pub(super) policy: astrid_workspace::SandboxPolicy, +} /// Maximum bytes buffered per stream (stdout or stderr). pub(super) const MAX_BUFFER_BYTES: usize = 1024 * 1024; @@ -23,14 +42,13 @@ pub(super) const MAX_BUFFER_BYTES: usize = 1024 * 1024; /// strands the handle if the wait times out (Gemini #752 finding). pub struct ManagedProcess { pub(super) child: Option, + pub(super) tree: Arc, pub(super) stdout_buf: Arc>>, pub(super) stderr_buf: Arc>>, - /// The full command string (cmd + args), kept for diagnostic / - /// audit purposes and surfaced when an operator queries spawn - /// telemetry. Not read by the host functions themselves yet — - /// `#[allow(dead_code)]` until the diagnostics surface lands. - #[allow(dead_code)] - pub(super) command: String, + /// Bounded audit descriptor for lifecycle operations. This is the + /// executable only: arguments may contain secrets and must never be + /// persisted by the signed signal-audit path. + pub(super) audit_descriptor: String, pub(super) creator: astrid_core::principal::PrincipalId, /// Cleanup guard for any read-only file injections wired into this child's /// sandbox. Lives as long as the handle: on Linux it keeps the ro-bind @@ -42,34 +60,197 @@ pub struct ManagedProcess { pub(super) injection_guard: Option, } -/// Synchronously kill a child process group on Unix and start the kill -/// on the child itself. Returns the exit code if reaping was possible -/// in the brief window before this call returns. +/// Foreground child owner that terminates the whole process tree before the +/// Tokio child handle drops when the wait future is cancelled. +pub(super) struct ForegroundProcess { + child: tokio::process::Child, + tree: Arc, + armed: bool, +} + +impl ForegroundProcess { + pub(super) fn new(mut child: tokio::process::Child) -> Result { + let tree = match super::platform::ProcessTree::attach(&child) { + Ok(tree) => tree, + Err(error) => { + let _ = child.start_kill(); + return Err(error); + }, + }; + Ok(Self { + child, + tree, + armed: true, + }) + } + + pub(super) fn pid(&self) -> u32 { + self.tree.pid() + } + + pub(super) fn tree(&self) -> Arc { + Arc::clone(&self.tree) + } + + pub(super) async fn write_stdin_prelude(&mut self, prelude: &[u8]) -> std::io::Result<()> { + use tokio::io::AsyncWriteExt as _; + + let Some(mut stdin) = self.child.stdin.take() else { + return if prelude.is_empty() { + Ok(()) + } else { + Err(std::io::Error::other("child stdin was not piped")) + }; + }; + stdin.write_all(prelude).await?; + stdin.shutdown().await + } + + /// Wait for exit while draining both pipes concurrently. + /// + /// This borrows the child instead of moving it into + /// `Child::wait_with_output`, so cancellation drops `Self` first and its + /// `Drop` implementation can terminate descendants before Tokio drops the + /// root handle. + pub(super) async fn wait_with_output(mut self) -> std::io::Result { + let mut stdout = self + .child + .stdout + .take() + .ok_or_else(|| std::io::Error::other("child stdout was not piped"))?; + let mut stderr = self + .child + .stderr + .take() + .ok_or_else(|| std::io::Error::other("child stderr was not piped"))?; + let mut stdout_bytes = Vec::new(); + let mut stderr_bytes = Vec::new(); + + #[cfg(windows)] + let wait_for_root = { + let tree = Arc::clone(&self.tree); + let child = &mut self.child; + async move { + let status = child.wait().await?; + // Descendants may inherit the root's stdout/stderr handles. + // Terminate the Job as soon as the root exits, before waiting + // for EOF, or pipe draining can wait forever on a descendant. + tree.terminate(super::platform::Termination::Force) + .map_err(|error| std::io::Error::other(format!("{error:?}")))?; + Ok::<_, std::io::Error>(status) + } + }; + #[cfg(not(windows))] + let wait_for_root = self.child.wait(); + + let (status, _, _) = tokio::try_join!( + wait_for_root, + stdout.read_to_end(&mut stdout_bytes), + stderr.read_to_end(&mut stderr_bytes), + )?; + self.armed = false; + Ok(std::process::Output { + status, + stdout: stdout_bytes, + stderr: stderr_bytes, + }) + } +} + +impl Drop for ForegroundProcess { + fn drop(&mut self) { + if !self.armed { + return; + } + #[cfg(windows)] + { + let _ = self.tree.terminate(super::platform::Termination::Force); + let _ = self.child.start_kill(); + } + #[cfg(not(windows))] + { + let _ = kill_and_reap(&mut self.child, &self.tree); + } + let _ = self.child.try_wait(); + } +} + +/// Synchronously apply the platform's established kill contract. /// -/// `tokio::process::Child::kill()` is async, but `Drop` and the kill -/// host-fn need a sync path. `start_kill` sends SIGKILL and returns -/// immediately; the kernel's `kill_on_drop(true)` flag on the spawning -/// Command ensures the tokio runtime reaps the zombie when the Child -/// is dropped. -pub(super) fn kill_and_reap(child: &mut tokio::process::Child) -> Option { +/// Unix retains its process-group SIGKILL plus root `start_kill` behavior. +/// Windows terminates the owned Job Object and derives `killed` from that +/// operation without racing a second root-only kill. +pub(super) fn kill_and_reap( + child: &mut tokio::process::Child, + tree: &super::platform::ProcessTree, +) -> Result<(bool, Option), ErrorCode> { #[cfg(unix)] { + let _ = tree; + // Preserve the established Unix contract: kill the spawned process + // group best-effort, then request root termination. Handle.kill reports + // true whenever it still owned a Child slot, exactly as before. if let Some(raw_pid) = child.id() { let pid = nix::unistd::Pid::from_raw(i32::try_from(raw_pid).unwrap_or(i32::MAX)); let _ = nix::sys::signal::killpg(pid, nix::sys::signal::Signal::SIGKILL); } + let _ = child.start_kill(); + Ok(( + true, + child + .try_wait() + .ok() + .flatten() + .and_then(|status| status.code()), + )) + } + #[cfg(windows)] + { + if let Some(status) = child.try_wait().ok().flatten() { + // Root already exited: terminate any descendants retained by the + // Job, but do not claim that this call killed the root process. + tree.terminate(super::platform::Termination::Force)?; + return Ok((false, status.code())); + } + // TerminateJobObject is the owned tree operation. Its success—not a + // racy second start_kill on the root—establishes killed=true. + tree.terminate(super::platform::Termination::Force)?; + Ok(( + true, + child + .try_wait() + .ok() + .flatten() + .and_then(|status| status.code()), + )) + } + #[cfg(not(any(unix, windows)))] + { + let _ = tree; + child + .start_kill() + .map_err(|error| ErrorCode::Unknown(format!("terminate root process: {error}")))?; + Ok(( + true, + child + .try_wait() + .ok() + .flatten() + .and_then(|status| status.code()), + )) } - let _ = child.start_kill(); - // Best-effort sync drain of the exit status. `try_wait` is - // non-blocking; if the SIGKILL hasn't been observed by the OS yet - // this returns Ok(None) and we surface `None` for the exit code. - child.try_wait().ok().flatten().and_then(|s| s.code()) } impl Drop for ManagedProcess { fn drop(&mut self) { - if let Some(mut child) = self.child.take() { - kill_and_reap(&mut child); + if let Some(mut child) = self.child.take() + && let Err(error) = kill_and_reap(&mut child, &self.tree) + { + tracing::warn!( + pid = self.tree.pid(), + ?error, + "failed to terminate managed process tree on drop" + ); } } } @@ -124,13 +305,12 @@ pub(super) fn spawn_reader_task( pub(super) fn prepare_sandboxed_command( cmd: &str, args: &[String], - workspace_root: &std::path::Path, context: &super::context::PreparedSpawnContext, - injections: &[astrid_workspace::RoInjection], - inject_env: &[(String, String)], - extra_masks: &[std::path::PathBuf], -) -> Result { - let mut inner_cmd = Command::new(cmd); + sandbox: SandboxInputs<'_>, +) -> Result { + let program = resolve_program(cmd, &context.cwd, &context.env) + .map_err(|_| PrepareCommandError::Invalid)?; + let mut inner_cmd = Command::new(program); let str_args: Vec<&str> = args.iter().map(String::as_str).collect(); inner_cmd.args(&str_args); // `cwd` has already been resolved and boundary-checked by the process host. @@ -138,23 +318,238 @@ pub(super) fn prepare_sandboxed_command( // `home://` request can instead target the invoking principal's home. inner_cmd.current_dir(&context.cwd); inner_cmd.env_clear(); + let mut child_env = BTreeMap::new(); for (key, value) in &context.env { - inner_cmd.env(key, value); + child_env.insert(key.clone(), value.clone()); } - for (k, v) in inject_env { - inner_cmd.env(k, v); + for (k, v) in sandbox.inject_env { + let key = super::context::canonical_env_key(k); + if child_env.insert(key, v.clone()).is_some() { + return Err(PrepareCommandError::Invalid); + } + } + for (key, value) in child_env { + inner_cmd.env(key, value); } - SandboxCommand::wrap_with_process_paths( + SandboxCommand::wrap_with_process_paths_and_policy( &inner_cmd, - workspace_root, - injections, - extra_masks, + sandbox.workspace_root, + sandbox.injections, + sandbox.extra_masks, &context.read_paths, &context.write_paths, true, + sandbox.policy, ) - .map_err(|e| format!("failed to wrap command in sandbox: {e}")) + .map_err(|error| { + let message = format!("failed to wrap command in sandbox: {error}"); + if error.kind() == std::io::ErrorKind::PermissionDenied { + PrepareCommandError::SandboxDenied(message) + } else { + let _ = message; + PrepareCommandError::Invalid + } + }) +} + +/// Build the sandboxed child for a persistent spawn. +pub(super) fn build_persistent_child( + request: &SpawnRequest, + context: &super::context::PreparedSpawnContext, + want_stdin: bool, + sandbox: SandboxInputs<'_>, +) -> Result<(tokio::process::Child, Arc), ErrorCode> { + let mut sandboxed = prepare_sandboxed_command(&request.cmd, &request.args, context, sandbox) + .map_err(|error| match error { + PrepareCommandError::SandboxDenied(_) => ErrorCode::CapabilityDenied, + PrepareCommandError::Invalid => ErrorCode::InvalidInput, + })?; + configure_piped(&mut sandboxed); + sandboxed.stdin(if want_stdin { + Stdio::piped() + } else { + Stdio::null() + }); + let mut command = tokio::process::Command::from(sandboxed); + command.kill_on_drop(true); + let mut child = command + .spawn() + .map_err(|error| ErrorCode::Unknown(format!("spawn-persistent failed: {error}")))?; + let tree = super::platform::ProcessTree::attach(&child).map_err(|error| { + let _ = child.start_kill(); + ErrorCode::Unknown(format!("spawn-persistent ownership failed: {error}")) + })?; + Ok((child, tree)) +} + +/// Deliver and close a background spawn's optional stdin prelude. +pub(super) fn write_background_stdin( + runtime: &tokio::runtime::Handle, + semaphore: &Semaphore, + cancel: &CancellationToken, + child: &mut tokio::process::Child, + prelude: Option>, +) -> Result<(), ErrorCode> { + let Some(prelude) = prelude else { + return Ok(()); + }; + let mut stdin = child.stdin.take().ok_or_else(|| { + ErrorCode::Unknown("spawn-background: child stdin was not piped".to_string()) + })?; + crate::engine::wasm::host::util::bounded_block_on_cancellable( + runtime, + semaphore, + cancel, + async move { + use tokio::io::AsyncWriteExt as _; + stdin.write_all(&prelude).await + }, + ) + .ok_or(ErrorCode::Cancelled)? + .map_err(|error| { + ErrorCode::Unknown(format!( + "spawn-background: stdin prelude write failed: {error}" + )) + }) +} + +#[cfg(not(windows))] +fn resolve_program( + command: &str, + _cwd: &std::path::Path, + _env: &[(String, String)], +) -> Result { + if command.contains('\0') { + return Err("process command contains a null byte".to_string()); + } + Ok(command.into()) +} + +/// Resolve a Windows program once before `CreateProcessW`. +/// +/// Bare program names search only absolute PATH entries. Explicit relative +/// paths resolve beneath the already-authorized child CWD. Batch files are +/// refused because Windows can only execute them by inserting `cmd.exe`. +#[cfg(windows)] +fn resolve_program( + command: &str, + cwd: &std::path::Path, + env: &[(String, String)], +) -> Result { + use std::ffi::OsStr; + use std::path::{Component, Path}; + + if command.is_empty() || command.contains('\0') { + return Err("process command is empty or contains a null byte".to_string()); + } + let requested = Path::new(command); + let has_path = requested.is_absolute() + || requested.components().count() != 1 + || requested + .components() + .any(|component| !matches!(component, Component::Normal(_))); + + let candidate = if has_path { + let candidate = if requested.is_absolute() { + requested.to_path_buf() + } else { + cwd.join(requested) + }; + resolve_windows_candidate(candidate)? + } else { + let path = env_value(env, "PATH") + .ok_or_else(|| "PATH is not set while resolving process command".to_string())?; + let extensions = windows_executable_extensions(env); + let mut found = None; + for directory in std::env::split_paths(&path).filter(|path| path.is_absolute()) { + for candidate in windows_candidates(&directory, requested, &extensions) { + if candidate.is_file() { + found = Some(resolve_windows_candidate(candidate)?); + break; + } + } + if found.is_some() { + break; + } + } + found.ok_or_else(|| { + format!("process command not found in absolute PATH entries: {command}") + })? + }; + + let extension = candidate + .extension() + .and_then(OsStr::to_str) + .unwrap_or_default(); + if extension.eq_ignore_ascii_case("cmd") || extension.eq_ignore_ascii_case("bat") { + return Err("batch files require an explicit cmd.exe invocation".to_string()); + } + Ok(candidate.into_os_string()) +} + +#[cfg(windows)] +fn resolve_windows_candidate(path: std::path::PathBuf) -> Result { + let canonical = path.canonicalize().map_err(|error| { + format!( + "cannot resolve process executable {}: {error}", + path.display() + ) + })?; + if !canonical.is_file() { + return Err(format!( + "process executable is not a file: {}", + canonical.display() + )); + } + Ok(canonical) +} + +#[cfg(windows)] +fn env_value<'a>(env: &'a [(String, String)], key: &str) -> Option<&'a std::ffi::OsStr> { + env.iter() + .find(|(candidate, _)| candidate.eq_ignore_ascii_case(key)) + .map(|(_, value)| std::ffi::OsStr::new(value)) +} + +#[cfg(windows)] +fn windows_executable_extensions(env: &[(String, String)]) -> Vec { + env_value(env, "PATHEXT") + .map(|value| { + value + .to_string_lossy() + .split(';') + .filter(|extension| !extension.is_empty()) + .map(|extension| { + if extension.starts_with('.') { + extension.to_string() + } else { + format!(".{extension}") + } + }) + .collect() + }) + .filter(|extensions: &Vec| !extensions.is_empty()) + .unwrap_or_else(|| vec![".COM".into(), ".EXE".into()]) +} + +#[cfg(windows)] +fn windows_candidates( + directory: &std::path::Path, + requested: &std::path::Path, + extensions: &[String], +) -> Vec { + if requested.extension().is_some() { + return vec![directory.join(requested)]; + } + extensions + .iter() + .map(|extension| { + let mut file_name = requested.as_os_str().to_os_string(); + file_name.push(extension); + directory.join(file_name) + }) + .collect() } /// Wire a freshly-spawned child's stdout / stderr into tokio reader @@ -173,11 +568,322 @@ pub(super) fn attach_pipes(managed: &mut ManagedProcess, runtime: &tokio::runtim /// Configure stdio + process-group on a std command. Caller converts /// to a `tokio::process::Command` afterwards. pub(super) fn configure_piped(sandboxed_cmd: &mut Command) { - #[cfg(unix)] - { - use std::os::unix::process::CommandExt as _; - sandboxed_cmd.process_group(0); - } + super::platform::configure_process_group(sandboxed_cmd); sandboxed_cmd.stdout(Stdio::piped()); sandboxed_cmd.stderr(Stdio::piped()); } + +#[cfg(test)] +mod tests { + use super::*; + + #[cfg(windows)] + #[test] + fn windows_program_resolution_is_explicit_and_rejects_batch_files() { + let temp = tempfile::tempdir().expect("temp"); + let executable = temp.path().join("astrid-probe.exe"); + std::fs::copy( + std::env::current_exe().expect("current test executable"), + &executable, + ) + .expect("copy probe executable"); + let batch = temp.path().join("astrid-probe.cmd"); + std::fs::write(&batch, b"@exit /b 0\r\n").expect("write batch file"); + let env = vec![ + ( + "Path".to_string(), + temp.path().to_string_lossy().into_owned(), + ), + ("PathExt".to_string(), ".EXE;.CMD".to_string()), + ]; + + assert_eq!( + resolve_program("astrid-probe", temp.path(), &env).expect("resolve bare executable"), + executable + .canonicalize() + .expect("canonical executable") + .into_os_string() + ); + assert!(resolve_program(batch.to_string_lossy().as_ref(), temp.path(), &env).is_err()); + } + + #[cfg(windows)] + #[test] + fn windows_prepared_command_preserves_empty_unicode_and_quoted_arguments() { + let executable = std::env::current_exe().expect("current test executable"); + let workspace = tempfile::tempdir().expect("workspace"); + let context = super::super::context::PreparedSpawnContext { + cwd: workspace.path().to_path_buf(), + env: Vec::new(), + read_paths: Vec::new(), + write_paths: Vec::new(), + }; + let args = vec![ + String::new(), + "snow-\u{2603}".to_string(), + "quote\"inside".to_string(), + "trailing\\".to_string(), + ]; + let command = prepare_sandboxed_command( + executable.to_string_lossy().as_ref(), + &args, + &context, + SandboxInputs { + workspace_root: workspace.path(), + injections: &[], + inject_env: &[], + extra_masks: &[], + policy: astrid_workspace::SandboxPolicy::Off, + }, + ) + .expect("prepare native command"); + + assert_eq!( + command + .get_args() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect::>(), + args + ); + } + + #[cfg(windows)] + #[tokio::test(flavor = "multi_thread")] + async fn windows_kill_reports_already_exited_process_as_not_killed() { + let mut command = Command::new(std::env::current_exe().expect("current test executable")); + command + .arg("astrid_test_filter_that_does_not_exist") + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + super::super::platform::configure_process_group(&mut command); + let mut command = tokio::process::Command::from(command); + command.kill_on_drop(true); + let mut child = command.spawn().expect("spawn short-lived child"); + let tree = + super::super::platform::ProcessTree::attach(&child).expect("attach process tree"); + let status = child.wait().await.expect("child exit"); + + let (killed, exit_code) = kill_and_reap(&mut child, &tree).expect("idempotent cleanup"); + assert!(!killed, "an already-exited process was reported as killed"); + assert_eq!(exit_code, status.code()); + } + + #[cfg(windows)] + #[tokio::test(flavor = "multi_thread")] + async fn windows_kill_propagates_process_tree_termination_failure() { + let temp = tempfile::tempdir().expect("temp"); + let mut command = Command::new(std::env::current_exe().expect("current test executable")); + command + .arg("windows_process_probe_child") + .arg("--nocapture") + .env("ASTRID_WINDOWS_PROCESS_PROBE", "tree-leaf") + .env("ASTRID_HEARTBEAT", temp.path().join("heartbeat")) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + super::super::platform::configure_process_group(&mut command); + let mut command = tokio::process::Command::from(command); + command.kill_on_drop(true); + let mut child = command.spawn().expect("spawn long-lived child"); + let tree = + super::super::platform::ProcessTree::attach(&child).expect("attach process tree"); + + tree.inject_termination_failure(true); + assert!(kill_and_reap(&mut child, &tree).is_err()); + tree.inject_termination_failure(false); + let _ = kill_and_reap(&mut child, &tree).expect("cleanup process tree"); + } + + #[cfg(windows)] + #[tokio::test(flavor = "multi_thread")] + async fn windows_foreground_root_exit_closes_inherited_descendant_pipes() { + let temp = tempfile::tempdir().expect("temp"); + let mut command = Command::new(std::env::current_exe().expect("current test executable")); + command + .arg("windows_process_probe_child") + .arg("--nocapture") + .env( + "ASTRID_WINDOWS_PROCESS_PROBE", + "tree-root-exit-inherit-stdio", + ) + .env("ASTRID_HEARTBEAT", temp.path().join("heartbeat")) + .env("ASTRID_LEAF_PID", temp.path().join("leaf-pid")) + .stdin(Stdio::null()); + configure_piped(&mut command); + let mut command = tokio::process::Command::from(command); + command.kill_on_drop(true); + let child = command.spawn().expect("spawn inherited-pipe root"); + let process = ForegroundProcess::new(child).expect("own suspended process"); + + let output = tokio::time::timeout( + std::time::Duration::from_secs(10), + process.wait_with_output(), + ) + .await + .expect("foreground output deadlocked on inherited descendant pipes") + .expect("wait for foreground output"); + assert_eq!(output.status.code(), Some(0)); + assert!( + temp.path().join("leaf-pid").is_file(), + "root did not create inherited-pipe descendant" + ); + } + + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread")] + async fn foreground_stdio_and_exit_are_preserved() { + let mut command = Command::new("sh"); + command + .args([ + "-c", + "IFS= read -r line; printf 'out:%s' \"$line\"; printf 'err:%s' \"$line\" >&2; exit 23", + ]) + .stdin(Stdio::piped()); + configure_piped(&mut command); + let mut command = tokio::process::Command::from(command); + command.kill_on_drop(true); + let child = command.spawn().expect("spawn child"); + let mut process = ForegroundProcess::new(child).expect("foreground owner"); + process + .write_stdin_prelude(b"hello world\n") + .await + .expect("write stdin"); + let output = process.wait_with_output().await.expect("wait with output"); + + assert_eq!(output.status.code(), Some(23)); + assert_eq!(output.stdout, b"out:hello world"); + assert_eq!(output.stderr, b"err:hello world"); + } + + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread")] + async fn unix_successful_foreground_wait_does_not_force_descendant_group() { + let temp = tempfile::tempdir().expect("temp"); + let pid_file = temp.path().join("descendant-pid"); + let mut command = Command::new("sh"); + command + .args([ + "-c", + "sleep 60 /dev/null 2>&1 & printf '%s' \"$!\" > \"$1\"; exit 0", + "astrid-test", + ]) + .arg(&pid_file) + .stdin(Stdio::null()); + configure_piped(&mut command); + let mut command = tokio::process::Command::from(command); + command.kill_on_drop(true); + let child = command.spawn().expect("spawn process tree"); + let process = ForegroundProcess::new(child).expect("foreground owner"); + let output = process.wait_with_output().await.expect("root wait"); + assert_eq!(output.status.code(), Some(0)); + + let descendant: i32 = std::fs::read_to_string(&pid_file) + .expect("descendant pid") + .parse() + .expect("decimal pid"); + let descendant = nix::unistd::Pid::from_raw(descendant); + assert!( + nix::sys::signal::kill(descendant, None).is_ok(), + "successful Unix root wait unexpectedly killed its process group" + ); + let _ = nix::sys::signal::kill(descendant, nix::sys::signal::Signal::SIGKILL); + } + + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread")] + async fn unix_public_signal_targets_root_not_process_group() { + use std::time::{Duration, Instant}; + + let temp = tempfile::tempdir().expect("temp"); + let pid_file = temp.path().join("descendant-pid"); + let mut command = Command::new("sh"); + command + .args([ + "-c", + "trap 'exit 0' TERM; sleep 60 & printf '%s' \"$!\" > \"$1\"; wait", + "astrid-test", + ]) + .arg(&pid_file) + .stdin(Stdio::null()); + configure_piped(&mut command); + let mut command = tokio::process::Command::from(command); + command.kill_on_drop(true); + let mut child = command.spawn().expect("spawn process tree"); + let tree = + super::super::platform::ProcessTree::attach(&child).expect("attach process identity"); + let deadline = Instant::now() + Duration::from_secs(10); + while !pid_file.is_file() && Instant::now() < deadline { + tokio::time::sleep(Duration::from_millis(10)).await; + } + let descendant: i32 = std::fs::read_to_string(&pid_file) + .expect("descendant pid") + .parse() + .expect("decimal pid"); + let descendant = nix::unistd::Pid::from_raw(descendant); + + super::super::platform::signal_root_process( + &tree, + crate::engine::wasm::bindings::astrid::process1_1_0::host::ProcessSignal::Term, + ) + .expect("signal root"); + if tokio::time::timeout(Duration::from_secs(5), child.wait()) + .await + .is_err() + { + let _ = tree.terminate(super::super::platform::Termination::Force); + panic!("Unix root did not exit after TERM"); + } + assert!( + nix::sys::signal::kill(descendant, None).is_ok(), + "public Unix signal unexpectedly targeted the process group" + ); + let _ = nix::sys::signal::kill(descendant, nix::sys::signal::Signal::SIGKILL); + } + + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread")] + async fn cancelling_foreground_wait_terminates_descendant_group() { + use std::time::{Duration, Instant}; + + let temp = tempfile::tempdir().expect("temp"); + let pid_file = temp.path().join("descendant-pid"); + let mut command = Command::new("sh"); + command + .args([ + "-c", + "sleep 60 & child=$!; printf '%s' \"$child\" > \"$1\"; wait", + "astrid-test", + ]) + .arg(&pid_file) + .stdin(Stdio::null()); + configure_piped(&mut command); + let mut command = tokio::process::Command::from(command); + command.kill_on_drop(true); + let child = command.spawn().expect("spawn process tree"); + let process = ForegroundProcess::new(child).expect("foreground owner"); + let wait = tokio::spawn(process.wait_with_output()); + + let deadline = Instant::now() + Duration::from_secs(10); + while !pid_file.is_file() && Instant::now() < deadline { + tokio::time::sleep(Duration::from_millis(25)).await; + } + let descendant_pid: i32 = std::fs::read_to_string(&pid_file) + .expect("descendant pid file") + .parse() + .expect("decimal descendant pid"); + + wait.abort(); + let _ = wait.await; + + let descendant = nix::unistd::Pid::from_raw(descendant_pid); + let deadline = Instant::now() + Duration::from_secs(10); + while nix::sys::signal::kill(descendant, None).is_ok() && Instant::now() < deadline { + tokio::time::sleep(Duration::from_millis(25)).await; + } + assert!( + nix::sys::signal::kill(descendant, None).is_err(), + "descendant remained alive after foreground cancellation" + ); + } +} diff --git a/crates/astrid-capsule/src/engine/wasm/host/process/mod.rs b/crates/astrid-capsule/src/engine/wasm/host/process/mod.rs index 634985dc9..186b059bf 100644 --- a/crates/astrid-capsule/src/engine/wasm/host/process/mod.rs +++ b/crates/astrid-capsule/src/engine/wasm/host/process/mod.rs @@ -1,28 +1,6 @@ -//! `astrid:process@1.1.0` host implementation (the `@1.0.0` shims live in -//! `compat.rs`). -//! -//! Both frozen contract versions are served off one implementation. This -//! module implements the `@1.1.0` `Host` / `HostProcessHandle` traits — the -//! SUPERSET, carrying the per-spawn read-only `file-injection` surface. The -//! `@1.0.0` traits are thin delegating shims in `compat.rs` that spawn with an -//! empty injection list; see that module for the version-bridging rationale. -//! -//! Desktop-only package (the WIT header explicitly notes hermit-rs -//! unikernel targets do not provide it). The kernel here: -//! -//! - `spawn` — synchronous sandboxed exec with stdout/stderr capture. -//! Full impl; ported from the legacy path. -//! - `spawn-background` — sandboxed exec returning -//! `Resource`. Stdout/stderr drain into 1 MiB-per-stream -//! ring buffers via reader threads. Tracked by the per-capsule cap -//! plus the per-principal profile sub-budget. -//! - `ProcessHandle.{read-logs, wait, kill, os-pid}` — full impls. -//! - `ProcessHandle.{write-stdin, close-stdin, signal, wait-with-output, -//! subscribe-exit, subscribe-logs}` — stubbed pending dedicated -//! follow-ups (stdin pipe storage + pollable wiring). -//! -//! `HostState.background_processes` is gone — the wasmtime resource -//! table is the canonical storage for `ManagedProcess`. +//! Shared `astrid:process@1.1.0` host implementation. The frozen `@1.0.0` +//! surface delegates through `compat.rs`; the resource table remains the +//! canonical storage for ephemeral process handles. mod audit; mod compat; @@ -31,28 +9,32 @@ mod handle; mod inject; mod managed; mod persistent; +mod platform; +mod support; mod tracker; use std::collections::VecDeque; -use std::process::Stdio; use std::sync::{Arc, Mutex}; -use tokio::process::Command as TokioCommand; use tracing::warn; use wasmtime::component::Resource; use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ - self as process, EnvVar, ErrorCode, ExitInfo, LogChunk, LogCursor, LogStream, ProcessHandle, + self as process, ErrorCode, ExitInfo, LogChunk, LogCursor, LogStream, ProcessHandle, ProcessInfo, ProcessResult, ProcessSignal, ReadLogsResult, SpawnRequest, }; use crate::engine::wasm::host::util; use crate::engine::wasm::host_state::HostState; -use context::{PreparedSpawnContext, prepare_spawn_context}; -use managed::{ManagedProcess, attach_pipes, configure_piped, prepare_sandboxed_command}; +use context::prepare_spawn_context; +use managed::{ + ForegroundProcess, ManagedProcess, PrepareCommandError, SandboxInputs, attach_pipes, + build_persistent_child, configure_piped, prepare_sandboxed_command, write_background_stdin, +}; +use support::{authenticated_principal, env_summary, extract_call_id, process_sandbox_policy}; pub(crate) use audit::{ - audit_process, audit_process_id, audit_process_injections, audit_spawn_result, - record_process_denied, + audit_process, audit_process_id, audit_process_injections, audit_process_signal, + audit_spawn_result, record_process_denied, }; pub use persistent::PersistentProcessRegistry; pub use tracker::ProcessTracker; @@ -67,75 +49,6 @@ pub(crate) const MAX_BACKGROUND_PROCESSES: usize = 8; /// 4 MiB per spawn"). Oversized preludes are rejected with `too-large`. const MAX_SPAWN_STDIN_BYTES: usize = 4 * 1024 * 1024; -/// Extract the call_id from the caller's IPC context if it carried a -/// `ToolExecuteRequest` payload. -fn extract_call_id(state: &HostState) -> Option { - state.caller_context.as_ref().and_then(|msg| { - if let astrid_events::ipc::IpcPayload::ToolExecuteRequest { call_id, .. } = &msg.payload { - Some(call_id.clone()) - } else { - None - } - }) -} - -/// Summarize environment keys for audit without recording their values. -fn env_summary(env: &[EnvVar]) -> String { - env.iter() - .map(|e| e.key.as_str()) - .collect::>() - .join(",") -} - -/// The AUTHENTICATED calling principal, or `None` when the call resolves to -/// the capsule-owner fallback (no caller in scope). `spawn-persistent` -/// refuses the fallback: a persistent id MUST be scoped to a real principal, -/// or unauthenticated paths would share one `default` namespace that -/// `list-processes` would enumerate across tenants. -fn authenticated_principal(state: &HostState) -> Option { - state - .caller_context - .as_ref() - .and_then(|m| m.principal.as_deref()) - .and_then(|p| astrid_core::principal::PrincipalId::new(p).ok()) -} - -/// Build the sandboxed `Child` for a persistent spawn: stdout/stderr piped, -/// stdin piped only when a prelude or `keep-stdin-open` needs it, own process -/// group (so signals reach descendants), `kill_on_drop` as the reap backstop. -fn build_persistent_child( - request: &SpawnRequest, - workspace_root: &std::path::Path, - context: &PreparedSpawnContext, - want_stdin: bool, - injections: &[astrid_workspace::RoInjection], - inject_env: &[(String, String)], - extra_masks: &[std::path::PathBuf], -) -> Result { - let mut sandboxed = prepare_sandboxed_command( - &request.cmd, - &request.args, - workspace_root, - context, - injections, - inject_env, - extra_masks, - ) - .map_err(|_| ErrorCode::InvalidInput)?; - // `configure_piped` sets the process group + stdout/stderr pipes. - configure_piped(&mut sandboxed); - if want_stdin { - sandboxed.stdin(Stdio::piped()); - } else { - sandboxed.stdin(Stdio::null()); - } - let mut tokio_cmd = TokioCommand::from(sandboxed); - tokio_cmd.kill_on_drop(true); - tokio_cmd - .spawn() - .map_err(|e| ErrorCode::Unknown(format!("spawn-persistent failed: {e}"))) -} - impl process::Host for HostState { fn spawn(&mut self, request: SpawnRequest) -> Result { let workspace_root = self.workspace_root.clone(); @@ -172,6 +85,16 @@ impl process::Host for HostState { return Err(ErrorCode::CapabilityDenied); } + if request + .stdin + .as_ref() + .is_some_and(|stdin| stdin.len() > MAX_SPAWN_STDIN_BYTES) + { + let result: Result = Err(ErrorCode::TooLarge); + audit_process(self, "astrid:process/host.spawn", &cmd_for_audit, &result); + return result; + } + let spawn_context = match prepare_spawn_context(self, &request) { Ok(context) => context, Err(error) => { @@ -200,14 +123,21 @@ impl process::Host for HostState { let mut sandboxed_cmd = match prepare_sandboxed_command( &request.cmd, &request.args, - &workspace_root, &spawn_context, - &prepared.sandbox, - &injection_env, - &self.spawn_mask_paths, + SandboxInputs { + workspace_root: &workspace_root, + injections: &prepared.sandbox, + inject_env: &injection_env, + extra_masks: &self.spawn_mask_paths, + policy: process_sandbox_policy(self), + }, ) { Ok(cmd) => cmd, - Err(_) => { + Err(PrepareCommandError::SandboxDenied(reason)) => { + record_process_denied(self, "astrid:process/host.spawn", &cmd_for_audit, &reason); + return Err(ErrorCode::CapabilityDenied); + }, + Err(PrepareCommandError::Invalid) => { // Sandbox construction failed before exec — audit the attempt as // Failed instead of returning silently via `?`. let result: Result = Err(ErrorCode::InvalidInput); @@ -221,10 +151,16 @@ impl process::Host for HostState { return result; }, }; - sandboxed_cmd.stdout(Stdio::piped()); - sandboxed_cmd.stderr(Stdio::piped()); + configure_piped(&mut sandboxed_cmd); + sandboxed_cmd.stdin(if request.stdin.is_some() { + std::process::Stdio::piped() + } else { + std::process::Stdio::null() + }); - let child = match sandboxed_cmd.spawn() { + let mut tokio_cmd = tokio::process::Command::from(sandboxed_cmd); + tokio_cmd.kill_on_drop(true); + let child = match tokio_cmd.spawn() { Ok(child) => child, Err(e) => { // Fork/exec failed — audit the attempt as Failed before @@ -241,20 +177,36 @@ impl process::Host for HostState { return result; }, }; - let pid = child.id(); - process_tracker.register(pid, call_id); + let foreground = match ForegroundProcess::new(child) { + Ok(process) => process, + Err(error) => { + let result: Result = + Err(ErrorCode::Unknown(format!("spawn failed: {error}"))); + audit_spawn_result( + self, + "astrid:process/host.spawn", + &cmd_for_audit, + &injection_audit, + &result, + ); + return result; + }, + }; + let pid = foreground.pid(); + let tree = foreground.tree(); + process_tracker.register_tree(&tree, call_id); + let stdin_prelude = request.stdin.unwrap_or_default(); let output_result = util::bounded_block_on_cancellable(&handle, &semaphore, &cancel_token, async move { - tokio::task::spawn_blocking(move || child.wait_with_output()) - .await - .map_err(std::io::Error::other) - .and_then(|r| r) + let mut foreground = foreground; + foreground.write_stdin_prelude(&stdin_prelude).await?; + foreground.wait_with_output().await }); let result: Result = match output_result { Some(Ok(output)) => { - process_tracker.unregister(pid); + process_tracker.unregister_tree(&tree); Ok(ProcessResult { stdout: String::from_utf8_lossy(&output.stdout).into_owned(), stderr: String::from_utf8_lossy(&output.stderr).into_owned(), @@ -265,19 +217,12 @@ impl process::Host for HostState { }) }, Some(Err(e)) => { - process_tracker.unregister(pid); + process_tracker.unregister_tree(&tree); Err(ErrorCode::Unknown(format!("exec failed: {e}"))) }, None => { warn!(capsule_id = %self.capsule_id, pid, "process cancelled"); - #[cfg(unix)] - if let Ok(raw) = i32::try_from(pid) { - let _ = nix::sys::signal::kill( - nix::unistd::Pid::from_raw(raw), - nix::sys::signal::Signal::SIGKILL, - ); - } - process_tracker.unregister(pid); + process_tracker.unregister_tree(&tree); Err(ErrorCode::Cancelled) }, }; @@ -319,6 +264,7 @@ impl process::Host for HostState { let capsule_id = self.capsule_id.as_str().to_owned(); let handle = self.runtime_handle.clone(); let semaphore = self.blocking_semaphore.clone(); + let cancel_token = self.effective_cancel_token(); let cmd_for_audit = request.cmd.clone(); if let Some(sec) = security { @@ -345,6 +291,21 @@ impl process::Host for HostState { return Err(ErrorCode::CapabilityDenied); } + if request + .stdin + .as_ref() + .is_some_and(|stdin| stdin.len() > MAX_SPAWN_STDIN_BYTES) + { + let result: Result, ErrorCode> = Err(ErrorCode::TooLarge); + audit_process( + self, + "astrid:process/host.spawn-background", + &cmd_for_audit, + &result, + ); + return result; + } + // Re-check the cancellation token AFTER the (potentially // semaphore-bounded) capability check has run. The window // between gate clearance and `spawn()` is small but @@ -391,14 +352,26 @@ impl process::Host for HostState { let mut sandboxed_cmd = match prepare_sandboxed_command( &request.cmd, &request.args, - &workspace_root, &spawn_context, - &prepared.sandbox, - &injection_env, - &self.spawn_mask_paths, + SandboxInputs { + workspace_root: &workspace_root, + injections: &prepared.sandbox, + inject_env: &injection_env, + extra_masks: &self.spawn_mask_paths, + policy: process_sandbox_policy(self), + }, ) { Ok(cmd) => cmd, - Err(_) => { + Err(PrepareCommandError::SandboxDenied(reason)) => { + record_process_denied( + self, + "astrid:process/host.spawn-background", + &cmd_for_audit, + &reason, + ); + return Err(ErrorCode::CapabilityDenied); + }, + Err(PrepareCommandError::Invalid) => { // Sandbox construction failed before exec — audit the attempt as // Failed instead of returning silently via `?`. let result: Result, ErrorCode> = @@ -414,6 +387,11 @@ impl process::Host for HostState { }, }; configure_piped(&mut sandboxed_cmd); + sandboxed_cmd.stdin(if request.stdin.is_some() { + std::process::Stdio::piped() + } else { + std::process::Stdio::null() + }); // Convert the prepared std::Command into a tokio::Command so the // spawned Child supports async wait(&mut self) without ownership @@ -421,11 +399,10 @@ impl process::Host for HostState { // stranded the handle inside spawn_blocking on timeout). // `kill_on_drop(true)` ensures the tokio runtime reaps the // zombie if `ManagedProcess` is dropped before the child exits. - let mut tokio_cmd = TokioCommand::from(sandboxed_cmd); + let mut tokio_cmd = tokio::process::Command::from(sandboxed_cmd); tokio_cmd.kill_on_drop(true); - let command_str = format!("{} {}", request.cmd, request.args.join(" ")); - let child = match tokio_cmd.spawn() { + let mut child = match tokio_cmd.spawn() { Ok(child) => child, Err(e) => { // Fork/exec failed — audit the attempt as Failed before @@ -442,23 +419,61 @@ impl process::Host for HostState { return result; }, }; + let tree = match platform::ProcessTree::attach(&child) { + Ok(tree) => tree, + Err(error) => { + let _ = child.start_kill(); + let result: Result, ErrorCode> = Err(ErrorCode::Unknown( + format!("spawn-background ownership failed: {error}"), + )); + audit_spawn_result( + self, + "astrid:process/host.spawn-background", + &cmd_for_audit, + &injection_audit, + &result, + ); + return result; + }, + }; + + if let Err(error) = write_background_stdin( + &handle, + &semaphore, + &cancel_token, + &mut child, + request.stdin, + ) { + let result: Result, ErrorCode> = + match tree.terminate(platform::Termination::Force) { + Ok(()) => Err(error), + Err(termination_error) => Err(ErrorCode::Unknown(format!( + "spawn-background input failed ({error:?}); tree cleanup failed: \ + {termination_error:?}" + ))), + }; + audit_spawn_result( + self, + "astrid:process/host.spawn-background", + &cmd_for_audit, + &injection_audit, + &result, + ); + return result; + } let stdout_buf: Arc>> = Arc::new(Mutex::new(VecDeque::new())); let stderr_buf: Arc>> = Arc::new(Mutex::new(VecDeque::new())); let mut managed = ManagedProcess { child: Some(child), + tree: Arc::clone(&tree), stdout_buf: Arc::clone(&stdout_buf), stderr_buf: Arc::clone(&stderr_buf), - command: command_str, + audit_descriptor: cmd_for_audit.clone(), creator: principal.clone(), injection_guard: Some(prepared.guard), }; - let pid = managed - .child - .as_ref() - .and_then(tokio::process::Child::id) - .unwrap_or(0); attach_pipes(&mut managed, &handle); // Register with the cancellation tracker so a @@ -468,17 +483,21 @@ impl process::Host for HostState { // common case), so the entry is registered with None — which // makes it eligible for the "conservative fallback" branch of // `cancel_by_call_ids` (cancelled by any matching event). - self.process_tracker.register(pid, None); - let res = match self.resource_table.push(managed) { Ok(res) => res, Err(e) => { - // The child has ALREADY forked (and is registered/kill-on-drop - // via `managed`, which drops here). The spawn genuinely happened, - // so audit it as a Failed spawn rather than returning with no - // trace of the exec. + // The child has already forked. Tracker registration deliberately + // happens only after this insertion, so there is no stale tracker + // entry to unregister; `managed` drops here and the explicit tree + // termination covers descendants. Audit the real failed spawn. + let cleanup = tree.terminate(platform::Termination::Force); let result: Result, ErrorCode> = - Err(ErrorCode::Unknown(format!("resource table: {e}"))); + Err(ErrorCode::Unknown(match cleanup { + Ok(()) => format!("resource table: {e}"), + Err(error) => { + format!("resource table: {e}; tree cleanup failed: {error:?}") + }, + })); audit_spawn_result( self, "astrid:process/host.spawn-background", @@ -489,6 +508,7 @@ impl process::Host for HostState { return result; }, }; + self.process_tracker.register_tree(&tree, None); self.process_count_total += 1; *self .process_count_by_principal @@ -505,22 +525,9 @@ impl process::Host for HostState { result } - // ================================================================ - // PERSISTENT TIER — `astrid:process@1.0.0`. - // - // Backed by the host-owned `PersistentProcessRegistry` - // (`self.persistent_processes`), shared across the capsule's pooled - // instances so an id survives instance reset. Every id-keyed op - // re-resolves the live `(principal, capsule)` and checks it against the - // recorded creator inside the registry; unknown / wrong-owner / - // wrong-capsule / reaped collapse to `no-such-process` with no oracle. - // - // Still deferred (and honest about it): `attach` (resource-handle - // materialisation), `watch` / `unwatch` (host-published lifecycle events - // — an OPEN publish-authority question in RFC host_abi; `status` + bounded - // `wait` is the working alternative), and the `(NOT YET ...)` items the - // WIT itself flags (resource-limit enforcement, cpu/mem stats, pollables). - // ================================================================ + // Persistent entries live in the host-owned registry shared by pooled + // instances. Every id operation rechecks the principal and capsule owner; + // unknown, wrong-owner, and reaped entries share one no-such-process result. fn spawn_persistent(&mut self, request: SpawnRequest) -> Result { let cmd_for_audit = request.cmd.clone(); @@ -658,16 +665,28 @@ impl process::Host for HostState { } let want_stdin = request.keep_stdin_open.unwrap_or(false) || request.stdin.is_some(); - let mut child = match build_persistent_child( + let (mut child, tree) = match build_persistent_child( &request, - &workspace_root, &spawn_context, want_stdin, - &prepared.sandbox, - &prepared.env, - &self.spawn_mask_paths, + SandboxInputs { + workspace_root: &workspace_root, + injections: &prepared.sandbox, + inject_env: &prepared.env, + extra_masks: &self.spawn_mask_paths, + policy: process_sandbox_policy(self), + }, ) { Ok(c) => c, + Err(ErrorCode::CapabilityDenied) => { + record_process_denied( + self, + "astrid:process/host.spawn-persistent", + &cmd_for_audit, + "native process sandbox is unavailable under required policy", + ); + return Err(ErrorCode::CapabilityDenied); + }, Err(e) => { let result: Result = Err(e); audit_process( @@ -682,19 +701,9 @@ impl process::Host for HostState { // Reject a missing/zero pid: `killpg(0)` / `kill(0)` would target the // daemon's OWN process group. A reaped child surfaces `None`; drop it // (kill_on_drop reaps) and fail rather than store an unsignalable entry. - let Some(os_pid) = child.id().filter(|&p| p != 0) else { - let result: Result = Err(ErrorCode::Unknown( - "spawn-persistent: child has no usable pid".to_string(), - )); - audit_process( - self, - "astrid:process/host.spawn-persistent", - &cmd_for_audit, - &result, - ); - return result; - }; + let os_pid = tree.pid(); let (Some(stdout), Some(stderr)) = (child.stdout.take(), child.stderr.take()) else { + tree.terminate(platform::Termination::Force)?; return Err(ErrorCode::Unknown( "spawn-persistent: missing stdio pipes".to_string(), )); @@ -712,9 +721,16 @@ impl process::Host for HostState { (pipe, r) }); if write_res.is_err() { - let result: Result = Err(ErrorCode::Unknown( - "spawn-persistent: stdin prelude write failed".to_string(), - )); + let result: Result = + match tree.terminate(platform::Termination::Force) { + Ok(()) => Err(ErrorCode::Unknown( + "spawn-persistent: stdin prelude write failed".to_string(), + )), + Err(error) => Err(ErrorCode::Unknown(format!( + "spawn-persistent: stdin prelude write failed; tree cleanup failed: \ + {error:?}" + ))), + }; audit_process( self, "astrid:process/host.spawn-persistent", @@ -738,6 +754,7 @@ impl process::Host for HostState { capsule_id: capsule_id_arc, command, os_pid, + tree, child, stdout, stderr, @@ -890,12 +907,19 @@ impl process::Host for HostState { } fn signal(&mut self, id: String, sig: ProcessSignal) -> Result<(), ErrorCode> { + let id_hash = blake3::hash(id.as_bytes()).to_hex(); + let descriptor = format!("persistent:{}", &id_hash[..16]); + if !platform::signal_supported(sig) { + let result = Err(ErrorCode::CapabilityDenied); + audit_process_signal(self, &descriptor, sig, &result); + return result; + } let principal = self.effective_principal(); let capsule_id = self.capsule_id.as_str().to_owned(); let result = self .persistent_processes .signal(&id, &principal, &capsule_id, sig); - audit_process_id(self, "astrid:process/host.signal", &id, &result); + audit_process_signal(self, &descriptor, sig, &result); result } diff --git a/crates/astrid-capsule/src/engine/wasm/host/process/persistent/entry.rs b/crates/astrid-capsule/src/engine/wasm/host/process/persistent/entry.rs index a0eb18ec3..705747f51 100644 --- a/crates/astrid-capsule/src/engine/wasm/host/process/persistent/entry.rs +++ b/crates/astrid-capsule/src/engine/wasm/host/process/persistent/entry.rs @@ -73,6 +73,7 @@ pub(super) struct PersistentEntry { pub(super) label: String, pub(super) command: String, pub(super) os_pid: u32, + pub(super) tree: Arc, pub(super) spawned_at: Instant, pub(super) max_lifetime: Duration, pub(super) idle_timeout: Duration, @@ -81,8 +82,8 @@ pub(super) struct PersistentEntry { /// Latches the exit so `wait` / `stop` await it without racing the /// monitor task or holding the core lock across an `await`. pub(super) exit_rx: watch::Receiver>, - /// The monitor task owning the `Child`. Aborting it drops the `Child`, - /// whose `kill_on_drop(true)` SIGKILLs the process — the reap backstop. + /// The monitor task owning the `Child`. Tree termination happens before + /// abort; dropping the child then remains the root-process reap backstop. pub(super) monitor: tokio::task::JoinHandle<()>, /// Cleanup guard for any read-only file injections wired into this child's /// sandbox. Held for the process's lifetime; its `Drop` (Linux scratch dir @@ -130,7 +131,7 @@ pub(super) struct Resolved { pub(super) key: [u8; 32], pub(super) core: Arc>, pub(super) exit_rx: watch::Receiver>, - pub(super) os_pid: u32, + pub(super) tree: Arc, } /// Read the current exit (if any) without holding a lock across `await`. @@ -157,15 +158,26 @@ pub(super) async fn wait_for_exit( /// Spawn the monitor task that owns the `Child`, records its exit into /// `core`, and notifies `exit_tx`. Returns the join handle (aborting it -/// drops the `Child`, whose `kill_on_drop` is the reap backstop). +/// drops the `Child`, whose `kill_on_drop` is the root-process reap backstop). pub(super) fn spawn_monitor( runtime: &tokio::runtime::Handle, mut child: tokio::process::Child, + tree: Arc, core: Arc>, exit_tx: watch::Sender>, ) -> tokio::task::JoinHandle<()> { runtime.spawn(async move { let status = child.wait().await; + #[cfg(windows)] + if let Err(error) = tree.terminate(super::super::platform::Termination::Force) { + tracing::warn!( + pid = tree.pid(), + ?error, + "failed to terminate persistent process descendants after root exit" + ); + } + #[cfg(not(windows))] + let _ = tree; let record = match status { Ok(st) => ExitRecord { exit_code: st.code(), @@ -185,7 +197,8 @@ pub(super) fn spawn_monitor( } let _ = exit_tx.send(Some(record)); // `child` drops here: already exited, so `kill_on_drop` is a no-op. - // If the task is ABORTED before exit, that drop SIGKILLs instead. + // If the task is aborted before exit, dropping the child forces its + // root process to terminate instead. }) } @@ -244,78 +257,20 @@ pub(super) fn spawn_ring_reader( }); } -/// Reap an entry removed from the map: SIGKILL the group (best effort) and -/// abort the monitor (dropping its `Child`, the `kill_on_drop` backstop). -pub(super) fn reap_entry(entry: PersistentEntry) { - if entry.is_live() { - let _ = send_signal(entry.os_pid, HostSignal::Kill); - } +/// Reap an entry removed from the map: force the tree (best effort) and abort +/// the monitor (dropping its `Child`, the `kill_on_drop` backstop). +pub(super) fn reap_entry(entry: PersistentEntry) -> Result<(), ErrorCode> { + let result = entry + .tree + .terminate(super::super::platform::Termination::Force); entry.monitor.abort(); + result } -/// Platform-neutral signal understood by the persistent-process registry. -#[derive(Clone, Copy, Debug)] -pub(super) enum HostSignal { - Term, - Hup, - Usr1, - Usr2, - Int, - Stop, - Cont, - Kill, -} - -/// Map the WIT `process-signal` to an internal signal. -pub(super) fn map_signal(sig: ProcessSignal) -> HostSignal { - match sig { - ProcessSignal::Term => HostSignal::Term, - ProcessSignal::Hup => HostSignal::Hup, - ProcessSignal::Usr1 => HostSignal::Usr1, - ProcessSignal::Usr2 => HostSignal::Usr2, - ProcessSignal::Int => HostSignal::Int, - ProcessSignal::Stop => HostSignal::Stop, - ProcessSignal::Cont => HostSignal::Cont, - } -} - -/// Send a signal to the child's PROCESS GROUP (it is spawned with -/// `process_group(0)`, so descendants are signalled too), falling back to -/// the bare pid if the group send fails. -pub(super) fn send_signal(pid: u32, sig: HostSignal) -> Result<(), ErrorCode> { - // Refuse pid 0: `killpg(0)` / `kill(0)` target the CALLER's (daemon's) own - // process group — never the child. A reaped child surfaces pid `None` - // (stored as 0); guard here as defense-in-depth (spawn also rejects it). - if pid == 0 { - return Err(ErrorCode::Closed); - } - #[cfg(unix)] - { - use nix::sys::signal::Signal; - - let raw = i32::try_from(pid).map_err(|_| ErrorCode::InvalidInput)?; - let target = nix::unistd::Pid::from_raw(raw); - let native = match sig { - HostSignal::Term => Signal::SIGTERM, - HostSignal::Hup => Signal::SIGHUP, - HostSignal::Usr1 => Signal::SIGUSR1, - HostSignal::Usr2 => Signal::SIGUSR2, - HostSignal::Int => Signal::SIGINT, - HostSignal::Stop => Signal::SIGSTOP, - HostSignal::Cont => Signal::SIGCONT, - HostSignal::Kill => Signal::SIGKILL, - }; - if nix::sys::signal::killpg(target, native).is_err() { - nix::sys::signal::kill(target, native) - .map_err(|e| ErrorCode::Unknown(format!("signal {sig:?}: {e}")))?; - } - Ok(()) - } - #[cfg(not(unix))] - { - let _ = (pid, sig); - Err(ErrorCode::Unknown( - "process signals unsupported on this platform".to_string(), - )) - } +/// Deliver a WIT signal through the platform backend. +pub(super) fn send_signal( + tree: &super::super::platform::ProcessTree, + signal: ProcessSignal, +) -> Result<(), ErrorCode> { + super::super::platform::signal_process_tree(tree, signal) } diff --git a/crates/astrid-capsule/src/engine/wasm/host/process/persistent/mod.rs b/crates/astrid-capsule/src/engine/wasm/host/process/persistent/mod.rs index 7c9e10d01..62b32d200 100644 --- a/crates/astrid-capsule/src/engine/wasm/host/process/persistent/mod.rs +++ b/crates/astrid-capsule/src/engine/wasm/host/process/persistent/mod.rs @@ -69,8 +69,8 @@ use config::{ MAX_STDIN_WRITE, MAX_STOP_GRACE, clamp_label, clamp_log_ring, overflow_from_wit, resolve_ttls, }; use entry::{ - HostSignal, PersistentEntry, Phase, ProcessCore, current_exit, map_signal, reap_entry, - send_signal, spawn_monitor, spawn_ring_reader, wait_for_exit, + PersistentEntry, Phase, ProcessCore, current_exit, reap_entry, send_signal, spawn_monitor, + spawn_ring_reader, wait_for_exit, }; use ids::mint_id; use ring::{LogRing, Stream, decode_cursor, encode_cursor}; @@ -88,6 +88,7 @@ pub(in crate::engine::wasm::host::process) struct SpawnParams { /// cmd + args, as the capsule requested it (for display / label default). pub(in crate::engine::wasm::host::process) command: String, pub(in crate::engine::wasm::host::process) os_pid: u32, + pub(in crate::engine::wasm::host::process) tree: Arc, pub(in crate::engine::wasm::host::process) child: tokio::process::Child, pub(in crate::engine::wasm::host::process) stdout: tokio::process::ChildStdout, pub(in crate::engine::wasm::host::process) stderr: tokio::process::ChildStderr, @@ -174,7 +175,7 @@ impl PersistentProcessRegistry { key, core: Arc::clone(&entry.core), exit_rx: entry.exit_rx.clone(), - os_pid: entry.os_pid, + tree: Arc::clone(&entry.tree), }) } @@ -230,7 +231,13 @@ impl PersistentProcessRegistry { spawn_ring_reader(&self.runtime, p.stdout, Arc::clone(&core), Stream::Out); spawn_ring_reader(&self.runtime, p.stderr, Arc::clone(&core), Stream::Err); let (exit_tx, exit_rx) = watch::channel::>(None); - let monitor = spawn_monitor(&self.runtime, p.child, Arc::clone(&core), exit_tx); + let monitor = spawn_monitor( + &self.runtime, + p.child, + Arc::clone(&p.tree), + Arc::clone(&core), + exit_tx, + ); let injection_guard = p.injection_guard; let mut id = mint_id(); @@ -242,7 +249,9 @@ impl PersistentProcessRegistry { tries += 1; if tries > 8 { // 256-bit space: unreachable in practice. Fail closed. + let cleanup = p.tree.terminate(super::platform::Termination::Force); monitor.abort(); + cleanup?; return Err(ErrorCode::Unknown( "process-id collision space exhausted".to_string(), )); @@ -257,6 +266,7 @@ impl PersistentProcessRegistry { label, command: p.command, os_pid: p.os_pid, + tree: p.tree, spawned_at: Instant::now(), max_lifetime, idle_timeout, @@ -409,7 +419,7 @@ impl PersistentProcessRegistry { { return Ok(()); } - send_signal(r.os_pid, map_signal(sig)) + send_signal(&r.tree, sig) } /// Write to stdin (requires `keep-stdin-open`). @@ -481,8 +491,8 @@ impl PersistentProcessRegistry { } } - /// Graceful terminal stop: SIGTERM → grace → SIGKILL, then REMOVE the id - /// (frees the concurrent + retained slot). + /// Graceful terminal stop: platform grace → forced tree termination, then + /// REMOVE the id (frees the concurrent + retained slot). pub(in crate::engine::wasm::host::process) async fn stop( &self, id: &str, @@ -496,12 +506,12 @@ impl PersistentProcessRegistry { let exit = if let Some(e) = current_exit(&r.core) { e } else { - let _ = send_signal(r.os_pid, HostSignal::Term); + r.tree.terminate(super::platform::Termination::Graceful)?; let mut rx = r.exit_rx.clone(); match tokio::time::timeout(grace, wait_for_exit(&mut rx)).await { Ok(Some(e)) => e, _ => { - let _ = send_signal(r.os_pid, HostSignal::Kill); + r.tree.terminate(super::platform::Termination::Force)?; let mut rx2 = r.exit_rx.clone(); match tokio::time::timeout(MAX_STOP_GRACE, wait_for_exit(&mut rx2)).await { Ok(Some(e)) => e, @@ -517,7 +527,7 @@ impl PersistentProcessRegistry { // syscall never stalls other registry ops. let removed = self.lock().remove(&r.key); if let Some(entry) = removed { - reap_entry(entry); + reap_entry(entry)?; } Ok(exit.into()) } @@ -544,7 +554,7 @@ impl PersistentProcessRegistry { let removed = map.remove(&key); drop(map); if let Some(entry) = removed { - reap_entry(entry); + reap_entry(entry)?; } Ok(()) } @@ -591,7 +601,9 @@ impl PersistentProcessRegistry { // Reap (killpg + abort) outside the map lock. let n = reaped.len(); for entry in reaped { - reap_entry(entry); + if let Err(error) = reap_entry(entry) { + tracing::warn!(?error, "persistent process sweep cleanup failed"); + } } n } @@ -600,14 +612,17 @@ impl PersistentProcessRegistry { pub fn shutdown(&self) { let drained: Vec = self.lock().drain().map(|(_, e)| e).collect(); for entry in drained { - reap_entry(entry); + if let Err(error) = reap_entry(entry) { + tracing::warn!(?error, "persistent process shutdown cleanup failed"); + } } } } fn reject_spawn(mut p: SpawnParams, err: ErrorCode) -> Result { - let _ = entry::send_signal(p.os_pid, HostSignal::Kill); + let cleanup = p.tree.terminate(super::platform::Termination::Force); let _ = p.child.start_kill(); let _ = p.child.try_wait(); + cleanup?; Err(err) } diff --git a/crates/astrid-capsule/src/engine/wasm/host/process/persistent/registry_tests.rs b/crates/astrid-capsule/src/engine/wasm/host/process/persistent/registry_tests.rs index 9702b675f..e6854ddc0 100644 --- a/crates/astrid-capsule/src/engine/wasm/host/process/persistent/registry_tests.rs +++ b/crates/astrid-capsule/src/engine/wasm/host/process/persistent/registry_tests.rs @@ -27,11 +27,7 @@ fn spawn_raw( .stdout(Stdio::piped()) .stderr(Stdio::piped()) .stdin(Stdio::null()); - #[cfg(unix)] - { - use std::os::unix::process::CommandExt as _; - std_cmd.process_group(0); - } + super::super::platform::configure_process_group(&mut std_cmd); let mut cmd = tokio::process::Command::from(std_cmd); cmd.kill_on_drop(true); let mut child = cmd.spawn().expect("spawn test child"); @@ -51,11 +47,13 @@ fn params( os_pid: u32, concurrent_cap: usize, ) -> SpawnParams { + let tree = super::super::platform::ProcessTree::attach(&child).expect("attach process tree"); SpawnParams { creator: creator.clone(), capsule_id: Arc::from(capsule), command: "sh -c ".to_string(), os_pid, + tree, child, stdout, stderr, @@ -89,11 +87,7 @@ fn spawn_raw_stdin( .stdout(Stdio::piped()) .stderr(Stdio::piped()) .stdin(Stdio::piped()); - #[cfg(unix)] - { - use std::os::unix::process::CommandExt as _; - std_cmd.process_group(0); - } + super::super::platform::configure_process_group(&mut std_cmd); let mut cmd = tokio::process::Command::from(std_cmd); cmd.kill_on_drop(true); let mut child = cmd.spawn().expect("spawn test child"); @@ -279,11 +273,13 @@ async fn write_stdin_delivers_survives_reset_and_close_eofs() { // writes unbuffered, so each line surfaces on stdout immediately. let (child, so, se, stdin, pid) = spawn_raw_stdin("while IFS= read -r line; do echo \"got:$line\"; done"); + let tree = super::super::platform::ProcessTree::attach(&child).expect("attach process tree"); let spawn_params = SpawnParams { creator: alice.clone(), capsule_id: Arc::from("cap"), command: "sh -c ".to_string(), os_pid: pid, + tree, child, stdout: so, stderr: se, diff --git a/crates/astrid-capsule/src/engine/wasm/host/process/platform.rs b/crates/astrid-capsule/src/engine/wasm/host/process/platform.rs new file mode 100644 index 000000000..eda245e5a --- /dev/null +++ b/crates/astrid-capsule/src/engine/wasm/host/process/platform.rs @@ -0,0 +1,874 @@ +//! Platform process creation and termination primitives. +//! +//! Commands always use `std::process::Command`; every guest argument remains a +//! distinct OS argument and Astrid never inserts a shell. Windows process trees +//! are owned by a kernel Job Object configured with `KILL_ON_JOB_CLOSE`. + +// The Windows backend is a narrow, reviewed FFI boundary over owned handles. +// Keep unsafe prohibited throughout the rest of astrid-capsule. +#![allow(unsafe_code)] + +use std::sync::Arc; +#[cfg(windows)] +use std::sync::atomic::AtomicBool; +use std::sync::atomic::{AtomicU64, Ordering}; + +use crate::engine::wasm::bindings::astrid::process1_1_0::host::{ErrorCode, ProcessSignal}; + +#[derive(Clone, Copy)] +pub(super) enum Termination { + Graceful, + Force, +} + +static NEXT_PROCESS_IDENTITY: AtomicU64 = AtomicU64::new(1); + +/// Stable ownership token for one spawned process tree. +pub(super) struct ProcessTree { + pid: u32, + identity: u64, + #[cfg(windows)] + job: std::os::windows::io::OwnedHandle, + #[cfg(windows)] + terminated: AtomicBool, + #[cfg(all(test, windows))] + fail_termination: AtomicBool, +} + +impl std::fmt::Debug for ProcessTree { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ProcessTree") + .field("pid", &self.pid) + .field("identity", &self.identity) + .finish_non_exhaustive() + } +} + +impl ProcessTree { + pub(super) fn attach(child: &tokio::process::Child) -> std::io::Result> { + Self::attach_inner(child, false) + } + + fn attach_inner( + child: &tokio::process::Child, + inject_assignment_failure: bool, + ) -> std::io::Result> { + let pid = child + .id() + .filter(|pid| *pid != 0) + .ok_or_else(|| std::io::Error::other("spawned child has no usable pid"))?; + let identity = NEXT_PROCESS_IDENTITY.fetch_add(1, Ordering::Relaxed); + + #[cfg(windows)] + { + Ok(Arc::new(Self { + pid, + identity, + job: create_assign_and_resume_job(child, inject_assignment_failure)?, + terminated: AtomicBool::new(false), + #[cfg(test)] + fail_termination: AtomicBool::new(false), + })) + } + #[cfg(not(windows))] + { + let _ = inject_assignment_failure; + Ok(Arc::new(Self { pid, identity })) + } + } + + #[cfg(all(test, windows))] + pub(super) fn inject_assignment_failure( + child: &tokio::process::Child, + ) -> std::io::Result> { + Self::attach_inner(child, true) + } + + pub(super) fn pid(&self) -> u32 { + self.pid + } + + pub(super) fn identity(&self) -> u64 { + self.identity + } + + /// Terminate the owned tree. Repeated successful calls are idempotent. + pub(super) fn terminate(&self, termination: Termination) -> Result<(), ErrorCode> { + #[cfg(all(test, windows))] + if self.fail_termination.load(Ordering::Acquire) { + return Err(ErrorCode::Unknown( + "injected process-tree termination failure".to_string(), + )); + } + #[cfg(unix)] + { + let signal = match termination { + Termination::Graceful => nix::sys::signal::Signal::SIGTERM, + Termination::Force => nix::sys::signal::Signal::SIGKILL, + }; + terminate_unix_process_group(self.pid, signal) + } + #[cfg(windows)] + { + let _ = termination; + if self.terminated.load(Ordering::Acquire) { + return Ok(()); + } + terminate_windows_job(&self.job).map_err(|error| { + ErrorCode::Unknown(format!("terminate Windows process job: {error}")) + })?; + self.terminated.store(true, Ordering::Release); + Ok(()) + } + #[cfg(not(any(unix, windows)))] + { + let _ = termination; + Err(ErrorCode::CapabilityDenied) + } + } + + #[cfg(all(test, windows))] + pub(super) fn inject_termination_failure(&self, fail: bool) { + self.fail_termination.store(fail, Ordering::Release); + } +} + +pub(super) fn configure_process_group(command: &mut std::process::Command) { + #[cfg(unix)] + { + use std::os::unix::process::CommandExt as _; + command.process_group(0); + } + #[cfg(windows)] + { + use std::os::windows::process::CommandExt as _; + use windows_sys::Win32::System::Threading::{CREATE_NEW_PROCESS_GROUP, CREATE_SUSPENDED}; + // The primary thread MUST remain suspended until ProcessTree::attach + // assigns the process to its kill-on-close Job Object. This closes the + // post-spawn escape window in which child code could create an + // unowned descendant before Job membership existed. + command.creation_flags(CREATE_NEW_PROCESS_GROUP | CREATE_SUSPENDED); + } +} + +/// Windows has no faithful equivalents for POSIX HUP/USR/INT/STOP/CONT. +pub(super) fn signal_supported(signal: ProcessSignal) -> bool { + #[cfg(windows)] + { + matches!(signal, ProcessSignal::Term) + } + #[cfg(not(windows))] + { + let _ = signal; + true + } +} + +pub(super) fn signal_process_tree( + tree: &ProcessTree, + signal: ProcessSignal, +) -> Result<(), ErrorCode> { + #[cfg(unix)] + { + let signal = match signal { + ProcessSignal::Term => nix::sys::signal::Signal::SIGTERM, + ProcessSignal::Hup => nix::sys::signal::Signal::SIGHUP, + ProcessSignal::Usr1 => nix::sys::signal::Signal::SIGUSR1, + ProcessSignal::Usr2 => nix::sys::signal::Signal::SIGUSR2, + ProcessSignal::Int => nix::sys::signal::Signal::SIGINT, + ProcessSignal::Stop => nix::sys::signal::Signal::SIGSTOP, + ProcessSignal::Cont => nix::sys::signal::Signal::SIGCONT, + }; + signal_unix_process_group(tree.pid, signal) + } + #[cfg(windows)] + { + match signal { + ProcessSignal::Term => tree.terminate(Termination::Graceful), + ProcessSignal::Hup + | ProcessSignal::Usr1 + | ProcessSignal::Usr2 + | ProcessSignal::Int + | ProcessSignal::Stop + | ProcessSignal::Cont => Err(ErrorCode::CapabilityDenied), + } + } + #[cfg(not(any(unix, windows)))] + { + let _ = (tree, signal); + Err(ErrorCode::CapabilityDenied) + } +} + +/// Signal only the root process on Unix, preserving the public handle +/// semantics that predate Windows tree ownership. Windows TERM necessarily +/// targets the owned Job because it has no faithful POSIX root signal. +pub(super) fn signal_root_process( + tree: &ProcessTree, + signal: ProcessSignal, +) -> Result<(), ErrorCode> { + #[cfg(unix)] + { + let signal = match signal { + ProcessSignal::Term => nix::sys::signal::Signal::SIGTERM, + ProcessSignal::Hup => nix::sys::signal::Signal::SIGHUP, + ProcessSignal::Usr1 => nix::sys::signal::Signal::SIGUSR1, + ProcessSignal::Usr2 => nix::sys::signal::Signal::SIGUSR2, + ProcessSignal::Int => nix::sys::signal::Signal::SIGINT, + ProcessSignal::Stop => nix::sys::signal::Signal::SIGSTOP, + ProcessSignal::Cont => nix::sys::signal::Signal::SIGCONT, + }; + let raw = i32::try_from(tree.pid).map_err(|_| ErrorCode::InvalidInput)?; + nix::sys::signal::kill(nix::unistd::Pid::from_raw(raw), signal) + .map_err(|error| ErrorCode::Unknown(format!("kill({signal:?}): {error}"))) + } + #[cfg(windows)] + { + signal_process_tree(tree, signal) + } + #[cfg(not(any(unix, windows)))] + { + let _ = (tree, signal); + Err(ErrorCode::CapabilityDenied) + } +} + +#[cfg(unix)] +fn signal_unix_process_group(pid: u32, signal: nix::sys::signal::Signal) -> Result<(), ErrorCode> { + let raw = i32::try_from(pid).map_err(|_| ErrorCode::InvalidInput)?; + let target = nix::unistd::Pid::from_raw(raw); + if nix::sys::signal::killpg(target, signal).is_err() { + nix::sys::signal::kill(target, signal) + .map_err(|error| ErrorCode::Unknown(format!("signal {signal:?}: {error}")))?; + } + Ok(()) +} + +#[cfg(unix)] +fn terminate_unix_process_group( + pid: u32, + signal: nix::sys::signal::Signal, +) -> Result<(), ErrorCode> { + let raw = i32::try_from(pid).map_err(|_| ErrorCode::InvalidInput)?; + let target = nix::unistd::Pid::from_raw(raw); + if nix::sys::signal::killpg(target, signal).is_ok() { + return Ok(()); + } + match nix::sys::signal::kill(target, signal) { + Ok(()) | Err(nix::errno::Errno::ESRCH) => Ok(()), + Err(error) => Err(ErrorCode::Unknown(format!("signal {signal:?}: {error}"))), + } +} + +#[cfg(windows)] +fn create_assign_and_resume_job( + child: &tokio::process::Child, + inject_assignment_failure: bool, +) -> std::io::Result { + use std::ffi::c_void; + use std::mem::size_of; + use std::os::windows::io::{AsRawHandle as _, FromRawHandle as _}; + use windows_sys::Win32::System::JobObjects::{ + AssignProcessToJobObject, CreateJobObjectW, JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, + JOBOBJECT_EXTENDED_LIMIT_INFORMATION, JobObjectExtendedLimitInformation, + SetInformationJobObject, + }; + let child_handle = child + .raw_handle() + .ok_or_else(|| std::io::Error::other("spawned child has no process handle"))?; + + // SAFETY: null attributes/name request a private unnamed Job Object. + let raw_job = unsafe { CreateJobObjectW(std::ptr::null(), std::ptr::null()) }; + if raw_job.is_null() { + return Err(std::io::Error::last_os_error()); + } + // SAFETY: CreateJobObjectW returned a new owned handle. + let job = unsafe { std::os::windows::io::OwnedHandle::from_raw_handle(raw_job.cast()) }; + let mut limits = JOBOBJECT_EXTENDED_LIMIT_INFORMATION::default(); + limits.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + // SAFETY: job is valid and `limits` matches the information class. + let configured = unsafe { + SetInformationJobObject( + job.as_raw_handle().cast(), + JobObjectExtendedLimitInformation, + (&raw const limits).cast::(), + u32::try_from(size_of::()) + .expect("job limit structure fits u32"), + ) + }; + if configured == 0 { + return Err(std::io::Error::last_os_error()); + } + if inject_assignment_failure { + return Err(std::io::Error::other( + "injected Job Object assignment failure", + )); + } + // SAFETY: both handles are borrowed and valid for the duration of the call. + let assigned = + unsafe { AssignProcessToJobObject(job.as_raw_handle().cast(), child_handle.cast()) }; + if assigned == 0 { + return Err(std::io::Error::last_os_error()); + } + resume_suspended_process_threads(child.id().ok_or_else(|| { + std::io::Error::other("assigned child lost its process id before resume") + })?)?; + Ok(job) +} + +#[cfg(windows)] +fn resume_suspended_process_threads(pid: u32) -> std::io::Result<()> { + use std::mem::size_of; + use std::os::windows::io::{AsRawHandle as _, FromRawHandle as _}; + use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE; + use windows_sys::Win32::System::Diagnostics::ToolHelp::{ + CreateToolhelp32Snapshot, TH32CS_SNAPTHREAD, THREADENTRY32, Thread32First, Thread32Next, + }; + use windows_sys::Win32::System::Threading::{OpenThread, ResumeThread, THREAD_SUSPEND_RESUME}; + + // Collect every thread handle before resuming any. A CREATE_SUSPENDED + // process normally has exactly its primary thread, but collecting first + // preserves the all-or-kill property if Windows adds a loader thread. + // SAFETY: fixed flags and process id 0 request a system thread snapshot. + let raw_snapshot = unsafe { CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0) }; + if raw_snapshot == INVALID_HANDLE_VALUE { + return Err(std::io::Error::last_os_error()); + } + // SAFETY: the snapshot call returned a newly owned, non-sentinel handle. + let snapshot = + unsafe { std::os::windows::io::OwnedHandle::from_raw_handle(raw_snapshot.cast()) }; + let mut entry = THREADENTRY32 { + dwSize: u32::try_from(size_of::()).expect("thread entry structure fits u32"), + ..THREADENTRY32::default() + }; + let mut threads = Vec::new(); + // SAFETY: snapshot and entry are valid for the enumeration calls. + let mut present = + unsafe { Thread32First(snapshot.as_raw_handle().cast(), &raw mut entry) } != 0; + while present { + if entry.th32OwnerProcessID == pid { + // SAFETY: thread id came from the live snapshot; the returned + // handle, when non-null, is newly owned. + let raw_thread = unsafe { OpenThread(THREAD_SUSPEND_RESUME, 0, entry.th32ThreadID) }; + if raw_thread.is_null() { + return Err(std::io::Error::last_os_error()); + } + // SAFETY: OpenThread returned a new owned handle. + threads.push(unsafe { + std::os::windows::io::OwnedHandle::from_raw_handle(raw_thread.cast()) + }); + } + // SAFETY: same valid snapshot/entry pair; false means enumeration end. + present = unsafe { Thread32Next(snapshot.as_raw_handle().cast(), &raw mut entry) } != 0; + } + if threads.is_empty() { + return Err(std::io::Error::other( + "suspended child has no enumerable threads", + )); + } + + for thread in threads { + // SAFETY: each handle was opened with THREAD_SUSPEND_RESUME. + let previous = unsafe { ResumeThread(thread.as_raw_handle().cast()) }; + if previous == u32::MAX { + return Err(std::io::Error::last_os_error()); + } + if previous != 1 { + return Err(std::io::Error::other(format!( + "unexpected suspended-thread count {previous} while starting owned child" + ))); + } + } + Ok(()) +} + +#[cfg(windows)] +fn terminate_windows_job(job: &std::os::windows::io::OwnedHandle) -> std::io::Result<()> { + use std::os::windows::io::AsRawHandle as _; + use windows_sys::Win32::System::JobObjects::TerminateJobObject; + + // SAFETY: `job` owns a valid Job Object handle for this call. + if unsafe { TerminateJobObject(job.as_raw_handle().cast(), 1) } == 0 { + Err(std::io::Error::last_os_error()) + } else { + Ok(()) + } +} + +#[cfg(test)] +mod tests { + #[cfg(windows)] + use std::process::Command; + + #[cfg(windows)] + use super::*; + + #[cfg(windows)] + #[tokio::test(flavor = "multi_thread")] + async fn unsupported_windows_signals_fail_closed() { + use std::process::Stdio; + + let executable = std::env::current_exe().expect("current test executable"); + let temp = tempfile::tempdir().expect("temp"); + let heartbeat = temp.path().join("heartbeat"); + let mut command = std::process::Command::new(executable); + command + .arg("windows_process_probe_child") + .arg("--nocapture") + .env("ASTRID_WINDOWS_PROCESS_PROBE", "tree-leaf") + .env("ASTRID_HEARTBEAT", heartbeat) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + configure_process_group(&mut command); + let mut command = tokio::process::Command::from(command); + command.kill_on_drop(true); + let mut child = command.spawn().expect("spawn signal probe"); + let tree = ProcessTree::attach(&child).expect("attach process tree"); + for signal in [ + ProcessSignal::Hup, + ProcessSignal::Usr1, + ProcessSignal::Usr2, + ProcessSignal::Int, + ProcessSignal::Stop, + ProcessSignal::Cont, + ] { + assert!(matches!( + signal_process_tree(&tree, signal), + Err(ErrorCode::CapabilityDenied) + )); + } + tree.terminate(Termination::Force).expect("terminate probe"); + let _ = child.start_kill(); + } + + #[cfg(windows)] + #[test] + #[allow(clippy::zombie_processes)] + fn windows_process_probe_child() { + use std::io::{Read as _, Write as _}; + use std::process::Stdio; + use std::time::Duration; + + let mode = std::env::var("ASTRID_WINDOWS_PROCESS_PROBE").unwrap_or_default(); + match mode.as_str() { + "touch" => { + std::fs::write( + std::env::var_os("ASTRID_SENTINEL").expect("sentinel path"), + b"executed", + ) + .expect("write sentinel"); + }, + "host-stdio" => { + assert_eq!( + std::env::current_dir().expect("child cwd"), + std::path::PathBuf::from( + std::env::var_os("ASTRID_EXPECTED_CWD").expect("expected cwd") + ) + ); + assert_eq!( + std::env::var("ASTRID_WINDOWS_EDGE").as_deref(), + Ok("unicode-\u{2603}-quote\"-slash\\") + ); + let mut input = String::new(); + std::io::stdin() + .read_to_string(&mut input) + .expect("read stdin"); + assert_eq!(input, "host stdin \u{2603} \" \\"); + std::io::stdout() + .write_all(b"host-stdout") + .expect("write stdout"); + std::io::stderr() + .write_all(b"host-stderr") + .expect("write stderr"); + std::process::exit(37); + }, + "stdio" => { + let skip_values = std::env::args_os() + .collect::>() + .windows(2) + .filter(|pair| pair[0] == "--skip") + .map(|pair| pair[1].to_string_lossy().into_owned()) + .collect::>(); + assert_eq!( + skip_values, + [ + "argument with spaces", + "quote\"inside", + "trailing\\", + "slashes\\\\\\\"quote", + ] + ); + assert_eq!( + std::env::var("ASTRID_WINDOWS_EDGE").as_deref(), + Ok("value with spaces \" and trailing\\") + ); + assert!(std::env::var_os("ASTRID_HOST_SECRET").is_none()); + assert_eq!( + std::env::current_dir().expect("child cwd"), + std::path::PathBuf::from( + std::env::var_os("ASTRID_EXPECTED_CWD").expect("expected cwd") + ) + ); + + let mut input = String::new(); + std::io::stdin() + .read_to_string(&mut input) + .expect("read stdin"); + assert_eq!(input, "stdin with spaces \" and slash\\"); + std::io::stdout() + .write_all(b"astrid-stdout") + .expect("write stdout"); + std::io::stderr() + .write_all(b"astrid-stderr") + .expect("write stderr"); + std::process::exit(37); + }, + "tree-root" => { + let executable = std::env::current_exe().expect("current test executable"); + let heartbeat = std::env::var_os("ASTRID_HEARTBEAT").expect("heartbeat path"); + let mut child = Command::new(executable); + child + .arg("windows_process_probe_child") + .arg("--nocapture") + .env("ASTRID_WINDOWS_PROCESS_PROBE", "tree-leaf") + .env("ASTRID_HEARTBEAT", heartbeat) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + let child = child.spawn().expect("spawn tree leaf"); + std::fs::write( + std::env::var_os("ASTRID_LEAF_PID").expect("leaf pid path"), + child.id().to_string(), + ) + .expect("write leaf pid"); + loop { + std::thread::sleep(Duration::from_secs(60)); + } + }, + "tree-root-immediate" => { + let executable = std::env::current_exe().expect("current test executable"); + Command::new(executable) + .arg("windows_process_probe_child") + .arg("--nocapture") + .env("ASTRID_WINDOWS_PROCESS_PROBE", "tree-leaf") + .env( + "ASTRID_HEARTBEAT", + std::env::var_os("ASTRID_HEARTBEAT").expect("heartbeat path"), + ) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("spawn immediate tree leaf"); + loop { + std::thread::sleep(Duration::from_secs(60)); + } + }, + "tree-root-exit" => { + let executable = std::env::current_exe().expect("current test executable"); + let heartbeat = std::env::var_os("ASTRID_HEARTBEAT").expect("heartbeat path"); + let mut child = std::process::Command::new(executable); + child + .arg("windows_process_probe_child") + .arg("--nocapture") + .env("ASTRID_WINDOWS_PROCESS_PROBE", "tree-leaf") + .env("ASTRID_HEARTBEAT", heartbeat) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + child.spawn().expect("spawn tree leaf"); + std::thread::sleep(Duration::from_millis(250)); + }, + "tree-root-exit-inherit-stdio" => { + let executable = std::env::current_exe().expect("current test executable"); + let child = Command::new(executable) + .arg("windows_process_probe_child") + .arg("--nocapture") + .env("ASTRID_WINDOWS_PROCESS_PROBE", "tree-leaf") + .env( + "ASTRID_HEARTBEAT", + std::env::var_os("ASTRID_HEARTBEAT").expect("heartbeat path"), + ) + .spawn() + .expect("spawn inherited-stdio leaf"); + std::fs::write( + std::env::var_os("ASTRID_LEAF_PID").expect("leaf pid path"), + child.id().to_string(), + ) + .expect("write inherited-stdio leaf pid"); + }, + "tree-leaf" => { + let heartbeat = std::path::PathBuf::from( + std::env::var_os("ASTRID_HEARTBEAT").expect("heartbeat path"), + ); + let mut counter = 0u64; + loop { + counter = counter.wrapping_add(1); + std::fs::write(&heartbeat, counter.to_string()).expect("write heartbeat"); + std::thread::sleep(Duration::from_millis(25)); + } + }, + _ => {}, + } + } + + #[cfg(windows)] + #[tokio::test(flavor = "multi_thread")] + async fn windows_native_args_env_cwd_stdio_and_exit_are_deterministic() { + use std::process::Stdio; + use tokio::io::AsyncWriteExt as _; + + let executable = std::env::current_exe().expect("current test executable"); + let cwd = tempfile::tempdir().expect("temp cwd"); + let mut command = Command::new(executable); + command + .arg("windows_process_probe_child") + .arg("--nocapture") + .arg("--skip") + .arg("argument with spaces") + .arg("--skip") + .arg("quote\"inside") + .arg("--skip") + .arg("trailing\\") + .arg("--skip") + .arg("slashes\\\\\\\"quote") + .current_dir(cwd.path()) + .env("ASTRID_HOST_SECRET", "must-not-leak") + .env_clear() + .env( + "SystemRoot", + std::env::var_os("SystemRoot").expect("SystemRoot"), + ) + .env("ASTRID_WINDOWS_PROCESS_PROBE", "stdio") + .env("ASTRID_WINDOWS_EDGE", "value with spaces \" and trailing\\") + .env("ASTRID_EXPECTED_CWD", cwd.path()) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + configure_process_group(&mut command); + + let mut command = tokio::process::Command::from(command); + command.kill_on_drop(true); + let mut child = command.spawn().expect("spawn suspended probe"); + let _tree = ProcessTree::attach(&child).expect("assign Job before resume"); + child + .stdin + .take() + .expect("piped stdin") + .write_all(b"stdin with spaces \" and slash\\") + .await + .expect("write probe stdin"); + let output = child.wait_with_output().await.expect("wait probe"); + assert_eq!(output.status.code(), Some(37)); + assert!( + String::from_utf8_lossy(&output.stdout).contains("astrid-stdout"), + "stdout was {}", + String::from_utf8_lossy(&output.stdout) + ); + assert!( + String::from_utf8_lossy(&output.stderr).contains("astrid-stderr"), + "stderr was {}", + String::from_utf8_lossy(&output.stderr) + ); + } + + #[cfg(windows)] + #[tokio::test(flavor = "multi_thread")] + async fn windows_forced_cancellation_terminates_descendants() { + use std::process::Stdio; + use std::time::{Duration, Instant}; + + let executable = std::env::current_exe().expect("current test executable"); + let temp = tempfile::tempdir().expect("temp dir"); + let heartbeat = temp.path().join("heartbeat"); + let leaf_pid = temp.path().join("leaf-pid"); + let mut command = Command::new(executable); + command + .arg("windows_process_probe_child") + .arg("--nocapture") + .env("ASTRID_WINDOWS_PROCESS_PROBE", "tree-root") + .env("ASTRID_HEARTBEAT", &heartbeat) + .env("ASTRID_LEAF_PID", &leaf_pid) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + configure_process_group(&mut command); + let mut command = tokio::process::Command::from(command); + command.kill_on_drop(true); + let mut root = command.spawn().expect("spawn tree root"); + let tree = ProcessTree::attach(&root).expect("attach process tree"); + + wait_for_file(&heartbeat, Duration::from_secs(10)); + wait_for_file(&leaf_pid, Duration::from_secs(10)); + tree.terminate(Termination::Force) + .expect("terminate process tree"); + tree.terminate(Termination::Force) + .expect("repeat termination is idempotent"); + let deadline = Instant::now() + Duration::from_secs(10); + while Instant::now() < deadline { + if root.try_wait().expect("poll root").is_some() { + break; + } + std::thread::sleep(Duration::from_millis(25)); + } + assert!(root.try_wait().expect("final root poll").is_some()); + + std::thread::sleep(Duration::from_millis(250)); + let first = std::fs::read_to_string(&heartbeat).expect("first heartbeat"); + std::thread::sleep(Duration::from_millis(250)); + let second = std::fs::read_to_string(&heartbeat).expect("second heartbeat"); + assert_eq!( + first, second, + "descendant kept running after tree cancellation" + ); + + fn wait_for_file(path: &std::path::Path, timeout: Duration) { + let deadline = Instant::now() + timeout; + while Instant::now() < deadline { + if path.is_file() { + return; + } + std::thread::sleep(Duration::from_millis(25)); + } + panic!("timed out waiting for {}", path.display()); + } + } + + #[cfg(windows)] + #[tokio::test(flavor = "multi_thread")] + async fn windows_job_owns_zero_sleep_immediate_descendant() { + use std::process::Stdio; + use std::time::Duration; + + let executable = std::env::current_exe().expect("current test executable"); + let temp = tempfile::tempdir().expect("temp dir"); + let heartbeat = temp.path().join("immediate-heartbeat"); + let mut command = Command::new(executable); + command + .arg("windows_process_probe_child") + .arg("--nocapture") + .env("ASTRID_WINDOWS_PROCESS_PROBE", "tree-root-immediate") + .env("ASTRID_HEARTBEAT", &heartbeat) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + configure_process_group(&mut command); + let mut command = tokio::process::Command::from(command); + command.kill_on_drop(true); + let mut root = command.spawn().expect("spawn suspended immediate root"); + let tree = ProcessTree::attach(&root).expect("assign Job before resume"); + + wait_for_file(&heartbeat, Duration::from_secs(10)); + tree.terminate(Termination::Force) + .expect("terminate owned immediate tree"); + root.wait().await.expect("reap immediate root"); + std::thread::sleep(Duration::from_millis(150)); + let stopped = std::fs::read_to_string(&heartbeat).expect("stopped heartbeat"); + std::thread::sleep(Duration::from_millis(250)); + assert_eq!( + stopped, + std::fs::read_to_string(&heartbeat).expect("final heartbeat"), + "zero-sleep descendant escaped Job ownership" + ); + + fn wait_for_file(path: &std::path::Path, timeout: Duration) { + let deadline = std::time::Instant::now() + timeout; + while std::time::Instant::now() < deadline { + if path.is_file() { + return; + } + std::thread::sleep(Duration::from_millis(10)); + } + panic!("timed out waiting for {}", path.display()); + } + } + + #[cfg(windows)] + #[tokio::test(flavor = "multi_thread")] + async fn windows_assignment_failure_never_runs_child_code() { + use std::process::Stdio; + + let executable = std::env::current_exe().expect("current test executable"); + let temp = tempfile::tempdir().expect("temp dir"); + let heartbeat = temp.path().join("must-not-start"); + let mut command = Command::new(executable); + command + .arg("windows_process_probe_child") + .arg("--nocapture") + .env("ASTRID_WINDOWS_PROCESS_PROBE", "tree-root-immediate") + .env("ASTRID_HEARTBEAT", &heartbeat) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + configure_process_group(&mut command); + let mut command = tokio::process::Command::from(command); + command.kill_on_drop(true); + let mut root = command.spawn().expect("spawn suspended root"); + + ProcessTree::inject_assignment_failure(&root) + .expect_err("injected assignment failure must fail attach"); + root.start_kill().expect("kill still-suspended root"); + root.wait().await.expect("reap still-suspended root"); + assert!( + !heartbeat.exists(), + "child code ran before successful Job assignment" + ); + } + + #[cfg(windows)] + #[tokio::test(flavor = "multi_thread")] + async fn windows_job_cleans_descendants_after_root_exits_first() { + use std::process::Stdio; + use std::time::Duration; + + let executable = std::env::current_exe().expect("current test executable"); + let temp = tempfile::tempdir().expect("temp dir"); + let heartbeat = temp.path().join("heartbeat"); + let mut command = std::process::Command::new(executable); + command + .arg("windows_process_probe_child") + .arg("--nocapture") + .env("ASTRID_WINDOWS_PROCESS_PROBE", "tree-root-exit") + .env("ASTRID_HEARTBEAT", &heartbeat) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + configure_process_group(&mut command); + let mut command = tokio::process::Command::from(command); + command.kill_on_drop(true); + let mut root = command.spawn().expect("spawn root"); + let tree = ProcessTree::attach(&root).expect("attach process tree"); + wait_for_file(&heartbeat, Duration::from_secs(10)); + root.wait().await.expect("root exits first"); + + let before = std::fs::read_to_string(&heartbeat).expect("heartbeat before cleanup"); + std::thread::sleep(Duration::from_millis(150)); + let alive = std::fs::read_to_string(&heartbeat).expect("heartbeat while job owned"); + assert_ne!( + before, alive, + "descendant should still be alive before cleanup" + ); + + tree.terminate(Termination::Force) + .expect("terminate descendants"); + std::thread::sleep(Duration::from_millis(250)); + let stopped = std::fs::read_to_string(&heartbeat).expect("stopped heartbeat"); + std::thread::sleep(Duration::from_millis(250)); + assert_eq!( + stopped, + std::fs::read_to_string(&heartbeat).expect("final heartbeat") + ); + + fn wait_for_file(path: &std::path::Path, timeout: Duration) { + let deadline = std::time::Instant::now() + timeout; + while std::time::Instant::now() < deadline { + if path.is_file() { + return; + } + std::thread::sleep(Duration::from_millis(25)); + } + panic!("timed out waiting for {}", path.display()); + } + } +} diff --git a/crates/astrid-capsule/src/engine/wasm/host/process/support.rs b/crates/astrid-capsule/src/engine/wasm/host/process/support.rs new file mode 100644 index 000000000..892f50292 --- /dev/null +++ b/crates/astrid-capsule/src/engine/wasm/host/process/support.rs @@ -0,0 +1,42 @@ +//! Small state-derived helpers shared by the process host surfaces. + +use crate::engine::wasm::bindings::astrid::process1_1_0::host::EnvVar; +use crate::engine::wasm::host_state::HostState; + +/// Extract the call id from an authenticated tool invocation, when present. +pub(super) fn extract_call_id(state: &HostState) -> Option { + state.caller_context.as_ref().and_then(|message| { + if let astrid_events::ipc::IpcPayload::ToolExecuteRequest { call_id, .. } = &message.payload + { + Some(call_id.clone()) + } else { + None + } + }) +} + +/// Summarize environment keys for audit without recording their values. +pub(super) fn env_summary(env: &[EnvVar]) -> String { + env.iter() + .map(|entry| entry.key.as_str()) + .collect::>() + .join(",") +} + +/// Return only an invocation-authenticated principal. Persistent processes +/// must not share the capsule-owner fallback namespace. +pub(super) fn authenticated_principal( + state: &HostState, +) -> Option { + state + .caller_context + .as_ref() + .and_then(|message| message.principal.as_deref()) + .and_then(|principal| astrid_core::principal::PrincipalId::new(principal).ok()) +} + +pub(super) fn process_sandbox_policy(state: &HostState) -> astrid_workspace::SandboxPolicy { + state + .process_sandbox_policy + .unwrap_or_else(astrid_workspace::SandboxPolicy::from_env) +} diff --git a/crates/astrid-capsule/src/engine/wasm/host/process/tracker.rs b/crates/astrid-capsule/src/engine/wasm/host/process/tracker.rs index 493a95801..9312d3fdd 100644 --- a/crates/astrid-capsule/src/engine/wasm/host/process/tracker.rs +++ b/crates/astrid-capsule/src/engine/wasm/host/process/tracker.rs @@ -3,22 +3,70 @@ //! the matching child processes. use std::collections::{HashMap, HashSet}; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, Weak}; #[cfg(unix)] use std::time::Duration; -#[cfg(unix)] +#[cfg(not(unix))] use tracing::warn; /// Grace period between SIGINT and SIGKILL when cancelling processes. #[cfg(unix)] const SIGKILL_GRACE_PERIOD: Duration = Duration::from_secs(2); +#[derive(Debug)] +struct TrackedProcess { + call_id: Option, + owner: TrackedOwner, +} + +#[derive(Debug)] +enum TrackedOwner { + /// Compatibility entry created through the original public PID API. + Legacy, + /// Internal process-tree owner used by the hardened host implementation. + Tree { + identity: u64, + tree: Weak, + }, +} + +#[derive(Debug)] +struct CancellationTarget { + pid: u32, + #[cfg(unix)] + identity: Option, + tree: Option>, +} + +impl TrackedProcess { + fn target(&self, pid: u32) -> Option { + match &self.owner { + TrackedOwner::Legacy => Some(CancellationTarget { + pid, + #[cfg(unix)] + identity: None, + tree: None, + }), + TrackedOwner::Tree { identity, tree } => { + #[cfg(not(unix))] + let _ = identity; + tree.upgrade().map(|tree| CancellationTarget { + pid, + #[cfg(unix)] + identity: Some(*identity), + tree: Some(tree), + }) + }, + } + } +} + /// Tracks active child process PIDs for cancellation, with optional /// call_id association for multi-session scoping. #[derive(Debug, Default)] pub struct ProcessTracker { - active_pids: Arc>>>, + active_pids: Arc>>, } impl ProcessTracker { @@ -29,14 +77,45 @@ impl ProcessTracker { } /// Register a child process PID with an optional call_id. + /// + /// This preserves the original public API for external callers. The + /// process host itself uses [`Self::register_tree`] so Windows cancellation + /// owns the whole descendant tree and PID reuse is identity-checked. pub fn register(&self, pid: u32, call_id: Option) { if pid == 0 { - return; // Guard: PID 0 means "no process" on some platforms. + return; } self.active_pids .lock() .expect("process tracker lock poisoned") - .insert(pid, call_id); + .insert( + pid, + TrackedProcess { + call_id, + owner: TrackedOwner::Legacy, + }, + ); + } + + /// Register a stable process-tree owner with an optional call_id. + pub(super) fn register_tree( + &self, + tree: &Arc, + call_id: Option, + ) { + self.active_pids + .lock() + .expect("process tracker lock poisoned") + .insert( + tree.pid(), + TrackedProcess { + call_id, + owner: TrackedOwner::Tree { + identity: tree.identity(), + tree: Arc::downgrade(tree), + }, + }, + ); } /// Whether any child process is currently registered as running. @@ -47,11 +126,15 @@ impl ProcessTracker { /// the tree under it would corrupt or destroy its work. #[must_use] pub fn has_active(&self) -> bool { - !self + let mut active = self .active_pids .lock() - .expect("process tracker lock poisoned") - .is_empty() + .expect("process tracker lock poisoned"); + active.retain(|_, tracked| match &tracked.owner { + TrackedOwner::Legacy => true, + TrackedOwner::Tree { tree, .. } => tree.strong_count() > 0, + }); + !active.is_empty() } /// Unregister a child process PID (process has exited). @@ -62,6 +145,29 @@ impl ProcessTracker { .remove(&pid); } + /// Unregister only if the stable identity still matches this tree owner. + pub(super) fn unregister_tree(&self, tree: &super::platform::ProcessTree) { + self.unregister_identity(tree.pid(), tree.identity()); + } + + fn unregister_identity(&self, pid: u32, identity: u64) { + let mut active = self + .active_pids + .lock() + .expect("process tracker lock poisoned"); + if active.get(&pid).is_some_and(|tracked| { + matches!( + &tracked.owner, + TrackedOwner::Tree { + identity: current, + .. + } if *current == identity + ) + }) { + active.remove(&pid); + } + } + /// Cancel processes matching the given call_ids. /// /// Kills processes whose call_id matches one of the provided IDs, @@ -71,74 +177,117 @@ impl ProcessTracker { return; } let call_id_set: HashSet<&String> = call_ids.iter().collect(); - let pids: Vec = self + let targets: Vec = self .active_pids .lock() .expect("process tracker lock poisoned") .iter() - .filter_map(|(&pid, stored_call_id)| match stored_call_id { - None => Some(pid), - Some(id) => call_id_set.contains(id).then_some(pid), + .filter_map(|(&pid, tracked)| match &tracked.call_id { + None => tracked.target(pid), + Some(id) if call_id_set.contains(id) => tracked.target(pid), + Some(_) => None, }) .collect(); - self.signal_pids(&pids, handle); + self.signal_targets(&targets, handle); } - /// Send SIGINT to all tracked processes, then SIGKILL after a grace - /// period. Used for capsule-level shutdown. + /// Cancel all tracked processes. Unix receives SIGINT then SIGKILL after + /// a grace period; Windows terminates each descendant tree immediately. pub fn cancel_all(&self, handle: &tokio::runtime::Handle) { - let pids: Vec = self + let targets: Vec = self .active_pids .lock() .expect("process tracker lock poisoned") - .keys() - .copied() + .iter() + .filter_map(|(&pid, tracked)| tracked.target(pid)) .collect(); - self.signal_pids(&pids, handle); + self.signal_targets(&targets, handle); } - fn signal_pids(&self, pids: &[u32], handle: &tokio::runtime::Handle) { - if pids.is_empty() { + fn signal_targets(&self, targets: &[CancellationTarget], handle: &tokio::runtime::Handle) { + if targets.is_empty() { return; } #[cfg(unix)] { - for &pid in pids { - let Some(raw) = i32::try_from(pid).ok() else { - warn!(pid, "PID overflows i32, skipping signal"); - continue; - }; - let _ = nix::sys::signal::kill( - nix::unistd::Pid::from_raw(raw), - nix::sys::signal::Signal::SIGINT, - ); + for target in targets { + if let Some(tree) = &target.tree { + let _ = super::platform::signal_root_process( + tree, + crate::engine::wasm::bindings::astrid::process1_1_0::host::ProcessSignal::Int, + ); + } else if let Ok(raw) = i32::try_from(target.pid) { + let _ = nix::sys::signal::kill( + nix::unistd::Pid::from_raw(raw), + nix::sys::signal::Signal::SIGINT, + ); + } } let tracker = self.active_pids.clone(); - let target_pids: Vec = pids.to_vec(); + let targets: Vec<(u32, Option)> = targets + .iter() + .map(|target| (target.pid, target.identity)) + .collect(); handle.spawn(async move { tokio::time::sleep(SIGKILL_GRACE_PERIOD).await; - let still_active = tracker.lock().expect("process tracker lock poisoned"); - for pid in target_pids { - if !still_active.contains_key(&pid) { - continue; + for (pid, identity) in targets { + let still_current = tracker + .lock() + .expect("process tracker lock poisoned") + .get(&pid) + .is_some_and(|tracked| match (&tracked.owner, identity) { + (TrackedOwner::Legacy, None) => true, + ( + TrackedOwner::Tree { + identity: current, .. + }, + Some(expected), + ) => *current == expected, + _ => false, + }); + if still_current && let Ok(raw) = i32::try_from(pid) { + let _ = nix::sys::signal::kill( + nix::unistd::Pid::from_raw(raw), + nix::sys::signal::Signal::SIGKILL, + ); } - let Some(raw) = i32::try_from(pid).ok() else { - continue; - }; - let _ = nix::sys::signal::kill( - nix::unistd::Pid::from_raw(raw), - nix::sys::signal::Signal::SIGKILL, - ); } }); } - #[cfg(not(unix))] + #[cfg(windows)] + { + let _ = handle; + for target in targets { + let Some(tree) = &target.tree else { + warn!( + pid = target.pid, + "legacy PID-only process tracking cannot own a Windows descendant tree" + ); + continue; + }; + if let Err(error) = tree.terminate(super::platform::Termination::Force) { + warn!( + pid = tree.pid(), + ?error, + "failed to terminate Windows process tree" + ); + } + } + } + + #[cfg(not(any(unix, windows)))] { - let _ = (pids, handle); + let _ = handle; + for target in targets { + warn!( + pid = target.pid, + "process cancellation unsupported on this platform; leaving tracker entry active" + ); + } } } @@ -168,11 +317,37 @@ mod tests { //! `spawn_background` relies on. use super::*; + fn register_tree_sentinel( + tracker: &ProcessTracker, + pid: u32, + identity: u64, + call_id: Option, + ) { + if pid == 0 { + return; + } + tracker + .active_pids + .lock() + .expect("process tracker lock poisoned") + .insert( + pid, + TrackedProcess { + call_id, + owner: TrackedOwner::Tree { + identity, + tree: Weak::new(), + }, + }, + ); + } + #[test] fn register_adds_pid() { let t = ProcessTracker::new(); t.register(42, None); assert_eq!(t.active_pids_snapshot(), vec![42]); + assert!(t.has_active()); } #[test] @@ -212,4 +387,25 @@ mod tests { t.unregister(42); assert!(t.active_pids_snapshot().is_empty()); } + + #[test] + fn stale_unregister_does_not_remove_reused_pid_owner() { + let t = ProcessTracker::new(); + register_tree_sentinel(&t, 42, 100, Some("old".into())); + register_tree_sentinel(&t, 42, 200, Some("new".into())); + + t.unregister_identity(42, 100); + assert_eq!(t.active_pids_snapshot(), vec![42]); + assert_eq!( + t.active_pids + .lock() + .expect("process tracker lock poisoned") + .get(&42) + .and_then(|tracked| match &tracked.owner { + TrackedOwner::Tree { identity, .. } => Some(*identity), + TrackedOwner::Legacy => None, + }), + Some(200) + ); + } } diff --git a/crates/astrid-capsule/src/engine/wasm/host_state.rs b/crates/astrid-capsule/src/engine/wasm/host_state.rs index b1330c204..a0b2cd4a5 100644 --- a/crates/astrid-capsule/src/engine/wasm/host_state.rs +++ b/crates/astrid-capsule/src/engine/wasm/host_state.rs @@ -262,6 +262,10 @@ pub struct HostState { /// promote/rollback gate. Threaded into `SandboxCommand::wrap_with_injections` /// as `extra_masks` on every spawn. Empty for git-managed / No-CoW workspaces. pub spawn_mask_paths: Vec, + /// Optional per-state override for unavailable native process sandboxes. + /// Production states leave this `None` and read `ASTRID_SANDBOX_POLICY`; + /// tests and embedders can make the operator choice explicit. + pub process_sandbox_policy: Option, /// The Virtual File System (VFS) instance for this plugin. pub vfs: Arc, /// The root capability handle for the VFS. diff --git a/crates/astrid-capsule/src/engine/wasm/host_state_hook.rs b/crates/astrid-capsule/src/engine/wasm/host_state_hook.rs index b5c591fb5..21302050b 100644 --- a/crates/astrid-capsule/src/engine/wasm/host_state_hook.rs +++ b/crates/astrid-capsule/src/engine/wasm/host_state_hook.rs @@ -65,6 +65,7 @@ impl HostState { workspace_root, // Hooks run a transient one-shot on a plain HostVfs with no CoW. spawn_mask_paths: Vec::new(), + process_sandbox_policy: None, vfs, vfs_root_handle, // Hooks intentionally do not support home:// or /tmp access — they run diff --git a/crates/astrid-capsule/src/engine/wasm/mod.rs b/crates/astrid-capsule/src/engine/wasm/mod.rs index eb5e01aa0..9dd62f345 100644 --- a/crates/astrid-capsule/src/engine/wasm/mod.rs +++ b/crates/astrid-capsule/src/engine/wasm/mod.rs @@ -1841,6 +1841,7 @@ impl ExecutionEngine for WasmEngine { // see ONE filesystem (see the VFS-branch selection above). workspace_root: effective_workspace_root.clone(), spawn_mask_paths: spawn_mask_paths.clone(), + process_sandbox_policy: None, vfs: Arc::clone(&workspace_vfs), vfs_root_handle: root_handle.clone(), home: None, @@ -2964,6 +2965,7 @@ async fn build_lifecycle_host_state( workspace_root: cfg.workspace_root.clone(), // Lifecycle hooks run on a plain HostVfs with no CoW, so nothing to mask. spawn_mask_paths: Vec::new(), + process_sandbox_policy: None, vfs: Arc::new(vfs), vfs_root_handle: root_handle, home: home_mount, diff --git a/crates/astrid-capsule/src/engine/wasm/test_fixtures.rs b/crates/astrid-capsule/src/engine/wasm/test_fixtures.rs index f5d44d9a3..801f32ca6 100644 --- a/crates/astrid-capsule/src/engine/wasm/test_fixtures.rs +++ b/crates/astrid-capsule/src/engine/wasm/test_fixtures.rs @@ -95,6 +95,7 @@ pub(crate) fn minimal_host_state(rt: tokio::runtime::Handle) -> HostState { capsule_id: CapsuleId::from_static("test"), workspace_root: PathBuf::from("/tmp"), spawn_mask_paths: Vec::new(), + process_sandbox_policy: None, vfs: Arc::new(astrid_vfs::HostVfs::new()), vfs_root_handle: astrid_capabilities::DirHandle::new(), home: None, diff --git a/crates/astrid-core/src/local_transport.rs b/crates/astrid-core/src/local_transport.rs index 32a466061..42769756f 100644 --- a/crates/astrid-core/src/local_transport.rs +++ b/crates/astrid-core/src/local_transport.rs @@ -88,6 +88,37 @@ pub enum ConnectOutcome { Stale, } +/// Whether this build has a concrete host-local transport backend. +/// +/// Callers use this semantic capability check instead of inferring support +/// from the OS family. A Windows named-pipe backend can switch this alongside +/// its implementation without changing capsule-host policy code. +#[must_use] +pub const fn backend_available() -> bool { + cfg!(any(unix, windows)) +} + +#[cfg(test)] +mod availability_tests { + #[cfg(unix)] + #[test] + fn unix_backend_is_reported_available() { + assert!(super::backend_available()); + } + + #[cfg(windows)] + #[test] + fn windows_backend_is_reported_available() { + assert!(super::backend_available()); + } + + #[cfg(not(any(unix, windows)))] + #[test] + fn unsupported_host_reports_no_backend() { + assert!(!super::backend_available()); + } +} + /// Connect to a host-local endpoint. /// /// # Errors diff --git a/crates/astrid-kernel/src/audit_sink.rs b/crates/astrid-kernel/src/audit_sink.rs index 2bb7fd950..d5a89e4c2 100644 --- a/crates/astrid-kernel/src/audit_sink.rs +++ b/crates/astrid-kernel/src/audit_sink.rs @@ -144,6 +144,30 @@ impl KernelAuditSink { ), } } + + fn append_action( + &self, + principal: &PrincipalId, + action: AuditAction, + outcome: HostAuditOutcome<'_>, + ) { + let (proof, audit_outcome) = Self::to_proof_outcome(outcome); + let result = block_on_audit(self.audit_log.append_with_principal( + self.session_id.clone(), + principal.clone(), + action, + proof, + audit_outcome, + )); + if let Err(e) = result { + warn!( + security_event = true, + %principal, + error = %e, + "Failed to persist per-action audit entry — continuing" + ); + } + } } /// Drive an audit-log future to completion synchronously from the host-fn @@ -199,26 +223,24 @@ impl HostAuditSink for KernelAuditSink { event: HostAuditEvent<'_>, outcome: HostAuditOutcome<'_>, ) { - let action = Self::to_action(event); - let (proof, audit_outcome) = Self::to_proof_outcome(outcome); - // Native-only sync bridge onto the async audit log; see - // [`block_on_audit`]. Persistence failure degrades to "continue + - // alert", never a panic or a blocked host call. - let result = block_on_audit(self.audit_log.append_with_principal( - self.session_id.clone(), - principal.clone(), - action, - proof, - audit_outcome, - )); - if let Err(e) = result { - warn!( - security_event = true, - %principal, - error = %e, - "Failed to persist per-action audit entry — continuing" - ); - } + self.append_action(principal, Self::to_action(event), outcome); + } + + fn record_process_signal( + &self, + principal: &PrincipalId, + process: &str, + signal: &str, + outcome: HostAuditOutcome<'_>, + ) { + self.append_action( + principal, + AuditAction::ProcessSignal { + process: truncate_guest_str(process), + signal: truncate_guest_str(signal), + }, + outcome, + ); } } @@ -232,6 +254,62 @@ mod tests { PrincipalId::new("alice").expect("valid principal") } + fn assert_event_mappings(entries: &[astrid_audit::AuditEntry]) { + assert_eq!(entries.len(), 7, "all seven events must persist"); + for entry in entries { + assert_eq!( + entry.principal.as_ref(), + Some(&principal()), + "principal must be stamped" + ); + } + assert!(matches!( + (&entries[0].action, &entries[0].outcome), + (AuditAction::FileRead { path }, AuditOutcome::Success { .. }) if path == "/w/r" + )); + assert!(matches!( + ( + &entries[6].action, + &entries[6].authorization, + &entries[6].outcome + ), + ( + AuditAction::ProcessSignal { process, signal }, + AuthorizationProof::Denied { .. }, + AuditOutcome::Failure { .. } + ) if process == "persistent:0123456789abcdef" && signal == "hup" + )); + assert!(matches!( + (&entries[1].action, &entries[1].outcome), + (AuditAction::FileWrite { path, content_hash }, AuditOutcome::Failure { .. }) + if path == "/w/w" && *content_hash == ContentHash::zero() + )); + assert!(matches!( + &entries[2].action, + AuditAction::FileDelete { path } if path == "/w/d" + )); + assert!(matches!( + &entries[3].action, + AuditAction::NetConnect { host, port } if host == "example.com" && *port == 443 + )); + assert!(matches!( + &entries[4].action, + AuditAction::NetBind { addr } if addr == "127.0.0.1:0" + )); + assert!(matches!( + ( + &entries[5].action, + &entries[5].authorization, + &entries[5].outcome + ), + ( + AuditAction::ProcessSpawn { command }, + AuthorizationProof::Denied { .. }, + AuditOutcome::Failure { .. } + ) if command == "ls" + )); + } + /// Every event kind, including a denial, lands a principal-stamped, /// correctly-mapped entry, and the resulting chain still verifies. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -278,57 +356,18 @@ mod tests { HostAuditEvent::ProcessSpawn { command: "ls" }, HostAuditOutcome::Denied("not in host_process allowlist"), ); + sink.record_process_signal( + &p, + "persistent:0123456789abcdef", + "hup", + HostAuditOutcome::Denied("unsupported signal"), + ); let entries = log .get_principal_entries(&session, Some(&p)) .await .expect("read principal entries"); - assert_eq!(entries.len(), 6, "all six events must persist"); - - // Every entry is stamped with the acting principal. - for e in &entries { - assert_eq!(e.principal.as_ref(), Some(&p), "principal must be stamped"); - } - - // FileRead → success. - assert!(matches!( - (&entries[0].action, &entries[0].outcome), - (AuditAction::FileRead { path }, AuditOutcome::Success { .. }) if path == "/w/r" - )); - // FileWrite Failed → Failure + zero content hash placeholder. - assert!(matches!( - (&entries[1].action, &entries[1].outcome), - (AuditAction::FileWrite { path, content_hash }, AuditOutcome::Failure { .. }) - if path == "/w/w" && *content_hash == ContentHash::zero() - )); - // FileDelete → success. - assert!(matches!( - &entries[2].action, - AuditAction::FileDelete { path } if path == "/w/d" - )); - // NetConnect → success with host + port. - assert!(matches!( - &entries[3].action, - AuditAction::NetConnect { host, port } if host == "example.com" && *port == 443 - )); - // NetBind → success with addr. - assert!(matches!( - &entries[4].action, - AuditAction::NetBind { addr } if addr == "127.0.0.1:0" - )); - // ProcessSpawn Denied → Failure + Denied proof. - assert!(matches!( - ( - &entries[5].action, - &entries[5].authorization, - &entries[5].outcome - ), - ( - AuditAction::ProcessSpawn { command }, - AuthorizationProof::Denied { .. }, - AuditOutcome::Failure { .. } - ) if command == "ls" - )); + assert_event_mappings(&entries); // The signed hash chain remains valid after the high-frequency // appends. diff --git a/crates/astrid-mcp/src/client.rs b/crates/astrid-mcp/src/client.rs index f25994c6f..f447c0c1d 100644 --- a/crates/astrid-mcp/src/client.rs +++ b/crates/astrid-mcp/src/client.rs @@ -11,7 +11,7 @@ use tracing::{debug, info, warn}; use crate::capabilities::{CapabilitiesHandler, ServerNotice}; use crate::config::{ServerConfig, ServersConfig}; use crate::error::{McpError, McpResult}; -use crate::server::ServerManager; +use crate::server::{ServerConnectionError, ServerManager}; use crate::types::{ToolDefinition, ToolResult}; use tokio::sync::mpsc; @@ -118,6 +118,15 @@ impl McpClient { /// /// Returns an error if the server cannot be started or connected. pub async fn connect(&self, server_name: &str) -> McpResult<()> { + self.connect_classified(server_name) + .await + .map_err(ServerConnectionError::into_mcp_error) + } + + pub(crate) async fn connect_classified( + &self, + server_name: &str, + ) -> Result<(), ServerConnectionError> { // Register the server if not already running if !self.servers.is_running(server_name).await { self.servers.start(server_name).await?; @@ -125,7 +134,7 @@ impl McpClient { // Establish the actual MCP connection self.servers - .connect_server( + .connect_server_classified( server_name, self.capabilities.clone(), Some(self.notice_tx.clone()), @@ -145,10 +154,20 @@ impl McpClient { /// # Errors /// Returns an error if the server is already running or cannot be started. pub async fn connect_dynamic(&self, name: &str, config: ServerConfig) -> McpResult<()> { + self.connect_dynamic_classified(name, config) + .await + .map_err(ServerConnectionError::into_mcp_error) + } + + pub(crate) async fn connect_dynamic_classified( + &self, + name: &str, + config: ServerConfig, + ) -> Result<(), ServerConnectionError> { self.servers.add_server(name, config).await?; self.servers - .connect_server( + .connect_server_classified( name, self.capabilities.clone(), Some(self.notice_tx.clone()), @@ -295,7 +314,7 @@ impl McpClient { } /// Refresh the tools cache from all running servers. - async fn refresh_tools_cache(&self) -> McpResult<()> { + pub(crate) async fn refresh_tools_cache(&self) -> McpResult<()> { let tools = self.servers.all_tools().await; let mut cache = self.tools_cache.write().await; *cache = tools; diff --git a/crates/astrid-mcp/src/secure.rs b/crates/astrid-mcp/src/secure.rs index 69353ad78..a6d83cf1a 100644 --- a/crates/astrid-mcp/src/secure.rs +++ b/crates/astrid-mcp/src/secure.rs @@ -16,7 +16,7 @@ use tracing::{debug, warn}; use crate::client::McpClient; use crate::config::ServerConfig; use crate::error::{McpError, McpResult}; -use crate::server::ServerManager; +use crate::server::{ServerConnectionError, ServerManager}; use crate::types::{ToolDefinition, ToolResult}; /// Authorization result for a tool call. @@ -68,6 +68,31 @@ impl SecureMcpClient { } } + async fn audit_server_start( + &self, + name: &str, + transport: String, + authorization: AuthorizationProof, + outcome: AuditOutcome, + ) { + if let Err(error) = self + .audit + .append( + self.session_id.clone(), + AuditAction::ServerStarted { + name: name.to_string(), + transport, + binary_hash: None, + }, + authorization, + outcome, + ) + .await + { + warn!(server = name, error = %error, "Failed to audit server connection"); + } + } + /// Check authorization for a tool call. /// /// This is a read-only validation - no audit entry is written. The audit @@ -280,34 +305,46 @@ impl SecureMcpClient { /// /// Returns an error if the server cannot be connected. pub async fn connect(&self, server_name: &str) -> McpResult<()> { - self.client.connect(server_name).await?; - - // Get the actual transport type for logging let transport = self .client .server_manager() .get_config(server_name) .map_or_else(|| "unknown".to_string(), |c| c.transport.to_string()); - - // Log server start - if let Err(e) = self - .audit - .append( - self.session_id.clone(), - AuditAction::ServerStarted { - name: server_name.to_string(), - transport, - binary_hash: None, - }, - AuthorizationProof::System { - reason: "server connection".to_string(), - }, - AuditOutcome::success(), + if let Err(failure) = self.client.connect_classified(server_name).await { + let (authorization, reason, error) = match failure { + ServerConnectionError::SandboxPolicyDenied { reason, error } => ( + AuthorizationProof::Denied { + reason: reason.clone(), + }, + reason, + error, + ), + ServerConnectionError::Other(error) => ( + AuthorizationProof::System { + reason: "server connection failed".to_string(), + }, + error.to_string(), + error, + ), + }; + self.audit_server_start( + server_name, + transport, + authorization, + AuditOutcome::failure(reason), ) - .await - { - warn!(server = server_name, error = %e, "Failed to audit server connection"); + .await; + return Err(error); } + self.audit_server_start( + server_name, + transport, + AuthorizationProof::System { + reason: "server connection".to_string(), + }, + AuditOutcome::success(), + ) + .await; Ok(()) } @@ -358,26 +395,41 @@ impl SecureMcpClient { /// Returns an error if the server is already running or cannot be started. pub async fn connect_dynamic(&self, name: &str, config: ServerConfig) -> McpResult<()> { let transport = config.transport.to_string(); - self.client.connect_dynamic(name, config).await?; - - if let Err(e) = self - .audit - .append( - self.session_id.clone(), - AuditAction::ServerStarted { - name: name.to_string(), - transport, - binary_hash: None, - }, - AuthorizationProof::System { - reason: "dynamic server connection".to_string(), - }, - AuditOutcome::success(), + if let Err(failure) = self.client.connect_dynamic_classified(name, config).await { + let (authorization, reason, error) = match failure { + ServerConnectionError::SandboxPolicyDenied { reason, error } => ( + AuthorizationProof::Denied { + reason: reason.clone(), + }, + reason, + error, + ), + ServerConnectionError::Other(error) => ( + AuthorizationProof::System { + reason: "dynamic server connection failed".to_string(), + }, + error.to_string(), + error, + ), + }; + self.audit_server_start( + name, + transport, + authorization, + AuditOutcome::failure(reason), ) - .await - { - warn!(server = name, error = %e, "Failed to audit dynamic server connection"); + .await; + return Err(error); } + self.audit_server_start( + name, + transport, + AuthorizationProof::System { + reason: "dynamic server connection".to_string(), + }, + AuditOutcome::success(), + ) + .await; Ok(()) } @@ -460,50 +512,25 @@ impl SecureMcpClient { /// Connect to all auto-start servers. /// - /// Each successfully started server is audit-logged. + /// Every attempted server connection is audit-logged, including + /// fail-closed platform denials. /// /// # Errors /// /// Returns an error only if refreshing the tools cache fails. pub async fn connect_auto_servers(&self) -> McpResult { - // Snapshot before so we only audit newly started servers. - let before: std::collections::HashSet = - self.client.list_servers().await.into_iter().collect(); - - let count = self.client.connect_auto_servers().await?; - - // Log only the servers that were actually started by this call. - for name in self.client.list_servers().await { - if before.contains(&name) { - continue; - } - let transport = self - .client - .server_manager() - .get_config(&name) - .map_or_else(|| "unknown".to_string(), |c| c.transport.to_string()); - - if let Err(e) = self - .audit - .append( - self.session_id.clone(), - AuditAction::ServerStarted { - name: name.clone(), - transport, - binary_hash: None, - }, - AuthorizationProof::System { - reason: "auto-start server".to_string(), - }, - AuditOutcome::success(), - ) - .await - { - warn!(server = %name, error = %e, "Failed to audit auto-start server"); + let names = self.client.server_manager().list_auto_start_names(); + let mut connected = 0usize; + for name in names { + match self.connect(&name).await { + Ok(()) => connected = connected.saturating_add(1), + Err(error) => { + warn!(server = %name, error = %error, "Failed to auto-connect server"); + }, } } - - Ok(count) + self.client.refresh_tools_cache().await?; + Ok(connected) } /// Get the underlying MCP client. @@ -616,6 +643,192 @@ mod tests { assert!(secure.list_tools().await.unwrap().is_empty()); } + #[tokio::test] + async fn failed_native_server_start_is_audited() { + let mut config = crate::config::ServersConfig::default(); + config + .add(ServerConfig::stdio( + "denied-native", + "astrid-definitely-missing-native-server", + )) + .expect("add server config"); + let client = McpClient::with_config(config); + let capabilities = Arc::new(CapabilityStore::in_memory()); + let audit = Arc::new(AuditLog::in_memory(KeyPair::generate())); + let session_id = SessionId::new(); + let secure = + SecureMcpClient::new(client, capabilities, Arc::clone(&audit), session_id.clone()); + + assert!(secure.connect("denied-native").await.is_err()); + let entries = audit + .get_session_entries(&session_id) + .await + .expect("read audit entries"); + assert_eq!(entries.len(), 1); + assert!(matches!( + &entries[0].action, + AuditAction::ServerStarted { name, .. } if name == "denied-native" + )); + assert!(matches!( + &entries[0].outcome, + AuditOutcome::Failure { error } + if error.contains("astrid-definitely-missing-native-server") + )); + assert!(matches!( + &entries[0].authorization, + AuthorizationProof::System { reason } + if reason == "server connection failed" + )); + } + + #[cfg(windows)] + #[tokio::test] + async fn windows_untrusted_native_server_denial_is_audited() { + let mut config = crate::config::ServersConfig::default(); + config + .add(ServerConfig::stdio( + "untrusted-native", + std::env::current_exe() + .expect("current test executable") + .to_string_lossy() + .into_owned(), + )) + .expect("add server config"); + let manager = ServerManager::new(config) + .with_sandbox_policy(astrid_workspace::SandboxPolicy::Required); + let client = McpClient::new(manager); + let capabilities = Arc::new(CapabilityStore::in_memory()); + let audit = Arc::new(AuditLog::in_memory(KeyPair::generate())); + let session_id = SessionId::new(); + let secure = + SecureMcpClient::new(client, capabilities, Arc::clone(&audit), session_id.clone()); + + let error = secure + .connect("untrusted-native") + .await + .expect_err("Windows must deny an untrusted native server without a sandbox"); + assert!( + error.to_string().contains("OS-level sandbox unavailable"), + "unexpected denial: {error}" + ); + let entries = audit + .get_session_entries(&session_id) + .await + .expect("read audit entries"); + assert_eq!(entries.len(), 1); + assert!(matches!( + (&entries[0].authorization, &entries[0].outcome), + ( + AuthorizationProof::Denied { reason }, + AuditOutcome::Failure { error } + ) if reason.contains("OS-level sandbox unavailable") + && reason.contains("policy is `required`") + && error == reason + )); + } + + #[cfg(windows)] + #[tokio::test] + async fn windows_dynamic_native_sandbox_denial_is_audited_as_denied() { + let manager = ServerManager::new(crate::config::ServersConfig::default()) + .with_sandbox_policy(astrid_workspace::SandboxPolicy::Required); + let audit = Arc::new(AuditLog::in_memory(KeyPair::generate())); + let session_id = SessionId::new(); + let secure = SecureMcpClient::new( + McpClient::new(manager), + Arc::new(CapabilityStore::in_memory()), + Arc::clone(&audit), + session_id.clone(), + ); + let config = ServerConfig::stdio( + "dynamic-untrusted", + std::env::current_exe() + .expect("current test executable") + .to_string_lossy() + .into_owned(), + ); + + secure + .connect_dynamic("dynamic-untrusted", config) + .await + .expect_err("required sandbox must deny dynamic native server"); + let entries = audit + .get_session_entries(&session_id) + .await + .expect("read audit entries"); + assert!(matches!( + (&entries[0].authorization, &entries[0].outcome), + ( + AuthorizationProof::Denied { reason }, + AuditOutcome::Failure { error } + ) if reason.contains("policy is `required`") && error == reason + )); + } + + #[cfg(windows)] + #[test] + fn windows_mcp_probe_child() { + if let Some(path) = std::env::var_os("ASTRID_WINDOWS_MCP_SENTINEL") { + std::fs::write(path, b"started").expect("write MCP probe sentinel"); + } + } + + #[cfg(windows)] + #[tokio::test] + async fn windows_explicit_off_starts_native_server_and_audits_handshake_failure() { + let temp = tempfile::tempdir().expect("temp"); + let sentinel = temp.path().join("mcp-started"); + let mut server = ServerConfig::stdio( + "trusted-dev-native", + std::env::current_exe() + .expect("current test executable") + .to_string_lossy() + .into_owned(), + ); + server.args = vec![ + "windows_mcp_probe_child".to_string(), + "--nocapture".to_string(), + ]; + server.env.insert( + "ASTRID_WINDOWS_MCP_SENTINEL".to_string(), + sentinel.to_string_lossy().into_owned(), + ); + let mut config = crate::config::ServersConfig::default(); + config.add(server).expect("add server config"); + let manager = + ServerManager::new(config).with_sandbox_policy(astrid_workspace::SandboxPolicy::Off); + let client = McpClient::new(manager); + let capabilities = Arc::new(CapabilityStore::in_memory()); + let audit = Arc::new(AuditLog::in_memory(KeyPair::generate())); + let session_id = SessionId::new(); + let secure = + SecureMcpClient::new(client, capabilities, Arc::clone(&audit), session_id.clone()); + + secure + .connect("trusted-dev-native") + .await + .expect_err("probe is not an MCP server"); + assert_eq!( + std::fs::read(&sentinel).expect("MCP process started"), + b"started" + ); + let entries = audit + .get_session_entries(&session_id) + .await + .expect("read audit entries"); + assert_eq!(entries.len(), 1); + assert!(matches!( + &entries[0].action, + AuditAction::ServerStarted { name, .. } if name == "trusted-dev-native" + )); + assert!(matches!(&entries[0].outcome, AuditOutcome::Failure { .. })); + assert!(matches!( + &entries[0].authorization, + AuthorizationProof::System { reason } + if reason == "server connection failed" + )); + } + #[tokio::test] async fn test_secure_client_clone_shares_state() { let secure = make_secure_client(); diff --git a/crates/astrid-mcp/src/server.rs b/crates/astrid-mcp/src/server.rs index 1cec0e1b4..2f8299713 100644 --- a/crates/astrid-mcp/src/server.rs +++ b/crates/astrid-mcp/src/server.rs @@ -26,6 +26,50 @@ use tokio::sync::mpsc; /// Type alias for a running MCP client service. type McpService = RunningService; +/// Internal connection failure classification. +/// +/// The public MCP API continues to expose [`McpError`]. Secure callers use +/// this crate-private wrapper so a fail-closed sandbox-policy rejection can +/// be distinguished from an ordinary process or protocol failure for audit +/// authorization semantics. +pub(crate) enum ServerConnectionError { + SandboxPolicyDenied { reason: String, error: McpError }, + Other(McpError), +} + +impl ServerConnectionError { + pub(crate) fn into_mcp_error(self) -> McpError { + match self { + Self::SandboxPolicyDenied { error, .. } | Self::Other(error) => error, + } + } +} + +impl From for ServerConnectionError { + fn from(error: McpError) -> Self { + Self::Other(error) + } +} + +fn classify_sandbox_prefix_error( + name: &str, + policy: astrid_workspace::SandboxPolicy, + source: &std::io::Error, +) -> ServerConnectionError { + let reason = source.to_string(); + let error = McpError::ServerStartFailed { + name: name.to_string(), + reason: reason.clone(), + }; + if policy == astrid_workspace::SandboxPolicy::Required + && source.kind() == std::io::ErrorKind::Unsupported + { + ServerConnectionError::SandboxPolicyDenied { reason, error } + } else { + ServerConnectionError::Other(error) + } +} + /// A running MCP server instance. pub(crate) struct RunningServer { /// Server configuration. @@ -335,6 +379,17 @@ impl ServerManager { handler: Arc, notice_tx: Option>, ) -> McpResult<()> { + self.connect_server_classified(name, handler, notice_tx) + .await + .map_err(ServerConnectionError::into_mcp_error) + } + + pub(crate) async fn connect_server_classified( + &self, + name: &str, + handler: Arc, + notice_tx: Option>, + ) -> Result<(), ServerConnectionError> { let config = { let running = self.running.read().await; let server = running @@ -355,7 +410,8 @@ impl ServerManager { "SSE transport not yet supported; enable `transport-streamable-http-client` \ feature in rmcp" .to_string(), - )); + ) + .into()); }, } @@ -369,7 +425,7 @@ impl ServerManager { config: &ServerConfig, handler: Arc, notice_tx: Option>, - ) -> McpResult<()> { + ) -> Result<(), ServerConnectionError> { let command = config.command.as_ref().ok_or_else(|| { McpError::ConfigError(format!("No command specified for stdio server {name}")) })?; @@ -377,7 +433,7 @@ impl ServerManager { let mut cmd = if config.trusted { build_unsandboxed_command(name, command, config) } else { - self.build_sandboxed_command(name, command, config)? + self.build_sandboxed_command_classified(name, command, config)? }; // Redirect capsule stderr to a per-capsule daily log file if configured. @@ -452,14 +508,25 @@ impl ServerManager { /// /// Applies OS-level sandboxing (bwrap on Linux, sandbox-exec on macOS), /// scrubs inherited environment variables, and hides `~/.astrid/`. - #[allow(clippy::too_many_lines)] + #[cfg(test)] fn build_sandboxed_command( &self, name: &str, command: &str, config: &ServerConfig, ) -> McpResult { - use astrid_workspace::ProcessSandboxConfig; + self.build_sandboxed_command_classified(name, command, config) + .map_err(ServerConnectionError::into_mcp_error) + } + + #[allow(clippy::too_many_lines)] + fn build_sandboxed_command_classified( + &self, + name: &str, + command: &str, + config: &ServerConfig, + ) -> Result { + use astrid_workspace::{ProcessSandboxConfig, SandboxPolicy}; // config.cwd doubles as both the sandbox writable root and the process CWD. // When set, the sandboxed process can write to its own working directory. @@ -489,12 +556,13 @@ impl ServerManager { Self::validate_sandbox_path(&astrid_home, "astrid_home")?; // Build sandbox config + let sandbox_policy = self + .sandbox_policy_override + .unwrap_or_else(SandboxPolicy::from_env); let mut sandbox_config = ProcessSandboxConfig::new(&writable_root) + .with_policy(sandbox_policy) .with_network(config.allow_network) .with_hidden(astrid_home); - if let Some(policy) = self.sandbox_policy_override { - sandbox_config = sandbox_config.with_policy(policy); - } // Add config-specified extra paths. Validated for: // 1. Absolute (avoid ambiguity about which directory they resolve relative to) @@ -568,13 +636,9 @@ impl ServerManager { // // Under `Off` the call returns `Ok(None)` silently. Either // way we don't double-log here. - let sandbox_prefix = - sandbox_config - .sandbox_prefix() - .map_err(|e| McpError::ServerStartFailed { - name: name.to_string(), - reason: e.to_string(), - })?; + let sandbox_prefix = sandbox_config + .sandbox_prefix() + .map_err(|error| classify_sandbox_prefix_error(name, sandbox_policy, &error))?; // Build the command let mut cmd = if let Some(prefix) = sandbox_prefix { @@ -1033,6 +1097,23 @@ impl std::fmt::Debug for ServerManager { mod tests { use super::*; + #[cfg(unix)] + fn non_utf8_test_path() -> std::path::PathBuf { + use std::os::unix::ffi::OsStringExt; + + std::ffi::OsString::from_vec(b"/tmp/\xff\xfe/workspace".to_vec()).into() + } + + #[cfg(windows)] + fn non_utf8_test_path() -> std::path::PathBuf { + use std::os::windows::ffi::OsStringExt; + + // Keep the path absolute so validation reaches the UTF-8 boundary. A + // lone UTF-16 surrogate cannot be represented as UTF-8. + std::ffi::OsString::from_wide(&[u16::from(b'C'), u16::from(b':'), u16::from(b'\\'), 0xD800]) + .into() + } + #[tokio::test] async fn test_server_manager_creation() { let configs = ServersConfig::default(); @@ -1042,6 +1123,36 @@ mod tests { assert!(manager.list_running().await.is_empty()); } + #[test] + fn required_sandbox_unavailability_is_classified_as_policy_denial() { + let failure = classify_sandbox_prefix_error( + "native", + astrid_workspace::SandboxPolicy::Required, + &std::io::Error::new(std::io::ErrorKind::Unsupported, "sandbox unavailable"), + ); + + assert!(matches!( + failure, + ServerConnectionError::SandboxPolicyDenied { reason, .. } + if reason == "sandbox unavailable" + )); + } + + #[test] + fn required_sandbox_configuration_error_is_not_policy_denial() { + let failure = classify_sandbox_prefix_error( + "native", + astrid_workspace::SandboxPolicy::Required, + &std::io::Error::new(std::io::ErrorKind::InvalidInput, "invalid sandbox path"), + ); + + assert!(matches!( + failure, + ServerConnectionError::Other(McpError::ServerStartFailed { name, reason }) + if name == "native" && reason == "invalid sandbox path" + )); + } + #[tokio::test] async fn test_server_not_found() { let configs = ServersConfig::default(); @@ -1549,12 +1660,8 @@ mod tests { #[test] fn test_validate_sandbox_path_rejects_non_utf8() { - use std::ffi::OsStr; - use std::os::unix::ffi::OsStrExt; - - let bad_bytes: &[u8] = b"/tmp/\xff\xfe/workspace"; - let bad_path = std::path::Path::new(OsStr::from_bytes(bad_bytes)); - let result = ServerManager::validate_sandbox_path(bad_path, "test_field"); + let bad_path = non_utf8_test_path(); + let result = ServerManager::validate_sandbox_path(&bad_path, "test_field"); assert!( matches!(result, Err(McpError::ConfigError(ref msg)) if msg.contains("not valid UTF-8")), "non-UTF-8 path should be rejected, got: {result:?}" @@ -1580,14 +1687,8 @@ mod tests { #[test] fn test_build_sandboxed_command_rejects_non_utf8_workspace_root() { - use std::ffi::OsStr; - use std::os::unix::ffi::OsStrExt; - - let bad_bytes: &[u8] = b"/tmp/\xff\xfe/workspace"; - let bad_path = std::path::PathBuf::from(OsStr::from_bytes(bad_bytes)); - let configs = ServersConfig::default(); - let manager = ServerManager::new(configs).with_workspace_root(bad_path); + let manager = ServerManager::new(configs).with_workspace_root(non_utf8_test_path()); let config = ServerConfig::stdio("test", "echo"); let result = manager.build_sandboxed_command("test", "echo", &config); diff --git a/crates/astrid-workspace/src/sandbox/mod.rs b/crates/astrid-workspace/src/sandbox/mod.rs index 73d85fcd1..f467d14ab 100644 --- a/crates/astrid-workspace/src/sandbox/mod.rs +++ b/crates/astrid-workspace/src/sandbox/mod.rs @@ -12,6 +12,7 @@ mod seatbelt; /// /// Rejects relative paths, non-UTF-8, double-quote, backslash, and null byte - /// all of which can break or bypass sandbox profile syntax. +#[cfg(any(target_os = "linux", target_os = "macos"))] fn validate_sandbox_str<'a>(path: &'a Path, label: &str) -> io::Result<&'a str> { if !path.is_absolute() { return Err(io::Error::new( @@ -251,261 +252,326 @@ impl SandboxCommand { extra_write_paths: &[PathBuf], clear_env: bool, ) -> io::Result { - // Validate on all platforms for defense in depth and API consistency. - // On macOS the validated string is needed for SBPL interpolation. - // On Linux bwrap passes paths as argv entries (no injection risk), - // but we still reject unsafe paths at the API boundary. - let _ = validate_sandbox_str(worktree_path, "worktree path")?; - for inj in injections { - let _ = validate_sandbox_str(&inj.source, "injection source")?; - let _ = validate_sandbox_str(&inj.target, "injection target")?; - } - for path in extra_read_paths { - let _ = validate_sandbox_str(path, "process read path")?; - if !path.exists() { - return Err(io::Error::new( - io::ErrorKind::NotFound, - format!("process read path does not exist: {}", path.display()), - )); - } - } - for path in extra_write_paths { - let _ = validate_sandbox_str(path, "process write path")?; - if !path.exists() { - return Err(io::Error::new( - io::ErrorKind::NotFound, - format!("process write path does not exist: {}", path.display()), - )); - } - } + Self::wrap_with_process_paths_and_policy( + inner_cmd, + worktree_path, + injections, + extra_masks, + extra_read_paths, + extra_write_paths, + clear_env, + SandboxPolicy::Required, + ) + } - // Every caller-supplied mask names copy-on-write bookkeeping the child - // must not reach (the overlayfs upper/work, or the APFS pristine). Each is - // validated exactly like the worktree and injection paths — absolute, - // UTF-8, SBPL-safe — because on macOS it is interpolated into the Seatbelt - // profile; then it must EXIST, since a path that does not exist is a wiring - // bug, not a no-op (silently skipping it leaves the child un-denied). The - // deny is security-critical, so either failure fails the spawn closed. - for masked in extra_masks { - let _ = validate_sandbox_str(masked, "workspace CoW mask")?; - if !masked.exists() { - return Err(io::Error::new( - io::ErrorKind::NotFound, - format!( - "workspace CoW mask path does not exist: {} — refusing to spawn \ - a child without the intended copy-on-write deny", - masked.display() - ), - )); - } - } - for granted in extra_read_paths.iter().chain(extra_write_paths) { - if extra_masks - .iter() - .any(|masked| paths_overlap(granted, masked)) - { - return Err(io::Error::new( + /// Like [`wrap_with_process_paths`](Self::wrap_with_process_paths), with + /// an explicit unavailable-sandbox policy. + /// + /// On platforms without a native sandbox backend, [`SandboxPolicy::Off`] + /// returns the original program/arguments/environment as a direct command. + /// [`SandboxPolicy::Required`] remains fail-closed. The unsupported-platform + /// branch runs before SBPL-specific path validation so ordinary Windows + /// paths are not rejected for containing backslashes. + /// + /// # Errors + /// + /// Returns an error under the same conditions as + /// [`wrap_with_process_paths`](Self::wrap_with_process_paths), or when the + /// sandbox is unavailable and policy is [`SandboxPolicy::Required`]. + #[allow(clippy::too_many_arguments)] + #[allow(clippy::too_many_lines)] + pub fn wrap_with_process_paths_and_policy( + inner_cmd: &Command, + worktree_path: &Path, + injections: &[RoInjection], + extra_masks: &[PathBuf], + extra_read_paths: &[PathBuf], + extra_write_paths: &[PathBuf], + clear_env: bool, + policy: SandboxPolicy, + ) -> io::Result { + #[cfg(not(any(target_os = "linux", target_os = "macos")))] + { + let _ = (worktree_path, extra_read_paths, extra_write_paths); + if policy == SandboxPolicy::Off { + if !injections.is_empty() || !extra_masks.is_empty() { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "sandbox policy off cannot enforce injections or copy-on-write masks", + )); + } + Ok(copy_direct_command(inner_cmd, clear_env)) + } else { + Err(io::Error::new( io::ErrorKind::PermissionDenied, - format!( - "process path {} overlaps a copy-on-write mask", - granted.display() - ), - )); + unsupported_os_hint(), + )) } } - #[cfg(target_os = "linux")] + #[cfg(any(target_os = "linux", target_os = "macos"))] { - // Bubblewrap implementation - paths are passed as separate argv entries (no injection). - // The process can only read the root OS, but can only write to the worktree and /tmp. - let mut bwrap = Command::new("bwrap"); - if clear_env { - bwrap.env_clear(); - } - bwrap - .arg("--ro-bind").arg("/").arg("/") // Read-only access to host OS (for binaries like /usr/bin/node) - .arg("--dev").arg("/dev") // Standard dev mounts - .arg("--proc").arg("/proc") // Standard proc mounts - .arg("--bind").arg(worktree_path).arg(worktree_path) // Write access to the worktree - .arg("--tmpfs").arg("/tmp"); // Disposable tmpfs + let _ = policy; - // Read-only file injections: bind each host-owned verified snapshot - // at its in-sandbox target. Placed AFTER the writable worktree - // --bind so a later bind can't shadow it, and BEFORE --unshare-all - // so the ro-bind sits within the namespace setup. The namespace - // creates the mount point, so `target` need not exist on the host. + // Validate on all platforms for defense in depth and API consistency. + // On macOS the validated string is needed for SBPL interpolation. + // On Linux bwrap passes paths as argv entries (no injection risk), + // but we still reject unsafe paths at the API boundary. + let _ = validate_sandbox_str(worktree_path, "worktree path")?; for inj in injections { - bwrap.arg("--ro-bind").arg(&inj.source).arg(&inj.target); + let _ = validate_sandbox_str(&inj.source, "injection source")?; + let _ = validate_sandbox_str(&inj.target, "injection target")?; + } + for path in extra_read_paths { + let _ = validate_sandbox_str(path, "process read path")?; + if !path.exists() { + return Err(io::Error::new( + io::ErrorKind::NotFound, + format!("process read path does not exist: {}", path.display()), + )); + } + } + for path in extra_write_paths { + let _ = validate_sandbox_str(path, "process write path")?; + if !path.exists() { + return Err(io::Error::new( + io::ErrorKind::NotFound, + format!("process write path does not exist: {}", path.display()), + )); + } } - // #856 read-hole fix: the `--ro-bind / /` above mounts the entire - // host filesystem read-only, which exposed Astrid's secret/key/state - // dirs and the operator's home credential stores to the spawned - // process. Shadow each masked path so the agent reads nothing (see - // `push_mask_arg`). Placed AFTER the root ro-bind (so it overlays) - // and the worktree/injection binds; `run/` (socket+token) and `etc/` - // stay reachable for daemon access. Fail-secure: refuse the spawn if - // the home is unresolvable. - let built_in_masks = Self::masked_paths()?; - let home_root = astrid_core::dirs::AstridHome::resolve()?.home_dir(); + // Every caller-supplied mask names copy-on-write bookkeeping the child + // must not reach (the overlayfs upper/work, or the APFS pristine). Each is + // validated exactly like the worktree and injection paths — absolute, + // UTF-8, SBPL-safe — because on macOS it is interpolated into the Seatbelt + // profile; then it must EXIST, since a path that does not exist is a wiring + // bug, not a no-op (silently skipping it leaves the child un-denied). The + // deny is security-critical, so either failure fails the spawn closed. + for masked in extra_masks { + let _ = validate_sandbox_str(masked, "workspace CoW mask")?; + if !masked.exists() { + return Err(io::Error::new( + io::ErrorKind::NotFound, + format!( + "workspace CoW mask path does not exist: {} — refusing to spawn \ + a child without the intended copy-on-write deny", + masked.display() + ), + )); + } + } for granted in extra_read_paths.iter().chain(extra_write_paths) { - if built_in_masks.iter().any(|masked| { - paths_overlap(granted, masked) - && !(masked == &home_root && granted.starts_with(masked)) - }) { + if extra_masks + .iter() + .any(|masked| paths_overlap(granted, masked)) + { return Err(io::Error::new( io::ErrorKind::PermissionDenied, format!( - "process path {} overlaps a sensitive runtime path", + "process path {} overlaps a copy-on-write mask", granted.display() ), )); } } - for masked in &built_in_masks { - Self::push_mask_arg(&mut bwrap, masked); - } - // Caller-supplied masks (the CoW upper/work dirs). Same mechanism, - // placed after the worktree/injection binds so they overlay; the - // CoW dirs live OUTSIDE the worktree, so no writable bind needs to - // punch back through. Existence is validated up front (a missing mask - // already failed the spawn), so every entry is masked unconditionally. - for masked in extra_masks { - Self::push_mask_arg(&mut bwrap, masked); - } + #[cfg(target_os = "linux")] + { + // Bubblewrap implementation - paths are passed as separate argv entries (no injection). + // The process can only read the root OS, but can only write to the worktree and /tmp. + let mut bwrap = Command::new("bwrap"); + if clear_env { + bwrap.env_clear(); + } + bwrap + .arg("--ro-bind").arg("/").arg("/") // Read-only access to host OS (for binaries like /usr/bin/node) + .arg("--dev").arg("/dev") // Standard dev mounts + .arg("--proc").arg("/proc") // Standard proc mounts + .arg("--bind").arg(worktree_path).arg(worktree_path) // Write access to the worktree + .arg("--tmpfs").arg("/tmp"); // Disposable tmpfs - // The root filesystem is already read-only, but the entire - // cross-principal home container is masked above. Re-bind only the - // authorized principal path, then punch through its explicitly - // declared writable subpaths. - for path in extra_read_paths { - bwrap.arg("--ro-bind").arg(path).arg(path); - } - for path in extra_write_paths { - bwrap.arg("--bind").arg(path).arg(path); - } + // Read-only file injections: bind each host-owned verified snapshot + // at its in-sandbox target. Placed AFTER the writable worktree + // --bind so a later bind can't shadow it, and BEFORE --unshare-all + // so the ro-bind sits within the namespace setup. The namespace + // creates the mount point, so `target` need not exist on the host. + for inj in injections { + bwrap.arg("--ro-bind").arg(&inj.source).arg(&inj.target); + } + + // #856 read-hole fix: the `--ro-bind / /` above mounts the entire + // host filesystem read-only, which exposed Astrid's secret/key/state + // dirs and the operator's home credential stores to the spawned + // process. Shadow each masked path so the agent reads nothing (see + // `push_mask_arg`). Placed AFTER the root ro-bind (so it overlays) + // and the worktree/injection binds; `run/` (socket+token) and `etc/` + // stay reachable for daemon access. Fail-secure: refuse the spawn if + // the home is unresolvable. + let built_in_masks = Self::masked_paths()?; + let home_root = astrid_core::dirs::AstridHome::resolve()?.home_dir(); + for granted in extra_read_paths.iter().chain(extra_write_paths) { + if built_in_masks.iter().any(|masked| { + paths_overlap(granted, masked) + && !(masked == &home_root && granted.starts_with(masked)) + }) { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + format!( + "process path {} overlaps a sensitive runtime path", + granted.display() + ), + )); + } + } + for masked in &built_in_masks { + Self::push_mask_arg(&mut bwrap, masked); + } - bwrap + // Caller-supplied masks (the CoW upper/work dirs). Same mechanism, + // placed after the worktree/injection binds so they overlay; the + // CoW dirs live OUTSIDE the worktree, so no writable bind needs to + // punch back through. Existence is validated up front (a missing mask + // already failed the spawn), so every entry is masked unconditionally. + for masked in extra_masks { + Self::push_mask_arg(&mut bwrap, masked); + } + + // The root filesystem is already read-only, but the entire + // cross-principal home container is masked above. Re-bind only the + // authorized principal path, then punch through its explicitly + // declared writable subpaths. + for path in extra_read_paths { + bwrap.arg("--ro-bind").arg(path).arg(path); + } + for path in extra_write_paths { + bwrap.arg("--bind").arg(path).arg(path); + } + + bwrap .arg("--unshare-all") // Drop namespaces (network, pid, etc.) .arg("--share-net") // Re-enable network so npm/cargo can fetch .arg("--die-with-parent"); // Prevent orphan processes - // Extract the original command and args, and append them to bwrap - bwrap.arg(inner_cmd.get_program()); - for arg in inner_cmd.get_args() { - bwrap.arg(arg); - } - - // Inherit the env and current_dir from the original command - for (k, v) in inner_cmd.get_envs() { - if let Some(v) = v { - bwrap.env(k, v); - } else { - bwrap.env_remove(k); + // Extract the original command and args, and append them to bwrap + bwrap.arg(inner_cmd.get_program()); + for arg in inner_cmd.get_args() { + bwrap.arg(arg); } - } - if let Some(dir) = inner_cmd.get_current_dir() { - bwrap.current_dir(dir); - } - Ok(bwrap) - } + // Inherit the env and current_dir from the original command + for (k, v) in inner_cmd.get_envs() { + if let Some(v) = v { + bwrap.env(k, v); + } else { + bwrap.env_remove(k); + } + } + if let Some(dir) = inner_cmd.get_current_dir() { + bwrap.current_dir(dir); + } - #[cfg(target_os = "macos")] - { - // Route through the shared Seatbelt profile builder so this path - // and the MCP spawn path (`ProcessSandboxConfig::sandbox_prefix`) - // generate one identical profile instead of two divergent ones. - // `build_seatbelt_prefix` carries the `(allow mach*)` and - // `(allow file-read* (literal "/"))` rules a dynamically-linked - // binary such as `node` needs to stat the filesystem root at - // startup. The inline profile that used to live here omitted the - // root-read rule, so Seatbelt correctly aborted such a process - // with SIGABRT — a fail-closed signal that was then mistaken for a - // macOS-15+ `sandbox-exec` incompatibility and papered over by - // disabling the sandbox entirely. `sandbox-exec` is deprecated but - // still enforces on current macOS. See #855. - // - // Seatbelt has no mount namespace, so the caller has already - // materialized the verified snapshot AT `target`; the profile - // grants read and a trailing deny-write on that literal path. - let mut config = ProcessSandboxConfig::new(worktree_path); - for path in extra_read_paths { - config = config.with_extra_read(path); - } - for path in extra_write_paths { - config = config.with_extra_write(path); - } - for inj in injections { - config = config.with_ro_inject(&inj.source, &inj.target); - } - // #856: mask the sensitive Astrid subpaths (see the Linux branch). - // macOS seatbelt is already `(deny default)` + an allowlist that - // excludes ~/.astrid, so these denies are belt-and-suspenders that - // still hold if the allowlist ever widens to include the home. - for masked in Self::masked_paths()? { - config = config.with_hidden(masked); + Ok(bwrap) } - // Caller-supplied masks (the CoW pristine workspace / upper dirs). - // The Seatbelt deny is last-match-wins, so it holds even where the - // masked path is under a broadly-writable location (`/var/folders`, - // `/private/tmp`). A masked path that is an ANCESTOR of the writable - // root is dropped by `build_seatbelt_prefix` (it would deny lstat on - // the writable root's own parents) — the CoW backend never masks an - // ancestor of `merged` for exactly this reason. - for masked in extra_masks { - config = config.with_hidden(masked.clone()); - } - let prefix = config.build_seatbelt_prefix()?; - let mut sb_cmd = Command::new(&prefix.program); - if clear_env { - sb_cmd.env_clear(); - } - sb_cmd.args(&prefix.args); + #[cfg(target_os = "macos")] + { + // Route through the shared Seatbelt profile builder so this path + // and the MCP spawn path (`ProcessSandboxConfig::sandbox_prefix`) + // generate one identical profile instead of two divergent ones. + // `build_seatbelt_prefix` carries the `(allow mach*)` and + // `(allow file-read* (literal "/"))` rules a dynamically-linked + // binary such as `node` needs to stat the filesystem root at + // startup. The inline profile that used to live here omitted the + // root-read rule, so Seatbelt correctly aborted such a process + // with SIGABRT — a fail-closed signal that was then mistaken for a + // macOS-15+ `sandbox-exec` incompatibility and papered over by + // disabling the sandbox entirely. `sandbox-exec` is deprecated but + // still enforces on current macOS. See #855. + // + // Seatbelt has no mount namespace, so the caller has already + // materialized the verified snapshot AT `target`; the profile + // grants read and a trailing deny-write on that literal path. + let mut config = ProcessSandboxConfig::new(worktree_path); + for path in extra_read_paths { + config = config.with_extra_read(path); + } + for path in extra_write_paths { + config = config.with_extra_write(path); + } + for inj in injections { + config = config.with_ro_inject(&inj.source, &inj.target); + } + // #856: mask the sensitive Astrid subpaths (see the Linux branch). + // macOS seatbelt is already `(deny default)` + an allowlist that + // excludes ~/.astrid, so these denies are belt-and-suspenders that + // still hold if the allowlist ever widens to include the home. + for masked in Self::masked_paths()? { + config = config.with_hidden(masked); + } + // Caller-supplied masks (the CoW pristine workspace / upper dirs). + // The Seatbelt deny is last-match-wins, so it holds even where the + // masked path is under a broadly-writable location (`/var/folders`, + // `/private/tmp`). A masked path that is an ANCESTOR of the writable + // root is dropped by `build_seatbelt_prefix` (it would deny lstat on + // the writable root's own parents) — the CoW backend never masks an + // ancestor of `merged` for exactly this reason. + for masked in extra_masks { + config = config.with_hidden(masked.clone()); + } + let prefix = config.build_seatbelt_prefix()?; - // Append the original program and its arguments. - sb_cmd.arg(inner_cmd.get_program()); - for arg in inner_cmd.get_args() { - sb_cmd.arg(arg); - } + let mut sb_cmd = Command::new(&prefix.program); + if clear_env { + sb_cmd.env_clear(); + } + sb_cmd.args(&prefix.args); - // Inherit env and working directory from the original command. - for (k, v) in inner_cmd.get_envs() { - if let Some(v) = v { - sb_cmd.env(k, v); - } else { - sb_cmd.env_remove(k); + // Append the original program and its arguments. + sb_cmd.arg(inner_cmd.get_program()); + for arg in inner_cmd.get_args() { + sb_cmd.arg(arg); + } + + // Inherit env and working directory from the original command. + for (k, v) in inner_cmd.get_envs() { + if let Some(v) = v { + sb_cmd.env(k, v); + } else { + sb_cmd.env_remove(k); + } + } + if let Some(dir) = inner_cmd.get_current_dir() { + sb_cmd.current_dir(dir); } - } - if let Some(dir) = inner_cmd.get_current_dir() { - sb_cmd.current_dir(dir); - } - Ok(sb_cmd) + Ok(sb_cmd) + } } + } +} - #[cfg(not(any(target_os = "linux", target_os = "macos")))] - { - // Without an OS-level sandbox neither the read-only injection - // guarantee nor a copy-on-write mask can be enforced; refuse rather - // than run a child without the intended deny (fail-secure). - let _ = ( - inner_cmd, - injections, - extra_masks, - extra_read_paths, - extra_write_paths, - ); - Err(io::Error::other( - "native process execution requires an OS sandbox (bwrap/Seatbelt); \ - unavailable on this platform", - )) +#[cfg(not(any(target_os = "linux", target_os = "macos")))] +fn copy_direct_command(inner: &Command, clear_env: bool) -> Command { + let mut direct = Command::new(inner.get_program()); + direct.args(inner.get_args()); + if clear_env { + direct.env_clear(); + } + for (key, value) in inner.get_envs() { + if let Some(value) = value { + direct.env(key, value); + } else { + direct.env_remove(key); } } + if let Some(cwd) = inner.get_current_dir() { + direct.current_dir(cwd); + } + direct } +#[cfg(any(target_os = "linux", target_os = "macos"))] fn paths_overlap(left: &Path, right: &Path) -> bool { left.starts_with(right) || right.starts_with(left) } @@ -649,6 +715,13 @@ impl SandboxPolicy { #[derive(Debug, Clone)] pub struct ProcessSandboxConfig { /// Root directory the sandboxed process can write to. + #[cfg_attr( + all(not(any(target_os = "linux", target_os = "macos")), not(test)), + expect( + dead_code, + reason = "uniform configuration contract; native backends consume this field" + ) + )] writable_root: PathBuf, /// Additional read-only paths beyond the OS defaults. extra_read_paths: Vec, @@ -763,43 +836,43 @@ impl ProcessSandboxConfig { /// # Errors /// /// Returns an error if: - /// - Any configured path is not valid UTF-8, not absolute, or - /// contains characters that would break sandbox profile syntax - /// (double-quote, backslash, or null byte). + /// - On a sandbox-backed platform under [`SandboxPolicy::Required`], any + /// configured path is not valid UTF-8, not absolute, or contains + /// characters that would break sandbox profile syntax. /// - The active policy is [`SandboxPolicy::Required`] and the /// OS-level sandbox is unavailable. The error message names the /// most likely cause (`kernel.apparmor_restrict_unprivileged_userns=1` /// on Ubuntu 24.04+) and the remediation (`sysctl` command or /// explicit policy override). pub fn sandbox_prefix(&self) -> io::Result> { - // Validate all configured paths up front, regardless of platform. - // This ensures the doc contract ("returns Err for non-UTF-8 or - // forbidden chars") holds on every OS, not just macOS where SBPL - // interpolation makes it exploitable. - self.validate_all_paths()?; - // `Off` short-circuits before any probe so the no-warn contract - // is honoured: the operator has explicitly opted out of - // subprocess containment and shouldn't see diagnostic noise. + // is honoured. It also runs before SBPL-specific validation: paths + // such as `C:\workspace` are ordinary on Windows and are never + // interpolated into a profile when containment is explicitly off. if self.policy == SandboxPolicy::Off { return Ok(None); } - #[cfg(target_os = "linux")] + #[cfg(any(target_os = "linux", target_os = "macos"))] { - if bwrap::bwrap_available() { - return Ok(Some(self.build_bwrap_prefix())); + self.validate_all_paths()?; + + #[cfg(target_os = "linux")] + { + if bwrap::bwrap_available() { + return Ok(Some(self.build_bwrap_prefix())); + } + self.handle_unavailable_sandbox(linux_unavailable_hint()) } - self.handle_unavailable_sandbox(linux_unavailable_hint()) - } - #[cfg(target_os = "macos")] - { - // Seatbelt is shipped with macOS and effectively always - // available; a failure here is genuinely exceptional (e.g. - // path validation tripping a sub-builder), so it surfaces - // through `build_seatbelt_prefix` regardless of policy. - self.build_seatbelt_prefix().map(Some) + #[cfg(target_os = "macos")] + { + // Seatbelt is shipped with macOS and effectively always + // available; a failure here is genuinely exceptional (e.g. + // path validation tripping a sub-builder), so it surfaces + // through `build_seatbelt_prefix` regardless of policy. + self.build_seatbelt_prefix().map(Some) + } } #[cfg(not(any(target_os = "linux", target_os = "macos")))] @@ -817,21 +890,25 @@ impl ProcessSandboxConfig { ))] fn handle_unavailable_sandbox(&self, hint: &str) -> io::Result> { match self.policy { - SandboxPolicy::Required => Err(io::Error::other(format!( - "OS-level sandbox unavailable and policy is `required` — \ - refusing to launch native subprocess capsule without \ - containment. {hint} To run without the sandbox anyway \ - (trusted dev environments, CI runners where the kernel \ - can't be configured), set `ASTRID_SANDBOX_POLICY=off`. \ - The `required` default exists to keep the security \ - guarantee documented in the README — see issue #655." - ))), + SandboxPolicy::Required => Err(io::Error::new( + io::ErrorKind::Unsupported, + format!( + "OS-level sandbox unavailable and policy is `required` — \ + refusing to launch native subprocess capsule without \ + containment. {hint} To run without the sandbox anyway \ + (trusted dev environments, CI runners where the kernel \ + can't be configured), set `ASTRID_SANDBOX_POLICY=off`. \ + The `required` default exists to keep the security \ + guarantee documented in the README — see issue #655." + ), + )), // Unreachable: `Off` short-circuits in `sandbox_prefix`. SandboxPolicy::Off => Ok(None), } } /// Validate all configured paths for safe use in sandbox profiles. + #[cfg(any(target_os = "linux", target_os = "macos"))] fn validate_all_paths(&self) -> io::Result<()> { validate_sandbox_str(&self.writable_root, "writable root")?; for p in &self.extra_read_paths { diff --git a/crates/astrid-workspace/src/sandbox/tests.rs b/crates/astrid-workspace/src/sandbox/tests.rs index 5a3e04e08..fbc4bb3b2 100644 --- a/crates/astrid-workspace/src/sandbox/tests.rs +++ b/crates/astrid-workspace/src/sandbox/tests.rs @@ -85,6 +85,7 @@ fn validate_sandbox_path_rejects_null_byte() { // --- SandboxCommand::wrap() tests --- +#[cfg(unix)] #[test] fn test_wrap_rejects_non_utf8_path() { use std::ffi::OsStr; @@ -102,6 +103,79 @@ fn test_wrap_rejects_non_utf8_path() { ); } +#[cfg(windows)] +#[test] +fn windows_off_accepts_native_paths_without_sbpl_validation() { + let mut command = Command::new(r"C:\Program Files\Astrid\aos.exe"); + command + .arg("capsule") + .arg("list") + .current_dir(r"C:\Users\Astrid Agent\workspace") + .env("ASTRID_TEST_VALUE", "unicode-\u{2603}"); + + let direct = SandboxCommand::wrap_with_process_paths_and_policy( + &command, + Path::new(r"C:\Users\Astrid Agent\workspace"), + &[], + &[], + &[], + &[], + true, + SandboxPolicy::Off, + ) + .expect("explicit off policy should preserve a native Windows command"); + + assert_eq!(direct.get_program(), command.get_program()); + assert_eq!( + direct.get_args().collect::>(), + command.get_args().collect::>() + ); + assert_eq!(direct.get_current_dir(), command.get_current_dir()); +} + +#[cfg(windows)] +#[test] +fn windows_required_refuses_before_exec() { + let command = Command::new(r"C:\Program Files\Astrid\aos.exe"); + let error = SandboxCommand::wrap_with_process_paths_and_policy( + &command, + Path::new(r"C:\Users\Astrid Agent\workspace"), + &[], + &[], + &[], + &[], + true, + SandboxPolicy::Required, + ) + .expect_err("required policy must fail closed without a Windows sandbox backend"); + + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied); +} + +#[cfg(windows)] +#[test] +fn windows_process_config_off_accepts_native_paths() { + let prefix = ProcessSandboxConfig::new(r"C:\Users\Astrid Agent\workspace") + .with_policy(SandboxPolicy::Off) + .sandbox_prefix() + .expect("off policy should not apply SBPL validation to Windows paths"); + assert!(prefix.is_none()); +} + +#[cfg(windows)] +#[test] +fn windows_process_config_required_reports_unavailable_not_invalid_path() { + let error = ProcessSandboxConfig::new(r"C:\Users\Astrid Agent\workspace") + .with_policy(SandboxPolicy::Required) + .sandbox_prefix() + .expect_err("required policy must fail closed"); + assert_eq!(error.kind(), io::ErrorKind::Unsupported); + assert!( + error.to_string().contains("OS-level sandbox unavailable"), + "ordinary Windows path was misclassified: {error}" + ); +} + #[test] fn test_wrap_rejects_double_quote_path() { let bad_path = Path::new("/tmp/evil\"injection/workspace"); @@ -529,6 +603,7 @@ fn test_sandbox_prefix_rejects_relative_writable_root() { assert!(config.sandbox_prefix().is_err()); } +#[cfg(unix)] #[test] fn test_sandbox_prefix_rejects_non_utf8_writable_root() { use std::ffi::OsStr; @@ -542,6 +617,7 @@ fn test_sandbox_prefix_rejects_non_utf8_writable_root() { assert!(result.unwrap_err().to_string().contains("not valid UTF-8")); } +#[cfg(unix)] #[test] fn test_sandbox_prefix_rejects_non_utf8_extra_paths() { use std::ffi::OsStr;