security: server.py Restbefunde aus Review (s) – ban/unban-Body vor ADMIN_LOCK, generische 500er auf oeffentlichen Routen

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Bernd Steckmeister
2026-07-04 18:56:42 +02:00
parent 4d4044b888
commit 13b333cbec
2 changed files with 68 additions and 0 deletions
+64
View File
@@ -13,6 +13,55 @@ Schritt (und beim Abbruch mitten im Schritt den Zwischenstand). Format:
---
## 2026-07-04 Fable-5-Review (s): (r)-Fixes geprüft korrekt, 2 kleinere Restbefunde gefunden und gefixt
**Erledigt:** Review-Pass über die jüngste erledigte Arbeit die drei
(r)-Restbefund-Fixes (Commit 5ada0e5) kritisch gegengelesen und als Punkt (s)
im Fable-5-Review-Abschnitt abgehakt. Der (r)-Diff selbst ist korrekt (gegen
Backup `server.py.bak-20260704-review-r` verifiziert, Dienst lief mit dem
aktuellen Code). Beim Gegenlesen der GANZEN Datei (Lehre aus (o)) zwei
kleinere Restbefunde in `server.py` gefunden und gefixt:
1. **ban/unban las den Body INNERHALB des `ADMIN_LOCK`:** Das 4-KB-Limit aus
(r) deckelt nur die Größe ein LAN-Client, der `Content-Length` ankündigt
und nichts schickt, hielt den Lock trotzdem bis zum 30-s-Socket-Timeout
fest (der (r)-Kommentar behauptete fälschlich, der Deckel verhindere das).
Seit (r) wartet auch der öffentliche GET-Leser von `registration-status`
auf diesen Lock → der hing mit. Fix: Body-Lesen/Validieren VOR den Lock
gezogen (Handler-Logik identisch, nur verschoben).
2. **Öffentliche Routen leakten Exception-Texte:** `livekit-token` und
`e2ee-diagnostics` antworteten Unauthentifizierten bei Fehlern mit
`str(e)` das kann interne Pfade (`livekit.yaml`-Pfad bei
FileNotFoundError) oder interne Adressen preisgeben. Fix: generische
500-Antwort („Interner Fehler"), Details per stderr ins Journal. Die
Admin-Routen behalten `str(e)` (LAN-only, für Bernd nützlich).
**Verifiziert (headless, nach Dienst-Neustart per kill/`Restart=always`):**
`py_compile` sauber, Diff gegen Backup minimal wie geplant
(`server.py.bak-20260704-review-s`). 8 Routen-Checks wie in (r) alle
unverändert (stats 200, registration-status 200, ban ohne Nutzer 400,
8-KB-Body 413, `Content-Length: -5` 413, livekit-token ungültig 401,
Diagnose falscher Token 403, gespoofter Cf-Header 403). **Lock-Beweis (neu):
ban-Request mit angekündigtem, nie gesendetem Body + paralleler
`registration-status`-GET → Antwort in 15 ms statt Lock-Blockade** (vorher
hätte der GET bis zu 30 s gewartet). **500-Beweis (neu):** ungültiges JSON an
livekit-token → `{"error": "Interner Fehler"}`, Detail
(`JSONDecodeError(...)`) steht im Journal. 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) Ein Größen-Limit ist KEIN Zeit-Limit: `read(n)` mit
kleinem n blockiert trotzdem bis zum Socket-Timeout, wenn der Client nichts
schickt blockierende I/O gehört grundsätzlich VOR einen Lock, nicht
hinein. (2) `str(e)` in Fehlerantworten öffentlicher Endpoints ist ein
Info-Leak (Pfade, interne Hosts) Details gehören ins Journal, nach draußen
nur Generisches.
---
## 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
@@ -813,6 +862,21 @@ schließt den Review ab.
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.
- [x] **(s) (r)-Fixes (5ada0e5) geprüft korrekt, 2 kleinere Restbefunde gefixt
(2026-07-04).** (r)-Diff gegen Backup verifiziert (GET-Leser-Lock,
Content-Length-Härtung, secrets-Token alles wie protokolliert), Dienst lief
mit aktuellem Code, kein unprotokollierter Rest im CHANGES-Hook-Log. Befunde
beim Ganzdatei-Gegenlesen: (1) ban/unban las den Body innerhalb des
`ADMIN_LOCK` ein Client mit angekündigtem, nie gesendetem Body hielt den
Lock bis zum 30-s-Socket-Timeout (das 4-KB-Limit deckelt nur die Größe, nicht
die Wartezeit; seit (r) hing damit auch der `registration-status`-Leser mit);
Fix: Body-Lesen/Validieren vor den Lock gezogen, Handler-Logik unverändert.
(2) `livekit-token`/`e2ee-diagnostics` leakten bei Fehlern `str(e)` an
Unauthentifizierte (interne Pfade/Adressen); Fix: generisches
`{"error": "Interner Fehler"}`, Details ins Journal (stderr). Headless
verifiziert: 8 Routen-Checks unverändert, Lock-Beweis (hängender ban-Body,
paralleler registration-status-GET in 15 ms), 500-Beweis (ungültiges JSON →
generisch, Detail im Journal). Details im PROGRESS-Eintrag „(s)" oben.
**Fazit:** Die Sonnet-5-Arbeit war handwerklich sauber; die zwei echten Lücken
(verlorene Kommentare, fehlender Soft-Logout-Guard) hatte die abgebrochene