Repository navigation
fix(core): send Cache-Control private, no-store on the admin shell - #1741
Conversation
🦋 Changeset detectedLatest commit: b26cd8f 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 |
github-actions
Bot
added
area/core
area/docs
size/S
review/needs-review
@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 the right change, implemented in the right place. The admin shell (/_emdash/admin/[...path]) is served by a single catch-all Astro route, and because it is personalized per request (locale) and drives authenticated flows, it must not be stored by shared caches. Setting Cache-Control: private, no-store in packages/core/src/astro/routes/admin.astro mirrors the existing API_CACHE_HEADERS pattern in api/error.ts and the middleware cache headers in astro/middleware/auth.ts. The header survives the finalizeResponse wrapper because that helper only sets-if-absent baseline security headers and does not touch Cache-Control.
The Cloudflare docs addition is accurate and useful, and the changeset is a user-facing patch note for the emdash package.
The only gap is the missing regression test. AGENTS.md emphasizes TDD for bugs, and this PR is explicitly categorized as a bug fix. The author acknowledged this by leaving the test checklist item unchecked, but a regression test—likely an integration or e2e test that hits /_emdash/admin/login and asserts the response header—would prevent future regressions if the shell is refactored. Since the code itself is correct and verified manually, I’m treating this as a non-blocking suggestion rather than a request for changes.
| // of the login redirect. The shell is also personalized per request (locale | ||
| // via cookie/Accept-Language). API routes already send `private, no-store` | ||
| // (API_CACHE_HEADERS); this closes the same gap for the admin HTML. | ||
| Astro.response.headers.set("Cache-Control", "private, no-store"); |
There was a problem hiding this comment.
[suggestion] The new Cache-Control: private, no-store header is correct, but this is a bug fix with no reproducing test. AGENTS.md requires TDD for bugs, and the author left the test checklist item unchecked. Consider adding an integration or e2e test that requests /_emdash/admin/login (or any admin-shell path) and asserts the response header is private, no-store, similar to the existing request-context-toolbar-cache.test.ts and cache-headers.test.ts regression tests.
github-actions
Bot
added
review/needs-rereview
|
Fair point on the missing regression test — added one in b26cd8f: an e2e test in |
What does this PR do?
The admin shell response (
/_emdash/admin/[...path],admin.astro) is served without anyCache-Controlheader. Shared caches that apply RFC 9111 heuristic freshness will store it — for example, Cloudflare's new Workers Cache ("cache": { "enabled": true }) caches header-less200s for 2 hours. In that setup an authenticated editor's200shell can be replayed to anonymous visitors instead of the login redirect, and since the shell is personalized per request (locale via cookie /Accept-Language), the cached variant can also be wrong for the requester.The API layer already guards against this (
API_CACHE_HEADERSsendsprivate, no-storeon every API response); this closes the same gap for the admin HTML by settingCache-Control: private, no-storeon the shell response.Also adds a short "Workers Cache" section to the Cloudflare deployment docs describing how EmDash behaves with the feature enabled and what to watch out for (heuristic freshness on header-less custom routes, visual-editing toolbar on cached public pages).
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
Added an e2e regression test (
e2e/tests/auth.spec.ts) that requests/_emdash/admin/loginand asserts thecache-control: private, no-storeresponse header.Verified on a deployed Cloudflare Worker with Workers Cache enabled: before the change
GET /_emdash/admin/loginreturned200with noCache-Control; after the change it returnscache-control: private, no-storeand Workers Cache reportsBYPASSinstead of storing the shell.pnpm --filter emdash typecheckand the core test suite pass locally (4269 passed; the 8 failures intests/unit/astro/vite-config.test.tsare environment-only — they require a built@emdash-cms/admin/dist, whose build currently fails on Node 24.1.0 withERR_INTERNAL_ASSERTIONin lingui, unrelated to this change).