fix(filesystem): apply search excludes before realpath#4181
Open
Sean-Kenneth-Doherty wants to merge 1 commit into
Open
Conversation
Author
|
Local validation on |
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as abuse.
12 tasks
JonoGitty
added a commit
to JonoGitty/servers
that referenced
this pull request
May 20, 2026
…r paths (modelcontextprotocol#4162) fs.realpath (in validatePath) and recursive fs.readdir (in search_files / directory_tree) are called with no timeout, and recursive search has no upper bound on entries visited. On macOS provider-backed paths (~/Library/CloudStorage/..., ~/Library/Mobile Documents/com~apple~CloudDocs/..., NFS/SMB) these can block for minutes while the provider fetches metadata; the host eventually reports the connector unresponsive while the subprocess stays stuck. Follow-up to modelcontextprotocol#4181, which only skipped excluded entries before realpath. lib.ts: - withFsTimeout() wraps fs.realpath and fs.readdir; on timeout rejects with a typed FsTimeoutError instead of hanging. A no-op catch on the timeout promise avoids spurious unhandled-rejection reports when the operation wins the race. - FS_SEARCH_MAX_VISITED caps recursive search; abort produces a clear "narrow the path / refine the pattern" error. - FS_SEARCH_EXCLUDE_PREFIXES (opt-in, empty default) refuses recursive operations on configured slow roots, via the shared boundary-aware assertSearchPathNotExcluded helper (reuses isPathWithinAllowedDirectories, expandHome applied symmetrically to the candidate root). - ENOENT branch in validatePath refactored so a hung parent realpath surfaces as FsTimeoutError instead of being masked as "Parent directory does not exist". - Per-entry catch in search re-throws FsTimeoutError so a search never returns silently-partial results. index.ts: - directory_tree gets the same timeout + visited-cap treatment and shares the exclude-prefix helper; throws immediately on cap-exceed rather than threading an aborted flag through unwinding recursion. Config (all env-var, conservative defaults; invalid values warn + fall back): FS_OP_TIMEOUT_MS=15000, FS_SEARCH_MAX_VISITED=50000, FS_SEARCH_EXCLUDE_PREFIXES=(empty) README documents the three vars, the NFS/SMB tuning note, and that FS_SEARCH_EXCLUDE_PREFIXES is a lexical prefilter (symlink-aware excludes tracked in modelcontextprotocol#4208). Tests: new __tests__/4162-timeouts-and-caps.test.ts (14 cases) using vitest fake timers + mocked fs/promises, so no real CloudStorage path is needed in CI. Full suite 160/160. Verified on a real macOS Google Drive tree by @alemuenchen. Deliberately out of scope: libuv threadpool concurrency, fs.opendir streaming for huge single directories, symlink-aware excludes (modelcontextprotocol#4208). Co-authored-by: Alessandro <alemuenchen@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
JonoGitty
added a commit
to JonoGitty/servers
that referenced
this pull request
May 20, 2026
…r paths (modelcontextprotocol#4162) fs.realpath (in validatePath) and recursive fs.readdir (in search_files / directory_tree) are called with no timeout, and recursive search has no upper bound on entries visited. On macOS provider-backed paths (~/Library/CloudStorage/..., ~/Library/Mobile Documents/com~apple~CloudDocs/..., NFS/SMB) these can block for minutes while the provider fetches metadata; the host eventually reports the connector unresponsive while the subprocess stays stuck. Follow-up to modelcontextprotocol#4181, which only skipped excluded entries before realpath. lib.ts: - withFsTimeout() wraps fs.realpath and fs.readdir; on timeout rejects with a typed FsTimeoutError instead of hanging. A no-op catch on the timeout promise avoids spurious unhandled-rejection reports when the operation wins the race. - FS_SEARCH_MAX_VISITED caps recursive search; on cap it throws a typed FsSearchTruncatedError (carrying visited + maxVisited), mirroring FsTimeoutError so callers can discriminate a safety-cap abort from a genuine empty result via instanceof rather than string-matching the message. - FS_SEARCH_EXCLUDE_PREFIXES (opt-in, empty default) refuses recursive operations on configured slow roots, via the shared boundary-aware assertSearchPathNotExcluded helper (reuses isPathWithinAllowedDirectories, expandHome applied symmetrically to the candidate root). - ENOENT branch in validatePath refactored so a hung parent realpath surfaces as FsTimeoutError instead of being masked as "Parent directory does not exist". - Per-entry catch in search re-throws FsTimeoutError so a search never returns silently-partial results. index.ts: - directory_tree gets the same timeout + visited-cap treatment and shares the exclude-prefix helper; throws FsSearchTruncatedError immediately on cap-exceed rather than threading an aborted flag through unwinding recursion. Config (all env-var, conservative defaults; invalid values warn + fall back): FS_OP_TIMEOUT_MS=15000, FS_SEARCH_MAX_VISITED=50000, FS_SEARCH_EXCLUDE_PREFIXES=(empty) README documents the three vars, the NFS/SMB tuning note, and that FS_SEARCH_EXCLUDE_PREFIXES is a lexical prefilter (symlink-aware excludes tracked in modelcontextprotocol#4208). Tests: new __tests__/4162-timeouts-and-caps.test.ts (14 cases) using vitest fake timers + mocked fs/promises, so no real CloudStorage path is needed in CI; cap test asserts the typed FsSearchTruncatedError + its visited/maxVisited fields. Full suite 160/160. Verified on a real macOS Google Drive tree by @alemuenchen. Deliberately out of scope: libuv threadpool concurrency, fs.opendir streaming for huge single directories, symlink-aware excludes (modelcontextprotocol#4208). Co-authored-by: Alessandro <alemuenchen@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
2 participants
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.
Summary
search_filesexclude patterns before validating each child pathfs.realpathon excluded entries, which makes provider-backed directories possible to skip before they can blockRefs #4162
Testing
npm test --workspace @modelcontextprotocol/server-filesystemnpm run build --workspace @modelcontextprotocol/server-filesystemgit diff --check origin/main...HEAD