From c3b2a34e91dade1b7a07163e2988b8819e973b2b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Jul 2026 11:21:51 +0000 Subject: [PATCH 1/2] feat(security): frame-ancestors on the viewer + noreferrer on external links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Harden the trusted viewer origin (shared with the authenticated API and the comment→agent channel) against the two OSS-side gaps flagged alongside the hosted iframe-CSRF work: - Clickjacking: the viewer HTML now sends `Content-Security-Policy: frame-ancestors 'self'` via the single configuredViewerHtml chokepoint (covers `/`, `/session/:id`, and the standalone `/s/:id` post view). The sandboxed `/s/:id?part=N` surface documents are meant to be framed and keep their own `sandbox` CSP, so they're untouched. - Referrer leak: external links the viewer opens — the openLink bridge's `window.open`, release-notes markdown links, and the image/trace/footer anchors — now carry `noreferrer` alongside `noopener`, so the current URL (which can hold the `?key=` deploy token) never rides an outbound Referer. Tests: api.test asserts frame-ancestors on the viewer routes (and its absence of the sandbox CSP); a new notes.test pins noreferrer on rendered external links. Changeset: patch. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017BLz2z8mwodKkzr8x1NapM --- .changeset/iframe-csrf-headers.md | 13 +++++++++++++ server/app.ts | 8 ++++++++ test/api.test.ts | 12 +++++++++++- test/notes.test.ts | 15 +++++++++++++++ viewer/src/App.tsx | 4 ++-- viewer/src/ImageSurface.tsx | 2 +- viewer/src/TraceSurface.tsx | 2 +- viewer/src/notes.ts | 2 +- 8 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 .changeset/iframe-csrf-headers.md create mode 100644 test/notes.test.ts diff --git a/.changeset/iframe-csrf-headers.md b/.changeset/iframe-csrf-headers.md new file mode 100644 index 0000000..0146a3b --- /dev/null +++ b/.changeset/iframe-csrf-headers.md @@ -0,0 +1,13 @@ +--- +"sideshow": patch +--- + +Harden the trusted viewer against clickjacking and referrer leaks. The viewer +HTML (the app origin, shared with the authenticated API and the comment→agent +channel) now sends `Content-Security-Policy: frame-ancestors 'self'`, refusing +cross-origin framing; the sandboxed `/s/:id` surface documents are unaffected +and keep their own `sandbox` CSP. External links the viewer opens — the +`openLink` bridge's `window.open`, the release-notes markdown links, and the +image/trace/footer anchors — now use `rel="noopener noreferrer"` (and the +`noreferrer` window feature), so the current URL (which can carry the `?key=` +deploy token) never rides an outbound `Referer`. diff --git a/server/app.ts b/server/app.ts index 310ba55..2e55df7 100644 --- a/server/app.ts +++ b/server/app.ts @@ -923,6 +923,14 @@ export function createApp({ }; const configuredViewerHtml = (c: Context, opts: { post?: Post; title?: string | null } = {}) => { + // The viewer HTML is the trusted app origin — it shares that origin with the + // authenticated API and the comment→agent channel, so a cross-origin page + // that frames it could clickjack actions or the prompt-injection channel. + // Refuse cross-origin framing (same-origin embedding still allowed). This is + // the trusted shell only; the sandboxed surface documents at /s/:id?part=N + // are *meant* to be framed and carry their own `sandbox` CSP header instead, + // so they never pass through here and are unaffected. + c.header("Content-Security-Policy", "frame-ancestors 'self'"); const pageTitle = opts.post?.title ?? opts.title; const html = withDocumentTitle( withViewerConfig( diff --git a/test/api.test.ts b/test/api.test.ts index eabb8e2..fb48a8f 100644 --- a/test/api.test.ts +++ b/test/api.test.ts @@ -253,7 +253,12 @@ test("GET /s/:id serves the viewer shell with link-preview metadata", async () = const page = await app.request(`https://board.test/s/${surface.id}`); assert.equal(page.status, 200); assert.ok(page.headers.get("content-type")?.includes("text/html")); - assert.equal(page.headers.get("content-security-policy"), null); + // The trusted viewer shell refuses cross-origin framing (anti-clickjacking); + // it carries frame-ancestors only, never the sandbox CSP the /s/:id?part=N + // surface documents use. + const shellCsp = page.headers.get("content-security-policy") ?? ""; + assert.match(shellCsp, /frame-ancestors 'self'/); + assert.doesNotMatch(shellCsp, /\bsandbox\b/); const body = await page.text(); assert.ok(body.includes("viewer"), "should serve the trusted viewer shell"); assert.doesNotMatch(body, /

