fix(audit): bring authentication events under the tamper-evident chain

internal/app/audit/service.go

@@ -201,7 +201,17 @@ func (s *AuditService) VerifyChain(ctx context.Context, tenantID shared.ID, limi
 		// 1. Fetch the original audit_log. If it's gone, flag it — a
 		//    deleted row is a tamper signal (FK ON DELETE RESTRICT
 		//    blocks it in production but not in every path).
-		log, err := s.auditRepo.GetByTenantAndID(ctx, tenantID, e.AuditLogID)
+		// System-chain entries point at audit_logs rows with tenant_id IS NULL,
+		// which the tenant-scoped getter cannot see — using it here would
+		// report every one of them as audit_log_missing, i.e. a fabricated
+		// tamper signal on the chain that exists to detect real ones.
+		var log *auditdom.AuditLog
+		var err error
+		if tenantID == auditdom.SystemChainTenantID {
+			log, err = s.auditRepo.GetSystemByID(ctx, e.AuditLogID)
+		} else {
+			log, err = s.auditRepo.GetByTenantAndID(ctx, tenantID, e.AuditLogID)
+		}
 		if err != nil {
 			res.Breaks = append(res.Breaks, ChainBreak{
 				AuditLogID:    e.AuditLogID.String(),
@@ -319,11 +329,15 @@ func (s *AuditService) RebaselineChain(ctx context.Context, tenantID shared.ID,
 // (pg_advisory_xact_lock) and is not wired here because the current
 // deployment is single-replica.
 func (s *AuditService) appendChainEntry(ctx context.Context, log *auditdom.AuditLog) {
-	tenantPtr := log.TenantID()
-	if tenantPtr == nil {
-		return // system-level events bypass the per-tenant chain
+	// Tenant-less events (every auth.login / auth.register / auth.failed —
+	// 86% of the trail on the live database) used to return here, which left
+	// them with no tamper evidence at all. They now extend a dedicated system
+	// chain instead. See auditdom.SystemChainTenantID for why a sentinel
+	// rather than a nullable column.
+	tid := auditdom.SystemChainTenantID
+	if tenantPtr := log.TenantID(); tenantPtr != nil {
+		tid = *tenantPtr
 	}
-	tid := *tenantPtr
 
 	s.chainMu.Lock()
 	defer s.chainMu.Unlock()

internal/app/audit/system_chain_db_test.go

@@ -0,0 +1,182 @@
+package audit_test
+
+import (
+	"context"
+	"database/sql"
+	"os"
+	"testing"
+
+	_ "github.com/lib/pq"
+
+	auditapp "github.com/openctemio/api/internal/app/audit"
+	"github.com/openctemio/api/internal/infra/postgres"
+	auditdom "github.com/openctemio/api/pkg/domain/audit"
+	"github.com/openctemio/api/pkg/logger"
+)
+
+// package audit_test, not audit: this test needs the real postgres repository,
+// and internal/infra/postgres -> internal/app -> internal/app/audit, so an
+// in-package test would be an import cycle. An external test package can
+// depend on packages that depend on the one under test.
+//
+// Driven through the real repository against a real database, because the
+// question this answers is not "does the Go branch take the right path" but
+// "does a row land in audit_log_chain". A mock would answer the first and
+// prove nothing about the second — and the whole reason this gap existed for
+// months is that the components were each individually correct.
+
+func openAuditDB(t *testing.T) *postgres.DB {
+	t.Helper()
+
+	dbURL := os.Getenv("DATABASE_URL")
+	if dbURL == "" {
+		t.Skip("DATABASE_URL not set; skipping audit system-chain DB tests")
+	}
+	db, err := sql.Open("postgres", dbURL)
+	if err != nil {
+		t.Skipf("open db: %v", err)
+	}
+	t.Cleanup(func() { _ = db.Close() })
+	if err := db.PingContext(context.Background()); err != nil {
+		t.Skipf("cannot reach DATABASE_URL: %v", err)
+	}
+	return &postgres.DB{DB: db}
+}
+
+// logSystemAuthEvent writes one tenant-less auth event the way the production
+// helper does, and returns its audit_log id.
+func logSystemAuthEvent(ctx context.Context, t *testing.T, db *postgres.DB) string {
+	t.Helper()
+
+	repo := postgres.NewAuditRepository(db)
+	svc := auditapp.NewAuditService(repo, logger.NewNop())
+
+	// No TenantID: this is what every auth.login carries.
+	if err := svc.LogUserLogin(ctx, auditapp.AuditContext{}, "", "chain-test@example.test"); err != nil {
+		t.Fatalf("log login event: %v", err)
+	}
+
+	var id string
+	if err := db.QueryRowContext(ctx, `
+		SELECT id FROM audit_logs
+		 WHERE tenant_id IS NULL AND resource_name = $1
+		 ORDER BY logged_at DESC LIMIT 1`,
+		"chain-test@example.test").Scan(&id); err != nil {
+		t.Fatalf("read back the audit log: %v", err)
+	}
+	t.Cleanup(func() {
+		bg := context.Background()
+		_, _ = db.ExecContext(bg, `DELETE FROM audit_log_chain WHERE audit_log_id = $1`, id)
+		_, _ = db.ExecContext(bg, `DELETE FROM audit_logs WHERE id = $1`, id)
+	})
+	return id
+}
+
+// The defect: an auth event produced no chain row at all, so deleting or
+// editing it left no evidence.
+func TestSystemChain_AuthEventIsChained(t *testing.T) {
+	ctx := context.Background()
+	db := openAuditDB(t)
+
+	id := logSystemAuthEvent(ctx, t, db)
+
+	var tenantID, hash string
+	err := db.QueryRowContext(ctx,
+		`SELECT tenant_id, hash FROM audit_log_chain WHERE audit_log_id = $1`, id,
+	).Scan(&tenantID, &hash)
+	if err != nil {
+		t.Fatalf("no chain row for a tenant-less auth event (%v). It is stored in "+
+			"audit_logs with no tamper evidence: an intruder can delete the record "+
+			"of their own login and the verifier will report the trail intact, "+
+			"because it only walks rows that were chained", err)
+	}
+
+	if tenantID != auditdom.SystemChainTenantID.String() {
+		t.Errorf("chain row tenant_id = %s, want the system chain sentinel %s",
+			tenantID, auditdom.SystemChainTenantID)
+	}
+	if len(hash) != 64 {
+		t.Errorf("hash = %q, want 64 hex chars", hash)
+	}
+}
+
+// A chain is only evidence if verification agrees with what was written. This
+// catches a write/verify payload mismatch — the failure mode that produced
+// months of false "chain break" alerts once before.
+func TestSystemChain_VerifiesClean(t *testing.T) {
+	ctx := context.Background()
+	db := openAuditDB(t)
+
+	id := logSystemAuthEvent(ctx, t, db)
+
+	repo := postgres.NewAuditRepository(db)
+	svc := auditapp.NewAuditService(repo, logger.NewNop())
+
+	res, err := svc.VerifyChain(ctx, auditdom.SystemChainTenantID, 10_000)
+	if err != nil {
+		t.Fatalf("VerifyChain: %v", err)
+	}
+	if res.Total == 0 {
+		t.Fatal("the system chain verified 0 entries: nothing is being checked")
+	}
+
+	for _, b := range res.Breaks {
+		if b.AuditLogID == id {
+			t.Fatalf("the entry just written verifies as broken (%s). Note "+
+				"audit_log_missing here means the verifier looked the row up with "+
+				"the tenant-scoped getter, which cannot see tenant_id IS NULL rows "+
+				"— a fabricated tamper signal on the chain that exists to detect "+
+				"real ones", b.Reason)
+		}
+	}
+}
+
+// Tenant-scoped events must keep going to their own chain. A fix that swept
+// everything into the system chain would destroy per-tenant isolation of the
+// audit trail.
+func TestSystemChain_TenantEventsStillUseTheirOwnChain(t *testing.T) {
+	ctx := context.Background()
+	db := openAuditDB(t)
+
+	var tenantID string
+	if err := db.QueryRowContext(ctx,
+		`INSERT INTO tenants (id, name, slug)
+		 VALUES (gen_random_uuid(), 'chain test', 'chain-test-' || gen_random_uuid())
+		 RETURNING id`).Scan(&tenantID); err != nil {
+		t.Fatalf("seed tenant: %v", err)
+	}
+	t.Cleanup(func() {
+		bg := context.Background()
+		_, _ = db.ExecContext(bg,
+			`DELETE FROM audit_log_chain WHERE audit_log_id IN
+			 (SELECT id FROM audit_logs WHERE tenant_id = $1)`, tenantID)
+		_, _ = db.ExecContext(bg, `DELETE FROM audit_logs WHERE tenant_id = $1`, tenantID)
+		_, _ = db.ExecContext(bg, `DELETE FROM tenants WHERE id = $1`, tenantID)
+	})
+
+	repo := postgres.NewAuditRepository(db)
+	svc := auditapp.NewAuditService(repo, logger.NewNop())
+
+	if err := svc.LogUserLogin(ctx, auditapp.AuditContext{TenantID: tenantID},
+		"", "tenant-scoped@example.test"); err != nil {
+		t.Fatalf("log tenant event: %v", err)
+	}
+
+	var chainTenant string
+	if err := db.QueryRowContext(ctx, `
+		SELECT c.tenant_id
+		  FROM audit_log_chain c
+		  JOIN audit_logs l ON l.id = c.audit_log_id
+		 WHERE l.tenant_id = $1
+		 ORDER BY c.chain_position DESC LIMIT 1`, tenantID).Scan(&chainTenant); err != nil {
+		t.Fatalf("no chain row for a tenant-scoped event: %v", err)
+	}
+
+	if chainTenant == auditdom.SystemChainTenantID.String() {
+		t.Fatal("a tenant's audit event was appended to the SYSTEM chain, merging " +
+			"tenants' trails into one shared chain")
+	}
+	if chainTenant != tenantID {
+		t.Errorf("chain tenant = %s, want %s", chainTenant, tenantID)
+	}
+}

internal/app/ingest/audit_chain_test.go

@@ -95,6 +95,10 @@ func (r *chainAuditRepo) GetByTenantAndID(_ context.Context, tenantID, id shared
 	return log, nil
 }
 
+func (r *chainAuditRepo) GetSystemByID(_ context.Context, id shared.ID) (*audit.AuditLog, error) {
+	return nil, audit.AuditLogNotFoundError(id)
+}
+
 // chainEntryFor returns the chain row covering an audit log, if any.
 func (r *chainAuditRepo) chainEntryFor(id shared.ID) (audit.ChainEntry, bool) {
 	r.mu.Lock()

internal/infra/controller/audit_chain_system_test.go

@@ -0,0 +1,92 @@
+package controller
+
+import (
+	"context"
+	"testing"
+
+	auditdom "github.com/openctemio/api/pkg/domain/audit"
+	"github.com/openctemio/api/pkg/domain/shared"
+)
+
+// The audit hash chain is keyed by tenant, and authentication events have no
+// tenant — at login a user may belong to several tenants and has not chosen
+// one. They were therefore skipped entirely. On the live database that left
+// 925 of 1075 audit rows (86%) with no tamper evidence, including every
+// auth.login, auth.register and auth.failed.
+//
+// They now extend a dedicated system chain. But writing hashes nobody checks is
+// not tamper evidence, and ListActiveTenantIDs can never return the system
+// chain because it is not a tenant. So the controller has to add it, and that
+// is what these tests hold in place.
+
+func TestAuditChainVerify_WalksTheSystemChain(t *testing.T) {
+	ids := mkTenantIDs(2)
+	verifier := &chainVerifierMock{}
+
+	c := newTestController(t, verifier, &tenantListerMock{ids: ids})
+	if _, err := c.Reconcile(context.Background()); err != nil {
+		t.Fatalf("Reconcile: %v", err)
+	}
+
+	var sawSystem bool
+	for _, got := range verifier.calls {
+		if got == auditdom.SystemChainTenantID.String() {
+			sawSystem = true
+			break
+		}
+	}
+	if !sawSystem {
+		t.Fatalf("the system chain was never verified. Every authentication event "+
+			"lives on it, so its hashes are stored but unchecked — which is not "+
+			"tamper evidence. Chains walked: %v", verifier.calls)
+	}
+}
+
+// A partial run — the context deadline expires part-way — must not be able to
+// skip the chain carrying the authentication records. Walking it first is the
+// property; asserting "it is in the list somewhere" would not catch a change
+// that appends it at the end.
+func TestAuditChainVerify_SystemChainIsWalkedFirst(t *testing.T) {
+	verifier := &chainVerifierMock{}
+
+	c := newTestController(t, verifier, &tenantListerMock{ids: mkTenantIDs(3)})
+	if _, err := c.Reconcile(context.Background()); err != nil {
+		t.Fatalf("Reconcile: %v", err)
+	}
+
+	if len(verifier.calls) == 0 {
+		t.Fatal("no chains were verified at all")
+	}
+	if verifier.calls[0] != auditdom.SystemChainTenantID.String() {
+		t.Fatalf("first chain walked = %v, want the system chain. A run cut short "+
+			"by its context would skip whatever is last, and the authentication "+
+			"trail is the part an intruder has the most reason to edit",
+			verifier.calls[0])
+	}
+}
+
+// The sentinel must never collide with a generated ID. Its version nibble is
+// 'f'; uuid.NewV7 and uuid.New can only ever emit 7 or 4 there. This asserts
+// the property rather than the constant, so it keeps holding if the value is
+// ever changed.
+func TestSystemChainTenantID_CannotCollideWithAGeneratedID(t *testing.T) {
+	sentinel := auditdom.SystemChainTenantID.String()
+
+	if sentinel == (shared.ID{}).String() {
+		t.Fatal("the sentinel is the zero value of shared.ID, which call sites " +
+			"already use IsZero() to mean \"unset\"")
+	}
+
+	// UUID version nibble: character 15 of 8-4-4-4-12.
+	if v := sentinel[14]; v == '4' || v == '7' {
+		t.Fatalf("sentinel version nibble is %q — a generated UUID could collide "+
+			"with it, and then a real tenant's chain would merge with the system "+
+			"chain", v)
+	}
+
+	for i := 0; i < 2000; i++ {
+		if shared.NewID().String() == sentinel {
+			t.Fatal("NewID produced the sentinel")
+		}
+	}
+}

internal/infra/controller/audit_chain_verify.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/openctemio/api/internal/app/audit"
 	"github.com/openctemio/api/internal/metrics"
+	auditdom "github.com/openctemio/api/pkg/domain/audit"
 	"github.com/openctemio/api/pkg/domain/shared"
 	tenantdom "github.com/openctemio/api/pkg/domain/tenant"
 	"github.com/openctemio/api/pkg/logger"
@@ -131,6 +132,14 @@ func (c *AuditChainVerifyController) Reconcile(ctx context.Context) (int, error)
 		return 0, fmt.Errorf("list active tenants: %w", err)
 	}
 
+	// The system chain is not a tenant, so ListActiveTenantIDs will never
+	// return it — and a chain nobody walks is not tamper-evident, it is just
+	// stored hashes. It carries every authentication event, which is the part
+	// of the trail an intruder has the most reason to edit, so it is walked
+	// FIRST rather than appended at the end where a partial run (ctx deadline)
+	// could skip it.
+	tenantIDs = append([]shared.ID{auditdom.SystemChainTenantID}, tenantIDs...)
+
 	processed := 0
 	totalBreaks := 0
 	newBreaks := 0

internal/infra/controller/audit_chain_verify_test.go

@@ -101,11 +101,13 @@ func TestAuditChainVerify_CleanChain_NoErrors(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Reconcile: %v", err)
 	}
-	if processed != 3 {
-		t.Errorf("processed: want 3, got %d", processed)
+	// 3 tenants + the system chain. The system chain is not a tenant, so
+	// ListActiveTenantIDs never returns it; the controller adds it.
+	if processed != 4 {
+		t.Errorf("processed: want 4 (3 tenants + system chain), got %d", processed)
 	}
-	if len(verifier.calls) != 3 {
-		t.Errorf("verifier called %d times, want 3", len(verifier.calls))
+	if len(verifier.calls) != 4 {
+		t.Errorf("verifier called %d times, want 4", len(verifier.calls))
 	}
 }
 
@@ -137,8 +139,8 @@ func TestAuditChainVerify_BreaksStillCountAsProcessed(t *testing.T) {
 	// VerifyChain. The break is emitted via the logger (tested via
 	// absence of error below — visual SIEM alerting is out of scope
 	// for unit tests).
-	if processed != 2 {
-		t.Errorf("processed: want 2 (both tenants visited), got %d", processed)
+	if processed != 3 {
+		t.Errorf("processed: want 3 (both tenants + system chain), got %d", processed)
 	}
 }
 
@@ -160,12 +162,12 @@ func TestAuditChainVerify_PerTenantErrorSkipsButContinues(t *testing.T) {
 		t.Fatalf("per-tenant error should not fail the run, got %v", err)
 	}
 	// processed counts only successful verifications; tenant[1] failed.
-	if processed != 2 {
-		t.Errorf("processed: want 2 (one failure skipped), got %d", processed)
+	if processed != 3 {
+		t.Errorf("processed: want 3 (4 chains, one failure skipped), got %d", processed)
 	}
 	// All three were attempted though.
-	if len(verifier.calls) != 3 {
-		t.Errorf("verifier should have been called for all 3 tenants even after one errored; got %d", len(verifier.calls))
+	if len(verifier.calls) != 4 {
+		t.Errorf("verifier should have been called for all 3 tenants + the system chain even after one errored; got %d", len(verifier.calls))
 	}
 }