Skip to content

fix: make media uploads reliable across storage backends - #2273

Merged
khoinguyenpham04 merged 9 commits into
mainfrom
fix/media-upload-2265
Jul 30, 2026
Merged

fix: make media uploads reliable across storage backends#2273
khoinguyenpham04 merged 9 commits into
mainfrom
fix/media-upload-2265

Conversation

@khoinguyenpham04

@khoinguyenpham04 khoinguyenpham04 commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

Fixes media uploads for storage backends, including native Cloudflare R2 bindings, that cannot generate signed upload URLs. The upload negotiation endpoint now returns an authenticated same-origin streaming endpoint in that case, while preserving direct signed uploads for S3-compatible storage.

The upload lifecycle is hardened end to end:

  • streams request bodies to storage without buffering full files in Worker memory;
  • validates MIME type and exact byte length, including zero-byte files;
  • publishes unique upload-attempt keys atomically so concurrent uploads cannot overwrite or confirm different content;
  • derives hashes from stored/uploaded bytes and only uses compatible MIME type and size matches for deduplication;
  • makes confirmation idempotent and conditionally transitions only pending rows;
  • atomically deletes abandoned pending rows before removing their returned storage keys, preventing cleanup from deleting newly confirmed media;
  • cleans stale upload attempts and safely releases downloaded storage streams;
  • skips server-side image placeholders above the bounded processing limit.

Includes the additive upload-attempt migration, OpenAPI/schema updates, release notes, architecture documentation, and regression coverage for the admin client, upload routes, storage, SQLite, PostgreSQL, and D1 behavior.

Closes #2256
Closes #2265

Type of change

  • Bug fix
  • Feature (requires maintainer-approved Discussion)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes
  • pnpm test passes (or targeted tests for my change)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation (if applicable). Do not include messages.po changes except in translation PRs — a workflow extracts catalogs on merge to main. (N/A: no new user-visible admin strings.)
  • I have added a changeset (if this PR changes a published package)
  • New features link to an approved Discussion: https://github.com/emdash-cms/emdash/discussions/... (N/A: this is a bug fix.)

AI-generated code disclosure

  • This PR includes AI-generated code — model/tool: OpenAI Codex (GPT-5)

Screenshots / test output

No visual UI changes.

  • Focused core regression suites: 115 passed.
  • Final media-focused core suites: 35 passed.
  • Admin browser suite: 105 files and 1,247 tests passed.
  • Core and admin typechecks passed.
  • Type-aware lint and formatting checks passed.
  • Full core suite: 5,061 passed; one unrelated existing macOS path-canonicalization assertion differs on /var/... versus /private/var/....

Try this PR

Open a fresh playground →

A full working EmDash site, deployed from this branch. Each visit gets its own session-scoped sandbox: no login needed and no shared state. Try the admin, edit content, hit the public site.

Tracks fix/media-upload-2265. Updated automatically when the playground redeploys.

Copilot AI review requested due to automatic review settings July 29, 2026 13:55
@changeset-bot

changeset-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ecdf0b2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages
Name Type
emdash Patch
@emdash-cms/admin Patch
@emdash-cms/cloudflare Patch
@emdash-cms/sandbox-workerd Patch
@emdash-cms/plugin-mcp-smoke Patch
@emdash-cms/fixture-perf-site Patch
@emdash-cms/perf-demo-site Patch
@emdash-cms/cache-demo-site Patch
@emdash-cms/do-demo-site Patch
@emdash-cms/do-solo-demo-site Patch
@emdash-cms/auth Patch
@emdash-cms/blocks Patch
@emdash-cms/gutenberg-to-portable-text Patch
@emdash-cms/x402 Patch
create-emdash Patch
@emdash-cms/auth-atproto Patch
@emdash-cms/plugin-embeds Patch

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added the review/awaiting-author Reviewed; waiting on the author to respond label Jul 29, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 29, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
docs ecdf0b2 Jul 30 2026, 11:21 AM

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 29, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-playground ecdf0b2 Jul 30 2026, 11:20 AM

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 29, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-demo-do ecdf0b2 Jul 30 2026, 11:21 AM

@github-actions

Copy link
Copy Markdown
Contributor

Scope check

This PR changes 2,626 lines across 32 files. Large PRs are harder to review and more likely to be closed without review.
This PR spans 4 different areas (area/core, area/admin, area/docs, area/cloudflare). Consider breaking it into smaller, focused PRs.

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.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 29, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-demo-cache ecdf0b2 Jul 30 2026, 11:21 AM

@pkg-pr-new

pkg-pr-new Bot commented Jul 29, 2026

Copy link
Copy Markdown

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@2273

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@2273

@emdash-cms/auth-atproto

npm i https://pkg.pr.new/@emdash-cms/auth-atproto@2273

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@2273

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@2273

@emdash-cms/contentful-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/contentful-to-portable-text@2273

emdash

npm i https://pkg.pr.new/emdash@2273

create-emdash

npm i https://pkg.pr.new/create-emdash@2273

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@2273

@emdash-cms/plugin-cli

npm i https://pkg.pr.new/@emdash-cms/plugin-cli@2273

@emdash-cms/plugin-types

npm i https://pkg.pr.new/@emdash-cms/plugin-types@2273

@emdash-cms/registry-client

npm i https://pkg.pr.new/@emdash-cms/registry-client@2273

@emdash-cms/registry-lexicons

npm i https://pkg.pr.new/@emdash-cms/registry-lexicons@2273

@emdash-cms/registry-verification

npm i https://pkg.pr.new/@emdash-cms/registry-verification@2273

@emdash-cms/sandbox-workerd

npm i https://pkg.pr.new/@emdash-cms/sandbox-workerd@2273

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@2273

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@2273

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@2273

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@2273

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@2273

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@2273

@emdash-cms/plugin-field-kit

npm i https://pkg.pr.new/@emdash-cms/plugin-field-kit@2273

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@2273

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@2273

commit: ecdf0b2

@khoinguyenpham04 khoinguyenpham04 added the bot:review Trigger an emdashbot code review on this PR label Jul 29, 2026
@khoinguyenpham04 khoinguyenpham04 self-assigned this Jul 29, 2026
@emdashbot emdashbot Bot removed the bot:review Trigger an emdashbot code review on this PR label Jul 29, 2026
@github-actions github-actions Bot added review/needs-rereview Author pushed changes since the last review and removed review/awaiting-author Reviewed; waiting on the author to respond labels Jul 29, 2026
@khoinguyenpham04 khoinguyenpham04 added the bot:review Trigger an emdashbot code review on this PR label Jul 29, 2026
@emdashbot emdashbot Bot removed the bot:review Trigger an emdashbot code review on this PR label Jul 29, 2026

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the right change for the stated problem. Uploading to storage backends that cannot produce signed URLs — especially the native Cloudflare R2 binding — needs a same-origin streaming fallback, and the PR adds that path without breaking the S3-style signed-URL flow. The atomic publish-via-upload-attempt table, the cleanup ordering (DB row deleted before storage object), and the size/MIME/hash guards are all sound and well covered by tests.

I reviewed the full diff, the changed routes, the repository/Clean-up code, the admin client, the migration, the OpenAPI updates, and the regression tests. No blocking logic bug, regression, security issue, or AGENTS.md convention violation stood out. The only things I noticed are two comment blocks that drift into PR-style narrative rather than code explanations — worth tightening before merge, but not blockers.


Findings

  • [suggestion] packages/core/src/astro/routes/api/media/[id]/confirm.ts:187-194

    This comment block narrates the PR’s rationale ("the signed-URL flow uploads directly to storage, so this confirm is the only point at which the server sees the bytes", "buffering a large original into a Worker heap to compute a 32px blurhash would OOM"). It repeats design trade-offs that belong in the PR description rather than the code. Per AGENTS.md, comments should explain non-obvious invariants, not re-state the change proposal.

    		// LQIP is best-effort; oversized images skip server-side placeholders.
    
  • [suggestion] packages/admin/src/lib/api/media.ts:16-19

    The constant name already says what it is; the surrounding JSDoc justifies it by pointing at a remote schema and asks readers to "keep in sync". Cross-file notes that require manual synchronization age quickly and violate the comment-discipline rule against justification/narrative. If both sides need the same limit, export it from a shared package; otherwise the name is enough.

    export const MEDIA_SEARCH_MAX_LENGTH = 200;
    

