diff --git a/CHANGELOG.md b/CHANGELOG.md index a0d93dcf2..de53c58db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +- **Fixed** Cached tasks no longer fail to spawn child processes in restricted sandboxes (e.g. rootless bubblewrap that denies the `seccomp` syscall). When file-access tracking cannot be set up for a process, the process runs untracked instead and the run is reported as not cached ([#700](https://github.com/voidzero-dev/vite-task/issues/700)). - **Fixed** `vp run` no longer hangs or fails when a task leaves a process running behind it, such as a dev server or a background helper, or when one of a task's processes is killed. The run finishes as soon as the task itself does, and the files the task used are still recorded ([#544](https://github.com/voidzero-dev/vite-task/issues/544), [#675](https://github.com/voidzero-dev/vite-task/pull/675)). - **Fixed** A task that reads or writes an unusually large number of files now runs to the end instead of being killed partway through. Vite+ reports the run as not cached, because it could not record every file the task used ([#533](https://github.com/voidzero-dev/vite-task/issues/533), [#675](https://github.com/voidzero-dev/vite-task/pull/675)). - **Fixed** Vite+ diagnostics now display individual paths and working directories without Rust debug formatting such as quoted paths or escaped Windows backslashes ([#534](https://github.com/voidzero-dev/vite-task/pull/534)). diff --git a/crates/fspy/src/command.rs b/crates/fspy/src/command.rs index fb150b26c..085304cc9 100644 --- a/crates/fspy/src/command.rs +++ b/crates/fspy/src/command.rs @@ -1,5 +1,6 @@ use std::{ ffi::{OsStr, OsString}, + io, path::{Path, PathBuf}, process::Stdio, }; @@ -167,13 +168,18 @@ impl Command { /// /// # Errors /// - /// Returns [`SpawnError`] if program resolution fails or the process cannot be spawned. + /// Returns [`SpawnError`] if program resolution fails, the tracking + /// machinery cannot be initialized (e.g. the preload library cannot be + /// materialized), or the process cannot be spawned. pub async fn spawn( mut self, cancellation_token: CancellationToken, ) -> Result { self.resolve_program()?; - SPY_IMPL.spawn(self, cancellation_token).await + match &*SPY_IMPL { + Ok(spy) => spy.spawn(self, cancellation_token).await, + Err(e) => Err(SpawnError::Injection(io::Error::new(e.kind(), e.to_string()))), + } } /// Resolve program name to full path using `PATH` and cwd. diff --git a/crates/fspy/src/lib.rs b/crates/fspy/src/lib.rs index 5621547f5..f1b89ed22 100644 --- a/crates/fspy/src/lib.rs +++ b/crates/fspy/src/lib.rs @@ -60,8 +60,8 @@ pub struct TrackedChild { pub process_handle: std::os::windows::io::OwnedHandle, } -pub(crate) static SPY_IMPL: LazyLock = LazyLock::new(|| { +pub(crate) static SPY_IMPL: LazyLock> = LazyLock::new(|| { let tmp_dir = temp_dir().join("fspy"); let _ = create_dir(&tmp_dir); - SpyImpl::init_in(&tmp_dir).expect("Failed to initialize global spy") + SpyImpl::init_in(&tmp_dir) }); diff --git a/crates/fspy/tests/untracked_fallback.rs b/crates/fspy/tests/untracked_fallback.rs new file mode 100644 index 000000000..3b980697b --- /dev/null +++ b/crates/fspy/tests/untracked_fallback.rs @@ -0,0 +1,94 @@ +//! Tests for the untracked-exec fallback: when the preload cannot install +//! its injection machinery, the exec must proceed untracked and the run must +//! be reported as incompletely tracked (so it is not cached), rather than +//! every spawn failing. Skipped on musl: no preload library exists there. +#![cfg(all(target_os = "linux", not(target_env = "musl")))] + +use std::{ + ffi::OsStr, + fs::{self, Permissions}, + os::unix::{ffi::OsStrExt as _, fs::PermissionsExt as _}, + path::{Path, PathBuf}, + process::Command, + sync::LazyLock, +}; + +use allocator_api2::alloc::Global; +use fspy_seccomp_unotify::payload::SeccompPayload; +use fspy_shared::ipc::{ + IpcStr, + channel::{RecordsLost, channel}, +}; +use fspy_shared_unix::payload::{Payload, encode_payload}; + +/// The preload cdylib, built as a dependency of this crate. +const PRELOAD_CDYLIB: &str = env!("CARGO_CDYLIB_FILE_FSPY_PRELOAD_UNIX"); + +const TEST_BIN_CONTENT: &[u8] = include_bytes!(env!("CARGO_BIN_FILE_FSPY_TEST_BIN")); + +fn test_bin_path() -> &'static Path { + static TEST_BIN_PATH: LazyLock = LazyLock::new(|| { + let test_bin_path = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join("fspy-test-bin"); + fs::write(&test_bin_path, TEST_BIN_CONTENT).expect("failed to write test binary"); + fs::set_permissions(&test_bin_path, Permissions::from_mode(0o755)) + .expect("failed to set permissions on test binary"); + test_bin_path + }); + TEST_BIN_PATH.as_path() +} + +/// A static binary exec'd from a traced process needs the preload's inline +/// seccomp install. When that install fails (here: the payload's supervisor +/// IPC path is bogus, simulating a sandbox that denies it), the binary must +/// still run — untracked — and the channel must report the loss. +#[test] +fn static_binary_runs_untracked_when_injection_fails() { + let receiver = channel(1 << 30, Global).unwrap(); + let preload_path: &IpcStr = Path::new(PRELOAD_CDYLIB).into(); + let payload = Payload { + ipc_channel_conf: receiver.conf(), + preload_path, + seccomp_payload: SeccompPayload::unreachable( + b"/nonexistent/fspy-unreachable-supervisor".to_vec(), + ), + }; + let bump = bumpalo::Bump::new(); + let encoded = encode_payload(payload, &bump); + + let output = Command::new("/bin/sh") + .arg("-c") + .arg(format!("exec {} stat /hello", test_bin_path().display())) + .env_clear() + .env("LD_PRELOAD", PRELOAD_CDYLIB) + .env("FSPY_PAYLOAD", OsStr::from_bytes(encoded.encoded_string.as_ref())) + .output() + .expect("failed to spawn the shell"); + assert!( + output.status.success(), + "the static binary did not run: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let Err(RecordsLost) = receiver.close() else { + panic!("the channel did not report the untracked exec"); + }; +} + +/// A preload loaded without a payload (e.g. a leaked LD_PRELOAD in an +/// env-scrubbed sandbox) must not abort its host process: the constructor +/// degrades and every exec forwards to the original. +#[test] +fn preload_without_payload_runs_untracked() { + let output = Command::new("/bin/sh") + .arg("-c") + .arg("exec /bin/true") + .env_clear() + .env("LD_PRELOAD", PRELOAD_CDYLIB) + .output() + .expect("failed to spawn the shell"); + assert!( + output.status.success(), + "the preload aborted its host process: {}", + String::from_utf8_lossy(&output.stderr) + ); +} diff --git a/crates/fspy_client_unix/src/lib.rs b/crates/fspy_client_unix/src/lib.rs index a0a7d58ce..a3480fa72 100644 --- a/crates/fspy_client_unix/src/lib.rs +++ b/crates/fspy_client_unix/src/lib.rs @@ -16,10 +16,35 @@ use fspy_shared::ipc::{PathAccess, channel::Sender}; use fspy_shared_unix::{ exec::ExecResolveConfig, payload::{EncodedPayload, decode_payload_from_env}, - spawn::{PreExec, handle_exec}, + spawn::{PreExec, prepare_exec, resolve_exec}, }; use raw_exec::RawExec; +/// Why [`Client::handle_exec`] failed. +#[derive(Debug)] +pub enum ExecInjectionError { + /// Program resolution failed the way the real exec would have; the errno + /// is authentic and the caller should surface it as the exec's own + /// failure (set errno and return -1, or return it from `posix_spawn`). + Resolution(nix::Error), + /// The tracing injection machinery failed after the program resolved; + /// the exec was never attempted. The caller should mark the run's trace + /// incomplete ([`Client::report_loss`]) and perform the operation + /// untracked. + Injection(nix::Error), +} + +impl std::fmt::Display for ExecInjectionError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Resolution(errno) => write!(f, "exec resolution failed: {errno}"), + Self::Injection(errno) => write!(f, "exec injection failed: {errno}"), + } + } +} + +impl std::error::Error for ExecInjectionError {} + pub struct Client<'a> { encoded_payload: EncodedPayload<'a>, ipc_sender: Option, @@ -51,16 +76,18 @@ impl<'a> Client<'a> { /// with no further ceremony — and the client never retains the /// allocator itself (see the `Send + Sync` assertion above). /// - /// # Panics - /// - /// Panics when the payload is missing, malformed, or cannot be decoded, - /// and when the channel is there but cannot be attached to (see - /// [`ChannelConf::sender`](fspy_shared::ipc::channel::ChannelConf::sender)). + /// Returns `None` when the payload is missing, malformed, or cannot be + /// decoded — e.g. a leaked `LD_PRELOAD` in an env-scrubbed sandbox. The + /// host process then runs untracked rather than dying in its preload + /// constructor. When the payload decodes but its channel cannot be + /// attached to, the client still functions with `ipc_sender: None`: it + /// reports nothing, and [`Client::report_loss`] is a no-op. + #[must_use] pub fn from_env( envs: impl Iterator, allocator: impl Allocator + Clone + 'a, - ) -> Self { - let encoded_payload = decode_payload_from_env(envs, allocator.clone()).unwrap(); + ) -> Option { + let encoded_payload = decode_payload_from_env(envs, allocator.clone()).ok()?; // `None` when the channel is already over, which happens when this // process starts after the root target exited. Nothing is said @@ -68,7 +95,7 @@ impl<'a> Client<'a> { // stderr corrupts whatever that process is printing. let ipc_sender = encoded_payload.payload.ipc_channel_conf.sender(allocator); - Self { encoded_payload, ipc_sender } + Some(Self { encoded_payload, ipc_sender }) } fn send(&self, mode: fspy_shared::ipc::AccessMode, path: &Path) { @@ -87,8 +114,26 @@ impl<'a> Client<'a> { ipc_sender.send(&PathAccess { mode, path: path.into() }); } + /// Marks the run's trace incomplete, so the receiver treats it as + /// untracked (and the runner does not cache it). Used before executing + /// something untracked, e.g. when the injection machinery failed and the + /// exec is forwarded to the OS as-is. A no-op when this client has no + /// channel sender. + pub fn report_loss(&self) { + if let Some(ipc_sender) = &self.ipc_sender { + ipc_sender.report_loss(); + } + } + /// Resolves and reports an exec before forwarding its transformed arguments. /// + /// The callback contract: capture the real exec's own outcome (return + /// value, errno) into `R` and return it as `Ok`, even when the exec + /// itself fails. Reserve `Err` for injection machinery failures, such as + /// [`PreExec::run`] failing to install the seccomp filter — the caller + /// maps those to [`ExecInjectionError::Injection`], marks the trace + /// incomplete, and retries the operation untracked. + /// /// # Safety /// /// `raw_exec` must contain the valid C strings and pointer arrays required @@ -97,22 +142,27 @@ impl<'a> Client<'a> { /// /// # Errors /// - /// Returns errors from exec resolution, platform preparation, or the - /// forwarding callback. + /// [`ExecInjectionError::Resolution`] when program resolution fails the + /// way the real exec would have; [`ExecInjectionError::Injection`] when + /// the injection machinery fails after the program resolved. pub unsafe fn handle_exec( &self, config: ExecResolveConfig, raw_exec: RawExec, allocator: impl Allocator, f: impl FnOnce(RawExec, Option) -> nix::Result, - ) -> nix::Result { + ) -> Result { // SAFETY: raw_exec contains valid pointers to C strings and // null-terminated arrays, as provided by the caller. let mut exec = unsafe { raw_exec.to_exec() }; - let pre_exec = handle_exec(&mut exec, config, &self.encoded_payload, |mode, path| { + resolve_exec(&mut exec, config, |mode, path| { self.send(mode, path); - })?; + }) + .map_err(ExecInjectionError::Resolution)?; + let pre_exec = prepare_exec(&mut exec, &self.encoded_payload) + .map_err(ExecInjectionError::Injection)?; RawExec::from_exec(exec, allocator, |raw_command| f(raw_command, pre_exec)) + .map_err(ExecInjectionError::Injection) } /// Resolves and reports one intercepted file access. diff --git a/crates/fspy_preload_unix/src/client.rs b/crates/fspy_preload_unix/src/client.rs index 9f250d8c3..e3d9b02cb 100644 --- a/crates/fspy_preload_unix/src/client.rs +++ b/crates/fspy_preload_unix/src/client.rs @@ -1,7 +1,7 @@ use std::sync::OnceLock; use convert::{ToAbsolutePath, ToAccessMode}; -pub use fspy_client_unix::{Client, convert, raw_exec}; +pub use fspy_client_unix::{Client, ExecInjectionError, convert, raw_exec}; static CLIENT: OnceLock> = OnceLock::new(); @@ -19,26 +19,36 @@ pub fn global_client() -> Option<&'static Client<'static>> { pub unsafe fn handle_open(path: impl ToAbsolutePath, mode: impl ToAccessMode) { if let Some(client) = global_client() { let allocator = fspy_nostd_alloc::pooled_bump(); + // The interception proceeds whether or not the record could be + // sent — a preload library can never panic its host process. // SAFETY: path and mode contain valid pointers/values forwarded // from the interposed function's caller. - unsafe { client.try_handle_open(path, mode, allocator) }.unwrap(); + let _ = unsafe { client.try_handle_open(path, mode, allocator) }; } } #[cfg(not(test))] #[ctor::ctor(unsafe)] fn init_client() { - // SAFETY: the ctor only reads the process environment while constructing - // the client and does not retain borrowed environment views. - let current = unsafe { fspy_nostd::env::current() }.unwrap(); + // Never panic here: a panic in a preload constructor aborts the host + // process. When the environment cannot be read or carries no valid + // payload (e.g. a leaked LD_PRELOAD in an env-scrubbed sandbox), CLIENT + // stays unset and the process runs untracked: the interposed calls + // forward to the originals untouched. + static BUMP: static_cell::StaticCell = + static_cell::StaticCell::new(); // The attach's storage: one page-backed bump housed in a static, so // its borrow is 'static by construction and the client comes out as // Client<'static> with no lifetime promotion anywhere. The bump is not // Sync, so this handle cannot be stored globally by any safe code, and // the Send/Sync assertion on Client proves the client keeps no handle. - static BUMP: static_cell::StaticCell = - static_cell::StaticCell::new(); let bump: &'static fspy_nostd_alloc::PageBump = BUMP.init(fspy_nostd_alloc::page_bump()); - let client = Client::from_env(current.envs(), bump); - CLIENT.set(client).unwrap(); + // SAFETY: the ctor only reads the process environment while constructing + // the client and does not retain borrowed environment views. + let client = unsafe { fspy_nostd::env::current() } + .ok() + .and_then(|current| Client::from_env(current.envs(), bump)); + if let Some(client) = client { + let _ = CLIENT.set(client); + } } diff --git a/crates/fspy_preload_unix/src/interceptions/spawn/exec/mod.rs b/crates/fspy_preload_unix/src/interceptions/spawn/exec/mod.rs index 729162d02..4b27b3bb3 100644 --- a/crates/fspy_preload_unix/src/interceptions/spawn/exec/mod.rs +++ b/crates/fspy_preload_unix/src/interceptions/spawn/exec/mod.rs @@ -6,7 +6,7 @@ use libc::{c_char, c_int}; use with_argv::with_argv; use crate::{ - client::{global_client, raw_exec::RawExec}, + client::{ExecInjectionError, global_client, raw_exec::RawExec}, macros::intercept, }; @@ -32,8 +32,13 @@ fn handle_exec( argv: *const *const libc::c_char, envp: *const *const libc::c_char, ) -> libc::c_int { - let client = - global_client().expect("exec unexpectedly called before client initialized in ctor"); + let Some(client) = global_client() else { + // The ctor left the client unset (no readable environment, or no + // valid payload): run untracked by forwarding to the real exec. + // SAFETY: prog, argv, and envp are valid pointers forwarded from the + // interposed exec function. + return unsafe { execve::original()(prog, argv, envp) }; + }; // SAFETY: prog, argv, and envp are valid pointers to C strings/arrays forwarded from the interposed exec function let result = unsafe { client.handle_exec( @@ -50,10 +55,24 @@ fn handle_exec( }; match result { Ok(ret) => ret, - Err(errno) => { + Err(ExecInjectionError::Resolution(errno)) => { + // Resolution failed the way the real exec would have; the errno + // is authentic. errno.set(); -1 } + Err(ExecInjectionError::Injection(_)) => { + // The injection machinery failed (e.g. the seccomp filter cannot + // be installed under a restrictive sandbox). Mark the run's trace + // incomplete so it is not cached, then run untracked with the + // original arguments. The original envp still carries + // LD_PRELOAD/FSPY_PAYLOAD, so each generation independently + // attempts tracking and independently degrades. + client.report_loss(); + // SAFETY: prog, argv, and envp are the interposed exec function's + // own valid arguments. + unsafe { execve::original()(prog, argv, envp) } + } } } diff --git a/crates/fspy_preload_unix/src/interceptions/spawn/posix_spawn.rs b/crates/fspy_preload_unix/src/interceptions/spawn/posix_spawn.rs index e76373ff7..e4d0ed1d9 100644 --- a/crates/fspy_preload_unix/src/interceptions/spawn/posix_spawn.rs +++ b/crates/fspy_preload_unix/src/interceptions/spawn/posix_spawn.rs @@ -4,7 +4,7 @@ use fspy_shared_unix::exec::ExecResolveConfig; use libc::{c_char, c_int}; use crate::{ - client::{global_client, raw_exec::RawExec}, + client::{ExecInjectionError, global_client, raw_exec::RawExec}, macros::intercept, }; @@ -39,8 +39,14 @@ unsafe fn handle_posix_spawn( // SAFETY: the raw pointers captured inside T are valid for the duration of the thread::scope call, so sending them to the scoped thread is safe unsafe impl Send for AssertSend {} - let client = global_client() - .expect("posix_spawn(p) unexpectedly called before client initialized in ctor"); + let Some(client) = global_client() else { + // The ctor left the client unset (no readable environment, or no + // valid payload): spawn untracked by forwarding to the real + // posix_spawn(p). + // SAFETY: all arguments are valid pointers forwarded from the + // interposed posix_spawn(p) function. + return unsafe { original(pid, file, file_actions, attrp, argv, envp) }; + }; // SAFETY: file, argv, and envp are valid pointers forwarded from the interposed posix_spawn(p) function let result = unsafe { @@ -78,8 +84,21 @@ unsafe fn handle_posix_spawn( ) }; match result { - Err(errno) => errno as _, Ok(ret) => ret, + Err(ExecInjectionError::Resolution(errno)) => { + // Resolution failed the way the real spawn would have; + // posix_spawn returns the errno code rather than -1. + errno as _ + } + Err(ExecInjectionError::Injection(_)) => { + // The injection machinery failed. Mark the run's trace incomplete + // so it is not cached, then spawn untracked with the original + // arguments. + client.report_loss(); + // SAFETY: all arguments are the interposed posix_spawn(p) + // function's own valid arguments. + unsafe { original(pid, file, file_actions, attrp, argv, envp) } + } } } diff --git a/crates/fspy_seccomp_unotify/src/payload/mod.rs b/crates/fspy_seccomp_unotify/src/payload/mod.rs index 6895bc55a..58c6fb671 100644 --- a/crates/fspy_seccomp_unotify/src/payload/mod.rs +++ b/crates/fspy_seccomp_unotify/src/payload/mod.rs @@ -7,3 +7,17 @@ pub struct SeccompPayload { pub(crate) ipc_path: Vec, pub(crate) filter: Filter, } + +impl SeccompPayload { + /// Builds a payload whose installation is guaranteed to fail: the filter + /// is empty (the kernel refuses it) and nothing listens on `ipc_path`. + /// + /// Test support for the untracked-exec fallback: integration tests in + /// dependent crates use it to exercise a failing [`crate::target`] + /// install without a restricted sandbox. + #[doc(hidden)] + #[must_use] + pub fn unreachable(ipc_path: Vec) -> Self { + Self { ipc_path, filter: Filter(Vec::new()) } + } +} diff --git a/crates/fspy_shared/src/ipc/channel/mod.rs b/crates/fspy_shared/src/ipc/channel/mod.rs index 97b3ebc5e..df8b98809 100644 --- a/crates/fspy_shared/src/ipc/channel/mod.rs +++ b/crates/fspy_shared/src/ipc/channel/mod.rs @@ -163,46 +163,41 @@ impl Drop for ShmKeeper { } impl ChannelConf<'_> { - /// Creates a sender, or `None` when the channel is already over. + /// Creates a sender, or `None` when attaching is not possible. /// - /// Never blocks. `None` means the receiver removed the backing file, - /// or sealed the region before removing it and this call caught the - /// gate in between. Either way whatever the caller does next happens - /// past the receiver's boundary, so recording nothing loses nothing. + /// Never blocks, never panics. `None` means the channel is already over + /// — the receiver removed the backing file, or sealed the region before + /// removing it and this call caught the gate in between — or that the + /// channel is there but cannot be attached to: its path is not a valid + /// C string, the file refuses to open or map, or the region cannot hold + /// the protocol. /// - /// # Panics - /// - /// When the channel is there but cannot be attached to: its path is - /// unreadable, the file refuses to open or map, or the region cannot - /// hold the protocol. A process with no sender has no way to tell the - /// receiver it recorded nothing, and a trace that silently omits every - /// access a process made is worse than no trace, so it stops here. + /// Returning `None` rather than panicking matters because a sender is + /// created inside arbitrary traced processes (a preload constructor), + /// where panicking kills the host process. A process with no sender + /// records nothing, and the receiver learns the run went untracked + /// through the loss-report path instead (see [`Sender::report_loss`]). #[must_use] pub fn sender(&self, allocator: A) -> Option { // The allocation is transient: the decoded path only has to outlive // the open call below, and dropping it hands the space back to a // bump allocator, whose most recent allocation it is. - let shm_path = self - .shm_id - .to_os_c_string_in(allocator) - .expect("the channel's shared-memory path is not a valid C string"); + let shm_path = self.shm_id.to_os_c_string_in(allocator)?; let mapping = match fspy_shm::open(shm_path.as_c_str().as_thin()) { - Ok(handle) => handle.map().expect("cannot map the shared-memory channel"), - Err(error) => { - let error = shm_error_to_io(error); - // The receiver removed the backing file, so it has already - // stopped collecting. - if error.kind() == io::ErrorKind::NotFound { - return None; - } - panic!("cannot open the shared-memory channel: {error}"); + Ok(handle) => handle.map().ok()?, + Err(_) => { + // NotFound means the receiver removed the backing file, so it + // has already stopped collecting. Any other open failure + // degrades to no sender for the same reason: the trace is + // incomplete either way and the host process must not die + // over it. + return None; } }; // SAFETY: `mapping` is a freshly mapped shared memory region created // zero-initialized by `channel` and accessed only through the // `shm_io` protocol by every attached process. - let writer = unsafe { ShmWriter::new(mapping, SLOTS) } - .expect("the shared-memory region cannot hold the channel"); + let writer = unsafe { ShmWriter::new(mapping, SLOTS) }?; // The receiver sealed the region but has not removed it yet. if writer.is_closed() { return None; @@ -216,6 +211,18 @@ pub struct Sender { } impl Sender { + /// Reports that this process went on to perform an operation it could + /// not record, sealing the channel as incomplete. + /// + /// Used when a traced process escapes tracing — e.g. a preload that + /// could not install its injection machinery and forwards the operation + /// to the OS untracked. The receiver's close then reports + /// [`RecordsLost`], so the run is treated as untracked rather than + /// cached from a partial trace. + pub fn report_loss(&self) { + self.writer.report_loss(); + } + /// Serializes one record into a committed frame. /// /// A claim the channel refuses is skipped, because that is all a sender @@ -358,8 +365,8 @@ mod tests { /// A capacity with no room for the table has to fail here, at /// creation. Everything downstream treats the region as able to host - /// the protocol: `sender` panics when it cannot, and so does - /// `Receiver::close`. + /// the protocol: `sender` returns `None` when it cannot, and + /// `Receiver::close` panics. #[test] fn a_capacity_too_small_for_the_table_fails_the_channel() { // The counters alone need sixteen bytes, and the table needs eight diff --git a/crates/fspy_shared/src/ipc/channel/shm_io/writer.rs b/crates/fspy_shared/src/ipc/channel/shm_io/writer.rs index 9cda0ab33..928c87c18 100644 --- a/crates/fspy_shared/src/ipc/channel/shm_io/writer.rs +++ b/crates/fspy_shared/src/ipc/channel/shm_io/writer.rs @@ -74,6 +74,22 @@ impl ShmWriter { self.mapped.claims().load(Ordering::Relaxed) & CLOSED != 0 } + /// Sets the CLOSED gate to report that this writer went on to perform an + /// operation it could not record. + /// + /// The seal then fails and every later claim is refused, so the receiver + /// learns the frames are incomplete rather than mistaking what arrived + /// for the whole trace. + /// + /// Storing the gate rather than or-ing it in drops the claim count, + /// which nothing reads once the gate is set. The seal fails on the + /// bit before it looks at the count, and a claim that reads the + /// cleared count reads the gate along with it, so it gives up + /// before using a slot index. + pub fn report_loss(&self) { + self.mapped.claims().store(CLOSED, Ordering::Relaxed); + } + /// Claims a frame of exactly `frame_size` bytes. Wait-free: two /// `fetch_add`s, no retry loop (rule 1). /// @@ -87,14 +103,8 @@ impl ShmWriter { // The loss report (rule 1): the gate marks the frames incomplete // and shuts the channel down for later claims. - // - // Storing the gate rather than or-ing it in drops the claim count, - // which nothing reads once the gate is set. The seal fails on the - // bit before it looks at the count, and a claim that reads the - // cleared count reads the gate along with it, so it gives up - // before using a slot index. let report_loss = || { - mapped.claims().store(CLOSED, Ordering::Relaxed); + self.report_loss(); ClaimError::Capacity }; diff --git a/crates/fspy_shared_unix/src/spawn/mod.rs b/crates/fspy_shared_unix/src/spawn/mod.rs index e48125772..8d250303f 100644 --- a/crates/fspy_shared_unix/src/spawn/mod.rs +++ b/crates/fspy_shared_unix/src/spawn/mod.rs @@ -19,27 +19,25 @@ use crate::{ payload::EncodedPayload, }; -/// Handles exec command resolution and injection +/// Resolves the exec's program path and reports the accesses that takes. /// -/// Resolves the program path and prepares the command for execution with -/// appropriate environment variables and hooks. +/// This is the half of [`handle_exec`] whose failures mean what the real +/// exec's failure would have meant (see [`Exec::resolve`]), so a caller can +/// forward the errno authentically. /// /// # Errors /// -/// Returns an error if: -/// - Program resolution fails (see [`Exec::resolve`] error variants, such as `ENOENT` (file not found) or `EACCES` (permission denied)) -/// - Environment variable operations fail (e.g., `ensure_env` may return `EINVAL` if an existing value conflicts) -/// - Platform-specific errors from `os_specific::handle_exec` +/// Returns an error if program resolution fails (see [`Exec::resolve`] error +/// variants, such as `ENOENT` (file not found) or `EACCES` (permission denied)). /// /// # Panics /// /// Panics if the current working directory cannot be determined when converting a relative path to absolute. -pub fn handle_exec( +pub fn resolve_exec( command: &mut Exec, config: ExecResolveConfig, - encoded_payload: &EncodedPayload, mut on_path_access: impl FnMut(AccessMode, &Path), -) -> nix::Result> { +) -> nix::Result<()> { let mut on_path_access = |mode: AccessMode, path: &Path| { if path.is_absolute() { on_path_access(mode, path); @@ -51,6 +49,50 @@ pub fn handle_exec( command.resolve(&mut on_path_access, config)?; on_path_access(AccessMode::READ, Path::new(OsStr::from_bytes(&command.program))); + Ok(()) +} +/// Prepares a resolved exec for tracked execution: injects the preload +/// environment, or arms the seccomp filter to install before exec. +/// +/// This is the half of [`handle_exec`] whose failures are the tracing +/// machinery's own, never the exec's. +/// +/// # Errors +/// +/// Returns an error if environment variable operations fail (e.g., +/// `ensure_env` may return `EINVAL` if an existing value conflicts) or from +/// platform-specific errors in `os_specific::handle_exec`. +pub fn prepare_exec( + command: &mut Exec, + encoded_payload: &EncodedPayload, +) -> nix::Result> { os_specific::handle_exec(command, encoded_payload) } + +/// Handles exec command resolution and injection +/// +/// Resolves the program path and prepares the command for execution with +/// appropriate environment variables and hooks. Composed of [`resolve_exec`] +/// followed by [`prepare_exec`]; call them separately to tell an authentic +/// resolution failure apart from an injection-machinery failure. +/// +/// # Errors +/// +/// Returns an error if: +/// - Program resolution fails (see [`Exec::resolve`] error variants, such as `ENOENT` (file not found) or `EACCES` (permission denied)) +/// - Environment variable operations fail (e.g., `ensure_env` may return `EINVAL` if an existing value conflicts) +/// - Platform-specific errors from `os_specific::handle_exec` +/// +/// # Panics +/// +/// Panics if the current working directory cannot be determined when converting a relative path to absolute. +pub fn handle_exec( + command: &mut Exec, + config: ExecResolveConfig, + encoded_payload: &EncodedPayload, + on_path_access: impl FnMut(AccessMode, &Path), +) -> nix::Result> { + resolve_exec(command, config, on_path_access)?; + prepare_exec(command, encoded_payload) +} diff --git a/crates/vt/src/napi_client.rs b/crates/vt/src/napi_client.rs index 0f300911b..e244339c7 100644 --- a/crates/vt/src/napi_client.rs +++ b/crates/vt/src/napi_client.rs @@ -13,9 +13,8 @@ use vt_path::{AbsolutePath, AbsolutePathBuf}; /// /// # Panics /// -/// Panics if the materialization fails on first call — this mirrors fspy's -/// `SPY_IMPL` and the same reasoning applies: if we can't write into the -/// system temp dir, the runner can't run tasks anyway. +/// Panics if the materialization fails on first call: if we can't write +/// into the system temp dir, the runner can't run tasks anyway. #[must_use] pub fn napi_client_path() -> &'static AbsolutePath { static PATH: LazyLock = LazyLock::new(|| { diff --git a/crates/vt/src/session/event.rs b/crates/vt/src/session/event.rs index b28b2f2d5..c054812e9 100644 --- a/crates/vt/src/session/event.rs +++ b/crates/vt/src/session/event.rs @@ -91,6 +91,10 @@ pub enum CacheNotUpdatedReason { /// (its `input` config includes auto-inference). Task ran but cannot /// be cached without tracked path accesses. FspyUnsupported, + /// fspy is compiled in, but the tracked spawn failed (e.g. the preload + /// library could not be materialized), so the task ran untracked and + /// cannot be cached without tracked path accesses. + FspyUnavailable, /// The runner's IPC server failed during execution, so the collected /// reports may be incomplete. Caching such a run would risk stale /// inputs/outputs on the next hit. Carries the underlying error for diff --git a/crates/vt/src/session/execute/cache_update.rs b/crates/vt/src/session/execute/cache_update.rs index 6d320a083..5290f921d 100644 --- a/crates/vt/src/session/execute/cache_update.rs +++ b/crates/vt/src/session/execute/cache_update.rs @@ -123,10 +123,10 @@ pub(super) async fn update_cache( } if fspy_outcome.is_none() && fspy.is_some() { - // Task requested fspy auto-inference but this binary was built without - // `cfg(fspy)`. Task ran, but we can't compute a valid cache entry + // Task requested fspy auto-inference but produced no tracked + // accesses. Task ran, but we can't compute a valid cache entry // without tracked path accesses. - return (CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::FspyUnsupported), None); + return (CacheUpdateStatus::NotUpdated(fspy_missing_reason(outcome)), None); } // Collect tool-reported tracked envs for the post-run fingerprint. Env @@ -194,6 +194,17 @@ pub(super) async fn update_cache( } } +/// Why a run that requested fspy auto-inference produced no tracked +/// accesses: either this binary was built without `cfg(fspy)`, or the +/// tracked spawn failed and the task ran untracked (see [`super::spawn`]). +const fn fspy_missing_reason(outcome: &ChildOutcome) -> CacheNotUpdatedReason { + if outcome.fspy_unavailable { + CacheNotUpdatedReason::FspyUnavailable + } else { + CacheNotUpdatedReason::FspyUnsupported + } +} + /// Summarize the run's fspy observations. `Some` iff tracking was both /// requested (`tracking.fspy.is_some()`) and compiled in (`cfg(fspy)`). On a /// `cfg(not(fspy))` build this is always `None`, and [`update_cache`] diff --git a/crates/vt/src/session/execute/spawn.rs b/crates/vt/src/session/execute/spawn.rs index 7e2abb59e..7206d80d7 100644 --- a/crates/vt/src/session/execute/spawn.rs +++ b/crates/vt/src/session/execute/spawn.rs @@ -45,6 +45,10 @@ pub struct ChildOutcome { /// `Err` when a tracked process could not record everything it did. #[cfg(fspy)] pub path_accesses: Option>, + /// `true` when fspy tracking was requested but the tracked spawn failed + /// and the child ran untracked (see [`spawn`]). Always `false` on builds + /// without `cfg(fspy)`. + pub fspy_unavailable: bool, } /// Spawn a command with the requested fspy and stdio configuration. @@ -58,6 +62,11 @@ pub struct ChildOutcome { /// /// On builds without `cfg(fspy)`, the `fspy` argument is ignored and the tokio /// path is always taken. +/// +/// When fspy is requested but the tracked spawn itself fails (e.g. the preload +/// library cannot be materialized), the command falls back to an untracked +/// spawn with [`ChildOutcome::fspy_unavailable`] set, so the run is not +/// cached — a task must never fail to run because its tracing did. #[tracing::instrument(level = "debug", skip_all)] pub async fn spawn( cmd: &SpawnCommand, @@ -71,9 +80,29 @@ where K: AsRef, V: AsRef, { + let extra_envs: Vec<(K, V)> = extra_envs.into_iter().collect(); + + #[cfg(fspy)] + let mut fspy_unavailable = false; + #[cfg(not(fspy))] + let fspy_unavailable = false; + #[cfg(fspy)] if fspy { - return spawn_fspy(cmd, stdio, cancellation_token, extra_envs).await; + match spawn_fspy( + cmd, + stdio, + cancellation_token.clone(), + extra_envs.iter().map(|(k, v)| (k, v)), + ) + .await + { + Ok(handle) => return Ok(handle), + Err(err) => { + tracing::warn!("fspy spawn failed, falling back to untracked spawn: {err:#}"); + fspy_unavailable = true; + } + } } #[cfg(not(fspy))] let _ = fspy; @@ -82,12 +111,17 @@ where tokio_cmd.args(cmd.args.iter().map(vt_str::Str::as_str)); tokio_cmd.env_clear(); tokio_cmd.envs(cmd.spawn_envs.iter()); - tokio_cmd.envs(extra_envs); + tokio_cmd.envs(extra_envs.iter().map(|(k, v)| (k, v))); tokio_cmd.current_dir(&*cmd.cwd); apply_stdio(&mut tokio_cmd, stdio); - spawn_tokio(tokio_cmd, cancellation_token) + spawn_tokio(tokio_cmd, cancellation_token, fspy_unavailable) } +/// Spawn through fspy's tracked [`fspy::Command`]. +/// +/// A failure here must not fail the task: [`spawn`] falls back to an +/// untracked spawn and marks the outcome [`ChildOutcome::fspy_unavailable`] +/// so the run is not cached. #[cfg(fspy)] async fn spawn_fspy( cmd: &SpawnCommand, @@ -148,6 +182,7 @@ where Ok(ChildOutcome { exit_status: termination.status, path_accesses: Some(termination.path_accesses), + fspy_unavailable: false, }) } .boxed_local(); @@ -158,6 +193,7 @@ where fn spawn_tokio( mut cmd: tokio::process::Command, cancellation_token: CancellationToken, + fspy_unavailable: bool, ) -> anyhow::Result { let mut child = cmd.spawn()?; @@ -192,6 +228,7 @@ fn spawn_tokio( exit_status, #[cfg(fspy)] path_accesses: None, + fspy_unavailable, }) } .boxed_local(); diff --git a/crates/vt/src/session/reporter/summary.rs b/crates/vt/src/session/reporter/summary.rs index 7bc36e5bb..6c2856b70 100644 --- a/crates/vt/src/session/reporter/summary.rs +++ b/crates/vt/src/session/reporter/summary.rs @@ -108,6 +108,12 @@ pub enum SpawnOutcome { /// Task ran successfully but cache was not updated. #[serde(default)] fspy_unsupported: bool, + /// `true` when the task required fspy auto-inference but the tracked + /// spawn failed (e.g. the preload library could not be materialized), + /// so the task ran untracked. Task ran successfully but cache was not + /// updated. + #[serde(default)] + fspy_unavailable: bool, /// Rendered message of the IPC server error that caused the cache to /// be skipped, if any. ipc_server_error: Option, @@ -329,6 +335,44 @@ impl TaskResult { saved_error: Option<&SavedExecutionError>, cache_update_status: &CacheUpdateStatus, ) -> Self { + let details = SuccessDetails::from_cache_update_status(cache_update_status); + + match cache_status { + CacheStatus::Hit { replayed_duration } => { + Self::CacheHit { saved_duration_ms: duration_to_ms(*replayed_duration) } + } + CacheStatus::Disabled(CacheDisabledReason::InProcessExecution) => Self::InProcess, + CacheStatus::Disabled(CacheDisabledReason::NoCacheMetadata) => Self::Spawned { + cache_status: SpawnedCacheStatus::Disabled, + outcome: spawn_outcome_from_execution(exit_status, saved_error, details), + }, + CacheStatus::Miss(cache_miss) => Self::Spawned { + cache_status: SpawnedCacheStatus::Miss(SavedCacheMissReason::from_cache_miss( + cache_miss, + )), + outcome: spawn_outcome_from_execution(exit_status, saved_error, details), + }, + } + } +} + +/// The cache-not-updated details a run carries into its [`SpawnOutcome::Success`]. +#[derive(Default)] +#[expect( + clippy::struct_excessive_bools, + reason = "each flag is a distinct cache-not-updated outcome mirroring a CacheNotUpdatedReason variant" +)] +struct SuccessDetails { + input_modified_path: Option, + fspy_unsupported: bool, + fspy_unavailable: bool, + ipc_server_error: Option, + tool_disabled_cache: bool, + tracking_incomplete: bool, +} + +impl SuccessDetails { + fn from_cache_update_status(cache_update_status: &CacheUpdateStatus) -> Self { let input_modified_path = match cache_update_status { CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::InputModified { path }) => { Some(Str::from(path.as_str())) @@ -339,6 +383,10 @@ impl TaskResult { cache_update_status, CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::FspyUnsupported) ); + let fspy_unavailable = matches!( + cache_update_status, + CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::FspyUnavailable) + ); let ipc_server_error = match cache_update_status { CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::IpcServerError(err)) => { Some(vt_str::format!("{err}")) @@ -353,38 +401,13 @@ impl TaskResult { cache_update_status, CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::TrackingIncomplete) ); - - match cache_status { - CacheStatus::Hit { replayed_duration } => { - Self::CacheHit { saved_duration_ms: duration_to_ms(*replayed_duration) } - } - CacheStatus::Disabled(CacheDisabledReason::InProcessExecution) => Self::InProcess, - CacheStatus::Disabled(CacheDisabledReason::NoCacheMetadata) => Self::Spawned { - cache_status: SpawnedCacheStatus::Disabled, - outcome: spawn_outcome_from_execution( - exit_status, - saved_error, - input_modified_path, - fspy_unsupported, - ipc_server_error, - tool_disabled_cache, - tracking_incomplete, - ), - }, - CacheStatus::Miss(cache_miss) => Self::Spawned { - cache_status: SpawnedCacheStatus::Miss(SavedCacheMissReason::from_cache_miss( - cache_miss, - )), - outcome: spawn_outcome_from_execution( - exit_status, - saved_error, - input_modified_path, - fspy_unsupported, - ipc_server_error, - tool_disabled_cache, - tracking_incomplete, - ), - }, + Self { + input_modified_path, + fspy_unsupported, + fspy_unavailable, + ipc_server_error, + tool_disabled_cache, + tracking_incomplete, } } } @@ -393,11 +416,7 @@ impl TaskResult { fn spawn_outcome_from_execution( exit_status: Option, saved_error: Option<&SavedExecutionError>, - input_modified_path: Option, - fspy_unsupported: bool, - ipc_server_error: Option, - tool_disabled_cache: bool, - tracking_incomplete: bool, + details: SuccessDetails, ) -> SpawnOutcome { match (exit_status, saved_error) { // Spawn error — process never ran @@ -405,11 +424,12 @@ fn spawn_outcome_from_execution( // Process exited successfully, possible infra error (Some(status), _) if status.success() => SpawnOutcome::Success { infra_error: saved_error.cloned(), - input_modified_path, - fspy_unsupported, - ipc_server_error, - tool_disabled_cache, - tracking_incomplete, + input_modified_path: details.input_modified_path, + fspy_unsupported: details.fspy_unsupported, + fspy_unavailable: details.fspy_unavailable, + ipc_server_error: details.ipc_server_error, + tool_disabled_cache: details.tool_disabled_cache, + tracking_incomplete: details.tracking_incomplete, }, // Process exited with non-zero code (Some(status), _) => { @@ -423,14 +443,15 @@ fn spawn_outcome_from_execution( } // No exit status, no error — this is the cache hit / in-process path, // handled by TaskResult::CacheHit / InProcess before reaching here. - // If we somehow get here, treat as success. + // If we somehow get here, treat as success with no details. (None, None) => SpawnOutcome::Success { infra_error: None, - input_modified_path: None, - fspy_unsupported: false, - ipc_server_error: None, - tool_disabled_cache: false, - tracking_incomplete: false, + input_modified_path: details.input_modified_path, + fspy_unsupported: details.fspy_unsupported, + fspy_unavailable: details.fspy_unavailable, + ipc_server_error: details.ipc_server_error, + tool_disabled_cache: details.tool_disabled_cache, + tracking_incomplete: details.tracking_incomplete, }, } } @@ -589,6 +610,15 @@ impl TaskResult { "→ Not cached: `input` auto-inference isn't supported on this OS. Configure `input` manually to enable caching.", ); } + // The tracked spawn failed, so the task ran untracked. + if let Self::Spawned { + outcome: SpawnOutcome::Success { fspy_unavailable: true, .. }, .. + } = self + { + return Str::from( + "→ Not cached: file access tracking failed to start, so the task ran untracked. Configure `input` manually to enable caching.", + ); + } match self { Self::CacheHit { saved_duration_ms } => {