fix(filesystem): allow a client Root in its advertised form as well as its realpath

.changeset/5183-filesystem-root-advertised-form.md

@@ -0,0 +1,5 @@
+---
+"@modelcontextprotocol/server-filesystem": patch
+---
+
+A client Root is now allowed in the form the client advertised it as well as in its resolved form, as command-line directories already were. A request that names a path through a symlinked Root's own spelling (for example `/tmp/...` for a Root of `file:///tmp/...` on macOS) is no longer refused, and `list_allowed_directories` lists both forms. Every request is still resolved and checked against the resolved directory, so a symlink inside a Root that points outside it stays refused.

src/filesystem/README.md

@@ -31,6 +31,8 @@ A client that connects with a 2025-era protocol version (the `initialize` handsh
 
 Roots notified by Client to Server, completely replace any server-side Allowed directories when provided.
 
+A Root reached through a symlink is allowed in both its advertised form and its resolved form, as a command-line directory is: a Root of `file:///tmp/project` on macOS allows `/tmp/project` and `/private/tmp/project`. Every requested path is still resolved and checked against the resolved directories, so a symlink inside a Root that points outside it is refused.
+
 **Important**: If server starts without command-line arguments AND a 2025-era client doesn't support roots protocol, the server fails at initialization: it logs the reason to stderr, closes the connection and exits with status 1. A client that supports roots but provides none (or none that are valid) keeps the connection, with no allowed directories until it sends `roots/list_changed` with valid roots.
 
 The 2026-07-28 protocol has no `initialize` handshake and no server-to-client `roots/list` request, so a 2026-07-28 client's roots are never asked for: the command-line directories are the whole allow-list. A server started with none answers every tool call on such a connection with an error saying so.
@@ -205,7 +207,7 @@ The server's directory access control follows this flow:
   - List all directories the server is allowed to access
   - No input required
   - Returns:
-    - Directories that this server can read/write from
+    - Directories that this server can read/write from. A directory reached through a symlink is listed twice, in the form it was given and in its resolved form
 
 ### Slow and very large trees
 

src/filesystem/__tests__/server-roots.test.ts

@@ -84,15 +84,148 @@ describe("initial roots", () => {
     );
   });
 
