vfs: fs hook gaps - #65852
Open
pipobscure wants to merge 3 commits into
Open
Conversation
Several `node:fs` entry points behave differently for a mounted path
than for a real one, because of how the call reaches the VFS hooks:
* `fs.watchFile` and `fs.promises.watch` call handler methods that do
not exist, so they throw a TypeError instead of watching.
* `fs.watch` on a path that does not exist returns a polling watcher
instead of throwing ENOENT, and that watcher keeps the process alive.
* `fs.utimesSync` and `fs.readdirSync` consult the hook before
validating
their arguments: numeric-string timestamps are ignored, an object
timestamp becomes NaN, and an invalid encoding is accepted.
* `fs.futimesSync` and `fs.fchmodSync` on a virtual descriptor are
no-ops while the path forms of the same operations work.
* `fs.mkdtempSync` with a prefix ending in a separator creates the
directory next to the intended parent, because the prefix is resolved
as a path before the suffix is appended.
* `fs.mkdirSync({ recursive: true })` returns the provider-relative path
of the first directory created instead of the mounted path.
* Disposing an already closed virtual `Dir` asynchronously rejects with
ERR_DIR_CLOSED; the real `Dir` treats disposal as idempotent.
This adds a test per gap, stating the real-fs outcome as the
expectation.
Proposed solution: add `watchFile`, `unwatchFile` and `promisesWatch`
handlers backed by the provider's stat watcher and async watcher, and
have `watch` stat the path first; move the hook calls in `utimesSync`
and `readdirSync` after argument validation, and coerce times with
`toUnixTimestamp` in the hook; route `futimes`/`fchmod` to the handle's
entry; strip the trailing separator only after computing the temp
name in `mkdtemp`; map the recursive `mkdir` result back under the mount
point; and make `VirtualDir`'s async dispose a no-op once closed.
Signed-off-by: Philipp Dunkel <pip@pipobscure.com>
Make the `node:fs` entry points that reach a mounted path behave as they do for a real one: * Add the `watchFile`, `unwatchFile` and `promisesWatch` handlers, backed by the provider's stat watcher and async watcher, and have `watch` refuse a path that does not exist with ENOENT instead of handing back a watcher that polls forever. * Convert timestamps and validate arguments before the hook runs in `utimes`, `lutimes` and `readdir` (sync, callback and promise forms), so a mounted path gets the same ERR_INVALID_ARG_* errors and the same seconds-since-epoch numbers as a real one. * Pass the mode and times through to the `fchmod` and `futimes` hooks and route them to the handle's entry, so descriptor operations take effect like their path forms; the memory handle validates the way a FileHandle would since one calls it directly. * Treat a `mkdtemp` prefix as text rather than a path when it ends in a separator, so the directory is created inside the intended parent. * Map the first directory a recursive `mkdir` created back under the mount point. * Make disposing an already closed virtual `Dir` a no-op, as on the native `Dir`. The existing file handle test asserted that `chmod()` and `utimes()` without arguments were no-ops; they now validate and apply, so it exercises that instead. Signed-off-by: Philipp Dunkel <pip@pipobscure.com>
nodejs-github-bot
added
lib / src
jasnell
approved these changes
Sep 6, 2026
pipobscure
marked this pull request as ready for review
September 6, 2026 14:23
github-actions
Bot
removed
the
request-ci
Collaborator
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #65852 +/- ##
==========================================
- Coverage 90.19% 90.18% -0.01%
==========================================
Files 771 771
Lines 264622 264717 +95
Branches 50223 50272 +49
==========================================
+ Hits 238663 238734 +71
- Misses 16965 16978 +13
- Partials 8994 9005 +11
🚀 New features to boost your workflow:
|
mcollina
reviewed
Sep 6, 2026
| async function utimes(path, atime, mtime) { | ||
| path = getValidatedPath(path); | ||
| // Converted before the VFS hook so a mounted path gets the same | ||
| // validation and the same seconds-since-epoch numbers as a real one. |
The previous commit added comments that narrate what the adjacent code does. Its commit message already carries the reasoning, so remove them. Signed-off-by: Philipp Dunkel <pip@pipobscure.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
vfs: close gaps in the fs hooks for mounted paths
Several
node:fsentry points behave differently for a mounted paththan for a real one, because of how the call reaches the VFS hooks:
fs.watchFileandfs.promises.watchcall handler methods that donot exist, so they throw a TypeError instead of watching.
fs.watchon a path that does not exist returns a polling watcherinstead of throwing ENOENT, and that watcher keeps the process alive.
fs.utimesSyncandfs.readdirSyncconsult the hook beforevalidating
their arguments: numeric-string timestamps are ignored, an object
timestamp becomes NaN, and an invalid encoding is accepted.
fs.futimesSyncandfs.fchmodSyncon a virtual descriptor areno-ops while the path forms of the same operations work.
fs.mkdtempSyncwith a prefix ending in a separator creates thedirectory next to the intended parent, because the prefix is resolved
as a path before the suffix is appended.
fs.mkdirSync({ recursive: true })returns the provider-relative pathof the first directory created instead of the mounted path.
Dirasynchronously rejects withERR_DIR_CLOSED; the real
Dirtreats disposal as idempotent.Note: Since these are gaps/defects in existing functionality, I decided to create the failing tests first (first commit) and then add the fix/solution as a second commit. That way whoever wants to review this can first prove out the issue, before applying the solution.
This goes with the VFS work by @mcollina and the bug-fix PRs by @trivikr.