fix: canonicalize filepath in Instance.containsPath() for symlink support#16665
Open
jmylchreest wants to merge 2 commits intoanomalyco:devfrom
Open
fix: canonicalize filepath in Instance.containsPath() for symlink support#16665jmylchreest wants to merge 2 commits intoanomalyco:devfrom
jmylchreest wants to merge 2 commits intoanomalyco:devfrom
Conversation
github-actions
bot
added
the
needs:compliance
Instance.containsPath() compared Instance.directory (always canonical after anomalyco#16651) against an uncanonicalized filepath argument. When callers passed a symlinked path, the lexical comparison failed even though the path resolved to a location inside the project. This caused false negatives in bash.ts, external-directory.ts, and file/index.ts — triggering unnecessary external_directory permission prompts or rejecting valid file reads via symlinked paths. Fix: resolve the filepath through Filesystem.resolve() before comparing, so both sides use canonical paths. Adds tests for: symlinks inside project, external symlinks to project, symlinks escaping project, dangling symlinks, and symlink cycles. Fixes anomalyco#16660
jmylchreest
force-pushed
the
fix/containsPath-symlink-canonicalization
branch
from
728e91a to
540f3d3
Compare
github-actions
bot
removed
the
needs:compliance
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
6 tasks
Author
|
The failing tests should resolve once #16651 is also applied |
…omalyco#16651 Tests for external symlinks, symlinks escaping project, and ELOOP propagation probe at runtime whether Filesystem.resolve() follows symlinks. They skip gracefully on dev (before anomalyco#16651) and activate once realpathSync lands.
This was referenced Mar 9, 2026
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
1 participant
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.
Issue for this PR
Closes #16660
Type of change
What does this PR do?
Instance.containsPath()does a lexical prefix check comparingInstance.directoryagainst afilepathargument. After #16651,Instance.directoryis always canonical (symlink-resolved), but thefilepathpassed by callers (bash.ts,external-directory.ts,file/index.ts) is not canonicalized.This mismatch causes symlinked paths to fail the containment check — triggering unnecessary
external_directorypermission prompts or rejecting valid file reads/trees via symlinked paths. The issue pre-existed #16651 but became more pronounced sinceInstance.directoryis now always canonical while inputs remained raw.Fix: call
Filesystem.resolve(filepath)insidecontainsPath()before comparing, so both sides use canonical paths.Depends on #16651 for
Filesystem.resolve()symlink resolution.How did you verify your code works?
bun test test/file/path-traversal.test.ts— 20 pass (15 existing + 5 new)bun test test/util/filesystem.test.ts— 55 passScreenshots / recordings
N/A — no UI changes.
Checklist