Skip to content

fix(security): remove allow-same-origin from MCP Apps sandbox iframes#1063

Closed
zeroasterisk wants to merge 26 commits intogoogle:mainfrom
zeroasterisk:main
Closed

fix(security): remove allow-same-origin from MCP Apps sandbox iframes#1063
zeroasterisk wants to merge 26 commits intogoogle:mainfrom
zeroasterisk:main

Conversation

@zeroasterisk
Copy link
Copy Markdown
Collaborator

@zeroasterisk zeroasterisk commented Apr 3, 2026

Summary

Fixes a security discrepancy between the MCP Apps guide/spec and the Lit sample implementation. Found during review of PR #1062.

Problem

The Lit sample (mcp-apps-component.ts) and shared sandbox proxy (sandbox.ts) included allow-same-origin in iframe sandbox attributes, violating:

  1. The A2UI guide: states the inner iframe "MUST NOT include allow-same-origin"
  2. The MCP Apps spec (SEP-1865): mandates iframe sandboxing without same-origin access
  3. The MCP Apps SDK docs: example uses sandbox: "allow-scripts" only

The Angular sample (mcp-app.ts) already correctly used sandbox: 'allow-scripts'.

Changes

File Before After
mcp-apps-component.ts (outer iframe) sandbox="allow-scripts allow-forms allow-popups allow-modals allow-same-origin" No sandbox attribute (guide says don't sandbox the proxy)
mcp-apps-component.ts (sendSandboxResourceReady) sandbox: "allow-scripts allow-forms allow-popups allow-modals allow-same-origin" sandbox: "allow-scripts"
sandbox.ts (inner iframe default) allow-scripts allow-same-origin allow-forms allow-scripts

This is a minimal, 2-file change — only the security fix, no unrelated changes.

Verification

  • Confirmed against MCP Apps spec (SEP-1865)
  • Confirmed against AppBridge SDK docs — example uses sandbox: "allow-scripts"
  • Angular sample already follows this pattern correctly

Related

Zaf and others added 26 commits March 12, 2026 03:10
…be component

- New guide: design-system-integration.md — step-by-step for adding A2UI
  to an existing Material Angular application
- Rewritten guide: custom-components.md — complete walkthrough for YouTube,
  Maps, and Charts custom components (replaces TODO skeleton)
- New sample component: YouTube embed for rizzcharts catalog
- Updated rizzcharts catalog.ts to include YouTube component
- Friction log documenting 8 friction points (P2/P3) encountered during
  development, with recommendations
- Added Design System Integration to mkdocs nav
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Remove friction log file (content already in issue google#825)
- YouTube component: add video ID regex validation (security)
- custom-components.md: rename to 'Custom Component Catalogs', reorder
  examples (media first), clarify basic catalog is optional, remove
  redundant heading, fix Maps input.required consistency, add
  encodeURIComponent to docs example
- design-system-integration.md: rewrite to focus on wrapping Material
  components as A2UI components (not using DEFAULT_CATALOG), show
  custom catalog without basic components, add mixed catalog example
- s/standard/basic/ throughout
…or A2UI Dojo

- Redesigned Top Command Center with glassmorphic header and functional timeline scrubber.
- Replaced native scenario select with Shadcn DropdownMenu.
- Polished Data Stream view with active state highlighting, glow effects, and auto-scrolling.
- Replaced native checkboxes with custom Tailwind styled toggles in Config view.
- Added dynamic grid layout for the Renderers Panel with sophisticated styling per surface (React Web, Discord dark mode replica, Lit Components).
- Applied custom slim scrollbars throughout for a premium feel.
- Replace invented v0.9 scenarios with real v0.8 samples from samples/agent/adk/
- Add restaurant-booking, restaurant-list, restaurant-grid, restaurant-confirmation
- Add contact-card, contact-list, org-chart, floor-plan scenarios
- Update index.ts to surface real scenarios as defaults
- Default scenario is now restaurant-booking (verified rendering)
- Fix transcoder.ts: pass v0.8 messages through unchanged
- Fix useA2UISurface.ts: only process v0.8 format components (id + component)
- Fix dataModelUpdate: parse ValueMap format correctly
- Restaurant booking now renders: column, text, image, textfield, datetime, button
- Locally verified with headless Chromium: all A2UI CSS classes present
- Build passes (bun run build)
- Switch import to @copilotkit/a2ui-renderer (npm-published)
- Remove file:../../renderers/react dep that breaks Vercel builds
- @copilotkit/a2ui-renderer uses @a2ui/lit under the hood (npm transitive dep)
- Remove Discord mock pane and multi-renderer grid
- Show single A2UI renderer (full width, centered)
- Add human-readable step summaries to JSONL pane
  (e.g. 'Update 9 components: Column, Text, Image, TextField...')
- Raw JSON collapsed by default behind 'Raw JSON ▸' toggle
- Steps are clickable to seek directly
- Wider left pane (35%) for better readability
- Remove unused renderer toggle state
- Remove northstar-tour, flight-status, weather-widget (v0.9 format)
- Remove kitchen-sink, component-gallery-stream (not real scenarios)
- Keep only verified v0.8 scenarios from samples/agent/adk/
- 12 working scenarios remain in dropdown
- Header: flex-row always, h-12 on mobile (was ~117px from flex-col stacking)
- Timeline scrubber: hidden on mobile (users step-click in left pane)
- Added compact step counter (progress/total) for mobile
- Scenario dropdown: icon-only on mobile, full text on desktop
- Playback buttons: slightly smaller on mobile (h-8/h-9 vs h-9/h-10)
- Speed button: compact on mobile (h-7 w-11)
- Reduced gaps and padding on mobile (gap-1.5, px-2)

Fixes google#236
Add 5 new scenarios from samples/agent/adk/:
- multi-surface: multi-surface layout (contact_multiple_surfaces)
- chart-node-click: chart with clickable nodes (contact_multiple_surfaces)
- rizzcharts-catalog-chart: rizzcharts catalog chart
- rizzcharts-catalog-map: rizzcharts catalog map
- standard-catalog-map: standard catalog map

Total scenarios now: 17 (up from 12)
The restaurant-booking scenario uses Text with usageHint 'h2', which
prepends '## ' to the text and passes it through the markdown directive.
Without a markdown renderer in Lit context, the directive falls back to
a plain <span> showing the raw '## Book a Table...' string.

Add A2UIMarkdownProvider (using Lit context-request events, avoiding the
accessor decorator unsupported by Turbopack) and wrap A2UIViewer with it
in the dojo page. Uses @a2ui/markdown-it's renderMarkdown function.
@google-cla
Copy link
Copy Markdown

google-cla bot commented Apr 3, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces comprehensive documentation for custom component catalogs and design system integration, alongside a new "Dojo" playback feature in the composer tool for debugging A2UI message streams. Key technical additions include a YouTube custom component for Angular and a transpiler to maintain compatibility between v0.9 and v0.8 message formats. Feedback focuses on improving code portability and robustness: specifically, removing hardcoded absolute paths and local environment dependencies in scripts, deleting redundant or backup files, and adding safety checks for data iteration. Additionally, a debugging console log in the React renderer should be removed.

messages.append(p.root.data)

if messages:
out_path = "/home/node/.openclaw/projects/A2UI/tools/composer/src/data/dojo/contact-lookup.json"
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.

high

The output path is hardcoded to an absolute path specific to the author's local environment (/home/node/...). This will cause the script to fail for other users. Please use a relative path or an environment variable instead.

Suggested change
out_path = "/home/node/.openclaw/projects/A2UI/tools/composer/src/data/dojo/contact-lookup.json"
import os
base_dir = os.path.dirname(os.path.abspath(__file__))
out_path = os.path.join(base_dir, "../../../../tools/composer/src/data/dojo/contact-lookup.json")


// Get surface - this will re-render when version changes
const surface = getSurface(surfaceId);
console.log('A2UIRenderer: surfaceId=', surfaceId, 'surface=', surface, 'version=', version);
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.

medium

Debugging console.log should be removed from production code to maintain clean logs and avoid potential performance overhead in the rendering loop.

@@ -0,0 +1,475 @@
'use client';
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.

medium

This file appears to be a backup or original version of page.tsx. It should not be committed to the repository.

@@ -0,0 +1,25 @@
const fs = require('fs');
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.

medium

This script performs string replacement on source files, which is a fragile approach to code modification. Furthermore, the changes it applies are already present in page.tsx, making this script redundant. It should be removed.

Comment on lines +2 to +3
source ~/.openclaw/credentials/secrets.sh
export GEMINI_API_KEY="$GEMINI_API_KEY_ALAN_WORK"
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.

medium

The script relies on a specific local file structure (~/.openclaw/...) and a custom environment variable (GEMINI_API_KEY_ALAN_WORK). This limits the portability of the sample. Consider using standard environment variables and providing instructions for setup.

Comment on lines +89 to +91
for (const child of item.valueMap) {
obj[child.key] = extractValueMapValue(child);
}
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.

medium

The code iterates over item.valueMap without verifying it is an array. If the agent provides a malformed payload, this will cause a runtime crash. Adding an Array.isArray check improves robustness.

    if (Array.isArray(item.valueMap)) {
      for (const child of item.valueMap) {
        obj[child.key] = extractValueMapValue(child);
      }
    }

@zeroasterisk zeroasterisk marked this pull request as draft April 3, 2026 14:03
@zeroasterisk
Copy link
Copy Markdown
Collaborator Author

Closing in favor of a clean PR with only the minimal security fix (2 files, 1 commit). The original PR had unrelated commits from the fork's main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant