You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
filesystem: FS_SEARCH_EXCLUDE_PREFIXES is lexical; should excludes be canonical/symlink-aware? #4208
Context. The proposed fix for #4162 (PR #4212) adds FS_SEARCH_EXCLUDE_PREFIXES as a fast lexical prefilter that refuses recursive search_files / directory_tree on configured slow roots, typically macOS CloudStorage / FileProvider paths.
Discussion on #4162 surfaced that the check is purely lexical: a symlink such as /allowed/sneaky whose target lives under an excluded prefix is not caught by the prefix check, and may still reach the slow provider path during canonicalisation / validation.
The case for keeping the #4162 fix scoped to the lexical prefilter, as discussed in that thread:
The bypass is pre-existing behaviour — the original searchFilesWithValidation exclude check is also lexical.
validatePath continues to enforce the allowed_directories escape boundary via its own realpath, so the security boundary is unaffected. This question is about whether excludes should be symlink-aware.
The impact of the bypass is performance / hang risk from reaching a slow provider path, not an authorization bypass.
Design question. Should excludes be hardened against symlink bypass?
One possible shape is two-stage:
Fast lexical check first, preserving current cheap behaviour.
If not excluded lexically, optionally perform bounded canonicalisation, e.g. withFsTimeout(fs.realpath(...)), then re-check the canonical path against excluded prefixes, with an explicit fail-policy on timeout — deny, allow, or return a typed error.
This affects both search_files and directory_tree, so it seems like a design decision rather than a drop-in fix. Not gating #4162; filing this separately so the symlink-aware exclude question is not lost.
Read through main for how far the symlink case actually goes (reasoned from the code, not run: I have no Node toolchain here).
In-tree symlinks are mostly inert. Both walkers recurse only on entry.isDirectory() from readdir({ withFileTypes: true }), and a Dirent for a symlink reports isDirectory() === false (it describes the link, not the target). So /allowed/sneaky -> /Users/x/Library/CloudStorage/... is never entered by searchFilesWithValidation (lib.ts:676) or buildTree (server.ts:652); directory_tree lists it as a "file" entry.
The one touch is in search_files: validatePath(fullPath, ...) runs on every entry before the exclude check (lib.ts:658), so each symlink costs one realpath on its target. That is one call per link, not a traversal, but on a stalled FileProvider mount it can still block, which is why a withFsTimeout around it matters regardless of excludes.
The case the lexical prefilter does miss is the root argument itself: search_files / directory_tree with path: "/allowed/sneaky". validatePath resolves it to the excluded target and the recursion then starts there, since rootPath is not a Dirent. That is the one place I would add the bounded canonical re-check.
On the fail policy for the timeout: deny with a typed error seems right for this feature, since the point of the prefix list is to avoid the slow mount and an "allow on timeout" would just reproduce the hang with a delay.
Context. The proposed fix for #4162 (PR #4212) adds
FS_SEARCH_EXCLUDE_PREFIXESas a fast lexical prefilter that refuses recursivesearch_files/directory_treeon configured slow roots, typically macOS CloudStorage / FileProvider paths.Discussion on #4162 surfaced that the check is purely lexical: a symlink such as
/allowed/sneakywhose target lives under an excluded prefix is not caught by the prefix check, and may still reach the slow provider path during canonicalisation / validation.The case for keeping the #4162 fix scoped to the lexical prefilter, as discussed in that thread:
searchFilesWithValidationexclude check is also lexical.validatePathcontinues to enforce theallowed_directoriesescape boundary via its ownrealpath, so the security boundary is unaffected. This question is about whether excludes should be symlink-aware.realpath-before-compare exclude check risks re-introducing the kind of hang filesystem: recursive search can hang on macOS CloudStorage / lazy provider paths #4162 is trying to avoid.Design question. Should excludes be hardened against symlink bypass?
One possible shape is two-stage:
withFsTimeout(fs.realpath(...)), then re-check the canonical path against excluded prefixes, with an explicit fail-policy on timeout — deny, allow, or return a typed error.This affects both
search_filesanddirectory_tree, so it seems like a design decision rather than a drop-in fix. Not gating #4162; filing this separately so the symlink-aware exclude question is not lost.