Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/contain-csp-and-bridge-source.md
Original file line number Diff line number Diff line change
@@ -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.
36 changes: 36 additions & 0 deletions e2e/isolation.spec.ts
Original file line number Diff line number Diff line change
@@ -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 = `<div id="r">running</div>
<script>
// Relative URL resolves against the frame's document (the board origin), so
// this targets the authenticated API. connect-src must refuse it.
fetch('/api/surfaces')
.then(function (res) { document.getElementById('r').textContent = 'LEAKED status ' + res.status; })
.catch(function () { document.getElementById('r').textContent = 'blocked'; });
</script>`;

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");
});
25 changes: 25 additions & 0 deletions test/surfacePage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: "<p>x</p>", 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(`<img src=x onerror="alert(1)">`),
Expand Down
26 changes: 23 additions & 3 deletions viewer/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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 (
Expand Down
Loading