fix(prezentacja): limit zadan po adresie klienta, nie proxy (PRE-16)
Testy / Testy warstwy logicznej (silnik) (push) Successful in 10m47s
Testy / Testy warstwy prezentacji (dostęp do baz) (push) Successful in 9m33s
Testy / Build obrazu silnika B (swisseph) (push) Successful in 31s
Testy / Kontrola składni wszystkich warstw (push) Successful in 21s
Testy / Testy warstwy logicznej (silnik) (pull_request) Successful in 10m46s
Testy / Testy warstwy prezentacji (dostęp do baz) (pull_request) Successful in 9m53s
Testy / Build obrazu silnika B (swisseph) (pull_request) Successful in 32s
Testy / Kontrola składni wszystkich warstw (pull_request) Successful in 20s

Po wlaczeniu TLS aplikacja stanie za Ingressem, a wtedy `request.client.host`
to adres POD-a Traefika — jednakowy dla wszystkich. Limiter wrzucalby caly ruch
do jednego wiadra 120/min i pierwsza osoba, ktora go wyklika, odcielaby
pozostalych. Cicha regresja, ktora ujawnilaby sie dopiero na produkcji.

Nowe `client_ip()` czyta adres z naglowka, ale WYLACZNIE przy TRUST_PROXY —
bo inaczej wystarczyloby dopisywac wlasny X-Forwarded-For, zeby przy kazdym
zadaniu wygladac na kogos innego i ominac limit calkowicie. Z tego samego
powodu bierzemy OSTATNI wpis listy: to jedyny, ktory dopisal nasz proxy;
wczesniejsze mogl podstawic klient, wiec nie znacza nic.

Szesc testow, w tym dwa istotne:
- podszycie sie pod X-Forwarded-For NIE resetuje wiadra przy wylaczonym
  TRUST_PROXY (inaczej baze dalo by sie pompowac bez ograniczen),
- za proxy dwa rozne adresy dostaja osobne wiadra i nie odcinaja sie nawzajem.

