brain-of-reese/.agents/phases/complete/81_login_rate_limit/00_phase.md

Phase 81 — Rate-limit failed sign-ins: 429 + Retry-After after repeated failures

Source: .agents/remediation_plan.md SEC-03 (security audit 2026-09-07, severity Medium — "No rate limiting / lockout on login + token-auth") Story: n/a (security hardening — audit-derived, no user story) Context: app/api/auth.py (POST /api/login — check_password then sign_in; POST /api/token-auth — find_active_by_token then session keys — both return one generic 401 on every failure shape, no throttling of any kind), app/core/auth.py (the constant-time check_password, sign_in/sign_out — untouched by this phase), app/main.py (middleware registration order — this phase adds no middleware), frontend/assets/login.js (the login form's error path: non-204 → Sign-in failed (HTTP ${r.status}) — try again. via showError — a 429 already renders there as a generic message, so no frontend change is required), tests/unit/test_auth.py + tests/integration/test_auth_api.py (the existing auth contract pins — every 401 shape, no enumeration).

Objective

A client IP that fails sign-in repeatedly (admin password or token) gets 429 + Retry-After instead of unlimited line-rate guesses: a stdlib-only in-memory sliding-window limiter shared by both login routes, counting only failed attempts, reset on success, failing open (a limiter bug must never lock the owner out).

Audit basis (read this, not the chat)

• Neither POST /api/login nor POST /api/token-auth throttles: the password is compared with compare_digest (fast), so an attacker who can reach :8000 can try thousands of guesses per second (audit PoC: 10 000 sequential logins all answered at line rate).
• The fix is deliberately small and stateless-per-process (A10: no new service, no DB table, no Redis): a per-IP deque of failure timestamps in module memory, lost on restart (an acceptable reset — it is a friction bump, not the boundary; the homelab HTTP posture is the documented owner decision).
• Threshold (locked in this phase): 10 failed attempts per client IP per 15-minute sliding window → subsequent attempts get 429 {"detail": "too many failed sign-in attempts — try again later"} + a Retry-After: <seconds> header until the window clears.
• The counter is SHARED by both routes (any auth failure from the IP counts — a token-spraying attack must not reset by alternating routes), and a SUCCESS on either route clears that IP's counter (a legitimate owner who fat-fingers twice is not poisoned).

Owner decisions (chat, 2026-09-07 — recorded per AGENTS.md rule 3)

• A1 — in-memory, stdlib-only: no new dependency, no DB table, per-process state (A10/A12 spirit). A restart clears the counter — accepted.
• A2 — 10 / 15 min, module constants: MAX_FAILURES = 10, WINDOW_SECONDS = 900 as documented module constants in the limiter module — NOT new BOR_ settings (a security control with a fixed, tested default; no env surface to mistype).
• A3 — 429 contract: one generic detail (no enumeration between login/token failures), Retry-After header carrying the whole-window seconds; the login page's existing Sign-in failed (HTTP 429) — try again. line renders it — no frontend change in this phase.
• A4 — client IP = request.client.host: the direct peer address. ASSUMPTION (locked here): the app is served directly (homelab), no reverse proxy — if a proxy is ever put in front, the limiter needs X-Forwarded-For handling (noted in the module docstring, out of scope now).

Design (shared by all tasks — the executor reads this, not the chat)

• app/core/rate_limit.py (new) — the limiter, pure stdlib (collections.deque, time.monotonic, threading.Lock for the dict — the ASGI event loop is single-threaded but the lock costs nothing and keeps the unit tests honest under pytest-xdist if ever added):

  MAX_FAILURES = 10
  WINDOW_SECONDS = 900
  
  _failures: dict[str, deque[float]] = {}
  _lock = threading.Lock()
  
  def record_failure(client_ip: str) -> None: ...      # append now(); prune expired; never raises
  def remaining_wait(client_ip: str) -> int: ...       # 0 = allowed; else whole-window seconds (ceil) until the oldest counted failure expires
  def reset(client_ip: str) -> None: ...               # drop the IP's entry (on success)
  

  Semantics (all unit-pinned): a failure is counted when it is recorded; the window slides — only failures within the last WINDOW_SECONDS count; the IP is blocked while the count of in-window failures >= MAX_FAILURES (the 10th failure already blocked? NO — the 10th failure is recorded and the 11th attempt is the first 429: the check happens before the attempt); remaining_wait returns int(ceil(WINDOW_SECONDS - (now - oldest))) while blocked, 0 otherwise; fail-open: any internal error (corrupt deque, clock skew) → remaining_wait returns 0 and record_failure no-ops — a limiter bug never denies the owner.
• app/api/auth.py — the two route changes (identical shape):
  • login: first line of the handler — ip = request.client.host; if (wait := _wait(ip)) > 0: raise HTTPException(429, detail=TOO_MANY_DETAIL, headers={"Retry-After": str(wait)}); on password failure → record_failure(ip) before the 401; on success → reset(ip) before the 204.
  • token_auth: same three points (pre-check, record on the 401 path, reset on the 204 path).
  • TOO_MANY_DETAIL = "too many failed sign-in attempts — try again later" (one string shared by both routes — no enumeration).
  • Module docstrings updated: the 429 case joins the documented status set of each route.
• Not touched: app/core/auth.py (password check + session helpers), the 401 contract of both routes (byte-identical on the non-throttled path), everything else in the app, all frontend files.

Dependencies

— (none; extends the completed phase-16/79 auth surface additively — the 401/204 contract is unchanged for non-throttled callers)

Tasks

1. 01_rate_limiter_core.md — app/core/rate_limit.py + the unit suite for the window semantics.
2. 02_auth_routes_wiring.md — the 429 pre-check + failure/success bookkeeping on both login routes + integration tests.
3. 03_verify_and_commit.md — full gate (suite + coverage + smoke E2E in isolation) + atomic commit.

Testing & Quality

• Unit — tests/unit/test_rate_limit.py (new): 9 failures → allowed; the 10th failure recorded; the next attempt → remaining_wait > 0; two independent IPs independent; a failure older than the window expires (monkeypatch a fake clock or pre-seed timestamps) and unblocks; reset clears (a success unblocks immediately); remaining_wait is the whole-window seconds, not the per-failure age; fail-open: a pre-corrupted state (e.g. a non-deque entry injected) → remaining_wait returns 0, record_failure doesn't raise.
• Integration — tests/integration/test_auth_api.py (extend): 10 wrong-password logins from the TestClient → 11th → 429 with the generic detail + a Retry-After header (int > 0); a correct password after 9 failures → 204 AND the counter is reset (the next 10 failures needed to block again — assert by 9 more failures still 401); token-auth failures share the counter (5 login + 5 token failures → the 11th of either → 429); the non-throttled 401 contract (existing pins) byte-identical.
• E2E (isolation gate per AGENTS.md rule 9): uv run pytest tests/e2e/test_smoke.py -v --no-cov green — no UI change; the login form's existing 429 rendering is covered by the integration contract (the generic HTTP ${status} line).
• Coverage: >90% on app/ — app/core/rate_limit.py is small and fully unit-exercised; the app/api/auth.py branches (pre-check hit/miss, record, reset) integration-covered.

Completion Criteria

• 11 rapid failed logins from one client: the first 10 → 401 (unchanged detail), the 11th → 429 + Retry-After; the token-auth route participates in the same counter (integration pins).
• A successful login/token-auth resets the IP's counter (integration pin).
• uv run pytest tests/unit/test_rate_limit.py -v green; full uv run pytest green; uv run pytest --cov=app --cov-report=term-missing >90%.
• uv run pytest tests/e2e/test_smoke.py -v --no-cov green in isolation.
• uv run ruff check . && uv run pyright clean.
• No dependency change (pyproject.toml / uv.lock untouched); git diff --stat limited to app/core/rate_limit.py, app/api/auth.py, the two test files, phase files.
• One atomic --no-gpg-sign commit (e.g. fix(auth): rate-limit failed logins and token-auth attempts with 429 + Retry-After); phase dir moved to .agents/phases/complete/.

Locked decisions

• A10 intact — no new service/table/middleware; one module + two route pre-checks.
• 401 contract unchanged — the no-enumeration single-401 discipline of phase 16/79 is byte-identical for every non-throttled attempt; 429 is a new status, not a rewording.
• No frontend change — login.js already renders any non-204 as Sign-in failed (HTTP ${status}) — try again. (the 429 reads correctly there).