fix(fspy): forward calls with -1 as the directory descriptor

CHANGELOG.md

@@ -1,5 +1,6 @@
 # Changelog
 
+- **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 <pid>` 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)).
 - **Fixed** On Linux, concurrent system calls in tracked programs, such as `pnpm dlx`, no longer cause a stack overflow ([#854](https://github.com/voidzero-dev/vite-task/pull/854)).
 - **Fixed** Cached tasks can now run scripts whose shebang names an interpreter path longer than 126 bytes. Before, the path was cut short and the script failed to start or ran the wrong program. On macOS, text after a `#` in the shebang line is now ignored, as it is when the script runs on its own ([#788](https://github.com/voidzero-dev/vite-task/pull/788)).

crates/fspy/tests/invalid_dirfd.rs

@@ -0,0 +1,193 @@
+//! A tracked program may pass `-1` as a directory descriptor, typically a
+//! failed `open` result that it does not check. With a relative path the call
+//! fails with `EBADF`; with an absolute path the descriptor is ignored. The
+//! interposers must forward both cases instead of aborting the program.
+//!
+//! procps-ng's `ps -p <pid>` does this when the process has exited: it opens
+//! `/proc/<pid>`, then calls `openat(-1, "status", ...)`.
+//!
+//! Directory descriptors exist only on Unix; Windows has no equivalent calls.
+#![cfg(unix)]
+
+mod test_utils;
+
+use fspy::PathAccess;
+use test_log::test;
+use test_utils::{assert_contains, assert_not_contains};
+
+fn errno() -> libc::c_int {
+    std::io::Error::last_os_error().raw_os_error().unwrap()
+}
+
+/// `STATX_BASIC_STATS`. The libc crate does not define it for every Linux
+/// target, such as musl without `musl_v1_2_3`.
+#[cfg(target_os = "linux")]
+const STATX_BASIC_STATS: libc::c_uint = 0x7ff;
+
+/// Storage for the kernel's 256-byte `struct statx`. The libc crate does not
+/// define the struct for every Linux target.
+#[cfg(target_os = "linux")]
+#[repr(C, align(8))]
+struct StatxBuf([u8; 256]);
+
+/// Calls `statx` through `syscall`, which fspy intercepts on glibc.
+///
+/// The libc `statx` wrapper is not called directly: glibc added it in 2.28,
+/// so a test built for an older glibc fails to link.
+#[cfg(target_os = "linux")]
+fn syscall_statx(
+    dirfd: libc::c_int,
+    path: *const libc::c_char,
+    flags: libc::c_int,
+) -> libc::c_long {
+    let mut buf = StatxBuf([0; 256]);
+    // SAFETY: `path` is null or a valid C string, and `buf` is writable
+    // storage of the size that the kernel writes.
+    unsafe {
+        libc::syscall(
+            libc::SYS_statx,
+            dirfd,
+            path,
+            flags,
+            STATX_BASIC_STATS,
+            core::ptr::from_mut(&mut buf),
+        )
+    }
+}
+
+/// Calls the glibc `statx` wrapper, resolved at run time. With `LD_PRELOAD`,
+/// the lookup finds the fspy interposer first.
+///
+/// # Panics
+///
+/// Panics if glibc has no `statx` (added in 2.28).
+#[cfg(all(target_os = "linux", target_env = "gnu"))]
+fn glibc_statx(dirfd: libc::c_int, path: *const libc::c_char, flags: libc::c_int) -> libc::c_int {
+    type Statx = unsafe extern "C" fn(
+        libc::c_int,
+        *const libc::c_char,
+        libc::c_int,
+        libc::c_uint,
+        *mut StatxBuf,
+    ) -> libc::c_int;
+    // SAFETY: the name is a valid C string.
+    let symbol = unsafe { libc::dlsym(libc::RTLD_DEFAULT, c"statx".as_ptr()) };
+    assert!(!symbol.is_null(), "glibc has no statx");
+    // SAFETY: glibc's `statx` has this signature.
+    let statx = unsafe { core::mem::transmute::<*mut libc::c_void, Statx>(symbol) };
+    let mut buf = StatxBuf([0; 256]);
+    // SAFETY: `path` is null or a valid C string, and `buf` is writable
+    // storage of the size that the kernel writes.
+    unsafe { statx(dirfd, path, flags, STATX_BASIC_STATS, core::ptr::from_mut(&mut buf)) }
+}
+
+/// Calls the glibc `execveat` wrapper, resolved at run time for the same
+/// reason as [`glibc_statx`].
+///
+/// # Panics
+///
+/// Panics if glibc has no `execveat` (added in 2.34).
+#[cfg(all(target_os = "linux", target_env = "gnu"))]
+fn glibc_execveat(dirfd: libc::c_int, path: &core::ffi::CStr) -> libc::c_int {
+    type Execveat = unsafe extern "C" fn(
+        libc::c_int,
+        *const libc::c_char,
+        *const *const libc::c_char,
+        *const *const libc::c_char,
+        libc::c_int,
+    ) -> libc::c_int;
+    // SAFETY: the name is a valid C string.
+    let symbol = unsafe { libc::dlsym(libc::RTLD_DEFAULT, c"execveat".as_ptr()) };
+    assert!(!symbol.is_null(), "glibc has no execveat");
+    // SAFETY: glibc's `execveat` has this signature.
+    let execveat = unsafe { core::mem::transmute::<*mut libc::c_void, Execveat>(symbol) };
+    let argv = [path.as_ptr(), core::ptr::null()];
+    let envp = [core::ptr::null()];
+    // SAFETY: `path` is a valid C string, and `argv` and `envp` are
+    // null-terminated arrays of valid C strings.
+    unsafe { execveat(dirfd, path.as_ptr(), argv.as_ptr(), envp.as_ptr(), 0) }
+}
+
+#[test(tokio::test)]
+async fn relative_path_at_invalid_dirfd() -> anyhow::Result<()> {
+    let accesses = track_fn!((), |(): ()| {
+        let path = c"hello";
+        // SAFETY: every call below receives a valid C string and writable
+        // output storage; the kernel rejects the descriptor.
+        unsafe {
+            assert_eq!(libc::openat(-1, path.as_ptr(), libc::O_RDONLY), -1);
+            assert_eq!(errno(), libc::EBADF);
+
+            let mut stat = std::mem::MaybeUninit::<libc::stat>::uninit();
+            assert_eq!(libc::fstatat(-1, path.as_ptr(), stat.as_mut_ptr(), 0), -1);
+            assert_eq!(errno(), libc::EBADF);
+
+            assert_eq!(libc::faccessat(-1, path.as_ptr(), libc::F_OK, 0), -1);
+            assert_eq!(errno(), libc::EBADF);
+
+            assert!(libc::fdopendir(-1).is_null());
+            assert_eq!(errno(), libc::EBADF);
+        }
+        #[cfg(target_os = "linux")]
+        {
+            assert_eq!(syscall_statx(-1, path.as_ptr(), 0), -1);
+            assert_eq!(errno(), libc::EBADF);
+        }
+        #[cfg(all(target_os = "linux", target_env = "gnu"))]
+        {
+            assert_eq!(glibc_statx(-1, path.as_ptr(), 0), -1);
+            assert_eq!(errno(), libc::EBADF);
+
+            assert_eq!(glibc_execveat(-1, path), -1);
+            assert_eq!(errno(), libc::EBADF);
+        }
+    })
+    .await?;
+    // `-1` is not the current directory. The child inherits the current
+    // directory of the test.
+    assert_not_contains(&accesses, &std::env::current_dir()?.join("hello"));
+    Ok(())
+}
+
+/// `statx` with a null path and `AT_EMPTY_PATH` uses only the descriptor.
+#[cfg(target_os = "linux")]
+#[test(tokio::test)]
+async fn empty_path_at_invalid_dirfd() -> anyhow::Result<()> {
+    track_fn!((), |(): ()| {
+        // Linux 6.11 and later accept a null path with `AT_EMPTY_PATH` and
+        // reject the descriptor. Earlier kernels reject the null path.
+        let rejected = [libc::EBADF, libc::EFAULT];
+
+        assert_eq!(syscall_statx(-1, core::ptr::null(), libc::AT_EMPTY_PATH), -1);
+        assert!(rejected.contains(&errno()), "unexpected errno {}", errno());
+
+        #[cfg(target_env = "gnu")]
+        {
+            assert_eq!(glibc_statx(-1, core::ptr::null(), libc::AT_EMPTY_PATH), -1);
+            assert!(rejected.contains(&errno()), "unexpected errno {}", errno());
+        }
+    })
+    .await?;
+    Ok(())
+}
+
+#[test(tokio::test)]
+async fn absolute_path_at_invalid_dirfd() -> anyhow::Result<()> {
+    let tmp_dir = tempfile::tempdir()?;
+    let tmp_path = std::fs::canonicalize(tmp_dir.path())?.join("hello");
+    std::fs::write(&tmp_path, "")?;
+
+    let accesses = track_fn!(tmp_path.to_str().unwrap().to_owned(), |tmp_path: String| {
+        let path = std::ffi::CString::new(tmp_path).unwrap();
+        // SAFETY: `path` is a valid C string; an absolute path makes the
+        // kernel ignore the descriptor.
+        let fd = unsafe { libc::openat(-1, path.as_ptr(), libc::O_RDONLY) };
+        assert!(fd >= 0, "openat failed with errno {}", errno());
+        // SAFETY: `fd` was opened above and is closed once.
+        unsafe { libc::close(fd) };
+    })
+    .await?;
+    assert_contains(&accesses, &tmp_path, PathAccess::Read);
+
+    Ok(())
+}

crates/fspy/tests/test_utils/mod.rs

@@ -57,6 +57,36 @@ pub fn assert_contains<'a>(
     );
 }
 
+/// Asserts that `unexpected_path` was not accessed in any way.
+///
+/// # Panics
+///
+/// Panics if the path was accessed.
+#[expect(
+    clippy::allow_attributes,
+    reason = "allow attribute required for conditionally-used helper"
+)]
+#[allow(dead_code, reason = "not all test files use this helper")]
+#[track_caller]
+pub fn assert_not_contains(accesses: &PathAccessIterable, unexpected_path: &Path) {
+    let matching = accesses
+        .iter()
+        .filter(|access| {
+            access.paths().any(|path| {
+                path.strip_path_prefix(unexpected_path, |strip_result| {
+                    strip_result.is_ok_and(|stripped| stripped.as_os_str().is_empty())
+                })
+            })
+        })
+        .collect::<Vec<_>>();
+    assert!(
+        matching.is_empty(),
+        "Expected no accesses to {}, found: {:?}",
+        unexpected_path.display(),
+        matching
+    );
+}
+
 /// Spawns a subprocess that executes the given function with file access tracking.
 ///
 /// - $arg: The argument to pass to the function

crates/fspy_client_unix/src/convert.rs

@@ -107,19 +107,38 @@ impl ToAbsolutePath for BorrowedFd<'_> {
     }
 }
 
