diff --git a/internal/audit/append_only_test.go b/internal/audit/append_only_test.go index 94d8c031..0a9d2e5e 100644 --- a/internal/audit/append_only_test.go +++ b/internal/audit/append_only_test.go @@ -4,9 +4,12 @@ import ( "context" "fmt" "os" + "strings" + "sync" "testing" "time" + "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/pgxpool" ) @@ -34,6 +37,110 @@ func setupAppendOnlyTest(t *testing.T) (*Log, *pgxpool.Pool, func()) { ); CREATE OR REPLACE FUNCTION audit_events_prevent_mutation() RETURNS TRIGGER AS $$ BEGIN + RAISE WARNING 'audit_events tamper attempt rejected: op=% current_user=% session_user=% application_name=% client_addr=% row_id=%', + TG_OP, + current_user, + session_user, + current_setting('application_name', true), + inet_client_addr(), + COALESCE(OLD.id::text, 'unknown'); + RAISE EXCEPTION 'audit_events ist append-only: % ist nicht erlaubt', TG_OP; + END; + $$ LANGUAGE plpgsql; + DROP TRIGGER IF EXISTS audit_events_no_update ON audit_events; + CREATE TRIGGER audit_events_no_update + BEFORE UPDATE ON audit_events + FOR EACH ROW EXECUTE FUNCTION audit_events_prevent_mutation(); + DROP TRIGGER IF EXISTS audit_events_no_delete ON audit_events; + CREATE TRIGGER audit_events_no_delete + BEFORE DELETE ON audit_events + FOR EACH ROW EXECUTE FUNCTION audit_events_prevent_mutation(); + DO $do$ + BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'nexarch_app') THEN + CREATE ROLE nexarch_app NOLOGIN; + END IF; + END + $do$; + REVOKE ALL ON audit_events FROM PUBLIC; + REVOKE ALL ON audit_events FROM nexarch_app; + GRANT INSERT, SELECT ON audit_events TO nexarch_app; + `); err != nil { + t.Fatalf("schema: %v", err) + } + + cleanup := func() { + pool.Close() + } + return NewLog(pool), pool, cleanup +} + +// notice sammelt eine ueber pgx' OnNotice-Hook empfangene Server-Meldung +// (z.B. RAISE WARNING) mitsamt Zeitpunkt des Empfangs. +type capturedNotices struct { + mu sync.Mutex + msgs []string +} + +func (c *capturedNotices) add(n *pgconn.Notice) { + c.mu.Lock() + defer c.mu.Unlock() + c.msgs = append(c.msgs, n.Message) +} + +func (c *capturedNotices) snapshot() []string { + c.mu.Lock() + defer c.mu.Unlock() + out := make([]string, len(c.msgs)) + copy(out, c.msgs) + return out +} + +// setupAppendOnlyTestWithNoticeCapture ist wie setupAppendOnlyTest, liefert +// aber zusaetzlich einen Sammler fuer RAISE WARNING/NOTICE-Meldungen der +// Verbindung (Pruefung 2: Ablehnungsversuch wird selbst sichtbar +// protokolliert). +func setupAppendOnlyTestWithNoticeCapture(t *testing.T) (*Log, *pgxpool.Pool, *capturedNotices, func()) { + t.Helper() + adminDSN := os.Getenv("TEST_ADMIN_DSN") + if adminDSN == "" { + t.Skip("TEST_ADMIN_DSN nicht gesetzt, Integrationstest uebersprungen") + } + ctx := context.Background() + + notices := &capturedNotices{} + + cfg, err := pgxpool.ParseConfig(adminDSN) + if err != nil { + t.Fatalf("parse dsn: %v", err) + } + cfg.ConnConfig.OnNotice = func(_ *pgconn.PgConn, n *pgconn.Notice) { + notices.add(n) + } + + pool, err := pgxpool.NewWithConfig(ctx, cfg) + if err != nil { + t.Fatalf("pool: %v", err) + } + if _, err := pool.Exec(ctx, ` + CREATE TABLE IF NOT EXISTS audit_events ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + occurred_at TIMESTAMPTZ NOT NULL DEFAULT now(), + tenant_slug TEXT NOT NULL CHECK (tenant_slug <> ''), + actor TEXT NOT NULL CHECK (actor <> ''), + action TEXT NOT NULL CHECK (action <> ''), + target TEXT NOT NULL, + metadata JSONB NOT NULL DEFAULT '{}'::jsonb + ); + CREATE OR REPLACE FUNCTION audit_events_prevent_mutation() RETURNS TRIGGER AS $$ + BEGIN + RAISE WARNING 'audit_events tamper attempt rejected: op=% current_user=% session_user=% application_name=% client_addr=% row_id=%', + TG_OP, + current_user, + session_user, + current_setting('application_name', true), + inet_client_addr(), + COALESCE(OLD.id::text, 'unknown'); RAISE EXCEPTION 'audit_events ist append-only: % ist nicht erlaubt', TG_OP; END; $$ LANGUAGE plpgsql; @@ -52,7 +159,7 @@ func setupAppendOnlyTest(t *testing.T) (*Log, *pgxpool.Pool, func()) { cleanup := func() { pool.Close() } - return NewLog(pool), pool, cleanup + return NewLog(pool), pool, notices, cleanup } // Akzeptanzkriterium 1 + Pruefung 1: direkter UPDATE/DELETE-Versuch wird von @@ -95,3 +202,152 @@ func TestAppendOnly_RejectsUpdateAndDelete(t *testing.T) { t.Fatalf("eintrag haette trotz fehlgeschlagener update/delete-versuche erhalten bleiben muessen, count=%d", count) } } + +// Akzeptanzkriterium 1 + Pruefung 1: die vom Core-Prozess verwendete +// Datenbankrolle (Mitglied der Gruppenrolle nexarch_app) hat auf DB-Ebene +// gar keine UPDATE/DELETE-Rechte auf audit_events — der Fehler muss ein +// Rechte-Fehler sein, nicht der Trigger-Fehler (der greift erst danach). +func TestAppendOnly_AppRoleHasNoUpdateOrDeletePrivilege(t *testing.T) { + log, adminPool, cleanup := setupAppendOnlyTest(t) + defer cleanup() + ctx := context.Background() + + tenantSlug := fmt.Sprintf("test_appendonly_role_%d", time.Now().UnixNano()) + if err := log.Record(ctx, Event{ + TenantSlug: tenantSlug, + Actor: "alice", + Action: "test.event", + Target: "x", + }); err != nil { + t.Fatalf("record: %v", err) + } + + loginRole := fmt.Sprintf("test_aud07_app_%d", time.Now().UnixNano()) + password := "aud07-test-only" + if _, err := adminPool.Exec(ctx, fmt.Sprintf( + `CREATE ROLE %s LOGIN PASSWORD '%s' IN ROLE nexarch_app`, loginRole, password, + )); err != nil { + t.Fatalf("testrolle anlegen: %v", err) + } + defer func() { + // Aktive Verbindungen der Testrolle beenden, bevor sie gedroppt wird. + _, _ = adminPool.Exec(ctx, `SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE usename = $1`, loginRole) + _, _ = adminPool.Exec(ctx, fmt.Sprintf(`DROP ROLE IF EXISTS %s`, loginRole)) + }() + + adminDSN := os.Getenv("TEST_ADMIN_DSN") + cfg, err := pgxpool.ParseConfig(adminDSN) + if err != nil { + t.Fatalf("parse dsn: %v", err) + } + cfg.ConnConfig.User = loginRole + cfg.ConnConfig.Password = password + + appPool, err := pgxpool.NewWithConfig(ctx, cfg) + if err != nil { + t.Fatalf("app-pool: %v", err) + } + defer appPool.Close() + + // INSERT/SELECT muessen weiterhin funktionieren (Akzeptanzkriterium 3). + if _, err := appPool.Exec(ctx, ` + INSERT INTO audit_events (tenant_slug, actor, action, target) + VALUES ($1, 'app-role', 'test.event', 'x')`, tenantSlug); err != nil { + t.Fatalf("insert mit app-rolle sollte erlaubt sein: %v", err) + } + var n int + if err := appPool.QueryRow(ctx, `SELECT count(*) FROM audit_events WHERE tenant_slug = $1`, tenantSlug).Scan(&n); err != nil { + t.Fatalf("select mit app-rolle sollte erlaubt sein: %v", err) + } + if n != 2 { + t.Fatalf("erwartet 2 eintraege nach insert der app-rolle, habe %d", n) + } + + _, err = appPool.Exec(ctx, `UPDATE audit_events SET actor = 'mallory' WHERE tenant_slug = $1`, tenantSlug) + if err == nil { + t.Fatal("erwartet rechte-fehler bei UPDATE mit app-rolle, habe nil") + } + if !strings.Contains(err.Error(), "permission denied") { + t.Fatalf("erwartet 'permission denied' (fehlende rechte, nicht nur trigger), habe: %v", err) + } + + _, err = appPool.Exec(ctx, `DELETE FROM audit_events WHERE tenant_slug = $1`, tenantSlug) + if err == nil { + t.Fatal("erwartet rechte-fehler bei DELETE mit app-rolle, habe nil") + } + if !strings.Contains(err.Error(), "permission denied") { + t.Fatalf("erwartet 'permission denied' (fehlende rechte, nicht nur trigger), habe: %v", err) + } +} + +// Akzeptanzkriterium 2 + Pruefung 2: der abgelehnte Aenderungsversuch wird +// von der Datenbank selbst sichtbar protokolliert (RAISE WARNING, das +// unabhaengig vom Transaktionsausgang ins Postgres-Log geschrieben und der +// Verbindung als Notice gemeldet wird), nicht nur mit einem generischen +// Fehler quittiert. +func TestAppendOnly_TamperAttemptIsLogged(t *testing.T) { + log, pool, notices, cleanup := setupAppendOnlyTestWithNoticeCapture(t) + defer cleanup() + ctx := context.Background() + + tenantSlug := fmt.Sprintf("test_appendonly_log_%d", time.Now().UnixNano()) + if err := log.Record(ctx, Event{ + TenantSlug: tenantSlug, + Actor: "alice", + Action: "test.event", + Target: "x", + }); err != nil { + t.Fatalf("record: %v", err) + } + + _, err := pool.Exec(ctx, `UPDATE audit_events SET actor = 'mallory' WHERE tenant_slug = $1`, tenantSlug) + if err == nil { + t.Fatal("erwartet fehler bei UPDATE auf audit_events, habe nil") + } + + found := false + for _, msg := range notices.snapshot() { + if strings.Contains(msg, "tamper attempt rejected") && strings.Contains(msg, "op=UPDATE") { + found = true + break + } + } + if !found { + t.Fatalf("erwartet protokollierte tamper-warnung fuer UPDATE, habe notices: %+v", notices.snapshot()) + } +} + +// Akzeptanzkriterium 3 + Pruefung 3: normales Schreiben/Lesen von +// Audit-Eintraegen funktioniert nach der technischen Durchsetzung +// unveraendert. +func TestAppendOnly_NormalWriteAndReadStillWork(t *testing.T) { + log, _, cleanup := setupAppendOnlyTest(t) + defer cleanup() + ctx := context.Background() + + tenantSlug := fmt.Sprintf("test_appendonly_regression_%d", time.Now().UnixNano()) + if err := log.Record(ctx, Event{ + TenantSlug: tenantSlug, + Actor: "alice", + Action: "test.event", + Target: "x", + }); err != nil { + t.Fatalf("record: %v", err) + } + if err := log.Record(ctx, Event{ + TenantSlug: tenantSlug, + Actor: "bob", + Action: "test.event.two", + Target: "y", + }); err != nil { + t.Fatalf("record: %v", err) + } + + count, err := log.CountByTenant(ctx, tenantSlug) + if err != nil { + t.Fatalf("count: %v", err) + } + if count != 2 { + t.Fatalf("erwartet 2 eintraege nach zwei normalen Record-Aufrufen, habe %d", count) + } +} diff --git a/migrations/0010_audit_events_role_enforcement.down.sql b/migrations/0010_audit_events_role_enforcement.down.sql new file mode 100644 index 00000000..3c28f36d --- /dev/null +++ b/migrations/0010_audit_events_role_enforcement.down.sql @@ -0,0 +1,13 @@ +-- Trigger-Funktion auf den AUD-02-Stand zuruecksetzen (ohne RAISE WARNING). +CREATE OR REPLACE FUNCTION audit_events_prevent_mutation() RETURNS TRIGGER AS $$ +BEGIN + RAISE EXCEPTION 'audit_events ist append-only: % ist nicht erlaubt', TG_OP; +END; +$$ LANGUAGE plpgsql; + +REVOKE INSERT, SELECT ON audit_events FROM nexarch_app; +-- PUBLIC hatte vor dieser Migration keine expliziten Rechte auf +-- audit_events (Postgres-Standard: nur der Tabelleneigentuemer), daher kein +-- Re-GRANT hier. Die nexarch_app-Rolle selbst bleibt bestehen (kann von +-- anderen Objekten referenziert sein); nur die audit_events-spezifischen +-- Rechte werden zurueckgenommen. diff --git a/migrations/0010_audit_events_role_enforcement.up.sql b/migrations/0010_audit_events_role_enforcement.up.sql new file mode 100644 index 00000000..568ab351 --- /dev/null +++ b/migrations/0010_audit_events_role_enforcement.up.sql @@ -0,0 +1,47 @@ +-- AUD-07: technische Durchsetzung des Append-only-Versprechens aus AUD-01/ +-- AUD-02 (siehe core-kanban/tickets/AUD-07.md). AUD-02 hat bereits einen +-- Trigger eingefuehrt, der UPDATE/DELETE auf audit_events ablehnt — das +-- schuetzt aber nicht davor, dass eine kompromittierte oder falsch +-- konfigurierte Anwendungsrolle den Trigger per +-- "ALTER TABLE ... DISABLE TRIGGER" oder als Tabelleneigentuemer umgeht. +-- Akzeptanzkriterium 1 verlangt fehlende Rechte, nicht nur einen fehlenden +-- Code-Pfad: die vom Core-Prozess verwendete Datenbankrolle bekommt daher +-- auf DB-Ebene ausschliesslich INSERT/SELECT auf audit_events. +-- +-- nexarch_app ist eine reine Gruppenrolle ohne LOGIN und ohne Passwort +-- (keine Zugangsdaten in Migrationen). Der Betrieb ordnet die tatsaechliche +-- Login-Rolle des Core-Prozesses dieser Gruppe zu: +-- GRANT nexarch_app TO ; +-- Migrationen selbst laufen weiterhin ueber die separate Owner-/Admin-Rolle +-- (NEXARCH_ADMIN_DSN), die von diesem REVOKE nicht betroffen ist. +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'nexarch_app') THEN + CREATE ROLE nexarch_app NOLOGIN; + END IF; +END +$$; + +REVOKE ALL ON audit_events FROM PUBLIC; +REVOKE ALL ON audit_events FROM nexarch_app; +GRANT INSERT, SELECT ON audit_events TO nexarch_app; + +-- Verteidigung in der Tiefe (Akzeptanzkriterium 2): selbst wenn eine Rolle +-- doch UPDATE/DELETE-Rechte haette (z.B. der Tabelleneigentuemer beim +-- Ausfuehren von Migrationen), lehnt der Trigger den Versuch weiterhin ab +-- UND protokolliert ihn selbst. RAISE WARNING wird unabhaengig vom Ausgang +-- der Transaktion sofort ins Postgres-Server-Log geschrieben (nicht +-- transaktional) und zusaetzlich als Notice an die Datenbankverbindung +-- gemeldet, bevor die Aenderung per RAISE EXCEPTION zurueckgewiesen wird. +CREATE OR REPLACE FUNCTION audit_events_prevent_mutation() RETURNS TRIGGER AS $$ +BEGIN + RAISE WARNING 'audit_events tamper attempt rejected: op=% current_user=% session_user=% application_name=% client_addr=% row_id=%', + TG_OP, + current_user, + session_user, + current_setting('application_name', true), + inet_client_addr(), + COALESCE(OLD.id::text, 'unknown'); + RAISE EXCEPTION 'audit_events ist append-only: % ist nicht erlaubt', TG_OP; +END; +$$ LANGUAGE plpgsql;