fix(service): verify-totp reports a store fault as 500, uncounted (OC-0377)

challengeSecret folded a GetUserByID error into the same 401 "invalid or
expired two-factor challenge" an expired challenge earns, and the attempt
had already been charged to the per-user totp_fail cap. Split the outcome:
a store error logs and returns the new service.ErrTOTPUnavailable
(ErrInternal, "two-factor verification temporarily unavailable"); an unknown
user or a missing secret still answers 401. The limiter reservation moves
after the store read — the rule authenticate already applies — and still
precedes the code compare, so the check-then-act it closes stays closed.

Characterization row flipped in the same commit: `VerifyTOTPFailurePaths/
user lookup fails -> 500, challenge kept, attempt not counted` — after the
fault ten wrong codes still answer 401 (the tenth would be 429 had the
fault counted), then the eleventh is refused. `per-user failure cap spans
challenges` unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A17Uq3d2C36rN82Jitf3wo

Server/api/auth_characterization_test.go

@@ -777,16 +777,31 @@ func TestAuthCharacterization_VerifyTOTPFailurePaths(t *testing.T) {
 	}
 	const challengeGone = "invalid or expired two-factor challenge"
 
-	t.Run("user lookup fails -> 401", func(t *testing.T) {
+	t.Run("user lookup fails -> 500, challenge kept, attempt not counted", func(t *testing.T) {
 		database, router, _, secret := setup(t)
 		pt := loginPartial(t, router, "two", "correctPass1", "", "")
 		hideTable(t, database, "users")
-		// known: totpChallengeSecret folds a database error into the same 401
-		// an expired challenge gets, so a DB fault during the second factor
-		// reads as a bad code and burns the caller's attempt budget (ledger
-		// OC-0377). The plan's rule is 5xx for a non-sentinel error; pinned
-		// as-is for B3-2, fixed in B3-9.
-		wantErr(t, verify(t, router, pt, totpCode(t, secret), ""), http.StatusUnauthorized, "UNAUTHORIZED", challengeGone)
+		// OC-0377 (fixed in B3-9): a store fault while loading the challenged
+		// user is an outage, not a bad challenge — 5xx, the challenge stays
+		// live, and the attempt is not charged to the per-user totp_fail cap.
+		wantErr(t, verify(t, router, pt, totpCode(t, secret), ""), http.StatusInternalServerError, "INTERNAL_ERROR", "two-factor verification temporarily unavailable")
+		if _, err := database.ExecContext(context.Background(), `ALTER TABLE users_gone RENAME TO users`); err != nil {
+			t.Fatalf("restore users: %v", err)
+		}
+		// The cap is 10 per user: ten wrong codes must all still answer 401
+		// "invalid two-factor code" — had the faulted attempt counted, the
+		// tenth would be the 429. The first five ride the surviving challenge
+		// (which proves it was kept), the next five a fresh one.
+		attempt := 0
+		for _, token := range []string{pt, loginPartial(t, router, "two", "correctPass1", "", "")} {
+			for range 5 {
+				attempt++
+				wantErr(t, verify(t, router, token, wrongTOTPCode(t, secret), fmt.Sprintf("203.0.113.%d", attempt)), http.StatusUnauthorized, "UNAUTHORIZED", "invalid two-factor code")
+			}
+		}
+		// Negative control: the counter is live — the eleventh attempt is refused.
+		pt = loginPartial(t, router, "two", "correctPass1", "", "")
+		wantErr(t, verify(t, router, pt, totpCode(t, secret), "203.0.113.99"), http.StatusTooManyRequests, "RATE_LIMITED", "too many failed attempts, try again later")
 	})
 	t.Run("secret removed after the challenge was issued -> 401", func(t *testing.T) {
 		database, router, uid, secret := setup(t)

Server/service/auth.go

@@ -216,7 +216,10 @@ var (
 	// Second factor.
 	ErrTOTPChallengeInvalid = &authError{ErrUnauthorized, "invalid or expired two-factor challenge"}
 	ErrTOTPSecretUnreadable = &authError{ErrInternal, "failed to verify two-factor code"}
-	ErrTOTPCodeInvalid      = &authError{ErrUnauthorized, "invalid two-factor code"}
+	// ErrTOTPUnavailable is a store fault while loading the challenged user
+	// (OC-0377): an outage, not a bad challenge, so no attempt is charged.
+	ErrTOTPUnavailable = &authError{ErrInternal, "two-factor verification temporarily unavailable"}
+	ErrTOTPCodeInvalid = &authError{ErrUnauthorized, "invalid two-factor code"}
 	// ErrTOTPAlreadyEnabled is written by the transport as 409
 	// TOTP_ALREADY_ENABLED, not the generic CONFLICT code.
 	ErrTOTPAlreadyEnabled   = &authError{ErrConflict, "disable 2FA before re-enabling"}
@@ -472,6 +475,11 @@ func (s *AuthService) VerifyTOTP(ctx context.Context, partialToken, code string)
 		return nil, ErrTOTPChallengeInvalid
 	}
 
+	user, secret, err := s.challengeSecret(ctx, challenge.UserID)
+	if err != nil {
+		return nil, err
+	}
+
 	totpRateLimitKey := auth.Key("totp_fail", challenge.UserID)
 	// Atomically record this attempt and reject once the per-user failure cap
 	// is reached. Recording up-front — rather than a read-only Check now and
@@ -485,15 +493,13 @@ func (s *AuthService) VerifyTOTP(ctx context.Context, partialToken, code string)
 	// multiplier exists for shared-NAT per-IP limits; scaling a per-user
 	// threshold with it would hand a distributed attacker more guesses.
 	// Mirrors loginUserFailureThreshold staying unscaled in authenticate.
+	// The reservation sits after the store read above, as in authenticate,
+	// so an outage does not consume attempts (OC-0377); it still precedes
+	// the code compare, the check-then-act the up-front record closes.
 	if !s.limiter.Allow(totpRateLimitKey, totpFailureRateLimit, totpFailureWindow) {
 		return nil, ErrTooManyAttempts
 	}
 
-	user, secret, err := s.challengeSecret(ctx, challenge.UserID)
-	if err != nil {
-		return nil, err
-	}
-
 	if !auth.VerifyTOTPCodeOnce(secret, strings.TrimSpace(code), time.Now().UTC(), user.ID, s.usedCodes) {
 		// The attempt was already recorded atomically up-front via
 		// limiter.Allow; only the per-partial-token counter is advanced here.
@@ -522,7 +528,14 @@ func (s *AuthService) VerifyTOTP(ctx context.Context, partialToken, code string)
 // returns their decrypted TOTP secret.
 func (s *AuthService) challengeSecret(ctx context.Context, challengeUserID int64) (*db.User, string, error) {
 	user, err := s.st.GetUserByID(ctx, challengeUserID)
-	if err != nil || user == nil || user.TOTPSecret == nil {
+	if err != nil {
+		// GetUserByID answers (nil, nil) for an unknown user, so a non-nil
+		// error is a store fault: report the outage, not a bad challenge
+		// (OC-0377). VerifyTOTP records no attempt for it.
+		slog.Error("verify-totp: GetUserByID failed", "err", err, "user_id", challengeUserID)
+		return nil, "", ErrTOTPUnavailable
+	}
+	if user == nil || user.TOTPSecret == nil {
 		return nil, "", ErrTOTPChallengeInvalid
 	}