security(totp): make recovery-code redemption single-use under concurrency

[lakhansamani]

The bug

ValidateRecoveryCode read the recovery-code blob, decided in Go whether the code was unconsumed, and wrote the mutated blob back with an unconditional UpdateAuthenticator. Nothing made the write conditional on the state it had just read.

Two consequences, both reproduced:

1. Double-spend. Concurrent redemptions of the same recovery code all read it unconsumed and all returned true. A probe measured 3 of 8 concurrent attempts accepted. A recovery code's entire security property is that it works once; without that, one leaked code is an unlimited supply of logins that bypass MFA.
2. Lost update. Concurrent redemptions of different codes clobbered each other — the last writer's blob won and the other code silently reverted to unconsumed.

The fix

The write decides, not the read.

New storage primitive, modelled on the existing DeleteSessionTokenByUserIDAndKey contract (internal/storage/provider.go:177):

ConsumeAuthenticatorRecoveryCode(ctx, id, oldCodes, newCodes string) (bool, error)

The swap lands only while the row still holds oldCodes, decided by one atomic database operation — never a read followed by an unconditional write. Implemented natively per backend rather than emulated:

Backend	Construct
SQL	WHERE id = ? AND recovery_codes = ? → RowsAffected
MongoDB	expected blob in the UpdateOne filter → MatchedCount
Cassandra/Scylla	lightweight transaction (IF recovery_codes = ?) on the partition key
DynamoDB	UpdateItem + ConditionExpression
ArangoDB	AQL FILTER … UPDATE … RETURN NEW
Couchbase	KV Get → compare → Replace with the document's CAS

ValidateRecoveryCode becomes a bounded compare-and-swap loop: on a lost race it re-reads, finds the code consumed, and rejects.

Two contract details worth review attention:

• oldCodes is compared byte for byte, so the caller passes the exact string it read. Re-marshalling the map would reorder keys and match nothing.
• Exhausting the retries returns an error, not false. The write failed; the credential did not. Answering "invalid" would spend a user's recovery code on a database problem — the "outage reported as bad input" failure AGENTS.md warns about. The existing caller in verify_otp.go already routes a non-nil error away from the lockout counter, so no caller change was needed.
• Retry bound is the recovery-code count (10), not an arbitrary 3. Every lost swap means another code was spent, and a user only has ten — so that count is the worst case for legitimate contention.

Tests

Both new integration subtests were confirmed to fail against the pre-fix implementation before being kept.

• internal/integration_tests — 8 racers × 25 rounds on one code → exactly one success; two distinct codes raced × 25 rounds → both succeed, all ten codes survive, only the two redeemed are marked consumed.
• internal/authenticators/totp — new unit suite over a miniature correct backend: 16 racers × 50 rounds, retry-after-lost-race (asserts each retry re-reads), the exact interleaving where the code is spent between read and swap, exhaustion-is-a-fault, storage-error propagation, rejections that must not write, only-the-matched-code-changes, legacy plaintext codes, corrupt blob.
• internal/storage/provider_test.go — parity subtest run against all 7 backends: a stale expectation is refused without writing, a matching one lands and stores the blob verbatim, the same before-blob cannot win twice, and an absent row is (false, nil) rather than an error.

Verification

• go build ./..., go vet ./... — clean
• make test (SQLite) — pass
• make test-all-db — exit 0; the parity subtest passes on all seven: couchbase, postgres, sqlite, mongodb, arangodb, scylladb, dynamodb
• make lint — 0 issues