Skip to content

fix: eliminate TOCTOU race in select-winners by moving eligibility ch… - #449

Open
macsonfleek wants to merge 1 commit into
geevapp:mainfrom
macsonfleek:fix/select-winners-toctou-race
Open

fix: eliminate TOCTOU race in select-winners by moving eligibility ch…#449
macsonfleek wants to merge 1 commit into
geevapp:mainfrom
macsonfleek:fix/select-winners-toctou-race

Conversation

@macsonfleek

Copy link
Copy Markdown

…eck inside transaction

Move the post status re-read and winner eligibility computation inside the $transaction with Serializable isolation level. This prevents two concurrent selection requests from both passing the "already completed" guard and over-selecting winners beyond maxWinners.

Key changes:

  • Re-read post status + existing winners inside the Serializable transaction
  • Compute remainingSlots (maxWinners - existingWinners) inside the transaction
  • Cap all selection methods at remainingSlots to prevent over-assignment
  • Surface domain errors as 400 responses instead of generic 500s
  • Add concurrency test asserting exactly one request succeeds under contention

Closes #426

🤖 Generated with Codebuff

Pull Request Template

Description

Please include a summary of the change and which issue is fixed. Also include relevant motivation and context.


Checklist

  • I have tested my changes locally
  • I have updated documentation as needed
  • I have run npx prisma generate after schema changes
  • I have run npx prisma migrate dev or npx prisma migrate deploy as appropriate

Post-Merge Steps for Maintainers

If this PR includes changes to the Prisma schema:

  1. Run the following command to apply the migration to your database:

    npx prisma migrate deploy

    or, for local development:

    npx prisma migrate dev
  2. Ensure your CI pipeline runs the migration before tests (add this step if missing):

    - name: Run Prisma Migrate
      run: npx prisma migrate deploy
  3. Make sure the database user in CI has permission to run migrations.


If you have any questions, please comment on this PR.

…eck inside transaction

Move the post status re-read and winner eligibility computation inside
the $transaction with Serializable isolation level. This prevents two
concurrent selection requests from both passing the "already completed"
guard and over-selecting winners beyond maxWinners.

Key changes:
- Re-read post status + existing winners inside the Serializable transaction
- Compute remainingSlots (maxWinners - existingWinners) inside the transaction
- Cap all selection methods at remainingSlots to prevent over-assignment
- Surface domain errors as 400 responses instead of generic 500s
- Add concurrency test asserting exactly one request succeeds under contention

Closes geevapp#426

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <[email protected]>
@drips-wave

drips-wave Bot commented Sep 1, 2026

Copy link
Copy Markdown

@macsonfleek Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

select-winners reads eligibility and status outside the transaction — TOCTOU race allows over-selection

1 participant