fix(filesystem): case-insensitive Windows allowed-path matching (#470)

src/filesystem/__tests__/path-validation.test.ts

@@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';
 import * as path from 'path';
 import * as fs from 'fs/promises';
 import * as os from 'os';
-import { isPathWithinAllowedDirectories } from '../path-validation.js';
+import { isPathWithin, isPathWithinAllowedDirectories } from '../path-validation.js';
 
 /**
  * Check if the current environment supports symlink creation
@@ -998,3 +998,93 @@ describe('Path Validation', () => {
     });
   });
 });
+
+describe('isPathWithin - pure containment helper', () => {
+  // Platform-appropriate fixture root so these assertions run everywhere
+  const root = path.sep === '\\' ? 'C:\\allowed-root' : '/allowed-root';
+
+  it('allows exact root match and subdirectories', () => {
+    expect(isPathWithin([root], root)).toBe(true);
+    expect(isPathWithin([root], path.join(root, 'sub'))).toBe(true);
+    expect(isPathWithin([root], path.join(root, 'sub', 'file.txt'))).toBe(true);
+  });
+
+  it('denies traversal escapes judged by resolved location (any platform)', () => {
+    expect(isPathWithin([root], path.join(root, '..', 'escape'))).toBe(false);
+    expect(isPathWithin([root], path.join(root, 'a', '..', '..', 'escape'))).toBe(false);
+    // Deep traversal that lands back inside must still be allowed
+    expect(isPathWithin([root], path.join(root, 'a', 'b', '..', 'c', 'file.txt'))).toBe(true);
+  });
+
+  it('denies sibling directories sharing a name prefix (segment boundary)', () => {
+    expect(isPathWithin([root], root + '2')).toBe(false);
+    expect(isPathWithin([root], root + '-backup')).toBe(false);
+  });
+
+  it('rejects invalid inputs without throwing', () => {
+    expect(isPathWithin([], '/x')).toBe(false);
+    expect(isPathWithin(null as any, '/x')).toBe(false);
+    expect(isPathWithin(['/x'], '')).toBe(false);
+    expect(isPathWithin(['/x'], null as any)).toBe(false);
+    expect(isPathWithin(['/home/user/project'], '/home/user/project\x00/etc/passwd')).toBe(false);
+    expect(isPathWithin(['/home/user/project\x00'], '/home/user/project')).toBe(false);
+  });
+
+  it('handles file-system roots as allowed roots', () => {
+    if (path.sep === '/') {
+      expect(isPathWithin(['/'], '/etc/passwd')).toBe(true);
+    } else {
+      // Drive root: same drive allowed, different drive denied
+      expect(isPathWithin(['C:\\'], 'C:\\anything\\here')).toBe(true);
+      expect(isPathWithin(['C:\\'], 'D:\\other')).toBe(false);
+    }
+  });
+});
+
+describe('Case-insensitive containment on Windows (issue #470)', () => {
+  const isWin = process.platform === 'win32';
+
+  it('allows drive-letter case mismatches on Windows', () => {
+    if (!isWin) return;
+    const allowed = ['c:\\source'];
+    expect(isPathWithin(allowed, 'C:\\source\\file.md')).toBe(true);
+    expect(isPathWithin(allowed, 'c:\\source\\file.md')).toBe(true);
+
+    // Same decision through the legacy wrapper used by validate_path in lib.ts
+    expect(isPathWithinAllowedDirectories('C:\\source\\file.md', ['c:\\source'])).toBe(true);
+    expect(isPathWithinAllowedDirectories('c:\\source', ['C:\\source'])).toBe(true);
+  });
+
+  it('allows inner-component case mismatches on Windows', () => {
+    if (!isWin) return;
+    const allowed = ['C:\\Users\\ADITY\\Documents'];
+    expect(isPathWithin(allowed, 'C:\\users\\adity\\documents\\notes\\file.md')).toBe(true);
+    expect(isPathWithin(allowed, 'c:\\Users\\Adity\\Documents')).toBe(true);
+    expect(isPathWithin(allowed, 'C:\\USERS\\adity\\DOCUMENTS\\a\\b\\c.txt')).toBe(true);
+  });
+
+  it('still rejects case-folded siblings and other drives on Windows', () => {
+    if (!isWin) return;
+    const allowed = ['c:\\source'];
+    expect(isPathWithin(allowed, 'C:\\source2\\file.md')).toBe(false);
+    expect(isPathWithin(allowed, 'C:\\source-backup\\file.md')).toBe(false);
+    expect(isPathWithin(allowed, 'D:\\source\\file.md')).toBe(false);
+  });
+
+  it('denies traversal escapes even with case folding on Windows', () => {
+    if (!isWin) return;
+    const allowed = ['c:\\source'];
+    expect(isPathWithin(allowed, 'C:\\SOURCE\\..\\..\\Windows\\system32')).toBe(false);
+    expect(isPathWithin(allowed, 'c:\\source\\sub\\..\\..\\elsewhere\\secret.txt')).toBe(false);
+  });
+
+  it('keeps POSIX comparison byte-exact (case-sensitive)', () => {
+    if (isWin) return;
+    const allowed = ['/home/user/project'];
+    expect(isPathWithin(allowed, '/Home/user/project/file.md')).toBe(false);
+    expect(isPathWithin(allowed, '/home/user/PROJECT/file.md')).toBe(false);
+    expect(isPathWithin(allowed, '/HOME/USER/PROJECT')).toBe(false);
+    expect(isPathWithin(allowed, '/home/user/project/file.md')).toBe(true);
+    expect(isPathWithinAllowedDirectories('/Home/user/project', ['/home/user/project'])).toBe(false);
+  });
+});

src/filesystem/index.ts

@@ -7,7 +7,7 @@ import {
   type Root,
 } from "@modelcontextprotocol/sdk/types.js";
 import fs from "fs/promises";
-import { createReadStream } from "fs";
+import { createReadStream, realpathSync } from "fs";
 import path from "path";
 import { pathToFileURL } from "url";
 import { z } from "zod";
@@ -38,33 +38,41 @@ if (args.length === 0) {
   console.error("At least one directory must be provided by EITHER method for the server to operate.");
 }
 
-// Store allowed directories in normalized and resolved form
+/**
+ * Resolves an allowed directory to its on-disk canonical form.
+ * realpathSync.native collapses symlinks and — on Windows — drive-letter and
+ * inner-component casing plus 8.3 short names (GitHub issue #470), so the
+ * containment checks in validate_path compare consistently-cased roots.
+ * Returns null when the path cannot be resolved (e.g. it does not exist yet).
+ */
+function canonicalizeAllowedDirectory(dir: string): string | null {
+  try {
+    return normalizePath(realpathSync.native(dir));
+  } catch {
+    return null;
+  }
+}
+
+// Store allowed directories in normalized and resolved form.
+// Each argv entry is path.resolve()d exactly once at startup, then passed
+// through native realpath canonicalization above.
 // We store BOTH the original path AND the resolved path to handle symlinks correctly
 // This fixes the macOS /tmp -> /private/tmp symlink issue where users specify /tmp
 // but the resolved path is /private/tmp
-let allowedDirectories = (await Promise.all(
-  args.map(async (dir) => {
-    const expanded = expandHome(dir);
-    const absolute = path.resolve(expanded);
-    const normalizedOriginal = normalizePath(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 fs.realpath(absolute);
-      const normalizedResolved = normalizePath(resolved);
-      // Return both original and resolved paths if they differ
-      // This allows matching against either /tmp or /private/tmp on macOS
-      if (normalizedOriginal !== normalizedResolved) {
-        return [normalizedOriginal, normalizedResolved];
-      }
-      return [normalizedResolved];
-    } catch (error) {
-      // If we can't resolve (doesn't exist), use the normalized absolute path
-      // This allows configuring allowed dirs that will be created later
-      return [normalizedOriginal];
-    }
-  })
-)).flat();
+let allowedDirectories = args.flatMap((dir) => {
+  const expanded = expandHome(dir);
+  const absolute = path.resolve(expanded);
+  const normalizedOriginal = normalizePath(absolute);
+  const canonical = canonicalizeAllowedDirectory(absolute);
+  // Return both original and resolved paths if they differ
+  // This allows matching against either /tmp or /private/tmp on macOS
+  if (canonical && canonical !== normalizedOriginal) {
+    return [normalizedOriginal, canonical];
+  }
+  // If we can't resolve (doesn't exist), use the normalized absolute path
+  // This allows configuring allowed dirs that will be created later
+  return [canonical ?? normalizedOriginal];
+});
 
 // Filter to only accessible directories, warn about inaccessible ones
 const accessibleDirectories: string[] = [];

src/filesystem/path-validation.ts

@@ -1,86 +1,108 @@
 import path from 'path';
 
 /**
- * Checks if an absolute path is within any of the allowed directories.
- * 
- * @param absolutePath - The absolute path to check (will be normalized)
- * @param allowedDirectories - Array of absolute allowed directory paths (will be normalized)
- * @returns true if the path is within an allowed directory, false otherwise
- * @throws Error if given relative paths after normalization
+ * Whether path containment should be compared case-insensitively.
+ *
+ * Windows file systems (NTFS/FAT) are case-insensitive but case-preserving,
+ * so the same directory may legitimately be reached as `c:\source`,
+ * `C:\source`, or `C:\Source` (GitHub issue #470). Only fold case when
+ * running on win32; POSIX file systems are case-sensitive and must keep
+ * byte-exact comparisons.
  */
-export function isPathWithinAllowedDirectories(absolutePath: string, allowedDirectories: string[]): boolean {
-  // Type validation
-  if (typeof absolutePath !== 'string' || !Array.isArray(allowedDirectories)) {
-    return false;
-  }
+const IS_WINDOWS = process.platform === 'win32';
+
+/**
+ * Folds a resolved absolute path into its comparable form.
+ * On Windows both sides of the comparison are lowercased so drive-letter and
+ * inner-component casing differences never cause false access denials.
+ * On POSIX the path is returned untouched.
+ */
+function comparablePath(absolutePath: string): string {
+  return IS_WINDOWS ? absolutePath.toLowerCase() : absolutePath;
+}
 
-  // Reject empty inputs
-  if (!absolutePath || allowedDirectories.length === 0) {
+/**
+ * Pure containment check: is `targetPath` equal to, or nested under, any of
+ * `allowedRoots`?
+ *
+ * Both sides are resolved with path.resolve() before comparing, so relative
+ * input, redundant separators, and `.`/`..` segments cannot smuggle a path
+ * across a boundary — a traversal escape is judged by its final resolved
+ * location. The root prefix must match up to a segment boundary, so
+ * `/allowed` never matches `/allowed2`.
+ *
+ * The comparison is case-insensitive when `process.platform === 'win32'`
+ * and byte-exact on every other platform.
+ *
+ * @param allowedRoots - Allowed directory roots (absolute recommended; resolved internally)
+ * @param targetPath - Path to test against the allowed roots
+ * @returns true if targetPath is within an allowed root, false otherwise
+ */
+export function isPathWithin(allowedRoots: unknown[], targetPath: unknown): boolean {
+  // Type validation
+  if (!Array.isArray(allowedRoots) || typeof targetPath !== 'string' || !targetPath) {
     return false;
   }
 
   // Reject null bytes (forbidden in paths)
-  if (absolutePath.includes('\x00')) {
+  if (targetPath.includes('\x00')) {
     return false;
   }
 
-  // Normalize the input path
-  let normalizedPath: string;
+  // Normalize the target once; resolve() also collapses `.`/`..` segments so
+  // traversal attempts are judged on where they actually land.
+  let resolvedTarget: string;
   try {
-    normalizedPath = path.resolve(path.normalize(absolutePath));
+    resolvedTarget = path.resolve(path.normalize(targetPath));
   } catch {
     return false;
   }
+  const foldedTarget = comparablePath(resolvedTarget);
 
-  // Verify it's absolute after normalization
-  if (!path.isAbsolute(normalizedPath)) {
-    throw new Error('Path must be absolute after normalization');
-  }
-
-  // Check against each allowed directory
-  return allowedDirectories.some(dir => {
-    if (typeof dir !== 'string' || !dir) {
-      return false;
-    }
-
-    // Reject null bytes in allowed dirs
-    if (dir.includes('\x00')) {
+  return allowedRoots.some(root => {
+    if (typeof root !== 'string' || !root || root.includes('\x00')) {
       return false;
     }
 
-    // Normalize the allowed directory
-    let normalizedDir: string;
+    let resolvedRoot: string;
     try {
-      normalizedDir = path.resolve(path.normalize(dir));
+      resolvedRoot = path.resolve(path.normalize(root));
     } catch {
       return false;
     }
+    const foldedRoot = comparablePath(resolvedRoot);
 
-    // Verify allowed directory is absolute after normalization
-    if (!path.isAbsolute(normalizedDir)) {
-      throw new Error('Allowed directories must be absolute paths after normalization');
-    }
-
-    // Check if normalizedPath is within normalizedDir
-    // Path is inside if it's the same or a subdirectory
-    if (normalizedPath === normalizedDir) {
+    // The target may be the allowed root itself
+    if (foldedTarget === foldedRoot) {
       return true;
     }
-
-    // Special case for root directory to avoid double slash
-    // On Windows, we need to check if both paths are on the same drive
-    if (normalizedDir === path.sep) {
-      return normalizedPath.startsWith(path.sep);
-    }
-
-    // On Windows, also check for drive root (e.g., "C:\")
-    if (path.sep === '\\' && normalizedDir.match(/^[A-Za-z]:\\?$/)) {
-      // Ensure both paths are on the same drive
-      const dirDrive = normalizedDir.charAt(0).toLowerCase();
-      const pathDrive = normalizedPath.charAt(0).toLowerCase();
-      return pathDrive === dirDrive && normalizedPath.startsWith(normalizedDir.replace(/\\?$/, '\\'));
-    }
-
-    return normalizedPath.startsWith(normalizedDir + path.sep);
+
+    // Require a segment-boundary prefix. Resolved roots only keep their
+    // trailing separator at a file-system root (`/` or `C:\`), so this one
+    // rule covers plain directories, POSIX root, and drive roots alike —
+    // including the different-drive rejection on Windows.
+    const rootPrefix = foldedRoot.endsWith(path.sep) ? foldedRoot : foldedRoot + path.sep;
+    return foldedTarget.startsWith(rootPrefix);
   });
 }
+
+/**
+ * Checks if an absolute path is within any of the allowed directories.
+ *
+ * Backward-compatible wrapper around {@link isPathWithin} that preserves the
+ * historic argument order (target path first). All containment decisions —
+ * including the Windows case-insensitive behavior for issue #470 — live in
+ * {@link isPathWithin}.
+ *
+ * @param absolutePath - The absolute path to check (will be normalized)
+ * @param allowedDirectories - Array of absolute allowed directory paths (will be normalized)
+ * @returns true if the path is within an allowed directory, false otherwise
+ */
+export function isPathWithinAllowedDirectories(absolutePath: string, allowedDirectories: string[]): boolean {
+  // Type validation
+  if (typeof absolutePath !== 'string' || !Array.isArray(allowedDirectories)) {
+    return false;
+  }
+
+  return isPathWithin(allowedDirectories, absolutePath);
+}