diff --git a/.agents/phases/complete/81_login_rate_limit/00_phase.md b/.agents/phases/complete/81_login_rate_limit/00_phase.md new file mode 100644 index 0000000..5981802 --- /dev/null +++ b/.agents/phases/complete/81_login_rate_limit/00_phase.md @@ -0,0 +1,69 @@ +# 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: ` 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): + ```python + 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). diff --git a/.agents/phases/complete/81_login_rate_limit/01_rate_limiter_core.md b/.agents/phases/complete/81_login_rate_limit/01_rate_limiter_core.md new file mode 100644 index 0000000..fe72877 --- /dev/null +++ b/.agents/phases/complete/81_login_rate_limit/01_rate_limiter_core.md @@ -0,0 +1,33 @@ +# Task 01 — The sliding-window limiter module + unit suite + +**Phase:** `81_login_rate_limit` · **Story:** n/a (security hardening — audit SEC-03) + +## Objective +`app/core/rate_limit.py` exists with the exact semantics from the phase design block, and `tests/unit/test_rate_limit.py` pins every one of them. + +## Work +1. `app/core/rate_limit.py` (new) — implement per the phase overview's design block: + - constants `MAX_FAILURES = 10`, `WINDOW_SECONDS = 900`; + - module state: `_failures: dict[str, deque[float]]` + a `threading.Lock`; + - `record_failure(client_ip: str)` — under the lock: get/create the deque, append `time.monotonic()`, prune entries older than `WINDOW_SECONDS`; **never raises** (wrap the body, `except Exception: return` — the fail-open contract); + - `remaining_wait(client_ip: str) -> int` — under the lock: prune; count in-window entries; if `< MAX_FAILURES` return `0`; else `max(1, int(-oldest - now + WINDOW_SECONDS))` as a ceiling — i.e. `int(math.ceil(WINDOW_SECONDS - (now - oldest)))` clamped to >= 1; **never raises** (fail-open → 0); + - `reset(client_ip: str)` — under the lock: `_failures.pop(client_ip, None)`; + - module docstring: the audit basis (SEC-03), the 10/15-min threshold, the shared-counter intent (both login routes), fail-open rationale (a limiter bug must never lock the owner out), and the `request.client.host` assumption — direct peer, no reverse proxy in the current homelab deployment (proxy deployments would need `X-Forwarded-For` handling — out of scope). +2. `tests/unit/test_rate_limit.py` (new) — pin, in this order: + - 9 × `record_failure("ip")` → `remaining_wait("ip") == 0`; + - a 10th `record_failure` → `remaining_wait("ip") > 0` (and `<= WINDOW_SECONDS`); + - a second IP is unaffected (`remaining_wait("ip2") == 0`); + - expiry: pre-seed one IP's deque with a timestamp `WINDOW_SECONDS + 1` in the past (reach into the module state, or a small `for` loop calling `record_failure` after monkeypatching `time.monotonic` to walk the clock forward) → `remaining_wait` back to 0; + - `reset` after 10 failures → `remaining_wait == 0`; + - `remaining_wait` for an unknown IP → 0; + - fail-open: inject a corrupt entry (`_failures["bad"] = object()` — reach into module state) → `remaining_wait("bad") == 0` and `record_failure("bad")` returns without raising; + - `reset` on an unknown IP → no error. + +## Testing & Quality +- `uv run pytest tests/unit/test_rate_limit.py -v` green. +- Coverage: **>90%** on `app/core/rate_limit.py` (the fail-open branches included — they are the point). + +## Completion Criteria +- [ ] `uv run pytest tests/unit/test_rate_limit.py -v` — all tests green. +- [ ] `uv run ruff check app/core/rate_limit.py tests/unit/test_rate_limit.py && uv run pyright` clean. +- [ ] `git diff --stat` for this task: exactly the two new files. diff --git a/.agents/phases/complete/81_login_rate_limit/02_auth_routes_wiring.md b/.agents/phases/complete/81_login_rate_limit/02_auth_routes_wiring.md new file mode 100644 index 0000000..f4d267f --- /dev/null +++ b/.agents/phases/complete/81_login_rate_limit/02_auth_routes_wiring.md @@ -0,0 +1,33 @@ +# Task 02 — Wire the limiter into both login routes + integration tests + +**Phase:** `81_login_rate_limit` · **Story:** n/a (security hardening — audit SEC-03) + +## Objective +`POST /api/login` and `POST /api/token-auth` enforce the 429 pre-check and record/reset the counter, and the integration suite pins the HTTP contract (429 + `Retry-After`, shared counter, success reset, unchanged 401s). + +## Work +1. `app/api/auth.py` — add the module constant `TOO_MANY_DETAIL = "too many failed sign-in attempts — try again later"` and import the limiter (`from app.core import rate_limit`). +2. `app/api/auth.py::login` — insert at the TOP of the handler (before `check_password`): + - `ip = request.client.host or "unknown"` (the `or` guards a scope with no client — TestClient edge; fail-open spirit); + - `if (wait := rate_limit.remaining_wait(ip)) > 0: raise HTTPException(status_code=429, detail=TOO_MANY_DETAIL, headers={"Retry-After": str(wait)})`; + - in the failure branch: `rate_limit.record_failure(ip)` immediately before the 401; + - in the success branch: `rate_limit.reset(ip)` immediately before the 204. +3. `app/api/auth.py::token_auth` — the identical three insertions (same `ip` derivation, same `TOO_MANY_DETAIL`, same pre-check/record/reset points — the counter is shared by design). +4. `app/api/auth.py` module docstring — extend the two route bullets with the 429 case (one line each: "while the per-IP failure window is exhausted → 429 + `Retry-After`, one generic detail — audit SEC-03"). +5. `tests/integration/test_auth_api.py` — extend (keep every existing pin intact; the TestClient's `request.client.host` is a fixed "testclient" value, so these tests share one counter — **add a fixture or helper that calls `rate_limit.reset("testclient")` between tests** so existing tests can't trip the limiter, and document why in the test module docstring): + - 10 × wrong-password `POST /api/login` → 11th → **429**, `detail == TOO_MANY_DETAIL`, `Retry-After` header present and `int(...) > 0`; + - after the 429, a CORRECT password → still 429 (the window is not bypassed by success — success only RESETS a clean counter; while blocked, the pre-check fires first; pin this: it is the intended semantics — an exhausted window stays exhausted until it slides); + - reset: with a clean counter, 9 failures + 1 success (204) → the next 9 failures are all 401 (the counter is back to 0 after the success); + - shared counter: 5 login failures + 5 token failures → the next token-auth attempt → 429; + - the existing non-throttled 401 pins (generic `invalid password` / `invalid token`) still green with the clean-counter fixture. + +## Testing & Quality +- `uv run pytest tests/integration/test_auth_api.py -v` green (new + existing pins). +- `uv run pytest tests/unit/test_rate_limit.py -v` still green (no regressions to task 01). +- Coverage: **>90%** on the modified `app/api/auth.py` branches (429 hit/miss, record, reset paths all hit by the integration tests). + +## Completion Criteria +- [ ] The 11th rapid failed login from one client is a 429 with `Retry-After` (integration pin); the token route shares the counter (integration pin). +- [ ] A success on a clean counter resets it (integration pin); a success while blocked does not unblock (integration pin). +- [ ] All pre-existing `test_auth_api.py` pins green (the clean-counter fixture is documented). +- [ ] `uv run ruff check . && uv run pyright` clean. diff --git a/.agents/phases/complete/81_login_rate_limit/03_verify_and_commit.md b/.agents/phases/complete/81_login_rate_limit/03_verify_and_commit.md new file mode 100644 index 0000000..6eaa56f --- /dev/null +++ b/.agents/phases/complete/81_login_rate_limit/03_verify_and_commit.md @@ -0,0 +1,28 @@ +# Task 03 — Full gate + atomic commit + +**Phase:** `81_login_rate_limit` · **Story:** n/a (security hardening — audit SEC-03) + +## Objective +Run the complete phase gate, land the phase as one atomic commit, and move the phase directory to `complete/`. + +## Work +1. **Full regression gate** (AGENTS.md rule 9): + - `uv run pytest` — unit + integration green. + - `uv run pytest --cov=app --cov-report=term-missing` — `app/` coverage **>90%**; confirm `app/core/rate_limit.py` and `app/api/auth.py` show no meaningful uncovered lines in the new branches. + - `uv run pytest tests/e2e/test_smoke.py -v --no-cov` — green **in isolation** (this phase's E2E contract: no UI change — `login.js`'s existing non-204 rendering covers the 429). + - `uv run ruff check . && uv run pyright` — clean. +2. **Manual live check** (keep the output in the session log): start the dev server (`uv run uvicorn app.main:app`) and, from the shell, 11 rapid wrong-password logins: `for i in $(seq 11); do curl -s -o /dev/null -w "%{http_code} " -X POST localhost:8000/api/login -H 'Content-Type: application/json' -d '{"password":"wrong"}'; done` → expect `401 ×10, 429` (and the 429 body carries `Retry-After`). Restart the server to clear the counter (the per-process state is by design). +3. **Commit** (AGENTS.md rule 8 — one atomic, Conventional-Commits commit, always `--no-gpg-sign`), staging `app/core/rate_limit.py`, `app/api/auth.py`, `tests/unit/test_rate_limit.py`, `tests/integration/test_auth_api.py`, and the phase files: + `fix(auth): rate-limit failed logins and token-auth attempts with 429 + Retry-After` + — body: security audit SEC-03 (2026-09-07) — neither login route throttled, so a reachable deployment took unlimited line-rate guesses; a stdlib-only sliding-window limiter (10 failures / IP / 15 min, shared by both routes, success resets, fail-open) now answers the 11th attempt with 429 + `Retry-After` while every non-throttled 401/204 is byte-identical. No dependency or frontend change. +4. Move the phase directory: `mv .agents/phases/todo/81_login_rate_limit .agents/phases/complete/` and include the move in the same commit. + +## Testing & Quality +- This task IS the phase-level gate — the commands above are the completion evidence. +- Coverage: >90% held. + +## Completion Criteria +- [ ] The live check shows `401 ×10, 429` (output kept in the session log); the 429 response carries `Retry-After`. +- [ ] `uv run pytest` green; coverage >90%; `tests/e2e/test_smoke.py` green in isolation; ruff + pyright clean. +- [ ] Exactly one new commit; `git show --stat HEAD` lists the five files above + the phase files (todo → complete move) — nothing else (in particular `pyproject.toml` / `uv.lock` / `frontend/` untouched). +- [ ] `.agents/phases/complete/81_login_rate_limit/` exists; `todo/` no longer contains it. diff --git a/.agents/phases/todo/82_security_headers/00_phase.md b/.agents/phases/todo/82_security_headers/00_phase.md new file mode 100644 index 0000000..af8e990 --- /dev/null +++ b/.agents/phases/todo/82_security_headers/00_phase.md @@ -0,0 +1,71 @@ +# Phase 82 — Security headers on every response: strict CSP, no-framing, nosniff + +**Source:** `.agents/remediation_plan.md` SEC-04 (security audit 2026-09-07, severity Medium — "No security headers (CSP, X-Frame-Options / frame-ancestors)") +**Story:** n/a (security hardening — audit-derived, no user story) +**Context:** no response in the app carries `Content-Security-Policy`, `X-Frame-Options`, or `X-Content-Type-Options` (checked `app/main.py`, `app/core/caching.py`, all templates — none do). The frontend is No-CDN and was verified for this phase: **no inline `