From 3ffbec5a13f75eb6177bfedc3820e788b1289635 Mon Sep 17 00:00:00 2001 From: adityachaudhary99 Date: Sat, 22 Aug 2026 01:53:55 +0530 Subject: [PATCH] fix(filesystem): tolerate quoted, spaced, and 8.3 Windows paths in config args (#447) --- src/filesystem/README.md | 24 ++++ src/filesystem/__tests__/path-utils.test.ts | 130 +++++++++++++++++- .../__tests__/startup-validation.test.ts | 40 ++++++ src/filesystem/index.ts | 23 ++-- src/filesystem/path-utils.ts | 55 ++++++++ 5 files changed, 260 insertions(+), 12 deletions(-) diff --git a/src/filesystem/README.md b/src/filesystem/README.md index 5a7ffe791d..c0184b364c 100644 --- a/src/filesystem/README.md +++ b/src/filesystem/README.md @@ -275,6 +275,30 @@ On Windows, use `cmd /c` to launch `npx`: } ``` +#### Windows paths + +In `claude_desktop_config.json`, write paths with spaces as plain JSON strings and escape backslashes — do not add shell quoting on top: + +```json +{ + "mcpServers": { + "filesystem": { + "command": "cmd", + "args": [ + "/c", + "npx", + "-y", + "@modelcontextprotocol/server-filesystem", + "C:\\Program Files\\My App\\Data", + "~/Documents" + ] + } + } +} +``` + +If an entry does arrive wrapped in literal double/single quotes (`"C:\\Program Files"`), the server strips them, trims stray whitespace, expands a leading `~` to your home directory, and converts Git Bash/MSYS style paths (`/c/Users/name`) on Windows. Windows 8.3 short names (e.g. `PROGRA~1`) are resolved to their full paths during startup validation. Entries that still cannot be accessed are skipped with a warning naming the offending path. + ## Usage with VS Code For quick installation, click the installation buttons below... diff --git a/src/filesystem/__tests__/path-utils.test.ts b/src/filesystem/__tests__/path-utils.test.ts index 3f6072377b..5dc7b725b7 100644 --- a/src/filesystem/__tests__/path-utils.test.ts +++ b/src/filesystem/__tests__/path-utils.test.ts @@ -1,5 +1,14 @@ import { describe, it, expect, afterEach } from 'vitest'; -import { normalizePath, expandHome, convertToWindowsPath } from '../path-utils.js'; +import * as path from 'path'; +import * as os from 'os'; +import { existsSync, realpathSync } from 'fs'; +import { + normalizePath, + expandHome, + convertToWindowsPath, + parseAllowedDirectory, + parseAllowedDirectories, +} from '../path-utils.js'; describe('Path Utilities', () => { describe('convertToWindowsPath', () => { @@ -379,4 +388,123 @@ describe('Path Utilities', () => { } }); }); + + describe('parseAllowedDirectories (issue #447 - Windows config paths)', () => { + const home = os.homedir(); + + it('preserves spaces in paths', () => { + if (process.platform === 'win32') { + expect(parseAllowedDirectory('C:\\Program Files\\Some App')) + .toBe('C:\\Program Files\\Some App'); + expect(parseAllowedDirectory('C:/Program Files/Some App')) + .toBe('C:\\Program Files\\Some App'); + } else { + expect(parseAllowedDirectory('/home/user/some app')) + .toBe('/home/user/some app'); + } + }); + + it('strips surrounding double quotes from entries', () => { + // Simulates a claude_desktop_config.json entry whose quote characters + // arrive as part of the argument itself + if (process.platform === 'win32') { + expect(parseAllowedDirectory('"C:\\Program Files\\App Name"')) + .toBe('C:\\Program Files\\App Name'); + } else { + expect(parseAllowedDirectory('"/home/user/app name"')) + .toBe('/home/user/app name'); + } + }); + + it('strips surrounding single quotes from entries', () => { + if (process.platform === 'win32') { + expect(parseAllowedDirectory("'C:\\Program Files\\App Name'")) + .toBe('C:\\Program Files\\App Name'); + } else { + expect(parseAllowedDirectory("'/home/user/app name'")) + .toBe('/home/user/app name'); + } + }); + + it('trims whitespace around entries', () => { + if (process.platform === 'win32') { + expect(parseAllowedDirectory(' C:\\dir with space ')) + .toBe('C:\\dir with space'); + } else { + expect(parseAllowedDirectory(' /home/user/dir ')) + .toBe('/home/user/dir'); + } + }); + + it('expands ~ to the home directory', () => { + const result = parseAllowedDirectory('~'); + expect(result).not.toContain('~'); + expect(result).toBe(path.resolve(home)); + }); + + it('expands ~/ prefixed paths (quotes included) keeping spaces', () => { + expect(parseAllowedDirectory('"~/my documents"')) + .toBe(path.join(home, 'my documents')); + expect(parseAllowedDirectory('~/projects/my work')) + .toBe(path.join(home, 'projects', 'my work')); + }); + + it('converts MSYS/Git-Bash style /c/ paths only on win32', () => { + if (process.platform === 'win32') { + expect(parseAllowedDirectory('/c/Users/name/App')) + .toBe('C:\\Users\\name\\App'); + expect(parseAllowedDirectory('/c/Program Files/App Name')) + .toBe('C:\\Program Files\\App Name'); + expect(parseAllowedDirectory('/d/data/project')) + .toBe('D:\\data\\project'); + // Bare drive roots convert too + expect(parseAllowedDirectory('/c/')).toBe('C:\\'); + } else { + // Off Windows, /c/foo is just an ordinary Unix path + expect(parseAllowedDirectory('/c/Users/name/App')) + .toBe('/c/Users/name/App'); + } + }); + + it('leaves WSL /mnt/c/ paths untouched', () => { + // The /^\/([A-Za-z])\// conversion regex must not match multi-letter mounts + expect(parseAllowedDirectory('/mnt/c/Users/name')) + .toBe(path.resolve('/mnt/c/Users/name')); + }); + + it('does not break 8.3 short names while parsing', () => { + if (process.platform === 'win32') { + expect(parseAllowedDirectory('C:\\PROGRA~1')) + .toBe('C:\\PROGRA~1'); + } + }); + + it('resolves relative paths to absolute ones', () => { + const result = parseAllowedDirectory('some relative dir'); + expect(path.isAbsolute(result)).toBe(true); + }); + + it('parses every argv entry in order', () => { + expect(parseAllowedDirectories(['~/a', '~/b'])) + .toEqual([path.join(home, 'a'), path.join(home, 'b')]); + }); + + it('expands real 8.3 short names via native realpath (startup behavior)', () => { + // Mirrors the startup resolution now used by index.ts. + // Skipped when 8.3 short-name generation is disabled on this volume. + if (process.platform !== 'win32') return; + const shortPath = 'C:\\PROGRA~1'; + if (!existsSync(shortPath)) return; + + const parsed = parseAllowedDirectories([shortPath])[0]; + // The parser must preserve short names verbatim + expect(parsed).toBe(shortPath); + + // Native realpath expands them to the long form + const longForm = realpathSync.native(parsed); + expect(existsSync(longForm)).toBe(true); + expect(longForm.toLowerCase()).not.toBe(shortPath.toLowerCase()); + expect(longForm.includes('~')).toBe(false); + }); + }); }); diff --git a/src/filesystem/__tests__/startup-validation.test.ts b/src/filesystem/__tests__/startup-validation.test.ts index 3be283df74..d0ffaf63e0 100644 --- a/src/filesystem/__tests__/startup-validation.test.ts +++ b/src/filesystem/__tests__/startup-validation.test.ts @@ -97,4 +97,44 @@ describe('Startup Directory Validation', () => { // Should still start with the valid directory expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio'); }); + + it('starts cleanly when a path with spaces arrives wrapped in literal quotes', async () => { + // claude_desktop_config.json entries sometimes carry their own quote + // characters around paths containing spaces (issue #447) + const quoted = `"${accessibleDir}"`; + + const result = await spawnServer([quoted]); + + expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio'); + expect(result.stderr).not.toContain('Warning:'); + }); + + it('accepts MSYS/Git-Bash style /c/ paths on Windows', async () => { + if (process.platform !== 'win32') return; + + // C:\...\accessible -> /c/.../accessible + const msysDir = `/${accessibleDir.charAt(0).toLowerCase()}${accessibleDir.slice(2).replace(/\\/g, '/')}`; + + const result = await spawnServer([msysDir]); + + expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio'); + expect(result.stderr).not.toContain('Warning:'); + }); + + it('expands 8.3 short names at startup via native realpath resolution', async () => { + if (process.platform !== 'win32') return; + + // Skip when 8.3 short-name generation is disabled on this volume + try { + await fs.stat('C:\\PROGRA~1'); + } catch { + return; + } + + const result = await spawnServer(['C:\\PROGRA~1']); + + // Startup must succeed without treating the short name as inaccessible + expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio'); + expect(result.stderr).not.toContain('Warning: Cannot access directory'); + }); }); diff --git a/src/filesystem/index.ts b/src/filesystem/index.ts index 234605bb13..55b9e4b799 100644 --- a/src/filesystem/index.ts +++ b/src/filesystem/index.ts @@ -7,12 +7,12 @@ 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"; import { minimatch } from "minimatch"; -import { normalizePath, expandHome } from './path-utils.js'; +import { normalizePath, parseAllowedDirectories } from './path-utils.js'; import { getValidRootDirectories } from './roots-utils.js'; import { // Function imports @@ -38,19 +38,20 @@ 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 -// 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 +// Parse allowed directories from raw argv entries before any validation. +// Entries coming from claude_desktop_config.json can carry surrounding +// quotes, stray whitespace, a leading ~, or MSYS/Git-Bash style /c/foo paths; +// parseAllowedDirectories normalizes each entry into an absolute native path. let allowedDirectories = (await Promise.all( - args.map(async (dir) => { - const expanded = expandHome(dir); - const absolute = path.resolve(expanded); - const normalizedOriginal = normalizePath(absolute); + parseAllowedDirectories(args).map(async (dir) => { + const normalizedOriginal = normalizePath(dir); 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); + // realpathSync.native is used so Windows 8.3 short names (e.g. PROGRA~1) + // are expanded to their long form; the default JS implementation leaves + // short-name segments untouched. + const resolved = realpathSync.native(dir); const normalizedResolved = normalizePath(resolved); // Return both original and resolved paths if they differ // This allows matching against either /tmp or /private/tmp on macOS diff --git a/src/filesystem/path-utils.ts b/src/filesystem/path-utils.ts index 6ab5a5969b..29299514d6 100644 --- a/src/filesystem/path-utils.ts +++ b/src/filesystem/path-utils.ts @@ -123,3 +123,58 @@ export function expandHome(filepath: string): string { return filepath; } +/** + * Parses a single allowed-directory argument as it arrives from argv or a + * client config file (e.g. claude_desktop_config.json). + * + * Config authors commonly quote paths that contain spaces (or wrap them in + * shell-style quotes) and may pass MSYS/Git-Bash style paths like /c/Users. + * Left unhandled, those entries fail startup validation and the server + * disconnects (issue #447). This hardens one entry: + * + * 1. Strips surrounding double/single quotes + * 2. Trims surrounding whitespace + * 3. Expands a leading `~` to os.homedir() + * 4. Converts MSYS/Git-Bash style /c/foo to C:\foo on win32 only + * 5. Resolves to an absolute path via path.resolve() + * + * @param rawArg The raw argument as received + * @returns Absolute, resolved path ready for directory validation + */ +export function parseAllowedDirectory(rawArg: string): string { + // Strip whitespace first so quote detection sees the actual boundary chars + let entry = rawArg.trim(); + + // Strip one pair of surrounding double or single quotes + if ( + entry.length >= 2 && + ((entry.startsWith('"') && entry.endsWith('"')) || + (entry.startsWith("'") && entry.endsWith("'"))) + ) { + entry = entry.slice(1, -1); + entry = entry.trim(); + } + + // Expand a leading ~ to the user's home directory + entry = expandHome(entry); + + // Convert MSYS/Git-Bash style paths (/c/foo -> C:\foo), win32 only. + // The regex does not match WSL-style /mnt/c/... paths, so those are left intact. + if (process.platform === 'win32') { + entry = entry.replace(/^\/([A-Za-z])\//, (_match, drive: string) => `${drive.toUpperCase()}:\\`); + } + + // Resolve relative paths against cwd and normalize separators per platform + return path.resolve(entry); +} + +/** + * Parses every allowed-directory argument before directory validation runs. + * Order is preserved; each entry is hardened by parseAllowedDirectory. + * @param rawArgs Raw argv entries for allowed directories + * @returns Absolute, resolved paths ready for directory validation + */ +export function parseAllowedDirectories(rawArgs: string[]): string[] { + return rawArgs.map(parseAllowedDirectory); +} +