diff --git a/.gitea/workflows/pflichttest-gate.yml b/.gitea/workflows/pflichttest-gate.yml new file mode 100644 index 0000000..bc229c0 --- /dev/null +++ b/.gitea/workflows/pflichttest-gate.yml @@ -0,0 +1,20 @@ +name: Core-Pflichttest-Gate + +on: + pull_request: {} + +jobs: + pflichttest-gate: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - uses: actions/setup-go@v5 + with: + go-version: "1.22" + - name: Gate bauen + run: go build -o /tmp/pflichttestgate ./cmd/pflichttestgate + - name: Geänderte Dateien gegen Pflichttest-Regel pruefen + run: | + git diff --name-only "origin/${{ github.base_ref }}...HEAD" | /tmp/pflichttestgate diff --git a/cmd/pflichttestgate/main.go b/cmd/pflichttestgate/main.go new file mode 100644 index 0000000..b61aa46 --- /dev/null +++ b/cmd/pflichttestgate/main.go @@ -0,0 +1,41 @@ +// Command pflichttestgate ist das CI-Gate aus docs/TESTSTRATEGIE-CORE.md Abschnitt 5. +// Aufruf: pflichttestgate +// Beendet sich mit Exit-Code 1 und einer Liste der Verstöße, wenn ein sicherheitskritisches +// Package ohne begleitende Testdatei geändert wurde. +package main + +import ( + "bufio" + "fmt" + "os" + + "gitea.perlbach24.de/scripte/nexarch/internal/pflichttestgate" +) + +func main() { + var changedFiles []string + scanner := bufio.NewScanner(os.Stdin) + for scanner.Scan() { + line := scanner.Text() + if line != "" { + changedFiles = append(changedFiles, line) + } + } + if err := scanner.Err(); err != nil { + fmt.Fprintf(os.Stderr, "pflichttestgate: Eingabe konnte nicht gelesen werden: %v\n", err) + os.Exit(2) + } + + violations := pflichttestgate.CheckDiff(changedFiles) + if len(violations) == 0 { + fmt.Println("pflichttestgate: bestanden — alle sicherheitskritischen Änderungen haben begleitende Tests.") + return + } + + fmt.Fprintln(os.Stderr, "pflichttestgate: FEHLGESCHLAGEN — Pflichttest fehlt für:") + for _, v := range violations { + fmt.Fprintf(os.Stderr, " - Package %q (Datei %q hat keine begleitende _test.go-Änderung)\n", v.Package, v.ChangedFile) + } + fmt.Fprintln(os.Stderr, "\nSiehe docs/TESTSTRATEGIE-CORE.md Abschnitt 3.") + os.Exit(1) +} diff --git a/docs/TESTSTRATEGIE-CORE.md b/docs/TESTSTRATEGIE-CORE.md new file mode 100644 index 0000000..7f013c8 --- /dev/null +++ b/docs/TESTSTRATEGIE-CORE.md @@ -0,0 +1,81 @@ +# NEXARCH Core – Teststrategie + +Stand: 2026-08-28. Ticket: QA-01. + +## 1. Warum dieses Dokument existiert + +archivdms hatte zur Zeit der Bestandsaufnahme nur 2 `_test.go`-Dateien in ganz `internal/`, davon 0 für +`auth`/`storage`/`permissions.go`. archivmail testete 2 von 18 Modulen. Beide Lücken wurden erst im +Betrieb entdeckt, nicht vor dem Merge. NEXARCH Core übernimmt daraus einen Grundsatz: **Testpflicht für +Auth, Tenant-Scoping und Policy-Enforcement ist ein Merge-Gate, keine Nachrüstung.** + +## 2. Testpyramide + +| Ebene | Werkzeug | Umfang | +|---|---|---| +| Unit | `go test` (Standardbibliothek) | Einzelne Funktionen/Typen, keine externe Abhängigkeit (DB, Netzwerk) | +| Integration | `go test` gegen echte PostgreSQL-Instanz (`nexarch_test`-Rolle) | Repository-/Handler-Schicht, Tenant-Scoping, Policy-Enforcement | +| Vertragstests | `internal/contracttest` (siehe QA-07) | Öffentliche API-Verträge zwischen Core und Modulen | +| Last-/Leistungstests | `internal/loadtest` (siehe QA-08) | Mehrmodul-Last, JWT-Verifikation, Connection-Pooling | +| Extern | Core `QA-06` Penetrationstest | Vor Produktivbetrieb, außerhalb dieses CI-Gates | + +Diese Kachel (QA-01) legt die Pflichtebenen fest und erzwingt sie technisch für die drei sicherheitskritischsten Bereiche; sie ersetzt nicht QA-07/QA-08, die eigene, bereits umgesetzte CI-Gates haben. + +## 3. Pflichttests als Merge-Gate + +Verbindlich für jeden Pull Request, der Dateien in einem der folgenden Bereiche ändert: + +- **Auth** (`internal/auth/`, `internal/iam/`, Login/Session/2FA/SSO-Pakete) +- **Tenant-Scoping** (`internal/tenant/`, jede Repository-Schicht mit `tenant_id`-Filterung) +- **Policy-Enforcement** (`internal/rbac/`, `internal/policy/`, jede Autorisierungsprüfung) + +Regel: **jede geänderte `.go`-Datei in einem dieser Bereiche muss von einer geänderten oder neuen +`_test.go`-Datei im selben Package begleitet sein.** Das CI-Gate (Abschnitt 5) prüft das automatisiert +und blockiert den Merge, wenn die Regel verletzt ist — analog zum bereits etablierten Sprintf-Verbot für +SQL (siehe `SICHERHEITSKONZEPT.md`), nur als technisch erzwungene statt nur dokumentierte Regel. + +Diese Regel gilt projektweit für alle sieben Boards, nicht nur Core — siehe die entsprechenden +Akzeptanzkriterien in den `QA-01`-Tickets von DMS, Mail, Archive, Workflow, AI, Connect +(`SICHERHEITSKONZEPT.md` Abschnitt 11, Punkt 8, 2026-08-28 geklärt). + +## 4. Testdatenbank-Strategie + +Isolation zwischen parallelen Testläufen ist die zentrale Lehre aus dem bisherigen Testbetrieb +(siehe Projekt-Testinfrastruktur): mehrere Go-Testpakete teilen sich dieselbe physische PostgreSQL-Instanz +auf dem Testhost, aber jedes Paket braucht einen isolierten Datenbestand. + +- **Rolle `nexarch_test`**: `CREATEDB`, kein Superuser, einmalig eingerichtet über `scripts/setup-test-env.sh`. +- **Reset vor jedem Testlauf**: `scripts/reset-test-env.sh` droppt die geteilte `tenants`-Registry-Tabelle + und alle `tenant_*`-Datenbanken in der `postgres`-Wartungs-DB. Nötig, weil verschiedene Branches + unterschiedliche Registry-Schemata erwarten, aber dieselbe physische Instanz teilen. +- **`-p 1` ist Pflicht** für `go test ./...`, sobald mehrere Pakete gegen die geteilte Registry-Tabelle + testen (z. B. `internal/tenant` + `internal/migrate`). Ohne `-p 1` laufen Paket-Testbinaries parallel + gegen dieselbe physische PostgreSQL-Instanz, ihre Registry-Einträge/DBs kollidieren + (falsche Tenant-Zählungen, „database already exists"-Fehler). +- **Isolation innerhalb eines Testlaufs**: jeder Test, der eine Tenant-Datenbank braucht, provisioniert + seine eigene, eindeutig benannte `tenant_*`-DB über dieselbe Provisionierungs-Logik wie die + Anwendung selbst (`TEN-01`) und räumt sie in einem `t.Cleanup()` wieder ab — keine geteilten + Fixture-Datenbanken zwischen Testfällen. + +Siehe `internal/testdbisolation/isolation_test.go` (dieses Ticket) für den automatisierten Nachweis, +dass zwei parallel laufende Tenant-Provisionierungen sich nicht gegenseitig sehen. + +## 5. CI-Gate + +`.gitea/workflows/pflichttest-gate.yml` führt `cmd/pflichttestgate` gegen den PR-Diff aus +(`git diff --name-only origin/...HEAD`). Das Programm: + +1. Filtert die geänderten Dateien auf die in Abschnitt 3 genannten Pfad-Muster. +2. Prüft je betroffenem Go-Package, ob mindestens eine `_test.go`-Datei desselben Packages ebenfalls + im Diff enthalten ist. +3. Beendet sich mit Exit-Code 1 und einer Liste der betroffenen Packages ohne Teständerung, wenn die + Regel verletzt ist — der CI-Job schlägt dann fehl, der Merge ist blockiert. + +Negativtest des Gates selbst: `internal/pflichttestgate/gate_test.go` enthält einen Testfall, der einen +Diff mit geänderter `internal/auth/login.go` ohne begleitende Testdatei simuliert und erwartet, dass das +Gate das als Verstoß erkennt (Prüfung 1 dieses Tickets). + +## 6. Dokumentation & Gegenlesen + +Dieses Dokument ist von einer zweiten Person gegenzulesen, bevor die Kachel als abgeschlossen gilt +(Akzeptanzkriterium 3 / Prüfung 3). Fund/Freigabe wird im Pull Request vermerkt. diff --git a/internal/pflichttestgate/gate.go b/internal/pflichttestgate/gate.go new file mode 100644 index 0000000..3229dea --- /dev/null +++ b/internal/pflichttestgate/gate.go @@ -0,0 +1,67 @@ +// Package pflichttestgate erzwingt die in docs/TESTSTRATEGIE-CORE.md Abschnitt 3 festgelegte Regel: +// jede geänderte Go-Datei in einem sicherheitskritischen Bereich (Auth, Tenant-Scoping, Policy) +// muss von einer geänderten oder neuen _test.go-Datei im selben Package begleitet sein. +package pflichttestgate + +import ( + "path" + "regexp" + "strings" +) + +// sensitivePathPatterns beschreibt die Bereiche aus TESTSTRATEGIE-CORE.md Abschnitt 3. +var sensitivePathPatterns = []*regexp.Regexp{ + regexp.MustCompile(`(^|/)internal/auth/`), + regexp.MustCompile(`(^|/)internal/iam/`), + regexp.MustCompile(`(^|/)internal/tenant/`), + regexp.MustCompile(`(^|/)internal/rbac/`), + regexp.MustCompile(`(^|/)internal/policy/`), +} + +// Violation beschreibt ein Package mit sicherheitskritischer Änderung ohne begleitende Testdatei. +type Violation struct { + Package string + ChangedFile string +} + +// isSensitive meldet, ob eine geänderte Datei in einen der Pflichttest-Bereiche fällt. +func isSensitive(file string) bool { + if !strings.HasSuffix(file, ".go") || strings.HasSuffix(file, "_test.go") { + return false + } + for _, re := range sensitivePathPatterns { + if re.MatchString(file) { + return true + } + } + return false +} + +// CheckDiff prüft eine Liste geänderter Dateipfade (z. B. aus `git diff --name-only`) gegen die +// Pflichttest-Regel und gibt für jedes betroffene Package ohne begleitende Testdatei eine Violation +// zurück. Ein leeres Ergebnis bedeutet: Gate besteht. +func CheckDiff(changedFiles []string) []Violation { + sensitiveByPkg := map[string]string{} // package -> erste sensible Datei (fürs Reporting) + testTouchedPkgs := map[string]bool{} + + for _, f := range changedFiles { + pkg := path.Dir(f) + if strings.HasSuffix(f, "_test.go") { + testTouchedPkgs[pkg] = true + continue + } + if isSensitive(f) { + if _, seen := sensitiveByPkg[pkg]; !seen { + sensitiveByPkg[pkg] = f + } + } + } + + var violations []Violation + for pkg, file := range sensitiveByPkg { + if !testTouchedPkgs[pkg] { + violations = append(violations, Violation{Package: pkg, ChangedFile: file}) + } + } + return violations +} diff --git a/internal/pflichttestgate/gate_test.go b/internal/pflichttestgate/gate_test.go new file mode 100644 index 0000000..94ea5f7 --- /dev/null +++ b/internal/pflichttestgate/gate_test.go @@ -0,0 +1,84 @@ +package pflichttestgate + +import "testing" + +// Prüfung 1 (QA-01): CI-Lauf mit absichtlich fehlendem Pflichttest schlägt kontrolliert fehl — +// Negativtest des Gates selbst. +func TestCheckDiff_BlocksAuthChangeWithoutTest(t *testing.T) { + changed := []string{ + "internal/auth/login.go", + "internal/apiserver/handler.go", // unkritisch, keine Testpflicht + } + + violations := CheckDiff(changed) + + if len(violations) != 1 { + t.Fatalf("erwartet 1 Verstoß, bekommen %d: %+v", len(violations), violations) + } + if violations[0].Package != "internal/auth" { + t.Errorf("erwartetes Package internal/auth, bekommen %q", violations[0].Package) + } + if violations[0].ChangedFile != "internal/auth/login.go" { + t.Errorf("erwartete Datei internal/auth/login.go, bekommen %q", violations[0].ChangedFile) + } +} + +func TestCheckDiff_PassesWhenTestFileAccompaniesChange(t *testing.T) { + changed := []string{ + "internal/auth/login.go", + "internal/auth/login_test.go", + } + + violations := CheckDiff(changed) + + if len(violations) != 0 { + t.Fatalf("erwartet keinen Verstoß, bekommen %+v", violations) + } +} + +func TestCheckDiff_PassesForNewTestFileEvenWithoutSourceChange(t *testing.T) { + // Nachtraeglich ergaenzter Test fuer bestehenden Code ist erlaubt/erwuenscht. + changed := []string{"internal/tenant/registry_test.go"} + + violations := CheckDiff(changed) + + if len(violations) != 0 { + t.Fatalf("erwartet keinen Verstoß, bekommen %+v", violations) + } +} + +func TestCheckDiff_IgnoresUnrelatedPackages(t *testing.T) { + changed := []string{"internal/config/config.go", "internal/db/db.go"} + + violations := CheckDiff(changed) + + if len(violations) != 0 { + t.Fatalf("erwartet keinen Verstoß fuer unkritische Pakete, bekommen %+v", violations) + } +} + +func TestCheckDiff_CoversAllSensitiveAreas(t *testing.T) { + cases := []string{ + "internal/auth/session.go", + "internal/iam/user.go", + "internal/tenant/provisioner.go", + "internal/rbac/enforcer.go", + "internal/policy/decision.go", + } + for _, f := range cases { + violations := CheckDiff([]string{f}) + if len(violations) != 1 { + t.Errorf("Datei %q sollte als sensibel erkannt werden, Verstöße: %+v", f, violations) + } + } +} + +func TestCheckDiff_IgnoresNonGoFiles(t *testing.T) { + changed := []string{"internal/auth/README.md"} + + violations := CheckDiff(changed) + + if len(violations) != 0 { + t.Fatalf("Nicht-Go-Dateien duerfen keine Testpflicht ausloesen, bekommen %+v", violations) + } +} diff --git a/internal/testdbisolation/isolation.go b/internal/testdbisolation/isolation.go new file mode 100644 index 0000000..ec7511b --- /dev/null +++ b/internal/testdbisolation/isolation.go @@ -0,0 +1,76 @@ +// Package testdbisolation demonstriert und prüft die in docs/TESTSTRATEGIE-CORE.md Abschnitt 4 +// beschriebene Testdatenbank-Isolation: jeder Test, der eine Tenant-Datenbank braucht, +// provisioniert seine eigene, eindeutig benannte DB und räumt sie über Cleanup wieder ab — +// keine geteilten Fixture-Datenbanken zwischen parallelen Testläufen. +package testdbisolation + +import ( + "context" + "fmt" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgxpool" +) + +// ProvisionEphemeralDB legt unter dem gegebenen Admin-DSN eine neue, eindeutig benannte Datenbank an +// und liefert eine Cleanup-Funktion, die sie wieder droppt. adminDSN muss auf eine Wartungs-DB +// (z. B. "postgres") zeigen, name muss ein gültiger, bereits eindeutiger Bezeichner sein. +func ProvisionEphemeralDB(ctx context.Context, adminDSN, name string) (cleanup func(context.Context) error, err error) { + conn, err := pgx.Connect(ctx, adminDSN) + if err != nil { + return nil, fmt.Errorf("verbindung zur wartungs-db fehlgeschlagen: %w", err) + } + defer conn.Close(ctx) + + if _, err := conn.Exec(ctx, fmt.Sprintf("CREATE DATABASE %s", pgx.Identifier{name}.Sanitize())); err != nil { + return nil, fmt.Errorf("datenbank %q konnte nicht angelegt werden: %w", name, err) + } + + cleanup = func(cleanupCtx context.Context) error { + adminConn, err := pgx.Connect(cleanupCtx, adminDSN) + if err != nil { + return fmt.Errorf("cleanup: verbindung fehlgeschlagen: %w", err) + } + defer adminConn.Close(cleanupCtx) + _, err = adminConn.Exec(cleanupCtx, fmt.Sprintf("DROP DATABASE IF EXISTS %s", pgx.Identifier{name}.Sanitize())) + return err + } + return cleanup, nil +} + +// WriteMarker schreibt einen eindeutigen Beleg-Datensatz in die angegebene Datenbank (verbindet +// per dsnForDB), um im Test nachzuweisen, dass zwei parallele Provisionierungen sich nicht sehen. +func WriteMarker(ctx context.Context, dsn, table, value string) error { + pool, err := pgxpool.New(ctx, dsn) + if err != nil { + return err + } + defer pool.Close() + + if _, err := pool.Exec(ctx, fmt.Sprintf( + "CREATE TABLE IF NOT EXISTS %s (value text primary key)", pgx.Identifier{table}.Sanitize(), + )); err != nil { + return err + } + _, err = pool.Exec(ctx, fmt.Sprintf("INSERT INTO %s (value) VALUES ($1)", pgx.Identifier{table}.Sanitize()), value) + return err +} + +// CountMarkers zählt die Belegzeilen in der angegebenen Datenbank/Tabelle. +func CountMarkers(ctx context.Context, dsn, table string) (int, error) { + pool, err := pgxpool.New(ctx, dsn) + if err != nil { + return 0, err + } + defer pool.Close() + + if _, err := pool.Exec(ctx, fmt.Sprintf( + "CREATE TABLE IF NOT EXISTS %s (value text primary key)", pgx.Identifier{table}.Sanitize(), + )); err != nil { + return 0, err + } + + var n int + err = pool.QueryRow(ctx, fmt.Sprintf("SELECT count(*) FROM %s", pgx.Identifier{table}.Sanitize())).Scan(&n) + return n, err +} diff --git a/internal/testdbisolation/isolation_test.go b/internal/testdbisolation/isolation_test.go new file mode 100644 index 0000000..488185e --- /dev/null +++ b/internal/testdbisolation/isolation_test.go @@ -0,0 +1,99 @@ +package testdbisolation + +import ( + "context" + "fmt" + "os" + "strings" + "sync" + "testing" + "time" +) + +// TestParallelProvisioning_AreIsolated ist der automatisierte Nachweis für QA-01 Prüfung 2: +// Testdatenbank-Isolation zwischen parallelen Testläufen. Zwei Provisionierungen laufen +// gleichzeitig, jede schreibt einen Beleg-Datensatz in ihre eigene DB — keine darf den +// Beleg der jeweils anderen sehen. +// +// Braucht TEST_ADMIN_DSN (siehe Projekt-Testinfrastruktur), läuft auf dem Testhost, nicht lokal +// auf der Entwicklungsmaschine (siehe "no installs"-Regel) — wird ohne gesetzte Variable übersprungen. +func TestParallelProvisioning_AreIsolated(t *testing.T) { + adminDSN := os.Getenv("TEST_ADMIN_DSN") + if adminDSN == "" { + t.Skip("TEST_ADMIN_DSN nicht gesetzt — Test läuft nur auf dem Testhost, siehe docs/TESTSTRATEGIE-CORE.md") + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + const numParallel = 2 + dbNames := make([]string, numParallel) + for i := range dbNames { + dbNames[i] = fmt.Sprintf("qa01_isolation_%d_%d", time.Now().UnixNano(), i) + } + + var wg sync.WaitGroup + errs := make([]error, numParallel) + cleanups := make([]func(context.Context) error, numParallel) + + for i, name := range dbNames { + wg.Add(1) + go func(idx int, dbName string) { + defer wg.Done() + + cleanup, err := ProvisionEphemeralDB(ctx, adminDSN, dbName) + if err != nil { + errs[idx] = fmt.Errorf("provisionierung %d fehlgeschlagen: %w", idx, err) + return + } + cleanups[idx] = cleanup + + dbDSN := dsnForDB(adminDSN, dbName) + // Jede Goroutine schreibt einen für sie eindeutigen Beleg. + if err := WriteMarker(ctx, dbDSN, "isolation_marker", fmt.Sprintf("marker-von-goroutine-%d", idx)); err != nil { + errs[idx] = fmt.Errorf("markierung %d fehlgeschlagen: %w", idx, err) + } + }(i, name) + } + wg.Wait() + + t.Cleanup(func() { + for _, cleanup := range cleanups { + if cleanup != nil { + _ = cleanup(context.Background()) + } + } + }) + + for i, err := range errs { + if err != nil { + t.Fatalf("goroutine %d: %v", i, err) + } + } + + // Isolationsnachweis: jede DB enthält genau ihren eigenen Beleg (1), keinen fremden. + for i, name := range dbNames { + dbDSN := dsnForDB(adminDSN, name) + count, err := CountMarkers(ctx, dbDSN, "isolation_marker") + if err != nil { + t.Fatalf("zaehlung in db %d fehlgeschlagen: %v", i, err) + } + if count != 1 { + t.Errorf("db %d (%s): erwartet genau 1 eigenen Beleg, gefunden %d — Isolation verletzt", i, name, count) + } + } +} + +// dsnForDB ersetzt den Datenbanknamen im Wartungs-DSN durch den gegebenen Namen. +func dsnForDB(adminDSN, dbName string) string { + idx := strings.LastIndex(adminDSN, "/") + if idx == -1 { + return adminDSN + } + base := adminDSN[:idx+1] + rest := adminDSN[idx+1:] + if q := strings.Index(rest, "?"); q != -1 { + return base + dbName + rest[q:] + } + return base + dbName +}