---
phase: 06-performance-production-hardening
plan: 05
type: execute
wave: 2
depends_on:
- 06-02
files_modified:
- backend/deps/utils.py
- backend/api/auth.py
- backend/services/rate_limiting.py
- backend/main.py
- backend/api/documents.py
- backend/api/cloud.py
- backend/tests/test_rate_limiting.py
autonomous: true
requirements:
- D-11
- D-12
- D-13
user_setup: []
must_haves:
truths:
- "get_client_ip in deps/utils.py uses a trusted-proxy CIDR check — requests from untrusted peers ignore X-Forwarded-For; requests from 127.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, ::1 read the leftmost XFF entry"
- "backend/api/auth.py uses key_func=get_client_ip on its Limiter — get_remote_address from slowapi.util is no longer imported"
- "A second Limiter instance (account_limiter) keyed by request.state.current_user.id exists in backend/services/rate_limiting.py and is reused by documents.py and cloud.py"
- "Every authenticated endpoint in documents.py and cloud.py that previously had no per-account limit now has @account_limiter.limit('100/minute') AND sets request.state.current_user = current_user as the first statement of its handler body"
- "Existing per-IP limits on auth endpoints (10/minute, 5/hour) are PRESERVED — only the key_func changed"
- "All 8 xfail stubs in backend/tests/test_rate_limiting.py from 06-01 now PASS (xfail markers removed)"
- "Assumption A1 (slowapi key_func evaluation order with request.state) is converted to a passing test before per-account decorators are applied to all endpoints"
artifacts:
- path: "backend/deps/utils.py"
provides: "Replaced get_client_ip body with trusted-proxy CIDR logic; module-level _TRUSTED_PROXY_NETS list; private _is_trusted_proxy helper"
contains: "_TRUSTED_PROXY_NETS"
- path: "backend/services/rate_limiting.py"
provides: "Single canonical account_limiter Limiter instance + _account_key function for per-user rate limiting (D-12)"
exports: ["account_limiter"]
min_lines: 25
- path: "backend/api/auth.py"
provides: "Limiter now uses key_func=get_client_ip (D-11/D-13); existing @limiter.limit decorators on register/login/refresh/password endpoints unchanged"
contains: "key_func=get_client_ip"
- path: "backend/api/documents.py"
provides: "Per-account 100/minute limit on every authenticated endpoint; request.state.current_user set as first handler line"
contains: "@account_limiter.limit"
- path: "backend/api/cloud.py"
provides: "Same per-account 100/minute limit pattern on every endpoint that uses get_regular_user"
contains: "@account_limiter.limit"
- path: "backend/main.py"
provides: "account_limiter imported alongside the existing auth limiter; both limiters live as module-level singletons"
contains: "from services.rate_limiting import account_limiter"
key_links:
- from: "backend/deps/utils.py get_client_ip"
to: "backend/api/auth.py Limiter(key_func=...)"
via: "import + key_func wiring"
pattern: "key_func=get_client_ip"
- from: "backend/services/rate_limiting.py _account_key"
to: "request.state.current_user (set by route handler)"
via: "getattr fallback to IP for safety (Pitfall 3)"
pattern: "request\\.state\\.current_user"
- from: "Route handlers in documents.py / cloud.py"
to: "_account_key"
via: "first line of handler body: request.state.current_user = current_user"
pattern: "request\\.state\\.current_user\\s*=\\s*current_user"
---
Close the rate-limit bypass and per-account fairness gaps for Phase 6. Replace the body of `get_client_ip` in backend/deps/utils.py with trusted-proxy CIDR logic (D-11). Switch the existing auth Limiter from slowapi's `get_remote_address` to `get_client_ip` (D-13 — preserves the existing 10/min and 5/hour limits but stops header spoofing from external clients). Add a second `account_limiter` Limiter instance keyed by `request.state.current_user.id` in a new shared module `backend/services/rate_limiting.py`, and decorate every authenticated endpoint in `documents.py` and `cloud.py` with `@account_limiter.limit("100/minute")` (D-12). Promote all 8 xfail stubs in `tests/test_rate_limiting.py` (from 06-01) to passing.
Purpose: Header-spoofing currently lets any external client claim an arbitrary IP and bypass the IP-based rate limiter. D-11 closes that. D-12 adds a second axis (per-account) so a single compromised account cannot exhaust the application's request capacity by rotating IPs. Together they form the rate-limit half of Phase 6's hardening.
Output: One body-replacement in deps/utils.py (NO new function, NO new public API), two lines changed in auth.py, a new ~30-line rate_limiting.py service module, and decorator/first-line additions across every authenticated endpoint in documents.py and cloud.py.
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
@$HOME/.claude/get-shit-done/templates/summary.md
@.planning/phases/06-performance-production-hardening/06-CONTEXT.md
@.planning/phases/06-performance-production-hardening/06-RESEARCH.md
@.planning/phases/06-performance-production-hardening/06-PATTERNS.md
@CLAUDE.md
@backend/deps/utils.py
@backend/api/auth.py
@backend/api/documents.py
@backend/api/cloud.py
@backend/main.py
@backend/tests/test_rate_limiting.py
@backend/tests/conftest.py
Existing get_client_ip (backend/deps/utils.py:10):
- Signature: `def get_client_ip(request: Request) -> Optional[str]:`
- Currently returns `request.headers.get("X-Forwarded-For") or (request.client.host if request.client else None)`
- TASK: replace BODY only. Signature is unchanged. Add no new public functions. `ipaddress` is stdlib — no new package dependency.
- Existing callers: backend/api/auth.py, audit log helpers, etc. They import by name from deps.utils; the import chain stays identical.
Existing slowapi limiter (backend/api/auth.py:37–44):
- `from slowapi.util import get_remote_address`
- `limiter = Limiter(key_func=get_remote_address)`
- TASK: drop the slowapi.util import; add `from deps.utils import get_client_ip`; change `key_func=get_remote_address` to `key_func=get_client_ip`. Two-line patch. Existing @limiter.limit decorators stay exactly as written.
New shared module (backend/services/rate_limiting.py):
- Exports: `account_limiter: Limiter`, `_account_key(request: Request) -> str` (module-private; reused only inside this file).
- `_account_key` reads `getattr(request.state, "current_user", None)` and returns `str(user.id)` when set; falls back to `request.client.host or "anonymous"` to avoid raising in untested code paths (Pitfall 3).
- Why a separate module: CLAUDE.md mandates one canonical definition for shared utilities; both documents.py and cloud.py will import the same `account_limiter` instance so rate-limit counters are shared across routers.
Per-account decorator contract on each authenticated route:
- `request: Request` MUST appear in the signature (slowapi looks it up by name on the function's first parameters).
- `@account_limiter.limit("100/minute")` decorates the handler.
- First line of handler body: `request.state.current_user = current_user` (so _account_key has the user object available when slowapi evaluates the key — A1 assumption).
- Apply to every route in documents.py and cloud.py that uses `Depends(get_regular_user)`.
Routes in documents.py to decorate (verified at planning time, lines 88, 129, 290, 399, 521, 565, 616, 693, 746):
POST /api/documents/upload-url
POST /api/documents/upload
POST /api/documents/{doc_id}/confirm
GET /api/documents
GET /api/documents/{doc_id}
PATCH /api/documents/{doc_id}
DELETE /api/documents/{doc_id}
POST /api/documents/{doc_id}/classify
GET /api/documents/{doc_id}/content
That's 9 endpoints. All currently take `current_user: User = Depends(get_regular_user)`.
Routes in cloud.py to decorate (verified at planning time, those using get_regular_user — at lines 315, 407, 553, 644, 677, 728, 773, 928 and others):
GET /api/cloud/oauth/initiate/{provider}
GET /api/cloud/oauth/callback/{provider}
POST /api/cloud/connections/webdav
GET /api/cloud/connections
GET /api/cloud/connections/{connection_id}/config
DELETE /api/cloud/connections/{connection_id}
GET /api/cloud/folders/{provider}/{folder_id:path}
(… and any other route in cloud.py that injects get_regular_user — apply uniformly)
Note: oauth_callback returns a RedirectResponse — still safe to apply the limit; per-account is rate-limit on internal users, not OAuth providers.
Wave 0 stubs to promote (backend/tests/test_rate_limiting.py — 8 xfails):
1. test_get_client_ip_untrusted_returns_direct_peer
2. test_get_client_ip_trusted_proxy_reads_xff_leftmost
3. test_get_client_ip_trusted_proxy_no_xff_falls_back
4. test_get_client_ip_invalid_peer_returns_none_or_string
5. test_account_limiter_key_uses_user_id
6. test_account_limiter_key_falls_back_to_ip_when_no_user
7. test_account_limiter_key_ordering_assumption (A1 verification — gates D-12 rollout)
8. test_authenticated_endpoint_429_after_100_per_minute
Task 1: Replace get_client_ip body in deps/utils.py + create services/rate_limiting.py + promote unit-level xfails (tests 1–7)
backend/deps/utils.py, backend/services/rate_limiting.py, backend/tests/test_rate_limiting.py
- backend/deps/utils.py (current body, lines 1–35 — replace the body of get_client_ip ONLY; do NOT add a second function, do NOT rename, do NOT add a module-level import that breaks existing callers)
- backend/tests/test_rate_limiting.py (the 8 xfail stubs from 06-01 — flip the first 7 to real assertions)
- .planning/phases/06-performance-production-hardening/06-RESEARCH.md (Pattern 3, Pattern 4, Pitfall 3, Assumption A1)
- .planning/phases/06-performance-production-hardening/06-PATTERNS.md (deps/utils.py section; account_limiter section; Critical Note 1)
- CLAUDE.md ("Backend: shared module map" — `deps/utils.py` and the rule that no router may define a local variant)
- get_client_ip(request) with request.client.host == "8.8.8.8" and X-Forwarded-For: "1.2.3.4" returns "8.8.8.8" (untrusted peer → ignore XFF; D-11).
- get_client_ip(request) with request.client.host == "127.0.0.1" and X-Forwarded-For: "1.2.3.4, 5.6.7.8" returns "1.2.3.4" (trusted peer → leftmost XFF; D-11).
- get_client_ip(request) with request.client.host == "172.16.5.5" and no XFF returns "172.16.5.5" (trusted peer fallback).
- get_client_ip(request) with request.client is None returns None and does not raise.
- _account_key(request) with request.state.current_user.id == UUID(...) returns the str(UUID) — never the IP.
- _account_key(request) with no request.state.current_user returns the IP (or "anonymous") and does NOT raise (Pitfall 3 — failure mode is "more limiting", not crash).
- A FastAPI ASGI app with one endpoint decorated `@account_limiter.limit("100/minute")` that sets `request.state.current_user = current_user` as its FIRST line, called 101 times within a minute with the same authenticated user, returns 429 on the 101st call AND the limiter's internal counter records the key as `str(user.id)`, not the IP. This proves A1.
Edit backend/deps/utils.py:
(a) Add module-level imports at the top of the existing import block: `import ipaddress`. The `Optional` and `Request` imports already exist; do NOT re-import.
(b) Add a module-level constant directly after the existing imports: `_TRUSTED_PROXY_NETS` as a list of `ipaddress.ip_network(...)` objects for "127.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "::1/128".
(c) Add a private helper `_is_trusted_proxy(host: str) -> bool` that wraps `ipaddress.ip_address(host)` in a try/except ValueError and returns True if the address is in any of the trusted nets.
(d) Replace the BODY of the existing `get_client_ip(request: Request) -> Optional[str]` function with the trusted-proxy logic (RESEARCH.md Pattern 3): compute `direct_peer = request.client.host if request.client else None`; if `direct_peer and _is_trusted_proxy(direct_peer)`, read `request.headers.get("X-Forwarded-For")` and if present return its leftmost comma-split entry stripped of whitespace; otherwise return `direct_peer`. Update the docstring to note "D-11 — trusted-proxy CIDR check" and remove the old "use for audit logging only" warning (the function is now safe for rate-limiting too).
(e) DO NOT add a second function. DO NOT rename. DO NOT touch parse_uuid.
Create backend/services/rate_limiting.py (new file). Module docstring: "Per-account rate limiter shared across document and cloud routers (D-12)." Pattern: `from __future__ import annotations`, `from fastapi import Request`, `from slowapi import Limiter`. Define private function `_account_key(request: Request) -> str` per the behaviour spec — read `getattr(request.state, "current_user", None)`; when set return `str(user.id)`; when None fall back to `request.client.host if request.client else "anonymous"`. Define module-level singleton `account_limiter = Limiter(key_func=_account_key)`. Export both symbols (no __all__ required — they are top-level names).
Edit backend/tests/test_rate_limiting.py:
For tests 1–7 (the unit-level tests — get_client_ip variants, _account_key key selection, A1 ordering), REMOVE the `@pytest.mark.xfail(...)` decorators and REPLACE the single-line `pytest.xfail(...)` bodies with real assertions per the behaviour spec above. Use a lightweight mock Request (or starlette.requests.Request constructed from a minimal scope dict) — do NOT spin up the full app for unit tests 1–6.
For test 7 (A1 ordering): build a minimal FastAPI app inline (within the test), register one GET endpoint that sets `request.state.current_user = SimpleNamespace(id=uuid.uuid4())` as its first line, decorate it with `@account_limiter.limit("100/minute")`, mount via TestClient. Send 101 requests in a tight loop with the SAME peer IP but VARYING X-Forwarded-For; assert the 101st response is 429 (proves the key is the constant user.id, NOT the per-request IP). Skip integration test #8 in this task — Task 2 handles it.
cd backend && pytest tests/test_rate_limiting.py -v --no-header -k "not test_authenticated_endpoint_429_after_100_per_minute" 2>&1 | tail -5 | grep -E "7 passed"
- `grep -c "^def get_client_ip" backend/deps/utils.py` returns 1 (single canonical definition — no new function added).
- `grep -c "_TRUSTED_PROXY_NETS" backend/deps/utils.py` returns ≥ 2 (definition + use).
- `grep -c "^import ipaddress" backend/deps/utils.py` returns 1.
- `grep -c "_is_trusted_proxy" backend/deps/utils.py` returns ≥ 2 (definition + use).
- The replaced get_client_ip body no longer reads X-Forwarded-For unconditionally: `awk '/^def get_client_ip/,/^def [a-zA-Z]/' backend/deps/utils.py | grep -cE 'request\\.headers\\.get\\("X-Forwarded-For"\\)\\s*or'` returns 0.
- `test -f backend/services/rate_limiting.py` exits 0.
- `grep -c "^account_limiter = Limiter" backend/services/rate_limiting.py` returns 1.
- `grep -c "def _account_key" backend/services/rate_limiting.py` returns 1.
- Module imports cleanly: `cd backend && python -c "from services.rate_limiting import account_limiter, _account_key"` exits 0.
- Tests 1–7 in test_rate_limiting.py are no longer xfail and now PASS: `cd backend && pytest tests/test_rate_limiting.py -v --no-header -k "not test_authenticated_endpoint_429_after_100_per_minute" | tail -3 | grep -c "7 passed"` returns 1.
- `grep -c "pytest.mark.xfail" backend/tests/test_rate_limiting.py` returns 1 (only the integration test #8 remains xfail until Task 2).
- All other existing pytest tests continue to pass: no NEW failures introduced.
get_client_ip body replaced in-place per CLAUDE.md single-canonical-definition rule; account_limiter singleton lives in services/rate_limiting.py; 7 unit tests pass including the A1 ordering test that gates Task 2.
Task 2: Switch auth.py Limiter to get_client_ip + decorate documents.py and cloud.py with @account_limiter + promote integration xfail (test 8)
backend/api/auth.py, backend/api/documents.py, backend/api/cloud.py, backend/main.py, backend/tests/test_rate_limiting.py
- backend/api/auth.py (lines 30–45 — Limiter declaration; lines 37–44 — current key_func)
- backend/api/documents.py (the 9 authenticated endpoints listed in the block; signatures around lines 88, 129, 290, 399, 521, 565, 616, 693, 746)
- backend/api/cloud.py (the endpoints using get_regular_user listed in the block)
- backend/main.py (existing app.state.limiter assignment and SlowAPIMiddleware registration — confirm both stay as-is for the IP limiter; account_limiter does not need app.state wiring)
- backend/tests/test_rate_limiting.py (the remaining xfail integration test #8)
- .planning/phases/06-performance-production-hardening/06-RESEARCH.md (Pattern 4 + Pitfall 3 — slowapi key_func evaluation order and Request-first-param requirement)
- .planning/phases/06-performance-production-hardening/06-PATTERNS.md (auth.py section; documents.py per-account pattern; cloud.py section)
- backend/api/auth.py imports get_client_ip from deps.utils and passes it as key_func to its Limiter; get_remote_address is no longer imported.
- Existing @limiter.limit("10/minute") and @limiter.limit("5/hour") decorators on register/login/refresh/password endpoints remain unchanged (D-13 — limits preserved).
- Every authenticated endpoint in documents.py (9 endpoints) has `@account_limiter.limit("100/minute")` directly above the route function definition and `request.state.current_user = current_user` as the FIRST line of the handler body.
- Every authenticated endpoint in cloud.py (the routes using Depends(get_regular_user)) has the same decorator and same first-line assignment.
- Every decorated endpoint's signature has `request: Request` as a parameter (some already have it; for those that don't, add it as a parameter so slowapi can find it by name).
- main.py imports `account_limiter` from `services.rate_limiting` (alongside the existing auth limiter wiring); no additional app.state wiring is required for account_limiter because its decorators are applied directly to handlers.
- Integration test #8 (test_authenticated_endpoint_429_after_100_per_minute) now PASSES — 101 GET /api/documents/ requests with the SAME auth token returns 429 on the 101st.
- Per-account limit shape: 100 requests per minute per authenticated user.id, shared across all decorated endpoints (because they share the same account_limiter instance).
- Existing IP limits on auth endpoints still trigger correctly — register/login 429s after 10 attempts/min from the same client IP (verified by re-running the existing auth rate-limit tests).
Edit backend/api/auth.py:
(a) Remove the `from slowapi.util import get_remote_address` import line.
(b) The existing `from deps.utils import get_client_ip` import is already present (line 34) — do NOT re-add.
(c) Change line 44: `limiter = Limiter(key_func=get_remote_address)` to `limiter = Limiter(key_func=get_client_ip)`.
(d) Do NOT modify any of the existing @limiter.limit(...) decorators — they preserve the existing 10/min and 5/hour limits per D-13.
Edit backend/main.py:
(a) Add an import line near the other service imports: `from services.rate_limiting import account_limiter`.
(b) Leave `app.state.limiter = auth_limiter` (or whatever name is currently used) UNCHANGED — SlowAPIMiddleware drives only the limiter assigned to app.state. The account_limiter is invoked directly via its decorators.
Edit backend/api/documents.py:
For EACH of the 9 endpoints listed in the block (request_upload_url, upload_document, confirm_upload, list_documents, get_document, patch_document, delete_document, classify_document, stream_document_content):
(a) Add at the top of the file (alongside the existing imports): `from services.rate_limiting import account_limiter`.
(b) Above each route function definition, immediately under the `@router.(...)` decorator, add `@account_limiter.limit("100/minute")`.
(c) Ensure the function signature has `request: Request` as a parameter (some handlers may already have it — keep). If a handler doesn't, ADD `request: Request` as the first parameter after any path/body params (FastAPI parameter order rules allow Request anywhere; slowapi finds it by type/name).
(d) Make `request.state.current_user = current_user` the FIRST executable statement of the handler body — before any other logic. This is the line that satisfies the A1 contract proven by Task 1's ordering test.
Edit backend/api/cloud.py:
Apply the same three changes (import account_limiter; decorate with @account_limiter.limit("100/minute"); first-line `request.state.current_user = current_user`) to EVERY endpoint in cloud.py whose signature contains `Depends(get_regular_user)`. Use `grep -n "Depends(get_regular_user)" backend/api/cloud.py` to enumerate the targets and ensure 100% coverage. The endpoint at /api/cloud/oauth/callback/{provider} returns a RedirectResponse — that's fine; rate-limit applies to the authenticated user regardless of response type.
Edit backend/tests/test_rate_limiting.py:
Remove the `@pytest.mark.xfail(...)` decorator from test_authenticated_endpoint_429_after_100_per_minute. Replace its single-line body with real assertions: use the async_client fixture from conftest.py and the auth_user fixture, hit GET /api/documents/ 101 times in a tight loop with the same Bearer token, assert the 101st response is 429. Implementation note: because slowapi's default in-memory storage is per-process, the test must run within a single test client lifetime; reset isn't required because each test gets a fresh app from conftest.
Do NOT modify any endpoint signature beyond adding `request: Request` where missing. Do NOT change @router.(...) URL paths, body models, response models, or dependency lists.
cd backend && pytest tests/test_rate_limiting.py tests/test_auth*.py -v --no-header 2>&1 | tail -5 | grep -E "passed"
- `grep -c "from slowapi.util import get_remote_address" backend/api/auth.py` returns 0 (import removed).
- `grep -c "key_func=get_client_ip" backend/api/auth.py` returns 1 (Limiter switched to trusted-proxy key).
- `grep -c "key_func=get_remote_address" backend/api/auth.py` returns 0.
- All existing `@limiter.limit(` decorators in auth.py remain — `grep -c "@limiter.limit(" backend/api/auth.py` returns the SAME number as before this plan (use git diff to confirm).
- `grep -c "from services.rate_limiting import account_limiter" backend/main.py` returns 1.
- `grep -c "from services.rate_limiting import account_limiter" backend/api/documents.py` returns 1.
- `grep -c "from services.rate_limiting import account_limiter" backend/api/cloud.py` returns 1.
- Every authenticated endpoint in documents.py has the per-account decorator: `grep -c "@account_limiter.limit" backend/api/documents.py` returns 9 (matches the 9 endpoints listed in ).
- Every documents.py decorated handler sets request.state.current_user as its first line: `grep -c "request\\.state\\.current_user\\s*=\\s*current_user" backend/api/documents.py` returns 9.
- Every cloud.py endpoint using get_regular_user has the decorator: `[ "$(grep -c 'Depends(get_regular_user)' backend/api/cloud.py)" = "$(grep -c '@account_limiter.limit' backend/api/cloud.py)" ]` exits 0 (equal counts).
- Every cloud.py decorated handler sets request.state.current_user as first line: `[ "$(grep -c '@account_limiter.limit' backend/api/cloud.py)" = "$(grep -c 'request\\.state\\.current_user\\s*=\\s*current_user' backend/api/cloud.py)" ]` exits 0.
- All 8 tests in test_rate_limiting.py PASS: `cd backend && pytest tests/test_rate_limiting.py -v --no-header | tail -3 | grep -c "8 passed"` returns 1.
- `grep -c "pytest.mark.xfail" backend/tests/test_rate_limiting.py` returns 0 (no remaining xfails).
- Existing auth rate-limit tests still pass (no regression on the per-IP limits): `cd backend && pytest tests/ -v --no-header -k "rate_limit or limiter or test_auth" | tail -3 | grep -c "failed"` returns 0.
- Full backend pytest run shows no NEW failures vs. the 06-04 baseline.
auth.py keyed by get_client_ip with old limits preserved; documents.py and cloud.py uniformly decorated with the per-account limiter; all 8 rate-limiting tests pass; no signature/URL/body-model changes leaked into the patch.
## Trust Boundaries
| Boundary | Description |
|----------|-------------|
| External client → FastAPI | Client controls all HTTP headers, including X-Forwarded-For; FastAPI must distinguish trusted-proxy origin from arbitrary internet origin |
| Reverse proxy → FastAPI (intra-cluster) | Trusted CIDR; X-Forwarded-For is authoritative for the originating client |
| Authenticated user → application capacity | A compromised account rotating IPs bypasses the IP limiter; per-account limit closes that hole |
## STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-06-05-01 | Tampering | Rate-limit bypass via X-Forwarded-For spoofing from external clients | mitigate | get_client_ip body replaced with trusted-proxy CIDR check (D-11); external clients can no longer claim arbitrary IPs |
| T-06-05-02 | Denial of Service | Compromised account exhausts request capacity by rotating IPs (defeats per-IP limit) | mitigate | account_limiter keyed by str(current_user.id) caps each user at 100 req/min across all decorated endpoints (D-12) |
| T-06-05-03 | Tampering | account_limiter key_func runs BEFORE request.state.current_user is set → silently falls back to IP, defeats D-12 | mitigate | A1 verification test (test_account_limiter_key_ordering_assumption) in Task 1 — the first-line `request.state.current_user = current_user` contract is verified before applying decorators to all endpoints in Task 2 |
| T-06-05-04 | Denial of Service | _account_key raises when request.state.current_user is unset → crashes request handling | mitigate | _account_key returns IP fallback or "anonymous" string instead of raising (Pitfall 3) |
| T-06-05-05 | Information Disclosure | Per-account 429 reveals authenticated user enumeration to attacker | accept | The 429 only fires for already-authenticated users; the attacker already proved knowledge of credentials. Information leak is bounded by the existing per-IP limit on login endpoints (preserved by D-13). |
| T-06-05-06 | Tampering | Duplicate get_client_ip definitions drift apart over time (CLAUDE.md anti-pattern) | mitigate | Replace BODY in-place; no new function created; grep gate verifies single canonical definition |
- `grep -c "^def get_client_ip" backend/deps/utils.py` returns 1.
- `grep -c "key_func=get_remote_address" backend/api/auth.py` returns 0.
- `grep -c "key_func=get_client_ip" backend/api/auth.py` returns 1.
- `grep -c "from services.rate_limiting import account_limiter" backend/main.py backend/api/documents.py backend/api/cloud.py` returns 3 (one per file).
- All 8 tests in backend/tests/test_rate_limiting.py PASS.
- Full pytest suite green; no regressions in existing auth rate-limit tests.
- Per-account decorator count matches authenticated endpoint count in documents.py (9) and equals the get_regular_user count in cloud.py.
- D-11 satisfied: get_client_ip uses trusted-proxy CIDR validation; external clients can no longer spoof XFF.
- D-12 satisfied: account_limiter Limiter keyed by user.id; all documents.py + cloud.py authenticated endpoints decorated; per-account 100/minute enforced.
- D-13 satisfied: existing per-IP limits on auth endpoints (10/min, 5/hour) preserved verbatim; only the key_func changed.
- All 8 Wave 0 xfails promoted to PASS — including A1 ordering verification.
- Zero new failures in the full backend pytest suite.