fix(agents): reject invalid bundled LSP framing (#105460)

* fix(agents): reject invalid bundled LSP framing * fix(agents): reject malformed LSP header lines Signed-off-by: sallyom <somalley@redhat.com> --------- Signed-off-by: sallyom <somalley@redhat.com> Co-authored-by: sallyom <somalley@redhat.com>
2026-07-14 00:16:05 +00:00 · 2026-07-13 23:43:06 +08:00
parent 2ac723282c
commit 417543b481
2 changed files with 198 additions and 32 deletions
--- a/src/agents/agent-bundle-lsp-runtime.test.ts
+++ b/src/agents/agent-bundle-lsp-runtime.test.ts
@@ -51,6 +51,10 @@ class MockChildProcess extends EventEmitter {
  constructor(
    private readonly initializeResponsePrefix = "",
    private readonly respondMethods?: ReadonlySet<string>,
+    private readonly frameResponse: (
+      body: Record<string, unknown>,
+      method: string,
+    ) => string | readonly string[] = encodeLspMessage,
  ) {
    super();
    this.stdin = new Writable({
@@ -78,11 +82,12 @@ class MockChildProcess extends EventEmitter {
    if (typeof body.id !== "number" || typeof body.method !== "string") {
      return;
    }
-    if (this.respondMethods && !this.respondMethods.has(body.method)) {
+    const method = body.method;
+    if (this.respondMethods && !this.respondMethods.has(method)) {
      return;
    }
    const result =
-      body.method === "initialize"
+      method === "initialize"
        ? {
            capabilities: {
              hoverProvider: true,
@@ -92,9 +97,12 @@ class MockChildProcess extends EventEmitter {
          }
        : null;
    queueMicrotask(() => {
-      this.stdout.write(
-        `${this.initializeResponsePrefix}${encodeLspMessage({ jsonrpc: "2.0", id: body.id, result })}`,
-      );
+      const response = { jsonrpc: "2.0", id: body.id, result };
+      const frame = this.frameResponse(response, method);
+      const chunks = typeof frame === "string" ? [frame] : frame;
+      for (const [index, chunk] of chunks.entries()) {
+        this.stdout.write(`${index === 0 ? this.initializeResponsePrefix : ""}${chunk}`);
+      }
    });
  }
 }
@@ -338,6 +346,108 @@ describe("bundle LSP runtime", () => {
    await runtime.dispose();
  });

+  it("accepts a Content-Type header alongside Content-Length", async () => {
+    configureSingleLspServer();
+    const child = new MockChildProcess("", undefined, (body) => {
+      const json = JSON.stringify(body);
+      return `Content-Type: application/vscode-jsonrpc; charset=utf-8\r\nContent-Length: ${Buffer.byteLength(json, "utf-8")}\r\n\r\n${json}`;
+    });
+    spawnMock.mockReturnValue(child);
+    const { createBundleLspToolRuntime } = await import("./agent-bundle-lsp-runtime.js");
+
+    const runtime = await createBundleLspToolRuntime({ workspaceDir: "/tmp/workspace" });
+
+    expect(runtime.tools.map((tool) => tool.name)).toContain("lsp_hover_typescript");
+    await runtime.dispose();
+  });
+
+  it("accepts a maximum-size header when its separator is split across chunks", async () => {
+    configureSingleLspServer();
+    const child = new MockChildProcess("", undefined, (body) => {
+      const json = JSON.stringify(body);
+      const headerPrefix = `Content-Length: ${Buffer.byteLength(json, "utf-8")}\r\nX-Padding: `;
+      const header = `${headerPrefix}${"x".repeat(8 * 1024 - headerPrefix.length)}`;
+      const frame = `${header}\r\n\r\n${json}`;
+      return [frame.slice(0, header.length + 1), frame.slice(header.length + 1)];
+    });
+    spawnMock.mockReturnValue(child);
+    const { createBundleLspToolRuntime } = await import("./agent-bundle-lsp-runtime.js");
+
+    const runtime = await createBundleLspToolRuntime({ workspaceDir: "/tmp/workspace" });
+
+    expect(runtime.tools.map((tool) => tool.name)).toContain("lsp_hover_typescript");
+    await runtime.dispose();
+  });
+
+  it.each([
+    {
+      name: "a suffixed Content-Length value",
+      frame: (body: Record<string, unknown>) => {
+        const json = JSON.stringify(body);
+        return `Content-Length: ${Buffer.byteLength(json, "utf-8")}junk\r\n\r\n${json}`;
+      },
+    },
+    {
+      name: "duplicate Content-Length fields",
+      frame: (body: Record<string, unknown>) => {
+        const json = JSON.stringify(body);
+        const length = Buffer.byteLength(json, "utf-8");
+        return `Content-Length: ${length}\r\nContent-Length: ${length}\r\n\r\n${json}`;
+      },
+    },
+    {
+      name: "a colonless header line",
+      frame: (body: Record<string, unknown>) => {
+        const json = JSON.stringify(body);
+        const length = Buffer.byteLength(json, "utf-8");
+        return `Content-Length: ${length}\r\nbroken\r\n\r\n${json}`;
+      },
+    },
+    {
+      name: "an oversized declared body",
+      frame: () => `Content-Length: ${64 * 1024 * 1024 + 1}\r\n\r\n`,
+    },
+    {
+      name: "an oversized unterminated header",
+      frame: () => `X-Header: ${"x".repeat(8 * 1024)}`,
+    },
+  ])("fails the LSP session immediately for $name", async ({ frame }) => {
+    configureSingleLspServer();
+    const child = new MockChildProcess(
+      "",
+      new Set(["initialize", "textDocument/hover"]),
+      (body, method) => (method === "initialize" ? encodeLspMessage(body) : frame(body)),
+    );
+    spawnMock.mockReturnValue(child);
+    const { createBundleLspToolRuntime } = await import("./agent-bundle-lsp-runtime.js");
+
+    const runtime = await createBundleLspToolRuntime({ workspaceDir: "/tmp/workspace" });
+    const hoverTool = runtime.tools.find((tool) => tool.name === "lsp_hover_typescript");
+    if (!hoverTool) {
+      throw new Error("expected hover tool");
+    }
+
+    const request = hoverTool.execute("call-1", {
+      uri: "file:///tmp/workspace/index.ts",
+      line: 0,
+      character: 0,
+    });
+    const outcome = await Promise.race([
+      request.then(
+        () => "resolved",
+        (error: unknown) => (error instanceof Error ? error.message : String(error)),
+      ),
+      new Promise<string>((resolve) => {
+        setTimeout(() => resolve("still pending"), 100);
+      }),
+    ]);
+
+    expect(outcome).toMatch(/LSP framing error/i);
+    expect(killProcessTreeMock).toHaveBeenCalledWith(4321, { graceMs: 1000 });
+
+    await runtime.dispose();
+  });
+
  it("disposes active LSP sessions from the global shutdown sweep", async () => {
    configureSingleLspServer();
    const child = new MockChildProcess();
--- a/src/agents/agent-bundle-lsp-runtime.ts
+++ b/src/agents/agent-bundle-lsp-runtime.ts
@@ -179,47 +179,95 @@ function encodeLspMessage(body: unknown): string {
  return `Content-Length: ${Buffer.byteLength(json, "utf-8")}\r\n\r\n${json}`;
 }

-function parseLspMessages(buffer: Buffer): { messages: unknown[]; remaining: Buffer } {
+const LSP_HEADER_SEPARATOR = Buffer.from("\r\n\r\n", "ascii");
+const MAX_LSP_HEADER_BYTES = 8 * 1024;
+const MAX_LSP_BODY_BYTES = 64 * 1024 * 1024;
+
+class LspFramingError extends Error {
+  override readonly name = "LspFramingError";
+}
+
+type LspParseResult =
+  | { readonly ok: true; readonly messages: unknown[]; readonly remaining: Buffer }
+  | { readonly ok: false; readonly messages: unknown[]; readonly error: LspFramingError };
+
+function framingError(messages: unknown[], detail: string): LspParseResult {
+  return {
+    ok: false,
+    messages,
+    error: new LspFramingError(`LSP framing error: ${detail}`),
+  };
+}
+
+function parseContentLength(header: string): number | LspFramingError {
+  const values: string[] = [];
+  for (const line of header.split("\r\n")) {
+    const separator = line.indexOf(":");
+    if (separator === -1) {
+      return new LspFramingError("LSP framing error: header line must contain a colon");
+    }
+    if (line.slice(0, separator).trim().toLowerCase() === "content-length") {
+      values.push(line.slice(separator + 1).trim());
+    }
+  }
+  if (values.length !== 1) {
+    return new LspFramingError(
+      `LSP framing error: expected exactly one Content-Length header, received ${values.length}`,
+    );
+  }
+  const value = values[0];
+  if (value === undefined || !/^[0-9]+$/.test(value)) {
+    return new LspFramingError("LSP framing error: Content-Length must be decimal digits");
+  }
+  const length = Number(value);
+  if (!Number.isSafeInteger(length) || length <= 0) {
+    return new LspFramingError("LSP framing error: Content-Length must be a positive safe integer");
+  }
+  if (length > MAX_LSP_BODY_BYTES) {
+    return new LspFramingError(
+      `LSP framing error: Content-Length exceeds ${MAX_LSP_BODY_BYTES} bytes`,
+    );
+  }
+  return length;
+}
+
+function parseLspMessages(buffer: Buffer): LspParseResult {
  const messages: unknown[] = [];
  let remaining = buffer;
-  const headerSeparator = Buffer.from("\r\n\r\n", "ascii");

  while (true) {
-    const headerEnd = remaining.indexOf(headerSeparator);
+    const headerEnd = remaining.indexOf(LSP_HEADER_SEPARATOR);
    if (headerEnd === -1) {
-      break;
+      const maxIncompleteHeaderBytes = MAX_LSP_HEADER_BYTES + LSP_HEADER_SEPARATOR.length - 1;
+      return remaining.length > maxIncompleteHeaderBytes
+        ? framingError(messages, `header exceeds ${MAX_LSP_HEADER_BYTES} bytes`)
+        : { ok: true, messages, remaining };
+    }
+    if (headerEnd > MAX_LSP_HEADER_BYTES) {
+      return framingError(messages, `header exceeds ${MAX_LSP_HEADER_BYTES} bytes`);
    }

-    const header = remaining.subarray(0, headerEnd).toString("ascii");
-    const match = header.match(/Content-Length:\s*(\d+)/i);
-    if (!match) {
-      remaining = remaining.subarray(headerEnd + headerSeparator.length);
-      continue;
+    const contentLength = parseContentLength(remaining.subarray(0, headerEnd).toString("ascii"));
+    if (contentLength instanceof LspFramingError) {
+      return { ok: false, messages, error: contentLength };
    }
-
-    const contentLengthText = match.at(1);
-    if (contentLengthText === undefined) {
-      remaining = remaining.subarray(headerEnd + headerSeparator.length);
-      continue;
-    }
-    const contentLength = Number.parseInt(contentLengthText, 10);
-    const bodyStart = headerEnd + headerSeparator.length;
+    const bodyStart = headerEnd + LSP_HEADER_SEPARATOR.length;
    const bodyEnd = bodyStart + contentLength;
-
    if (remaining.length < bodyEnd) {
-      break;
+      return { ok: true, messages, remaining };
    }

+    const body = remaining.subarray(bodyStart, bodyEnd).toString("utf8");
    try {
-      const body = remaining.subarray(bodyStart, bodyEnd).toString("utf8");
      messages.push(JSON.parse(body));
-    } catch {
-      // skip malformed
+    } catch (error) {
+      return framingError(
+        messages,
+        `body is not valid JSON: ${error instanceof Error ? error.message : String(error)}`,
+      );
    }
    remaining = remaining.subarray(bodyEnd);
  }
-
-  return { messages, remaining };
 }

 function lspAbortError(signal?: AbortSignal): Error {
@@ -276,10 +324,14 @@ function handleIncomingData(session: LspSession, chunk: Buffer | string) {
    session.buffer,
    typeof chunk === "string" ? Buffer.from(chunk, "utf8") : chunk,
  ]);
-  const { messages, remaining } = parseLspMessages(session.buffer);
-  session.buffer = remaining.length === 0 ? Buffer.alloc(0) : Buffer.from(remaining);
+  const parsed = parseLspMessages(session.buffer);
+  session.buffer = parsed.ok
+    ? parsed.remaining.length === 0
+      ? Buffer.alloc(0)
+      : Buffer.from(parsed.remaining)
+    : Buffer.alloc(0);

-  for (const msg of messages) {
+  for (const msg of parsed.messages) {
    if (typeof msg !== "object" || msg === null) {
      continue;
    }
@@ -300,6 +352,10 @@ function handleIncomingData(session: LspSession, chunk: Buffer | string) {
      logDebug(`bundle-lsp:${session.serverName}: notification ${String(record.method)}`);
    }
  }
+  if (!parsed.ok) {
+    failLspSession(session, parsed.error);
+    terminateLspProcessTree(session);
+  }
 }

 async function initializeSession(session: LspSession): Promise<LspServerCapabilities> {