From 3beb6e9a3e18bab655a94a9f7802da3e30dd96be Mon Sep 17 00:00:00 2001 From: Aaron Reisman Date: Thu, 24 Sep 2026 06:45:41 +0700 Subject: [PATCH 1/2] fix(fspy): run exec untracked when injection fails If the preload can't set up tracking for an exec (for example, the seccomp filter for a static binary can't be installed because the sandbox denies the syscall), mark the trace incomplete and run the original call untracked instead of failing it. Resolution errors such as ENOENT or EACCES are still returned as-is. Each exec variant falls back to its own libc original with its original arguments, so execvp and execlp still search PATH, execveat keeps its dirfd and flags, and fexecve keeps its fd. Refs #700 --- CHANGELOG.md | 1 + crates/fspy/tests/untracked_fallback.rs | 116 ++++++++++++++++++ crates/fspy_client_unix/src/lib.rs | 60 ++++++++- crates/fspy_preload_unix/src/client.rs | 2 +- .../src/interceptions/spawn/exec/mod.rs | 70 ++++++----- .../src/interceptions/spawn/posix_spawn.rs | 17 ++- .../fspy_seccomp_unotify/src/payload/mod.rs | 14 +++ crates/fspy_shared/src/ipc/channel/mod.rs | 12 ++ .../src/ipc/channel/shm_io/writer.rs | 24 ++-- crates/fspy_shared_unix/src/spawn/mod.rs | 62 ++++++++-- 10 files changed, 324 insertions(+), 54 deletions(-) create mode 100644 crates/fspy/tests/untracked_fallback.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cadeec13..962d7b00a 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), [#701](https://github.com/voidzero-dev/vite-task/pull/701)). - **Fixed** On Windows, environment variable names used by `vp run` now match regardless of ASCII letter case. Assignments in task commands override earlier assignments and inherited variables spelled differently, and `FORCE_COLOR`, `VP_RUN_CONCURRENCY_LIMIT`, and variables requested through `@voidzero-dev/vite-task-client` are found under any spelling ([#747](https://github.com/voidzero-dev/vite-task/pull/747)). - **Changed** A task's cache settings now go inside `cache`, e.g. `cache: { env: ["NODE_ENV"], input: ["src/**"] }`; `cache: true` is the same as `cache: {}`. `env`, `untrackedEnv`, `input`, and `output` are no longer supported at the top level of a task ([#749](https://github.com/voidzero-dev/vite-task/pull/749)). - **Fixed** Cached tasks on macOS no longer intermittently fail with exit 2 and `oils I/O error (main): No such process` when a fast command finishes before the shell gets scheduled. The bundled shell that runs task commands is updated to Oils 0.38.0, which fixes this race ([#702](https://github.com/voidzero-dev/vite-task/issues/702), [#703](https://github.com/voidzero-dev/vite-task/pull/703)). diff --git a/crates/fspy/tests/untracked_fallback.rs b/crates/fspy/tests/untracked_fallback.rs new file mode 100644 index 000000000..1679162d6 --- /dev/null +++ b/crates/fspy/tests/untracked_fallback.rs @@ -0,0 +1,116 @@ +//! 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"); + }; +} + +/// The untracked fallback must replay the interposed call as it was made, +/// not as `execve`. GNU `env` runs its command through `execvp`, so a bare +/// program name found only on `PATH` still runs when injection fails; +/// forwarding it to `execve` would fail with `ENOENT` instead. +#[test] +fn execvp_fallback_keeps_path_search() { + 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 test_bin = test_bin_path(); + let output = Command::new("/usr/bin/env") + .arg(test_bin.file_name().unwrap()) + .arg("stat") + .arg("/hello") + .env_clear() + .env("PATH", test_bin.parent().unwrap()) + .env("LD_PRELOAD", PRELOAD_CDYLIB) + .env("FSPY_PAYLOAD", OsStr::from_bytes(encoded.encoded_string.as_ref())) + .current_dir("/") + .output() + .expect("failed to spawn env"); + assert!( + output.status.success(), + "the execvp fallback lost its PATH search: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let Err(RecordsLost) = receiver.close() else { + panic!("the channel did not report the untracked exec"); + }; +} diff --git a/crates/fspy_client_unix/src/lib.rs b/crates/fspy_client_unix/src/lib.rs index a0a7d58ce..d82d6853c 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, @@ -87,8 +112,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 +140,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..af0389e54 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(); 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..ab9e402f0 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, }; @@ -25,12 +25,19 @@ pub unsafe fn environ() -> *const *const c_char { unsafe { environ } } +/// Resolves, reports, and performs a tracked exec. +/// +/// `untracked` performs the interposed call as the caller made it, through +/// that function's own original: `execvp` keeps its `PATH` search, +/// `execveat` its `dirfd` and flags, `fexecve` its descriptor. It runs when +/// the injection machinery fails, so the process still execs, untracked. fn handle_exec( allocator: impl Allocator, config: ExecResolveConfig, prog: *const libc::c_char, argv: *const *const libc::c_char, envp: *const *const libc::c_char, + untracked: impl FnOnce() -> libc::c_int, ) -> libc::c_int { let client = global_client().expect("exec unexpectedly called before client initialized in ctor"); @@ -50,10 +57,22 @@ 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 the interposed call + // untracked. Its envp still carries LD_PRELOAD/FSPY_PAYLOAD, so + // each generation independently attempts tracking and + // independently degrades. + client.report_loss(); + untracked() + } } } @@ -73,6 +92,8 @@ unsafe extern "C" fn execve( prog, argv, envp, + // SAFETY: the interposed execve's own arguments. + || unsafe { execve::original()(prog, argv, envp) }, ) } @@ -92,6 +113,9 @@ unsafe extern "C" fn execl(path: *const c_char, arg0: *const c_char, valist: ... path, args.as_ptr(), environ(), + // A variadic original cannot be forwarded a collected argv; + // execl is execv over the same argv. + || execv::original()(path, args.as_ptr()), ) }) } @@ -113,6 +137,8 @@ unsafe extern "C" fn execlp(path: *const c_char, arg0: *const c_char, valist: .. path, args.as_ptr(), environ(), + // execlp is execvp over the same argv, PATH search included. + || execvp::original()(path, args.as_ptr()), ) }) } @@ -135,6 +161,8 @@ unsafe extern "C" fn execle(path: *const c_char, arg0: *const c_char, valist: .. path, args.as_ptr(), envp, + // execle is execve over the same argv and envp. + || execve::original()(path, args.as_ptr(), envp), ) }) } @@ -142,11 +170,6 @@ unsafe extern "C" fn execle(path: *const c_char, arg0: *const c_char, valist: .. intercept!(execv(64): unsafe extern "C" fn(path: *const c_char, argv: *const *const c_char) -> c_int); unsafe extern "C" fn execv(path: *const c_char, argv: *const *const c_char) -> c_int { - #[expect( - clippy::no_effect_underscore_binding, - reason = "suppresses unused warning on *::original" - )] - let _unused = execv::original; // SAFETY: path, argv are valid pointers forwarded from the interposed function; environ() returns the process environment unsafe { handle_exec( @@ -155,6 +178,7 @@ unsafe extern "C" fn execv(path: *const c_char, argv: *const *const c_char) -> c path, argv, environ(), + || execv::original()(path, argv), ) } } @@ -164,18 +188,15 @@ intercept!(execvp(64): unsafe extern "C" fn( argv: *const *const libc::c_char, ) -> c_int); unsafe extern "C" fn execvp(prog: *const c_char, argv: *const *const c_char) -> c_int { - #[expect( - clippy::no_effect_underscore_binding, - reason = "suppresses unused warning on *::original" - )] - let _unused = execvp::original; - // SAFETY: environ() returns the valid process environment pointer handle_exec( fspy_nostd_alloc::pooled_bump(), ExecResolveConfig::search_path_enabled(None), prog, argv, + // SAFETY: environ() returns the valid process environment pointer unsafe { environ() }, + // SAFETY: the interposed execvp's own arguments. + || unsafe { execvp::original()(prog, argv) }, ) } @@ -206,17 +227,14 @@ mod linux_only { argv: *const *const libc::c_char, envp: *const *const libc::c_char, ) -> c_int { - #[expect( - clippy::no_effect_underscore_binding, - reason = "suppresses unused warning on *::original" - )] - let _unused = execvpe::original; handle_exec( fspy_nostd_alloc::pooled_bump(), ExecResolveConfig::search_path_enabled(None), file, argv, envp, + // SAFETY: the interposed execvpe's own arguments. + || unsafe { execvpe::original()(file, argv, envp) }, ) } intercept!(execveat(64): unsafe extern "C" fn( @@ -233,11 +251,6 @@ mod linux_only { envp: *const *mut libc::c_char, flags: c_int, // TODO: conform to semantics of flags ) -> libc::c_int { - #[expect( - clippy::no_effect_underscore_binding, - reason = "suppresses unused warning on *::original" - )] - let _unused = execveat::original; let arena = fspy_nostd_alloc::pooled_bump(); // SAFETY: dirfd and pathname are valid arguments from the interposed execveat call. @@ -262,6 +275,8 @@ mod linux_only { abs_path.as_ptr().cast(), argv.cast(), envp.cast(), + // SAFETY: the interposed execveat's own arguments. + || unsafe { execveat::original()(dirfd, pathname, argv, envp, flags) }, ) } @@ -275,11 +290,6 @@ mod linux_only { argv: *const *const libc::c_char, envp: *const *const libc::c_char, ) -> libc::c_int { - #[expect( - clippy::no_effect_underscore_binding, - reason = "suppresses unused warning on *::original" - )] - let _unused = fexecve::original; let prog = format!("/proc/self/fd/{fd}\0"); let prog = prog.as_ptr(); handle_exec( @@ -288,6 +298,10 @@ mod linux_only { prog.cast(), argv, envp, + // The descriptor itself, not its /proc path: a sandbox without + // /proc can still fexecve. + // SAFETY: the interposed fexecve's own arguments. + || unsafe { fexecve::original()(fd, 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..44f5f1ce7 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, }; @@ -78,8 +78,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..c4bd5342c 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 const 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..510ea7a5e 100644 --- a/crates/fspy_shared/src/ipc/channel/mod.rs +++ b/crates/fspy_shared/src/ipc/channel/mod.rs @@ -216,6 +216,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 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) +} From f3f2a54d838cdf3d8c334421054fb65098957db5 Mon Sep 17 00:00:00 2001 From: Aaron Reisman Date: Thu, 24 Sep 2026 06:47:38 +0700 Subject: [PATCH 2/2] fix(fspy): don't abort traced processes when the preload has no payload The preload constructor panicked when FSPY_PAYLOAD was missing or invalid, or when the shared-memory channel couldn't be opened, which aborts the host process. This happens with a leaked LD_PRELOAD in a sandbox that clears the environment. Leave the client unset in that case and forward every exec and posix_spawn call to its original. --- CHANGELOG.md | 1 + crates/fspy/tests/untracked_fallback.rs | 19 +++++++ crates/fspy_client_unix/src/lib.rs | 18 ++++--- crates/fspy_preload_unix/src/client.rs | 26 ++++++--- .../src/interceptions/spawn/exec/mod.rs | 10 ++-- .../src/interceptions/spawn/posix_spawn.rs | 10 +++- crates/fspy_shared/src/ipc/channel/mod.rs | 53 +++++++++---------- 7 files changed, 87 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 962d7b00a..6b72ed1f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +- **Fixed** A process that inherits Vite+'s file-access tracking library without its configuration, such as in a sandbox that scrubs environment variables, no longer aborts at startup; it runs untracked instead ([#753](https://github.com/voidzero-dev/vite-task/pull/753)). - **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), [#701](https://github.com/voidzero-dev/vite-task/pull/701)). - **Fixed** On Windows, environment variable names used by `vp run` now match regardless of ASCII letter case. Assignments in task commands override earlier assignments and inherited variables spelled differently, and `FORCE_COLOR`, `VP_RUN_CONCURRENCY_LIMIT`, and variables requested through `@voidzero-dev/vite-task-client` are found under any spelling ([#747](https://github.com/voidzero-dev/vite-task/pull/747)). - **Changed** A task's cache settings now go inside `cache`, e.g. `cache: { env: ["NODE_ENV"], input: ["src/**"] }`; `cache: true` is the same as `cache: {}`. `env`, `untrackedEnv`, `input`, and `output` are no longer supported at the top level of a task ([#749](https://github.com/voidzero-dev/vite-task/pull/749)). diff --git a/crates/fspy/tests/untracked_fallback.rs b/crates/fspy/tests/untracked_fallback.rs index 1679162d6..7518b0ca7 100644 --- a/crates/fspy/tests/untracked_fallback.rs +++ b/crates/fspy/tests/untracked_fallback.rs @@ -114,3 +114,22 @@ fn execvp_fallback_keeps_path_search() { 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 d82d6853c..a3480fa72 100644 --- a/crates/fspy_client_unix/src/lib.rs +++ b/crates/fspy_client_unix/src/lib.rs @@ -76,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 @@ -93,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) { diff --git a/crates/fspy_preload_unix/src/client.rs b/crates/fspy_preload_unix/src/client.rs index af0389e54..e3d9b02cb 100644 --- a/crates/fspy_preload_unix/src/client.rs +++ b/crates/fspy_preload_unix/src/client.rs @@ -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 ab9e402f0..d3dfbd12d 100644 --- a/crates/fspy_preload_unix/src/interceptions/spawn/exec/mod.rs +++ b/crates/fspy_preload_unix/src/interceptions/spawn/exec/mod.rs @@ -30,7 +30,8 @@ pub unsafe fn environ() -> *const *const c_char { /// `untracked` performs the interposed call as the caller made it, through /// that function's own original: `execvp` keeps its `PATH` search, /// `execveat` its `dirfd` and flags, `fexecve` its descriptor. It runs when -/// the injection machinery fails, so the process still execs, untracked. +/// this process has no tracking client or the injection machinery fails, so +/// the process still execs, untracked. fn handle_exec( allocator: impl Allocator, config: ExecResolveConfig, @@ -39,8 +40,11 @@ fn handle_exec( envp: *const *const libc::c_char, untracked: impl FnOnce() -> libc::c_int, ) -> 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. + return untracked(); + }; // SAFETY: prog, argv, and envp are valid pointers to C strings/arrays forwarded from the interposed exec function let result = unsafe { client.handle_exec( 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 44f5f1ce7..e4d0ed1d9 100644 --- a/crates/fspy_preload_unix/src/interceptions/spawn/posix_spawn.rs +++ b/crates/fspy_preload_unix/src/interceptions/spawn/posix_spawn.rs @@ -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 { diff --git a/crates/fspy_shared/src/ipc/channel/mod.rs b/crates/fspy_shared/src/ipc/channel/mod.rs index 510ea7a5e..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; @@ -370,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