From 16745291e9c200e619ab833654aa170540959a3c Mon Sep 17 00:00:00 2001 From: Implementer 2 Date: Tue, 28 Apr 2026 09:27:34 +0800 Subject: [PATCH] filesystem: add structured startup validation errors --- .../__tests__/startup-validation.test.ts | 77 ++++++++++++++++- src/filesystem/index.ts | 86 +++++++++++++++---- 2 files changed, 146 insertions(+), 17 deletions(-) diff --git a/src/filesystem/__tests__/startup-validation.test.ts b/src/filesystem/__tests__/startup-validation.test.ts index 3be283df74..8801245009 100644 --- a/src/filesystem/__tests__/startup-validation.test.ts +++ b/src/filesystem/__tests__/startup-validation.test.ts @@ -6,6 +6,17 @@ import * as os from 'os'; const SERVER_PATH = path.join(__dirname, '..', 'dist', 'index.js'); +interface StartupValidationErrorPayload { + type: 'startup_validation_error'; + code: 'no_accessible_directories'; + message: string; + rejectedInputs: Array<{ + input: string; + checkedPaths: string[]; + reason: 'inaccessible' | 'not_directory'; + }>; +} + /** * Spawns the filesystem server with given arguments and returns exit info */ @@ -36,6 +47,28 @@ async function spawnServer(args: string[], timeoutMs = 2000): Promise<{ exitCode }); } +function extractStructuredStartupError( + stderr: string, +): StartupValidationErrorPayload | null { + const lines = stderr + .split('\n') + .map((line) => line.trim()) + .filter(Boolean); + + for (const line of lines) { + try { + const parsed = JSON.parse(line); + if (parsed?.type === 'startup_validation_error') { + return parsed as StartupValidationErrorPayload; + } + } catch { + continue; + } + } + + return null; +} + describe('Startup Directory Validation', () => { let testDir: string; let accessibleDir: string; @@ -58,6 +91,7 @@ describe('Startup Directory Validation', () => { // Server starts and runs (we kill it after timeout, so exit code is null or from SIGTERM) expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio'); expect(result.stderr).not.toContain('Error:'); + expect(extractStructuredStartupError(result.stderr)).toBeNull(); }); it('should skip inaccessible directory and continue with accessible one', async () => { @@ -71,9 +105,10 @@ describe('Startup Directory Validation', () => { // Should still start successfully expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio'); + expect(extractStructuredStartupError(result.stderr)).toBeNull(); }); - it('should exit with error when ALL directories are inaccessible', async () => { + it('should emit a structured startup error when ALL directories are inaccessible', async () => { const nonExistent1 = path.join(testDir, 'non-existent-1'); const nonExistent2 = path.join(testDir, 'non-existent-2'); @@ -81,7 +116,24 @@ describe('Startup Directory Validation', () => { // Should exit with error expect(result.exitCode).toBe(1); - expect(result.stderr).toContain('Error: None of the specified directories are accessible'); + const structuredError = extractStructuredStartupError(result.stderr); + expect(structuredError).toMatchObject({ + type: 'startup_validation_error', + code: 'no_accessible_directories', + message: 'None of the specified directories are accessible', + }); + expect(structuredError?.rejectedInputs).toEqual([ + { + input: nonExistent1, + checkedPaths: [nonExistent1], + reason: 'inaccessible', + }, + { + input: nonExistent2, + checkedPaths: [nonExistent2], + reason: 'inaccessible', + }, + ]); }); it('should warn when path is not a directory', async () => { @@ -96,5 +148,26 @@ describe('Startup Directory Validation', () => { // Should still start with the valid directory expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio'); + expect(extractStructuredStartupError(result.stderr)).toBeNull(); + }); + + it('should classify non-directory fatal inputs in the structured error payload', async () => { + const filePath = path.join(testDir, 'not-a-directory-fatal.txt'); + await fs.writeFile(filePath, 'content'); + + const result = await spawnServer([filePath]); + + expect(result.exitCode).toBe(1); + const structuredError = extractStructuredStartupError(result.stderr); + expect(structuredError).toMatchObject({ + type: 'startup_validation_error', + code: 'no_accessible_directories', + }); + expect(structuredError?.rejectedInputs).toHaveLength(1); + expect(structuredError?.rejectedInputs[0]).toMatchObject({ + input: filePath, + reason: 'not_directory', + }); + expect(structuredError?.rejectedInputs[0]?.checkedPaths).toContain(filePath); }); }); diff --git a/src/filesystem/index.ts b/src/filesystem/index.ts index 7b67e63e58..5e60cacd8b 100644 --- a/src/filesystem/index.ts +++ b/src/filesystem/index.ts @@ -28,6 +28,27 @@ import { setAllowedDirectories, } from './lib.js'; +type StartupValidationFailureReason = "inaccessible" | "not_directory"; + +interface StartupValidationFailure { + input: string; + checkedPaths: string[]; + reason: StartupValidationFailureReason; +} + +interface StartupValidationErrorPayload { + type: "startup_validation_error"; + code: "no_accessible_directories"; + message: string; + rejectedInputs: StartupValidationFailure[]; +} + +function emitStartupValidationError( + payload: StartupValidationErrorPayload, +): void { + console.error(JSON.stringify(payload)); +} + // Command line argument parsing const args = process.argv.slice(2); if (args.length === 0) { @@ -42,7 +63,7 @@ if (args.length === 0) { // We store BOTH the original path AND the resolved path to handle symlinks correctly // This fixes the macOS /tmp -> /private/tmp symlink issue where users specify /tmp // but the resolved path is /private/tmp -let allowedDirectories = (await Promise.all( +const allowedDirectoryInputs = await Promise.all( args.map(async (dir) => { const expanded = expandHome(dir); const absolute = path.resolve(expanded); @@ -55,35 +76,70 @@ let allowedDirectories = (await Promise.all( // Return both original and resolved paths if they differ // This allows matching against either /tmp or /private/tmp on macOS if (normalizedOriginal !== normalizedResolved) { - return [normalizedOriginal, normalizedResolved]; + return { + input: dir, + checkedPaths: [normalizedOriginal, normalizedResolved], + }; } - return [normalizedResolved]; + return { + input: dir, + checkedPaths: [normalizedResolved], + }; } catch (error) { // If we can't resolve (doesn't exist), use the normalized absolute path // This allows configuring allowed dirs that will be created later - return [normalizedOriginal]; + return { + input: dir, + checkedPaths: [normalizedOriginal], + }; } }) -)).flat(); +); + +let allowedDirectories = allowedDirectoryInputs.flatMap( + ({ checkedPaths }) => checkedPaths, +); // Filter to only accessible directories, warn about inaccessible ones const accessibleDirectories: string[] = []; -for (const dir of allowedDirectories) { - try { - const stats = await fs.stat(dir); - if (stats.isDirectory()) { - accessibleDirectories.push(dir); - } else { - console.error(`Warning: ${dir} is not a directory, skipping`); +const rejectedInputs: StartupValidationFailure[] = []; +for (const { input, checkedPaths } of allowedDirectoryInputs) { + const validPaths: string[] = []; + let rejectionReason: StartupValidationFailureReason = "inaccessible"; + + for (const candidatePath of checkedPaths) { + try { + const stats = await fs.stat(candidatePath); + if (stats.isDirectory()) { + validPaths.push(candidatePath); + } else { + rejectionReason = "not_directory"; + console.error(`Warning: ${candidatePath} is not a directory, skipping`); + } + } catch (error) { + console.error(`Warning: Cannot access directory ${candidatePath}, skipping`); } - } catch (error) { - console.error(`Warning: Cannot access directory ${dir}, skipping`); + } + + if (validPaths.length > 0) { + accessibleDirectories.push(...validPaths); + } else { + rejectedInputs.push({ + input, + checkedPaths, + reason: rejectionReason, + }); } } // Exit only if ALL paths are inaccessible (and some were specified) if (accessibleDirectories.length === 0 && allowedDirectories.length > 0) { - console.error("Error: None of the specified directories are accessible"); + emitStartupValidationError({ + type: "startup_validation_error", + code: "no_accessible_directories", + message: "None of the specified directories are accessible", + rejectedInputs, + }); process.exit(1); }