From 62e69ecde2d9770835d8977b94df3ec04271b687 Mon Sep 17 00:00:00 2001 From: nexxo Date: Mon, 3 Aug 2026 19:45:35 +0200 Subject: [PATCH] Aufgabe 2 gestrichen: das Passwortversteck schuetzte vor nichts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Die Pruefung hat nachgerechnet, was ich im Entwurf behauptet hatte, und es stimmte nicht. guestExec faehrt die GANZE Zeile als /bin/sh -c auf der Kunden-VM; die innere Shell setzt den Wert vor dem exec ein, er steht also auch im Argv von php occ; und Nextcloud legt ihn danach ohnehin im Klartext in config.php ab. Eingespart wurde einzig das Argv des docker-Aufrufs — auf einer Maschine, die denselben Wert an zwei anderen Stellen zeigt. Statt den Kommentar schoenzureden faellt der Mechanismus weg. Aufgabe 3 schreibt das Passwort als gewoehnliches Argument und sagt in ihrem Kopfkommentar, wo es ueberall sichtbar ist. Geprueft wird stattdessen, was wirklich gilt: dass der Wert maskiert ist und keinen zweiten Befehl starten kann — der Aufruf laeuft als root auf einer Kundenmaschine. Die Eingrenzung liegt in der Infrastruktur: Versandkonto nur mit Senderecht, Versandport nur fuer die eigenen Hostadressen, Sendelimit je Konto. --- .../plans/2026-08-03-mitarbeiterverwaltung.md | 205 ++++++++---------- ...2026-08-03-mitarbeiterverwaltung-design.md | 107 ++++++--- 2 files changed, 167 insertions(+), 145 deletions(-) diff --git a/docs/superpowers/plans/2026-08-03-mitarbeiterverwaltung.md b/docs/superpowers/plans/2026-08-03-mitarbeiterverwaltung.md index c02acc7..df95a82 100644 --- a/docs/superpowers/plans/2026-08-03-mitarbeiterverwaltung.md +++ b/docs/superpowers/plans/2026-08-03-mitarbeiterverwaltung.md @@ -282,116 +282,57 @@ MSG --- -## Task 2: Ein occ-Befehl, dessen Wert erst im Container entsteht +## Task 2: ENTFAELLT — der Mechanismus schuetzte vor nichts -**Files:** -- Modify: `app/Support/NextcloudOcc.php` -- Test: `tests/Feature/NextcloudOccTest.php` (anlegen, falls nicht vorhanden) +**Diese Aufgabe wurde waehrend der Umsetzung gestrichen.** Sie sollte eine +occ-Befehlsform bauen, bei der das SMTP-Passwort erst von der Shell IM +Container eingesetzt wird, damit es nicht in der Prozessliste der Kunden-VM +steht. Die Pruefung hat nachgerechnet, dass das nichts bewirkt: -**Interfaces:** -- Produces: `NextcloudOcc::commandExpandingEnv(string $arguments, array $env): string` +1. `guestExec()` uebergibt die GANZE Zeile als `['/bin/sh','-c',…]` an den + Gastagenten — das Zuweisungspraefix mit dem Geheimnis steht also im Argv + eines VM-Prozesses. Debians `dash` behaelt diesen Prozess fuer die volle + Dauer; `bash` wuerde ihn wegoptimieren, `dash` tut es nicht. +2. Die innere Shell setzt `"$VAR"` VOR dem `exec` ein — der Klartext steht + danach im Argv von `php occ` im Container. +3. Nextcloud legt `mail_smtppassword` anschliessend im Klartext in + `config/config.php` ab. Wer im Container `/proc` liest, liest auch diese + Datei; wer auf der VM Zugriff hat, ebenso. -`config:system:set mail_smtppassword --value=…` verlangt den Wert als Argument. Die bestehende `command()` setzt Umgebungswerte VOR dem `docker compose exec`, wo die äußere Shell sie einsetzt — der Wert stünde damit in der Prozessliste der ganzen VM, nicht nur des Containers. +Eingespart wurde einzig das Argv des `docker`-Aufrufs — auf einer Maschine, +auf der derselbe Wert zwei Zeilen darueber und dauerhaft in einer Datei steht. +Ein Versteck, das dieselbe Person nicht aussperrt, die den Ablageort ohnehin +lesen kann, ist keines. -- [ ] **Schritt 1: Die fehlschlagende Prüfung schreiben** +**Was stattdessen gilt:** Aufgabe 3 schreibt das Passwort mit dem gewoehnlichen +`NextcloudOcc::command()` als Argument und sagt in ihrem Kopfkommentar +ausdruecklich, wo es ueberall sichtbar ist. Die Eingrenzung liegt in der +Infrastruktur — Versandkonto nur mit Senderecht, Versandport nur fuer die +eigenen Hostadressen, Sendelimit je Konto — und die traegt der Betreiber, +nicht der Code. -```php - 'geheim'], - ); - - // Das Geheimnis steht als Zuweisung da (die aeussere Shell reicht es - // durch), aber der occ-Aufruf traegt nur den VARIABLENNAMEN — eingesetzt - // wird er von der sh INNERHALB des Containers. - expect($befehl)->toContain("CLUPILOT_SMTP_PW='geheim'") - ->and($befehl)->toContain('-e CLUPILOT_SMTP_PW') - ->and($befehl)->toContain('sh -c') - // Entscheidend: hinter `php occ` steht der Name, nicht der Wert. - ->and(substr($befehl, strpos($befehl, 'sh -c')))->not->toContain('geheim'); -}); - -it('haelt sich an dasselbe Verzeichnis und denselben Benutzer wie command()', function () { - $befehl = NextcloudOcc::commandExpandingEnv('status', []); - - expect($befehl)->toStartWith('cd '.NextcloudOcc::DIRECTORY) - ->and($befehl)->toContain('-u '.NextcloudOcc::USER); -}); -``` - -- [ ] **Schritt 2: Laufen lassen, Fehlschlag bestätigen** - -Ausführen: `docker compose exec -u 1000:1000 -T app php artisan test tests/Feature/NextcloudOccTest.php` -Erwartet: FEHLSCHLAG, `Call to undefined method … ::commandExpandingEnv()` - -- [ ] **Schritt 3: Die Methode schreiben** - -An `app/Support/NextcloudOcc.php` anfügen: - -```php - /** - * Wie command(), aber der Wert wird erst von der Shell IM Container - * eingesetzt. - * - * command() taugt fuer `--password-from-env`, wo occ selbst die - * Umgebungsvariable liest. `config:system:set` kann das nicht: es will den - * Wert als Argument. Setzte die aeussere Shell ihn ein, stuende das - * Passwort in der Prozessliste der KUNDEN-VM — dort, wo jeder mit einer - * Shell auf der Maschine `ps` ausfuehren kann. So steht es nur in der des - * Containers. - * - * EHRLICHERWEISE ist das Hygiene, kein Schutz: Nextcloud legt - * `mail_smtppassword` anschliessend im Klartext in config/config.php ab. - * Wer auf der Maschine eine Shell hat, liest es dort. Die eigentliche - * Eingrenzung liegt woanders — das Versandkonto kann nur senden, und der - * Versandport nimmt nur die eigenen Hostadressen an. Diese Methode senkt - * die Gelegenheit, sie beseitigt sie nicht. - * - * @param array $env - */ - public static function commandExpandingEnv(string $arguments, array $env): string - { - $assignments = ''; - $forwards = ''; - - foreach ($env as $name => $value) { - $assignments .= $name.'='.escapeshellarg($value).' '; - $forwards .= '-e '.$name.' '; - } - - return 'cd '.self::DIRECTORY.' && '.$assignments - .'docker compose exec -T -u '.self::USER.' '.$forwards - .'app sh -c '.escapeshellarg('php occ '.$arguments); - } -``` - -- [ ] **Schritt 4: Laufen lassen, grün** - -Ausführen: `docker compose exec -u 1000:1000 -T app php artisan test tests/Feature/NextcloudOccTest.php` -Erwartet: 2 grün - -- [ ] **Schritt 5: Die bestehende Regel prüfen** - -Ausführen: `docker compose exec -u 1000:1000 -T app php artisan test --filter=DeploymentRunsAsTheAppUser` -Erwartet: grün — die Regel „nichts ausser NextcloudOcc schreibt `docker compose exec`" gilt weiter. - -- [ ] **Schritt 6: Festschreiben** +- [ ] **Schritt 2: Festschreiben** ```bash git commit -F- -- app/Support/NextcloudOcc.php tests/Feature/NextcloudOccTest.php <<'MSG' -Eine occ-Befehlsform, deren Wert erst im Container eingesetzt wird +commandExpandingEnv zurueckgenommen — sie schuetzte vor nichts -config:system:set will den Wert als Argument. Setzte ihn die aeussere Shell -ein, stuende das SMTP-Passwort in der Prozessliste der Kunden-VM. Mit `sh -c` -im Container steht dort nur der Variablenname. +Die Methode sollte das SMTP-Passwort aus der Prozessliste der Kunden-VM +halten. Nachgerechnet: guestExec faehrt die ganze Zeile als /bin/sh -c, die +innere Shell setzt den Wert vor dem exec ein, und Nextcloud legt ihn danach +ohnehin im Klartext in config.php ab. Eingespart wurde einzig das Argv des +docker-Aufrufs, auf einer Maschine, die den Wert an zwei anderen Stellen +zeigt. -Im Kopfkommentar steht ausdruecklich, dass das Hygiene ist und kein Schutz: -Nextcloud legt den Wert danach im Klartext in config.php ab. +Ein Versteck, das dieselbe Person nicht aussperrt, die den Ablageort lesen +kann, ist keines — und ein Kommentar, der Schutz behauptet, haelt den +Naechsten vom Nachsehen ab. MSG ``` @@ -405,7 +346,7 @@ MSG - Create: `tests/Feature/Provisioning/ConfigureInstanceMailTest.php` **Interfaces:** -- Consumes: `GuestMailConfig::for()`, `NextcloudOcc::commandExpandingEnv()`, `CustomerStep::guest()` +- Consumes: `GuestMailConfig::for()`, `NextcloudOcc::command()`, `CustomerStep::guest()` - Produces: Schrittschlüssel `configure_instance_mail`; Pipeline `instance-mail` - [ ] **Schritt 1: Die fehlschlagende Prüfung schreiben** @@ -457,22 +398,32 @@ it('traegt jeden Wert einzeln in den Gast', function () { ->and($befehle)->toContain('clupilot.cloud'); }); -it('schreibt das Passwort NIE als Klartext-Argument in den Docker-Aufruf', function () { - // Die Zusicherung aus Aufgabe 2, hier am ECHTEN Befehl geprueft — nicht - // am Baustein allein. - versandbereitFuerSchritt(); +it('maskiert das Passwort, sodass es keinen zweiten Befehl starten kann', function () { + // Verstecken laesst sich der Wert auf dieser Maschine nicht (siehe + // Kopfkommentar des Schrittes). Was sehr wohl gilt und geprueft gehoert: + // er darf aus seiner Klammerung nicht ausbrechen. Der Befehl laeuft als + // root auf einer Kundenmaschine — ein Semikolon im Passwort waere dort + // ein zweiter Befehl. + Settings::set('mail.host', 'mail.clupilot.cloud'); + Settings::set('mail.port', 587); + Mailbox::factory()->create([ + 'key' => 'instance-relay', 'address' => 'noreply@clupilot.cloud', + 'username' => 'noreply@clupilot.cloud', + 'password' => "boes'; touch /tmp/PWNED; echo '", + 'active' => true, 'authenticates' => true, + ]); + $pve = new FakeProxmoxClient; app()->instance(ProxmoxClient::class, $pve); app(ConfigureInstanceMail::class)->execute(laufMitInstanz(Instance::factory()->create())); - foreach ($pve->guestCommands as $befehl) { - if (! str_contains($befehl, 'mail_smtppassword')) { - continue; - } - // Hinter `sh -c` darf das Geheimnis nicht stehen. - expect(substr($befehl, strpos($befehl, 'sh -c')))->not->toContain('geheim'); - } + $passwortbefehl = collect($pve->guestCommands) + ->first(fn ($b) => str_contains($b, 'mail_smtppassword')); + + // Der ganze Wert steht in EINEM maskierten Argument: das Semikolon darf + // nicht ausserhalb der Anfuehrungszeichen stehen. + expect($passwortbefehl)->toContain(escapeshellarg("boes'; touch /tmp/PWNED; echo '")); }); it('schreibt GAR NICHTS, wenn der Mailserver fehlt', function () { @@ -540,6 +491,28 @@ use App\Support\NextcloudOcc; * Ohne Server oder Absenderpostfach wird NICHTS geschrieben und der Schritt * scheitert mit einem Grund. Eine halb eingetragene Mailkonfiguration waere * schlimmer als keine. + * + * WO DAS SMTP-PASSWORT UEBERALL SICHTBAR IST — ehrlich aufgeschrieben, weil + * ein erster Entwurf es zu verstecken versuchte und das Verstecken nichts + * brachte: + * + * - im Argv des `/bin/sh -c`, das der Gastagent fuer diesen Aufruf startet + * (guestExec baut die ganze Zeile so). Debians `dash` behaelt diesen + * Prozess fuer die volle Dauer des Aufrufs; + * - im Argv von `php occ` im Container; + * - und DAUERHAFT im Klartext in /opt/nextcloud … config/config.php, wo + * Nextcloud es selbst ablegt. + * + * Der dritte Punkt erledigt die ersten beiden: wer die Datei lesen kann, muss + * gar nicht erst in eine Prozessliste schauen. Ein Versteck, das dieselbe + * Person nicht aussperrt, ist keines — und ein Kommentar, der eines + * behauptet, haelt den Naechsten vom Nachsehen ab. + * + * Die Eingrenzung liegt deshalb in der Infrastruktur, nicht hier: das + * Versandkonto kann nur senden, der Versandport nimmt nur die eigenen + * Hostadressen an, und der Mailserver drosselt je Konto. Was hier gilt und + * geprueft wird, ist etwas anderes: der Wert wird maskiert, damit er keinen + * zweiten Befehl starten kann. */ class ConfigureInstanceMail extends CustomerStep { @@ -577,11 +550,10 @@ class ConfigureInstanceMail extends CustomerStep )); } - // Das Passwort getrennt und ueber die Form, die den Wert erst im - // Container einsetzt — siehe NextcloudOcc::commandExpandingEnv(). - $this->guest($pve, $run, NextcloudOcc::commandExpandingEnv( - 'config:system:set mail_smtppassword --value="$CLUPILOT_SMTP_PW"', - ['CLUPILOT_SMTP_PW' => $config->password()], + // Das Passwort wie jeder andere Wert. Verstecken laesst es sich auf + // dieser Maschine nicht — siehe Kopfkommentar. + $this->guest($pve, $run, NextcloudOcc::command( + 'config:system:set mail_smtppassword --value='.escapeshellarg($config->password()) )); return StepResult::advance(); @@ -1937,7 +1909,8 @@ Keine Lücke. **Namensgleichheit über die Aufgaben** `GuestMailConfig::for()/available()/problem()/values()/password()` (1) → benutzt in 3. -`NextcloudOcc::commandExpandingEnv()` (2) → benutzt in 3. +`NextcloudOcc` bleibt unveraendert (Aufgabe 2 entfaellt) — Aufgabe 3 benutzt +das bestehende `command()`. `Seat::STATE_*`, `Seat::GROUPS`, `isReadonly()`, `linkToInstanceAdmin()` (5) → benutzt in 6, 7. `NextcloudUsers::invite/applyRole/disable/enable` (6) → benutzt in 7. `SyncSeatToNextcloud(string $seatUuid, string $action)` (7) → in sich geschlossen. diff --git a/docs/superpowers/specs/2026-08-03-mitarbeiterverwaltung-design.md b/docs/superpowers/specs/2026-08-03-mitarbeiterverwaltung-design.md index d486158..d245706 100644 --- a/docs/superpowers/specs/2026-08-03-mitarbeiterverwaltung-design.md +++ b/docs/superpowers/specs/2026-08-03-mitarbeiterverwaltung-design.md @@ -79,39 +79,66 @@ Geschäft läuft. Zwei Domains, zwei Rufe, ein Ausfall bleibt einer. Ein neuer Bereitstellungsschritt schreibt in jeden Gast: -| Schlüssel | Wert | -|---|---| -| `mail_smtpmode` | `smtp` | -| `mail_smtphost` | aus dem CluPilot-Postfach | -| `mail_smtpport` | aus dem CluPilot-Postfach | -| `mail_smtpsecure` | aus `App\Services\Mail\MailTlsPolicy` des Postfachs | -| `mail_smtpauth` | `true` | -| `mail_smtpname` | Benutzername des Postfachs | -| `mail_smtppassword` | Passwort des Postfachs | -| `mail_from_address` | `noreply` | -| `mail_domain` | `clupilot.cloud` | -| `mail_sendmailmode` | entfällt | +Server und Zugangsdaten liegen in diesem Projekt an **zwei** Orten, und das ist +Absicht: der Server steht einmal in `app_settings` („All mailboxes live on one +host, so it sits once in app_settings — a provider change is then one card, not +five rows"), die Zugangsdaten je Absender am `Mailbox`-Datensatz. + +| Schlüssel | Wert | Herkunft | +|---|---|---| +| `mail_smtpmode` | `smtp` | fest | +| `mail_smtphost` | z. B. `smtp.…` | `Settings::get('mail.host')` | +| `mail_smtpport` | z. B. `587` | `Settings::get('mail.port')` | +| `mail_smtpsecure` | `tls` / `ssl` / leer | `Settings::get('mail.encryption')` | +| `mail_smtpauth` | `true` | fest | +| `mail_smtpname` | Benutzername | `Mailbox::smtpUsername()` | +| `mail_smtppassword` | Passwort | `Mailbox` (unter `SECRETS_KEY`) | +| `mail_from_address` | `noreply` | linker Teil der Postfachadresse | +| `mail_domain` | `clupilot.cloud` | rechter Teil der Postfachadresse | + +Fehlt der Server (`mail.host` leer) oder das Postfach, wird **nichts** +geschrieben und der Schritt meldet einen klaren Grund. Eine halb eingetragene +Mailkonfiguration ist schlimmer als keine: Nextcloud versucht dann zu senden +und scheitert bei jedem einzelnen Versand still. Der Anzeigename ist der Cloud-Name des Kunden, damit eine Mail aus *seiner* Cloud kommt und nicht aus einem Produkt, das er nicht kennt. -### Das Passwort darf nicht in die Prozessliste +### Das Passwort ist auf der Kundenmaschine nicht zu verstecken -`occ config:system:set mail_smtppassword --value=…` würde das Passwort als -Befehlszeilen-Argument in den Gast tragen. Jeder, der dort `ps` ausführen kann, -liest es mit — und das ist auf einer Kundenmaschine niemand, dem wir das -SMTP-Passwort unserer Versandinfrastruktur anvertrauen wollen. +Der ursprüngliche Entwurf wollte das SMTP-Passwort an der Prozessliste +vorbeischmuggeln — erst über die Standardeingabe, dann über eine Variable, die +erst im Container eingesetzt wird. **Beides ist Theater, und zwar nachweislich.** -Deshalb geht die ganze Mailkonfiguration als **JSON über die Standardeingabe** -hinein: +Nachgerechnet in der Umsetzung: -``` -occ config:import < /dev/stdin -``` +1. `ProxmoxClient::guestExec()` übergibt die **ganze** Befehlszeile als + `['/bin/sh', '-c', …]` an den Gastagenten. Was immer darin steht, steht im + Argv eines Prozesses auf der Kunden-VM. Debians `dash` — die Standard-Shell + dieser Maschinen — behält diesen Elternprozess für die volle Dauer des + Aufrufs; `bash` würde ihn wegoptimieren, `dash` tut es nicht. +2. Eine Variable, die die innere Shell im Container einsetzt, landet danach im + Argv von `php occ` — die Expansion passiert **vor** dem `exec`. +3. Und wichtiger als beides: **Nextcloud legt `mail_smtppassword` anschließend + im Klartext in `config/config.php` ab.** Wer im Container `/proc` lesen + kann, liest auch diese Datei. Wer auf der VM Zugriff hat, ebenso. -Die Zeile wird in `App\Support\NextcloudOcc` gebaut, wie jeder andere -occ-Aufruf auch. Nichts in `app/` schreibt `docker compose exec` selbst — das -ist eine bestehende, testerzwungene Regel dieses Projekts. +Ein Versteck, das dieselbe Person nicht aussperrt, die den dauerhaften +Ablageort ohnehin lesen kann, ist kein Versteck. Der Wert geht deshalb als +gewöhnliches Argument über `NextcloudOcc::command()` hinein — wie jeder andere +occ-Aufruf dieses Projekts —, und der Schritt sagt in seinem Kopfkommentar +ausdrücklich, wo das Passwort überall sichtbar ist. + +**Die Eingrenzung liegt woanders, und dort liegt sie wirklich:** + +- Das Versandkonto kann **nur senden** — kein IMAP, kein Postfach, und + ausschließlich auf der Absenderdomain der Instanzen. +- Der Versandport nimmt **nur die eigenen Hostadressen** an. Wer die + Zugangsdaten aus einer VM zieht, kann sie von anderswo nicht benutzen. +- Der Mailserver kennt ein Sendelimit je Konto. + +Das ist die ehrliche Rechnung: der Wert ist auf der Maschine des Kunden +lesbar, und was er damit anfangen kann, ist eng begrenzt. ### Bestand nachrüsten @@ -176,10 +203,32 @@ Zeile sind und kein Umbau. | `member` | `mitarbeiter` | normaler Benutzer | | `readonly` | `nur-lesen` | Speicherplatz `0 B` — kann nichts hochladen | -Rollenwechsel: `group:adduser` und `group:removeuser`. Wird `readonly` -**gesetzt**, geht der Speicherplatz auf `0 B`; wird sie **verlassen**, geht er -auf `default` zurück — nicht auf einen festen Wert, denn die Vorgabe der -Instanz hängt am gekauften Paket (`ApplyStorageQuota` setzt sie). +Rollenwechsel: `group:adduser` und `group:removeuser`. Dazu bei `readonly` der +Speicherplatz — und hier lauert eine Falle, die in diesem Repo bereits +aufgeschrieben steht. + +### Die Speicherplatz-Falle + +`ApplyStorageQuota` setzt bewusst `files default_quota`, den **instanzweiten +Vorgabewert**, und begründet im Kopfkommentar, warum kein Konto einen eigenen +bekommen darf: + +> „An account with an explicit quota stops following the default, so writing one +> onto the admin account would freeze that account at today's figure and quietly +> exclude it from the next plan change." + +Genau das täte ein Speicherplatz am `readonly`-Konto. Für `readonly` ist das +richtig — dieses Konto SOLL keinen Platz bekommen, egal was das Paket später +sagt. Aber beim **Verlassen** der Rolle genügt es nicht, irgendeinen Wert zu +setzen: der eigene Wert muss **weg**, damit das Konto der Vorgabe wieder folgt. + +- `readonly` **gesetzt** → eigener Speicherplatz `0 B` +- `readonly` **verlassen** → eigenen Speicherplatz **löschen**, nicht + überschreiben + +Ein Konto, das mit einem festen Wert aus der Rolle herauskommt, wäre bei der +nächsten Paketänderung stumm ausgenommen — und niemand würde es merken, bis +der Kunde fragt, warum sein Mitarbeiter weniger Platz hat als bezahlt. ### Was `readonly` ehrlicherweise NICHT ist