Skip to content

Commit 2dfa15d

Browse files
committed
fix(filesystem): gracefully handle unavailable directories
Previously, the server would crash if any configured directory was unavailable (e.g., unmounted external drive). Now it: - Filters out inaccessible directories with a warning - Continues operating with remaining accessible directories - Only fails if NO directories are accessible Fixes #2815
1 parent 55a3056 commit 2dfa15d

File tree

2 files changed

+117
-8
lines changed

2 files changed

+117
-8
lines changed
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
2+
import { spawn } from 'child_process';
3+
import * as path from 'path';
4+
import * as fs from 'fs/promises';
5+
import * as os from 'os';
6+
7+
const SERVER_PATH = path.join(__dirname, '..', 'dist', 'index.js');
8+
9+
/**
10+
* Spawns the filesystem server with given arguments and returns exit info
11+
*/
12+
async function spawnServer(args: string[], timeoutMs = 2000): Promise<{ exitCode: number | null; stderr: string }> {
13+
return new Promise((resolve) => {
14+
const proc = spawn('node', [SERVER_PATH, ...args], {
15+
stdio: ['pipe', 'pipe', 'pipe'],
16+
});
17+
18+
let stderr = '';
19+
proc.stderr?.on('data', (data) => {
20+
stderr += data.toString();
21+
});
22+
23+
const timeout = setTimeout(() => {
24+
proc.kill('SIGTERM');
25+
}, timeoutMs);
26+
27+
proc.on('close', (code) => {
28+
clearTimeout(timeout);
29+
resolve({ exitCode: code, stderr });
30+
});
31+
32+
proc.on('error', (err) => {
33+
clearTimeout(timeout);
34+
resolve({ exitCode: 1, stderr: err.message });
35+
});
36+
});
37+
}
38+
39+
describe('Startup Directory Validation', () => {
40+
let testDir: string;
41+
let accessibleDir: string;
42+
let accessibleDir2: string;
43+
44+
beforeEach(async () => {
45+
testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'fs-startup-test-'));
46+
accessibleDir = path.join(testDir, 'accessible');
47+
accessibleDir2 = path.join(testDir, 'accessible2');
48+
await fs.mkdir(accessibleDir, { recursive: true });
49+
await fs.mkdir(accessibleDir2, { recursive: true });
50+
});
51+
52+
afterEach(async () => {
53+
await fs.rm(testDir, { recursive: true, force: true });
54+
});
55+
56+
it('should start successfully with all accessible directories', async () => {
57+
const result = await spawnServer([accessibleDir, accessibleDir2]);
58+
// Server starts and runs (we kill it after timeout, so exit code is null or from SIGTERM)
59+
expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio');
60+
expect(result.stderr).not.toContain('Error:');
61+
});
62+
63+
it('should skip inaccessible directory and continue with accessible one', async () => {
64+
const nonExistentDir = path.join(testDir, 'non-existent-dir-12345');
65+
66+
const result = await spawnServer([nonExistentDir, accessibleDir]);
67+
68+
// Should warn about inaccessible directory
69+
expect(result.stderr).toContain('Warning: Cannot access directory');
70+
expect(result.stderr).toContain(nonExistentDir);
71+
72+
// Should still start successfully
73+
expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio');
74+
});
75+
76+
it('should exit with error when ALL directories are inaccessible', async () => {
77+
const nonExistent1 = path.join(testDir, 'non-existent-1');
78+
const nonExistent2 = path.join(testDir, 'non-existent-2');
79+
80+
const result = await spawnServer([nonExistent1, nonExistent2]);
81+
82+
// Should exit with error
83+
expect(result.exitCode).toBe(1);
84+
expect(result.stderr).toContain('Error: None of the specified directories are accessible');
85+
});
86+
87+
it('should warn when path is not a directory', async () => {
88+
const filePath = path.join(testDir, 'not-a-directory.txt');
89+
await fs.writeFile(filePath, 'content');
90+
91+
const result = await spawnServer([filePath, accessibleDir]);
92+
93+
// Should warn about non-directory
94+
expect(result.stderr).toContain('Warning:');
95+
expect(result.stderr).toContain('not a directory');
96+
97+
// Should still start with the valid directory
98+
expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio');
99+
});
100+
});

src/filesystem/index.ts

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,28 @@ let allowedDirectories = await Promise.all(
5656
})
5757
);
5858

59-
// Validate that all directories exist and are accessible
60-
await Promise.all(allowedDirectories.map(async (dir) => {
59+
// Filter to only accessible directories, warn about inaccessible ones
60+
const accessibleDirectories: string[] = [];
61+
for (const dir of allowedDirectories) {
6162
try {
6263
const stats = await fs.stat(dir);
63-
if (!stats.isDirectory()) {
64-
console.error(`Error: ${dir} is not a directory`);
65-
process.exit(1);
64+
if (stats.isDirectory()) {
65+
accessibleDirectories.push(dir);
66+
} else {
67+
console.error(`Warning: ${dir} is not a directory, skipping`);
6668
}
6769
} catch (error) {
68-
console.error(`Error accessing directory ${dir}:`, error);
69-
process.exit(1);
70+
console.error(`Warning: Cannot access directory ${dir}, skipping`);
7071
}
71-
}));
72+
}
73+
74+
// Exit only if ALL paths are inaccessible (and some were specified)
75+
if (accessibleDirectories.length === 0 && allowedDirectories.length > 0) {
76+
console.error("Error: None of the specified directories are accessible");
77+
process.exit(1);
78+
}
79+
80+
allowedDirectories = accessibleDirectories;
7281

7382
// Initialize the global allowedDirectories in lib.ts
7483
setAllowedDirectories(allowedDirectories);

0 commit comments

Comments
 (0)