From 72bf112c984dc6589b41f042842e11d997676b36 Mon Sep 17 00:00:00 2001 From: Claude Lin & Lay Date: Sun, 21 Jun 2026 14:14:10 +0900 Subject: [PATCH] test(worker): extract verifyGitHubSignature to signature.ts + unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the module-private verifyGitHubSignature out of worker/src/index.ts into a new worker/src/signature.ts (exported verbatim, behavior unchanged) and replace the inline definition with an import. index.ts pulls the full agent / oauth / store graph, so importing the function directly was infeasible for a lightweight unit test; extracting it isolates the pure HMAC-SHA256 logic. Add worker/test/signature.test.ts (tsx --test, node:crypto as an independent signature oracle): valid signature, tampered body, wrong secret, and malformed signature formats (empty / no sha256= prefix / wrong-length hex) all asserted. 署名検証は trust boundary（送信元が本物の GitHub であることと本文改竄なしを 同時保証する）であり subtractive の明示保護領域のためテスト価値が高い。 挙動は不変で、関数を移動して import に差し替えただけ。 Refs #227 --- worker/src/index.ts | 21 +----------- worker/src/signature.ts | 29 ++++++++++++++++ worker/test/signature.test.ts | 64 +++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 20 deletions(-) create mode 100644 worker/src/signature.ts create mode 100644 worker/test/signature.test.ts diff --git a/worker/src/index.ts b/worker/src/index.ts index 790439a..4b4859c 100644 --- a/worker/src/index.ts +++ b/worker/src/index.ts @@ -29,6 +29,7 @@ import { } from "./oauth.js"; import { isGitHubWebhookIP } from "./github-ip.js"; import { checkWebhookRateLimit, checkApiRateLimit, checkTenantQuota, rateLimitResponse } from "./rate-limit.js"; +import { verifyGitHubSignature } from "./signature.js"; export { WebhookMcpAgent, WebhookStore, TenantRegistry }; @@ -39,26 +40,6 @@ interface Env extends OAuthEnv { GITHUB_WEBHOOK_SECRET?: string; } -async function verifyGitHubSignature( - secret: string, - body: string, - signature: string, -): Promise { - const encoder = new TextEncoder(); - const key = await crypto.subtle.importKey( - "raw", - encoder.encode(secret), - { name: "HMAC", hash: "SHA-256" }, - false, - ["sign"], - ); - 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; -} - /** * Resolve installation_id to account_id via TenantRegistry DO. * On installation.created, registers the mapping first. diff --git a/worker/src/signature.ts b/worker/src/signature.ts new file mode 100644 index 0000000..6823d50 --- /dev/null +++ b/worker/src/signature.ts @@ -0,0 +1,29 @@ +/** + * GitHub webhook signature verification (#227). + * + * 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=` 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. + */ +export async function verifyGitHubSignature( + secret: string, + body: string, + signature: string, +): Promise { + const encoder = new TextEncoder(); + const key = await crypto.subtle.importKey( + "raw", + encoder.encode(secret), + { name: "HMAC", hash: "SHA-256" }, + false, + ["sign"], + ); + 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; +} diff --git a/worker/test/signature.test.ts b/worker/test/signature.test.ts new file mode 100644 index 0000000..f45ab57 --- /dev/null +++ b/worker/test/signature.test.ts @@ -0,0 +1,64 @@ +/** + * 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 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"; + +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 ─────────────────────────────────────────────────────── + +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 () => { + // 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 () => { + const signature = sign(SECRET, BODY); + assert.equal(await verifyGitHubSignature("wrong-secret", BODY, signature), false); +}); + +// ── Malformed signature formats ────────────────────────────────────── + +test("rejects an empty signature string", async () => { + assert.equal(await verifyGitHubSignature(SECRET, BODY, ""), 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 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); +});