feat: enhance sandbox capability negotiation #158
Conversation
idosal
requested review from
alexi-openai,
antonpk1,
Copilot,
liady and
ochafik
commit: |
There was a problem hiding this comment.
Pull request overview
This PR enhances the MCP Apps sandbox capability negotiation by adding support for additional CSP directives and permissions policies. The changes enable apps to request nested iframe support, custom base URI configuration, and browser permissions (camera, microphone, geolocation) through resource metadata.
Key changes:
- Added
frameDomainsandbaseUriDomainsfields to CSP configuration for controlling nested iframes and base URI directives - Introduced
permissionsmetadata for requesting camera, microphone, and geolocation access via Permission Policy - Extended host capability negotiation to include CSP support indicators (
frameDomainsandbaseUriDomains)
Reviewed changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Added exports for new McpUiResourcePermissions type and schema |
| src/spec.types.ts | Defined new interfaces for permissions and extended CSP configuration with frame/baseUri domains |
| src/generated/schema.ts | Added Zod schemas for permissions and extended CSP schemas with new domain fields |
| src/generated/schema.test.ts | Updated type inference tests to include new permissions schema |
| src/generated/schema.json | Generated JSON schemas for new permissions and CSP extensions |
| specification/draft/apps.mdx | Documented new CSP fields and permissions with examples and security guidance |
| examples/simple-host/sandbox.html | Added new sandbox proxy HTML implementation (missing CSP/permissions handling) |
| examples/basic-host/src/sandbox.ts | Implemented CSP meta tag building and iframe allow attribute for permissions |
| examples/basic-host/src/implementation.ts | Extended resource data extraction to include new CSP and permissions metadata |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| baseUriDomains?: string[]; | ||
| }; | ||
| permissions?: { | ||
| camera?: boolean; |
There was a problem hiding this comment.
let's make these objects (as in capabilities) for future extensions?
e.g. what if one day there's fine-grained vers. coarse geolocation, or front vs. back camera permission, etc.
There was a problem hiding this comment.
Like CSP, Permissions are coupled to the browser spec (Permissions Policy). I don't think we should diverge at this point. WDYT?
There was a problem hiding this comment.
There are 40+ Permission Policies you can stuff in an iFrame. Consider keeping permissions very simple and flexible, just have it be a string[].
Benefits:
- Client can set all permission policies with a single string. Flexible for future extensions.
- Enforcing this on the client side is simple. We just stuff the string into an iFrame's
allow. This is safe because invalid strings are silently ignored. - Still easily parseable on the server side. Server developer only has to do
permissions.contains("camera");.
There was a problem hiding this comment.
Alternatively create some interface and have permission be an array of that interface if type safety and enforcement is of high importance.
interface Permissions {
camera: "camera",
....
}
permissions?: Permissions[]
ochafik
left a comment
ochafik
left a comment
There was a problem hiding this comment.
We should tell app what permissions were granted (and potentially which domains were allowed)
|
Is there an iFrame in the |
|
I can see this being useful if the MCP server needs to know what the browser capabilities / permissions are. However, I'm pretty sure you can also fetch the browser capability within the widget too using https://developer.mozilla.org/en-US/docs/Web/API/FeaturePolicy/allowedFeatures I perhaps React widgets should be using this instead as a source of truth. |
I have no clue why midi is on there. That's for music instruments haha |
Proposal to tackle #58 -
frameDomainsandbaseUriDomainsoverrides to Resource Metadata'sui/cspattributepermissionsattribute(ui/permissions) to the Resource Metadata. Currently, it supportscamera,microphone, andgeolocation. There are many other permissions, but I think we should only add fields as they're needed. @alexi-openai could you please share what OpenAI allows?cspto Host<>App capability negotiation (since it's non-trivial for the app to detect at runtime)If it's acceptable, I'll add an E2E test before merging.
Thoughts?