Revert #230 (constant-time signature swap) to restore live webhook delivery

worker/src/signature.ts

@@ -1,14 +1,10 @@
 /**
- * GitHub webhook signature verification (#227, hardened in #229).
+ * GitHub webhook signature verification (#227).
  *
- * Validates GitHub's `X-Hub-Signature-256` header — an HMAC-SHA256 digest of the
- * raw request body keyed with the shared webhook secret — using Web Crypto's
- * `crypto.subtle.verify`, which recomputes the HMAC and compares it in CONSTANT
- * TIME. The previous implementation compared the recomputed digest with the
- * supplied signature via `===`, which early-exits on the first mismatched
- * character and leaks the expected signature through response timing — a timing
- * side-channel at the webhook trust boundary. The functional contract is
- * unchanged: a correct signature returns true; any mismatch (tampered body,
+ * Verifies GitHub's `X-Hub-Signature-256` header — an HMAC-SHA256 digest of the
+ * raw request body keyed with the shared webhook secret — using Web Crypto
+ * (`crypto.subtle`). Returns true only when the recomputed `sha256=<hex>` digest
+ * exactly matches the supplied signature string; any mismatch (tampered body,
  * wrong secret, malformed signature) returns false. This is the webhook trust
  * boundary: it proves both authentic GitHub origin and body integrity.
  */
@@ -17,26 +13,17 @@ export async function verifyGitHubSignature(
   body: string,
   signature: string,
 ): Promise<boolean> {
-  const prefix = "sha256=";
-  if (!signature.startsWith(prefix)) return false;
-  const hex = signature.slice(prefix.length);
-  // An HMAC-SHA256 digest is 32 bytes = exactly 64 hex chars. Reject any other
-  // shape before touching crypto so malformed input fails fast and uniformly.
-  if (!/^[0-9a-fA-F]{64}$/.test(hex)) return false;
-
-  const sigBytes = new Uint8Array(32);
-  for (let i = 0; i < 32; i++) {
-    sigBytes[i] = parseInt(hex.slice(i * 2, i * 2 + 2), 16);
-  }
-
   const encoder = new TextEncoder();
   const key = await crypto.subtle.importKey(
     "raw",
     encoder.encode(secret),
     { name: "HMAC", hash: "SHA-256" },
     false,
-    ["verify"],
+    ["sign"],
   );
-  // verify() recomputes the HMAC and compares it in constant time.
-  return crypto.subtle.verify("HMAC", key, sigBytes, encoder.encode(body));
+  const sig = await crypto.subtle.sign("HMAC", key, encoder.encode(body));
+  const expected = "sha256=" + Array.from(new Uint8Array(sig))
+    .map((b) => b.toString(16).padStart(2, "0"))
+    .join("");
+  return expected === signature;
 }

worker/test/signature.test.ts

@@ -1,76 +1,64 @@
 /**
- * Unit tests for worker/src/signature.ts (#227, constant-time switch in #229).
+ * Unit tests for worker/src/signature.ts (#227).
  *
  * verifyGitHubSignature is the webhook trust boundary: it must accept a body
- * signed with the matching secret and reject every other case. The primary
- * oracle is GitHub's OWN published example vector from "Validating webhook
- * deliveries" (secret / body / signature below), which cross-checks the impl
- * against GitHub's real contract independently of any code under test. An
- * independent node:crypto createHmac oracle is also kept for additional
- * generated cases.
- *
- * #229 switched the internal compare from a non-constant-time `===` (which
- * early-exits and leaks the expected signature through timing) to
- * crypto.subtle.verify (constant-time internal compare). The timing property
- * itself is not unit-observable; these tests pin the FUNCTIONAL contract that
- * the constant-time implementation preserves. crypto.subtle exists in Node 20+,
- * so this runs as a pure tsx --test unit test (no Miniflare).
+ * signed with the matching secret and reject every other case. The expected
+ * valid signature is generated INDEPENDENTLY via Node's node:crypto
+ * (createHmac → "sha256=" + hex digest), giving an oracle that does not call
+ * the function under test. crypto.subtle exists in Node 20+, so this runs as a
+ * pure tsx --test unit test (no Miniflare).
  */
 import { test } from "node:test";
 import assert from "node:assert/strict";
 import { createHmac } from "node:crypto";
 
 import { verifyGitHubSignature } from "../src/signature.js";
 
