Einstellungen: ACME-Kontaktadresse konfigurierbar (+ Wurzelfix am Token-Guard) - #232
Merged
Conversation
Express runs with strict:false/caseSensitive:false, so `/acme-email/` and `/ACME-EMAIL` reach the same route while the anchored TOKEN_FORBIDDEN regexes only matched the exact spelling. Every session-only settings route was writable with a plain API token (and token requests skip CSRF). Lower-casing and stripping the trailing slash fixes all entries at once.
Review-Befunde: Feld hatte ohne .env-Placeholder keinen zugänglichen Namen (aria-label ergänzt, gleicher Text wie Kartenüberschrift, kein neuer i18n-Key). Fallback t['settings.acme_email.push_failed'] war tot (beide Seiten derselbe Schlüssel) und hätte bei einer künftigen zweiten Warnung fälschlich den push_failed-Text gezeigt statt der generischen Autosave-Fehlermeldung.
…ted local part tests/validate_email.test.js hielt ein NUL-Byte und einen vertikalen Tabulator als rohe Bytes statt als Escape-Sequenzen -> Git stufte die Datei als binaer ein, PR-Diff blieb unsichtbar, git grep uebersprang die Datei. Ersetzt durch \0 / \v, gleicher geprüfter String. validateEmail prüfte bisher nur die Domain auf führenden/abschließenden Punkt und Doppelpunkt, nicht den Local Part vor dem @. '.me@example.com', 'me.@example.com' und 'me..you@example.com' galten faelschlich als gueltig, obwohl RFC 5322 und Let's Encrypt sie ablehnen. Ergaenzt um dieselbe Pruefung im Local Part, im Stil der bestehenden Domain-Pruefung (kein Regex).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Die Kontaktadresse für Let's Encrypt lässt sich in Einstellungen → Allgemein setzen und wirkt sofort, ohne Container-Neustart. Bisher war sie nur über
GC_CADDY_EMAILin der.envkonfigurierbar.Warum das mehr ist als Komfort
Ablaufwarnungen von Let's Encrypt sind der offensichtliche Nutzen — der eigentliche steckt tiefer.
buildTlsAutomationsteigt bei leerer Adresse komplett aus (caddyTlsAutomation.js:30). Damit verschwindet nicht nur der ACME-Kontakt, sondern der ganze TLS-Automation-Block, inklusive der Regeln, die interne Domains auf den internen Aussteller lenken. Genau daraus entstanden die dauerhaften ACME-Fehlversuche fürgc-owner.invalid(PR #230 hat dort das Symptom behandelt).Die leere Adresse ist also kein neutraler Zustand — sie schaltet still Verhalten ab, und sie ist der ausgelieferte Standard.
Umgesetzt
effectiveAcmeEmail()incaddyConfig.js: gesetzte Einstellung schlägt die.env, leeres Feld fällt darauf zurück.buildTlsAutomationbleibt unverändert und seiteneffektfrei — aufgelöst wird am Aufrufpunkt.PUT /api/v1/settings/acme-emailim bestehenden Settings-Router. Ein fehlendes Feld ist ein Client-Fehler, kein Löschbefehl — sonst löschte ein abgeschnittener Request still eine gültige Konfiguration. Der Push gilt als gescheitert, wennsyncToCaddywirft oderfalseliefert (Ownership-Guard); der Wert bleibt dann gespeichert und die Oberfläche warnt.validateEmailinsrc/utils/validate.js, der die modulprivate Kopie in den Skoda-Konten ablöst. Streng, weil eine von Let's Encrypt abgewiesene Adresse Ausstellung und Erneuerung für alle Domains blockiert..env-Wert hin, ohne ihn zu zeigen —config.caddy.emailsteht in keiner API-Antwort, Klartext wäre eine neue Preisgabe.Enthalten: ein Wurzelfix am Token-Guard (Security)
Beim Absichern der neuen Route fiel auf, dass
TOKEN_FORBIDDENumgehbar war: die Muster sind exakt verankert und werden gegenreq.pathgeprüft, Express läuft aber mitstrict: false/caseSensitive: false.Vorbestehend und systemisch, betraf alle 15 gesperrten Routen. Behoben wurde die Ursache im Guard (Pfadnormalisierung), nicht die eine Route. Der Reviewer hat 18 Umgehungsvarianten über rohe TCP-Sockets geprüft — superagent normalisiert clientseitig und hätte einen falschen Freibrief ausgestellt.
Wechselwirkung mit #231
Der Branch wurde auf den neuen
masterrebased. PR #231 fasst dieselbe Stelle an (Verwaltungs-Host in der TLS-Automation); der Konflikt wurde so aufgelöst, dass die Domainliste von #231 und die Adressauflösung dieses Branches beide bestehen bleiben. Der Abschlussreviewer hat alle vier Kombinationen durchgespielt: in der DB-Variante wird #231s Fix dadurch überhaupt erst wirksam, weil eine leere.envsonst die gesamte TLS-Automation gekippt hätte.Prüfung
Volle Suite 2331 bestanden, 0 fehlgeschlagen. Spec und Plan durchliefen je einen Preflight (vier bzw. zwei unabhängige Prüfer), jede der sechs Tasks einen eigenen Review, dazu ein Abschlussreview über den ganzen Branch — Urteil merge-reif.
Was die Reviews gefangen haben, bevor es live ging: ein
ReferenceErrorim Auflöser (settingsist incaddyConfig.jsnicht auf Modulebene importiert), eine Push-Warnung, die der Autosave-Helfer sofort mit „Gespeichert" überschrieben hätte, und ein NUL-Byte in der Testdatei, das sie für Git zur Binärdatei machte — ausgerechnet den Test, der die Vertrauensgrenze absichert, hätte niemand im Diff lesen können.Nebenbei behoben
Die Autosave-Bestätigung zeigte auch in der deutschen Oberfläche „Saved" — der Übersetzungsschlüssel fehlte in der Client-Whitelist aller drei Designs.
Nicht enthalten
Eine Rollenprüfung für
/api/v1/settings/*— heute schützt dort nurrequireAuth, und das betrifft alle Settings-Routen gleichermaßen. Ein einzelner Guard auf einer von zwanzig Routen würde Schutz vortäuschen, den die Nachbarn nicht haben. Eigener Vorgang.