mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-15 10:59:32 +01:00
fix(auth): widen the per-IP login throttle to 50, keep the per-account lockout at 10
One AUTH_FAILURE_MAX (10 per 15 min) gated the single-user per-IP bucket, the multi-user per-IP pre-gate, the per-username lockout and the hook-secret bucket. Clients sharing one egress IP (NAT, a tunnel, an automated client retrying) were throttled after 10 failures in total. Add AUTH_IP_FAILURE_MAX = 50 and use it for the two per-IP login buckets only. The per-username lockout and the hook-secret bucket keep AUTH_FAILURE_MAX = 10, so guessing against any one account, or against the hook secret, is no easier than before.
This commit is contained in:
1 parent
002de32a08
commit
ebb9e00de9
5 files changed
+33
-6
No files matched your search
@@ -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) |
|
| **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` |
|
| **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 |
|
| **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 |
|
| **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) |
|
| **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_*`) |
|
| **Validation** | Zod schemas, Unicode-aware path allowlist regex, env prefix allowlist (`CLAUDE_CODE_*`/`OPENCODE_*`/`CODEX_*`/`GEMINI_*`/`GOOGLE_*`/`ANTIGRAVITY_*`/`PI_*`) |
|
||||||
|
|||||||
@@ -149,7 +149,7 @@ by presence in a server‑side map, so they cannot be forged offline.
|
|||||||
|
|
||||||
### Rate limiting / lockout recovery
|
### 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 QR path has its own separate limiter.
|
||||||
|
|
||||||
The lockout check sits **after** the cookie/credential checks (step 4, not first).
|
The lockout check sits **after** the cookie/credential checks (step 4, not first).
|
||||||
|
|||||||
@@ -21,9 +21,20 @@ export const MAX_AUTH_SESSIONS = 100;
|
|||||||
// Rate Limiting
|
// 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;
|
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) */
|
/** Failed auth attempt tracking window (ms) */
|
||||||
export const AUTH_FAILURE_WINDOW_MS = 15 * 60 * 1000;
|
export const AUTH_FAILURE_WINDOW_MS = 15 * 60 * 1000;
|
||||||
|
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ import {
|
|||||||
AUTH_SESSION_TTL_MS,
|
AUTH_SESSION_TTL_MS,
|
||||||
MAX_AUTH_SESSIONS,
|
MAX_AUTH_SESSIONS,
|
||||||
AUTH_FAILURE_MAX,
|
AUTH_FAILURE_MAX,
|
||||||
|
AUTH_IP_FAILURE_MAX,
|
||||||
AUTH_FAILURE_WINDOW_MS,
|
AUTH_FAILURE_WINDOW_MS,
|
||||||
} from '../../config/auth-config.js';
|
} from '../../config/auth-config.js';
|
||||||
import { getHookSecret, HOOK_SECRET_HEADER } from '../../config/hook-secret.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.
|
// Rate limit only requests that failed to authenticate on this attempt.
|
||||||
const failures = authFailures.get(clientIp) ?? 0;
|
const failures = authFailures.get(clientIp) ?? 0;
|
||||||
if (failures >= AUTH_FAILURE_MAX) {
|
if (failures >= AUTH_IP_FAILURE_MAX) {
|
||||||
sendAuthRateLimit(reply, authFailures, clientIp);
|
sendAuthRateLimit(reply, authFailures, clientIp);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -573,7 +574,7 @@ function registerMultiUserAuthHook(
|
|||||||
// Per-IP pre-gate bounds scrypt CPU cost from one source (does NOT gate on the
|
// Per-IP pre-gate bounds scrypt CPU cost from one source (does NOT gate on the
|
||||||
// per-username bucket here; see below).
|
// per-username bucket here; see below).
|
||||||
const ipFail = authFailures.get(clientIp) ?? 0;
|
const ipFail = authFailures.get(clientIp) ?? 0;
|
||||||
if (ipFail >= AUTH_FAILURE_MAX) {
|
if (ipFail >= AUTH_IP_FAILURE_MAX) {
|
||||||
sendAuthRateLimit(reply, authFailures, clientIp);
|
sendAuthRateLimit(reply, authFailures, clientIp);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import { WebServer } from '../src/web/server.js';
|
|||||||
import { TmuxManager } from '../src/tmux-manager.js';
|
import { TmuxManager } from '../src/tmux-manager.js';
|
||||||
import { SettingsUpdateSchema } from '../src/web/schemas.js';
|
import { SettingsUpdateSchema } from '../src/web/schemas.js';
|
||||||
import { getHookSecret, HOOK_SECRET_HEADER } from '../src/config/hook-secret.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_USER = 'admin';
|
||||||
const TEST_PASS = 'test-password-12345';
|
const TEST_PASS = 'test-password-12345';
|
||||||
@@ -46,7 +47,7 @@ async function getSessionCookie(baseUrl: string): Promise<string> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async function exhaustAuthFailures(baseUrl: string, prefix: string): Promise<void> {
|
async function exhaustAuthFailures(baseUrl: string, prefix: string): Promise<void> {
|
||||||
for (let i = 0; i < 10; i++) {
|
for (let i = 0; i < AUTH_IP_FAILURE_MAX; i++) {
|
||||||
const res = await fetch(`${baseUrl}/api/status`, {
|
const res = await fetch(`${baseUrl}/api/status`, {
|
||||||
headers: { Authorization: basicAuthHeader(TEST_USER, `${prefix}-${i}`) },
|
headers: { Authorization: basicAuthHeader(TEST_USER, `${prefix}-${i}`) },
|
||||||
});
|
});
|
||||||
@@ -207,6 +208,20 @@ describe('Auth Security', () => {
|
|||||||
expect(res.headers.get('retry-after')).toMatch(/^\d+$/);
|
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 () => {
|
it('should allow an existing valid session cookie during auth failure lockout', async () => {
|
||||||
const cookie = await getSessionCookie(rateBaseUrl);
|
const cookie = await getSessionCookie(rateBaseUrl);
|
||||||
await exhaustAuthFailures(rateBaseUrl, 'cod21-cookie');
|
await exhaustAuthFailures(rateBaseUrl, 'cod21-cookie');
|
||||||
|
|||||||
Reference in new issue
Block a user