fix(auth): prevent password-reset target lockout and link invalidation

[sayed710]

Summary

• Remove the attacker-spendable per-target hard reset-request bucket while retaining per-IP admission.
• Issue at most one live reset token per account across API replicas, preserving existing links and bounding email. Discard only a definitively undelivered token.
• Answer reset requests before account lookup to preserve anti-enumeration timing, with tracked background delivery and graceful draining.
• Add focused in-memory and PostgreSQL two-replica regressions; document the contract in ADR-0146 and PROJECT_STATE.

Validation

• npm run build
• npm run lint
• npm test (19 hermetic workspaces, zero skips)
• npm run test:integration:postgres --workspace @chess-platform/api (62 tests)
• npm run test:integration:postgres --workspace @chess-platform/persistence (110 tests)
• npm run test:scripts (308 tests)
• ADR, topology, CI parity, deployment, engine/variant pin, and Docker build-order checks
• Mutation/falsification tests for duplicate issuance and duplicate email suppression

Migration 0039 adds a concurrent partial index for the live reset-token lookup; the common duplicate-request path avoids the user-row lock. No merge performed.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 3f5b674e-e6ac-45b6-a913-03c739ad2c32

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-code-review]

PR Summary by Qodo

Prevent password-reset lockout and preserve live links

🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Removes attacker-spendable per-target reset throttling while retaining per-IP admission.
• Preserves one live reset token and email across concurrent API replicas.
• Returns uniform early responses and drains tracked delivery work safely.

Diagram

sequenceDiagram
  actor Client
  participant Route as API Route
  participant Auth as Auth Service
  participant DB as PostgreSQL
  participant Mail as Email Provider
  Client->>Route: Reset request
  Route->>Route: Per-IP admission
  Route-->>Client: 202 Accepted
  Route->>Auth: Schedule work
  Auth->>DB: Lock user and issue
  alt Token issued
    DB-->>Auth: Issued
    Auth->>Mail: Send reset link
    alt Definitively unsent
      Mail-->>Auth: Reject or throttle
      Auth->>DB: Discard exact token
    else Delivered or ambiguous
      Mail-->>Auth: Preserve token
    end
  else Live token exists
    DB-->>Auth: Suppress issuance
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Database uniqueness constraint

• ➕ Moves the one-live-token invariant into a declarative database constraint.
• ➕ Protects against future repository implementations bypassing the issuance method.
• ➖ Requires a schema migration and explicit representation or cleanup of expired active tokens.
• ➖ A simple partial index cannot directly model time-dependent expiry safely.

2. Distributed lock service

• ➕ Could serialize issuance without locking the user row.
• ➕ Supports coordination across heterogeneous persistence backends.
• ➖ Introduces additional infrastructure and failure modes.
• ➖ Provides less durable coupling between the lock and token transaction.

Recommendation: Keep the PR's transactional user-row lock. The user row is stable, already shared by every API replica, and lets the live-token check and insertion remain in one PostgreSQL transaction without a migration. A unique active-token model may be worthwhile if token lifecycle state is redesigned later, but distributed locking is unnecessary here.

Files changed (14) +460 / -37

Bug fix (5) +100 / -20

service.tsPreserve live reset tokens through tracked background delivery +29/-8

Preserve live reset tokens through tracked background delivery

• Issues reset tokens only when none remain usable and suppresses duplicate email. Adds early scheduling, exact-token cleanup after definitive non-delivery, and recursive draining of nested background tasks.

packages/api/src/auth/service.ts

fakes.tsModel one-live-reset-token semantics in memory +16/-0

Model one-live-reset-token semantics in memory

• Implements conditional reset issuance and exact unused-token removal for hermetic API tests.

packages/api/src/fakes.ts

routes.tsReturn reset acceptance before account-specific work +8/-12

Return reset acceptance before account-specific work

• Removes target-based admission and schedules reset processing after the route commits to a uniform 202 response.

