From 64007e7251a3ed1d0cccc2a51ef3d0c83ebb5ec3 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 24 Sep 2026 01:49:20 -0400 Subject: [PATCH 1/3] fix(cli): redact URL query secrets in the error envelope (#2423) classifyError now runs the finished envelope through core's redactUrlQuery: the url field, plus every http(s) URL embedded in message and cause. Classification still reads the unredacted text. Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: cliffhall --- clients/cli/__tests__/error-handler.test.ts | 72 +++++++++++++++++++++ clients/cli/src/error-handler.ts | 59 ++++++++++++++++- 2 files changed, 130 insertions(+), 1 deletion(-) diff --git a/clients/cli/__tests__/error-handler.test.ts b/clients/cli/__tests__/error-handler.test.ts index 28b83ce8d8..8c286db86a 100644 --- a/clients/cli/__tests__/error-handler.test.ts +++ b/clients/cli/__tests__/error-handler.test.ts @@ -295,3 +295,75 @@ describe("formatErrorOutput", () => { expect(parsed.error.url).toBe("https://ctx.example/mcp"); }); }); + +/** + * #2423: the envelope is written verbatim to stderr, so every field that can + * carry a URL gets the same query redaction the web client's Network log does. + */ +describe("envelope URL redaction", () => { + const SECRET_URL = + "https://srv.example/mcp?code=abc123&access_token=tok456&tenant=acme"; + const REDACTED_URL = + "https://srv.example/mcp?code=%5BREDACTED%5D&access_token=%5BREDACTED%5D&tenant=acme"; + + it("redacts sensitive query params in the context url", () => { + const { envelope } = classifyError(new Error("nope"), { url: SECRET_URL }); + expect(envelope.url).toBe(REDACTED_URL); + }); + + it("redacts sensitive query params in a CliExitCodeError's own url", () => { + const { envelope } = classifyError( + new CliExitCodeError(EXIT_CODES.AUTH_REQUIRED, "login", { + url: SECRET_URL, + }), + ); + expect(envelope.url).toBe(REDACTED_URL); + }); + + it("redacts a URL embedded in the message, keeping trailing punctuation", () => { + const { envelope } = classifyError( + new Error(`Request to ${SECRET_URL}. Retry later`), + ); + expect(envelope.message).toBe(`Request to ${REDACTED_URL}. Retry later`); + expect(envelope.message).not.toContain("abc123"); + expect(envelope.message).not.toContain("tok456"); + }); + + it("redacts a URL embedded in the cause chain", () => { + const { envelope } = classifyError( + new Error("fetch failed", { + cause: new Error(`connect ECONNREFUSED (${SECRET_URL})`), + }), + ); + expect(envelope.cause).toBe(`connect ECONNREFUSED (${REDACTED_URL})`); + }); + + it("classifies on the unredacted text", () => { + // The only auth signal is inside a parameter value that redaction + // replaces; classifying the redacted copy would fall through to USAGE. + const { exitCode, envelope } = classifyError( + new Error("Rejected https://srv.example/cb?token=invalid_token"), + ); + expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED); + expect(envelope.message).toBe( + "Rejected https://srv.example/cb?token=%5BREDACTED%5D", + ); + }); + + it("leaves text and URLs without sensitive params untouched", () => { + const text = "Failed at https://srv.example/mcp?tenant=acme and nowhere"; + const { envelope } = classifyError(new Error(text), { + url: "https://srv.example/mcp", + }); + expect(envelope.message).toBe(text); + expect(envelope.url).toBe("https://srv.example/mcp"); + }); + + it("keeps the redaction in the serialized stderr line", () => { + const { stderr } = formatErrorOutput(new Error(`boom ${SECRET_URL}`), { + url: SECRET_URL, + }); + expect(stderr).not.toContain("abc123"); + expect(stderr).not.toContain("tok456"); + }); +}); diff --git a/clients/cli/src/error-handler.ts b/clients/cli/src/error-handler.ts index 91cf247937..a5b3d0f97b 100644 --- a/clients/cli/src/error-handler.ts +++ b/clients/cli/src/error-handler.ts @@ -1,3 +1,4 @@ +import { redactUrlQuery } from "@inspector/core/mcp/fetchTracking.js"; import { awaitableError } from "./utils/awaitable-log.js"; /** @@ -138,14 +139,70 @@ function statusOf(error: unknown): number | undefined { const UNREACHABLE_PATTERN = /ENOTFOUND|ECONNREFUSED|ECONNRESET|EAI_AGAIN|ETIMEDOUT|fetch failed|getaddrinfo|connect(?:ion)? timed out|aborted/i; +/** + * An `http(s)://` URL embedded in free text. Stops at whitespace and at the + * quote/bracket characters that commonly delimit a URL inside a message. + */ +const EMBEDDED_URL_PATTERN = /\bhttps?:\/\/[^\s"'<>]+/g; + +/** Sentence punctuation a message may put right after a URL. */ +const TRAILING_PUNCTUATION = /[.,;:!?)\]]+$/; + +/** + * Apply {@link redactUrlQuery} to every URL embedded in `text`. Trailing + * sentence punctuation is split off first and re-appended, so a URL ending a + * sentence (`…?code=abc.`) keeps its full stop instead of having it folded into + * the redacted parameter value. + */ +function redactUrlsInText(text: string): string { + return text.replace(EMBEDDED_URL_PATTERN, (match) => { + const trailing = TRAILING_PUNCTUATION.exec(match)?.[0] ?? ""; + const url = match.slice(0, match.length - trailing.length); + return redactUrlQuery(url) + trailing; + }); +} + +/** + * Scrub query-string secrets out of every envelope field that can carry a URL + * (#2423). The envelope is written verbatim to stderr — a terminal, a CI log, + * a pipe into another tool — so it gets the same {@link redactUrlQuery} + * guarantee the web client's Network log and `OAuthRequestTimeoutError` + * already have: an OAuth `code`, `access_token` or `client_secret` in a server + * URL is replaced, while the path and non-sensitive parameters stay readable. + * + * Applied to the finished envelope rather than to the inputs, so + * classification still reads the error's own text: redaction rewrites + * parameter values, and a pattern test run on the rewritten copy could land a + * different exit code. + */ +function redactEnvelope(envelope: ErrorEnvelope): ErrorEnvelope { + return { + ...envelope, + message: redactUrlsInText(envelope.message), + ...(envelope.cause !== undefined && { + cause: redactUrlsInText(envelope.cause), + }), + ...(envelope.url !== undefined && { url: redactUrlQuery(envelope.url) }), + }; +} + /** * Classify an arbitrary error into an exit code and envelope. Used both by the * binary's {@link handleError} and by callers that want to throw a - * {@link CliExitCodeError} with the right code up front. + * {@link CliExitCodeError} with the right code up front. Every URL in the + * returned envelope is query-redacted (see {@link redactEnvelope}). */ export function classifyError( error: unknown, context?: { url?: string }, +): { exitCode: number; envelope: ErrorEnvelope } { + const { exitCode, envelope } = classifyUnredacted(error, context); + return { exitCode, envelope: redactEnvelope(envelope) }; +} + +function classifyUnredacted( + error: unknown, + context?: { url?: string }, ): { exitCode: number; envelope: ErrorEnvelope } { const message = error instanceof Error From d5de6ee78940737c6d84dd146aaf51136e3c18de Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 24 Sep 2026 02:01:12 -0400 Subject: [PATCH 2/3] fix(cli): match URL schemes case-insensitively when redacting (#2488 review) Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: cliffhall --- clients/cli/__tests__/error-handler.test.ts | 11 +++++++++++ clients/cli/src/error-handler.ts | 4 +++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/clients/cli/__tests__/error-handler.test.ts b/clients/cli/__tests__/error-handler.test.ts index 8c286db86a..f16e226426 100644 --- a/clients/cli/__tests__/error-handler.test.ts +++ b/clients/cli/__tests__/error-handler.test.ts @@ -329,6 +329,17 @@ describe("envelope URL redaction", () => { expect(envelope.message).not.toContain("tok456"); }); + it("redacts a URL whose scheme is upper- or mixed-case", () => { + const { envelope } = classifyError( + new Error( + "a HTTP://srv.example/cb?access_token=tok456 b HttpS://srv.example/cb?code=abc123", + ), + ); + expect(envelope.message).toBe( + "a HTTP://srv.example/cb?access_token=%5BREDACTED%5D b HttpS://srv.example/cb?code=%5BREDACTED%5D", + ); + }); + it("redacts a URL embedded in the cause chain", () => { const { envelope } = classifyError( new Error("fetch failed", { diff --git a/clients/cli/src/error-handler.ts b/clients/cli/src/error-handler.ts index a5b3d0f97b..c12e52ecde 100644 --- a/clients/cli/src/error-handler.ts +++ b/clients/cli/src/error-handler.ts @@ -142,8 +142,10 @@ const UNREACHABLE_PATTERN = /** * An `http(s)://` URL embedded in free text. Stops at whitespace and at the * quote/bracket characters that commonly delimit a URL inside a message. + * Case-insensitive because URI schemes are: `HTTPS://…?code=…` is the same + * URL and must not slip past the redaction (Copilot). */ -const EMBEDDED_URL_PATTERN = /\bhttps?:\/\/[^\s"'<>]+/g; +const EMBEDDED_URL_PATTERN = /\bhttps?:\/\/[^\s"'<>]+/gi; /** Sentence punctuation a message may put right after a URL. */ const TRAILING_PUNCTUATION = /[.,;:!?)\]]+$/; From 82adeee516a8ce46d7eec954fda18ed98d4ad5e1 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 24 Sep 2026 02:17:07 -0400 Subject: [PATCH 3/3] fix(cli): split comma-joined URLs and redact through apostrophes (#2488 review) Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: cliffhall --- clients/cli/__tests__/error-handler.test.ts | 29 +++++++++++++++++++++ clients/cli/src/error-handler.ts | 15 +++++++---- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/clients/cli/__tests__/error-handler.test.ts b/clients/cli/__tests__/error-handler.test.ts index f16e226426..740f44aea5 100644 --- a/clients/cli/__tests__/error-handler.test.ts +++ b/clients/cli/__tests__/error-handler.test.ts @@ -340,6 +340,35 @@ describe("envelope URL redaction", () => { ); }); + it("redacts each of two comma-joined URLs separately", () => { + const { envelope } = classifyError( + new Error( + "https://one.example/cb?state=ok,https://two.example/cb?code=secret", + ), + ); + expect(envelope.message).toBe( + "https://one.example/cb?state=ok,https://two.example/cb?code=%5BREDACTED%5D", + ); + }); + + it("redacts through an apostrophe inside a query value", () => { + const { envelope } = classifyError( + new Error("at https://srv.example/cb?code=abc'def now"), + ); + expect(envelope.message).toBe( + "at https://srv.example/cb?code=%5BREDACTED%5D now", + ); + }); + + it("keeps the closing quote of a single-quoted URL", () => { + const { envelope } = classifyError( + new Error("at 'https://srv.example/cb?code=abc123'."), + ); + expect(envelope.message).toBe( + "at 'https://srv.example/cb?code=%5BREDACTED%5D'.", + ); + }); + it("redacts a URL embedded in the cause chain", () => { const { envelope } = classifyError( new Error("fetch failed", { diff --git a/clients/cli/src/error-handler.ts b/clients/cli/src/error-handler.ts index c12e52ecde..9a1d286208 100644 --- a/clients/cli/src/error-handler.ts +++ b/clients/cli/src/error-handler.ts @@ -140,15 +140,20 @@ const UNREACHABLE_PATTERN = /ENOTFOUND|ECONNREFUSED|ECONNRESET|EAI_AGAIN|ETIMEDOUT|fetch failed|getaddrinfo|connect(?:ion)? timed out|aborted/i; /** - * An `http(s)://` URL embedded in free text. Stops at whitespace and at the - * quote/bracket characters that commonly delimit a URL inside a message. + * An `http(s)://` URL embedded in free text. Stops at whitespace, at the + * double-quote/angle-bracket characters that commonly delimit a URL inside a + * message, and where a second `http(s)://` begins — so two URLs joined by a + * comma are redacted separately rather than the second one's query being read + * as part of the first one's last value (Copilot). An apostrophe is kept in the + * match because it is legal inside a query value; a *trailing* one is peeled + * off as punctuation below, which still handles a `'…'`-quoted URL. * Case-insensitive because URI schemes are: `HTTPS://…?code=…` is the same * URL and must not slip past the redaction (Copilot). */ -const EMBEDDED_URL_PATTERN = /\bhttps?:\/\/[^\s"'<>]+/gi; +const EMBEDDED_URL_PATTERN = /\bhttps?:\/\/(?:(?!https?:\/\/)[^\s"<>])+/gi; -/** Sentence punctuation a message may put right after a URL. */ -const TRAILING_PUNCTUATION = /[.,;:!?)\]]+$/; +/** Sentence punctuation (or a closing quote) a message may put right after a URL. */ +const TRAILING_PUNCTUATION = /[.,;:!?)\]']+$/; /** * Apply {@link redactUrlQuery} to every URL embedded in `text`. Trailing