# #036 Reviewbefunde aus der Durchsicht von Phase 2 **Status:** erledigt **Im Tracker:** – **Meilenstein:** v0.2.0 **Labels:** phase-2, backend, test, docs **Geschätzt:** 4 h **Tatsächlich:** 3 h 40 min ## Ziel Acht Meilensteine von Phase 2 (M15–M22) lagen ohne fremden Blick auf `main`. Ein Review-Agent, der den Code nicht geschrieben hat, hat sie gegen die Akzeptanzkriterien und gegen `CLAUDE.md` geprüft — mit eigener Datenbank, Gegenproben und einem vollen Suitenlauf. **Ergebnis:** Die Kernregeln halten. Fünf von fünf Gegenproben gegen Versionierung, Mandantengrenze, Abschlussregel und Selbstauskunft wurden rot. Alle Befunde lagen am Rand — in zwei Route Handlern, einem Export-Pfad, in Betriebsdokumentation und in Kommentaren. **Die drei wertvollsten Befunde waren Gegenproben, die grün blieben.** Das ist das wiederkehrende Muster dieses Projekts: nicht der falsche Code fällt auf, sondern der Code, der *aussieht*, als prüfte er etwas. ## Behoben ### Schwer - [x] **Der Link in der Mail „Verzeichnis-Abgleich fehlgeschlagen“ war ein 404.** Gebaut wurde `/verwaltung/verzeichnis` — `verwaltung` ist der Name des **Rechts**, die Seite liegt unter `/admin/verzeichnis`. Das ist die eine Meldung, die an **alle** Administratoren geht, und zwar im Moment eines Ausfalls: der Empfänger klickt und landet auf der Nicht-gefunden-Seite. Die Pfade stehen jetzt als `MELDUNGS_PFADE` an einer Stelle, und ein Test prüft **jeden gegen die Routen unter `src/app/`** — ein Pfad, den es nicht gibt, fällt beim Testlauf auf und nicht im Postfach. - [x] **Der Export eines Entwurfs mit Baustein lieferte einen leeren Abschnitt**, unter dem Editor-Platzhalter „wird ersetzt“, und der Import nahm das stillschweigend an. Ein Redakteur bringt eine noch unveröffentlichte Vorlage in eine zweite Installation — genau der Zweck des Formats —, und der geteilte Abschnitt kommt dort ohne einen einzigen Schritt an. Baustein- Abschnitte werden beim Export jetzt **aufgelöst**, wie beim Veröffentlichen; der Verweis wandert weiterhin nicht mit. ### Mittel - [x] **Der Route Handler, der Anhänge ausliefert, hatte keinen einzigen Test.** Die ganze Rechteprüfung ließ sich durch einen hartcodierten ADMIN ersetzen, ohne dass ein Test rot wurde. Das ist die **einzige** Stelle der Anwendung, die rohe Dateibytes ausliefert. Neu: `tests/anhang-route.test.ts` fährt den Handler echt, acht Prüfungen — darunter das Akzeptanzkriterium von #032 („nicht abrufbar **über seine Adresse**“), das bis hierher nur auf Dienstebene geprüft war. Die Gegenprobe ist jetzt dreifach rot. - [x] **`/api/scim` in der Proxy-Allowlist stand in keinem Test.** Die Zeile ließ sich entfernen, ohne dass etwas rot wurde — und ohne sie leitet der Proxy jede Zustellung auf `/anmelden` um: der Anbieter meldet „Provisioning failed“, in Laufzettel steht nichts, das Provisioning hört still auf zu arbeiten. `OEFFENTLICH` ist jetzt exportiert, und ein Test prüft **in beide Richtungen**: wer die Liste erweitert, muss es im Test aufschreiben. Eine Aufzählung, die nur in einer Richtung geprüft wird, läuft dem Code hinterher. - [x] **Ein offener Ereignisstrom überlebte Abmelden, Deaktivieren und Sitzungswiderruf.** Das Recht wurde genau einmal geprüft, beim Aufbau, und Kennung samt Rolle für die Lebensdauer des Stroms festgehalten. Ein Administrator deaktiviert ein Konto — und eine offen gelassene Lasche bekommt weiter live Status, Schrittzustände und Zählwerte, bis jemand sie schließt. `docs/architektur.md` verspricht „ein deaktivierter Benutzer wird auch mit gültigem Cookie abgewiesen“; für einen Strom galt das nicht. Der Herzschlag prüft jetzt nach (`sitzungNochGueltig`), eine Herabstufung engt den Strom ein statt ihn zu beenden. - [x] **`README.md` kannte die Rolle `MITARBEITER` nicht**, und ein Satz daneben war seit #035 falsch: „Unbekannte Benutzer werden beim ersten Login angelegt.“ Wer auf v0.2.0 hebt, liest das README, nimmt an, für Leute ohne Rollengruppe bleibe alles wie bisher — und sperrt danach genau diese Leute aus. #035 hatte den Satz in `docs/betrieb/authentik.md` richtiggestellt und das README stehengelassen. Dazu: der Stand stand weiter auf „in Entwicklung auf v0.1.0“. - [x] **`docs/betrieb/sicherung.md` nannte einen Anhänge-Pfad, den es nirgends gibt** (`/opt/laufzettel/daten/anhaenge`) — als Rückfallwert in einem Skript, in dem ein `rm -rf` steht. `deploy/install.sh` setzt `/var/lib/laufzettel/anhaenge`, und ohne die Variable nimmt die Anwendung `./daten/anhaenge` relativ zu ihrem Arbeitsverzeichnis. Der Rückfall passte zu **keinem** der drei. Jetzt `${ANHAENGE_PFAD:?…}`: fehlt die Variable, **bricht** der Befehl ab, statt zu raten — vor dem `rm -rf`. Dasselbe Dokument hat denselben Fehler in #032 schon einmal gehabt. - [x] **Attributnamen wurden nur in der Server Action geprüft**, während sich `src/server/graph/leser.ts` im Kommentar darauf berief („feste Feldnamen aus `ATTRIBUT_MUSTER`, also ist hier nichts zu maskieren“). Nachgewiesen: ein Name mit Komma und `$filter` kam durch den Dienst und schrieb einen **zweiten `$filter`** in die Graph-Adresse. Hereinkommen konnte er nur mit `verwaltung` oder per Skript, Seed und `psql`, die Sicherheitswirkung ist also klein — der Befund ist der **Ort** der Regel. Jetzt `zuordnungPruefen` im Dienst (wirft) und Verwerfen in `zuordnungLesen` (damit eine kaputte Zeile nicht die Quellenliste sprengt). ### Leicht - [x] **Drei Kommentare versprachen mehr als der Code hält.** Der Kommentar am Konstanten-Block der Wartezeiten sagte „nach dem vierten Fehlschlag wird aufgegeben“, während es der fünfte ist — und das ist der Kommentar, den man zuerst liest. Die Migration der Benachrichtigungen behauptete „der Dienst prüft dasselbe noch einmal, nur mit besserer Meldung“; tut er nicht, und er soll es auch nicht (zwei Instanzen hinter einem Lastverteiler fragen beide und bekommen beide „nein“ — nur der partielle Index hält). **Die Migration selbst bleibt unverändert:** eine angewandte Migration wird nicht umgeschrieben, so wie eine veröffentlichte `VorlagenVersion` nicht. Die Richtigstellung steht bei `einreihen`, wo sie gelesen wird. - [x] **Der Protokolleintrag „UPN mehrdeutig“ nannte immer „2“**, auch bei sieben Treffern: `personZuUpn` hatte `take: 2`, und dieselbe Zahl ging in den Eintrag. Wer das Protokoll liest, liest es, **um den Fall aufzulösen** — er muss wissen, wie viele Zeilen zu bereinigen sind. `take` entfernt, Test mit vier Treffern. - [x] **`anzahlSchritte` im Protokoll eines neuen Laufs zählte die Version, nicht den Lauf.** Bei einer Vorlage mit abteilungsgebundenen Abschnitten (#031) behauptete der Eintrag mehr Schritte, als der Lauf hat. Das Protokoll ist seit #023 ansehbar. - [x] **Die Phase-2-Seiten fehlten in der 390-px-Prüfung.** Ergänzt: Bausteinliste, Schnellstart, Protokoll, Zustellungen — gerade die Tabellen, also die Stellen, an denen horizontales Scrollen entsteht. Jedes Issue berichtet eine Klickprobe von Hand; eine Probe von Hand ist kein Regressionstest. - [x] **Der Graph-Seitenabruf folgte `@odata.nextLink` ohne Host-Prüfung**, mit einem mandantenweit gültigen Token im Kopf. Graph über TLS zu trauen ist vertretbar — aber `GRAPH_API_URL` ist absichtlich einstellbar (nationale Clouds, Test-Doppelgänger), und ein Tippfehler darin schickte das Token woandershin. `naechsteSeite` vergleicht jetzt den Ursprung. ## Nicht behoben, bewusst - **`anhaengeDesLaufs` liegt in `services/` und wendet keine Mandantensicht an.** Beide Aufrufer prüfen den Lauf vorher, und die Kommentare sagen das auch — aber es steht gegen die Regel aus `CLAUDE.md` („`queries/*` wenden ihre Sicht selbst an, damit kein Aufrufer sie vergessen kann“). Zwei Aufrufer in zwei Schichten gibt es schon; der dritte ist der, der es vergisst. Eigenes Issue: [#037](037-anhaenge-lesemodell.md). - **`/meine-geraete` fehlt weiter in der 390-px-Prüfung.** Die Seite verlangt `selbstauskunft`, und das hat nur `MITARBEITER`; der E2E-Ablauf meldet sich als Admin an und würde die Verweigerungsseite messen. Dafür braucht es ein eigenes Konto samt Personenbindung — siehe [#037](037-anhaenge-lesemodell.md). - **Das Graph-Token wird einmal je Abgleich geholt**, vor der Seitenschleife. Bei einem Mandanten, dessen Abgleich länger als die Token-Lebensdauer läuft, hilft der Sicherheitsabstand nicht. Das gehört auf die offene Prüfliste in `docs/betrieb/entra-id.md` und ist ohne echten Mandanten nicht zu beurteilen. ## Was der Review *nicht* gefunden hat Benannt, weil es den Befunden ihr Gewicht gibt: keine `kundeId` oder `quelleId` aus der Eingabe statt aus Sitzung bzw. Token; kein Lesemodell, das seine Sicht ganz vergisst; keine Client-Komponente mit einem Wert-Import aus `src/server/*`; kein Geheimnis in einer Sicht, Antwort oder einem Protokolleintrag; kein gespeichertes `{{generiertes_passwort}}`; kein Codepfad, der annimmt, dass nur ein Mensch einen Schritt erledigt. Ein Befund des Reviews war **falsch**: der Zustands-Handler habe keinen Vitest-Test. Er hat acht direkte Aufrufe in `tests/lauf-ereignisse.test.ts`, darunter „weist ohne Anmeldung ab“ und „liefert für einen fremden Kunden keinen Strom“. Nachgeprüft, bevor etwas geändert wurde.