Repository navigation
Conversation
🦋 Changeset detectedLatest commit: 9b7b641 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Rebuilt on current
The part worth your judgement is a test. I also added a guard that a symlink cycle still reports
Verified again that the test is red without the fix: on |
|
Retargeted onto On |
validatePath on a symlink with nothing at the other end reported "Parent directory does not exist: <dir>" for a directory that is plainly there. resolveUnicodeEquivalentPath lets the ENOENT from fs.realpath escape bare, and validatePath's catch reads any ENOENT as a missing parent. Naming the symlink needs the catch inside the equivalent-path walk, where the failing entry is known. Only ENOENT is renamed; the realpath error is attached as the cause, the way the Parent directory error below it already does. The characterization test for this case asserted the old message. Its name and assertion now follow the fix; the security assertion in it, that nothing is created outside the allowed directory, is unchanged. Added a guard that a symlink cycle still reports ELOOP, so a later, broader catch cannot rename every realpath failure into a broken symlink. Signed-off-by: Dmitry Voropaev <dy.voropaev@gmail.com>
f35a018 to
9b7b641
Compare
Description
A symlink pointing at nothing makes the filesystem server report a missing parent directory for a directory that is plainly there.
Server Details
src/filesystem/lib.ts), one new test fileMotivation and Context
validatePathcatches ENOENT out ofresolveUnicodeEquivalentPathand turns it intoThat message never fits the case it names. When a path component is genuinely missing, the resolver does not throw: it returns the joined tail, with a comment saying so, precisely so
create_directorycanmkdir -pit. What does raise ENOENT in there isfs.realpathon an entry that appears in the directory listing but points nowhere.So with an allowed directory
Rholding a danglinglink.txt:Rexists,fs.stat(R)succeeds, and the caller is sent looking at the wrong thing. A stalenode_modules/.binentry gives the same shape.The fix classifies the error where it happens, at the
realpathcall, rather than guessing from an errno two frames up:I left the existing fallback in place. The other ENOENT sources in that function are a vanished allowed directory and an intermediate directory removed mid-walk, where "parent directory does not exist" reads closer to true.
I first tried putting an
fs.stat(parent)check in the catch to tell the two apart. That is worse:__tests__/lib.test.tsauto-mocksfs/promises, sofs.statresolves to undefined and the check passes for both cases. Classifying at the source needs no such guess.How Has This Been Tested?
New
src/filesystem/__tests__/broken-symlink.test.ts, two cases against a real temp directory: a dangling symlink now reportsBroken symlinkwhile its parent still stats fine, and a genuinely missing path still resolves to the joined tail socreate_directorykeeps working. The first fails on current main with the message quoted above.The three failures are in
__tests__/unicode-paths.test.tsand are the same three on unmodifiedmainin this checkout; they look like the composed/decomposed distinction not surviving on APFS. Not touched by this change.Not tested through an LLM client, this is an error-message path.
Breaking Changes
None. No client configuration changes, and the only difference a caller sees is the wording of an error that was already being raised.
Types of changes
Checklist