diff --git a/src/filesystem/__tests__/roots-utils.test.ts b/src/filesystem/__tests__/roots-utils.test.ts index 1a39483953..bbef8e3b0e 100644 --- a/src/filesystem/__tests__/roots-utils.test.ts +++ b/src/filesystem/__tests__/roots-utils.test.ts @@ -1,10 +1,13 @@ 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, realpathSync, symlinkSync } from 'fs'; import { tmpdir } from 'os'; import { join } from 'path'; +import { pathToFileURL } from 'url'; import type { Root } from '@modelcontextprotocol/sdk/types.js'; +const toFileUri = (filePath: string) => pathToFileURL(filePath).href; + describe('getValidRootDirectories', () => { let testDir1: string; let testDir2: string; @@ -32,7 +35,7 @@ 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: toFileUri(testDir1), name: 'File URI' }, { uri: testDir2, name: 'Plain path' }, { uri: testDir3 } // Plain path without name property ]; @@ -48,9 +51,9 @@ describe('getValidRootDirectories', () => { it('should normalize complex paths', async () => { const subDir = join(testDir1, 'subdir'); mkdirSync(subDir); - + const roots = [ - { uri: `file://${testDir1}/./subdir/../subdir`, name: 'Complex Path' } + { uri: `${toFileUri(testDir1)}/./subdir/../subdir`, name: 'Complex Path' } ]; const result = await getValidRootDirectories(roots); @@ -58,6 +61,33 @@ describe('getValidRootDirectories', () => { expect(result).toHaveLength(1); expect(result[0]).toBe(subDir); }); + + it('keeps both original and resolved forms for roots that resolve differently', async (context) => { + const realDir = join(testDir1, 'real-root'); + const aliasDir = join(testDir1, 'alias-root'); + mkdirSync(realDir); + + try { + symlinkSync(realDir, aliasDir); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'EPERM') { + context.skip('symlink creation is not permitted in this environment'); + return; + } + throw error; + } + + const roots: Root[] = [ + { uri: toFileUri(aliasDir), name: 'Symlink Root' } + ]; + + const result = await getValidRootDirectories(roots); + const resolvedDir = realpathSync(aliasDir); + + expect(result).toContain(aliasDir); + expect(result).toContain(resolvedDir); + expect(result).toHaveLength(2); + }); }); describe('error handling', () => { @@ -66,9 +96,9 @@ 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: toFileUri(testDir1), name: 'Valid Dir' }, + { uri: toFileUri(nonExistentDir), name: 'Non-existent Dir' }, + { uri: toFileUri(testFile), name: 'File Not Dir' }, { uri: `file://${invalidPath}`, name: 'Invalid Path' } ]; @@ -81,4 +111,4 @@ describe('getValidRootDirectories', () => { expect(result).toHaveLength(1); }); }); -}); \ No newline at end of file +}); diff --git a/src/filesystem/roots-utils.ts b/src/filesystem/roots-utils.ts index 5e26bb246b..fba87de902 100644 --- a/src/filesystem/roots-utils.ts +++ b/src/filesystem/roots-utils.ts @@ -6,19 +6,30 @@ import type { Root } from '@modelcontextprotocol/sdk/types.js'; import { fileURLToPath } from "url"; /** - * Converts a root URI to a normalized directory path with basic security validation. + * Converts a root URI to normalized directory paths with basic security validation. + * + * Returns both the original normalized path and the resolved path when they differ. + * This keeps roots-provided directories symmetric with command-line directories, + * so paths addressed through either a symlink/mapped-drive form or its resolved + * target continue to pass allow-list validation. + * * @param rootUri - File URI (file://...) or plain directory path - * @returns Promise resolving to validated path or null if invalid + * @returns Promise resolving to validated paths or null if invalid */ -async function parseRootUri(rootUri: string): Promise { +async function parseRootUri(rootUri: string): Promise { try { const rawPath = rootUri.startsWith('file://') ? fileURLToPath(rootUri) : rootUri; - const expandedPath = rawPath.startsWith('~/') || rawPath === '~' - ? path.join(os.homedir(), rawPath.slice(1)) + const expandedPath = rawPath.startsWith('~/') || rawPath === '~' + ? path.join(os.homedir(), rawPath.slice(1)) : rawPath; const absolutePath = path.resolve(expandedPath); + const normalizedOriginal = normalizePath(absolutePath); const resolvedPath = await fs.realpath(absolutePath); - return normalizePath(resolvedPath); + const normalizedResolved = normalizePath(resolvedPath); + + return normalizedOriginal === normalizedResolved + ? [normalizedResolved] + : [normalizedOriginal, normalizedResolved]; } catch { return null; // Path doesn't exist or other error } @@ -41,11 +52,11 @@ function formatDirectoryError(dir: string, error?: unknown, reason?: string): st /** * Resolves requested root directories from MCP root specifications. - * + * * Converts root URI specifications (file:// URIs or plain paths) into normalized * directory paths, validating that each path exists and is a directory. * Includes symlink resolution for security. - * + * * @param requestedRoots - Array of root specifications with URI and optional name * @returns Promise resolving to array of validated directory paths */ @@ -53,25 +64,32 @@ export async function getValidRootDirectories( requestedRoots: readonly Root[] ): Promise { const validatedDirectories: string[] = []; - + const seenDirectories = new Set(); + for (const requestedRoot of requestedRoots) { - const resolvedPath = await parseRootUri(requestedRoot.uri); - if (!resolvedPath) { + const candidatePaths = await parseRootUri(requestedRoot.uri); + if (!candidatePaths) { console.error(formatDirectoryError(requestedRoot.uri, undefined, 'invalid path or inaccessible')); continue; } - - try { - const stats: Stats = await fs.stat(resolvedPath); - if (stats.isDirectory()) { - validatedDirectories.push(resolvedPath); - } else { - console.error(formatDirectoryError(resolvedPath, undefined, 'non-directory root')); + + for (const candidatePath of candidatePaths) { + try { + const stats: Stats = await fs.stat(candidatePath); + if (!stats.isDirectory()) { + console.error(formatDirectoryError(candidatePath, undefined, 'non-directory root')); + continue; + } + + if (!seenDirectories.has(candidatePath)) { + validatedDirectories.push(candidatePath); + seenDirectories.add(candidatePath); + } + } catch (error) { + console.error(formatDirectoryError(candidatePath, error)); } - } catch (error) { - console.error(formatDirectoryError(resolvedPath, error)); } } - + return validatedDirectories; -} \ No newline at end of file +}