Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions packages/payments/stripe/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,21 @@
throw new Error('Stripe-Signature missing t or v1');
}

// Reject replayed webhooks per Stripe's guidance:
// https://docs.stripe.com/webhooks/signatures#replay-prevention
const TOLERANCE_SECONDS = 300; // 5 minutes — matches Stripe's own SDK default
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 TOLERANCE_SECONDS is not configurable, making integration tests awkward

TOLERANCE_SECONDS is hardcoded inside the function with no way to override it. Stripe's own stripe-node SDK exposes this as a parameter (defaulting to 300 s). Without an override path, test authors must either pin Date.now via fake timers or generate signatures at runtime — both add friction. Consider accepting an optional tolerance parameter in verifyStripeSignature so callers can pass Infinity or a wider window in test environments.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

const ts = Number(timestamp);
if (!Number.isFinite(ts)) {
throw new Error('Stripe-Signature t is not numeric');
}
const skew = Math.abs(Math.floor(Date.now() / 1000) - ts);
if (skew > TOLERANCE_SECONDS) {
throw new Error(

Check failure on line 196 in packages/payments/stripe/src/index.ts

View workflow job for this annotation

GitHub Actions / test

packages/payments/stripe/src/index.test.ts > payment-stripe > verifies Stripe webhook signatures and normalizes checkout events

Error: Stripe webhook timestamp 1700000000 is outside the 300s tolerance (skew: 80925794s). Reject to prevent replay attacks. ❯ verifyStripeSignature packages/payments/stripe/src/index.ts:196:11 ❯ Object.verifyWebhook packages/payments/stripe/src/index.ts:46:5 ❯ packages/payments/stripe/src/index.test.ts:82:35
`Stripe webhook timestamp ${ts} is outside the ${TOLERANCE_SECONDS}s tolerance (skew: ${skew}s). ` +
'Reject to prevent replay attacks.',
);
}
Comment on lines +187 to +200
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Existing tests break because timestamp is hardcoded to a stale value

The test file uses timestamp = '1700000000' (November 2023), which is now more than 2 years outside the 300-second window. Two tests in index.test.ts will fail after this change:

  1. 'verifies Stripe webhook signatures…' (line 62) — the timestamp check fires before the HMAC, so the previously-valid test now throws "Stripe webhook timestamp … is outside the tolerance" and the test fails.
  2. 'rejects invalid Stripe webhook signatures' (line 99) — same stale t=1700000000 causes the rejection test to throw the wrong error, so rejects.toThrow('Invalid Stripe webhook signature') fails because the actual message is about the timestamp, not the signature.

Neither test is updated in this PR. Both need to use vi.setSystemTime / vi.useFakeTimers to pin Date.now() to the fixture timestamp, or generate the timestamp dynamically from Math.floor(Date.now() / 1000) at test runtime.


const expected = createHmac('sha256', secret)
.update(`${timestamp}.${rawBody}`)
.digest('hex');
Expand Down
Loading