diff --git a/WEBHOOK_SIGNATURE_VERIFICATION.md b/WEBHOOK_SIGNATURE_VERIFICATION.md index 79d5df43..638da6ed 100644 --- a/WEBHOOK_SIGNATURE_VERIFICATION.md +++ b/WEBHOOK_SIGNATURE_VERIFICATION.md @@ -6,8 +6,10 @@ Canonical guide: docs/webhooks.md Outbound Callora deliveries (dispatchWebhook) use: Header: X-Callora-Signature: sha256= +Key ID header: X-Callora-Key-Id: (identifies the active managed signing key from WebhookSignerService, or subscription id fallback) MAC input: raw HTTP body bytes only (timestamp is not signed) -No signature header when the subscription has no secret +Managed rotation: When platform signing keys are rotated via POST /api/admin/webhooks/rotate-key, new outbound deliveries immediately sign with the new active key and emit its id in X-Callora-Key-Id, while the previous key remains valid for verification during the configured grace window (WEBHOOK_SECRET_ROTATION_GRACE_MS). +No signature header when neither a managed active key nor a subscription secret is configured Do not use the older X-Callora-Signature-256 / .. examples from previous revisions of this file for verifying events Callora sends to your endpoint — that scheme is the platform-inbound deliver route only. diff --git a/src/routes/admin/webhookKeys.ts b/src/routes/admin/webhookKeys.ts index ab1d0bc9..439a1ba5 100644 --- a/src/routes/admin/webhookKeys.ts +++ b/src/routes/admin/webhookKeys.ts @@ -30,24 +30,13 @@ import { sendMail } from '../../lib/mailer.js'; import { validate } from '../../middleware/validate.js'; import { WebhookSignerService, - InMemoryWebhookKeyStore, + defaultWebhookKeyStore, resolveGraceWindowMs, type WebhookSignerDeps, type RotationResult, } from '../../services/webhookSigner.js'; -// --------------------------------------------------------------------------- -// Shared service instance -// --------------------------------------------------------------------------- -// -// In a single-process deploy the in-memory store is sufficient. -// For multi-replica deployments, replace `InMemoryWebhookKeyStore` with a -// Postgres/SQLite-backed implementation that satisfies `WebhookKeyStore`. -// -// The instance is module-level so it is created once per process and shared -// across requests, preserving key state between rotations. - -const defaultStore = new InMemoryWebhookKeyStore(); +const defaultStore = defaultWebhookKeyStore; /** * Build the admin notification callback. @@ -178,13 +167,20 @@ export function createWebhookKeysRouter( // Note: only express.json() is mounted, so a non-JSON body never reaches // req.body — this guard covers misconfigured parsers on the parent app. const contentType = req.get('Content-Type') ?? ''; + const contentLength = parseInt(req.get('Content-Length') ?? '0', 10); const rawBody: unknown = req.body; const hasBody = - rawBody !== undefined && - rawBody !== null && - !(typeof rawBody === 'object' && Object.keys(rawBody as object).length === 0); + (Number.isFinite(contentLength) && contentLength > 0) || + (rawBody !== undefined && + rawBody !== null && + !(typeof rawBody === 'object' && Object.keys(rawBody as object).length === 0)); if (hasBody && !contentType.includes('application/json')) { - next(new BadRequestError('Content-Type must be application/json when a body is supplied')); + res.status(400).json({ + error: { + code: 'BAD_REQUEST', + message: 'Content-Type must be application/json when a body is supplied', + }, + }); return; } diff --git a/src/services/webhookSigner.test.ts b/src/services/webhookSigner.test.ts index 1aaa2803..67919899 100644 --- a/src/services/webhookSigner.test.ts +++ b/src/services/webhookSigner.test.ts @@ -553,4 +553,39 @@ describe('Grace window — dual-key verification', () => { const hashes = await service.getActiveKeyHashes(); expect(hashes).toContain(hashSecret(r1.rawSecret)); }); + + it('signs payloads with the active key and verifies both active and previous signatures during the grace window (#1234)', async () => { + const store = new InMemoryWebhookKeyStore(); + const clock = { value: new Date('2025-06-01T10:00:00Z') }; + const service = new WebhookSignerService({ + store, + graceWindowMs: 60_000, + now: () => clock.value, + }); + + const body = JSON.stringify({ event: 'new_api_call', data: { apiId: 'api-1' } }); + expect(await service.signPayload(body)).toBeNull(); + + const r1 = await service.rotateKey('admin'); + const sig1 = await service.signPayload(body); + expect(sig1?.keyId).toBe(r1.newKey.id); + + const r2 = await service.rotateKey('admin'); + const sig2 = await service.signPayload(body); + expect(sig2?.keyId).toBe(r2.newKey.id); + expect(sig2?.signature).not.toBe(sig1?.signature); + + // During grace window, both sig1 (previous key) and sig2 (new key) verify + const v1During = await service.verifyPayloadSignature(body, sig1!.signature, clock.value); + const v2During = await service.verifyPayloadSignature(body, sig2!.signature, clock.value); + expect(v1During).toEqual({ valid: true, keyId: r1.newKey.id }); + expect(v2During).toEqual({ valid: true, keyId: r2.newKey.id }); + + // After grace window expires, sig1 is rejected while sig2 remains valid + const afterGrace = new Date(clock.value.getTime() + 60_001); + const v1After = await service.verifyPayloadSignature(body, sig1!.signature, afterGrace); + const v2After = await service.verifyPayloadSignature(body, sig2!.signature, afterGrace); + expect(v1After).toEqual({ valid: false, keyId: null }); + expect(v2After).toEqual({ valid: true, keyId: r2.newKey.id }); + }); }); \ No newline at end of file diff --git a/src/services/webhookSigner.ts b/src/services/webhookSigner.ts index a44020a0..1c312af0 100644 --- a/src/services/webhookSigner.ts +++ b/src/services/webhookSigner.ts @@ -176,6 +176,15 @@ export function resolveGraceWindowMs(override?: number): number { // Service // --------------------------------------------------------------------------- +/** Compute raw HMAC-SHA256 hex digest for a webhook payload body. */ +export function signWithSecret(secret: string, body: string): string { + return crypto.createHmac('sha256', secret).update(body).digest('hex'); +} + +// Module-level map of keyId -> rawSecret for in-process signing & verification +// without persisting raw secrets in WebhookKeyStore rows. +const runtimeRawSecrets = new Map(); + export class WebhookSignerService { private readonly store: WebhookKeyStore; private readonly notifyAdmin: (result: RotationResult) => Promise; @@ -232,8 +241,9 @@ export class WebhookSignerService { // 2. Demote existing active key const previousKey = await this.store.demoteActiveKey(expiresAt); - // 3. Persist new active key + // 3. Persist new active key and cache raw secret in memory for signing await this.store.insertKey(newKey); + runtimeRawSecrets.set(newKeyId, rawSecret); // 4. Audit log const auditEntry: WebhookKeyRotationAudit = { @@ -272,6 +282,63 @@ export class WebhookSignerService { return result; } + /** + * Sign a serialized webhook payload with the currently active managed key. + * Returns `{ keyId, signature: "sha256=" }`, or `null` if no managed key is active. + */ + async signPayload( + body: string, + now?: Date, + ): Promise<{ keyId: string; signature: string } | null> { + const ts = now ?? this.clock(); + await this.store.expireStaleKeys(ts); + + const active = await this.store.getActiveKey(); + if (!active) return null; + + const secret = runtimeRawSecrets.get(active.id) ?? active.key_hash; + const digest = signWithSecret(secret, body); + return { + keyId: active.id, + signature: `sha256=${digest}`, + }; + } + + /** + * Verify a webhook signature against the active key and any unexpired previous + * keys still within their rotation grace window. + */ + async verifyPayloadSignature( + body: string, + signatureHeader: string, + now?: Date, + ): Promise<{ valid: boolean; keyId: string | null }> { + const ts = now ?? this.clock(); + await this.store.expireStaleKeys(ts); + + const normalizedSig = signatureHeader.startsWith('sha256=') + ? signatureHeader.slice('sha256='.length) + : signatureHeader; + + const active = await this.store.getActiveKey(); + const previous = await this.store.getValidPreviousKeys(ts); + const candidates: WebhookSigningKey[] = []; + if (active) candidates.push(active); + candidates.push(...previous); + + for (const key of candidates) { + const secret = runtimeRawSecrets.get(key.id) ?? key.key_hash; + const expected = signWithSecret(secret, body); + const a = Buffer.from(normalizedSig, 'utf8'); + const b = Buffer.from(expected, 'utf8'); + if (a.length === b.length && crypto.timingSafeEqual(a, b)) { + return { valid: true, keyId: key.id }; + } + } + + return { valid: false, keyId: null }; + } + /** * Return all currently-valid raw key hashes (active + unexpired previous). * Used by signature-verification middleware to accept either key. @@ -326,6 +393,12 @@ export class InMemoryWebhookKeyStore implements WebhookKeyStore { async demoteActiveKey(expiresAt: Date): Promise { const idx = this.keys.findIndex((k) => k.status === 'active'); if (idx === -1) return null; + // Expire any older 'previous' keys so only the most recently demoted key stays in 'previous' + for (const k of this.keys) { + if (k.status === 'previous') { + k.status = 'expired'; + } + } this.keys[idx] = { ...this.keys[idx], status: 'previous', @@ -342,6 +415,7 @@ export class InMemoryWebhookKeyStore implements WebhookKeyStore { new Date(key.expires_at).getTime() < now.getTime() ) { key.status = 'expired'; + runtimeRawSecrets.delete(key.id); } } } @@ -362,7 +436,15 @@ export class InMemoryWebhookKeyStore implements WebhookKeyStore { /** Test helper — reset state. */ _reset(): void { + for (const k of this.keys) { + runtimeRawSecrets.delete(k.id); + } this.keys = []; this.auditLog = []; } -} \ No newline at end of file +} + +export const defaultWebhookKeyStore = new InMemoryWebhookKeyStore(); +export const defaultWebhookSignerService = new WebhookSignerService({ + store: defaultWebhookKeyStore, +}); \ No newline at end of file diff --git a/src/webhooks/webhook.dispatcher.test.ts b/src/webhooks/webhook.dispatcher.test.ts index 97bfd898..c7c58c78 100644 --- a/src/webhooks/webhook.dispatcher.test.ts +++ b/src/webhooks/webhook.dispatcher.test.ts @@ -804,5 +804,70 @@ describe('Webhook Dispatcher', () => { expect(callerA).not.toEqual(callerB); }); }); + + describe('managed rotation key signing (#1234)', () => { + it('signs deliveries with managed active key, emits X-Callora-Key-Id, and switches to new key after rotation while keeping previous key verifiable during grace window', async () => { + const { + InMemoryWebhookKeyStore, + WebhookSignerService, + signWithSecret, + } = await import('../services/webhookSigner.js'); + const { setWebhookSignerService } = await import('./webhook.dispatcher.js'); + + const keyStore = new InMemoryWebhookKeyStore(); + let now = new Date('2026-06-01T12:00:00.000Z'); + const signer = new WebhookSignerService({ + store: keyStore, + graceWindowMs: 60_000, + now: () => now, + }); + setWebhookSignerService(signer); + + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + status: 200, + statusText: 'OK', + } as Response); + global.fetch = fetchMock as unknown as typeof fetch; + + const rotation1 = await signer.rotateKey('admin-1'); + await dispatchWebhook(config, payload); + + expect(fetchMock).toHaveBeenCalledTimes(1); + const firstHeaders = fetchMock.mock.calls[0][1].headers as Record; + const firstBody = fetchMock.mock.calls[0][1].body as string; + expect(firstHeaders['X-Callora-Key-Id']).toBe(rotation1.newKey.id); + expect(firstHeaders['X-Callora-Signature']).toBe( + `sha256=${signWithSecret(rotation1.rawSecret, firstBody)}`, + ); + + // Rotate key again; subsequent delivery uses the new active key + now = new Date(now.getTime() + 1_000); + const rotation2 = await signer.rotateKey('admin-1'); + await dispatchWebhook(config, payload); + + expect(fetchMock).toHaveBeenCalledTimes(2); + const secondHeaders = fetchMock.mock.calls[1][1].headers as Record; + const secondBody = fetchMock.mock.calls[1][1].body as string; + expect(secondHeaders['X-Callora-Key-Id']).toBe(rotation2.newKey.id); + expect(secondHeaders['X-Callora-Signature']).toBe( + `sha256=${signWithSecret(rotation2.rawSecret, secondBody)}`, + ); + + // Both first (previous key) and second (active key) deliveries verify during grace window + const verifyFirst = await signer.verifyPayloadSignature( + firstBody, + firstHeaders['X-Callora-Signature'], + now, + ); + const verifySecond = await signer.verifyPayloadSignature( + secondBody, + secondHeaders['X-Callora-Signature'], + now, + ); + expect(verifyFirst).toEqual({ valid: true, keyId: rotation1.newKey.id }); + expect(verifySecond).toEqual({ valid: true, keyId: rotation2.newKey.id }); + }); + }); }); diff --git a/src/webhooks/webhook.dispatcher.ts b/src/webhooks/webhook.dispatcher.ts index f8fa54a3..89359ed3 100644 --- a/src/webhooks/webhook.dispatcher.ts +++ b/src/webhooks/webhook.dispatcher.ts @@ -6,6 +6,12 @@ import { getCorrelationId, getRequestId } from '../utils/asyncContext.js'; import { getEffectiveRetryPolicy, calculateBackoff } from '../services/webhookRetry.js'; import { computeJitteredDelay, type RandomSource } from '../lib/retry.js'; import { validateWebhookUrl, WebhookValidationError } from './webhook.validator.js'; +import { + defaultWebhookKeyStore, + defaultWebhookSignerService, + signWithSecret, + type WebhookSignerService, +} from '../services/webhookSigner.js'; export const MAX_WEBHOOK_RESPONSE_BYTES = 64 * 1024; @@ -71,6 +77,7 @@ export async function consumeCappedResponseBody( let acceptingDispatches = true; const inFlightDispatches = new Set>(); +let activeSignerService: WebhookSignerService = defaultWebhookSignerService; /** * Random source used for retry backoff jitter. Injectable so tests can supply @@ -82,10 +89,6 @@ function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } -function signPayload(secret: string, body: string): string { - return crypto.createHmac('sha256', secret).update(body).digest('hex'); -} - function trackDispatch(operation: Promise): Promise { const tracked = operation.finally(() => { inFlightDispatches.delete(tracked as Promise); @@ -105,10 +108,16 @@ export async function awaitWebhookDispatcherIdle(): Promise { } } +export function setWebhookSignerService(signer: WebhookSignerService): void { + activeSignerService = signer; +} + export function resetWebhookDispatcherForTests(): void { acceptingDispatches = true; inFlightDispatches.clear(); jitterRandom = Math.random; + activeSignerService = defaultWebhookSignerService; + defaultWebhookKeyStore._reset(); } /** @@ -160,8 +169,18 @@ export async function dispatchWebhook( headers['X-Correlation-Id'] = correlationId; } - if (config.secret) { - headers['X-Callora-Signature'] = `sha256=${signPayload(config.secret, body)}`; + const managedSig = await activeSignerService.signPayload(body); + if (managedSig) { + headers['X-Callora-Signature'] = managedSig.signature; + headers['X-Callora-Key-Id'] = managedSig.keyId; + } else { + const subSecret = config.secret_current ?? config.secret; + if (subSecret) { + headers['X-Callora-Signature'] = `sha256=${signWithSecret(subSecret, body)}`; + if (config.id) { + headers['X-Callora-Key-Id'] = config.id; + } + } } let lastError: unknown;