diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index f446f4a1..0cabeaf7 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -1163,7 +1163,7 @@ Tests: `test/mobile-prompt-composer.test.ts` (in the CI gate, deliberately not u | **CSRF / Origin** | Always-on cross-site Origin guard rejects state-changing requests from foreign origins (covers self-update, session create/input, settings/tunnel toggles). **A missing Origin is allowed** so curl/CLI and Claude Code hooks keep working. The global body parser keeps `text/plain` RAW (no auto-JSON-parse, which had enabled simple-request CSRF); `/api/crash-diag` self-parses. WebSocket upgrade validates Origin+Host (anti-CSWSH) in `ws-routes.ts`. Added in `c669518` (closes 2026-06-09 review CRITICALs) | | **QR Auth** | Single-use 6-char tokens (60s TTL) for tunnel login. See `docs/qr-auth-plan.md` | | **Sessions** | 24h cookie (`codeman_session`), auto-extend, device context audit | -| **Rate limit** | 10 failed auth/IP → 429 (15min decay). QR has separate limiter | +| **Rate limit** | 50 failed logins/IP (`AUTH_IP_FAILURE_MAX`) and 10 per username → 429 (15min decay). QR has separate limiter | | **Hook bypass** | `/api/hook-event` (and `/api/status-telemetry`, the statusLine exporter) skip Basic auth (localhost-only, schema-validated). When auth is active (`CODEMAN_PASSWORD` set), the loopback bypass requires the per-instance `X-Codeman-Hook-Secret` header **unconditionally** — COD-54 introduced it tunnel-gated; COD-91 (PR #127) made it always-on because Codeman can't detect a user's own loopback reverse proxy (own cloudflared/`tailscale serve`/nginx → 127.0.0.1), closing that residual plain-bypass gap. Hook curls cat the secret file at exec time via `$CODEMAN_HOOK_SECRET_FILE` (session env, `config/hook-secret.ts`); a missing/wrong secret gets 401 and rate-limits in a dedicated bucket (never locks out login). Tunnel enable **refuses** without `CODEMAN_PASSWORD` unless exposure is acknowledged — via `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1` (env, COD-55) **or** the per-request `acknowledgeUnauthTunnel:true` action field (1.1.9): the welcome/settings tunnel toggle pops a security confirm dialog and, on confirm, resends with that flag (server logs a loud warning on every passwordless tunnel start; curl/API stay refused without password/env/flag). The flag is an action field, never persisted | | **Env vars** | `CODEMAN_MUX` (managed session), `CODEMAN_API_URL` (auto-set for hooks), `CODEMAN_ALLOWED_HOSTS` (extra Host/Origin allowlist entries for reverse proxies, comma-separated; bare `.suffix` matches subdomains), `CODEMAN_DOCKER_BRIDGE_HOOKS`=1 (opt-in hooks-only listener on the docker bridge gateway so in-container hooks reach a loopback-bound server; bind IP from `CODEMAN_DOCKER_BRIDGE_HOST` or auto-detect) | | **Validation** | Zod schemas, Unicode-aware path allowlist regex, env prefix allowlist (`CLAUDE_CODE_*`/`OPENCODE_*`/`CODEX_*`/`GEMINI_*`/`GOOGLE_*`/`ANTIGRAVITY_*`/`PI_*`) | diff --git a/docs/security-architecture.md b/docs/security-architecture.md index bf349a55..a7e50c04 100644 --- a/docs/security-architecture.md +++ b/docs/security-architecture.md @@ -149,7 +149,7 @@ by presence in a server‑side map, so they cannot be forged offline. ### Rate limiting / lockout recovery -Failed auth is tracked **per IP**: 10 failures → `429`, with a 15‑minute decay. +Failed logins are tracked **per IP**: 50 failures (`AUTH_IP_FAILURE_MAX`) → `429`, with a 15‑minute decay. In multi-user mode a separate **per-username** bucket still returns `429` after 10 failed guesses (`AUTH_FAILURE_MAX`), so the wider per-IP cap (for clients sharing one egress IP) does not widen guessing against any one account. The QR path has its own separate limiter. The lockout check sits **after** the cookie/credential checks (step 4, not first). diff --git a/src/config/auth-config.ts b/src/config/auth-config.ts index 9aa60482..c9ade053 100644 --- a/src/config/auth-config.ts +++ b/src/config/auth-config.ts @@ -21,9 +21,20 @@ export const MAX_AUTH_SESSIONS = 100; // Rate Limiting // ============================================================================ -/** Max failed auth attempts per IP before 429 rejection */ +/** + * Max failed auth attempts per bucket before 429 rejection. Applies to the + * per-username lockout and the hook-secret bucket. + */ export const AUTH_FAILURE_MAX = 10; +/** + * Max failed login attempts per client IP before 429 rejection. Wider than + * AUTH_FAILURE_MAX because one egress IP (NAT, a tunnel, an automated client) can + * carry many users' or retries' failures; the per-username lockout above still + * caps guesses against any one account. + */ +export const AUTH_IP_FAILURE_MAX = 50; + /** Failed auth attempt tracking window (ms) */ export const AUTH_FAILURE_WINDOW_MS = 15 * 60 * 1000; diff --git a/src/web/middleware/auth.ts b/src/web/middleware/auth.ts index 8773d669..44660ee7 100644 --- a/src/web/middleware/auth.ts +++ b/src/web/middleware/auth.ts @@ -17,6 +17,7 @@ import { AUTH_SESSION_TTL_MS, MAX_AUTH_SESSIONS, AUTH_FAILURE_MAX, + AUTH_IP_FAILURE_MAX, AUTH_FAILURE_WINDOW_MS, } from '../../config/auth-config.js'; import { getHookSecret, HOOK_SECRET_HEADER } from '../../config/hook-secret.js'; @@ -447,7 +448,7 @@ export function registerAuthMiddleware(app: FastifyInstance, https: boolean, bas // Rate limit only requests that failed to authenticate on this attempt. const failures = authFailures.get(clientIp) ?? 0; - if (failures >= AUTH_FAILURE_MAX) { + if (failures >= AUTH_IP_FAILURE_MAX) { sendAuthRateLimit(reply, authFailures, clientIp); return; } @@ -573,7 +574,7 @@ function registerMultiUserAuthHook( // Per-IP pre-gate bounds scrypt CPU cost from one source (does NOT gate on the // per-username bucket here; see below). const ipFail = authFailures.get(clientIp) ?? 0; - if (ipFail >= AUTH_FAILURE_MAX) { + if (ipFail >= AUTH_IP_FAILURE_MAX) { sendAuthRateLimit(reply, authFailures, clientIp); return; } diff --git a/test/auth-security.test.ts b/test/auth-security.test.ts index 2f6e633d..2bf91914 100644 --- a/test/auth-security.test.ts +++ b/test/auth-security.test.ts @@ -15,6 +15,7 @@ import { WebServer } from '../src/web/server.js'; import { TmuxManager } from '../src/tmux-manager.js'; import { SettingsUpdateSchema } from '../src/web/schemas.js'; import { getHookSecret, HOOK_SECRET_HEADER } from '../src/config/hook-secret.js'; +import { AUTH_FAILURE_MAX, AUTH_IP_FAILURE_MAX } from '../src/config/auth-config.js'; const TEST_USER = 'admin'; const TEST_PASS = 'test-password-12345'; @@ -46,7 +47,7 @@ async function getSessionCookie(baseUrl: string): Promise { } async function exhaustAuthFailures(baseUrl: string, prefix: string): Promise { - for (let i = 0; i < 10; i++) { + for (let i = 0; i < AUTH_IP_FAILURE_MAX; i++) { const res = await fetch(`${baseUrl}/api/status`, { headers: { Authorization: basicAuthHeader(TEST_USER, `${prefix}-${i}`) }, }); @@ -207,6 +208,20 @@ describe('Auth Security', () => { expect(res.headers.get('retry-after')).toMatch(/^\d+$/); }); + it('leaves per-IP headroom above the per-account cap before throttling', async () => { + // One egress IP (NAT, tunnel, an automated client) can carry more failures + // than one person does, so the per-IP login bucket is wider than the + // per-username one. + expect(AUTH_IP_FAILURE_MAX).toBe(50); + expect(AUTH_IP_FAILURE_MAX).toBeGreaterThan(AUTH_FAILURE_MAX); + for (let i = 0; i <= AUTH_FAILURE_MAX; i++) { + const res = await fetch(`${rateBaseUrl}/api/status`, { + headers: { Authorization: basicAuthHeader(TEST_USER, `ip-headroom-${i}`) }, + }); + expect(res.status).toBe(401); + } + }); + it('should allow an existing valid session cookie during auth failure lockout', async () => { const cookie = await getSessionCookie(rateBaseUrl); await exhaustAuthFailures(rateBaseUrl, 'cod21-cookie');