Fix-Welle Ganz-Branch-Review: weltlesbare .env-Sicherung, blinder Test, drei Installer-Texte

Fuenf Befunde aus der abschliessenden Review, in einer Runde behoben:

- EnvFileEditor::backup() liess PHPs copy() die Umask entscheiden statt den
  Modus der Quelle zu uebernehmen — .env stand mit deploy/install.sh auf 0600,
  jede Sicherung landete trotzdem weltlesbar bei 0644, mit APP_KEY,
  DB_PASSWORD, VPN_CONFIG_KEY und STRIPE_SECRET darin. Reproduziert (per
  kurzzeitigem git stash des Fixes: 420 statt 384) und jetzt durch einen
  expliziten chmod nach dem Kopieren sowie einen neuen Test verhindert.

- HostSeparationTest pruefte "jede Route hat einen Hostnamen" nur scheinbar
  allgemein — deploy/install.sh schreibt ADMIN_HOST_EXCLUSIVE=false als
  Vorgabe, und im nicht-exklusiven Fallback registriert routes/web.php jede
  /admin/*-Route absichtlich ohne Domain. Der Test setzte zwar
  ADMIN_HOST_EXCLUSIVE=true, sagte aber nirgends, dass genau das die
  Voraussetzung der Pruefung ist. hostSeparationTable() gibt AdminArea::
  isExclusive() jetzt als Out-Parameter zurueck, gelesen waehrend die zweite
  Anwendung noch gebootet ist, und der Test besteht darauf.

- clupilot:bind-hosts existierte, aber nichts sagte einem Operator, dass es
  ihn braucht. deploy/update.sh druckt jetzt einen eigenen Hinweis, wenn
  APP_HOST in .env leer ist — mit der vollen docker-compose-Zeile statt der
  internen in_app-Abkuerzung, weil der Operator sie in seiner eigenen Shell
  eintippt.

- ask STATUS_DOMAIN und ask FILES_DOMAIN versprachen "blank to keep it auf
  jedem Host", fuellten Enter aber ueber den dritten ask()-Parameter mit dem
  Default. Fuer FILES_DOMAIN war das kein Schoenheitsfehler: der Default
  verschiebt /bootstrap.tar.gz vom Portal weg, bevor DNS fuer den neuen Namen
  existiert. Beide Defaults entfernt.

- ask WWW_DOMAIN erklaerte nicht, dass SITE_HOST mehrere kommagetrennte Namen
  traegt (der erste kanonisch, der Rest leitet dauerhaft um) — ein Operator,
  der die Apex-Domain zusaetzlich zu www. binden wollte, hatte keinen Weg,
  das aus dem Prompt zu erfahren. Nur der Prompt-Text geaendert, kein neuer
  Prompt, Default unveraendert.

Voller Testlauf: 2953 passed (Baseline 2952 + der neue Backup-Berechtigungs-
Test), 0 failed. routes/web.php, RestrictAdminHost, config/fortify.php und
PublicSiteGate unangetastet.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feat/versandtakt
nexxo 2026-08-04 15:51:35 +02:00
parent 432ecfbf4d
commit 8b631acb72
5 changed files with 89 additions and 5 deletions

View File

@ -130,6 +130,17 @@ final class EnvFileEditor
throw new \RuntimeException("Could not back up [{$this->path()}] before writing it.");
}
// .env trägt jedes Zugangsdatum dieser Installation — APP_KEY,
// DB_PASSWORD, VPN_CONFIG_KEY, STRIPE_SECRET. File::copy() ruft PHPs
// copy() auf, und das legt die Kopie mit der Prozess-Umask an statt mit
// dem Modus der Quelldatei: deploy/install.sh:347 setzt .env auf 0600,
// die Kopie landete mit einer üblichen Umask trotzdem bei 0644 —
// weltlesbar direkt neben dem Original. Der Modus wird deshalb explizit
// vom Original übernommen statt der Umask überlassen, und das direkt
// nach dem Kopieren, damit die Kopie so kurz wie möglich mit offenen
// Rechten daliegt.
File::chmod($target, fileperms($this->path()) & 0777);
return $target;
}

View File

@ -109,14 +109,21 @@ log "CluPilot installer on ${PRETTY_NAME}"
# ── 1. Answers we need before doing anything ─────────────────────────────────
ask APP_DOMAIN "Public domain for the customer portal" "app.clupilot.com"
ask WWW_DOMAIN "Domain for the marketing site" "www.clupilot.com"
ask WWW_DOMAIN "Domain for the marketing site (comma-separated for several, first is canonical, e.g. clupilot.com,www.clupilot.com)" "www.clupilot.com"
ask ADMIN_DOMAIN "Private domain for the operator console" "admin.clupilot.com"
ask WS_DOMAIN "Domain for the websocket server" "ws.clupilot.com"
# Stripe posts server-to-server and never sees the portal, so the webhook has
# its own name. Naming the portal here was wrong in the printed instructions.
ask API_DOMAIN "Domain Stripe posts its webhooks to" "api.clupilot.com"
ask STATUS_DOMAIN "Public status page (blank to keep it on every host)" "status.clupilot.com"
ask FILES_DOMAIN "Domain for downloads: terms, DPA, bootstrap archive (blank to keep them on the portal)" "files.clupilot.com"
# No third argument (default) on either of these two: ask() fills a blank
# answer with the default via ${answer:-$default}, so a default here would make
# the "blank to keep it on …" hint in the prompt a lie — pressing Enter would
# never actually leave it blank. For FILES_DOMAIN this was not cosmetic: a
# default here moves /bootstrap.tar.gz off the portal onto a name whose DNS
# does not exist yet, and a server in rescue mode then cannot fetch its own
# archive.
ask STATUS_DOMAIN "Public status page (blank to keep it on every host)"
ask FILES_DOMAIN "Domain for downloads: terms, DPA, bootstrap archive (blank to keep them on the portal)"
ask ADMIN_EMAIL "Your login address"
ask ADMIN_NAME "Your display name" "Administrator"
ask_secret ADMIN_PASSWORD "Your password (min. 12 characters)"

View File

@ -777,4 +777,19 @@ if [[ -n "$agent_hint" ]]; then
printf ' %s\n' "Run once: sudo bash $(pwd)/deploy/install-agent.sh"
fi
# Wer clupilot:bind-hosts braucht, ist genau der, der diese Zeile nicht liest.
# APP_HOST bleibt leer auf jeder Maschine, die vor den vier Host-Variablen
# installiert wurde (install.sh schreibt sie nur bei einer NEUEN .env, siehe
# BindHosts-Klassenkommentar) — und ohne APP_HOST laufen Website und Portal
# host-unabhängig: BEIDE antworten unter JEDEM Namen, ohne Fehler, ohne
# Logzeile, die davon erzählt. Nur ein Hinweis, kein Aufruf: `in_app` läuft mit
# `-T` für dieses Skript und ist der Operatorin, die den Befehl gleich selbst
# in ihrer eigenen Shell eintippt, gar nicht bekannt — wie schon beim
# `docker compose exec …` in finish() oben braucht der Text hier die
# vollständige Zeile, keine Abkürzung, die nur innerhalb dieses Skripts gilt.
if [[ -z "$(sed -n 's/^APP_HOST=//p' .env 2>/dev/null | tail -1)" ]]; then
printf '\033[1;33m !\033[0m %s\n' "Hostnamen sind nicht gebunden — Website und Portal antworten unter jedem Namen."
printf ' %s\n' "Beheben mit: docker compose exec -u www-data app php artisan clupilot:bind-hosts"
fi
log "Done — $(release_version) on $(git rev-parse --short HEAD) (${source_ref})"

View File

@ -92,6 +92,24 @@ it('backs up the previous content before writing the new content', function () {
->and(File::get($path))->toBe("NEW_KEY=new-value\n");
});
it('backs up .env with the same restrictive mode as the original, not the umask', function () {
// .env trägt jedes Zugangsdatum der Installation — APP_KEY, DB_PASSWORD,
// VPN_CONFIG_KEY, STRIPE_SECRET. File::copy() ruft PHPs copy() auf, und das
// wendet die Prozess-Umask an statt den Modus der Quelle zu übernehmen:
// deploy/install.sh setzt .env auf 0600, eine unbehandelte Kopie landete
// mit einer üblichen Umask trotzdem bei 0644 — weltlesbar. Verifiziert auf
// dieser Maschine, bevor der Fix stand: .env und .env.bak-* existierten,
// die Sicherung war 644.
$path = envEditorPath();
File::put($path, "OLD_KEY=old-value\n");
chmod($path, 0600);
$editor = new EnvFileEditor($path);
$backupPath = $editor->write("NEW_KEY=new-value\n");
expect(fileperms($backupPath) & 0777)->toBe(0600);
});
it('does not collide when two writes land in the same second', function () {
$path = envEditorPath();
File::put($path, "V=1\n");

View File

@ -1,5 +1,6 @@
<?php
use App\Support\AdminArea;
use Illuminate\Container\Container;
use Illuminate\Contracts\Console\Kernel;
use Illuminate\Database\Eloquent\Model;
@ -25,8 +26,14 @@ use Illuminate\Support\Facades\Facade;
* leere :memory:-Datenbank, und der restliche Testlauf fände danach keine
* einzige Tabelle mehr. Das ist kein hypothetisches Risiko genau so ist es
* beim Bau dieses Tests passiert.
*
* @param bool|null $exclusive Out-Parameter: nach dem Aufruf der Wert von
* AdminArea::isExclusive() der ZWEITEN
* Anwendung. Muss hier gelesen werden, nicht
* nach dem Rückgabewert dieser Funktion siehe
* Kommentar im try-Block unten.
*/
function hostSeparationTable(): array
function hostSeparationTable(?bool &$exclusive = null): array
{
$env = [
'APP_HOST' => 'app.clupilot.test',
@ -63,6 +70,15 @@ function hostSeparationTable(): array
$app = require base_path('bootstrap/app.php');
$app->make(Kernel::class)->bootstrap();
// AdminArea::isExclusive() liest von config(), und config() beantwortet
// JEDE Anwendung, die gerade Container::getInstance() ist — hier also
// die zweite. Nach diesem try-Block ist sie im finally schon wieder
// abgebaut und Container::getInstance() zeigt zurück auf die erste
// Testanwendung; ein Aufruf nach return hätte also nicht den hier
// gesetzten ADMIN_HOST_EXCLUSIVE gelesen, sondern den der Suite, und
// die Prüfung unten hätte sich still auf das Falsche verlassen.
$exclusive = AdminArea::isExclusive();
$table = [];
foreach ($app['router']->getRoutes()->getRoutes() as $route) {
$table[] = [
@ -156,8 +172,25 @@ it('bindet jede Route an einen Hostnamen, außer den hier benannten', function (
'status',
];
$table = hostSeparationTable($exclusive);
// Diese Zusicherung ist der ganze Sinn der Prüfung unten, und ohne sie war
// sie nicht wahr: deploy/install.sh:311 schreibt ADMIN_HOST_EXCLUSIVE=false
// als Vorgabe für jede neue Installation. Im nicht-exklusiven Fallback
// registriert routes/web.php:127-139 JEDE /admin/*-Route ohne Domain — die
// Konsole lebt dort absichtlich unter /admin auf jedem Hostnamen, damit ein
// Upgrade niemanden aussperrt (siehe AdminArea-Klassenkommentar). "Jede
// Route hat einen Hostnamen" ist in diesem Modus also schlicht falsch und
// nicht gemeint — die Prüfung hat nur im exklusiven Modus etwas zu sagen,
// und $env oben setzt ADMIN_HOST_EXCLUSIVE=true genau deshalb explizit.
expect($exclusive)->toBeTrue(
'ADMIN_HOST_EXCLUSIVE aus $env wurde nicht wirksam — ohne exklusiven '
.'Modus bindet routes/web.php admin/* an keine Domain, und die '
.'Prüfung unten würde das nicht bemerken.'
);
$unbound = [];
foreach (hostSeparationTable() as $route) {
foreach ($table as $route) {
if ($route['domain'] === null) {
$unbound[] = $route['uri'];
}