From 42ff23de73dbc671d1acd7087ece6857b0d1b264 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Sun, 24 May 2026 16:05:11 +0000 Subject: [PATCH] Studio: fix HTML/SVG preview sanitizer, sandbox, and streaming gaps Bundle of follow-ups to the HTML/SVG fence renderer landed earlier in this PR. Each item came out of either the parallel reviewer pass or a manual Playwright probe against the live Studio with an Anthropic provider attached. Sanitizer: - filter, mask, and clip-path are now in FORBID_ATTR. They accept url(https://...) values and the CSS engine still fetches that URL when the SVG renders, which previously slipped past the FORBID list. - href and xlink:href are no longer blanket-forbidden; they survive only when the value is a same-document fragment (href="#id"), which is what textPath, gradient, and use refs need. External schemes are dropped via a uponSanitizeAttribute hook so a beacon href cannot make it through. - The hook approach replaces DOMPurify's ALLOWED_URI_REGEXP, which also filtered presentation attrs (cx, cy, r, fill, width, height) and rendered circles with r=0. SVG preview: - Inner stylesheet caps both max-width AND max-height so a square viewBox (200x200) scaled to the container width no longer overflows the fixed-height iframe and clips at the bottom. HTML preview: - srcdoc carries a defense-in-depth meta-CSP (default-src 'none', connect-src 'none', frame-src 'none', img-src data: blob:, script-src 'self' 'unsafe-inline', style-src 'self' 'unsafe-inline'). The host CSP already blocks inline scripts; this layer also blocks network egress, nested iframes, and form submission so a future host-CSP relaxation does not silently turn the preview into an exfiltration channel. - Sandbox grows allow-modals so alert/confirm/prompt are not silently no-oped if the host CSP ever permits inline scripts. - Pop-out spacer now uses the live HTML iframe height instead of hardcoded DEFAULT_PREVIEW_HEIGHT, so popping out a short preview does not leave a 500px hole in the chat bubble. - autoHeight resets on source change so a long-running session that swaps from a tall demo to a short one no longer keeps the previous iframe size during the gap before the new doc posts its height. Streaming and a11y: - parseIncompleteCodeFence parses an in-flight open fence (no closing backticks yet). markdown-text falls back to it when streaming is incomplete, so the advertised isIncomplete -> Code-tab-lock path actually runs. - Tab buttons gain aria-controls / aria-labelledby wiring and a roving tabindex so the WAI-ARIA tab pattern is complete. - Pop-out modal gets role="dialog" and aria-modal. Tooling: - vitest now runs in the Studio Frontend CI workflow so sanitizer or renderer regressions block the gate. - test-setup shims URL.createObjectURL / revokeObjectURL for jsdom in case future iframe work needs it. - frame-src in the host CSP is now declared explicitly as 'self' so a future change that loosens it leaves a visible diff for review. Tests added: ARIA wiring, SVG height fit, srcdoc meta-CSP shape, incomplete-fence helper, filter/mask/clip-path attr stripping, safe fragment-href survival, external-href rejection. Vitest passes 21/21, tsc -b and vite build are clean. --- .github/workflows/studio-frontend-ci.yml | 7 + studio/backend/main.py | 4 + studio/backend/tests/test_middleware.py | 18 ++ .../__tests__/html-svg-renderer.test.tsx | 121 ++++++++++- .../assistant-ui/html-svg-renderer.tsx | 204 +++++++++++++++--- .../components/assistant-ui/markdown-text.tsx | 9 +- studio/frontend/src/test-setup/setup.ts | 14 ++ 7 files changed, 342 insertions(+), 35 deletions(-) diff --git a/.github/workflows/studio-frontend-ci.yml b/.github/workflows/studio-frontend-ci.yml index 1270a57ef6..f22b5909c8 100644 --- a/.github/workflows/studio-frontend-ci.yml +++ b/.github/workflows/studio-frontend-ci.yml @@ -109,6 +109,13 @@ jobs: - name: Typecheck run: npm run typecheck + - name: Frontend unit tests (vitest) + # New vitest suite covers the HtmlSvgRenderer iframe sandbox / + # CSP / sanitizer contract. Run it before the build so a + # sanitizer regression fails the gate even if the bundle still + # builds clean. + run: npm run test + - name: Build run: npm run build diff --git a/studio/backend/main.py b/studio/backend/main.py index 004ae404cd..19202efcb1 100644 --- a/studio/backend/main.py +++ b/studio/backend/main.py @@ -327,6 +327,10 @@ def _build_csp(script_nonce: "str | None" = None) -> str: "style-src 'self' 'unsafe-inline'; " f"{script_src}; " "font-src 'self' data:; " + # Restrict iframe sources to same-origin only. The assistant + # HTML/SVG previews use srcdoc (no URL involved) and inherit this + # CSP, so this also bounds what the preview iframe can do. + "frame-src 'self'; " "frame-ancestors 'none'; " "form-action 'self'; " "base-uri 'self'" diff --git a/studio/backend/tests/test_middleware.py b/studio/backend/tests/test_middleware.py index bbaf20298d..f2a216b9e5 100644 --- a/studio/backend/tests/test_middleware.py +++ b/studio/backend/tests/test_middleware.py @@ -196,6 +196,24 @@ class TestSecurityHeadersMiddleware: nonced = main_module._build_csp("XYZ") assert "script-src 'self' 'nonce-XYZ';" in nonced + def test_frame_src_is_explicitly_self_only(self, main_module): + # The assistant HTML/SVG preview iframe uses srcdoc (no URL fetch), + # so frame-src does not need to permit data: / blob:. Pinning to + # 'self' explicitly is the strictest setting CSP allows here, and + # leaves a visible directive a reviewer can grep for if a future + # change tries to relax it without an audit. + csp = main_module._build_csp() + frame_src = next( + chunk.strip() + for chunk in csp.split(";") + if chunk.strip().startswith("frame-src ") + ) + tokens = frame_src.split() + assert tokens[0] == "frame-src" + assert "'self'" in tokens + assert "data:" not in tokens + assert "blob:" not in tokens + def test_img_src_allows_google_favicons(self, main_module): # sources.tsx fetches https://www.google.com/s2/favicons?... ; without # this allowlist entry citation favicons fall back to gray initials. diff --git a/studio/frontend/src/components/assistant-ui/__tests__/html-svg-renderer.test.tsx b/studio/frontend/src/components/assistant-ui/__tests__/html-svg-renderer.test.tsx index 1b64114575..4de79e7b7c 100644 --- a/studio/frontend/src/components/assistant-ui/__tests__/html-svg-renderer.test.tsx +++ b/studio/frontend/src/components/assistant-ui/__tests__/html-svg-renderer.test.tsx @@ -8,6 +8,7 @@ import { isHtmlFence, isSvgFence, parseCodeFence, + parseIncompleteCodeFence, sanitizeSvgSource, } from "../html-svg-renderer"; @@ -24,15 +25,25 @@ describe("HtmlSvgRenderer", () => { "html-svg-renderer-iframe", ) as HTMLIFrameElement; expect(iframe.tagName).toBe("IFRAME"); - // SECURITY: allow-scripts only; never allow-same-origin or - // allow-top-navigation. If this ever changes, the preview can read - // parent.document and exfiltrate session data. - expect(iframe.getAttribute("sandbox")).toBe("allow-scripts"); - expect(iframe.getAttribute("sandbox")).not.toContain("allow-same-origin"); - expect(iframe.getAttribute("sandbox")).not.toContain( - "allow-top-navigation", - ); - expect(iframe.getAttribute("srcdoc") ?? iframe.srcdoc).toContain("hello"); + // SECURITY: allow-scripts + allow-modals leave script / alert / + // confirm operative IF the inherited host CSP ever permits inline; + // NEVER allow-same-origin or allow-top-navigation -- those would let + // the preview read parent.document and exfiltrate session data. + const sandbox = iframe.getAttribute("sandbox") ?? ""; + expect(sandbox.split(/\s+/)).toContain("allow-scripts"); + expect(sandbox).not.toContain("allow-same-origin"); + expect(sandbox).not.toContain("allow-top-navigation"); + // srcdoc carries the assistant HTML plus a defense-in-depth meta CSP + // that adds ``connect-src 'none'`` and ``frame-src 'none'`` on top of + // the inherited host CSP. Inline ``'; + +// Meta-CSP enforced INSIDE the srcdoc iframe. The iframe inherits the host +// Studio CSP (every srcdoc / data: / blob: scheme does, per CSP3 ยง Initialize +// document CSP), so the host's ``script-src 'self'`` already blocks inline +// `, - [source], - ); + // srcdoc keeps the assistant HTML rendering same-origin-blocked while + // still showing layout, images, styles, and Streamdown-syntax-highlighted + // source in the Code tab. Inline