internal/auditpush at 895208db30, reviewed by claude-code, verified by codex.
WriteCheckpoint prunes a ledger directory without ever verifying the chain, so `corral ledger checkpoint` silently deletes a tampered entry and leaves a genesis that verifies with no problem — the one verb that destroys evidence is the one verb that never looks at it.
#!/bin/sh
set -e
cat > internal/auditpush/zz_coldreview_checkpoint_test.go <<'EOF'
package auditpush
import (
"bytes"
"compress/gzip"
"crypto/ed25519"
"os"
"path/filepath"
"testing"
)
func TestColdReviewCheckpointLaundersABrokenChain(t *testing.T) {
dir := t.TempDir()
pub, priv, err := ed25519.GenerateKey(nil)
if err != nil {
t.Fatal(err)
}
signer := Ed25519LedgerSigner{KeyID: "corral-certify", Key: priv}
SetLedgerSigner(signer)
defer SetLedgerSigner(nil)
for _, c := range []string{"aaa1", "bbb2", "ccc3"} {
if _, err := PushBundle(dir, Bundle{Scan: ScanRow{Repo: "r", Commit: c, Audited: 1}}); err != nil {
t.Fatal(err)
}
}
entries, err := ReadLedgerDir(dir)
if err != nil || len(entries) != 3 {
t.Fatalf("setup: %v, %d entries", err, len(entries))
}
// Tamper with the MIDDLE entry's payload, leaving its hash and signature
// exactly as written: 1 audited file becomes 999.
path := filepath.Join(dir, ScansSubdir, entries[1].File)
raw, err := readMaybeGzip(path)
if err != nil {
t.Fatal(err)
}
old := []byte(`"Audited": 1`)
if !bytes.Contains(raw, old) {
t.Fatalf("setup: %q not found in the entry as written", old)
}
edited := bytes.Replace(raw, old, []byte(`"Audited": 999`), 1)
var buf bytes.Buffer
zw := gzip.NewWriter(&buf)
if _, err := zw.Write(edited); err != nil {
t.Fatal(err)
}
if err := zw.Close(); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(path, buf.Bytes(), 0o644); err != nil {
t.Fatal(err)
}
before, err := VerifyLedgerDir(dir, pub)
if err != nil {
t.Fatal(err)
}
broken := ""
for _, c := range before {
if c.Problem != "" {
broken = c.File + ": " + c.Problem
}
}
if broken == "" {
t.Fatal("setup failed: the tampered entry verified clean, so there is nothing to launder")
}
t.Logf("BEFORE: the chain reports the tampering -> %s", broken)
name, pruned, err := WriteCheckpoint(dir, signer)
if err != nil {
t.Fatalf("NOT DEMONSTRATED: WriteCheckpoint refused the broken chain: %v", err)
}
after, err := VerifyLedgerDir(dir, pub)
if err != nil {
t.Fatal(err)
}
for _, c := range after {
if c.Problem != "" {
t.Fatalf("NOT DEMONSTRATED: the chain still reports %q after the checkpoint", c.Problem)
}
}
t.Logf("AFTER: checkpoint %s placed, %d entries deleted; chain is now %d entry with problem=%q", name, pruned, len(after), after[0].Problem)
t.Logf("AFTER: signed=%v verified=%v note=%q", after[0].Signed, after[0].SigOK, after[0].Note)
t.Logf("EVIDENCE: WriteCheckpoint never called VerifyLedgerDir. The tampered file is gone, the record verifies clean, and the genesis that replaced it carries the operator's own signature.")
}
EOF
go test -count=1 -v -run TestColdReviewCheckpointLaundersABrokenChain ./internal/auditpush/=== RUN TestColdReviewCheckpointLaundersABrokenChain
zz_coldreview_checkpoint_test.go:69: BEFORE: the chain reports the tampering -> 20260908T161632Z-bbb2-9d2735e58d0d.json.gz: entry bytes do not match its hash — edited after it was written
zz_coldreview_checkpoint_test.go:84: AFTER: checkpoint 20260908T161632Z-checkpoint-62f8bebce9d8.json.gz placed, 3 entries deleted; chain is now 1 entry with problem=""
zz_coldreview_checkpoint_test.go:85: AFTER: signed=true verified=true note="chain begins at a checkpoint: 3 earlier entries (through 2026-09-08, head 62f8bebce9d8) are not present"
zz_coldreview_checkpoint_test.go:86: EVIDENCE: WriteCheckpoint never called VerifyLedgerDir. The tampered file is gone, the record verifies clean, and the genesis that replaced it carries the operator's own signature.
--- PASS: TestColdReviewCheckpointLaundersABrokenChain (0.01s)
PASS
ok github.com/pdbethke/corralai/internal/auditpush 0.013sI checked WriteCheckpoint and the verification path. WriteCheckpoint reads entries, trusts the stored head hash, places a newly signed checkpoint, and deletes the old files without calling VerifyLedgerDir or recomputing the entries' hashes. The reproduced input therefore demonstrates the stated laundering behavior.
ref 55f0793d5e86b57f414102fe55592fa053d1bbdab7b935e43ae9bf4e263af9c0#R1
An entry's keyid is deleted before hashing and is never checked against the signature, so rewriting `keyid` in a placed, signed entry leaves HashOK, SigOK and Problem untouched while `corral verify --ledger` prints the attacker-chosen key name as the signer.
#!/bin/sh
set -e
cat > internal/auditpush/zz_coldreview_keyid_test.go <<'EOF'
package auditpush
import (
"bytes"
"compress/gzip"
"crypto/ed25519"
"os"
"path/filepath"
"testing"
)
func TestColdReviewKeyIDIsOutsideTheHashAndUnchecked(t *testing.T) {
dir := t.TempDir()
pub, priv, err := ed25519.GenerateKey(nil)
if err != nil {
t.Fatal(err)
}
SetLedgerSigner(Ed25519LedgerSigner{KeyID: "corral-certify", Key: priv})
defer SetLedgerSigner(nil)
if _, err := PushBundle(dir, Bundle{Scan: ScanRow{Repo: "r", Commit: "aaa1", Audited: 1}}); err != nil {
t.Fatal(err)
}
entries, err := ReadLedgerDir(dir)
if err != nil || len(entries) != 1 {
t.Fatalf("setup: %v, %d entries", err, len(entries))
}
before, err := VerifyLedgerDir(dir, pub)
if err != nil {
t.Fatal(err)
}
if !before[0].HashOK || !before[0].SigOK || before[0].Problem != "" || before[0].KeyID != "corral-certify" {
t.Fatalf("setup: the untouched entry did not verify clean: %+v", before[0])
}
path := filepath.Join(dir, ScansSubdir, entries[0].File)
raw, err := readMaybeGzip(path)
if err != nil {
t.Fatal(err)
}
old := []byte(`"keyid": "corral-certify"`)
if !bytes.Contains(raw, old) {
t.Fatalf("setup: %q not found in the entry as written", old)
}
edited := bytes.Replace(raw, old, []byte(`"keyid": "acme-corp-hsm-key-7"`), 1)
var buf bytes.Buffer
zw := gzip.NewWriter(&buf)
if _, err := zw.Write(edited); err != nil {
t.Fatal(err)
}
if err := zw.Close(); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(path, buf.Bytes(), 0o644); err != nil {
t.Fatal(err)
}
after, err := VerifyLedgerDir(dir, pub)
if err != nil {
t.Fatal(err)
}
c := after[0]
if c.KeyID != "acme-corp-hsm-key-7" {
t.Fatalf("setup: the edit did not take, keyid = %q", c.KeyID)
}
if !c.HashOK || !c.SigOK || c.Problem != "" {
t.Fatalf("NOT DEMONSTRATED: the chain caught the keyid edit: %+v", c)
}
t.Logf("EVIDENCE: keyid rewritten \"corral-certify\" -> %q with no other change.", c.KeyID)
t.Logf("EVIDENCE: VerifyLedgerDir reports HashOK=%v LinkOK=%v Signed=%v SigOK=%v Problem=%q", c.HashOK, c.LinkOK, c.Signed, c.SigOK, c.Problem)
t.Logf("EVIDENCE: canonicalEntryBytes deletes \"keyid\" from the tree before hashing (ledgerdir.go:294-296) and nothing ever compares it to the key the signature verified under, so cmd/corral/verify_attest.go:550 prints \"signed by %s and verified against ...\".", c.KeyID)
}
EOF
go test -count=1 -v -run TestColdReviewKeyIDIsOutsideTheHashAndUnchecked ./internal/auditpush/=== RUN TestColdReviewKeyIDIsOutsideTheHashAndUnchecked
zz_coldreview_keyid_test.go:68: EVIDENCE: keyid rewritten "corral-certify" -> "acme-corp-hsm-key-7" with no other change.
zz_coldreview_keyid_test.go:69: EVIDENCE: VerifyLedgerDir reports HashOK=true LinkOK=true Signed=true SigOK=true Problem=""
zz_coldreview_keyid_test.go:70: EVIDENCE: canonicalEntryBytes deletes "keyid" from the tree before hashing (ledgerdir.go:294-296) and nothing ever compares it to the key the signature verified under, so cmd/corral/verify_attest.go:550 prints "signed by acme-corp-hsm-key-7 and verified against ...".
--- PASS: TestColdReviewKeyIDIsOutsideTheHashAndUnchecked (0.00s)
PASS
ok github.com/pdbethke/corralai/internal/auditpush 0.008scanonicalEntryBytes explicitly deletes keyid alongside hash and signature before hashing. VerifyLedgerDir reports the parsed KeyID but verifies only the signature against the supplied public key; it never binds or compares that label to the key. Thus changing only keyid preserves HashOK and SigOK while changing the displayed signer label.
ref 55f0793d5e86b57f414102fe55592fa053d1bbdab7b935e43ae9bf4e263af9c0#R2
CanonicalizeForWarehouse mutates the caller's row slices in place — the exact thing its partner BlankUnpushedSource copies to avoid — so the statement hasher (prepareRowsForHash, which calls it before copying) silently truncates the caller's event timestamps and nils the caller's kill rates on a --push-source bundle.
#!/bin/sh
set -e
cat > internal/auditpush/zz_coldreview_alias_test.go <<'EOF'
package auditpush
import (
"testing"
"time"
)
// The caller (cmd/corral/certify_repo.go prepareRowsForHash) takes the bundle
// BY VALUE, then calls BlankUnpushedSource and CanonicalizeForWarehouse before
// it copies any slice. BlankUnpushedSource copies Files and Mutants -- and
// returns immediately, copying nothing, when SourcePushed is set. Events are
// never copied by it at all. CanonicalizeForWarehouse then writes through the
// shared backing arrays.
func TestColdReviewCanonicalizeMutatesTheCallersBundle(t *testing.T) {
loc := time.FixedZone("x", -4*3600)
ts := time.Date(2026, 9, 8, 12, 0, 0, 123456789, loc)
kr := 0.5
orig := Bundle{
Events: []EventRow{{Repo: "r", Seq: 1, TS: ts}},
Files: []Row{{Repo: "r", Path: "a.go", Uncovered: true, KillRate: &kr}},
SourcePushed: true, // a --push-source run: the custody copy is skipped
}
hashed := orig // exactly what taking the bundle by value gives you
BlankUnpushedSource(&hashed)
CanonicalizeForWarehouse(&hashed)
if orig.Events[0].TS.Equal(ts) {
t.Fatalf("NOT DEMONSTRATED: the caller's event TS was untouched (%v)", orig.Events[0].TS)
}
if orig.Files[0].KillRate != nil {
t.Fatalf("NOT DEMONSTRATED: the caller's KillRate was untouched (%v)", *orig.Files[0].KillRate)
}
t.Logf("EVIDENCE: caller's event TS %v -> %v (nanoseconds and zone gone)", ts, orig.Events[0].TS)
t.Logf("EVIDENCE: caller's KillRate %v -> <nil> (the value the run measured, destroyed by a hash function)", kr)
t.Logf("EVIDENCE: BlankUnpushedSource's own doc says a rule that destroys the caller's data is a different rule; CanonicalizeForWarehouse, called from the same two places, does not copy.")
// And with SourcePushed false, Files is copied but Events still is not.
ts2 := time.Date(2026, 9, 8, 13, 0, 0, 987654321, loc)
orig2 := Bundle{Events: []EventRow{{Repo: "r", Seq: 1, TS: ts2}}}
h2 := orig2
BlankUnpushedSource(&h2)
CanonicalizeForWarehouse(&h2)
if orig2.Events[0].TS.Equal(ts2) {
t.Fatalf("NOT DEMONSTRATED (no-source case): event TS untouched (%v)", orig2.Events[0].TS)
}
t.Logf("EVIDENCE (SourcePushed=false): caller's event TS %v -> %v; BlankUnpushedSource copies Files and Mutants only.", ts2, orig2.Events[0].TS)
}
EOF
go test -count=1 -v -run TestColdReviewCanonicalizeMutatesTheCallersBundle ./internal/auditpush/=== RUN TestColdReviewCanonicalizeMutatesTheCallersBundle
zz_coldreview_alias_test.go:34: EVIDENCE: caller's event TS 2026-09-08 12:00:00.123456789 -0400 x -> 2026-09-08 16:00:00.123456 +0000 UTC (nanoseconds and zone gone)
zz_coldreview_alias_test.go:35: EVIDENCE: caller's KillRate 0.5 -> <nil> (the value the run measured, destroyed by a hash function)
zz_coldreview_alias_test.go:36: EVIDENCE: BlankUnpushedSource's own doc says a rule that destroys the caller's data is a different rule; CanonicalizeForWarehouse, called from the same two places, does not copy.
zz_coldreview_alias_test.go:47: EVIDENCE (SourcePushed=false): caller's event TS 2026-09-08 13:00:00.987654321 -0400 x -> 2026-09-08 17:00:00.987654 +0000 UTC; BlankUnpushedSource copies Files and Mutants only.
--- PASS: TestColdReviewCanonicalizeMutatesTheCallersBundle (0.00s)
PASS
ok github.com/pdbethke/corralai/internal/auditpush 0.005sCanonicalizeForWarehouse writes directly through Bundle.Events and Bundle.Files slice backing arrays. Passing Bundle by value does not copy those arrays. BlankUnpushedSource returns immediately when SourcePushed is true and, when false, still does not copy Events, so the reproduced aliasing and caller mutation follow from the code.
ref 55f0793d5e86b57f414102fe55592fa053d1bbdab7b935e43ae9bf4e263af9c0#R3
PushLedgerDir stamps source_pushed = withSource on every scan entry it pushes, so `corral ledger push --push-source` records the custody claim "our code left the box" for entries that were written with source already blanked and therefore carry none.
I examined the sink-specific source_pushed rationale and tests, but it does not refute the named case. PushLedgerDir unconditionally assigns withSource to both source flags before BlankUnpushedSource; when an entry was originally persisted with its source fields blank, --push-source cannot restore those bytes, yet the warehouse row is still marked true. The field's own ScanRow documentation describes whether the run carried source bytes to the warehouse, making the claim materially inaccurate for this input.
ref 55f0793d5e86b57f414102fe55592fa053d1bbdab7b935e43ae9bf4e263af9c0#R4
corral_findings has refutation_script and refutation_script_sha256 but no refutation_output column, so a warehouse reader can see the verifier's script and its exit code but never what it printed — the evidence for a demotion is unrecoverable, while the reviewer's own stdout IS stored.
The corral_findings schema and INSERT retain refutation_script, its hash, refutation_output_sha256, and refutation_exit_code, but have no refutation output column or argument. In contrast, reviewer stdout is stored in output when custody permits. The warehouse alone therefore cannot recover the verifier's printed evidence.
ref 55f0793d5e86b57f414102fe55592fa053d1bbdab7b935e43ae9bf4e263af9c0#R5
AppendLedgerEntry links a new entry to `existing[n-1].Hash` as stored, without recomputing it, so a push or `corral ledger append` onto a chain whose head has been edited extends and signs the tampered chain rather than refusing.
AppendLedgerEntry calls ReadLedgerDir, takes existing[len(existing)-1].Hash verbatim, and passes it to placeEntry. ReadLedgerDir parses entries but does not verify their hashes or links. Consequently an edited head retaining its old stored hash is accepted as the predecessor and a new signed entry is appended without refusing the already-broken chain.
ref 55f0793d5e86b57f414102fe55592fa053d1bbdab7b935e43ae9bf4e263af9c0#R6
Retracted() is built over every retraction entry including ones that have themselves been retracted, so retracting a retraction leaves the original entry still out of the record — there is no way to undo a mistaken retraction, and WriteRetraction accepts the attempt without saying so.
Retracted collects the target of every KindRetract entry without first determining whether that retraction entry is itself live. LiveEntries then removes every collected target in one pass. WriteRetraction permits targeting a retraction unless that retraction is already targeted, so the attempted undo is accepted but does not revive the original entry.
ref 55f0793d5e86b57f414102fe55592fa053d1bbdab7b935e43ae9bf4e263af9c0#R7
CanonicalSparseJSON decodes without UseNumber, so every number in the warehouse-rows hash passes through float64 — the exact loss its sibling CanonicalFullJSON documents itself as avoiding — and two bundles differing only in an integer past 2^53 (token counts, millis) hash identically.
CanonicalSparseJSON marshals and then calls json.Unmarshal into any without UseNumber, causing JSON numbers to become float64. pruneEmpty preserves those float64 values, which are subsequently remarshaled. Distinct integers beyond exact float64 precision can therefore collapse to the same canonical bytes and hash.
ref 55f0793d5e86b57f414102fe55592fa053d1bbdab7b935e43ae9bf4e263af9c0#R8
pruneEmpty removes empty ELEMENTS from arrays rather than only empty keys, so a bundle whose Files/Mutants slice contains an all-zero row hashes to the same warehouse-rows value as one without that row — the sparse form is not injective over slice length.
In pruneEmpty's []any branch, elements whose recursive result is empty are omitted from the output slice. An all-zero row decodes as an object whose keys are all pruned, making the object empty and causing the array element itself to disappear. The sparse representation therefore does not preserve the stated slice-length distinction.
ref 55f0793d5e86b57f414102fe55592fa053d1bbdab7b935e43ae9bf4e263af9c0#R9
When a prune removal fails part-way, WriteCheckpoint returns pruned = 0 although entries have already been deleted, so `corral ledger checkpoint` prints "0 earlier entries pruned" for a directory it has partly emptied.
WriteCheckpoint removes old files sequentially. If os.Remove fails after earlier removals succeeded, the function immediately returns name, 0, error and does not report the completed deletions. Thus the returned pruned count can be zero after the directory was partly emptied, exactly as claimed.
ref 55f0793d5e86b57f414102fe55592fa053d1bbdab7b935e43ae9bf4e263af9c0#R10
Recorded so that "found nothing here" can be told apart from "never looked here".
This package carries the repository's whole integrity claim — an append-only, hash-linked, signed record — and the reading is mostly careful: the hash-form split between entries (full) and warehouse rows (sparse) is argued, the custody switch is enforced in one place, the additive migrations are probed rather than blindly ALTERed, and the scan_uid derivation really is recomputable from the stored row. Where it is wrong, it is wrong in the same shape the codebase's own comments keep naming: a rule enforced at one door and missing at another. Finding 1 is the serious one — `corral ledger checkpoint` is the only verb in the package that DESTROYS entries, and it is the only verb that never checks the chain first, so a tampered entry is deleted and the laundered result verifies clean under the operator's own signature. Finding 2 is narrower but sits on the line a reader is shown: the keyid printed as 'signed by X and verified against Y' is not covered by the hash and is never checked, so X is whatever the file says. Finding 3 is the mirror image of a rule the package states explicitly for BlankUnpushedSource and does not hold for its partner. Findings 4-9 are read-only claims: 4 and 5 are false or missing custody/evidence in the warehouse grains, 6-9 are smaller. I did not find a way to forge an entry that verifies against a key the forger does not hold, and I could not break the scan_uid, migration, lock or MotherDuck-autocreate paths.
R1-R10 all stand. The three reproduced findings match the implementation, and code inspection reveals no narrower invariant that defeats them. R4's sink-specific interpretation of source_pushed does not save a row marked true when the source was already absent. R5 genuinely omits refutation stdout while retaining only its hash. R6-R10 follow directly from the relevant algorithms and error paths.