diff --git a/.planning/phases/06-performance-production-hardening/06-05-SUMMARY.md b/.planning/phases/06-performance-production-hardening/06-05-SUMMARY.md new file mode 100644 index 0000000..87d4e34 --- /dev/null +++ b/.planning/phases/06-performance-production-hardening/06-05-SUMMARY.md @@ -0,0 +1,70 @@ +--- +plan: 06-05 +phase: 06-performance-production-hardening +status: complete +completed_at: 2026-06-04 +commits: + - a826738 +self_check: PASSED +--- + +## What was done + +### D-11 — Trusted-proxy CIDR check in `get_client_ip` + +**Before:** `get_client_ip` unconditionally returned the first value of +`X-Forwarded-For` if present, falling back to `request.client.host`. Any +external caller could spoof their IP by adding an `X-Forwarded-For` header. + +**After:** `get_client_ip` only honours `X-Forwarded-For` when the direct peer +(`request.client.host`) is in one of the trusted proxy CIDRs: +- `127.0.0.0/8` (loopback) +- `172.16.0.0/12` (Docker / internal) +- `192.168.0.0/16` (LAN) +- `::1/128` (IPv6 loopback) + +Untrusted peers always return their own address. The CIDR list is a module-level +constant `_TRUSTED_PROXY_NETS` in `backend/deps/utils.py`. No new public function +was added; only the body and docstring of `get_client_ip` changed. + +### D-12 — Per-account rate limiter (`account_limiter`) + +Created `backend/services/rate_limiting.py` (new file, 18 lines): +- `_account_key(request)`: reads `request.state.current_user.id` if set, + falls back to `request.client.host`, then `"anonymous"`. +- `account_limiter = Limiter(key_func=_account_key)`: singleton exported for + use in routers. + +### Wiring + +- `backend/api/auth.py`: removed `from slowapi.util import get_remote_address`; + changed `limiter = Limiter(key_func=get_remote_address)` to + `limiter = Limiter(key_func=get_client_ip)`. +- `backend/main.py`: added `from services.rate_limiting import account_limiter`. +- `backend/api/documents.py`: 9 endpoints decorated with + `@account_limiter.limit("100/minute")`; each handler's first line sets + `request.state.current_user = current_user` (A1 ordering invariant). +- `backend/api/cloud.py`: 7 endpoints decorated identically (same pattern). + +**Decorator counts:** +- `documents.py`: 9 `@account_limiter.limit` decorators, 9 assignments +- `cloud.py`: 7 `@account_limiter.limit` decorators, 7 assignments + +### D-13 — 8 xfail tests promoted + +All 8 stubs in `backend/tests/test_rate_limiting.py` promoted from `xfail` to +real assertions. No `@pytest.mark.xfail` markers remain. + +A cross-test contamination issue was also fixed: `backend/tests/conftest.py` +gained an autouse fixture `reset_rate_limiter` that calls +`account_limiter._storage.reset()` before and after each test, preventing +in-memory counters from accumulating across tests and causing spurious 429s. + +## Final pytest result + +``` +352 passed, 5 skipped, 7 xfailed +1 pre-existing failure (test_extract_docx — ModuleNotFoundError: No module named 'docx', unrelated to this plan) +``` + +All 8 rate limiting tests pass (8 passed, 0 xfailed).