-pub struct PathAt<'fd, 'path>(pub BorrowedFd<'fd>, pub fspy_nostd::CStr<'path, fspy_nostd::Thin>);
+impl<T: ToAbsolutePath> ToAbsolutePath for Option<T> {
+    fn to_absolute_path<'a, A: Allocator>(
+        self,
+        allocator: &'a A,
+    ) -> nix::Result<Option<fspy_nostd::CStr<'a, fspy_nostd::Fat>>>
+    where
+        Self: 'a,
+    {
+        self.map_or(Ok(None), |path| path.to_absolute_path(allocator))
+    }
+}
+
+/// A pathname resolved against a directory descriptor, as in `openat`.
+///
+/// The descriptor is `None` when the caller passed `-1`. The kernel ignores
+/// it for an absolute pathname and fails a relative one with `EBADF`.
+pub struct PathAt<'fd, 'path>(
+    pub Option<BorrowedFd<'fd>>,
+    pub fspy_nostd::CStr<'path, fspy_nostd::Thin>,
+);
 
 impl PathAt<'_, '_> {
     /// Borrows raw directory-descriptor and pathname arguments.
     ///
     /// # Safety
     ///
-    /// `fd` must remain valid while the returned value is used, and `path`
-    /// must point to a valid NUL-terminated string.
+    /// `fd` must be `-1` or remain valid while the returned value is used,
+    /// and `path` must point to a valid NUL-terminated string.
     #[must_use]
     pub const unsafe fn borrow_raw(fd: c_int, path: *const c_char) -> Self {
         // SAFETY: both invariants are upheld by the caller.
-        Self(unsafe { BorrowedFd::borrow_raw(fd) }, unsafe {
+        Self(unsafe { BorrowedFd::try_borrow_raw(fd) }, unsafe {
             fspy_nostd::CStr::from_ptr(path.cast())
         })
     }
@@ -140,7 +159,11 @@ impl ToAbsolutePath for PathAt<'_, '_> {
             // Already absolute, and already NUL-terminated by the caller.
             Ok(Some(counted))
         } else {
-            let Some(mut base) = get_fd_path(allocator, self.0)? else {
+            // A relative pathname at `-1` accesses nothing: the call fails.
+            let Some(dirfd) = self.0 else {
+                return Ok(None);
+            };
+            let Some(mut base) = get_fd_path(allocator, dirfd)? else {
                 return Ok(None);
             };
             if !pathname.is_empty() {
@@ -167,7 +190,7 @@ impl ToAbsolutePath for fspy_nostd::CStr<'_, fspy_nostd::Thin> {
     where
         Self: 'a,
     {
-        PathAt(CWD, self).to_absolute_path(allocator)
+        PathAt(Some(CWD), self).to_absolute_path(allocator)
     }
 }
 

crates/fspy_nostd/src/fd.rs

@@ -36,6 +36,27 @@ impl BorrowedFd<'_> {
         Self { fd, lifetime: PhantomData }
     }
 
+    /// Borrows a descriptor argument passed by a caller, or returns `None`
+    /// when it is `-1`.
+    ///
+    /// Callers can pass `-1` unchecked from a failed call such as `open`.
+    /// The kernel rejects it with `EBADF` wherever it uses the descriptor,
+    /// so it is not a caller error to report here.
+    ///
+    /// # Safety
+    ///
+    /// Any value other than `-1` must satisfy [`Self::borrow_raw`].
+    #[must_use]
+    pub const unsafe fn try_borrow_raw(fd: RawFd) -> Option<Self> {
+        if fd == -1 {
+            None
+        } else {
+            // SAFETY: `fd` is not `-1`, and the caller upholds the
+            // remaining requirements of `borrow_raw`.
+            Some(unsafe { Self::borrow_raw(fd) })
+        }
+    }
+
     /// Returns the raw descriptor without transferring ownership.
     #[must_use]
     pub const fn as_raw_fd(self) -> RawFd {

crates/fspy_preload_unix/src/interceptions/access.rs

@@ -23,7 +23,7 @@ unsafe extern "C" fn faccessat(
     mode: c_int,
     flags: c_int,
 ) -> c_int {
-    // SAFETY: dirfd and pathname are valid arguments provided by the caller of the interposed function
+    // SAFETY: dirfd is -1 or a valid file descriptor, and pathname is a valid C string, provided by the caller of the interposed function
     unsafe {
         handle_open(PathAt::borrow_raw(dirfd, pathname), AccessMode::Read);
     }

crates/fspy_preload_unix/src/interceptions/dirent.rs

@@ -51,8 +51,8 @@ mod macos_only {
         buf_len: usize,
         basep: *mut i64,
     ) -> isize {
-        // SAFETY: fd is a valid file descriptor provided by the caller of __getdirentries64
-        unsafe { handle_open(BorrowedFd::borrow_raw(fd), AccessMode::ReadDir) };
+        // SAFETY: fd is -1 or a valid file descriptor provided by the caller of __getdirentries64
+        unsafe { handle_open(BorrowedFd::try_borrow_raw(fd), AccessMode::ReadDir) };
         // SAFETY: calling the original libc __getdirentries64() with the same arguments forwarded from the interposed function
         unsafe { __getdirentries64::original()(fd, buf, buf_len, basep) }
     }
@@ -65,16 +65,16 @@ unsafe extern "C" fn getdirentries(
     nbytes: c_int,
     basep: *mut c_long,
 ) -> c_int {
-    // SAFETY: fd is a valid file descriptor provided by the caller of the interposed function
-    unsafe { handle_open(BorrowedFd::borrow_raw(fd), AccessMode::ReadDir) };
+    // SAFETY: fd is -1 or a valid file descriptor provided by the caller of the interposed function
+    unsafe { handle_open(BorrowedFd::try_borrow_raw(fd), AccessMode::ReadDir) };
     // SAFETY: calling the original libc getdirentries() with the same arguments forwarded from the interposed function
     unsafe { getdirentries::original()(fd, buf, nbytes, basep) }
 }
 
 intercept!(fdopendir(64): unsafe extern "C" fn (fd: c_int) -> *mut DIR);
 unsafe extern "C" fn fdopendir(fd: c_int) -> *mut DIR {
-    // SAFETY: fd is a valid file descriptor provided by the caller of the interposed function
-    unsafe { handle_open(BorrowedFd::borrow_raw(fd), AccessMode::ReadDir) };
+    // SAFETY: fd is -1 or a valid file descriptor provided by the caller of the interposed function
+    unsafe { handle_open(BorrowedFd::try_borrow_raw(fd), AccessMode::ReadDir) };
     // SAFETY: calling the original libc fdopendir() with the same arguments forwarded from the interposed function
     unsafe { fdopendir::original()(fd) }
 }

crates/fspy_preload_unix/src/interceptions/linux_syscall.rs

@@ -38,11 +38,11 @@ unsafe extern "C" fn syscall(syscall_no: c_long, mut args: ...) -> c_long {
         let flags = a2 as c_int;
         if pathname.is_null() {
             if flags & libc::AT_EMPTY_PATH != 0 {
-                // SAFETY: dirfd is provided by the statx syscall caller.
-                unsafe { handle_open(BorrowedFd::borrow_raw(dirfd), AccessMode::Read) };
+                // SAFETY: dirfd is -1 or a valid file descriptor provided by the statx syscall caller.
+                unsafe { handle_open(BorrowedFd::try_borrow_raw(dirfd), AccessMode::Read) };
             }
         } else {
-            // SAFETY: pathname is a non-null C string pointer provided by the statx syscall caller.
+            // SAFETY: dirfd is -1 or a valid file descriptor, and pathname is a non-null C string pointer, provided by the statx syscall caller.
             unsafe {
                 handle_open(PathAt::borrow_raw(dirfd, pathname), AccessMode::Read);
             };