fix(audit): length-framed v2 hash encoding; verify rows by stored version

[alanshurafa]

Summary

compute_hash concatenates variable-length fields into the SHA-256 preimage with no framing, so the boundary between adjacent fields can shift without changing the digest: (object_id "x", detail 12) and (object_id "x1", detail 2) hash identically, and a detail edit can survive verify_chain. The regression test in this PR reproduces the issue's exact collision pair. (created_at-rfc3339 → action is a second unframed variable-length boundary with the same latent problem.)

Because any preimage change invalidates existing chains, the fix is a versioned encoding rather than a hard cutover:

• audit_log gains hash_version SMALLINT NOT NULL DEFAULT 1 (migration 0027 + schema/schema.sql). Existing rows stay v1 and keep verifying byte-for-byte — no data migration, no re-hash.
• v2 encoding (all new entries): a leading version byte, then every variable-length field (created_at rfc3339, action, actor_pubkey, object_id, canonical JSON detail) length-prefixed with u64-BE; presence tags kept. Fixed-width fields (community_id, seq, prev_hash) stay unframed.
• The write path is structurally v2-only: log_inner stamps CURRENT_HASH_VERSION; callers can't choose (nothing outside buzz-audit constructs AuditEntry — external callers use NewAuditEntry, which is untouched).
• verify_chain recomputes each row under its stored version, and surfaces a v1-row-after-v2-row as a tracing::warn! — deliberately not a hard error. During a rolling deploy across this migration (the chart's default: RollingUpdate + boot-time auto-migrate, no pre-upgrade job), pre-column pods keep writing correctly-hashed v1 rows after upgraded pods have written v2 rows, and those rows persist in the chain forever; a hard monotonicity error would permanently brand every such chain as tampered — the baseline-false-positive failure buzz-audit: every hash chain fails verification — created_at is hashed at nanosecond precision but stored at microsecond precision #2637/fix(audit): hash created_at at the precision Postgres stores #2638 taught this crate to avoid. A PG-gated test pins that a mixed-version chain verifies. Honest residual: an attacker with INSERT access can still plant a correctly-hashed v1 row (it verifies, with a warning); fully closing that requires either the hard cutover below or a strict-mode flag once operators can attest no pre-upgrade writers remain — natural to pair with the later DEFAULT-drop cleanup.
• v2 also closes a v1 genesis ambiguity while it still can: in v1, prev_hash: None hashes as the all-zero GENESIS_HASH, indistinguishable from a literal Some(GENESIS_HASH). v2 gives prev_hash the same presence-tag treatment as the other optional fields (impossible to change once v2 rows exist in the field; a test pins both the v1 ambiguity and the v2 fix).

Design notes for review:

• Rolling deploys: DEFAULT 1 is deliberate and must outlive this release — pods that predate the column still INSERT without naming it mid-rollout, and their rows (v1-hashed, v1-stamped) verify fine. Dropping the DEFAULT is a later cleanup. The Rust-side write guard, not the schema, is what prevents new v1 rows from this codebase.
• Downgrade tamper is detected: flipping a stored v2 row's hash_version to 1 changes the recomputed preimage, so it surfaces as HashMismatch.
• Exploit shape: the unique index (community_id, hash) means colliding entries can never coexist as rows — the v1 weakness is strictly an UPDATE-in-place substitution, which slightly sharpens the issue's "low as shipped" severity assessment.
• Forward compatibility: a hash_version above what a build implements is a hard UnsupportedHashVersion error, never a guess — a deliberate choice (verifying with an encoding you don't implement is worse than refusing), noted here because it means a future v3 rollout should sequence verification tooling after the fleet upgrade.
• Known pre-existing gap, untouched: verify_chain accepts a prefix-truncated range (expected_prev starts unanchored and seq continuity isn't asserted). That is exactly what feat(audit): detect chain truncation and add buzz-admin audit verify #2715 addresses — kept out of this diff to avoid colliding with it.
• Today both colliding fields are relay-resolved, so this is hardening against the issue's "first caller-supplied object_id next to a scalar detail" future, not a live exploit.
• Alternative considered: a hard cutover (single new encoding + one-time Rust re-hash of all chains). Simpler end state, stronger property (no v1 anywhere), but it rewrites every chain in place and needs a data migration; verify_chain currently has no production call site in-tree, so either choice is deployable. Happy to switch if maintainers prefer it. Precedent: fix(audit): hash created_at at the precision Postgres stores #2638 chose preimage-format preservation for the analogous trap.

Related issue

Fixes #4173. Closest adjacent work: #2715 (chain-truncation detection; rewrites the verify_chain walk into a pure verify_entries). The two compose — this PR's per-row version dispatch and v1-after-v2 warning thread directly through that signature; whichever lands second rebases mechanically. Update: #4249 has since been filed for this issue — it closes the reported boundary by field reorder, which invalidates every existing chain (no shipped re-hash path) and keeps the preimage's other unframed variable-length adjacencies. The two approaches could also compose: that reorder could be a subsequent encoding version under this PR's dispatch.

Testing

• cargo test -p buzz-audit --lib — passed. New (all Postgres-free): the issue's exact A/B collision pair collides under v1 and not under v2; v2 determinism, per-field sensitivity, None vs Some(empty) presence; v1 digest byte-stability pin (legacy chains must keep verifying); v1 ≠ v2 for the same entry; unknown version is a hard error.
• Postgres-gated (#[ignore]) additions: new rows stamp v2; a rolling-deploy-shaped mixed chain (v2 row, then a v1 row INSERTed without naming the column, as a pre-upgrade pod would) verifies.
• cargo test -p buzz-db --lib — passed (migration count 26→27 and the 0027 content assertions, per the embedded-migrator test's house style; schema/schema.sql updated in the same commit — CI provisions test databases from it via pgschema).
• cargo clippy --workspace --all-targets -- -D warnings, cargo fmt --all -- --check — clean.
• Toolchain note: Windows x86_64-pc-windows-gnu (as in fix(audit): hash created_at at the precision Postgres stores #2638); the Postgres-gated suites were reviewed but not run locally.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 36a52f88da

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Add the required Signed-off-by trailer

The reviewed commit message contains no Signed-off-by trailer, so the repository's required DCO check will reject any PR containing this commit. Recreate or rebase the commit with --signoff before submission.

AGENTS.md reference: AGENTS.md:L111-L111

Useful? React with 👍 / 👎.

[alanshurafa]

The reviewed commit's message does end with Signed-off-by: Agent57 <agent57@shurafa.com> — as does the current head after rebase — and the DCO Check on this PR is green.

[alanshurafa]

@block/buzz-oss-team — could a maintainer please approve the workflow runs for this fork PR and do a code-owner review? The reachable checks (Semgrep OSS, zizmor, DCO) pass and the branch is rebased on current main (28ae6cd). Thanks!