From 8f6a2446dc3f2e050af1627e57e22b4ba4388730 Mon Sep 17 00:00:00 2001 From: Ben Vinegar <2153+benvinegar@users.noreply.github.com> Date: Fri, 19 Jun 2026 14:36:54 -0400 Subject: [PATCH] harden(viewer): gate host-affecting bridge messages + pin CSP isolation in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The postMessage bridge honored switch-session and open-link from any frame, while resize/send-prompt were already gated to a recognized surface frame. Gate the former two on isOwnFrame(ev.source) so a stray or nested frame can't drive session navigation or pop an open-link dialog. switch-session is sent only by html frames, but open-link is also sent by rich-part frames (which are not in the html-only frameForSource registry), so isOwnFrame recognizes any iframe the viewer embedded. Also pin the load-bearing isolation guarantee directly, where it was only covered by the sandbox attribute as a proxy: - unit: the board origin is never a connect-src/script-src source (img/media only) — the exact exfil hole that 'self'/wildcard/`https:` checks miss. - e2e: script inside an html part is CSP-blocked from fetching the board API, asserted on real Chromium and WebKit via a self-reporting probe. Co-Authored-By: Claude Opus 4.8 (1M context) --- .changeset/contain-csp-and-bridge-source.md | 13 ++++++++ e2e/isolation.spec.ts | 36 +++++++++++++++++++++ test/surfacePage.test.ts | 25 ++++++++++++++ viewer/src/App.tsx | 26 +++++++++++++-- 4 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 .changeset/contain-csp-and-bridge-source.md create mode 100644 e2e/isolation.spec.ts diff --git a/.changeset/contain-csp-and-bridge-source.md b/.changeset/contain-csp-and-bridge-source.md new file mode 100644 index 0000000..d4fdba9 --- /dev/null +++ b/.changeset/contain-csp-and-bridge-source.md @@ -0,0 +1,13 @@ +--- +"sideshow": patch +--- + +Harden surface isolation against regressions and stray frames. The viewer's +postMessage bridge now only honors host-affecting messages (`switch-session`, +`open-link`) from a frame the viewer actually embedded, matching the source +check `resize`/`send-prompt` already enforced — so a stray or nested frame can't +drive session navigation or pop an open-link dialog. New tests pin the +load-bearing guarantee directly: a unit test asserts the board origin is never a +`connect-src`/`script-src` source (only `img-src`/`media-src`, for asset +embedding), and an e2e test proves on real Chromium and WebKit that script +running inside an html part is CSP-blocked from fetching the board API. diff --git a/e2e/isolation.spec.ts b/e2e/isolation.spec.ts new file mode 100644 index 0000000..0e3eec7 --- /dev/null +++ b/e2e/isolation.spec.ts @@ -0,0 +1,36 @@ +import { expect, publish, test } from "./fixtures.ts"; + +// The sandbox attribute (asserted across the part specs) is the *shape* of the +// isolation; this spec asserts the *behavior* the project's core invariant +// promises: script that runs inside an html part cannot reach the board API, +// because the CSP connect-src omits the server origin. A regression that put the +// board origin back into connect-src (or dropped the CSP meta tag) would keep +// the sandbox attribute intact and pass every other test while silently opening +// exfil — this is the test that catches it, on real Chromium and WebKit. +// +// The probe can't phone home (that's the point), so it self-reports the outcome +// into its own DOM; Playwright reads that across the opaque origin. +const PROBE = `
running
+`; + +test("an html part's script is CSP-blocked from fetching the board API", async ({ + page, + server, +}) => { + await publish(server.url, { html: PROBE, title: "probe", agent: "e2e" }); + + await page.goto(server.url); + const card = page.locator(".card:not(#whatsNew)").first(); + const probe = card.frameLocator("iframe").locator("#r"); + + // the fetch is refused before it leaves the frame -> the catch runs + await expect(probe).toHaveText("blocked", { timeout: 10_000 }); + // and it must never have succeeded + await expect(probe).not.toContainText("LEAKED"); +}); diff --git a/test/surfacePage.test.ts b/test/surfacePage.test.ts index d59019c..a904ba2 100644 --- a/test/surfacePage.test.ts +++ b/test/surfacePage.test.ts @@ -159,6 +159,31 @@ test("html parts keep their CDN allowlist (rich-part tightening did not leak)", assert.ok("connect-src" in html, "html parts still have connect-src"); }); +test("the board origin is never a connect/script source — img/media only", () => { + // The server origin is deliberately in img-src/media-src so uploaded assets + // embed by URL. It must NEVER reach connect-src or script-src: that origin + // serves the authenticated board API and the comment->agent channel, so a + // contained script that could fetch it would defeat the whole sandbox. This + // is the exact exfil hole the existing 'self'/wildcard/`https:` checks miss — + // localhost:4000 is none of those, so it would slip past them. + for (const make of [ + () => renderHtmlPage({ title: "t", html: "

x

", origin: ORIGIN }), + () => renderSandboxedPart({ body: "x", css: "", origin: ORIGIN }), + ]) { + const d = cspDirectives(make()); + assert.ok( + !(d["connect-src"] ?? []).includes(ORIGIN), + "board origin must not be a connect source", + ); + assert.ok( + !(d["script-src"] ?? []).includes(ORIGIN), + "board origin must not be a script source", + ); + // it is present where it's meant to be, so this test can't pass vacuously + assert.ok(d["img-src"]?.includes(ORIGIN), "board origin should still embed images"); + } +}); + test("escapeHtml neutralizes markup metacharacters", () => { assert.equal( escapeHtml(``), diff --git a/viewer/src/App.tsx b/viewer/src/App.tsx index b373849..f23debb 100644 --- a/viewer/src/App.tsx +++ b/viewer/src/App.tsx @@ -255,9 +255,17 @@ async function onBridgeMessage(ev: MessageEvent) { key?: string; } | null; if (!d || !d.__sideshow) return; - // A surface iframe forwarded the session-switch shortcut because focus was - // inside it (see server/surfacePage.ts). Mirror the parent keydown handler. + // Every host-affecting message must come from a frame the viewer actually + // embedded — never an unexpected/nested frame. send-prompt and resize prove + // this implicitly (frameForSource resolves the exact html frame); the + // remaining types reach the host UI directly, so gate them on isOwnFrame. + // (frameForSource only knows html-part frames; switch-session is sent only by + // those, but open-link is sent by rich-part frames too, so use the broader + // check that recognizes any embedded iframe.) if (d.type === "switch-session") { + if (!isOwnFrame(ev.source)) return; + // A surface iframe forwarded the session-switch shortcut because focus was + // inside it (see server/surfacePage.ts). Mirror the parent keydown handler. void selectAdjacent(d.key === "ArrowUp" ? -1 : 1); return; } @@ -272,11 +280,23 @@ async function onBridgeMessage(ev: MessageEvent) { body: JSON.stringify({ surface: src.id, text: String(d.text), author: "user" }), }); toast("Sent to agent: " + d.text); - } else if (d.type === "open-link") { + } else if (d.type === "open-link" && isOwnFrame(ev.source)) { if (confirm(`Open external link?\n\n${d.url}`)) window.open(d.url, "_blank", "noopener"); } } +// True when `source` is the contentWindow of an iframe the viewer embedded +// (html or rich part). frameForSource only tracks html-part frames; this is the +// broader gate for messages rich-part frames also send (open-link). Identity +// comparison works across the opaque-origin boundary even though the frame's +// document is unreadable. +function isOwnFrame(source: unknown): boolean { + for (const f of document.querySelectorAll("iframe")) { + if (f.contentWindow === source) return true; + } + return false; +} + function SessionItem(props: { session: SessionRow }) { const label = () => sessionLabel(props.session); return (