diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bca0d5da..2ab7e8893 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # 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** A mistyped or misplaced setting in a task now gets an error that names it. For example, `cache: { outputs: [...] }` now fails with an `unknown field` error that names `outputs` and lists the accepted settings, instead of `data did not match any variant of untagged enum UserTaskDefinition` ([vite-plus#2635](https://github.com/voidzero-dev/vite-plus/issues/2635), [#815](https://github.com/voidzero-dev/vite-task/pull/815)). - **Fixed** In cached tasks on Linux and macOS, programs that pass `-1` as a directory descriptor to calls such as `openat` no longer abort. For example, `ps -p ` from procps-ng 4.0.6 or later aborted when the process had already exited ([#862](https://github.com/voidzero-dev/vite-task/pull/862)). - **Fixed** `vp run` no longer panics before running a script that contains non-ASCII characters, such as `echo こんにちは`, and no longer cuts off such commands where it displays them ([#861](https://github.com/voidzero-dev/vite-task/pull/861)). diff --git a/crates/fspy/tests/untracked_fallback.rs b/crates/fspy/tests/untracked_fallback.rs new file mode 100644 index 000000000..7518b0ca7 --- /dev/null +++ b/crates/fspy/tests/untracked_fallback.rs @@ -0,0 +1,135 @@ +//! 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"); + }; +} + +/// 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 0a0869286..56c9bec13 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::{AccessMode, 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: AccessMode, path: &Path) { @@ -87,8 +114,26 @@ impl<'a> Client<'a> { ipc_sender.send(&mode.with_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, |path| { + resolve_exec(&mut exec, config, |path| { self.send(AccessMode::Read, 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 6aa15e4aa..8a4897d30 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,15 +25,26 @@ 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 +/// 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, 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"); + 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( @@ -50,10 +61,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 +96,8 @@ unsafe extern "C" fn execve( prog, argv, envp, + // SAFETY: the interposed execve's own arguments. + || unsafe { execve::original()(prog, argv, envp) }, ) } @@ -92,6 +117,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 +141,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 +165,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 +174,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 +182,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 +192,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 +231,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 +255,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 is -1 or a valid file descriptor, and pathname is a valid C string, from the interposed execveat call. @@ -262,6 +279,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 +294,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 +302,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..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..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 0148450d2..10288bdf5 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 @@ -385,8 +392,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 4e957636a..16bbc75d4 100644 --- a/crates/fspy_shared_unix/src/spawn/mod.rs +++ b/crates/fspy_shared_unix/src/spawn/mod.rs @@ -18,27 +18,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_read: impl FnMut(&Path), -) -> nix::Result> { +) -> nix::Result<()> { let mut on_path_read = |path: &Path| { if path.is_absolute() { on_path_read(path); @@ -50,6 +48,50 @@ pub fn handle_exec( command.resolve(&mut on_path_read, config)?; on_path_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_read: impl FnMut(&Path), +) -> nix::Result> { + resolve_exec(command, config, on_path_read)?; + prepare_exec(command, encoded_payload) +}