@github-actions github-actions Bot added review/awaiting-author Reviewed; waiting on the author to respond review/needs-rereview Author pushed changes since the last review and removed review/needs-rereview Author pushed changes since the last review review/awaiting-author Reviewed; waiting on the author to respond labels Jul 29, 2026
@khoinguyenpham04 khoinguyenpham04 added the bot:review Trigger an emdashbot code review on this PR label Jul 29, 2026

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the right change for the stated problem and the latest revision is clean. The same-origin streaming fallback lets native R2/local storage work without signed URLs, while the S3 signed-URL flow is preserved. The atomic publishPendingStorageKey update, the upload-attempt cleanup table, and the delete-before-storage cleanup ordering all prevent the races this PR targets. MIME-type/size guards on deduplication, optional client hashing, and the 8 MiB LQIP cap are sensible.

I re-read the full diff plus the changed routes, repository, cleanup code, admin client, migration, OpenAPI document, and regression tests. Both comment-style findings from the previous review are fixed: confirm.ts now has only the concise LQIP invariant, and the stale cross-file JSDoc on MEDIA_SEARCH_MAX_LENGTH is gone.

No new blocking logic bug, regression, security issue, or AGENTS.md convention violation stands out. The OpenAPI schema for POST /media/upload-url correctly models the union of upload-target and existing-media responses, the migration indexes follow naming conventions, authorization uses the existing media:upload/media:edit_any permissions, and the extensive concurrency/race test coverage exercises the publication, retry, cleanup, and deletion paths.

@emdashbot emdashbot Bot removed the bot:review Trigger an emdashbot code review on this PR label Jul 29, 2026
@github-actions github-actions Bot added overlap and removed review/needs-rereview Author pushed changes since the last review labels Jul 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot added review/needs-rereview Author pushed changes since the last review and removed review/approved Approved; no new commits since labels Jul 30, 2026
@khoinguyenpham04 khoinguyenpham04 added the bot:review Trigger an emdashbot code review on this PR label Jul 30, 2026
@khoinguyenpham04 khoinguyenpham04 added bot:review Trigger an emdashbot code review on this PR and removed cla: signed bot:review Trigger an emdashbot code review on this PR labels Jul 30, 2026
@emdashbot emdashbot Bot removed the bot:review Trigger an emdashbot code review on this PR label Jul 30, 2026

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review: no new blockers

This is the right change for the stated problem. The same-origin streaming fallback lets native R2/local storage work without signed URLs, while the S3 signed-URL flow is preserved. The atomic publishPendingStorageKey update, the upload-attempt cleanup table, and the delete-before-storage cleanup ordering all prevent the races this PR targets. MIME-type/size guards on deduplication, optional client hashing, and the 8 MiB LQIP cap remain sensible.

I re-read the full diff plus the changed routes, repository, cleanup code, admin client, migration, OpenAPI document, and regression tests. Both comment-style findings from the previous review are still fixed: confirm.ts has only the concise LQIP invariant, and the stale cross-file JSDoc on MEDIA_SEARCH_MAX_LENGTH is gone.

No new blocking logic bug, regression, security issue, or AGENTS.md convention violation stands out in this revision. The OpenAPI schema for POST /media/upload-url continues to model the union of upload-target and existing-media responses correctly, migration indexes follow naming conventions, authorization uses the existing media:upload/media:edit_any permissions, and the concurrency/race test coverage exercises the publication, retry, cleanup, and deletion paths.

LGTM.

@github-actions github-actions Bot added review/approved Approved; no new commits since and removed review/needs-rereview Author pushed changes since the last review labels Jul 30, 2026
@khoinguyenpham04
khoinguyenpham04 merged commit b0c7880 into main Jul 30, 2026
51 checks passed
@khoinguyenpham04
khoinguyenpham04 deleted the fix/media-upload-2265 branch July 30, 2026 11:29
@emdashbot emdashbot Bot mentioned this pull request Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Image upload not working Admin media upload probes signed upload URLs with R2 binding storage and returns NOT_SUPPORTED

2 participants