Skip to content

Commit fdf4ffc

Browse files
benvinegarclaude
andauthored
harden(viewer): gate host-affecting bridge messages + pin CSP isolation in tests (#75)
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) <noreply@anthropic.com>
1 parent 0d87069 commit fdf4ffc

4 files changed

Lines changed: 97 additions & 3 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"sideshow": patch
3+
---
4+
5+
Harden surface isolation against regressions and stray frames. The viewer's
6+
postMessage bridge now only honors host-affecting messages (`switch-session`,
7+
`open-link`) from a frame the viewer actually embedded, matching the source
8+
check `resize`/`send-prompt` already enforced — so a stray or nested frame can't
9+
drive session navigation or pop an open-link dialog. New tests pin the
10+
load-bearing guarantee directly: a unit test asserts the board origin is never a
11+
`connect-src`/`script-src` source (only `img-src`/`media-src`, for asset
12+
embedding), and an e2e test proves on real Chromium and WebKit that script
13+
running inside an html part is CSP-blocked from fetching the board API.

e2e/isolation.spec.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { expect, publish, test } from "./fixtures.ts";
2+
3+
// The sandbox attribute (asserted across the part specs) is the *shape* of the
4+
// isolation; this spec asserts the *behavior* the project's core invariant
5+
// promises: script that runs inside an html part cannot reach the board API,
6+
// because the CSP connect-src omits the server origin. A regression that put the
7+
// board origin back into connect-src (or dropped the CSP meta tag) would keep
8+
// the sandbox attribute intact and pass every other test while silently opening
9+
// exfil — this is the test that catches it, on real Chromium and WebKit.
10+
//
11+
// The probe can't phone home (that's the point), so it self-reports the outcome
12+
// into its own DOM; Playwright reads that across the opaque origin.
13+
const PROBE = `<div id="r">running</div>
14+
<script>
15+
// Relative URL resolves against the frame's document (the board origin), so
16+
// this targets the authenticated API. connect-src must refuse it.
17+
fetch('/api/surfaces')
18+
.then(function (res) { document.getElementById('r').textContent = 'LEAKED status ' + res.status; })
19+
.catch(function () { document.getElementById('r').textContent = 'blocked'; });
20+
</script>`;
21+
22+
test("an html part's script is CSP-blocked from fetching the board API", async ({
23+
page,
24+
server,
25+
}) => {
26+
await publish(server.url, { html: PROBE, title: "probe", agent: "e2e" });
27+
28+
await page.goto(server.url);
29+
const card = page.locator(".card:not(#whatsNew)").first();
30+
const probe = card.frameLocator("iframe").locator("#r");
31+
32+
// the fetch is refused before it leaves the frame -> the catch runs
33+
await expect(probe).toHaveText("blocked", { timeout: 10_000 });
34+
// and it must never have succeeded
35+
await expect(probe).not.toContainText("LEAKED");
36+
});

test/surfacePage.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,31 @@ test("html parts keep their CDN allowlist (rich-part tightening did not leak)",
159159
assert.ok("connect-src" in html, "html parts still have connect-src");
160160
});
161161

162+
test("the board origin is never a connect/script source — img/media only", () => {
163+
// The server origin is deliberately in img-src/media-src so uploaded assets
164+
// embed by URL. It must NEVER reach connect-src or script-src: that origin
165+
// serves the authenticated board API and the comment->agent channel, so a
166+
// contained script that could fetch it would defeat the whole sandbox. This
167+
// is the exact exfil hole the existing 'self'/wildcard/`https:` checks miss —
168+
// localhost:4000 is none of those, so it would slip past them.
169+
for (const make of [
170+
() => renderHtmlPage({ title: "t", html: "<p>x</p>", origin: ORIGIN }),
171+
() => renderSandboxedPart({ body: "x", css: "", origin: ORIGIN }),
172+
]) {
173+
const d = cspDirectives(make());
174+
assert.ok(
175+
!(d["connect-src"] ?? []).includes(ORIGIN),
176+
"board origin must not be a connect source",
177+
);
178+
assert.ok(
179+
!(d["script-src"] ?? []).includes(ORIGIN),
180+
"board origin must not be a script source",
181+
);
182+
// it is present where it's meant to be, so this test can't pass vacuously
183+
assert.ok(d["img-src"]?.includes(ORIGIN), "board origin should still embed images");
184+
}
185+
});
186+
162187
test("escapeHtml neutralizes markup metacharacters", () => {
163188
assert.equal(
164189
escapeHtml(`<img src=x onerror="alert(1)">`),

viewer/src/App.tsx

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -255,9 +255,17 @@ async function onBridgeMessage(ev: MessageEvent) {
255255
key?: string;
256256
} | null;
257257
if (!d || !d.__sideshow) return;
258-
// A surface iframe forwarded the session-switch shortcut because focus was
259-
// inside it (see server/surfacePage.ts). Mirror the parent keydown handler.
258+
// Every host-affecting message must come from a frame the viewer actually
259+
// embedded — never an unexpected/nested frame. send-prompt and resize prove
260+
// this implicitly (frameForSource resolves the exact html frame); the
261+
// remaining types reach the host UI directly, so gate them on isOwnFrame.
262+
// (frameForSource only knows html-part frames; switch-session is sent only by
263+
// those, but open-link is sent by rich-part frames too, so use the broader
264+
// check that recognizes any embedded iframe.)
260265
if (d.type === "switch-session") {
266+
if (!isOwnFrame(ev.source)) return;
267+
// A surface iframe forwarded the session-switch shortcut because focus was
268+
// inside it (see server/surfacePage.ts). Mirror the parent keydown handler.
261269
void selectAdjacent(d.key === "ArrowUp" ? -1 : 1);
262270
return;
263271
}
@@ -272,11 +280,23 @@ async function onBridgeMessage(ev: MessageEvent) {
272280
body: JSON.stringify({ surface: src.id, text: String(d.text), author: "user" }),
273281
});
274282
toast("Sent to agent: " + d.text);
275-
} else if (d.type === "open-link") {
283+
} else if (d.type === "open-link" && isOwnFrame(ev.source)) {
276284
if (confirm(`Open external link?\n\n${d.url}`)) window.open(d.url, "_blank", "noopener");
277285
}
278286
}
279287

288+
// True when `source` is the contentWindow of an iframe the viewer embedded
289+
// (html or rich part). frameForSource only tracks html-part frames; this is the
290+
// broader gate for messages rich-part frames also send (open-link). Identity
291+
// comparison works across the opaque-origin boundary even though the frame's
292+
// document is unreadable.
293+
function isOwnFrame(source: unknown): boolean {
294+
for (const f of document.querySelectorAll("iframe")) {
295+
if (f.contentWindow === source) return true;
296+
}
297+
return false;
298+
}
299+
280300
function SessionItem(props: { session: SessionRow }) {
281301
const label = () => sessionLabel(props.session);
282302
return (

0 commit comments

Comments
 (0)