From 1385b8076e0098ff1d8ce2d9020e24848d334a27 Mon Sep 17 00:00:00 2001
From: Daniel Han <danielhanchen@gmail.com>
Date: Fri, 22 May 2026 17:01:30 +0000
Subject: [PATCH] Studio: sandbox SVG previews inside a CSP-locked iframe

Reviewers flagged the inline SVG preview as a regression from the
pre-PR data-URI <img> path: DOMPurify's default SVG profile keeps
<style> tags, style attributes, and <image>/<use> href targets, all
of which now reach the host Studio document because we mount the
sanitized SVG with dangerouslySetInnerHTML. That lets a model
response hide UI with body{display:none}, or beacon to attacker
URLs via <image href=...>.

Fix in two layers so a single regression cannot reopen the hole:

  * Tighten SVG_PURIFY_CONFIG -- FORBID_TAGS adds style, image, use,
    link, meta; FORBID_ATTR drops href, xlink:href, and style. The
    surviving markup can no longer carry inline CSS or external
    resource refs.

  * Move SvgPreview into a sandbox='' iframe (no scripts, no
    same-origin) with a default-src 'none' CSP. Even if a future
    sanitizer pass leaks a URL-bearing attribute, the browser blocks
    the request and the SVG cannot touch parent.document.
---
 .../__tests__/html-svg-renderer.test.tsx      | 51 ++++++++++---
 .../assistant-ui/html-svg-renderer.tsx        | 76 ++++++++++++++++---
 2 files changed, 106 insertions(+), 21 deletions(-)

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 6efb1c300f..1b64114575 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
@@ -35,7 +35,7 @@ describe("HtmlSvgRenderer", () => {
     expect(iframe.getAttribute("srcdoc") ?? iframe.srcdoc).toContain("hello");
   });
 
