diff --git a/app/Console/Commands/BindHosts.php b/app/Console/Commands/BindHosts.php index 585d372..15c9fb2 100644 --- a/app/Console/Commands/BindHosts.php +++ b/app/Console/Commands/BindHosts.php @@ -4,6 +4,7 @@ namespace App\Console\Commands; use App\Services\Env\EnvFileEditor; use App\Services\Env\InvalidEnvContentException; +use App\Support\HostnamePattern; use Illuminate\Console\Command; /** @@ -125,9 +126,12 @@ class BindHosts extends Command /** Eine Komma-Liste, in der JEDER Eintrag ein Hostname ist. */ private function isHostList(string $value): bool { + // `explode(',', $value)` liefert nie ein leeres Array — selbst für + // `''` steht `['']` da, und das faengt der `in_array('', …)` gleich + // darunter ab. $names = array_map('trim', explode(',', $value)); - if ($names === [] || in_array('', $names, true)) { + if (in_array('', $names, true)) { return false; } @@ -143,7 +147,9 @@ class BindHosts extends Command /** * Dasselbe Muster, das der root-eigene Helfer in `apply-proxy-hosts` * benutzt, bevor er einen Namen in die Proxy-Konfiguration schreibt - * (deploy/install-agent.sh). + * (deploy/install-agent.sh) — jetzt in HostnamePattern, denn + * PublishTunnelNames braucht dieselbe Prüfung fürs Lesen, ohne diese + * Klasse dafür kennen zu müssen. * * Der Anlass ist konkret: `[www.example.com](https://www.example.com)` ist * als Hostname offensichtlich Unsinn und wurde trotzdem geschrieben, weil @@ -153,10 +159,7 @@ class BindHosts extends Command */ private function isHost(string $name): bool { - return (bool) preg_match( - '/^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)+$/', - $name, - ); + return HostnamePattern::matches($name); } /** diff --git a/app/Console/Commands/PublishTunnelNames.php b/app/Console/Commands/PublishTunnelNames.php index 5b0fbac..8a9836f 100644 --- a/app/Console/Commands/PublishTunnelNames.php +++ b/app/Console/Commands/PublishTunnelNames.php @@ -3,7 +3,9 @@ namespace App\Console\Commands; use App\Services\Dns\HostDnsDirectory; +use App\Support\HostnamePattern; use Illuminate\Console\Command; +use Symfony\Component\Console\Output\ConsoleOutputInterface; /** * Macht Portal, Website und Statusseite im Management-Tunnel auflösbar. @@ -43,12 +45,31 @@ class PublishTunnelNames extends Command return self::SUCCESS; } - $names = array_values(array_unique(array_filter(array_merge( + $candidates = array_values(array_unique(array_filter(array_merge( [(string) config('admin_access.app_host')], array_map('strval', (array) config('admin_access.site_hosts', [])), [(string) config('admin_access.status_host')], )))); + // `bind-hosts` weist einen NEUEN Wert dieser Form inzwischen zurück, + // aendert aber nichts an einem, der schon in der .env steht. Auf der + // Produktivmaschine stand dort bereits ein Markdown-Link aus einem + // Kopiervorgang (`[www.clupilot.com](https://www.clupilot.com)`), von + // VOR dieser Prüfung — geschrieben, weil EnvFileEditor nur prüft, ob + // die ZEILE die Form KEY=value hat. Ungeprüft ginge so ein Wert in + // `platform.hosts`, und der Resolver böge einen Unsinns-Namen um. + $names = []; + + foreach ($candidates as $candidate) { + if (HostnamePattern::matches($candidate)) { + $names[] = $candidate; + + continue; + } + + $this->skipped($candidate); + } + $hub = (string) config('provisioning.wireguard.hub_address'); $dns->writeMany(self::KEY, $names, $hub); @@ -65,4 +86,26 @@ class PublishTunnelNames extends Command return self::SUCCESS; } + + /** + * Nicht still: eine ausgelassene `.env`-Altlast soll auffallen, auch wenn + * niemand die Ausgabe dieses Befehls Zeile für Zeile liest. + * + * Auf STDERR statt über `$this->error()` — das faerbt nur denselben Strom + * rot ein, den auch `$this->line()` benutzt. `deploy/update.sh` ruft + * diesen Befehl mit `2>&1` auf und sieht den Unterschied nicht, aber ein + * Betreiber, der ihn von Hand startet und nur STDOUT protokolliert, soll + * die Meldung trotzdem bekommen. `getOutput()` liefert in einem echten + * CLI-Lauf die `ConsoleOutput`, die einen echten zweiten Strom hat; im + * Testlauf ist es der gepufferte Mock aus PendingCommand, der keiner ist + * — dort faellt das auf den normalen Strom zurück, und Tests sehen die + * Meldung trotzdem. + */ + private function skipped(string $value): void + { + $stream = $this->output->getOutput(); + $target = $stream instanceof ConsoleOutputInterface ? $stream->getErrorOutput() : $stream; + + $target->writeln(" uebersprungen: „{$value}“ ist kein Hostname."); + } } diff --git a/app/Services/Dns/FakeHostDnsDirectory.php b/app/Services/Dns/FakeHostDnsDirectory.php index 639476e..e1f08f2 100644 --- a/app/Services/Dns/FakeHostDnsDirectory.php +++ b/app/Services/Dns/FakeHostDnsDirectory.php @@ -45,7 +45,13 @@ class FakeHostDnsDirectory implements HostDnsDirectory } if ($fqdns === []) { - unset($this->groups[$key], $this->ips[$key]); + // FileHostDnsDirectory::writeMany() ruft bei einer leeren Liste + // remove() auf, und remove() loescht die Datei komplett — nicht + // nur die Zeilen darin, die als IP-Zuordnung durchgingen. Ohne + // $fqdns hier mit zu leeren, wich der Fake vom echten Verhalten + // ab: ein Test haette `$fake->fqdns[$key]` noch stehen sehen, + // waehrend die echte Datei laengst weg war. + unset($this->groups[$key], $this->ips[$key], $this->fqdns[$key]); return; } diff --git a/app/Support/HostnamePattern.php b/app/Support/HostnamePattern.php new file mode 100644 index 0000000..4220c0d --- /dev/null +++ b/app/Support/HostnamePattern.php @@ -0,0 +1,29 @@ +/de docker compose --profile vpn restart vpn-dns vpn-gateway >/dev/null 2>&1 || true fi +# Die Namen leben in der .env, und die .env aendert sich zwischen Deployments, +# ohne dass jemand von Hand einen Befehl nachfaehrt — ein neu gesetzter +# STATUS_HOST, ein per bind-hosts nachgetragener Name, eine frisch installierte +# Maschine, ein neu angelegtes dns-hosts-Volume. Bisher stand der Befehl nur im +# Plan als manueller Einzeiler, und der Aufloeser blieb genau so lange aktuell, +# wie zuletzt jemand daran gedacht hatte — das war: nie automatisch. +# +# Nach dem Cache-Rebuild oben, damit `config('admin_access…')` die eben +# geschriebenen Werte liest und nicht den Stand von vorher. Vor der +# Bereitschaftspruefung, damit ein frisch gebundener Name schon aufloest, wenn +# `reconcile_vpn_readiness` gleich danach den Tunnel als bereit meldet. +# +# `|| true`, wie bei den Nachbaraufrufen in dieser Funktion: ein Deployment darf +# daran nicht scheitern, und ein Fehlschlag hier heisst hoechstens, dass die +# Namen bis zum naechsten Lauf stehen bleiben, wie sie waren. +in_app php artisan clupilot:publish-tunnel-names >/dev/null 2>&1 || true + reconcile_vpn_readiness # ── Host packages the update can install itself ────────────────────────────── diff --git a/docker/caddy/vpn-entrypoint.sh b/docker/caddy/vpn-entrypoint.sh index 7531c2d..271b8ea 100755 --- a/docker/caddy/vpn-entrypoint.sh +++ b/docker/caddy/vpn-entrypoint.sh @@ -18,6 +18,9 @@ HEALTH="${VPN_HEALTH_PORT:-8081}" CERT_DIR="${VPN_CERT_DIR:-/certs}" OUT="${VPN_CONFIG_OUT:-/tmp/vpn.Caddyfile}" +# Die Namen, fuer die schon ein Site-Block geschrieben wurde — siehe emit_site(). +EMITTED="" + # Die Zertifikate, die dieser Lauf wirklich geladen hat. Der Update-Agent # ueberwacht sie und startet den Gateway nach einer Erneuerung neu — Caddys # `tls` liest die Datei EINMAL beim Start, und ohne Neustart liefe der Tunnel @@ -37,6 +40,21 @@ CERT_LIST="${VPN_CERT_LIST:-/tmp/vpn-certs.list}" # den der Gateway keinen Zweck hat. emit_site() { name="$1" + + # Schon bedient? Caddy lehnt eine doppelte Site-Definition nicht bloss ab — + # es startet dann ueberhaupt nicht („ambiguous site definition"), der + # Container laeuft in eine Neustartschleife, und mit ihm ist die Konsole aus + # dem Tunnel verschwunden. Ein doppelter Name ist kein exotischer Fall: + # APP_HOST auch in SITE_HOST, SITE_HOST zweimal derselbe Name, oder einer + # davon gleich dem Konsolennamen — alles Tippfehler, die in einer .env + # vorkommen. + case " $EMITTED " in + *" $name "*) + echo " uebersprungen: $name — bereits als Site-Block geschrieben" >&2 + return 0 + ;; + esac + # `-print -quit`, nicht `| head -1`: eine Pipe, aus der head aussteigt, # waehrend find noch schreibt, liefert SIGPIPE — install-agent.sh hat sich # daran schon einmal selbst beendet. `|| true`, damit ein leeres Ergebnis @@ -62,6 +80,7 @@ emit_site() { } >> "$OUT" echo "$crt" >> "$CERT_LIST" + EMITTED="$EMITTED $name" } # `if`, nicht `[ … ] && …`. Bei leerem Wert gibt die AND-OR-Liste 1 zurueck, und @@ -75,10 +94,17 @@ fi # Portal, Website und Statusseite. files. steht hier nie drin: dort holt ein # Server im Rettungssystem sein Archiv, und der ist nicht im Tunnel. -echo "${VPN_TUNNEL_HOSTS:-}" | tr ',' '\n' | while read -r host; do +# +# Als Here-Dokument, nicht als Pipe: eine Pipe haengt die Schleife in eine +# Subshell, und EMITTED — dort veraendert — waere nach der Zeile wieder leer. +# `read -r` mit der Standard-IFS trimmt umgebende Leerzeichen weiterhin, worauf +# Eintraege wie " www.x" sich verlassen. +while read -r host; do [ -n "$host" ] || continue emit_site "$host" -done +done <toBe("10.66.0.1 a.test\n10.66.0.1 b.test\n"); }); +it('überspringt einen Wert, der schon vor der bind-hosts-Prüfung in der .env stand', function () { + // `bind-hosts` weist einen NEUEN Wert dieser Form inzwischen zurück — aber + // auf einer Produktivmaschine stand genau das schon in der .env, von VOR + // dieser Prüfung. Ungeprüft ginge der Unsinn hier in platform.hosts, und + // der Resolver böge einen Namen um, den keine Anfrage je trifft. + config()->set('admin_access.app_host', 'app.clupilot.test'); + config()->set('admin_access.site_hosts', ['[www.clupilot.test](https://www.clupilot.test)']); + + $this->artisan('clupilot:publish-tunnel-names') + ->expectsOutputToContain('www.clupilot.test') + ->assertSuccessful(); + + $written = file_get_contents($this->dir.'/platform.hosts'); + + expect($written)->toContain('10.66.0.1 app.clupilot.test') + ->and($written)->not->toContain('[www.clupilot.test]'); +}); + it('ist dieselbe Datei-Umsetzung wie für die Host-Namen', function () { // Nicht Fake gegen Fake geprüft: die Rechte und das Anlegen des // Verzeichnisses sind der Teil, der in Produktion schiefgeht. + // + // Kein ->skip() hier: die Bedingung `! app()->environment('testing')` kann + // in einem Testlauf nie zutreffen — der Test lief seit seiner Einführung + // also immer, und ein Skip, der nie greift, ist bloss eine Behauptung. expect(app(HostDnsDirectory::class))->toBeInstanceOf(FileHostDnsDirectory::class); -})->skip(fn () => ! app()->environment('testing'), 'nur im Testlauf'); +}); diff --git a/tests/Feature/VpnGatewayConfigTest.php b/tests/Feature/VpnGatewayConfigTest.php index 17d3926..8b86940 100644 --- a/tests/Feature/VpnGatewayConfigTest.php +++ b/tests/Feature/VpnGatewayConfigTest.php @@ -170,6 +170,25 @@ it('schreibt die geladenen Zertifikate mit, damit die Erneuerung greift', functi ->and($written)->not->toContain('www.clupilot.test'); }); +it('schreibt einen doppelt genannten Namen nur als EINEN Site-Block', function () { + // Der Anlass fuer diesen Test: Caddy lehnt eine doppelte Site-Definition + // nicht bloss ab, es startet dann UEBERHAUPT NICHT + // ("ambiguous site definition: https://…:443") — der Container laeuft in + // eine Neustartschleife, und mit ihm ist die Konsole aus dem Tunnel weg. + // Ein Tippfehler in der .env reicht: APP_HOST auch in SITE_HOST gelistet, + // SITE_HOST mit demselben Namen zweimal, oder einer davon gleich dem + // Konsolennamen. + $config = renderVpnConfig([ + // admin.clupilot.test ist zugleich der Konsolenname UND in + // VPN_TUNNEL_HOSTS gelistet. + 'VPN_INTERNAL_HOST' => 'admin.clupilot.test', + 'VPN_TUNNEL_HOSTS' => 'admin.clupilot.test,app.clupilot.test,app.clupilot.test', + ], ['admin.clupilot.test', 'app.clupilot.test']); + + expect(substr_count($config, 'https://admin.clupilot.test:443 {'))->toBe(1) + ->and(substr_count($config, 'https://app.clupilot.test:443 {'))->toBe(1); +}); + it('gibt jedem Block die Weiterleitung mit der echten Quelladresse', function () { // Ohne X-Forwarded-For sähe die Anwendung den Gateway statt des Anrufers, // und die Freigabeliste prüfte die falsche Adresse.