pl_mail/CODE-REVIEW.md

Code Review — Sicherheit & UX

Projekt: plMail · Stand: 132c1bf (v0.1.9) · Datum: 2026-08-22 Fokus: Sicherheit und UX · Umfang: 143.503 LOC PHP in src/, Templates, Stimulus-Controller, Deployment-Konfiguration

---

1. Gesamteindruck

Das ist eine überdurchschnittlich sorgfältig gebaute Codebase. Die sicherheitskritischen Kernentscheidungen sind nicht nur richtig getroffen, sondern im Code auch begründet — der Image-Proxy (ImageProxyFetcher) gehört zum Besten, was man an SSRF-Abwehr in einer PHP-Anwendung sieht (DNS-Pinning gegen Rebinding, manuelle Redirect-Behandlung mit Revalidierung pro Hop, Port- und Scheme-Pinning). Die Reading-Frame-Isolierung (MessageRenderer) ist ein Lehrbuchbeispiel: opaker Origin, Nonce-CSP, img-src auf den eigenen Origin beschränkt, damit Blocker und CSP beide versagen müssten, bevor ein Tracking-Pixel feuert.

Genau deshalb fällt der zentrale Fund umso stärker auf: die sorgfältig gebaute Sandbox wird beim Antworten auf eine Mail umgangen. Der Lesepfad ist gehärtet, der Antwortpfad nicht — und beide arbeiten mit demselben, vom Angreifer kontrollierten HTML.

Ergebnis in Zahlen: 1 kritischer, 2 hohe, 2 mittlere, 5 niedrige Sicherheitsbefunde; 4 kleinere UX-/Accessibility-Befunde. Autorisierung (OwnershipVoter), Credential-Verschlüsselung (Encryptor), 2FA-Throttling, CSRF und SQL-Parametrisierung wurden geprüft und sind sauber.

Methodik und Grenzen dieses Reviews

Geprüft wurde statisch (Code, Templates, JS, Deployment-Config) plus die anonym erreichbaren Antworten der Live-Instanz https://mail.vpn.cpy-pst.de/.

Zwei Einschränkungen, die für die Bewertung der UX relevant sind:

• Claude for Chrome stand in dieser Session nicht zur Verfügung. Es waren keine Browser-Tools geladen, nur WebFetch. Der Frontend-Teil beruht daher auf Templates, Stimulus-Controllern und der anonym ausgelieferten Login-Seite — nicht auf tatsächlicher Bedienung.
• Kein Login möglich (keine Zugangsdaten). Alles hinter der Anmeldung — Inbox, Compose, Kalender, Admin — ist ausschließlich aus dem Code beurteilt. Aussagen zu Laufzeitverhalten, Ladezeiten und tatsächlicher Tastaturbedienung fehlen deshalb bewusst.

Die Sicherheitsbefunde sind davon nicht betroffen: alle sind am Code vollständig nachvollzogen und mit Datei- und Zeilenangaben belegt.

---

1a. Bearbeitungsstand

Gepflegt beim Abarbeiten — eine Zeile pro Befund, mit dem Commit, der ihn schließt. Ein Befund gilt erst als erledigt, wenn ein Test ihn festhält; „behoben" ohne Test heißt hier „behoben, bis es jemand versehentlich zurückdreht".

ID	Stand	Commit	Anmerkung
S-01	✅ behoben	016d9f3	Alle drei empfohlenen Stellen, plus ein Sanitizer-Profil fürs Verfassen, das die Review nicht vorgesehen hatte — siehe unten.
S-02	✅ behoben	8c3080d	Positivliste statt Präfix-Test (InlineDisposition), plus sandbox-CSP auf der Antwort.
S-03	✅ behoben	a4d09b8	In Produktion erzwungen, nicht nur Report-Only — siehe unten, warum das möglich wurde.
S-04	✅ behoben	8c3080d	Nicht-Bilder fallen auf den selbst erzeugten Platzhalter zurück, nicht auf einen Download.
S-05	✅ behoben	8c3080d	Header-Block im Caddyfile, mit defer, damit die strengeren Per-Response-Policies gewinnen.
S-06	✅ behoben	8c3080d, siehe Nachtrag	subscribe: []. Die Annahme „Topic-Isolierung greift" war falsch: alle Updates waren öffentlich — seit dem Nachtrag privat (UserUpdate).
S-07	⚪️ bewusst so	—	Siehe „Bewusst so belassen" unten.
S-08	✅ behoben	8c3080d	Mit S-02 mitgezogen.
S-09	⚪️ bewusst so	—	Siehe unten.
S-10	⚪️ bewusst so	—	Siehe unten.
U-01	✅ behoben	0f80d2c	Ein echter Fund von vieren — die anderen drei sind Fehlbefunde, siehe unten.
U-02	✅ behoben	016d9f3, 59cbbc9	Sicherheitshälfte mit S-01 Punkt 2; UX-Hälfte (Paste-Normalisierung) in 59cbbc9.
U-03	⚪️ kein Fund	—	Alle neun sind korrekt identisch, siehe unten.
U-04	⚪️ bewusst so	—	Novelty-Locale, Fallback funktioniert.

Nachtrag zu S-03 — warum „erzwungen" statt „Report-Only"

Die Review empfiehlt einen Rollout über Content-Security-Policy-Report-Only, und das war richtig als Vorgehen und nicht als Endzustand: ein Report-Only-Header schützt vor nichts. Der Weg dorthin war, die Verstöße zu beseitigen statt sie zu erlauben.