Oba sprawdzone celowym zepsuciem implementacji (zawsze ufaj naglowkowi +
bierz pierwszy wpis) — testy wtedy czerwienieja. 23 passed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-07-22 23:26:57 +02:00
parent 163ace4283
commit 91644a44e3
2 changed files with 102 additions and 3 deletions
+34 -3
View File
@@ -7,7 +7,9 @@ jakiegokolwiek modelu językowego. Ten moduł zamyka tę drogę.
Dwa mechanizmy: Dwa mechanizmy:
* **HTTP Basic** — wejście do aplikacji; włącza się, gdy ustawiono APP_PASSWORD. * **HTTP Basic** — wejście do aplikacji; włącza się, gdy ustawiono APP_PASSWORD.
* **limit żądań** — hamuje masowe odpytywanie (eksfiltrację przez pętlę zapytań). * **limit żądań** — hamuje masowe odpytywanie (eksfiltrację przez pętlę zapytań);
rozliczany per adres klienta, a za odwrotnym proxy — po TRUST_PROXY=true —
per adres z nagłówka, nie per adres proxy (patrz `client_ip`).
Świadomie NIE logujemy treści żądań ani promptów — logi to kolejny nośnik wycieku. Świadomie NIE logujemy treści żądań ani promptów — logi to kolejny nośnik wycieku.
@@ -46,6 +48,11 @@ def app_password() -> str:
def rate_limit_per_min() -> int: def rate_limit_per_min() -> int:
return int(os.getenv("RATE_LIMIT_PER_MIN", "120")) return int(os.getenv("RATE_LIMIT_PER_MIN", "120"))
def trust_proxy() -> bool:
return os.getenv("TRUST_PROXY", "").strip().lower() in {"1", "true", "yes", "on"}
PUBLIC_PATHS = frozenset({"/health"}) PUBLIC_PATHS = frozenset({"/health"})
PUBLIC_PREFIXES = ("/static/",) PUBLIC_PREFIXES = ("/static/",)
@@ -74,6 +81,31 @@ def _authorized(header: str | None) -> bool:
return ok_user and ok_pass return ok_user and ok_pass
def client_ip(request: Request) -> str:
"""Adres, po którym rozliczamy limit żądań.
Za odwrotnym proxy (u nas: Ingress/Traefik po włączeniu TLS — PRE-16)
`request.client.host` to adres POD-a proxy, jednakowy dla wszystkich. Bez
poprawki cały ruch trafiałby do jednego wiadra i pierwsza osoba, która
wyklika limit, odcięłaby pozostałe.
Nagłówkom wierzymy WYŁĄCZNIE przy TRUST_PROXY — bo inaczej wystarczyłoby
dopisać własny `X-Forwarded-For`, żeby przy każdym żądaniu wyglądać na kogoś
innego i ominąć limit całkowicie. Z tego samego powodu bierzemy OSTATNI wpis
listy: to jedyny, który dopisał nasz proxy. Wcześniejsze mógł podstawić
klient, więc nie znaczą nic.
"""
peer = request.client.host if request.client else "?"
if not trust_proxy():
return peer
forwarded = request.headers.get("x-forwarded-for", "")
if forwarded:
last = forwarded.rsplit(",", 1)[-1].strip()
if last:
return last
return request.headers.get("x-real-ip", "").strip() or peer
def _rate_limited(client: str) -> bool: def _rate_limited(client: str) -> bool:
cap = rate_limit_per_min() cap = rate_limit_per_min()
if cap <= 0: if cap <= 0:
@@ -105,8 +137,7 @@ def install(app) -> None:
if _is_public(request.url.path): if _is_public(request.url.path):
return await call_next(request) return await call_next(request)
client = request.client.host if request.client else "?" if _rate_limited(client_ip(request)):
if _rate_limited(client):
return JSONResponse( return JSONResponse(
{"detail": "Zbyt wiele żądań — spróbuj za chwilę."}, {"detail": "Zbyt wiele żądań — spróbuj za chwilę."},
status_code=429, headers={"Retry-After": "60"}, status_code=429, headers={"Retry-After": "60"},
@@ -131,3 +131,71 @@ def test_rate_limit_disabled_when_zero(monkeypatch):
monkeypatch.delenv("APP_PASSWORD", raising=False) monkeypatch.delenv("APP_PASSWORD", raising=False)
client = TestClient(_app()) client = TestClient(_app())
assert all(client.get("/significators").status_code == 200 for _ in range(30)) assert all(client.get("/significators").status_code == 200 for _ in range(30))
# ------------------------------------------ adres klienta za odwrotnym proxy
#
# Po włączeniu TLS (PRE-16) aplikacja stoi za Ingressem, więc bezpośredni peer
# to zawsze POD proxy. Te testy pilnują obu stron kompromisu: żeby limit dalej
# rozróżniał ludzi, a jednocześnie żeby nagłówek nie stał się furtką do jego
# ominięcia.
class _Req:
"""Minimalny zamiennik Request — `client_ip` czyta tylko te dwa pola."""
def __init__(self, peer: str | None, **headers: str):
self.client = type("C", (), {"host": peer})() if peer else None
self.headers = {k.replace("_", "-"): v for k, v in headers.items()}
def test_client_ip_ignores_headers_without_trust_proxy(monkeypatch):
"""Bez TRUST_PROXY nagłówek jest bezwartościowy — każdy może go dopisać."""
monkeypatch.delenv("TRUST_PROXY", raising=False)
req = _Req("10.42.0.7", x_forwarded_for="1.2.3.4", x_real_ip="5.6.7.8")
assert security.client_ip(req) == "10.42.0.7"
def test_client_ip_takes_last_forwarded_entry(monkeypatch):
"""Ostatni wpis dopisał NASZ proxy; wcześniejsze mógł podstawić klient."""
monkeypatch.setenv("TRUST_PROXY", "true")
req = _Req("10.42.0.7", x_forwarded_for="1.1.1.1, 2.2.2.2, 192.168.1.50")
assert security.client_ip(req) == "192.168.1.50"
def test_client_ip_falls_back_to_real_ip(monkeypatch):
monkeypatch.setenv("TRUST_PROXY", "true")
req = _Req("10.42.0.7", x_real_ip="192.168.1.50")
assert security.client_ip(req) == "192.168.1.50"
def test_client_ip_falls_back_to_peer_when_headers_missing(monkeypatch):
monkeypatch.setenv("TRUST_PROXY", "true")
assert security.client_ip(_Req("10.42.0.7")) == "10.42.0.7"
def test_spoofed_forwarded_header_cannot_dodge_the_limit(monkeypatch):
"""Sedno sprawy: bez zaufania do proxy podszywanie się NIE resetuje wiadra.
Gdyby limiter brał pierwszy lepszy `X-Forwarded-For`, wystarczyłoby zmieniać
go co żądanie, żeby pompować bazę bez ograniczeń.
"""
monkeypatch.delenv("TRUST_PROXY", raising=False)
monkeypatch.setenv("RATE_LIMIT_PER_MIN", "3")
monkeypatch.delenv("APP_PASSWORD", raising=False)
client = TestClient(_app())
codes = [client.get("/significators", headers={"X-Forwarded-For": f"9.9.9.{i}"}).status_code
for i in range(6)]
assert 429 in codes
def test_proxied_clients_get_separate_buckets(monkeypatch):
"""Za proxy dwie różne osoby nie mogą się nawzajem odcinać."""
monkeypatch.setenv("TRUST_PROXY", "true")
monkeypatch.setenv("RATE_LIMIT_PER_MIN", "3")
monkeypatch.delenv("APP_PASSWORD", raising=False)
client = TestClient(_app())
first = [client.get("/significators", headers={"X-Forwarded-For": "192.168.1.50"}).status_code
for _ in range(5)]
second = client.get("/significators", headers={"X-Forwarded-For": "192.168.1.51"})
assert 429 in first, "limit musi zadziałać dla pierwszego adresu"
assert second.status_code == 200, "drugi adres ma własne wiadro"