Skip to content

fix(core): allow the configured storage endpoint origin in connect-src#1738

Merged
ascorbic merged 3 commits into
emdash-cms:mainfrom
Emdash-Bug-Testing:fix/205-csp-connect-src-storage
Jul 6, 2026
Merged

fix(core): allow the configured storage endpoint origin in connect-src#1738
ascorbic merged 3 commits into
emdash-cms:mainfrom
Emdash-Bug-Testing:fix/205-csp-connect-src-storage

Conversation

@marcusbellamyshaw-cell

Copy link
Copy Markdown
Contributor

What does this PR do?

Media uploads for S3-compatible storage (R2, S3, Minio, etc.) do a direct browser PUT to the storage endpoint's origin. The admin's strict CSP connect-src only allowed 'self' (and, if configured, the registry aggregator origin), so it blocked these signed uploads whenever the storage endpoint was a different origin than the site. buildEmDashCsp() now also accepts the configured storage endpoint and adds its origin to connect-src.

Closes #205

Type of change

  • Bug fix

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes
  • pnpm test passes (targeted: packages/core/tests/unit/middleware/csp.test.ts)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • I have added a changeset (if this PR changes a published package)

AI-generated code disclosure

  • This PR includes AI-generated code — model/tool: Claude (Claude Code CLI)

Media uploads for S3-compatible storage do a direct browser PUT to the
storage endpoint's origin, which the strict admin CSP's connect-src
blocked unless it matched 'self'. buildEmDashCsp() now also accepts a
storage endpoint and adds its origin to connect-src alongside the
existing registry aggregator origin.

Fixes emdash-cms#205
getConfiguredStorageEndpoint() cast StorageDescriptor["config"] (typed
unknown) straight to Record<string, unknown>, which oxlint's
no-unsafe-type-assertion rule flags since a wider type has no
guarantee of narrowing correctly. Use an `in` operator check instead,
which TS narrows safely without a cast.
@changeset-bot

changeset-bot Bot commented Jul 3, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e3237aa

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

This PR includes changesets to release 16 packages
Name Type
emdash Patch
@emdash-cms/cloudflare Patch
@emdash-cms/sandbox-workerd 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/admin 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

@github-actions github-actions Bot added review/needs-review No maintainer or bot review yet area/core size/M labels Jul 3, 2026
@pkg-pr-new

pkg-pr-new Bot commented Jul 3, 2026

Copy link
Copy Markdown

Open in StackBlitz

@emdash-cms/admin

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

@emdash-cms/auth

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

@emdash-cms/auth-atproto

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

@emdash-cms/blocks

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

@emdash-cms/cloudflare

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

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

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

emdash

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

create-emdash

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

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

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

@emdash-cms/plugin-cli

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

@emdash-cms/plugin-types

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

@emdash-cms/registry-client

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

@emdash-cms/registry-lexicons

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

@emdash-cms/sandbox-workerd

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

@emdash-cms/x402

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

@emdash-cms/plugin-ai-moderation

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

@emdash-cms/plugin-atproto

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

@emdash-cms/plugin-audit-log

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

@emdash-cms/plugin-color

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

@emdash-cms/plugin-embeds

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

@emdash-cms/plugin-field-kit

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

@emdash-cms/plugin-forms

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

@emdash-cms/plugin-webhook-notifier

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

commit: e3237aa

@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.

The bug and the approach are right: direct browser uploads to S3-compatible storage need the endpoint origin in the admin's connect-src CSP, and the change adds that cleanly. The implementation is focused, uses safe URL parsing, deduplicates origins, and the tests cover explicit configuration, non-HTTP(S) endpoints, malformed URLs, and shared origins.

The one real gap is that getConfiguredStorageEndpoint() only reads config.endpoint from the build-time StorageDescriptor. The s3() adapter documents that omitted fields are resolved from the matching S3_* environment variables at runtime, especially S3_ENDPOINT. Users who configure storage as s3() with only S3_ENDPOINT set will still have their upload origin missing from connect-src, so the bug (#205) remains for that documented path. A complete fix should cover the env-var fallback (with a test), or read the resolved endpoint from the runtime storage instance.

I reviewed the diff, the changed CSP/auth/test files, and the storage adapter/runtime config serialization paths statically.

Comment on lines +24 to 32
export function getConfiguredStorageEndpoint(
storage: StorageDescriptor | undefined,
): string | undefined {
const config = storage?.config;
if (typeof config !== "object" || config === null || !("endpoint" in config)) return undefined;
const endpoint = config.endpoint;
return typeof endpoint === "string" ? endpoint : undefined;
}

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.

[needs fixing] This only looks at config.endpoint from the build-time StorageDescriptor. The s3() adapter documents that omitted fields are resolved from S3_* environment variables at runtime, so a common documented setup is s3() plus S3_ENDPOINT. For those users the storage origin is still missing from connect-src and bug #205 remains.

Add the same S3_ENDPOINT fallback here (or, cleaner, expose the resolved endpoint from the runtime S3Storage and read it from context.locals.emdash.storage) and add a test for env-var-only configuration.

Suggested change
export function getConfiguredStorageEndpoint(
storage: StorageDescriptor | undefined,
): string | undefined {
const config = storage?.config;
if (typeof config !== "object" || config === null || !("endpoint" in config)) return undefined;
const endpoint = config.endpoint;
return typeof endpoint === "string" ? endpoint : undefined;
}
export function getConfiguredStorageEndpoint(
storage: StorageDescriptor | undefined,
): string | undefined {
const config = storage?.config;
if (typeof config !== "object" || config === null) return undefined;
if ("endpoint" in config && typeof config.endpoint === "string") {
return config.endpoint;
}
// The S3 adapter resolves endpoint from S3_ENDPOINT at runtime; mirror
// that fallback so env-var-only configs also get the origin in connect-src.
if (storage?.entrypoint === "emdash/storage/s3" && typeof process !== "undefined") {
const envEndpoint = process.env.S3_ENDPOINT;
if (typeof envEndpoint === "string") return envEndpoint;
}
return undefined;
}

@github-actions github-actions Bot added review/awaiting-author Reviewed; waiting on the author to respond and removed review/needs-review No maintainer or bot review yet labels Jul 3, 2026
…ig omits it

Addresses review feedback on emdash-cms#1738: getConfiguredStorageEndpoint() only
read config.endpoint from the build-time StorageDescriptor. The s3()
adapter documents that omitted fields are resolved from the matching
S3_* env var at runtime (S3_ENDPOINT included), so a site configured as
s3({ ... }) with only S3_ENDPOINT set had no endpoint in the descriptor
and its upload origin was still missing from connect-src (emdash-cms#205 remained
for that path).

getConfiguredStorageEndpoint() now falls back to process.env.S3_ENDPOINT
when the descriptor is the s3 adapter and its config has no explicit
endpoint. Explicit config still wins; non-s3 adapters (local, r2, ...)
are unaffected.
@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 3, 2026

@ascorbic ascorbic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@ascorbic ascorbic merged commit a3ec23d into emdash-cms:main Jul 6, 2026
45 checks passed
@emdashbot emdashbot Bot mentioned this pull request Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/core review/needs-rereview Author pushed changes since the last review size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CSP connect-src blocks S3 signed uploads from admin UI

2 participants