packages/api/src/routes.ts

repositories.tsSerialize reset-token issuance in PostgreSQL +43/-0

Serialize reset-token issuance in PostgreSQL

• Locks the stable user row before checking and inserting a reset token, preventing duplicate issuance across replicas. Adds exact unused-token deletion for definitive delivery failures.

packages/persistence/src/pg/repositories.ts

repositories.tsExpose conditional reset issuance and cleanup operations +4/-0

Expose conditional reset issuance and cleanup operations

• Extends the identity-token repository contract with one-live-token issuance and exact-token discard methods.

packages/persistence/src/repositories.ts

Tests (5) +315 / -13

password-reset-safety.integration.test.tsVerify reset issuance across PostgreSQL-backed replicas +122/-0

Verify reset issuance across PostgreSQL-backed replicas

• Adds two-replica concurrency coverage for single issuance, bounded email, hashed storage, consumption, expiry, and race-safe token cleanup.

packages/api/test/password-reset-safety.integration.test.ts

password-reset-safety.test.tsCover password-reset safety and delivery outcomes +170/-0

Cover password-reset safety and delivery outcomes

• Tests rotating-source attacks, link preservation, response parity, non-blocking lookup, definitive rejection cleanup, ambiguous delivery retention, and graceful draining.

packages/api/test/password-reset-safety.test.ts

rate-limit-structure.test.tsEnforce the reset route's single admission bucket +10/-2

Enforce the reset route's single admission bucket

• Removes password reset from multi-bucket expectations and asserts that only the per-IP key remains.

packages/api/test/rate-limit-structure.test.ts

rate-limit.test.tsVerify per-IP-only reset throttling +9/-11

Verify per-IP-only reset throttling

• Confirms requests from rotating sources cannot exhaust a target allowance while repeated requests from one IP remain limited.

packages/api/test/rate-limit.test.ts

recovery.test.tsSynchronize recovery tests with background reset work +4/-0

Synchronize recovery tests with background reset work

• Drains tracked authentication tasks before asserting reset-email delivery and using generated tokens.

packages/api/test/recovery.test.ts

Documentation (3) +45 / -2

PROJECT_STATE.mdRecord the password-reset safety increment +11/-1

Record the password-reset safety increment

• Documents the removal of target lockout, one-live-token behavior, asynchronous response contract, delivery cleanup, and regression coverage.

docs/PROJECT_STATE.md

0145-login-step-up.mdLink login step-up limits to the reset safety decision +1/-1

Link login step-up limits to the reset safety decision

• Clarifies that ADR-0146 replaces the previously documented password-reset per-target limit.

docs/adr/0145-login-step-up.md

0146-password-reset-request-safety.mdDefine the safe password-reset request contract +33/-0

Define the safe password-reset request contract

• Records the threat model and decision to combine per-IP admission, serialized one-live-token issuance, early uniform responses, and outcome-aware cleanup.

docs/adr/0146-password-reset-request-safety.md

Other (1) +0 / -2

config.tsRemove password-reset per-target rate-limit configuration +0/-2

Remove password-reset per-target rate-limit configuration

• Deletes the attacker-spendable target bucket while retaining the existing per-IP reset-request limit.

packages/api/src/config.ts

[greptile-apps]

Confidence Score: 5/5

[Critical risk] Rewrites password-reset token issuance and rate limiting.

The PR appears safe to merge, with the previously identified ambiguous-delivery recovery limitation remaining non-blocking.

Summary

The PR removes the attacker-spendable per-target password-reset limit while retaining per-IP admission. It answers requests before account lookup, limits each account to one live reset token and email, and adds a live-token index, documentation, and in-memory and PostgreSQL regressions.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Reset request] --> B[Per-IP admission]
  B --> C[Return 202]
  C --> D[Tracked background lookup]
  D --> E{Live token?}
  E -->|Yes| F[Keep existing link; send no email]
  E -->|No| G[Lock user and recheck]
  G --> H[Issue token and send email]
  H --> I{Definitely unsent?}
  I -->|Yes| J[Discard that unused token]
  I -->|No or uncertain| K[Keep token]

