fix(filesystem): do not trim an allowed directory's name after realpath

.changeset/filesystem-realpath-no-trim.md

@@ -0,0 +1,5 @@
+---
+"@modelcontextprotocol/server-filesystem": patch
+---
+
+Keep an allowed directory's name exactly as the file system spells it. A command-line directory, or a client Root's realpath, whose name ends in a space or a quote was trimmed, so the server allowed the sibling directory without that suffix (and refused the directory that was actually given).

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

@@ -13,6 +13,7 @@ import {
   mkdirSync,
   writeFileSync,
   realpathSync,
+  symlinkSync,
 } from "fs";
 import { homedir, tmpdir } from "os";
 import { join } from "path";
@@ -139,3 +140,35 @@ describe("getValidRootDirectories", () => {
     });
   });
 });
+
+// #5320: a Root's realpath is kept as the file system spells it. Trimming a
+// trailing space or quote from it names the sibling without the suffix.
+// Windows refuses or strips these characters in names, so POSIX only.
+describe.skipIf(process.platform === "win32")(
+  "getValidRootDirectories: a realpath ending in a space or quote (#5320)",
+  () => {
+    let base: string;
+
+    beforeEach(() => {
+      base = realpathSync.native(mkdtempSync(join(tmpdir(), "mcp-fs-5320-")));
+    });
+
+    afterEach(() => {
+      rmSync(base, { recursive: true, force: true });
+    });
+
+    it.each([" ", "'", '"'])(
+      "keeps the resolved form untrimmed when it ends in %j",
+      async (suffix) => {
+        const target = join(base, `real${suffix}`);
+        mkdirSync(target);
+        mkdirSync(join(base, "real"));
+        const link = join(base, "link");
+        symlinkSync(target, link);
+        expect(
+          await getValidRootDirectories([{ uri: pathToFileURL(link).href }]),
+        ).toEqual([link, target]);
+      },
+    );
+  },
+);

src/filesystem/__tests__/stdio.test.ts

@@ -275,3 +275,76 @@ describe("isEntryPoint", () => {
     expect(isEntryPoint(undefined, moduleUrl)).toBe(false);
   });
 });
+
+// #5320: a command-line directory whose name ends in a space or quote is
+// allowed under that exact name. Trimming it, in its given form or its
+// realpath, names the sibling without the suffix and allows that instead.
+// Windows refuses or strips these characters in names, so POSIX only.
+describe.skipIf(process.platform === "win32").each([
+  ["a space", " "],
+  ["a single quote", "'"],
+  ["a double quote", '"'],
+])("an allowed directory whose name ends in %s (#5320)", (_, suffix) => {
+  let allowed: string;
+  let sibling: string;
+
+  beforeEach(async () => {
+    allowed = path.join(dir, `project${suffix}`);
+    sibling = path.join(dir, "project");
+    await fs.mkdir(allowed);
+    await fs.mkdir(sibling);
+    await fs.writeFile(path.join(allowed, "mine.txt"), "allowed");
+    await fs.writeFile(path.join(sibling, "secret.txt"), "not allowed");
+  });
+
+  async function readOver(args: string[], file: string) {
+    await session.start(args);
+    await session.initialize();
+    session.send(2, "tools/call", {
+      name: "list_allowed_directories",
+      arguments: {},
+    });
+    session.send(3, "tools/call", {
+      name: "read_text_file",
+      arguments: { path: file },
+    });
+    return {
+      listed: (await session.response(2)).result?.structuredContent,
+      read: (await session.response(3)).result,
+    };
+  }
+
+  it("lists it untrimmed and refuses the sibling, given directly", async () => {
+    const { listed, read } = await readOver(
+      [allowed],
+      path.join(sibling, "secret.txt"),
+    );
+    expect(listed).toEqual({ content: `Allowed directories:\n${allowed}` });
+    expect(read?.isError).toBe(true);
+    expect(JSON.stringify(read?.content)).toMatch(
+      /Access denied - path outside allowed directories/,
+    );
+  });
+
+  it("keeps the realpath untrimmed and refuses the sibling, given through a symlink", async () => {
+    const link = path.join(dir, "link");
+    await fs.symlink(allowed, link);
+    const { listed, read } = await readOver(
+      [link],
+      path.join(sibling, "secret.txt"),
+    );
+    expect(listed).toEqual({
+      content: `Allowed directories:\n${link}\n${allowed}`,
+    });
+    expect(read?.isError).toBe(true);
+    expect(JSON.stringify(read?.content)).toMatch(
+      /Access denied - path outside allowed directories/,
+    );
+  });
+
+  it("still reads the allowed directory's own file", async () => {
+    const { read } = await readOver([allowed], path.join(allowed, "mine.txt"));
+    expect(read?.isError).toBeFalsy();
+    expect(read?.structuredContent).toEqual({ content: "allowed" });
+  });
+});

src/filesystem/index.ts

@@ -19,7 +19,11 @@ import {
   StdioServerTransport,
   type StdioServerHandle,
 } from "@modelcontextprotocol/server/stdio";
-import { normalizePath, expandHome, realpathWithRetry } from "./path-utils.js";
+import {
+  normalizePathFormat,
+  expandHome,
+  realpathWithRetry,
+} from "./path-utils.js";
 import { createServer } from "./server.js";
 
 /**
@@ -39,12 +43,18 @@ export async function resolveAllowedDirectories(
       args.map(async (dir) => {
         const expanded = expandHome(dir);
         const absolute = path.resolve(expanded);
-        const normalizedOriginal = normalizePath(absolute);
+        // normalizePathFormat, not normalizePath, for both forms: a trailing
+        // space or quote is part of a POSIX filename, so trimming it would
+        // allow the sibling that lacks it (#5320). Trimming after
+        // path.resolve could only ever remove a trailing character, never
+        // shell-quoting leftovers at the start, so the argument is taken
+        // exactly as the shell passed it.
+        const normalizedOriginal = normalizePathFormat(absolute);
         try {
           // Security: Resolve symlinks in allowed directories during startup
           // This ensures we know the real paths and can validate against them later
           const resolved = await realpathWithRetry(absolute);
-          const normalizedResolved = normalizePath(resolved);
+          const normalizedResolved = normalizePathFormat(resolved);
           // Return both original and resolved paths if they differ
           // This allows matching against either /tmp or /private/tmp on macOS
           if (normalizedOriginal !== normalizedResolved) {

src/filesystem/roots-utils.ts

@@ -1,11 +1,7 @@
 import { promises as fs, type Stats } from "fs";
 import path from "path";
 import os from "os";
-import {
-  normalizePath,
-  normalizePathFormat,
-  realpathWithRetry,
-} from "./path-utils.js";
+import { normalizePathFormat, realpathWithRetry } from "./path-utils.js";
 import type { Root } from "@modelcontextprotocol/server";
 import { fileURLToPath } from "url";
 
@@ -36,10 +32,11 @@ async function parseRootUri(rootUri: string): Promise<RootPaths | null> {
     const absolutePath = path.resolve(expandedPath);
     const resolvedPath = await realpathWithRetry(absolutePath);
     return {
-      // normalizePathFormat, not normalizePath: trimming whitespace or quotes
-      // from the client's spelling would name a different directory.
+      // normalizePathFormat, not normalizePath, for both: trimming whitespace
+      // or quotes from the client's spelling or from the realpath would name
+      // a different directory, and allow it (#5183, #5320).
       advertised: normalizePathFormat(absolutePath),
-      resolved: normalizePath(resolvedPath),
+      resolved: normalizePathFormat(resolvedPath),
     };
   } catch {
     return null; // Path doesn't exist or other error