From 7262e7730e0c7a28f54475c73bd83c03b0c99096 Mon Sep 17 00:00:00 2001
From: Athena Kaminsky <thegiantsnail@gmail.com>
Date: Thu, 20 Aug 2026 01:10:06 -0500
Subject: [PATCH] fix(preview): harden sandbox and transformation

---
 interface/web/index.html                      |  3 ++
 interface/web/src/Markdown.jsx                | 23 +++++----
 interface/web/src/preview/jsx-transform.js    | 47 ++++++++++++++++++-
 .../web/src/preview/jsx-transform.test.js     | 14 ++++++
 synapse/tools.py                              | 23 +++++++--
 tests/test_smoke.py                           | 14 ++++++
 tests/test_tools.py                           | 18 ++++++-
 7 files changed, 125 insertions(+), 17 deletions(-)
diff --git a/interface/web/index.html b/interface/web/index.html
index 6c697db..6b32eed 100644
--- a/interface/web/index.html
+++ b/interface/web/index.html
@@ -2,6 +2,9 @@
 <html lang="en">
   <head>
     <meta charset="UTF-8" />
+    <!-- Preview documents use data: URLs. Any later navigation of that child
+         browsing context is denied before a network request is sent. -->
+    <meta http-equiv="Content-Security-Policy" content="frame-src data:;" />
     <link rel="icon" type="image/svg+xml" href="/n small.png" />
     <meta name="viewport" content="width=device-width, initial-scale=1.0" />
     <title>NexusOS</title>
diff --git a/interface/web/src/Markdown.jsx b/interface/web/src/Markdown.jsx
index 35370f5..4aeefd8 100644
--- a/interface/web/src/Markdown.jsx
+++ b/interface/web/src/Markdown.jsx
@@ -304,8 +304,8 @@ const _MIN_PREVIEW_H = 160;
 const _MAX_PREVIEW_H = 720;
 const _MAX_H_STEPS = 60;
 
-// Live preview for a ```html or ```svg fenced block: a Preview/Code toggle,
-// rendered via a sandboxed <iframe srcDoc>.
+// Live preview for a renderable fenced block: a Preview/Code toggle rendered
+// via a sandboxed iframe whose document is an encoded data: URL.
 //
 // Trust boundary: `sandbox="allow-scripts"` — deliberately without
 // allow-same-origin, allow-forms, allow-popups, or allow-top-navigation. No
@@ -313,11 +313,13 @@ const _MAX_H_STEPS = 60;
 // actually matters here: even the inline scripts the CSP allows to run can't
 // read this app's cookies/localStorage, can't call its API (no credentialed
 // or same-origin fetch is possible), and can't reach `window.parent`. The CSP
