From 8e8c1bbad9b17217f4f77e7de434e14b32bf6f17 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 23 Aug 2026 23:21:41 -0400 Subject: [PATCH] test(web): prove the dedicated app origin end to end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `frame-ancestors` is checked against every ancestor, not just the parent. A published app document is framed by the sandbox proxy, which is framed by the Inspector page — so admitting only the proxy's origin blocked the frame outright (it rendered as a chrome-error frame that never reached the bridge). `appDocumentEmbedders` now admits both. That failure was invisible to every unit test, so `smoke:web:app` grows a second phase: the same demo app declaring `_meta.ui.domain`, driven through the same connect → open → ready chain, then asserting the two facts that only hold on the dedicated path — the inner frame was navigated to a published document, and its own `location.origin` is a real http origin rather than "null". Both phases end at `data-app-status="ready"`, so nothing short of reading the frame's origin can tell them apart. Adds `test-servers/configs/mcp-app-domain-http.json` (and a `domain` param on the `mcp_app_demo_widget` preset) as the showcase server for both the smoke and hand testing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh Signed-off-by: cliffhall --- clients/web/server/app-origin-controller.ts | 36 +++-- clients/web/server/server.ts | 7 +- clients/web/server/vite-hono-plugin.ts | 7 +- .../server/app-origin-controller.test.ts | 29 +++- scripts/smoke-web-app.mjs | 124 ++++++++++++++++-- test-servers/configs/mcp-app-domain-http.json | 17 +++ test-servers/src/preset-registry.ts | 2 +- test-servers/src/test-server-fixtures.ts | 10 +- 8 files changed, 199 insertions(+), 33 deletions(-) create mode 100644 test-servers/configs/mcp-app-domain-http.json diff --git a/clients/web/server/app-origin-controller.ts b/clients/web/server/app-origin-controller.ts index 6f274e31..f455542a 100644 --- a/clients/web/server/app-origin-controller.ts +++ b/clients/web/server/app-origin-controller.ts @@ -75,23 +75,33 @@ const MAX_DOCUMENTS = 32; const DOCUMENT_TTL_MS = 60 * 60 * 1000; /** - * The sandbox proxy's origin as a one-entry list for - * {@link AppOriginControllerOptions.embedderOrigins}, or `undefined` when the - * sandbox never bound (its controller degrades to a null URL). Passing - * `undefined` is what makes the app-origin `frame-ancestors` fall back to - * loopback rather than to `'none'`, which would block the frame outright. + * The origins that may appear in a published document's ancestor chain, for + * {@link AppOriginControllerOptions.embedderOrigins}. + * + * There are **two**, and getting this wrong blocks the frame outright: CSP's + * `frame-ancestors` is checked against *every* ancestor, not just the immediate + * parent. A published document is framed by the sandbox proxy, which is itself + * framed by the Inspector app page — so the directive has to admit the proxy's + * origin **and** the app's, which is exactly the backend's origin allow-list. + * + * `undefined` when neither is known, which makes the directive fall back to the + * loopback family rather than to `'none'`. */ -export function sandboxOriginList( +export function appDocumentEmbedders( sandboxUrl: string | null | undefined, + allowedOrigins?: string[], ): string[] | undefined { - if (!sandboxUrl) return undefined; - try { - return [new URL(sandboxUrl).origin]; - } catch { - /* v8 ignore next -- the sandbox controller only ever emits a URL it built - itself; an unparseable value would be a bug there, not input. */ - return undefined; + const origins: string[] = []; + if (sandboxUrl) { + try { + origins.push(new URL(sandboxUrl).origin); + } catch { + /* v8 ignore next -- the sandbox controller only ever emits a URL it built + itself; an unparseable value would be a bug there, not input. */ + } } + origins.push(...(allowedOrigins ?? [])); + return origins.length > 0 ? origins : undefined; } export interface AppOriginControllerOptions { diff --git a/clients/web/server/server.ts b/clients/web/server/server.ts index 3a43e5b3..f128685c 100644 --- a/clients/web/server/server.ts +++ b/clients/web/server/server.ts @@ -18,7 +18,7 @@ import { formatHostForUrl } from "../../../core/node/hostUrl.ts"; import { createSandboxController } from "./sandbox-controller.js"; import { createAppOriginController, - sandboxOriginList, + appDocumentEmbedders, } from "./app-origin-controller.js"; import { injectAuthToken } from "./inject-auth-token.js"; import type { WebServerConfig } from "./web-server-config.js"; @@ -54,7 +54,10 @@ export async function startHonoServer( const appOriginController = createAppOriginController({ port: config.appOriginPort, host: config.sandboxHost, - embedderOrigins: sandboxOriginList(sandboxController.getUrl()), + embedderOrigins: appDocumentEmbedders( + sandboxController.getUrl(), + config.allowedOrigins, + ), }); await appOriginController.start(); diff --git a/clients/web/server/vite-hono-plugin.ts b/clients/web/server/vite-hono-plugin.ts index c38f3fca..59cee730 100644 --- a/clients/web/server/vite-hono-plugin.ts +++ b/clients/web/server/vite-hono-plugin.ts @@ -18,7 +18,7 @@ import { createRemoteApp } from "../../../core/mcp/remote/node/server.ts"; import { createSandboxController } from "./sandbox-controller.js"; import { createAppOriginController, - sandboxOriginList, + appDocumentEmbedders, } from "./app-origin-controller.js"; import { injectAuthToken } from "./inject-auth-token.js"; import type { WebServerConfig } from "./web-server-config.js"; @@ -77,7 +77,10 @@ export function honoMiddlewarePlugin(config: WebServerConfig): Plugin { const appOriginController = createAppOriginController({ port: config.appOriginPort, host: config.sandboxHost, - embedderOrigins: sandboxOriginList(sandboxController.getUrl()), + embedderOrigins: appDocumentEmbedders( + sandboxController.getUrl(), + config.allowedOrigins, + ), }); await appOriginController.start(); // Resolved before the API is built so `/api/config` and the banner diff --git a/clients/web/src/test/integration/server/app-origin-controller.test.ts b/clients/web/src/test/integration/server/app-origin-controller.test.ts index 098be332..dced7e5e 100644 --- a/clients/web/src/test/integration/server/app-origin-controller.test.ts +++ b/clients/web/src/test/integration/server/app-origin-controller.test.ts @@ -19,7 +19,7 @@ import { DEFAULT_APP_ORIGIN_PORT, parseDocumentId, resolveAppOriginPort, - sandboxOriginList, + appDocumentEmbedders, } from "../../../../server/app-origin-controller.js"; describe("resolveAppOriginPort", () => { @@ -85,20 +85,37 @@ describe("parseDocumentId", () => { }); }); -describe("sandboxOriginList", () => { - it("reduces the sandbox URL to its origin", () => { - expect(sandboxOriginList("http://127.0.0.1:6275/sandbox")).toEqual([ +describe("appDocumentEmbedders", () => { + it("admits the sandbox proxy AND the Inspector page", () => { + // `frame-ancestors` is checked against EVERY ancestor, not just the parent: + // the document is framed by the proxy, which is framed by the app page. + // Omitting the app page's origin blocks the frame outright (it renders as a + // chrome-error frame that never reaches the bridge). + expect( + appDocumentEmbedders("http://127.0.0.1:6275/sandbox", [ + "http://127.0.0.1:6274", + "http://localhost:6274", + ]), + ).toEqual([ "http://127.0.0.1:6275", + "http://127.0.0.1:6274", + "http://localhost:6274", + ]); + }); + + it("still returns the allow-list when the sandbox never bound", () => { + expect(appDocumentEmbedders(null, ["http://127.0.0.1:6274"])).toEqual([ + "http://127.0.0.1:6274", ]); }); it.each([[null], [undefined], [""]])( - "returns undefined for %j so frame-ancestors falls back to loopback", + "returns undefined for %j with no allow-list, so frame-ancestors falls back to loopback", (value) => { // Not `[]`: an empty list would make the directive `'none'` and block the // app frame outright, which is worse than a permissive loopback fallback. expect( - sandboxOriginList(value as string | null | undefined), + appDocumentEmbedders(value as string | null | undefined), ).toBeUndefined(); }, ); diff --git a/scripts/smoke-web-app.mjs b/scripts/smoke-web-app.mjs index b849f357..485f9e21 100644 --- a/scripts/smoke-web-app.mjs +++ b/scripts/smoke-web-app.mjs @@ -8,6 +8,13 @@ * unexercised by any smoke. This closes that gap: it drives the full * **connect → open app → widget ready** chain against a real MCP App server. * + * A second phase (#2056) drives the same app declaring `_meta.ui.domain`, and + * asserts the thing that field exists for: the app document is served from the + * backend's dedicated app-origin listener and runs at a REAL origin, not the + * opaque one whose requests carry `Origin: null`. Both phases end at + * `data-app-status="ready"`, so only reading the inner frame's own + * `location.origin` can tell them apart. + * * The assertion is the `data-app-status="ready"` contract documented in * clients/web/README.md ("MCP Apps screen automation contract"): the renderer * only reports `ready` once the widget has loaded inside the sandbox iframe and @@ -72,6 +79,14 @@ const appConfig = join( "configs", "mcp-app-http.json", ); +// The same demo app, with `_meta.ui.domain` declared (#2056). Phase 2 drives it +// to prove the dedicated-origin path end to end. +const appDomainConfig = join( + repoRoot, + "test-servers", + "configs", + "mcp-app-domain-http.json", +); // The path clients/web/server/sandbox-controller.ts resolves at runtime, from // the built runner at clients/web/build/. Kept in sync with the `join(__dirname, // "../static/sandbox_proxy.html")` there. @@ -104,6 +119,7 @@ const FATAL_CONSOLE = /^Uncaught\b|Failed to fetch dynamically imported module/; let mcpUrl = null; let mcpServer = null; +let domainMcpServer = null; let browser = null; const server = startProdWebServer({ host: HOST, @@ -127,6 +143,14 @@ async function shutdown() { mcpServer = null; await stopChild(child, { label: "smoke:web:app", what: "MCP test server" }); } + if (domainMcpServer) { + const child = domainMcpServer; + domainMcpServer = null; + await stopChild(child, { + label: "smoke:web:app", + what: "MCP test server (domain)", + }); + } } async function fail(message) { @@ -176,15 +200,18 @@ function ensureTestServer() { * createTestServerHttp resolves through findAvailablePort(), which walks upward * when the configured value is taken. */ -async function startMcpServer() { +async function startMcpServer( + config = appConfig, + publish = (child) => { + mcpServer = child; + }, +) { const { match } = await startAnnouncedChild({ command: process.execPath, - args: [composableServer, "--config", appConfig], + args: [composableServer, "--config", config], cwd: repoRoot, pattern: /listening at (http:\/\/\S+)/i, - onSpawn: (child) => { - mcpServer = child; - }, + onSpawn: publish, what: `MCP test server (${composableServer})`, }); return { url: match[1] }; @@ -256,10 +283,11 @@ try { // Deep link: connect, switch to the Apps tab, pre-select the app tool, and // fire "Open App". autoConnect/autoOpen must equal the session token (CSRF // gate). Shape owned by clients/web/README.md#deep-link-auto-connect. - const url = - `${server.baseUrl}/?serverUrl=${encodeURIComponent(mcpUrl)}` + + const deepLink = (target) => + `${server.baseUrl}/?serverUrl=${encodeURIComponent(target)}` + `&transport=http&autoConnect=${TOKEN}&openApp=${APP_TOOL}` + `&appArgs=${encodeAppArgs({ title: "smoke:web:app" })}&autoOpen=${TOKEN}`; + const url = deepLink(mcpUrl); const drive = async () => { const response = await page.goto(url, { @@ -309,10 +337,88 @@ try { } }; + /** + * Phase 2 (#2056): the same app, this time declaring `_meta.ui.domain`. + * + * Phase 1 proves the default render works; it cannot distinguish it from the + * dedicated-origin render, because both end at `data-app-status="ready"`. The + * thing #2056 is actually about is the *origin the app document runs at* — an + * opaque origin sends `Origin: null`, which no CORS / OAuth-callback / + * API-key allowlist can admit. So this phase asserts the two facts that only + * hold on the dedicated path: the inner frame was navigated to a published + * document on the backend's app-origin listener, and the document's own + * `location.origin` is a real origin rather than the literal "null". + * + * It reuses the running web server and browser page — only a second MCP + * server and a second deep-link navigate are added. + */ + const driveDedicatedOrigin = async () => { + const { url: domainUrl } = await startMcpServer( + appDomainConfig, + (child) => { + domainMcpServer = child; + }, + ); + await page.goto(deepLink(domainUrl), { + waitUntil: "domcontentloaded", + timeout: 30_000, + }); + await page + .locator('[data-testid="connection-status"][data-status="connected"]') + .waitFor({ state: "attached", timeout: 45_000 }); + try { + await page + .locator('[data-testid="apps-form"][data-app-status="ready"]') + .waitFor({ state: "attached", timeout: 45_000 }); + } catch { + const form = page.locator('[data-testid="apps-form"]'); + const appStatus = (await form.count()) + ? await form.getAttribute("data-app-status") + : "(no apps-form)"; + const appError = (await form.count()) + ? await form.getAttribute("data-app-error") + : null; + throw new Error( + `dedicated-origin app never reached data-app-status="ready" (last: ` + + `"${appStatus}"${appError ? `, data-app-error="${appError}"` : ""}) — ` + + `frames: ${page + .frames() + .map((f) => f.url() || "(blank)") + .join(", ")}`, + ); + } + + const appFrame = page + .frames() + .find((f) => /\/app-document\/[0-9a-f]{32}$/.test(f.url())); + if (!appFrame) { + throw new Error( + "app declaring _meta.ui.domain was not served from the dedicated app " + + `origin — no frame at /app-document/ (frames: ${page + .frames() + .map((f) => f.url() || "(blank)") + .join(", ")})`, + ); + } + const origin = await appFrame.evaluate(() => window.location.origin); + if (!/^http:\/\/[^/]+$/.test(origin)) { + throw new Error( + `app document ran at origin "${origin}" — expected a real http origin. ` + + "An opaque origin sends `Origin: null`, which is the bug #2056 fixes.", + ); + } + return { origin, url: appFrame.url() }; + }; + // Race against launcher death so a mid-run server crash is reported as the // real cause instead of a downstream timeout. + let dedicated = null; try { await Promise.race([server.whenChildExits(), drive()]); + dedicated = await Promise.race([ + server.whenChildExits(), + driveDedicatedOrigin(), + ]); } catch (err) { const diagnostics = [ ...pageErrors, @@ -345,7 +451,9 @@ try { console.log( `smoke:web:app OK — connected to ${mcpUrl}, opened "${APP_TOOL}", ` + - `widget reached data-app-status="ready" through the sandbox proxy`, + `widget reached data-app-status="ready" through the sandbox proxy; ` + + `and an app declaring _meta.ui.domain rendered from the dedicated origin ` + + `${dedicated?.origin} (${dedicated?.url})`, ); await shutdown(); process.exit(0); diff --git a/test-servers/configs/mcp-app-domain-http.json b/test-servers/configs/mcp-app-domain-http.json new file mode 100644 index 00000000..6d99d514 --- /dev/null +++ b/test-servers/configs/mcp-app-domain-http.json @@ -0,0 +1,17 @@ +{ + "serverInfo": { + "name": "mcp-app-domain-showcase", + "version": "1.0.0" + }, + "tools": [{ "preset": "echo" }, { "preset": "mcp_app_demo" }], + "resources": [ + { + "preset": "mcp_app_demo_widget", + "params": { "domain": "my-app-domain.example.com" } + } + ], + "transport": { + "type": "streamable-http", + "port": 3134 + } +} diff --git a/test-servers/src/preset-registry.ts b/test-servers/src/preset-registry.ts index b535bb20..f3f4bdfc 100644 --- a/test-servers/src/preset-registry.ts +++ b/test-servers/src/preset-registry.ts @@ -253,7 +253,7 @@ function resolveResourcePreset( case "numbered_resources": return createNumberedResources(Number(get("count")) || 3); case "mcp_app_demo_widget": - return createMcpAppDemoResource(); + return createMcpAppDemoResource(get("domain") as string | undefined); case "choose_option_app": return createAppElicitationResource(); case "rfc6570_base": diff --git a/test-servers/src/test-server-fixtures.ts b/test-servers/src/test-server-fixtures.ts index 5022e0f3..2cc29cf2 100644 --- a/test-servers/src/test-server-fixtures.ts +++ b/test-servers/src/test-server-fixtures.ts @@ -1617,8 +1617,15 @@ export function createMcpAppDemoTool(): ToolDefinition { * `_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. + * + * `domain` is the spec field by which a server asks its host for a stable, + * dedicated origin (#2056). Omitted by default so the default opaque-origin + * render stays the thing this fixture exercises; pass one to drive the + * dedicated-origin path instead. Its *value* is not an address the Inspector + * serves — the spec makes the format host-dependent, and the Inspector reads + * any non-empty string as "give me a real origin". */ -export function createMcpAppDemoResource(): ResourceDefinition { +export function createMcpAppDemoResource(domain?: string): ResourceDefinition { return { name: "mcp_app_demo_widget", uri: MCP_APP_DEMO_URI, @@ -1630,6 +1637,7 @@ export function createMcpAppDemoResource(): ResourceDefinition { csp: { connectDomains: [], resourceDomains: [] }, permissions: { clipboard: false }, prefersBorder: true, + ...(domain ? { domain } : {}), }, }, };