[P2] Testabdeckung 304 von 18.285 Zeilen (1,7 %) — kritische Pfade absichern vor Produktivbetrieb #230

Open
opened 2026-07-27 10:44:23 +00:00 by chinux · 1 comment
Owner

Befund

Zeilen
server/**/*.py (ohne Tests) 18.285
server/tests/*.py 304
Anteil ~1,7 %

Vorhandene Tests:

tests/test_smoke.py         test_smoke, test_python_version
tests/test_tenant_scope.py  test_command_logs_scope, test_audit_commands_scope,
                            test_update_history_scope, test_tunnel_history_scope_and_limit,
                            test_bulk_scan_bg_scope

Fünf davon prüfen Mandanten-Scope-Filter, zwei sind Smoke-Tests. Die Harness in tests/conftest.py ist gut gebaut — eigene Postgres-Datenbank theprox_test, echte ORM-Modelle, keine SQLite-Krücken. Das Fundament ist also da und wird nur kaum genutzt.

Ungetestet ist unter anderem

  • der komplette Agent-WS-Handshake (agent_ws.py, 918 Zeilen) — Authentifizierung, Enrollment, Nachrichtenverarbeitung
  • websocket/manager.py — Kommando-Lebenszyklus, Timeouts, Future-Aufräumen, Owner-Prüfung (#115)
  • scheduler.py — Job-Auslösung, Cron-Auswertung, Intervall-Gates
  • tunneling/manager.py (619 Zeilen) — Tunnel-Wiederherstellung, Aufräumen
  • services/docgen.py — Rendering, Backup-CSV-Merge
  • Rollen- und Scope-Durchsetzung an den ~180 Endpunkten (nur fünf Pfade abgedeckt)
  • Enrollment-Einmaligkeit, Token-Verbrennung, Rate-Limiting

Warum das jetzt relevant ist

Vor dem Anbinden von Produktiv-Nodes stehen mehrere invasive Änderungen an — Scheduler-Nebenläufigkeit, Pool-Dimensionierung, guest_view-Extraktion (#212), Autorisierung im Scheduler-Pfad. Ohne Tests ist nach jeder dieser Änderungen unklar, ob etwas kaputtgegangen ist. Insbesondere die Scope-Durchsetzung ist der Teil, dessen Bruch man erst merkt, wenn ein Mandant Daten eines anderen sieht.

Das ist keine Aufforderung, 18.000 Zeilen nachzutesten. Es geht um die Pfade, deren Fehlverhalten teuer ist.


Prompt für Claude Code

Baue die Testabdeckung in theProx an den kritischen Pfaden aus. Die Harness
existiert (server/tests/conftest.py, echte Postgres-Testdatenbank) — nutze sie,
baue keine zweite und führe kein Mock-Framework ein.

REIHENFOLGE NACH RISIKO, nicht nach Bequemlichkeit. Arbeite von oben nach
unten und liefere jede Gruppe als eigenen Commit.

--- GRUPPE 1: Authentifizierung und Enrollment ---
tests/test_agent_auth.py
  - Handshake mit gültiger Signatur -> verbunden
  - falsche Signatur -> auth_fail, keine Session
  - unbekannter pubkey -> auth_fail, gleiche Meldung wie bei falscher Signatur
    (kein Oracle, welcher Teil fehlschlug)
  - Nonce wird pro Verbindung neu erzeugt und nie wiederverwendet
  - Signatur einer ALTEN Nonce -> abgelehnt (Replay)
  - Rate-Limit: nach N Fehlversuchen wird die IP vor dem Challenge abgewiesen

tests/test_enrollment.py
  - Enrollment mit gültigem Token -> pubkey gesetzt, Token verbrannt
  - zweiter Versuch mit demselben Token -> 403
  - abgelaufener Token -> 403, gleiche Meldung
  - ungültiges pubkey-Format -> 400
  - nach Enrollment ist Node.token None

--- GRUPPE 2: Autorisierung über die Breite ---
tests/test_endpoint_authz.py
Kein Einzeltest je Endpunkt, sondern ein datengetriebener Test: alle Routen aus
app.routes einsammeln und prüfen, dass jede /api/-Route mindestens eine
Auth-Dependency in ihrer Signatur hat. Ausnahmenliste explizit im Test
(auth/login, auth/refresh, auth/logout, rdp/_internal_resolve, health).

Das ist der Test, der verhindert, dass ein künftig hinzugefügter Endpunkt die
Prüfung vergisst — mehr Wert als 50 Einzeltests.

Zusätzlich für die drei Rollenstufen je einen Vertreterendpunkt:
viewer darf lesen aber nicht schreiben; operator darf ausführen; nur
superadmin erreicht die Superadmin-Pfade.

--- GRUPPE 3: Mandanten-Isolation erweitern ---
tests/test_tenant_scope.py ergänzen (bestehendes Muster fortsetzen):
  - Node-Liste, VM-Liste, Logs, Backups, Tasks, Notifications
  - je Endpunkt: fremder Mandant -> nicht enthalten bzw. 403/404
  - TenantVMRange-Fall: geteilte Node, Guest aus fremdem VMID-Bereich ->
    nicht sichtbar (hängt an #212, dort ist der Zuordnungsfehler beschrieben)

--- GRUPPE 4: Kommando-Lebenszyklus ---
tests/test_command_manager.py (Agent gemockt, nur die Manager-Logik):
  - send_command: Antwort kommt -> Future aufgelöst, futures/cmd_owner leer
  - Timeout -> 504, futures/cmd_owner trotzdem leer (finally-Pfad)
  - Owner-Prüfung (#115): fremder Node schickt command.output -> verworfen
  - Agent trennt mitten im Kommando -> kein hängendes Future

--- GRUPPE 5: Scheduler ---
tests/test_scheduler.py (Jobs gemockt):
  - fälliger Job wird ausgeführt, next_run_at wird vorher gesetzt
  - Cron-Ausdruck bestimmt next_run korrekt
  - ungültiger Cron -> Fallback auf Intervall, kein Absturz
  - Job auf offline-Node -> Status error, kein Absturz
  - deaktivierter Job läuft nicht

--- LAUFBARKEIT ---
Am Ende MUSS gelten: ein einziger Befehl führt alles aus, so wie in
conftest.py dokumentiert:

    docker exec theprox-backend sh -c 'cd /app && python -m pytest tests/ -q'

Falls dafür etwas fehlt (pytest-asyncio, Fixtures), ergänzen und in
conftest.py dokumentieren. Ein Testbestand, der nicht mit einem Befehl läuft,
wird nicht ausgeführt.

Keine Coverage-Zielzahl. Die fünf Gruppen oben sind das Ziel.

Definition of Done

  • Alle fünf Gruppen vorhanden und grün
  • Der Routen-Test erkennt einen neu hinzugefügten Endpunkt ohne Auth-Dependency
  • Ein Befehl führt den gesamten Bestand aus
  • Keine zweite Test-Harness, kein Mock-Framework
## Befund | | Zeilen | |---|---| | `server/**/*.py` (ohne Tests) | **18.285** | | `server/tests/*.py` | **304** | | Anteil | **~1,7 %** | Vorhandene Tests: ``` tests/test_smoke.py test_smoke, test_python_version tests/test_tenant_scope.py test_command_logs_scope, test_audit_commands_scope, test_update_history_scope, test_tunnel_history_scope_and_limit, test_bulk_scan_bg_scope ``` Fünf davon prüfen Mandanten-Scope-Filter, zwei sind Smoke-Tests. Die Harness in `tests/conftest.py` ist gut gebaut — eigene Postgres-Datenbank `theprox_test`, echte ORM-Modelle, keine SQLite-Krücken. Das Fundament ist also da und wird nur kaum genutzt. ## Ungetestet ist unter anderem - der komplette Agent-WS-Handshake (`agent_ws.py`, 918 Zeilen) — Authentifizierung, Enrollment, Nachrichtenverarbeitung - `websocket/manager.py` — Kommando-Lebenszyklus, Timeouts, Future-Aufräumen, Owner-Prüfung (#115) - `scheduler.py` — Job-Auslösung, Cron-Auswertung, Intervall-Gates - `tunneling/manager.py` (619 Zeilen) — Tunnel-Wiederherstellung, Aufräumen - `services/docgen.py` — Rendering, Backup-CSV-Merge - Rollen- und Scope-Durchsetzung an den ~180 Endpunkten (nur fünf Pfade abgedeckt) - Enrollment-Einmaligkeit, Token-Verbrennung, Rate-Limiting ## Warum das jetzt relevant ist Vor dem Anbinden von Produktiv-Nodes stehen mehrere invasive Änderungen an — Scheduler-Nebenläufigkeit, Pool-Dimensionierung, `guest_view`-Extraktion (#212), Autorisierung im Scheduler-Pfad. Ohne Tests ist nach jeder dieser Änderungen unklar, ob etwas kaputtgegangen ist. Insbesondere die Scope-Durchsetzung ist der Teil, dessen Bruch man erst merkt, wenn ein Mandant Daten eines anderen sieht. Das ist keine Aufforderung, 18.000 Zeilen nachzutesten. Es geht um die Pfade, deren Fehlverhalten teuer ist. --- ## Prompt für Claude Code ``` Baue die Testabdeckung in theProx an den kritischen Pfaden aus. Die Harness existiert (server/tests/conftest.py, echte Postgres-Testdatenbank) — nutze sie, baue keine zweite und führe kein Mock-Framework ein. REIHENFOLGE NACH RISIKO, nicht nach Bequemlichkeit. Arbeite von oben nach unten und liefere jede Gruppe als eigenen Commit. --- GRUPPE 1: Authentifizierung und Enrollment --- tests/test_agent_auth.py - Handshake mit gültiger Signatur -> verbunden - falsche Signatur -> auth_fail, keine Session - unbekannter pubkey -> auth_fail, gleiche Meldung wie bei falscher Signatur (kein Oracle, welcher Teil fehlschlug) - Nonce wird pro Verbindung neu erzeugt und nie wiederverwendet - Signatur einer ALTEN Nonce -> abgelehnt (Replay) - Rate-Limit: nach N Fehlversuchen wird die IP vor dem Challenge abgewiesen tests/test_enrollment.py - Enrollment mit gültigem Token -> pubkey gesetzt, Token verbrannt - zweiter Versuch mit demselben Token -> 403 - abgelaufener Token -> 403, gleiche Meldung - ungültiges pubkey-Format -> 400 - nach Enrollment ist Node.token None --- GRUPPE 2: Autorisierung über die Breite --- tests/test_endpoint_authz.py Kein Einzeltest je Endpunkt, sondern ein datengetriebener Test: alle Routen aus app.routes einsammeln und prüfen, dass jede /api/-Route mindestens eine Auth-Dependency in ihrer Signatur hat. Ausnahmenliste explizit im Test (auth/login, auth/refresh, auth/logout, rdp/_internal_resolve, health). Das ist der Test, der verhindert, dass ein künftig hinzugefügter Endpunkt die Prüfung vergisst — mehr Wert als 50 Einzeltests. Zusätzlich für die drei Rollenstufen je einen Vertreterendpunkt: viewer darf lesen aber nicht schreiben; operator darf ausführen; nur superadmin erreicht die Superadmin-Pfade. --- GRUPPE 3: Mandanten-Isolation erweitern --- tests/test_tenant_scope.py ergänzen (bestehendes Muster fortsetzen): - Node-Liste, VM-Liste, Logs, Backups, Tasks, Notifications - je Endpunkt: fremder Mandant -> nicht enthalten bzw. 403/404 - TenantVMRange-Fall: geteilte Node, Guest aus fremdem VMID-Bereich -> nicht sichtbar (hängt an #212, dort ist der Zuordnungsfehler beschrieben) --- GRUPPE 4: Kommando-Lebenszyklus --- tests/test_command_manager.py (Agent gemockt, nur die Manager-Logik): - send_command: Antwort kommt -> Future aufgelöst, futures/cmd_owner leer - Timeout -> 504, futures/cmd_owner trotzdem leer (finally-Pfad) - Owner-Prüfung (#115): fremder Node schickt command.output -> verworfen - Agent trennt mitten im Kommando -> kein hängendes Future --- GRUPPE 5: Scheduler --- tests/test_scheduler.py (Jobs gemockt): - fälliger Job wird ausgeführt, next_run_at wird vorher gesetzt - Cron-Ausdruck bestimmt next_run korrekt - ungültiger Cron -> Fallback auf Intervall, kein Absturz - Job auf offline-Node -> Status error, kein Absturz - deaktivierter Job läuft nicht --- LAUFBARKEIT --- Am Ende MUSS gelten: ein einziger Befehl führt alles aus, so wie in conftest.py dokumentiert: docker exec theprox-backend sh -c 'cd /app && python -m pytest tests/ -q' Falls dafür etwas fehlt (pytest-asyncio, Fixtures), ergänzen und in conftest.py dokumentieren. Ein Testbestand, der nicht mit einem Befehl läuft, wird nicht ausgeführt. Keine Coverage-Zielzahl. Die fünf Gruppen oben sind das Ziel. ``` ## Definition of Done - [ ] Alle fünf Gruppen vorhanden und grün - [ ] Der Routen-Test erkennt einen neu hinzugefügten Endpunkt ohne Auth-Dependency - [ ] Ein Befehl führt den gesamten Bestand aus - [ ] Keine zweite Test-Harness, kein Mock-Framework
Author
Owner

Aus dem Code-Review vom 2026-07-27 (Commit 4152c4d). Gesamtblock: #226, #227, #228, #229, #230, #231.

Abarbeitung niedrig → hoch: #231#230#229#228#227#226.

Aus dem Code-Review vom 2026-07-27 (Commit 4152c4d). Gesamtblock: #226, #227, #228, #229, #230, #231. Abarbeitung niedrig → hoch: #231 → #230 → #229 → #228 → #227 → #226.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
chinux/theProx#230
No description provided.