2 Commits

Author	SHA1	Message	Date
shankar0123	c4ed3da30b	fix(ci): Sprint 6 CI follow-up — staticcheck ST1021 + tenant-query baseline + skip inventory ... Sprint 6 push (commits 43836ac + 663b14b) tripped three CI guards. Fixing all three in this single follow-up — each is a small, mechanical correction that doesn't change behavior: 1. staticcheck ST1021: AuditChainSnapshot doc comment was on the wrong type. internal/service/audit_chain_metric.go:91 had: // Snapshot returns the current counter state for the Prometheus // exposer. Reads use atomic loads — no mutex. type AuditChainSnapshot struct { ... } The comment described Snapshot() (the method on AuditChainCounter) but sat directly above the AuditChainSnapshot struct. staticcheck ST1021 requires exported-type comments to start with the type's name + optional leading article. Rewrote to lead with "AuditChainSnapshot is the point-in-time view ...". 2. multi-tenant-query-coverage: baseline drifted 31 → 32 because Sprint 6 COMP-002-RETENTION added UserRepository.ListDeactivatedBefore at internal/repository/postgres/user.go:191 — legitimately tenant-spanning by design. The retention policy is control-plane-wide (one CERTCTL_USER_RETENTION_WINDOW for the whole deployment, not per-tenant). The scheduler's userRetentionLoop walks every tenant's deactivated users on the same tick. A per-tenant tenant_id filter would require the scheduler to iterate every tenant — more code for equivalent semantics. Per the guard's own documentation (option b), legitimately tenant-spanning queries get an inline rationale comment + a baseline lift. Both delivered: - Inline comment block on the SELECT in user.go::ListDeactivatedBefore. - BASELINE_COUNT 31 → 32 in scripts/ci-guards/multi-tenant-query-coverage.sh, with the Sprint 6 rebase entry added to the rebase-history comment. 3. skip-inventory-drift: docs/testing/skip-inventory.md was stale. COMP-001-HASH added three new t.Skip sites in internal/repository/postgres/audit_chain_test.go (the three testing.Short() gates on the testcontainers integration tests). Re-ran ./scripts/skip-inventory.sh to regenerate the doc — totals went from 144 → 147 sites + 78 → 82 short-mode guards. Verified locally: bash scripts/ci-guards/multi-tenant-query-coverage.sh (clean) bash scripts/ci-guards/skip-inventory-drift.sh (clean) go vet ./... (clean) staticcheck ./internal/service/... (clean) Closes the three Sprint 6 CI failures. The next CI run should green out.	2026-05-16 06:24:09 +00:00
shankar0123	43836aca7c	feat(audit): COMP-001-HASH — per-row hash chain on audit_events (tamper-evidence) ... Sprint 6 closure of the audit's HIGH-severity COMP-001-HASH finding. Pre-fix posture: migration 000018 installs a WORM trigger on audit_events that blocks UPDATE / DELETE for the application role. But the trigger header itself documents a compliance-superuser bypass (backup restore, retention purges, breach recovery). Without a hash chain, that role can rewrite any row's actor / action / details / timestamp / event_category with no on-disk trace. HIPAA §164.312(b), FedRAMP AU-9, NIST 800-53 AU-10 want tamper- EVIDENCE, not just tamper-prevention. This commit ships the evidence layer. Wire shape: migrations/000047_audit_events_hash_chain.up.sql + pgcrypto extension (digest function) + audit_chain_head: single-row sentinel table holding the most recent row_hash; FOR UPDATE row-lock serialises chain writes under concurrent INSERTs so two parallel writers can't read the same prev_hash and produce a forked chain + audit_events: prev_hash + row_hash columns + audit_events_canonical_payload(): centralised hash input builder. UTC + microsecond ISO-8601 keeps the hash session- timezone-independent. All columns separated by '|' so a concatenation-ambiguity exploit can't fabricate a collision + audit_events_compute_hash_chain(): BEFORE-INSERT trigger function. Reads sentinel FOR UPDATE → computes sha256(prev_hash || id || actor || actor_type || action || resource_type || resource_id || details::text || timestamp_utc_iso || event_category) → writes both columns + advances the sentinel + backfill loop walks every existing row in (timestamp ASC, id ASC) order; WORM trigger temporarily DISABLEd inside this migration's transaction so backfill UPDATEs land cleanly, ENABLEd before COMMIT + audit_events_verify_chain(): STABLE plpgsql verifier. Walks the chain end-to-end and returns the first break: (first_break_id TEXT, first_break_pos INT, row_count INT) internal/repository/postgres/audit.go + AuditRepository.VerifyHashChain — calls the SQL function and maps the OUT parameters to Go return values internal/repository/interfaces.go + AuditRepository.VerifyHashChain in the contract; every in-memory mock + stub picks up the no-op implementation internal/scheduler/scheduler.go + AuditChainVerifier + AuditChainBreakRecorder interfaces + auditChainVerifyInterval (default 6h) + auditChainVerifyLoop: runs once on start + every tick; atomic.Bool guard + 5-min per-tick context timeout match every other GC loop's pattern internal/service/audit_chain_metric.go + AuditChainCounter type with atomic counters. Sticky-first- detection on (BrokenAtID, BrokenAtPos) so the actionable alarm doesn't drift across walks. Snapshot() returns the full state for the metrics handler internal/api/handler/metrics.go + AuditChainCounterSnapshotter interface + Prometheus exposition for four series: certctl_audit_chain_break_detected_total counter (the alarm) certctl_audit_chain_verify_total counter (walks done) certctl_audit_chain_rows gauge (last walk size) certctl_audit_chain_last_verified_at gauge (unix seconds) internal/config/config.go + AuditChainConfig{ VerifyInterval } + CERTCTL_AUDIT_CHAIN_VERIFY_INTERVAL cmd/server/main.go + wires AuditChainCounter into both the scheduler (recorder) + metrics handler (snapshotter) — single instance shared so the writer + reader are guaranteed to converge internal/repository/postgres/audit_chain_test.go (NEW) + TestAuditEventsHashChain_FreshTable: empty walk → clean + TestAuditEventsHashChain_AppendLinksRows: three INSERTs produce a strictly-linked chain; prev_hash on row 0 is NULL; verifier walks clean over the 3 rows + TestAuditEventsHashChain_VerifierDetectsTampering: simulate the compliance-superuser threat model (DISABLE WORM, UPDATE a middle row, ENABLE WORM); verifier returns the tampered row's id at position 1 docs/operator/audit-chain.md (NEW) + Layered-defenses explainer (WORM + hash chain). Verifier function reference. Recommended Prometheus alert rule. Performance scaling table (10k to 10M rows). Step-by-step runbook for what to do when a break is detected. Operator configuration table. Test-stub additions for AuditRepository.VerifyHashChain: internal/service/testutil_test.go — mockAuditRepo internal/service/acme_test.go — fakeAuditRepo internal/integration/lifecycle_test.go — mockAuditRepository internal/api/handler/scep_intune_e2e_test.go — intuneE2EAuditRepo Verified locally: go vet ./... (clean) gofmt -l internal/ cmd/ (clean) go test -short -count=1 ./internal/scheduler/... ./internal/config/... ./internal/service/... ./internal/api/handler/... ./internal/repository/... (all green) Verified with testcontainers + postgres:16-alpine + the migration runner (not gated under -short — requires docker): go test -count=1 -run TestAuditEventsHashChain ./internal/repository/postgres/... Closes COMP-001-HASH leg of Sprint 6. COMP-002-RETENTION lands in the next commit (separate concern: federated-user PII retention).	2026-05-16 06:17:15 +00:00