-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(plugins): opt-in raw request body for native plugin routes #1983
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "emdash": minor | ||
| --- | ||
|
|
||
| Adds opt-in raw request body access for native plugin routes: set `rawBody: true` on a route to receive the unparsed body as `ctx.rawBody`, enabling webhook signature verification and non-JSON payloads. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -151,6 +151,29 @@ Key details of this configuration: | |
| - **`id`, `version`, and `capabilities` appear twice.** Once on the descriptor, once on `definePlugin()`. They should match. The descriptor's copy is what `astro.config.mjs` sees at build time; the `definePlugin()` copy is what runs at request time. | ||
| - **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) | ||
|
|
||
| The dispatcher parses the request body before your handler runs and exposes it as `ctx.input`; the body stream is consumed, so `ctx.request.text()` is unavailable. That's fine for normal routes — but webhook providers (Stripe, GitHub, Svix, …) sign the payload _exactly as delivered_, and an HMAC computed over a re-serialized `ctx.input` will never match (whitespace and key order don't survive a parse/stringify round-trip). | ||
|
|
||
| Routes that need the unparsed body opt in with `rawBody: true`: | ||
|
|
||
| ```typescript | ||
| routes: { | ||
| "webhooks/provider": { | ||
| public: true, | ||
| rawBody: true, | ||
| handler: async (ctx) => { | ||
| const signature = ctx.request.headers.get("X-Signature") ?? ""; | ||
| await verifyHmac(ctx.rawBody ?? "", signature, secret); // throw on mismatch | ||
| const event = JSON.parse(ctx.rawBody ?? "{}"); | ||
| // ... | ||
| }, | ||
| }, | ||
| }, | ||
| ``` | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion] Same wording issue as line 155: "exactly as received" implies the original byte stream, while
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Already addressed — the comment and both doc sections now describe |
||
| `ctx.rawBody` is the delivered body as a UTF-8 decoded string — webhook payloads are UTF-8 text in practice, so signature libraries that accept a string (or `new TextEncoder().encode(ctx.rawBody)`) work directly; binary bodies are not preserved byte-exactly. `ctx.input` still works as usual (parsed from the same buffer). Non-JSON payloads — e.g. form-encoded webhook deliveries — arrive with `ctx.input` undefined but `ctx.rawBody` intact, so the handler can parse them itself. The flag is native-format only; sandboxed plugins receive requests across a serialization boundary and don't support it yet. | ||
|
|
||
| ## Plugin id rules | ||
|
|
||
| The `id` field must match `/^[a-z][a-z0-9_-]*$/` — start with a lowercase letter, then letters, digits, hyphens, or underscores. The id is used as a single path segment in plugin route URLs and as part of generated SQL identifiers for plugin storage indexes, so anything outside that pattern fails at runtime. The following values show which ids are accepted: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3342,14 +3342,20 @@ export class EmDashRuntime { | |
|
|
||
| const routeKey = path.replace(LEADING_SLASH_PATTERN, ""); | ||
|
|
||
| // Buffer the body as text so routes with `rawBody: true` can see the | ||
| // payload exactly as delivered — as a UTF-8 decoded string, which is | ||
| // what webhook signature verification needs in practice; 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(); | ||
|
Comment on lines
3344
to
+3352
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion] The new unit tests in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done — added |
||
| if (rawBody) body = JSON.parse(rawBody); | ||
| } catch { | ||
| // No body or not JSON | ||
| // No body or not JSON — rawBody (when read) is still passed through | ||
| } | ||
|
|
||
| return routeRegistry.invoke(pluginId, routeKey, { request, body }); | ||
| return routeRegistry.invoke(pluginId, routeKey, { request, body, rawBody }); | ||
| } | ||
|
|
||
| // Check sandboxed (marketplace) plugins second | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| /** | ||
| * Trusted plugin routes with `rawBody: true` must receive the delivered | ||
| * body as a UTF-8 string on `ctx.rawBody`, alongside the parsed `ctx.input`. | ||
| * | ||
| * The dispatcher reads `request.text()` once, parses JSON from the same | ||
| * buffer into `ctx.input`, and only surfaces `rawBody` to opted-in routes — | ||
| * the behavioral core of the raw-body feature that unit tests invoking | ||
| * `PluginRouteHandler.invoke` directly do not exercise. | ||
| */ | ||
|
|
||
| import { randomUUID } from "node:crypto"; | ||
|
|
||
| import Database from "better-sqlite3"; | ||
| import { SqliteDialect } from "kysely"; | ||
| import { describe, expect, it } from "vitest"; | ||
|
|
||
| import { EmDashRuntime } from "../../../src/emdash-runtime.js"; | ||
| import type { RuntimeDependencies } from "../../../src/emdash-runtime.js"; | ||
| import { definePlugin } from "../../../src/plugins/define-plugin.js"; | ||
| import type { Storage } from "../../../src/storage/types.js"; | ||
|
|
||
| const stubStorage: Storage = { | ||
| async upload() { | ||
| throw new Error("storage not used by this test"); | ||
| }, | ||
| async download() { | ||
| throw new Error("storage not used by this test"); | ||
| }, | ||
| async delete() {}, | ||
| async exists() { | ||
| return false; | ||
| }, | ||
| async list() { | ||
| return { items: [] }; | ||
| }, | ||
| async getSignedUploadUrl() { | ||
| throw new Error("storage not used by this test"); | ||
| }, | ||
| getPublicUrl: (key) => `/media/${key}`, | ||
| }; | ||
|
|
||
| interface Captured { | ||
| hasRawBody: boolean; | ||
| rawBody: string | undefined; | ||
| input: unknown; | ||
| } | ||
|
|
||
| function createDeps(captured: Captured, rawBodyOptIn: boolean): RuntimeDependencies { | ||
| const entrypoint = `test-plugin-raw-body-${randomUUID()}`; | ||
| return { | ||
| config: { | ||
| database: { entrypoint, config: {}, type: "sqlite" }, | ||
| storage: { entrypoint, config: {} }, | ||
| }, | ||
| plugins: [ | ||
| definePlugin({ | ||
| id: "webhook-sink", | ||
| version: "1.0.0", | ||
| capabilities: [], | ||
| routes: { | ||
| hook: { | ||
| rawBody: rawBodyOptIn, | ||
| handler: async (ctx) => { | ||
| captured.hasRawBody = "rawBody" in ctx && ctx.rawBody !== undefined; | ||
| captured.rawBody = ctx.rawBody; | ||
| captured.input = ctx.input; | ||
| return { ok: true }; | ||
| }, | ||
| }, | ||
| }, | ||
| }), | ||
| ], | ||
| createDialect: () => new SqliteDialect({ database: new Database(":memory:") }), | ||
| createStorage: () => stubStorage, | ||
| sandboxEnabled: false, | ||
| sandboxedPluginEntries: [], | ||
| createSandboxRunner: null, | ||
| }; | ||
| } | ||
|
|
||
| function post(json: string): Request { | ||
| return new Request("http://test.local/_emdash/api/plugin/webhook-sink/hook", { | ||
| method: "POST", | ||
| headers: { "content-type": "application/json" }, | ||
| body: json, | ||
| }); | ||
| } | ||
|
|
||
| describe("EmDashRuntime.handlePluginApiRoute — rawBody", () => { | ||
| it("populates ctx.rawBody with the exact delivered text and ctx.input with parsed JSON", async () => { | ||
| const captured: Captured = { hasRawBody: false, rawBody: undefined, input: undefined }; | ||
| const runtime = await EmDashRuntime.create(createDeps(captured, true)); | ||
| try { | ||
| // Whitespace and key order must survive in rawBody even though | ||
| // ctx.input is the parsed equivalent. | ||
| const payload = '{ "b": 1,\n "a": "x" }'; | ||
| const result = await runtime.handlePluginApiRoute( | ||
| "webhook-sink", | ||
| "POST", | ||
| "/hook", | ||
| post(payload), | ||
| ); | ||
|
|
||
| expect(result.success).toBe(true); | ||
| expect(captured.hasRawBody).toBe(true); | ||
| expect(captured.rawBody).toBe(payload); | ||
| expect(captured.input).toEqual({ a: "x", b: 1 }); | ||
| } finally { | ||
| await runtime.stopCron(); | ||
| } | ||
| }); | ||
|
|
||
| it("leaves ctx.input undefined for non-JSON payloads but still exposes rawBody", async () => { | ||
| const captured: Captured = { hasRawBody: false, rawBody: undefined, input: undefined }; | ||
| const runtime = await EmDashRuntime.create(createDeps(captured, true)); | ||
| try { | ||
| const result = await runtime.handlePluginApiRoute( | ||
| "webhook-sink", | ||
| "POST", | ||
| "/hook", | ||
| post("not json at all"), | ||
| ); | ||
|
|
||
| expect(result.success).toBe(true); | ||
| expect(captured.rawBody).toBe("not json at all"); | ||
| expect(captured.input).toBeUndefined(); | ||
| } finally { | ||
| await runtime.stopCron(); | ||
| } | ||
| }); | ||
|
|
||
| it("does not populate ctx.rawBody for routes that did not opt in", async () => { | ||
| const captured: Captured = { hasRawBody: false, rawBody: undefined, input: undefined }; | ||
| const runtime = await EmDashRuntime.create(createDeps(captured, false)); | ||
| try { | ||
| const result = await runtime.handlePluginApiRoute( | ||
| "webhook-sink", | ||
| "POST", | ||
| "/hook", | ||
| post('{"a":1}'), | ||
| ); | ||
|
|
||
| expect(result.success).toBe(true); | ||
| expect(captured.hasRawBody).toBe(false); | ||
| expect(captured.input).toEqual({ a: 1 }); | ||
| } finally { | ||
| await runtime.stopCron(); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[suggestion] This says webhook providers sign the "exact raw bytes", but
ctx.rawBodyis 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 thatctx.rawBodyis the UTF-8 decoded body.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already addressed — the comment and both doc sections now describe
ctx.rawBodyas the body "as a UTF-8 decoded string" and explicitly note that binary/non-UTF8 bodies are not preserved byte-exactly (with theTextEncoderre-encode path for signature libraries).