diagram<\/p>/, "should not inline agent HTML"); @@ -279,6 +284,11 @@ test("GET /session/:id serves the viewer shell with the session title", async () const page = await app.request(`/session/${surface.sessionId}`); assert.equal(page.status, 200); assert.ok(page.headers.get("content-type")?.includes("text/html")); + // Every viewer-HTML route (here, `/` and `/session/:id`) is anti-clickjacking + // framed, sharing the one chokepoint (configuredViewerHtml). + assert.match(page.headers.get("content-security-policy") ?? "", /frame-ancestors 'self'/); + const root = await app.request("/"); + assert.match(root.headers.get("content-security-policy") ?? "", /frame-ancestors 'self'/); const body = await page.text(); assert.ok(body.includes("viewer"), "should serve the trusted viewer shell"); assert.match(body, /Auth refactor · sideshow<\/title>/); diff --git a/test/notes.test.ts b/test/notes.test.ts new file mode 100644 index 0000000..74f39ff --- /dev/null +++ b/test/notes.test.ts @@ -0,0 +1,15 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { renderNotes } from "../viewer/src/notes.ts"; + +// Release notes come from the GitHub release at runtime and render in the trusted +// viewer chrome. An external link must open with BOTH noopener (no window.opener +// reverse-tabnabbing) and noreferrer (the viewer URL — which can carry the `?key=` +// deploy token — never leaks via Referer to the destination site). +test("renderNotes external links carry rel=noopener noreferrer", () => { + const html = renderNotes("See [the docs](https://example.com/x) for details."); + assert.match( + html, + /<a href="https:\/\/example\.com\/x" target="_blank" rel="noopener noreferrer">the docs<\/a>/, + ); +}); diff --git a/viewer/src/App.tsx b/viewer/src/App.tsx index cb8cc7f..d8c8227 100644 --- a/viewer/src/App.tsx +++ b/viewer/src/App.tsx @@ -335,7 +335,7 @@ function StandaloneView(props: { post: Post }) { <main class="standalone-main"> <Card post={props.post} standalone /> <footer class="standalone-foot"> - <a href="https://sideshow.sh" target="_blank" rel="noopener"> + <a href="https://sideshow.sh" target="_blank" rel="noopener noreferrer"> made with <strong>sideshow</strong> </a> </footer> @@ -465,7 +465,7 @@ async function onBridgeMessage(ev: MessageEvent) { } if (link.protocol !== "http:" && link.protocol !== "https:") return; if (confirm(`Open external link?\n\n${link.href}`)) - window.open(link.href, "_blank", "noopener"); + window.open(link.href, "_blank", "noopener,noreferrer"); } else if (d.type === "copy" && isOwnFrame(ev.source)) { void navigator.clipboard?.writeText(String(d.text)).catch(() => {}); } diff --git a/viewer/src/ImageSurface.tsx b/viewer/src/ImageSurface.tsx index a0a1ebb..a93fcde 100644 --- a/viewer/src/ImageSurface.tsx +++ b/viewer/src/ImageSurface.tsx @@ -14,7 +14,7 @@ export function ImageSurface(props: { surface: ImageSurfaceData }) { when={!failed()} fallback={<div class="asset-gone">Image unavailable — it may have been evicted.</div>} > - <a href={src()} target="_blank" rel="noopener"> + <a href={src()} target="_blank" rel="noopener noreferrer"> <img class="asset-img" src={src()} diff --git a/viewer/src/TraceSurface.tsx b/viewer/src/TraceSurface.tsx index 251ceda..410ef21 100644 --- a/viewer/src/TraceSurface.tsx +++ b/viewer/src/TraceSurface.tsx @@ -28,7 +28,7 @@ export function TraceSurface(props: { surface: TraceSurfaceData }) { <span class="trace-title">{props.surface.title ?? "Agent trace"}</span> <Show when={assetUrl()} keyed> {(url) => ( - <a class="trace-dl" href={url} target="_blank" rel="noopener"> + <a class="trace-dl" href={url} target="_blank" rel="noopener noreferrer"> download ↓ </a> )} diff --git a/viewer/src/notes.ts b/viewer/src/notes.ts index 5782caf..d6235cc 100644 --- a/viewer/src/notes.ts +++ b/viewer/src/notes.ts @@ -11,7 +11,7 @@ const inline = (s: string) => .replace(/\*\*([^*]+)\*\*/g, "<strong>$1</strong>") .replace( /\[([^\]]+)\]\((https?:[^)\s]+)\)/g, - '<a href="$2" target="_blank" rel="noopener">$1</a>', + '<a href="$2" target="_blank" rel="noopener noreferrer">$1</a>', ); export function renderNotes(md: string): string { From 83de2c4cb57d96833c0b4948300252722b482794 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Sat, 4 Jul 2026 15:42:50 +0000 Subject: [PATCH 2/2] test(security): pin surface-doc frame-ancestors absence + alias; rel=noopener on Card links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address sonnet review nits on the iframe-CSRF hardening: - api.test now asserts surface docs (/s/:id?part=N) never carry frame-ancestors (the invariant that keeps them framable), and that the nested post-permalink alias (/session/:id/p/:id) is frame-ancestors protected like its siblings. - Card.tsx's two bare target="_blank" links (open-in-tab, open-as-PNG) get rel="noopener" for reverse-tabnabbing hygiene. They're same-origin, so no noreferrer (keeps the useful same-origin Referer) — the external links keep noopener noreferrer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017BLz2z8mwodKkzr8x1NapM --- test/api.test.ts | 7 +++++++ viewer/src/Card.tsx | 2 ++ 2 files changed, 9 insertions(+) diff --git a/test/api.test.ts b/test/api.test.ts index fb48a8f..a5765ec 100644 --- a/test/api.test.ts +++ b/test/api.test.ts @@ -230,6 +230,10 @@ test("publishes a combined html+diff surface; /s server-renders both surfaces op assert.match(csp, /\bsandbox\b/); assert.match(csp, /\ballow-scripts\b/); assert.doesNotMatch(csp, /allow-same-origin/); + // Surface docs are MEANT to be framed by the viewer, so they must never pick + // up the viewer shell's anti-clickjacking frame-ancestors (that would refuse + // the embedding iframe). The two CSPs are mutually exclusive by construction. + assert.doesNotMatch(csp, /frame-ancestors/); } }); @@ -289,6 +293,9 @@ test("GET /session/:id serves the viewer shell with the session title", async () assert.match(page.headers.get("content-security-policy") ?? "", /frame-ancestors 'self'/); const root = await app.request("/"); assert.match(root.headers.get("content-security-policy") ?? "", /frame-ancestors 'self'/); + // The nested post-permalink alias shares the same configuredViewerHtml chokepoint. + const aliased = await app.request(`/session/${surface.sessionId}/p/${surface.id}`); + assert.match(aliased.headers.get("content-security-policy") ?? "", /frame-ancestors 'self'/); const body = await page.text(); assert.ok(body.includes("viewer"), "should serve the trusted viewer shell"); assert.match(body, /<title>Auth refactor · sideshow<\/title>/); diff --git a/viewer/src/Card.tsx b/viewer/src/Card.tsx index bccbe2a..b827d5e 100644 --- a/viewer/src/Card.tsx +++ b/viewer/src/Card.tsx @@ -417,6 +417,7 @@ export function Card(props: { post: Post; standalone?: boolean }) { <a class="act icon open" target="_blank" + rel="noopener" href={postLink(props.post.id)} title="Open in a new tab" aria-label="Open in a new tab" @@ -443,6 +444,7 @@ export function Card(props: { post: Post; standalone?: boolean }) { <a class="act icon shot" target="_blank" + rel="noopener" href={postImageLink(props.post.id)} title="Open first surface as an image (PNG)" aria-label="Open first surface as an image (PNG)"