Files

T

StefanandClaude Fable 5 33c728411f X-Forwarded-For nur noch dem Proxy glauben, Durchsicht vom 14.09. notiert

Das CMD startete uvicorn mit --forwarded-allow-ips "*": jeder, der den
Container-Port direkt erreicht, konnte die Absender-IP frei erfinden und
bekam pro erfundener IP einen frischen Rate-Limit-Zaehler - und die
Limits sind das Einzige vor dem frei zugaenglichen Reservieren-Endpunkt.
Zusaetzlich liess sich so die HTTPS-Erkennung fuers Sitzungs-Cookie
faelschen.

Jetzt liest uvicorn die Grenze aus FORWARDED_ALLOW_IPS (ohne Wert: nur
127.0.0.1, also keinem externen Absender trauen). docker-compose.betrieb.yml
reicht die Variable mit Erklaerung durch; das README beschreibt, welche
Adresse hineingehoert und dass der Port nur fuer den Proxy erreichbar
sein darf. Wird die Variable nicht gesetzt, teilen sich alle Besucher
die Zaehler ueber die Proxy-IP - das kleinere Uebel.

Plan-Verbesserungen.md: Ergebnis der erneuten Durchsicht vom 14.09.2026
ergaenzt (GZip/Cache/Sicherung vom 13.09. nachgeprueft und fuer gut
befunden; drei P3-Funde: oeffentlicher 500er ueber ?limit=abc, nicht
atomares Reservieren, 500 statt Fehlermeldung in Betreiber-Formularen).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

2026-09-14 17:19:12 +02:00

8.5 KiB

Raw Blame History

Plan: Verbesserungen

Erstellt am 13.09.2026 nach einer Durchsicht des gesamten Repos, parallel zur Kantone-App. Ergänzt Plan.md, der den ursprünglichen Aufbau beschreibt.

Noch nichts davon ist umgesetzt.

Vorabprüfung (13.09.2026, erledigt)


Arbeitsverzeichnis	sauber
Offene Branches	keine
Tests	81 grün in 12 Sekunden
Letzter Tag	v1.0.0

Der CI-Lauf zu v1.0.0 wurde nie bestätigt. Der Tag wurde einmal auf dem Server gelöscht und danach auf e0212cf neu gesetzt. Vor dem nächsten Release prüfen, ob in der Registry tatsächlich ein Image liegt.

Zum Zustand des Codes

Die Durchsicht hat wenig ergeben, und das ist das Ergebnis: Escaping, CSRF, Token-Vergleich in konstanter Zeit, Bild-Upload mit Inhaltsprüfung statt Endung, bewusst vermiedenes N+1, CHECK-Constraints in der Datenbank statt nur in Python — das ist alles vorhanden und richtig gemacht.

Die Punkte unten sind Ergänzungen, keine Korrekturen.

P1 — Keine Sicherung der Datenbank ✅ ERLEDIGT 13.09.2026

Der wichtigste offene Punkt, und hier wiegt er schwerer als bei der Kantone-App: In kleiderboerse.sqlite stecken Einträge und in uploads/ die dazugehörigen Fotos. Beides ist nicht wiederherstellbar — anders als die Kantonsdaten, die sich neu importieren liessen.

docker-compose.betrieb.yml sieht keinen Sicherungsschritt vor.

Ein cp der laufenden Datei genügt nicht; sie kann mitten in einer Schreiboperation erwischt werden. Richtig ist sqlite3 ... ".backup ..." oder VACUUM INTO, beide konsistent auch bei laufendem Zugriff.

Vorgehen

Täglicher Lauf, sieben Stände, danach rollierend überschreiben.
Die Bilder gehören dazu — eine Datenbank ohne uploads/ ist wertlos.
Einmal einen Stand tatsächlich zurückspielen und prüfen, dass die Galerie danach vollständig ist. Eine ungetestete Sicherung ist keine.

