feat: add sandboxed inline HTML widgets to chat transcript#106
feat: add sandboxed inline HTML widgets to chat transcript#106Stealinglight wants to merge 1 commit into
Conversation
Review Summary by QodoAdd sandboxed inline HTML widgets to chat transcripts with theme-aware rendering and streaming safety
WalkthroughsDescription• **Adds sandboxed inline HTML widget support to chat transcripts** via <widget> and <mcwidget> tags, enabling Chart.js visualizations, Tailwind-styled components, and interactive tools to render directly in assistant responses as deterministic iframes • **Pure TypeScript parser** (parseWidgetSegments) with streaming-safe truncation, FNV-1a content hashing for stable widget IDs, and exhaustive edge-case handling (unclosed tags, nested widgets, malformed attributes) • **Sandboxed iframe component** (InlineWidget) with hard-coded "allow-scripts allow-popups" sandbox attribute, source-identity postMessage validation, height clamping [60–600px], and per-widget error boundary with "View source" fallback • **Theme-aware rendering** via 9 CSS variables (light/dark fallbacks), live theme broadcast using MutationObserver on document.documentElement class changes, and in-place iframe style updates without srcDoc rebuild • **Widget capability auto-injection** into TOOLS.md at compose time (not persisted) with idempotent sentinel-based detection; agents get widget support by default • **Comprehensive test coverage** including 334-line parser unit tests (95%+ branch coverage), React component tests, e2e Playwright tests validating sandbox enforcement and replay parity, and regression tests ensuring widget-less markdown renders identically • **Security hardening** via ESLint rule banning allow-same-origin sandbox token, source-identity validation on all postMessages, and height clamping on both widget and parent sides • **Documentation** including agent capability prompt (~280 tokens) with worked Chart.js example, CSP guidance, and design decision rationale Diagramflowchart LR
A["Assistant Response Text"] -->|parseWidgetSegments| B["Markdown + Widget Segments"]
B -->|AssistantMarkdownContent| C["ReactMarkdown + InlineWidget"]
C -->|InlineWidget| D["Sandboxed iframe srcDoc"]
D -->|buildWidgetSrcDoc| E["HTML with Theme Vars + CDN URLs"]
F["Theme Changes"] -->|MutationObserver| G["widgetThemeBroadcast"]
G -->|postMessage| D
D -->|height:update| H["widgetMessageRegistry"]
H -->|clamp + rate-limit| I["Parent Height Update"]
J["TOOLS.md Compose"] -->|composeToolsContentWithWidgetPrompt| K["WIDGET_PROMPT Injected"]
File Changes1. tests/unit/parseWidgetSegments.test.ts
|
Code Review by Qodo
1. widget-csp.md recommends unsafe-inline
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38b33abe66
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <div className="agent-markdown text-foreground"> | ||
| <ReactMarkdown remarkPlugins={[remarkGfm]}>{rewritten}</ReactMarkdown> | ||
| </div> | ||
| <AssistantMarkdownContent text={contentText} isStreaming={true} /> |
There was a problem hiding this comment.
Route streaming widgets through the widget renderer
This new streaming renderer is still only reached after the legacy MEDIA: checks above. When an assistant streams <widget title="...">... without a MEDIA: line, the branch returns the raw whitespace div instead, so widget parsing and the “Generating widget…” indicator never run until the message stops streaming; route streaming assistant text through this component whenever widget tags are present, not only when media markdown is detected.
Useful? React with 👍 / 👎.
| {!collapsed ? ( | ||
| <iframe | ||
| ref={iframeRef} | ||
| title={titleText} | ||
| sandbox={WIDGET_SANDBOX} | ||
| srcDoc={srcDoc} | ||
| className={IFRAME_CLASSES} | ||
| style={{ height: `${height}px` }} | ||
| /> |
There was a problem hiding this comment.
Keep iframe registrations valid after expanding
When the user collapses a widget, this conditional unmounts the iframe, but the height/theme effects that registered its contentWindow only depend on id/srcDoc and are not re-run just because the ref points to a new iframe after expanding. The registry therefore keeps the removed iframe's window, so postMessages from the newly expanded iframe fail the source check and height/theme updates stop working for that widget.
Useful? React with 👍 / 👎.
| kind: "widget", | ||
| title: span.title, | ||
| html: span.html, | ||
| widgetId: fnv1a(span.html).toString(16), |
There was a problem hiding this comment.
Make widget IDs unique per occurrence
Hashing only span.html gives two widgets with identical HTML the same widgetId in the same transcript. Because both the height registry and theme broadcaster are keyed by widgetId, the later widget replaces the earlier entry; height messages from the first iframe then fail the event.source check and it stays at the default height. Include an occurrence/position component while preserving stability across re-renders.
Useful? React with 👍 / 👎.
| ### `script-src 'unsafe-inline' https://cdn.jsdelivr.net` | ||
|
|
||
| Widgets execute inline `<script>` tags inside their srcDoc. Browsers | ||
| treat `about:srcdoc` document CSP inheritance unevenly — Chrome and | ||
| Firefox have historically differed on which directives flow into the | ||
| srcDoc context. To stay compatible, the parent page's `script-src` | ||
| must allow inline scripts AND the jsdelivr CDN that hosts the widget | ||
| runtime libraries. | ||
|
|
||
| - `'unsafe-inline'` — required for `<script>` blocks in widget HTML. | ||
| The sandbox prevents these scripts from reaching parent-origin | ||
| resources, so `'unsafe-inline'` here does **not** weaken Studio's | ||
| own attack surface. | ||
| - `https://cdn.jsdelivr.net` — origin for the pinned Tailwind v4 | ||
| browser CDN (`@tailwindcss/browser@4`) and Chart.js | ||
| (`[email protected]/dist/chart.umd.min.js`). Both are loaded inside | ||
| the iframe at widget-mount time. | ||
|
|
||
| ### `frame-src 'self'` | ||
|
|
||
| Studio embeds widgets via `<iframe srcdoc>`, which navigates to the | ||
| synthetic URL `about:srcdoc`. Browsers consistently allow `srcdoc` | ||
| iframes when `frame-src` permits the embedding origin (`'self'`). | ||
| Adding `'self'` is sufficient — do **not** add `data:` or `about:` | ||
| schemes; they don't apply to `srcdoc` and add unrelated attack | ||
| surface. | ||
|
|
||
| ### `img-src *` | ||
|
|
||
| Widget HTML may reference user-supplied or agent-generated image URLs | ||
| from arbitrary origins (chart screenshots, status icons, external | ||
| assets). Restricting `img-src` would block legitimate widget content. | ||
| Images load from the iframe context only, where the sandbox prevents | ||
| them from reading parent storage; allowing `*` here is consistent with | ||
| the sandbox boundary. |
There was a problem hiding this comment.
1. widget-csp.md recommends unsafe-inline 📘 Rule violation ⛨ Security
The new CSP guidance instructs adding permissive directives like script-src 'unsafe-inline' and img-src *, which is unsafe and not appropriate as generic open-source repository guidance because it encourages weakening CSP protections broadly.
Agent Prompt
## Issue description
`docs/widget-csp.md` currently presents permissive CSP directives (e.g., `script-src 'unsafe-inline'` and `img-src *`) as required guidance. This is risky and not suitable as generic open-source documentation because it encourages broadly weakening CSP on the parent page.
## Issue Context
The compliance checklist requires repository instructions/documentation to remain generic and safe for open source audiences. CSP guidance should avoid prescribing insecure defaults; if exceptions are necessary, they should be framed as last-resort with safer alternatives (nonces/hashes, tighter allowlists, widget-only isolation) and clear security warnings.
## Fix Focus Areas
- docs/widget-csp.md[26-60]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| segments.push({ | ||
| kind: "widget", | ||
| title: span.title, | ||
| html: span.html, | ||
| widgetId: fnv1a(span.html).toString(16), | ||
| }); |
There was a problem hiding this comment.
2. Widgetid collision breaks updates 🐞 Bug ≡ Correctness
parseWidgetSegments derives widgetId solely from widget HTML, so two widgets with identical bodies in the same message will share the same id. InlineWidget uses this id to key the global height/theme registries, so the later widget overwrites the earlier entry and the first iframe stops receiving height/theme updates.
Agent Prompt
## Issue description
`parseWidgetSegments` sets `widgetId` to `fnv1a(span.html).toString(16)`, which is not unique per widget instance. When two widgets have identical `html` in one assistant message, they get the same `widgetId`, and `widgetMessageRegistry` / `widgetThemeBroadcast` (both `Map`s keyed by `widgetId`) will overwrite the earlier widget’s entry, breaking height updates and theme broadcasts for the first widget.
## Issue Context
- `AssistantMarkdownContent` passes `segment.widgetId` as the `id` prop to `<InlineWidget />`.
- `<InlineWidget />` registers height/theme subscriptions keyed by `id`.
- The registries use `Map.set(widgetId, ...)` which replaces previous entries.
## Fix Focus Areas
- src/features/agents/components/widgets/parseWidgetSegments.ts[190-224]
- src/features/agents/components/widgets/AssistantMarkdownContent.tsx[82-112]
- src/features/agents/components/widgets/widgetMessageRegistry.ts[35-113]
- src/features/agents/components/widgets/widgetThemeBroadcast.ts[16-75]
## Implementation notes
- Change `widgetId` to be deterministic **and unique per occurrence** within the parsed message, e.g.:
- Maintain a `let widgetOrdinal = 0;` inside `parseInner`, increment per emitted widget, and set `widgetId` to `${fnv1a(span.html).toString(16)}-${widgetOrdinal}`; or
- Incorporate the open-tag byte index (`openStart`) into the id (`${hash}-${openStart}`), which is deterministic for a given message.
- Update affected tests that currently assert two identical HTML bodies produce identical `widgetId` (they should now be different IDs but still stable across renders).
- If you still want a “content hash” for iframe DOM stability, keep it as a separate field (e.g. `contentHash`) and use the unique instance id only for registry routing.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
## Summary Adds support for `<widget title="...">...</widget>` tags in assistant responses. Each widget renders as a sandboxed `<iframe srcDoc>` inline with the surrounding markdown — enabling Chart.js visualizations, Tailwind-styled status cards, color-coded tables, and simple interactive tools to appear directly in chat. Also accepts `<mcwidget>` as an alias for compatibility with agents trained on that prefix. The change is browser-only. No new gateway methods, no new API routes, no edits to control-plane / intent / runtime code paths. Theme-aware via Studio's existing CSS variables; live theme broadcast via postMessage so widgets re-theme on dark/light toggle without remounting their Chart.js instances. ## Design decisions - **Parser pre-splits widget tags out of the raw markdown string before ReactMarkdown sees it.** Avoids depending on `rehype-raw`, which would enable arbitrary HTML passthrough for the entire markdown pipeline and entangle iframe key stability with markdown reconciliation. The parser is pure TypeScript with zero new runtime deps, exhaustively tested for edge cases (unclosed tags during streaming, nested tags, malformed attributes, curly-quote normalization). - **Sandbox attribute is the literal constant `"allow-scripts allow-popups"`.** Hard-coded module constant; never templated. ESLint rule (`no-restricted-syntax`) bans the literal string `"allow-same-origin"` anywhere in the `src/` tree to prevent accidental sandbox-escape via the WHATWG-documented document-reload pattern. - **Content-hash widget keys (FNV-1a 32-bit)** keep the iframe DOM node stable across SSE chunks during streaming. Avoids the `Date.now()`/`Math.random()` pattern that would remount on every render — Chart.js restart, scroll jumps, listener leaks. - **Live theme broadcast via postMessage** with a single MutationObserver on `document.documentElement` class changes. Iframes update `:root` style vars in place; no srcDoc rebuild on toggle, so chart animations and JS state survive theme changes. - **Single global `window.message` listener** (ref-counted registry keyed by widgetId). Validates `event.source === iframe.contentWindow` on every message — opaque-origin srcdoc iframes make `event.origin` meaningless, so source-identity is the trust boundary. Heights are clamped to `[60, 600]` on receive (parent side), independent of any widget-side clamping. - **Per-widget React error boundary** with a "View source" disclosure fallback. A single bad widget cannot crash the surrounding transcript; the user sees the raw HTML the agent emitted in a collapsed `<details>` block. - **`WIDGET_PROMPT` auto-inject into TOOLS.md at compose time** (not written to disk). Agents get widget capability by default; turning it off in a future release is a single-line revert with no per-agent migration. Idempotent on a literal `[WIDGETS]` sentinel to avoid double-injection. ## Testing - `npm run lint` — passes (2 pre-existing baseline errors in `AgentChatPanel.tsx:707` and `GatewayConnectScreen.tsx:90`, authored upstream; this PR introduces zero new lint errors) - `npm run typecheck` — passes (exit 0) - `npm run test` — 909/909 vitest tests pass - `npm run e2e` — 6/6 Playwright tests pass (`tests/e2e/widget-replay-parity.spec.ts`) - `@axe-core/playwright` baseline: zero new accessibility violations vs a no-widgets baseline page - Manual perf readout (synthetic transcript with 10 widgets): LCP 1.3s, CLS 0.27 (high, see Out of Scope), TBT moderate. The CLS is iframe-mount layout shift; v1.x mitigation via min-height reservation or CSS height transition. ## Files changed New widget code under `src/features/agents/components/widgets/`: - `parseWidgetSegments.ts` — pure streaming-safe parser - `buildWidgetSrcDoc.ts` — pure iframe srcDoc assembler - `widgetTheme.ts` — theme snapshot + Studio-token mapping - `widgetPromptSnippet.ts` — `WIDGET_PROMPT` agent guidance - `widgetConstants.ts` — sandbox literal, height clamps, etc. - `widgetMessageRegistry.ts` — single global postMessage listener - `widgetThemeBroadcast.ts` — MutationObserver theme propagation - `InlineWidget.tsx` — memoized iframe component - `InlineWidgetErrorBoundary.tsx` — per-widget React error boundary - `AssistantMarkdownContent.tsx` — wrapper integrating with markdown Tests: - `tests/unit/*.test.ts` — 10 new vitest files - `tests/e2e/widget-replay-parity.spec.ts` — Playwright e2e - `tests/e2e/helpers/widgetTranscriptStub.ts` — test harness - `tests/eslint-rules/sandbox-allow-same-origin.fixture.tsx` — ESLint rule fixture - `tests/setup.ts` — ResizeObserver shim (jsdom) Modified for integration: - `src/features/agents/components/AgentChatPanel.tsx` — two ReactMarkdown call sites in the assistant render path swapped for `<AssistantMarkdownContent />`. Tool/thinking/user content paths unchanged. - `src/lib/agents/personalityBuilder.ts` — `WIDGET_PROMPT` auto-injected into `TOOLS.md` at the existing `serializePersonalityFiles` boundary, idempotent on a `[WIDGETS]` sentinel. Modified for tooling: - `eslint.config.mjs` — `no-restricted-syntax` rule banning `"allow-same-origin"` in `src/` - `vitest.config.ts` — coverage scope for `parseWidgetSegments` - `package.json` / `package-lock.json` — adds two devDependencies only: `@axe-core/playwright` (e2e a11y baseline) and `@vitest/coverage-v8` (parser branch-coverage gate) New shipped doc: - `docs/widget-csp.md` — minimum CSP directives required if Studio ever ships a Content-Security-Policy header. Documents the `script-src` / `frame-src` / `img-src` requirements for the Tailwind v4 + Chart.js CDN pattern used inside srcdoc. ## Out of scope (deliberately deferred) - `allow-same-origin` sandbox token — banned at the lint level; WHATWG-documented sandbox-escape pattern. - `allow-forms`, `allow-top-navigation` — banned for similar reasons. - Two-way widget→agent RPC (postMessage protocol beyond height-reporting) — opens a security-review surface; v2 design. - Side-panel "artifacts"-style render — wrong product shape; widgets are defined by being inline in the transcript. - `window.claude.complete()`-style agent callbacks inside widgets — out of scope for the v1 inline-render contract. - Web-vitals CI gate — perf budgets documented in this PR description; CI gating is v1.x once min-height reservation mitigates the CLS hot-spot. - Prompt-eval harness for agent malformed-tag rate — useful follow-up; not blocking v1. ## Gateway behaviour changed: no This PR is browser-only. Zero changes to: - `src/lib/controlplane/**` - `src/lib/gateway/GatewayClient.ts` - `server/**` - `/api/gateway/*`, `/api/intents/*`, `/api/runtime/*` The Studio reducer, SSE pipeline, SQLite outbox, and all gateway adapter code are untouched. `git diff main...HEAD --name-only` shows only widget code, tests, two integration-point files, and tooling config.
38b33ab to
e785a49
Compare
|
Thanks for the review. Pushed 1. CSP doc tone — added a "Scope" callout near the top of 2. widgetId collision — 3. Height protocol mismatch — the auto-injected reporter in 4. SEC-01 fixture not CI-run — added Verification: |
|
/agentic_review Re-review request: PR head is now |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
feat: add sandboxed inline HTML widgets to chat transcript
Summary
Adds support for
<widget title="...">...</widget>tags in assistantresponses. Each widget renders as a sandboxed
<iframe srcDoc>inlinewith the surrounding markdown — enabling Chart.js visualizations,
Tailwind-styled status cards, color-coded tables, and simple
interactive tools to appear directly in chat. Also accepts
<mcwidget>as an alias for compatibility with agents trained on that prefix.
The change is browser-only. No new gateway methods, no new API routes,
no edits to control-plane / intent / runtime code paths. Theme-aware
via Studio's existing CSS variables; live theme broadcast via
postMessage so widgets re-theme on dark/light toggle without remounting
their Chart.js instances.
Design decisions
ReactMarkdown sees it. Avoids depending on
rehype-raw, which wouldenable arbitrary HTML passthrough for the entire markdown pipeline
and entangle iframe key stability with markdown reconciliation. The
parser is pure TypeScript with zero new runtime deps, exhaustively
tested for edge cases (unclosed tags during streaming, nested tags,
malformed attributes, curly-quote normalization).
"allow-scripts allow-popups".Hard-coded module constant; never templated. ESLint rule
(
no-restricted-syntax) bans the literal string"allow-same-origin"anywhere in the
src/tree to prevent accidental sandbox-escape viathe WHATWG-documented document-reload pattern.
stable across SSE chunks during streaming. Avoids the
Date.now()/Math.random()pattern that would remount on everyrender — Chart.js restart, scroll jumps, listener leaks.
MutationObserver on
document.documentElementclass changes.Iframes update
:rootstyle vars in place; no srcDoc rebuild ontoggle, so chart animations and JS state survive theme changes.
window.messagelistener (ref-counted registrykeyed by widgetId). Validates
event.source === iframe.contentWindowon every message — opaque-origin srcdoc iframes make
event.originmeaningless, so source-identity is the trust boundary. Heights are
clamped to
[60, 600]on receive (parent side), independent of anywidget-side clamping.
fallback. A single bad widget cannot crash the surrounding transcript;
the user sees the raw HTML the agent emitted in a collapsed
<details>block.
WIDGET_PROMPTauto-inject into TOOLS.md at compose time (notwritten to disk). Agents get widget capability by default; turning
it off in a future release is a single-line revert with no
per-agent migration. Idempotent on a literal
[WIDGETS]sentinelto avoid double-injection.
Testing
npm run lint— passes (2 pre-existing baseline errors inAgentChatPanel.tsx:707andGatewayConnectScreen.tsx:90,authored upstream; this PR introduces zero new lint errors)
npm run typecheck— passes (exit 0)npm run test— 909/909 vitest tests passnpm run e2e— 6/6 Playwright tests pass(
tests/e2e/widget-replay-parity.spec.ts)@axe-core/playwrightbaseline: zero new accessibility violationsvs a no-widgets baseline page
LCP 1.3s, CLS 0.27 (high, see Out of Scope), TBT moderate. The
CLS is iframe-mount layout shift; v1.x mitigation via min-height
reservation or CSS height transition.
Files changed
New widget code under
src/features/agents/components/widgets/:parseWidgetSegments.ts— pure streaming-safe parserbuildWidgetSrcDoc.ts— pure iframe srcDoc assemblerwidgetTheme.ts— theme snapshot + Studio-token mappingwidgetPromptSnippet.ts—WIDGET_PROMPTagent guidancewidgetConstants.ts— sandbox literal, height clamps, etc.widgetMessageRegistry.ts— single global postMessage listenerwidgetThemeBroadcast.ts— MutationObserver theme propagationInlineWidget.tsx— memoized iframe componentInlineWidgetErrorBoundary.tsx— per-widget React error boundaryAssistantMarkdownContent.tsx— wrapper integrating with markdownTests:
tests/unit/*.test.ts— 10 new vitest filestests/e2e/widget-replay-parity.spec.ts— Playwright e2etests/e2e/helpers/widgetTranscriptStub.ts— test harnesstests/eslint-rules/sandbox-allow-same-origin.fixture.tsx— ESLint rule fixturetests/setup.ts— ResizeObserver shim (jsdom)Modified for integration:
src/features/agents/components/AgentChatPanel.tsx— twoReactMarkdown call sites in the assistant render path swapped
for
<AssistantMarkdownContent />. Tool/thinking/user contentpaths unchanged.
src/lib/agents/personalityBuilder.ts—WIDGET_PROMPTauto-injected into
TOOLS.mdat the existingserializePersonalityFilesboundary, idempotent on a[WIDGETS]sentinel.
Modified for tooling:
eslint.config.mjs—no-restricted-syntaxrule banning"allow-same-origin"insrc/vitest.config.ts— coverage scope forparseWidgetSegmentspackage.json/package-lock.json— adds two devDependenciesonly:
@axe-core/playwright(e2e a11y baseline) and@vitest/coverage-v8(parser branch-coverage gate)New shipped doc:
docs/widget-csp.md— minimum CSP directives required if Studioever ships a Content-Security-Policy header. Documents the
script-src/frame-src/img-srcrequirements for theTailwind v4 + Chart.js CDN pattern used inside srcdoc.
Out of scope (deliberately deferred)
allow-same-originsandbox token — banned at the lint level;WHATWG-documented sandbox-escape pattern.
allow-forms,allow-top-navigation— banned for similar reasons.height-reporting) — opens a security-review surface; v2 design.
are defined by being inline in the transcript.
window.claude.complete()-style agent callbacks inside widgets —out of scope for the v1 inline-render contract.
description; CI gating is v1.x once min-height reservation
mitigates the CLS hot-spot.
follow-up; not blocking v1.
Gateway behaviour changed: no
This PR is browser-only. Zero changes to:
src/lib/controlplane/**src/lib/gateway/GatewayClient.tsserver/**/api/gateway/*,/api/intents/*,/api/runtime/*The Studio reducer, SSE pipeline, SQLite outbox, and all gateway
adapter code are untouched.
git diff main...HEAD --name-onlyshows only widget code, tests, two integration-point files, and
tooling config.