diff --git a/.planning/quick/260921-oxm-imap-starttls-wirklich-erzwingen-und-anh/260921-oxm-SUMMARY.md b/.planning/quick/260921-oxm-imap-starttls-wirklich-erzwingen-und-anh/260921-oxm-SUMMARY.md new file mode 100644 index 0000000..f185c94 --- /dev/null +++ b/.planning/quick/260921-oxm-imap-starttls-wirklich-erzwingen-und-anh/260921-oxm-SUMMARY.md @@ -0,0 +1,194 @@ +--- +phase: quick-260921-oxm +plan: 01 +subsystem: apps/api/src/inbox +tags: [imap, imapflow, starttls, sicherheit, anhaenge, dkv, tdd] +status: complete +requires: + - "260921-m34 (Befunde B-05 und B-06)" +provides: + - "ImapProvider erzwingt STARTTLS ueber die Option, die imapflow wirklich kennt" + - "ImapProvider erkennt Anhangs-Dateinamen aus Content-Disposition" +affects: + - "apps/api/src/inbox/imap.provider.ts" + - "apps/api/src/inbox/imap.provider.spec.ts" +tech-stack: + added: [] + patterns: + - "Konstruktoroptionen einer Fremdbibliothek im Test gegen die uebergebenen Werte pruefen, nicht gegen eine echte Verbindung" +key-files: + created: [] + modified: + - apps/api/src/inbox/imap.provider.ts + - apps/api/src/inbox/imap.provider.spec.ts +decisions: + - "doSTARTTLS bei ssl-tls auf false statt weglassen: schliesst die Unvertraeglichkeit secure=true + doSTARTTLS=true aus und schaltet STARTTLS dort ausdruecklich ab" + - "Einhaengen des Testdoppels in einen Helfer gezogen, damit die neuen Faelle ohne eigene Umdeutung auskommen" +metrics: + duration: "35 min (17:55 bis 18:30 Uhr, 21.09.2026)" + completed: 2026-09-21 +actuals: + tokens: 31000 + tasks: 2 + commits: 4 +plan_head_before: ad83407 +--- + +# Quick-Aufgabe 260921-oxm: IMAP-STARTTLS wirklich erzwingen und Anhangs-Dateinamen richtig lesen Summary + +Zwei falsche Annahmen im IMAP-Postfachzugriff sind behoben: die Einstellung +"STARTTLS" bewirkt jetzt tatsaechlich, was ihr Name verspricht, und +Rechnungsanhaenge aus Outlook werden wieder am Dateinamen erkannt. Beide +Reparaturen haengen an Tests, die gegen den vorherigen Stand nachweislich rot +waren. + +## Was sich fuer den Betrieb aendert + +**Die eine gewollte Verhaltensaenderung, in einem Satz:** Ein Postfach, das auf +"STARTTLS" eingestellt ist, dessen Server diese Verschluesselung aber gar nicht +anbietet, meldet ab jetzt einen Verbindungsfehler — bisher hat Tessera in genau +diesem Fall stillschweigend unverschluesselt weitergemacht und Benutzername und +Kennwort im Klartext uebertragen. Wer so ein Postfach hat, sieht den Fehler +sofort und kann die Einstellung richtigstellen; vorher hat niemand etwas +gemerkt. + +**Die zweite Aenderung ist eine Reparatur, keine Umstellung:** Anhaenge, die ein +Absender als `application/octet-stream` verschickt — was Outlook regelmaessig +tut — wurden bisher nur dann als PDF erkannt, wenn der Dateiname zusaetzlich im +Inhaltstyp stand. Der zweite, haeufigere Weg ueber die Angabe +"Content-Disposition" wurde zwar abgefragt, lieferte aber baulich bedingt nie +ein Ergebnis. Er funktioniert jetzt. Betrifft den DKV-Rechnungseinzug. + +## B-06 — STARTTLS wurde nie erzwungen + +`buildClient()` uebergab `requireTLS: config.encryption === 'starttls'` an +`new ImapFlow(...)`. Diese Option kennt imapflow 1.4.3 nicht: weder +`ImapFlowOptions` in `lib/imap-flow.d.ts` noch der Laufzeitcode in +`lib/imap-flow.js` erwaehnen sie — beides durchsucht, kein einziger Treffer. Sie +wurde also entgegengenommen und weggeworfen. Verdeckt hat das die Zusicherung +`} as any` am Ende derselben Funktion: sie hat dem Compiler verboten, die +unbekannte Option zu bemaengeln. + +Ohne gesetzte Option galt das Standardverhalten der Bibliothek, das sie selbst +so beschreibt: bei `secure=false` auf TLS hochstufen, *falls* der Server es +anbietet, sonst unverschluesselt weitermachen — mit dem ausdruecklichen Zusatz +*"This may expose the connection to a downgrade attack."* + +**Reparatur:** `doSTARTTLS: config.encryption === 'starttls'` (deklariert in +`imap-flow.d.ts:81`, ausgewertet in `imap-flow.js:1183`). Bei `starttls` ergibt +der Ausdruck `true` und die Verbindung scheitert, wenn der Server kein STARTTLS +kann. Bei `ssl-tls` ergibt er `false`, was STARTTLS ausdruecklich abschaltet +(`imap-flow.js:1210`) — das ist wichtiger als es aussieht: die Bibliothek wirft +bei `secure=true` zusammen mit `doSTARTTLS=true` einen Konfigurationsfehler +(`imap-flow.js:1201`). Ein schlichtes `true`/`undefined` waere hier also falsch +gewesen. Genau diese Kombination prueft der zweite Test mit. + +**Die Zusicherung konnte ersatzlos entfallen.** Nach der Reparatur sind alle +sechs uebergebenen Felder in `ImapFlowOptions` deklariert; `tsc` ist ohne das +`as any` fehlerfrei. Damit ist die Stelle nicht nur getypt, sondern kann kuenftig +auch keine weitere erfundene Option mehr verstecken. + +## B-05 — der Dateiname kam aus dem falschen Feld + +`collectPdfParts()` las `(node as any).disposition?.parameters?.filename`. +imapflow deklariert `disposition` aber als **Zeichenkette** +(`imap-flow.d.ts:448` — der Wert ist "attachment" oder "inline") und legt die +zugehoerigen Parameter in ein eigenes Feld `dispositionParameters` +(`imap-flow.d.ts:450`). Der Ausdruck las also `.parameters` von einer +Zeichenkette und war zur Laufzeit **immer** `undefined`. + +**Reparatur:** `node.dispositionParameters?.filename?.toLowerCase() ?? ''` — +getypt, ohne Zusicherung. Nachgeprueft, nicht geraten: imapflow fuellt das Feld +in `tools.js:887` ueber `getStructuredParams()`, und diese Funktion schreibt die +Schluessel **kleingeschrieben** (`tools.js:648`). `filename` ist damit der +richtige Schluessel, unabhaengig davon, wie der Absender die Angabe gross- oder +kleingeschrieben hat. + +Der zweite moegliche Fundort, den die Aufgabe erwaehnt — der Name in den +Parametern des Inhaltstyps — war bereits vorhanden und wird weiter geprueft +(`node.parameters?.name`). Ein dritter Fall wurde nicht erfunden. Die +RFC-2231-Fortsetzungsparameter (`filename*0`, `filename*1` ...) setzt imapflow +selbst wieder zu einem einzigen `filename` zusammen (`tools.js:662` ff.), es +braucht dafuer hier also nichts. + +## Die Tests, und der Beleg dass sie rot waren + +Beide Faelle liegen in `apps/api/src/inbox/imap.provider.spec.ts`. Gegen den +Stand `7691d1f` (Tests vorhanden, Reparatur noch nicht) scheiterten genau drei +von zwoelf Faellen: + +``` + × ImapProvider - Transportverschluesselung (B-06) > erzwingt STARTTLS, wenn die Verschluesselung auf starttls steht + -> expected undefined to be true // Object.is equality + × ImapProvider - Transportverschluesselung (B-06) > setzt doSTARTTLS nicht auf true, wenn die Verschluesselung auf ssl-tls steht + -> expected { host: 'imap.example.com', ...(5) } to not have property "requireTLS" + × ImapProvider.fetchPdfAttachments - Dateiname aus Content-Disposition (B-05) > erkennt einen application/octet-stream-Anhang am Dateinamen aus dispositionParameters + -> expected [] to have a length of 1 but got +0 + + Test Files 1 failed (1) + Tests 3 failed | 9 passed (12) +``` + +Die erste Zeile ist der Kern von B-06: `doSTARTTLS` war schlicht nicht gesetzt. +Die zweite belegt, dass stattdessen ein Feld `requireTLS` ankam, das die +Bibliothek nicht auswertet. Die dritte belegt B-06 nicht, sondern B-05: der +Anhang wurde gar nicht erst eingesammelt. + +Zwei weitere neue Faelle waren von Anfang an gruen und sollen das auch bleiben — +sie sichern, dass die Erkennung ueber den Inhaltstyp-Namen weiter greift und +dass ein `octet-stream`-Anhang **ohne** `.pdf`-Endung weiterhin liegen bleibt. +Ohne sie haette die Reparatur unbemerkt zu viel einsammeln koennen. + +Nach der Reparatur: 12 von 12 gruen. + +## Messungen + +| Groesse | vorher (ad83407) | nachher (d0266bf) | +|---|---:|---:| +| `lint/suspicious/noExplicitAny` in `apps/api/src` | 15 | **13** | +| `lint/style/noNonNullAssertion` in `apps/api/src` | 56 | 56 | +| `as unknown as` in `apps/api/src` | 33 | **27** | +| `biome-ignore` in `apps/api/src` | 1 | 1 | +| `ts-expect-error` / `@ts-ignore` | 0 | 0 | +| `pnpm type-check` | 4/4 | 4/4 | +| `pnpm lint` | 5/5, 0 Fehler | 5/5, 0 Fehler | +| Tests `apps/api` | 72 Dateien / 1143 | 72 Dateien / **1148** | +| Tests `apps/web` | 73 / 531 | 73 / 531 | + +Zwei Zeilen brauchen eine Erklaerung. + +**`noExplicitAny` 15 auf 13:** beide verbliebenen imapflow-Stellen aus dem +Urteilsregister von 260921-m34 (Nummern 14 und 15) sind weg. Die 13 +verbleibenden sind unveraendert die dort begruendeten: sechs an node-forge, drei +an der Cron-Beschaffung, vier an der Transaktionshilfe. + +**`as unknown as` 33 auf 27:** das ist keine Nebenwirkung der Reparatur, sondern +Absicht. Die Testdatei haengte ihr Testdoppel in jedem einzelnen Fall mit +derselben Umdeutung des Konstruktors ein — zwoelf Mal, sobald die neuen Faelle +dazukamen. Diese eine Zeile steht jetzt in einem Helfer `useMockClient()`, und +die neuen Faelle brauchen keine eigene Umdeutung mehr. Die Alternative waere +gewesen, fuenf neue Umdeutungen hinzuzufuegen und den Zaehler zu heben; das war +ausgeschlossen. Der Zaehler faellt, er steigt an keiner Stelle. + +## Deviations from Plan + +Eine, und sie steht schon oben: der Helfer `useMockClient()` in der Testdatei +war im Plan nicht vorgesehen. Er wurde noetig, weil die neuen Faelle das +Testdoppel sonst nur ueber fuenf zusaetzliche Umdeutungen haetten einhaengen +koennen — was die Vorgabe "Zaehler duerfen nicht steigen" verletzt haette. Die +Aenderung ist mechanisch (dieselbe Zeile, an einer Stelle statt an zwoelf) und +aendert an keinem bestehenden Fall das Verhalten; alle sieben Altfaelle sind +unveraendert gruen. + +Sonst nichts: keine neue Abhaengigkeit, kein Versionssprung, kein repo-weites +Umformatieren, keine Aenderung an `STATE.md` oder `ROADMAP.md`. + +## Known Stubs + +Keine. + +## Self-Check: PASSED + +Alle vier genannten Dateien liegen auf der Platte, alle drei Commits sind in +`git log` auffindbar (f23671a, 7691d1f, d0266bf). Die Messungen der Tabelle oben +stammen aus tatsaechlich gelaufenen Befehlen, nicht aus einer Schaetzung.