From 666ba64fcc3fb13c59d2e8a22f6546ece1906b63 Mon Sep 17 00:00:00 2001 From: cschlaefke Date: Tue, 3 Nov 2026 13:50:25 +0100 Subject: [PATCH 1/4] fix(ci): document fork-PR risk for docker-host runner and guard supabase-tests Add ADR-0003 with Forgejo 8.0.3 options and a step-by-step guide for runner/Forgejo settings, correct the misleading guard comments in ci.yml and apply the comfort guard to supabase-tests too. Refs #43 Co-Authored-By: Claude Sonnet 5.5 --- .forgejo/workflows/ci.yml | 15 ++++-- docs/adr/0003-ci-runner-sicherheit.md | 76 +++++++++++++++++++++++++++ docs/auth-konfiguration.md | 2 +- 3 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 docs/adr/0003-ci-runner-sicherheit.md diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 423c16f..8f686ca 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -153,11 +153,14 @@ jobs: # (erreichbar über das Default-Gateway des Job-Netzes). Supabase-Ports liegen dann im dind; # die Supabase-CLI nimmt den Host aus DOCKER_HOST automatisch. # Nutzt die eingecheckte supabase/config.toml. Ohne passenden Runner Job bei Bedarf deaktivieren. + # SICHERHEIT: siehe Kommentar am Job e2e und docs/adr/0003-ci-runner-sicherheit.md. Der `if:` ist nur Komfort + # (keine hängenden Läufe für Fork-PRs), kein Schutz. supabase-tests: runs-on: docker-host concurrency: group: docker-host-supabase cancel-in-progress: false + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository env: DOCKER_CLI_VERSION: 27.3.1 DIND_PORT: 2375 @@ -189,11 +192,13 @@ jobs: # Web-E2E (Playwright: api, web, throttle) gegen lokalen Supabase-Stack und gebaute Flutter-Web-App (Issue #38, Spec #1 D2). # Läuft wie supabase-tests auf "docker-host" (Docker-CLI + dind per TCP). Der Stack liegt im dind; ein # TCP-Relay (e2e/support/tcp-forward.mjs) macht ihn im Job unter 127.0.0.1 erreichbar. - # SICHERHEIT: Der Job führt PR-Code mit Zugriff auf den dind-Daemon des Runners aus (Daemon = Root-Äquivalent). - # Der `if:` unten ist KEIN Schutz vor böswilligen Forks: Bei pull_request kommt der Workflow aus dem PR-Head, - # ein Fork kann die Bedingung einfach entfernen. Er verhindert nur versehentliche Läufe. Die echte Absicherung - # gehört in Repo-/Runner-Einstellungen (Freigabe für Fork-PRs, Runner nur für vertrauenswürdige Repos; Folge-Issue). - # Gleiches Risiko hat supabase-tests. + # SICHERHEIT (ADR-0003, Issue #43): Der Job führt PR-Code mit Zugriff auf den dind-Daemon des Runners aus + # (Daemon = Root-Äquivalent). Der `if:` unten ist KEIN Schutz vor böswilligen Forks: Bei pull_request kommt die + # Workflow-Datei aus dem PR-Head, ein Fork kann die Bedingung entfernen. Er verhindert nur, dass ehrliche Fork-PRs + # auf einen Lauf warten, den niemand freigibt. Die tatsächliche Absicherung liegt außerhalb dieser Datei: + # Freigabe-Pflicht für Fork-PRs (Maintainer prüft die Workflow-Änderungen vor „Freigeben“) und ein isolierter + # Runner/Daemon. Schritt-für-Schritt-Anleitung und Restrisiken: docs/adr/0003-ci-runner-sicherheit.md. + # Gleiches gilt für supabase-tests. # needs + concurrency: Beide Jobs belegen dieselben festen Container-Namen/Ports (project_id shuffle, 54321 ...) im dind. # `needs` serialisiert innerhalb eines Laufs; die concurrency-Gruppe (auch an supabase-tests) soll Läufe verschiedener # PRs hintereinander schalten. Ob Forgejo 8.0.3 / Runner v5.0.4 Job-concurrency auswertet, ist hier nicht verifiziert; diff --git a/docs/adr/0003-ci-runner-sicherheit.md b/docs/adr/0003-ci-runner-sicherheit.md new file mode 100644 index 0000000..d8af087 --- /dev/null +++ b/docs/adr/0003-ci-runner-sicherheit.md @@ -0,0 +1,76 @@ +# ADR-0003: CI-Sicherheit – Fork-PRs und der Runner `docker-host` + +- **Status:** Vorgeschlagen (Chris entscheidet über die Maßnahmen unten), Issue #43 +- **Datum:** 2026-11-03 + +## Kontext +Das Repo ist öffentlich. Der Runner `shuffle-mac-runner` (Label `docker-host`) läuft auf Chris' Mac mit Docker-in-Docker; +die Jobs `supabase-tests` und `e2e` sprechen den dind-Daemon per TCP an (`DOCKER_HOST=tcp://:2375`, ohne TLS). +Wer dort Code ausführt, hat Root-Äquivalent im dind (Container mit Host-Mounts, Zugriff auf alles, was der dind sieht). + +Bei `on: pull_request` wird die Workflow-Datei aus dem PR-Head gelesen. Ein Fork-PR kann Bedingungen wie +`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. + +## 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. | +| `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. | +| Event-/Repo-Filter im Runner (`runner.yaml`) | Nicht vorhanden | -- | +| Repo privat stellen | Eingebaut | Beseitigt das Problem (Fremde können keine PRs öffnen), kostet Öffentlichkeit. | +| Forks instanzweit verbieten (`app.ini`, Forgejo-Doku prüfen: `[repository] DISABLE_FORKS`/ähnlich) | In 8.0.3 nicht verifiziert | Nur als Prüfpunkt. | + +## Entscheidung (Vorschlag) +Verteidigung in drei Schichten; nur die erste lässt sich im Repo ausdrücken, der Rest braucht Chris: + +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. +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 +ersten externen Beitrag, spätestens vor Release, entweder 3. umsetzen oder das Repo privat stellen. + +## Anleitung für Chris (nicht automatisiert; nichts davon wurde ausgeführt) + +### A. Verhalten prüfen (10 Minuten, empfohlen als Erstes) +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 + 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. + +### 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 + 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.). +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. +- 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. diff --git a/docs/auth-konfiguration.md b/docs/auth-konfiguration.md index e8bfa76..32c7995 100644 --- a/docs/auth-konfiguration.md +++ b/docs/auth-konfiguration.md @@ -202,7 +202,7 @@ Der Job `e2e` in `.forgejo/workflows/ci.yml` läuft auf dem Runner `docker-host` 4. `.forgejo/scripts/e2e-run.sh` startet den TCP-Relay `e2e/support/tcp-forward.mjs` (127.0.0.1:54321/54322/54324/9998 -> dind-Gateway) und ruft `npx playwright test --project=api --project=web --project=throttle` auf. Der Relay ist nötig, weil die Ports im dind liegen, die Tests, die Web-App (CORS/Origin) und `e2e/support/db.ts` (erlaubt nur lokale Hosts) aber 127.0.0.1 erwarten. Die statische Auslieferung der Web-App auf Port 3000 übernimmt das `webServer` der Playwright-Konfiguration (`serve -s`). 5. Bei Fehlern werden `e2e/playwright-report` und `e2e/test-results` (Traces) als Artefakt `playwright-` hochgeladen. Der Schritt „Aufräumen“ (`if: always()`) entfernt den Extra-Container und stoppt den Supabase-Stack. -Das Projekt `spike` (SMTP-Ausfall, `SPIKE_GOTRUE_URL`) läuft in CI nicht. Der Job hat `needs: supabase-tests` (gleiche Container-Namen und Ports im dind) und läuft nicht für Fork-PRs: Wer PR-Code auf `docker-host` ausführt, hat Zugriff auf den dind-Daemon des Runners. +Das Projekt `spike` (SMTP-Ausfall, `SPIKE_GOTRUE_URL`) läuft in CI nicht. Der Job hat `needs: supabase-tests` (gleiche Container-Namen und Ports im dind) und läuft nicht für Fork-PRs (nur Komfort-Guard, kein Schutz vor böswilligen Forks): Wer PR-Code auf `docker-host` ausführt, hat Zugriff auf den dind-Daemon des Runners. Absicherung und offene Schritte: `docs/adr/0003-ci-runner-sicherheit.md`. **Achtung Artefakte:** Playwright-Reports, Traces und Screenshots (CI-Artefakte, `test-results/`, `playwright-report/`) enthalten Tokens, Passwörter und Mailinhalte der angelegten Testkonten. Die Konten existieren nur in der Wegwerf-Instanz (lokale CLI/CI), dennoch Artefakte nicht in ein öffentliches Repo oder eine öffentliche Release-Seite legen und nur kurz aufbewahren. Die Tests lehnen nicht-lokale Hosts ab (`SUPABASE_URL`, `MAILPIT_URL`, `E2E_DB_URL`; CI-Hosts nur explizit über `E2E_ALLOWED_HOSTS`). -- 2.45.2 From 501310f43b41b258376fc7fa239f90f0193b10f4 Mon Sep 17 00:00:00 2001 From: cschlaefke Date: Tue, 3 Nov 2026 13:51:46 +0100 Subject: [PATCH 2/4] 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. -- 2.45.2 From 25610c1ec90cfc4e1f6aecd2543d0a1fc9ad053f Mon Sep 17 00:00:00 2001 From: cschlaefke Date: Tue, 3 Nov 2026 13:58:09 +0100 Subject: [PATCH 3/4] docs(ci): correct guard rationale and unverified claims in ADR-0003 Refs #43 Co-Authored-By: Claude Sonnet 5.5 --- .forgejo/workflows/ci.yml | 10 +++--- docs/adr/0003-ci-runner-sicherheit.md | 52 ++++++++++++++++++--------- docs/adr/README.md | 7 ++++ 3 files changed, 48 insertions(+), 21 deletions(-) create mode 100644 docs/adr/README.md diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 8f686ca..3144e53 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -153,8 +153,7 @@ jobs: # (erreichbar über das Default-Gateway des Job-Netzes). Supabase-Ports liegen dann im dind; # die Supabase-CLI nimmt den Host aus DOCKER_HOST automatisch. # Nutzt die eingecheckte supabase/config.toml. Ohne passenden Runner Job bei Bedarf deaktivieren. - # SICHERHEIT: siehe Kommentar am Job e2e und docs/adr/0003-ci-runner-sicherheit.md. Der `if:` ist nur Komfort - # (keine hängenden Läufe für Fork-PRs), kein Schutz. + # SICHERHEIT: siehe Kommentar am Job e2e und docs/adr/0003-ci-runner-sicherheit.md. Der `if:` ist kein Schutz. supabase-tests: runs-on: docker-host concurrency: @@ -194,9 +193,10 @@ jobs: # TCP-Relay (e2e/support/tcp-forward.mjs) macht ihn im Job unter 127.0.0.1 erreichbar. # SICHERHEIT (ADR-0003, Issue #43): Der Job führt PR-Code mit Zugriff auf den dind-Daemon des Runners aus # (Daemon = Root-Äquivalent). Der `if:` unten ist KEIN Schutz vor böswilligen Forks: Bei pull_request kommt die - # Workflow-Datei aus dem PR-Head, ein Fork kann die Bedingung entfernen. Er verhindert nur, dass ehrliche Fork-PRs - # auf einen Lauf warten, den niemand freigibt. Die tatsächliche Absicherung liegt außerhalb dieser Datei: - # Freigabe-Pflicht für Fork-PRs (Maintainer prüft die Workflow-Änderungen vor „Freigeben“) und ein isolierter + # Workflow-Datei aus dem PR-Head, ein Fork kann die Bedingung entfernen. Er hält nur einen unveränderten, bereits + # freigegebenen Fork-PR von docker-host fern; er verhindert weder das Warten auf die Freigabe (gilt für den ganzen Lauf, + # das Job-`if` wird erst danach ausgewertet) noch einen manipulierten Lauf. Die tatsächliche Absicherung liegt außerhalb + # dieser Datei: Freigabe-Pflicht (eine Freigabe vertraut dem Nutzer dauerhaft, also Fork-PRs hier nicht freigeben) und ein isolierter # Runner/Daemon. Schritt-für-Schritt-Anleitung und Restrisiken: docs/adr/0003-ci-runner-sicherheit.md. # Gleiches gilt für supabase-tests. # needs + concurrency: Beide Jobs belegen dieselben festen Container-Namen/Ports (project_id shuffle, 54321 ...) im dind. diff --git a/docs/adr/0003-ci-runner-sicherheit.md b/docs/adr/0003-ci-runner-sicherheit.md index d7d6733..8f93e93 100644 --- a/docs/adr/0003-ci-runner-sicherheit.md +++ b/docs/adr/0003-ci-runner-sicherheit.md @@ -19,8 +19,8 @@ das Verhalten des Runners und die Aussagen zu Runner-Konfiguration stammen aus D ## 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**. 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. | +| Freigabe für Fork-PR-Workflows (`need approval`) | Eingebaut, **keine Repo-Einstellung zum Ein-/Ausschalten**. Entscheidet über den ganzen Lauf. Laut Quelltext v8.0.3 braucht ein Fork-PR (Event `pull_request`) Freigabe, außer der Nutzer hat Schreibrecht oder es gibt im Repo schon einen freigegebenen Lauf dieses Nutzers. | Hauptschutz mit Lücke, siehe Absatz unter der Tabelle. | +| Nutzer `restricted` (Administration -> Benutzerkonten; `app.ini` `[service] DEFAULT_USER_IS_RESTRICTED` für neue Konten, **unverifiziert**) | Eingebaut | Erzwingt die Freigabe bei jedem Lauf (Quelltext). Nebenwirkung vermutlich: keine Sicht auf öffentliche Repos ohne Zuordnung (**unverifiziert**), siehe C.2. | | `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. | @@ -28,14 +28,19 @@ das Verhalten des Runners und die Aussagen zu Runner-Konfiguration stammen aus D | Repo privat stellen | Eingebaut | Beseitigt das Problem (Fremde können keine PRs öffnen), kostet Öffentlichkeit. | | Forks instanzweit verbieten (`app.ini`, Forgejo-Doku prüfen: `[repository] DISABLE_FORKS`/ähnlich) | In 8.0.3 nicht verifiziert | Nur als Prüfpunkt. | +**Vertrauenslücke der Freigabe:** Eine einzige Freigabe vertraut dem Nutzer dauerhaft. Für alle künftigen PRs dieses Nutzers im +Repo (nicht nur für diesen PR) laufen Fork-PR-Workflows ohne weitere Freigabe, auch nach einem Push mit manipuliertem Workflow. +Eine Freigabe ist also faktisch die Entscheidung „diesem Nutzer vertraue ich den Runner an“. + ## Entscheidung (Vorschlag) Verteidigung in drei Schichten; nur die erste lässt sich im Repo ausdrücken, der Rest braucht Chris: -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. +1. **Repo (umgesetzt):** Der `if:`-Guard an `supabase-tests` und `e2e` hält nur einen unveränderten, bereits freigegebenen + Fork-PR von `docker-host` fern. Er verhindert weder das Warten auf Freigabe (die gilt für den ganzen Lauf, das Job-`if` + wird erst danach ausgewertet) noch einen manipulierten Lauf. Die Kommentare in `ci.yml` sagen das. 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. + nicht privat ist. Grund: Eine Freigabe gilt für alle künftigen PRs des Nutzers im Repo. Fremde Beiträge lokal prüfen oder als + Branch ins Haupt-Repo übernehmen. 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 @@ -49,7 +54,12 @@ ersten externen Beitrag, spätestens vor Release, entweder 3. umsetzen oder das 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 (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 „einmal freigegeben, immer freigegeben“ bestätigt. Danach das Testkonto als `restricted` markieren und wiederholen: Jetzt muss jeder Lauf freigegeben werden. +3. Zweiten Push im selben PR machen: Läuft er jetzt ohne erneute Freigabe? Dann ist „einmal freigegeben, immer freigegeben“ bestätigt. +4. Aufräumen: Testkonto und Fork **löschen** (dauerhaft vertrautes Konto!). Falls das nicht geht: mindestens `restricted` setzen und + Anmeldung verbieten (Administration -> Benutzerkonten). + +Hinweis: Der Test mit `restricted` (jeder Lauf einzeln freigegeben) ist eventuell nicht durchführbar, weil eingeschränkte Nutzer +vermutlich keine öffentlichen Repos ohne Zuordnung sehen (**unverifiziert**, siehe C.2). Dann entfällt er; die Quelltext-Aussage bleibt. ### B. Auf dem Mac (Runner/dind) 1. Labels sind keine Grenze: Ein Fork-PR kann `runs-on: docker-host` selbst setzen. Eine Trennung `docker` / `docker-host` im @@ -62,18 +72,28 @@ ersten externen Beitrag, spätestens vor Release, entweder 3. umsetzen oder das 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). +3. Notbremse: Runner-Prozess auf dem Mac beenden (z. B. `launchctl unload` des Dienstes oder Prozess/Container stoppen; der genaue Weg hängt von der Einrichtung ab, **unverifiziert**). In Forgejo (Einstellungen -> Actions -> Runner) gibt es in 8.0.3 evtl. nur Bearbeiten/Löschen statt Deaktivieren (**unverifiziert**); Löschen macht den Runner unbrauchbar, bis er neu registriert wird. 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 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. +1. Fork-PR-Läufe nicht freigeben (siehe Entscheidung 2.). +2. Fremde Konten als `restricted` zu führen ist mit Vorsicht zu genießen: Eingeschränkte Nutzer sehen vermutlich keine öffentlichen + Repos ohne explizite Zuordnung (**unverifiziert**). Dann könnten sie keinen Fork/PR anlegen, d. h. `restricted` sperrt externe + Beiträge faktisch aus. Das ist eine Alternative zum Repo-privat-Stellen, kein Mittel für „jeder Lauf einzeln freigeben“. + Erst mit dem Testkonto aus A prüfen. +3. Optional: Branch-Schutz für `main` mit geschützten Dateimustern (UI-Bezeichnung in 8.0.3 **unverifiziert**, bei + Gitea „Protected file patterns“) `.forgejo/**;e2e/**;supabase/config.toml`. Achtung, vermutlich blockiert das auch das + Mergen von PRs, die solche Dateien ändern, also auch deine eigenen CI-PRs (**unverifiziert**); nur nutzen, wenn du dafür eine + Ausnahme (Admin-Merge/Whitelist) hast, sonst weglassen. Es schützt nur `main`, nicht die PR-Läufe. +4. Repo privat stellen, wenn externe Beiträge nicht gewollt sind (löst das Problem vollständig). +5. Runner-Token rotieren (Registrierungs-Token ist kein Dauer-Geheimnis, das `.runner` auf dem Mac enthält das Laufzeit-Token): + Runner in Forgejo (Einstellungen -> Actions -> Runner) löschen, im selben Dialog ein neues Registrierungs-Token erzeugen, den + Runner auf dem Mac mit `forgejo-runner register` neu registrieren und die alte `.runner` löschen. Vorschlag: alle 3 Monate + sowie sofort bei Verdacht. Token nie in Chats, Logs oder ins Repo kopieren. ## Konsequenzen -- 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. +- Fork-PRs bekommen ohne Freigabe kein CI (der ganze Lauf wartet, mit allen Jobs). Eine Freigabe vertraut dem Nutzer den ganzen + Runner dauerhaft an. Fremde Beiträge daher als Branch ins Haupt-Repo übernehmen (dort läuft CI ohne Freigabe, weil Schreibrecht). +- Externe Beiträge laufen nicht auf `docker-host`, bis der Runner isoliert ist oder das Repo privat ist. +- Der `if:`-Guard ändert daran nichts (siehe Entscheidung 1). - Offen: Ergebnis der Prüfung A und Umsetzung B. Bis dahin Issue #43 nicht schließen. diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 0000000..c18c2c8 --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,7 @@ +# Architecture Decision Records + +| Nr. | Titel | Status | +|---|---|---| +| [0001](0001-tech-stack.md) | Tech-Stack | Angenommen | +| [0002](0002-hosting.md) | Hosting: Supabase selbst gehostet auf Hetzner | Angenommen | +| [0003](0003-ci-runner-sicherheit.md) | CI-Sicherheit: Fork-PRs und Runner `docker-host` | Vorgeschlagen | -- 2.45.2 From 6c4e2cc2e919a4587060f68d7e49d739b2e5c93c Mon Sep 17 00:00:00 2001 From: cschlaefke Date: Tue, 3 Nov 2026 13:59:02 +0100 Subject: [PATCH 4/4] docs(ci): align auth-konfiguration guard wording with ADR-0003 Refs #43 Co-Authored-By: Claude Sonnet 5.5 --- docs/auth-konfiguration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/auth-konfiguration.md b/docs/auth-konfiguration.md index 32c7995..5096816 100644 --- a/docs/auth-konfiguration.md +++ b/docs/auth-konfiguration.md @@ -202,7 +202,7 @@ Der Job `e2e` in `.forgejo/workflows/ci.yml` läuft auf dem Runner `docker-host` 4. `.forgejo/scripts/e2e-run.sh` startet den TCP-Relay `e2e/support/tcp-forward.mjs` (127.0.0.1:54321/54322/54324/9998 -> dind-Gateway) und ruft `npx playwright test --project=api --project=web --project=throttle` auf. Der Relay ist nötig, weil die Ports im dind liegen, die Tests, die Web-App (CORS/Origin) und `e2e/support/db.ts` (erlaubt nur lokale Hosts) aber 127.0.0.1 erwarten. Die statische Auslieferung der Web-App auf Port 3000 übernimmt das `webServer` der Playwright-Konfiguration (`serve -s`). 5. Bei Fehlern werden `e2e/playwright-report` und `e2e/test-results` (Traces) als Artefakt `playwright-` hochgeladen. Der Schritt „Aufräumen“ (`if: always()`) entfernt den Extra-Container und stoppt den Supabase-Stack. -Das Projekt `spike` (SMTP-Ausfall, `SPIKE_GOTRUE_URL`) läuft in CI nicht. Der Job hat `needs: supabase-tests` (gleiche Container-Namen und Ports im dind) und läuft nicht für Fork-PRs (nur Komfort-Guard, kein Schutz vor böswilligen Forks): Wer PR-Code auf `docker-host` ausführt, hat Zugriff auf den dind-Daemon des Runners. Absicherung und offene Schritte: `docs/adr/0003-ci-runner-sicherheit.md`. +Das Projekt `spike` (SMTP-Ausfall, `SPIKE_GOTRUE_URL`) läuft in CI nicht. Der Job hat `needs: supabase-tests` (gleiche Container-Namen und Ports im dind) und läuft nicht für Fork-PRs (der Guard hält nur einen unveränderten, bereits freigegebenen Fork-PR von `docker-host` fern; er verhindert weder das Warten auf Freigabe noch einen manipulierten Lauf): Wer PR-Code auf `docker-host` ausführt, hat Zugriff auf den dind-Daemon des Runners. Absicherung und offene Schritte: `docs/adr/0003-ci-runner-sicherheit.md`. **Achtung Artefakte:** Playwright-Reports, Traces und Screenshots (CI-Artefakte, `test-results/`, `playwright-report/`) enthalten Tokens, Passwörter und Mailinhalte der angelegten Testkonten. Die Konten existieren nur in der Wegwerf-Instanz (lokale CLI/CI), dennoch Artefakte nicht in ein öffentliches Repo oder eine öffentliche Release-Seite legen und nur kurz aufbewahren. Die Tests lehnen nicht-lokale Hosts ab (`SUPABASE_URL`, `MAILPIT_URL`, `E2E_DB_URL`; CI-Hosts nur explizit über `E2E_ALLOWED_HOSTS`). -- 2.45.2