Repository navigation
Agent chat: a reply's html/svg fence draws as a sandboxed inline preview - #30673
Conversation
…e preview A closed ```html or ```svg fence now draws as its document in an iframe sandboxed with allow-scripts and no same-origin. Its srcdoc starts with the artifact policy (no network); before Run only the frame's nonce'd height reporter runs. Height follows the frame's own report, capped at 480 px. Code shows the source card (Copy kept). An open fence stays plain code. Scripted documents wait for Run, then use the native artifact frame. Meta refresh is made inert, link clicks are refused, and a frame that loads twice falls back to the source. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-92fa78a91e29868c5eaa
…-svg-fence-preview
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Gallery before/after for
States
Pick other states with a Passes: CI passes on CI passes on Written by red: cmux-next checks (god files, concurrency, crash safety, l10n); NEW: Shipped in:
|
GitHub Actions
Inactive
|
Owned by an agent session · session |
teamleaderleo
left a comment
teamleaderleo
left a comment
There was a problem hiding this comment.
SW-15 independent exact-head review — FIX [P2] at bfce631069df8b0bd2a86eb69fe9dc563cb1b942, cmux #30673 (@teamleaderleo). The sandbox/CSP/nonce approach is well bounded; an explicit Run click can nevertheless authorize a different chat's scripted HTML without another click.
InlinePreview.tsx:44-50 initializes each new InlinePreview with const [running,setRunning] = useState(() => ranRenders.has(code)), importing the module-global ranRenders from RenderCard.tsx:23-25. Clicking Run stores ranRenders.add(code) (InlinePreview.tsx:74-76). Markdown.tsx:984-987 creates preview components by code text only. After someone runs <button onclick="...">... in chat A, a subsequent new reply in chat B containing the same HTML bytes starts with running === true, enters FrameCard run={true}, mounts NativeFrame on a native pane and feeds the frame the script immediately. The current reply's Run button is bypassed. The set is shared across chats and tool render cards for the entire webview JS context; it does not include session ID, turn or code-fence identity. A malicious agent can deliberately repeat a previously approved snippet to trigger it without a fresh click. Sandbox isolation remains intact, but a busy-loop script can monopolize the shared WebKit process, and the PR specifically presents Run as a per-document consent gesture. This extends an earlier render-card cache to newly auto-rendered reply fences.
Minimal fix: Keep running as component-local state initialized to false (preserving source↔preview toggles within one document), and do not read/write the global render-card cache for inline fenced HTML. Or persist by a stable chat+turn+fence identity if preserving Run through virtualization is a requirement. Add regression: Run scripted HTML in one Markdown tree; unmount that tree/switch chat; render an identical scripted fence in a new tree; it must show Run and no iframe. A Code↔Preview toggle within the original component should retain its running state. Test both native and gallery conditions where feasible.
Source-only logic proof; no browser process or credential access. This is separate from the CSP policy and navigation guards and doesn't require changing them. I can provide the two-file light patch in a separate draft stacked on this owner branch, leaving the owner branch intact.
| // The frame left its document (its frame counts its loads): the card shows the source. | ||
| const [left, setLeft] = useState(false); | ||
| // Run needs a frame that may run script: the native artifact frame, or the gallery's srcdoc. | ||
| const canRun = scripted && (artifactFrameURL() !== undefined || artifactGallery()); |
There was a problem hiding this comment.
[P2] Run consent leaks between chats. This initializer reads ranRenders.has(code) from a module-global set also used by render cards. After a click to Run a scripted fence in chat A, the first render of identical code in chat B initializes running=true and mounts the native scripted iframe without clicking Run there. Please scope Run to a document identity (at minimum component-local state) and add a cross-chat same-code remount regression.
| type="button" | ||
| className="cv-codeblock__action cv-codeblock__action--text" | ||
| onClick={() => { | ||
| ranRenders.add(code); |
There was a problem hiding this comment.
ranRenders.add(code) makes user consent to run this document's script persistent for every future exact-text document in any chat. The sandbox limits network and parent access, but scripts can still monopolize the pane's WebKit process. Keep this state per fenced document (or key by chat+turn+fence) so subsequent replies require their own Run action.
|
SW-15 follow-up to the exact-head Run-consent review: I implemented the narrow fix as isolated draft stacked PR #30701 against this feature's head
|
|
Review (codex, router-review): SHIP at bfce631 Reviewer: codex gpt-6-astra high, fast tier. Follow-ups: 1. Each lands as an issue in this repo that links this receipt; the lane that opened the PR fixes it forward. SHIP at bfce631 Visual verdict: no images; Leo tries the build
Follow-ups:
|
![cmux-bot[bot]](https://avatars.githubusercontent.com/u/171392238?s=60&v=4){kind=small}
…pted documents stay code - http-equiv and <link> in the document are made inert (meta refresh navigates script-free; dns-prefetch/preconnect reach the network past CSP); x-dns-prefetch-control off. - The frame script refuses link clicks; a second load tears the frame down to the source. - A document with script never previews inline: it stays the code card, and Open takes it to the side panel where Run already is (#26216). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-92fa78a91e29868c5eaa
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Review (codex, router-review): SHIP at c6ba4ce Reviewer: codex gpt-6-astra high, fast tier. Follow-ups: 1. Each lands as an issue in this repo that links this receipt; the lane that opened the PR fixes it forward. SHIP at c6ba4ce Visual verdict: no images; Leo tries the build No correctness, regression, security, or test-breaking findings. Follow-ups:
|
GitHub Actions
Inactive
|
Found 7 test failures on Blacksmith runners: Failures
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
…ive DNS of agent-written hosts) Review of design/artifacts (gpt-6.1-sol) BLOCK: CSP does not stop an engine resolving <a href> hosts. Clicks on them are refused anyway, so cross-origin href/xlink:href become # with the original in data-href; fragments and inline data:/blob: images stay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-92fa78a91e29868c5eaa
GitHub Actions
Inactive
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 8 files
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/design-surfaces.json">
<violation number="1" location=".github/design-surfaces.json:158">
P3: The `artifacts` surface omits the files that wire the preview into the transcript: `conversation/Markdown.tsx` (fence routing) and `conversation/conversation.css` (the `.cv-inline-preview` styles). Add them so spec-check covers changes to the inline preview's integration and look.</violation>
</file>
<file name="webviews/test/inline-html-svg-preview-browser.test.ts">
<violation number="1" location="webviews/test/inline-html-svg-preview-browser.test.ts:109">
P3: The test title promises that fragments still scroll, but nothing asserts a scroll or fragment navigation. `inPrevented: false` only shows the default action was not cancelled. Either assert the result (for example, `location.hash === "#in"` in the frame) or drop "fragments still scroll" from the title.</violation>
</file>
<file name="webviews/src/agent-session/acpmux/conversation/InlinePreview.tsx">
<violation number="1" location="webviews/src/agent-session/acpmux/conversation/InlinePreview.tsx:51">
P3: This toggle changes its label and also sets `aria-pressed`. Keep one fixed accessible name and let `aria-pressed` carry the state, or drop `aria-pressed` and keep the label change.</violation>
</file>
<file name="webviews/test/inline-html-svg-preview.test.tsx">
<violation number="1" location="webviews/test/inline-html-svg-preview.test.tsx:171">
P3: This test dispatches two `load` events, so it would still pass if the frame tore down only after two extra loads. Assert `frame()` is non-null after the document's own load, then dispatch exactly one extra `load` and assert the fallback. That pins the `loads > 1` boundary in InlinePreview.tsx.</violation>
</file>
<file name="webviews/src/agent-session/acpmux/conversation/previewDocument.ts">
<violation number="1" location="webviews/src/agent-session/acpmux/conversation/previewDocument.ts:72">
P3: `inertDocument` rewrites every occurrence, including text nodes: `<p>http-equiv</p>` renders as `data-http-equiv`. Rename the meta attribute without mutating the whole document.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| "webviews/src/agent-session/acpmux/conversation/artifactSandbox.ts", | ||
| "webviews/src/agent-session/acpmux/conversation/artifactVersions.ts", | ||
| "webviews/src/agent-session/acpmux/conversation/RenderCard.tsx", | ||
| "webviews/src/agent-session/acpmux/conversation/renderCall.ts" |
There was a problem hiding this comment.
P3: The artifacts surface omits the files that wire the preview into the transcript: conversation/Markdown.tsx (fence routing) and conversation/conversation.css (the .cv-inline-preview styles). Add them so spec-check covers changes to the inline preview's integration and look.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At .github/design-surfaces.json, line 158:
<comment>The `artifacts` surface omits the files that wire the preview into the transcript: `conversation/Markdown.tsx` (fence routing) and `conversation/conversation.css` (the `.cv-inline-preview` styles). Add them so spec-check covers changes to the inline preview's integration and look.</comment>
<file context>
@@ -142,6 +142,21 @@
+ "webviews/src/agent-session/acpmux/conversation/artifactSandbox.ts",
+ "webviews/src/agent-session/acpmux/conversation/artifactVersions.ts",
+ "webviews/src/agent-session/acpmux/conversation/RenderCard.tsx",
+ "webviews/src/agent-session/acpmux/conversation/renderCall.ts"
+ ]
}
</file context>
| "webviews/src/agent-session/acpmux/conversation/renderCall.ts" | |
| "webviews/src/agent-session/acpmux/conversation/renderCall.ts", | |
| "webviews/src/agent-session/acpmux/conversation/Markdown.tsx", | |
| "webviews/src/agent-session/acpmux/conversation/conversation.css" |
| } | ||
|
|
||
| for (const engine of ["chromium", "webkit"]) { | ||
| test(`${engine}: a passive frame stays on its document: no refresh, no link out, fragments still scroll`, async () => { |
There was a problem hiding this comment.
P3: The test title promises that fragments still scroll, but nothing asserts a scroll or fragment navigation. inPrevented: false only shows the default action was not cancelled. Either assert the result (for example, location.hash === "#in" in the frame) or drop "fragments still scroll" from the title.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webviews/test/inline-html-svg-preview-browser.test.ts, line 109:
<comment>The test title promises that fragments still scroll, but nothing asserts a scroll or fragment navigation. `inPrevented: false` only shows the default action was not cancelled. Either assert the result (for example, `location.hash === "#in"` in the frame) or drop "fragments still scroll" from the title.</comment>
<file context>
@@ -0,0 +1,152 @@
+ }
+
+ for (const engine of ["chromium", "webkit"]) {
+ test(`${engine}: a passive frame stays on its document: no refresh, no link out, fragments still scroll`, async () => {
+ const { page, frame, requests } = await passiveFrame(engine);
+ const result = await frame.evaluate(() => {
</file context>
| <button | ||
| type="button" | ||
| className="cv-codeblock__action cv-codeblock__action--text" | ||
| aria-pressed={showCode} |
There was a problem hiding this comment.
P3: This toggle changes its label and also sets aria-pressed. Keep one fixed accessible name and let aria-pressed carry the state, or drop aria-pressed and keep the label change.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webviews/src/agent-session/acpmux/conversation/InlinePreview.tsx, line 51:
<comment>This toggle changes its label and also sets `aria-pressed`. Keep one fixed accessible name and let `aria-pressed` carry the state, or drop `aria-pressed` and keep the label change.</comment>
<file context>
@@ -0,0 +1,177 @@
+ <button
+ type="button"
+ className="cv-codeblock__action cv-codeblock__action--text"
+ aria-pressed={showCode}
+ onClick={() => setShowCode((value) => !value)}
+ >
</file context>
| await show("```svg\n" + SVG.replace("red", "blue") + "\n```\n"); | ||
| const iframe = frame()!; | ||
| // Its first load is its own document (jsdom may or may not have fired it); two more are past it. | ||
| for (const _ of [1, 2]) |
There was a problem hiding this comment.
P3: This test dispatches two load events, so it would still pass if the frame tore down only after two extra loads. Assert frame() is non-null after the document's own load, then dispatch exactly one extra load and assert the fallback. That pins the loads > 1 boundary in InlinePreview.tsx.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webviews/test/inline-html-svg-preview.test.tsx, line 171:
<comment>This test dispatches two `load` events, so it would still pass if the frame tore down only after two extra loads. Assert `frame()` is non-null after the document's own load, then dispatch exactly one extra `load` and assert the fallback. That pins the `loads > 1` boundary in InlinePreview.tsx.</comment>
<file context>
@@ -0,0 +1,205 @@
+ await show("```svg\n" + SVG.replace("red", "blue") + "\n```\n");
+ const iframe = frame()!;
+ // Its first load is its own document (jsdom may or may not have fired it); two more are past it.
+ for (const _ of [1, 2])
+ await act(() => {
+ iframe.dispatchEvent(new dom.window.Event("load"));
</file context>
| /// as written). | ||
| export function inertDocument(html: string): string { | ||
| return html | ||
| .replace(/http-equiv/gi, "data-http-equiv") |
There was a problem hiding this comment.
P3: inertDocument rewrites every occurrence, including text nodes: <p>http-equiv</p> renders as data-http-equiv. Rename the meta attribute without mutating the whole document.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webviews/src/agent-session/acpmux/conversation/previewDocument.ts, line 72:
<comment>`inertDocument` rewrites every occurrence, including text nodes: `<p>http-equiv</p>` renders as `data-http-equiv`. Rename the meta attribute without mutating the whole document.</comment>
<file context>
@@ -0,0 +1,112 @@
+/// as written).
+export function inertDocument(html: string): string {
+ return html
+ .replace(/http-equiv/gi, "data-http-equiv")
+ .replace(/<link(?=[\s/>])/gi, "<cmux-inert-link")
+ .replace(LINK_ATTRIBUTE, (whole, before: string, name: string, equals: string, value: string) => {
</file context>
|
Review (codex gpt-6.1-sol xhigh, router-review): BLOCK at 74ebf5a BLOCK at 74ebf5a
Source parsing and pure-function probes ran; Bun and browser suites were unavailable here. Follow-ups:
|
GitHub Actions
Failure
… link rewrite boundary; fragments scroll in place Review (gpt-6.1-sol) BLOCK at 74ebf5a: - a srcdoc frame takes the pane's CSP, which runs no inline script, so the nonce'd reporter never ran in the app. In the app the preview now uses the native artifact frame (own policy, no network, height report, host navigation delegate) with the inert, script-free document; srcdoc stays for the gallery; other pages show the source. - the href rewrite consumed the boundary quote, so adjacent attributes slipped past; it is a lookbehind now, with that input covered. - in srcdoc a #fragment resolves against the parent page; the frame script scrolls to it in place. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-92fa78a91e29868c5eaa
GitHub Actions
Inactive
GitHub Actions
Failure
|
Review (codex, router-review): SHIP at 9936b66 Reviewer: codex gpt-6-astra high, fast tier. Follow-ups: 1. Each lands as an issue in this repo that links this receipt; the lane that opened the PR fixes it forward. SHIP at 9936b66 Visual verdict: no images; Leo tries the build No correctness findings. Follow-ups: [P2][follow-up] No after screenshots were available for visual comparison. |
|
Review (independent Codex gpt-6.1-sol, high, via the review router; full diff from merge base 9f5e2e1 to this head): SHIP at 9936b66. [P2] [follow-up] webviews/src/agent-session/acpmux/conversation/previewDocument.ts:90 - Native previews leave |
A reply's closed
```htmlor```svgfence now draws inline as the document it holds (a chart, a table, a styled section) instead of as source, in a sandboxed frame. Code swaps in the source card (Copy stays), Open preview still opens the side panel. Nothing renders while the fence is still streaming.Leo, 2026-11-01, after watching Google's "Generate, edit, and preview files in Gemini" clip:
Spec: leo-workspace
design/artifacts.md(new surface; this PR adds its.github/design-surfaces.jsonentry).Security model
<iframe sandbox="allow-scripts">, neverallow-same-origin, so the document is an opaque origin with no access toparent.document, the pane's storage, its bridge or acpmux. No top navigation, popups, forms or modals.cmux-agent://render/artifact, the same host the side panel's Run uses): its own CSP (the artifact policy:default-src 'none'; connect-src 'none'; frame-src 'none'; form-action 'none'; base-uri 'none', images and fonts onlydata:/blob:), no network, its own height report, and the host's navigation delegate. The parent posts it the inert, script-free document. Asrcdocframe would take the pane's CSP, which runs no inline script, sosrcdocis used only in the gallery, behind the same policy with a per-frame noncescript-srcfor the client's frame script. Any other page shows the source.on*handlers andjavascript:URLs never run inline.artifactNeedsRun) never previews inline. It stays the code card, and Open takes it to the side panel, where Run already lives (feat(agent-chat): sandbox HTML artifacts before Run #26216) in the native artifact frame. Nothing auto-runs.<link>elements are made inert (dns-prefetchandpreconnecthints reach the network pastconnect-src),x-dns-prefetch-controlis off, and everyhref/xlink:hrefthat leaves the document becomes#(matched with a non-consuming boundary, so adjacent attributes are caught too) (original indata-href), so no engine can speculatively resolve an agent-written host. Fragments and inlinedata:/blob:images keep their value.http-equivattributes in the document are made inert, because<meta http-equiv="refresh">is the one way script-free markup can navigate its own frame, and neither CSP nor the sandbox stops it. The frame's own script refuses link clicks (a#fragmentscrolls in place, since insrcdocit would resolve against the parent page). A frame that loads a second time anyway is torn down and the card shows the source.cmux-render-sizeand the gallery framecmux-preview-size. The parent only takes it from that frame's owncontentWindow, clamps it to 48 px minimum, and caps the card at 480 px (the document scrolls past that; Expand raises the cap to 4000 px).Perf
block.open), and the frame mounts once the fence closes. It's keyed by the document text, so an unchanged block keeps its frame across streamed snapshots.useNearViewport), so a long chat doesn't start every frame at once.ResizeObserverinside the frame. It has no timer and is cleaned up with the frame.Tests
webviews/test/inline-html-svg-preview.test.tsx(jsdom): in the pane the frame is the native artifact frame, its sandbox attribute is exactlyallow-scriptswithoutallow-same-origin, and on ready it gets the inert document. The dev server shows the source. The gallery srcdoc starts with the CSP (noncescript-src, nounsafe-inline,connect-src 'none'). Height follows only the frame's own report and is capped, and Expand lifts the cap. A refresh meta and resource-hint links are inert, and outbound links point to#. A second load falls back to the source. An unclosed fence renders no iframe. Code and Preview toggle back and forth with Copy kept. A scripted document stays the code card with no inline Run.webviews/test/inline-html-svg-preview-browser.test.ts(real Chromium and WebKit): a passive frame has no live refresh, its link out points to#, and a fragment still scrolls. The document's script never runs, but the height report does. Even with inline scripts allowed, a script readingparent.documentthrows,fetchis refused, and no request reaches the probe host.agent-pane.inline-previewhas an SVG bar chart, an HTML table plus line chart, the Code view, a streaming (unclosed) fence, and a scripted document that stays code.Out of scope / next
DiagramBlock.Builds on #19473 (side panel), #26216 / #26286 (passive before Run, artifact host) and #21760 (render cards). It doesn't touch
CodeBlock.tsx,HighlightedCode.tsxorStreamingCode.tsx, which another open change is reworking.Fleet runs of both new test files plus
artifact-panel*.test.tsxpass (29/29). Lint, types, design-tokens, fixed-waits and browser-lanes checks are clean.Agent session: agent session