-  it("resolve a symlinked root to its target", async () => {
+  // #5183: a Root is allowed in its advertised spelling as well as its
+  // realpath, as a command-line directory is, so a request that uses the
+  // Root's own spelling is not refused.
+  it("admit a symlinked root in both its advertised and its resolved form (#5183)", async () => {
     const link = path.join(cliDir, "link-to-root");
     await fs.symlink(rootDir, link);
+    await fs.writeFile(path.join(rootDir, "note.txt"), "inside the root");
     const { client } = await connectTracked([], {
       capabilities: ROOTS,
       listRoots: () => [rootOf(link)],
     });
     await vi.waitFor(async () =>
-      expect(await allowedDirectoriesOf(client)).toEqual([rootDir]),
+      expect(await allowedDirectoriesOf(client)).toEqual([link, rootDir]),
+    );
+    const viaLink = await call(client, "read_text_file", {
+      path: path.join(link, "note.txt"),
+    });
+    expect(textOf(viaLink)).toBe("inside the root");
+    const viaTarget = await call(client, "read_text_file", {
+      path: path.join(rootDir, "note.txt"),
+    });
+    expect(textOf(viaTarget)).toBe("inside the root");
+  });
+
+  // #5183: admitting the advertised spelling does not widen access. Every
+  // request is still realpath'd and re-checked, so a symlink inside the Root
+  // that points outside it is refused under either spelling.
+  it("refuse a symlink inside a symlinked root that points outside it (#5183)", async () => {
+    const link = path.join(cliDir, "link-to-root");
+    await fs.symlink(rootDir, link);
+    const secret = path.join(cliDir, "secret.txt");
+    await fs.writeFile(secret, "outside the root");
+    await fs.symlink(secret, path.join(rootDir, "escape.txt"));
+    const { client } = await connectTracked([], {
+      capabilities: ROOTS,
+      listRoots: () => [rootOf(link)],
+    });
+    await vi.waitFor(async () =>
+      expect(await allowedDirectoriesOf(client)).toEqual([link, rootDir]),
+    );
+    for (const dir of [link, rootDir]) {
+      const result = await call(client, "read_text_file", {
+        path: path.join(dir, "escape.txt"),
+      });
+      expect(textOf(result)).toMatch(/^Access denied/);
+    }
+  });
+
+  // #5183: the advertised form names a symlink, which can be retargeted after
+  // the Roots load. A new file or directory under it must still be refused
+  // when the symlink now leads outside every allowed directory.
+  it("refuse creating under a root alias retargeted outside the root (#5183)", async () => {
+    const link = path.join(cliDir, "link-to-root");
+    await fs.symlink(rootDir, link);
+    const { client } = await connectTracked([], {
+      capabilities: ROOTS,
+      listRoots: () => [rootOf(link)],
+    });
+    await vi.waitFor(async () =>
+      expect(await allowedDirectoriesOf(client)).toEqual([link, rootDir]),
+    );
+    const outside = path.join(cliDir, "outside");
+    await fs.mkdir(outside);
+    await fs.rm(link);
+    await fs.symlink(outside, link);
+
+    const written = await call(client, "write_file", {
+      path: path.join(link, "new.txt"),
+      content: "x",
+    });
+    expect(textOf(written)).toMatch(/^Access denied/);
+    const created = await call(client, "create_directory", {
+      path: path.join(link, "a", "b"),
+    });
+    expect(textOf(created)).toMatch(/^Access denied/);
+    expect(await fs.readdir(outside)).toEqual([]);
+  });
+
+  // #5183: the advertised form is kept as the client spelled it. Trimming
+  // whitespace or quotes from it would name a different directory, here an
+  // unrelated sibling, and allow it. POSIX only: Windows filenames cannot end
+  // in a space.
+  it.skipIf(process.platform === "win32")(
+    "keep a root's advertised form verbatim, not trimmed to a sibling's name (#5183)",
+    async () => {
+      const link = path.join(cliDir, "alias ");
+      await fs.symlink(rootDir, link);
+      const sibling = path.join(cliDir, "alias");
+      await fs.mkdir(sibling);
+      await fs.writeFile(path.join(sibling, "secret.txt"), "not in the root");
+      const { client } = await connectTracked([], {
+        capabilities: ROOTS,
+        listRoots: () => [rootOf(link)],
+      });
+      await vi.waitFor(async () =>
+        expect(await allowedDirectoriesOf(client)).toEqual([link, rootDir]),
+      );
+      const result = await call(client, "read_text_file", {
+        path: path.join(sibling, "secret.txt"),
+      });
+      expect(textOf(result)).toMatch(/^Access denied/);
+    },
+  );
+
+  // #5183 as reported: on macOS /tmp is a symlink to /private/tmp, so a Root
+  // of file:///tmp/... was allowed only as /private/tmp/... and a request
+  // naming /tmp/... was refused.
+  it.runIf(process.platform === "darwin")(
+    "admit a macOS /tmp root under its /tmp spelling (#5183)",
+    async () => {
+      const tmpRoot = await fs.mkdtemp("/tmp/mcp-fs-5183-");
+      try {
+        await fs.writeFile(path.join(tmpRoot, "note.txt"), "in /tmp");
+        const { client } = await connectTracked([], {
+          capabilities: ROOTS,
+          listRoots: () => [rootOf(tmpRoot)],
+        });
+        await vi.waitFor(async () =>
+          expect(await allowedDirectoriesOf(client)).toEqual([
+            tmpRoot,
+            await fs.realpath(tmpRoot),
+          ]),
+        );
+        const result = await call(client, "read_text_file", {
+          path: path.join(tmpRoot, "note.txt"),
+        });
+        expect(textOf(result)).toBe("in /tmp");
+      } finally {
+        await fs.rm(tmpRoot, { recursive: true, force: true });
+      }
+    },
+  );
+
+  it("list a root once when it is advertised in both its forms (#5183)", async () => {
+    const link = path.join(cliDir, "link-to-root");
+    await fs.symlink(rootDir, link);
+    const { client } = await connectTracked([], {
+      capabilities: ROOTS,
+      listRoots: () => [rootOf(link), rootOf(rootDir)],
+    });
+    await vi.waitFor(async () =>
+      expect(await allowedDirectoriesOf(client)).toEqual([link, rootDir]),
     );
   });
 

src/filesystem/lib.ts

@@ -376,6 +376,19 @@ async function resolveUnicodeEquivalentPath(
   }
 
   let currentPath = await realpathWithTimeout(allowedDirectory);
+  // An allowed directory can be a symlink (the advertised form of a Root or a
+  // command-line directory), retargeted since it was allowed, so its realpath
+  // is checked like every component below it (#5183).
+  if (
+    !isPathWithinAllowedDirectories(
+      normalizePathFormat(currentPath),
+      allowedDirectories,
+    )
+  ) {
+    throw new Error(
+      `Access denied - symlink target outside allowed directories: ${currentPath} not in ${allowedDirectories.join(", ")}`,
+    );
+  }
   const relativeParts = path
     .relative(allowedDirectory, absolutePath)
     .split(path.sep)

src/filesystem/roots-utils.ts

@@ -1,16 +1,30 @@
 import { promises as fs, type Stats } from "fs";
 import path from "path";
 import os from "os";
-import { normalizePath, realpathWithRetry } from "./path-utils.js";
+import {
+  normalizePath,
+  normalizePathFormat,
+  realpathWithRetry,
+} from "./path-utils.js";
 import type { Root } from "@modelcontextprotocol/server";
 import { fileURLToPath } from "url";
 
+/** A root's two spellings: as the client advertised it, and its realpath. */
+interface RootPaths {
+  advertised: string;
+  resolved: string;
+}
+
 /**
- * Converts a root URI to a normalized directory path with basic security validation.
+ * Converts a root URI to its normalized absolute path and its realpath.
+ * Both are kept, as resolveAllowedDirectories in index.ts keeps both forms of
+ * a command-line directory, so a request may use either spelling (#5183).
+ * validatePath still realpaths every request, so access stays bounded by the
+ * resolved directory.
  * @param rootUri - File URI (file://...) or plain directory path
- * @returns Promise resolving to validated path or null if invalid
+ * @returns Promise resolving to both forms, or null if invalid
  */
-async function parseRootUri(rootUri: string): Promise<string | null> {
+async function parseRootUri(rootUri: string): Promise<RootPaths | null> {
   try {
     const rawPath = rootUri.startsWith("file://")
       ? fileURLToPath(rootUri)
@@ -21,7 +35,12 @@ async function parseRootUri(rootUri: string): Promise<string | null> {
         : rawPath;
     const absolutePath = path.resolve(expandedPath);
     const resolvedPath = await realpathWithRetry(absolutePath);
-    return normalizePath(resolvedPath);
+    return {
+      // normalizePathFormat, not normalizePath: trimming whitespace or quotes
+      // from the client's spelling would name a different directory.
+      advertised: normalizePathFormat(absolutePath),
+      resolved: normalizePath(resolvedPath),
+    };
   } catch {
     return null; // Path doesn't exist or other error
   }
@@ -51,19 +70,20 @@ function formatDirectoryError(
  *
  * Converts root URI specifications (file:// URIs or plain paths) into normalized
  * directory paths, validating that each path exists and is a directory.
- * Includes symlink resolution for security.
+ * Each root contributes its advertised form and, when it differs, its resolved
+ * (realpath) form, deduplicated across roots.
  *
  * @param requestedRoots - Array of root specifications with URI and optional name
  * @returns Promise resolving to array of validated directory paths
  */
 export async function getValidRootDirectories(
   requestedRoots: readonly Root[],
 ): Promise<string[]> {
-  const validatedDirectories: string[] = [];
+  const validatedDirectories = new Set<string>();
 
   for (const requestedRoot of requestedRoots) {
-    const resolvedPath = await parseRootUri(requestedRoot.uri);
-    if (!resolvedPath) {
+    const rootPaths = await parseRootUri(requestedRoot.uri);
+    if (!rootPaths) {
       console.error(
         formatDirectoryError(
           requestedRoot.uri,
@@ -74,10 +94,12 @@ export async function getValidRootDirectories(
       continue;
     }
 
+    const { advertised, resolved: resolvedPath } = rootPaths;
     try {
       const stats: Stats = await fs.stat(resolvedPath);
       if (stats.isDirectory()) {
-        validatedDirectories.push(resolvedPath);
+        validatedDirectories.add(advertised);
+        validatedDirectories.add(resolvedPath);
       } else {
         console.error(
           formatDirectoryError(resolvedPath, undefined, "non-directory root"),
@@ -88,5 +110,5 @@ export async function getValidRootDirectories(
     }
   }
 
-  return validatedDirectories;
+  return [...validatedDirectories];
 }