security: server.py Restbefunde aus Review (r) – GET-Leser-Lock, Content-Length-Haertung, secrets-Token

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

CHANGES.md

@@ -1265,3 +1265,11 @@
 2026-07-04 14:19 [Edit] /home/steggi/pyramid/PROGRESS.md
 2026-07-04 14:20 [Edit] /home/steggi/pyramid/PROGRESS.md
 2026-07-04 14:20 [Edit] /home/steggi/pyramid/ROADMAP.md
+2026-07-04 14:43 [Edit] /home/steggi/matrix/server.py
+2026-07-04 14:44 [Edit] /home/steggi/matrix/server.py
+2026-07-04 14:44 [Edit] /home/steggi/matrix/server.py
+2026-07-04 14:44 [Edit] /home/steggi/matrix/server.py
+2026-07-04 14:44 [Edit] /home/steggi/matrix/server.py
+2026-07-04 14:45 [Edit] /home/steggi/pyramid/PROGRESS.md
+2026-07-04 14:46 [Edit] /home/steggi/pyramid/PROGRESS.md
+2026-07-04 14:46 [Edit] /home/steggi/pyramid/ROADMAP.md

PROGRESS.md

@@ -13,6 +13,59 @@ Schritt (und beim Abbruch mitten im Schritt den Zwischenstand). Format:
 
 ---
 
+## 2026-07-04 – Fable-5-Review (r): ADMIN_LOCK-Umbau (q) geprüft – korrekt, aber 3 kleinere Restbefunde gefunden und gefixt
+
+**Erledigt:** Review-Pass über die jüngste erledigte Arbeit – den ADMIN_LOCK-Umbau
+aus (q) (Commit c8ff850) kritisch gegengelesen und als Punkt (r) im
+Fable-5-Review-Abschnitt abgehakt. Der Umbau selbst ist korrekt (Locking-Struktur,
+Routen-Reihenfolge, Handler-Logik unverändert). Drei kleinere Restbefunde in
+`server.py` gefunden und gefixt:
+
+1. **GET-Leser-Race (gleiche Familie wie (q), dort übersehen):** (q) hatte nur
+   POST-gegen-POST betrachtet – `GET /api/registration-status` liest
+   `conduit.toml` aber OHNE Lock, während `toggle-registration`/`create-invite`
+   die Datei per Truncate+Write in-place neu schreiben → seit multithreaded (p)
+   kann der Leser eine halb geschriebene/leere Datei sehen (Dashboard zeigt
+   transient falschen Registrierungs-Status). Fix: Lesen unter `ADMIN_LOCK`.
+   **Wichtige Erkenntnis dabei:** Der naheliegendere Fix (atomares `os.replace`
+   statt In-Place-Write) wäre FALSCH – `conduit.toml` ist als **einzelne Datei**
+   in den Container gemountet (`./conduit.toml:/etc/conduit.toml:ro`), ein
+   Replace tauscht den Inode und der Container sähe für immer die alte Datei.
+   Im Code als Kommentar festgehalten, damit das nie jemand „verbessert".
+2. **Negative `Content-Length`:** `livekit-token`, `e2ee-diagnostics` und
+   `ban`/`unban` prüften nur `length > MAX` – ein negativer Wert wäre
+   durchgerutscht und `rfile.read(-n)` liest bis EOF (Thread hängt bis zum
+   30-s-Timeout). Fix: `length < 0` wird wie „zu groß" abgelehnt (413).
+3. **`ban`/`unban` ohne Body-Limit + `random` für Einladungs-Token:**
+   ban/unban lasen beliebig große Bodies (und halten dabei seit (q) den
+   ADMIN_LOCK) → 4-KB-Deckel wie bei livekit-token. Einladungs-Token
+   (`create-invite`) kamen aus dem vorhersagbaren `random`-PRNG, obwohl sie
+   die Registrierung am Homeserver freischalten → `secrets.choice`.
+
+**Verifiziert (headless, nach Dienst-Neustart per kill/`Restart=always`):**
+`py_compile` sauber, Diff gegen Backup minimal wie geplant
+(`server.py.bak-20260704-review-r`). 8 Routen-Checks: stats.html 200,
+registration-status 200 (liest jetzt unter Lock), ban ohne Nutzer localhost →
+400 „Kein Nutzer angegeben", **ban mit 8-KB-Body → 413 (neu)**, **livekit-token
+mit `Content-Length: -5` → sofort 413 statt Hänger (neu)**, livekit-token
+ungültiger Matrix-Token → 401, e2ee-diagnostics falscher Token → 403,
+Admin-Route öffentlich über die Dashboard-Domain → 403. Test-Deny-Zeile aus
+`security_alerts.log` wieder entfernt.
+
+**Offen/Nächster Schritt:** Unverändert – alle offenen ROADMAP-Punkte brauchen
+PC/Gerät (SQLCipher, LiveKit-Client-Umstellung, Härtetest) oder Bernd
+(Secret-Rotation, Dashboard-Fernzugriff A/B, Call-Pilot-Go).
+
+**Stolperfallen:** (1) Bei Nebenläufigkeits-Reviews nicht nur Schreiber gegen
+Schreiber prüfen, sondern auch die LESER derselben Ressource – `do_GET` war in
+(q) komplett außen vor. (2) „Atomares Schreiben per `os.replace`" ist bei
+Docker-**Datei**-Bind-Mounts eine Falle: der Mount hängt am Inode, nach einem
+Replace sieht der Container die alte Datei weiter. In-Place-Write ist dort
+absichtlich richtig. (3) `Content-Length` kann negativ sein – wer nur nach oben
+deckelt, lässt `read()` bis EOF blockieren.
+
+---
+
 ## 2026-07-04 – Fable-5-Review (q): Threading-Härtung (p) tiefer geprüft – echte Nebenläufigkeits-Race in den Admin-Routen gefunden und mit ADMIN_LOCK behoben
 
 **Erledigt:** Review-Pass über die jüngste erledigte Arbeit – die Threading-Umstellung
