Skip to content

Commit a5c8d8c

Browse files
kiluazenclaude
andcommitted
fix(viewer): render rich parts same-origin with a nonce'd CSP
Rich parts (markdown/diff/terminal/mermaid) render in opaque-origin sandboxed srcdoc iframes. On Chrome 149 a field-trial experiment defers layout in opaque-origin srcdoc iframes specifically: every measurement (scrollHeight/offsetHeight/getBoundingClientRect) reads 0 until layout eventually lands — and for off-screen frames it may never land — so the in-frame bridge reports 0 and the part renders blank/collapsed. The fixed-timer reporting (and #85's single re-parse) can't reliably catch the late layout; it's a timing race that differs on every refresh. Fix: remove the actual trigger by rendering rich parts same-origin (sandbox="allow-scripts allow-same-origin") — same-origin srcdoc frames lay out synchronously — and preserve the #65 isolation with a per-document nonce CSP: script-src 'nonce-<n>' instead of 'unsafe-inline', with the nonce only on the trusted bridge script. An injected <script> (sanitizer regression) lacks the nonce and is blocked by CSP, so the same-origin access can't be abused. Drops the now-unnecessary re-parse workaround. HTML parts (/s/:id) are intentionally unchanged. Verified on Chrome/149.0.0.0: rich parts render identically on every reload (deterministic); no CSP violations. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent a4033cf commit a5c8d8c

16 files changed

Lines changed: 139 additions & 99 deletions

AGENTS.md

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,11 @@ consciously, not as a side effect):
5151
wraps an html part (CDN-allowlist CSP + the postMessage bridge: resize,
5252
sendPrompt, openLink) and injects any opted-in kits (`kits.ts`).
5353
`renderSandboxedPart` wraps markup the viewer rendered
54-
to a string (markdown/mermaid/diff/terminal) under a tighter CSP (no
55-
`connect-src`, no CDN) — see `viewer/src/SandboxedPart.tsx`. Image and trace
56-
parts stay native because they have no HTML sink (the viewer renders them with
57-
text nodes / `<img>` / JSX). No agent markup is ever set as `innerHTML` in the
58-
trusted viewer origin.
54+
to a string (markdown/mermaid/diff/terminal) under a tighter CSP (nonce-only
55+
bridge script, no `connect-src`, no CDN) — see
56+
`viewer/src/SandboxedPart.tsx`. Image and trace parts stay native because
57+
they have no HTML sink (the viewer renders them with text nodes / `<img>` /
58+
JSX). No agent markup is ever set as `innerHTML` in the trusted viewer origin.
5959
- `server/themes.ts` — theme registry (github/gruvbox/one), runtime-agnostic so
6060
both server and viewer import it. One `Palette` per light/dark per theme; the
6161
viewer-chrome vars and the html-part `--color-*` tokens are both _derived_
@@ -96,10 +96,12 @@ consciously, not as a side effect):
9696
render with Solid text nodes / element attributes**, which escape by
9797
construction (image, trace). String-building in the viewer is fine — a string
9898
is not a DOM sink; danger only starts when it reaches the DOM, which must
99-
happen at an opaque origin. When you add a part kind, pick (a) or (b); never a
100-
third way. The iframes are sandboxed without `allow-same-origin` (opaque
101-
origin) and `connect-src`-free for rich parts (no exfil even if contained
102-
script runs); never weaken this.
99+
happen inside a sandboxed iframe. When you add a part kind, pick (a) or (b);
100+
never a third way. HTML-part iframes stay sandboxed without
101+
`allow-same-origin` (opaque origin). Rich `srcdoc` iframes use
102+
`allow-same-origin` to avoid Chrome's opaque-origin srcdoc layout bug, so
103+
their CSP must stay nonce-only for scripts and `connect-src`-free; never
104+
weaken this.
103105
- WebKit quirk in sandboxed iframes: ResizeObserver's initial callback may not
104106
fire and `documentElement.scrollHeight` ratchets to viewport height — the
105107
bridge reports `body.scrollHeight` on `load` plus staggered timers. Don't

