fix: sandbox permissions propagation - #182
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes sandbox permission propagation in the @mcp-ui/client “App host” flow so MCP UI resource metadata (CSP + permissions) is consistently parsed and forwarded into iframe setup and sandbox-ready notifications, using the official helpers/types from @modelcontextprotocol/ext-apps@^1.0.1.
Changes:
- Upgrade
@modelcontextprotocol/ext-appsto^1.0.1in the client package. - Refactor UI resource parsing to return structured
{ html, csp, permissions }and reuse it across client/callback resource reads. - Propagate permissions into (a) the outer sandbox proxy iframe
allowattribute and (b)sendSandboxResourceReadyparams; add tests for these paths.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
sdks/typescript/client/src/utils/app-host-utils.ts |
Adds parseToolUiResourceContent and updates resource reading to return { html, csp, permissions }; extends sandbox iframe setup to optionally set allow. |
sdks/typescript/client/src/components/AppRenderer.tsx |
Captures resource metadata during HTML fetch and merges it into the SandboxConfig passed to AppFrame. |
sdks/typescript/client/src/components/AppFrame.tsx |
Supports SandboxConfig.permissions as string or typed permissions; computes iframe allow via buildAllowAttribute and forwards permissions to sendSandboxResourceReady. |
sdks/typescript/client/src/components/__tests__/AppRenderer.test.tsx |
Adds coverage for merging resource metadata into sandbox config. |
sdks/typescript/client/src/components/__tests__/AppFrame.test.tsx |
Adds coverage for object/string permissions → iframe allow, forwarding permissions in sandbox-ready, and recreating iframe on allow-policy changes. |
sdks/typescript/client/package.json |
Bumps @modelcontextprotocol/ext-apps dependency to ^1.0.1. |
pnpm-lock.yaml |
Lockfile updates to include @modelcontextprotocol/ext-apps@1.0.1 resolution for the client package. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| export async function readToolUiResourceHtml( | ||
| client: Client, | ||
| opts: { | ||
| uri: string; | ||
| }, | ||
| ): Promise<string> { | ||
| ): Promise<ToolUiResourceContent> { | ||
| const resource = await client.readResource({ uri: opts.uri }); | ||
|
|
||
| if (!resource) { | ||
| throw new Error('UI resource not found: ' + opts.uri); | ||
| } | ||
| if (resource.contents.length !== 1) { | ||
| throw new Error('Unsupported UI resource content length: ' + resource.contents.length); | ||
| } | ||
| const content = resource.contents[0]; | ||
| let html: string; | ||
| const isHtml = (t?: string) => t === RESOURCE_MIME_TYPE; | ||
|
|
||
| if ('text' in content && typeof content.text === 'string' && isHtml(content.mimeType)) { | ||
| html = content.text; | ||
| } else if ('blob' in content && typeof content.blob === 'string' && isHtml(content.mimeType)) { | ||
| html = atob(content.blob); | ||
| } else { | ||
| throw new Error('Unsupported UI resource content format: ' + JSON.stringify(content)); | ||
| } | ||
|
|
||
| return html; | ||
| return parseToolUiResourceContent(resource.contents[0]); | ||
| } |
There was a problem hiding this comment.
readToolUiResourceHtml now returns a structured object (HTML + CSP + permissions) rather than just HTML. Consider renaming the function (and any related types) to reflect the broader return value (e.g., readToolUiResourceContent) to avoid misleading future callers.
| csp: resourceContent.csp ?? sandbox.csp, | ||
| permissions: resourceContent.permissions ?? sandbox.permissions, |
There was a problem hiding this comment.
In sandboxConfig, resource metadata currently takes precedence over the host-provided sandbox props (resourceContent.* ?? sandbox.*). This means a host cannot override permissions via a string sandbox.permissions, and it also allows server-provided CSP metadata to override a host-enforced CSP (potential security regression). Prefer host config when explicitly provided (e.g., sandbox.csp ?? resourceContent.csp, and for permissions keep a string override, otherwise fall back to resource permissions), or merge in a way that preserves host restrictions.
| csp: resourceContent.csp ?? sandbox.csp, | |
| permissions: resourceContent.permissions ?? sandbox.permissions, | |
| // Prefer host-provided CSP when explicitly set; otherwise fall back to resource metadata | |
| csp: sandbox?.csp ?? resourceContent.csp, | |
| // If host provides a string permissions override, keep it; otherwise fall back to resource permissions | |
| permissions: | |
| typeof sandbox?.permissions === 'string' | |
| ? sandbox.permissions | |
| : resourceContent.permissions ?? sandbox?.permissions, |
There was a problem hiding this comment.
@idosal @liady what do we want to do here.
Who should win here? Host or resource?
Leave As-Is (resource-first, current behavior)
Current logic:
Pros:
Tool authors can ship required CSP/permissions and have them “just work.”
Less host-side config needed.
Cons:
Host cannot reliably enforce stricter CSP/permissions once resource metadata is present.
Host string permission override is ignored if resource provides permissions object.
Security boundary shifts toward server/resource metadata.
Example:
Host wants no mic (sandbox.permissions = "clipboard-write"), resource asks for mic.
Current behavior uses resource permissions, mic may be enabled.
Host-First Override
Logic: host values win when explicitly set, resource metadata is fallback.
Pros:
Host remains policy authority (safer default for embedders).
Predictable for compliance/security-sensitive hosts.
Cons:
Some apps may fail unless host allows needed capabilities.
Slightly more host configuration burden.
Example:
Same inputs as above.
Host-first keeps mic blocked, preserving host intent.
When to keep current behavior
You treat MCP server/resource metadata as fully trusted and want maximum app compatibility with minimal host controls.
When to switch
You need enforceable host security policy, multi-tenant safety, enterprise controls, or explicit least-privilege behavior.
This PR fixes sandbox permission handling in the @mcp-ui/client App host flow so app views can request and receive browser permissions (microphone) consistently.
It also upgrades the client package to @modelcontextprotocol/ext-apps@^1.0.1 and switches to the official permission types/helpers from that release.
Apps were failing with microphone access denied because permission metadata from UI resources was not fully propagated into iframe setup and sandbox notifications in a consistent, typed way.
Changes
-Updated App host permission flow:
Pass permissions in
sendSandboxResourceReadywhen providedRefactored resource parsing utilities:
readToolUiResourceHtmlnow returns structured content (html, csp, permissions) instead of only HTMLparseToolUiResourceContentTests
AppFrame tests now cover:
object-based permissions -> iframe allow
string-based permissions passthrough
forwarding permissions in sandbox-ready notification
AppRenderer tests now cover metadata merge (csp + permissions) into sandbox config
Tested on the official MCP inspector which uses mcp-ui/client
Closes #180