diff --git a/bun.lock b/bun.lock index 602b76f1d..4bf491885 100644 --- a/bun.lock +++ b/bun.lock @@ -13,6 +13,7 @@ "@corbits/evals": "workspace:*", "@corbits/ollama-adapter": "workspace:*", "@corbits/run-scope": "workspace:*", + "@corbits/webhook-triggers": "workspace:*", "@corbits/workflow-catalog": "workspace:*", "@eslint/js": "^10.0.0", "@intx/db": "workspace:*", @@ -766,7 +767,7 @@ }, "packages/github-tools": { "name": "@corbits/github-tools", - "version": "0.0.8", + "version": "0.0.9", "dependencies": { "@intx/agent": "workspace:*", "@intx/types": "workspace:*", @@ -919,7 +920,7 @@ }, "packages/interaction-tools": { "name": "@corbits/interaction-tools", - "version": "0.0.2", + "version": "0.0.4", "dependencies": { "@intx/agent": "workspace:*", "@intx/types": "workspace:*", diff --git a/package.json b/package.json index 5cebca4b7..64191e09d 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,8 @@ "puppeteer-core": "^25.7.0", "typescript": "6.0.3", "typescript-eslint": "^8.42.0", - "@corbits/run-scope": "workspace:*" + "@corbits/run-scope": "workspace:*", + "@corbits/webhook-triggers": "workspace:*" }, "engines": { "bun": ">=1.2.0" diff --git a/packages/evals/src/targets/real-target.ts b/packages/evals/src/targets/real-target.ts index 1e172e528..306c5c606 100644 --- a/packages/evals/src/targets/real-target.ts +++ b/packages/evals/src/targets/real-target.ts @@ -37,6 +37,7 @@ import { import { signPayload, WEBHOOK_SIGNATURE_HEADER, + WEBHOOK_TIMESTAMP_HEADER, } from "@corbits/webhook-triggers"; import type { @@ -891,13 +892,15 @@ export async function bootMyraTarget( ); } const rawBody = JSON.stringify(payload); + const timestamp = String(Math.floor(Date.now() / 1000)); const response = await fetch( new URL(`/api/webhooks/${triggerId}`, hub.baseUrl), { method: "POST", headers: { "content-type": "application/json", - [WEBHOOK_SIGNATURE_HEADER]: signPayload(secret, rawBody), + [WEBHOOK_TIMESTAMP_HEADER]: timestamp, + [WEBHOOK_SIGNATURE_HEADER]: signPayload(secret, timestamp, rawBody), }, body: rawBody, }, diff --git a/packages/webhook-triggers/src/index.ts b/packages/webhook-triggers/src/index.ts index 06c7e4b19..88dcf45c3 100644 --- a/packages/webhook-triggers/src/index.ts +++ b/packages/webhook-triggers/src/index.ts @@ -16,6 +16,7 @@ export { signPayload, verifySignature, WEBHOOK_SIGNATURE_HEADER, + WEBHOOK_TIMESTAMP_HEADER, } from "./signature"; export { renderInputTemplate } from "./mapping"; export { diff --git a/packages/webhook-triggers/src/ingress-routes.ts b/packages/webhook-triggers/src/ingress-routes.ts index 81a2a7290..86b859c8c 100644 --- a/packages/webhook-triggers/src/ingress-routes.ts +++ b/packages/webhook-triggers/src/ingress-routes.ts @@ -2,18 +2,26 @@ // Workbench's control (Granola, or anything else) posts here to kick // off a workflow run. This is THE trust boundary — no session cookie, // no tenant membership, nothing about the caller is trusted except -// what the HMAC signature over the raw body proves. Every failure -// mode here is loud and specific in the log, but every one of unknown -// trigger, disabled trigger, and bad/missing signature returns the -// SAME generic 401 `unauthorized` response, so a probe against a +// what the HMAC signature over `timestamp.rawBody` proves, and proves +// only within the freshness window `verifySignature` enforces (see +// `./signature.ts`) — so a captured, byte-for-byte replay of a real +// delivery stops verifying once it goes stale. Every failure mode +// here is loud and specific in the log, but every one of unknown +// trigger, disabled trigger, and bad/missing/stale signature returns +// the SAME generic 401 `unauthorized` response, so a probe against a // wrong triggerId cannot distinguish "no such trigger" from "disabled" -// from "wrong secret". +// from "wrong secret" from "replayed". import { Hono } from "hono"; import type { Env } from "hono"; import { getLogger } from "@intx/log"; import type { LaunchedWebhookTrigger } from "./launch"; -import { WEBHOOK_SIGNATURE_HEADER, verifySignature } from "./signature"; +import { + WEBHOOK_SIGNATURE_HEADER, + WEBHOOK_TIMESTAMP_HEADER, + isFreshTimestamp, + verifySignature, +} from "./signature"; import type { WebhookTriggerRow } from "./schema"; import type { WebhookTriggerStore } from "./store"; @@ -72,13 +80,22 @@ export function createWebhookIngressRoutes( const rawBody = await c.req.text(); const signatureHeader = c.req.header(WEBHOOK_SIGNATURE_HEADER); - if (!verifySignature(trigger.secret, rawBody, signatureHeader)) { - log.warn( - "Rejected webhook delivery for trigger {triggerId}: bad signature", - { - triggerId, - }, - ); + const timestampHeader = c.req.header(WEBHOOK_TIMESTAMP_HEADER); + if ( + !verifySignature( + trigger.secret, + timestampHeader, + rawBody, + signatureHeader, + ) + ) { + const reason = isFreshTimestamp(timestampHeader) + ? "bad signature" + : "missing or stale timestamp"; + log.warn("Rejected webhook delivery for trigger {triggerId}: {reason}", { + triggerId, + reason, + }); return c.json(unauthorizedResponse(), 401); } diff --git a/packages/webhook-triggers/src/signature.ts b/packages/webhook-triggers/src/signature.ts index 3d7e877c9..90c96a012 100644 --- a/packages/webhook-triggers/src/signature.ts +++ b/packages/webhook-triggers/src/signature.ts @@ -1,11 +1,11 @@ // The trust boundary this package exists to guard: an inbound HTTP // call from a network Workbench does not control. Every byte off the // wire is untrusted until its signature verifies against the -// trigger's own secret. +// trigger's own secret, over a delivery recent enough to matter. // // Security-model note: the secret is generated server-side with // `crypto.randomBytes` and shown to the caller exactly once, at -// creation or rotation. `store.ts` now encrypts it at rest through +// creation or rotation. `store.ts` encrypts it at rest through // Interchange's `CredentialCipher` seam (`@intx/types`), closing the // "database dump discloses every signing secret" half of this // tradeoff — but this function still needs the *raw* secret in process @@ -18,39 +18,84 @@ // means moving to asymmetric signing (caller signs with a private key, // this package verifies with a stored public key), a bigger v2 change. // Flagged, not silently accepted. +// +// The other half of that same tradeoff — one shared secret, verified +// by recomputing the same MAC, with nothing to stop a captured +// request from being replayed forever — is closed here by binding an +// `X-Webhook-Timestamp` into the signed material instead of signing +// `rawBody` alone. `verifySignature` rejects a delivery whose +// timestamp falls outside a 5-minute window on either side of now — +// both a stale replay of an old capture and a timestamp forged far +// into the future — the same window Stripe's `timestamp.signature` +// scheme uses. This still doesn't stop a delivery replayed within +// that window; closing that fully needs a tracked, per-delivery nonce +// (a bigger v2 change, like the asymmetric-signing one above). import { createHmac, randomBytes, timingSafeEqual } from "node:crypto"; export const WEBHOOK_SIGNATURE_HEADER = "x-webhook-signature"; +export const WEBHOOK_TIMESTAMP_HEADER = "x-webhook-timestamp"; const SECRET_BYTES = 32; +const TIMESTAMP_TOLERANCE_SECONDS = 300; /** A fresh, high-entropy secret for a new or rotated trigger. */ export function generateWebhookSecret(): string { return randomBytes(SECRET_BYTES).toString("hex"); } -/** The HMAC-SHA256 hex digest of `rawBody` under `secret`. */ -export function signPayload(secret: string, rawBody: string): string { - return createHmac("sha256", secret).update(rawBody, "utf8").digest("hex"); +function signedMaterial(timestamp: string, rawBody: string): string { + return `${timestamp}.${rawBody}`; +} + +/** The HMAC-SHA256 hex digest of `timestamp.rawBody` under `secret`. */ +export function signPayload( + secret: string, + timestamp: string, + rawBody: string, +): string { + return createHmac("sha256", secret) + .update(signedMaterial(timestamp, rawBody), "utf8") + .digest("hex"); } /** - * Verifies an inbound `X-Webhook-Signature` header against the - * trigger's stored secret, in constant time so a timing side-channel - * never leaks how many leading bytes of a guess were correct. - * `timingSafeEqual` throws on a length mismatch rather than returning - * false, so an obviously-wrong-length header is rejected explicitly - * before comparison, not left to throw past this function. + * Exported only so `ingress-routes.ts` can log a more specific reason + * for a rejected delivery than "bad signature" — the HTTP response + * stays the same generic 401 either way (see the module doc comment). + */ +export function isFreshTimestamp( + timestampHeader: string | undefined, +): timestampHeader is string { + if (timestampHeader === undefined || timestampHeader === "") return false; + const seconds = Number(timestampHeader); + if (!Number.isFinite(seconds)) return false; + return Math.abs(Date.now() / 1000 - seconds) <= TIMESTAMP_TOLERANCE_SECONDS; +} + +/** + * Verifies an inbound `X-Webhook-Signature` header, over + * `X-Webhook-Timestamp.rawBody`, against the trigger's stored secret — + * in constant time so a timing side-channel never leaks how many + * leading bytes of a guess were correct, and only for a timestamp + * within `TIMESTAMP_TOLERANCE_SECONDS` of now so a captured delivery + * stops verifying once it goes stale. `timingSafeEqual` throws on a + * length mismatch rather than returning false, so an obviously-wrong- + * length header is rejected explicitly before comparison, not left to + * throw past this function. */ export function verifySignature( secret: string, + timestampHeader: string | undefined, rawBody: string, providedSignatureHex: string | undefined, ): boolean { if (providedSignatureHex === undefined || providedSignatureHex === "") { return false; } - const expected = signPayload(secret, rawBody); + if (!isFreshTimestamp(timestampHeader)) { + return false; + } + const expected = signPayload(secret, timestampHeader, rawBody); const expectedBuffer = Buffer.from(expected, "hex"); let providedBuffer: Buffer; try { diff --git a/packages/webhook-triggers/test/ingress-routes.test.ts b/packages/webhook-triggers/test/ingress-routes.test.ts index f89e7e96f..7bed35274 100644 --- a/packages/webhook-triggers/test/ingress-routes.test.ts +++ b/packages/webhook-triggers/test/ingress-routes.test.ts @@ -1,15 +1,24 @@ // Exercises `createWebhookIngressRoutes`' HTTP surface: signature -// verification (valid/invalid/missing), unknown/disabled-trigger +// verification (valid/invalid/missing/stale), unknown/disabled-trigger // handling, and payload parsing — with a fake `launch` seam so no // database or folded-run launch machinery is involved. This is the // trust-boundary route: no session, no tenant middleware. Unknown -// trigger, disabled trigger, and bad signature must all come back as -// the same generic 401 so a probe can't tell them apart. +// trigger, disabled trigger, bad signature, and a stale/replayed +// timestamp must all come back as the same generic 401 so a probe +// can't tell them apart. import { describe, expect, test } from "bun:test"; import { createWebhookIngressRoutes } from "../src/ingress-routes"; -import { signPayload, WEBHOOK_SIGNATURE_HEADER } from "../src/signature"; +import { + signPayload, + WEBHOOK_SIGNATURE_HEADER, + WEBHOOK_TIMESTAMP_HEADER, +} from "../src/signature"; import { createInMemoryWebhookTriggerStore } from "./test-support"; +function nowSeconds(): string { + return String(Math.floor(Date.now() / 1000)); +} + async function seedTrigger( store: ReturnType, overrides: { enabled?: boolean } = {}, @@ -43,7 +52,7 @@ function buildApp( } describe("POST /:triggerId", () => { - test("launches on a validly signed payload", async () => { + test("launches on a validly signed payload with a fresh timestamp", async () => { let launchedWith: unknown; const { app, store } = buildApp(async (_trigger, payload) => { launchedWith = payload; @@ -52,11 +61,13 @@ describe("POST /:triggerId", () => { await seedTrigger(store); const body = JSON.stringify({ note: { title: "Q3 planning" } }); + const timestamp = nowSeconds(); const response = await app.request("/ins_trigger1", { method: "POST", headers: { "content-type": "application/json", - [WEBHOOK_SIGNATURE_HEADER]: signPayload("s3cr3t", body), + [WEBHOOK_TIMESTAMP_HEADER]: timestamp, + [WEBHOOK_SIGNATURE_HEADER]: signPayload("s3cr3t", timestamp, body), }, body, }); @@ -77,7 +88,10 @@ describe("POST /:triggerId", () => { const response = await app.request("/ins_trigger1", { method: "POST", - headers: { "content-type": "application/json" }, + headers: { + "content-type": "application/json", + [WEBHOOK_TIMESTAMP_HEADER]: nowSeconds(), + }, body: "{}", }); @@ -85,16 +99,60 @@ describe("POST /:triggerId", () => { expect(await response.json()).toEqual(expectedUnauthorizedBody); }); + test("rejects a missing timestamp header with the generic unauthorized response", async () => { + const { app, store } = buildApp(); + await seedTrigger(store); + + const body = "{}"; + const response = await app.request("/ins_trigger1", { + method: "POST", + headers: { + "content-type": "application/json", + [WEBHOOK_SIGNATURE_HEADER]: signPayload("s3cr3t", nowSeconds(), body), + }, + body, + }); + + expect(response.status).toBe(401); + expect(await response.json()).toEqual(expectedUnauthorizedBody); + }); + + test("rejects a stale timestamp (a replayed delivery) with the generic unauthorized response", async () => { + const { app, store } = buildApp(); + await seedTrigger(store); + + const body = "{}"; + const staleTimestamp = String(Math.floor(Date.now() / 1000) - 301); + const response = await app.request("/ins_trigger1", { + method: "POST", + headers: { + "content-type": "application/json", + [WEBHOOK_TIMESTAMP_HEADER]: staleTimestamp, + [WEBHOOK_SIGNATURE_HEADER]: signPayload("s3cr3t", staleTimestamp, body), + }, + body, + }); + + expect(response.status).toBe(401); + expect(await response.json()).toEqual(expectedUnauthorizedBody); + }); + test("rejects a signature computed with the wrong secret with the generic unauthorized response", async () => { const { app, store } = buildApp(); await seedTrigger(store); const body = "{}"; + const timestamp = nowSeconds(); const response = await app.request("/ins_trigger1", { method: "POST", headers: { "content-type": "application/json", - [WEBHOOK_SIGNATURE_HEADER]: signPayload("wrong-secret", body), + [WEBHOOK_TIMESTAMP_HEADER]: timestamp, + [WEBHOOK_SIGNATURE_HEADER]: signPayload( + "wrong-secret", + timestamp, + body, + ), }, body, }); @@ -118,9 +176,13 @@ describe("POST /:triggerId", () => { await seedTrigger(store, { enabled: false }); const body = "{}"; + const timestamp = nowSeconds(); const response = await app.request("/ins_trigger1", { method: "POST", - headers: { [WEBHOOK_SIGNATURE_HEADER]: signPayload("s3cr3t", body) }, + headers: { + [WEBHOOK_TIMESTAMP_HEADER]: timestamp, + [WEBHOOK_SIGNATURE_HEADER]: signPayload("s3cr3t", timestamp, body), + }, body, }); @@ -133,9 +195,13 @@ describe("POST /:triggerId", () => { await seedTrigger(store); const body = "not json"; + const timestamp = nowSeconds(); const response = await app.request("/ins_trigger1", { method: "POST", - headers: { [WEBHOOK_SIGNATURE_HEADER]: signPayload("s3cr3t", body) }, + headers: { + [WEBHOOK_TIMESTAMP_HEADER]: timestamp, + [WEBHOOK_SIGNATURE_HEADER]: signPayload("s3cr3t", timestamp, body), + }, body, }); diff --git a/packages/webhook-triggers/test/signature.test.ts b/packages/webhook-triggers/test/signature.test.ts index 35f2647dd..6ebb697e2 100644 --- a/packages/webhook-triggers/test/signature.test.ts +++ b/packages/webhook-triggers/test/signature.test.ts @@ -5,6 +5,10 @@ import { verifySignature, } from "../src/signature"; +function nowSeconds(): string { + return String(Math.floor(Date.now() / 1000)); +} + describe("generateWebhookSecret", () => { test("mints distinct high-entropy secrets", () => { const a = generateWebhookSecret(); @@ -18,32 +22,94 @@ describe("verifySignature", () => { const secret = generateWebhookSecret(); const body = JSON.stringify({ hello: "world" }); - test("accepts a correctly signed payload", () => { - const signature = signPayload(secret, body); - expect(verifySignature(secret, body, signature)).toBe(true); + test("accepts a correctly signed payload with a fresh timestamp", () => { + const timestamp = nowSeconds(); + const signature = signPayload(secret, timestamp, body); + expect(verifySignature(secret, timestamp, body, signature)).toBe(true); }); test("rejects a payload signed with a different secret", () => { - const wrongSignature = signPayload(generateWebhookSecret(), body); - expect(verifySignature(secret, body, wrongSignature)).toBe(false); + const timestamp = nowSeconds(); + const wrongSignature = signPayload( + generateWebhookSecret(), + timestamp, + body, + ); + expect(verifySignature(secret, timestamp, body, wrongSignature)).toBe( + false, + ); }); test("rejects a tampered body against the original signature", () => { - const signature = signPayload(secret, body); + const timestamp = nowSeconds(); + const signature = signPayload(secret, timestamp, body); const tamperedBody = JSON.stringify({ hello: "mallory" }); - expect(verifySignature(secret, tamperedBody, signature)).toBe(false); + expect(verifySignature(secret, timestamp, tamperedBody, signature)).toBe( + false, + ); }); test("rejects a missing signature header", () => { - expect(verifySignature(secret, body, undefined)).toBe(false); - expect(verifySignature(secret, body, "")).toBe(false); + const timestamp = nowSeconds(); + expect(verifySignature(secret, timestamp, body, undefined)).toBe(false); + expect(verifySignature(secret, timestamp, body, "")).toBe(false); }); test("rejects a non-hex signature without throwing", () => { - expect(verifySignature(secret, body, "not-hex-at-all!!")).toBe(false); + const timestamp = nowSeconds(); + expect(verifySignature(secret, timestamp, body, "not-hex-at-all!!")).toBe( + false, + ); }); test("rejects a well-formed-but-wrong-length hex signature", () => { - expect(verifySignature(secret, body, "abcd")).toBe(false); + const timestamp = nowSeconds(); + expect(verifySignature(secret, timestamp, body, "abcd")).toBe(false); + }); + + test("rejects a signature computed over a different timestamp than the one presented", () => { + const timestamp = nowSeconds(); + const signature = signPayload(secret, timestamp, body); + const laterTimestamp = String(Number(timestamp) + 1); + expect(verifySignature(secret, laterTimestamp, body, signature)).toBe( + false, + ); + }); + + test("rejects a replayed delivery once its timestamp is outside the tolerance window", () => { + const staleTimestamp = String(Math.floor(Date.now() / 1000) - 301); + const signature = signPayload(secret, staleTimestamp, body); + expect(verifySignature(secret, staleTimestamp, body, signature)).toBe( + false, + ); + }); + + test("accepts a timestamp comfortably inside the tolerance window", () => { + // 299s, not the exact 300s boundary: the boundary itself is racy + // against wall-clock time elapsing between signing and verifying. + const edgeTimestamp = String(Math.floor(Date.now() / 1000) - 299); + const signature = signPayload(secret, edgeTimestamp, body); + expect(verifySignature(secret, edgeTimestamp, body, signature)).toBe(true); + }); + + test("rejects a timestamp forged too far into the future", () => { + const futureTimestamp = String(Math.floor(Date.now() / 1000) + 301); + const signature = signPayload(secret, futureTimestamp, body); + expect(verifySignature(secret, futureTimestamp, body, signature)).toBe( + false, + ); + }); + + test("rejects a missing timestamp header", () => { + const signature = signPayload(secret, nowSeconds(), body); + expect(verifySignature(secret, undefined, body, signature)).toBe(false); + expect(verifySignature(secret, "", body, signature)).toBe(false); + }); + + test("rejects a non-numeric timestamp header without treating it as fresh", () => { + const signature = signPayload(secret, "not-a-number", body); + expect(verifySignature(secret, "not-a-number", body, signature)).toBe( + false, + ); }); }); diff --git a/scripts/e2e/routine-trigger-input.test.ts b/scripts/e2e/routine-trigger-input.test.ts index 8915364cc..bb0eb1569 100644 --- a/scripts/e2e/routine-trigger-input.test.ts +++ b/scripts/e2e/routine-trigger-input.test.ts @@ -17,7 +17,11 @@ // assertion here reads `session_mail` straight out of Postgres, joined // through `agent_session`/`workflow_run` on the run's own instance id — // a harness-side fact, not a public contract. -import { createHmac } from "node:crypto"; +import { + signPayload, + WEBHOOK_SIGNATURE_HEADER, + WEBHOOK_TIMESTAMP_HEADER, +} from "@corbits/webhook-triggers"; import { describe, test } from "bun:test"; import { resetSchema, setupDatabase } from "../db-setup.ts"; @@ -543,16 +547,16 @@ describe.skipIf(databaseUrl === undefined)( topic: "Deploy finished", source: "ci", }); - const signature = createHmac("sha256", secret) - .update(rawBody, "utf8") - .digest("hex"); + const timestamp = String(Math.floor(Date.now() / 1000)); + const signature = signPayload(secret, timestamp, rawBody); const delivered = await fetch( `${hub.baseUrl}/api/webhooks/${triggerId}`, { method: "POST", headers: { "content-type": "application/json", - "x-webhook-signature": signature, + [WEBHOOK_SIGNATURE_HEADER]: signature, + [WEBHOOK_TIMESTAMP_HEADER]: timestamp, }, body: rawBody, }, diff --git a/scripts/e2e/smoke-webhook.test.ts b/scripts/e2e/smoke-webhook.test.ts index c11cecb50..f114154b2 100644 --- a/scripts/e2e/smoke-webhook.test.ts +++ b/scripts/e2e/smoke-webhook.test.ts @@ -18,7 +18,11 @@ // already does for the sidecar identity row — and confirms the trigger // itself recorded the delivery (`lastFiredAt`). -import { createHmac } from "node:crypto"; +import { + signPayload, + WEBHOOK_SIGNATURE_HEADER, + WEBHOOK_TIMESTAMP_HEADER, +} from "@corbits/webhook-triggers"; import { describe, expect, test } from "bun:test"; import { resetSchema, setupDatabase } from "../db-setup.ts"; @@ -381,14 +385,14 @@ describe.skipIf(databaseUrl === undefined)("smoke: webhook trigger", () => { "a correctly signed delivery is accepted and launches a run", async () => { const rawBody = JSON.stringify({ event: "smoke-test" }); - const signature = createHmac("sha256", secret) - .update(rawBody, "utf8") - .digest("hex"); + const timestamp = String(Math.floor(Date.now() / 1000)); + const signature = signPayload(secret, timestamp, rawBody); const res = await fetch(`${hub.baseUrl}/api/webhooks/${triggerId}`, { method: "POST", headers: { "content-type": "application/json", - "x-webhook-signature": signature, + [WEBHOOK_SIGNATURE_HEADER]: signature, + [WEBHOOK_TIMESTAMP_HEADER]: timestamp, }, body: rawBody, }); @@ -416,7 +420,8 @@ describe.skipIf(databaseUrl === undefined)("smoke: webhook trigger", () => { method: "POST", headers: { "content-type": "application/json", - "x-webhook-signature": "0".repeat(64), + [WEBHOOK_SIGNATURE_HEADER]: "0".repeat(64), + [WEBHOOK_TIMESTAMP_HEADER]: String(Math.floor(Date.now() / 1000)), }, body: rawBody, });