fix(filesystem): validate the request path as it will be used

src/filesystem/__tests__/sibling-paths.test.ts

@@ -0,0 +1,41 @@
+import fs from 'fs/promises';
+import os from 'os';
+import path from 'path';
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import { setAllowedDirectories, validatePath } from '../lib.js';
+
+// A request path must be checked exactly as it will be used: a sibling directory
+// whose name is the allowed directory's name plus a quote or whitespace is outside it.
+describe('validatePath with sibling names that differ by quotes or whitespace', () => {
+  let parentDir: string;
+  let allowedDir: string;
+
+  beforeEach(async () => {
+    parentDir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-sibling-')));
+    allowedDir = path.join(parentDir, 'allowed');
+    await fs.mkdir(allowedDir);
+    setAllowedDirectories([allowedDir]);
+  });
+
+  afterEach(async () => {
+    setAllowedDirectories([]);
+    await fs.rm(parentDir, { recursive: true, force: true });
+  });
+
+  it.each(["'", '"', ' ', '\t'])('rejects an existing sibling named allowed%j', async (suffix) => {
+    await fs.mkdir(allowedDir + suffix);
+    await fs.writeFile(path.join(allowedDir + suffix, 'secret.txt'), 'outside');
+    await expect(validatePath(allowedDir + suffix)).rejects.toThrow('Access denied');
+    await expect(validatePath(path.join(allowedDir + suffix, 'secret.txt'))).rejects.toThrow('Access denied');
+  });
+
+  it('rejects a new file next to the allowed directory', async () => {
+    await expect(validatePath(allowedDir + '\t')).rejects.toThrow('Access denied');
+  });
+
+  it('still allows a file inside the allowed directory whose name ends with a quote', async () => {
+    const file = path.join(allowedDir, "it's'");
+    await fs.writeFile(file, 'inside');
+    await expect(validatePath(file)).resolves.toBe(file);
+  });
+});

src/filesystem/lib.ts

@@ -5,7 +5,7 @@ import { randomBytes } from 'crypto';
 import { StringDecoder } from 'string_decoder';
 import { diffLines, createTwoFilesPatch } from 'diff';
 import { minimatch } from 'minimatch';
-import { normalizePath, expandHome } from './path-utils.js';
+import { normalizePathFormat, expandHome } from './path-utils.js';
 import { isPathWithinAllowedDirectories } from './path-validation.js';
 
 // Global allowed directories - set by the main module
@@ -83,7 +83,7 @@ function resolveRelativePathAgainstAllowedDirectories(relativePath: string): str
   // Try to resolve relative path against each allowed directory
   for (const allowedDir of allowedDirectories) {
     const candidate = path.resolve(allowedDir, relativePath);
-    const normalizedCandidate = normalizePath(candidate);
+    const normalizedCandidate = normalizePathFormat(candidate);
 
     // Check if the resulting path lies within any allowed directory
     if (isPathWithinAllowedDirectories(normalizedCandidate, allowedDirectories)) {
@@ -100,7 +100,7 @@ function resolveRelativePathAgainstAllowedDirectories(relativePath: string): str
 async function resolveUnicodeEquivalentPath(absolutePath: string): Promise<string> {
   const allowedDirectory = [...allowedDirectories]
     .sort((left, right) => right.length - left.length)
-    .find(directory => isPathWithinAllowedDirectories(normalizePath(absolutePath), [directory]));
+    .find(directory => isPathWithinAllowedDirectories(normalizePathFormat(absolutePath), [directory]));
 
   if (!allowedDirectory) {
     return absolutePath;
@@ -129,7 +129,7 @@ async function resolveUnicodeEquivalentPath(absolutePath: string): Promise<strin
     }
 
     currentPath = await fs.realpath(path.join(currentPath, equivalentMatches[0]));
-    if (!isPathWithinAllowedDirectories(normalizePath(currentPath), allowedDirectories)) {
+    if (!isPathWithinAllowedDirectories(normalizePathFormat(currentPath), allowedDirectories)) {
       throw new Error(`Access denied - symlink target outside allowed directories: ${currentPath} not in ${allowedDirectories.join(', ')}`);
     }
   }
@@ -149,7 +149,7 @@ export async function validatePath(requestedPath: string): Promise<string> {
     ? path.resolve(expandedPath)
     : resolveRelativePathAgainstAllowedDirectories(expandedPath);
 
-  const normalizedRequested = normalizePath(absolute);
+  const normalizedRequested = normalizePathFormat(absolute);
 
   // Security: Check if path is within allowed directories before any file operations
   const isAllowed = isPathWithinAllowedDirectories(normalizedRequested, allowedDirectories);
@@ -161,7 +161,7 @@ export async function validatePath(requestedPath: string): Promise<string> {
   // This prevents attackers from creating symlinks that point outside allowed directories
   try {
     const realPath = await fs.realpath(absolute);
-    const normalizedReal = normalizePath(realPath);
+    const normalizedReal = normalizePathFormat(realPath);
     if (!isPathWithinAllowedDirectories(normalizedReal, allowedDirectories)) {
       throw new Error(`Access denied - symlink target outside allowed directories: ${realPath} not in ${allowedDirectories.join(', ')}`);
     }

src/filesystem/path-utils.ts

@@ -38,8 +38,16 @@ export function convertToWindowsPath(p: string): string {
  */
 export function normalizePath(p: string): string {
   // Remove any surrounding quotes and whitespace
-  p = p.trim().replace(/^["']|["']$/g, '');
+  return normalizePathFormat(p.trim().replace(/^["']|["']$/g, ''));
+}
 
+/**
+ * Like normalizePath, but keeps surrounding quotes and whitespace, which can be
+ * part of a real file name. Use it for paths that are checked and then used as-is.
+ * @param p The path to normalize
+ * @returns Normalized path
+ */
+export function normalizePathFormat(p: string): string {
   // Check if this is a Unix path that should not be converted
   // WSL paths (/mnt/) should ALWAYS be preserved as they work correctly in WSL with Node.js fs
   // Regular Unix paths should also be preserved