Skip to content

buzz-audit: hash chain is tamper-evident in schema but not operationally verifiable; verify_chain accepts truncated chains #2620

Description

@SeanGearin

The gap

buzz-audit implements a real per-community SHA-256 hash chain, but "tamper-evident" is currently a property of the schema, not of operations:

  1. verify_chain has no operational caller. It's invoked only from tests. The conformance notes (crates/buzz-test-client/tests/conformance_multitenant.rs:2686) describe audit reads as "operator-internal (consumed by buzz-admin)" — but buzz-admin declares the buzz-audit dependency and never uses it. Nothing deployed can actually verify a chain.

  2. The verifier accepts truncated chains:

    • Prefix truncation: verify_chain(c, 1, N) seeds expected_prev = None, so the first row is never checked to be genesis — DELETE ... WHERE seq <= k leaves a suffix that verifies clean.
    • Unanchored segments: verify_chain(c, 5, 10) never compares row 5's prev_hash against row 4's stored hash; the left edge of a mid-chain range is unchecked.
    • Tail truncation: nothing records what the walk covered vs. what should exist, so deleting the newest rows is invisible to a walk over stored rows (the head is defined by what's stored).
    • Interior deletions surface as a generic ChainViolation, misdiagnosed as mutation rather than deletion.

So the strongest thing the docs claim — a tamper-evident audit chain — isn't checkable by anything an operator can run, and several deletion classes verify clean.

Proposed approach (no schema or wire changes)

  • A pure verify_entries (shared chain walk with an explicit left edge — genesis or an anchor hash — plus seq contiguity, prev-hash linkage, per-entry hash recompute), unit-testable without Postgres.
  • verify_chain keeps its signature but becomes anchored (from_seq > 1 requires the from_seq-1 entry to exist and be linked).
  • A new verify_full_chain returning {entries_verified, head_seq, head_hash}; passing a previously-recorded head as --expect-head closes the tail-truncation hole.
  • New AuditError variants (MissingGenesis, MissingAnchor, SequenceGap, TruncatedTail), carrying seq values only (same error-text sanitization fence as today).
  • A buzz-admin audit verify [--expect-head N] command reading Postgres directly on the same plane as the rest of buzz-admin — deliberately not a new HTTP endpoint, since the conformance notes treat the absence of an audit wire surface as an isolation property and CONTRIBUTING discourages new endpoint-specific JSON APIs.

No migrations, no /api/* routes, no wire-format changes, no new dependencies.

Status

Implemented and tested: six new Postgres-free verify_entries_* unit tests (each builds a real chain and mutilates it — prefix deletion, re-rooted genesis, interior deletion, content tamper, bad/skipped anchors), four #[ignore] Postgres integration tests, clippy-clean on both crates, wired into just test-unit.

Per CONTRIBUTING's "for significant changes, open an issue first to discuss the approach," I'm holding the PR pending your nod rather than dropping ~600 lines of new security-surface code on you unsolicited. Two things I'd want your read on first:

  • Is the buzz-admin operator-command surface (rather than any wire endpoint) the shape you want?
  • The externally-recorded-head --expect-head approach for tail-truncation detection — acceptable, or would you prefer the head anchor persisted in-repo?

Happy to open the PR immediately if the direction sounds right.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions