From 1f321b893409bc3830328b879e05ba1d2be229c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BCp=20Can=20Akman?= Date: Mon, 20 Apr 2026 16:14:14 +0300 Subject: [PATCH 1/2] Fix R2 publicUrl being ignored for rendered media --- .changeset/quiet-rivers-bloom.md | 5 +++ packages/core/src/astro/middleware.ts | 5 ++- packages/core/src/astro/types.ts | 1 + .../core/src/components/EmDashImage.astro | 7 ++-- packages/core/src/components/Image.astro | 3 +- packages/core/src/media/url.ts | 22 ++++++++++++ packages/core/src/storage/s3.ts | 4 +-- packages/core/tests/unit/media/url.test.ts | 35 +++++++++++++++++++ packages/core/tests/unit/storage/s3.test.ts | 14 ++++++++ 9 files changed, 88 insertions(+), 8 deletions(-) create mode 100644 .changeset/quiet-rivers-bloom.md create mode 100644 packages/core/src/media/url.ts create mode 100644 packages/core/tests/unit/media/url.test.ts diff --git a/.changeset/quiet-rivers-bloom.md b/.changeset/quiet-rivers-bloom.md new file mode 100644 index 000000000..a29a5ac73 --- /dev/null +++ b/.changeset/quiet-rivers-bloom.md @@ -0,0 +1,5 @@ +--- +"emdash": minor +--- + +Renders local media through storage `publicUrl` when configured. `EmDashImage` and the Portable Text image block now call a new `locals.emdash.getPublicMediaUrl()` helper, so R2 and S3 deployments with a custom domain serve images from that domain. `S3Storage.getPublicUrl` now returns the `/_emdash/api/media/file/{key}` path when no `publicUrl` is set (previously `{endpoint}/{bucket}/{key}`). diff --git a/packages/core/src/astro/middleware.ts b/packages/core/src/astro/middleware.ts index 315e9c469..a220b7a1a 100644 --- a/packages/core/src/astro/middleware.ts +++ b/packages/core/src/astro/middleware.ts @@ -43,6 +43,7 @@ import { } from "../emdash-runtime.js"; import { setI18nConfig } from "../i18n/config.js"; import type { Database, Storage } from "../index.js"; +import { resolvePublicMediaUrl } from "../media/url.js"; import type { SandboxRunner } from "../plugins/sandbox/types.js"; import type { ResolvedPlugin } from "../plugins/types.js"; import { getRequestContext, runWithContext } from "../request-context.js"; @@ -301,10 +302,11 @@ export const onRequest = defineMiddleware(async (context, next) => { try { const runtime = await getRuntime(config, initSubTimings); setupVerified = true; - // eslint-disable-next-line typescript-eslint(no-unsafe-type-assertion) -- partial object; getPageRuntime() only checks for these two methods + // eslint-disable-next-line typescript-eslint(no-unsafe-type-assertion) -- partial object; getPageRuntime() only checks for the page-contribution methods locals.emdash = { collectPageMetadata: runtime.collectPageMetadata.bind(runtime), collectPageFragments: runtime.collectPageFragments.bind(runtime), + getPublicMediaUrl: (key: string) => resolvePublicMediaUrl(runtime.storage, key), } as EmDashHandlers; } catch { // Non-fatal — EmDashHead will fall back to base SEO contributions @@ -445,6 +447,7 @@ export const onRequest = defineMiddleware(async (context, next) => { // Direct access (for advanced use cases) storage: runtime.storage, db: runtime.db, + getPublicMediaUrl: (key: string) => resolvePublicMediaUrl(runtime.storage, key), hooks: runtime.hooks, email: runtime.email, configuredPlugins: runtime.configuredPlugins, diff --git a/packages/core/src/astro/types.ts b/packages/core/src/astro/types.ts index 9ec810b7a..083503155 100644 --- a/packages/core/src/astro/types.ts +++ b/packages/core/src/astro/types.ts @@ -339,6 +339,7 @@ export interface EmDashHandlers { // Direct access to storage and database for advanced use cases storage: import("../index.js").Storage | null; db: Kysely; + getPublicMediaUrl?: (storageKey: string) => string; // Hook pipeline for plugin integrations hooks: import("../plugins/hooks.js").HookPipeline; diff --git a/packages/core/src/components/EmDashImage.astro b/packages/core/src/components/EmDashImage.astro index 88c26a35f..100899056 100644 --- a/packages/core/src/components/EmDashImage.astro +++ b/packages/core/src/components/EmDashImage.astro @@ -57,10 +57,9 @@ function normalizeImage( */ function buildLocalImageUrl(img: MediaValue): string { const storageKey = (img.meta?.storageKey as string) || img.id; - if (storageKey) { - return `/_emdash/api/media/file/${storageKey}`; - } - return ""; + if (!storageKey) return ""; + const resolve = Astro.locals.emdash?.getPublicMediaUrl; + return resolve ? resolve(storageKey) : `/_emdash/api/media/file/${storageKey}`; } /** diff --git a/packages/core/src/components/Image.astro b/packages/core/src/components/Image.astro index 8d4284269..ccdb51a98 100644 --- a/packages/core/src/components/Image.astro +++ b/packages/core/src/components/Image.astro @@ -125,7 +125,8 @@ if (providerId && providerId !== "local") { // Fallback for local provider — prefer stored URL (includes storage key with extension), // fall back to _ref (bare ULID, works if media file endpoint supports ID lookup) if (!src) { - src = asset.url || `/_emdash/api/media/file/${asset._ref}`; + const resolve = Astro.locals.emdash?.getPublicMediaUrl; + src = asset.url || resolve?.(asset._ref) || `/_emdash/api/media/file/${asset._ref}`; } // Build placeholder background style diff --git a/packages/core/src/media/url.ts b/packages/core/src/media/url.ts new file mode 100644 index 000000000..0e444beb8 --- /dev/null +++ b/packages/core/src/media/url.ts @@ -0,0 +1,22 @@ +/** + * Public media URL resolution. + * + * Used at render time by the Image components to decide whether a storage + * key should be served from the configured `publicUrl` (R2 custom domain, + * S3 CDN) or through the internal `/_emdash/api/media/file/{key}` route. + */ +import type { Storage } from "../storage/types.js"; + +/** + * Resolve the public URL for a locally stored media key. Returns an empty + * string when no key is given. When a storage adapter is supplied, defers to + * `storage.getPublicUrl()`; otherwise returns the internal proxy route. + */ +export function resolvePublicMediaUrl( + storage: Storage | null | undefined, + storageKey: string, +): string { + if (!storageKey) return ""; + if (storage) return storage.getPublicUrl(storageKey); + return `/_emdash/api/media/file/${storageKey}`; +} diff --git a/packages/core/src/storage/s3.ts b/packages/core/src/storage/s3.ts index d51ba3044..4abb3e719 100644 --- a/packages/core/src/storage/s3.ts +++ b/packages/core/src/storage/s3.ts @@ -317,8 +317,8 @@ export class S3Storage implements Storage { if (this.publicUrl) { return `${this.publicUrl.replace(TRAILING_SLASH_PATTERN, "")}/${key}`; } - // Default to endpoint + bucket + key - return `${this.endpoint.replace(TRAILING_SLASH_PATTERN, "")}/${this.bucket}/${key}`; + // No public URL configured; defer to the /_emdash/api/media/file route. + return `/_emdash/api/media/file/${key}`; } } diff --git a/packages/core/tests/unit/media/url.test.ts b/packages/core/tests/unit/media/url.test.ts new file mode 100644 index 000000000..8a204f609 --- /dev/null +++ b/packages/core/tests/unit/media/url.test.ts @@ -0,0 +1,35 @@ +import { describe, it, expect } from "vitest"; + +import { resolvePublicMediaUrl } from "../../../src/media/url.js"; +import type { Storage } from "../../../src/storage/types.js"; + +function storageWith(publicUrl: string): Storage { + return { + upload: async () => ({ key: "", url: "", size: 0 }), + download: async () => { + throw new Error("not used"); + }, + delete: async () => {}, + exists: async () => true, + list: async () => ({ files: [] }), + getSignedUploadUrl: async () => { + throw new Error("not used"); + }, + getPublicUrl: (key) => `${publicUrl}/${key}`, + }; +} + +describe("resolvePublicMediaUrl", () => { + it("returns an empty string when storageKey is empty", () => { + expect(resolvePublicMediaUrl(null, "")).toBe(""); + }); + + it("uses the proxied media endpoint when no storage is provided", () => { + expect(resolvePublicMediaUrl(null, "01ABC.jpg")).toBe("/_emdash/api/media/file/01ABC.jpg"); + }); + + it("uses storage.getPublicUrl when a storage adapter is provided", () => { + const storage = storageWith("https://media.example.com"); + expect(resolvePublicMediaUrl(storage, "01ABC.jpg")).toBe("https://media.example.com/01ABC.jpg"); + }); +}); diff --git a/packages/core/tests/unit/storage/s3.test.ts b/packages/core/tests/unit/storage/s3.test.ts index d83be64d2..5c83befeb 100644 --- a/packages/core/tests/unit/storage/s3.test.ts +++ b/packages/core/tests/unit/storage/s3.test.ts @@ -248,4 +248,18 @@ describe("resolveS3Config", () => { expect(typeof storage.getPublicUrl).toBe("function"); }); }); + + describe("getPublicUrl", () => { + it("uses publicUrl when configured", () => { + setEnv({ ...FULL_ENV, S3_PUBLIC_URL: "https://cdn.example.com/" }); + const storage = createStorage({}); + expect(storage.getPublicUrl("01ABC.jpg")).toBe("https://cdn.example.com/01ABC.jpg"); + }); + + it("falls back to the proxied media endpoint when no publicUrl is configured", () => { + setEnv({ ...FULL_ENV, S3_PUBLIC_URL: undefined }); + const storage = createStorage({}); + expect(storage.getPublicUrl("01ABC.jpg")).toBe("/_emdash/api/media/file/01ABC.jpg"); + }); + }); }); From c4bff283b28aa53784e461ecec7bc9530153f4ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BCp=20Can=20Akman?= Date: Fri, 24 Apr 2026 00:49:34 +0300 Subject: [PATCH 2/2] Tighten public URL rewrite to safe storage keys Copilot flagged that asset._ref and img.id are bare ULIDs; calling storage.getPublicUrl() on them builds `{publicUrl}/{ulid}`, which 404s because stored object keys carry the extension. Route render-time lookups through a new buildRenderMediaUrl helper: it calls the public resolver only when a real storage key is present, and the url-rewrite branch accepts only `[A-Za-z0-9._-]+` so attacker-controlled portable-text content can't traverse to sibling prefixes on a shared CDN origin. Gallery now routes through the same helper so R2 publicUrl is honored there too. Middleware closure extracted into createPublicMediaUrlResolver. Tests in url.test.ts cover every branch including the security-reject cases. --- packages/core/src/astro/middleware.ts | 6 +- .../core/src/components/EmDashImage.astro | 12 ++- packages/core/src/components/Gallery.astro | 8 +- packages/core/src/components/Image.astro | 12 ++- packages/core/src/media/normalize.ts | 2 +- packages/core/src/media/url.ts | 56 ++++++++++ packages/core/tests/unit/media/url.test.ts | 100 +++++++++++++++++- 7 files changed, 179 insertions(+), 17 deletions(-) diff --git a/packages/core/src/astro/middleware.ts b/packages/core/src/astro/middleware.ts index a220b7a1a..939064bf6 100644 --- a/packages/core/src/astro/middleware.ts +++ b/packages/core/src/astro/middleware.ts @@ -43,7 +43,7 @@ import { } from "../emdash-runtime.js"; import { setI18nConfig } from "../i18n/config.js"; import type { Database, Storage } from "../index.js"; -import { resolvePublicMediaUrl } from "../media/url.js"; +import { createPublicMediaUrlResolver } from "../media/url.js"; import type { SandboxRunner } from "../plugins/sandbox/types.js"; import type { ResolvedPlugin } from "../plugins/types.js"; import { getRequestContext, runWithContext } from "../request-context.js"; @@ -306,7 +306,7 @@ export const onRequest = defineMiddleware(async (context, next) => { locals.emdash = { collectPageMetadata: runtime.collectPageMetadata.bind(runtime), collectPageFragments: runtime.collectPageFragments.bind(runtime), - getPublicMediaUrl: (key: string) => resolvePublicMediaUrl(runtime.storage, key), + getPublicMediaUrl: createPublicMediaUrlResolver(runtime.storage), } as EmDashHandlers; } catch { // Non-fatal — EmDashHead will fall back to base SEO contributions @@ -447,7 +447,7 @@ export const onRequest = defineMiddleware(async (context, next) => { // Direct access (for advanced use cases) storage: runtime.storage, db: runtime.db, - getPublicMediaUrl: (key: string) => resolvePublicMediaUrl(runtime.storage, key), + getPublicMediaUrl: createPublicMediaUrlResolver(runtime.storage), hooks: runtime.hooks, email: runtime.email, configuredPlugins: runtime.configuredPlugins, diff --git a/packages/core/src/components/EmDashImage.astro b/packages/core/src/components/EmDashImage.astro index 100899056..710c58974 100644 --- a/packages/core/src/components/EmDashImage.astro +++ b/packages/core/src/components/EmDashImage.astro @@ -20,6 +20,7 @@ import type { MediaValue } from "../fields/types.js"; import type { HTMLAttributes } from "astro/types"; import type { ImageEmbed } from "../media/types.js"; import { getMediaProvider } from "../media/provider-loader.js"; +import { buildRenderMediaUrl } from "../media/url.js"; // Standard responsive breakpoints const BREAKPOINTS = [640, 750, 828, 960, 1080, 1280, 1600, 1920]; @@ -53,13 +54,14 @@ function normalizeImage( } /** - * Build the URL for a local image + * Build the URL for a local image. Prefers `meta.storageKey`; falls back to + * the internal proxy with `img.id` when no storage key is available. */ function buildLocalImageUrl(img: MediaValue): string { - const storageKey = (img.meta?.storageKey as string) || img.id; - if (!storageKey) return ""; - const resolve = Astro.locals.emdash?.getPublicMediaUrl; - return resolve ? resolve(storageKey) : `/_emdash/api/media/file/${storageKey}`; + return buildRenderMediaUrl(Astro.locals.emdash?.getPublicMediaUrl, { + storageKey: img.meta?.storageKey as string | undefined, + id: img.id, + }); } /** diff --git a/packages/core/src/components/Gallery.astro b/packages/core/src/components/Gallery.astro index 7d2bf5b3a..01a89594b 100644 --- a/packages/core/src/components/Gallery.astro +++ b/packages/core/src/components/Gallery.astro @@ -6,6 +6,7 @@ * Uses Astro's Image component for optimization when dimensions are available. */ import { Image as AstroImage } from "astro:assets"; +import { buildRenderMediaUrl } from "../media/url.js"; export interface Props { node: { @@ -39,9 +40,10 @@ if (!images.length) {
{ images.map((image) => { - const src = - image.asset.url || - `/_emdash/api/media/file/${image.asset._ref}`; + const src = buildRenderMediaUrl(Astro.locals.emdash?.getPublicMediaUrl, { + url: image.asset.url, + id: image.asset._ref, + }); const hasSize = image.width && image.height; return (
diff --git a/packages/core/src/components/Image.astro b/packages/core/src/components/Image.astro index ccdb51a98..59dfaea6c 100644 --- a/packages/core/src/components/Image.astro +++ b/packages/core/src/components/Image.astro @@ -7,6 +7,7 @@ */ import type { ImageEmbed } from "../media/types.js"; import { getMediaProvider } from "../media/provider-loader.js"; +import { buildRenderMediaUrl } from "../media/url.js"; // Standard responsive breakpoints const BREAKPOINTS = [640, 750, 828, 960, 1080, 1280, 1600, 1920]; @@ -122,11 +123,14 @@ if (providerId && providerId !== "local") { } } -// Fallback for local provider — prefer stored URL (includes storage key with extension), -// fall back to _ref (bare ULID, works if media file endpoint supports ID lookup) +// Fallback for local provider. `asset.url` carries the storage key with +// extension when present; `asset._ref` is a bare ULID that only the internal +// `/file/{id}` route can resolve. `buildRenderMediaUrl` picks the right shape. if (!src) { - const resolve = Astro.locals.emdash?.getPublicMediaUrl; - src = asset.url || resolve?.(asset._ref) || `/_emdash/api/media/file/${asset._ref}`; + src = buildRenderMediaUrl(Astro.locals.emdash?.getPublicMediaUrl, { + url: asset.url, + id: asset._ref, + }); } // Build placeholder background style diff --git a/packages/core/src/media/normalize.ts b/packages/core/src/media/normalize.ts index 2c0ca87fe..8d4488295 100644 --- a/packages/core/src/media/normalize.ts +++ b/packages/core/src/media/normalize.ts @@ -11,7 +11,7 @@ import type { MediaProvider, MediaProviderItem, MediaValue } from "./types.js"; -const INTERNAL_MEDIA_PREFIX = "/_emdash/api/media/file/"; +export const INTERNAL_MEDIA_PREFIX = "/_emdash/api/media/file/"; const URL_PATTERN = /^https?:\/\//; /** diff --git a/packages/core/src/media/url.ts b/packages/core/src/media/url.ts index 0e444beb8..888fe668e 100644 --- a/packages/core/src/media/url.ts +++ b/packages/core/src/media/url.ts @@ -6,6 +6,13 @@ * S3 CDN) or through the internal `/_emdash/api/media/file/{key}` route. */ import type { Storage } from "../storage/types.js"; +import { INTERNAL_MEDIA_PREFIX } from "./normalize.js"; + +// Keys accepted by the public-URL rewrite: the `{ulid}{ext}` shape produced by +// the upload pipeline, with letters, digits, dots, dashes, and underscores. +// Slashes, `?`, `#`, and `%` are rejected so attacker-controlled content in a +// portable-text `asset.url` cannot traverse or reroute on the CDN origin. +const SAFE_STORAGE_KEY = /^[A-Za-z0-9._-]+$/; /** * Resolve the public URL for a locally stored media key. Returns an empty @@ -20,3 +27,52 @@ export function resolvePublicMediaUrl( if (storage) return storage.getPublicUrl(storageKey); return `/_emdash/api/media/file/${storageKey}`; } + +/** + * Build the `getPublicMediaUrl` closure attached to `Astro.locals.emdash`. + * Shared by the anonymous fast path and the full-runtime path in middleware. + * + * @internal + */ +export function createPublicMediaUrlResolver( + storage: Storage | null | undefined, +): (key: string) => string { + return (key) => resolvePublicMediaUrl(storage, key); +} + +/** Input shape for {@link buildRenderMediaUrl}. */ +export interface RenderMediaRef { + /** Storage key with extension (the canonical shape from the upload pipeline). */ + storageKey?: string; + /** Pre-baked URL (either an internal proxy URL or an external URL). */ + url?: string; + /** Bare media id (ULID without extension); only the internal proxy can look this up. */ + id?: string; +} + +/** + * Build a render-time media URL. Prefers `storageKey`, then rewrites an + * internal `url` via `resolve`, then falls back to the internal proxy for a + * bare `id`. External URLs and non-matching internal-looking URLs pass + * through untouched. Returns `""` when nothing usable is present. + * + * @internal + */ +export function buildRenderMediaUrl( + resolve: ((key: string) => string) | undefined, + ref: RenderMediaRef, +): string { + const { storageKey, url, id } = ref; + if (storageKey) { + return resolve ? resolve(storageKey) : `${INTERNAL_MEDIA_PREFIX}${storageKey}`; + } + if (url) { + if (resolve && url.startsWith(INTERNAL_MEDIA_PREFIX)) { + const key = url.slice(INTERNAL_MEDIA_PREFIX.length); + if (SAFE_STORAGE_KEY.test(key)) return resolve(key); + } + return url; + } + if (id) return `${INTERNAL_MEDIA_PREFIX}${id}`; + return ""; +} diff --git a/packages/core/tests/unit/media/url.test.ts b/packages/core/tests/unit/media/url.test.ts index 8a204f609..c41295710 100644 --- a/packages/core/tests/unit/media/url.test.ts +++ b/packages/core/tests/unit/media/url.test.ts @@ -1,6 +1,10 @@ import { describe, it, expect } from "vitest"; -import { resolvePublicMediaUrl } from "../../../src/media/url.js"; +import { + buildRenderMediaUrl, + createPublicMediaUrlResolver, + resolvePublicMediaUrl, +} from "../../../src/media/url.js"; import type { Storage } from "../../../src/storage/types.js"; function storageWith(publicUrl: string): Storage { @@ -33,3 +37,97 @@ describe("resolvePublicMediaUrl", () => { expect(resolvePublicMediaUrl(storage, "01ABC.jpg")).toBe("https://media.example.com/01ABC.jpg"); }); }); + +describe("createPublicMediaUrlResolver", () => { + it("returns a closure that reuses the storage adapter", () => { + const resolver = createPublicMediaUrlResolver(storageWith("https://media.example.com")); + expect(resolver("01ABC.jpg")).toBe("https://media.example.com/01ABC.jpg"); + expect(resolver("01XYZ.png")).toBe("https://media.example.com/01XYZ.png"); + }); + + it("falls back to the internal proxy when no storage is given", () => { + const resolver = createPublicMediaUrlResolver(null); + expect(resolver("01ABC.jpg")).toBe("/_emdash/api/media/file/01ABC.jpg"); + }); +}); + +describe("buildRenderMediaUrl", () => { + const resolveCdn = (key: string) => `https://media.example.com/${key}`; + + it("routes an explicit storageKey through resolve", () => { + expect(buildRenderMediaUrl(resolveCdn, { storageKey: "01ABC.jpg" })).toBe( + "https://media.example.com/01ABC.jpg", + ); + }); + + it("uses the internal proxy for storageKey when resolve is absent", () => { + expect(buildRenderMediaUrl(undefined, { storageKey: "01ABC.jpg" })).toBe( + "/_emdash/api/media/file/01ABC.jpg", + ); + }); + + it("rewrites an internal url via resolve so publicUrl is honored", () => { + expect( + buildRenderMediaUrl(resolveCdn, { + url: "/_emdash/api/media/file/01ABC.jpg", + id: "01ABC", + }), + ).toBe("https://media.example.com/01ABC.jpg"); + }); + + it("leaves an external url untouched even when resolve is given", () => { + expect( + buildRenderMediaUrl(resolveCdn, { + url: "https://other-cdn.example.com/01ABC.jpg", + }), + ).toBe("https://other-cdn.example.com/01ABC.jpg"); + }); + + it("returns an internal url as-is when no resolve is given", () => { + expect( + buildRenderMediaUrl(undefined, { + url: "/_emdash/api/media/file/01ABC.jpg", + }), + ).toBe("/_emdash/api/media/file/01ABC.jpg"); + }); + + it("uses the internal proxy for a bare id", () => { + expect(buildRenderMediaUrl(resolveCdn, { id: "01ABC" })).toBe("/_emdash/api/media/file/01ABC"); + }); + + it("returns an empty string when no fields are usable", () => { + expect(buildRenderMediaUrl(resolveCdn, {})).toBe(""); + }); + + it("does not rewrite a url that only shares the media prefix", () => { + expect( + buildRenderMediaUrl(resolveCdn, { + url: "/_emdash/api/media/file-list/01ABC.jpg", + }), + ).toBe("/_emdash/api/media/file-list/01ABC.jpg"); + }); + + it("passes an internal url through when the captured key contains a slash", () => { + expect( + buildRenderMediaUrl(resolveCdn, { + url: "/_emdash/api/media/file/../other-tenant/secret.pdf", + }), + ).toBe("/_emdash/api/media/file/../other-tenant/secret.pdf"); + }); + + it("passes an internal url through when the captured key contains a query string", () => { + expect( + buildRenderMediaUrl(resolveCdn, { + url: "/_emdash/api/media/file/01ABC.jpg?v=2", + }), + ).toBe("/_emdash/api/media/file/01ABC.jpg?v=2"); + }); + + it("passes an internal url through when the captured key is percent-encoded", () => { + expect( + buildRenderMediaUrl(resolveCdn, { + url: "/_emdash/api/media/file/01%2FABC.jpg", + }), + ).toBe("/_emdash/api/media/file/01%2FABC.jpg"); + }); +});