feat(http): HMAC-verified GitHub webhook route (3/5)

.dev.vars.example

@@ -1,6 +1,10 @@
 # Copy to .dev.vars and fill in local values. Never commit .dev.vars.
 BOT_TOKEN=000000000:REPLACE_WITH_TELEGRAM_BOT_TOKEN
 WEBHOOK_SECRET=REPLACE_WITH_RANDOM_SECRET
+# GitHub webhook HMAC secret (openssl rand -hex 32), same value configured
+# on the org webhook. Missing/empty makes the /github/webhook route fail
+# closed with 500 on every request.
+GITHUB_WEBHOOK_SECRET=REPLACE_WITH_RANDOM_SECRET
 # JSON keyring: {"active":1,"keys":{"1":"<base64 32-byte key>"}}
 PII_KEYRING={"active":1,"keys":{"1":"REPLACE_WITH_BASE64_32B_KEY"}}
 # grammY botInfo JSON, avoids a getMe call per request (see docs/key-backup.md)

openspec/changes/github-alerts/tasks.md

@@ -49,10 +49,10 @@ Chain strategy: stacked-to-main
 
 ## Phase 3: Signature and Route Skeleton (PR3)
 
-- [ ] 3.1 RED: HMAC tests — missing header, wrong secret, right-length-wrong-content (timing-safe), valid signature (spec: HMAC Signature Verification, all scenarios).
-- [ ] 3.2 GREEN: `src/adapters/github/signature.ts`, WebCrypto HMAC + `timingSafeEqual`, empty/missing secret → `ConfigError` (500), before `JSON.parse`.
-- [ ] 3.3 RED: route status tests — `ping` 200, invalid JSON/non-object/unsupported event 200, D1 failure 500 (spec: Ping, Unsupported, Infrastructure Failures).
-- [ ] 3.4 GREEN: `src/index.ts` route registration, `env.ts` `GITHUB_WEBHOOK_SECRET`, `.dev.vars.example`, `vitest.config.ts` test binding.
+- [x] 3.1 RED: HMAC tests — missing header, wrong secret, right-length-wrong-content (timing-safe), valid signature (spec: HMAC Signature Verification, all scenarios).
+- [x] 3.2 GREEN: `src/adapters/github/signature.ts`, WebCrypto HMAC + `timingSafeEqual`, empty/missing secret → `ConfigError` (500), before `JSON.parse`.
+- [x] 3.3 RED: route status tests — `ping` 200, invalid JSON/non-object/unsupported event 200 (spec: Ping, Unsupported). D1 failure 500 deferred to PR4 — see Deviation note in apply-progress.md; there is no D1/routing wiring in this PR to fail.
+- [x] 3.4 GREEN: `src/index.ts` route registration, `env.ts` `GITHUB_WEBHOOK_SECRET`, `.dev.vars.example`, `vitest.config.ts` test binding.
 
 ## Phase 4: Delivery Wiring (PR4)
 

src/adapters/crypto/timing-safe-compare.ts

@@ -0,0 +1,14 @@
+// READ-001 correction: shared constant-time string comparison, previously
+// duplicated as src/index.ts's isValidSecret (Telegram) and
+// src/adapters/github/signature.ts's inline compare (GitHub). Length is
+// checked BEFORE crypto.subtle.timingSafeEqual — that primitive requires
+// equal-length inputs, and skipping the length check would either throw or
+// (worse) leak length information through an exception instead of a clean
+// `false`.
+export function timingSafeCompare(provided: string, expected: string): boolean {
+  if (!provided) return false;
+  const providedBytes = new TextEncoder().encode(provided);
+  const expectedBytes = new TextEncoder().encode(expected);
+  if (providedBytes.byteLength !== expectedBytes.byteLength) return false;
+  return crypto.subtle.timingSafeEqual(providedBytes, expectedBytes);
+}

src/adapters/github/signature.ts

