feat(plugins): opt-in raw request body for native plugin routes#1983
feat(plugins): opt-in raw request body for native plugin routes#1983swissky wants to merge 2 commits into
Conversation
Routes that set rawBody: true receive the unparsed request body as ctx.rawBody, alongside the parsed ctx.input. Enables webhook signature verification (HMAC over exact raw bytes) and non-JSON payloads, which were impossible since the dispatcher consumes the body stream. The dispatcher already buffers the body; it now reads text() once and parses input from the same buffer, so there is no extra I/O and no behavior change for existing routes.
🦋 Changeset detectedLatest commit: 4d62748 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}
There was a problem hiding this comment.
This is a clean, additive feature that solves a real pain point: native plugin routes need the unparsed body to verify webhook signatures and to handle non-JSON deliveries. The approach is idiomatic (opt-in flag on the route, dispatcher reads text() once, handler conditionally exposes rawBody) and fits EmDash's existing route flow without changing behavior for existing routes.
I did not run the test suite, linter, or builds (no shell/runtime access), so I take the author's test/type-check claims as unverified. Code-wise the change is straightforward, but I spotted two gaps worth addressing before merge:
-
The implementation in
emdash-runtime.tslacks an integration test. The new tests only exercisePluginRouteHandler.invokeand passrawBodyin manually; they don't verify thatEmDashRuntime.handlePluginApiRouteactually readsrequest.text(), buffers the string, parses JSON when valid, and leavesctx.inputundefined for non-JSON payloads. The repo already has an integration test following this exact pattern (plugin-media-route.test.ts), so a matchingplugin-raw-body-route.test.tswould close the gap. -
Docs/comments claim "exact raw bytes" while
ctx.rawBodyis a decoded string. Webhook signatures are HMACs over bytes, andrequest.text()/ctx.rawBody: stringis a UTF-8 decoding step away from bytes. In practice Stripe/GitHub/Svix deliver UTF-8 JSON, so re-encoding withnew TextEncoder()works, but the wording is technically incorrect and could mislead users handling binary or non-UTF8 payloads. Consider softening the claim or documenting the UTF-8 assumption.
Neither is a blocker, but both are worth a quick pass.
|
|
||
| // Buffer the body as text so routes with `rawBody: true` can see the | ||
| // exact bytes (webhook signature verification); parse JSON from the | ||
| // same buffer for `ctx.input`. | ||
| let body: unknown = undefined; | ||
| let rawBody: string | undefined; | ||
| try { | ||
| body = await request.json(); | ||
| rawBody = await request.text(); |
There was a problem hiding this comment.
[suggestion] The new unit tests in routes.test.ts pass rawBody directly into PluginRouteHandler.invoke; they don't exercise this block, which is the real behavioral change of the PR. There is already an integration-test pattern for EmDashRuntime.handlePluginApiRoute (see packages/core/tests/integration/runtime/plugin-media-route.test.ts). Adding an integration test would verify that request.text() is read once, JSON is parsed into ctx.input, non-JSON payloads leave ctx.input undefined, and ctx.rawBody is only populated for opted-in routes.
|
|
||
| // Buffer the body as text so routes with `rawBody: true` can see the | ||
| // exact bytes (webhook signature verification); parse JSON from the |
There was a problem hiding this comment.
[suggestion] The comment says this buffers the body so routes can see the "exact bytes" for webhook signatures, but request.text() decodes the bytes to a UTF-8 string and ctx.rawBody is typed as string. For the common webhook case (UTF-8 JSON) this is fine — users can re-encode with new TextEncoder() — but for non-UTF8 or binary payloads it is not byte-exact. Consider softening the comment to "exact text" or documenting the UTF-8 assumption.
| - **Native route handlers take a single argument** — `(ctx: RouteContext)` where `ctx.input`, `ctx.request`, and `ctx.requestMeta` are merged with the regular `PluginContext` properties. This is the opposite of standard format's two-argument shape. See [API routes](/plugins/creating-plugins/api-routes/) for the full surface (everything else is identical). | ||
|
|
||
| ## Raw request bodies (webhook signatures) | ||
|
|
There was a problem hiding this comment.
[suggestion] This says webhook providers sign the "exact raw bytes", but ctx.rawBody is a UTF-8 decoded string rather than bytes. In practice most webhook payloads are UTF-8 JSON and signature libraries accept a string, but the wording over-promises for binary or non-UTF8 deliveries. Consider changing to "exact raw text" or adding a note that ctx.rawBody is the UTF-8 decoded body.
| }, | ||
| }, | ||
| ``` | ||
|
|
There was a problem hiding this comment.
[suggestion] Same wording issue as line 155: "exactly as received" implies the original byte stream, while ctx.rawBody is a decoded string. A small doc tweak would set the right expectation.
Review follow-up: adds an integration test exercising the actual EmDashRuntime.handlePluginApiRoute path (buffer text() once, parse input from the same buffer, forward rawBody only to opted-in routes), and corrects the "exact raw bytes" wording — ctx.rawBody is the UTF-8 decoded body string, which is what webhook signatures need in practice; binary payloads are not byte-exact.
|
Both points addressed in 4d62748:
|
github-actions
Bot
added
size/L
review/needs-rereview
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
1 participant
What does this PR do?
Adds an opt-in
rawBody: trueflag for native plugin routes. Routes that set it receive the unparsed request body asctx.rawBody, alongside the parsedctx.input.Why: webhook providers (Stripe, GitHub, Svix, …) sign the exact raw bytes of the delivery. Since the dispatcher consumes the body stream to build
ctx.input(andctx.request.text()is guarded per #1293), plugins currently have no way to verify those signatures — an HMAC over a re-serializedctx.inputnever matches. Non-JSON payloads (form-encoded webhook deliveries) are also silently lost today.How: the dispatcher already buffers the body to parse it. It now reads
text()once, parsesctx.inputfrom the same buffer, and forwards the string only to routes that opted in. No extra I/O, no behavior change for existing routes. The #1293 guard message now points atrawBody: trueas the escape hatch. Native format only; sandboxed plugins cross a serialization boundary and would need separate plumbing.Discussion: #1982
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. (n/a — no admin UI changes)AI-generated code disclosure
Screenshots / test output
New tests cover: raw body delivered on opted-in routes (byte-exact, alongside validated input),
ctx.rawBodyundefined without the flag, and non-JSON payloads reaching the handler withinputundefined.