Repository navigation
fix(otp): make a correct code single-use and never compare a code without expiry - #9789
Merged
Merged
Conversation
…hout expiry A correct verification deleted the code unconditionally after an atomic reservation, so concurrent correct submissions each got a session and a slow success could delete a newer code issued meanwhile. Success now requires a conditional consume (delete only while the stored code is still the one compared), so exactly one request wins and newer codes survive. Reservations no longer delete; a code with every attempt spent just stops reserving, and the final wrong guess consumes it. The Redis reservation checks the remaining lifetime before anything else, so a key left without an expiry is never compared.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
 [Critical impact] The PR appears safe to merge; no actionable issues remain from this review. SummaryThis PR makes successful OTP verification depend on removing the same stored code that was compared.
No new actionable issues were found. Tests were not run during this review. Reviews (2) · Last reviewed commit: "test(otp): prove a newer code survives a..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…mission Store a replacement code between the old submission's reservation and its consume on both backends; the old submission must fail and the newer code must still verify. An unconditional delete that reports removal fails it.
Collaborator
Author
Collaborator
Author
|
@cubic-dev-ai review this PR |
Contributor
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
This branch was previously deployed
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
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
DELETE ... RETURNING). Exactly one correct submission wins; a newer code survivescode:attemptsstrictly, matching the Postgres path; shared SQL expressions for the stored code and attempt countsafeComparestill does the comparisonType of Change
Testing
route.integration.ts(real Postgres + Redis): new cases for concurrent correct submissions (exactly one 200 + cookie) and a Redis code with no expiry (rejected, key removed); both fail on the previous code. Reporting success without the conditional consume turns the single-use case red on both backends. Existing concurrency/lockout/expiry/authorization cases pass, 3 consecutive runscontent-sharing.integration.ts(file-share OTP) passesroute.test.ts,bun run lint,bun run check:audits(only a docs artifact that is generated in CI fails locally)Checklist
test-auditauthoring gate)🤖 Generated with Claude Code