diff --git a/app/Mail/Transport/MailboxTransport.php b/app/Mail/Transport/MailboxTransport.php index ba28ea9..479f861 100644 --- a/app/Mail/Transport/MailboxTransport.php +++ b/app/Mail/Transport/MailboxTransport.php @@ -40,8 +40,14 @@ class MailboxTransport implements TransportInterface * not the exact string 'log'. `null` is here because a stored setting can * decode to it just as easily as to a real value (see the port note in * delegate()): an unconfigured mailer must fail SAFE, not fail open. + * + * Public: this is the one place that knows what "does not really send" + * means, and App\Support\Readiness\DeliveryChecks reads it directly + * rather than carrying a second, narrower copy of the same rule (a + * `!== 'log'` check once passed under this suite's own MAIL_MAILER=array, + * which delegate() below has always treated as just as inert as 'log'). */ - private const NON_DELIVERING = ['log', 'array', null]; + public const NON_DELIVERING = ['log', 'array', null]; private ?TransportInterface $delegate = null; diff --git a/app/Support/Readiness/DeliveryChecks.php b/app/Support/Readiness/DeliveryChecks.php index 0728e1e..530c2ff 100644 --- a/app/Support/Readiness/DeliveryChecks.php +++ b/app/Support/Readiness/DeliveryChecks.php @@ -2,6 +2,7 @@ namespace App\Support\Readiness; +use App\Mail\Transport\MailboxTransport; use App\Models\Mailbox; use App\Models\MailTemplate; use App\Services\Secrets\SecretVault; @@ -26,7 +27,12 @@ final class DeliveryChecks label: __('readiness.delivery.mailer_not_log'), breaks: __('readiness.delivery.mailer_not_log_breaks'), tab: 'mail', - satisfied: config('mail.default') !== 'log', + // Reads the transport's own definition of "does not really + // send" rather than a second, narrower one. `!== 'log'` alone + // called 'array' — the MAIL_MAILER this whole suite runs + // under — delivered, which MailboxTransport::delegate() has + // never agreed with. + satisfied: ! in_array(config('mail.default'), MailboxTransport::NON_DELIVERING, true), ), new Check( key: 'delivery.mailbox', diff --git a/app/Support/Readiness/ProvisioningChecks.php b/app/Support/Readiness/ProvisioningChecks.php index 13700d5..2a3b841 100644 --- a/app/Support/Readiness/ProvisioningChecks.php +++ b/app/Support/Readiness/ProvisioningChecks.php @@ -81,14 +81,19 @@ final class ProvisioningChecks new Check( key: 'provisioning.monitoring_token', group: self::GROUP, - severity: Check::SEVERITY_WARNING, + // Follows the same condition RegisterMonitoring itself reads + // (RegisterMonitoring.php: `if (config('provisioning. + // monitoring.required', false)) { throw $e; }`). Warning by + // default — monitoring degrades, then the run continues — but + // an operator who has switched that setting on made + // monitoring a real gate, and a fixed severity would then + // claim a looser rule than the pipeline actually enforces. + severity: config('provisioning.monitoring.required', false) + ? Check::SEVERITY_BLOCKING + : Check::SEVERITY_WARNING, label: __('readiness.provisioning.monitoring_token'), breaks: __('readiness.provisioning.monitoring_token_breaks'), tab: 'integrations', - // Warning, never blocking: RegisterMonitoring itself already - // degrades (retries, then continues) rather than fail the run - // — this page must not claim a stricter rule than the pipeline - // actually enforces. satisfied: filled(app(SecretVault::class)->get('monitoring.token')), ), ]; diff --git a/lang/de/readiness.php b/lang/de/readiness.php index ae5cee6..b3fb0dc 100644 --- a/lang/de/readiness.php +++ b/lang/de/readiness.php @@ -41,7 +41,7 @@ return [ 'dns_token' => 'Hetzner-DNS-Token', 'dns_token_breaks' => 'Ohne ihn scheitert ConfigureDnsAndTls am A-Eintrag für die neue Instanz — mitten in der Bereitstellung, nachdem die Maschine schon läuft und der Kunde schon bezahlt hat.', 'dns_zone' => 'DNS-Zone', - 'dns_zone_breaks' => 'Jede Kunden-Subdomain, der Link in der Fertig-Mail und die Überwachungs-URL entstehen, indem diese Zone an einen Hostnamen gehängt wird. Leer bedeutet: jede neue Instanz bekommt einen Namen, der auf nichts zeigt.', + 'dns_zone_breaks' => 'Leer, und HttpHetznerDnsClient sucht bei Hetzner nach einer Zone ohne Namen: zoneId() findet keine, wirft „DNS zone not found", und ConfigureDnsAndTls scheitert damit an genau der Stelle, an der auch der fehlende DNS-Token zuschlägt — mitten in der Bereitstellung, nachdem der Kunde schon bezahlt hat.', 'traefik_path' => 'Traefik-Konfigurationspfad', 'traefik_path_breaks' => 'Leer, und SshTraefikWriter schreibt die Routendatei an die Wurzel des Dateisystems statt in Traefiks eigenes Verzeichnis. Der Schritt meldet trotzdem Erfolg — Traefik sieht die neue Route nur nie, und die Cloud des Kunden bleibt über ihre Adresse unerreichbar.', 'usable_host' => 'Mindestens ein einsetzbarer Host', @@ -49,16 +49,16 @@ return [ 'vm_template' => 'VM-Vorlage je veröffentlichter Version', 'vm_template_breaks' => 'Eine veröffentlichte Version ohne template_vmid lässt CloneVirtualMachine sofort mit „template_missing" scheitern — für jede Bestellung dieses Plans, nachdem der Kunde schon bezahlt hat.', 'monitoring_token' => 'Überwachungs-Token', - 'monitoring_token_breaks' => 'Ohne ihn scheitert RegisterMonitoring; die Cloud wird trotzdem ausgeliefert, nur ohne Überwachung, bis der Schlüssel nachgetragen wird. Überwachung darf eine Bereitstellung nie aufhalten.', + 'monitoring_token_breaks' => 'Ohne ihn scheitert RegisterMonitoring. Ist `monitoring.required` nicht gesetzt (die Vorgabe), liefert die Bereitstellung trotzdem aus — nur ohne Überwachung, bis der Schlüssel nachgetragen wird. Ist es gesetzt, bricht der Lauf stattdessen ab, und die Bereitstellung hält an, bis der Schlüssel da ist.', ], 'delivery' => [ - 'mailer_not_log' => 'Mailtransport ist kein Logfile', - 'mailer_not_log_breaks' => 'Bei mail.default=log läuft die Maschine, der Beleg wird ausgestellt, und die Mail, die dem Kunden seine fertige Cloud ankündigt, landet in einer Datei auf dem Server. Der Kunde erfährt nie, dass er bezahlt hat und etwas läuft.', + 'mailer_not_log' => 'Mailtransport stellt tatsächlich zu', + 'mailer_not_log_breaks' => 'Bei mail.default=log, =array oder ganz ohne gesetzten Standard läuft die Maschine, der Beleg wird ausgestellt, und die Mail, die dem Kunden seine fertige Cloud ankündigt, verlässt den Server nie — sie landet in einer Datei oder verschwindet spurlos. Der Kunde erfährt nie, dass er bezahlt hat und etwas läuft.', 'mailbox' => 'Mindestens eine Mailbox', 'mailbox_breaks' => 'Ohne Mailbox wirft der Versand aus CompleteProvisioning eine Ausnahme — der letzte Schritt der Bereitstellung schlägt fehl und wiederholt sich, obwohl die Maschine längst läuft und der Kunde schon bezahlt hat.', 'mail_templates' => 'Mindestens eine Antwortvorlage', - 'mail_templates_breaks' => 'Ohne Vorlage tippt jeder Support-Mitarbeiter dieselbe Antwort neu — und zwei Kunden bekommen zwei verschiedene Auskünfte zur selben Frage.', + 'mail_templates_breaks' => 'Auf der Kundenseite bleibt die Vorlagen-Auswahl leer, wo sie sonst anklickbare Antworten anbietet — der Support-Mitarbeiter merkt es sofort beim Öffnen, findet aber nichts zum Einfügen und tippt jede Antwort neu. Zwei Kunden mit derselben Frage bekommen dadurch leicht zwei unterschiedliche Auskünfte, ohne dass irgendwo ein Fehler erscheint.', 'inbound_password' => 'Passwort für eingehende Mail', 'inbound_password_breaks' => 'Ohne es liefert der Mail-Abruf jedes Mal eine leere Liste. Der Zeitplan läuft unauffällig weiter, aber keine Kundenantwort erreicht je die Inbox der Konsole, und nirgends erscheint ein Fehler.', ], diff --git a/lang/en/readiness.php b/lang/en/readiness.php index 84b691b..ebd716a 100644 --- a/lang/en/readiness.php +++ b/lang/en/readiness.php @@ -41,7 +41,7 @@ return [ 'dns_token' => 'Hetzner DNS token', 'dns_token_breaks' => 'Without it, ConfigureDnsAndTls fails to create the A record for the new instance — in the middle of provisioning, after the machine is already running and the customer has already paid.', 'dns_zone' => 'DNS zone', - 'dns_zone_breaks' => 'Every customer subdomain, the link in the ready mail, and the monitoring URL are built by appending this zone to a hostname. Empty means every new instance gets a name that points at nothing.', + 'dns_zone_breaks' => 'Empty, and HttpHetznerDnsClient asks Hetzner for a zone with no name: zoneId() finds none, throws "DNS zone not found", and ConfigureDnsAndTls fails at exactly the point the missing DNS token already breaks — in the middle of provisioning, after the customer has already paid.', 'traefik_path' => 'Traefik config path', 'traefik_path_breaks' => 'Empty, and SshTraefikWriter writes the route file at the filesystem root instead of Traefik\'s own directory. The step still reports success — Traefik simply never sees the new route, and the customer\'s cloud stays unreachable at its address.', 'usable_host' => 'At least one usable host', @@ -49,16 +49,16 @@ return [ 'vm_template' => 'VM template for every published version', 'vm_template_breaks' => 'A published version with no template_vmid makes CloneVirtualMachine fail instantly with "template_missing" — for every order of that plan, after the customer has already paid.', 'monitoring_token' => 'Monitoring token', - 'monitoring_token_breaks' => 'Without it, RegisterMonitoring fails; the cloud still gets delivered, just without monitoring until the key is added. Monitoring must never hold up a delivery.', + 'monitoring_token_breaks' => 'Without it, RegisterMonitoring fails. With `monitoring.required` unset (the default), the cloud still gets delivered, just without monitoring, until the key is added. With it set, the run aborts instead, and delivery stalls until the key is there.', ], 'delivery' => [ - 'mailer_not_log' => 'Mail transport is not a log file', - 'mailer_not_log_breaks' => 'With mail.default=log the machine runs, the invoice is issued, and the mail announcing the finished cloud lands in a file on the server. The customer never learns they paid for something that is running.', + 'mailer_not_log' => 'Mail transport actually delivers', + 'mailer_not_log_breaks' => 'With mail.default=log, =array, or no default configured at all, the machine runs, the invoice is issued, and the mail announcing the finished cloud never leaves the server — it lands in a file or vanishes without a trace. The customer never learns they paid for something that is running.', 'mailbox' => 'At least one mailbox', 'mailbox_breaks' => 'With no mailbox, sending from CompleteProvisioning throws — the last step of provisioning fails and keeps retrying, even though the machine is already running and the customer has already paid.', 'mail_templates' => 'At least one reply template', - 'mail_templates_breaks' => 'Without a template, every support reply is typed from scratch — and two customers get two different answers to the same question.', + 'mail_templates_breaks' => 'On the customer page, the template picker sits empty where it would otherwise offer ready answers to click — the support agent notices immediately on opening it, finds nothing to insert, and types every reply from scratch. Two customers asking the same question can then easily get two different answers, with no error appearing anywhere.', 'inbound_password' => 'Inbound mail password', 'inbound_password_breaks' => 'Without it, fetching mail returns an empty list every time. The schedule keeps running quietly, but no customer reply ever reaches the console inbox, and no error appears anywhere.', ], diff --git a/tests/Feature/Readiness/DeliveryChecksTest.php b/tests/Feature/Readiness/DeliveryChecksTest.php index 639d83f..08dcea2 100644 --- a/tests/Feature/Readiness/DeliveryChecksTest.php +++ b/tests/Feature/Readiness/DeliveryChecksTest.php @@ -25,6 +25,26 @@ it('refuses to call an installation ready while mail goes to the log file', func expect(deliveryCheck('delivery.mailer_not_log')->satisfied)->toBeFalse(); }); +/** + * Fix-Runde (Befund 1): `!== 'log'` allein ist zu eng. MailboxTransport:: + * NON_DELIVERING kennt bereits die vollständige Liste — 'array' (der + * MAIL_MAILER, den phpunit.xml der GANZEN Suite aufzwingt) und der + * unkonfigurierte Fall (null) liefern genauso wenig zu wie 'log'. Eine + * Prüfung, die nur 'log' kennt, war unter dem eigenen Testlauf grün, obwohl + * hier nie eine Mail hinausginge — genau das Grün-aus-falschem-Grund, gegen + * das diese Seite gebaut ist. Jeder Wert der Liste wird hier durchgegangen, + * nicht nur der eine, den der ursprüngliche Vorfall zeigte. + */ +it('does not accept any value MailboxTransport itself treats as non-delivering', function (?string $default) { + config()->set('mail.default', $default); + + expect(deliveryCheck('delivery.mailer_not_log')->satisfied)->toBeFalse(); +})->with([ + 'log' => ['log'], + 'array (the suite\'s own MAIL_MAILER)' => ['array'], + 'unset' => [null], +]); + it('accepts a real mailer', function () { config()->set('mail.default', 'smtp'); diff --git a/tests/Feature/Readiness/ProvisioningChecksTest.php b/tests/Feature/Readiness/ProvisioningChecksTest.php index cd58f97..dfefc93 100644 --- a/tests/Feature/Readiness/ProvisioningChecksTest.php +++ b/tests/Feature/Readiness/ProvisioningChecksTest.php @@ -39,12 +39,31 @@ it('does not accept a host that carries a token but is not active', function () expect(provisioningCheck('provisioning.usable_host')->satisfied)->toBeFalse(); }); -it('treats the monitoring token as a warning, never as a blocker', function () { - // Überwachung darf eine Bereitstellung nie aufhalten. +it('treats the monitoring token as a warning while monitoring is not required', function () { + // Überwachung darf eine Bereitstellung nie aufhalten — solange der + // Betreiber sie nicht selbst zur Pflicht gemacht hat. Explizit gesetzt, + // statt sich auf die Vorgabe dieser Installation zu verlassen. + config()->set('provisioning.monitoring.required', false); + expect(provisioningCheck('provisioning.monitoring_token')->severity) ->toBe(Check::SEVERITY_WARNING); }); +/** + * Fix-Runde (Befund 2): RegisterMonitoring wirft die Ausnahme weiter, statt + * zu degradieren, sobald `provisioning.monitoring.required` auf true steht + * (RegisterMonitoring.php: `if (config('provisioning.monitoring.required', + * false)) { throw $e; }`). Ein starrer Schweregrad hätte in genau diesem + * Fall etwas Falsches behauptet — die Seite müsste eine Sperre melden, die + * es unter dieser Einstellung tatsächlich gibt. + */ +it('treats the monitoring token as a blocker once the operator makes monitoring required', function () { + config()->set('provisioning.monitoring.required', true); + + expect(provisioningCheck('provisioning.monitoring_token')->severity) + ->toBe(Check::SEVERITY_BLOCKING); +}); + it('needs a dns token', function () { OperatingMode::set(OperatingMode::Live);