Auto-allow localhost in sandbox CSP for MCP Apps#1501
Conversation
In srcdoc iframes, CSP 'self' resolves to about:srcdoc which is useless for allowing local dev server connections. This caused MCP app developers to declare localhost domains in their ui.csp metadata just to avoid CSP violations from Inspector infrastructure (Vite HMR, API server, etc). - Auto-include localhost/127.0.0.1 wildcard origins in connect-src (HTTP, HTTPS, WS, WSS) and resource directives - Add 'unsafe-eval' to script-src for Vite dev mode compatibility - Add worker-src directive for Web Worker support - Remove debug console.log statements from buildCSP() Co-Authored-By: Claude <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
dosubot
bot
added
the
size:M
|
Related Documentation Checked 0 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
WalkthroughThe sandbox proxy configuration has been updated to enhance Content Security Policy (CSP) construction for development environments. Changes include adding explicit localhost and 127.0.0.1 allowances across multiple CSP directives to support development tools like Vite HMR. The CSP assembly mechanism has shifted from static, host-agnostic configuration to dynamic implementation using mutable variables. Default script policies now include 'unsafe-eval' alongside 'unsafe-inline'. Helper functions for localhost source allowances have been introduced, and the configuration now consistently applies these to resource, connect, and worker directives. Debugging statements have been removed. Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mcpjam-inspector/server/routes/apps/mcp-apps/sandbox-proxy.html (1)
163-175:⚠️ Potential issue | 🔴 Critical
script-srcinheritsdata:andblob:fromresourceSrc— this is a CSP bypass vector.
resourceSrcis built as"data: blob: <localhost> <domains>". When appended toscript-srcon line 165, the resulting directive permitsdata:URIs as script sources (e.g.,<script src="data:text/javascript,…">), which is a well-documented XSS bypass that largely negates the CSP. The same concern applies toblob:.Resource-loading directives (
img-src,font-src,media-src) legitimately needdata:andblob:, butscript-srcshould not inherit them. Consider constructing a separate source list for script-src that includes only the CDN domains and localhost, withoutdata:orblob:.Sketch of a possible separation
+ // Script sources: localhost + declared resource domains (no data:/blob:) + var scriptSrc = + resourceDomains.length > 0 + ? [localhostSrc, ...resourceDomains].join(" ") + : localhostSrc; + return [ "default-src 'none'", - "script-src 'unsafe-inline' 'unsafe-eval' " + resourceSrc, + "script-src 'unsafe-inline' 'unsafe-eval' " + scriptSrc, "style-src 'unsafe-inline' " + resourceSrc,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/server/routes/apps/mcp-apps/sandbox-proxy.html` around lines 163 - 175, The current CSP builder concatenates resourceSrc (which contains "data:" and "blob:") into the "script-src" directive, allowing script: data/blob URIs and enabling CSP bypass; modify the generator to create a separate script-safe source list (e.g., scriptResourceSrc) that is identical to resourceSrc but excludes "data:" and "blob:" (keep the existing "'unsafe-inline' 'unsafe-eval'" tokens and host entries like localhost/CDNs), and use that new symbol in the "script-src" entry instead of resourceSrc while leaving img-src/font-src/media-src to use the original resourceSrc.
🧹 Nitpick comments (2)
mcpjam-inspector/server/routes/apps/mcp-apps/sandbox-proxy.html (2)
129-160:var→constwhere no reassignment occurs.None of
connectDomains,resourceDomains,frameDomains,baseUriDomains,connectSrc,resourceSrc,frameSrc, orbaseUriare reassigned after initialization.constcommunicates immutability and prevents accidental reassignment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/server/routes/apps/mcp-apps/sandbox-proxy.html` around lines 129 - 160, Update the variable declarations to use const instead of var for connectDomains, resourceDomains, frameDomains, baseUriDomains, connectSrc, resourceSrc, frameSrc, and baseUri since none are reassigned; locate the block where sanitizeDomain is used and replace the var declarations (for connectDomains, resourceDomains, frameDomains, baseUriDomains) and the subsequent var assignments for connectSrc, resourceSrc, frameSrc, and baseUri with const so immutability is enforced (references: sanitizeDomain, localhostConnectSrc, localhostSrc).
170-170: Duplicateblob:inworker-src.
resourceSrcalready containsblob:, so prefixing anotherblob:here yields a redundant token. Harmless at runtime, but a symptom of the tangled source composition noted above — separating script/worker sources from resource sources would resolve this naturally.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/server/routes/apps/mcp-apps/sandbox-proxy.html` at line 170, The CSP header assembly is duplicating the "blob:" scheme by concatenating "worker-src blob: " with resourceSrc (which already includes "blob:"), so update the construction that builds the worker-src directive (the expression producing "worker-src blob: " + resourceSrc) to avoid double-prefixing: either remove the hardcoded "blob:" prefix and emit "worker-src " + resourceSrc, or normalize resourceSrc at creation (e.g., strip a leading "blob:" or conditionally prepend "blob:" only when missing) so the worker-src directive is always formed with a single "blob:" when required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mcpjam-inspector/server/routes/apps/mcp-apps/sandbox-proxy.html`:
- Line 114: The "no-CSP" (restrictive defaults) branch currently includes
"script-src 'unsafe-inline' 'unsafe-eval'", which undermines the intent by
allowing eval; remove "'unsafe-eval'" from that string so the no-CSP/defaults
path becomes "script-src 'unsafe-inline'". Keep "'unsafe-eval'" only in the
declared-CSP branch where the code already constructs the CSP for apps that
opted in (the branch that currently adds "'unsafe-eval'"/Vite HMR support).
---
Outside diff comments:
In `@mcpjam-inspector/server/routes/apps/mcp-apps/sandbox-proxy.html`:
- Around line 163-175: The current CSP builder concatenates resourceSrc (which
contains "data:" and "blob:") into the "script-src" directive, allowing script:
data/blob URIs and enabling CSP bypass; modify the generator to create a
separate script-safe source list (e.g., scriptResourceSrc) that is identical to
resourceSrc but excludes "data:" and "blob:" (keep the existing "'unsafe-inline'
'unsafe-eval'" tokens and host entries like localhost/CDNs), and use that new
symbol in the "script-src" entry instead of resourceSrc while leaving
img-src/font-src/media-src to use the original resourceSrc.
---
Nitpick comments:
In `@mcpjam-inspector/server/routes/apps/mcp-apps/sandbox-proxy.html`:
- Around line 129-160: Update the variable declarations to use const instead of
var for connectDomains, resourceDomains, frameDomains, baseUriDomains,
connectSrc, resourceSrc, frameSrc, and baseUri since none are reassigned; locate
the block where sanitizeDomain is used and replace the var declarations (for
connectDomains, resourceDomains, frameDomains, baseUriDomains) and the
subsequent var assignments for connectSrc, resourceSrc, frameSrc, and baseUri
with const so immutability is enforced (references: sanitizeDomain,
localhostConnectSrc, localhostSrc).
- Line 170: The CSP header assembly is duplicating the "blob:" scheme by
concatenating "worker-src blob: " with resourceSrc (which already includes
"blob:"), so update the construction that builds the worker-src directive (the
expression producing "worker-src blob: " + resourceSrc) to avoid
double-prefixing: either remove the hardcoded "blob:" prefix and emit
"worker-src " + resourceSrc, or normalize resourceSrc at creation (e.g., strip a
leading "blob:" or conditionally prepend "blob:" only when missing) so the
worker-src directive is always formed with a single "blob:" when required.
| return [ | ||
| "default-src 'none'", | ||
| "script-src 'unsafe-inline'", | ||
| "script-src 'unsafe-inline' 'unsafe-eval'", |
There was a problem hiding this comment.
'unsafe-eval' in the "restrictive defaults" path undermines the stated intent.
When no CSP is declared by the MCP app, the comment on line 108 says "use restrictive defaults." Adding 'unsafe-eval' here contradicts that — it allows eval(), Function(), and setTimeout(string) in guest code that declared no CSP at all. The Vite HMR rationale doesn't apply when the widget hasn't opted into any external domains.
Consider removing 'unsafe-eval' from this no-CSP branch and keeping it only in the declared-CSP path (line 165), where the developer has explicitly configured connectivity.
Proposed fix
- "script-src 'unsafe-inline' 'unsafe-eval'",
+ "script-src 'unsafe-inline'",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "script-src 'unsafe-inline' 'unsafe-eval'", | |
| "script-src 'unsafe-inline'", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/server/routes/apps/mcp-apps/sandbox-proxy.html` at line 114,
The "no-CSP" (restrictive defaults) branch currently includes "script-src
'unsafe-inline' 'unsafe-eval'", which undermines the intent by allowing eval;
remove "'unsafe-eval'" from that string so the no-CSP/defaults path becomes
"script-src 'unsafe-inline'". Keep "'unsafe-eval'" only in the declared-CSP
branch where the code already constructs the CSP for apps that opted in (the
branch that currently adds "'unsafe-eval'"/Vite HMR support).
2 participants
Summary
localhost/127.0.0.1wildcard origins in the sandbox proxy'sbuildCSP()so MCP app developers don't need to declare localhost domains in theirui.cspmetadata'self'resolves toabout:srcdoc(useless), so localhost must be explicitly allowed for Inspector infrastructure (Vite HMR WebSocket, API server, local MCP servers)'unsafe-eval'toscript-srcfor Vite dev mode compatibilityworker-srcdirective for Web Worker support from CDN resourceDomainsconsole.logstatements frombuildCSP()Test plan
connectDomains— verify external domains work AND noconnect-srcviolations forws://localhost:5173(Vite HMR)script-src evalviolations in dev mode🤖 Generated with Claude Code