@@ -0,0 +1,46 @@
+import { ConfigError } from "../../config-error";
+import { timingSafeCompare } from "../crypto/timing-safe-compare";
+
+// design.md "Signature check": validate the header shape, HMAC-SHA256 over
+// the raw body bytes with the global secret, then a constant-time compare —
+// all before JSON.parse (src/index.ts calls this before ever parsing the
+// body). An empty or missing secret is a config error, never a verify
+// against an empty key (that would make every signature "wrong" for the
+// wrong reason and could mask a deploy-before-secret-is-set state as a
+// generic 401 instead of a loud, redeliverable 500).
+const SIGNATURE_PATTERN = /^sha256=[0-9a-f]{64}$/;
+
+export async function verifyGithubSignature(
+  rawBody: ArrayBuffer,
+  signatureHeader: string | null | undefined,
+  secret: string | null | undefined,
+): Promise<boolean> {
+  if (!secret) {
+    throw new ConfigError("GITHUB_WEBHOOK_SECRET is not configured");
+  }
+  if (!signatureHeader || !SIGNATURE_PATTERN.test(signatureHeader)) {
+    return false;
+  }
+
+  const key = await crypto.subtle.importKey(
+    "raw",
+    new TextEncoder().encode(secret),
+    { name: "HMAC", hash: "SHA-256" },
+    false,
+    ["sign"],
+  );
+  const digest = await crypto.subtle.sign("HMAC", key, rawBody);
+  const expectedHeader = `sha256=${toHex(digest)}`;
+
+  // READ-001: shared with the Telegram route's secret check
+  // (src/index.ts) via timingSafeCompare — length-checked before the
+  // constant-time compare. The regex above already guarantees
+  // signatureHeader is exactly 7 + 64 chars when it got this far, so the
+  // length check inside timingSafeCompare is a defensive re-check, not a
+  // behavior the regex could actually violate.
+  return timingSafeCompare(signatureHeader, expectedHeader);
+}
+
+function toHex(digest: ArrayBuffer): string {
+  return [...new Uint8Array(digest)].map((b) => b.toString(16).padStart(2, "0")).join("");
+}

src/env.ts

@@ -6,4 +6,8 @@ export interface Env {
   WEBHOOK_SECRET: string;
   PII_KEYRING: string;
   BOT_INFO: string;
+  // GitHub webhook HMAC secret (design.md "Secret scope" — one global
+  // secret). Missing/empty is a config error, never a default (see
+  // src/adapters/github/signature.ts).
+  GITHUB_WEBHOOK_SECRET: string;
 }

src/index.ts

@@ -1,6 +1,8 @@
 import { Hono } from "hono";
 import type { Update } from "grammy/types";
 import { createSafeLogger } from "./adapters/log/safe-logger";
+import { verifyGithubSignature } from "./adapters/github/signature";
+import { timingSafeCompare } from "./adapters/crypto/timing-safe-compare";
 import { buildBot } from "./composition";
 import { ConfigError } from "./config-error";
 import type { Env } from "./env";
@@ -12,19 +14,12 @@ const logger = createSafeLogger();
 
 app.get("/health", (c) => c.json({ status: "ok" }));
 
