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

.changeset/filesystem-unc-share-root.md

@@ -0,0 +1,5 @@
+---
+"@modelcontextprotocol/server-filesystem": patch
+---
+
+On Windows, a UNC share root given as an allowed directory (`\\server\share` or `\\server\share\`) now admits the files and subdirectories under it. Before, only the share root itself was accessible and every path below it was refused. Sibling shares such as `\\server\share-evil` are still refused.

src/filesystem/__tests__/win32.test.ts

@@ -1,9 +1,7 @@
 // Windows path handling, characterized on any host (#4854). The `path` module
 // is replaced by `path.win32` and `process.platform` reads "win32", so the
 // Windows-only branches of path-utils.ts and path-validation.ts run here:
-// drive roots, bare drive letters, backslash conversion and UNC shares. #3527
-// (a UNC share as the allowed root refuses its own subdirectories) is pinned
-// as it stands.
+// drive roots, bare drive letters, backslash conversion and UNC shares.
 
 import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
 
@@ -85,14 +83,61 @@ describe("isPathWithinAllowedDirectories on win32", () => {
     ).toBe(true);
   });
 
-  // KNOWN BUG #3527: pins current (wrong) behavior; the fix changes this assertion.
-  // #3527: path.resolve keeps a UNC root's trailing backslash, and the prefix
-  // check appends another, so nothing below the share matches.
-  it("refuses a subdirectory of a UNC share allowed directory (#3527)", () => {
+  // #3527: path.resolve keeps a UNC share root's trailing backslash, so the
+  // prefix check must not append a second one.
+  it("accepts a subdirectory of a UNC share allowed directory (#3527)", () => {
     expect(
       isPathWithinAllowedDirectories("\\\\server\\share\\sub", [
         "\\\\server\\share\\",
       ]),
+    ).toBe(true);
+  });
+
+  it("accepts a deep path under a UNC share given without a trailing backslash", () => {
+    expect(
+      isPathWithinAllowedDirectories("\\\\server\\share\\a b\\.c\\f.txt", [
+        "\\\\server\\share",
+      ]),
+    ).toBe(true);
+  });
+
+  it("refuses a sibling share whose name extends the allowed share's", () => {
+    expect(
+      isPathWithinAllowedDirectories("\\\\server\\share-evil\\x", [
+        "\\\\server\\share\\",
+      ]),
+    ).toBe(false);
+    expect(
+      isPathWithinAllowedDirectories("\\\\server\\share-evil", [
+        "\\\\server\\share",
+      ]),
+    ).toBe(false);
+  });
+
+  it("refuses another share, and the same share on another server", () => {
+    expect(
+      isPathWithinAllowedDirectories("\\\\server\\other\\x", [
+        "\\\\server\\share\\",
+      ]),
+    ).toBe(false);
+    expect(
+      isPathWithinAllowedDirectories("\\\\server2\\share\\x", [
+        "\\\\server\\share\\",
+      ]),
+    ).toBe(false);
+  });
+
+  it("keeps a .. that climbs past a UNC share root inside that share", () => {
+    // Node clamps .. at the share root, so this resolves to \\server\share\other.
+    expect(
+      isPathWithinAllowedDirectories("\\\\server\\share\\..\\other\\x", [
+        "\\\\server\\share\\",
+      ]),
+    ).toBe(true);
+    expect(
+      isPathWithinAllowedDirectories("\\\\server\\share\\sub\\..\\..\\x", [
+        "\\\\server\\share\\sub",
+      ]),
     ).toBe(false);
   });
 });

src/filesystem/path-validation.ts

@@ -93,6 +93,14 @@ export function isPathWithinAllowedDirectories(
       );
     }
 
-    return normalizedPath.startsWith(normalizedDir + path.sep);
+    // path.resolve strips a trailing separator except from a root, and a UNC
+    // share root (\\server\share\) keeps one, so appending another would give
+    // \\server\share\\, a prefix nothing matches (#3527). Append one only when
+    // it is missing; the boundary stays at a separator either way, so a
+    // sibling such as \\server\share-evil still does not match.
+    const dirWithSep = normalizedDir.endsWith(path.sep)
+      ? normalizedDir
+      : normalizedDir + path.sep;
+    return normalizedPath.startsWith(dirWithSep);
   });
 }