From 01f31cbf2d6c195f82993072d1a284dfc5c139c3 Mon Sep 17 00:00:00 2001 From: Jeff Repanich Date: Sun, 26 Jul 2026 12:50:37 -0400 Subject: [PATCH] fix: bound MCP input before line parsing Closes #2 --- src/mcp.ts | 71 +++++++++++++++++++++++++++++++++++++---------- tests/mcp.test.ts | 18 ++++++++++++ 2 files changed, 74 insertions(+), 15 deletions(-) diff --git a/src/mcp.ts b/src/mcp.ts index 0c633b4..6e3ea84 100644 --- a/src/mcp.ts +++ b/src/mcp.ts @@ -1,7 +1,6 @@ import type { AuthContext } from "@askrjs/auth"; import type { McpServer } from "@askrjs/server/mcp"; import type { Readable, Writable } from "node:stream"; -import { createInterface } from "node:readline"; export interface McpStdioOptions { dependencies: Dependencies; @@ -34,7 +33,6 @@ export function connectMcpStdio( const input = options.input ?? process.stdin; const output = options.output ?? process.stdout; const diagnostics = options.diagnostics ?? process.stderr; - const lines = createInterface({ input, crlfDelay: Infinity, terminal: false }); const controllers = new Map(); const sessionId = crypto.randomUUID(); let finish!: () => void; @@ -49,6 +47,7 @@ export function connectMcpStdio( if (!Number.isInteger(maxConcurrency) || maxConcurrency <= 0) throw new TypeError("MCP maxConcurrency must be a positive integer."); let active = 0; + let cleanupInput = () => undefined; const write = (message: unknown) => new Promise((resolve, reject) => { output.write(`${JSON.stringify(message)}\n`, (error) => (error ? reject(error) : resolve())); @@ -56,7 +55,7 @@ export function connectMcpStdio( const close = async () => { if (ended) return closed; ended = true; - lines.close(); + cleanupInput(); for (const controller of controllers.values()) controller.abort(); controllers.clear(); mcp.terminateSession(sessionId); @@ -64,15 +63,7 @@ export function connectMcpStdio( return closed; }; options.signal?.addEventListener("abort", () => void close(), { once: true }); - lines.on("line", (line) => { - if (Buffer.byteLength(line) > maxLineBytes) { - void write({ - jsonrpc: "2.0", - id: null, - error: { code: -32600, message: "Request line exceeds the configured limit" }, - }).catch(() => void close()); - return; - } + const handleLine = (line: string) => { if (active >= maxConcurrency) { void write({ jsonrpc: "2.0", @@ -135,12 +126,62 @@ export function connectMcpStdio( .finally(() => { active -= 1; }); - }); - lines.once("close", () => { + }; + const lineBuffer = Buffer.allocUnsafe(maxLineBytes); + let lineBytes = 0; + let discardingOversizedLine = false; + const rejectOversizedLine = () => { + void write({ + jsonrpc: "2.0", + id: null, + error: { code: -32600, message: "Request line exceeds the configured limit" }, + }).catch(() => void close()); + }; + const onData = (chunk: string | Buffer | Uint8Array) => { + const bytes = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + let offset = 0; + while (offset < bytes.byteLength) { + const newline = bytes.indexOf(0x0a, offset); + const end = newline < 0 ? bytes.byteLength : newline; + const segmentBytes = end - offset; + if (!discardingOversizedLine) { + if (lineBytes + segmentBytes > maxLineBytes) { + discardingOversizedLine = true; + lineBytes = 0; + rejectOversizedLine(); + } else if (segmentBytes > 0) { + bytes.copy(lineBuffer, lineBytes, offset, end); + lineBytes += segmentBytes; + } + } + if (newline < 0) break; + if (!discardingOversizedLine) { + const length = + lineBytes > 0 && lineBuffer[lineBytes - 1] === 0x0d ? lineBytes - 1 : lineBytes; + handleLine(lineBuffer.subarray(0, length).toString("utf8")); + } + lineBytes = 0; + discardingOversizedLine = false; + offset = newline + 1; + } + }; + const finishInput = () => { if (!ended) { + if (!discardingOversizedLine && lineBytes > 0) { + handleLine(lineBuffer.subarray(0, lineBytes).toString("utf8")); + } ended = true; + cleanupInput(); finish(); } - }); + }; + input.on("data", onData); + input.once("end", finishInput); + input.once("close", finishInput); + cleanupInput = () => { + input.off("data", onData); + input.off("end", finishInput); + input.off("close", finishInput); + }; return { closed, close }; } diff --git a/tests/mcp.test.ts b/tests/mcp.test.ts index af48860..247751b 100644 --- a/tests/mcp.test.ts +++ b/tests/mcp.test.ts @@ -64,6 +64,24 @@ describe("MCP stdio", () => { expect(io.lines[0]).toMatchObject({ id: null, error: { code: -32700 } }); }); + it("should reject oversized input before a newline can be buffered", async () => { + const io = harness(); + const connection = connectMcpStdio(createMcpServer({ name: "stdio", version: "1" }), { + dependencies: undefined, + maxLineBytes: 128, + ...io, + }); + connections.push(connection); + io.input.write("x".repeat(64)); + expect(io.lines).toEqual([]); + io.input.write("x".repeat(65)); + await until(() => io.lines.length === 1); + expect(io.lines[0]).toMatchObject({ + id: null, + error: { code: -32600, message: "Request line exceeds the configured limit" }, + }); + }); + it("should bound line size and concurrent request handling", async () => { const io = harness(); let release: (() => void) | undefined;