fix: verify raw bytes, decode Standard Webhooks secrets, cap body size (#21)

Reject an empty bearer secret. Unprefixed Standard Webhooks secrets
still verify as raw bytes, as 0.1 read them. Refs CL-9451.

Author: TheGreatAxios

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
 

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", {

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<LiveRun[]>;
 
+/** 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) {

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<ArrayBuffer>,
+  id: string,
+  timestamp: string,
+  body: Uint8Array,
+): Promise<Headers> {
+  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,14 +149,16 @@ 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 () => {
     const headers = new Headers({
       "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,
+    );
   });
 });

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<ArrayBuffer>,
+  prefix: string,
+  body: Uint8Array,
+): Promise<Buffer> {
+  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<ArrayBuffer>[] {
+  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<boolean> {
   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<boolean> {
   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);
 }