Fix F-03: make token renewal atomic

- db/accounts.go: add RenewToken(oldJTI, reason, newJTI,
  accountID, issuedAt, expiresAt) which wraps RevokeToken +
  TrackToken in a single BEGIN/COMMIT transaction; if either
  step fails the whole tx rolls back, so the user is never
  left with neither old nor new token valid
- server.go (handleRenewToken): replace separate RevokeToken +
  TrackToken calls with single RenewToken call; failure now
  returns 500 instead of silently losing revocation
- grpcserver/auth.go (RenewToken): same replacement
- db/db_test.go: TestRenewTokenAtomic verifies old token is
  revoked with correct reason, new token is tracked and not
  revoked, and a second renewal on the already-revoked old
  token returns an error
- AUDIT.md: mark F-03 as fixed
Security: without atomicity a crash/error between revoke and
  track could leave the old token active alongside the new one
  (two live tokens) or revoke the old token without tracking
  the new one (user locked out). The transaction ensures
  exactly one of the two tokens is valid at all times.

AUDIT.md

@@ -222,7 +222,7 @@ The REST `handleTokenIssue` and gRPC `IssueServiceToken` both revoke the existin
 |--------|----|----------|-------|--------|
 | Yes | F-01 | MEDIUM | TOTP enrollment sets required=1 before confirmation | Small |
 | No  | F-02 | MEDIUM | Password in HTML hidden fields during TOTP step | Medium |
-| No  | F-03 | MEDIUM | Token renewal not atomic (race window) | Small |
+| Yes | F-03 | MEDIUM | Token renewal not atomic (race window) | Small |
 | Yes | F-04 | MEDIUM | Rate limiter not applied to REST login endpoint | Small |
 | Yes | F-11 | MEDIUM | Missing security headers on UI responses | Small |
 | No  | F-05 | LOW | No `nbf` claim in issued JWTs | Trivial |

internal/db/accounts.go

@@ -559,6 +559,56 @@ func (db *DB) RevokeToken(jti, reason string) error {
 	return nil
 }
 
+// RenewToken atomically revokes the old token and tracks the new one in a
+// single SQLite transaction.
+//
+// Security: the two operations must be atomic so that a failure between them
+// cannot leave the user with neither a valid old token nor a new one.  With
+// MaxOpenConns(1) and SQLite's serialised write path, BEGIN IMMEDIATE acquires
+// the write lock immediately and prevents any other writer from interleaving.
+func (db *DB) RenewToken(oldJTI, reason, newJTI string, accountID int64, issuedAt, expiresAt time.Time) error {
+	tx, err := db.sql.Begin()
+	if err != nil {
+		return fmt.Errorf("db: renew token begin tx: %w", err)
+	}
+	defer func() { _ = tx.Rollback() }()
+
+	n := now()
+
+	// Revoke the old token.
+	result, err := tx.Exec(`
+		UPDATE token_revocation
+		SET revoked_at = ?, revoke_reason = ?
+		WHERE jti = ? AND revoked_at IS NULL
+	`, n, nullString(reason), oldJTI)
+	if err != nil {
+		return fmt.Errorf("db: renew token revoke old %q: %w", oldJTI, err)
+	}
+	rows, err := result.RowsAffected()
+	if err != nil {
+		return fmt.Errorf("db: renew token revoke rows affected: %w", err)
+	}
+	if rows == 0 {
+		return fmt.Errorf("db: renew token: old token %q not found or already revoked", oldJTI)
+	}
+
+	// Track the new token.
+	_, err = tx.Exec(`
+		INSERT INTO token_revocation (jti, account_id, issued_at, expires_at)
+		VALUES (?, ?, ?, ?)
+	`, newJTI, accountID,
+		issuedAt.UTC().Format(time.RFC3339),
+		expiresAt.UTC().Format(time.RFC3339))
+	if err != nil {
+		return fmt.Errorf("db: renew token track new %q: %w", newJTI, err)
+	}
+
+	if err := tx.Commit(); err != nil {
+		return fmt.Errorf("db: renew token commit: %w", err)
+	}
+	return nil
+}
+
 // RevokeAllUserTokens revokes all non-expired, non-revoked tokens for an account.
 func (db *DB) RevokeAllUserTokens(accountID int64, reason string) error {
 	n := now()

internal/db/db_test.go

@@ -326,6 +326,55 @@ func TestPGCredentials(t *testing.T) {
 	}
 }
 
+func TestRenewTokenAtomic(t *testing.T) {
+	db := openTestDB(t)
+	acct, err := db.CreateAccount("judy", model.AccountTypeHuman, "hash")
+	if err != nil {
+		t.Fatalf("CreateAccount: %v", err)
+	}
+
+	now := time.Now().UTC()
+	exp := now.Add(time.Hour)
+
+	// Set up the old token.
+	oldJTI := "renew-old-jti"
+	newJTI := "renew-new-jti"
+	if err := db.TrackToken(oldJTI, acct.ID, now, exp); err != nil {
+		t.Fatalf("TrackToken old: %v", err)
+	}
+
+	// RenewToken should atomically revoke old and track new.
+	if err := db.RenewToken(oldJTI, "renewed", newJTI, acct.ID, now, exp); err != nil {
+		t.Fatalf("RenewToken: %v", err)
+	}
+
+	// Old token must be revoked.
+	oldRec, err := db.GetTokenRecord(oldJTI)
+	if err != nil {
+		t.Fatalf("GetTokenRecord old: %v", err)
+	}
+	if !oldRec.IsRevoked() {
+		t.Error("old token should be revoked after RenewToken")
+	}
+	if oldRec.RevokeReason != "renewed" {
+		t.Errorf("old token revoke reason = %q, want %q", oldRec.RevokeReason, "renewed")
+	}
+
+	// New token must be tracked and not revoked.
+	newRec, err := db.GetTokenRecord(newJTI)
+	if err != nil {
+		t.Fatalf("GetTokenRecord new: %v", err)
+	}
+	if newRec.IsRevoked() {
+		t.Error("new token should not be revoked")
+	}
+
+	// RenewToken on an already-revoked old token must fail (atomicity guard).
+	if err := db.RenewToken(oldJTI, "renewed", "other-jti", acct.ID, now, exp); err == nil {
+		t.Error("expected error when renewing an already-revoked token")
+	}
+}
+
 func TestRevokeAllUserTokens(t *testing.T) {
 	db := openTestDB(t)
 	acct, err := db.CreateAccount("ivan", model.AccountTypeHuman, "hash")

internal/grpcserver/auth.go

@@ -149,8 +149,9 @@ func (a *authServiceServer) RenewToken(ctx context.Context, _ *mciasv1.RenewToke
 		return nil, status.Error(codes.Internal, "internal error")
 	}
 
-	_ = a.s.db.RevokeToken(claims.JTI, "renewed")
-	if err := a.s.db.TrackToken(newClaims.JTI, acct.ID, newClaims.IssuedAt, newClaims.ExpiresAt); err != nil {
+	// Security: revoke old + track new in a single transaction (F-03) so that a
+	// failure between the two steps cannot leave the user with no valid token.
+	if err := a.s.db.RenewToken(claims.JTI, "renewed", newClaims.JTI, acct.ID, newClaims.IssuedAt, newClaims.ExpiresAt); err != nil {
 		return nil, status.Error(codes.Internal, "internal error")
 	}
 

internal/server/server.go

@@ -284,12 +284,10 @@ func (s *Server) handleRenew(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// Revoke the old token and track the new one atomically is not possible
-	// in SQLite without a transaction. We do best-effort: revoke old, track new.
-	if err := s.db.RevokeToken(claims.JTI, "renewed"); err != nil {
-		s.logger.Error("revoke old token on renew", "error", err)
-	}
-	if err := s.db.TrackToken(newClaims.JTI, acct.ID, newClaims.IssuedAt, newClaims.ExpiresAt); err != nil {
+	// Security: revoke old + track new in a single transaction (F-03) so that a
+	// failure between the two steps cannot leave the user with no valid token.
+	if err := s.db.RenewToken(claims.JTI, "renewed", newClaims.JTI, acct.ID, newClaims.IssuedAt, newClaims.ExpiresAt); err != nil {
+		s.logger.Error("renew token atomic", "error", err)
 		middleware.WriteError(w, http.StatusInternalServerError, "internal error", "internal_error")
 		return
 	}