fix(filesystem): handle paths containing tilde (~) without crashing#3595
Open
VoidChecksum wants to merge 1 commit intomodelcontextprotocol:mainfrom
Open
fix(filesystem): handle paths containing tilde (~) without crashing#3595VoidChecksum wants to merge 1 commit intomodelcontextprotocol:mainfrom
VoidChecksum wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
Fix silent server crashes when configured directories contain tilde characters, addressing both the crash mechanism and root cause. Changes: - Add safeFileURLToPath() to handle malformed file:// URIs where tilde in the path gets misinterpreted as the URI authority component (e.g. file://~/path treats ~ as hostname, causing fileURLToPath to throw) - Replace inline tilde expansion in parseRootUri with shared expandHome() to ensure consistent behavior across all code paths - Add descriptive error logging in parseRootUri instead of silently returning null, so users can diagnose path resolution failures - Replace unhandled throw in oninitialized with console.error + process.exit to prevent silent crashes from unhandled promise rejections - Add warning when all client-provided roots fail validation, listing the URIs that failed so users can identify the problematic paths - Improve expandHome documentation to clarify tilde handling semantics Tests added: - Directories with literal tilde in name via file URI and plain path - Malformed file://~/path URIs (tilde in authority position) - Multiple directories with tildes - validatePath with tilde in directory names - Home expansion combined with literal tilde in subdirectory names - Windows short names with tilde (PROGRA~1) Closes modelcontextprotocol#3412
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a crash in the filesystem MCP server when configured roots include ~ (either as home shorthand or as a literal character) by hardening root URI/path parsing, improving diagnostics, and adding regression tests.
Changes:
- Add
safeFileURLToPath()and update root parsing to handle malformedfile://URIs and use sharedexpandHome(). - Improve initialization-time diagnostics and exit behavior when no allowed directories can be established.
- Expand
expandHome()documentation and add tests covering tilde-related scenarios.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/filesystem/roots-utils.ts |
Adds safer file-URI conversion, consolidates tilde expansion via expandHome(), and logs root-resolution failures. |
src/filesystem/path-utils.ts |
Clarifies expandHome() behavior for literal ~ vs home shorthand. |
src/filesystem/index.ts |
Adds warning when all client roots fail validation; exits cleanly when no directories are available. |
src/filesystem/__tests__/roots-utils.test.ts |
Adds coverage for literal-tilde directory names and malformed file://~/... URIs. |
src/filesystem/__tests__/path-utils.test.ts |
Adds coverage for literal ~ segments and Windows short-name tildes. |
src/filesystem/__tests__/lib.test.ts |
Adds coverage ensuring validatePath() accepts literal-tilde paths and ~/.../~dir combinations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+22
to
+31
| // Try to recover by extracting the path after "file://" and normalizing it. | ||
| try { | ||
| const withoutScheme = uri.slice('file://'.length); | ||
| // If the path starts with / it's absolute (file:///path or file:///~/path) | ||
| if (withoutScheme.startsWith('/')) { | ||
| return decodeURIComponent(withoutScheme); | ||
| } | ||
| // Otherwise treat the whole part after file:// as a path | ||
| // (e.g., file://~/folder -> ~/folder) | ||
| return decodeURIComponent(withoutScheme); |
Comment on lines
+64
to
+67
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| console.error(`Warning: Could not resolve root path "${rootUri}": ${message}`); | ||
| return null; |
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.
Summary
Fixes #3412 — the filesystem MCP server crashes silently when configured directories contain tilde (
~) characters in their names.Root causes identified and fixed:
Malformed
file://URIs: When a path like~/folderis naively concatenated asfile://~/folder, the URL parser interprets~as the hostname, causingfileURLToPath()to throw. AddedsafeFileURLToPath()that recovers from this by extracting and decoding the path component.Silent error swallowing:
parseRootUri()caught all errors and returnednullwith no logging. When all roots failed validation, the server continued with zero allowed directories and no indication of what went wrong. Now logs descriptive warnings for each failed root.Unhandled promise rejection crash: The
oninitializedhandler threw anErrorinside an async callback when no directories were available. This caused an unhandled promise rejection that crashed the Node.js process silently. Replaced withconsole.error()+process.exit(1)for a clean exit with a descriptive message.Duplicated tilde expansion logic:
parseRootUri()had its own inline tilde expansion instead of using the sharedexpandHome()function. Consolidated to use the shared function for consistent behavior.Additional improvements:
expandHome()documentation to clarify handling of literal~in directory names vs home directory shorthandChanges
roots-utils.tssafeFileURLToPath(), use sharedexpandHome(), add error loggingindex.tsthrowwithprocess.exit(1), add root validation warningpath-utils.tsexpandHome()documentation__tests__/roots-utils.test.ts__tests__/path-utils.test.tsexpandHome()__tests__/lib.test.tsvalidatePath()with tilde pathsTest plan
~in name resolve correctly via file URI~in name resolve correctly via plain pathfile://~/pathURIs are handled gracefully (not crash)validatePathaccepts paths with~in directory names~/) combined with literal~in subdirectory names worksPROGRA~1) are preservedGenerated with Claude Code