From fa5514a380fade07c64e8479b487274e52f4a741 Mon Sep 17 00:00:00 2001
From: cliffhall <cliff@futurescale.com>
Date: Wed, 1 Jul 2026 19:10:25 -0400
Subject: [PATCH] fix(cli): guard numeric .code to HTTP range; bound
 cause-chain depth

Address @claude review of #1583:

- statusOf() now only treats a numeric `.code` as an HTTP status when it
  is in the 100-599 range, so an MCP SDK McpError JSON-RPC code (e.g.
  -32601 MethodNotFound) is no longer leaked into the envelope `status`
  or misclassified as AUTH_REQUIRED (it falls back to USAGE).
- causeOf() now carries a depth cap (MAX_CAUSE_DEPTH) so a cyclic/
  self-referential `error.cause` chain terminates instead of recursing
  infinitely.
- Add tests for both: a -32601 McpError-shaped error (status undefined,
  exit USAGE) and a self-referential cause chain (finite string).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX
---
 clients/cli/__tests__/error-handler.test.ts | 22 +++++++++++++++++++++
 clients/cli/src/error-handler.ts            | 20 ++++++++++++++-----
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/clients/cli/__tests__/error-handler.test.ts b/clients/cli/__tests__/error-handler.test.ts
index 64ad53e5..fa7bac1d 100644
--- a/clients/cli/__tests__/error-handler.test.ts
+++ b/clients/cli/__tests__/error-handler.test.ts
@@ -159,6 +159,18 @@ describe("classifyError", () => {
     ).toBe("error");
   });
 
+  it("terminates on a self-referential cause chain without recursing infinitely", () => {
+    // A cyclic `error.cause` must not hang the classifier; the depth cap bounds
+    // the walk and returns a finite string.
+    const err = new Error("looping");
+    (err as { cause?: unknown }).cause = err;
+    const { envelope } = classifyError(err);
+    expect(envelope.message).toBe("looping");
+    // Bounded by the depth cap: a finite string, not an infinite loop.
+    expect(typeof envelope.cause).toBe("string");
+    expect(envelope.cause).toContain("looping");
+  });
+
   it("captures a non-Error cause as a string", () => {
     const err = new Error("outer");
     (err as { cause?: unknown }).cause = { reason: "blocked" };
@@ -181,6 +193,16 @@ describe("classifyError", () => {
     expect(envelope.status).toBe(403);
   });
 
+  it("ignores a JSON-RPC McpError numeric .code (outside the HTTP range) as a status", () => {
+    // MCP SDK's McpError carries a numeric `.code` that is a JSON-RPC error
+    // code (e.g. -32601 MethodNotFound), NOT an HTTP status. It must not leak
+    // into `status`, nor get misclassified as AUTH_REQUIRED.
+    const err = Object.assign(new Error("Method not found"), { code: -32601 });
+    const { exitCode, envelope } = classifyError(err);
+    expect(exitCode).toBe(EXIT_CODES.USAGE);
+    expect(envelope.status).toBeUndefined();
+  });
+
   it("ignores a non-numeric node error code when reading the status", () => {
     const err = Object.assign(new Error("getaddrinfo ENOTFOUND host"), {
       code: "ENOTFOUND",
diff --git a/clients/cli/src/error-handler.ts b/clients/cli/src/error-handler.ts
index b6506e9a..5578076c 100644
--- a/clients/cli/src/error-handler.ts
+++ b/clients/cli/src/error-handler.ts
@@ -49,13 +49,18 @@ export class CliExitCodeError extends Error {
   }
 }
 
+/** Depth cap for {@link causeOf} — bounds a pathological/self-referential
+ * `error.cause` chain so a cycle can never recurse infinitely. */
+const MAX_CAUSE_DEPTH = 16;
+
 /** Read an `Error.cause` chain into a single readable string, if present. */
-function causeOf(error: unknown): string | undefined {
+function causeOf(error: unknown, depth = 0): string | undefined {
+  if (depth >= MAX_CAUSE_DEPTH) return undefined;
   if (!(error instanceof Error)) return undefined;
   const cause = (error as { cause?: unknown }).cause;
   if (cause === undefined || cause === null) return undefined;
   if (cause instanceof Error) {
-    const nested = causeOf(cause);
+    const nested = causeOf(cause, depth + 1);
     return nested ? `${cause.message}: ${nested}` : cause.message;
   }
   return String(cause);
@@ -71,9 +76,14 @@ function statusOf(error: unknown): number | undefined {
   };
   if (typeof e.status === "number") return e.status;
   if (typeof e.response?.status === "number") return e.response.status;
-  // SDK SSEError exposes `.code` as the HTTP status; numeric only (avoid
-  // mistaking node error codes like "ENOTFOUND" for a status).
-  if (typeof e.code === "number") return e.code;
+  // SDK SseError / StreamableHTTPError expose `.code` as the HTTP status. Guard
+  // to the HTTP range (100-599) so a JSON-RPC McpError code (e.g. -32601
+  // MethodNotFound) is not mistaken for an HTTP status and leaked into the
+  // envelope or misclassified as AUTH_REQUIRED. String node codes like
+  // "ENOTFOUND" are already excluded by the numeric check.
+  if (typeof e.code === "number" && e.code >= 100 && e.code <= 599) {
+    return e.code;
+  }
   return undefined;
 }