diff --git a/clients/web/src/test/integration/mcp/inspectorClient.test.ts b/clients/web/src/test/integration/mcp/inspectorClient.test.ts index 78e25418..f0c85846 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient.test.ts @@ -55,6 +55,7 @@ import type { FetchRequestEntryBase, RequestMetadata, } from "@inspector/core/mcp/types.js"; +import type { InspectorLogger } from "@inspector/core/logging/logger.js"; import type { JsonValue } from "@inspector/core/json/jsonUtils.js"; import type { TypedEvent, @@ -1464,6 +1465,49 @@ describe("InspectorClient", () => { }, ); + it("warns with the rejected progressToken's type, never its value", async () => { + // This logger is persisted by real clients (the TUI writes it to + // `~/.mcp-inspector/auth.log`), and an invalid `progressToken` can now be + // an object holding credentials — so the warning must not echo it. + const warn = vi.fn(); + const noop = () => {}; + const logger: InspectorLogger = { + level: "warn", + fatal: noop, + error: noop, + warn, + info: noop, + debug: noop, + trace: noop, + silent: noop, + child: () => logger, + }; + client = new InspectorClient( + { + type: "stdio", + command: serverCommand.command, + args: serverCommand.args, + }, + { + environment: { transport: createTransportNode, logger }, + progress: false, + defaultMetadata: { + progressToken: { accessToken: "sk-live-should-not-appear" }, + }, + }, + ); + await client.connect(); + await client.listTools(); + + const call = warn.mock.calls.find((c) => + String(c[1]).includes("progressToken"), + ); + expect(call).toBeDefined(); + expect(call![0]).toEqual({ received: "object" }); + // Belt and braces: the secret must not appear anywhere in the record. + expect(JSON.stringify(warn.mock.calls)).not.toContain("sk-live"); + }); + it.each([ ["a string", "tok-1"], ["an integer", 7], diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index ecec24ef..b1d6689f 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -369,6 +369,16 @@ interface ToolCallRequest { // server can't spin a tight zero-delay loop; a successful acknowledgement resets // the count, and past the cap we give up (mark the stream ended) rather than // retry a persistently-failing re-list forever. +/** + * A one-word name for a JSON value's type, for diagnostics that must not + * disclose the value itself. + */ +function describeJsonType(value: unknown): string { + if (value === null) return "null"; + if (Array.isArray(value)) return "array"; + return typeof value; +} + /** * Whether a `_meta.progressToken` value is one the protocol allows. * @@ -1078,8 +1088,13 @@ export class InspectorClient extends InspectorClientEventTarget { // the request stays well-formed and simply asks for no progress. Warn, // because a silently dropped key the user typed is worse than a noisy one. if ("progressToken" in merged && !isProgressToken(merged.progressToken)) { + // Log the rejected value's *type*, never the value. `_meta` now carries + // arbitrary JSON, so an invalid `progressToken` can be an object holding + // credentials — and this logger is persisted by real clients (the TUI + // writes it to `~/.mcp-inspector/auth.log`). The type is what makes the + // warning actionable anyway; the value would only leak. this.logger.warn( - { progressToken: merged.progressToken }, + { received: describeJsonType(merged.progressToken) }, "Dropping `_meta.progressToken` — expected a string or an integer.", ); delete merged.progressToken;