-// above closes the remaining gap same-origin-denial doesn't cover on its own:
-// arbitrary outbound network access to the open internet. Nothing here is
-// treated as a substitute for real code execution sandboxing (Docker, a WASM
-// runtime, etc.) - this block never runs anything outside the browser's own
-// iframe sandbox, which is the point: no backend attack surface at all.
+// above blocks resource and script-initiated network access. The embedding
+// document's `frame-src data:` policy in index.html closes a separate CSP gap:
+// a child is otherwise allowed to navigate its own browsing context to a URL.
+// The initial data: document is allowed and inherits the parent policy, while
+// an http(s) navigation is rejected before its request is sent. Nothing here
+// substitutes for a general code-execution sandbox (Docker, WASM, etc.);
+// model-authored code runs only inside the browser's sandboxed frame.
 function RenderBlock({ lang, value, streaming }) {
   const [tab, setTab] = useState("preview");
   const [expanded, setExpanded] = useState(false);
@@ -331,7 +333,7 @@ function RenderBlock({ lang, value, streaming }) {
   };
 
   // Don't preview a block whose fence hasn't closed yet - it's incomplete
-  // markup by definition, and re-pointing an iframe's srcDoc at a half-formed
+  // markup by definition, and re-pointing an iframe at a half-formed
   // document on every streamed token is both wasteful and flickery. Code view
   // already has its own streaming indicator (the same dot CodeBlock uses).
   const showPreview = tab === "preview" && !streaming;
@@ -434,16 +436,17 @@ function PreviewFrame({ lang, value, expanded }) {
   // A build failure (JSX that doesn't parse) has no document to show at all, so
   // the message stands in for the frame rather than sitting under it.
   const { doc, error: buildError } = buildSrcDoc(lang, value);
+  const frameUrl = doc ? `data:text/html;charset=utf-8,${encodeURIComponent(doc)}` : "";
   const shown = buildError || error;
 
   return (
     <>
-      {doc && (
+      {frameUrl && (
         <iframe
           ref={frameRef}
           title="rendered output"
           sandbox="allow-scripts"
-          srcDoc={doc}
+          src={frameUrl}
           style={{
             width: "100%",
             height: expanded ? "70vh" : `${height}px`,
diff --git a/interface/web/src/preview/jsx-transform.js b/interface/web/src/preview/jsx-transform.js
index 1331707..8a7570a 100644
--- a/interface/web/src/preview/jsx-transform.js
+++ b/interface/web/src/preview/jsx-transform.js
@@ -298,7 +298,7 @@ function handleWord(sc, word) {
   if (word === "import" && atStatement) {
     if (sc.peek() === "(" || sc.peek() === ".") { sc.emit(word); return true; }  // import()/import.meta
     const from = sc.i;
-    skipStatement(sc);
+    skipImportStatement(sc);
     recordImport(sc, sc.src.slice(from, sc.i));
     return true;
   }
@@ -400,6 +400,51 @@ function skipStatement(sc) {
   sc.prevWord = word;
 }
 
+/**
+ * Drop a static import, including the common multiline named-import form.
+ * A bare newline before the module string is part of the import; one after it
+ * ends a semicolon-free import. Semicolons always win so a malformed import
+ * (for example a missing `}`) cannot swallow the rest of the component.
+ */
+function skipImportStatement(sc) {
+  const from = sc.i;
+  const sig = sc.prevSig;
+  const word = sc.prevWord;
+  let sawModule = false;
+
+  while (!sc.eof) {
+    const ch = sc.peek();
+    if (ch === ";") { sc.i++; break; }
+    if (ch === "\n" && sawModule) break;
+
+    if (ch === '"' || ch === "'") {
+      const outLength = sc.out.length;
+      copyString(sc, ch);
+      sc.out.length = outLength;
+      sawModule = true;
+      continue;
+    }
+    if (ch === "/" && sc.peek(1) === "/") {
+      const outLength = sc.out.length;
+      copyLineComment(sc);
+      sc.out.length = outLength;
+      continue;
+    }
+    if (ch === "/" && sc.peek(1) === "*") {
+      const outLength = sc.out.length;
+      copyBlockComment(sc);
+      sc.out.length = outLength;
+      continue;
+    }
+    sc.i++;
+  }
+
+  // Keep runtime error line numbers aligned with the source shown in Code.
+  sc.out.push(missingNewlines(sc.src.slice(from, sc.i), ""));
+  sc.prevSig = sig;
+  sc.prevWord = word;
+}
+
 function skipBalancedBraces(sc) {
   const from = sc.i;
   const sig = sc.prevSig;
diff --git a/interface/web/src/preview/jsx-transform.test.js b/interface/web/src/preview/jsx-transform.test.js
index cb19fee..287dddf 100644
--- a/interface/web/src/preview/jsx-transform.test.js
+++ b/interface/web/src/preview/jsx-transform.test.js
@@ -165,6 +165,20 @@ test("imports are dropped - the sandbox has no module loader", () => {
   assert.equal(js('import "./styles.css";\nconst x = 1;'), "const x = 1;");
 });
 
+test("multiline named imports are dropped as one statement", () => {
+  const source = `import {
+  useState,
+  useEffect,
+} from "react";
+const x = 1;`;
+  const out = transform(source);
+  assert.equal(out.code, "\n\n\n\nconst x = 1;");
+  assert.deepEqual(out.imports, [
+    { names: ["useState", "useEffect"], module: "react" },
+  ]);
+  assert.doesNotThrow(() => new Function(out.code));
+});
+
 test("dropped imports report the bindings they would have provided", () => {
   // So a missing name can say where it was supposed to come from, instead of
   // surfacing as "useInView is not defined" at its first use.
diff --git a/synapse/tools.py b/synapse/tools.py
index fccc95b..392f0d0 100644
--- a/synapse/tools.py
+++ b/synapse/tools.py
@@ -393,7 +393,15 @@ def _critique_shared(markup: str) -> list[str]:
     issues: list[str] = []
     lower = markup.lower()
 
-    if re.search(r"""(?i)(?:src|href)\s*=\s*['"]https?://""", markup):
+    external_attr = re.search(
+        r"""(?i)\b(?:src|srcset|href|xlink:href|poster|action|formaction|data)\b\s*=\s*(?:['"]\s*)?https?://""",
+        markup,
+    )
+    external_css = re.search(
+        r"""(?i)(?:url\s*\(\s*['"]?\s*https?://|@import\s+(?:url\s*\(\s*)?['"]?\s*https?://)""",
+        markup,
+    )
+    if external_attr or external_css:
         issues.append(
             "Remove external http(s) URLs — the sandboxed preview blocks them. "
             "Inline CSS/JS; use data: URIs for images/fonts."
@@ -1014,14 +1022,15 @@ STANDING_TOOLS = frozenset({"render_preview"})
 # User-message cues that justify running the (slow, non-stream) tool loop with
 # render_preview. Kept narrow so ordinary chat isn't blocked behind a tool turn.
 _RENDER_HINTS = (
-    "visual", "visualize", "visualization", "chart", "graph", "diagram",
-    "canvas", "plot", "interactive", "animation", "render_preview",
+    "visual", "visuals", "visualize", "visualization", "chart", "charts",
+    "graph", "graphs", "diagram", "diagrams", "canvas", "plot", "plots",
+    "interactive", "animation", "animations", "render_preview",
     "render preview", "svg", "draw me", "live preview",
     "demonstrate", "demo", "html demo", "html snippet", "html file",
     # Ways of asking for something that reacts to the pointer. "interactive"
     # alone missed "mouse-over sensitive", and with it the whole feature.
     "hover", "mouse", "drag", "click on", "real-time", "realtime",
-    "simulation", "simulate", "particle", "animate",
+    "simulation", "simulations", "simulate", "particle", "particles", "animate",
     # Every language the render window can display. Naming one is asking for a
     # preview, and this way a language added to PREVIEW_LANGS starts hinting
     # for itself instead of being unreachable until someone edits this tuple -
@@ -1031,8 +1040,12 @@ _RENDER_HINTS = (
 
 def wants_render_preview(message: str) -> bool:
     """True when this turn should advertise render_preview / enter the tool loop."""
+    import re
     lower = (message or "").lower()
-    return any(h in lower for h in _RENDER_HINTS)
+    return any(
+        re.search(rf"(?<![A-Za-z0-9_]){re.escape(hint)}(?![A-Za-z0-9_])", lower)
+        for hint in _RENDER_HINTS
+    )
 
 
 def is_action(name: str) -> bool:
diff --git a/tests/test_smoke.py b/tests/test_smoke.py
index 46cf2a2..80314ac 100644
--- a/tests/test_smoke.py
+++ b/tests/test_smoke.py
@@ -533,3 +533,17 @@ def test_update_apply_spawns_detached_and_refuses_a_second_run(monkeypatch):
 
     # Double-click must not launch a second pull/rebuild over the first.
     assert client.post("/update/apply").json()["started"] is False
+
+
+def test_preview_iframe_cannot_navigate_to_a_network_url():
+    """The child CSP blocks resource loads; the parent CSP must separately
+    block a sandboxed frame from navigating its own browsing context."""
+    index = (REPO_ROOT / "interface" / "web" / "index.html").read_text(encoding="utf-8")
+    markdown = (REPO_ROOT / "interface" / "web" / "src" / "Markdown.jsx").read_text(
+        encoding="utf-8"
+    )
+    assert "frame-src data:" in index
+    assert 'sandbox="allow-scripts"' in markdown
+    assert "encodeURIComponent(doc)" in markdown
+    assert "src={frameUrl}" in markdown
+    assert "srcDoc={doc}" not in markdown
diff --git a/tests/test_tools.py b/tests/test_tools.py
index 3d6d74e..872eb72 100644
--- a/tests/test_tools.py
+++ b/tests/test_tools.py
@@ -514,9 +514,25 @@ def test_asking_for_a_preview_language_or_pointer_interaction_offers_the_tool():
         assert tools.wants_render_preview(prompt), prompt
 
     # Still narrow: ordinary chat must not pay for a tool turn.
-    for prompt in ("what's the weather vibe today", "summarise this email thread"):
+    for prompt in (
+        "what's the weather vibe today",
+        "summarise this email thread",
+        "write a concise paragraph about caching",
+    ):
         assert not tools.wants_render_preview(prompt), prompt
 
+    assert tools.wants_render_preview("compare these graphs")
+
+
+def test_external_preview_resources_are_rejected_in_attributes_and_css():
+    for markup in (
+        '<img src=https://example.com/chart.png alt="chart">',
+        '<style>.chart { background: url("https://example.com/chart.png"); }</style>',
+        '<style>@import "https://example.com/chart.css";</style>',
+    ):
+        issues = tools._critique_shared(markup)
+        assert any("external http(s)" in issue for issue in issues), markup
+
 
 def test_every_preview_language_hints_for_itself():
     for lang in tools.PREVIEW_LANGS:
