Skip to content

filesystem: FS_SEARCH_EXCLUDE_PREFIXES is lexical; should excludes be canonical/symlink-aware? #4208

Description

@JonoGitty

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.
  • An unbounded 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:

  1. Fast lexical check first, preserving current cheap behaviour.
  2. 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.

Activity

  1. norika1207-lab commented on Jun 2, 2026

    @norika1207-lab
  2. arhancanli commented on Oct 11, 2026

    @arhancanli

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions