Skip to content

fix(webhooks): replace non-atomic SELECT+check with atomic UPDATE claim to eliminate duplicate fulfillments#368

Open
Srejoye wants to merge 18 commits into
Ixotic27:mainfrom
Srejoye:361-fix-webhook-race-conditions
Open

fix(webhooks): replace non-atomic SELECT+check with atomic UPDATE claim to eliminate duplicate fulfillments#368
Srejoye wants to merge 18 commits into
Ixotic27:mainfrom
Srejoye:361-fix-webhook-race-conditions

Conversation

@Srejoye

@Srejoye Srejoye commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a race condition present in all four payment webhook handlers (Stripe, Cashfree, NOWPayments, AbacatePay). Concurrent webhook retries — which all providers send — could both read status = "pending" before either had written "completed", causing both to call fulfillItemPurchase. This led to duplicate item grants, corrupted consumable quantities, duplicate activity_feed inserts, and duplicate Discord notifications and emails.

The fix is the same single pattern applied consistently across all four files:

Instead of SELECT (read status) → check → fulfill → UPDATE, the handler now does a conditional UPDATE first:

const { data: claimed } = await sb
  .from("purchases")
  .update({ status: "processing" })
  .eq("provider_tx_id", ...)
  .eq("status", "pending")        // ← only matches if still pending
  .eq("provider", "...")
  .select("id, developer_id, item_id, gifted_to")
  .maybeSingle();

if (!claimed) break; // another request already won — stop here
// safe: only one request reaches this line

Supabase (Postgres UPDATE ... WHERE ... RETURNING) is atomic at the row level — exactly one concurrent caller gets the row back; all others get null and exit immediately. fulfillItemPurchase, autoEquipIfSolo, feed inserts, and all notifications are only reached by the single winner.

Sky ads paths are untouched — they already use .eq("active", false) as an implicit atomic guard on their UPDATE, which is the same pattern.

Files changed: stripe/route.ts, cashfree/route.ts, nowpayments/route.ts, abacatepay/route.ts — fulfillment block only in each, no changes to auth, parsing, or expiry/refund handling.

Related issue

Fixes #361

Checklist

  • My code follows the project's coding style
  • I have tested my changes locally
  • My changes do not break existing functionality
  • I have linked the related issue above

…rent retries

All four payment webhook handlers (Stripe, Cashfree, NOWPayments, AbacatePay) used a non-atomic SELECT-then-UPDATE pattern. Two concurrent retry requests could both read status='pending' and both proceed to fulfillItemPurchase, causing duplicate item grants, corrupted consumable quantities, duplicate activity feed inserts, and duplicate notifications/emails.

Fix: replace the SELECT+check with a conditional UPDATE that transitions pending → processing in a single round-trip. Supabase only returns a row to the request that wins the update; the loser gets null and breaks early. fulfillItemPurchase and all downstream side-effects are only reached by the winner.

Sky ads paths are unaffected — they already use .eq('active', false) as an implicit atomic guard on their UPDATE.
@vercel

vercel Bot commented Jun 7, 2026

Copy link
Copy Markdown

@Srejoye is attempting to deploy a commit to the ixotic27-8245's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot left a comment

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.

Security Scan: Clean

No suspicious patterns detected. The official Copilot bot will provide detailed AI feedback shortly.

If you enjoyed contributing, please consider starring the repository!

@github-actions github-actions Bot added backend Backend/API related good first issue Good for newcomers Gssoc 26 Part of GirlScript Summer of Code 2026 gssoc:approved Approved GSSoC contribution level:intermediate Intermediate difficulty level type:bug Something isn't working as expected labels Jun 7, 2026
@Ixotic27

Ixotic27 commented Jun 7, 2026

Copy link
Copy Markdown
Owner

@Srejoye check pr #355
Then only your further pr's will be accepted

@Ixotic27

Ixotic27 commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Hi @Srejoye! 👋 I've reviewed your PR code for resolving concurrent webhook double-fulfillment (Issue #377):

