Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/filesystem/__tests__/lib.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
38 changes: 23 additions & 15 deletions src/filesystem/__tests__/roots-utils.test.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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');
Expand All @@ -32,31 +36,35 @@ 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 () => {
const subDir = join(testDir1, 'subdir');
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));
});
});

Expand All @@ -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);
Expand Down
27 changes: 15 additions & 12 deletions src/filesystem/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,10 @@ export async function validatePath(requestedPath: string): Promise<string> {
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);
Expand All @@ -120,19 +122,20 @@ export async function validatePath(requestedPath: string): Promise<string> {
}
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;
Expand Down
10 changes: 9 additions & 1 deletion src/filesystem/roots-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,15 @@ async function parseRootUri(rootUri: string): Promise<string | null> {
? 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
Expand Down
Loading