From 501310f43b41b258376fc7fa239f90f0193b10f4 Mon Sep 17 00:00:00 2001 From: cschlaefke Date: Tue, 3 Nov 2026 13:51:46 +0100 Subject: [PATCH] docs(ci): verify fork-PR approval logic and correct runner advice in ADR-0003 Refs #43 Co-Authored-By: Claude Sonnet 5.5 --- docs/adr/0003-ci-runner-sicherheit.md | 37 +++++++++++++++------------ 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/docs/adr/0003-ci-runner-sicherheit.md b/docs/adr/0003-ci-runner-sicherheit.md index d8af087..d7d6733 100644 --- a/docs/adr/0003-ci-runner-sicherheit.md +++ b/docs/adr/0003-ci-runner-sicherheit.md @@ -12,13 +12,15 @@ Bei `on: pull_request` wird die Workflow-Datei aus dem PR-Head gelesen. Ein Fork `head.repo.full_name == github.repository` entfernen oder ein beliebiges `runs-on:` setzen. **Ein `if:` in `ci.yml` schützt daher nicht** und kein Inhalt des Repos kann einen Fork-PR daran hindern, `runs-on: docker-host` anzufordern. -Stand der Prüfung (Forgejo 8.0.3+gitea-1.22.0, Runner v5.0.4): Die Aussagen zum Verhalten stammen aus Kenntnis der -Gitea-1.22-/Forgejo-8-Quellen und der Doku, **nicht** aus einem Live-Test. Prüfschritte stehen unten. +Stand der Prüfung (Forgejo 8.0.3+gitea-1.22.0, Runner v5.0.4): Die Freigabe-Logik wurde im Quelltext von Forgejo v8.0.3 +gelesen (`services/actions/notifier_helper.go`, Funktion `ifNeedApproval`). Es gab **keinen Live-Test** mit einem Fork-PR; +das Verhalten des Runners und die Aussagen zu Runner-Konfiguration stammen aus Doku-Kenntnis. Prüfschritte stehen unten. ## Was Forgejo 8.0.3 bietet | Mechanismus | Verfügbarkeit | Bewertung | |---|---|---| -| Freigabe für Fork-PR-Workflows (`need approval`) | Eingebaut, **keine Repo-Einstellung zum Ein-/Ausschalten**. Läufe von Fork-PRs (Event `pull_request`) von Nutzern ohne Schreibrecht warten auf „Freigeben“ durch einen Maintainer. | Hauptschutz. Schwäche: Nach einer Freigabe gelten spätere Läufe desselben Nutzers im Repo als freigegeben (Quelltext-Annahme), d. h. ein neuer Push mit geändertem Workflow läuft evtl. ohne erneute Freigabe. | +| Freigabe für Fork-PR-Workflows (`need approval`) | Eingebaut, **keine Repo-Einstellung zum Ein-/Ausschalten**. Laut Quelltext v8.0.3: Fork-PR (Event `pull_request`) braucht Freigabe, außer der Nutzer hat Schreibrecht, **oder es gibt im Repo schon einen freigegebenen Lauf dieses Nutzers**. Eingeschränkte Nutzer (`restricted`) brauchen immer Freigabe. | Hauptschutz, aber mit Lücke: **Eine einzige Freigabe vertraut dem Nutzer dauerhaft.** Spätere Pushes mit manipulierten Workflows laufen ohne weitere Freigabe. Die Freigabe ist also faktisch eine Entscheidung „diesem Nutzer vertraue ich den Runner an“. | +| Nutzer `restricted` (Admin-Bereich -> Benutzerkonten, bzw. `app.ini` `[service] DEFAULT_USER_IS_RESTRICTED = true` für neue Konten) | Eingebaut | Erzwingt die Freigabe bei **jedem** Lauf (Quelltext). Sinnvoll, wenn Registrierung offen ist. | | `pull_request_target` | Läuft mit Workflow aus dem Base-Branch | Nicht verwenden, um PR-Code auf `docker-host` zu bauen (klassische Pwn-Request-Lücke). | | Runner-Scoping (Instanz/Organisation/Nutzer/Repo) | Eingebaut | Begrenzt, *welche Repos* den Runner nutzen, nicht *welche Events*. Hilft hier nicht gegen Fork-PRs im selben Repo. | | Runner-Labels | Eingebaut | Jeder Workflow darf jedes Label anfordern. Kein Schutz. | @@ -31,9 +33,9 @@ Verteidigung in drei Schichten; nur die erste lässt sich im Repo ausdrücken, d 1. **Repo (umgesetzt):** `if:`-Guard an `supabase-tests` und `e2e` bleibt als Komfort (ehrliche Fork-PRs hängen nicht in „wartet auf Freigabe“); Kommentare in `ci.yml` benennen ihn ausdrücklich als keinen Schutz. -2. **Forgejo (Chris):** Freigabe-Pflicht nutzen und Disziplin: Fork-PR-Läufe **nie** freigeben, ohne vorher die Änderungen an - `.forgejo/`, `e2e/`, `supabase/`, `app/pubspec*` und allen Skripten gelesen zu haben. Bis zur Isolation (3.) bevorzugt: - Fork-PRs gar nicht ausführen, sondern den Branch lokal prüfen oder in ein Branch im Haupt-Repo übernehmen. +2. **Forgejo (Chris):** Fork-PR-Läufe auf diesem Runner **nicht freigeben**, solange er nicht isoliert ist (3.) oder das Repo + nicht privat ist. Grund: Eine Freigabe gilt für alle späteren Läufe des Nutzers. Fremde Beiträge lokal prüfen oder als Branch + ins Haupt-Repo übernehmen. Neue/fremde Konten als `restricted` führen, damit jeder Lauf einzeln freigegeben werden muss. 3. **Runner/Mac (Chris):** Blast Radius des dind begrenzen (siehe Anleitung). Solange 2. und 3. nicht erledigt sind, gilt `docker-host` als **nicht für externe Beiträge geeignet**. Empfehlung: vor dem @@ -45,32 +47,33 @@ ersten externen Beitrag, spätestens vor Release, entweder 3. umsetzen oder das 1. Zweites Forgejo-Konto ohne Schreibrecht anlegen (oder Test-Nutzer), Repo forken, im Fork `ci.yml` ändern (`if:`-Zeilen entfernen, harmlosen Schritt `echo hallo` ergänzen), PR gegen `main` öffnen. 2. Erwartung: Lauf steht auf „Wartet auf Freigabe“ und startet erst nach Klick auf **Freigeben** (PR-Seite bzw. Actions-Lauf). - Falls er ohne Freigabe startet: Das Repo sofort privat stellen (Einstellungen -> Repository -> Sichtbarkeit) und + Falls er ohne Freigabe startet (Quelltext sagt anderes): Das Repo sofort privat stellen (Einstellungen -> Repository -> Sichtbarkeit) und Runner pausieren (B.3), dann Forgejo-Version/Doku prüfen. -3. Zweiten Push im selben PR machen: Läuft er jetzt ohne erneute Freigabe? Dann ist die Annahme „einmal freigegeben, immer freigegeben“ bestätigt. +3. Zweiten Push im selben PR machen: Läuft er jetzt ohne erneute Freigabe? Dann ist „einmal freigegeben, immer freigegeben“ bestätigt. Danach das Testkonto als `restricted` markieren und wiederholen: Jetzt muss jeder Lauf freigegeben werden. ### B. Auf dem Mac (Runner/dind) -1. Prüfen, ob `docker`-Jobs (nicht nur `docker-host`) den dind erreichen: In einem Testlauf mit `runs-on: docker` einen Schritt - `curl -s -m 3 http://:2375/version` ausführen (Gateway wie in `ci.yml` aus `/proc/net/route`). Antwortet er, - kann jeder Fork-PR-Code auf **jedem** Label den Daemon übernehmen. Dann dind-Port nur an einen eigenen Docker-Netzwerk-Alias - binden, der nur `docker-host`-Jobs zugewiesen ist (`container.network`/`options` in der Runner-Konfiguration), oder - TLS (Port 2376 mit Client-Zertifikaten) einführen, wobei die Zertifikate nur in `docker-host`-Jobs gemountet werden - (`valid_volumes` in der Runner-Konfiguration). -2. Besser: Runner in eine Wegwerf-VM (z. B. Lima/Colima/UTM) oder auf einen separaten Rechner/Hetzner-Server verlagern, ohne +1. Labels sind keine Grenze: Ein Fork-PR kann `runs-on: docker-host` selbst setzen. Eine Trennung `docker` / `docker-host` im + Workflow schützt deshalb nichts, und `container.network`/`valid_volumes` der Runner-Konfiguration gelten für den ganzen + Runner, nicht je Label. Schutz bieten nur Freigabe, ein Daemon/VM ohne Wertvolles (Punkt 2) oder ein privates Repo. Sollen + `docker` und `docker-host` unterschiedlich exponiert sein, braucht es getrennte Runner-Registrierungen mit eigener + Konfiguration, idealerweise auf getrennten Hosts. Prüfen kannst du, ob ein `docker`-Job den dind erreicht + (`curl -s -m 3 http://:2375/version`, Gateway wie in `ci.yml` aus `/proc/net/route`); antwortet er, + gilt die Warnung für alle Jobs des Runners. +2. Wirksam: Runner in eine Wegwerf-VM (z. B. Lima/Colima/UTM) oder auf einen separaten Rechner/Hetzner-Server verlagern, ohne Host-Mounts, ohne Zugriff aufs LAN, ohne Zugangsdaten und ohne persönliche Daten auf dem Mac. Ziel: Ein Ausbruch aus dem dind trifft nichts Wertvolles. Eigenes Registrierungs-Token verwenden; `.runner` und Token nicht ins Repo und nicht in Logs. 3. Notbremse: Runner stoppen (`launchctl`/Prozess beenden oder in Forgejo unter Einstellungen -> Actions -> Runner deaktivieren). 4. Runner nur für dieses Repo registrieren (Repo-Scope statt Instanz-/Nutzer-Scope), damit andere Repos ihn nicht ansprechen können. ### C. In Forgejo -1. Fork-PR-Läufe nur nach Review der Workflow-/Skriptänderungen freigeben (siehe Entscheidung 2.). +1. Fork-PR-Läufe nicht freigeben (siehe Entscheidung 2.); fremde Konten unter Administration -> Benutzerkonten als eingeschränkt markieren. 2. Optional: Branch-Schutz für `main` mit **Geschützte Dateimuster** `.forgejo/**;e2e/**;supabase/config.toml`, damit Änderungen daran nie ohne Review in `main` landen (wirkt auf Pushes nach `main`, nicht auf PR-Läufe). 3. Repo privat stellen, wenn externe Beiträge nicht gewollt sind (löst das Problem vollständig). 4. Runner-Token regelmäßig erneuern. ## Konsequenzen -- Externe Beiträge brauchen manuelle Freigabe und Review mit Blick auf CI-Dateien. +- Externe Beiträge laufen nicht auf `docker-host`, bis der Runner isoliert ist. - Fork-PRs bekommen für `supabase-tests`/`e2e` keine Prüfung, solange der `if:`-Guard steht; sie laufen dann nur auf `docker` (Format, Analyse, Tests, Web-Build). Ein Maintainer kann den Branch bei Bedarf ins Haupt-Repo übernehmen, um die Docker-Jobs zu starten. - Offen: Ergebnis der Prüfung A und Umsetzung B. Bis dahin Issue #43 nicht schließen.