From 73d7439320a77bca7556171d8ddfe103874db7cd Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 27 Jun 2026 18:47:51 -0400 Subject: [PATCH 01/46] docs: add CONTRIBUTORS.md issues-only policy (#1517) Document the contribution policy: we accept issues, not pull requests. Maintainers handle design and implementation through a prompt-driven workflow. Contributors who have already built a change locally should share the prompt they used rather than a diff. Cross-reference the policy from AGENTS.md. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 7 +++++ CONTRIBUTORS.md | 79 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 CONTRIBUTORS.md diff --git a/AGENTS.md b/AGENTS.md index aeec0408b..81f28bae1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -73,6 +73,13 @@ After installing, `npm run build` builds all clients. The launcher scripts (`npm - v1.5 - https://github.com/orgs/modelcontextprotocol/projects/39 - v1 - https://github.com/orgs/modelcontextprotocol/projects/11 +## Contributing + +External contributions are accepted as **issues, not pull requests** — +maintainers handle design and implementation through a prompt-driven workflow. +If you've already built a change locally, share the **prompt** you used, not a +diff. See [`CONTRIBUTORS.md`](./CONTRIBUTORS.md) for the full policy. + ## Project Status and Direction * The main branch currently contains the legacy version of the Inspector, which we are accepting bug fixes and minor improvement PRs for. diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md new file mode 100644 index 000000000..eebc62983 --- /dev/null +++ b/CONTRIBUTORS.md @@ -0,0 +1,79 @@ +# Contributing to MCP Inspector + +Thank you for your interest in improving the MCP Inspector. Contributors are +genuinely valued — the goal of this document is to channel your input into a +form we can act on quickly and consistently. + +## TL;DR + +**We accept issues, not pull requests.** Design and implementation are done by +the maintainers. If you've already built a fix or feature locally, share **the +prompt you used** to produce it — not the source code. + +## Why this policy exists + +The Inspector v2 is developed with an AI-assisted, prompt-driven workflow built +around a consistent architecture, a shared design system, and strict testing +gates (see [`AGENTS.md`](./AGENTS.md)). Every component follows the same +conventions: "dumb" components that take data and callbacks as props, Mantine +for all UI, theme variants instead of ad-hoc CSS, and a uniform per-file +coverage gate of ≥ 90% on lines, statements, functions, and branches. + +Accepting raw source PRs creates friction: a diff written outside this pipeline +has to be reverse-engineered to fit our component/theme conventions, coverage +gates, and review process — often it's faster to re-derive the change than to +adapt the patch. Capturing your **intent** (a well-formed issue) or the +**prompt** that generated your local change lets us reproduce the work inside +our own workflow and standards, with the quality bar already baked in. + +This policy is about efficiency, not gatekeeping. Your ideas, bug reports, and +prompts directly shape what gets built. + +## How to contribute a bug report or feature request + +Open a well-scoped issue on the appropriate version board. The Inspector +maintains three lines of development, each with its own board and base branch: + +| Target | Board | Base branch | Label | +| --- | --- | --- | --- | +| v2 (current focus) | [v2 board](https://github.com/orgs/modelcontextprotocol/projects/28) | `v2/main` | `v2` | +| v1.5 | [v1.5 board](https://github.com/orgs/modelcontextprotocol/projects/39) | `v1.5/main` | `v1.5` | +| v1 (legacy) | [v1 board](https://github.com/orgs/modelcontextprotocol/projects/11) | `main` | `v1` | + +Most new work targets **v2** — open it on the v2 board and label it `v2`. Apply +the label that matches the target version (`v1` / `v1.5` / `v2`) so the issue +can be filtered correctly; see the "Label by version" convention in +[`AGENTS.md`](./AGENTS.md). + +## If you've already fixed it locally + +Please don't send a diff or open a pull request. Instead, open an issue that +includes: + +- **The prompt(s) you used** to generate the change — the exact text, so we can + reproduce it through our own workflow. +- **A description of the behavior before and after** your change. +- **How you verified it** (steps you ran, tests you added, what you observed). + +We'll reproduce the change through our pipeline so it lands with the right +conventions, tests, and coverage. + +## What makes a good issue or prompt submission + +A great submission gives us everything we need to act without a round-trip: + +- **Clear reproduction or use case** — exact steps to reproduce a bug, or a + concrete description of the feature and the problem it solves. +- **Expected vs. actual behavior** — what you saw, and what you expected + instead. +- **Affected client** — which incarnation is involved: **Web**, **TUI**, or + **CLI** (or "all" / "core" if it's shared logic). +- **Environment details** when relevant — OS, Node version, the MCP server you + were inspecting, and any relevant config. +- **The exact prompt text**, if you generated a local change and want us to + reproduce it. + +## Questions + +If you're unsure which board or label applies, open the issue on the v2 board +and say so — we'll route it. Thanks for helping make the Inspector better. From cc1e707ff05b8be0d3c26434180f9804dfe013b2 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 27 Jun 2026 18:49:27 -0400 Subject: [PATCH 02/46] docs: add PR template redirecting external authors to issues-only policy (#1517) GitHub auto-populates pull_request_template.md into every new PR body, so it steers would-be external PR authors to CONTRIBUTORS.md before they submit a diff. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/pull_request_template.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 .github/pull_request_template.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..3c8167a97 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,28 @@ + + +> **Heads up:** this repository accepts **issues, not pull requests** from +> external contributors. Please read [`CONTRIBUTORS.md`](../blob/v2/main/CONTRIBUTORS.md) +> before continuing. If you're an external contributor, open an issue (and +> share the prompt you used, if you've already built the change) rather than +> this PR. From f7a746c3cd7d1ebd9d726e373d30b53794802d79 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 27 Jun 2026 19:45:08 -0400 Subject: [PATCH 03/46] docs: drop version-board details from CONTRIBUTORS.md (#1517) The policy takes effect after v2/main merges into main, leaving a single line of development. Tell contributors to open a well-formed issue rather than routing them to per-version project boards. Co-Authored-By: Claude Opus 4.8 (1M context) --- CONTRIBUTORS.md | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index eebc62983..530455647 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -31,19 +31,11 @@ prompts directly shape what gets built. ## How to contribute a bug report or feature request -Open a well-scoped issue on the appropriate version board. The Inspector -maintains three lines of development, each with its own board and base branch: - -| Target | Board | Base branch | Label | -| --- | --- | --- | --- | -| v2 (current focus) | [v2 board](https://github.com/orgs/modelcontextprotocol/projects/28) | `v2/main` | `v2` | -| v1.5 | [v1.5 board](https://github.com/orgs/modelcontextprotocol/projects/39) | `v1.5/main` | `v1.5` | -| v1 (legacy) | [v1 board](https://github.com/orgs/modelcontextprotocol/projects/11) | `main` | `v1` | - -Most new work targets **v2** — open it on the v2 board and label it `v2`. Apply -the label that matches the target version (`v1` / `v1.5` / `v2`) so the issue -can be filtered correctly; see the "Label by version" convention in -[`AGENTS.md`](./AGENTS.md). +Open a well-formed issue describing the bug or the feature you have in mind. +A great issue gives us everything we need to act on it without a round-trip — +see [What makes a good issue or prompt submission](#what-makes-a-good-issue-or-prompt-submission) +below. That's the whole process: you describe the intent, we handle the design +and implementation. ## If you've already fixed it locally @@ -75,5 +67,5 @@ A great submission gives us everything we need to act without a round-trip: ## Questions -If you're unsure which board or label applies, open the issue on the v2 board -and say so — we'll route it. Thanks for helping make the Inspector better. +If you're unsure how to scope something, open the issue anyway and say so — +we'll help shape it. Thanks for helping make the Inspector better. From a5d8d80ffc4a3f950f948a43df70529e885c4450 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 1 Jul 2026 18:08:56 -0400 Subject: [PATCH 04/46] feat(cli): exit-code map + structured JSON error envelopes (closes #1564) Give the CLI a stable machine-readable failure surface: - EXIT_CODES map (0 ok / 1 usage / 2 no-app / 3 auth-required / 4 unreachable / 5 tool-error) - ErrorEnvelope interface + CliExitCodeError for pre-classified exits - pure classifyError() (status/cause/pattern heuristics) and formatErrorOutput()/handleError() that emit one JSON line on stderr - in-process cli-runner mirrors the binary via formatErrorOutput so tests observe the real exit code and envelope - document the exit-code map + envelope shape in clients/cli/README.md Wave 1 foundation of the PR #1510 decomposition; shapes kept clean for reuse by --app-info (exit 2), stored-auth (exit 3), and --format json. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX --- clients/cli/README.md | 26 +++ clients/cli/__tests__/error-handler.test.ts | 198 ++++++++++++++++-- clients/cli/__tests__/helpers/cli-runner.ts | 24 +-- clients/cli/src/error-handler.ts | 214 ++++++++++++++++++-- 4 files changed, 420 insertions(+), 42 deletions(-) diff --git a/clients/cli/README.md b/clients/cli/README.md index ce03c465d..85ee31941 100644 --- a/clients/cli/README.md +++ b/clients/cli/README.md @@ -143,6 +143,32 @@ npx @modelcontextprotocol/inspector --cli --catalog mcp.json --server my-http-se See [EMA / enterprise-managed auth](../../specification/v2_auth_ema.md) and [OAuth smoke testing](../../specification/v2_auth_smoke_testing.md) for configuration details and staging servers. +## Exit codes & error envelopes + +Every non-zero exit maps to a stable failure class, so a programmatic caller +(CI, a script, an agent) can branch on _why_ the CLI failed without scraping +prose from stderr: + +| Code | Meaning | +| ---- | ------- | +| `0` | Success. | +| `1` | Usage / unexpected error (the catch-all). | +| `2` | No MCP App found on the tool (`--app-info` probe). | +| `3` | Server requires authentication (401/403, `WWW-Authenticate`, OAuth). | +| `4` | Server unreachable (DNS, connection refused, timeout, `fetch failed`). | +| `5` | Tool error (`tools/call` returned `isError:true`, or the tool was not found). | + +On any non-zero exit the CLI also writes a single JSON line to **stderr** — the +`ErrorEnvelope`: + +```json +{ "error": { "code": "auth_required", "message": "Unauthorized", "status": 401, "url": "https://api.example/mcp" } } +``` + +The `code` is a stable identifier for the failure class; `message` is the +human-readable error; `cause`, `status`, and `url` are included when known. +Because it is one line, a caller can parse it with `2>&1 | tail -1 | jq .error`. + ## Why use the CLI? While the Web Client provides a rich visual interface, the CLI is designed for: diff --git a/clients/cli/__tests__/error-handler.test.ts b/clients/cli/__tests__/error-handler.test.ts index 79d0eab8d..64ad53e58 100644 --- a/clients/cli/__tests__/error-handler.test.ts +++ b/clients/cli/__tests__/error-handler.test.ts @@ -1,5 +1,11 @@ import { describe, it, expect, vi, afterEach } from "vitest"; -import { handleError } from "../src/error-handler.js"; +import { + CliExitCodeError, + EXIT_CODES, + classifyError, + formatErrorOutput, + handleError, +} from "../src/error-handler.js"; /** * `handleError` is the binary's last-resort error sink (wired up in @@ -13,33 +19,195 @@ describe("handleError", () => { vi.restoreAllMocks(); }); - it("logs an Error's message and exits with code 1", () => { - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + it("emits a JSON error envelope on stderr and exits with the classified code", () => { + const writeSpy = vi + .spyOn(process.stderr, "write") + .mockImplementation((() => true) as never); const exitSpy = vi .spyOn(process, "exit") .mockImplementation((() => undefined) as never); handleError(new Error("boom")); - expect(errorSpy).toHaveBeenCalledWith("boom"); - expect(exitSpy).toHaveBeenCalledWith(1); + expect(exitSpy).toHaveBeenCalledWith(EXIT_CODES.USAGE); + const written = writeSpy.mock.calls[0]![0] as string; + const parsed = JSON.parse(written) as { + error: { code: string; message: string }; + }; + expect(parsed.error.code).toBe("error"); + expect(parsed.error.message).toBe("boom"); }); - it("logs a string error verbatim", () => { - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - vi.spyOn(process, "exit").mockImplementation((() => undefined) as never); + it("uses a CliExitCodeError's exitCode and envelope code", () => { + vi.spyOn(process.stderr, "write").mockImplementation((() => true) as never); + const exitSpy = vi + .spyOn(process, "exit") + .mockImplementation((() => undefined) as never); + + handleError( + new CliExitCodeError(EXIT_CODES.NO_APP, "no app", { code: "no_app" }), + ); + + expect(exitSpy).toHaveBeenCalledWith(EXIT_CODES.NO_APP); + }); +}); - handleError("plain failure"); +describe("classifyError", () => { + it("classifies a 401 status as AUTH_REQUIRED", () => { + const err = Object.assign(new Error("Unauthorized"), { status: 401 }); + const { exitCode, envelope } = classifyError(err, { + url: "https://x.example/mcp", + }); + expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED); + expect(envelope.code).toBe("auth_required"); + expect(envelope.status).toBe(401); + expect(envelope.url).toBe("https://x.example/mcp"); + }); + + it("classifies a WWW-Authenticate message as AUTH_REQUIRED without a status", () => { + const { exitCode } = classifyError( + new Error("Dynamic client registration failed: WWW-Authenticate Bearer"), + ); + expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED); + }); - expect(errorSpy).toHaveBeenCalledWith("plain failure"); + it("classifies ENOTFOUND / fetch failed as UNREACHABLE", () => { + const err = new Error("fetch failed"); + (err as { cause?: unknown }).cause = new Error( + "getaddrinfo ENOTFOUND no.such.host", + ); + const { exitCode, envelope } = classifyError(err); + expect(exitCode).toBe(EXIT_CODES.UNREACHABLE); + expect(envelope.cause).toContain("ENOTFOUND"); }); - it("falls back to 'Unknown error' for non-Error, non-string values", () => { - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - vi.spyOn(process, "exit").mockImplementation((() => undefined) as never); + it("classifies a connection-refused cause as UNREACHABLE", () => { + const err = new Error("connect ECONNREFUSED 127.0.0.1:1"); + expect(classifyError(err).exitCode).toBe(EXIT_CODES.UNREACHABLE); + }); + + it("falls back to USAGE for an unrecognized Error", () => { + expect(classifyError(new Error("something else")).exitCode).toBe( + EXIT_CODES.USAGE, + ); + }); + + it("handles a string error", () => { + const { exitCode, envelope } = classifyError("plain failure"); + expect(exitCode).toBe(EXIT_CODES.USAGE); + expect(envelope.message).toBe("plain failure"); + }); + + it("handles a non-Error, non-string value", () => { + const { envelope } = classifyError({ unexpected: true }); + expect(envelope.message).toBe("Unknown error"); + }); + + it("reads an HTTP status from a nested response object", () => { + const err = { response: { status: 401 } }; + const { exitCode, envelope } = classifyError(err); + expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED); + expect(envelope.status).toBe(401); + }); - handleError({ unexpected: true }); + it("preserves a CliExitCodeError's explicit envelope code over the default", () => { + const { envelope } = classifyError( + new CliExitCodeError(EXIT_CODES.TOOL_ERROR, "x", { + code: "tool_not_found", + }), + ); + expect(envelope.code).toBe("tool_not_found"); + }); + + it("carries a CliExitCodeError's explicit status and url onto the envelope", () => { + const { envelope } = classifyError( + new CliExitCodeError(EXIT_CODES.AUTH_REQUIRED, "denied", { + status: 401, + url: "https://api.example/mcp", + }), + ); + expect(envelope.status).toBe(401); + expect(envelope.url).toBe("https://api.example/mcp"); + }); + + it("falls back to the context url for a CliExitCodeError without its own url", () => { + const { envelope } = classifyError( + new CliExitCodeError(EXIT_CODES.UNREACHABLE, "down"), + { url: "https://ctx.example/mcp" }, + ); + expect(envelope.url).toBe("https://ctx.example/mcp"); + }); + + it("derives a default envelope code for a bare CliExitCodeError", () => { + expect( + classifyError(new CliExitCodeError(EXIT_CODES.UNREACHABLE, "x")).envelope + .code, + ).toBe("unreachable"); + expect( + classifyError(new CliExitCodeError(EXIT_CODES.NO_APP, "x")).envelope.code, + ).toBe("no_app"); + expect( + classifyError(new CliExitCodeError(EXIT_CODES.AUTH_REQUIRED, "x")) + .envelope.code, + ).toBe("auth_required"); + expect( + classifyError(new CliExitCodeError(EXIT_CODES.TOOL_ERROR, "x")).envelope + .code, + ).toBe("tool_error"); + expect( + classifyError(new CliExitCodeError(EXIT_CODES.USAGE, "x")).envelope.code, + ).toBe("error"); + }); + + it("captures a non-Error cause as a string", () => { + const err = new Error("outer"); + (err as { cause?: unknown }).cause = { reason: "blocked" }; + const { envelope } = classifyError(err); + expect(envelope.cause).toBe("[object Object]"); + }); + + it("flattens a nested Error cause chain into one string", () => { + const inner = new Error("getaddrinfo ENOTFOUND host"); + const middle = Object.assign(new Error("connect failed"), { cause: inner }); + const outer = Object.assign(new Error("fetch failed"), { cause: middle }); + const { envelope } = classifyError(outer); + expect(envelope.cause).toBe("connect failed: getaddrinfo ENOTFOUND host"); + }); + + it("reads a numeric SDK-style .code as an HTTP status", () => { + const err = Object.assign(new Error("forbidden"), { code: 403 }); + const { exitCode, envelope } = classifyError(err); + expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED); + expect(envelope.status).toBe(403); + }); + + it("ignores a non-numeric node error code when reading the status", () => { + const err = Object.assign(new Error("getaddrinfo ENOTFOUND host"), { + code: "ENOTFOUND", + }); + const { exitCode, envelope } = classifyError(err); + // The message matches the UNREACHABLE pattern, and the string node code + // must NOT be treated as an HTTP status. + expect(exitCode).toBe(EXIT_CODES.UNREACHABLE); + expect(envelope.status).toBeUndefined(); + }); +}); + +describe("formatErrorOutput", () => { + it("emits one JSON line on stderr ending in a newline", () => { + const { stderr, exitCode } = formatErrorOutput(new Error("nope")); + expect(stderr.endsWith("\n")).toBe(true); + expect(stderr.split("\n").length).toBe(2); + const parsed = JSON.parse(stderr) as { error: { message: string } }; + expect(parsed.error.message).toBe("nope"); + expect(exitCode).toBe(EXIT_CODES.USAGE); + }); - expect(errorSpy).toHaveBeenCalledWith("Unknown error"); + it("threads the context url into the envelope", () => { + const { stderr } = formatErrorOutput(new Error("nope"), { + url: "https://ctx.example/mcp", + }); + const parsed = JSON.parse(stderr) as { error: { url?: string } }; + expect(parsed.error.url).toBe("https://ctx.example/mcp"); }); }); diff --git a/clients/cli/__tests__/helpers/cli-runner.ts b/clients/cli/__tests__/helpers/cli-runner.ts index 38b401d14..d5c8abe66 100644 --- a/clients/cli/__tests__/helpers/cli-runner.ts +++ b/clients/cli/__tests__/helpers/cli-runner.ts @@ -1,4 +1,5 @@ import { runCli as invokeCli } from "../../src/cli.js"; +import { formatErrorOutput } from "../../src/error-handler.js"; export interface CliResult { exitCode: number | null; @@ -52,7 +53,8 @@ function captureWrite(append: (text: string) => void) { * (`exitCode` / `stdout` / `stderr` / `output`) so every existing test and the * shared assertion helpers keep working unchanged: * - stdout/stderr are captured by temporarily patching `process.std*.write` - * (and `console.error`/`console.warn`, which commander and error paths use). + * (which also captures `console.error`/`console.warn`, since those route + * through `process.stderr.write`). * - A thrown error (bad args, failed connect, unsupported method) maps to * `exitCode: 1` with the message appended to stderr — mirroring how the real * binary (`index.ts`) routes errors through `handleError` → `process.exit(1)`. @@ -77,8 +79,6 @@ export async function runCli( const originalStdoutWrite = process.stdout.write; const originalStderrWrite = process.stderr.write; - const originalConsoleError = console.error; - const originalConsoleWarn = console.warn; // Snapshot and apply env overrides; `undefined` records keys that did not // exist so they can be deleted (not set to the string "undefined") on restore. @@ -93,15 +93,12 @@ export async function runCli( process.stdout.write = captureWrite((text) => { stdout += text; }) as typeof process.stdout.write; + // `console.error` / `console.warn` route through `process.stderr.write`, so + // patching the underlying stream captures them too — no separate console + // override is needed (the previous explicit overrides were redundant). process.stderr.write = captureWrite((text) => { stderr += text; }) as typeof process.stderr.write; - console.error = (...parts: unknown[]) => { - stderr += parts.map(String).join(" ") + "\n"; - }; - console.warn = (...parts: unknown[]) => { - stderr += parts.map(String).join(" ") + "\n"; - }; // `runCli` reads `process.argv.slice(2)`, so prepend two placeholder entries // ([node, script]) to mirror how the binary is launched. @@ -127,14 +124,15 @@ export async function runCli( try { await Promise.race([invokeCli(argv), timeout]); } catch (error) { - exitCode = 1; - stderr += (error instanceof Error ? error.message : String(error)) + "\n"; + // Mirror the binary's `handleError` exactly so tests observe the same exit + // code and stderr (one JSON envelope line) the real CLI would emit. + const out = formatErrorOutput(error); + exitCode = out.exitCode; + stderr += out.stderr; } finally { if (timer) clearTimeout(timer); process.stdout.write = originalStdoutWrite; process.stderr.write = originalStderrWrite; - console.error = originalConsoleError; - console.warn = originalConsoleWarn; for (const [key, value] of Object.entries(envBackup)) { if (value === undefined) delete process.env[key]; else process.env[key] = value; diff --git a/clients/cli/src/error-handler.ts b/clients/cli/src/error-handler.ts index 972577453..b6506e9a8 100644 --- a/clients/cli/src/error-handler.ts +++ b/clients/cli/src/error-handler.ts @@ -1,20 +1,206 @@ -function formatError(error: unknown): string { - let message: string; - - if (error instanceof Error) { - message = error.message; - } else if (typeof error === "string") { - message = error; - } else { - message = "Unknown error"; +/** + * Exit-code map. Non-zero codes let an automated caller (CI, an agent) branch + * on the failure class without regex-scraping stderr: + * + * - 0: success + * - 1: usage / unexpected error (the catch-all; same as before this map) + * - 2: `--app-info` probe found no MCP App on the tool + * - 3: server requires authentication (401 / WWW-Authenticate / OAuth) + * - 4: server unreachable (DNS, connect refused, timeout, fetch failure) + * - 5: tool error (`tools/call` returned `isError:true`, or tool not found) + */ +export const EXIT_CODES = { + OK: 0, + USAGE: 1, + NO_APP: 2, + AUTH_REQUIRED: 3, + UNREACHABLE: 4, + TOOL_ERROR: 5, +} as const; + +/** Machine-readable error envelope written as one JSON line on stderr. */ +export interface ErrorEnvelope { + /** Stable identifier for the failure class (e.g. "auth_required"). */ + code: string; + /** Human-readable message. */ + message: string; + /** The underlying error's `cause` (e.g. undici's ENOTFOUND), if any. */ + cause?: string; + /** HTTP status when the failure was an HTTP-level response. */ + status?: number; + /** The server URL the failure was against, when known. */ + url?: string; +} + +/** + * Thrown by the CLI to request a specific non-zero exit code without routing + * through the generic error path. {@link formatErrorOutput} reads `exitCode` + * and `envelope`; the in-process test runner does the same so tests observe + * the real code and stderr. + */ +export class CliExitCodeError extends Error { + constructor( + public readonly exitCode: number, + message: string, + public readonly envelope?: Partial, + ) { + super(message); + this.name = "CliExitCodeError"; } +} - return message; +/** Read an `Error.cause` chain into a single readable string, if present. */ +function causeOf(error: unknown): string | 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); + return nested ? `${cause.message}: ${nested}` : cause.message; + } + return String(cause); } -export function handleError(error: unknown): never { - const errorMessage = formatError(error); - console.error(errorMessage); +/** Best-effort HTTP status from common error shapes (undici, MCP SDK, fetch). */ +function statusOf(error: unknown): number | undefined { + if (error == null || typeof error !== "object") return undefined; + const e = error as { + status?: unknown; + code?: unknown; + response?: { status?: unknown }; + }; + 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; + return undefined; +} + +const UNREACHABLE_PATTERN = + /ENOTFOUND|ECONNREFUSED|ECONNRESET|EAI_AGAIN|ETIMEDOUT|fetch failed|getaddrinfo|connect timed out|aborted/i; + +/** + * 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. + */ +export function classifyError( + error: unknown, + context?: { url?: string }, +): { exitCode: number; envelope: ErrorEnvelope } { + const message = + error instanceof Error + ? error.message + : typeof error === "string" + ? error + : "Unknown error"; + const cause = causeOf(error); + const status = statusOf(error); + const url = context?.url; - process.exit(1); + // A pre-classified error carries its own exit code; fill in any envelope + // fields the thrower didn't supply. + if (error instanceof CliExitCodeError) { + return { + exitCode: error.exitCode, + envelope: { + code: error.envelope?.code ?? codeForExit(error.exitCode), + message, + ...(cause !== undefined && { cause }), + ...(error.envelope?.status !== undefined && { + status: error.envelope.status, + }), + ...((error.envelope?.url ?? url) !== undefined && { + url: error.envelope?.url ?? url, + }), + }, + }; + } + + // 401 / OAuth-required → AUTH_REQUIRED so the caller can kick the auth flow. + if ( + status === 401 || + status === 403 || + /WWW-Authenticate|Unauthorized|invalid_token|OAuth/i.test( + message + " " + (cause ?? ""), + ) + ) { + return { + exitCode: EXIT_CODES.AUTH_REQUIRED, + envelope: { + code: "auth_required", + message, + ...(cause !== undefined && { cause }), + ...(status !== undefined && { status }), + ...(url !== undefined && { url }), + }, + }; + } + + // Network-layer failure → UNREACHABLE so the caller can retry via a proxy. + if (UNREACHABLE_PATTERN.test(message + " " + (cause ?? ""))) { + return { + exitCode: EXIT_CODES.UNREACHABLE, + envelope: { + code: "unreachable", + message, + ...(cause !== undefined && { cause }), + ...(status !== undefined && { status }), + ...(url !== undefined && { url }), + }, + }; + } + + return { + exitCode: EXIT_CODES.USAGE, + envelope: { + code: "error", + message, + ...(cause !== undefined && { cause }), + ...(status !== undefined && { status }), + ...(url !== undefined && { url }), + }, + }; +} + +function codeForExit(exitCode: number): string { + switch (exitCode) { + case EXIT_CODES.NO_APP: + return "no_app"; + case EXIT_CODES.AUTH_REQUIRED: + return "auth_required"; + case EXIT_CODES.UNREACHABLE: + return "unreachable"; + case EXIT_CODES.TOOL_ERROR: + return "tool_error"; + default: + return "error"; + } +} + +/** + * Single source of truth for mapping a thrown error to the CLI's exit code and + * stderr text. The binary entry ({@link handleError}) and the in-process test + * runner both call this, so tests observe exactly what the binary would emit. + * + * stderr is one JSON line `{"error":{...}}` so a caller can `2>&1 | tail -1 | + * jq .error`; the message is inside the envelope so nothing is lost vs. the + * previous bare-message behaviour. + */ +export function formatErrorOutput( + error: unknown, + context?: { url?: string }, +): { exitCode: number; stderr: string } { + const { exitCode, envelope } = classifyError(error, context); + return { + exitCode, + stderr: JSON.stringify({ error: envelope }) + "\n", + }; +} + +export function handleError(error: unknown): never { + const { exitCode, stderr } = formatErrorOutput(error); + process.stderr.write(stderr); + process.exit(exitCode); } From 381027063d898fac8f4200db7b6adb047799e99d Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 1 Jul 2026 18:19:59 -0400 Subject: [PATCH 05/46] feat(core): redact sensitive headers in the fetch log (closes #1561) Redact Authorization, Cookie, Set-Cookie, Proxy-Authorization, X-Api-Key, and x-mcp-remote-auth to [REDACTED] in createFetchTracker before any request/response entry reaches the in-memory log, pino logger, or persisted session storage. Comparison is case-insensitive; original key casing is preserved and the live outbound request is untouched. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX --- .../src/test/core/mcp/fetchTracking.test.ts | 120 +++++++++++++++++- core/mcp/fetchTracking.ts | 55 +++++++- 2 files changed, 167 insertions(+), 8 deletions(-) diff --git a/clients/web/src/test/core/mcp/fetchTracking.test.ts b/clients/web/src/test/core/mcp/fetchTracking.test.ts index c83057cc3..d14fa477d 100644 --- a/clients/web/src/test/core/mcp/fetchTracking.test.ts +++ b/clients/web/src/test/core/mcp/fetchTracking.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect, vi } from "vitest"; -import { createFetchTracker } from "@inspector/core/mcp/fetchTracking.js"; +import { + createFetchTracker, + redactSensitiveHeaders, + REDACTED_HEADER_VALUE, +} from "@inspector/core/mcp/fetchTracking.js"; import type { FetchRequestEntryBase } from "@inspector/core/mcp/types.js"; // The tracker fires `trackRequest` synchronously with an entry whose @@ -8,6 +12,16 @@ import type { FetchRequestEntryBase } from "@inspector/core/mcp/types.js"; // microtask so the background read can complete before assertions. const flush = () => new Promise((r) => setTimeout(r, 0)); +// happy-dom's Headers.forEach preserves the original key casing whereas +// Node's lowercases — normalise in tests so assertions are env-independent. +function lowerKeys( + headers: Record | undefined, +): Record { + const out: Record = {}; + for (const [k, v] of Object.entries(headers ?? {})) out[k.toLowerCase()] = v; + return out; +} + describe("createFetchTracker", () => { it("tracks a successful GET request and emits the response body asynchronously", async () => { const baseFetch = vi.fn( @@ -262,4 +276,108 @@ describe("createFetchTracker", () => { expect(tracked[0]?.responseBody).toBeUndefined(); expect(bodies).toHaveLength(0); }); + + it("redacts Authorization and Cookie request headers in the recorded entry", async () => { + let outboundInit: RequestInit | undefined; + const baseFetch = vi.fn( + async (_input: RequestInfo | URL, init?: RequestInit) => { + outboundInit = init; + return new Response("ok"); + }, + ); + const tracked: FetchRequestEntryBase[] = []; + const fetcher = createFetchTracker(baseFetch as typeof fetch, { + trackRequest: (entry) => tracked.push(entry), + }); + await fetcher("https://example.com/mcp", { + method: "POST", + headers: { + Authorization: "Bearer live-access-token", + cookie: "session=secret", + "X-Api-Key": "sk-123", + "x-mcp-remote-auth": "Bearer inspector-backend-token", + "X-Custom": "kept", + }, + }); + const headers = lowerKeys(tracked[0]!.requestHeaders); + expect(headers["authorization"]).toBe(REDACTED_HEADER_VALUE); + expect(headers["cookie"]).toBe(REDACTED_HEADER_VALUE); + expect(headers["x-api-key"]).toBe(REDACTED_HEADER_VALUE); + expect(headers["x-mcp-remote-auth"]).toBe(REDACTED_HEADER_VALUE); + expect(headers["x-custom"]).toBe("kept"); + expect(JSON.stringify(tracked[0])).not.toContain("live-access-token"); + expect(JSON.stringify(tracked[0])).not.toContain("session=secret"); + expect(JSON.stringify(tracked[0])).not.toContain("inspector-backend-token"); + // The actual outbound request still carries the live token — redaction is + // only for the recorded entry. + expect(new Headers(outboundInit?.headers).get("authorization")).toBe( + "Bearer live-access-token", + ); + }); + + it("redacts Authorization on the error path too", async () => { + const baseFetch = vi.fn(async () => { + throw new Error("network down"); + }); + const tracked: FetchRequestEntryBase[] = []; + const fetcher = createFetchTracker(baseFetch as typeof fetch, { + trackRequest: (entry) => tracked.push(entry), + }); + await expect( + fetcher("https://example.com/fail", { + headers: { Authorization: "Bearer leaked-on-error" }, + }), + ).rejects.toThrow("network down"); + expect(lowerKeys(tracked[0]!.requestHeaders)["authorization"]).toBe( + REDACTED_HEADER_VALUE, + ); + expect(JSON.stringify(tracked[0])).not.toContain("leaked-on-error"); + }); + + it("redacts sensitive response headers in the recorded entry", async () => { + // Set-Cookie is a forbidden response-header name in the Fetch API (the + // browser strips it from a constructed Response), so exercise the + // response-side redaction with x-api-key instead — it proves the same + // wiring without fighting the test environment. + const baseFetch = vi.fn( + async () => + new Response("ok", { + headers: { + "x-api-key": "issued-secret", + "content-type": "text/plain", + }, + }), + ); + const tracked: FetchRequestEntryBase[] = []; + const fetcher = createFetchTracker(baseFetch as typeof fetch, { + trackRequest: (entry) => tracked.push(entry), + }); + await fetcher("https://example.com/login"); + const responseHeaders = lowerKeys(tracked[0]!.responseHeaders); + expect(responseHeaders["x-api-key"]).toBe(REDACTED_HEADER_VALUE); + expect(responseHeaders["content-type"]).toBe("text/plain"); + expect(JSON.stringify(tracked[0])).not.toContain("issued-secret"); + }); +}); + +describe("redactSensitiveHeaders", () => { + it("redacts case-insensitively while preserving the original key casing", () => { + const out = redactSensitiveHeaders({ + Authorization: "Bearer x", + "PROXY-AUTHORIZATION": "Basic y", + "X-Trace": "abc", + }); + expect(out).toEqual({ + Authorization: REDACTED_HEADER_VALUE, + "PROXY-AUTHORIZATION": REDACTED_HEADER_VALUE, + "X-Trace": "abc", + }); + }); + + it("returns a new object and never mutates the input", () => { + const input = { authorization: "Bearer x" }; + const out = redactSensitiveHeaders(input); + expect(out).not.toBe(input); + expect(input.authorization).toBe("Bearer x"); + }); }); diff --git a/core/mcp/fetchTracking.ts b/core/mcp/fetchTracking.ts index 27eb62695..a25e24786 100644 --- a/core/mcp/fetchTracking.ts +++ b/core/mcp/fetchTracking.ts @@ -1,5 +1,43 @@ import type { FetchRequestEntryBase } from "./types.js"; +/** + * Header names whose values are replaced with `REDACTED_HEADER_VALUE` before a + * fetch entry is recorded. The recorded entry flows to the in-memory log, the + * pino logger, and (via session storage) to disk — none of those sinks should + * ever see a live bearer token or session cookie. + */ +const SENSITIVE_HEADERS: ReadonlySet = new Set([ + "authorization", + "cookie", + "set-cookie", + "proxy-authorization", + "x-api-key", + // The inspector backend's own bearer (createRemoteFetch stamps this on every + // proxied request); same exposure as Authorization. + "x-mcp-remote-auth", +]); + +/** Placeholder substituted for sensitive header values in recorded entries. */ +export const REDACTED_HEADER_VALUE = "[REDACTED]"; + +/** + * Returns a copy of `headers` with every {@link SENSITIVE_HEADERS} value + * replaced by {@link REDACTED_HEADER_VALUE}. Comparison is case-insensitive + * (HTTP header names are case-insensitive); the original casing of every key is + * preserved so the recorded entry still shows what the client actually sent. + */ +export function redactSensitiveHeaders( + headers: Record, +): Record { + const out: Record = {}; + for (const [key, value] of Object.entries(headers)) { + out[key] = SENSITIVE_HEADERS.has(key.toLowerCase()) + ? REDACTED_HEADER_VALUE + : value; + } + return out; +} + /** * Whether a response represents an unbounded (long-lived) HTTP stream * whose body cannot be cloned + read to completion. The streamable HTTP @@ -57,19 +95,21 @@ export function createFetchTracker( : input.url; const method = init?.method || "GET"; - // Extract headers - const requestHeaders: Record = {}; + // Extract headers, redacting sensitive values BEFORE they reach any + // downstream sink (logger, in-memory list, persisted session storage). + const rawRequestHeaders: Record = {}; if (input instanceof Request) { input.headers.forEach((value, key) => { - requestHeaders[key] = value; + rawRequestHeaders[key] = value; }); } if (init?.headers) { const headers = new Headers(init.headers); headers.forEach((value, key) => { - requestHeaders[key] = value; + rawRequestHeaders[key] = value; }); } + const requestHeaders = redactSensitiveHeaders(rawRequestHeaders); // Extract body (if present and readable) let requestBody: string | undefined; @@ -122,11 +162,12 @@ export function createFetchTracker( const responseStatus = response.status; const responseStatusText = response.statusText; - // Extract response headers - const responseHeaders: Record = {}; + // Extract response headers (redacted — Set-Cookie etc. are credentials too) + const rawResponseHeaders: Record = {}; response.headers.forEach((value, key) => { - responseHeaders[key] = value; + rawResponseHeaders[key] = value; }); + const responseHeaders = redactSensitiveHeaders(rawResponseHeaders); // Skip body reading only for *long-lived* streams. On streamable HTTP, // GET /mcp opens an unbounded SSE channel for server-to-client pushes From 1a6e32be50079fb9a2969b31a04f780c901ccae4 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 1 Jul 2026 18:20:13 -0400 Subject: [PATCH 06/46] feat(core): add AppInfo + extractAppInfo() to core/mcp/apps.ts (closes #1556) Add a machine-readable summary of an MCP App's UI metadata: the AppInfo interface plus extractAppInfo(), which merges a tool's _meta.ui (resourceUri, visibility) with the linked UI resource's _meta.ui (csp, permissions, domain, prefersBorder, mimeType). Content-block matching is exact-URI first, then a lowercase + trailing-slash normalized match, then a single-block fallback since resources/read returns the requested resource by definition. Shared plumbing for the programmatic-review path (CLI --app-info, integration tests) without rendering the app. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX --- clients/web/src/test/core/apps.test.ts | 157 ++++++++++++++++++++++++- core/mcp/apps.ts | 123 ++++++++++++++++++- 2 files changed, 277 insertions(+), 3 deletions(-) diff --git a/clients/web/src/test/core/apps.test.ts b/clients/web/src/test/core/apps.test.ts index 466d2746c..5e518a5a2 100644 --- a/clients/web/src/test/core/apps.test.ts +++ b/clients/web/src/test/core/apps.test.ts @@ -1,6 +1,13 @@ import { describe, it, expect } from "vitest"; -import type { Tool } from "@modelcontextprotocol/sdk/types.js"; -import { getAppResourceUri, isAppTool } from "@inspector/core/mcp/apps.js"; +import type { + ReadResourceResult, + Tool, +} from "@modelcontextprotocol/sdk/types.js"; +import { + extractAppInfo, + getAppResourceUri, + isAppTool, +} from "@inspector/core/mcp/apps.js"; const NESTED_URI = "ui://demo/widget"; const FLAT_URI = "ui://legacy/widget"; @@ -92,3 +99,149 @@ describe("isAppTool", () => { ).toThrow(/Invalid UI resource URI/); }); }); + +describe("extractAppInfo", () => { + const APP_TOOL = toolWith({ + ui: { resourceUri: NESTED_URI, visibility: ["model", "app"] }, + }); + + const resourceWith = ( + ui: Record | undefined, + ): ReadResourceResult => ({ + contents: [ + { + uri: NESTED_URI, + mimeType: "text/html", + text: "", + ...(ui ? { _meta: { ui } } : {}), + }, + ], + }); + + it("returns hasApp:false for a non-App tool", () => { + expect(extractAppInfo(toolWith(undefined))).toEqual({ + hasApp: false, + toolName: "demo", + }); + }); + + it("returns tool-side info (resourceUri, visibility) when no resource is supplied", () => { + expect(extractAppInfo(APP_TOOL)).toEqual({ + hasApp: true, + toolName: "demo", + resourceUri: NESTED_URI, + visibility: ["model", "app"], + }); + }); + + it("merges resource-side csp/permissions/domain/prefersBorder/mimeType from the matched content item", () => { + const csp = { connectDomains: ["https://api.example.com"] }; + const permissions = { clipboard: true }; + const info = extractAppInfo( + APP_TOOL, + resourceWith({ csp, permissions, domain: "abc.example.net" }), + ); + expect(info).toEqual({ + hasApp: true, + toolName: "demo", + resourceUri: NESTED_URI, + visibility: ["model", "app"], + csp, + permissions, + domain: "abc.example.net", + resourceMimeType: "text/html", + }); + }); + + it("falls back to result-level _meta.ui when no content item matches the URI", () => { + const result: ReadResourceResult = { + contents: [{ uri: "ui://other", text: "" }], + _meta: { ui: { domain: "fallback.example.net" } }, + }; + expect(extractAppInfo(APP_TOOL, result).domain).toBe( + "fallback.example.net", + ); + }); + + it("includes prefersBorder:false when explicitly set", () => { + expect( + extractAppInfo(APP_TOOL, resourceWith({ prefersBorder: false })) + .prefersBorder, + ).toBe(false); + }); + + it("propagates the underlying throw for a malformed resourceUri", () => { + expect(() => + extractAppInfo(toolWith({ ui: { resourceUri: "https://x" } })), + ).toThrow(/Invalid UI resource URI/); + }); + + describe("resource-content URI matching", () => { + const csp = { connectDomains: ["https://api.example.com"] }; + + function resourceAt( + uris: string[], + uiOn: number | "result" = 0, + ): ReadResourceResult { + return { + contents: uris.map((uri, i) => ({ + uri, + mimeType: "text/html", + text: "", + ...(uiOn === i ? { _meta: { ui: { csp } } } : {}), + })), + ...(uiOn === "result" ? { _meta: { ui: { csp } } } : {}), + }; + } + + it("finds the content block on exact URI match", () => { + expect(extractAppInfo(APP_TOOL, resourceAt([NESTED_URI])).csp).toEqual( + csp, + ); + }); + + it("finds the content block when the server's URI differs only by case or trailing slash", () => { + const tool = toolWith({ + ui: { resourceUri: "ui://Demo/Widget", visibility: ["app"] }, + }); + // Two blocks so the single-block fallback can't paper over a failed + // normalize — the match must be on the normalized URI. + const info = extractAppInfo( + tool, + resourceAt(["ui://other/x", "ui://demo/widget/"], 1), + ); + expect(info.csp).toEqual(csp); + expect(info.resourceMimeType).toBe("text/html"); + }); + + it("falls back to the sole content block when the URI does not match — resources/read returns the requested resource by definition", () => { + const info = extractAppInfo( + APP_TOOL, + resourceAt(["ui://demo/widget?v=2"]), + ); + expect(info.csp).toEqual(csp); + }); + + it("does not pick an arbitrary block when multiple non-matching blocks are returned", () => { + const info = extractAppInfo( + APP_TOOL, + resourceAt(["ui://other/a", "ui://other/b"], "result"), + ); + // No content match → no resourceMimeType from a content block … + expect(info.resourceMimeType).toBeUndefined(); + // … but the result-level _meta.ui still flows through. + expect(info.csp).toEqual(csp); + }); + + it("normalize is just lowercase + trailing-slash strip; an empty URI does not match", () => { + const result: ReadResourceResult = { + contents: [ + { uri: "", text: "x", _meta: { ui: { csp } } }, + { uri: "ui://other", text: "y" }, + ], + }; + const info = extractAppInfo(APP_TOOL, result); + expect(info.csp).toBeUndefined(); + }); + }); +}); diff --git a/core/mcp/apps.ts b/core/mcp/apps.ts index 1729fb466..37198e525 100644 --- a/core/mcp/apps.ts +++ b/core/mcp/apps.ts @@ -1,5 +1,8 @@ import { getToolUiResourceUri } from "@modelcontextprotocol/ext-apps/app-bridge"; -import type { Tool } from "@modelcontextprotocol/sdk/types.js"; +import type { + ReadResourceResult, + Tool, +} from "@modelcontextprotocol/sdk/types.js"; /** * Returns the UI resource URI advertised by an MCP App tool, or `undefined` @@ -35,3 +38,121 @@ export function getAppResourceUri(tool: Tool): string | undefined { export function isAppTool(tool: Tool): boolean { return getAppResourceUri(tool) !== undefined; } + +/** + * Machine-readable summary of an MCP App's UI metadata, combining what the + * tool advertises (`resourceUri`, `visibility`) with what the UI resource + * itself declares (`csp`, `permissions`, `domain`, `prefersBorder`). Emitted by + * the CLI's `--app-info` mode and reusable by any client that needs to surface + * an app's security posture without rendering it. + */ +export interface AppInfo { + hasApp: boolean; + toolName: string; + resourceUri?: string; + visibility?: readonly string[]; + /** From the UI resource's `_meta.ui` (per spec, csp/permissions/domain live on the resource, not the tool). Absent when the resource was not read. */ + csp?: Readonly>; + permissions?: Readonly>; + domain?: string; + prefersBorder?: boolean; + resourceMimeType?: string; +} + +type WithUiMeta = { _meta?: { ui?: unknown } }; + +/** + * Structural shape for `_meta.ui` carriers — the ext-apps package's named + * types (`McpUiToolMeta`, `McpUiResourceMeta`) are not importable under + * NodeNext module resolution because of an extensionless re-export in the + * package's `.d.ts`, so we read the fields structurally instead. The values + * pass through verbatim into {@link AppInfo}; callers can narrow them against + * the published Zod schemas if they need strict typing. + */ +interface UiMetaShape { + visibility?: readonly string[]; + csp?: Record; + permissions?: Record; + domain?: string; + prefersBorder?: boolean; +} + +function readUiMeta(carrier: WithUiMeta | undefined): UiMetaShape | undefined { + const ui = carrier?._meta?.ui; + return ui !== null && typeof ui === "object" + ? (ui as UiMetaShape) + : undefined; +} + +/** + * Normalizes a `ui://…` URI for comparison so trivial server-side differences + * (case, trailing slash) don't cause us to miss the resource block and silently + * drop its `_meta.ui` (csp/permissions/domain). `ui://` is a non-special scheme + * so the WHATWG URL parser preserves case and trailing slashes verbatim — we + * apply a targeted lowercase + single trailing-slash strip instead. + */ +function normalizeResourceUri(uri: string): string { + const lowered = uri.toLowerCase(); + return lowered.endsWith("/") ? lowered.slice(0, -1) : lowered; +} + +/** + * Locates the content block in a `resources/read` result that corresponds to + * the requested UI resource. Tries exact match, then normalized-URI match, then + * falls back to the sole content block when there is only one — `resources/read` + * by definition returns the content for the URI we asked for, so a single-block + * response is the right block even when the server echoes the URI back in a + * slightly different form. + */ +function findResourceContent( + resource: ReadResourceResult, + requestedUri: string, +): ReadResourceResult["contents"][number] | undefined { + const exact = resource.contents.find((c) => c.uri === requestedUri); + if (exact) return exact; + const norm = normalizeResourceUri(requestedUri); + const normalized = resource.contents.find( + (c) => normalizeResourceUri(c.uri) === norm, + ); + if (normalized) return normalized; + return resource.contents.length === 1 ? resource.contents[0] : undefined; +} + +/** + * Build an {@link AppInfo} from a tool definition and (optionally) the + * `resources/read` result for its UI resource. Per the spec, `csp` / + * `permissions` / `domain` belong on the resource — not the tool — so a caller + * that wants the security posture must read the resource and pass it here. + * + * Returns `{ hasApp: false, toolName }` for non-App tools. Throws on a + * malformed `resourceUri` for the same reason {@link getAppResourceUri} does. + */ +export function extractAppInfo( + tool: Tool, + resource?: ReadResourceResult, +): AppInfo { + const resourceUri = getAppResourceUri(tool); + if (resourceUri === undefined) { + return { hasApp: false, toolName: tool.name }; + } + const toolUi = readUiMeta(tool as WithUiMeta); + const content = resource && findResourceContent(resource, resourceUri); + const resourceUi = + readUiMeta(content as WithUiMeta | undefined) ?? + readUiMeta(resource as WithUiMeta | undefined); + return { + hasApp: true, + toolName: tool.name, + resourceUri, + ...(toolUi?.visibility ? { visibility: toolUi.visibility } : {}), + ...(resourceUi?.csp ? { csp: resourceUi.csp } : {}), + ...(resourceUi?.permissions ? { permissions: resourceUi.permissions } : {}), + ...(resourceUi?.domain ? { domain: resourceUi.domain } : {}), + ...(resourceUi?.prefersBorder !== undefined + ? { prefersBorder: resourceUi.prefersBorder } + : {}), + ...(typeof content?.mimeType === "string" + ? { resourceMimeType: content.mimeType } + : {}), + }; +} From 26478159bee598e618b5581cbaf31ee278a51872 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 1 Jul 2026 18:21:41 -0400 Subject: [PATCH 07/46] feat(web): generalize downloadFile.ts with downloadBlob, fileNameFromUri, isHttpUrl (closes #1560) Extend the browser download library beyond JSON so the Apps host's ui/download-file support can download arbitrary embedded resources and open http(s) links safely, with no UI wiring in this change: - downloadBlob(): temp-anchor download core with a setTimeout(...,0)- deferred revokeObjectURL so a synchronous revoke can't abort the scheduled download (Firefox/Safari, intermittently Chrome). - downloadJsonFile(): refactored on top of downloadBlob; callers unchanged. - fileNameFromUri(): last path segment sanitized (control/format chars stripped, disallowed filename chars -> _, capped at 255, "download" fallback). - isHttpUrl(): parse-and-allowlist http(s) only, else null. Re-implementation of the downloadFile.ts slice of PR #1510 as Wave 1 of the #1579 decomposition. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX --- clients/web/src/lib/downloadFile.ts | 55 ++++++++++++--- clients/web/src/test/lib/downloadFile.test.ts | 68 +++++++++++++++++-- 2 files changed, 109 insertions(+), 14 deletions(-) diff --git a/clients/web/src/lib/downloadFile.ts b/clients/web/src/lib/downloadFile.ts index 77590717d..4ce5057f3 100644 --- a/clients/web/src/lib/downloadFile.ts +++ b/clients/web/src/lib/downloadFile.ts @@ -2,22 +2,23 @@ * Browser-side helpers for triggering file downloads from in-memory content. * * Centralized so the temp-anchor incantation (`appendChild` for Firefox, - * try/finally to keep cleanup bulletproof, `revokeObjectURL` to release the - * object URL) lives in one place — and so the wiring is unit-testable - * under happy-dom without dragging React along. + * deferred `revokeObjectURL` so the scheduled download can read the blob) + * lives in one place — and so the wiring is unit-testable under happy-dom + * without dragging React along. */ /** - * Download an in-memory string as a JSON file. Uses a temporary anchor + * Download an in-memory {@link Blob} as `filename`. Uses a temporary anchor * element to trigger the browser's save dialog. The append-to-body step is * for older Firefox versions that wouldn't fire `click()` on a detached * anchor; modern browsers don't require it but it stays as the safe path. * - * The cleanup (removeChild + revokeObjectURL) runs in a `finally` so even - * a thrown `click()` doesn't leak the DOM node or the object URL. + * The object-URL revoke is deferred to a task: `link.click()` only schedules + * the download, and revoking the URL synchronously can abort it before the + * browser reads the blob (Firefox/Safari, intermittently Chrome for larger + * blobs). */ -export function downloadJsonFile(filename: string, json: string): void { - const blob = new Blob([json], { type: "application/json" }); +export function downloadBlob(filename: string, blob: Blob): void { const url = URL.createObjectURL(blob); const anchor = document.createElement("a"); anchor.href = url; @@ -27,7 +28,43 @@ export function downloadJsonFile(filename: string, json: string): void { anchor.click(); } finally { document.body.removeChild(anchor); - URL.revokeObjectURL(url); + setTimeout(() => URL.revokeObjectURL(url), 0); + } +} + +/** Download an in-memory JSON string as `filename`. */ +export function downloadJsonFile(filename: string, json: string): void { + downloadBlob(filename, new Blob([json], { type: "application/json" })); +} + +/** + * Derive a safe suggested filename from a resource URI: the last path segment, + * stripped of control/format characters, path separators, and characters + * disallowed in filenames on common platforms. Falls back to `"download"` when + * nothing usable remains. + */ +export function fileNameFromUri(uri: string): string { + const tail = uri.split(/[\\/]/).pop() ?? ""; + const safe = tail + .replace(/[\p{Cc}\p{Cf}]+/gu, "") + .replace(/[\\/:*?"<>|]+/g, "_") + .trim(); + return safe.length > 0 ? safe.slice(0, 255) : "download"; +} + +/** + * Parse `url` and return it only if its scheme is `http:` or `https:`; + * otherwise null. Shared http(s)-only allowlist for opening or downloading + * server-supplied URLs. + */ +export function isHttpUrl(url: string): URL | null { + try { + const parsed = new URL(url); + return parsed.protocol === "https:" || parsed.protocol === "http:" + ? parsed + : null; + } catch { + return null; } } diff --git a/clients/web/src/test/lib/downloadFile.test.ts b/clients/web/src/test/lib/downloadFile.test.ts index de1ae9944..36cc656db 100644 --- a/clients/web/src/test/lib/downloadFile.test.ts +++ b/clients/web/src/test/lib/downloadFile.test.ts @@ -1,19 +1,26 @@ /** - * Tests for the downloadJsonFile helper. Mocks URL.createObjectURL + + * Tests for the download helpers. Mocks URL.createObjectURL + * URL.revokeObjectURL (happy-dom doesn't ship them) and asserts the temp - * anchor's attributes + click + cleanup sequence. + * anchor's attributes + click + deferred cleanup sequence. */ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; -import { downloadJsonFile, buildExportFilename } from "../../lib/downloadFile"; - -describe("downloadJsonFile", () => { +import { + downloadBlob, + downloadJsonFile, + buildExportFilename, + fileNameFromUri, + isHttpUrl, +} from "../../lib/downloadFile"; + +describe("downloadBlob / downloadJsonFile", () => { const originalCreate = URL.createObjectURL; const originalRevoke = URL.revokeObjectURL; let createMock: ReturnType; let revokeMock: ReturnType; beforeEach(() => { + vi.useFakeTimers(); createMock = vi.fn().mockReturnValue("blob:mock-url"); revokeMock = vi.fn(); // happy-dom doesn't implement URL.createObjectURL/revokeObjectURL. @@ -22,6 +29,7 @@ describe("downloadJsonFile", () => { }); afterEach(() => { + vi.useRealTimers(); URL.createObjectURL = originalCreate; URL.revokeObjectURL = originalRevoke; vi.restoreAllMocks(); @@ -54,6 +62,9 @@ describe("downloadJsonFile", () => { // After the call, the anchor must be removed and the URL revoked. expect(document.body.contains(anchor)).toBe(false); + // Revoke is deferred so the scheduled download can read the blob first. + expect(revokeMock).not.toHaveBeenCalled(); + vi.runAllTimers(); expect(revokeMock).toHaveBeenCalledWith("blob:mock-url"); }); @@ -82,6 +93,7 @@ describe("downloadJsonFile", () => { // Cleanup still ran despite the throw. expect(removed).toHaveLength(1); + vi.runAllTimers(); expect(revokeMock).toHaveBeenCalledWith("blob:mock-url"); }); @@ -91,6 +103,15 @@ describe("downloadJsonFile", () => { const text = await blob.text(); expect(text).toBe('{"x":1}'); }); + + it("downloadBlob passes the given blob through and honours its type", async () => { + const blob = new Blob(["plain text"], { type: "text/plain" }); + downloadBlob("note.txt", blob); + expect(createMock).toHaveBeenCalledWith(blob); + const passed = createMock.mock.calls[0]?.[0] as Blob; + expect(passed.type).toBe("text/plain"); + expect(await passed.text()).toBe("plain text"); + }); }); describe("buildExportFilename", () => { @@ -127,3 +148,40 @@ describe("buildExportFilename", () => { ); }); }); + +describe("fileNameFromUri", () => { + it("returns the last path segment", () => { + expect(fileNameFromUri("https://x/y/z/report.pdf")).toBe("report.pdf"); + }); + it("splits on backslashes as well as forward slashes", () => { + expect(fileNameFromUri("C:\\Users\\me\\report.pdf")).toBe("report.pdf"); + }); + it("strips control/format chars and disallowed filename chars", () => { + // U+200B (zero-width space, Cf) and U+202E (RTL override, Cf) are + // stripped; the disallowed `*?` run collapses to a single `_`. + const uri = "https://x/a\u200bb\u202ec*?.txt"; + expect(fileNameFromUri(uri)).toBe("abc_.txt"); + }); + it("truncates very long names to 255 characters", () => { + const long = "a".repeat(300) + ".txt"; + expect(fileNameFromUri(`https://x/${long}`)).toHaveLength(255); + }); + it("falls back to 'download' when nothing usable remains", () => { + expect(fileNameFromUri("https://x/")).toBe("download"); + }); +}); + +describe("isHttpUrl", () => { + it("accepts http and https", () => { + expect(isHttpUrl("https://example.com")?.href).toBe("https://example.com/"); + expect(isHttpUrl("http://localhost:3000/a")?.href).toBe( + "http://localhost:3000/a", + ); + }); + it("rejects other schemes and unparsable input", () => { + expect(isHttpUrl("javascript:alert(1)")).toBeNull(); + expect(isHttpUrl("data:text/html,")).toBeNull(); + expect(isHttpUrl("file:///etc/passwd")).toBeNull(); + expect(isHttpUrl("not a url")).toBeNull(); + }); +}); From 0d50b908a3efa6f77d9471c2fe4d5bc2b7f432af Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 1 Jul 2026 18:22:25 -0400 Subject: [PATCH 08/46] feat(test-servers): mcp_app_demo preset + _meta on tool/resource defs (closes #1557) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an `mcp_app_demo` tool/resource preset — a self-contained MCP App widget that exercises size-changed, ui/message, log notifications, and host-context rendering with no external server. Requires plumbing an optional `_meta` field through the composable test server's `ToolDefinition` and `ResourceDefinition` so clients can read tool-level `_meta.ui.resourceUri` and resource-level `_meta.ui.csp`. - composable-test-server: optional `_meta` on ToolDefinition/ResourceDefinition, passed through to registerTool config and the resource content item. - test-server-fixtures: `createMcpAppDemoTool()`, `createMcpAppDemoResource()`, inline `MCP_APP_DEMO_HTML` widget; wired into getDefaultServerConfig. - preset-registry: `mcp_app_demo` tool + `mcp_app_demo_widget` resource presets. - integration test asserting the `_meta` plumbing end-to-end. Wave 1 of PR #1510 decomposition. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX --- .../test/integration/mcp/mcp-app-demo.test.ts | 108 ++++++++++++ test-servers/src/composable-test-server.ts | 6 + test-servers/src/preset-registry.ts | 6 + test-servers/src/test-server-fixtures.ts | 161 ++++++++++++++++++ 4 files changed, 281 insertions(+) create mode 100644 clients/web/src/test/integration/mcp/mcp-app-demo.test.ts diff --git a/clients/web/src/test/integration/mcp/mcp-app-demo.test.ts b/clients/web/src/test/integration/mcp/mcp-app-demo.test.ts new file mode 100644 index 000000000..4da5d92c6 --- /dev/null +++ b/clients/web/src/test/integration/mcp/mcp-app-demo.test.ts @@ -0,0 +1,108 @@ +import { describe, it, expect, afterEach } from "vitest"; +import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; +import { createTransportNode } from "@inspector/core/mcp/node/transport.js"; +import { + createTestServerHttp, + type TestServerHttp, + createTestServerInfo, + createMcpAppDemoTool, + createMcpAppDemoResource, +} from "@modelcontextprotocol/inspector-test-server"; +import type { Tool } from "@modelcontextprotocol/sdk/types.js"; + +const MCP_APP_DEMO_URI = "ui://demo/widget.html"; + +/** + * Exercises the `mcp_app_demo` preset added in #1557 end-to-end: it confirms the + * `_meta` plumbing through the composable test server's tool/resource + * definitions surfaces on `tools/list` and `resources/read`, which is what the + * downstream Apps-host and CLI `--app-info` tests rely on. + */ +describe("mcp_app_demo preset", () => { + let client: InspectorClient | null = null; + let server: TestServerHttp | null = null; + + afterEach(async () => { + if (client) { + try { + await client.disconnect(); + } catch { + // Ignore disconnect errors + } + client = null; + } + if (server) { + try { + await server.stop(); + } catch { + // Ignore server stop errors + } + server = null; + } + }); + + async function connect(): Promise { + server = createTestServerHttp({ + serverInfo: createTestServerInfo("mcp-app-demo-test", "1.0.0"), + tools: [createMcpAppDemoTool()], + resources: [createMcpAppDemoResource()], + }); + await server.start(); + const connected = new InspectorClient( + { type: "streamable-http", url: server.url }, + { environment: { transport: createTransportNode } }, + ); + await connected.connect(); + client = connected; + return connected; + } + + it("exposes tool-level _meta.ui.resourceUri on tools/list", async () => { + const connected = await connect(); + const { tools } = await connected.listTools(); + const tool = tools.find((t) => t.name === "mcp_app_demo"); + expect(tool).toBeDefined(); + const meta = (tool as Tool)._meta as + | { ui?: { resourceUri?: string; visibility?: string[] } } + | undefined; + expect(meta?.ui?.resourceUri).toBe(MCP_APP_DEMO_URI); + expect(meta?.ui?.visibility).toEqual(["model", "app"]); + }); + + it("exposes resource-level _meta.ui on resources/read", async () => { + const connected = await connect(); + const invocation = await connected.readResource(MCP_APP_DEMO_URI); + const contents = invocation.result.contents; + expect(contents).toHaveLength(1); + const entry = contents[0]; + expect(entry.mimeType).toBe("text/html"); + expect("text" in entry ? String(entry.text) : "").toContain("mcp-app-demo"); + const meta = entry._meta as + | { + ui?: { + csp?: { connectDomains?: string[]; resourceDomains?: string[] }; + permissions?: { clipboard?: boolean }; + prefersBorder?: boolean; + }; + } + | undefined; + expect(meta?.ui?.csp).toEqual({ + connectDomains: [], + resourceDomains: [], + }); + expect(meta?.ui?.permissions).toEqual({ clipboard: false }); + expect(meta?.ui?.prefersBorder).toBe(true); + }); + + it("echoes the input title from the tool handler", async () => { + const connected = await connect(); + const { tools } = await connected.listTools(); + const tool = tools.find((t) => t.name === "mcp_app_demo")!; + const result = await connected.callTool(tool, { title: "Widget A" }); + expect(result.success).toBe(true); + const content = result.result!.content; + const text = + content[0] && "text" in content[0] ? String(content[0].text) : ""; + expect(text).toBe('mcp_app_demo rendered with title="Widget A"'); + }); +}); diff --git a/test-servers/src/composable-test-server.ts b/test-servers/src/composable-test-server.ts index 44ca60572..55ebb9a47 100644 --- a/test-servers/src/composable-test-server.ts +++ b/test-servers/src/composable-test-server.ts @@ -102,6 +102,8 @@ export interface ToolDefinition { inputSchema?: ToolInputSchema; /** Optional Zod object schema for tool output; when set, handler must return structuredContent. */ outputSchema?: unknown; + /** Passed through to the SDK so clients can read tool-level `_meta` (e.g. `_meta.ui.resourceUri` for MCP App tools). */ + _meta?: Record; handler: ( params: Record, context?: TestServerContext, @@ -123,6 +125,8 @@ export interface ResourceDefinition { description?: string; mimeType?: string; text?: string; + /** Included on the returned content item so clients can read resource-level `_meta` (e.g. `_meta.ui.csp` for MCP App UI resources). */ + _meta?: Record; } export interface PromptDefinition { @@ -500,6 +504,7 @@ export function createMcpServer(config: ServerConfig): McpServer { ...(tool.outputSchema != null && { outputSchema: tool.outputSchema as AnySchema, }), + ...(tool._meta != null && { _meta: tool._meta }), }, async (args, extra) => { const result = await tool.handler( @@ -565,6 +570,7 @@ export function createMcpServer(config: ServerConfig): McpServer { uri: resource.uri, mimeType: resource.mimeType || "text/plain", text: resource.text ?? "", + ...(resource._meta != null && { _meta: resource._meta }), }, ], }; diff --git a/test-servers/src/preset-registry.ts b/test-servers/src/preset-registry.ts index 5e2f3ea9f..51dd2db75 100644 --- a/test-servers/src/preset-registry.ts +++ b/test-servers/src/preset-registry.ts @@ -40,6 +40,8 @@ import { createOptionalTaskTool, createForbiddenTaskTool, createImmediateReturnTaskTool, + createMcpAppDemoTool, + createMcpAppDemoResource, createArchitectureResource, createTestCwdResource, createTestEnvResource, @@ -153,6 +155,8 @@ function resolveToolPreset( get("name") as string | undefined, Number(get("delayMs")) || undefined, ); + case "mcp_app_demo": + return createMcpAppDemoTool(); default: throw new Error(`Unknown tool preset: ${name}`); } @@ -175,6 +179,8 @@ function resolveResourcePreset( return createTestArgvResource(); case "numbered_resources": return createNumberedResources(Number(get("count")) || 3); + case "mcp_app_demo_widget": + return createMcpAppDemoResource(); default: throw new Error(`Unknown resource preset: ${name}`); } diff --git a/test-servers/src/test-server-fixtures.ts b/test-servers/src/test-server-fixtures.ts index 95c8cbeda..80874b28c 100644 --- a/test-servers/src/test-server-fixtures.ts +++ b/test-servers/src/test-server-fixtures.ts @@ -754,6 +754,165 @@ export function createArgsPrompt( }; } +const MCP_APP_DEMO_URI = "ui://demo/widget.html"; + +/** + * Minimal MCP App widget that exercises the host-side UI protocol surface + * (size-changed, ui/message, log notification, host-context render). Kept as a + * single inline HTML string with no external scripts so the sandbox CSP's + * locked-down defaults are sufficient. + * + * The lone `rgba(0,0,0,0.06)` is a deliberate exception to the AGENTS.md + * color-token rule: this is a self-contained static fixture served into the + * sandbox iframe, not a Mantine component, so the `--inspector-*` CSS custom + * properties (defined in the web client's App.css) are not in scope here. + */ +const MCP_APP_DEMO_HTML = ` + + + + mcp-app-demo widget + + + +

mcp-app-demo

+
waiting for ui/initialize…
+ + +`; + +/** + * Tool definition for the MCP App demo. Carries `_meta.ui.resourceUri` so + * clients recognize it as an App tool; the call result echoes the input title + * so the rendered widget can be visually correlated with the call. + */ +export function createMcpAppDemoTool(): ToolDefinition { + return { + name: "mcp_app_demo", + description: + "Render a minimal MCP App widget that exercises size-changed, ui/message, logging, and host-context rendering.", + inputSchema: { + title: z.string().describe("Heading shown in the rendered widget"), + }, + _meta: { + ui: { resourceUri: MCP_APP_DEMO_URI, visibility: ["model", "app"] }, + }, + handler: async (params: Record) => { + return toToolResult( + `mcp_app_demo rendered with title="${String(params.title)}"`, + ); + }, + }; +} + +/** + * UI resource for {@link createMcpAppDemoTool}. Declares a permissive + * `_meta.ui.csp` (no external connect/resource domains) and a sample + * `permissions` block so `--app-info` and the host's CSP enforcement both have + * something to read. + */ +export function createMcpAppDemoResource(): ResourceDefinition { + return { + name: "mcp_app_demo_widget", + uri: MCP_APP_DEMO_URI, + description: "Inline HTML widget for the mcp_app_demo tool", + mimeType: "text/html", + text: MCP_APP_DEMO_HTML, + _meta: { + ui: { + csp: { connectDomains: [], resourceDomains: [] }, + permissions: { clipboard: false }, + prefersBorder: true, + }, + }, + }; +} + /** * Create an "architecture" resource definition */ @@ -1883,6 +2042,7 @@ export function getDefaultServerConfig(): ServerConfig { createGetTempExtraTool(), createSendNotificationTool(), createWriteToStderrTool(), + createMcpAppDemoTool(), ], prompts: [createSimplePrompt(), createArgsPrompt()], resources: [ @@ -1890,6 +2050,7 @@ export function getDefaultServerConfig(): ServerConfig { createTestCwdResource(), createTestEnvResource(), createTestArgvResource(), + createMcpAppDemoResource(), ], resourceTemplates: [ createFileResourceTemplate(), From 5831b5c3d97102c2eb91b9e93bf745a1fdb6f644 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 1 Jul 2026 18:22:46 -0400 Subject: [PATCH 09/46] feat(web): extract hostContext utilities for the Apps host (closes #1559) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add clients/web/src/components/elements/AppRenderer/hostContext.ts — a pure utilities module for the MCP Apps host: - currentTheme(): read the resolved Mantine color scheme from the DOM - currentStyles(): map the inspector's Mantine/-inspector design tokens to a McpUiHostStyles snapshot via STYLE_VARIABLE_SOURCES - measureContainerDimensions(): whole-pixel container measurement - snapshotHostContext(): assemble the initial McpUiHostContext seed Wave 1 of the PR #1510 decomposition (#1579). Pure extraction only — the AppRenderer/AppsScreen/createAppBridgeFactory rewrites belong to Wave 2. Full unit coverage (100% lines/statements/functions/branches). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX --- .../elements/AppRenderer/hostContext.test.ts | 208 ++++++++++++++++++ .../elements/AppRenderer/hostContext.ts | 156 +++++++++++++ 2 files changed, 364 insertions(+) create mode 100644 clients/web/src/components/elements/AppRenderer/hostContext.test.ts create mode 100644 clients/web/src/components/elements/AppRenderer/hostContext.ts diff --git a/clients/web/src/components/elements/AppRenderer/hostContext.test.ts b/clients/web/src/components/elements/AppRenderer/hostContext.test.ts new file mode 100644 index 000000000..230413327 --- /dev/null +++ b/clients/web/src/components/elements/AppRenderer/hostContext.test.ts @@ -0,0 +1,208 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { McpUiDisplayMode } from "@modelcontextprotocol/ext-apps/app-bridge"; +import { + currentStyles, + currentTheme, + measureContainerDimensions, + snapshotHostContext, +} from "./hostContext"; + +const COLOR_SCHEME_ATTR = "data-mantine-color-scheme"; + +/** + * Build a fake CSSStyleDeclaration whose `getPropertyValue` reads from a + * lookup table, so tests control exactly which CSS variables resolve (and to + * what) without depending on happy-dom's custom-property resolution. + */ +function fakeComputedStyle( + values: Record, +): CSSStyleDeclaration { + return { + getPropertyValue: (name: string) => values[name] ?? "", + } as unknown as CSSStyleDeclaration; +} + +function stubComputedStyle(values: Record) { + vi.spyOn(window, "getComputedStyle").mockReturnValue( + fakeComputedStyle(values), + ); +} + +afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + document.documentElement.removeAttribute(COLOR_SCHEME_ATTR); +}); + +describe("currentTheme", () => { + it("returns the resolved dark scheme written by Mantine", () => { + document.documentElement.setAttribute(COLOR_SCHEME_ATTR, "dark"); + expect(currentTheme()).toBe("dark"); + }); + + it("returns the resolved light scheme written by Mantine", () => { + document.documentElement.setAttribute(COLOR_SCHEME_ATTR, "light"); + expect(currentTheme()).toBe("light"); + }); + + it("ignores an unrecognized attribute value and falls back", () => { + document.documentElement.setAttribute(COLOR_SCHEME_ATTR, "sepia"); + vi.stubGlobal("matchMedia", () => ({ matches: false })); + expect(currentTheme()).toBe("light"); + }); + + it("falls back to matchMedia dark when the attribute is absent", () => { + document.documentElement.removeAttribute(COLOR_SCHEME_ATTR); + vi.stubGlobal("matchMedia", (query: string) => ({ + matches: query.includes("dark"), + })); + expect(currentTheme()).toBe("dark"); + }); + + it("falls back to light when matchMedia does not match dark", () => { + document.documentElement.removeAttribute(COLOR_SCHEME_ATTR); + vi.stubGlobal("matchMedia", () => ({ matches: false })); + expect(currentTheme()).toBe("light"); + }); + + it("falls back to light when matchMedia is unavailable", () => { + document.documentElement.removeAttribute(COLOR_SCHEME_ATTR); + vi.stubGlobal("matchMedia", undefined); + expect(currentTheme()).toBe("light"); + }); + + it("returns light in a non-DOM environment (no document, no window)", () => { + vi.stubGlobal("document", undefined); + vi.stubGlobal("window", undefined); + expect(currentTheme()).toBe("light"); + }); + + it("consults matchMedia when document is absent but window is present", () => { + vi.stubGlobal("document", undefined); + vi.stubGlobal("matchMedia", (query: string) => ({ + matches: query.includes("dark"), + })); + expect(currentTheme()).toBe("dark"); + }); +}); + +describe("currentStyles", () => { + it("maps resolved Mantine tokens to spec style variables", () => { + stubComputedStyle({ + "--mantine-color-body": "#101113", + "--mantine-color-text": "#c9c9c9", + "--mantine-radius-md": "0.5rem", + }); + const styles = currentStyles(); + expect(styles).toEqual({ + variables: { + "--color-background-primary": "#101113", + "--color-text-primary": "#c9c9c9", + "--border-radius-md": "0.5rem", + }, + }); + }); + + it("trims whitespace and omits variables that resolve to empty", () => { + stubComputedStyle({ + "--mantine-color-body": " #ffffff ", + "--mantine-color-text": " ", + }); + const styles = currentStyles(); + expect(styles?.variables?.["--color-background-primary"]).toBe("#ffffff"); + expect(styles?.variables?.["--color-text-primary"]).toBeUndefined(); + }); + + it("returns undefined when nothing resolves", () => { + stubComputedStyle({}); + expect(currentStyles()).toBeUndefined(); + }); + + it("returns undefined in a non-DOM environment (no document)", () => { + vi.stubGlobal("document", undefined); + expect(currentStyles()).toBeUndefined(); + }); + + it("returns undefined in a non-DOM environment (no window)", () => { + vi.stubGlobal("window", undefined); + expect(currentStyles()).toBeUndefined(); + }); +}); + +describe("measureContainerDimensions", () => { + it("returns whole-pixel width and height for a laid-out element", () => { + const el = document.createElement("div"); + vi.spyOn(el, "getBoundingClientRect").mockReturnValue({ + width: 320.6, + height: 199.4, + } as DOMRect); + expect(measureContainerDimensions(el)).toEqual({ width: 321, height: 199 }); + }); + + it("returns undefined for a 0x0 (unattached) element", () => { + const el = document.createElement("div"); + vi.spyOn(el, "getBoundingClientRect").mockReturnValue({ + width: 0, + height: 0, + } as DOMRect); + expect(measureContainerDimensions(el)).toBeUndefined(); + }); + + it("returns undefined when only one dimension is zero", () => { + const el = document.createElement("div"); + vi.spyOn(el, "getBoundingClientRect").mockReturnValue({ + width: 100, + height: 0, + } as DOMRect); + expect(measureContainerDimensions(el)).toBeUndefined(); + }); + + it("returns undefined when getBoundingClientRect is unavailable", () => { + const el = {} as unknown as HTMLElement; + expect(measureContainerDimensions(el)).toBeUndefined(); + }); +}); + +describe("snapshotHostContext", () => { + const MODES: readonly McpUiDisplayMode[] = ["inline", "fullscreen"]; + + it("seeds theme, inline display mode, and a copied available-modes list", () => { + document.documentElement.setAttribute(COLOR_SCHEME_ATTR, "dark"); + stubComputedStyle({}); + const context = snapshotHostContext(null, MODES); + expect(context.theme).toBe("dark"); + expect(context.displayMode).toBe("inline"); + expect(context.availableDisplayModes).toEqual(["inline", "fullscreen"]); + // A copy, not the caller's array. + expect(context.availableDisplayModes).not.toBe(MODES); + expect(context.styles).toBeUndefined(); + expect(context.containerDimensions).toBeUndefined(); + }); + + it("includes styles and container dimensions when both resolve", () => { + document.documentElement.setAttribute(COLOR_SCHEME_ATTR, "light"); + stubComputedStyle({ "--mantine-color-body": "#ffffff" }); + const container = document.createElement("div"); + vi.spyOn(container, "getBoundingClientRect").mockReturnValue({ + width: 640, + height: 480, + } as DOMRect); + const context = snapshotHostContext(container, MODES); + expect(context.theme).toBe("light"); + expect(context.styles).toEqual({ + variables: { "--color-background-primary": "#ffffff" }, + }); + expect(context.containerDimensions).toEqual({ width: 640, height: 480 }); + }); + + it("omits container dimensions when the container has no layout box", () => { + stubComputedStyle({}); + const container = document.createElement("div"); + vi.spyOn(container, "getBoundingClientRect").mockReturnValue({ + width: 0, + height: 0, + } as DOMRect); + const context = snapshotHostContext(container, MODES); + expect(context.containerDimensions).toBeUndefined(); + }); +}); diff --git a/clients/web/src/components/elements/AppRenderer/hostContext.ts b/clients/web/src/components/elements/AppRenderer/hostContext.ts new file mode 100644 index 000000000..9ea6f8f08 --- /dev/null +++ b/clients/web/src/components/elements/AppRenderer/hostContext.ts @@ -0,0 +1,156 @@ +import type { + McpUiDisplayMode, + McpUiHostContext, + McpUiHostStyles, + McpUiStyles, + McpUiStyleVariableKey, +} from "@modelcontextprotocol/ext-apps/app-bridge"; + +/** + * Resolve the host theme from the DOM. Mantine writes the resolved color + * scheme to ``. Reading it here (rather than + * capturing React state) keeps the bridge factory's identity stable across + * theme toggles — the renderer treats a new factory identity as "rebuild the + * bridge", which would reload a running app's iframe on every theme flip. + * + * The attribute is only ever `"light"` or `"dark"` — Mantine resolves + * `defaultColorScheme="auto"` to the system value before paint and never + * writes `"auto"` here, so no `auto` branch is needed. The matchMedia + * fallback only covers the attribute being absent (e.g. a hydration race). + */ +export function currentTheme(): "light" | "dark" { + if (typeof document !== "undefined") { + const attr = document.documentElement.getAttribute( + "data-mantine-color-scheme", + ); + if (attr === "dark" || attr === "light") return attr; + } + if ( + typeof window !== "undefined" && + window.matchMedia?.("(prefers-color-scheme: dark)").matches + ) { + return "dark"; + } + return "light"; +} + +/** + * Maps the spec's host-style variable keys ({@link McpUiStyleVariableKey}) to + * the inspector's underlying CSS custom properties. The inspector themes itself + * with Mantine, so each spec token resolves to the matching Mantine design-token + * variable (or an `--inspector-*` token layered on top of one). Only a curated + * subset of the ~90 spec keys is mapped — the ones the inspector has a + * meaningful equivalent for; the rest are omitted, which the spec allows (hosts + * may provide any subset). + */ +const STYLE_VARIABLE_SOURCES: Partial> = { + "--color-background-primary": "--mantine-color-body", + "--color-background-secondary": "--inspector-surface-card", + "--color-background-tertiary": "--inspector-surface-subtle", + "--color-text-primary": "--mantine-color-text", + "--color-text-secondary": "--inspector-text-secondary", + "--color-text-inverse": "--inspector-text-inverse", + "--color-text-info": "--inspector-log-info", + "--color-text-danger": "--inspector-log-error", + "--color-text-success": "--inspector-status-connected", + "--color-text-warning": "--inspector-log-warning", + "--color-border-primary": "--inspector-border-default", + "--color-border-secondary": "--inspector-border-subtle", + "--font-sans": "--mantine-font-family", + "--font-mono": "--mantine-font-family-monospace", + "--font-text-xs-size": "--mantine-font-size-xs", + "--font-text-sm-size": "--mantine-font-size-sm", + "--font-text-md-size": "--mantine-font-size-md", + "--font-text-lg-size": "--mantine-font-size-lg", + "--border-radius-xs": "--mantine-radius-xs", + "--border-radius-sm": "--mantine-radius-sm", + "--border-radius-md": "--mantine-radius-md", + "--border-radius-lg": "--mantine-radius-lg", + "--border-radius-xl": "--mantine-radius-xl", + "--shadow-sm": "--mantine-shadow-sm", + "--shadow-md": "--mantine-shadow-md", + "--shadow-lg": "--mantine-shadow-lg", +}; + +const STYLE_VARIABLE_ENTRIES = Object.entries(STYLE_VARIABLE_SOURCES) as [ + McpUiStyleVariableKey, + string, +][]; + +/** + * Resolve the inspector's design tokens into a {@link McpUiHostStyles} for + * hostContext, so style-aware apps can theme themselves from the host instead + * of falling back to their own defaults. Reads the computed value of each + * mapped CSS variable from the document root — which reflects the active + * Mantine color scheme — and keeps only the ones that resolve to a non-empty + * value. Returns undefined when nothing resolves (e.g. a non-DOM/test + * environment) so we never advertise an empty styles object. + */ +export function currentStyles(): McpUiHostStyles | undefined { + if (typeof document === "undefined" || typeof window === "undefined") { + return undefined; + } + const computed = window.getComputedStyle(document.documentElement); + const variables: McpUiStyles = {} as McpUiStyles; + let resolved = false; + for (const [specKey, sourceVar] of STYLE_VARIABLE_ENTRIES) { + const value = computed.getPropertyValue(sourceVar).trim(); + if (value) { + variables[specKey] = value; + resolved = true; + } + } + return resolved ? { variables } : undefined; +} + +/** + * Spec shape for `hostContext.containerDimensions`. Derived from + * {@link McpUiHostContext} so the seed and live-push paths share one source of + * truth and stay in lockstep with the spec types. + */ +export type ContainerDimensions = NonNullable< + McpUiHostContext["containerDimensions"] +>; + +/** + * Measure the host container an app renders into and return its concrete + * `{ width, height }` (whole pixels). Returns undefined when the element has + * no layout box yet (0×0 — e.g. before the iframe is attached, or in a + * non-DOM/test environment) so a meaningless size is never seeded into + * hostContext. The return type is the concrete pair rather than the spec's + * {@link ContainerDimensions} union so callers can compare both fields. + */ +export function measureContainerDimensions( + element: HTMLElement, +): { width: number; height: number } | undefined { + if (typeof element.getBoundingClientRect !== "function") return undefined; + const rect = element.getBoundingClientRect(); + const width = Math.round(rect.width); + const height = Math.round(rect.height); + if (width <= 0 || height <= 0) return undefined; + return { width, height }; +} + +/** + * Read the live host UI state into a {@link McpUiHostContext} for the bridge + * handshake — the single place that decides which fields the inspector seeds. + * Optional fields are omitted (not set undefined) so the SDK's diff stays + * accurate; subsequent live changes are pushed by the renderer's observers as + * partial `host-context-changed` notifications. + */ +export function snapshotHostContext( + container: HTMLElement | null, + availableDisplayModes: readonly McpUiDisplayMode[], +): McpUiHostContext { + const styles = currentStyles(); + const containerDimensions = container + ? measureContainerDimensions(container) + : undefined; + return { + theme: currentTheme(), + displayMode: "inline", + availableDisplayModes: [...availableDisplayModes], + ...(styles ? { styles } : {}), + ...(containerDimensions ? { containerDimensions } : {}), + }; +} From 77e57e0d2539db2462d215c2337e133effbd77fb Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 1 Jul 2026 18:23:13 -0400 Subject: [PATCH 10/46] feat(web): add sandbox CSP builder library (closes #1558) Add a pure, DOM-free CSP-builder library at clients/web/src/lib/sandbox-csp.ts that validates app-supplied `_meta.ui.csp` requests, filters them to safe sources, builds the locked-down Content-Security-Policy string, and wraps untrusted widget HTML in a fixed host shell whose first child is the CSP meta. - SAFE_CSP_SOURCE: strict regex that rejects directive/attribute breakouts (`;`, `"`, `<`, `>`, whitespace). - approveCspSources(): drops unsafe entries (with a warning) and omits empty keys; echoes back only what the host will enforce. - buildSandboxCspPolicy(): `default-src 'none'` catch-all, `form-action 'none'`, source-allowlist filtering, `'unsafe-inline'` for the app's own inline script/style. - escapeHtmlAttr() / wrapSandboxedHtml(): defense-in-depth so untrusted bytes never precede the applied policy. Re-implements the sandbox-csp portion of PR #1510 (Wave 1 of #1579) as a standalone library; wiring into the app bridge is a follow-up issue. Adds full unit tests (100% lines/statements/functions/branches) and lists the file in the web coverage `include` so it is gated (the untested legacy src/lib siblings stay out until they get tests). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX --- clients/web/src/lib/sandbox-csp.test.ts | 164 ++++++++++++++++++++++++ clients/web/src/lib/sandbox-csp.ts | 111 ++++++++++++++++ clients/web/vite.config.ts | 4 + 3 files changed, 279 insertions(+) create mode 100644 clients/web/src/lib/sandbox-csp.test.ts create mode 100644 clients/web/src/lib/sandbox-csp.ts diff --git a/clients/web/src/lib/sandbox-csp.test.ts b/clients/web/src/lib/sandbox-csp.test.ts new file mode 100644 index 000000000..8beee9781 --- /dev/null +++ b/clients/web/src/lib/sandbox-csp.test.ts @@ -0,0 +1,164 @@ +import { describe, expect, it, vi } from "vitest"; +import { + SAFE_CSP_SOURCE, + approveCspSources, + buildSandboxCspPolicy, + escapeHtmlAttr, + wrapSandboxedHtml, +} from "./sandbox-csp"; + +describe("SAFE_CSP_SOURCE", () => { + it.each([ + "*", + "https://api.example.com", + "https://api.example.com:8443", + "https://api.example.com/path/seg", + "wss://realtime.service.com", + "*.example.com", + "https://*.cloudflare.com", + "data:", + "blob:", + "example.com", + ])("accepts %s", (s) => { + expect(SAFE_CSP_SOURCE.test(s)).toBe(true); + }); + + it.each([ + "https://a.com; script-src *", + 'https://a.com" onload=', + "https://a.com>", + "app`; + const wrapped = wrapSandboxedHtml(evil, "default-src 'none'"); + // The first in the output is OURS; the untrusted token only + // appears after . + const ourHead = wrapped.indexOf(""); + const body = wrapped.indexOf(""); + const evilHead = wrapped.indexOf("", ourHead + 1); + expect(ourHead).toBeLessThan(body); + expect(evilHead).toBeGreaterThan(body); + }); + + it("does not introduce a host-authored