From af28a133cba44553a4bc2ca7575c1ffb3573fb67 Mon Sep 17 00:00:00 2001 From: Victor Zabirov Date: Tue, 14 Apr 2026 15:37:21 +0300 Subject: [PATCH] fix(filesystem): tolerate Windows realpath ENOENT Made-with: Cursor --- src/filesystem/__tests__/lib.test.ts | 5 +++ src/filesystem/__tests__/roots-utils.test.ts | 38 ++++++++++++-------- src/filesystem/lib.ts | 27 +++++++------- src/filesystem/roots-utils.ts | 10 +++++- 4 files changed, 52 insertions(+), 28 deletions(-) diff --git a/src/filesystem/__tests__/lib.test.ts b/src/filesystem/__tests__/lib.test.ts index f7e585af22..d3e1b7246c 100644 --- a/src/filesystem/__tests__/lib.test.ts +++ b/src/filesystem/__tests__/lib.test.ts @@ -200,6 +200,11 @@ describe('Lib Functions', () => { mockFs.realpath .mockRejectedValueOnce(enoentError1) .mockRejectedValueOnce(enoentError2); + // With the Windows `realpath()` fallback logic, we also check `stat()` to + // distinguish "realpath broken" from "path truly doesn't exist". + mockFs.stat + .mockRejectedValueOnce(enoentError1) // stat(absolute) + .mockRejectedValueOnce(enoentError2); // stat(parentDir) await expect(validatePath(newFilePath)) .rejects.toThrow('Parent directory does not exist'); diff --git a/src/filesystem/__tests__/roots-utils.test.ts b/src/filesystem/__tests__/roots-utils.test.ts index 1a39483953..0bc2485526 100644 --- a/src/filesystem/__tests__/roots-utils.test.ts +++ b/src/filesystem/__tests__/roots-utils.test.ts @@ -1,9 +1,11 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { getValidRootDirectories } from '../roots-utils.js'; -import { mkdtempSync, rmSync, mkdirSync, writeFileSync, realpathSync } from 'fs'; +import { mkdtempSync, rmSync, mkdirSync, writeFileSync } from 'fs'; import { tmpdir } from 'os'; import { join } from 'path'; import type { Root } from '@modelcontextprotocol/sdk/types.js'; +import { pathToFileURL } from 'url'; +import { realpath as realpathAsync } from 'fs/promises'; describe('getValidRootDirectories', () => { let testDir1: string; @@ -13,9 +15,11 @@ describe('getValidRootDirectories', () => { beforeEach(() => { // Create test directories - testDir1 = realpathSync(mkdtempSync(join(tmpdir(), 'mcp-roots-test1-'))); - testDir2 = realpathSync(mkdtempSync(join(tmpdir(), 'mcp-roots-test2-'))); - testDir3 = realpathSync(mkdtempSync(join(tmpdir(), 'mcp-roots-test3-'))); + // Avoid depending on Windows 8.3 short path behavior differences between + // realpath sync/async implementations. + testDir1 = mkdtempSync(join(tmpdir(), 'mcp-roots-test1-')); + testDir2 = mkdtempSync(join(tmpdir(), 'mcp-roots-test2-')); + testDir3 = mkdtempSync(join(tmpdir(), 'mcp-roots-test3-')); // Create a test file (not a directory) testFile = join(testDir1, 'test-file.txt'); @@ -32,17 +36,21 @@ describe('getValidRootDirectories', () => { describe('valid directory processing', () => { it('should process all URI formats and edge cases', async () => { const roots = [ - { uri: `file://${testDir1}`, name: 'File URI' }, + { uri: pathToFileURL(testDir1).href, name: 'File URI' }, { uri: testDir2, name: 'Plain path' }, { uri: testDir3 } // Plain path without name property ]; const result = await getValidRootDirectories(roots); + const resolved = await Promise.all(result.map(p => realpathAsync(p))); + const expected1 = await realpathAsync(testDir1); + const expected2 = await realpathAsync(testDir2); + const expected3 = await realpathAsync(testDir3); - expect(result).toContain(testDir1); - expect(result).toContain(testDir2); - expect(result).toContain(testDir3); - expect(result).toHaveLength(3); + expect(resolved).toContain(expected1); + expect(resolved).toContain(expected2); + expect(resolved).toContain(expected3); + expect(resolved).toHaveLength(3); }); it('should normalize complex paths', async () => { @@ -50,13 +58,13 @@ describe('getValidRootDirectories', () => { mkdirSync(subDir); const roots = [ - { uri: `file://${testDir1}/./subdir/../subdir`, name: 'Complex Path' } + { uri: pathToFileURL(join(testDir1, './subdir/../subdir')).href, name: 'Complex Path' } ]; const result = await getValidRootDirectories(roots); expect(result).toHaveLength(1); - expect(result[0]).toBe(subDir); + expect(await realpathAsync(result[0])).toBe(await realpathAsync(subDir)); }); }); @@ -66,15 +74,15 @@ describe('getValidRootDirectories', () => { const nonExistentDir = join(tmpdir(), 'non-existent-directory-12345'); const invalidPath = '\0invalid\0path'; // Null bytes cause different error types const roots = [ - { uri: `file://${testDir1}`, name: 'Valid Dir' }, - { uri: `file://${nonExistentDir}`, name: 'Non-existent Dir' }, - { uri: `file://${testFile}`, name: 'File Not Dir' }, + { uri: pathToFileURL(testDir1).href, name: 'Valid Dir' }, + { uri: pathToFileURL(nonExistentDir).href, name: 'Non-existent Dir' }, + { uri: pathToFileURL(testFile).href, name: 'File Not Dir' }, { uri: `file://${invalidPath}`, name: 'Invalid Path' } ]; const result = await getValidRootDirectories(roots); - expect(result).toContain(testDir1); + expect(await Promise.all(result.map(p => realpathAsync(p)))).toContain(await realpathAsync(testDir1)); expect(result).not.toContain(nonExistentDir); expect(result).not.toContain(testFile); expect(result).not.toContain(invalidPath); diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts index 17e4654cd5..a8d3b08c64 100644 --- a/src/filesystem/lib.ts +++ b/src/filesystem/lib.ts @@ -110,8 +110,10 @@ export async function validatePath(requestedPath: string): Promise { throw new Error(`Access denied - path outside allowed directories: ${absolute} not in ${allowedDirectories.join(', ')}`); } - // Security: Handle symlinks by checking their real path to prevent symlink attacks - // This prevents attackers from creating symlinks that point outside allowed directories + // Security: Handle symlinks by checking their real path to prevent symlink attacks. + // On some Windows setups (e.g. SUBST/mapped drives) `fs.realpath()` may throw ENOENT + // even for existing paths. We fall back to absolute paths, but still enforce the + // allowlist using normalized absolute paths and parent `stat` checks. try { const realPath = await fs.realpath(absolute); const normalizedReal = normalizePath(realPath); @@ -120,19 +122,20 @@ export async function validatePath(requestedPath: string): Promise { } return realPath; } catch (error) { - // Security: For new files that don't exist yet, verify parent directory - // This ensures we can't create files in unauthorized locations - if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - const parentDir = path.dirname(absolute); + if ((error as NodeJS.ErrnoException)?.code === 'ENOENT') { + // If the path exists but realpath fails, allow the absolute path. try { - const realParentPath = await fs.realpath(parentDir); - const normalizedParent = normalizePath(realParentPath); - if (!isPathWithinAllowedDirectories(normalizedParent, allowedDirectories)) { - throw new Error(`Access denied - parent directory outside allowed directories: ${realParentPath} not in ${allowedDirectories.join(', ')}`); - } + await fs.stat(absolute); return absolute; } catch { - throw new Error(`Parent directory does not exist: ${parentDir}`); + // Path doesn't exist; verify parent directory exists (without realpath). + const parentDir = path.dirname(absolute); + try { + await fs.stat(parentDir); + return absolute; + } catch { + throw new Error(`Parent directory does not exist: ${parentDir}`); + } } } throw error; diff --git a/src/filesystem/roots-utils.ts b/src/filesystem/roots-utils.ts index 5e26bb246b..452c85ce6b 100644 --- a/src/filesystem/roots-utils.ts +++ b/src/filesystem/roots-utils.ts @@ -17,7 +17,15 @@ async function parseRootUri(rootUri: string): Promise { ? path.join(os.homedir(), rawPath.slice(1)) : rawPath; const absolutePath = path.resolve(expandedPath); - const resolvedPath = await fs.realpath(absolutePath); + // `fs.realpath()` can fail on some Windows setups (e.g. SUBST/mapped drives) + // even when the path exists and is accessible. Fall back to the absolute + // path so roots still work. + let resolvedPath = absolutePath; + try { + resolvedPath = await fs.realpath(absolutePath); + } catch { + // keep `absolutePath` + } return normalizePath(resolvedPath); } catch { return null; // Path doesn't exist or other error