fix: skip symlink cycles during pack instead of failing with ELOOP#293
Open
gagan-53 wants to merge 1 commit into
Open
fix: skip symlink cycles during pack instead of failing with ELOOP#293gagan-53 wants to merge 1 commit into
gagan-53 wants to merge 1 commit into
Conversation
getAllFiles and getAllFilesWithCount follow symlinks via statSync and recurse into anything reported as a directory, so a symlink cycle (e.g. a self-referential link) descends until the OS raises ELOOP and pack aborts with a cryptic error. Track the real paths of directories on the current traversal path and, when recursing would revisit one, warn with the offending path and skip it. Non-cyclic symlinks are still followed and packed as before. Fixes modelcontextprotocol#292 Co-Authored-By: Claude Fable 5 <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
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.
Fixes #292.
Problem
When an extension directory contains a symlink cycle (e.g.
ln -s . selfloop),mcpb packaborts with a raw OS error and produces no archive:getAllFilesandgetAllFilesWithCountinsrc/node/files.tswalk the tree withstatSync(which follows symlinks) and recurse into anything reported as a directory, with no tracking of already-visited paths, so a cyclic link descends until the OS raisesELOOP.Fix
Track the real paths (
realpathSync) of the directories on the current traversal path. Before recursing into a directory, resolve its real path; if it is already on the traversal path, emit a warning naming the offending path and skip it:Packing then continues and succeeds. Behavior for everything else is unchanged:
visitedRealPathsparameter is optional and trailing on both exported functions, so the public API is backward compatible.Testing
test/symlink-cycle.test.ts: self-referential symlink, mutually recursive symlinks (A→B, B→A), a non-cyclic symlink still being followed, and coverage of bothgetAllFilesandgetAllFilesWithCount. The cycle tests fail withELOOPwithout the fix. Symlinks are created with typejunction(works unprivileged on Windows; ignored elsewhere) and the tests no-op if the environment cannot create symlinks.yarn test: 129 passed, 9 suites.yarn lint: clean.ERROR: Archive error: ELOOP: ..., exit 1, no archive; after — warning + successful pack, archive written, exit 0.