From f84531a298403291993197019807940c1f7814df Mon Sep 17 00:00:00 2001 From: tobin Date: Fri, 19 Jun 2026 14:45:27 +0000 Subject: [PATCH 01/55] feat(web/apps): enforce per-app _meta.ui.csp in the sandbox proxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per-app `_meta.ui.csp` was plumbed through `sendSandboxResourceReady` but the sandbox proxy never read it, so the inspector advertised CSP support without applying one. The MCP UI spec defines `csp.connectDomains`, `resourceDomains`, `frameDomains`, and `baseUriDomains` as the allowlists a host MUST enforce on the rendered app. - `static/sandbox_proxy.html`: add `buildCspContent(csp)` (translates McpUiResourceCsp into a Content-Security-Policy string with locked-down defaults for omitted lists) and `injectCsp(html, content)` (inserts the `` as the first `` child so it governs every subresource). The resource-ready handler now writes secured HTML via both the document.write and srcdoc paths. CSP is always applied — a resource that declares no `csp` still gets the locked-down defaults. - `createAppBridgeFactory.ts`: build the bridge with a per-app copy of `HOST_CAPABILITIES` and, on `sandboxready`, set `hostCapabilities.sandbox = { permissions, csp }` so the approved sandbox config is echoed to the view via the initialize result. - Tests: 3 new cases (echo of declared csp+permissions, empty-csp default, no-echo before sandboxready / on read failure). --- .../createAppBridgeFactory.test.ts | 70 ++++++++++++++ .../AppRenderer/createAppBridgeFactory.ts | 19 +++- clients/web/static/sandbox_proxy.html | 91 +++++++++++++++++-- 3 files changed, 169 insertions(+), 11 deletions(-) diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts index 54dd6020a..9993496aa 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts @@ -166,6 +166,76 @@ describe("createAppBridgeFactory", () => { }); }); + it("echoes the approved csp + permissions in hostCapabilities after sandboxready", async () => { + const readResource = vi.fn().mockResolvedValue( + uiResource("

weather

", { + permissions: { geolocation: {} }, + csp: { connectDomains: ["https://api.example.com"] }, + }), + ); + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource, + }); + await factory(makeIframe(), tool); + const bridge = bridgeInstances[0]; + + bridge.emit("sandboxready"); + await flush(); + + // The capabilities object passed to the bridge constructor is mutated in + // place, so the ui/initialize handshake (which reads it lazily) echoes the + // approved sandbox config back to the view. + expect(bridge.ctorArgs[2]).toMatchObject({ + sandbox: { + permissions: { geolocation: {} }, + csp: { connectDomains: ["https://api.example.com"] }, + }, + }); + }); + + it("advertises an empty csp object when the resource declares no csp", async () => { + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource: vi.fn().mockResolvedValue(uiResource("

hi

")), + }); + await factory(makeIframe(), tool); + const bridge = bridgeInstances[0]; + + bridge.emit("sandboxready"); + await flush(); + + const sandbox = ( + bridge.ctorArgs[2] as { + sandbox?: { csp?: unknown; permissions?: unknown }; + } + ).sandbox; + expect(sandbox?.csp).toEqual({}); + expect(sandbox?.permissions).toBeUndefined(); + }); + + it("does not echo a sandbox capability before sandboxready or on read failure", async () => { + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource: vi.fn().mockRejectedValue(new Error("read boom")), + }); + await factory(makeIframe(), tool); + const bridge = bridgeInstances[0]; + + // Nothing read yet → no sandbox echo. + expect( + (bridge.ctorArgs[2] as { sandbox?: unknown }).sandbox, + ).toBeUndefined(); + + bridge.emit("sandboxready"); + await flush(); + + // Read failed → still no sandbox echo. + expect( + (bridge.ctorArgs[2] as { sandbox?: unknown }).sandbox, + ).toBeUndefined(); + }); + it("does not push when the tool has no UI resource uri", async () => { const readResource = vi.fn(); const factory = createAppBridgeFactory({ diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts index 079d26650..ca9d6bbd5 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts @@ -99,7 +99,8 @@ function extractHtmlAndMeta(result: ReadResourceResult): { * 1. constructs a host-side {@link AppBridge} over the SDK client (so the view * can call tools/resources/prompts directly), * 2. on the sandbox proxy's `sandboxready` signal, reads the tool's UI - * resource and pushes its HTML + sandbox/permissions into the inner iframe, + * resource and pushes its HTML + sandbox/permissions/CSP into the inner + * iframe, echoing the applied sandbox config back via hostCapabilities, * 3. handles `openLinks` by opening http(s) URLs in a new tab, * 4. connects a {@link PostMessageTransport} to the iframe and returns the * live bridge. @@ -123,7 +124,11 @@ export function createAppBridgeFactory( throw new Error("Cannot render MCP App: sandbox iframe has no window."); } - const bridge = new AppBridge(client, HOST_INFO, HOST_CAPABILITIES, { + // Per-app copy so the approved-sandbox echo (set on sandboxready below) never + // mutates the shared HOST_CAPABILITIES constant — each app may declare its own + // csp/permissions. + const hostCapabilities: McpUiHostCapabilities = { ...HOST_CAPABILITIES }; + const bridge = new AppBridge(client, HOST_INFO, hostCapabilities, { hostContext: { theme: currentTheme() }, }); @@ -139,6 +144,16 @@ export function createAppBridgeFactory( if (!uri) return; const result = await deps.readResource(uri); const { html, meta } = extractHtmlAndMeta(result); + // Echo the sandbox config the host applied so the view can read what was + // granted via hostCapabilities.sandbox. The CSP is always enforced (the + // sandbox proxy falls back to secure defaults), so advertise at least an + // empty csp object even when the resource declares none. Set before + // sendSandboxResourceReady: the view only sends ui/initialize once it has + // the HTML, so the bridge reflects this in the initialize result. + hostCapabilities.sandbox = { + permissions: meta?.permissions, + csp: meta?.csp ?? {}, + }; await bridge.sendSandboxResourceReady({ html, permissions: meta?.permissions, diff --git a/clients/web/static/sandbox_proxy.html b/clients/web/static/sandbox_proxy.html index 87db2ad1f..f5a3f86e6 100644 --- a/clients/web/static/sandbox_proxy.html +++ b/clients/web/static/sandbox_proxy.html @@ -4,12 +4,16 @@ + frame below ("allow-scripts allow-same-origin allow-forms") and the + parent-origin allowlist (ALLOWED_REFERRER_PATTERN + + EXPECTED_HOST_ORIGIN check on incoming messages) are the structural + isolation boundary. On top of that, network/resource egress from the + inner document is constrained per app: the resource-ready handler + below translates the UI resource's _meta.ui.csp into a + `` injected into the inner + document before it loads (see buildCspContent). No CSP *header* is set + by the sandbox controller; the per-document meta policy is the enforced + network boundary. --> MCP-UI Proxy + + +

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 +2008,7 @@ export function getDefaultServerConfig(): ServerConfig { createGetTempExtraTool(), createSendNotificationTool(), createWriteToStderrTool(), + createMcpAppDemoTool(), ], prompts: [createSimplePrompt(), createArgsPrompt()], resources: [ @@ -1890,6 +2016,7 @@ export function getDefaultServerConfig(): ServerConfig { createTestCwdResource(), createTestEnvResource(), createTestArgvResource(), + createMcpAppDemoResource(), ], resourceTemplates: [ createFileResourceTemplate(), From c8428fa2a57cffb8212a67d4924c6454aacb5329 Mon Sep 17 00:00:00 2001 From: tobin Date: Mon, 22 Jun 2026 15:15:18 +0000 Subject: [PATCH 20/55] fix(web,test-servers): document deep-link CSRF gate and harden demo widget origin checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `utils/deepLink.ts`: expand the JSDoc on `validateServerUrl` and `parseDeepLink` to record the threat-model decisions — `serverUrl` is intentionally not restricted to public addresses (the inspector exists to connect to local servers; the manual form has no such restriction either), and `autoConnect` carries the per-launch ephemeral API token as a CSRF gate. That token-in-URL exposure is the same as the existing `?MCP_INSPECTOR_API_TOKEN=` query param the launcher already prints; the value never crosses to a third-party referer because the inspector backend serves every navigation. `test-server-fixtures.ts` (`mcp_app_demo` widget): capture the host origin from the first `ui/initialize` message, then check `ev.origin` against it on every subsequent receive and use it as the `postMessage` target origin on every send. The fixture is the example developers will copy, so it should model the origin discipline a real widget needs. --- clients/web/src/utils/deepLink.ts | 13 +++++++++++++ test-servers/src/test-server-fixtures.ts | 11 ++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/clients/web/src/utils/deepLink.ts b/clients/web/src/utils/deepLink.ts index db8424b82..41093d6bb 100644 --- a/clients/web/src/utils/deepLink.ts +++ b/clients/web/src/utils/deepLink.ts @@ -50,6 +50,13 @@ function decodeAppArgs(encoded: string | null): Record { * new capability — but a deep link can be crafted by a third party, and we do * not want a click to drive a `javascript:` / `file:` / `data:` value into the * connect path. + * + * Loopback and private-range hosts are intentionally **not** blocked here: + * connecting to a locally running MCP server is the inspector's primary + * development use case, and the manual connect form imposes no such + * restriction either. The CSRF gate on `autoConnect` (see + * {@link parseDeepLink}) is what prevents a third-party page from driving a + * connect the user did not initiate. */ function validateServerUrl(raw: string): string | undefined { let url: URL; @@ -72,6 +79,12 @@ function validateServerUrl(raw: string): string | undefined { * defeats the "send a developer a crafted localhost URL" SSRF / auto-invocation * vector while keeping the one-URL automated flow (the launcher knows the * token, so it can always build a valid link). + * + * Carrying the token in the URL is the same exposure surface as the existing + * `?MCP_INSPECTOR_API_TOKEN=…` query param the launcher banner already prints + * (see `getAuthToken()` in `App.tsx`): the value is ephemeral (regenerated per + * launch), the page is loopback-only, and it never crosses to a third-party + * referer because the inspector's own backend serves every navigation. */ export function parseDeepLink( search: string, diff --git a/test-servers/src/test-server-fixtures.ts b/test-servers/src/test-server-fixtures.ts index f76ad6236..e657e7071 100644 --- a/test-servers/src/test-server-fixtures.ts +++ b/test-servers/src/test-server-fixtures.ts @@ -777,8 +777,15 @@ const MCP_APP_DEMO_HTML = ` 
waiting for ui/initialize…
`; From 8976842aeca014bed80287bb5248cbaf5a331552 Mon Sep 17 00:00:00 2001 From: tobin Date: Mon, 22 Jun 2026 16:03:31 +0000 Subject: [PATCH 27/55] fix(test-servers): mcp_app_demo merges host-context-changed partials McpUiHostContextChangedNotification.params IS the partial host context (only changed fields), not {hostContext: ...}. The fixture was reading m.params.hostContext (undefined) and replacing wholesale, so a theme flip rendered {} instead of {theme:"dark", displayMode:..., containerDimensions:...}. Now keeps a running snapshot and merges each partial, matching how a real app would consume the notification. --- test-servers/src/test-server-fixtures.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test-servers/src/test-server-fixtures.ts b/test-servers/src/test-server-fixtures.ts index d8f2ee77f..ece0ad74b 100644 --- a/test-servers/src/test-server-fixtures.ts +++ b/test-servers/src/test-server-fixtures.ts @@ -794,12 +794,16 @@ const MCP_APP_DEMO_HTML = ` { jsonrpc: "2.0", ...msg }, HOST_ORIGIN ?? "*", ); - const renderCtx = (ctx) => { + let lastCtx = {}; + const renderCtx = (patch) => { + // host-context-changed carries a PARTIAL (only changed fields per + // spec); merge into the running snapshot so unchanged fields persist. + lastCtx = { ...lastCtx, ...patch }; document.getElementById("ctx").textContent = JSON.stringify( { - theme: ctx?.theme, - displayMode: ctx?.displayMode, - containerDimensions: ctx?.containerDimensions, + theme: lastCtx.theme, + displayMode: lastCtx.displayMode, + containerDimensions: lastCtx.containerDimensions, }, null, 2, @@ -839,7 +843,8 @@ const MCP_APP_DEMO_HTML = ` HOST_ORIGIN = ev.origin; onInitialized(m.result.hostContext); } else if (m.method === "ui/notifications/host-context-changed") { - renderCtx(m.params.hostContext); + // params IS the partial McpUiHostContext (spec.types.d.ts:290). + renderCtx(m.params); } }); // Kick off the handshake. From 5c1948fcc6133c651a044643561be15fb4ef6b69 Mon Sep 17 00:00:00 2001 From: tobin Date: Mon, 22 Jun 2026 16:06:38 +0000 Subject: [PATCH 28/55] fix(web,test-servers): deep-link upsert race + ui/notifications/initialized method name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit App.tsx: collapse the two-effect deep-link flow into one effect with two phases. The previous ensure-effect latched its ref against the pre-hydration empty servers list, called addServer (which 409s when the row already exists on disk), and never reached updateServer — so a relaunch with a different serverUrl connected to the stale persisted URL. Now addServer is a one-shot that swallows the 409; the connect+update phase runs only once servers actually contains the row, against a fresh closure. test-server-fixtures.ts: the demo widget sent "notifications/initialized" but the spec method is "ui/notifications/initialized" (McpUiInitializedNotification). The bridge never marked the view ready, so the subsequent ui/message request was dropped pre-init. --- clients/web/src/App.tsx | 58 ++++++++++++++---------- test-servers/src/test-server-fixtures.ts | 4 +- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index a262eb01b..748700757 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -1573,23 +1573,33 @@ function App() { }, [inspectorClient]); // Deep-link auto-connect (closes #1183 for the URL-driven case). Split into - // two effects so the connect step always runs against a `servers` array that - // actually contains the deep-link row — calling `onToggleConnection` in the - // same closure as `addServer` would resolve the just-added id against a - // stale (often empty) `servers` and silently no-op. The OAuth callback path - // takes precedence; a deep link on `/oauth/callback` would be a - // misconfiguration, and the callback handler clears the URL. + // `useServers` hydrates asynchronously (initial `servers` is `[]`), so this + // effect runs in two phases: first render attempts a one-shot `addServer` + // (a 409 means the row already exists on disk and hydration will surface it); + // a later render where `servers` actually contains the deep-link row then + // updates the URL if it has changed and connects. Connecting in the same + // closure as `addServer` would resolve against a stale (empty) `servers` + // and silently no-op. The OAuth callback path takes precedence; a deep link + // on `/oauth/callback` would be a misconfiguration, and the callback handler + // clears the URL. useEffect(() => { if (!deepLink) return; - if (deepLinkEnsureRef.current) return; if (window.location.pathname === OAUTH_CALLBACK_PATH) return; - deepLinkEnsureRef.current = true; + const existing = servers.find((s) => s.id === deepLink.serverId); + if (!existing) { + if (deepLinkEnsureRef.current) return; + deepLinkEnsureRef.current = true; + void addServer(deepLink.serverId, deepLink.serverConfig).catch(() => { + // 409 = already on disk; hydration will surface it on a later render. + }); + return; + } + + if (deepLinkConnectRef.current) return; + deepLinkConnectRef.current = true; void (async () => { - const existing = servers.find((s) => s.id === deepLink.serverId); - if (!existing) { - await addServer(deepLink.serverId, deepLink.serverConfig); - } else if ( + if ( "url" in existing.config && "url" in deepLink.serverConfig && existing.config.url !== deepLink.serverConfig.url @@ -1600,20 +1610,18 @@ function App() { deepLink.serverConfig, ); } + if (activeServerId !== deepLink.serverId) { + await onToggleConnection(deepLink.serverId); + } })(); - }, [deepLink, servers, addServer, updateServer]); - - useEffect(() => { - if (!deepLink) return; - if (deepLinkConnectRef.current) return; - if (window.location.pathname === OAUTH_CALLBACK_PATH) return; - if (!servers.some((s) => s.id === deepLink.serverId)) return; - if (activeServerId === deepLink.serverId) return; - deepLinkConnectRef.current = true; - void (async () => { - await onToggleConnection(deepLink.serverId); - })(); - }, [deepLink, servers, activeServerId, onToggleConnection]); + }, [ + deepLink, + servers, + activeServerId, + addServer, + updateServer, + onToggleConnection, + ]); // --- Action handlers that route directly to the InspectorClient. --- diff --git a/test-servers/src/test-server-fixtures.ts b/test-servers/src/test-server-fixtures.ts index ece0ad74b..8d6ecd2f7 100644 --- a/test-servers/src/test-server-fixtures.ts +++ b/test-servers/src/test-server-fixtures.ts @@ -811,8 +811,8 @@ const MCP_APP_DEMO_HTML = ` }; const onInitialized = (hostContext) => { renderCtx(hostContext); - // Signal the view is ready (host gates host-context-changed on this). - send({ method: "notifications/initialized" }); + // Signal the view is ready (host gates view→host requests on this). + send({ method: "ui/notifications/initialized" }); // Standard MCP log notification — surfaced by the host's log panel. send({ method: "notifications/message", From 6f9a181a942776bf360a753df3b63bf4f1892991 Mon Sep 17 00:00:00 2001 From: tobin Date: Tue, 23 Jun 2026 04:48:48 +0000 Subject: [PATCH 29/55] fix(core/mcp): redact Authorization/Cookie headers in the fetch-request log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createFetchTracker captured request and response headers verbatim into FetchRequestEntry, which then flows to the pino logger (InspectorClient.dispatchFetchRequest), the in-memory Network panel, and — via FetchRequestLogState + InspectorClientStorage — to disk under ~/.mcp-inspector/storage/. Any Authorization: Bearer token sent on an MCP request would be persisted in clear in all three sinks. Redact at the source: a redactSensitiveHeaders helper replaces the value of authorization / cookie / set-cookie / proxy-authorization / x-api-key (case-insensitive, original key casing preserved) with "[REDACTED]" before the entry is built. The outbound request itself is unchanged; only the recorded copy is redacted. Adds 5 tests: request-side redaction (incl. asserting the live token still reaches the wire), error-path redaction, response-side redaction, and two unit tests for the helper. --- .../src/test/core/mcp/fetchTracking.test.ts | 117 +++++++++++++++++- core/mcp/fetchTracking.ts | 52 ++++++-- 2 files changed, 161 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..d2d65e5b6 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( @@ -241,6 +255,85 @@ describe("createFetchTracker", () => { expect(tracked[0]?.requestBody).toBeUndefined(); }); + 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-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-custom"]).toBe("kept"); + expect(JSON.stringify(tracked[0])).not.toContain("live-access-token"); + expect(JSON.stringify(tracked[0])).not.toContain("session=secret"); + // 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"); + }); + it("does not call updateResponseBody when response.clone() throws", async () => { const tracked: FetchRequestEntryBase[] = []; const bodies: Array<{ id: string; body: string }> = []; @@ -263,3 +356,25 @@ describe("createFetchTracker", () => { expect(bodies).toHaveLength(0); }); }); + +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..bc5028178 100644 --- a/core/mcp/fetchTracking.ts +++ b/core/mcp/fetchTracking.ts @@ -1,5 +1,40 @@ 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", +]); + +/** 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 +92,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 +159,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 95352a34842690a1b7b429fc02fd83fbaf299145 Mon Sep 17 00:00:00 2001 From: tobin Date: Tue, 23 Jun 2026 04:58:52 +0000 Subject: [PATCH 30/55] feat(core/auth): web client stores OAuth via backend; storage getters await hydration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The web client now uses RemoteOAuthStorage instead of BrowserOAuthStorage, so OAuth state (including the access_token after the callback completes) is POSTed to /api/storage/oauth and lands in ~/.mcp-inspector/storage/oauth.json (mode 0600) on the host running the backend. A token obtained in the browser is then readable by the CLI/TUI on the same host without copying it from DevTools. That route hydrates over async HTTP, so OAuthStorageBase now captures the zustand persist hydration promise (via hasHydrated/onFinishHydration) and every read used on the post-redirect callback path — getCodeVerifier, getServerMetadata, getClientInformation, getTokens — awaits it before reading state. Without this the OAuth callback handler would call codeVerifier() against an empty store and throw "No code verifier saved for session". With a synchronous adapter (sessionStorage, file) hydration finishes in the constructor and the await resolves immediately. getScope stays synchronous: its only caller is the SDK-required sync clientMetadata getter, and scope is always saved in-session before being read (pre-redirect), so the in-memory store has it regardless of hydration. Ripples: BaseOAuthClientProvider.codeVerifier() and getServerMetadata() become async (the SDK OAuthClientProvider.codeVerifier already accepts a Promise); state-machine token_request step awaits both. Tests: new oauth-storage.test.ts asserts each getter blocks until hydration completes (gated async adapter) and that ready() resolves immediately for a sync adapter; storage-remote.test.ts gains an explicit "value persisted before construction" hydration case; environmentFactory.test.ts asserts the web environment uses RemoteOAuthStorage. Existing storage-browser/storage-remote/oauthManager tests updated for the now-async signatures. --- .../web/src/lib/environmentFactory.test.ts | 22 ++++ clients/web/src/lib/environmentFactory.ts | 15 ++- .../src/test/core/auth/oauth-storage.test.ts | 121 ++++++++++++++++++ .../test/core/auth/storage-browser.test.ts | 8 +- .../src/test/core/auth/storage-remote.test.ts | 45 ++++++- .../src/test/core/mcp/oauthManager.test.ts | 7 +- core/auth/oauth-storage.ts | 42 +++++- core/auth/providers.ts | 8 +- core/auth/state-machine.ts | 6 +- core/auth/storage.ts | 10 +- 10 files changed, 255 insertions(+), 29 deletions(-) create mode 100644 clients/web/src/lib/environmentFactory.test.ts create mode 100644 clients/web/src/test/core/auth/oauth-storage.test.ts diff --git a/clients/web/src/lib/environmentFactory.test.ts b/clients/web/src/lib/environmentFactory.test.ts new file mode 100644 index 000000000..4ac19962e --- /dev/null +++ b/clients/web/src/lib/environmentFactory.test.ts @@ -0,0 +1,22 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { createWebEnvironment } from "./environmentFactory"; +import { RemoteOAuthStorage } from "@inspector/core/auth/remote/index.js"; + +describe("createWebEnvironment", () => { + beforeEach(() => { + // RemoteOAuthStorage's persist adapter issues a hydration GET on + // construction; stub global fetch so the test doesn't depend on a backend. + vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(JSON.stringify({}), { status: 200 }), + ); + }); + + it("uses RemoteOAuthStorage so OAuth state lands on the backend", () => { + const { environment } = createWebEnvironment( + "test-auth-token", + { getRedirectUrl: () => "http://localhost/callback" }, + undefined, + ); + expect(environment.oauth?.storage).toBeInstanceOf(RemoteOAuthStorage); + }); +}); diff --git a/clients/web/src/lib/environmentFactory.ts b/clients/web/src/lib/environmentFactory.ts index bba83fc32..2e6ccd180 100644 --- a/clients/web/src/lib/environmentFactory.ts +++ b/clients/web/src/lib/environmentFactory.ts @@ -4,10 +4,8 @@ import { createRemoteFetch, createRemoteLogger, } from "@inspector/core/mcp/remote/index.js"; -import { - BrowserOAuthStorage, - BrowserNavigation, -} from "@inspector/core/auth/browser/index.js"; +import { BrowserNavigation } from "@inspector/core/auth/browser/index.js"; +import { RemoteOAuthStorage } from "@inspector/core/auth/remote/index.js"; import type { RedirectUrlProvider } from "@inspector/core/auth/index.js"; export interface WebEnvironmentResult { @@ -20,8 +18,11 @@ export interface WebEnvironmentResult { * - transport / fetch / logger all routed through the in-process Hono * backend at `window.location.origin` (the `clients/web/server` * dev-backend wires this in `/api/*`). - * - OAuth storage + navigation use the `BrowserOAuthStorage` (sessionStorage) - * and `BrowserNavigation` (full-page redirect) adapters. + * - OAuth storage uses `RemoteOAuthStorage` (POSTs to the same backend's + * `/api/storage/oauth`, which writes `~/.mcp-inspector/storage/oauth.json` + * mode 0600), so a token obtained in this browser session is also + * available to the CLI/TUI on the same host. Navigation still uses + * `BrowserNavigation` (full-page redirect). * * Returns both the assembled environment and the logger so callers can share * the same pino instance for any direct logging they need to do, instead of @@ -67,7 +68,7 @@ export function createWebEnvironment( }), logger, oauth: { - storage: new BrowserOAuthStorage(), + storage: new RemoteOAuthStorage({ baseUrl, authToken, fetchFn }), navigation: new BrowserNavigation(undefined, onBeforeOAuthRedirect), redirectUrlProvider, }, diff --git a/clients/web/src/test/core/auth/oauth-storage.test.ts b/clients/web/src/test/core/auth/oauth-storage.test.ts new file mode 100644 index 000000000..cdfbe6b10 --- /dev/null +++ b/clients/web/src/test/core/auth/oauth-storage.test.ts @@ -0,0 +1,121 @@ +import { describe, it, expect } from "vitest"; +import { createJSONStorage } from "zustand/middleware"; +import { OAuthStorageBase } from "@inspector/core/auth/oauth-storage.js"; +import { createOAuthStore } from "@inspector/core/auth/store.js"; + +/** + * Builds an OAuthStorageBase backed by an *async* in-memory storage adapter + * pre-seeded with the given persisted blob. Hydration of the store completes + * only after the returned `release()` is called, so tests can assert that + * getters wait for it. + */ +function makeAsyncBackedStorage( + persistedServers: Record = {}, +) { + let mem: string | null = JSON.stringify({ + state: { servers: persistedServers }, + version: 0, + }); + let releaseGetItem!: () => void; + const gate = new Promise((resolve) => { + releaseGetItem = resolve; + }); + const adapter = createJSONStorage(() => ({ + getItem: async () => { + await gate; + return mem; + }, + setItem: async (name: string, value: string) => { + void name; + mem = value; + }, + removeItem: async () => { + mem = null; + }, + })); + const store = createOAuthStore(adapter); + return { storage: new OAuthStorageBase(store), release: releaseGetItem }; +} + +const SERVER = "https://example.com/mcp"; +const METADATA = { + issuer: SERVER, + authorization_endpoint: `${SERVER}/authorize`, + token_endpoint: `${SERVER}/token`, + response_types_supported: ["code"], +}; + +describe("OAuthStorageBase — async hydration", () => { + it("getCodeVerifier awaits hydration before reading state", async () => { + const { storage, release } = makeAsyncBackedStorage({ + [SERVER]: { codeVerifier: "persisted-verifier" }, + }); + // The store is empty until release() lets the async getItem resolve. + let resolvedWith: string | undefined | "pending" = "pending"; + const p = storage.getCodeVerifier(SERVER).then((v) => { + resolvedWith = v; + }); + // Give the microtask queue a chance — the getter should NOT have resolved + // because hydration is still gated. + await Promise.resolve(); + expect(resolvedWith).toBe("pending"); + release(); + await p; + expect(resolvedWith).toBe("persisted-verifier"); + }); + + it("getServerMetadata awaits hydration before reading state", async () => { + const { storage, release } = makeAsyncBackedStorage({ + [SERVER]: { serverMetadata: METADATA }, + }); + const p = storage.getServerMetadata(SERVER); + release(); + expect(await p).toEqual(METADATA); + }); + + it("getTokens and getClientInformation await hydration", async () => { + const { storage, release } = makeAsyncBackedStorage({ + [SERVER]: { + tokens: { access_token: "t", token_type: "Bearer" }, + clientInformation: { client_id: "abc" }, + }, + }); + const tokens = storage.getTokens(SERVER); + const client = storage.getClientInformation(SERVER); + release(); + expect(await tokens).toEqual({ access_token: "t", token_type: "Bearer" }); + expect(await client).toEqual({ client_id: "abc" }); + }); + + it("ready() resolves once hydration completes", async () => { + const { storage, release } = makeAsyncBackedStorage(); + let ready = false; + void storage.ready().then(() => { + ready = true; + }); + await Promise.resolve(); + expect(ready).toBe(false); + release(); + await storage.ready(); + expect(ready).toBe(true); + }); + + it("ready() resolves immediately when the adapter hydrates synchronously", async () => { + let mem: string | null = null; + const syncAdapter = createJSONStorage(() => ({ + getItem: () => mem, + setItem: (name: string, value: string) => { + void name; + mem = value; + }, + removeItem: () => { + mem = null; + }, + })); + const storage = new OAuthStorageBase(createOAuthStore(syncAdapter)); + // hasHydrated() is true at construction with a sync adapter, so ready() + // is already resolved — both forms should settle without any release step. + await storage.ready(); + expect(await storage.getCodeVerifier(SERVER)).toBeUndefined(); + }); +}); diff --git a/clients/web/src/test/core/auth/storage-browser.test.ts b/clients/web/src/test/core/auth/storage-browser.test.ts index f78f74b21..791400381 100644 --- a/clients/web/src/test/core/auth/storage-browser.test.ts +++ b/clients/web/src/test/core/auth/storage-browser.test.ts @@ -285,9 +285,9 @@ describe("BrowserOAuthStorage", () => { it("clearCodeVerifier removes only the PKCE verifier", async () => { storage.saveCodeVerifier(testServerUrl, "verifier"); - expect(storage.getCodeVerifier(testServerUrl)).toBe("verifier"); + expect(await storage.getCodeVerifier(testServerUrl)).toBe("verifier"); storage.clearCodeVerifier(testServerUrl); - expect(storage.getCodeVerifier(testServerUrl)).toBeUndefined(); + expect(await storage.getCodeVerifier(testServerUrl)).toBeUndefined(); }); it("clearScope removes only the scope", async () => { @@ -305,9 +305,9 @@ describe("BrowserOAuthStorage", () => { response_types_supported: ["code"], }; storage.saveServerMetadata(testServerUrl, metadata); - expect(storage.getServerMetadata(testServerUrl)).toEqual(metadata); + expect(await storage.getServerMetadata(testServerUrl)).toEqual(metadata); storage.clearServerMetadata(testServerUrl); - expect(storage.getServerMetadata(testServerUrl)).toBeNull(); + expect(await storage.getServerMetadata(testServerUrl)).toBeNull(); }); }); diff --git a/clients/web/src/test/core/auth/storage-remote.test.ts b/clients/web/src/test/core/auth/storage-remote.test.ts index b9075e14d..ea45b4bc3 100644 --- a/clients/web/src/test/core/auth/storage-remote.test.ts +++ b/clients/web/src/test/core/auth/storage-remote.test.ts @@ -64,9 +64,9 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => { it("codeVerifier round-trip and clearCodeVerifier", async () => { await storage.saveCodeVerifier(serverUrl, "verifier"); - expect(storage.getCodeVerifier(serverUrl)).toBe("verifier"); + expect(await storage.getCodeVerifier(serverUrl)).toBe("verifier"); storage.clearCodeVerifier(serverUrl); - expect(storage.getCodeVerifier(serverUrl)).toBeUndefined(); + expect(await storage.getCodeVerifier(serverUrl)).toBeUndefined(); }); it("scope round-trip and clearScope", async () => { @@ -84,9 +84,9 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => { response_types_supported: ["code"], }; await storage.saveServerMetadata(serverUrl, md); - expect(storage.getServerMetadata(serverUrl)).toEqual(md); + expect(await storage.getServerMetadata(serverUrl)).toEqual(md); storage.clearServerMetadata(serverUrl); - expect(storage.getServerMetadata(serverUrl)).toBeNull(); + expect(await storage.getServerMetadata(serverUrl)).toBeNull(); }); it("clear() wipes all state for a server", async () => { @@ -100,6 +100,43 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => { expect(await storage.getTokens(serverUrl)).toBeUndefined(); }); + it("getCodeVerifier waits for the async hydration GET before reading", async () => { + // Unlike the other tests in this file (which write-then-read in the same + // session), this asserts that a value PERSISTED on the backend before the + // store was constructed is still returned — i.e. the getter awaits the + // remote storage adapter's hydration GET. + let releaseGet!: () => void; + const getGate = new Promise((resolve) => { + releaseGet = resolve; + }); + const fetchFn = vi.fn( + async (url: RequestInfo | URL, init?: RequestInit) => { + if ((init?.method ?? "GET") === "GET") { + await getGate; + return new Response( + JSON.stringify({ + state: { + servers: { [serverUrl]: { codeVerifier: "from-disk" } }, + }, + version: 0, + }), + { status: 200 }, + ); + } + void url; + return new Response("{}", { status: 200 }); + }, + ); + const s = new RemoteOAuthStorage({ + baseUrl: "http://remote.example", + storeId: "hydration-test", + fetchFn: fetchFn as unknown as typeof fetch, + }); + const p = s.getCodeVerifier(serverUrl); + releaseGet(); + expect(await p).toBe("from-disk"); + }); + it("default storeId is 'oauth' when omitted", () => { const s = new RemoteOAuthStorage({ baseUrl: "http://r.example", diff --git a/clients/web/src/test/core/mcp/oauthManager.test.ts b/clients/web/src/test/core/mcp/oauthManager.test.ts index f45e85274..29360e5be 100644 --- a/clients/web/src/test/core/mcp/oauthManager.test.ts +++ b/clients/web/src/test/core/mcp/oauthManager.test.ts @@ -21,14 +21,15 @@ function createMockParams( const dispatchOAuthError = vi.fn(); const storage = { - getScope: vi.fn().mockResolvedValue(undefined), + ready: vi.fn().mockResolvedValue(undefined), + getScope: vi.fn().mockReturnValue(undefined), getClientInformation: vi.fn().mockResolvedValue(undefined), saveClientInformation: vi.fn().mockResolvedValue(undefined), savePreregisteredClientInformation: vi.fn().mockResolvedValue(undefined), saveScope: vi.fn().mockResolvedValue(undefined), getTokens: vi.fn().mockResolvedValue(undefined), saveTokens: vi.fn().mockResolvedValue(undefined), - getCodeVerifier: vi.fn().mockReturnValue("verifier"), + getCodeVerifier: vi.fn().mockResolvedValue("verifier"), saveCodeVerifier: vi.fn().mockResolvedValue(undefined), clear: vi.fn(), clearClientInformation: vi.fn(), @@ -36,7 +37,7 @@ function createMockParams( clearCodeVerifier: vi.fn(), clearScope: vi.fn(), clearServerMetadata: vi.fn(), - getServerMetadata: vi.fn().mockReturnValue(null), + getServerMetadata: vi.fn().mockResolvedValue(null), saveServerMetadata: vi.fn().mockResolvedValue(undefined), }; diff --git a/core/auth/oauth-storage.ts b/core/auth/oauth-storage.ts index df3617b17..fa0a353d6 100644 --- a/core/auth/oauth-storage.ts +++ b/core/auth/oauth-storage.ts @@ -14,18 +14,45 @@ import { type createOAuthStore, type ServerOAuthState } from "./store.js"; * Concrete OAuthStorage implementation parameterized on a Zustand store. * The store carries the storage adapter (sessionStorage, file, remote HTTP, …), * so the same body works for browser, Node, and remote environments. + * + * With an async storage adapter (file, remote HTTP) the persist middleware + * hydrates the store after construction, so `store.getState()` is empty until + * that completes. Every read used on the post-OAuth-redirect callback path + * (`getCodeVerifier`, `getServerMetadata`, `getClientInformation`, `getTokens`) + * therefore awaits {@link hydrated} before reading state. With a synchronous + * adapter (sessionStorage) hydration finishes before the constructor returns, + * so the await resolves immediately and the behaviour is unchanged. */ export class OAuthStorageBase implements OAuthStorage { private readonly store: ReturnType; + private readonly hydrated: Promise; constructor(store: ReturnType) { this.store = store; + this.hydrated = this.store.persist.hasHydrated() + ? Promise.resolve() + : new Promise((resolve) => { + const unsub = this.store.persist.onFinishHydration(() => { + unsub(); + resolve(); + }); + }); + } + + /** + * Resolves once the underlying persist adapter has hydrated the store. + * Callers that need to read state outside the typed getters can await this + * directly (e.g. before reading the store via `getState()` for diagnostics). + */ + ready(): Promise { + return this.hydrated; } async getClientInformation( serverUrl: string, isPreregistered?: boolean, ): Promise { + await this.hydrated; const state = this.store.getState().getServerState(serverUrl); const clientInfo = isPreregistered ? state.preregisteredClientInformation @@ -69,6 +96,7 @@ export class OAuthStorageBase implements OAuthStorage { } async getTokens(serverUrl: string): Promise { + await this.hydrated; const state = this.store.getState().getServerState(serverUrl); if (!state.tokens) { return undefined; @@ -85,7 +113,8 @@ export class OAuthStorageBase implements OAuthStorage { this.store.getState().setServerState(serverUrl, { tokens: undefined }); } - getCodeVerifier(serverUrl: string): string | undefined { + async getCodeVerifier(serverUrl: string): Promise { + await this.hydrated; const state = this.store.getState().getServerState(serverUrl); return state.codeVerifier; } @@ -103,6 +132,14 @@ export class OAuthStorageBase implements OAuthStorage { .setServerState(serverUrl, { codeVerifier: undefined }); } + /** + * Intentionally synchronous. The only caller is + * {@link BaseOAuthClientProvider.scope}, a sync getter the SDK requires + * (it feeds the sync `clientMetadata` getter). Scope is always set via + * {@link saveScope} in the same session before it's read (during the + * pre-redirect half of the flow), so the in-memory store has the value + * regardless of async hydration. + */ getScope(serverUrl: string): string | undefined { const state = this.store.getState().getServerState(serverUrl); return state.scope; @@ -116,7 +153,8 @@ export class OAuthStorageBase implements OAuthStorage { this.store.getState().setServerState(serverUrl, { scope: undefined }); } - getServerMetadata(serverUrl: string): OAuthMetadata | null { + async getServerMetadata(serverUrl: string): Promise { + await this.hydrated; const state = this.store.getState().getServerState(serverUrl); return state.serverMetadata || null; } diff --git a/core/auth/providers.ts b/core/auth/providers.ts index f61dc4705..6661d990e 100644 --- a/core/auth/providers.ts +++ b/core/auth/providers.ts @@ -238,8 +238,8 @@ export class BaseOAuthClientProvider implements OAuthClientProvider { await this.storage.saveCodeVerifier(this.serverUrl, codeVerifier); } - codeVerifier(): string { - const verifier = this.storage.getCodeVerifier(this.serverUrl); + async codeVerifier(): Promise { + const verifier = await this.storage.getCodeVerifier(this.serverUrl); if (!verifier) { throw new Error("No code verifier saved for session"); } @@ -250,8 +250,8 @@ export class BaseOAuthClientProvider implements OAuthClientProvider { this.storage.clear(this.serverUrl); } - getServerMetadata(): OAuthMetadata | null { - return this.storage.getServerMetadata(this.serverUrl); + async getServerMetadata(): Promise { + return await this.storage.getServerMetadata(this.serverUrl); } async saveServerMetadata(metadata: OAuthMetadata): Promise { diff --git a/core/auth/state-machine.ts b/core/auth/state-machine.ts index dc5c69187..545dfe650 100644 --- a/core/auth/state-machine.ts +++ b/core/auth/state-machine.ts @@ -204,15 +204,15 @@ export const oauthTransitions: Record = { token_request: { canTransition: async (context) => { - const hasMetadata = !!context.provider.getServerMetadata(); + const hasMetadata = !!(await context.provider.getServerMetadata()); const clientInfo = context.state.oauthClientInfo ?? (await context.provider.clientInformation()); return !!context.state.authorizationCode && hasMetadata && !!clientInfo; }, execute: async (context) => { - const codeVerifier = context.provider.codeVerifier(); - const metadata = context.provider.getServerMetadata(); + const codeVerifier = await context.provider.codeVerifier(); + const metadata = await context.provider.getServerMetadata(); if (!metadata) { throw new Error("OAuth metadata not available"); diff --git a/core/auth/storage.ts b/core/auth/storage.ts index 6cbe13b5b..47cce1391 100644 --- a/core/auth/storage.ts +++ b/core/auth/storage.ts @@ -9,6 +9,12 @@ import type { * Supports both browser (sessionStorage) and Node.js (Zustand) environments */ export interface OAuthStorage { + /** + * Resolves once any async hydration of the underlying store has completed. + * With a synchronous storage adapter this resolves immediately. + */ + ready(): Promise; + /** * Get client information (preregistered or dynamically registered) */ @@ -56,7 +62,7 @@ export interface OAuthStorage { /** * Get code verifier (for PKCE) */ - getCodeVerifier(serverUrl: string): string | undefined; + getCodeVerifier(serverUrl: string): Promise; /** * Save code verifier (for PKCE) @@ -86,7 +92,7 @@ export interface OAuthStorage { /** * Get server metadata (for guided mode) */ - getServerMetadata(serverUrl: string): OAuthMetadata | null; + getServerMetadata(serverUrl: string): Promise; /** * Save server metadata (for guided mode) From 0760266e5067139fa62e04a890cdc68f94e237c4 Mon Sep 17 00:00:00 2001 From: tobin Date: Tue, 23 Jun 2026 05:13:38 +0000 Subject: [PATCH 31/55] feat(cli): --use-stored-auth reads backend-persisted OAuth token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the web inspector completes an OAuth flow, RemoteOAuthStorage writes the access_token to ~/.mcp-inspector/storage/oauth.json. This flag reads that file via NodeOAuthStorage (same Zustand store shape, file adapter), looks up tokens for --server-url, and injects Authorization: Bearer into the outgoing transport headers — so the CLI can drive tools/list and tools/call against an OAuth-gated server after the auth was completed in a browser. NodeOAuthStorage.getTokens already awaits the file adapter's async hydration (per the previous commit), so no manual rehydrate is needed. getStateFilePath gains an MCP_INSPECTOR_OAUTH_STATE_PATH environment override so tests and scripted runs can point at an isolated fixture without touching ~/.mcp-inspector. parseArgs becomes async (its only caller, runCli, was already async). Header injection is the prototype path; wiring NodeOAuthStorage into the SDK auth provider so token refresh works is a follow-up. Tests: 4 cases via the in-process cli-runner against a real http test server — Authorization lands on the wire, merges with --header, missing --server-url errors, no-stored-token errors with the URL in the message. --- clients/cli/__tests__/stored-auth.test.ts | 130 ++++++++++++++++++++++ clients/cli/src/cli.ts | 36 +++++- core/auth/node/storage-node.ts | 12 +- 3 files changed, 172 insertions(+), 6 deletions(-) create mode 100644 clients/cli/__tests__/stored-auth.test.ts diff --git a/clients/cli/__tests__/stored-auth.test.ts b/clients/cli/__tests__/stored-auth.test.ts new file mode 100644 index 000000000..f58b70ba6 --- /dev/null +++ b/clients/cli/__tests__/stored-auth.test.ts @@ -0,0 +1,130 @@ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import { mkdtempSync, writeFileSync, rmSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { runCli } from "./helpers/cli-runner.js"; +import { expectCliFailure, expectCliSuccess } from "./helpers/assertions.js"; +import { + createTestServerHttp, + createEchoTool, + createTestServerInfo, +} from "@modelcontextprotocol/inspector-test-server"; + +/** + * Writes a Zustand-persist–shaped oauth.json fixture (the same blob the web + * inspector's RemoteOAuthStorage POSTs to /api/storage/oauth) into a temp dir + * and returns the file path. The CLI's NodeOAuthStorage reads this format via + * the same `createOAuthStore` factory. + */ +function writeOAuthFixture(servers: Record): string { + const dir = mkdtempSync(join(tmpdir(), "inspector-cli-stored-auth-")); + const file = join(dir, "oauth.json"); + writeFileSync( + file, + JSON.stringify({ state: { servers }, version: 0 }), + "utf8", + ); + return file; +} + +describe("--use-stored-auth", () => { + let server: ReturnType; + let serverUrl: string; + let fixturePath: string; + const TOKEN = "stored-access-token-abc"; + + beforeAll(async () => { + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + }); + await server.start(); + serverUrl = server.url; + fixturePath = writeOAuthFixture({ + [serverUrl]: { tokens: { access_token: TOKEN, token_type: "Bearer" } }, + }); + }); + + afterAll(async () => { + await server.stop(); + rmSync(fixturePath, { force: true }); + }); + + it("injects the stored token as Authorization: Bearer on the outgoing request", async () => { + const result = await runCli( + [ + "--cli", + "--transport", + "http", + "--server-url", + serverUrl, + "--use-stored-auth", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixturePath } }, + ); + expectCliSuccess(result); + const recorded = server.getRecordedRequests(); + expect(recorded.length).toBeGreaterThan(0); + const last = recorded[recorded.length - 1]!; + expect(last.method).toBe("tools/list"); + // Express normalises header names to lowercase. + expect(last.headers?.authorization).toBe(`Bearer ${TOKEN}`); + }); + + it("merges with --header (explicit headers + stored auth coexist)", async () => { + const result = await runCli( + [ + "--cli", + "--transport", + "http", + "--server-url", + serverUrl, + "--use-stored-auth", + "--header", + "X-Trace: abc", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixturePath } }, + ); + expectCliSuccess(result); + const last = server.getRecordedRequests().at(-1)!; + expect(last.headers?.authorization).toBe(`Bearer ${TOKEN}`); + expect(last.headers?.["x-trace"]).toBe("abc"); + }); + + it("errors clearly when --server-url is missing", async () => { + const result = await runCli( + ["--cli", "--use-stored-auth", "--method", "tools/list"], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixturePath } }, + ); + expectCliFailure(result); + expect(result.stderr).toContain("--use-stored-auth requires --server-url"); + }); + + it("errors clearly when no token is stored for the given server URL", async () => { + const empty = writeOAuthFixture({}); + try { + const result = await runCli( + [ + "--cli", + "--transport", + "http", + "--server-url", + serverUrl, + "--use-stored-auth", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: empty } }, + ); + expectCliFailure(result); + expect(result.stderr).toContain("No stored OAuth token"); + expect(result.stderr).toContain(serverUrl); + } finally { + rmSync(empty, { force: true }); + } + }); +}); diff --git a/clients/cli/src/cli.ts b/clients/cli/src/cli.ts index ddefb9d2a..e0059b389 100644 --- a/clients/cli/src/cli.ts +++ b/clients/cli/src/cli.ts @@ -286,11 +286,11 @@ function parseKeyValuePair( return { ...previous, [key as string]: parsedValue }; } -function parseArgs(argv?: string[]): { +async function parseArgs(argv?: string[]): Promise<{ serverConfig: MCPServerConfig; serverSettings: InspectorServerSettings | undefined; methodArgs: MethodArgs & { method: string }; -} { +}> { const program = new Command(); // On a parse/usage ERROR (exitCode !== 0), throw the CommanderError instead // of letting commander call process.exit(). The binary entry (index.ts) still @@ -414,6 +414,10 @@ function parseArgs(argv?: string[]): { .option( "--app-info", "Probe the tool's MCP App UI metadata (resourceUri, csp, permissions, domain) and emit it as one JSON line; exit 2 when the tool has no app. Use with --method tools/call --tool-name ; the tool itself is not invoked.", + ) + .option( + "--use-stored-auth", + "Read the OAuth access token for --server-url from ~/.mcp-inspector/storage/oauth.json (written by the web inspector) and inject it as Authorization: Bearer.", ); program.parse(preArgs); @@ -437,6 +441,7 @@ function parseArgs(argv?: string[]): { serverUrl?: string; header?: Record; appInfo?: boolean; + useStoredAuth?: boolean; }; const serverOptions = { @@ -454,6 +459,31 @@ function parseArgs(argv?: string[]): { headers: options.header, }; + if (options.useStoredAuth) { + if (!options.serverUrl) { + throw new Error("--use-stored-auth requires --server-url"); + } + // NodeOAuthStorage reads ~/.mcp-inspector/storage/oauth.json (or + // MCP_INSPECTOR_OAUTH_STATE_PATH when set) via the same Zustand store the + // web inspector's RemoteOAuthStorage writes to. getTokens awaits the file + // adapter's hydration, so this works even though the read is async. Header + // injection is the prototype path — wiring NodeOAuthStorage into the SDK + // auth provider (so refresh works) is a follow-up. + const { NodeOAuthStorage } = + await import("@inspector/core/auth/node/storage-node.js"); + const oauthStorage = new NodeOAuthStorage(); + const tokens = await oauthStorage.getTokens(options.serverUrl); + if (!tokens?.access_token) { + throw new Error( + `No stored OAuth token for ${options.serverUrl} in ~/.mcp-inspector/storage/oauth.json. Complete the OAuth flow in the web inspector first.`, + ); + } + serverOptions.headers = { + ...(serverOptions.headers ?? {}), + Authorization: `Bearer ${tokens.access_token}`, + }; + } + // Shared with the TUI: resolves the catalog/config source (or ad-hoc target), // enforces the conflict matrix, and lifts disk headers/timeouts/OAuth into // per-server settings. `--server` selects one when the file has several. @@ -510,7 +540,7 @@ function parseArgs(argv?: string[]): { } export async function runCli(argv?: string[]): Promise { - const { serverConfig, serverSettings, methodArgs } = parseArgs( + const { serverConfig, serverSettings, methodArgs } = await parseArgs( argv ?? process.argv, ); await callMethod(serverConfig, serverSettings, methodArgs); diff --git a/core/auth/node/storage-node.ts b/core/auth/node/storage-node.ts index 952cb13b4..8e66ffe66 100644 --- a/core/auth/node/storage-node.ts +++ b/core/auth/node/storage-node.ts @@ -10,11 +10,17 @@ import { const DEFAULT_STATE_PATH = getStoreFilePath(getDefaultStorageDir(), "oauth"); /** - * Get path to OAuth state file. - * @param customPath - Optional custom path (full path to state file). Default: ~/.mcp-inspector/storage/oauth.json + * Get path to OAuth state file. Resolution order: explicit `customPath`, then + * the `MCP_INSPECTOR_OAUTH_STATE_PATH` environment variable (so tests and + * scripted runs can point at an isolated fixture without touching + * `~/.mcp-inspector`), then the default `~/.mcp-inspector/storage/oauth.json`. */ export function getStateFilePath(customPath?: string): string { - return customPath ?? DEFAULT_STATE_PATH; + return ( + customPath ?? + process.env.MCP_INSPECTOR_OAUTH_STATE_PATH ?? + DEFAULT_STATE_PATH + ); } const storeCache = new Map>(); From 9e7acbf21528f92b6e6cdd689672a7e6715b0beb Mon Sep 17 00:00:00 2001 From: tobin Date: Tue, 23 Jun 2026 05:15:31 +0000 Subject: [PATCH 32/55] fix(core/auth): refuse to generate OAuth state without a cryptographic RNG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit generateOAuthState produces the OAuth CSRF state parameter. The previous Math.random fallback would silently emit a predictable value when crypto.getRandomValues is unavailable. Every supported runtime (browsers, Node ≥15) provides it globally, so throw instead of degrading. --- core/auth/utils.ts | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/core/auth/utils.ts b/core/auth/utils.ts index 5b6799f9c..07a833f8c 100644 --- a/core/auth/utils.ts +++ b/core/auth/utils.ts @@ -36,20 +36,17 @@ export const parseOAuthCallbackParams = (location: string): CallbackParams => { * @returns A random state for the OAuth 2.0 flow. */ export const generateOAuthState = (): string => { - // Generate a random state - const array = new Uint8Array(32); - - // Use crypto.getRandomValues (available in both browser and Node.js) - if (typeof crypto !== "undefined" && crypto.getRandomValues) { - crypto.getRandomValues(array); - } else { - // Fallback for environments without crypto.getRandomValues - // This should not happen in modern environments - for (let i = 0; i < array.length; i++) { - array[i] = Math.floor(Math.random() * 256); - } + // OAuth state is a CSRF token — it MUST be unpredictable. crypto.getRandomValues + // is available in every supported runtime (browsers, Node ≥15); if it's somehow + // missing, fail loudly rather than silently degrading to Math.random (whose + // output is predictable from a small amount of observed state). + if (typeof crypto === "undefined" || !crypto.getRandomValues) { + throw new Error( + "crypto.getRandomValues is not available; refusing to generate an OAuth state with a non-cryptographic RNG.", + ); } - + const array = new Uint8Array(32); + crypto.getRandomValues(array); return Array.from(array, (byte) => byte.toString(16).padStart(2, "0")).join( "", ); From af8b008a555c0ce53c01981004c6f56abbc66674 Mon Sep 17 00:00:00 2001 From: tobin Date: Tue, 23 Jun 2026 16:26:04 +0000 Subject: [PATCH 33/55] fix(web/apps): drop allow-same-origin from app sandbox; deliver via srcdoc; harden proxy CSP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inner iframe previously ran with sandbox="allow-scripts allow-same-origin allow-forms" and was populated via document.open()/write(). allow-scripts + allow-same-origin on a same-origin embed lets the untrusted widget reach window.parent (this proxy page), which had no CSP of its own — so a widget could bypass its declared per-app CSP entirely by executing in the parent's realm (window.parent.fetch(...), or stripping its own sandbox attribute and reloading). - Inner frame now sandboxed without allow-same-origin → opaque ("null") origin; the widget cannot touch the proxy's DOM. - HTML delivered via srcdoc (document.write requires the same-origin access we no longer grant). Known trade-off: a small set of libraries (CesiumJS Map) don't initialise under srcdoc; accepted for the isolation guarantee. - Server-supplied sandbox values have allow-same-origin stripped before applying, so a malicious _meta.ui.sandbox cannot request it back. - Relay's inner-frame branch now expects event.origin === "null" (event.source === inner.contentWindow stays the unforgeable identity check). - sandbox-controller serves the proxy with its own CSP header (default-src 'none'; frame-ancestors http://127.0.0.1:* http://localhost:*) — defense-in-depth so any future regression that re-grants same-origin still gains nothing. - Integration test asserts the CSP header on /sandbox. --- clients/web/server/sandbox-controller.ts | 17 ++++ .../server/sandbox-controller.test.ts | 7 ++ clients/web/static/sandbox_proxy.html | 85 +++++++++++-------- 3 files changed, 72 insertions(+), 37 deletions(-) diff --git a/clients/web/server/sandbox-controller.ts b/clients/web/server/sandbox-controller.ts index 16ae5d3c0..5c6491b60 100644 --- a/clients/web/server/sandbox-controller.ts +++ b/clients/web/server/sandbox-controller.ts @@ -47,6 +47,22 @@ export function createSandboxController( let server: Server | null = null; let sandboxUrl: string | null = null; + // Defense-in-depth for the proxy page itself: the inner iframe runs the + // untrusted app under an opaque origin (no allow-same-origin), so it cannot + // reach this document — but if a future change accidentally re-grants that, + // this header ensures escaping into the proxy still cannot fetch, embed + // remote script/style, or be framed by anything except the local inspector. + // The proxy's own bootstrap script and style are inline, hence the + // 'unsafe-inline' on those two directives only. frame-src is unrestricted so + // the inner srcdoc iframe (and any data:/blob: variants) can load. + const SANDBOX_PROXY_CSP = [ + "default-src 'none'", + "script-src 'unsafe-inline'", + "style-src 'unsafe-inline'", + "frame-src data: blob: *", + "frame-ancestors http://127.0.0.1:* http://localhost:*", + ].join("; "); + let sandboxHtml: string; try { const sandboxHtmlPath = join(__dirname, "../static/sandbox_proxy.html"); @@ -88,6 +104,7 @@ export function createSandboxController( "Content-Type": "text/html; charset=utf-8", "Cache-Control": "no-store, no-cache, must-revalidate", Pragma: "no-cache", + "Content-Security-Policy": SANDBOX_PROXY_CSP, }); res.end(sandboxHtml); }); diff --git a/clients/web/src/test/integration/server/sandbox-controller.test.ts b/clients/web/src/test/integration/server/sandbox-controller.test.ts index 33455b320..3bb5ca2b5 100644 --- a/clients/web/src/test/integration/server/sandbox-controller.test.ts +++ b/clients/web/src/test/integration/server/sandbox-controller.test.ts @@ -79,6 +79,13 @@ describe("createSandboxController", () => { const res = await fetch(url); expect(res.status).toBe(200); expect(res.headers.get("content-type")).toContain("text/html"); + // Defense-in-depth CSP on the proxy itself: the inner sandboxed frame is + // opaque-origin, but if a future change re-grants allow-same-origin, + // this header is what stops an escaped widget from fetching or being + // re-framed by a non-local page. + const csp = res.headers.get("content-security-policy") ?? ""; + expect(csp).toContain("default-src 'none'"); + expect(csp).toContain("frame-ancestors http://127.0.0.1:*"); const body = await res.text(); // Either the real proxy file (sandbox-resource-ready) or the fallback // "Sandbox not loaded" string, depending on whether static/ resolves. diff --git a/clients/web/static/sandbox_proxy.html b/clients/web/static/sandbox_proxy.html index 171625ae4..46e46b0de 100644 --- a/clients/web/static/sandbox_proxy.html +++ b/clients/web/static/sandbox_proxy.html @@ -3,23 +3,25 @@ - + src/lib/sandbox-csp.ts). The proxy assigns that pre-wrapped document + to `srcdoc` verbatim — it never parses or injects into the untrusted + bytes — so the CSP `` is guaranteed to be the first `` + child of the inner document. --> MCP-UI Proxy