mirror of
https://github.com/modelcontextprotocol/servers.git
synced 2026-04-17 23:53:24 +02:00
Merge pull request #2609 from modelcontextprotocol/claude/issue-2526-20250824-0240
fix: resolve relative paths against allowed directories instead of process.cwd()
This commit is contained in:
@@ -204,6 +204,30 @@ describe('Lib Functions', () => {
|
||||
await expect(validatePath(newFilePath))
|
||||
.rejects.toThrow('Parent directory does not exist');
|
||||
});
|
||||
|
||||
it('resolves relative paths against allowed directories instead of process.cwd()', async () => {
|
||||
const relativePath = 'test-file.txt';
|
||||
const originalCwd = process.cwd;
|
||||
|
||||
// Mock process.cwd to return a directory outside allowed directories
|
||||
const disallowedCwd = process.platform === 'win32' ? 'C:\\Windows\\System32' : '/root';
|
||||
(process as any).cwd = vi.fn(() => disallowedCwd);
|
||||
|
||||
try {
|
||||
const result = await validatePath(relativePath);
|
||||
|
||||
// Result should be resolved against first allowed directory, not process.cwd()
|
||||
const expectedPath = process.platform === 'win32'
|
||||
? path.resolve('C:\\Users\\test', relativePath)
|
||||
: path.resolve('/home/user', relativePath);
|
||||
|
||||
expect(result).toBe(expectedPath);
|
||||
expect(result).not.toContain(disallowedCwd);
|
||||
} finally {
|
||||
// Restore original process.cwd
|
||||
process.cwd = originalCwd;
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -72,12 +72,35 @@ export function createUnifiedDiff(originalContent: string, newContent: string, f
|
||||
);
|
||||
}
|
||||
|
||||
// Helper function to resolve relative paths against allowed directories
|
||||
function resolveRelativePathAgainstAllowedDirectories(relativePath: string): string {
|
||||
if (allowedDirectories.length === 0) {
|
||||
// Fallback to process.cwd() if no allowed directories are set
|
||||
return path.resolve(process.cwd(), relativePath);
|
||||
}
|
||||
|
||||
// Try to resolve relative path against each allowed directory
|
||||
for (const allowedDir of allowedDirectories) {
|
||||
const candidate = path.resolve(allowedDir, relativePath);
|
||||
const normalizedCandidate = normalizePath(candidate);
|
||||
|
||||
// Check if the resulting path lies within any allowed directory
|
||||
if (isPathWithinAllowedDirectories(normalizedCandidate, allowedDirectories)) {
|
||||
return candidate;
|
||||
}
|
||||
}
|
||||
|
||||
// If no valid resolution found, use the first allowed directory as base
|
||||
// This provides a consistent fallback behavior
|
||||
return path.resolve(allowedDirectories[0], relativePath);
|
||||
}
|
||||
|
||||
// Security & Validation Functions
|
||||
export async function validatePath(requestedPath: string): Promise<string> {
|
||||
const expandedPath = expandHome(requestedPath);
|
||||
const absolute = path.isAbsolute(expandedPath)
|
||||
? path.resolve(expandedPath)
|
||||
: path.resolve(process.cwd(), expandedPath);
|
||||
: resolveRelativePathAgainstAllowedDirectories(expandedPath);
|
||||
|
||||
const normalizedRequested = normalizePath(absolute);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user