fix(core): allow external HTTPS images in admin CSP#352
fix(core): allow external HTTPS images in admin CSP#352barckcode wants to merge 3 commits intoemdash-cms:mainfrom
Conversation
The admin UI's Content Security Policy blocked external images referenced in content (e.g. blog.helmcode.com, media.tenor.com) because img-src only allowed 'self', data:, and blob:. Add https: to the img-src directive so the admin can display images from any HTTPS origin. This is standard for CMS admin UIs that manage user content with external image references. Extract buildEmDashCsp() into its own module for testability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 2bfbabb The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
This PR updates the admin/API Content Security Policy (CSP) used on /_emdash routes to allow loading images from external HTTPS origins, and factors CSP construction into a dedicated middleware module with unit coverage.
Changes:
- Add
https:to theimg-srcdirective so admin-rendered content can display externally hosted HTTPS images. - Extract
buildEmDashCsp()intopackages/core/src/astro/middleware/csp.tsfor easier testing/reuse. - Add unit tests covering the updated CSP
img-srcbehavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/core/src/astro/middleware/csp.ts | New module containing buildEmDashCsp() with updated img-src sources. |
| packages/core/src/astro/middleware/auth.ts | Switch middleware to import CSP builder from the new module (and re-export it). |
| packages/core/tests/unit/middleware/csp.test.ts | New unit tests asserting https: is present in img-src and marketplace behavior remains. |
| .changeset/fix-admin-csp-external-images.md | Patch changeset describing the CSP change for the emdash package. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The function is only used internally by auth.ts. No external consumers import it from this module. Avoids accidental public API surface expansion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note: This PR overlaps with #226 (CSP fix for S3 storage |
|
This effectively removes CSP restrictions for images, which is a legitimate thing to suggest, but in this scenario we might as well remove the CSP rules for them entirely including special handling for the marketplace etc. The very strict CSP was originally done in order to help prevent malicious plugins using it as a vector to exfiltrate user data. This was before we introduced block kit for sandboxed plugins, so may no longer be needed, but I would like to investigate it a bit more. |
Simplify buildEmDashCsp() by removing the marketplaceUrl parameter. With https: already in img-src, per-origin marketplace logic is redundant. Additionally, marketplace images are served through server-side proxies (/_emdash/api/admin/plugins/marketplace/[id]/icon), so the browser never loads directly from the marketplace origin. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Hey @ascorbic, thanks for the feedback. I dug into the codebase to understand the full security model before suggesting a path forward. Why relaxing The original strict CSP was designed to prevent malicious plugins from exfiltrating data. After reviewing the current architecture, the plugin isolation doesn't rely on browser CSP at all. There are 5 layers before CSP even comes into play:
The only exfiltration vector via The Beyond
These fetch from the marketplace on the server and return the content as I've pushed the simplification -- removed the Suggested next step: align with #226 on This also aligns with #226 (S3 storage CSP fix by @WebDevByCam). Both PRs overlap on function buildEmDashCsp(storageEndpoint?: string): string {
const connectSources = ["'self'"];
if (storageEndpoint) {
try {
connectSources.push(new URL(storageEndpoint).origin);
} catch { /* ignore invalid URL */ }
}
return [
"default-src 'self'",
"script-src 'self' 'unsafe-inline'",
"style-src 'self' 'unsafe-inline'",
`connect-src ${connectSources.join(" ")}`,
"form-action 'self'",
"frame-ancestors 'none'",
"img-src 'self' https: data: blob:",
"object-src 'none'",
"base-uri 'self'",
].join("; ");
}This would:
Happy to add the |
What does this PR do?
The admin UI's Content Security Policy blocks external images referenced in content. When users have content with images from external HTTPS origins (e.g.
blog.helmcode.com,media.tenor.com), the browser blocks them becauseimg-srconly allows'self' data: blob:.This adds
https:to theimg-srcCSP directive so the admin can display images from any HTTPS origin. This is standard practice for CMS admin UIs that manage user content with external image references (WordPress, Ghost, Sanity Studio all allow this).Also extracts
buildEmDashCsp()into its own module (middleware/csp.ts) for unit testability.Error reproduced in production:
Related: #205 (similar CSP issue for
connect-srcwith S3 storage)Type of change
Checklist
pnpm typecheckpassespnpm --silent lint:json | jq '.diagnostics | length'returns 0pnpm testpasses (or targeted tests for my change)pnpm formathas been runAI-generated code disclosure
Screenshots / test output
CSP header before fix:
CSP header after fix:
Test output: