Pre-Merge Bug Review with Independent Repros

ADVERSARIAL PRE-MERGE REVIEW: Auth Refactor
Branch: feat/session-redesign | Target: production | Risk Level: CRITICAL

REVIEWER TEAM OUTPUT

LOGIC REVIEWER
• Finding 1: Session rotation logic runs twice on refresh token
  → Impact: Users may see double sign-outs in logs
  → Severity: Medium
  → Repro: Open two tabs, click refresh in both within 1 second
• Finding 2: No validation that old session is revoked before new session is issued
  → Impact: Session tokens can multiply if network retries
  → Severity: High
  → Repro script: Send concurrent refresh requests, capture all returned tokens

SECURITY REVIEWER
• Finding 1: Timing attack possible on token comparison
  → Vulnerable code: if (token == storedToken) instead of constant-time compare
  → Impact: Attackers brute-force tokens at 100 attempts/second
  → Severity: Critical
  → Repro: time for token in {AAA,AAB,...}; measure response times
• Finding 2: No rate limiting on failed auth attempts
  → Impact: Account lockout bruteforce via 10,000 login attempts
  → Severity: High
  → Repro: curl -X POST /auth with wrong password 10k times

PERFORMANCE REVIEWER
• Finding: Database query for every session check (no caching)
  → Impact: 500ms latency added to every request at 5k req/sec
  → Severity: High
  → Repro: Load test with 1000 concurrent users

CRITICAL BLOCKERS: 2
1. Timing attack (use constant-time comparison library)
2. Rate limiting (add Redis counter, 10-request/minute limit)

HIGH PRIORITY: 2
1. Session duplication on retry (idempotent key)
2. Database caching (5-minute TTL in Redis)

RECOMMENDATION: Do not merge until critical blockers fixed. Estimated fix time: 6 hours. Suggest staging deployment with canary rollout (5% traffic) before full production release.