Punkt 3 ist kein Formalismus: Am 13.09.2026 ist Jellyfin auf demselben Netzwerk an einer Datenbank-Migration hängengeblieben. Dass es dort Sicherungen gab, war der einzige Grund, warum die Lage beherrschbar blieb.

P2 — Antworten werden nicht komprimiert ✅ ERLEDIGT 13.09.2026

app/main.py bindet keine GZipMiddleware ein. Jede HTML- und HTMX-Antwort geht unkomprimiert über die Leitung — bei einer Galerie mit vielen Kacheln merkbar, besonders über Mobilfunk.

from fastapi.middleware.gzip import GZipMiddleware
app.add_middleware(GZipMiddleware, minimum_size=1000)

P2 — Statische Dateien ohne Cache-Dauer ✅ ERLEDIGT 13.09.2026

app.mount("/static", StaticFiles(directory=...), name="static")

Ohne max_age werden htmx.min.js, theme.js und die beiden Swagger-Dateien bei jedem Aufruf neu verhandelt. Eine lange Cache-Dauer setzen und bei Änderungen einen Versionsanhänger an die URL hängen.

Korrektur: Für die hochgeladenen Bilder galt das nicht — die haben ihre Cache-Header längst (app/routers/images.py setzt max-age=31536000, immutable). Das war eine Fehlannahme beim Erstellen dieses Plans.

P4 — Zwei Filterspalten ohne Index

crud.items_suchen() filtert unter anderem nach gender und season. Beide Spalten haben keinen Index; status, size und category_id haben einen.

Bei den zu erwartenden Stückzahlen ist das ohne praktische Bedeutung — der Vollständigkeit halber notiert, nicht als Handlungsbedarf.

Reihenfolge

Sicherungen einrichten, inklusive Bilder, mit einer echten Rückspielprobe
GZip und Cache-Dauer — zusammen wenige Zeilen
Rest nach Bedarf

Umsetzung am 13.09.2026 (Branch `verbesserungen`)

Punkt	Ergebnis
Sicherung	`sicherung.sh`, Datenbank und Bilder, geprüft
GZip	Startseite 9725 → 1900 Byte (−80 %)
Statische Dateien	`Cache-Control: public, max-age=3600`
Tests	81 grün, unverändert

Die Sicherung wurde gegen den laufenden Container erprobt: 4 Einträge, 4 Bilder, alle in der Datenbank verzeichneten Bilder auch wirklich im Sicherungsordner. Gegenprobe mit entferntem Bild wird erkannt.

Offen bleibt P4 (Indizes auf gender/season) — ohne praktische Bedeutung.

Durchsicht 14.09.2026

Erneute Durchsicht des gesamten Repos, mit Schwerpunkt auf den seit dem 13.09. gemergten Änderungen (c986161, Branch verbesserungen) — die waren bisher ungeprüft.

Ergebnis der Nachprüfung: GZip, Cache-Header und Sicherungsskript sind korrekt umgesetzt. Tests lokal ausgeführt: 80 von 81 grün — der eine rote (test_heic_vom_iphone_wird_angenommen) liegt allein daran, dass pillow-heif in der improvisierten Umgebung fehlte (Python 3.14, kein Wheel); im Docker-Testimage ist das Paket enthalten. Kein Code-Fehler.

P2 — `--forwarded-allow-ips "*"` macht die Rate-Limits umgehbar ✅ ERLEDIGT 14.09.2026

Das CMD im Dockerfile vertraut X-Forwarded-For von jedem Absender. Wer den Container-Port direkt erreicht (das Compose veröffentlicht 8090:8000 auf dem Host), setzt den Header selbst und bekommt pro erfundener IP einen frischen Zähler — die Rate-Limits sind laut eigenem Kommentar „das Einzige, was zwischen einem Skript und dem Bestand steht", und genau die fallen damit. Zusätzlich lässt sich so die HTTPS-Erkennung fürs Cookie fälschen.