Der letzte davon waren die eigenen Stylesheets der Anwendung. AssetMapper setzt import './app.css' als data:application/javascript,-Modul um, das zur Laufzeit ein <link> anhängt — jedes Stylesheet war für den Browser also ein Skript, und der einzige Weg, sie zuzulassen, wäre script-src … data: gewesen. Das lässt einen der ältesten Injektionsvektoren wieder herein und gibt den größten Teil des Werts der Direktive her. Beide Stylesheets werden jetzt aus dem Layout ge<link>t, was AssetMapper für CSS ohnehin empfiehlt und nebenbei ein Flash-of-unstyled-content beseitigt.

Ergebnis: script-src 'self' 'nonce-…' wird in Produktion erzwungen. Unter Debug läuft die volle Policy als Report-Only mit, weil Profiler-Toolbar und VarDumper Inline-Skripte einfügen, die niemand noncen kann — csp.spec.ts hält fest, dass die Anwendungsflächen null Verstöße erzeugen.

Nachtrag zu U-01 und U-03 — drei bzw. neun Fehlbefunde

Nachgeprüft, statt sie zu „beheben":

U-01. Von vier Treffern ist einer echt: der Schließen-Button des Composers hatte nur title und ist oberhalb md der einzige Ausgang — er hat jetzt ein aria-label. Die anderen drei nicht. Der Avatar-Picker-Button bezieht seinen Namen bereits aus dem alt seines Bildes, was korrekte Praxis ist; und die beiden „<img> ohne alt" liegen beide in Twig-Kommentaren — die Zeilen _thread_row.html.twig:77 und _attachment_chip.html.twig:17 sind Prosa, die <img src=x …> erwähnt. Das Anhang-Thumbnail trägt längst alt="".

U-03. Alle neun Werte sind zu Recht identisch: Administrator ist deutsch, %from% – %to% und %date%, %time% bestehen nur aus Platzhaltern, in ist in beiden Sprachen dasselbe Wort, Firebase Cloud Messaging und google-services.json sind Produkt- bzw. Dateinamen, und Kathleen Booth ist ein Personenname in Demodaten. Eine „Übersetzung" hätte die deutsche Oberfläche schlechter gemacht.

Bewusst so belassen — S-07, S-09, S-10

Die Review schlägt für diese drei „bewusst entscheiden und dokumentieren" vor. Hiermit:

S-07 — signierte Image-Proxy-URLs laufen nicht ab. Bleibt so. Die Zieladresse steht in der Signatur, und ImageProxyFetcher führt bei jedem Abruf sämtliche SSRF-Prüfungen erneut durch: eine geleakte URL erlaubt, den Server diese eine feste, öffentliche HTTPS-Ressource abrufen zu lassen. Ein exp würde die Signatur um einen Zustand erweitern, den sonst nichts an diesem Pfad hat.

S-09 — Trust-on-first-use bei /install. Bleibt so, aber das Handbuch sagt es jetzt: die Ersteinrichtung gehört unmittelbar nach dem ersten Start erledigt. Ein Einmal-Token aus generated.env wäre für selbst gehostete Software unverhältnismäßig — wer den Container startet, hat ohnehin Dateizugriff.

S-10 — kein Rate-Limit auf der JMAP-Authentifizierung. Bleibt vorerst so. Brute-Force ist chancenlos (32 Byte CSPRNG, SHA-256, indizierter Vergleich); was fehlt, ist ein Schutz gegen Ressourcenverbrauch, und das ist eine Verfügbarkeitsfrage für eine Instanz, die ihre JMAP-Schnittstelle öffentlich exponiert. Notiert als Aufgabe, nicht als Befund.

Nachtrag zu S-01 — zwei Fallen, die die empfohlene Behebung so nicht überlebt hätte

Die Analyse des Reviews stimmt vollständig; die vorgeschlagene Korrektur hätte in Punkt 2 zwei Dinge kaputt gemacht, die beide von vorhandenen Tests gehalten wurden.

Die Allow-Liste verwirft jedes data--Attribut. Das ist für fremdes HTML richtig und für einen selbst verfassten Text zerstörerisch: data-quoted ist, woran das Zitat gefunden wird — zum Einklappen, und um es aus dem Snippet zu schneiden —, data-pl-signature ist, was ein Absenderwechsel ersetzt, und data-cid ist die gesamte Brücke zwischen einem Inline-Bild im Editor und der cid:-Referenz, die auf die Leitung geht. sanitizeFragment() auf bodyHtml in markAsDraft() hätte den Body sicher und den Composer kaputt hinterlassen. Deshalb ein eigenes Profil (sanitizeComposedBody()) mit genau diesen fünf Markern — geschlossene Liste, nicht data-* pauschal.

HtmlSanitizerConfig ist unveränderlich. Jede allow*-Methode liefert einen Klon. Ein $config->allowAttribute(...) als Anweisung geschrieben konfiguriert eine Kopie, die anschließend verfällt — es sieht aus wie eine funktionierende Allow-Liste und entfernt alles, was sie erlauben sollte.

Und der Sanitizer maskiert @ zu @. In einem Attributwert ist das korrektes HTML und im Browser folgenlos. Für eine cid:-Referenz ist es fatal: Symfony\Component\Mime\Email::prepareParts() verbindet einen eingebetteten Teil mit dem Body über einen Vergleich der Zeichenkette cid:<name>, und InlineAttachmentDetector entscheidet beim Ingest genauso. Keiner von beiden parst HTML. Ohne Gegenmaßnahme wäre jede gesendete Mail mit Inline-Bild mit einem kaputten Bild rausgegangen — der Befund wäre geschlossen und ein Sendefehler eingebaut gewesen. restoreCidReferences() stellt diese Referenzen wieder her, und nur dort, wo die dekodierte Fassung noch wie eine Referenz aussieht: einen Attributwert allgemein zu dekodieren wäre ein Weg aus ihm heraus.

