Support pre-existing LD_PRELOAD/DYLD_INSERT_LIBRARIES

CHANGELOG.md

@@ -1,5 +1,6 @@
 # Changelog
 
+- **Fixed** `vp run` no longer aborts with `failed to prepare the command for injection: Invalid argument` when the user environment already has `LD_PRELOAD` (Linux) or `DYLD_INSERT_LIBRARIES` (macOS) set. The tracer shim is now appended to any existing value and placed last, so user preloads keep their symbol-interposition precedence ([#340](https://github.com/voidzero-dev/vite-task/issues/340))
 - **Changed** Arguments passed after a task name (e.g. `vp run test some-filter`) are now forwarded only to that task. Tasks pulled in via `dependsOn` no longer receive them ([#324](https://github.com/voidzero-dev/vite-task/issues/324))
 - **Fixed** Windows file access tracking no longer panics when a task touches malformed paths that cannot be represented as workspace-relative inputs ([#330](https://github.com/voidzero-dev/vite-task/pull/330))
 - **Fixed** `vp run --cache` now supports running without a task specifier and opens the interactive task selector, matching bare `vp run` behavior ([#312](https://github.com/voidzero-dev/vite-task/pull/313))

crates/fspy_shared_unix/src/exec/mod.rs

@@ -175,3 +175,122 @@ pub fn ensure_env(
     envs.push((name.to_owned(), Some(value.to_owned())));
     Ok(())
 }
+
+/// Ensures `value` is the trailing colon-separated entry of env var `name`.
+///
+/// Used for `LD_PRELOAD` / `DYLD_INSERT_LIBRARIES`, which the dynamic loader
+/// treats as colon-separated lists. Appending (rather than overwriting)
+/// preserves any user-provided preload, and appending to the *end* keeps
+/// fspy's shim as the last interposer so a user preload that short-circuits
+/// a call (returning without forwarding to libc) stays invisible to fspy —
+/// mirroring what the OS actually did.
+///
+/// - Absent: inserts `(name, value)`.
+/// - Present with `value` already as the last colon-separated entry: no
+///   change (idempotent across nested execs within the preloaded shim).
+/// - Present otherwise: rewrites to `{existing}:{value}`. If `existing` is
+///   empty, sets to `value` alone to avoid a leading `:` (which glibc's
+///   `ld.so` interprets as the current directory).
+pub fn append_path_env(
+    envs: &mut Vec<(BString, Option<BString>)>,
+    name: impl AsRef<BStr>,
+    value: impl AsRef<BStr>,
+) {
+    let name = name.as_ref();
+    let value = value.as_ref();
+    if let Some(entry) = envs.iter_mut().find(|(n, _)| n == name) {
+        let existing: &[u8] = entry.1.as_deref().map_or(&[][..], |v| v.as_ref());
+        let value_bytes: &[u8] = value.as_ref();
+        let already_last = existing == value_bytes
+            || (existing.len() > value_bytes.len()
+                && existing.ends_with(value_bytes)
+                && existing[existing.len() - value_bytes.len() - 1] == b':');
+        if already_last {
+            return;
+        }
+        let mut new_value = Vec::with_capacity(existing.len() + 1 + value_bytes.len());
+        if !existing.is_empty() {
+            new_value.extend_from_slice(existing);
+            new_value.push(b':');
+        }
+        new_value.extend_from_slice(value_bytes);
+        entry.1 = Some(BString::from(new_value));
+    } else {
+        envs.push((name.to_owned(), Some(value.to_owned())));
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use bstr::BString;
+
+    use super::append_path_env;
+
+    fn env(envs: &[(BString, Option<BString>)], name: &[u8]) -> Option<Vec<u8>> {
+        envs.iter()
+            .find(|(n, _)| AsRef::<[u8]>::as_ref(n) == name)
+            .and_then(|(_, v)| v.as_ref().map(|v| AsRef::<[u8]>::as_ref(v).to_vec()))
+    }
+
+    #[test]
+    fn inserts_when_absent() {
+        let mut envs: Vec<(BString, Option<BString>)> = vec![];
+        append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
+        assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/a.so".to_vec()));
+    }
+
+    #[test]
+    fn noop_when_equal() {
+        let mut envs = vec![(BString::from("LD_PRELOAD"), Some(BString::from("/a.so")))];
+        append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
+        assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/a.so".to_vec()));
+    }
+
+    #[test]
+    fn noop_when_value_is_last_entry() {
+        let mut envs = vec![(BString::from("LD_PRELOAD"), Some(BString::from("/user.so:/a.so")))];
+        append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
+        assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/user.so:/a.so".to_vec()));
+    }
+
+    #[test]
+    fn appends_with_colon_when_present_and_different() {
+        let mut envs = vec![(BString::from("LD_PRELOAD"), Some(BString::from("/user.so")))];
+        append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
+        assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/user.so:/a.so".to_vec()));
+    }
+
+    #[test]
+    fn sets_without_leading_colon_when_existing_is_empty() {
+        let mut envs = vec![(BString::from("LD_PRELOAD"), Some(BString::from("")))];
+        append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
+        assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/a.so".to_vec()));
+    }
+
+    #[test]
+    fn idempotent_on_repeat() {
+        let mut envs: Vec<(BString, Option<BString>)> = vec![];
+        append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
+        append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
+        append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
+        assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/a.so".to_vec()));
+    }
+
+    #[test]
+    fn does_not_false_match_prefix_without_preceding_colon() {
+        // `lib/a.so` ends with `/a.so` as bytes, but the preceding byte is
+        // `b` not `:`, so it must NOT be treated as already-present.
+        let mut envs = vec![(BString::from("LD_PRELOAD"), Some(BString::from("/lib/a.so")))];
+        append_path_env(&mut envs, "LD_PRELOAD", "a.so");
+        assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/lib/a.so:a.so".to_vec()));
+    }
+
+    #[test]
+    fn inserts_when_present_with_none_value() {
+        // An env var present in the list but with `None` value (name without
+        // `=`) should be rewritten to `Some(value)`.
+        let mut envs = vec![(BString::from("LD_PRELOAD"), None)];
+        append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
+        assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/a.so".to_vec()));
+    }
+}

crates/fspy_shared_unix/src/spawn/linux/mod.rs

@@ -6,7 +6,11 @@ use fspy_seccomp_unotify::{payload::SeccompPayload, target::install_target};
 use memmap2::Mmap;
 
 #[cfg(not(target_env = "musl"))]
-use crate::{elf, exec::ensure_env, open_exec::open_executable};
+use crate::{
+    elf,
+    exec::{append_path_env, ensure_env},
+    open_exec::open_executable,
+};
 use crate::{
     exec::Exec,
     payload::{EncodedPayload, PAYLOAD_ENV_NAME},
@@ -40,11 +44,15 @@ pub fn handle_exec(
             nix::Error::try_from(io_error).unwrap_or(nix::Error::UnknownErrno)
         })?;
         if elf::is_dynamically_linked_to_libc(executable_mmap)? {
-            ensure_env(
+            // Append (don't overwrite) so a user-provided LD_PRELOAD keeps
+            // working. fspy's shim goes last so user preloads that
+            // short-circuit a libc call stay invisible to fspy — what the
+            // OS actually executed is what we want to record.
+            append_path_env(
                 &mut command.envs,
                 LD_PRELOAD,
                 encoded_payload.payload.preload_path.as_os_str().as_bytes(),
-            )?;
+            );
             ensure_env(&mut command.envs, PAYLOAD_ENV_NAME, &encoded_payload.encoded_string)?;
             return Ok(None);
         }

crates/fspy_shared_unix/src/spawn/macos.rs

@@ -8,7 +8,7 @@ use std::{
 use phf::{Set, phf_set};
 
 use crate::{
-    exec::{Exec, ensure_env},
+    exec::{Exec, append_path_env, ensure_env},
     payload::{EncodedPayload, PAYLOAD_ENV_NAME},
 };
 
@@ -60,11 +60,15 @@ pub fn handle_exec(
     };
 
     if injectable {
-        ensure_env(
+        // Append (don't overwrite) so a user-provided DYLD_INSERT_LIBRARIES
+        // keeps working. fspy's shim goes last so user preloads that
+        // short-circuit a libc call stay invisible to fspy — what the OS
+        // actually executed is what we want to record.
+        append_path_env(
             &mut command.envs,
             DYLD_INSERT_LIBRARIES,
             encoded_payload.payload.preload_path.as_os_str().as_bytes(),
-        )?;
+        );
         ensure_env(&mut command.envs, PAYLOAD_ENV_NAME, &encoded_payload.encoded_string)?;
     } else {
         command.envs.retain(|(name, _)| {

crates/preload_test_lib/Cargo.toml

@@ -0,0 +1,16 @@
+[package]
+name = "preload_test_lib"
+version = "0.0.0"
+edition.workspace = true
+publish = false
+
+[lib]
+crate-type = ["cdylib"]
+test = false
+doctest = false
+
+[target.'cfg(target_os = "linux")'.dependencies]
+libc = { workspace = true }
+
+[lints]
+workspace = true

crates/preload_test_lib/src/lib.rs

@@ -0,0 +1,161 @@
+//! Test-only `LD_PRELOAD` library used by the `preexisting_ld_preload` e2e
+//! fixture. Intercepts `open`/`openat` (and their `64` variants) to exercise
+//! two behaviours fspy must tolerate when appended to a pre-existing
+//! `LD_PRELOAD` list:
+//!
+//! 1. For paths containing the marker `preload_test_short_circuit`, the
+//!    call is short-circuited with `ENOENT` *without* forwarding to the
+//!    next preloaded library. Because fspy is appended after this library
+//!    in the preload list, fspy never observes the call — exactly the
+//!    property we want to verify.
+//! 2. For every other path the call is forwarded via
+//!    `dlsym(RTLD_NEXT, …)`, so fspy still sees the real accesses and can
+//!    track them as cache inputs.
+#![cfg(target_os = "linux")]
+#![feature(c_variadic)]
+
+use std::{
+    ffi::{CStr, c_char, c_int},
+    sync::OnceLock,
+};
+
+const MARKER: &[u8] = b"preload_test_short_circuit";
+
+fn should_short_circuit(path: *const c_char) -> bool {
+    if path.is_null() {
+        return false;
+    }
+    // SAFETY: callers of `open`/`openat` pass a valid NUL-terminated C string
+    // (or NULL, handled above).
+    let bytes = unsafe { CStr::from_ptr(path) }.to_bytes();
+    bytes.windows(MARKER.len()).any(|w| w == MARKER)
+}
+
+fn fail_with_enoent() -> c_int {
+    // SAFETY: `__errno_location` is async-signal-safe and always returns a
+    // valid pointer to the per-thread errno.
+    unsafe { *libc::__errno_location() = libc::ENOENT };
+    -1
+}
+
+const fn has_mode_arg(flags: c_int) -> bool {
+    flags & libc::O_CREAT != 0 || flags & libc::O_TMPFILE != 0
+}
+
+type OpenFn = unsafe extern "C" fn(*const c_char, c_int, ...) -> c_int;
+type OpenatFn = unsafe extern "C" fn(c_int, *const c_char, c_int, ...) -> c_int;
+
+fn load_next_fn<F: Copy>(name: &CStr) -> F {
+    // SAFETY: `dlsym` with `RTLD_NEXT` returns either NULL or a valid
+    // function pointer for a symbol that must exist in libc. The cast is
+    // valid because the caller supplies a `F` whose layout is a function
+    // pointer of the corresponding libc signature.
+    let ptr = unsafe { libc::dlsym(libc::RTLD_NEXT, name.as_ptr()) };
+    assert!(!ptr.is_null(), "dlsym RTLD_NEXT returned null");
+    // SAFETY: see above.
+    unsafe { std::mem::transmute_copy(&ptr) }
+}
+
+fn next_open() -> OpenFn {
+    static S: OnceLock<OpenFn> = OnceLock::new();
+    *S.get_or_init(|| load_next_fn(c"open"))
+}
+fn next_open64() -> OpenFn {
+    static S: OnceLock<OpenFn> = OnceLock::new();
+    *S.get_or_init(|| load_next_fn(c"open64"))
+}
+fn next_openat() -> OpenatFn {
+    static S: OnceLock<OpenatFn> = OnceLock::new();
+    *S.get_or_init(|| load_next_fn(c"openat"))
+}
+fn next_openat64() -> OpenatFn {
+    static S: OnceLock<OpenatFn> = OnceLock::new();
+    *S.get_or_init(|| load_next_fn(c"openat64"))
+}
+
+/// # Safety
+/// Interposer over libc `open(2)`; same contract as the real function. Must
+/// only be called by the dynamic loader after installation via `LD_PRELOAD`.
+#[unsafe(no_mangle)]
+pub unsafe extern "C" fn open(path: *const c_char, flags: c_int, mut args: ...) -> c_int {
+    if should_short_circuit(path) {
+        return fail_with_enoent();
+    }
+    if has_mode_arg(flags) {
+        // SAFETY: `O_CREAT`/`O_TMPFILE` guarantees a `mode_t` follows per
+        // the `open(2)` contract.
+        let mode: libc::mode_t = unsafe { args.arg() };
+        // SAFETY: forwarding the caller's arguments unchanged.
+        unsafe { next_open()(path, flags, mode) }
+    } else {
+        // SAFETY: forwarding the caller's arguments unchanged.
+        unsafe { next_open()(path, flags) }
+    }
+}
+
+/// # Safety
+/// Interposer over libc `open64(2)`; same contract as the real function.
+#[unsafe(no_mangle)]
+pub unsafe extern "C" fn open64(path: *const c_char, flags: c_int, mut args: ...) -> c_int {
+    if should_short_circuit(path) {
+        return fail_with_enoent();
+    }
+    if has_mode_arg(flags) {
+        // SAFETY: `O_CREAT`/`O_TMPFILE` guarantees a `mode_t` follows per
+        // the `open64(2)` contract.
+        let mode: libc::mode_t = unsafe { args.arg() };
+        // SAFETY: forwarding the caller's arguments unchanged.
+        unsafe { next_open64()(path, flags, mode) }
+    } else {
+        // SAFETY: forwarding the caller's arguments unchanged.
+        unsafe { next_open64()(path, flags) }
+    }
+}
+
+/// # Safety
+/// Interposer over libc `openat(2)`; same contract as the real function.
+#[unsafe(no_mangle)]
+pub unsafe extern "C" fn openat(
+    dirfd: c_int,
+    path: *const c_char,
+    flags: c_int,
+    mut args: ...
+) -> c_int {
+    if should_short_circuit(path) {
+        return fail_with_enoent();
+    }
+    if has_mode_arg(flags) {
+        // SAFETY: `O_CREAT`/`O_TMPFILE` guarantees a `mode_t` follows per
+        // the `openat(2)` contract.
+        let mode: libc::mode_t = unsafe { args.arg() };
+        // SAFETY: forwarding the caller's arguments unchanged.
+        unsafe { next_openat()(dirfd, path, flags, mode) }
+    } else {
+        // SAFETY: forwarding the caller's arguments unchanged.
+        unsafe { next_openat()(dirfd, path, flags) }
+    }
+}
+
+/// # Safety
+/// Interposer over libc `openat64(2)`; same contract as the real function.
+#[unsafe(no_mangle)]
+pub unsafe extern "C" fn openat64(
+    dirfd: c_int,
+    path: *const c_char,
+    flags: c_int,
+    mut args: ...
+) -> c_int {
+    if should_short_circuit(path) {
+        return fail_with_enoent();
+    }
+    if has_mode_arg(flags) {
+        // SAFETY: `O_CREAT`/`O_TMPFILE` guarantees a `mode_t` follows per
+        // the `openat64(2)` contract.
+        let mode: libc::mode_t = unsafe { args.arg() };
+        // SAFETY: forwarding the caller's arguments unchanged.
+        unsafe { next_openat64()(dirfd, path, flags, mode) }
+    } else {
+        // SAFETY: forwarding the caller's arguments unchanged.
+        unsafe { next_openat64()(dirfd, path, flags) }
+    }
+}