Your atomic claim pattern using UPDATE to change the status from pending to processing is very neat! However, there is still a race condition in the fallback/retry handler logic:

If two identical webhook requests arrive concurrently, the first request will successfully claim the row and transition it to processing status. The second request will fail the atomic claim (returning null) and fall into the else block. In the else block, the second request queries for an existing completed transaction with the transaction ID (provider_tx_id = txId). Because the first request is still running fulfillItemPurchase and has not yet written the provider_tx_id to the database, the second request will find no existing completed transaction (existing is null), pass the if (!existing) guard, and proceed to call fulfillItemPurchase a second time. This results in duplicate consumable rewards being granted before the transaction ID uniqueness check takes effect.

Please address this race condition so we can review the PR again! Thank you!

@github-actions github-actions Bot added the status:blocked This PR is blocked due to a failing CI check. label Jun 8, 2026
@Srejoye Srejoye force-pushed the 361-fix-webhook-race-conditions branch from a69fdae to 5c11ce0 Compare June 8, 2026 10:21
@github-actions github-actions Bot removed the status:blocked This PR is blocked due to a failing CI check. label Jun 8, 2026
@Ixotic27

Ixotic27 commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Hi @Srejoye! 👋 I've reviewed your PR code for resolving issue #361.

It looks like this PR has a regression and conflict:

  1. Conflict with PR fix(webhook): prevent duplicate reward fulfillment from concurrent Stripe webhooks (#377) #402: The Stripe webhook concurrency check has already been resolved in main via PR fix(webhook): prevent duplicate reward fulfillment from concurrent Stripe webhooks (#377) #402. The changes to Stripe's webhook handler in this PR conflict with the main branch.
  2. Missing Transaction ID (provider_tx_id) on Claim: In your Stripe handler changes, when a pending purchase is atomically claimed (status updated to 'processing'), the code does not write the provider_tx_id (payment intent/session ID) to the purchase record. We need to save this ID to associate the database record with the actual Stripe payment.

The atomic claim fixes are still very valuable for AbacatePay, Cashfree, and NOWPayments! Please refactor the PR to preserve the Stripe webhook implementation currently on main while applying the atomic claim fixes to the other webhook endpoints.

Thank you!

@Srejoye Srejoye force-pushed the 361-fix-webhook-race-conditions branch from 8b3b25c to 5089dde Compare June 8, 2026 17:55

@github-actions github-actions Bot left a comment

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.

🔍 Security Scan: Review Needed

The following patterns were detected in the latest changes:

⚠️ direct env access (review needed) — found in 1 added line(s):
if (!process.env.ABACATEPAY_WEBHOOK_SECRET) {
⚠️ hardcoded secret/key reference — found in 1 added line(s):
if (!process.env.ABACATEPAY_WEBHOOK_SECRET) {

A maintainer should review these findings before merging.

Hey @Srejoye, please review these flagged items! 🛠️

@github-actions github-actions Bot added the status:blocked This PR is blocked due to a failing CI check. label Jun 9, 2026
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

🚨 Hey @Srejoye, the CI Pipeline is failing on this PR and it has been marked as status:blocked.

🔍 What failed:

  • Production Build failed at step(s): Build

📋 Error Details (first 2):

Please fix the issues before this can be reviewed. Here's how:

1. Run checks locally before pushing:

npm run lint           # Run ESLint
npm run build          # Verify production build passes

2. Auto-fix common issues:

npm run lint -- --fix  # Auto-fix lint errors where possible

3. Check the full failure log here:
👉 View CI Run

Once you push a fix and the CI passes, the status:blocked label will be removed automatically. 💪

@github-actions github-actions Bot removed the status:blocked This PR is blocked due to a failing CI check. label Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Backend/API related good first issue Good for newcomers gssoc:approved Approved GSSoC contribution Gssoc 26 Part of GirlScript Summer of Code 2026 level:intermediate Intermediate difficulty level type:bug Something isn't working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Concurrency Bug: Race conditions in payment webhooks lead to duplicate fulfillments and notifications

2 participants