Skip to content

Commit d57c47f

Browse files
committed
fix: bind outgoing webhook signatures to a timestamp
registerWebhook's response has always documented HMAC-SHA256(timestamp + "." + body) with an x-webhook-timestamp header as the signature contract - middleware/webhookSecurity.ts already verifies inbound webhooks this way - but outbound delivery never sent the header or included the timestamp in what it signed. Any receiver following NEPA's own documented instructions would fail every signature check. - Add X-Webhook-Timestamp header; sign `${timestamp}.${body}` instead of the body alone, across real delivery, retry, and testWebhook - Compute the signature fresh inside attemptWebhookDelivery per attempt rather than once at event-creation time - retries can now happen up to an hour later (Phase 3's delay cap), so a signature computed at attempt 0 would sign an already-stale timestamp by the time a retry actually fires - Add round-trip tests: capture the real axios.post call and verify it via WebhookSecurityService.validateSignature (the real verification path), plus a negative check that an un-timestamped signature is correctly rejected 71/71 tests passing; tsc error count unchanged (760, all pre-existing)
1 parent f810487 commit d57c47f

2 files changed

Lines changed: 95 additions & 11 deletions

File tree

backend/services/WebhookService.ts

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,6 @@ export class WebhookService {
242242
*/
243243
private async deliverWebhookEvent(webhook: Webhook, payload: WebhookPayload): Promise<void> {
244244
try {
245-
// Create event record
246-
const payloadString = JSON.stringify(payload);
247-
const signature = WebhookService.generateSignature(payloadString, webhook.secret);
248-
249245
const event = await prisma.webhookEvent.create({
250246
data: {
251247
webhookId: webhook.id,
@@ -257,7 +253,7 @@ export class WebhookService {
257253
});
258254

259255
// Attempt delivery
260-
await this.attemptWebhookDelivery(webhook, event, payload, signature, 0);
256+
await this.attemptWebhookDelivery(webhook, event, payload, 0);
261257
} catch (error) {
262258
logger.error(`Failed to deliver webhook event for webhook ${webhook.id}: ${error}`);
263259
}
@@ -270,14 +266,26 @@ export class WebhookService {
270266
webhook: Webhook,
271267
event: WebhookEvent,
272268
payload: WebhookPayload,
273-
signature: string,
274269
attemptNumber: number
275270
): Promise<void> {
276271
try {
277272
const payloadString = JSON.stringify(payload);
273+
274+
// Bind the timestamp into the signed string (matching the contract
275+
// documented in registerWebhook's response and the convention
276+
// middleware/webhookSecurity.ts already uses to verify *inbound*
277+
// signatures) so a receiver can enforce a replay window. Computed
278+
// fresh per attempt, not once at event-creation time — retries can
279+
// now happen up to an hour later (see calculateRetryDelay's cap), so
280+
// a signature computed at attempt 0 would otherwise sign a
281+
// timestamp that's long since expired by the time a retry fires.
282+
const timestamp = Math.floor(Date.now() / 1000);
283+
const signature = WebhookService.generateSignature(`${timestamp}.${payloadString}`, webhook.secret);
284+
278285
const headers: { [key: string]: string } = {
279286
'Content-Type': 'application/json',
280287
'X-Webhook-Signature': signature,
288+
'X-Webhook-Timestamp': String(timestamp),
281289
'X-Webhook-ID': webhook.id,
282290
'X-Event-Type': payload.eventType,
283291
'X-Delivery-ID': event.id,
@@ -454,9 +462,8 @@ export class WebhookService {
454462
}
455463

456464
const payload: WebhookPayload = JSON.parse(JSON.stringify(event.payload));
457-
const signature = WebhookService.generateSignature(JSON.stringify(payload), webhook.secret);
458465

459-
await this.attemptWebhookDelivery(webhook, event, payload, signature, event.attempts);
466+
await this.attemptWebhookDelivery(webhook, event, payload, event.attempts);
460467
}
461468
} catch (error) {
462469
logger.error(`Failed to process pending webhook retries: ${error}`);
@@ -590,11 +597,13 @@ export class WebhookService {
590597
};
591598

592599
const payloadString = JSON.stringify(testPayload);
593-
const signature = WebhookService.generateSignature(payloadString, webhook.secret);
600+
const timestamp = Math.floor(Date.now() / 1000);
601+
const signature = WebhookService.generateSignature(`${timestamp}.${payloadString}`, webhook.secret);
594602

595603
const headers = {
596604
'Content-Type': 'application/json',
597605
'X-Webhook-Signature': signature,
606+
'X-Webhook-Timestamp': String(timestamp),
598607
'X-Webhook-ID': webhook.id,
599608
'X-Event-Type': 'webhook.test',
600609
'X-Test-Delivery': 'true',
@@ -694,7 +703,6 @@ export class WebhookService {
694703
}
695704

696705
const payload: WebhookPayload = JSON.parse(JSON.stringify(event.payload));
697-
const signature = WebhookService.generateSignature(JSON.stringify(payload), webhook.secret);
698706

699707
// Reset attempts for retry
700708
await prisma.webhookEvent.update({
@@ -705,7 +713,7 @@ export class WebhookService {
705713
},
706714
});
707715

708-
await this.attemptWebhookDelivery(webhook, event, payload, signature, 0);
716+
await this.attemptWebhookDelivery(webhook, event, payload, 0);
709717
logger.info(`Webhook event retried: ${eventId}`);
710718
} catch (error) {
711719
logger.error(`Failed to retry webhook event: ${error}`);

backend/tests/webhook/WebhookService.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,56 @@ describe('WebhookService', () => {
385385
data: expect.objectContaining({ status: 'FAILED', attempts: 1 }),
386386
});
387387
});
388+
389+
it('should sign deliveries so WebhookSecurityService.validateSignature accepts them', async () => {
390+
// registerWebhook's response documents this exact contract to API
391+
// consumers (algorithm HMAC-SHA256, header x-webhook-timestamp,
392+
// format "timestamp + '.' + body") — this proves the delivery code
393+
// actually implements what's documented, using the same verification
394+
// path a real receiver would run.
395+
const secret = 'test-secret';
396+
const eventType = 'payment.success';
397+
const mockWebhooks = [
398+
{
399+
id: 'webhook-1',
400+
url: testWebhookUrl,
401+
events: [eventType],
402+
secret,
403+
isActive: true,
404+
retryPolicy: 'FIXED',
405+
maxRetries: 3,
406+
retryDelaySeconds: 60,
407+
timeoutSeconds: 30,
408+
headers: null,
409+
},
410+
];
411+
const mockEvent = { id: 'event-1', webhookId: 'webhook-1', eventType, status: 'PENDING' };
412+
413+
mockPrisma.webhook.findMany.mockResolvedValue(mockWebhooks as any);
414+
mockPrisma.webhookEvent.create.mockResolvedValue(mockEvent as any);
415+
mockPrisma.webhookAttempt.create.mockResolvedValue({} as any);
416+
mockPrisma.webhookEvent.update.mockResolvedValue({} as any);
417+
mockPrisma.webhookLog.create.mockResolvedValue({} as any);
418+
mockedAxios.post.mockResolvedValue({ status: 200, data: { ok: true } });
419+
420+
const before = Math.floor(Date.now() / 1000);
421+
await webhookService.triggerWebhook(eventType, { amount: 100 });
422+
const after = Math.floor(Date.now() / 1000);
423+
424+
const [, body, config] = mockedAxios.post.mock.calls[0];
425+
const sentTimestamp = Number(config.headers['X-Webhook-Timestamp']);
426+
const sentSignature = config.headers['X-Webhook-Signature'];
427+
428+
expect(sentTimestamp).toBeGreaterThanOrEqual(before);
429+
expect(sentTimestamp).toBeLessThanOrEqual(after);
430+
expect(
431+
WebhookSecurityService.validateSignature(`${sentTimestamp}.${body}`, sentSignature, secret)
432+
).toBe(true);
433+
// A signature computed without the timestamp binding (the old,
434+
// broken behavior) must NOT validate — otherwise this test would
435+
// pass even if the timestamp weren't actually part of what's signed.
436+
expect(WebhookSecurityService.validateSignature(body, sentSignature, secret)).toBe(false);
437+
});
388438
});
389439

390440
describe('processPendingRetries', () => {
@@ -629,6 +679,32 @@ describe('WebhookService', () => {
629679

630680
await expect(webhookService.testWebhook('non-existent')).rejects.toThrow('Webhook not found');
631681
});
682+
683+
it('should sign test deliveries with the same timestamp-bound contract as real deliveries', async () => {
684+
const secret = 'test-secret';
685+
const mockWebhook = {
686+
id: webhookId,
687+
url: testWebhookUrl,
688+
secret,
689+
timeoutSeconds: 30,
690+
headers: null,
691+
};
692+
693+
mockPrisma.webhook.findUnique.mockResolvedValue(mockWebhook as any);
694+
mockPrisma.webhookLog.create.mockResolvedValue({} as any);
695+
mockedAxios.post.mockResolvedValue({ status: 200, data: 'Success' });
696+
697+
await webhookService.testWebhook(webhookId);
698+
699+
const [, body, config] = mockedAxios.post.mock.calls[0];
700+
const sentTimestamp = config.headers['X-Webhook-Timestamp'];
701+
const sentSignature = config.headers['X-Webhook-Signature'];
702+
703+
expect(sentTimestamp).toEqual(expect.any(String));
704+
expect(
705+
WebhookSecurityService.validateSignature(`${sentTimestamp}.${body}`, sentSignature, secret)
706+
).toBe(true);
707+
});
632708
});
633709

634710
describe('processWebhookEvent', () => {

0 commit comments

Comments
 (0)