Skip to content

Commit 31a71c7

Browse files
committed
filesystem: classify root and path validation failures (#3606)
1 parent b60eca1 commit 31a71c7

4 files changed

Lines changed: 37 additions & 14 deletions

File tree

src/filesystem/__tests__/lib.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ describe('Lib Functions', () => {
169169
it('rejects disallowed paths', async () => {
170170
const testPath = process.platform === 'win32' ? 'C:\\Windows\\System32\\file.txt' : '/etc/passwd';
171171
await expect(validatePath(testPath))
172-
.rejects.toThrow('Access denied - path outside allowed directories');
172+
.rejects.toThrow('[path_outside_allowed_roots] Access denied - path outside allowed directories');
173173
});
174174

175175
it('handles non-existent files by checking parent directory', async () => {
@@ -200,9 +200,9 @@ describe('Lib Functions', () => {
200200
mockFs.realpath
201201
.mockRejectedValueOnce(enoentError1)
202202
.mockRejectedValueOnce(enoentError2);
203-
203+
204204
await expect(validatePath(newFilePath))
205-
.rejects.toThrow('Parent directory does not exist');
205+
.rejects.toThrow('[path_parent_missing] Parent directory does not exist');
206206
});
207207

208208
it('resolves relative paths against allowed directories instead of process.cwd()', async () => {

src/filesystem/__tests__/startup-validation.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,9 @@ describe('Startup Directory Validation', () => {
8181

8282
// Should exit with error
8383
expect(result.exitCode).toBe(1);
84-
expect(result.stderr).toContain('Error: None of the specified directories are accessible');
84+
expect(result.stderr).toMatch(
85+
/argv_no_accessible_directories|missing_roots|Error: None of the specified directories are accessible/
86+
);
8587
});
8688

8789
it('should warn when path is not a directory', async () => {

src/filesystem/index.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ for (const dir of allowedDirectories) {
8383

8484
// Exit only if ALL paths are inaccessible (and some were specified)
8585
if (accessibleDirectories.length === 0 && allowedDirectories.length > 0) {
86-
console.error("Error: None of the specified directories are accessible");
86+
console.error("Error [missing_roots]: None of the specified directories are accessible");
8787
process.exit(1);
8888
}
8989

@@ -745,8 +745,8 @@ server.server.oninitialized = async () => {
745745
} else {
746746
if (allowedDirectories.length > 0) {
747747
console.error("Client does not support MCP Roots, using allowed directories set from server args:", allowedDirectories);
748-
}else{
749-
throw new Error(`Server cannot operate: No allowed directories available. Server was started without command-line directories and client either does not support MCP roots protocol or provided empty roots. Please either: 1) Start server with directory arguments, or 2) Use a client that supports MCP roots protocol and provides valid root directories.`);
748+
} else {
749+
throw new Error(`[missing_roots] Server cannot operate: No allowed directories available. Server was started without command-line directories and client either does not support MCP roots protocol or provided empty roots. Please either: 1) Start server with directory arguments, or 2) Use a client that supports MCP roots protocol and provides valid root directories.`);
750750
}
751751
}
752752
};

src/filesystem/lib.ts

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ import { isPathWithinAllowedDirectories } from './path-validation.js';
1010
// Global allowed directories - set by the main module
1111
let allowedDirectories: string[] = [];
1212

13+
function formatFilesystemValidationError(code: string, detail: string): Error {
14+
return new Error(`[${code}] ${detail}`);
15+
}
16+
1317
// Function to set allowed directories from the main module
1418
export function setAllowedDirectories(directories: string[]): void {
1519
allowedDirectories = [...directories];
@@ -98,16 +102,24 @@ function resolveRelativePathAgainstAllowedDirectories(relativePath: string): str
98102
// Security & Validation Functions
99103
export async function validatePath(requestedPath: string): Promise<string> {
100104
const expandedPath = expandHome(requestedPath);
101-
const absolute = path.isAbsolute(expandedPath)
102-
? path.resolve(expandedPath)
103-
: resolveRelativePathAgainstAllowedDirectories(expandedPath);
105+
let absolute: string;
106+
try {
107+
absolute = path.isAbsolute(expandedPath)
108+
? path.resolve(expandedPath)
109+
: resolveRelativePathAgainstAllowedDirectories(expandedPath);
110+
} catch {
111+
throw formatFilesystemValidationError('path_invalid', `Invalid path: ${requestedPath}`);
112+
}
104113

105114
const normalizedRequested = normalizePath(absolute);
106115

107116
// Security: Check if path is within allowed directories before any file operations
108117
const isAllowed = isPathWithinAllowedDirectories(normalizedRequested, allowedDirectories);
109118
if (!isAllowed) {
110-
throw new Error(`Access denied - path outside allowed directories: ${absolute} not in ${allowedDirectories.join(', ')}`);
119+
throw formatFilesystemValidationError(
120+
'path_outside_allowed_roots',
121+
`Access denied - path outside allowed directories: ${absolute} not in ${allowedDirectories.join(', ')}`
122+
);
111123
}
112124

113125
// Security: Handle symlinks by checking their real path to prevent symlink attacks
@@ -116,7 +128,10 @@ export async function validatePath(requestedPath: string): Promise<string> {
116128
const realPath = await fs.realpath(absolute);
117129
const normalizedReal = normalizePath(realPath);
118130
if (!isPathWithinAllowedDirectories(normalizedReal, allowedDirectories)) {
119-
throw new Error(`Access denied - symlink target outside allowed directories: ${realPath} not in ${allowedDirectories.join(', ')}`);
131+
throw formatFilesystemValidationError(
132+
'path_outside_allowed_roots',
133+
`Access denied - symlink target outside allowed directories: ${realPath} not in ${allowedDirectories.join(', ')}`
134+
);
120135
}
121136
return realPath;
122137
} catch (error) {
@@ -128,13 +143,19 @@ export async function validatePath(requestedPath: string): Promise<string> {
128143
const realParentPath = await fs.realpath(parentDir);
129144
const normalizedParent = normalizePath(realParentPath);
130145
if (!isPathWithinAllowedDirectories(normalizedParent, allowedDirectories)) {
131-
throw new Error(`Access denied - parent directory outside allowed directories: ${realParentPath} not in ${allowedDirectories.join(', ')}`);
146+
throw formatFilesystemValidationError(
147+
'path_outside_allowed_roots',
148+
`Access denied - parent directory outside allowed directories: ${realParentPath} not in ${allowedDirectories.join(', ')}`
149+
);
132150
}
133151
return absolute;
134152
} catch {
135-
throw new Error(`Parent directory does not exist: ${parentDir}`);
153+
throw formatFilesystemValidationError('path_parent_missing', `Parent directory does not exist: ${parentDir}`);
136154
}
137155
}
156+
if ((error as NodeJS.ErrnoException).code === 'EACCES' || (error as NodeJS.ErrnoException).code === 'EPERM') {
157+
throw formatFilesystemValidationError('path_permission_denied', `Permission denied: ${absolute}`);
158+
}
138159
throw error;
139160
}
140161
}

0 commit comments

Comments
 (0)