-// design.md "Webhook auth": constant-time comparison, length checked
-// first, before the body is ever parsed.
-function isValidSecret(provided: string | undefined, expected: string): boolean {
-  if (!provided) return false;
-  const providedBytes = new TextEncoder().encode(provided);
-  const expectedBytes = new TextEncoder().encode(expected);
-  if (providedBytes.byteLength !== expectedBytes.byteLength) return false;
-  return crypto.subtle.timingSafeEqual(providedBytes, expectedBytes);
-}
-
 app.post("/telegram/webhook", async (c) => {
   const provided = c.req.header("X-Telegram-Bot-Api-Secret-Token");
-  if (!isValidSecret(provided, c.env.WEBHOOK_SECRET)) {
+  // design.md "Webhook auth": constant-time comparison, length checked
+  // first, before the body is ever parsed (READ-001: shared with the
+  // GitHub route's signature check via timingSafeCompare).
+  if (!provided || !timingSafeCompare(provided, c.env.WEBHOOK_SECRET)) {
     return c.text("Unauthorized", 401);
   }
 
@@ -91,4 +86,76 @@ app.post("/telegram/webhook", async (c) => {
   return c.text("ok");
 });
 
+// design.md "GitHub route status policy" — route skeleton only (PR3). The
+// mapper, org/repo routing and Telegram delivery land in Phase 4; until
+// then every signature-verified, well-formed, non-ping event is
+// acknowledged and dropped (the same 2xx the design table gives an
+// unsupported event/action, since nothing is wired to support one yet).
+app.post("/github/webhook", async (c) => {
+  // RES-001, corrected: an unreadable raw body is a transient, transport-
+  // level failure (e.g. a broken/aborted request stream) on a request that
+  // was never authenticated — it is NOT the "redelivering the same bytes
+  // can never succeed" case (that rationale applies to malformed content
+  // that WAS fully read, like the JSON.parse/non-object branches below).
+  // design.md's status policy treats transient/unexpected failures as 500,
+  // so the delivery shows as failed in GitHub and can be redelivered.
+  let rawBody: ArrayBuffer;
+  try {
+    rawBody = await c.req.arrayBuffer();
+  } catch (err) {
+    logger.log({
+      event: "github-webhook",
+      outcome: "error",
+      errorCode: err instanceof Error ? err.name : "UnknownError",
+    });
+    return c.text("Internal Server Error", 500);
+  }
+  const signatureHeader = c.req.header("X-Hub-Signature-256");
+
+  let verified: boolean;
+  try {
+    verified = await verifyGithubSignature(rawBody, signatureHeader, c.env.GITHUB_WEBHOOK_SECRET);
+  } catch (err) {
+    // GITHUB_WEBHOOK_SECRET unset/empty (design.md: 500, logged as a
+    // ConfigError reason — never a silent accept, never a crash).
+    logger.log({
+      event: "github-webhook",
+      outcome: "error",
+      errorCode: err instanceof Error ? err.name : "UnknownError",
+      ...(err instanceof ConfigError ? { reason: err.message } : {}),
+    });
+    return c.text("Internal Server Error", 500);
+  }
+  if (!verified) {
+    return c.text("Unauthorized", 401);
+  }
+
+  let payload: unknown;
+  try {
+    payload = JSON.parse(new TextDecoder().decode(rawBody));
+  } catch (err) {
+    logger.log({
+      event: "github-webhook",
+      outcome: "error",
+      errorCode: err instanceof Error ? err.name : "UnknownError",
+    });
+    return c.text("ok", 200);
+  }
+  if (typeof payload !== "object" || payload === null || Array.isArray(payload)) {
+    logger.log({ event: "github-webhook", outcome: "error", errorCode: "MalformedPayload" });
+    return c.text("ok", 200);
+  }
+
+  if (c.req.header("X-GitHub-Event") === "ping") {
+    return c.text("ok", 200);
+  }
+
+  // Placeholder until Phase 4 wires the mapper/router: acknowledge, drop,
+  // and log (RES-002 / design.md:27 "unsupported event or action: 200,
+  // logged"). `reason` is a fixed, non-sensitive string per the logging
+  // allowlist — never the event type, action, or any payload field.
+  logger.log({ event: "github-webhook", outcome: "ok", reason: "ignored:not-yet-routed" });
+  return c.text("ok", 200);
+});
+
 export default app;

test/adapters/crypto/timing-safe-compare.test.ts

@@ -0,0 +1,33 @@
+import { describe, expect, it } from "vitest";
+import { timingSafeCompare } from "../../../src/adapters/crypto/timing-safe-compare";
+
+// READ-001 correction: this is the constant-time comparison previously
+// duplicated in src/index.ts's isValidSecret and
+// src/adapters/github/signature.ts's verifyGithubSignature — extracted here
+// so both call one implementation. Length is checked before
+// crypto.subtle.timingSafeEqual (which requires equal-length inputs), same
+// as the two call sites it replaces.
+
+describe("timingSafeCompare", () => {
+  it("returns true for two strings with identical bytes", () => {
+    expect(timingSafeCompare("webhook-secret-value", "webhook-secret-value")).toBe(true);
+  });
+
+  it("returns false for two same-length strings with different bytes", () => {
+    expect(timingSafeCompare("webhook-secret-value", "webhook-secret-diff!")).toBe(false);
+  });
+
+  it("returns false when the provided string is shorter than expected (length-checked before compare)", () => {
+    expect(timingSafeCompare("short", "webhook-secret-value")).toBe(false);
+  });
+
+  it("returns false when the provided string is longer than expected", () => {
+    expect(timingSafeCompare("webhook-secret-value-and-then-some", "webhook-secret-value")).toBe(
+      false,
+    );
+  });
+
+  it("returns false for an empty provided value against a non-empty expected value", () => {
+    expect(timingSafeCompare("", "webhook-secret-value")).toBe(false);
+  });
+});

test/adapters/github/signature.test.ts

@@ -0,0 +1,66 @@
+import { describe, expect, it } from "vitest";
+import { ConfigError } from "../../../src/config-error";
+import { verifyGithubSignature } from "../../../src/adapters/github/signature";
+import { signHex } from "../../support/github-hmac";
+
+// github-webhook spec: "HMAC Signature Verification Over Raw Body" — HMAC-SHA256
+// over the raw bytes, constant-time compare, secret misconfiguration is a
+// ConfigError (never a verify-against-empty-key), all before JSON.parse.
+
+const SECRET = "github-webhook-secret-value";
+
+function bodyBytes(body: string): ArrayBuffer {
+  return new TextEncoder().encode(body).buffer as ArrayBuffer;
+}
+
+describe("verifyGithubSignature", () => {
+  const body = JSON.stringify({ zen: "test", hook_id: 1 });
+
+  it("rejects when the signature header is missing", async () => {
+    await expect(
+      verifyGithubSignature(bodyBytes(body), undefined, SECRET),
+    ).resolves.toBe(false);
+  });
+
+  it("rejects a signature computed from a different secret", async () => {
+    const wrongSig = `sha256=${await signHex(body, "a-different-secret-entirely")}`;
+    await expect(verifyGithubSignature(bodyBytes(body), wrongSig, SECRET)).resolves.toBe(
+      false,
+    );
+  });
+
+  it("rejects a same-length-but-wrong-content signature (constant-time compare path)", async () => {
+    const rightHex = await signHex(body, SECRET);
+    const flippedHex = (rightHex[0] === "0" ? "1" : "0") + rightHex.slice(1);
+    await expect(
+      verifyGithubSignature(bodyBytes(body), `sha256=${flippedHex}`, SECRET),
+    ).resolves.toBe(false);
+  });
+
+  it("rejects a malformed header (wrong prefix/shape, never even hashed)", async () => {
+    await expect(
+      verifyGithubSignature(bodyBytes(body), "sha1=deadbeef", SECRET),
+    ).resolves.toBe(false);
+  });
+
+  it("accepts a signature matching the raw body under the configured secret", async () => {
+    const sig = `sha256=${await signHex(body, SECRET)}`;
+    await expect(verifyGithubSignature(bodyBytes(body), sig, SECRET)).resolves.toBe(true);
+  });
+
+  it("throws ConfigError instead of verifying when the secret is an empty string", async () => {
+    await expect(
+      verifyGithubSignature(bodyBytes(body), "sha256=anything", ""),
+    ).rejects.toBeInstanceOf(ConfigError);
+  });
+
+  it("throws ConfigError instead of verifying when the secret is undefined", async () => {
+    await expect(
+      verifyGithubSignature(
+        bodyBytes(body),
+        "sha256=anything",
+        undefined as unknown as string,
+      ),
+    ).rejects.toBeInstanceOf(ConfigError);
+  });
+});