fix: COD-29 relax auth lockout recovery

src/web/middleware/auth.ts

@@ -8,7 +8,7 @@
  * - CORS (localhost only)
  */
 
-import { FastifyInstance } from 'fastify';
+import type { FastifyInstance, FastifyReply } from 'fastify';
 import { randomBytes, timingSafeEqual } from 'node:crypto';
 import { StaleExpirationMap } from '../../utils/index.js';
 import type { AuthSessionRecord } from '../ports/auth-port.js';
@@ -69,6 +69,13 @@ export function registerAuthMiddleware(app: FastifyInstance, https: boolean): Au
   const authSessions = state.authSessions;
   const authFailures = state.authFailures;
 
+  function sendAuthRateLimit(reply: FastifyReply, clientIp: string): void {
+    const remainingMs = authFailures.getRemainingTtl(clientIp) ?? AUTH_FAILURE_WINDOW_MS;
+    const retryAfterSeconds = Math.max(1, Math.ceil(remainingMs / 1000));
+    reply.header('Retry-After', String(retryAfterSeconds));
+    reply.code(429).send('Too Many Requests — try again later');
+  }
+
   app.addHook('onRequest', (req, reply, done) => {
     // Hook events come from local Claude Code hooks (curl from localhost) — no auth headers available.
     // Safe: validated by HookEventSchema, only triggers broadcasts.
@@ -90,13 +97,6 @@ export function registerAuthMiddleware(app: FastifyInstance, https: boolean): Au
 
     const clientIp = req.ip;
 
-    // Rate limit: reject if too many failed attempts from this IP
-    const failures = authFailures.get(clientIp) ?? 0;
-    if (failures >= AUTH_FAILURE_MAX) {
-      reply.code(429).send('Too Many Requests — try again later');
-      return;
-    }
-
     // Check session cookie first (avoids re-sending credentials on every request)
     // Use get() instead of has() so refreshOnGet extends the TTL on active sessions
     const sessionToken = req.cookies[AUTH_COOKIE_NAME];
@@ -140,6 +140,13 @@ export function registerAuthMiddleware(app: FastifyInstance, https: boolean): Au
       return;
     }
 
+    // Rate limit only requests that failed to authenticate on this attempt.
+    const failures = authFailures.get(clientIp) ?? 0;
+    if (failures >= AUTH_FAILURE_MAX) {
+      sendAuthRateLimit(reply, clientIp);
+      return;
+    }
+
     // Auth failed — track failure count
     authFailures.set(clientIp, failures + 1);
 

test/auth-security.test.ts

@@ -10,20 +10,53 @@
  *
  * Port: 3160 (auth tests), 3161 (loopback no-auth tests), 3162 (network override tests)
  */
-import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach, vi } from 'vitest';
 import { WebServer } from '../src/web/server.js';
+import { TmuxManager } from '../src/tmux-manager.js';
 import { SettingsUpdateSchema } from '../src/web/schemas.js';
 
 const AUTH_PORT = 3160;
 const NOAUTH_PORT = 3161;
 const NETWORK_OVERRIDE_PORT = 3162;
+const AUTH_RATE_LIMIT_PORT = 3220;
 const TEST_USER = 'admin';
 const TEST_PASS = 'test-password-12345';
 
+vi.spyOn(TmuxManager, 'isTmuxAvailable').mockReturnValue(true);
+
 function basicAuthHeader(user: string, pass: string): string {
   return 'Basic ' + Buffer.from(`${user}:${pass}`).toString('base64');
 }
 
