Files
ssh-jumphost/FORTSETZUNG_Teil_C.md
2026-09-02 20:30:44 +02:00

313 lines
18 KiB
Markdown

# Fortsetzungsdokument: Umsetzungsauftrag_Sonnet5.md — Teil C (Mandanten-Rueckbau)
Stand: 2026-09-02. Teil C ist **vollstaendig abgeschlossen** (inkl. echtem
`pytest`-Lauf, siehe Abschnitt 3). **Update 2026-09-02:** alle 7 in
Abschnitt 3 dokumentierten vorbestehenden Bugs sind jetzt behoben, voller
Regressionslauf 164/164 gruen -- der zuvor zurueckgestellte Livetest kann
jetzt als echter Abnahmetest stattfinden (siehe Abschnitt 5).
Repo: `ssh_jumphost` (FastAPI/SQLite SSH/RDP-Jumphost), erreichbar ueber die
device_bash-Bridge unter `$HOME/mnt/ssh_jumphost`.
**Reihenfolge des Gesamtauftrags** (verbindlich, siehe
`Umsetzungsauftrag_Sonnet5.md`): Teil A/B -> Teil E -> Teil C (abgeschlossen,
dieses Dokument) -> **Teil D (naechster Schritt)** -> Teil F, plus eine vom
Nutzer nachtraeglich hinzugefuegte Aufgabe #11 (SSH Copy-Paste +
Terminal-Farben).
**Erledigt: Teil A, Teil B, Teil E, Teil C** -- Teil A/B/E vollstaendig
abgeschlossen in frueheren Sessions, hier nicht mehr relevant. Teil C siehe
unten.
---
## 0) Wichtige Umgebungs-Erkenntnis fuer die naechste Session
Alle Sessions vor dieser hier haben ausschliesslich in einer Cloud-Sandbox
ohne Netzwerk/pip gearbeitet und konnten deshalb **nie** echten `pytest`
laufen lassen (nur `py_compile` + selbstgebaute Undefined-Name-/Tag-Balance-
Checker). Diese Session laeuft stattdessen ueber die `device_bash`-Bridge
**direkt auf dem Nutzer-Rechner** -- dort **funktioniert Netzwerk/pip**.
Das aendert die Verifikationsmoeglichkeiten grundlegend:
- Eine funktionierende venv liegt bereits unter `.venv/` im Projektordner
(Python 3.10, pip 22.0.2). Alle `requirements.txt`+`requirements-dev.txt`-
Pakete sind installiert und mit `.venv/bin/python3 -m pytest` lauffaehig.
**`.venv/bin/pip` laesst sich NICHT upgraden/loeschen** (device_bash darf
nicht loeschen, siehe Abschnitt 4 unten) -- das ist unschaedlich, die
vorhandene pip-Version reicht zum Installieren.
- Vor jedem `pytest`-Lauf muessen diese Env-Vars gesetzt sein (sonst schlaegt
JEDER Test mit `PermissionError: /var/lib/jumphost` fehl, weil
`app/config.py::Settings.data_dir` beim ERSTEN Modul-Import aus der Umgebung
gelesen wird und ohne `JUMPHOST_DATA_DIR` auf den Produktionspfad
`/var/lib/jumphost` faellt):
```bash
export JUMPHOST_DATA_DIR=/tmp/jumphost-test
export JUMPHOST_ENV=development
export JUMPHOST_DEV_KEK=$(python3 -c "import secrets;print(secrets.token_hex(32))")
export JUMPHOST_DEV_SESSION_SECRET=$(python3 -c "import secrets;print(secrets.token_hex(32))")
.venv/bin/python3 -m pytest -q
```
- `device_bash`-Aufrufe laufen **jeweils in einer frischen, isolierten
Sandbox** (bwrap, eigener Network-Namespace) -- Hintergrundprozesse per
`nohup ... & disown` ueberleben das Ende des Aufrufs NICHT (anders als ein
normaler Shell-Hintergrundprozess auf einer echten Maschine). Lang laufende
Kommandos (z.B. `pip install`) muessen daher als EIN Aufruf mit `timeout`
unterhalb des 180s-Caps laufen; bei Abbruch einfach denselben Befehl erneut
ausfuehren (pip ist idempotent, laedt bereits installierte Pakete nicht neu
herunter).
- `rm -rf` auf Dateien, die device_bash selbst angelegt hat (z.B. eine kaputte
`.venv`), schlaegt mit `Operation not permitted` fehl (siehe Abschnitt 4) --
nicht wundern, das ist die bekannte Loesch-Restriktion, keine defekte venv.
---
## 1) Betreiberentscheidungen (Phase 0 von Teil C) -- weiterhin verbindlich
Diese drei Entscheidungen wurden vom Nutzer per Rueckfrage getroffen und
**duerfen von keiner Folge-Session stillschweigend geaendert werden**:
1. **`tenant_admins`-Behandlung** -> *"Zu is_admin=1 befoerdern"* (jeder
bestehende `tenant_admins`-Eintrag wird zu vollem `is_admin=1` befoerdert,
NICHT demotet). Umgesetzt durch `scripts/promote_tenant_admins.py`
(Phase 1, muss vor erstem Start mit neuem Code auf der echten
Produktions-DB laufen, siehe Abschnitt 4 im alten Dokumentstand /
weiterhin offen).
2. **`users.home_tenant_id`** -> *"Vollstaendiger Rebuild (Recommended)"* (die
Spalte wurde beim Tabellen-Rebuild vollstaendig entfernt). Umgesetzt in
`0014_drop_tenants.sql`.
3. **Bestehende API-Tokens** -> *"Bestehende Tokens weiterlaufen lassen"* --
**weicht bewusst von der Empfehlung des Umsetzungsauftrags selbst ab**.
Es fand KEIN Massenwiderruf statt. **Diese Abweichung ist Absicht und darf
nicht "korrigiert" werden.**
---
## 2) Teil C: vollstaendiger Abschluss-Stand
### Phase 1 (Operator-Skript) -- weiterhin offen fuer den Betreiber
`scripts/promote_tenant_admins.py` ist gebaut und verifiziert (`py_compile`),
aber **noch nie gegen eine echte Produktions-DB gelaufen** -- es gibt in
keiner bisherigen Session-Umgebung eine solche DB. **Muss vom Betreiber vor
dem ersten Produktions-Deployment dieses Codestands einmalig ausgefuehrt
werden, idealerweise zuerst mit `--dry-run`.**
### Phase 2 (Code-Aenderungen) -- vollstaendig, in frueherer Session erledigt
Siehe Git-Historie / vorherige Version dieses Dokuments: `app/tenancy.py`
entfernt (nach `_to_delete/` verschoben), `app/auth/deps.py`,
`app/auth/routes.py`, `app/models/schemas.py`, `app/admin/routes.py`,
`app/admin/log_ws.py`, `templates/admin.html`, `static/js/admin.js`,
`app/db/migrations/0014_drop_tenants.sql`, `app/db.py::_apply_migrations()`
(FK-Check nach jeder Migration) -- alles bereits fertiggestellt und jetzt
zusaetzlich durch den echten `pytest`-Lauf bestaetigt (siehe Abschnitt 3).
### Phase 3 (Tests + README) -- in DIESER Session abgeschlossen
- `tests/test_phase9.py`: `test_sessions_list_requires_global_admin_not_tenant_admin`
zu `test_sessions_list_requires_admin` vereinfacht (normaler User statt
`tenant_admins`-Konstrukt), `"tenant_id": 1` aus dem rdp-credentials-Payload
entfernt.
- `tests/test_phase12.py`: alle 7 `tenant_id`-Spalten/-Werte aus den
`host_groups`/`ssh_keys`-INSERTs entfernt (auch in den `_pre_0010_db`-
Migrationstests -- die Spalte ist dort nullable, das Entfernen aendert
nichts am getesteten Verhalten von Migration 0010).
- `tests/test_phase13.py`: `"tenant_id": 1` aus rdp-credentials-Payload
entfernt, `tenant_id`/`tenant_name` aus der erwarteten SSH-Key-Spaltenliste
entfernt, `test_session_recording_entries_requires_global_admin` von einem
Mandanten-Admin-Aufbau (`POST /admin/tenants` -- existiert nicht mehr) auf
einen normalen Nicht-Admin-User umgebaut.
- `README.md`: alle im vorherigen Dokumentstand gelisteten Fundstellen
bereinigt -- Phase-8-Abschnitt zu einer kurzen historischen Rueckschau
umformuliert (inkl. Neu-Nummerierung der verbleibenden 6 Unterpunkte),
Migrations-Cross-Referenzen von `0006_tenants.sql` auf
`0014_drop_tenants.sql` umgestellt, Testzahlen/-beschreibungen an
`tests/test_admin_crud.py` (Nachfolger von `tests/test_tenants.py`)
angepasst, Hinweis auf dauerhaft erhaltene `tenant_*`-Audit-Log-Eintraege
ergaenzt, "Mandantenfaehigkeit" aus der offenen-Punkte-Aufzaehlung
gestrichen.
- `pytest.ini`: `norecursedirs = _to_delete .venv .git` ergaenzt (sonst
sammelt pytest die nach `_to_delete/` verschobene `test_tenants.py` weiter
ein und importiert das ebenfalls verschobene `app/tenancy.py` -> Import-
Error bei jedem Lauf).
Alle Aenderungen mit `py_compile` verifiziert; die Test-Datei-Aenderungen
zusaetzlich durch den echten Suite-Lauf (Abschnitt 3).
---
## 3) Echter `pytest`-Lauf: Ergebnis und Einordnung (AKTUALISIERT -- alle 7 Bugs behoben)
**Update (Folgesession, auf ausdruecklichen Nutzerwunsch "livetest spaeter
machen zuerst den rest machen das das dann wirkliche abnahmetests sind"):**
Alle 7 nachfolgend dokumentierten Root-Cause-Bugs hinter den urspruenglich
18 Fehlschlaegen sind jetzt behoben und einzeln per echtem `pytest`
verifiziert. Der abschliessende volle Regressionslauf
(`.venv/bin/python3 -m pytest -q`, frisches `JUMPHOST_DATA_DIR` +
Dev-Secrets wie in Abschnitt 0) liefert: **164 passed, 0 failed** (6
harmlose, vorbestehende `DeprecationWarning`s in `app/admin/routes.py:562`
-- ausserhalb des Scopes dieser 7 Bugs, siehe Fussnote unten). Der urspruengliche
Lauf hatte 132 Tests gezaehlt (124 passed/18 failed); die Gesamtzahl ist auf
164 gestiegen, weil zwischenzeitlich in Teil F/Aufgabe #11 neue Testdateien
hinzugekommen sind (`test_teil_f_schritt5.py` u.a.), die alle bereits gruen
waren.
Damit ist die Grundlage fuer eine **echte Abnahmetest-Session** (Live-Browser/
SSH/RDP-Test) gelegt: der zuvor bekannte Bug-Rucksack existiert nicht mehr,
der Live-Test in einer kuenftigen Session prueft ein tatsaechlich
vollstaendig korrigiertes System und nicht eines mit bekannten offenen
Defekten.
Die folgenden 7 Eintraege sind nur noch als **Nachweis-Dokumentation**
(Root-Cause + Fix) erhalten, nicht mehr als offene Punkte:
1. **`tests/test_phase15.py`** (7 Tests, urspruenglich) -- Signatur-Drift
zwischen Testcode und Implementierung: `Request()` ohne `scope`,
`active_sessions.register()` ohne `user_id`, `fake_connect_to_host()`
ohne `user_id`-Keyword, `UploadFile("name", bytes)` mit veralteter
Zwei-Positional-Argumente-Signatur (echte Starlette-Signatur inzwischen
`UploadFile(file, *, filename=...)`), `start_sftp_client()`-Fakes ohne
passenden `async`/`await`-Vertrag und fehlendem `exit()`, sowie
`asyncssh.Error(reason=...)` ohne das inzwischen pflicht-positionale
`code`-Argument. **Fix:** ausschliesslich in `tests/test_phase15.py` --
alle Fake-Objekte/Aufrufe an die echten, aktuellen Signaturen angepasst
(kein Produktivcode veraendert, reiner Test-Drift). Verifiziert: 11/11
gruen.
2. **`tests/test_phase12.py`** (4 Tests) + **`tests/test_phase10.py`**
(1 Test) -- `build_rdp_params()` (`app/rdp_proxy/guacd_client.py:413`)
verlangt inzwischen `session_id` als Pflicht-Keyword-Argument, die Tests
riefen ohne dieses Argument auf. **Fix:** in beiden Testdateien an allen
betroffenen Aufrufstellen `session_id=1` (Testwert, fachlich irrelevant
fuer die geprueften Assertions) ergaenzt. Verifiziert: 26/26 gruen.
3. **`tests/test_session_reaper.py`** (2 Tests) -- echter, reproduzierbarer
Bug: `app/security/session_reaper.py::reap_orphaned_sessions()` fuehrte
ein `UPDATE sessions ...` aus (oeffnet bei Test-Verbindungen ohne
`isolation_level=None` implizit eine Transaktion), rief danach
`write_audit_event()` auf, das seinerseits `BEGIN IMMEDIATE` versucht --
doppeltes BEGIN, `sqlite3.OperationalError`. **Fix:** in
`app/security/session_reaper.py` ein explizites `await conn.commit()`
direkt nach dem `UPDATE` ergaenzt (schliesst die implizite Transaktion,
bevor `write_audit_event()` seine eigene oeffnet -- funktioniert sowohl
mit Produktions-Autocommit-Verbindungen als auch mit dem
Test-`:memory:`-Verbindungsstil). Verifiziert: 3/3 gruen.
4. **`tests/test_phase13.py::test_session_recording_entries_ssh_playback_and_audit`**
-- `SessionRecorder` (`app/recordings/recorder.py`) hat nur noch
`async def aclose()`, der Test rief synchron `.close()` (existiert
nicht). **Fix:** Testaufruf auf `await recorder.aclose()` umgestellt
(reiner Test-Drift, kein Produktivcode veraendert). Verifiziert: gruen.
5. **`tests/test_phase13.py::test_ssh_password_credentials_set_and_delete`**
-- Test-Bug, kein echtes Leck: `assert "password" not in <json>` schlug
fehl, weil bereits die Response-FELDNAMEN
(`ssh_password_credentials_set`/`_username`) die Zeichenkette "password"
enthalten -- Substring-Treffer auf den Feldnamen, nicht auf einen
Passwort-Klartextwert. **Fix:** Assertion auf den tatsaechlich
sicherheitsrelevanten Fall praezisiert: geprueft wird jetzt, dass der
konkrete im Test gesetzte Passwort-WERT
(`"sehr-geheimes-passwort"`) nicht im JSON auftaucht. Verifiziert:
11/11 gruen (ganze Datei).
6. **`tests/test_pentest_security.py::test_password_change_invalidates_old_session_everywhere`**
-- **sicherheitsrelevant, per empirischer Debug-Sitzung (temporaeres,
danach geloeschtes Debug-Testskript) untersucht.** Ergebnis: die
Sicherheitslogik selbst (`session_version`-Vergleich in
`app/auth/deps.py::get_current_user`) war die ganze Zeit KORREKT -- der
alte Cookie wurde bei isolierter Pruefung zuverlaessig mit 401
abgelehnt. Der Fehlschlag war ein reines Test-Harness-Artefakt: die
`get_current_user()`-Dependency schreibt bei jeder Anfrage bereits ein
eigenes Refresh-Set-Cookie (gleitender Idle-Timeout), und
`change_password()`/`logout()`/`logout_everywhere()` haengten danach
noch ein ZWEITES, eigenes Set-Cookie an dieselbe Antwort -- zwei
`Set-Cookie: jh_session=...`-Header in einer Antwort. Der
*geteilte* httpx-Test-Client verwechselte beim naeherungsweisen
Ueberschreiben seines Cookie-Jars (`client.cookies.set(...)` auf einem
Jar, das durch die doppelten Set-Cookie-Header bereits einen
widerspruechlichen Zustand hatte) den alten mit dem neuen Cookie-Wert --
ein von httpx selbst dokumentiertes Ambiguitaetsproblem beim
Ueberschreiben/per-Request-Setzen von Cookies in einem bereits
befuellten Jar. **Fix (zwei Teile):** (a) Anwendungsseitig in
`app/auth/routes.py` -- in `change_password()`, `logout()` und
`logout_everywhere()` wird das von der Dependency bereits gesetzte
Set-Cookie explizit entfernt (`del response.headers["set-cookie"]`),
bevor die jeweilige Routenfunktion ihr eigenes, autoritatives
Set-Cookie schreibt -- Verteidigung in der Tiefe, sauberere
HTTP-Antworten unabhaengig vom Test. (b) Testseitig in
`tests/test_pentest_security.py` -- der alte Cookie wird jetzt gegen
einen komplett FRISCHEN `httpx.AsyncClient` (leeres Cookie-Jar) geprueft
statt gegen den geteilten, bereits "verschmutzten" Test-Client.
Verifiziert: 17/17 gruen, keine httpx-Warnungen mehr.
7. **`tests/test_admin_crud.py::test_user_update_deactivate_and_delete_semantics`**
-- echter Bug: `app/admin/routes.py::delete_user()` pruefte
`SELECT 1 FROM audit_log WHERE user_id = ? LIMIT 1` mit der ID des
ZIEL-Nutzers, aber `user_updated`/`user_deactivated`-Events werden mit
`user_id=<handelnder Admin>` geschrieben -- der betroffene Nutzer steht
stattdessen unter `details_json.target_user_id` (dieselbe Konvention wie
bei `user_added_to_group`/`removed_from_group`, `api_token_issued`,
`session_terminated_by_admin`, per Grep bestaetigt). Der
Audit-Historie-Check fand deshalb nie etwas fuer den Zielnutzer und
loeschte hart, obwohl ein anonymisierendes Soft-Delete erwartet war --
**eine echte, sicherheitsrelevante Datenverlust-Regression** (Accounts
mit Aenderungshistorie waeren faelschlich hart geloescht statt
anonymisiert worden). **Fix:** die `SELECT`-Bedingung in
`app/admin/routes.py::delete_user()` um
`OR json_extract(details_json, '$.target_user_id') = ?` erweitert
(SQLite-JSON1-Verfuegbarkeit vorab eigenstaendig verifiziert). Verifiziert:
9/9 gruen (ganze Datei).
**Fussnote (nicht Teil der 7 Bugs, nicht behoben):** der volle
Regressionslauf zeigt 6 `DeprecationWarning`s in `app/admin/routes.py:562`
(`payload.ssh_username, payload.rdp_username, payload.rdp_domain` --
vermutlich ein veraltetes Pydantic-Zugriffsmuster). Harmlos (keine
Testfehlschlaege), ausserhalb des vom Nutzer beauftragten Scopes dieser
Session, aber als Hinweis fuer eine kuenftige Aufraeum-Session vermerkt.
## 4) Wichtiger Hinweis: zwei Dateien liegen weiterhin in `_to_delete/`
Unveraendert gegenueber dem vorherigen Dokumentstand: `device_bash` darf
standardmaessig keine Dateien loeschen. In `_to_delete/` liegen
`tenancy.py` (ehemals `app/tenancy.py`) und `test_tenants.py` (ehemals
`tests/test_tenants.py`), technisch noch vorhanden, aber aus dem aktiven
Code-/Test-Pfad entfernt (pytest sammelt den Ordner seit dem
`pytest.ini`-Update in dieser Session auch nicht mehr ein). Dazu aeltere,
nicht zusammenhaengende Reste (`appsrc.tgz`, `jh.tgz`,
`tmp_stage_1788191215`). **Der Nutzer kann diesen Ordner jederzeit selbst
loeschen**, oder eine kuenftige Session kann `device_request_delete_permission`
versuchen.
---
## 5) Empfohlene naechste Schritte
1. **Livetest/Abnahmetest** (Browser/SSH/RDP, echtes Deployment) fuer
Teil F.6 und Aufgabe #11 -- war vom Nutzer ausdruecklich zurueckgestellt
("livetest spaeter machen zuerst den rest machen das das dann wirkliche
abnahmetests sind"), bis die in Abschnitt 3 dokumentierten 7 Bugs
behoben sind. **Das ist jetzt der Fall** (alle 7 behoben, voller
Regressionslauf 164/164 gruen) -- der Livetest kann in der naechsten
Session als echter, aussagekraeftiger Abnahmetest durchgefuehrt werden.
2. **Teil D -- Berechtigungen ausschliesslich ueber Benutzergruppen**
(naechster Abschnitt in `Umsetzungsauftrag_Sonnet5.md`, direkt nach
Teil C) -- noch nicht gelesen/begonnen.
3. Weiterhin offen, unabhaengig von der Code-Reihenfolge: Phase-1-Skript
(`scripts/promote_tenant_admins.py`) auf der echten Produktions-DB
ausfuehren, sobald ein Deployment-Fenster dafuer ansteht.
4. Kleinere, nicht dringende Aufraeumarbeit (siehe Fussnote Abschnitt 3):
die 6 `DeprecationWarning`s in `app/admin/routes.py:562` bei
Gelegenheit beheben.
---
## 6) Technische Hinweise fuer die naechste Session
- Repo ueber `mcp__remote-devices__device_bash` erreichbar, gemountet unter
`$HOME/mnt/ssh_jumphost`. Diese Bridge laeuft direkt auf dem Nutzer-Rechner
(nicht die evtl. veraltete Cloud-Container-Kopie unter
`/mnt/user-data/uploads/ssh_jumphost/`, die nur zum schnellen Lesen taugt).
- Siehe Abschnitt 0 fuer venv/Env-Var-Setup und die Eigenheiten der
device_bash-Sandbox (frische Namespaces pro Aufruf, 180s-Cap,
Loesch-Restriktion).
- Alle Datei-Edits weiterhin ueber `device_bash`-Python-Skripte mit
`content.count(old) == 1`-Assert-dann-Replace-Muster durchfuehren (nie
blindes `sed`), danach IMMER per `grep` gegenpruefen, dass alles
persistiert wurde.
- Fuer SQL-Migrationsaenderungen weiterhin einen unabhaengigen
`sqlite3`-Test-Harness bauen (wie in einer frueheren Session unter
`/tmp/migtest/run_test.py` im Cloud-Container) -- jetzt aber alternativ
auch direkt mit echtem `pytest` gegen die volle FastAPI-App verifizierbar,
siehe Abschnitt 0/3.