fix(plugins): require manage + CSRF on private plugin routes regardless of HTTP method#1863
Merged
Merged
Conversation
…ss of method Plugin API routes dispatch by name only — the HTTP method never selects a different handler — so a route reached via GET or HEAD runs exactly the same code and side effects as one reached via POST. The catch-all tiered permission by method (GET/HEAD/OPTIONS -> plugins:read, else plugins:manage) and skipped CSRF for the read tier, so an editor (plugins:read) could invoke an admin-only mutating route by choosing GET, and a cross-origin page could reach it via HEAD (CORS-safe, and Astro dispatches HEAD to the GET export). Gate every private invocation on plugins:manage and require the CSRF header regardless of method. Public routes are unchanged. Fixes emdash-cms#1853
🦋 Changeset detectedLatest commit: 7c6b4a2 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
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}
Contributor
There was a problem hiding this comment.
This is a clean, narrowly-scoped security fix for #1853.
Approach: Correct. Plugin routes dispatch by route name, not by HTTP method, so gating GET/HEAD/OPTIONS on plugins:read and skipping CSRF allowed editors (and cross-origin pages via CORS-safe HEAD) to invoke state-changing admin routes. Removing the method-based tier and requiring plugins:manage + the X-EmDash-Request CSRF header for every private invocation is the right fix. Public routes remain unauthenticated as designed.
What I checked:
- The catch-all route now requires
plugins:manage, theadmintoken scope, and CSRF for all non-public invocations regardless of method. HEADdispatches toGETin Astro, so the GET-export test correctly covers the HEAD bypass vector.- Token-authenticated requests retain the CSRF exemption because they are not ambient credentials.
- Error response codes (401/403) and the
apiErrorenvelope shape follow project conventions. - The new regression test has 7 cases that exercise editor GET/HEAD/POST, admin-with/without CSRF, and public-route behavior.
- The changeset is present, present-tense, user-facing, and clearly frames the breaking security behavior change.
No blocking issues, style problems, or convention violations found.
ascorbic
approved these changes
Jul 7, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes #1853 — a privilege escalation / CSRF hole on the plugin API catch-all (
/_emdash/api/plugins/{id}/*).Plugin routes dispatch by name only — the HTTP method never selects a different handler, so a route reached via
GETorHEADruns exactly the same code (and side effects) as one reached viaPOST. But the catch-all tiered permission by method:GET/HEAD/OPTIONS→plugins:read(editor) + CSRF skippedplugins:manage(admin) + CSRF requiredSo an editor could invoke an admin-only, state-changing plugin route by sending it as
GET, and a cross-origin page could reach it viaHEAD(a CORS-safe method that Astro dispatches to theGETexport, then strips the body — the handler still runs). The reporter verified this end-to-end on 0.25.1:HEAD /_emdash/api/plugins/<id>/updateHomeConfigreturned 200 and overwrote stored plugin state.The fix stops trusting the method as a read/write signal: every private plugin invocation now requires
plugins:manageand theX-EmDash-RequestCSRF header, regardless of method. Public routes (public: true) are unchanged — they still skip auth and CSRF as before.Behavior note: the
plugins:read/GETtier is removed. In practice every first-party admin caller (plugin admin pages, fieldoptionsRoute) already usesPOST, which already requiredplugins:manage, so this doesn't regress existing flows — it only closes the method-chosen bypass. A per-route method/permission declaration (issue suggestion 2/3) would be a larger, additive change; happy to go that way instead if maintainers prefer to keep a read tier.Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runAI-generated code disclosure
Screenshots / test output
New regression test
tests/unit/astro/plugin-api-route-auth.test.ts(7 cases). The GET- and HEAD-as-editor cases fail onmainand pass with the fix (verified by reverting the source change):