Files
kleiderboerse/Plan-Verbesserungen.md
T
StefanandClaude Fable 5 c0a18ac735 Wettlauf beim Reservieren, 500er bei unsinnigen Eingaben
Drei P3-Funde aus der Durchsicht vom 14.09.:

Reservieren: Statusprüfung und Schreiben waren getrennt - zwei
gleichzeitige Anfragen konnten beide passieren, die zweite überschrieb
Name und Token der ersten, ohne dass die es erfuhr. Jetzt entscheidet
ein UPDATE mit Status-Bedingung; der Verlierer bekommt None und die
Route meldet "schon weg" (Seite) bzw. 409 (API).

/?limit=abc lieferte jedem anonymen Besucher einen internen
Serverfehler, limit=-1 hiess in SQLite "alles". _limit_lesen() fällt
bei Unsinn auf die Seitengrösse zurück und deckelt bei 500.

Erfassen/Bearbeiten: int(category_id) und die Pydantic-Prüfung warfen
im Handler - 500 statt Fehlermeldung. Jetzt Meldung; ausserdem werden
die Angaben VOR den Bildern geprüft, damit bei abgelehnten Angaben
keine verwaisten Bilddateien liegen bleiben.

Vier neue Tests, jeder einmal gegen den alten Code gelaufen und dabei
rot geworden. 84 lokal grün (HEIC-Test braucht pillow-heif, Docker).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-09-14 17:22:54 +02:00

9.3 KiB
Raw Permalink 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

  1. Täglicher Lauf, sieben Stände, danach rollierend überschreiben.
  2. Die Bilder gehören dazu — eine Datenbank ohne uploads/ ist wertlos.
  3. 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

  1. Sicherungen einrichten, inklusive Bilder, mit einer echten Rückspielprobe
  2. GZip und Cache-Dauer — zusammen wenige Zeilen
  3. 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 ERLEDIGT 14.09.2026

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 ERLEDIGT 14.09.2026

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 ERLEDIGT 14.09.2026

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.

Umsetzung der drei P3 am 14.09.2026

Punkt Ergebnis
?limit=abc _limit_lesen() in pages.py: ungültig → Seitengrösse, Deckel 500
Betreiber-Formulare Angaben werden vor dem Bildspeichern geprüft; ValueError → Meldung statt 500 (und keine verwaisten Bilddateien mehr)
Reservieren crud.reservieren() schreibt per UPDATE … WHERE status='available'; der Verlierer des Wettlaufs bekommt None → „schon weg"

Vier neue Tests decken genau diese Fälle ab; alle vier wurden nach der Hausregel einmal gegen den alten Code laufen gelassen und sind dabei nachweislich rot geworden. Suite: 85 Tests (84 lokal grün, der HEIC-Test braucht pillow-heif und läuft im Docker-Testimage).