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 <[email protected]>
| 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?
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)
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?