fix(media): add image/bmp to the default media upload allowlist - #3066
fix(media): add image/bmp to the default media upload allowlist#3066zaochih wants to merge 2 commits into
Conversation
BMP uploads fail with "File type not allowed" because image/bmp is missing from GLOBAL_UPLOAD_ALLOWLIST — the same shape of regression as the earlier missing-AVIF bug (emdash-cms#2683). BMP is a plain raster format with no active-content risk, so it needs no security tradeoff to allow, unlike SVG. Also adds image/bmp to the two "safe to render inline" sets (SAFE_INLINE_TYPES in the media file route, SAFE_INLINE_IMAGE_TYPES in media/image-endpoint.ts) so an uploaded BMP renders in an <img> instead of being forced to download — without this a BMP would upload but still be unusable as an image. Extends .bmp extension shorthand support and the admin upload dialog/allowed-types preset to match. Closes emdash-cms#3063
🦋 Changeset detectedLatest commit: 222120f The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 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 |
github-actions
Bot
added
area/admin
area/core
area/docs
size/S
review/needs-review
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is a clean, narrowly scoped bug fix: it adds image/bmp to all the relevant allowlists and inline-render sets, mirroring the earlier AVIF-shaped fix. I traced the change through the REST upload route, signed-URL route, MCP media_upload handler, the file-serving route, and the image-endpoint fallback; all now accept or serve BMP consistently. The admin preset, upload dialog accept string, and both EXTENSION_TO_MIME maps are updated in sync. The docs and changeset accurately describe the user-facing effect. The added tests assert observable behavior (allowlist match, Content-Disposition: inline, extension shorthand) rather than pinning constants. No AGENTS.md conventions are violated, no new queries or user-facing strings are introduced, and no security concerns are apparent for a plain raster format. LGTM.
There was a problem hiding this comment.
🔵 Needs a closer look
The media file route still needs a direct regression test for its separate BMP inline allowlist.
Pull request overview
This PR adds BMP support across media uploads, rendering, admin controls, documentation, and release metadata.
Changes:
- Adds
image/bmpto upload and inline-rendering allowlists. - Adds
.bmpMIME handling and admin support. - Updates tests, documentation, and the changeset.
File summaries
| File | Summary |
|---|---|
packages/core/tests/unit/media/mime.test.ts |
Tests BMP MIME shorthand support. |
packages/core/tests/unit/media/media-allowlist.test.ts |
Tests BMP upload allowlisting. |
packages/core/tests/unit/media/image-endpoint.test.ts |
Tests inline BMP response headers. |
packages/core/src/media/mime.ts |
Adds .bmp MIME mapping. |
packages/core/src/media/image-endpoint.ts |
Allows BMP inline rendering. |
packages/core/src/astro/routes/api/media/file/[...key].ts |
Allows BMP inline file responses. |
packages/core/src/api/handlers/media-allowlist.ts |
Adds BMP to the default upload allowlist. |
packages/admin/src/lib/mime-utils.ts |
Adds admin-side .bmp support. |
packages/admin/src/components/MediaUploadDialog.tsx |
Enables BMP uploads and previews. |
packages/admin/src/components/AllowedTypesEditor.tsx |
Adds BMP to the Images preset. |
docs/src/content/docs/guides/media-library.mdx |
Documents BMP support. |
.changeset/add-bmp-to-allowlist.md |
Records the package patch release. |
Review details
Suppressed comments (1)
packages/core/src/astro/routes/api/media/file/[...key].ts:24
- This is a separate private allowlist from
SAFE_INLINE_IMAGE_TYPESinmedia/image-endpoint.ts, but the new test only exercises that other set. The existingroutes.test.tssuite already calls thisGEThandler; without a route-levelimage/bmpassertion, removing this entry would make BMP downloads useContent-Disposition: attachmentwhile all current tests still pass. Add a case there or centralize the two allowlists so the route behavior is covered.
- Files reviewed: 12/12 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot review on this PR flagged that SAFE_INLINE_TYPES in the media file route (astro/routes/api/media/file/[...key].ts) is a separate set from SAFE_INLINE_IMAGE_TYPES in media/image-endpoint.ts, and only the latter had test coverage. Removing "image/bmp" from the route's own set would silently regress to Content-Disposition: attachment for BMP with no test catching it. Adds a route-level test asserting image/bmp renders inline through the actual GET handler, plus the SVG-stays-attachment negative case, using the existing mockMediaContext helper in this file.
|
All contributors have signed the CLA ✍️ ✅ |
github-actions
Bot
added
review/needs-rereview
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
What does this PR do?
BMP uploads fail with "File type not allowed" because image/bmp is missing from GLOBAL_UPLOAD_ALLOWLIST — the same shape of regression as the earlier missing-AVIF bug (#2683). BMP is a plain raster format with no active-content risk, so it needs no security tradeoff to allow, unlike SVG.
Also adds image/bmp to the two "safe to render inline" sets (SAFE_INLINE_TYPES in the media file route, SAFE_INLINE_IMAGE_TYPES in media/image-endpoint.ts) so an uploaded BMP renders in an![]()
instead of being forced to download — without this a BMP would upload but still be unusable as an image. Extends .bmp extension shorthand support and the admin upload dialog/allowed-types preset to match.
Closes #3063
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runUser-visible strings in the admin UI are wrapped for translation (if applicable). Do not includeNot applicable.messages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.New features link to an approved Discussion: https://github.com/emdash-cms/emdash/discussions/...Not applicable.I have included screenshots below if this PR changes the UINot applicable.AI-generated code disclosure
Screenshots / test output
Not applicable