---

2. Befundübersicht

ID	Schwere	Titel	Ort
S-01	🔴 Kritisch	Stored XSS: Antworten/Weiterleiten zitiert unsanitisiertes bodyHtml in den App-Origin	ReplyDraftBuilder.php:213
S-02	🟠 Hoch	Stored XSS: SVG-Anhang wird inline ausgeliefert, per cid:-Link aus der Mail erreichbar	AttachmentController.php:68
S-03	🟠 Hoch	Keine CSP auf dem Anwendungsdokument — Verstärker für S-01/S-02	global
S-04	🟡 Mittel	File-Picker-Preview liefert fremden MIME-Typ ohne Content-Disposition inline aus	FilePickerController.php:134
S-05	🟡 Mittel	Anwendung liefert keinerlei Security-Header aus; Schutz hängt am Proxy des Betreibers	frankenphp/Caddyfile
S-06	🟢 Niedrig	Mercure-Default subscribe: '*' als latente Fehlerquelle	config/packages/mercure.yaml:9
S-07	🟢 Niedrig	Signierte Image-Proxy-URLs laufen nie ab	ImageProxySigner.php:43
S-08	🟢 Niedrig	Gleiches Inline-Muster wie S-02 im JMAP-Download (latent)	JmapDownloadController.php:82
S-09	🟢 Niedrig	/install — Trust-on-first-use-Fenster beim Erststart	InstallGuard.php:27
S-10	ℹ️ Info	Kein Rate-Limit auf der JMAP-Authentifizierung	security.yaml (Firewall jmap)
U-01	🟡 UX	4 Accessibility-Lücken (Icon-Buttons ohne Namen, <img> ohne alt)	Templates
U-02	🟢 UX	Eingefügtes HTML im Composer wird nicht bereinigt — geht so auf die Leitung	compose_controller.js:3077
U-03	🟢 UX	9 unübersetzte deutsche Strings	translations/messages.de.yaml
U-04	ℹ️ UX	en_PI fehlen 218 Keys (Fallback auf Englisch)	translations/messages.en_PI.yaml

---

3. Sicherheitsbefunde im Detail

🔴 S-01 — Stored XSS über zitierten Mail-Inhalt beim Antworten

Ort: src/Service/Mail/ReplyDraftBuilder.php:213, ausgegeben in templates/compose/_window.html.twig:487

Das Problem

Message trägt zwei Body-Felder, und der Unterschied ist die gesamte Sicherheitsgrenze der Anwendung:

• bodyHtml — das rohe HTML des Absenders, unverändert wie empfangen
• bodyHtmlSafe — die von MailBodySanitizer bereinigte Fassung

Der Lesepfad benutzt korrekt bodyHtmlSafe und rendert es zusätzlich in einem sandboxed iframe. ReplyDraftBuilder::quote() greift jedoch auf das rohe Feld zu:

// src/Service/Mail/ReplyDraftBuilder.php:213
$html = trim($original->bodyHtml ?? '');   // ← roh, nicht bodyHtmlSafe
$text = trim($original->bodyText ?? '');
$body = '' !== $html ? $html : nl2br(htmlspecialchars($text, ENT_QUOTES, 'UTF-8'));

$body wird unverändert in den Entwurf eingebettet ($draft->bodyHtml = $lead . $this->quote(...), Zeile 142). Das Compose-Fenster gibt dieses Feld dann per |raw in ein contenteditable-Div aus — im Hauptdokument der Anwendung, nicht im iframe:

{# templates/compose/_window.html.twig:487 #}
empty:before:pointer-events-none">{{ form.bodyHtml.vars.value|raw ?: '' }}</div>

DraftPersister::markAsDraft() ruft zwar $this->bodySanitizer->sanitize($message) auf (DraftPersister.php:154) — aber MailBodySanitizer::sanitize() schreibt sein Ergebnis nach bodyHtmlSafe und lässt bodyHtml unangetastet (MailBodySanitizer.php:72). Der Sanitizer läuft also, rettet aber genau das Feld nicht, das ausgegeben wird.

Angriffskette

1. Angreifer sendet eine Mail mit <img src=x onerror="…"> im HTML-Body an eine plMail-Adresse.
2. Ingest speichert das rohe HTML in bodyHtml (MessageSyncer.php:487, GmailMessageBuilder.php:159, GraphMessageBuilder.php:194), die bereinigte Fassung in bodyHtmlSafe.
3. Das Opfer liest die Mail — sicher, die Sandbox greift.
4. Das Opfer klickt Antworten (GET /compose/reply/{id}, ComposeController.php:136).
5. Das rohe HTML landet über quote() im Entwurf und wird per |raw ins Hauptdokument geschrieben.
6. onerror feuert im Origin der Anwendung — mit Session, ohne CSP (siehe S-03).

Reply, Reply-All und Forward sind alle betroffen (ComposeController.php:136, :152, :168).

Auswirkung

Vollständige Übernahme der Sitzung im Kontext der Anwendung. Konkret erreichbar: sämtliche Mail lesen und exfiltrieren, ein App-Passwort erzeugen (ApiToken) und damit dauerhaften JMAP-Zugriff etablieren, der ein Passwort-Reset überlebt, sowie eine Weiterleitungsregel anlegen. Der Angriff braucht keinen zweiten Klick über „Antworten" hinaus — eine Handlung, die bei einer Mail, die eine Antwort provoziert, der Normalfall ist.

Das HttpOnly-Flag auf PHPSESSID verhindert das Auslesen des Cookies, nicht aber Aktionen im Namen des Nutzers.

Behebung

Drei Stellen, gestaffelt — die erste behebt den Befund, die anderen beiden verhindern die Wiederkehr:

1. Zitat aus der bereinigten Fassung bauen (ReplyDraftBuilder):

public function __construct(
    private readonly SignatureProvider  $signatures,
    private readonly MailBodySanitizer  $sanitizer,   // neu
) {}

// in quote():
$html = $this->sanitizer->sanitizeFragment($original->bodyHtml);

sanitizeFragment() existiert bereits und wird für Signaturen genau zu diesem Zweck verwendet (MailBodySanitizer.php:95). bodyHtmlSafe direkt zu nehmen wäre die Alternative, ist aber schlechter: dort sind cid:-Referenzen bereits in App-URLs umgeschrieben, was den Umweg über InlineImageRewriter::toCid() beim Speichern nötig macht.

2. bodyHtml beim Persistieren bereinigen (DraftPersister::markAsDraft()) — deckt zusätzlich U-02 ab, also alles, was über Paste oder einen manipulierten POST in den Composer gelangt, und stellt sicher, dass die versendete Mail kein Script trägt:

$message->bodyHtml = $this->bodySanitizer->sanitizeFragment($message->bodyHtml);
$this->bodySanitizer->sanitize($message);

3. Ausgabe absichern. Das |raw in _window.html.twig:487 sollte nicht auf einem Feld stehen, dessen Sicherheit von einem entfernten Aufrufer abhängt. Sauber wäre eine Twig-Funktion analog zu message_render() — der Kommentar in MessageRender beschreibt genau dieses Prinzip („Exists so that no Twig file has to ask a security question") und sollte auch hier gelten.

Zusätzlich: JmapDraftWriter baut Entwürfe für Nicht-Browser-Clients und ist laut eigenem Docblock eine bereits abgedriftete Kopie dieser Logik — bei der Korrektur mitprüfen.

---

🟠 S-02 — Stored XSS über SVG-Anhang

Ort: src/Controller/Mail/AttachmentController.php:68

Das Problem

$inlineAllowed = true === str_starts_with($contentType, 'image/');

image/svg+xml erfüllt dieses Prädikat. SVG ist aber kein passives Bildformat: als Top-Level-Dokument geladen führt es <script> aus. X-Content-Type-Options: nosniff hilft hier nicht — der deklarierte Typ ist image/svg+xml, es wird nichts fehlinterpretiert. $contentType stammt aus dem MIME-Header der eingehenden Mail und ist vollständig angreiferkontrolliert.

Warum das erreichbar ist

Die Templates verlinken Anhänge durchweg mit ?download=1 (_attachment_chip.html.twig:53), was DISPOSITION_ATTACHMENT erzwingt. Der Inline-Pfad ist über die normale Oberfläche also nicht erreichbar — er ist es über die Mail selbst:

MailBodySanitizer::resolveCids() ersetzt jedes Vorkommen von cid: im gesamten Dokument, nicht nur in img src:

// src/Service/Mail/MailBodySanitizer.php:131
return (string)preg_replace_callback('/cid:([^"\'\)\s>]+)/i', /* … */, $html);

Der Sanitizer erlaubt anschließend <a href> mit relativen Links (allowRelativeLinks(), Zeile 200) und setzt target="_blank". Damit gilt:

1. Angreifer hängt ein SVG mit <script> an, Content-ID logo.
2. Im Body steht <a href="cid:logo">Rechnung ansehen</a>.
3. resolveCids() schreibt das in <a href="/mail/attachment/4711" target="_blank"> um — der Angreifer muss die Part-ID nicht kennen, die Anwendung setzt sie selbst ein.
4. Klick öffnet einen neuen Tab, ohne ?download=1 → DISPOSITION_INLINE, Content-Type: image/svg+xml.
5. Das SVG läuft als Dokument im App-Origin.

Die Auswirkung entspricht S-01; der Unterschied ist ein Klick auf einen Link, dessen Text der Angreifer frei wählt.

Behebung

Eine Positivliste statt eines Präfix-Tests — und zusätzlich eine CSP auf der Antwort selbst, wie es ImageProxyController.php:84 bereits vorbildlich macht:

private const array INLINE_TYPES = [
    'image/jpeg', 'image/png', 'image/gif', 'image/webp', 'image/avif', 'image/bmp',
];

$inlineAllowed = in_array(strtolower(trim(explode(';', $contentType)[0])), self::INLINE_TYPES, true);

$response->headers->set('Content-Security-Policy', "default-src 'none'; sandbox");

image/svg+xml fällt damit auf Download zurück, und die CSP entwertet den Pfad selbst dann, wenn die Liste künftig einmal zu großzügig wird. Das Abschneiden der MIME-Parameter (; charset=…) ist nötig, weil image/svg+xml; charset=utf-8 sonst an einem exakten Vergleich vorbeiläuft.

---

🟠 S-03 — Keine Content-Security-Policy auf dem Anwendungsdokument

Ort: anwendungsweit — verifiziert an der Live-Instanz

$ curl -sS -D - -o /dev/null https://mail.vpn.cpy-pst.de/login
HTTP/1.1 200 OK
X-Frame-Options: SAMEORIGIN
X-Content-Type-Options: nosniff
Set-Cookie: PHPSESSID=…; path=/; secure; httponly; samesite=lax

Kein Content-Security-Policy-Header. Der Reading-Frame trägt eine sorgfältig konstruierte CSP, der Image-Proxy ebenfalls — das Hauptdokument, in dem Composer, Einstellungen und Admin laufen, trägt keine.

Das ist der Grund, warum S-01 und S-02 von „XSS in einem contenteditable" zu „vollständige Sitzungsübernahme" eskalieren. Eine CSP mit script-src auf Nonce-Basis hätte beide Befunde zu Fehlfunktionen statt zu Sicherheitslücken gemacht.

Behebung

Ein ResponseListener, der eine Nonce-basierte CSP auf HTML-Antworten setzt. Die Anwendung nutzt AssetMapper/Importmap mit Stimulus, also gibt es Inline-Scripts (u. a. die Importmap selbst) — der Rollout braucht deshalb eine Nonce, die in <script>-Tags durchgereicht wird, und sollte über Content-Security-Policy-Report-Only eingefahren werden, bevor er scharf geschaltet wird.

Realistisches Ziel:

default-src 'self';
script-src 'self' 'nonce-{nonce}';
style-src 'self' 'unsafe-inline';
img-src 'self' data: blob:;
frame-src 'self';
object-src 'none';
base-uri 'none';
form-action 'self';
frame-ancestors 'self';

style-src 'unsafe-inline' ist wegen Tailwind und der Theme-Variablen vorerst schwer vermeidbar und ist nicht der Teil, der hier trägt — script-src ist es.

---

🟡 S-04 — File-Picker-Preview liefert fremde Inhalte inline aus

Ort: src/Controller/Integration/FilePickerController.php:134

[$body, $mime] = null === $preview
    ? [$this->placeholderSvg($fileId), 'image/svg+xml']
    : [$preview->contents, $preview->mime];

$response = new Response($body, Response::HTTP_OK, [
    'Content-Type'           => $mime,
    'X-Content-Type-Options' => 'nosniff',
]);

Hier fehlt jede Content-Disposition — die Antwort ist damit implizit inline — und $mime kommt vom angebundenen Fremdsystem (Nextcloud, Immich, Dropbox, OneDrive, Google Drive). Liefert eines davon text/html oder image/svg+xml zurück, rendert es im App-Origin.

Das ist deutlich schwerer auszunutzen als S-01/S-02: es braucht eine Datei, die der Angreifer in einen für das Opfer sichtbaren Ordner legen kann (geteilter Nextcloud-Ordner, geteiltes Dropbox-Verzeichnis) — ein realistisches Szenario in genau den Umgebungen, für die diese Integrationen gedacht sind.

Der selbst erzeugte Platzhalter-SVG ist unbedenklich, weil er serverseitig gebaut wird.

Behebung: dieselbe Positivliste wie in S-02, plus Content-Security-Policy: default-src 'none'; sandbox auf der Antwort. Ein Preview ist immer ein Bild; alles andere sollte gar nicht erst ausgeliefert werden.

---

🟡 S-05 — Die Anwendung liefert keine Security-Header aus

Eine Suche über das gesamte Repository findet X-Frame-Options, Strict-Transport-Security, Referrer-Policy und Permissions-Policy an keiner Stelle — weder in frankenphp/Caddyfile, noch in config/, noch in einem Event-Listener. Die Header, die die Live-Instanz zeigt, stammen aus dem Reverse-Proxy des Betreibers.

Das ist relevant, weil das README als Installationsweg docker compose up -d und „Open https://localhost" nennt. Eine so aufgesetzte Instanz läuft ohne X-Frame-Options (Clickjacking), ohne HSTS und ohne Referrer-Policy — und der Betreiber hat keinen Anhaltspunkt, dass er das selbst nachrüsten muss.

Behebung: ein header-Block in frankenphp/Caddyfile, damit das mitgelieferte Deployment von sich aus sicher ist:

header {
    Strict-Transport-Security "max-age=31536000; includeSubDomains"
    X-Frame-Options "SAMEORIGIN"
    X-Content-Type-Options "nosniff"
    Referrer-Policy "strict-origin-when-cross-origin"
    Permissions-Policy "geolocation=(), microphone=(), camera=(), interest-cohort=()"
    -Server
}

-Server entfernt nebenbei die Versionskennung FrankenPHP Caddy.

---

🟢 S-06 — Mercure-Default subscribe: '*'

config/packages/mercure.yaml:9 setzt für das vom Bundle erzeugte JWT publish: '*' und subscribe: '*'.

Aktuell nicht ausnutzbar: beide Stellen, die ein Subscriber-Cookie ausstellen, übergeben explizite Topics — MercureCookieSubscriber.php:72 und MercureAuthController.php:42 verwenden je ['mail/user/'.$user->id]. Die Topic-Isolierung greift also.

Der Default ist trotzdem die falsche Vorgabe: ein künftiger Aufruf von createCookie($request) ohne Topic-Argument würde stillschweigend ein Abonnement auf alle Topics ausstellen, also die Mail-Ereignisse aller Nutzer. Das ist ein Fehler, der beim Schreiben unauffällig aussieht und beim Review leicht durchrutscht.

Behebung: subscribe: [] als Default. Die Anwendung publiziert nur, sie abonniert serverseitig nicht — der Publish-Anspruch bleibt, der Subscribe-Anspruch entfällt und muss dann pro Cookie explizit gesetzt werden, was ohnehin bereits geschieht.

Nachtrag — die Isolierung griff nicht. Die Einschätzung „aktuell nicht ausnutzbar" oben ist falsch. Der Topic-Anspruch im Cookie wirkt nur auf private Updates; alle acht Notifier publizierten öffentliche (new Update(...) ohne private: true), und die liefert der Hub an jeden Subscriber aus, der das Topic nennt — mit oder ohne passenden Anspruch. Jede angemeldete Person konnte also mail/user/<fremde id> abonnieren und Sync-Ereignisse, Label-Markup samt CSRF-Tokens und Betreffzeilen fehlgeschlagener Sendungen mitlesen. Behoben mit App\Infrastructure\Mercure\UserUpdate, das jedes Update privat baut; MercureUpdatesArePrivateTest lässt kein new Update( außerhalb davon zu.

---

🟢 S-07 — Signierte Image-Proxy-URLs sind unbegrenzt gültig

src/Service/Mail/ImageProxySigner.php:43 signiert nur die URL, ohne Ablaufzeitpunkt:

return substr(hash_hmac('sha256', $url, $this->secret), 0, self::SIGNATURE_LENGTH);

Die Route ist PUBLIC_ACCESS. Eine einmal geleakte signierte URL (Browser-History, Logs, Referrer eines Dritten) erlaubt dauerhaft, den Server diese eine URL abrufen zu lassen.

Die Auswirkung ist gering, weil die Zieladresse in der Signatur festgeschrieben ist und ImageProxyFetcher bei jedem Abruf sämtliche SSRF-Prüfungen erneut durchführt — es ist kein offener Proxy, sondern ein dauerhaft gültiger Abruf einer festen, öffentlichen HTTPS-Ressource. 128 Bit Signaturlänge sind ausreichend.

Optional: einen exp-Timestamp in die Signatur aufnehmen und mitprüfen. Angesichts der Restriktionen des Fetchers vertretbar, es so zu lassen — dann aber als bewusste Entscheidung dokumentieren, wie es im Rest der Codebase üblich ist.

---

🟢 S-08 — Gleiches Inline-Muster im JMAP-Download

src/Jmap/Controller/JmapDownloadController.php:82 verwendet denselben Präfix-Test wie S-02:

$inlineAllowed = true === str_starts_with($blob->contentType, 'image/');

Praktisch kaum ausnutzbar: die jmap-Firewall ist stateless und verlangt einen Authorization-Header, den eine Top-Level-Navigation im Browser nicht mitsendet — ein direkter Aufruf endet bei 401. Der Befund ist als Konsistenzproblem geführt: dieselbe Regel sollte an beiden Stellen gelten, sonst hängt die Sicherheit an einer Firewall-Eigenschaft, die anderswo dokumentiert ist. Bei der Korrektur von S-02 mitziehen.

Anzumerken: der Kommentar direkt darüber (Zeile 74) behandelt den verwandten Fall — accept wird bewusst ignoriert, damit ein Aufrufer HTML nicht als Bild deklarieren kann. Die Überlegung ist also da; sie greift nur eine Ebene zu kurz, weil der gespeicherte Typ ebenso wenig vertrauenswürdig ist.

---

🟢 S-09 — Trust-on-first-use beim Erststart

src/Service/Setup/InstallGuard.php:27 gibt /install frei, solange kein Nutzer existiert. Wer eine frische Instanz vor dem Betreiber erreicht, wird deren Administrator.

Das ist für selbst gehostete Software ein verbreitetes und weitgehend akzeptiertes Muster, und der 404-statt-Redirect-Ansatz ist richtig gelöst. Erwähnenswert bleibt es, weil das README empfiehlt, hinter einen Reverse-Proxy zu gehen und Port 30080 (TrueNAS) zu exponieren — zwischen Container-Start und abgeschlossener Einrichtung existiert ein reales Fenster.

Optional: ein Einmal-Token aus var/secrets/generated.env, das die Setup-Seite abfragt, oder eine Bindung des Fensters an die ersten N Minuten Uptime. Für den Anwendungsfall vermutlich unverhältnismäßig — aber im Handbuch sollte stehen, dass die Ersteinrichtung ohne Verzögerung erfolgen sollte.

---

ℹ️ S-10 — Kein Rate-Limit auf der JMAP-Authentifizierung

Die main-Firewall hat login_throttling, das 2FA-Formular hat einen eigenen Limiter (TwoFactorThrottle, sehr sauber gelöst), die jmap-Firewall hat keinen.

Ein Brute-Force auf App-Passwörter ist chancenlos — 32 Byte CSPRNG, als SHA-256 gespeichert und per indiziertem Gleichheitsvergleich nachgeschlagen (ApiToken::create(), ApiTokenRepository::findActiveBySecret()). Die Begründung für SHA-256 statt eines Password-Hashers steht im Docblock und ist korrekt.

Was fehlt, ist ein Schutz gegen Ressourcenverbrauch: jeder unauthentifizierte Request löst eine Datenbankabfrage aus. Ein Limiter pro IP auf fehlgeschlagene JMAP-Authentifizierungen wäre eine günstige Ergänzung, kein dringender Befund.

---

4. Was geprüft wurde und sauber ist

Damit die Befundliste im Verhältnis gelesen wird — folgende Bereiche wurden gezielt untersucht, ohne Beanstandung:

Autorisierung. OwnershipVoter behandelt alle zehn nutzergebundenen Entitäten über eine einzige Regel und schlägt konsequent fehl statt durchzulassen: null-Owner wird abgelehnt statt verglichen, nicht authentifizierte Tokens ebenso, und der Vergleich ist Identität statt ID-Gleichheit (was bei unpersistierten Entitäten mit beidseitig null-ID sonst durchginge). Die Begründung, warum die Message über $account und nicht über den Mailbox-Thread-Pfad erreicht wird, ist stichhaltig. Alle Controller-Routen, die eine Entität laden, rufen denyAccessUnlessGranted auf.

Credential-Verschlüsselung. Encryptor nutzt libsodium secretbox (XSalsa20-Poly1305) mit frischer Nonce pro Verschlüsselung, versionsbehaftetem Präfix und #[SensitiveParameter] an den richtigen Stellen. Die Verlagerung der Schlüsselvalidierung aus dem Konstruktor in key() ist begründet und schwächt nichts ab.

SQL-Injection. EmailFilterCompiler erzeugt ausschließlich generierte Parameternamen ('f'.$index) und bindet alle Werte; FreeTextCompiler reduziert Lexeme auf für den tsquery-Parser bedeutungslose Zeichen und übergibt das Ergebnis trotzdem gebunden; likePattern() escaped \, % und _. Die sprintf-Aufrufe in den Repositories interpolieren nur Spaltennamen und Parameternamen, keine Werte.

CSRF. Stateless Tokens sind durchgehend konfiguriert. Die Begründung für die Aufnahme von compose in stateless_token_ids (config/packages/csrf.yaml) ist die beste Dokumentation eines Session-Race-Conditions, die ich in einer Konfigurationsdatei gesehen habe. Admin-POSTs prüfen explizit (SystemActionController.php:77). Die CSRF-Freiheit der öffentlichen Booking-POST ist begründet und durch einen IP-basierten Fixed-Window-Limiter kompensiert.

Webhooks. Alle vier Endpunkte vergleichen ihr Geheimnis mit hash_equals — Gmail Pub/Sub, Graph Mail, Graph Calendar, Google Calendar. Pro Subscription gemintete 256-Bit-clientState/pushSecret.

SSRF. ImageProxyFetcher und IcsFeedClient implementieren beide: HTTPS-only, Port-Pinning, Auflösung und Prüfung aller Adressen gegen private/reservierte Bereiche, Verbindungs-Pinning per resolve gegen DNS-Rebinding, manuelle Redirect-Verfolgung mit Revalidierung jedes Hops, Größen- und Zeitlimits. Fehlerursachen gehen ins Log, nie in die Antwort.

Path Traversal. AppearanceController::showBackground() prüft dreifach ab: Route-Requirement per Regex, Abgleich gegen den auf dem Nutzer gespeicherten Dateinamen, is_file(). Dateinamen werden serverseitig als UUIDv7 vergeben. Über JMAP ist backgroundFile ausdrücklich nicht setzbar (AppearanceSetMethod.php:182).

Session/Cookies. Live verifiziert: secure; httponly; samesite=lax. trusted_proxies ist über Environment konfigurierbar.

Frontend-DOM. Der einzige Ort, an dem Stimulus-Controller Markup aus Daten statt aus Server-Antworten bauen, ist search_controller.js — und dort läuft jeder eingesetzte Wert durch _escape() (Zeilen 666, 704, 705). Auch die Kontakt-Labels, die aus Absendernamen stammen und damit angreiferkontrolliert sind.

Modal-Accessibility. modal_controller.js behandelt inert, Fokus-Rückgabe an das auslösende Element, Escape-Handling und die Race Condition zwischen Fade-out und einem gleichzeitig rendernden Turbo-Frame. Das ist deutlich gründlicher als üblich.

Reading-Frame-Isolierung. Bereits eingangs gewürdigt; ergänzend: dass Warnbanner und die „Bilder anzeigen"-Leiste bewusst außerhalb des Frames liegen, damit eine Mail sie nicht fälschen kann, und dass die Link-Vorschau im Frame erkannt, aber vom Parent gezeichnet wird — das sind die Details, an denen man sieht, dass die Trust-Grenze verstanden und nicht nur gesetzt wurde.

---

5. UX-Befunde

Wie oben angemerkt: ohne Login und ohne Browser-Tooling beruht dieser Abschnitt auf Templates und Controllern. Er erhebt keinen Anspruch, eine Bedienungsprüfung zu ersetzen.

🟡 U-01 — Vier Accessibility-Lücken

Ein Scan aller 200+ Twig-Templates auf fehlende Accessible Names ergab nur vier Treffer — das ist ein sehr gutes Ergebnis, und alle vier sind schnell behoben:

Ort	Problem
compose/_window.html.twig:262	Schließen-Button hat nur title, kein aria-label; Inhalt ist ein <i>-Icon
_partials/_avatar_picker.html.twig:60	Submit-Button mit nur einem Bild als Inhalt, Name nur über title
_partials/_thread_row.html.twig:77	<img> ohne alt
mail/_attachment_chip.html.twig:17	Anhang-Thumbnail <img> ohne alt

title ist kein verlässlicher Ersatz für einen Accessible Name — Screenreader behandeln es uneinheitlich, und auf Touch-Geräten ist es gar nicht erreichbar. Der Schließen-Button des Composers ist davon der wichtigste: das Fenster hat unterhalb md laut Kommentar einen alternativen Ausgang, oberhalb ist dieser Button der einzige.

Für die beiden Bilder genügt alt="", sofern sie rein dekorativ neben einem Textlabel stehen — beim Anhang-Thumbnail ist das der Fall, beim Thread-Row-Avatar vermutlich auch. Explizit leer ist richtig; ganz fehlend lässt Screenreader den Dateinamen vorlesen.

🟢 U-02 — Eingefügtes HTML im Composer wird nicht bereinigt

compose_controller.js:3077 behandelt beim Paste ausschließlich Bilder (event.clipboardData?.files). HTML aus der Zwischenablage geht ungefiltert ins contenteditable.

Das hat zwei Seiten:

• UX: Eingefügter Text aus Word oder einer Webseite bringt dessen komplettes Markup mit — Schriftgrößen, Hintergrundfarben, verschachtelte Tabellen. Das ist der klassische „warum sieht meine Mail so aus"-Effekt, und Empfänger sehen ihn in voller Schönheit.
• Sicherheit: Das eingefügte Markup landet in bodyHtml, wird nicht bereinigt (siehe S-01, Punkt 2) und geht so auf die Leitung. Für den Nutzer selbst ist das Self-XSS und damit unkritisch; dass plMail aber unsanitisiertes HTML versendet, ist unabhängig davon unschön.

Behebung: ein paste-Handler, der text/html abfängt und durch dieselbe Allow-Liste schickt wie serverseitig — oder, pragmatischer und in vielen Clients so gelöst, auf text/plain normalisiert und Rich-Paste über eine explizite Aktion („Mit Formatierung einfügen") anbietet. Der Server-seitige Teil der Korrektur aus S-01 (sanitizeFragment in DraftPersister) deckt das Sicherheitsproblem in jedem Fall ab.

🟢 U-03 — Neun unübersetzte deutsche Strings

Deutsch ist mit 2390 von 2391 Keys praktisch vollständig — bemerkenswert für ein Projekt dieser Größe, und es fehlt kein einziger Key. Neun Werte sind allerdings identisch mit dem englischen Original:

admin.push.fcm.heading
admin.push.fcm.field.google_services
admin.push.fcm.application_id
admin.users.field.is_admin
calendar.share.window_label
compose.schedule.when
settings.accounts.type.oauth
settings.appearance.preview.sender3
settings.filters.summary.in_account

Einige sind legitim identisch (oauth, vermutlich sender3 als Beispieldatensatz). admin.users.field.is_admin, calendar.share.window_label, compose.schedule.when und settings.filters.summary.in_account sind in der deutschen Oberfläche jedoch sichtbare englische Strings.

ℹ️ U-04 — en_PI unvollständig

Der Pirate-Locale fehlen 218 Keys (2173 von 2391). Der Fallback auf Englisch funktioniert, das Ergebnis ist ein Sprachmix. Bei einem Novelty-Locale vermutlich Absicht und kein Handlungsbedarf — hier nur der Vollständigkeit halber vermerkt.

---

6. Empfohlene Reihenfolge

Sofort — die Kette schließen:

1. S-01 — ReplyDraftBuilder::quote() auf sanitizeFragment() umstellen. Einzeiler plus Konstruktor-Injection, behebt den kritischen Befund.
2. S-01, Teil 2 — DraftPersister::markAsDraft() bereinigt zusätzlich bodyHtml. Schließt Paste, manipulierte POSTs und ausgehende Mail mit ab (deckt die Sicherheitshälfte von U-02).
3. S-02 — Positivliste statt Präfix-Test in AttachmentController, plus Content-Security-Policy: default-src 'none'; sandbox auf der Antwort. S-08 und S-04 in derselben Änderung mitziehen.

Kurzfristig — Verteidigung in der Tiefe:

4. S-03 — CSP für das Anwendungsdokument, eingeführt über Report-Only. Das ist die Maßnahme, die die nächste Lücke dieser Art zu einer Fehlfunktion statt zu einem Vorfall macht.
5. S-05 — Header-Block in den Caddyfile, damit das mitgelieferte Deployment nicht auf den Proxy des Betreibers angewiesen ist.
6. U-01 — vier aria-label/alt-Attribute.

Wenn Zeit ist:

7. S-06 — subscribe: [] als Mercure-Default.
8. U-02 — Paste-Normalisierung im Composer (UX-Hälfte).
9. U-03 — neun deutsche Strings.
10. S-07, S-09, S-10 — bewusst entscheiden und dokumentieren; jeweils vertretbar, so zu bleiben.

---

7. Schlussbemerkung

Die Sicherheitsarchitektur dieser Anwendung ist durchdacht, und das ist keine Höflichkeitsfloskel: SSRF-Abwehr, Reading-Frame-Isolierung, Ownership-Autorisierung, 2FA-Throttling und Token-Handling sind jeweils auf einem Niveau, das man in Projekten dieser Größe selten findet. Die Kommentare erklären durchgehend nicht nur was, sondern warum — inklusive der verworfenen Alternativen. Das ist der Grund, warum dieses Review so präzise werden konnte.

Der kritische Befund ist deshalb auch kein Versäumnis im Verständnis, sondern eine Lücke in der Abdeckung: bodyHtml und bodyHtmlSafe sind eine korrekte und klar dokumentierte Trennung, die im Lesepfad konsequent eingehalten wird. ReplyDraftBuilder greift auf das falsche der beiden Felder zu — an einer Stelle, die zwei Ebenen von der Sandbox entfernt liegt und deshalb nicht mehr wie eine Sicherheitsentscheidung aussah.

Genau dagegen hilft das Prinzip, das MessageRender bereits formuliert: „Exists so that no Twig file has to ask a security question." Das gilt für Services genauso. Solange Message::bodyHtml öffentlich lesbar ist und irgendwo ein |raw darauf zeigt, bleibt die Trennung eine Konvention statt einer Garantie. Der nachhaltigste Schritt nach den Sofortmaßnahmen wäre, den Zugriff auf das rohe Feld strukturell auf Sanitizer und Versandpfad zu beschränken.