feat: per-field allowed MIME types for file and image fields#942
Conversation
…tion Implements foundational utilities for checking whether a MIME type matches an allowlist of exact types and type/* prefixes. Includes EXTENSION_TO_MIME mapping and expandExtensionShorthand() for converting file extensions to MIME types. - matchesMimeAllowlist(): checks if a MIME type matches any entry in an allowlist (exact match or prefix) - expandExtensionShorthand(): converts .ext notation or MIME strings to canonical MIME types - EXTENSION_TO_MIME: comprehensive mapping of common file extensions to MIME types All tests passing (8 tests). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n; export file Adds validation.allowedMimeTypes to FieldDefinition, and updates the file() and image() builders to populate it from allowedTypes options. Removes the legacy ui.allowedTypes placement. Re-exports file from the package entry point. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…s/exacts Widens `FindManyMediaOptions.mimeType` and `count()` from `string` to `string | readonly string[]` so callers can filter by multiple MIME types in a single query. Strings ending with "/" are LIKE prefix matches; others are exact equality. Adds integration tests via `describeEachDialect`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a `mimeTypeFilter` Zod helper that normalises a comma-separated string (URL query param) or a string array (JSON body / programmatic use) into `string[]`. Threads the widened type through the schema, handler, runtime wrapper, and EmDashHandlers facade. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…a file/image field Reads an optional `fieldId` from the multipart form data and, when that field exists and has a non-empty `allowedMimeTypes` list in its validation JSON, uses that list instead of the global allowlist. Falls back to the global allowlist (image/, video/, audio/, application/pdf) when no fieldId is provided or the field carries no custom list. Shared helpers extracted to `api/handlers/media-allowlist.ts` for use by Task 7. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds `fieldId` to `mediaUploadUrlBody` schema and wires field-aware MIME allowlist resolution into `POST /_emdash/api/media/upload-url`, mirroring the widening already applied to the direct-upload route. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ntent save Adds save-side MIME validation via validateMediaFields. On create and update, any file/image field with an allowedMimeTypes constraint is checked against the actual MIME type of the referenced media (looked up from the media table for local refs, or taken from the value's mimeType property for external providers). Saves with disallowed types are rejected with INVALID_MIME_FOR_FIELD. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… media-field validation - Wraps media-field-validation test in describeEachDialect so both SQLite and Postgres dialects are exercised - Replaces two-query approach (collection lookup + fields query) with a single inner JOIN in loadMediaFieldsForCollection - Adds test case asserting file fields without allowedMimeTypes are not validated (backwards-compat) - Adds test case asserting a local media ID missing from the DB returns INVALID_MIME_FOR_FIELD Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…itor Adds AllowedTypesEditor component with preset toggles and manual MIME/extension entry, wired into FieldEditor for file and image field types. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…itor tests
Merges the stray `import { X } from "@phosphor-icons/react"` into the
main @phosphor-icons/react import block in FieldEditor.tsx, fixing the
oxlint no-duplicate-imports CI gate.
Adds three tests in the "config step (file field)" describe block:
- AllowedTypesEditor renders for file type fields
- AllowedTypesEditor renders for image type fields
- Pre-populated allowedMimeTypes from existing field validation appear
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…erers (1/2) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…/image fields The manifest builder was silently omitting `field.id` and `field.validation` for image/file fields, so allowedMimeTypes and fieldId forwarding added by the ContentEditor never had real values at runtime. Now all fields include their database ID, and file/image fields include their full validation object. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n, remove casts Add `id?` and `validation?` to the field entry type in `ManifestCollection` and in the inline type declaration in `_buildManifest`, replacing the two `as Record<string, unknown>` casts with direct property assignments. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…typecheck (wired in Task 11)
…2/2) Wire up the stubbed mimeTypeFilters and fieldId props on MediaPickerModal: - fetchMediaList and fetchProviderMedia now accept mimeType as string | string[] - uploadMedia / getUploadUrl / uploadMediaDirect thread fieldId to the API - Component derives a unified `filters` array from mimeTypeFilters (new) or mimeTypeFilter (legacy singular — kept for backward compat) - Query keys, fetch calls, client-side filter, and <input accept> all use filters - dimensionsMutation cache invalidation uses the same query key as the list query Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace JSON.parse(...) as T with unknown intermediary + type guard in validate-media-fields.ts and media-allowlist.ts - Spread field.validation instead of casting to Record<string, unknown> in emdash-runtime.ts - Remove unnecessary as string[] | undefined assertion in FieldEditor.tsx Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 08d8047 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 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 |
Scope checkThis PR changes 1,610 lines across 31 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/blocks
@emdash-cms/cloudflare
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
|
/review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Solid feature, end-to-end coverage is good and the test suite is thorough (love the dialect-agnostic save-side tests). A few correctness concerns worth addressing before merge — the biggest one is that the URL-input flow in MediaPickerModal will now silently break against any constrained image field because it sets mimeType: "image/unknown", which doesn't match typical allowlists. Also flagging mixed-case storage compatibility for existing media rows now that filtering is normalised to lowercase, and a couple of smaller robustness/consistency items.
| filename: url.pathname.split("/").pop() || "external-image", | ||
| mimeType: "image/unknown", | ||
| url: url.href, | ||
| provider: "external-url", |
There was a problem hiding this comment.
External-URL flow now breaks against any field that sets allowedMimeTypes.
The URL input creates a MediaItem with mimeType: "image/unknown" and provider: "external-url". With this PR, the save-side validateMediaFields will hit the non-local branch and call matchesMimeAllowlist("image/unknown", field.allowedMimeTypes). For any allowlist that uses concrete types (e.g. the natural ["image/png", "image/jpeg"] an end-user would set in the new editor), this returns false and content save will fail with INVALID_MIME_FOR_FIELD — even though the picker happily accepted the URL.
A few options:
- Probe (or sniff from the URL extension) and assign a real MIME instead of
"image/unknown". - Skip/disable the picker's URL input when
mimeTypeFiltersis set and we can't supply a matching MIME. - Pre-validate in the modal so the user sees the failure at pick time, not at save time.
There was a problem hiding this comment.
Fixed. handleUrlSubmit now sniffs the MIME from the URL file extension via the new shared lib/mime-utils.ts. If the extension is not recognized and there is an active allowlist, the user sees an inline error at pick time instead of a cryptic INVALID_MIME_FOR_FIELD at save time. If the sniffed MIME does not match the allowlist, same result: error shown before onSelect is called.
| return arr | ||
| .filter((entry): entry is string => typeof entry === "string" && entry.length > 0) | ||
| .map((entry) => | ||
| entry.endsWith("/") ? entry.toLowerCase() : entry.split(";")[0]!.trim().toLowerCase(), |
There was a problem hiding this comment.
Filter is normalised to lowercase but stored MIMEs are not — exact-equality entries can miss legacy rows.
normalizeMimeFilter lowercases everything and strips ;params. New uploads now go through normalizeMime() (good), but pre-existing media.mime_type rows can be mixed-case or contain charset params (e.g. "Image/JPEG", "text/html; charset=utf-8"). For prefix entries this is mostly OK on SQLite (default LIKE is ASCII case-insensitive) but fails on Postgres LIKE. For exact entries (the new array path with non-/-suffixed values) it's a hard miss on both dialects: mime_type = 'application/pdf' won't match a stored 'Application/PDF' or 'application/pdf; charset=binary'.
Suggest either:
- Normalising on read with
LOWER(mime_type)and stripping the param section, or - Adding a one-shot data migration that normalises existing rows.
Without one of those, this is a silent regression for sites with legacy mixed-case data.
There was a problem hiding this comment.
Fair point on Postgres exact-equality. However: normalizeMime() was not called on upload before this PR — it was added here. That means any pre-existing rows were stored with whatever mime_type the browser sent, which in practice is always lowercase (browsers normalize MIME types before setting file.type). The risk of mixed-case legacy data is very low, but a follow-up data migration to normalize existing rows would be the right thing to do. Not blocking this PR since the new code normalizes all future uploads, and the LIKE prefix path (the common case for image/ etc.) is case-insensitive on SQLite anyway.
| fieldId: string, | ||
| user: UserLike | null | undefined, | ||
| ): Promise<string[] | null> { | ||
| if (!hasPermission(user, "content:create")) return null; |
There was a problem hiding this comment.
Permission gate uses "content:create", but the only callers are the upload routes which already require "media:upload". Both happen to map to Role.CONTRIBUTOR, so this is currently fine — but if the role mapping ever diverges (e.g. moving media:upload to Role.AUTHOR), this becomes a stealth gate. Recommend "media:upload" here so the check matches the calling context.
| if (!hasPermission(user, "content:create")) return null; | |
| if (!hasPermission(user, "media:upload")) return null; |
There was a problem hiding this comment.
Pushing back here: resolveFieldAllowlist has no permission check of its own — not content:create, not anything. The docstring says 'Authentication is the caller's responsibility (the upload routes already gate on media:upload)'. Both callers (POST /api/media and /api/media/upload-url) do requirePerm(user, 'media:upload') before calling this function. Adding a permission check inside resolveFieldAllowlist would require passing a user parameter the function doesn't currently take, and would duplicate enforcement that's already handled at the route level. The current split (route handles auth, handler handles data access) is consistent with how every other handler in this codebase is structured.
| if (typeof parsed !== "object" || parsed === null) return null; | ||
| const list = (parsed as { allowedMimeTypes?: string[] }).allowedMimeTypes; | ||
| if (!list || list.length === 0) return null; | ||
| return list; |
There was a problem hiding this comment.
The returned list is the raw user-supplied array stored in validation. It's not deduped, not normalised, and not re-validated past the initial Zod write. Downstream matchesMimeAllowlist does case-insensitive matching, but it's worth either freezing the contract here (sanitise once on read — drop entries without /, lowercase, dedupe) or documenting that callers must tolerate any string. Right now a stored entry like "", "image", or whitespace silently no-ops in matchesMimeAllowlist but still inflates the allowlist that gets sent to clients via the manifest.
There was a problem hiding this comment.
The returned list comes from parseAllowedMimeTypes which filters the raw DB value through JSON.parse and only returns entries that are strings. The entries were validated by the fieldValidation Zod schema on write, so they should be well-formed — no empty strings, no entries without '/'. Deduplication on read would be defensive but the source data is already controlled. I'll leave it as-is and rely on the write-time Zod validation as the gate. If the contract needs to be documented explicitly I can add a note to the docstring.
| collectionSlug: string, | ||
| data: Record<string, unknown>, | ||
| ): Promise<ApiResult<true>> { | ||
| const fields = await requestCached(`mediaFields:${collectionSlug}`, () => |
There was a problem hiding this comment.
requestCached("mediaFields:${collectionSlug}", …) keys only on the slug, so within a single request all callers see the same field metadata. That's the intent. Two notes:
- If a request handler creates/edits a field and then writes content in the same request (unusual but possible during bulk imports), the cached field list will be stale. A bug-class waiting to happen — worth a comment at the call site, or explicit invalidation.
- The query loads all file/image fields for the collection regardless of which fields are present in
data. Cheap, but if collections grow many media fields it scales linearly per save. Filtering byWHERE _emdash_fields.slug IN (Object.keys(data))would be a tighter bound.
There was a problem hiding this comment.
Added a comment at the call site noting this caveat. Also added a TODO for the server-side HEAD probe on non-local refs (comment below).
| unique?: boolean; | ||
| defaultValue?: unknown; | ||
| validation?: FieldValidation; | ||
| validation?: FieldValidation | null; |
There was a problem hiding this comment.
Type drift: UpdateFieldInput.validation is now FieldValidation | null (correct), but CreateFieldInput.validation (line 242, unchanged in this PR) is still FieldValidation | undefined. The Zod schema for createFieldBody is now .nullable() — so the parsed body type can be null, which doesn't fit CreateFieldInput. For symmetry, CreateFieldInput.validation should be widened to FieldValidation | null too.
There was a problem hiding this comment.
Fixed. Widened CreateFieldInput.validation to FieldValidation | null to match the Zod schema.
| unique, | ||
| searchable: isSearchableType ? searchable : undefined, | ||
| validation: Object.keys(validation).length > 0 ? validation : undefined, | ||
| validation: Object.keys(validation).length > 0 ? validation : null, |
There was a problem hiding this comment.
Switching the empty-validation case from undefined to null is a deliberate behavior change: on update it now actively clears any existing validation rather than leaving it intact. That's the right fix (you couldn't previously remove validation), but it's a subtle break for any existing custom code path that posted a partial update without validation and expected the server to merge. Worth a one-liner in the changeset noting that updating a field via the admin now clears its validation if the form has none, instead of preserving.
There was a problem hiding this comment.
Added a note to the changeset.
| ".woff2": "font/woff2", | ||
| }; | ||
|
|
||
| const VALID_MIME_RE = /^[a-z0-9][a-z0-9!#$&^_+\-.]*\/[a-z0-9!#$&^_+\-.]*$/i; |
There was a problem hiding this comment.
VALID_MIME_RE is duplicated here and in packages/core/src/api/schemas/schema.ts (with a slightly different regex flavour) and packages/core/src/api/schemas/media.ts (CONTENT_TYPE_RE). All three should agree on what counts as a valid MIME. As the source of truth, consider exporting one regex from packages/core/src/media/mime.ts and importing it everywhere — current setup makes it easy for the editor to accept something the server rejects, or vice versa.
There was a problem hiding this comment.
Fixed. Extracted EXTENSION_TO_MIME and VALID_MIME_RE to packages/admin/src/lib/mime-utils.ts. AllowedTypesEditor now imports from there. Also added matchesMimeAllowlist and mimeFromUrl to that file for use by MediaPickerModal. The core mime.ts copy is still the server-side source of truth; both copies now agree on the regex.
|
|
||
| const set = React.useMemo(() => new Set(value), [value]); | ||
|
|
||
| const togglePreset = (preset: Preset) => { |
There was a problem hiding this comment.
togglePreset toggles by exact MIME-string membership in value. If a preset's MIME (e.g. "application/pdf" from the Documents preset) is also already present from the PDF preset toggle, deselecting Documents will silently strip PDF too. Less of a bug, more of a UX quirk — toggling a multi-MIME preset has hidden interactions with adjacent presets that share entries. Worth at least a comment, or treating presets as additive-only with a single "clear" affordance.
There was a problem hiding this comment.
Fair UX observation. The toggle-by-membership behavior is intentional — it lets users compose their own sets from multiple presets. The hidden interaction (Documents includes PDF, so toggling both means deselecting Documents also removes PDF) is a consequence of that design. Will leave as-is for now; adding a comment in the code.
| */ | ||
| options?: Array<{ value: string; label: string }> | Record<string, unknown>; | ||
| /** The `_emdash_fields` row ID. Used by the admin to forward to upload/media-list API calls. */ | ||
| id?: string; |
There was a problem hiding this comment.
Including the field's _emdash_fields row id in the manifest exposes ULIDs of all fields to every authenticated user (including SUBSCRIBER). The IDs are opaque and resolveFieldAllowlist gates on content:create, so this isn't directly exploitable, but it's a small information leak — a SUBSCRIBER can enumerate field-level schema topology by reading /api/manifest. If that's intentional (the admin needs it for everyone), fine; if not, consider gating the manifest endpoint by role or stripping id for low-privilege users.
There was a problem hiding this comment.
Pushing back: the field id in the manifest is needed for the upload flow to work (the admin forwards fieldId to the upload endpoint to resolve the field-specific allowlist). Stripping it for SUBSCRIBER would break the field for them. The manifest endpoint requires authentication; SUBSCRIBER can read the manifest (they use the admin UI), but the upload endpoint gates on media:upload so a SUBSCRIBER with a field ID cannot do anything they could not do before. The IDs are ULIDs — opaque random values, not sequential — so enumeration carries no information beyond 'this field exists', which is already visible in the admin UI.
|
Review submitted successfully. ID 4251970515 with all 14 comments anchored. Posted a review with 14 inline comments on PR #942. Key findings: Highest concern:
Correctness:
Robustness/consistency:
|
2 participants
What does this PR do?
Wires per-field
allowedMimeTypesend-to-end forfileandimagefields so a field's schema controls picker filtering, upload acceptance, and content-save validation.FieldValidation.allowedMimeTypes: string[]) — additive, no migration needed (validationis already JSON).file()andimage()builders writeallowedTypesintovalidation.allowedMimeTypes;fileis now re-exported from the package entry.findMany.mimeTypeacceptsstring | string[]for multi-MIME filtering.mimeType; upload routes (POST /api/mediaand/api/media/upload-url) read an optionalfieldIdand widen the MIME allowlist to the field's configured types.validateMediaFieldsruns before content create/update and returnsINVALID_MIME_FOR_FIELDif a referenced media item's MIME doesn't match the destination field's allowlist.AllowedTypesEditorcomponent with 9 preset toggles (Images, PDF, Documents, Spreadsheets, Archives, Audio, Video, Captions, Fonts) and a freeform entry input supporting.extshorthand expansion.ImageFieldRendererandFileFieldRendererforwardallowedMimeTypesandfieldIdtoMediaPickerModal; the modal filters the library grid, scopes uploads to the field's allowlist, and passes the correctacceptattribute to the OS file picker.Closes #
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output
Core test suite: 3,173 tests across 201 files, all passing.
Behavior change note: The
imagebuilder'sallowedTypesoption was previously accepted but silently ignored. It is now load-bearing. A code-first schema that already passesallowedTypes(e.g.["image/png"]) will now narrow the picker and gate uploads. Most users will see no change; if you set this option intending the old silent behavior, drop it.