fix(fspy): don't treat opening a file as a folder as reading it

CHANGELOG.md

@@ -10,7 +10,7 @@
 - **Fixed** `vp run` now exits with code 1 when a task's cache can't be checked, for example because the cache database can't be read. It already stopped there, but exited 0 ([#960](https://github.com/voidzero-dev/vite-task/pull/960)).
 - **Changed** The helper files `vp run` writes to the system temp directory, such as the library that tracks file accesses and the JSON Schema that `vp run --json` points to, now share one folder that only the current user can write to, like `/tmp/vp_run-1000` on Linux, instead of a folder for each kind of file ([#974](https://github.com/voidzero-dev/vite-task/pull/974)).
 - **Perf** When several tasks become ready at once, `vp run` now checks their cache entries at the same time instead of one after another, so cache hits start sooner. Hashing a task's input files for its cache lookup no longer holds up other tasks ([#982](https://github.com/voidzero-dev/vite-task/pull/982)).
-- **Fixed** On Windows, the files a task deletes or moves, such as a build cleaning its output folder before writing it again, are no longer among its inputs, as on Linux and macOS. Before, when a later step changed one of those files, for example by stamping a version into the rebuilt output, the next run missed the cache with a reason like `'dist/index.js' modified`, though the task never read the file ([#980](https://github.com/voidzero-dev/vite-task/pull/980)).
+- **Fixed** On Windows and macOS, the files a task deletes or moves, such as a build cleaning its output folder before writing it again, are no longer among its inputs, as on Linux. On macOS, this affected the files Node's `fs.rmSync` deletes from Node 23 on, since it first tries to open each one as a folder. Before, when a later step changed one of those files, for example by stamping a version into the rebuilt output, the next run missed the cache with a reason like `'dist/index.js' modified`, though the task never read the file ([#980](https://github.com/voidzero-dev/vite-task/pull/980), [#991](https://github.com/voidzero-dev/vite-task/pull/991)).
 - **Changed** In `@voidzero-dev/vite-task-client`, a `getEnvs` query that matches every env name, `{ prefix: '' }` or `*`, now returns nothing instead of every environment variable, and no longer adds them to the cache key ([#976](https://github.com/voidzero-dev/vite-task/pull/976)).
 - **Added** `vp run --json` prints every package and task in the workspace, with each task's `dependsOn` resolved to task IDs, as one JSON document for scripts, editors and coding agents. It runs nothing and, like `vp run` without a task, loads every package's config. It prints the same document from any directory in the workspace: indented in a terminal, on one line when captured. Its `$schema` field points to a JSON Schema that describes each field, which `vp run` writes to the system temp directory ([#948](https://github.com/voidzero-dev/vite-task/pull/948)).
 - **Fixed** On Linux, where every user shares the temp folder `/tmp`, the helper files `vp run` writes there, such as the library that tracks file accesses, now go in a folder only the current user can write to, like `/tmp/fspy-1000`. Before, they went in a folder such as `/tmp/fspy` that belonged to whoever ran `vp run` first: running a different version as another user failed, and the same version used files that user could replace ([#947](https://github.com/voidzero-dev/vite-task/pull/947)).

crates/fspy/src/unix/syscall_handler/mod.rs

@@ -18,6 +18,7 @@ use fspy_seccomp_unotify::{
     supervisor::handler::arg::{CStrPtr, Caller, Fd},
 };
 use fspy_shared::ipc::{AccessMode, PathAccess, channel::Sender};
+use fspy_shared_unix::open_flags::open_access_mode;
 
 const PATH_MAX: usize = libc::PATH_MAX as usize;
 
@@ -43,10 +44,8 @@ impl SyscallHandler {
         path_ptr: CStrPtr,
         flags: c_int,
     ) -> io::Result<()> {
-        let mode = match flags & libc::O_ACCMODE {
-            libc::O_RDWR => AccessMode::ReadWrite,
-            libc::O_WRONLY => AccessMode::Write,
-            _ => AccessMode::Read,
+        let Some(mode) = open_access_mode(flags) else {
+            return Ok(());
         };
         self.handle_path(caller, dir_fd, path_ptr, mode)
     }

crates/fspy/tests/rust_std.rs

@@ -35,6 +35,35 @@ async fn open_write() -> anyhow::Result<()> {
     Ok(())
 }
 
+/// An `O_DIRECTORY` open reads nothing, so it isn't recorded, whether it
+/// fails on a file, as when libc++'s `std::filesystem::remove_all` opens each
+/// path it removes, or opens a folder. Reading the folder's entries is
+/// recorded on its own (see `readdir`).
+#[cfg(unix)]
+#[test(tokio::test)]
+async fn open_directory() -> anyhow::Result<()> {
+    let tmp_dir = tempfile::tempdir()?;
+    let tmp_path = tmp_dir.path().canonicalize()?;
+    File::create(tmp_path.join("file"))?;
+    fs::create_dir(tmp_path.join("folder"))?;
+    let tmp_path_str = tmp_path.to_str().unwrap().to_owned();
+    let accesses = track_fn!(tmp_path_str, |tmp_path_str: String| {
+        use std::os::unix::fs::OpenOptionsExt as _;
+        let tmp_path = std::path::Path::new(&tmp_path_str);
+        for name in ["file", "folder"] {
+            let _ = OpenOptions::new()
+                .read(true)
+                .custom_flags(libc::O_DIRECTORY)
+                .open(tmp_path.join(name));
+        }
+    })
+    .await?;
+    test_utils::assert_not_contains(&accesses, &tmp_path.join("file"));
+    test_utils::assert_not_contains(&accesses, &tmp_path.join("folder"));
+
+    Ok(())
+}
+
 #[test(tokio::test)]
 async fn rename_destination() -> anyhow::Result<()> {
     let tmp_dir = tempfile::tempdir()?;
@@ -249,9 +278,9 @@ async fn readdir() -> anyhow::Result<()> {
     //   and if that fails, not read dir call (NtQueryDirectoryFile/NtQueryDirectoryFileEx) is made.
     // - macOS/Linux:
     //   - opendir results in a read_dir access. This call is directly made without trying to open the directory as a fd first.
-    //   - open + fopendir results in READ access, because open would fail with ENOENT, and fopendir is not called.
+    //   - open + fdopendir results in no access, because the `O_DIRECTORY` open isn't recorded, and it fails with ENOENT, so fdopendir is not called.
     //
-    // This difference is acceptable because both will result in a "not found" fingerprint in vite-task.
+    // READ and read_dir both result in a "not found" fingerprint in vite-task.
     // To keep the test consistent across platforms, we create the directory first.
     std::fs::create_dir(tmpdir.path().join("hello_dir"))?;
 

crates/fspy/tests/static_executable.rs

@@ -8,7 +8,7 @@ use std::path::Path;
 use fspy::PathAccessIterable;
 use test_log::test;
 
-use crate::test_utils::{assert_contains, test_bin_path};
+use crate::test_utils::{assert_contains, assert_not_contains, test_bin_path};
 
 mod test_utils;
 
@@ -44,6 +44,14 @@ async fn open_readwrite() {
     assert_contains(&accesses, Path::new("/hello"), fspy::PathAccess::ReadWrite);
 }
 
+#[test(tokio::test)]
+async fn open_directory() {
+    // The open fails on a file without reading it.
+    let file = tempfile::NamedTempFile::new().unwrap();
+    let accesses = track_test_bin(&["open_directory", file.path().to_str().unwrap()], None).await;
+    assert_not_contains(&accesses, file.path());
+}
+
 #[test(tokio::test)]
 async fn openat2_read() {
     let accesses = track_test_bin(&["openat2_read", "/hello"], None).await;

crates/fspy_client_unix/src/convert.rs

@@ -4,6 +4,7 @@ use allocator_api2::{alloc::Allocator, vec::Vec};
 use bstr::ByteSlice;
 use fspy_nostd::{BorrowedFd, CWD};
 use fspy_shared::ipc::AccessMode;
+use fspy_shared_unix::open_flags::open_access_mode;
 use libc::{c_char, c_int};
 
 #[cfg(target_os = "linux")]
@@ -196,44 +197,41 @@ impl ToAbsolutePath for fspy_nostd::CStr<'_, fspy_nostd::Thin> {
 }
 
 pub trait ToAccessMode {
-    /// Converts the intercepted operation's mode into an access mode.
+    /// Converts the intercepted operation's mode into the access to record,
+    /// or `None` for one that isn't recorded.
     ///
     /// # Safety
     ///
     /// Implementations backed by raw process pointers require those pointers
     /// to remain valid for the conversion.
-    unsafe fn to_access_mode(self) -> AccessMode;
+    unsafe fn to_access_mode(self) -> Option<AccessMode>;
 }
 
 impl ToAccessMode for AccessMode {
-    unsafe fn to_access_mode(self) -> AccessMode {
-        self
+    unsafe fn to_access_mode(self) -> Option<AccessMode> {
+        Some(self)
     }
 }
 
 pub struct OpenFlags(pub c_int);
 impl ToAccessMode for OpenFlags {
-    unsafe fn to_access_mode(self) -> AccessMode {
-        match self.0 & libc::O_ACCMODE {
-            libc::O_RDWR => AccessMode::ReadWrite,
-            libc::O_WRONLY => AccessMode::Write,
-            _ => AccessMode::Read,
-        }
+    unsafe fn to_access_mode(self) -> Option<AccessMode> {
+        open_access_mode(self.0)
     }
 }
 
 pub struct ModeStr(pub *const c_char);
 impl ToAccessMode for ModeStr {
-    unsafe fn to_access_mode(self) -> AccessMode {
+    unsafe fn to_access_mode(self) -> Option<AccessMode> {
         // SAFETY: self.0 is a non-null pointer to a valid null-terminated C
         // string, as guaranteed by the libc calling convention.
         let mode_str = unsafe { CStr::from_ptr(self.0) }.to_bytes().as_bstr();
         let has_read = mode_str.contains(&b'r');
         let has_write = mode_str.contains(&b'w') || mode_str.contains(&b'a');
-        match (has_read, has_write) {
+        Some(match (has_read, has_write) {
             (false, true) => AccessMode::Write,
             (true, true) => AccessMode::ReadWrite,
             _ => AccessMode::Read,
-        }
+        })
     }
 }

crates/fspy_client_unix/src/lib.rs

@@ -143,7 +143,9 @@ impl<'a> Client<'a> {
     ) -> anyhow::Result<()> {
         // SAFETY: mode contains a valid pointer (if ModeStr) or a plain value,
         // as provided by the caller.
-        let mode = unsafe { mode.to_access_mode() };
+        let Some(mode) = (unsafe { mode.to_access_mode() }) else {
+            return Ok(());
+        };
         let Some(abs_path) = path.to_absolute_path(&allocator)? else {
             return Ok(());
         };

crates/fspy_shared_unix/src/lib.rs

@@ -2,6 +2,7 @@
 
 pub mod exec;
 pub(crate) mod open_exec;
+pub mod open_flags;
 pub mod payload;
 pub mod spawn;
 

crates/fspy_shared_unix/src/open_flags.rs

@@ -0,0 +1,25 @@
+use fspy_shared::ipc::AccessMode;
+use nix::libc::{O_ACCMODE, O_DIRECTORY, O_RDWR, O_WRONLY, c_int};
+
+/// The access an open with `flags` is recorded as, or `None` if it isn't
+/// recorded.
+///
+/// An `O_DIRECTORY` open reads nothing: it opens only a folder, and reading
+/// the folder's entries is recorded on its own. libc++'s
+/// `std::filesystem::remove_all`, which Node's `fs.rmSync` uses on macOS,
+/// opens every path it removes that way and removes the path as a file when
+/// the open fails, so recording the open would make every file it removes an
+/// input. On Linux, `O_TMPFILE` includes `O_DIRECTORY`; such an open makes a
+/// file without a name, and giving it one with `linkat` is recorded as a
+/// write.
+#[must_use]
+pub const fn open_access_mode(flags: c_int) -> Option<AccessMode> {
+    if flags & O_DIRECTORY != 0 {
+        return None;
+    }
+    Some(match flags & O_ACCMODE {
+        O_RDWR => AccessMode::ReadWrite,
+        O_WRONLY => AccessMode::Write,
+        _ => AccessMode::Read,
+    })
+}

crates/fspy_test_bin/src/main.rs

@@ -29,6 +29,10 @@ fn main() {
         "open_readwrite" => {
             let _ = File::options().read(true).write(true).open(path);
         }
+        "open_directory" => {
+            use std::os::unix::fs::OpenOptionsExt as _;
+            let _ = File::options().read(true).custom_flags(nix::libc::O_DIRECTORY).open(path);
+        }
         "openat2_read" => {
             let _ = openat2(AT_FDCWD, path, OpenHow::new().flags(OFlag::O_RDONLY));
         }

crates/vt_bin/src/vtt/fs.rs

@@ -23,7 +23,9 @@ const USAGE: &str = "Usage: vtt fs <op> <arg>... [<op> <arg>...]...";
 ///   missing folder isn't an error.
 /// - `rm <path>`: remove the file, or the folder and everything in it, like
 ///   `rm -r`. A missing path isn't an error, like a tool cleaning its output
-///   folder.
+///   folder. On Linux and macOS, it opens each path as a folder first and
+///   removes the path as a file when it isn't one, like Node's `fs.rmSync`
+///   on macOS, which uses libc++'s `std::filesystem::remove_all`.
 /// - `mkdir <dir>`: create the folder and any missing parents, like
 ///   `mkdir -p`.
 /// - `rename <from> <to>`: move the file or folder `<from>` to `<to>`, like
@@ -46,7 +48,7 @@ pub fn run(args: &[String]) -> Result<(), Box<dyn std::error::Error>> {
             ["stat", path, rest @ ..] => (allow_missing(fs::metadata(path)), rest),
             ["list", dir, rest @ ..] => (allow_missing(list(dir)), rest),
             ["walk", dir, rest @ ..] => (allow_missing(walk(Path::new(dir))), rest),
-            ["rm", path, rest @ ..] => (allow_missing(remove(path)), rest),
+            ["rm", path, rest @ ..] => (allow_missing(remove(Path::new(path))), rest),
             ["mkdir", dir, rest @ ..] => (fs::create_dir_all(dir), rest),
             ["rename", from, to, rest @ ..] => (fs::rename(from, to), rest),
             _ => return Err(format!("{USAGE}\nunexpected arguments: {}", ops.join(" ")).into()),
@@ -72,7 +74,30 @@ fn walk(dir: &Path) -> io::Result<()> {
     Ok(())
 }
 
-fn remove(path: &str) -> io::Result<()> {
+#[cfg(unix)]
+fn remove(path: &Path) -> io::Result<()> {
+    use std::os::unix::fs::OpenOptionsExt as _;
+
+    match fs::OpenOptions::new()
+        .read(true)
+        .custom_flags(libc::O_DIRECTORY | libc::O_NOFOLLOW)
+        .open(path)
+    {
+        Ok(_) => {
+            for entry in fs::read_dir(path)? {
+                remove(&entry?.path())?;
+            }
+            fs::remove_dir(path)
+        }
+        Err(err) if matches!(err.raw_os_error(), Some(libc::ENOTDIR | libc::ELOOP)) => {
+            fs::remove_file(path)
+        }
+        Err(err) => Err(err),
+    }
+}
+
+#[cfg(windows)]
+fn remove(path: &Path) -> io::Result<()> {
     if fs::symlink_metadata(path)?.is_dir() {
         fs::remove_dir_all(path)
     } else {