docs(06-05): add plan summary — D-11/D-12 trusted-proxy IP + per-account limiter

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-04 18:28:30 +02:00
parent a826738e18
commit 5c82a9840a
1 changed files with 70 additions and 0 deletions
@@ -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).