fix(filesystem): admit paths under a UNC share root allowed directory

[cliffhall]

Closes #3527

Part of #5004 (Wave 2)

Description

isPathWithinAllowedDirectories() refused every path below an allowed directory that is a UNC share root. path.resolve() keeps the trailing separator on a share root (\\server\share and \\server\share\ both resolve to \\server\share\), just as it does on a drive root, and the final prefix check appended another separator, comparing against \\server\share\\, which nothing matches. Only the share root itself got through (the exact-match branch); a UNC subfolder as the allowed directory already worked, since resolve gives it no trailing separator.

The fix appends the separator only when the resolved directory does not already end in one. The match boundary stays at a separator, so a sibling such as \\server\share-evil is still refused.

Credit: the fix is the one proposed in #4219 by @he-yufeng (closed unmerged), ported with Co-authored-by; the root-cause analysis on #3527 is by @gdimas38-eng. Of the candidate PRs: #3615 and #3601 work from the theory that path.resolve corrupts UNC paths, and both still compare against the doubled separator for a share root, so neither fixes the reported case; #4689 is about a different issue (#4686).

Server Details

• Server: filesystem
• Changed: src/filesystem/path-validation.ts (the final prefix check in isPathWithinAllowedDirectories); src/filesystem/__tests__/win32.test.ts; a patch changeset.

Motivation and Context

#3527: with a network share allowed (\\server\share), listing the share works but every subdirectory and file in it is denied, so the only workaround was to list each subfolder separately.

How Has This Been Tested?

There is no Windows host or real UNC share in this environment (Linux sandbox), so no Inspector or LLM-client run against a real share was possible. The change has no client surface beyond the access check, so the evidence is two targeted probes of that check with Node's path.win32 semantics:

1. The pinning test, now asserting the fix. win32.test.ts runs path-validation.ts with path replaced by path.win32 and process.platform set to win32. The KNOWN BUG #3527 test now asserts \\server\share\sub is within \\server\share\, marker removed, plus new cases. With the fix reverted (source only, new tests kept), three fail:

× accepts a subdirectory of a UNC share allowed directory (#3527)
× accepts a deep path under a UNC share given without a trailing backslash
× keeps a .. that climbs past a UNC share root inside that share
Tests  3 failed | 12 passed (15)

With the fix: Tests 15 passed (15).

2. The shipped JavaScript. A Node ESM loader hook maps path to path.win32 and calls isPathWithinAllowedDirectories from the built src/filesystem/dist/path-validation.js, and the same function transpiled from origin/v2/main:

path	allowed dir	v2/main	this branch
\\server\share\sub	\\server\share\	false	true
\\server\share\sub\f.txt	\\server\share	false	true
\\server\share	\\server\share\	true	true
\\server\share-evil\x	\\server\share\	false	false
\\server\other\x	\\server\share	false	false
\\server2\share\x	\\server\share	false	false
C:\Temp2\a.txt	C:\Temp	false	false
C:\Users\me	C:\	true	true

Security cases kept refused, as tests: a sibling share whose name extends the allowed one (share-evil), another share on the same server, the same share on another server, and .. (Node clamps .. at the share root, so \\server\share\..\other\x stays inside the share, while \\server\share\sub\..\..\x is refused against \\server\share\sub).

npm run local:gate exits 0 (filesystem per-file coverage at 100% lines, gate green).

Breaking Changes

None. No client configuration changes. Windows users who allowed a UNC share root now get access to its contents, which is what the configuration already asked for.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Protocol Documentation
• My changes follow MCP security best practices (boundary still at a separator; sibling-share, other-share, other-server and .. cases tested)
• I have updated the server's README accordingly (not applicable: no documented behavior or option changes; the README already describes allowed directories)
• I have added a changeset (npm run changeset) if this changes what a TypeScript server publishes (.changeset/filesystem-unc-share-root.md, patch)
• I have tested this with an LLM client (not done: no Windows host or UNC share available here; see the probes above)
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling (not applicable: no new failure modes; the check returns a boolean as before)
• I have documented all environment variables and configuration options (not applicable: none added)

Additional context

Kept to the one comparison in isPathWithinAllowedDirectories, so it does not overlap the parallel path-normalization work in #5004 (#1970).

🤖 Generated with Claude Code

[changeset-bot]

🦋 Changeset detected

Latest commit: fd4dff7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@modelcontextprotocol/server-filesystem	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Copilot]

Copilot review overview

🟢 Approval recommended

The focused fix preserves path boundaries and includes comprehensive regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes UNC share-root containment checks in the filesystem server.

Changes:

• Avoids duplicating the separator after normalized UNC roots.
• Adds regression and boundary-security tests.
• Adds a patch changeset.

File	Description
src/​filesystem/​path-validation.ts	Corrects separator-aware prefix matching.
src/​filesystem/​__tests__/​win32.test.ts	Covers UNC descendants and containment boundaries.
.changeset/​filesystem-unc-share-root.md	Documents the published fix.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.