From 00cd432e9416cc5acb0642f1cda35600300fe57b Mon Sep 17 00:00:00 2001
From: Sawyer Cutler <sawyer@dirtroad.dev>
Date: Sat, 26 Sep 2026 19:46:31 -0700
Subject: [PATCH] fix(verify): reject a zero-length signing key instead of
 throwing

An empty standard-webhooks or Slack secret made WebCrypto throw on a zero-length HMAC key, so the hook answered 500. It now fails verification with 401. The README says an unprefixed base64 secret is tried decoded first, then raw, and that a crash between claiming a replay key and forwarding holds the key until it expires.

Closes CL-9457
---
 README.md          |  4 ++--
 src/verify.test.ts | 23 +++++++++++++++++++++++
 src/verify.ts      |  6 ++++--
 3 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/README.md b/README.md
index 79043e3..989a811 100644
--- a/README.md
+++ b/README.md
@@ -47,9 +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`, 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.
+With `standard-webhooks`, the secret is base64-decoded after stripping an optional `whsec_` prefix, as the spec requires. An unprefixed secret that is valid base64 is tried decoded first, then as raw bytes, which is how 0.1 read it; one that is not base64 is only tried raw. An empty secret never verifies, under any scheme.
 
-A replayed delivery gets `409`. Standard Webhooks deliveries are keyed on the credential and `webhook-id`, Slack on the credential and signature, each kept until its timestamp leaves the ±300s window. The seen-set is the `replay` table in the `webhooks` schema, created by `runWebhookMigrations` (see [Using with Interchange](#using-with-interchange)), so every replica sharing the database rejects the replay. A failed delivery releases its key so the sender's retry goes through. Bearer requests carry no id and are not deduplicated.
+A replayed delivery gets `409`. Standard Webhooks deliveries are keyed on the credential and `webhook-id`, Slack on the credential and signature, each kept until its timestamp leaves the ±300s window. The seen-set is the `replay` table in the `webhooks` schema, created by `runWebhookMigrations` (see [Using with Interchange](#using-with-interchange)), so every replica sharing the database rejects the replay. A failed delivery releases its key so the sender's retry goes through. If the hub crashes between claiming a key and forwarding the delivery, the key stays held until it expires with its timestamp window, so the sender's retries get `409` until then. Bearer requests carry no id and are not deduplicated.
 
 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.
 
diff --git a/src/verify.test.ts b/src/verify.test.ts
index 5686bd3..13545ed 100644
--- a/src/verify.test.ts
+++ b/src/verify.test.ts
@@ -104,6 +104,29 @@ describe("verifyStandardWebhooks", () => {
   });
 });
 
+describe("zero-length signing keys", () => {
+  test.each(["", "whsec_"])(
+    "standard-webhooks rejects the empty secret %p instead of throwing",
+    async (secret) => {
+      const body = bytes("{}");
+      const headers = new Headers({
+        "webhook-id": "evt_1",
+        "webhook-timestamp": now(),
+        "webhook-signature": "v1,AAAA",
+      });
+      expect(await verifyStandardWebhooks(secret, headers, body)).toBe(false);
+    },
+  );
+
+  test("slack rejects an empty secret instead of throwing", async () => {
+    const headers = new Headers({
+      "x-slack-request-timestamp": now(),
+      "x-slack-signature": "v0=00",
+    });
+    expect(await verifySlack("", headers, bytes("{}"))).toBe(false);
+  });
+});
+
 describe("verifyBearer", () => {
   test("accepts Authorization Bearer", () => {
     const headers = new Headers({ authorization: "Bearer s3cret" });
diff --git a/src/verify.ts b/src/verify.ts
index 6d3e33e..f8ea7a1 100644
--- a/src/verify.ts
+++ b/src/verify.ts
@@ -65,7 +65,9 @@ export async function verifyStandardWebhooks(
     const [ver, val] = part.split(",", 2);
     return ver === "v1" && val !== undefined && val !== "" ? [val] : [];
   });
-  for (const key of standardWebhooksKeys(secret)) {
+  // A zero-length HMAC key is unusable (WebCrypto throws), so it never matches.
+  const keys = standardWebhooksKeys(secret).filter((key) => key.length > 0);
+  for (const key of keys) {
     const mac = await hmacSha256(key, `${id}.${timestamp}.`, body);
     const expected = mac.toString("base64");
     if (candidates.some((c) => timingEqual(c, expected))) return true;
@@ -90,7 +92,7 @@ export async function verifySlack(
 ): Promise<boolean> {
   const timestamp = headers.get("x-slack-request-timestamp");
   const signature = headers.get("x-slack-signature");
-  if (!timestamp || !signature) return false;
+  if (secret === "" || !timestamp || !signature) return false;
   const ts = Number(timestamp);
   if (
     !Number.isFinite(ts) ||