fix(auth): decide a single winner when two refreshes race one token

[Bccorb]

Closes #279 (P1).

What was wrong

Rotation reads the session, checks it has not already been rotated, creates the replacement and links the two, in four statements with no transaction, row lock or conditional write. Two refreshes carrying the same token both pass the reuse check and both write the link. The second write wins.

Both callers then hold working refresh tokens, and the replacement belonging to the first is live while reachable from nothing. When the token is presented again, revokeSessionChain starts at the old session and follows replacedBySessionId forward, so it reaches the winner and stops. The other session survives.

That is the case reuse detection exists for. Someone who copied a refresh token and raced the legitimate client kept a session that the revocation triggered by that theft could not reach, while the trail recorded refresh_token_suspicious and reported that it had fired.

The fix

claimSessionRotation writes the link conditional on it still being unset (and the session not revoked), in one statement the database serialises:

UPDATE sessions SET "replacedBySessionId" = :new
WHERE id = :old AND "replacedBySessionId" IS NULL AND "revokedAt" IS NULL

No affected rows means another rotation got there first, which is the same condition an already rotated token presents, and it is answered the same way:

1. the replacement this request created is revoked with rotation_race_lost, since it is reachable from nothing and its refresh token was never returned to anyone,
2. the session is reloaded, so the chain walk follows the link the winner wrote rather than the null this instance still holds, which would stop at the old session and leave the winner live,
3. the chain is revoked from there, refresh_token_suspicious is recorded, and the caller gets 401 refresh_token_reused.

The reload in step 2 is the part that is easy to leave out and would quietly reproduce the original defect.

The tradeoff, stated

Two legitimate refreshes racing each other now end the session chain, the same as presenting a rotated token twice in sequence does. A client that fires concurrent refreshes of one token signs its user out.

That is the direction this has to fail. The alternative, letting the loser fail quietly while the winner keeps its session, means whoever wins the race keeps a session, and the winner is not always the client that should have it: if the attacker wins, the theft goes undetected entirely. The changeset and docs/security-posture.md both say so.

Noted, not fixed here

The security review turned up one pre-existing imperfection this change does not touch: revokeSessionChain reads current.replacedBySessionId from the in-memory instance after save(), so a rotation of a chain member committing between that node's findByPk and its save would be missed. Exploiting it requires a full network round trip plus several queries to fit inside one local UPDATE, and any second loser's walk re-reads the link and catches it, so it is theoretical rather than practical. Happy to file it separately if you want it tracked.

Tests

Three added. The controller test fails on main:

× refuses the rotation that lost the race and revokes what it made
AssertionError: expected 200 to be 401
Tests  1 failed | 36 passed (37)

The other two cover claimSessionRotation directly: the where clause it issues, and that it leaves the instance alone when it loses.

Checks

• npm run typecheck, npm run lint, npm run format:check, npm run build: clean
• npm run test:run: 113 files, 1336 passed, 1 skipped, 1 todo
• npm run coverage: 98.75% statements, 96.02% branches, 98.96% functions, 98.95% lines, all above threshold
• /security-review on the branch: no findings. It confirmed Sequelize 6.37.8 emits IS NULL rather than = NULL here, that Model.update returns a real rowCount with no hooks on Session, and that claimSessionRotation is now the only writer of replacedBySessionId in src/.