From 17afdfeb8d258e1cfa70c951200e53296ac3426e Mon Sep 17 00:00:00 2001
From: Daniel Han <danielhanchen@gmail.com>
Date: Mon, 25 May 2026 10:48:41 +0000
Subject: [PATCH] Studio: fix two Codex findings on PR 5717 head

P1 -- ``scripts/check_new_install_scripts.py``: the head-only
rejection only refused new HEAD entries, not deletions. That left a
two-step bypass open:
  1. PR A removes ``studio/frontend/.install-script-allowlist`` on
     main (passes, since the lockfile has no new install-script
     deps).
  2. PR B then hits the bootstrap path (base allowlist missing)
     and self-allowlists any newly introduced install-script
     dependency, because bootstrap mode accepts head as-is.
Now also fail when head DROPS trusted base entries. Allowlist
deletions must land via their own reviewed commit instead of
chaining into the bootstrap window.

P2 -- ``html-svg-renderer.tsx``: ``<style>`` is removed from the
SVG sanitizer's FORBID_TAGS. The original justification was "inline
CSS would leak to the host page selectors", but the SVG preview
runs inside ``sandbox=""`` plus ``default-src 'none'`` -- the inner
``<style>`` cannot reach host page selectors and cannot fetch
external URLs (the CSP blocks ``@import`` and ``url(...)``).
Stripping ``<style>`` was breaking legitimate class-styled SVG
exports from real diagram tools. The existing
"strips inline <style>" test is replaced with one that proves
class-styled SVG renders as authored.
---
 scripts/check_new_install_scripts.py          | 25 +++++++++++++++++++
 .../__tests__/html-svg-renderer.test.tsx      | 15 ++++++++---
 .../assistant-ui/html-svg-renderer.tsx        |  8 ++++--
 3 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/scripts/check_new_install_scripts.py b/scripts/check_new_install_scripts.py
index a536c9bbe1..18cad0a25c 100644
--- a/scripts/check_new_install_scripts.py
+++ b/scripts/check_new_install_scripts.py
@@ -372,6 +372,31 @@ def main(argv: list[str] | None = None) -> int:
                 )
             return 1
 
+        # Also refuse a PR that DROPS trusted base entries. Without
+        # this, an attacker could land a two-step bypass:
+        #   1. PR A removes ``.install-script-allowlist`` from base
+        #      (passes -- no new lockfile findings).
+        #   2. PR B then hits the bootstrap path (base allowlist
+        #      missing) and self-allowlists a newly introduced
+        #      install-script dependency.
+        # Removing an allowlist entry is a security-sensitive change
+        # and must land via the same review path that added it.
+        removed_from_head = sorted(base_allowlist - head_allowlist)
+        if removed_from_head:
+            print(
+                "[install-script-diff] FAIL: PR removes trusted base "
+                "allowlist entries. Allowlist deletions must land in a "
+                "separate, isolated commit so a follow-up PR cannot "
+                "exploit the bootstrap path.",
+                file = sys.stderr,
+            )
+            for entry in removed_from_head:
+                print(
+                    f"  dropped allowlist entry: {entry}",
+                    file = sys.stderr,
+                )
+            return 1
+
         # Only the trusted base allowlist participates in the skip set.
         allowlist = base_allowlist
 
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 2b13345ed2..7a89654c11 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
@@ -268,12 +268,19 @@ describe("sanitizeSvgSource", () => {
     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>`;
+  it("keeps inline <style> blocks so class-styled SVG exports still render", () => {
+    // The SVG preview iframe is fully sandboxed (sandbox="") and the
+    // inner CSP is default-src 'none', so the iframe's <style> cannot
+    // reach the host page selectors or fetch external URLs (CSP blocks
+    // @import / url(...)). Stripping <style> broke legitimate class-
+    // styled SVG exports from many diagram tools, which is the bigger
+    // real-world cost than the (already-mitigated) selector leak.
+    const svg = `<svg xmlns="http://www.w3.org/2000/svg"><style>.fg{fill:red}</style><rect class="fg"/></svg>`;
     const clean = sanitizeSvgSource(svg).toLowerCase();
-    expect(clean).not.toContain("<style");
-    expect(clean).not.toContain("display:none");
+    expect(clean).toContain("<style");
+    expect(clean).toContain(".fg");
     expect(clean).toContain("<rect");
+    expect(clean).toContain('class="fg"');
   });
 
   it("strips style attributes so inline CSS cannot fire url()/@import requests", () => {
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 d257ab9756..526c4f2a1a 100644
--- a/studio/frontend/src/components/assistant-ui/html-svg-renderer.tsx
+++ b/studio/frontend/src/components/assistant-ui/html-svg-renderer.tsx
@@ -53,13 +53,17 @@ const HEURISTIC_UNSAFE_SVG_RE =
 // https://developer.mozilla.org/en-US/docs/Web/SVG/Guides/SVG_as_an_image
 const SVG_PURIFY_CONFIG = {
   USE_PROFILES: { svg: true, svgFilters: true },
-  // ``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.
+  // ``script`` / ``link`` / ``meta`` / ``iframe`` / ``embed`` / ``object``
+  //   are unconditional XSS / network surfaces. ``<style>`` is kept --
+  //   the SVG preview iframe is fully sandboxed (``sandbox=""``) with a
+  //   ``default-src 'none'`` CSP, so class-based styling that real
+  //   diagram exporters emit cannot leak to the host page or fetch
+  //   external URLs (the CSP blocks ``@import`` and ``url(...)``).
   FORBID_TAGS: [
     "script",
-    "style",
     "foreignObject",
     "iframe",
     "embed",