58 lines
3.0 KiB
Markdown
58 lines
3.0 KiB
Markdown
# 07. Fix webhook replay via missing event ID deduplication
|
|
|
|
meta:
|
|
id: security-fixes-07
|
|
feature: security-fixes
|
|
priority: P1
|
|
depends_on: [security-fixes-06]
|
|
tags: [implementation, tests-required, medium-severity, database-migration]
|
|
|
|
objective:
|
|
- Prevent Stripe webhook replay attacks by implementing event ID deduplication for all event types
|
|
|
|
deliverables:
|
|
- New database table for webhook event ID tracking (via Drizzle migration)
|
|
- Updated webhook handler at `web/src/routes/api/stripe/webhook.ts` to check and record event IDs
|
|
- Updated `billing.service.ts` to use the deduplication mechanism
|
|
- Unit and integration tests for replay detection
|
|
|
|
steps:
|
|
1. Create a Drizzle migration to add a `stripe_webhook_events` table:
|
|
- `id` (TEXT, primary key, stores Stripe `event.id`)
|
|
- `type` (TEXT, event type like `invoice.paid`)
|
|
- `processed_at` (DATETIME, timestamp)
|
|
- Consider adding a TTL/cleanup mechanism for old records
|
|
2. Update `web/src/routes/api/stripe/webhook.ts:18-21` to check the event ID against the table before processing
|
|
3. For each event type (`checkout.session.completed`, `invoice.paid`, `invoice.payment_failed`, `customer.subscription.updated`, `customer.subscription.deleted`):
|
|
- Check if `event.id` already exists in the table
|
|
- If yes, log and return early (idempotent replay)
|
|
- If no, insert the event ID and proceed with processing
|
|
4. Ensure the insert uses `onConflictDoNothing()` or a unique constraint to handle race conditions
|
|
5. Add a cleanup job or TTL to prevent unbounded table growth (e.g., delete events older than 30 days)
|
|
|
|
tests:
|
|
- Unit: Duplicate event ID returns early without re-processing
|
|
- Unit: New event ID is inserted and processing continues
|
|
- Unit: Race condition (two identical events arrive simultaneously) is handled by unique constraint
|
|
- Integration: Sending the same webhook event twice results in only one processing
|
|
- Integration: Different event types with the same ID are handled correctly (should not happen in practice but test the constraint)
|
|
|
|
acceptance_criteria:
|
|
- All webhook event types check for duplicate event IDs before processing
|
|
- Duplicate events are logged and skipped without re-processing
|
|
- Race conditions are handled by database constraints (unique index on event ID)
|
|
- Old event IDs are cleaned up to prevent unbounded table growth
|
|
- No regression in existing webhook processing behavior
|
|
|
|
validation:
|
|
- `cd web && bun test` — all tests pass
|
|
- Run the Drizzle migration and verify the `stripe_webhook_events` table exists
|
|
- Send the same webhook event twice and verify only the first is processed
|
|
- Check the database to confirm the event ID was recorded
|
|
|
|
notes:
|
|
- Finding p8-007: Only `checkout.session.completed` currently has partial dedup via `onConflictDoNothing()`
|
|
- Depends on task 06 because validated data shapes from that fix are needed for the dedup logic
|
|
- Stripe guarantees event IDs are unique, so the event ID is a safe dedup key
|
|
- Consider a background job or cron to clean up old event IDs (e.g., older than 30 days)
|