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; }