-  it("renders an SVG preview and strips malicious <script> + on* payloads", () => {
+  it("renders an SVG preview inside a no-script sandboxed iframe with srcdoc carrying the sanitized markup", () => {
     const malicious = `<svg xmlns="http://www.w3.org/2000/svg" width="50" height="50">
   <circle cx="25" cy="25" r="20" fill="blue" onclick="alert('pwn')" />
   <script>window.parent.alert("pwn")</script>
@@ -43,15 +43,22 @@ describe("HtmlSvgRenderer", () => {
 
     render(<HtmlSvgRenderer language="svg" source={malicious} />);
 
-    const previewHost = screen.getByTestId("html-svg-renderer-svg-preview");
-    // The circle survives, but every script tag and on* handler must be
-    // stripped by DOMPurify before the SVG is inserted into the DOM.
-    const circle = previewHost.querySelector("circle");
-    expect(circle).not.toBeNull();
-    expect(circle?.getAttribute("onclick")).toBeNull();
-    expect(previewHost.querySelector("script")).toBeNull();
-    expect(previewHost.innerHTML.toLowerCase()).not.toContain("onclick");
-    expect(previewHost.innerHTML.toLowerCase()).not.toContain("alert");
+    const iframe = screen.getByTestId(
+      "html-svg-renderer-svg-preview",
+    ) as HTMLIFrameElement;
+    expect(iframe.tagName).toBe("IFRAME");
+    // SECURITY: SVG iframe must NEVER allow scripts or same-origin -- those
+    // would re-introduce the host-page-leak / XSS regressions the iframe
+    // boundary is here to prevent.
+    expect(iframe.getAttribute("sandbox")).toBe("");
+    const srcdoc = (iframe.getAttribute("srcdoc") ?? iframe.srcdoc).toLowerCase();
+    expect(srcdoc).toContain("<circle");
+    expect(srcdoc).not.toContain("<script");
+    expect(srcdoc).not.toContain("onclick");
+    expect(srcdoc).not.toContain("alert");
+    // CSP is the second line of defence: block all network egress except
+    // data: images so a future sanitizer regression cannot beacon out.
+    expect(srcdoc).toContain("default-src 'none'");
   });
 
   it("toggles between Preview and Code tabs", () => {
@@ -164,4 +171,28 @@ describe("sanitizeSvgSource", () => {
     expect(clean.startsWith("<?xml")).toBe(false);
     expect(clean).toContain("<rect");
   });
+
+  it("strips inline <style> blocks so SVG CSS cannot retarget host selectors", () => {
+    const svg = `<svg xmlns="http://www.w3.org/2000/svg"><style>body{display:none!important}</style><rect/></svg>`;
+    const clean = sanitizeSvgSource(svg).toLowerCase();
+    expect(clean).not.toContain("<style");
+    expect(clean).not.toContain("display:none");
+    expect(clean).toContain("<rect");
+  });
+
+  it("strips style attributes so inline CSS cannot fire url()/@import requests", () => {
+    const svg = `<svg xmlns="http://www.w3.org/2000/svg"><circle style="background:url(https://evil.example/x)"/></svg>`;
+    const clean = sanitizeSvgSource(svg).toLowerCase();
+    expect(clean).toContain("<circle");
+    expect(clean).not.toContain("style=");
+    expect(clean).not.toContain("evil.example");
+  });
+
+  it("drops <image>/<use> tags so SVG cannot beacon to external URLs", () => {
+    const svg = `<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><image href="https://evil.example/pixel"/><use xlink:href="https://evil.example/use"/></svg>`;
+    const clean = sanitizeSvgSource(svg).toLowerCase();
+    expect(clean).not.toContain("<image");
+    expect(clean).not.toContain("<use");
+    expect(clean).not.toContain("evil.example");
+  });
 });
diff --git a/studio/frontend/src/components/assistant-ui/html-svg-renderer.tsx b/studio/frontend/src/components/assistant-ui/html-svg-renderer.tsx
index e269c58055..1ae96f3b3b 100644
--- a/studio/frontend/src/components/assistant-ui/html-svg-renderer.tsx
+++ b/studio/frontend/src/components/assistant-ui/html-svg-renderer.tsx
@@ -42,14 +42,38 @@ const COPY_RESET_MS = 2000;
 const HEURISTIC_UNSAFE_SVG_RE =
   /<script[\s>]|<foreignObject[\s>]|<iframe[\s>]|<embed[\s>]|<object[\s>]/i;
 
-// SVGs may legitimately reference fonts/images via href, so we keep those.
-// We strip every event handler attribute (on*) and any tag DOMPurify would
-// otherwise allow that could escape the SVG sandbox.
+// SVG previews used to live inside a `<img src="data:image/svg+xml,...">` tag
+// where the browser treats the SVG as an image and disables scripts and
+// external resource loads. Mounting sanitized SVG directly into the host
+// Studio document loses those guarantees, so we now (a) strip every node that
+// can leak into the host page (`<style>`, `<image>`, `<use>`, scripts, etc.)
+// and (b) render the surviving markup in a fully sandboxed iframe at
+// `SvgPreview` below for defence in depth. See:
+// https://developer.mozilla.org/en-US/docs/Web/SVG/Guides/SVG_as_an_image
 const SVG_PURIFY_CONFIG = {
   USE_PROFILES: { svg: true, svgFilters: true },
-  FORBID_TAGS: ["script", "foreignObject", "iframe", "embed", "object"],
-  // DOMPurify already drops on* handlers when USE_PROFILES is set, but we
-  // call this out explicitly so the intent is obvious to future readers.
+  // ``style`` -- inline CSS would otherwise leak to the host page selectors.
+  // ``image`` / ``use`` -- carry ``href``/``xlink:href`` and would let an
+  //   assistant fetch attacker-controlled URLs from the user's browser.
+  // ``foreignObject`` -- can embed HTML inside the SVG and re-introduce XSS.
+  FORBID_TAGS: [
+    "script",
+    "style",
+    "foreignObject",
+    "iframe",
+    "embed",
+    "object",
+    "image",
+    "use",
+    "link",
+    "meta",
+  ],
+  // Drop URL-bearing attributes that would still trigger network requests on
+  // surviving SVG elements (filter url(...), animateMotion mpath, etc.), and
+  // strip inline ``style`` because CSS ``@import`` / ``url()`` would do the
+  // same. DOMPurify already drops on* handlers under USE_PROFILES, but we
+  // call ALLOW_DATA_ATTR off explicitly so the intent is obvious.
+  FORBID_ATTR: ["href", "xlink:href", "style"],
   ALLOW_DATA_ATTR: false,
 };
 
@@ -148,14 +172,44 @@ function TabButton({
   );
 }
 
+// SVG preview goes inside a sandboxed iframe (no allow-scripts, no
+// allow-same-origin) plus a `default-src 'none'` CSP so the sanitizer is
+// not the only line of defence -- even if a future DOMPurify regression
+// leaks a URL-bearing attribute, the browser blocks the request.
+const SVG_IFRAME_CSP =
+  "default-src 'none'; img-src data:; style-src 'unsafe-inline'; font-src data:;";
+
+function buildSvgSrcDoc(safeSvg: string): string {
+  return [
+    "<!doctype html>",
+    `<meta http-equiv="Content-Security-Policy" content="${SVG_IFRAME_CSP}">`,
+    "<style>html,body{margin:0;padding:0;background:white;}",
+    "body{display:flex;align-items:center;justify-content:center;padding:16px;}",
+    "svg{max-width:100%;height:auto;}</style>",
+    safeSvg,
+  ].join("");
+}
+
 function SvgPreview({ source }: { source: string }) {
-  const safe = useMemo(() => sanitizeSvgSource(source), [source]);
+  const srcDoc = useMemo(() => buildSvgSrcDoc(sanitizeSvgSource(source)), [
+    source,
+  ]);
   return (
-    <div
+    <iframe
       data-testid="html-svg-renderer-svg-preview"
-      className="flex justify-center bg-white p-4 dark:bg-neutral-100"
-      // biome-ignore lint/security/noDangerouslySetInnerHtml: sanitized by DOMPurify above.
-      dangerouslySetInnerHTML={{ __html: safe }}
+      title="SVG preview"
+      srcDoc={srcDoc}
+      // SECURITY: sandbox="" forbids scripts AND blocks the iframe from
+      // inheriting the host origin, so even sanitized SVG cannot reach the
+      // Studio document or run network requests against host-cookied URLs.
+      sandbox=""
+      style={{
+        width: "100%",
+        height: 360,
+        border: "none",
+        display: "block",
+        background: "white",
+      }}
     />
   );
 }