Skip to content

fix(security): sandbox voice panel HTML in iframe (CRITICAL XSS) (#821) - #824

Merged
vybe merged 1 commit into
devfrom
feature/821-voice-panel-xss-iframe
May 13, 2026
Merged

fix(security): sandbox voice panel HTML in iframe (CRITICAL XSS) (#821)#824
vybe merged 1 commit into
devfrom
feature/821-voice-panel-xss-iframe

Conversation

@AndriiPasternak31

Copy link
Copy Markdown
Contributor

Fixes #821

Summary

Agent-controlled HTML from update_panel was DOMPurify-sanitized with ADD_TAGS:['script'], then _execScripts() cloned surviving <script> tags into live DOM nodes that executed in the parent origin. Since the agent's html argument is influenceable by any external input (chat, files, MCP tool results, webhooks, channel messages, agent-website visitors), this was a generic indirect-prompt-injection XSS sink in the admin UI with JWT-theft / cross-agent-pivot blast radius.

What changed

Render the panel inside an <iframe sandbox="allow-scripts"> instead. Without allow-same-origin / allow-forms / allow-popups / allow-top-navigation / allow-modals, the iframe has an opaque origin: scripts run (Chart.js still works) but cannot read parent localStorage / cookies / JWT, submit forms, navigate the parent, or open popups.

  • Drop DOMPurify ADD_TAGS:['script'] and the _execScripts() shim
  • Drop the parent-side window.Chart global injection
  • Load Chart.js inside the iframe via Vite ?url asset import (relative path; chart.js 4 doesn't expose the UMD bundle via package exports)
  • s_open / s_close concat avoids the Vue SFC parser tripping on the inner <script> tag inside the srcdoc template

Why this shape

The previous DOMPurify-then-execScripts pattern conflates content sanitization with code execution: any element that survives sanitization gets cloned into a live script tag. Even tightening the DOMPurify rules wouldn't eliminate the sink — the agent-controlled html is the input boundary, and the parent origin is the wrong place for it to evaluate.

The sandbox iframe inverts this: the agent's script runs (preserving the panel's intended Chart.js functionality) but in an origin that has no access to anything that matters. The fix is structural, not a filter tweak.

Test plan

  • Voice panel still renders Chart.js panels correctly
  • Voice panel scripts cannot read parent.localStorage (verified via test panel attempting to access)
  • Voice panel scripts cannot post to backend endpoints with parent's JWT
  • Vue SFC parser doesn't trip on the srcdoc template

Out of scope

This PR was previously bundled into #798 (circuit-breaker fix) — split out for isolated security review.

🤖 Generated with Claude Code

Agent-controlled HTML from update_panel was DOMPurify-sanitized with
ADD_TAGS:['script'], then _execScripts() cloned surviving <script> tags
into live DOM nodes that executed in the parent origin. Since the agent's
html argument is influenceable by any external input (chat, files, MCP
tool results, webhooks, channel messages, agent-website visitors), this
was a generic indirect-prompt-injection XSS sink in the admin UI with
JWT-theft / cross-agent-pivot blast radius.

Render the panel inside an <iframe sandbox="allow-scripts"> instead.
Without allow-same-origin / allow-forms / allow-popups / allow-top-
navigation / allow-modals, the iframe has an opaque origin: scripts run
(Chart.js still works) but cannot read parent localStorage / cookies /
JWT, submit forms, navigate the parent, or open popups.

- Drop DOMPurify ADD_TAGS:['script'] and the _execScripts() shim
- Drop the parent-side window.Chart global injection
- Load Chart.js inside the iframe via Vite ?url asset import (relative
  path; chart.js 4 doesn't expose the UMD bundle via package exports)
- s_open/s_close concat avoids the Vue SFC parser tripping on the inner
  <script> tag inside the srcdoc template

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct structural fix for the P0 XSS sink. Sandboxed iframe with opaque origin is the right defense — Chart.js still works, parent JWT/localStorage/cookies are fully isolated. Clean, minimal scope. Ship it.

@vybe
vybe merged commit 3007b69 into dev May 13, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants