fix(auth): burn email OTP verification token on attempt to prevent brute force (#2221)

[massmarketconsumer-arch]

Resolves #2221

Summary

useVerificationToken in packages/database/auth/drizzle-adapter.ts previously queried exclusively by verificationTokens.token. On an incorrect 6-digit code guess, no row was matched, leaving the active token untouched in the database for its full 10-minute TTL and exposing email OTP logins to brute-force attacks.

This fix aligns NextAuth email OTP verification with the security invariant already enforced in the mobile login route (apps/web/app/api/mobile/[...route]/route.ts:543-548):

1. Looks up the verification record by normalizedIdentifier (falling back to token if identifier is absent).
2. Deletes the verification row from the database immediately upon the attempt, burning the code on the first wrong attempt.
3. Evaluates identifier match, expiration, and token equality. Returns null on any mismatch or expiration with the token safely burned.

Testing

Added unit tests in apps/web/__tests__/unit/use-verification-token.test.ts:

• Token is immediately burned and returns null on an incorrect token guess.
• Token is burned and returns null when expired.
• Token is burned and returns valid token record on matching credentials.
• Returns null without attempting deletion when no token exists for identifier.

Confidence Score: 1/5

This PR is not safe to merge until arbitrary OTP invalidation and non-atomic concurrent consumption are addressed.

Findings

1.   Arbitrary OTP invalidation ▶
2.   Token consumption remains non-atomic ▶
3.  Mocks ignore database predicates ▶

Fix with agent prompt

### Issue 1
packages/database/auth/drizzle-adapter.ts:460-465
An unauthenticated caller can submit a victim’s email with any incorrect code. The identifier-only lookup finds the victim’s active verification row and deletes it before checking whether the supplied code matches. Because the email callback has no server-side attempt limit, repeated requests can continuously invalidate newly issued codes and prevent the victim from signing in.

**How this was verified:** The unauthenticated email callback forwards the supplied identifier and token to this adapter, which selects by identifier and deletes using the stored token before comparing it with the supplied token.

### Issue 2
packages/database/auth/drizzle-adapter.ts:477-484
Concurrent callbacks can both read the live row before either deletes it. Since the deletion result is ignored, a request whose deletion affected no rows still validates and returns its previously read token. This allows parallel redemption of a nominally single-use code and lets concurrent guesses race the intended burn. Require deletion of exactly one row, or consume the token atomically, before returning it.

**How this was verified:** The code performs separate pooled SELECT and DELETE statements and returns the previously selected row without checking whether its DELETE removed anything.

### Issue 3
apps/web/__tests__/unit/use-verification-token.test.ts:16-33
The SELECT mock always returns its configured row without checking the WHERE clause, and the DELETE mock only records that it was called. These tests would therefore still pass if the implementation queried or deleted the wrong row-for example, if it queried by the incorrect guessed token and a real database would leave the active token untouched. Assert the predicates against seeded rows or use a data-backed database fixture so the tests protect the security behavior.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

• Adds first-attempt token burning for incorrect and expired codes.
• Adds unit coverage for successful, incorrect, expired, and missing-token cases.
• The resulting path permits arbitrary invalidation of another user’s token and does not atomically enforce single consumption.
• The test doubles do not validate the database predicates central to the fix.

_{Reviews (1) · Last reviewed commit: "fix(auth): burn verification token on ve..."}

[superagent-security]

Superagent found 1 security concern(s).

[superagent-security]

P2: Token lookup and deletion are not atomic, allowing concurrent OTP reuse

Separate SELECT and DELETE let concurrent requests validate the same OTP before either request consumes it.

Consume the row atomically with locking or conditional delete, and add a concurrent-use test.

AI prompt

Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="packages/database/auth/drizzle-adapter.ts">
<violation number="1" location="packages/database/auth/drizzle-adapter.ts:478">
<priority>P2</priority>
<title>Token lookup and deletion are not atomic, allowing concurrent OTP reuse</title>
<evidence>The new implementation first selects a verification row by identifier and only afterward deletes it. Two concurrent useVerificationToken calls can both complete the select before either delete executes, then both validate the same still-present token and return it successfully, defeating the one-attempt/one-use guarantee under concurrent requests.</evidence>
<recommendation>Make consumption atomic: use a transaction with row locking (for example, SELECT ... FOR UPDATE followed by DELETE), or perform a conditional delete/consume operation whose affected row/result is used to decide whether verification may succeed. Add a concurrency test asserting that only one simultaneous call can return the token.</recommendation>
</violation>
</file>

[massmarketconsumer-arch]

Addressed in 6e9e367. Wrapped verification and deletion inside db.transaction using row locking (for("update")), and added a concurrent verification test asserting that only one simultaneous call can return the token while parallel attempts receive null.

[superagent-security]

Developer resolved the race condition by wrapping the verification and deletion in a transaction with row-level locking (for("update")) and added a concurrency test ensuring only one simultaneous request can consume the token. This directly fixes the atomicity issue and satisfies the finding.

[greptile-apps]

Arbitrary OTP invalidation

An unauthenticated caller can submit a victim’s email with any incorrect code. The identifier-only lookup finds the victim’s active verification row and deletes it before checking whether the supplied code matches. Because the email callback has no server-side attempt limit, repeated requests can continuously invalidate newly issued codes and prevent the victim from signing in.

How this was verified: The unauthenticated email callback forwards the supplied identifier and token to this adapter, which selects by identifier and deletes using the stored token before comparing it with the supplied token.

Knowledge Base Used: Data and identity platform

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/database/auth/drizzle-adapter.ts
Line: 460-465

Comment:
**Arbitrary OTP invalidation**

An unauthenticated caller can submit a victim’s email with any incorrect code. The identifier-only lookup finds the victim’s active verification row and deletes it before checking whether the supplied code matches. Because the email callback has no server-side attempt limit, repeated requests can continuously invalidate newly issued codes and prevent the victim from signing in.

**How this was verified:** The unauthenticated email callback forwards the supplied identifier and token to this adapter, which selects by identifier and deletes using the stored token before comparing it with the supplied token.

**Knowledge Base Used:** [Data and identity platform](https://app.greptile.com/cap/-/custom-context/knowledge-base/capsoftware/cap/-/docs/data-and-identity-platform.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Token consumption remains non-atomic

Concurrent callbacks can both read the live row before either deletes it. Since the deletion result is ignored, a request whose deletion affected no rows still validates and returns its previously read token. This allows parallel redemption of a nominally single-use code and lets concurrent guesses race the intended burn. Require deletion of exactly one row, or consume the token atomically, before returning it.

How this was verified: The code performs separate pooled SELECT and DELETE statements and returns the previously selected row without checking whether its DELETE removed anything.

Knowledge Base Used: Data and identity platform

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/database/auth/drizzle-adapter.ts
Line: 477-484

Comment:
**Token consumption remains non-atomic**

Concurrent callbacks can both read the live row before either deletes it. Since the deletion result is ignored, a request whose deletion affected no rows still validates and returns its previously read token. This allows parallel redemption of a nominally single-use code and lets concurrent guesses race the intended burn. Require deletion of exactly one row, or consume the token atomically, before returning it.

**How this was verified:** The code performs separate pooled SELECT and DELETE statements and returns the previously selected row without checking whether its DELETE removed anything.

**Knowledge Base Used:** [Data and identity platform](https://app.greptile.com/cap/-/custom-context/knowledge-base/capsoftware/cap/-/docs/data-and-identity-platform.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Mocks ignore database predicates

The SELECT mock always returns its configured row without checking the WHERE clause, and the DELETE mock only records that it was called. These tests would therefore still pass if the implementation queried or deleted the wrong row—for example, if it queried by the incorrect guessed token and a real database would leave the active token untouched. Assert the predicates against seeded rows or use a data-backed database fixture so the tests protect the security behavior.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/web/__tests__/unit/use-verification-token.test.ts
Line: 16-33

Comment:
**Mocks ignore database predicates**

The SELECT mock always returns its configured row without checking the WHERE clause, and the DELETE mock only records that it was called. These tests would therefore still pass if the implementation queried or deleted the wrong row—for example, if it queried by the incorrect guessed token and a real database would leave the active token untouched. Assert the predicates against seeded rows or use a data-backed database fixture so the tests protect the security behavior.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!