diff --git a/README.md b/README.md index 0f3b2f1..8cca06d 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,9 @@ Responses: `202` delivered, `200 { challenge }` for Slack `url_verification`, `4 With neither set, the credential name is matched against definition and asset names, then the tenant's only live run. A live run has status `deployed` or `running`. -With `standard-webhooks`, a `whsec_…` secret is base64-decoded before use as the HMAC key; any other secret is used as-is. Bot tokens for chat integrations belong in the workflow's `credentialBindings`, not in the signing secret. +With `standard-webhooks`, the secret is base64-decoded after stripping an optional `whsec_` prefix, as the spec requires. An unprefixed secret also verifies when the sender used it as raw bytes, which is how 0.1 read it. `bearer` rejects an empty secret. + +Signatures are checked over the raw request bytes, and the body is forwarded unchanged. Bodies over 1 MiB get `413`; a verified body that is not UTF-8 gets `415`. Bot tokens for chat integrations belong in the workflow's `credentialBindings`, not in the signing secret. ### Exports diff --git a/src/hooks.test.ts b/src/hooks.test.ts index b2b738d..bdd61c8 100644 --- a/src/hooks.test.ts +++ b/src/hooks.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "bun:test"; import { Hono } from "hono"; -import { createHookApp } from "./hooks.js"; +import { createHookApp, MAX_BODY_BYTES } from "./hooks.js"; import type { LoadedHook, LiveRun } from "./resolve.js"; const JIMMY: LiveRun = { @@ -209,6 +209,49 @@ describe("createHookApp", () => { expect(res.status).toBe(503); }); + test("413s a body over the size limit before verifying", async () => { + const { app, delivered } = mount({ + loaded: hook({ meta: { verify: "bearer", to: JIMMY.address } }), + runs: [JIMMY], + }); + const res = await app.request("/api/hooks/slack", { + method: "POST", + body: "x".repeat(MAX_BODY_BYTES + 1), + headers: { authorization: "Bearer s3cret" }, + }); + expect(res.status).toBe(413); + expect(delivered).toEqual([]); + }); + + test("forwards the verified body unchanged", async () => { + const { app, delivered } = mount({ + loaded: hook({ meta: { verify: "bearer", to: JIMMY.address } }), + runs: [JIMMY], + }); + const body = ' {"n":"é"}\r\n'; + const res = await app.request("/api/hooks/slack", { + method: "POST", + body: new TextEncoder().encode(body), + headers: { authorization: "Bearer s3cret" }, + }); + expect(res.status).toBe(202); + expect(delivered[0]?.content).toBe(body); + }); + + test("415s a verified body that is not UTF-8", async () => { + const { app, delivered } = mount({ + loaded: hook({ meta: { verify: "bearer", to: JIMMY.address } }), + runs: [JIMMY], + }); + const res = await app.request("/api/hooks/slack", { + method: "POST", + body: new Uint8Array([0x7b, 0xe9, 0x7d]), + headers: { authorization: "Bearer s3cret" }, + }); + expect(res.status).toBe(415); + expect(delivered).toEqual([]); + }); + test("404 when the hook is ambiguous", async () => { const { app } = mount({ loaded: "ambiguous" }); const res = await app.request("/api/hooks/slack", { diff --git a/src/hooks.ts b/src/hooks.ts index 44a1995..041a7ca 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -1,4 +1,5 @@ import { Hono, type Context } from "hono"; +import { bodyLimit } from "hono/body-limit"; import type { MailDeliverer } from "./deliver.js"; import type { LoadedHook, LiveRun } from "./resolve.js"; @@ -12,12 +13,21 @@ export type LoadHook = ( export type ListRuns = (tenantId: string) => Promise; +/** Largest request body accepted, checked before anything is hashed. */ +export const MAX_BODY_BYTES = 1024 * 1024; + export function createHookApp(opts: { deliver: MailDeliverer; loadHook: LoadHook; listRuns: ListRuns; }): Hono { const app = new Hono(); + app.use( + bodyLimit({ + maxSize: MAX_BODY_BYTES, + onError: (c) => c.json({ error: "payload_too_large" }, 413), + }), + ); app.post("/", (c) => handle(c, opts)); app.post("/:id", (c) => handle(c, opts)); app.post("/:tenantId/:name", (c) => handle(c, opts)); @@ -59,17 +69,28 @@ async function handle( return c.json({ error: "unknown_hook" }, 404); } - const body = await c.req.text(); + const raw = new Uint8Array(await c.req.arrayBuffer()); let ok = false; if (loaded.meta.verify === "bearer") { ok = verifyBearer(loaded.secret, c.req.raw.headers); } else if (loaded.meta.verify === "standard-webhooks") { - ok = await verifyStandardWebhooks(loaded.secret, c.req.raw.headers, body); + ok = await verifyStandardWebhooks(loaded.secret, c.req.raw.headers, raw); } else if (loaded.meta.verify === "slack") { - ok = await verifySlack(loaded.secret, c.req.raw.headers, body); + ok = await verifySlack(loaded.secret, c.req.raw.headers, raw); } if (!ok) return c.json({ error: "unauthorized" }, 401); + // Trigger mail carries text, so a body that is not UTF-8 cannot be forwarded + // unchanged; refuse it rather than substitute replacement characters. + let body: string; + try { + body = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }).decode( + raw, + ); + } catch { + return c.json({ error: "unsupported_body" }, 415); + } + if (loaded.meta.verify === "slack") { const challenge = slackChallenge(body); if (challenge !== undefined) { diff --git a/src/verify.test.ts b/src/verify.test.ts index 9e4de04..5686bd3 100644 --- a/src/verify.test.ts +++ b/src/verify.test.ts @@ -2,6 +2,35 @@ import { describe, expect, test } from "bun:test"; import { verifyBearer, verifySlack, verifyStandardWebhooks } from "./verify.js"; +const bytes = (text: string) => new TextEncoder().encode(text); + +async function swSign( + key: Uint8Array, + id: string, + timestamp: string, + body: Uint8Array, +): Promise { + const hmac = await crypto.subtle.importKey( + "raw", + key, + { name: "HMAC", hash: "SHA-256" }, + false, + ["sign"], + ); + const mac = await crypto.subtle.sign( + "HMAC", + hmac, + Buffer.concat([bytes(`${id}.${timestamp}.`), body]), + ); + return new Headers({ + "webhook-id": id, + "webhook-timestamp": timestamp, + "webhook-signature": `v1,${Buffer.from(mac).toString("base64")}`, + }); +} + +const now = () => String(Math.floor(Date.now() / 1000)); + describe("verifyStandardWebhooks", () => { test("accepts a valid v1 signature", async () => { const secret = `whsec_${Buffer.from("supersecret").toString("base64")}`; @@ -25,18 +54,53 @@ describe("verifyStandardWebhooks", () => { "webhook-timestamp": timestamp, "webhook-signature": `v1,${Buffer.from(mac).toString("base64")}`, }); + expect(await verifyStandardWebhooks(secret, headers, bytes(body))).toBe( + true, + ); + }); + + test("verifies over raw bytes that are not valid UTF-8", async () => { + const key = bytes("supersecret"); + const body = new Uint8Array([0x7b, 0x22, 0xe9, 0x22, 0x7d]); + const headers = await swSign(key, "evt_1", now(), body); + const secret = `whsec_${Buffer.from(key).toString("base64")}`; expect(await verifyStandardWebhooks(secret, headers, body)).toBe(true); }); + test("base64-decodes a secret without the whsec_ prefix", async () => { + const key = bytes("supersecret"); + const headers = await swSign(key, "evt_1", now(), bytes("{}")); + const secret = Buffer.from(key).toString("base64"); + expect(await verifyStandardWebhooks(secret, headers, bytes("{}"))).toBe( + true, + ); + }); + + test("still accepts an unprefixed secret used as raw bytes", async () => { + const secret = "plain-secret"; + const headers = await swSign(bytes(secret), "evt_1", now(), bytes("{}")); + expect(await verifyStandardWebhooks(secret, headers, bytes("{}"))).toBe( + true, + ); + }); + + test("does not use a whsec_ secret as raw bytes", async () => { + const secret = `whsec_${Buffer.from("supersecret").toString("base64")}`; + const headers = await swSign(bytes(secret), "evt_1", now(), bytes("{}")); + expect(await verifyStandardWebhooks(secret, headers, bytes("{}"))).toBe( + false, + ); + }); + test("rejects a bad signature", async () => { const headers = new Headers({ "webhook-id": "evt_1", "webhook-timestamp": String(Math.floor(Date.now() / 1000)), "webhook-signature": "v1,nope", }); - expect(await verifyStandardWebhooks("whsec_xxxx", headers, "{}")).toBe( - false, - ); + expect( + await verifyStandardWebhooks("whsec_xxxx", headers, bytes("{}")), + ).toBe(false); }); }); @@ -51,6 +115,13 @@ describe("verifyBearer", () => { expect(verifyBearer("s3cret", headers)).toBe(true); }); + test.each([{ authorization: "Bearer " }, { "x-webhook-secret": "" }])( + "rejects an empty secret (%o)", + (init) => { + expect(verifyBearer("", new Headers(init))).toBe(false); + }, + ); + test("rejects a mismatch", () => { const headers = new Headers({ authorization: "Bearer nope" }); expect(verifyBearer("s3cret", headers)).toBe(false); @@ -78,7 +149,7 @@ describe("verifySlack", () => { "x-slack-request-timestamp": timestamp, "x-slack-signature": `v0=${Buffer.from(mac).toString("hex")}`, }); - expect(await verifySlack(secret, headers, body)).toBe(true); + expect(await verifySlack(secret, headers, bytes(body))).toBe(true); }); test("rejects a bad signature", async () => { @@ -86,6 +157,8 @@ describe("verifySlack", () => { "x-slack-request-timestamp": String(Math.floor(Date.now() / 1000)), "x-slack-signature": "v0=00", }); - expect(await verifySlack("signing-secret", headers, "{}")).toBe(false); + expect(await verifySlack("signing-secret", headers, bytes("{}"))).toBe( + false, + ); }); }); diff --git a/src/verify.ts b/src/verify.ts index 01d7b66..ff6fe5e 100644 --- a/src/verify.ts +++ b/src/verify.ts @@ -9,10 +9,43 @@ export function timingEqual(a: string, b: string): boolean { return crypto.timingSafeEqual(left, right); } +async function hmacSha256( + key: Uint8Array, + prefix: string, + body: Uint8Array, +): Promise { + const hmac = await crypto.subtle.importKey( + "raw", + key, + { name: "HMAC", hash: "SHA-256" }, + false, + ["sign"], + ); + const message = Buffer.concat([new TextEncoder().encode(prefix), body]); + return Buffer.from(await crypto.subtle.sign("HMAC", hmac, message)); +} + +const BASE64 = + /^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$/; + +/** + * Standard Webhooks keys are base64, optionally behind `whsec_`. Before 0.2 an + * unprefixed secret was used as raw bytes, so those still verify under that + * key too. + */ +function standardWebhooksKeys(secret: string): Uint8Array[] { + if (secret.startsWith("whsec_")) { + const encoded = secret.slice("whsec_".length); + return BASE64.test(encoded) ? [Buffer.from(encoded, "base64")] : []; + } + const raw = new TextEncoder().encode(secret); + return BASE64.test(secret) ? [Buffer.from(secret, "base64"), raw] : [raw]; +} + export async function verifyStandardWebhooks( secret: string, headers: Headers, - body: string, + body: Uint8Array, ): Promise { const id = headers.get("webhook-id"); const timestamp = headers.get("webhook-timestamp"); @@ -22,30 +55,20 @@ export async function verifyStandardWebhooks( if (!Number.isFinite(ts) || Math.abs(Date.now() / 1000 - ts) > 300) { return false; } - const keyBytes = secret.startsWith("whsec_") - ? Buffer.from(secret.slice("whsec_".length), "base64") - : Buffer.from(secret); - const key = await crypto.subtle.importKey( - "raw", - keyBytes, - { name: "HMAC", hash: "SHA-256" }, - false, - ["sign"], - ); - const mac = await crypto.subtle.sign( - "HMAC", - key, - new TextEncoder().encode(`${id}.${timestamp}.${body}`), - ); - const expected = Buffer.from(mac).toString("base64"); const candidates = signature.split(/\s+/).flatMap((part) => { const [ver, val] = part.split(",", 2); return ver === "v1" && val !== undefined && val !== "" ? [val] : []; }); - return candidates.some((c) => timingEqual(c, expected)); + for (const key of standardWebhooksKeys(secret)) { + const mac = await hmacSha256(key, `${id}.${timestamp}.`, body); + const expected = mac.toString("base64"); + if (candidates.some((c) => timingEqual(c, expected))) return true; + } + return false; } export function verifyBearer(secret: string, headers: Headers): boolean { + if (secret === "") return false; const auth = headers.get("authorization"); if (auth?.startsWith("Bearer ")) { return timingEqual(auth.slice("Bearer ".length), secret); @@ -57,7 +80,7 @@ export function verifyBearer(secret: string, headers: Headers): boolean { export async function verifySlack( secret: string, headers: Headers, - body: string, + body: Uint8Array, ): Promise { const timestamp = headers.get("x-slack-request-timestamp"); const signature = headers.get("x-slack-signature"); @@ -66,18 +89,11 @@ export async function verifySlack( if (!Number.isFinite(ts) || Math.abs(Date.now() / 1000 - ts) > 300) { return false; } - const key = await crypto.subtle.importKey( - "raw", + const mac = await hmacSha256( new TextEncoder().encode(secret), - { name: "HMAC", hash: "SHA-256" }, - false, - ["sign"], - ); - const mac = await crypto.subtle.sign( - "HMAC", - key, - new TextEncoder().encode(`v0:${timestamp}:${body}`), + `v0:${timestamp}:`, + body, ); - const expected = `v0=${Buffer.from(mac).toString("hex")}`; + const expected = `v0=${mac.toString("hex")}`; return timingEqual(signature, expected); }