-// GitHub's published example vector ("Validating webhook deliveries" docs).
-const SECRET = "It's a Secret to Everybody";
-const BODY = "Hello, World!";
-const VALID =
-  "sha256=757107ea0eb2509fc211221cce984b8a37570b6d7586c22c46f4379c8b043e17";
+const SECRET = "s3cr3t-webhook-key";
+const BODY = JSON.stringify({ action: "opened", number: 42 });
 
 /** Independent oracle: GitHub's X-Hub-Signature-256 = "sha256=" + HMAC-SHA256 hex. */
 function sign(secret: string, body: string): string {
   return "sha256=" + createHmac("sha256", secret).update(body).digest("hex");
 }
 
-// ── Valid case (GitHub's published example vector) ───────────────────
+// ── Valid case ───────────────────────────────────────────────────────
 
-test("accepts GitHub's published example vector", async () => {
-  assert.equal(await verifyGitHubSignature(SECRET, BODY, VALID), true);
+test("accepts a signature generated with the same secret and body", async () => {
+  const signature = sign(SECRET, BODY);
+  assert.equal(await verifyGitHubSignature(SECRET, BODY, signature), true);
 });
 
 // ── Tampered body ────────────────────────────────────────────────────
 
 test("rejects when the body was tampered after signing", async () => {
-  // Verify a different body against the signature for the original body.
-  assert.equal(await verifyGitHubSignature(SECRET, "Hello, World?", VALID), false);
+  // Sign body A, verify against body B.
+  const signature = sign(SECRET, BODY);
+  const tamperedBody = JSON.stringify({ action: "opened", number: 9999 });
+  assert.equal(await verifyGitHubSignature(SECRET, tamperedBody, signature), false);
 });
 
 // ── Wrong secret ─────────────────────────────────────────────────────
 
 test("rejects when verified with a different secret", async () => {
-  assert.equal(await verifyGitHubSignature("wrong-secret", BODY, VALID), false);
+  const signature = sign(SECRET, BODY);
+  assert.equal(await verifyGitHubSignature("wrong-secret", BODY, signature), false);
 });
 
 // ── Malformed signature formats ──────────────────────────────────────
 
-test("rejects a signature without the sha256= prefix", async () => {
-  // Bare hex, no "sha256=" prefix.
-  const bareHex = createHmac("sha256", SECRET).update(BODY).digest("hex");
-  assert.equal(await verifyGitHubSignature(SECRET, BODY, bareHex), false);
-});
-
 test("rejects an empty signature string", async () => {
   assert.equal(await verifyGitHubSignature(SECRET, BODY, ""), false);
 });
 
-test("rejects a sha256=-prefixed hex of the wrong length", async () => {
-  assert.equal(await verifyGitHubSignature(SECRET, BODY, "sha256=deadbeef"), false);
+test("rejects a signature without the sha256= prefix", async () => {
+  // Same hex digest but missing the "sha256=" prefix.
+  const hexOnly = createHmac("sha256", SECRET).update(BODY).digest("hex");
+  assert.equal(await verifyGitHubSignature(SECRET, BODY, hexOnly), false);
 });
 
-test("rejects a sha256=-prefixed non-hex string of the right length", async () => {
-  assert.equal(
-    await verifyGitHubSignature(SECRET, BODY, "sha256=" + "z".repeat(64)),
-    false,
-  );
+test("rejects a sha256=-prefixed hex of the wrong length", async () => {
+  // Truncated digest (valid prefix, too few hex chars).
+  const shortHex = createHmac("sha256", SECRET).update(BODY).digest("hex").slice(0, 32);
+  assert.equal(await verifyGitHubSignature(SECRET, BODY, "sha256=" + shortHex), false);
 });