fs: stop reading the shared stat buffer in realpathSync - #65113
Open
unstubbable wants to merge 2 commits into
Open
fs: stop reading the shared stat buffer in realpathSync#65113unstubbable wants to merge 2 commits into
realpathSync#65113unstubbable wants to merge 2 commits into
Conversation
While walking a path, `realpathSync` skips the components it already knows are real, and in that branch it reads the shared stat buffer to decide whether the walk has reached a pipe or a socket. That buffer holds the result of the last stat made anywhere in the process rather than the last one made by the walk, so an unrelated stat of a FIFO ends the walk early and the path comes back with its symlinks unresolved. The unresolved path is then written to the cache, so every later resolution repeats it. The walk only takes that branch once the ancestors are established as real, which is the state the module loader's cache is in. The test goes through `require()` to reach it, where the stale read costs a second copy of a module reached through a symlink. Signed-off-by: Hendrik Liebau <mail@hendrik-liebau.de>
`realpathSync` decided whether a walk had reached a pipe or a socket by reading `statValues`, which holds the result of the last stat made anywhere in the process rather than the last one made by the walk itself. Any unrelated stat of a FIFO or a socket therefore ended the walk early, returning the path with its symlinks unresolved and caching it in that form. It now tracks whether the symlink it resolved last pointed at a pipe or a socket, which is the value the check was always meant to read. The async `realpath()` carries the same check and the same latent problem; that is left for a separate change, since no test here reaches it. Signed-off-by: Hendrik Liebau <mail@hendrik-liebau.de>
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
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.
fs.realpathSync()can return a path with its symlinks unresolved, depending on what the process happened to stat beforehand.While walking a path, the components already known to be real are skipped:
statValuesis the shared stat buffer, holding the result of the last stat made anywhere in the process. In the intended flow that is thestat()the walk itself made when following a symlink, which is the signal the check wants (added in #13028 sorealpath('/dev/stdin')resolves). But nothing keeps it that way: any unrelated stat overwrites it. If that stat was of a FIFO or a socket, the walk breaks early and the remaining components, including any symlink, are never resolved. The unresolved path is then written to the cache, so every later resolution of it repeats the same answer.It reproduces through public API alone, because the module loader keeps a realpath cache in exactly the state that takes the skipped-component branch:
This is not theoretical. A process that stats a socket or a pipe can make
require()resolve a package through itsnode_modules/<pkg>symlink rather than its realpath, and load a second copy of every module underneath it.I ran into it in the Next.js dev server, which I work on: on a pnpm install it stats a pipe while talking to its worker threads, and from that point
next/dist/...resolves through thenode_modules/nextsymlink, so the process ends up with two copies of theAsyncLocalStorageinstances that hold per-request state, and a request handler reads a store that nothing ever entered.The fix has the walk track whether the symlink it resolved last pointed at a pipe or a socket, taking it from the
stat()that follows the link, which is the value the check was always reading out of the shared buffer.The first commit adds the test and is expected to fail on its own, so you can check it out and see the bug before the second commit fixes it. Please squash when landing.
Two things to flag:
/dev/stdin, and on Linux that resolves throughpipe:[N], a link target that is not a real path. That path needs CI, or someone on Linux.realpath()carries the same check and the same latent problem. I left it alone: that walk fills the buffer with its ownfs.stat()before reaching the branch, so I could not construct a failing case, and module resolution never reaches it. Happy to fix both together if you would rather.Reproduces on v20.19.6, v22.13.1, v24.16.0 and v25.2.1.