e2e/diff.spec.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,12 @@ test("a diff part renders a highlighted diff inside a sandbox iframe", async ({
2121
await page.goto(server.url);
2222
const card = page.locator(".card:not(#whatsNew)").first();
2323

24-
// the diff renders in an opaque-origin sandbox iframe (no allow-same-origin),
25-
// so a @pierre/diffs DOM-building regression can't reach the board
26-
await expect(card.locator("iframe.diffframe")).toHaveAttribute("sandbox", "allow-scripts");
24+
// the diff renders in a sandbox iframe. It is same-origin to avoid Chrome
25+
// 149's opaque-origin srcdoc layout bug; CSP blocks all non-nonced scripts.
26+
await expect(card.locator("iframe.diffframe")).toHaveAttribute(
27+
"sandbox",
28+
"allow-scripts allow-same-origin",
29+
);
2730
const frame = card.frameLocator("iframe.diffframe");
2831

2932
// the @pierre/diffs SSR fragment mounts in a declarative shadow root; its

e2e/markdown.spec.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,12 @@ test("a markdown part renders typed prose with a highlighted code block", async
2828
await page.goto(server.url);
2929
const card = page.locator(".card");
3030

31-
// markdown renders inside an opaque-origin sandbox iframe (defense in depth:
32-
// even a markdown-it/shiki regression can't reach the board). The sandbox has
33-
// NO allow-same-origin — that's the isolation guarantee.
34-
await expect(card.locator("iframe.mdframe")).toHaveAttribute("sandbox", "allow-scripts");
31+
// markdown renders inside a sandbox iframe. It is same-origin to avoid Chrome
32+
// 149's opaque-origin srcdoc layout bug; CSP blocks all non-nonced scripts.
33+
await expect(card.locator("iframe.mdframe")).toHaveAttribute(
34+
"sandbox",
35+
"allow-scripts allow-same-origin",
36+
);
3537
const md = card.frameLocator("iframe.mdframe");
3638

3739
// structured typography inside the frame

e2e/mermaid.spec.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,12 @@ test("a mermaid part renders a diagram as inline SVG in the viewer", async ({ pa
1818
const card = page.locator(".card:not(#sessionThread)");
1919
const mermaid = card.locator(".mermaidpart");
2020

21-
// the diagram renders inside an opaque-origin sandbox iframe — a second
22-
// boundary behind mermaid's DOMPurify. No allow-same-origin.
23-
await expect(mermaid.locator("iframe.mermaidframe")).toHaveAttribute("sandbox", "allow-scripts");
21+
// the diagram renders inside a sandbox iframe. It is same-origin to avoid
22+
// Chrome 149's opaque-origin srcdoc layout bug; CSP blocks non-nonced scripts.
23+
await expect(mermaid.locator("iframe.mermaidframe")).toHaveAttribute(
24+
"sandbox",
25+
"allow-scripts allow-same-origin",
26+
);
2427
const frame = mermaid.frameLocator("iframe.mermaidframe");
2528

2629
const svg = frame.locator("svg");

e2e/terminal.spec.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,12 @@ test("a terminal part renders ANSI in a sandbox iframe, escaping raw HTML", asyn
1717
await page.goto(server.url);
1818
const card = page.locator(".card:not(#whatsNew)").first();
1919

20-
// terminal output renders inside an opaque-origin sandbox iframe — ansi_up's
21-
// escaping is no longer the only thing between agent text and the board
22-
await expect(card.locator("iframe.termframe")).toHaveAttribute("sandbox", "allow-scripts");
20+
// terminal output renders inside a sandbox iframe. It is same-origin to avoid
21+
// Chrome 149's opaque-origin srcdoc layout bug; CSP blocks non-nonced scripts.
22+
await expect(card.locator("iframe.termframe")).toHaveAttribute(
23+
"sandbox",
24+
"allow-scripts allow-same-origin",
25+
);
2326
const frame = card.frameLocator("iframe.termframe");
2427

2528
// SGR escape became an inline-styled span (a color), not literal text

e2e/viewer.spec.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ test("comment typed in the composer round-trips to the API", async ({ page, serv
112112
await input.press("Enter");
113113

114114
// renders in the thread (via SSE) and is persisted server-side. The comment
115-
// text renders inside its own opaque-origin sandbox iframe.
115+
// text renders inside its own sandbox iframe.
116116
await expect(card.frameLocator(".cmtframe").locator("body")).toContainText("ship it");
117117
await expect(card.locator(".cmt .who")).toHaveText("you");
118118
await expect
@@ -226,8 +226,11 @@ test("a comment containing raw HTML is sandboxed and escaped, never a live node"
226226
await input.fill("<img src=x onerror=alert(1)> hi");
227227
await input.press("Enter");
228228

229-
// the comment renders inside an opaque-origin sandbox iframe...
230-
await expect(card.locator(".cmtframe")).toHaveAttribute("sandbox", "allow-scripts");
229+
// the comment renders inside a sandbox iframe with nonce-gated scripts.
230+
await expect(card.locator(".cmtframe")).toHaveAttribute(
231+
"sandbox",
232+
"allow-scripts allow-same-origin",
233+
);
231234
const frame = card.frameLocator(".cmtframe");
232235
// ...with the raw HTML escaped to text, never a live <img>
233236
await expect(frame.locator("img")).toHaveCount(0);

server/surfacePage.ts

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ const SVG_DEFS = `<svg width="0" height="0" style="position:absolute" aria-hidde
131131

132132
// Bridge to the host viewer: sendPrompt/openLink mirror Claude's widget
133133
// globals, and a ResizeObserver reports content height so the parent can
134-
// size the sandboxed (opaque-origin) iframe.
134+
// size the sandboxed iframe.
135135
const BRIDGE_JS = `
136136
window.sendPrompt = function (text) {
137137
parent.postMessage({ __sideshow: true, type: 'send-prompt', text: String(text) }, '*');
@@ -177,34 +177,56 @@ if (window.ResizeObserver) {
177177
export const escapeHtml = (s: string) =>
178178
s.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;").replace(/"/g, "&quot;");
179179

180+
function randomNonce(): string {
181+
const bytes = new Uint8Array(16);
182+
const webCrypto = (
183+
globalThis as typeof globalThis & {
184+
crypto: { getRandomValues(array: Uint8Array): Uint8Array };
185+
}
186+
).crypto;
187+
webCrypto.getRandomValues(bytes);
188+
const alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
189+
let out = "";
190+
for (let i = 0; i < bytes.length; i += 3) {
191+
const a = bytes[i]!;
192+
const b = bytes[i + 1];
193+
const c = bytes[i + 2];
194+
out += alphabet[a >> 2];
195+
out += alphabet[((a & 3) << 4) | ((b ?? 0) >> 4)];
196+
out += b === undefined ? "=" : alphabet[((b & 15) << 2) | ((c ?? 0) >> 6)];
197+
out += c === undefined ? "=" : alphabet[c & 63];
198+
}
199+
return out;
200+
}
201+
180202
// Wrap one html part in the themed, sandboxed document the iframe loads. The
181203
// board's color tokens (theme-dependent) are injected first so the static base
182204
// + kit resolve against them; `theme` defaults to the github preset.
183205
// CSP for a rich part (markdown/mermaid/diff). These render markup our own
184206
// libraries produced — they never load CDN scripts and never need the network,
185-
// so the policy is *tighter* than an html part's: only the inline bridge runs,
186-
// and there is no `connect-src`, so even if a sanitizer regression let agent
187-
// markup execute, the script is boxed into an opaque origin with no way to
188-
// phone home. `img-src origin` lets inline markdown images at <origin>/a/:id
189-
// load (the iframe is opaque-origin, so `'self'` matches nothing — same reason
190-
// buildCsp adds it explicitly).
191-
function buildRichCsp(origin: string): string {
207+
// so the policy is *tighter* than an html part's: only the nonce-bearing bridge
208+
// runs, and there is no `connect-src`. The frame uses allow-same-origin to avoid
209+
// Chrome 149's opaque-origin srcdoc layout bug, so injected scripts must not be
210+
// able to run at all. `img-src origin` lets inline markdown images at
211+
// <origin>/a/:id load without making the board origin a script/connect source.
212+
function buildRichCsp(origin: string, nonce: string): string {
192213
return [
193214
`default-src 'none'`,
194-
`script-src 'unsafe-inline'`,
215+
`script-src 'nonce-${nonce}'`,
195216
`style-src 'unsafe-inline'`,
196217
`img-src https: data: blob: ${origin}`,
197218
`font-src data:`,
198219
].join("; ");
199220
}
200221

201222
// Wrap pre-rendered, *untrusted* markup (markdown HTML, a mermaid SVG, a diff's
202-
// SSR output) in the same opaque-origin sandbox html parts get. The markup was
203-
// built as a STRING in the trusted viewer (string building is not a DOM sink),
204-
// and only becomes live DOM here, inside the iframe — so a markdown-it / shiki /
205-
// mermaid / DOMPurify / @pierre-diffs sanitizer bypass can no longer reach the
206-
// board. `css` is the part-specific stylesheet (prose/diff/mermaid rules);
207-
// chrome theme vars come from viewerThemeCss so the part matches the viewer.
223+
// SSR output) in a sandboxed iframe document. The markup was built as a STRING
224+
// in the trusted viewer (string building is not a DOM sink), and only becomes
225+
// live DOM here, inside a document whose CSP allows only the nonce-bearing
226+
// bridge script — so a markdown-it / shiki / mermaid / DOMPurify /
227+
// @pierre-diffs sanitizer bypass can no longer run script in the board origin.
228+
// `css` is the part-specific stylesheet (prose/diff/mermaid rules); chrome
229+
// theme vars come from viewerThemeCss so the part matches the viewer.
208230
export function renderSandboxedPart(doc: {
209231
body: string;
210232
css: string;
@@ -213,12 +235,13 @@ export function renderSandboxedPart(doc: {
213235
}): string {
214236
const theme =
215237
typeof doc.theme === "string" || doc.theme == null ? themeById(doc.theme) : doc.theme;
238+
const nonce = randomNonce();
216239
return `<!doctype html>
217240
<html lang="en">
218241
<head>
219242
<meta charset="utf-8">
220243
<meta name="viewport" content="width=device-width, initial-scale=1">
221-
<meta http-equiv="Content-Security-Policy" content="${buildRichCsp(doc.origin)}">
244+
<meta http-equiv="Content-Security-Policy" content="${buildRichCsp(doc.origin, nonce)}">
222245
<!-- srcdoc's base URL is about:srcdoc, so relative URLs (e.g. a markdown
223246
image at /a/:id) would not resolve; pin the base to the server origin.
224247
img-src in buildRichCsp allows that origin. (html parts don't need this —
@@ -228,7 +251,7 @@ export function renderSandboxedPart(doc: {
228251
</head>
229252
<body>
230253
${doc.body}
231-
<script>${BRIDGE_JS}</script>
254+
<script nonce="${nonce}">${BRIDGE_JS}</script>
232255
</body>
233256
</html>`;
234257
}

test/surfacePage.test.ts

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ function cspDirectives(doc: string): Record<string, string[]> {
2626
return out;
2727
}
2828

29+
function bridgeNonce(doc: string): string {
30+
const m = /<script nonce="([^"]+)">/.exec(doc);
31+
assert.ok(m, "bridge script must carry a nonce");
32+
return m![1]!;
33+
}
34+
2935
// The CDN allowlist html parts may load from. This is a deliberate, fixed set —
3036
// the test pins it so widening it (a new origin, a wildcard) is a conscious edit
3137
// that updates this list, never an accident.
@@ -130,28 +136,39 @@ test("renderSandboxedPart embeds the body and css inside the sandbox doc", () =>
130136
assert.ok(doc.includes(`<base href="${ORIGIN}/">`), "base href pins the origin");
131137
// the resize/openLink bridge ships in the frame so it can self-size
132138
assert.ok(doc.includes("postMessage"), "bridge is present");
139+
assert.match(bridgeNonce(doc), /^[A-Za-z0-9+/]+={0,2}$/, "bridge nonce is base64");
133140
// chrome theme vars are injected (viewerThemeCss) so the part matches the viewer
134141
assert.ok(doc.includes("--bg:"), "theme vars are injected");
135142
});
136143

137144
test("renderSandboxedPart uses a tighter CSP than html parts: no connect-src, no CDN", () => {
138-
const d = cspDirectives(renderSandboxedPart({ body: "x", css: "", origin: ORIGIN }));
145+
const doc = renderSandboxedPart({ body: "x", css: "", origin: ORIGIN });
146+
const nonce = bridgeNonce(doc);
147+
const d = cspDirectives(doc);
139148
assert.deepEqual(d["default-src"], ["'none'"], "locked-down default");
140-
// script-src is EXACTLY the inline bridge — no CDN sources leak in
141-
assert.deepEqual(d["script-src"], ["'unsafe-inline'"], "only the inline bridge runs");
149+
// script-src is EXACTLY the nonce-bearing bridge — no injected inline script
150+
// and no CDN sources can run in the same-origin rich srcdoc frame.
151+
assert.deepEqual(d["script-src"], [`'nonce-${nonce}'`], "only the trusted bridge runs");
152+
assert.ok(!d["script-src"]?.includes("'unsafe-inline'"), "rich scripts must require a nonce");
142153
// a contained script must have no way to phone home
143154
assert.ok(!("connect-src" in d), "no connect-src");
144155
// uploaded images still embed by absolute origin URL
145156
assert.ok(d["img-src"]?.includes(ORIGIN), "origin allowed for images");
146157
});
147158

159+
test("renderSandboxedPart generates a fresh nonce per document", () => {
160+
const a = renderSandboxedPart({ body: "x", css: "", origin: ORIGIN });
161+
const b = renderSandboxedPart({ body: "x", css: "", origin: ORIGIN });
162+
assert.notEqual(bridgeNonce(a), bridgeNonce(b), "nonce must be per-render");
163+
});
164+
148165
test("html parts keep their CDN allowlist (rich-part tightening did not leak)", () => {
149166
const html = cspDirectives(renderHtmlPage({ title: "t", html: "<b>x</b>", origin: ORIGIN }));
150-
const rich = cspDirectives(renderSandboxedPart({ body: "x", css: "", origin: ORIGIN }));
151-
// rich parts lock script-src to the inline bridge alone; html parts add the
152-
// CDN sources on top, so html's source list is strictly larger. (Asserting on
153-
// the count rather than a host literal keeps this off the URL-substring path.)
154-
assert.deepEqual(rich["script-src"], ["'unsafe-inline'"], "rich = inline bridge only");
167+
const richDoc = renderSandboxedPart({ body: "x", css: "", origin: ORIGIN });
168+
const rich = cspDirectives(richDoc);
169+
// rich parts lock script-src to the nonce-bearing bridge alone; html parts add
170+
// inline scripts and CDN sources for agent-authored html widgets.
171+
assert.deepEqual(rich["script-src"], [`'nonce-${bridgeNonce(richDoc)}'`], "rich = bridge only");
155172
assert.ok(
156173
html["script-src"].length > rich["script-src"].length,
157174
"html parts keep extra (CDN) script sources",

viewer/src/App.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,7 @@ async function onBridgeMessage(ev: MessageEvent) {
319319
// True when `source` is the contentWindow of an iframe the viewer embedded
320320
// (html or rich part). frameForSource only tracks html-part frames; this is the
321321
// broader gate for messages rich-part frames also send (open-link). Identity
322-
// comparison works across the opaque-origin boundary even though the frame's
323-
// document is unreadable.
322+
// comparison works for both opaque html-part frames and same-origin rich frames.
324323
function isOwnFrame(source: unknown): boolean {
325324
for (const f of root().querySelectorAll("iframe")) {
326325
if (f.contentWindow === source) return true;

viewer/src/Card.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,10 @@ import {
4848

4949
// Comment text is plain text — it already renders as an escaped text node — but
5050
// it is shown right beside agent-rendered surfaces, so for consistency it goes
51-
// through the same opaque-origin sandbox: the text is escaped to a string here
52-
// and only parsed inside the iframe. `pre-wrap` preserves the author's line
53-
// breaks; the height comes from the resize bridge (a one-liner clamps to ~24px).
51+
// through the same sandboxed rich-part path: the text is escaped to a string
52+
// here and only parsed inside the iframe. `pre-wrap` preserves the author's
53+
// line breaks; the height comes from the resize bridge (a one-liner clamps to
54+
// ~24px).
5455
const CMT_CSS = `
5556
body {
5657
margin: 0;

0 commit comments

Comments
 (0)