fix(cli): redact URL query secrets in the error envelope

clients/cli/__tests__/error-handler.test.ts

@@ -295,3 +295,115 @@ 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 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 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", {
+        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");
+  });
+});

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,77 @@ 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, 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?:\/\/(?:(?!https?:\/\/)[^\s"<>])+/gi;
+
+/** 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
+ * 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