Hinter einem sauber konfigurierten Proxy, der den Header ersetzt, und mit einem von aussen unerreichbaren Port 8000 ist alles gut — aber darauf verlässt sich das Image derzeit stillschweigend.

Vorgehen: FORWARDED_ALLOW_IPS als Umgebungsvariable durchreichen (Standard: leer = keinem trauen) und in docker-compose.betrieb.yml die Proxy-IP eintragen; dort ausserdem den Hinweis ergänzen, dass der veröffentlichte Port nur für den Proxy erreichbar sein darf.

P3 — Öffentlicher 500er über `?limit=abc`

pages.py (galerie, liste_ausschnitt) rechnet int(request.query_params.get("limit") or SEITE) ohne Prüfung — /?limit=x liefert jedem anonymen Besucher einen internen Serverfehler, ?limit=-1 bedeutet in SQLite „alles". Die API-Seite macht es mit Query(ge=1, le=200) längst richtig; dieselbe Grenze hier nachziehen (ungültig → Standardwert).

P3 — Betreiber-Formulare: 500 statt Fehlermeldung

In erfassen und bearbeiten (pages.py) wird int(category_id) ohne Prüfung gerechnet, und ItemAnlegen(...) wirft bei ungültigen Werten eine ValidationError im Handler — beides ergibt einen 500er statt der sonst üblichen freundlichen Meldung. Erreichbar nur angemeldet, und die HTML-Formulare (Dropdown, maxlength) verhindern es auf dem normalen Weg — es bricht aber das Muster, das reservieren mit try/except vormacht.

P3 — Reservieren: Prüfen und Setzen sind nicht atomar

reservieren prüft erst status == available und schreibt dann. Zwei gleichzeitige Anfragen können beide die Prüfung passieren; der zweite Commit überschreibt Name und Token der ersten Person, ohne dass die es erfährt — und genau dieser Fall (zwei wollen dasselbe Stück) ist der wahrscheinlichste Konflikt der ganzen App. Robust wäre ein UPDATE … SET status='reserved' … WHERE id=? AND status='available' mit Prüfung der betroffenen Zeilen.

P4 — Kleinigkeiten (neu)

HEIC-Test könnte mit pytest.importorskip("pillow_heif") sauber übersprungen werden, wenn das Paket fehlt — dann bleibt die Suite auch ausserhalb des Docker-Images aussagekräftig.
description ohne Längenbegrenzung (schemas.py) — Titel und Grösse haben eine, das Beschreibungsfeld nicht. Nur vom Betreiber befüllbar.
Freigabe-Link trägt das Token in der URL (GET /kleid/…/freigeben) — landet damit in Proxy-Logs und Browserverlauf. Für den Zweck (einmalige Selbst-Freigabe) hingenommen, der Vollständigkeit halber notiert.

Stand der alten Punkte

P4 (Indizes auf gender/season) bleibt offen und bleibt bedeutungslos.

Umsetzung des P2 am 14.09.2026

--forwarded-allow-ips "*" ist aus dem CMD entfernt; uvicorn liest die Grenze jetzt aus der Umgebungsvariable FORWARDED_ALLOW_IPS (ohne Wert: nur 127.0.0.1, also keinem externen Absender trauen — fail closed). docker-compose.betrieb.yml reicht die Variable mit Erklärung durch, das README beschreibt, welche Adresse hineingehört und dass der Port nur für den Proxy erreichbar sein darf.

Bewusste Folge der sicheren Vorgabe: Wer die Variable nicht setzt, hat funktionierende Rate-Limits, aber alle Besucher teilen sich die Zähler über die Proxy-IP. Das ist das kleinere Übel gegenüber frei erfindbaren Absenderadressen. Beim nächsten Deployment die Proxy-IP eintragen und einmal prüfen, dass request.client.host die echte Besucher-IP zeigt.

8.5 KiB Raw Blame History Unescape Escape