+async function startAuthServer(port: number): Promise<{ server: WebServer; baseUrl: string }> {
+  process.env.CODEMAN_PASSWORD = TEST_PASS;
+  process.env.CODEMAN_USERNAME = TEST_USER;
+  const server = new WebServer(port, false, true);
+  await server.start();
+  return { server, baseUrl: `http://localhost:${port}` };
+}
+
+async function getSessionCookie(baseUrl: string): Promise<string> {
+  const res = await fetch(`${baseUrl}/api/status`, {
+    headers: { Authorization: basicAuthHeader(TEST_USER, TEST_PASS) },
+  });
+  expect(res.status).toBe(200);
+  const setCookie = res.headers.get('set-cookie');
+  expect(setCookie).toBeTruthy();
+  const cookieMatch = setCookie!.match(/codeman_session=([^;]+)/);
+  expect(cookieMatch).toBeTruthy();
+  return `codeman_session=${cookieMatch![1]}`;
+}
+
+async function exhaustAuthFailures(baseUrl: string, prefix: string): Promise<void> {
+  for (let i = 0; i < 10; i++) {
+    const res = await fetch(`${baseUrl}/api/status`, {
+      headers: { Authorization: basicAuthHeader(TEST_USER, `${prefix}-${i}`) },
+    });
+    expect(res.status).toBe(401);
+  }
+}
+
 describe('Auth Security', () => {
   let server: WebServer;
   let baseUrl: string;
@@ -155,29 +188,63 @@ describe('Auth Security', () => {
   });
 
   describe('Rate Limiting', () => {
-    it('should block after too many failed attempts', async () => {
-      // Send 10 failed attempts
-      for (let i = 0; i < 10; i++) {
-        await fetch(`${baseUrl}/api/status`, {
-          headers: { Authorization: basicAuthHeader(TEST_USER, 'wrong-' + i) },
-        });
-      }
+    let rateServer: WebServer;
+    let rateBaseUrl: string;
 
-      // 11th attempt should be rate-limited
-      const res = await fetch(`${baseUrl}/api/status`, {
+    beforeEach(async () => {
+      ({ server: rateServer, baseUrl: rateBaseUrl } = await startAuthServer(AUTH_RATE_LIMIT_PORT));
+    });
+
+    afterEach(async () => {
+      await rateServer.stop();
+    });
+
+    it('should rate-limit wrong credentials after too many failed attempts', async () => {
+      await exhaustAuthFailures(rateBaseUrl, 'cod21-wrong');
+
+      const res = await fetch(`${rateBaseUrl}/api/status`, {
         headers: { Authorization: basicAuthHeader(TEST_USER, 'wrong-again') },
       });
+
       expect(res.status).toBe(429);
+      expect(res.headers.get('retry-after')).toMatch(/^\d+$/);
     });
 
-    it('should rate-limit even with correct credentials after lockout', async () => {
-      // After being rate-limited, even correct credentials should fail
-      const res = await fetch(`${baseUrl}/api/status`, {
+    it('should allow an existing valid session cookie during auth failure lockout', async () => {
+      const cookie = await getSessionCookie(rateBaseUrl);
+      await exhaustAuthFailures(rateBaseUrl, 'cod21-cookie');
+
+      const res = await fetch(`${rateBaseUrl}/api/status`, {
+        headers: { Cookie: cookie },
+      });
+
+      expect(res.status).toBe(200);
+    });
+
+    it('should allow correct credentials to recover from auth failure lockout', async () => {
+      await exhaustAuthFailures(rateBaseUrl, 'cod21-recover');
+
+      const res = await fetch(`${rateBaseUrl}/api/status`, {
         headers: { Authorization: basicAuthHeader(TEST_USER, TEST_PASS) },
       });
-      // Rate limit is per-IP and the previous test used the same IP
-      // This test verifies rate limiting isn't bypassed by correct creds
-      expect(res.status).toBe(429);
+
+      expect(res.status).toBe(200);
+      expect(res.headers.get('set-cookie')).toContain('codeman_session=');
+    });
+
+    it('should clear failed attempt count after correct credentials recover access', async () => {
+      await exhaustAuthFailures(rateBaseUrl, 'cod21-clear');
+
+      const recoveryRes = await fetch(`${rateBaseUrl}/api/status`, {
+        headers: { Authorization: basicAuthHeader(TEST_USER, TEST_PASS) },
+      });
+      expect(recoveryRes.status).toBe(200);
+
+      const wrongAfterRecovery = await fetch(`${rateBaseUrl}/api/status`, {
+        headers: { Authorization: basicAuthHeader(TEST_USER, 'wrong-after-recovery') },
+      });
+
+      expect(wrongAfterRecovery.status).toBe(401);
     });
   });