Loading

_{Reviews (2) · Last reviewed commit: "perf(auth): index and bypass live reset-..."}

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Reset spam can occupy the DB pool ✓ Resolved 🐞 Bug ➹ Performance

Description

issuePasswordReset acquires a pooled connection and an exclusive user-row lock before checking for
an already-live token, and that check has no supporting identity-token index. After the target
bucket is removed, requests from rotating addresses for one known account serialize while holding
pool clients and repeatedly scan the growing token table, potentially starving unrelated database
work.

Code

packages/persistence/src/pg/repositories.ts[1032]

+      const owner = await client.query('SELECT id FROM users WHERE id = $1 FOR UPDATE', [token.userId]);

Relevance

●● Moderate

Pool exhaustion concerns align with accepted connection-capacity findings, but no close precedent
specifically requires indexing this query.

PR-#55

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The public route now retains only the per-IP bucket, so rotating source addresses can submit
unlimited requests for the same target. Every matching request connects to PostgreSQL and takes `FOR
UPDATE before querying identity_tokens`; the original table has only the token-hash primary key,
while the only later identity-token index is scoped to login step-up tokens, leaving this
password-reset predicate unsupported.

packages/api/src/routes.ts[654-661]
packages/persistence/src/pg/repositories.ts[1027-1042]
packages/persistence/migrations/0007_identity_hardening.sql[7-13]
packages/persistence/migrations/0038_login_step_up_index.sql[1-3]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Repeated public reset requests acquire a database client and user-row lock before checking for the live token, while the check lacks a supporting index. Distributed requests for one account can therefore queue transactions, repeatedly scan token history, and exhaust the shared pool.

## Fix Focus Areas
- packages/persistence/src/pg/repositories.ts[1027-1053]
- packages/persistence/migrations/0007_identity_hardening.sql[7-13]
- packages/api/src/routes.ts[654-661]

## Recommended Fix
Add an index supporting live password-reset lookup by user, kind, unused status, and expiry. Perform an indexed live-token check before opening the locking transaction so the common suppression path returns immediately; when no token is found, acquire the serialization lock and repeat the check inside the transaction before inserting to preserve concurrency correctness.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Project status claims a future update ✗ Dismissed 🐞 Bug ⚙ Maintainability

Description

PROJECT_STATE.md records its “Last updated” date as 2026-09-26 even though the supplied current
date is 2026-09-25. The new password-reset ADR is dated the same future day, so later maintainers
following the project-state chronology will see this increment as having been recorded after the
present review date.

Code

docs/PROJECT_STATE.md[9]

+_Last updated: 2026-09-26 — M15 Increment 68: Password-reset request safety without target lockout._

Relevance

●●● Strong

Future-dated project chronology is a deterministic documentation error; recent project-state
consistency corrections were accepted.

PR-#62
PR-#35

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The project-state header introduced by this PR says it was last updated on 2026-09-26, and the new
ADR carries the same date; both conflict with the supplied current date of 2026-09-25.

docs/PROJECT_STATE.md[6-13]
docs/adr/0146-password-reset-request-safety.md[1-4]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The project-state entry and the newly added ADR are dated one day after the current review date, which makes the repository chronology inaccurate.

## Fix Focus Areas
- docs/PROJECT_STATE.md[9-13]
- docs/adr/0146-password-reset-request-safety.md[3-4]

## Recommended Fix
Change the added increment and ADR dates to the actual update date, 2026-09-25, keeping the project-state and ADR timestamps consistent.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

Review mode: 🧠 Deep: This security-sensitive authentication change spans API behavior, asynchronous delivery and shutdown, distributed PostgreSQL token issuance, rate limiting, and multiple independent regression paths, creating a high density of subtle defects that benefits from redundant review passes.

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