From 448ae06e73744fffeb425b004314c644f77f6209 Mon Sep 17 00:00:00 2001 From: Saeed Al Mansouri Date: Thu, 26 Mar 2026 16:10:48 +0400 Subject: [PATCH 1/2] fix(security): resolve symlinks before path validation to prevent directory traversal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a file doesn't exist yet (e.g., write operations), fs.realpath() fails with ENOENT and the code fell back to the unresolved path for the allowedDirectories check. An attacker could create a symlink inside an allowed directory pointing to a restricted location, then write a new file through that symlink — the path appeared to be inside the allowed directory but actually targeted a restricted one. The fix resolves the parent directory (and walks up to the deepest existing ancestor if needed) so symlinks anywhere in the path chain are followed to their real targets before the directory allowlist check runs. Closes #219 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/tools/filesystem.ts | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/tools/filesystem.ts b/src/tools/filesystem.ts index 78c2b2de..2214f530 100644 --- a/src/tools/filesystem.ts +++ b/src/tools/filesystem.ts @@ -202,6 +202,36 @@ export async function validatePath(requestedPath: string): Promise { }); throw new Error(`Failed to resolve symlink for path: ${absoluteOriginal}. Error: ${err.message}`); } + + // SECURITY FIX: When the full path doesn't exist (e.g., writing a new file), + // resolve the parent directory to detect symlinks in the path chain. + // Without this, an attacker could create a symlink inside an allowed directory + // pointing to a restricted location, then write to a non-existent file through + // that symlink — bypassing the directory restriction check. + try { + const parentDir = path.dirname(absoluteOriginal); + const resolvedParent = await fs.realpath(parentDir, { encoding: 'utf8' }); + const basename = path.basename(absoluteOriginal); + resolvedRealPath = path.join(resolvedParent, basename); + } catch { + // Parent also doesn't exist — walk up the tree to find + // the deepest existing ancestor and resolve it + let current = absoluteOriginal; + let remaining: string[] = []; + while (true) { + const parent = path.dirname(current); + if (parent === current) break; // reached filesystem root + remaining.unshift(path.basename(current)); + current = parent; + try { + const resolvedAncestor = await fs.realpath(current, { encoding: 'utf8' }); + resolvedRealPath = path.join(resolvedAncestor, ...remaining); + break; + } catch { + // keep walking up + } + } + } } const pathForNextCheck = resolvedRealPath ?? absoluteOriginal; From 45935e610225f99108552868c845c30ce8944296 Mon Sep 17 00:00:00 2001 From: Saeed Al Mansouri Date: Fri, 27 Mar 2026 12:09:53 +0400 Subject: [PATCH 2/2] fix: serialize notification writes to prevent stdout drain listener accumulation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #391 — during long sessions with heavy tool usage, the Node.js MaxListenersExceededWarning fires on process.stdout because multiple concurrent MCP response writes each register an `once('drain', ...)` listener while the stdout buffer is backed up, exceeding the default limit of 10. Root cause: `sendLogNotification`, `sendProgress`, and `sendCustomNotification` all called `originalStdoutWrite` directly in fire-and-forget fashion, without backpressure handling. Each call could fill the write buffer, causing subsequent SDK `send()` calls to pile up drain listeners. Fix: route all custom notification writes through a serialised write queue (`writeNotificationToStdout` / `flushNotificationQueue`) that registers at most one `once('drain', ...)` listener at a time. Also raise `process.stdout.setMaxListeners(50)` as defence-in-depth to accommodate the SDK's own legitimate concurrent drain listeners during heavy tool usage without false-positive warnings. No API changes. No effect on normal operation when stdout is not full. --- src/custom-stdio.ts | 62 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/src/custom-stdio.ts b/src/custom-stdio.ts index e1c3bb4c..bb5d1de9 100644 --- a/src/custom-stdio.ts +++ b/src/custom-stdio.ts @@ -33,8 +33,18 @@ export class FilteredStdioServerTransport extends StdioServerTransport { private clientName: string = 'unknown'; private disableNotifications: boolean = false; + // Serialised write queue for notification/progress/custom writes. + // Ensures at most one `once('drain', ...)` listener is registered at a time, + // preventing the MaxListenersExceededWarning on process.stdout (#391). + private stdoutQueue: string[] = []; + private stdoutDraining: boolean = false; + constructor() { super(); + + // Allow enough listeners for concurrent MCP response drain waits during + // heavy tool usage, without triggering false-positive MaxListeners warnings. + process.stdout.setMaxListeners(50); // Store original methods this.originalConsole = { @@ -123,6 +133,39 @@ export class FilteredStdioServerTransport extends StdioServerTransport { return this.messageBuffer.length; } + /** + * Write a serialised line to stdout via the notification write queue. + * At most one `once('drain', ...)` listener is registered at any time, + * preventing MaxListenersExceededWarning during heavy notification load. + */ + private writeNotificationToStdout(data: string): void { + this.stdoutQueue.push(data); + if (!this.stdoutDraining) { + this.flushNotificationQueue(); + } + } + + private flushNotificationQueue(): void { + while (this.stdoutQueue.length > 0) { + const data = this.stdoutQueue[0]; + const flushed = this.originalStdoutWrite.call(process.stdout, data); + if (flushed) { + this.stdoutQueue.shift(); + } else { + // stdout buffer is full — wait for drain before writing more. + // Using once() ensures the listener is removed immediately after firing. + if (!this.stdoutDraining) { + this.stdoutDraining = true; + process.stdout.once('drain', () => { + this.stdoutDraining = false; + this.flushNotificationQueue(); + }); + } + return; + } + } + } + private setupConsoleRedirection() { console.log = (...args: any[]) => { if (this.isInitialized) { @@ -258,8 +301,9 @@ export class FilteredStdioServerTransport extends StdioServerTransport { } }; - // Send as valid JSON-RPC notification - this.originalStdoutWrite.call(process.stdout, JSON.stringify(notification) + '\n'); + // Route through the serialised write queue to avoid accumulating + // drain listeners on process.stdout during heavy notification load. + this.writeNotificationToStdout(JSON.stringify(notification) + '\n'); } catch (error) { // Fallback to a simple JSON-RPC error notification if JSON serialization fails const fallbackNotification = { @@ -271,7 +315,7 @@ export class FilteredStdioServerTransport extends StdioServerTransport { data: `Log serialization failed: ${args.join(' ')}` } }; - this.originalStdoutWrite.call(process.stdout, JSON.stringify(fallbackNotification) + '\n'); + this.writeNotificationToStdout(JSON.stringify(fallbackNotification) + '\n'); } } @@ -307,7 +351,7 @@ export class FilteredStdioServerTransport extends StdioServerTransport { } }; - this.originalStdoutWrite.call(process.stdout, JSON.stringify(notification) + '\n'); + this.writeNotificationToStdout(JSON.stringify(notification) + '\n'); } catch (error) { // Fallback to basic JSON-RPC notification const fallbackNotification = { @@ -319,7 +363,7 @@ export class FilteredStdioServerTransport extends StdioServerTransport { data: `sendLog failed: ${message}` } }; - this.originalStdoutWrite.call(process.stdout, JSON.stringify(fallbackNotification) + '\n'); + this.writeNotificationToStdout(JSON.stringify(fallbackNotification) + '\n'); } } @@ -343,7 +387,7 @@ export class FilteredStdioServerTransport extends StdioServerTransport { } }; - this.originalStdoutWrite.call(process.stdout, JSON.stringify(notification) + '\n'); + this.writeNotificationToStdout(JSON.stringify(notification) + '\n'); } catch (error) { // Fallback to basic JSON-RPC notification for progress const fallbackNotification = { @@ -355,7 +399,7 @@ export class FilteredStdioServerTransport extends StdioServerTransport { data: `Progress ${token}: ${value}${total ? `/${total}` : ''}` } }; - this.originalStdoutWrite.call(process.stdout, JSON.stringify(fallbackNotification) + '\n'); + this.writeNotificationToStdout(JSON.stringify(fallbackNotification) + '\n'); } } @@ -375,7 +419,7 @@ export class FilteredStdioServerTransport extends StdioServerTransport { params: params }; - this.originalStdoutWrite.call(process.stdout, JSON.stringify(notification) + '\n'); + this.writeNotificationToStdout(JSON.stringify(notification) + '\n'); } catch (error) { // Fallback to basic JSON-RPC notification for custom notifications const fallbackNotification = { @@ -387,7 +431,7 @@ export class FilteredStdioServerTransport extends StdioServerTransport { data: `Custom notification failed: ${method}: ${JSON.stringify(params)}` } }; - this.originalStdoutWrite.call(process.stdout, JSON.stringify(fallbackNotification) + '\n'); + this.writeNotificationToStdout(JSON.stringify(fallbackNotification) + '\n'); } }