fix(sec/d24): auth hardening — login lockout, rate limits, constant-time, trust proxy

Red-team highs/mediums (auth surface):
- staff_user gains failed_count/locked_until; login() locks an account for 15min
  after 8 consecutive failures, clears the counter on success (both engines).
- login() is now constant-time: a missing email still spends one scrypt verify
  against a fixed dummy hash — no timing/enumeration oracle. Returns a typed
  {ok:false,reason} instead of null.
- POST /auth/login gets per-IP (20/min) AND per-email (8/15min) fixed-window
  rate limiting layered over the lockout; 429 on trip.
- Credential reveal routes (db-password, module password, module secret) get a
  per-user reveal limiter (20/min) so one insider cannot bulk-decrypt everything;
  added missing getClientModule 404 guards.
- app.set(trust proxy,1) so req.ip is the real client behind nginx/Caddy and the
  per-IP limiters actually bucket per client.
- makeRateLimiter extracted to rate-limit.ts (shared, no server<->api cycle).
Tests: lockout, counter-reset, unknown-email, and existing auth all green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

apps/hq/src/api.ts

@@ -60,6 +60,7 @@ import {
 import { pullMonthlyCosts } from './aws-costs'
 import { clientProfitability, duesAging, moduleRevenue, type DateRange } from './repos-reports'
 import { listUsageRateCards, setUsageRateCard } from './repos-rate-card'
+import { makeRateLimiter } from './rate-limit'
 import {
   addProjectMilestone, ensureProjectMilestones, listProjectMilestones, milestoneBoard,
   projectsPendingMilestone, setMilestone,
@@ -107,12 +108,42 @@ export function apiRouter(
   r.get('/health', (_req, res) => {
     res.json({ ok: true, service: 'sims-hq', version: '0.1.0' })
   })
+  // D24 (red-team): brute-force defense. Two fixed-window limiters — per client IP and
+  // per target email — layered over the persistent per-account lockout in login(). The
+  // per-email bucket trips even when an attacker rotates source IPs; the per-IP bucket
+  // stops one host hammering many accounts. login() itself is constant-time.
+  const loginIpLimit = makeRateLimiter(20, 60_000)      // 20 attempts / IP / minute
+  const loginEmailLimit = makeRateLimiter(8, 15 * 60_000) // 8 attempts / email / 15 min
+  // D24 (red-team): cap credential reveals per user so one insider account cannot silently
+  // bulk-decrypt every client's banking DB password / portal secret. Every reveal is also
+  // audited; this bounds the burst. Generous for legitimate support use.
+  const revealLimit = makeRateLimiter(20, 60_000)        // 20 reveals / user / minute
+  const revealGuard = (res: Response): boolean => {
+    const ok = revealLimit((res.locals['staff'] as { id: string }).id)
+    if (!ok) res.status(429).json({ ok: false, error: 'Too many reveals in a short time. Please slow down.' })
+    return ok
+  }
   r.post('/auth/login', async (req, res) => {
     try {
       const { email, password } = req.body as { email?: string; password?: string }
-      const out = typeof email === 'string' && typeof password === 'string' ? await login(db, email, password) : null
-      if (!out) { res.status(401).json({ ok: false, error: 'Invalid email or password' }); return }
-      res.json({ ok: true, ...out })
+      const ip = req.ip ?? 'unknown'
+      if (!loginIpLimit(ip)) {
+        res.status(429).json({ ok: false, error: 'Too many attempts. Please wait a minute and try again.' }); return
+      }
+      if (typeof email !== 'string' || typeof password !== 'string') {
+        res.status(401).json({ ok: false, error: 'Invalid email or password' }); return
+      }
+      if (!loginEmailLimit(email.toLowerCase())) {
+        res.status(429).json({ ok: false, error: 'Too many attempts for this account. Please try again later.' }); return
+      }
+      const out = await login(db, email, password)
+      if (!out.ok) {
+        const msg = out.reason === 'locked'
+          ? 'This account is temporarily locked after too many failed attempts. Try again in 15 minutes.'
+          : 'Invalid email or password'
+        res.status(401).json({ ok: false, error: msg }); return
+      }
+      res.json({ ok: true, token: out.token, staff: out.staff })
     } catch (err) {
       res.status(400).json({ ok: false, error: err instanceof Error ? err.message : String(err) })
     }
@@ -242,6 +273,7 @@ export function apiRouter(
     if (!isManagerial(viewer.role)) {
       res.status(403).json({ ok: false, error: 'Owner or manager only' }); return
     }
+    if (!revealGuard(res)) return
     const id = String(req.params['id'] ?? '')
     if ((await getClient(db, id)) === null) {
       res.status(404).json({ ok: false, error: 'Client not found' }); return
@@ -585,8 +617,12 @@ export function apiRouter(
     if (!isManagerial(viewer.role)) {
       res.status(403).json({ ok: false, error: 'Owner or manager only' }); return
     }
+    if (!revealGuard(res)) return
     try {
       const id = String(req.params['id'] ?? '')
+      if ((await getClientModule(db, id)) === null) {
+        res.status(404).json({ ok: false, error: 'Client module not found' }); return
+      }
       res.json({ ok: true, password: await revealModulePassword(db, viewer.id, id, gmail().keyHex) })
     } catch (err) {
       res.status(400).json({ ok: false, error: err instanceof Error ? err.message : String(err) })
@@ -624,6 +660,7 @@ export function apiRouter(
     if (!isManagerial(viewer.role)) {
       res.status(403).json({ ok: false, error: 'Owner or manager only' }); return
     }
+    if (!revealGuard(res)) return
     try {
       const key = (req.body as { key?: unknown }).key
       if (typeof key !== 'string') throw new Error('key must be a string')

apps/hq/src/auth.ts

@@ -8,6 +8,13 @@ import type { DB } from './db'
 
 const SESSION_DAYS = 14
 
+// D24 (red-team) — brute-force lockout policy.
+const MAX_FAILED = 8               // consecutive failures before the account locks
+const LOCK_MINUTES = 15            // how long a locked account stays barred
+// A fixed dummy hash so a login for a NON-existent email still spends one scrypt
+// verification — constant time, no email-enumeration oracle. Computed once at load.
+const DUMMY_HASH = hashPin('x'.repeat(24))
+
 export async function createStaff(db: DB, input: {
   email: string; displayName: string; role: 'owner' | 'manager' | 'staff'; password: string
 }): Promise<{ id: string }> {
@@ -22,17 +29,54 @@ export async function createStaff(db: DB, input: {
   return { id }
 }
 
-interface StaffRow { id: string; email: string; display_name: string; role: string; pw_salt: string; pw_hash: string; active: number }
+interface StaffRow {
+  id: string; email: string; display_name: string; role: string
+  pw_salt: string; pw_hash: string; active: number
+  failed_count: number; locked_until: string | null
+}
+
+export type LoginResult =
+  | { ok: true; token: string; staff: { id: string; displayName: string; role: string } }
+  | { ok: false; reason: 'invalid' | 'locked' }
 
-export async function login(db: DB, email: string, password: string):
-  Promise<{ token: string; staff: { id: string; displayName: string; role: string } } | null> {
+/**
+ * Authenticate, with brute-force lockout (D24). Always spends exactly one scrypt
+ * verification — a real one for an existing account, a dummy for a missing one — so
+ * response time never reveals whether an email exists. After MAX_FAILED consecutive
+ * failures the account locks for LOCK_MINUTES; a success clears the counter. The generic
+ * 'invalid' reason is returned for both wrong-password and unknown-email.
+ */
+export async function login(db: DB, email: string, password: string): Promise<LoginResult> {
   const row = await db.get<StaffRow>(`SELECT * FROM staff_user WHERE email=? AND active=1`, email.toLowerCase())
-  if (!row || !verifyPin(password, { salt: row.pw_salt, hash: row.pw_hash })) return null
+  const now = Date.now()
+
+  // Locked? Still spend a scrypt op (constant time) before returning.
+  if (row && row.locked_until !== null && Date.parse(row.locked_until) > now) {
+    verifyPin(password, { salt: DUMMY_HASH.salt, hash: DUMMY_HASH.hash })
+    return { ok: false, reason: 'locked' }
+  }
+
+  const stored = row ? { salt: row.pw_salt, hash: row.pw_hash } : DUMMY_HASH
+  const passwordOk = verifyPin(password, stored)
+
+  if (!row || !passwordOk) {
+    if (row) {
+      // Count the failure; lock once the threshold is crossed.
+      const failed = row.failed_count + 1
+      const lockUntil = failed >= MAX_FAILED ? new Date(now + LOCK_MINUTES * 60_000).toISOString() : null
+      await db.run(`UPDATE staff_user SET failed_count=?, locked_until=? WHERE id=?`, failed, lockUntil, row.id)
+    }
+    return { ok: false, reason: 'invalid' }
+  }
+
   const token = randomBytes(32).toString('hex')
-  const expires = new Date(Date.now() + SESSION_DAYS * 86_400_000).toISOString()
-  await db.run(`INSERT INTO session (token, staff_id, expires_at) VALUES (?, ?, ?)`, token, row.id, expires)
-  await writeAudit(db, row.id, 'login', 'staff_user', row.id)
-  return { token, staff: { id: row.id, displayName: row.display_name, role: row.role } }
+  const expires = new Date(now + SESSION_DAYS * 86_400_000).toISOString()
+  await db.transaction(async () => {
+    await db.run(`UPDATE staff_user SET failed_count=0, locked_until=NULL WHERE id=?`, row.id)
+    await db.run(`INSERT INTO session (token, staff_id, expires_at) VALUES (?, ?, ?)`, token, row.id, expires)
+    await writeAudit(db, row.id, 'login', 'staff_user', row.id)
+  })
+  return { ok: true, token, staff: { id: row.id, displayName: row.display_name, role: row.role } }
 }
 
 export async function verifySession(db: DB, token: string): Promise<{ id: string; role: string } | null> {

apps/hq/src/db.ts

@@ -73,7 +73,11 @@ CREATE TABLE IF NOT EXISTS staff_user (
   id TEXT PRIMARY KEY, email TEXT NOT NULL UNIQUE, display_name TEXT NOT NULL,
   role TEXT NOT NULL CHECK (role IN ('owner','manager','staff')),
   phone TEXT, title TEXT, -- D18 WS-E: contact + designation shown on Employees/profile; nothing routes on them
-  pw_salt TEXT NOT NULL, pw_hash TEXT NOT NULL, active INTEGER NOT NULL DEFAULT 1
+  pw_salt TEXT NOT NULL, pw_hash TEXT NOT NULL, active INTEGER NOT NULL DEFAULT 1,
+  -- D24 (red-team): brute-force lockout. failed_count counts consecutive failed logins;
+  -- locked_until is an ISO timestamp the account is barred until (NULL = not locked).
+  -- Both reset to 0/NULL on a successful login.
+  failed_count INTEGER NOT NULL DEFAULT 0, locked_until TEXT
 );
 CREATE TABLE IF NOT EXISTS session (
   token TEXT PRIMARY KEY, staff_id TEXT NOT NULL, expires_at TEXT NOT NULL
@@ -320,6 +324,13 @@ function migrate(db: SqliteRaw): void {
   if (!staffCols.some((c) => c.name === 'title')) {
     db.exec(`ALTER TABLE staff_user ADD COLUMN title TEXT`)
   }
+  // D24 (red-team): login lockout columns.
+  if (!staffCols.some((c) => c.name === 'failed_count')) {
+    db.exec(`ALTER TABLE staff_user ADD COLUMN failed_count INTEGER NOT NULL DEFAULT 0`)
+  }
+  if (!staffCols.some((c) => c.name === 'locked_until')) {
+    db.exec(`ALTER TABLE staff_user ADD COLUMN locked_until TEXT`)
+  }
   const clientCols = db.prepare(`PRAGMA table_info(client)`).all() as { name: string }[]
   if (!clientCols.some((c) => c.name === 'owner_id')) {
     db.exec(`ALTER TABLE client ADD COLUMN owner_id TEXT`)

apps/hq/src/migrations-pg.ts

@@ -235,6 +235,13 @@ CREATE TABLE IF NOT EXISTS project_milestone (
   done integer NOT NULL DEFAULT 0, done_on text, sort integer NOT NULL DEFAULT 0,
   UNIQUE (client_module_id, key)
 );
+`,
+  },
+  {
+    id: '006-login-lockout',
+    sql: `
+ALTER TABLE staff_user ADD COLUMN IF NOT EXISTS failed_count integer NOT NULL DEFAULT 0;
+ALTER TABLE staff_user ADD COLUMN IF NOT EXISTS locked_until text;
 `,
   },
 ]

apps/hq/src/rate-limit.ts

@@ -0,0 +1,21 @@
+/**
+ * A dependency-free fixed-window rate limiter keyed by an opaque string (client IP,
+ * account email, staff id, …). Returns true while the key is under `limit` in the
+ * current `windowMs` window, false once it is exhausted. Expired buckets are swept
+ * lazily so the map cannot grow without bound under key rotation. In its own module
+ * so both the api router and the public-share route can share it without a cycle.
+ */
+export function makeRateLimiter(limit: number, windowMs: number): (key: string, now?: number) => boolean {
+  const buckets = new Map<string, { count: number; resetAt: number }>()
+  return (key: string, now = Date.now()): boolean => {
+    if (buckets.size > 5000) for (const [k, b] of buckets) if (now >= b.resetAt) buckets.delete(k)
+    const bucket = buckets.get(key)
+    if (bucket === undefined || now >= bucket.resetAt) {
+      buckets.set(key, { count: 1, resetAt: now + windowMs })
+      return true
+    }
+    if (bucket.count >= limit) return false
+    bucket.count += 1
+    return true
+  }
+}

apps/hq/src/server.ts

@@ -8,6 +8,7 @@ import { maybePullAwsCosts } from './aws-costs'
 import { pollBounces } from './bounces'
 import { openDb, type DB } from './db'
 import { openPgDb } from './db-pg'
+import { makeRateLimiter } from './rate-limit'
 import { renderPdf } from './pdf'
 import { getClient } from './repos-clients'
 import { validateShare } from './repos-shares'
@@ -66,26 +67,9 @@ async function companySettings(db: DB): Promise<Record<string, string>> {
   return Object.fromEntries(rows.map((row) => [row.key, row.value]))
 }
 
-/**
- * A dependency-free fixed-window rate limiter keyed by an opaque string (the caller
- * passes the client IP). Returns true while the key is under `limit` in the current
- * `windowMs` window, false once it is exhausted. Expired buckets are swept lazily so
- * the map cannot grow without bound under IP rotation.
- */
-export function makeRateLimiter(limit: number, windowMs: number): (key: string, now?: number) => boolean {
-  const buckets = new Map<string, { count: number; resetAt: number }>()
-  return (key: string, now = Date.now()): boolean => {
-    if (buckets.size > 5000) for (const [k, b] of buckets) if (now >= b.resetAt) buckets.delete(k)
-    const bucket = buckets.get(key)
-    if (bucket === undefined || now >= bucket.resetAt) {
-      buckets.set(key, { count: 1, resetAt: now + windowMs })
-      return true
-    }
-    if (bucket.count >= limit) return false
-    bucket.count += 1
-    return true
-  }
-}
+// Re-exported for existing importers/tests; the source of truth is rate-limit.ts,
+// shared with the api router without a server↔api import cycle.
+export { makeRateLimiter } from './rate-limit'
 
 /** Minimal, self-contained "this link is gone" page. Carries no document data. */
 function shareGonePage(message: string): string {
@@ -155,6 +139,11 @@ export function mountPublicShare(app: express.Express, db: DB, deps: PublicShare
 
 export async function startServer(port: number, opts: { scheduler?: boolean } = {}): Promise<http.Server> {
   const app = express()
+  // D24 (red-team): behind the reverse proxy (nginx/Caddy), trust the first hop so
+  // req.ip reflects the real client via X-Forwarded-For — without this every request
+  // shares the proxy's IP and per-IP rate limiting collapses to one global bucket.
+  // The proxy MUST strip client-supplied X-Forwarded-For so it cannot be spoofed.
+  app.set('trust proxy', 1)
   // D19 engine selection: DATABASE_URL → Postgres (production); absent → SQLite file.
   const pgUrl = process.env['DATABASE_URL'] ?? ''
   const db = pgUrl !== '' ? await openPgDb(pgUrl) : openDb(process.env['HQ_DATA_DIR'])

apps/hq/test/auth.test.ts

@@ -7,10 +7,10 @@ describe('hq auth', () => {
   it('logs in with correct password, rejects wrong one', async () => {
     const db = openDb(':memory:')
     await createStaff(db, { email: 'admin@tecnostac.com', displayName: 'Owner', role: 'owner', password: 'let-me-in-9' })
-    expect(await login(db, 'admin@tecnostac.com', 'wrong')).toBeNull()
+    expect(await login(db, 'admin@tecnostac.com', 'wrong')).toMatchObject({ ok: false, reason: 'invalid' })
     const ok = await login(db, 'admin@tecnostac.com', 'let-me-in-9')
-    expect(ok).not.toBeNull()
-    const staff = await verifySession(db, ok!.token)
+    expect(ok.ok).toBe(true)
+    const staff = await verifySession(db, ok.ok ? ok.token : '')
     expect(staff).toMatchObject({ role: 'owner' })
   })
   it('rejects passwords under 8 chars at creation', async () => {
@@ -19,4 +19,24 @@ describe('hq auth', () => {
       createStaff(db, { email: 'a@b.c', displayName: 'X', role: 'staff', password: 'short' }),
     ).rejects.toThrow(/8/)
   })
+  it('locks the account after repeated failures, then a wrong-but-formerly-valid attempt is barred (D24)', async () => {
+    const db = openDb(':memory:')
+    await createStaff(db, { email: 'lock@t.in', displayName: 'L', role: 'staff', password: 'correct-horse-9' })
+    for (let i = 0; i < 8; i++) expect((await login(db, 'lock@t.in', 'nope')).ok).toBe(false)
+    // Now locked — even the CORRECT password is refused with reason 'locked'.
+    expect(await login(db, 'lock@t.in', 'correct-horse-9')).toMatchObject({ ok: false, reason: 'locked' })
+  })
+  it('a successful login clears the failure counter (D24)', async () => {
+    const db = openDb(':memory:')
+    await createStaff(db, { email: 'reset@t.in', displayName: 'R', role: 'staff', password: 'correct-horse-9' })
+    for (let i = 0; i < 3; i++) await login(db, 'reset@t.in', 'nope')
+    expect((await login(db, 'reset@t.in', 'correct-horse-9')).ok).toBe(true)
+    const row = await db.get<{ failed_count: number; locked_until: string | null }>(
+      `SELECT failed_count, locked_until FROM staff_user WHERE email='reset@t.in'`)
+    expect(row).toMatchObject({ failed_count: 0, locked_until: null })
+  })
+  it('an unknown email still returns invalid (constant-time, no enumeration) (D24)', async () => {
+    const db = openDb(':memory:')
+    expect(await login(db, 'ghost@nowhere.in', 'whatever')).toMatchObject({ ok: false, reason: 'invalid' })
+  })
 })

apps/hq/test/employees-routes.test.ts

@@ -82,8 +82,8 @@ describe('/employees routes', () => {
 
     const pw = await call(token, 'POST', `/employees/${id}/password`, { password: 'brand-new-secret' })
     expect(pw.status).toBe(200)
-    expect(await login(ctx.db, 'new@test.in', 'password-9')).toBeNull()
-    expect(await login(ctx.db, 'new@test.in', 'brand-new-secret')).not.toBeNull()
+    expect((await login(ctx.db, 'new@test.in', 'password-9')).ok).toBe(false)
+    expect((await login(ctx.db, 'new@test.in', 'brand-new-secret')).ok).toBe(true)
   })
 
   it('non-owner gets 403 on every mutation route', async () => {

apps/hq/test/employees.test.ts

@@ -45,7 +45,7 @@ describe('employee foundation', () => {
   it('deactivation kills the live session immediately and purges its rows', async () => {
     const { db, ownerId } = await withOwner()
     const emp = await createEmployee(db, ownerId, { email: 's@x.co', displayName: 'Stf', role: 'staff', password: 'password2' })
-    const session = (await login(db, 's@x.co', 'password2'))!
+    const login2 = await login(db, 's@x.co', 'password2'); if (!login2.ok) throw new Error('login failed'); const session = login2
     expect(await verifySession(db, session.token)).toMatchObject({ id: emp.id })
     await deactivateEmployee(db, ownerId, emp.id)
     expect(await verifySession(db, session.token)).toBeNull()
@@ -66,16 +66,16 @@ describe('employee foundation', () => {
     const { db, ownerId } = await withOwner()
     const emp = await createEmployee(db, ownerId, { email: 's@x.co', displayName: 'Stf', role: 'staff', password: 'password2' })
     await deactivateEmployee(db, ownerId, emp.id)
-    expect(await login(db, 's@x.co', 'password2')).toBeNull()
+    expect((await login(db, 's@x.co', 'password2')).ok).toBe(false)
     expect((await getEmployee(db, emp.id))!.active).toBe(false)
     await reactivateEmployee(db, ownerId, emp.id)
-    expect(await login(db, 's@x.co', 'password2')).not.toBeNull()
+    expect((await login(db, 's@x.co', 'password2')).ok).toBe(true)
   })
 
   it('password reset invalidates existing sessions immediately (compromised-credential lockout)', async () => {
     const { db, ownerId } = await withOwner()
     const emp = await createEmployee(db, ownerId, { email: 's@x.co', displayName: 'Stf', role: 'staff', password: 'password2' })
-    const session = (await login(db, 's@x.co', 'password2'))!
+    const login2 = await login(db, 's@x.co', 'password2'); if (!login2.ok) throw new Error('login failed'); const session = login2
     expect(await verifySession(db, session.token)).toMatchObject({ id: emp.id })
     await setEmployeePassword(db, ownerId, emp.id, 'new-secret-9')
     expect(await verifySession(db, session.token)).toBeNull()
@@ -104,8 +104,8 @@ describe('employee foundation', () => {
     const { db, ownerId } = await withOwner()
     const emp = await createEmployee(db, ownerId, { email: 's@x.co', displayName: 'Stf', role: 'staff', password: 'password2' })
     await setEmployeePassword(db, ownerId, emp.id, 'new-secret-9')
-    expect(await login(db, 's@x.co', 'password2')).toBeNull()
-    expect(await login(db, 's@x.co', 'new-secret-9')).not.toBeNull()
+    expect((await login(db, 's@x.co', 'password2')).ok).toBe(false)
+    expect((await login(db, 's@x.co', 'new-secret-9')).ok).toBe(true)
     const audit = (await listAudit(db)).find((a) => a.action === 'reset_password' && a.entity_id === emp.id)
     expect(audit).toBeDefined()
     expect(audit!.before_json ?? '').not.toMatch(/hash/)