feat(mcp): add media_upload tool for programmatic media management#1954
feat(mcp): add media_upload tool for programmatic media management#1954swissky wants to merge 2 commits into
Conversation
Adds a media_upload MCP tool that accepts base64-encoded file data or a public URL, runs the same pipeline as the REST upload route (global MIME allowlist, size limit, content-hash dedupe, storage upload, image metadata enrichment), and returns the media item ready to reference from content fields. URL fetches go through ssrfSafeFetch so redirects and private hosts are rejected. Closes emdash-cms#620, closes emdash-cms#1825.
🦋 Changeset detectedLatest commit: f91f8c7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 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 582 lines across 8 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/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@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-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is a well-scoped feature that follows the approved Discussion. The new media_upload MCP tool reuses EmDash’s existing SSRF fetch, global MIME allowlist, content-hash dedupe, storage cleanup on failure, and image-enrichment pipeline, and the authorization (media:write scope + Role.CONTRIBUTOR) is consistent with the RBAC floor for media:upload and the existing media_create MCP tool.
The tests cover the important paths (base64, URL, validation, allowlist, size limits, SSRF, dedupe, auth), and the docs/changeset are updated.
I found one real gap that should be fixed before merge: the MIME type string is never validated, so a crafted contentType can bypass the allowlist (normalizeMime only trims ends and matchesMimeAllowlist just checks startsWith) and reach the storage backend’s ContentType header, then be echoed by the media file serving route. I also suggested tightening the MCP input schema for contentType as minor hardening.
| const { bytes } = acquired; | ||
| const mimeType = normalizeMime(acquired.mimeType); | ||
|
|
||
| if (!matchesMimeAllowlist(mimeType, GLOBAL_UPLOAD_ALLOWLIST)) { |
There was a problem hiding this comment.
[needs fixing] The mimeType from base64 input and from the remote Content-Type header is normalized and allow-listed without validating the raw string first. normalizeMime only strips the parameter suffix and trims leading/trailing whitespace, and matchesMimeAllowlist only checks startsWith, so a value such as image/png\r\nX-Evil: 1 passes the allowlist and is later passed straight to storage.upload(...). That value reaches S3/R2 ContentType and is echoed by GET /_emdash/api/media/file/:key.
Re-use the existing CONTENT_TYPE_RE from api/schemas/media.ts (export it if necessary) and reject malformed values before the allowlist/storage check:
| if (!matchesMimeAllowlist(mimeType, GLOBAL_UPLOAD_ALLOWLIST)) { | |
| import { CONTENT_TYPE_RE, DEFAULT_MAX_UPLOAD_SIZE, formatFileSize } from "../schemas/media.js"; | |
| // ... | |
| export async function handleMediaUpload( | |
| db: Kysely<Database>, | |
| storage: Storage, | |
| input: MediaUploadInput, | |
| ): Promise<MediaUploadResult> { | |
| if (!input.base64 === !input.url) { | |
| return fail("VALIDATION_ERROR", "Provide exactly one of 'base64' or 'url'"); | |
| } | |
| const rawMax = input.maxUploadSize ?? DEFAULT_MAX_UPLOAD_SIZE; | |
| if (!Number.isFinite(rawMax) || rawMax <= 0) { | |
| return fail("CONFIGURATION_ERROR", "Invalid maxUploadSize configuration"); | |
| } | |
| const acquired = await acquireBytes(input, rawMax); | |
| if ("success" in acquired) return acquired; | |
| const { bytes } = acquired; | |
| if (!CONTENT_TYPE_RE.test(acquired.mimeType)) { | |
| return fail("VALIDATION_ERROR", "Invalid content type"); | |
| } | |
| const mimeType = normalizeMime(acquired.mimeType); |
There was a problem hiding this comment.
Fixed in f91f8c7 — exported CONTENT_TYPE_RE from api/schemas/media.ts and the handler now rejects the raw MIME string before normalize/allowlist, exactly as suggested. Added two regression tests: a crafted base64 contentType (image/png\r\nX-Evil: 1) and a malformed remote Content-Type header both fail with VALIDATION_ERROR and never reach storage.
| .describe( | ||
| "Public http(s) URL to fetch the file from. Provide exactly one of base64 / url.", | ||
| ), | ||
| contentType: z |
There was a problem hiding this comment.
[suggestion] Consider tightening the MCP input schema so malformed input is rejected before the handler runs and before the server buffers/fetches any bytes. api/schemas/media.ts already has a suitable CONTENT_TYPE_RE.
| contentType: z | |
| contentType: z | |
| .string() | |
| .optional() | |
| .regex(CONTENT_TYPE_RE, "Invalid content type") | |
| .describe( | |
| "MIME type (e.g. 'image/png'). Required with base64; with url it " + | |
| "defaults to the response's Content-Type header.", | |
| ), |
(You may also want .url() on the url field, although ssrfSafeFetch will reject invalid schemes/addresses too.)
There was a problem hiding this comment.
Done in f91f8c7 — contentType now has .regex(CONTENT_TYPE_RE) and url has .url() in the tool input schema, so malformed input is rejected at the schema layer before any bytes are buffered or fetched. The handler-level check stays as defense in depth for the remote Content-Type header path.
The prefix-based allowlist check let a crafted contentType like 'image/png\r\nX-Evil: 1' through to the storage backend's ContentType header (echoed by the media file route). Validate against the existing CONTENT_TYPE_RE first, and tighten the MCP input schema (regex on contentType, .url() on url) so malformed input is rejected before any bytes are buffered or fetched.
github-actions
Bot
added
review/needs-rereview
There was a problem hiding this comment.
This is the right follow-up to the previous review. The approved Discussion asked for an MCP tool that can put binary media into the media library, and the implementation reuses EmDash’s existing SSRF-safe fetch, global MIME allowlist, content-hash dedupe, storage cleanup, and image-enrichment pipeline rather than inventing a parallel upload path. The prior MIME-validation gap is fixed: CONTENT_TYPE_RE is now exported from the shared media schema and applied both in the MCP input schema and in the handler before the MIME is normalized or allowlisted, so crafted contentType values with CR/LF or bogus tokens can no longer reach storage or the file-serving response headers.
What I checked:
- Authorization:
media:writeis a real API-token scope with aCONTRIBUTORfloor, and the tool correctly checksrequireScope("media:write")plusrequireRole(Role.CONTRIBUTOR)before touching storage. Themedia_createtool stays atAUTHOR, matching the docs split between “upload” and “register”. - Security: URL fetches go through
ssrfSafeFetchwith redirect re-validation and credential stripping; private IP literals are rejected beforefetch()is called. The handler validates the raw MIME string, runs the allowlist, enforcesmaxUploadSize, and cleans up the storage object if DB record creation fails. - Consistency: the returned URL shape (
/_emdash/api/media/file/${storageKey}), dedupe behavior, image enrichment, and error envelope all mirror the RESTPOST /_emdash/api/mediaroute. - Tests: the new handler tests cover base64/URL paths, validation, allowlist, size limits, SSRF, dedupe, and the prior MIME-injection case; the auth tests cover role, scope, and
NO_STORAGE. - AGENTS.md conventions: no raw SQL, no locale-filter gap, no admin UI strings/Tailwind, changeset present.
No remaining blockers. LGTM.
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. |
github-actions
Bot
added
review/approved
1 participant
What does this PR do?
Adds a
media_uploadMCP tool so agent workflows can get file bytes into the media library without dropping to the CLI or the raw multipart API (which needs an active session, CSRF headers, and multipart form data).Design discussion: #1953
The tool accepts either
base64-encoded file contents or a publicurlto fetch from (exactly one of the two), plusfilename, optionalcontentType(required withbase64; withurlit defaults to the responseContent-Type), and optionalalt. It returns the media item withid,storageKey, and a relativeurl— ready to reference from content fields viacontent_create/content_update, exactly as requested in #620.The handler (
api/handlers/media-upload.ts) runs the same pipeline as the RESTPOST /_emdash/api/mediaroute:image/,video/,audio/,application/pdf) — no field-specific widening over MCPmaxUploadSizefrom the integration config, with cheap prechecks (encoded-string length,Content-Length) before bufferingdeduplicated: trueinstead of a duplicate recordenrichImageMetadataSecurity:
ssrfSafeFetch: private/internal hosts are rejected before any request, redirects are re-validated hop by hop, and credential headers are stripped on cross-origin redirects.media:writescope plus Contributor minimum role (the floor of themedia:uploadpermission inrbac.ts).Also updates the
media_createtool description and the MCP reference docs to point binary uploads at the new tool, and adds a byte-leveldecodeBase64Bytesnext to the existing base64 utils.Closes #620
Closes #1825
Type of change
How was this change tested?
tests/unit/api/handlers/media-upload.test.ts(10 tests, real SQLite + fake storage): base64 upload with enrichment, dedupe, exactly-one-of-base64/urlvalidation, missing/invalidcontentTypeand base64, allowlist rejection, size limit, URL fetch honoring the responseContent-Type, HTTP error surfacing, and SSRF rejection of a private address (fetch never called)tests/unit/mcp/authorization.test.ts: Subscriber role rejected, missingmedia:writescope rejected,NO_STORAGEwhen no storage adapter is configuredpnpm typecheck,pnpm lint,pnpm formatall cleanChecklist
pnpm changeset) if this change affects published packagesAI-generated code disclosure
Which tool? Cursor (Fable 5)