fix: atomic promo redemption duplicate guard (fixes #92) - #110
Merged
ralyodio merged 2 commits intoAug 1, 2026
Merged
Conversation
Close the TOCTOU between quotePromo()'s duplicate check and recordPromoRedemption()'s insert by making the redemption row itself the single atomic authority: INSERT OR IGNORE + rowsAffected check. Returns whether the redemption was newly recorded; skips the uses increment when the row already exists. Also keeps the capped increment (uses < max_uses) so over-redemption is impossible even if the quote check races. Tests: new recordPromoRedemption suite (records + bumps, uppercase normalization, duplicate no-op, blank code).
Same TDZ fix as the rate-limit test: create the mocked execute via vi.hoisted() so the hoisted vi.mock factory doesn't reference it before initialization.
ralyodio
added a commit
that referenced
this pull request
Aug 1, 2026
…sponse (#115) #110 dropped the try/catch around recordPromoRedemption to make the duplicate guard atomic. Both callers grant the entitlement BEFORE recording the redemption and neither handles errors: - app/api/payments/status/route.ts:58 - app/api/payments/create-checkout/route.ts:59 So a transient DB failure in the INSERT now 500s a response that has already granted the purchase — and the status route's payload carries grant.apiKeyPlaintext, which is shown exactly once. The user would lose their API key to that 500. Restore the catch (log + return false) while keeping the atomic INSERT OR IGNORE guard and the capped increment. The recorded flag is tracked outside the try so a failed counter bump still reports the redemption row that did land. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #92 (TOCTOU race in the promo code duplicate redemption check). Previously quotePromo() checked for an existing redemption and recordPromoRedemption() inserted it as two separate, lock-free database operations - so concurrent requests could both pass the quote check and double-record.
Change
apps/web/lib/entitlements.ts - recordPromoRedemption() now uses a single atomic statement as the source of truth:
apps/web/lib/entitlements.test.ts - new test suite: records and bumps uses, uppercase normalization, duplicate no-op (no UPDATE issued), blank code no-op.
Verification
Notes
Complements PR #106 (capped increment); the two changes overlap in the same function and this PR already includes the capped increment so either merge order works. Existing callers ignore the new boolean return value and are unaffected.