@@ -745,6 +798,21 @@ schließt den Review ab.
   vorher; Race-Beweis (2 parallele `run-stats` 2633 ms ≈ 2× einzeln 1388 ms →
   serialisiert); Non-Blocking-Beweis (öffentliches `livekit-token` während gehaltenem
   Lock in 11 ms). Details im PROGRESS-Eintrag „(q)" oben.
+- [x] **(r) ADMIN_LOCK-Umbau aus (q) (c8ff850) geprüft – Umbau korrekt, 3 kleinere
+  Restbefunde gefixt (2026-07-04).** Locking-Struktur und Routen-Reihenfolge von (q)
+  bestätigt (öffentliche Routen vor dem Gate/Lock, Handler-Logik unverändert).
+  Befunde: (1) `GET /api/registration-status` las `conduit.toml` ohne Lock, während
+  die Admin-Routen sie per Truncate+Write neu schreiben – Leser konnte seit
+  multithreaded (p) eine halb geschriebene Datei sehen; Fix: Lesen unter `ADMIN_LOCK`
+  (bewusst KEIN `os.replace`: die Datei ist als Einzeldatei in den Container
+  gemountet, Inode-Falle – im Code dokumentiert). (2) Negative `Content-Length`
+  ließ `read()` bis EOF blockieren (Thread hing bis zum 30-s-Timeout) – bei
+  `livekit-token`/`e2ee-diagnostics`/`ban`/`unban` wird `< 0` jetzt mit 413
+  abgelehnt. (3) `ban`/`unban` ohne Body-Limit (hielten dabei den ADMIN_LOCK) →
+  4-KB-Deckel; Einladungs-Token von `random` auf `secrets` umgestellt (Token
+  schaltet Registrierung frei). Headless verifiziert (8 Checks inkl. 8-KB-Body →
+  413, `Content-Length: -5` → sofort 413 statt Hänger). Details im
+  PROGRESS-Eintrag „(r)" oben.
 
 **Fazit:** Die Sonnet-5-Arbeit war handwerklich sauber; die zwei echten Lücken
 (verlorene Kommentare, fehlender Soft-Logout-Guard) hatte die abgebrochene

ROADMAP.md

@@ -79,6 +79,10 @@ Punkte abhaken (`[x]`), wenn erledigt UND in `PROGRESS.md` protokolliert.
       `send_admin`-Readback) ist mit `ADMIN_LOCK` geschlossen, das NUR die
       Admin-Mutationen serialisiert – die öffentlichen Routen bleiben nebenläufig;
       headless verifiziert (2× parallel = 2× Laufzeit, öffentlicher Pfad 11 ms).
+      Zusatz (Fable-5-Review (r), 2026-07-04): drei Restbefunde gefixt –
+      `registration-status`-Leser jetzt unter ADMIN_LOCK (Truncate-Write-Race),
+      negative/übergroße `Content-Length` überall abgelehnt (kein Thread-Hänger,
+      ban/unban 4-KB-Deckel), Einladungs-Token per `secrets` statt `random`.
 - [x] **LiveKit-Token-Route server-seitig gebaut + verifiziert** (Fable-5-Review (n),
       PROGRESS.md 2026-07-04). `POST /api/livekit-token` läuft live in `server.py`
       (whoami-Auth, stdlib-HS256-Minting aus `livekit.yaml`), headless geprüft