internal/certify/signbuild.go at b663803169, reviewed by claude-code:claude-fable-5-1, verified by codex:gpt-6-astra.
The signed execution step records duration_s: 0 for an unmeasured run, so the hash-chained ledger the statement commits to claims the build took zero seconds while the statement itself omits durationS; the R1 test's premise that the step 'omits an unmeasured duration_s on purpose' is false of the code.
set -e
cat > internal/certify/zz_coldreview_dur_test.go <<'EOF'
package certify
import (
"bytes"
"crypto/ed25519"
"fmt"
"testing"
)
func TestColdReviewStepClaimsZeroDuration(t *testing.T) {
priv := ed25519.NewKeyFromSeed(bytes.Repeat([]byte{1}, 32))
sb, err := SignBuild(BuildRecord{Repo: "r", Commit: "c", Actor: "corral-certify", Command: "go test"}, 0, priv, "corral-certify")
if err != nil {
t.Fatal(err)
}
ann := sb.Statement["predicate"].(map[string]any)["runDetails"].(map[string]any)["byproducts"].([]map[string]any)[1]["annotations"].(map[string]any)
_, stmtHas := ann["durationS"]
det := sb.Steps[1]["detail"].(map[string]any)
v, stepHas := det["duration_s"]
fmt.Printf("EVIDENCE: statement durationS present=%v (omitted as unmeasured); signed ledger step duration_s present=%v value=%v; head %s commits to that step\n", stmtHas, stepHas, v, sb.Head)
if stmtHas || !stepHas || v != float64(0) {
t.Fatal("defect not demonstrated")
}
}
EOF
go test ./internal/certify -run TestColdReviewStepClaimsZeroDuration -v 2>&1 | grep -E 'EVIDENCE|^(ok|FAIL|--- )' || true
go test ./internal/certify -run TestColdReviewStepClaimsZeroDuration >/dev/nullEVIDENCE: statement durationS present=false (omitted as unmeasured); signed ledger step duration_s present=true value=0; head 3fe0051e22eb35c7d08025eeb8de8a9aef6d21dd12e3936e60ee52dbf8936daa commits to that step --- PASS: TestColdReviewStepClaimsZeroDuration (0.00s) ok github.com/pdbethke/corralai/internal/certify 0.004s
I checked whether normalization or ledger construction removes the unmeasured value. SignBuild normalizes only rec.DurationS at internal/certify/signbuild.go:41, while line 60 unconditionally inserts rawDurationS into the execution detail. BuildLedger preserves that detail while hashing it (internal/certify/certify.go:121-132); certificationAnnotations omits the nil statement duration (lines 458-460). The comment documenting raw storage explains the implementation but does not eliminate the signed zero for the specified input.
confirmed by pdbethke — real, reproduced, recheck on main b6638031: still-reproduces. SignBuild's execution step writes duration_s: rawDurationS unconditionally (signbuild.go:60), so an unmeasured run's hash-linked step signs 0 while the statement omits it — the 167dd45b25cc R1 rule held at the statement door and the certify --local door (advpool/gate.go signedDurationS) but not this one. One correction to the claim: the test comment it calls false describes the certify --local step, which does omit; it is this door that does not.
ref f5015cf48e540c6d94861407e91bad96750fbf6a03f496dd664cad020d7b9f63#R1
SignBuild never stamps a time and neither production caller (cmd/corral/certify_change.go:111, internal/brain/buildcert.go:98) sets StartedTS/FinishedTS, so every signed statement carries metadata startedOn: 0 / finishedOn: 0 and every ledger step ts: 0, a signed claim that the build ran at the Unix epoch; published site records (site/public/records/record-29.json) show exactly this.
set -e
echo "Go code that assigns StartedTS/FinishedTS outside tests (expect none):"
! grep -rn --include='*.go' -E 'StartedTS:|FinishedTS:|\.StartedTS =|\.FinishedTS =' cmd internal | grep -v _test.go
cat > internal/certify/zz_coldreview_epoch_test.go <<'EOF'
package certify
import (
"bytes"
"crypto/ed25519"
"fmt"
"testing"
)
// Called exactly as cmd/corral/certify_change.go:111 calls it.
func TestColdReviewSignsEpochTimes(t *testing.T) {
priv := ed25519.NewKeyFromSeed(bytes.Repeat([]byte{1}, 32))
sb, err := SignBuild(BuildRecord{Repo: "r", Commit: "c", Branch: "b", Actor: "corral-certify", Command: "go test", ExitCode: 0, OutputDigest: "sha256:x"}, 2.5, priv, "corral-certify")
if err != nil {
t.Fatal(err)
}
md := sb.Statement["predicate"].(map[string]any)["runDetails"].(map[string]any)["metadata"].(map[string]any)
fmt.Printf("EVIDENCE: signed metadata startedOn=%v finishedOn=%v; step ts=[%v %v]\n", md["startedOn"], md["finishedOn"], sb.Steps[0]["ts"], sb.Steps[1]["ts"])
if md["startedOn"] != float64(0) || md["finishedOn"] != float64(0) || sb.Steps[0]["ts"] != float64(0) || sb.Steps[1]["ts"] != float64(0) {
t.Fatal("defect not demonstrated")
}
}
EOF
go test ./internal/certify -run TestColdReviewSignsEpochTimes -v 2>&1 | grep -E 'EVIDENCE|^(ok|FAIL|--- )' || true
go test ./internal/certify -run TestColdReviewSignsEpochTimes >/dev/null
grep -n '"startedOn": 0' site/public/records/record-29.jsonGo code that assigns StartedTS/FinishedTS outside tests (expect none): EVIDENCE: signed metadata startedOn=0 finishedOn=0; step ts=[0 0] --- PASS: TestColdReviewSignsEpochTimes (0.00s) ok github.com/pdbethke/corralai/internal/certify 0.004s 67: "startedOn": 0
I traced both production constructors and the downstream ledger and statement builders. cmd/corral/certify_change.go:111-115 and internal/brain/buildcert.go:98-107 leave StartedTS and FinishedTS zero. SignBuild's step literals also leave TS zero; BuildLedger assigns only sequence and hash-chain fields (internal/certify/certify.go:124-129), and BuildAttestation emits both record timestamps unconditionally (lines 226-228). The cited fixture contains zero metadata timestamps and step timestamps. No intervening timestamp assignment refutes the behavior on these production paths.
confirmed by pdbethke — real, reproduced, recheck on main b6638031: still-reproduces. No non-test code sets StartedTS/FinishedTS, SignBuild stamps no time, so every record through certify_change.go:111 and brain/buildcert.go:98 signs startedOn 0, finishedOn 0 and step ts 0 — a signed claim the build ran at the Unix epoch, visible in site/public/records/record-29.json.
ref f5015cf48e540c6d94861407e91bad96750fbf6a03f496dd664cad020d7b9f63#R2
SignBuild accepts rec.DurationS and unconditionally overwrites it from rawDurationS at line 41, so a caller that supplies a MeasuredSeconds value with a zero or different rawDurationS has its measurement silently discarded; the recipe's own golden test passes DurationS on both cases and never learns it is ignored.
The overwrite occurs, but it is explicitly the API's documented contract, not an unnoticed loss of an authoritative measurement. internal/certify/signbuild.go:35-39 states that rawDurationS supplies the execution duration, that the statement carries SecondsOrUnmeasured of it, and that SignBuild sets rec.DurationS itself so the representations cannot disagree. Thus a conflicting rec.DurationS is intentionally superseded by the designated duration argument. Both production callers follow this contract and leave rec.DurationS unset (cmd/corral/certify_change.go:111-115; internal/brain/buildcert.go:98-107). The golden test's redundant matching values do not demonstrate a contract violation.
refuted by pdbethke — not a defect, agreeing with the verifier: signbuild.go:35-39 documents that rawDurationS is the one duration input and that SignBuild sets rec.DurationS itself so the two cannot disagree; both callers leave rec.DurationS unset. The test passing it redundantly is untidy, not a lost measurement.
ref f5015cf48e540c6d94861407e91bad96750fbf6a03f496dd664cad020d7b9f63#R3
A NaN or Inf rawDurationS makes stepHash's json.Marshal fail and panic inside BuildLedger, so SignBuild crashes the process instead of returning the error its signature promises; no current caller can produce such a value (both read a JSON document), so this is latent.
set -e
cat > internal/certify/zz_coldreview_nan_test.go <<'EOF'
package certify
import (
"bytes"
"crypto/ed25519"
"fmt"
"math"
"testing"
)
func TestColdReviewNaNPanics(t *testing.T) {
priv := ed25519.NewKeyFromSeed(bytes.Repeat([]byte{1}, 32))
defer func() {
r := recover()
fmt.Printf("EVIDENCE: SignBuild panicked instead of returning an error: %v\n", r)
if r == nil {
t.Fatal("no panic")
}
}()
_, err := SignBuild(BuildRecord{Repo: "r", Commit: "c"}, math.NaN(), priv, "k")
t.Fatalf("returned normally, err=%v", err)
}
EOF
go test ./internal/certify -run TestColdReviewNaNPanics -v 2>&1 | grep -E 'EVIDENCE|^(ok|FAIL|--- )' || true
go test ./internal/certify -run TestColdReviewNaNPanics >/dev/nullEVIDENCE: SignBuild panicked instead of returning an error: certify: step is not JSON-marshalable: json: unsupported value: NaN --- PASS: TestColdReviewNaNPanics (0.00s) ok github.com/pdbethke/corralai/internal/certify 0.003s
I checked for validation, normalization, and recovery before hashing. Regardless of SecondsOrUnmeasured's result, rawDurationS enters the step unchanged at internal/certify/signbuild.go:60. BuildLedger calls stepHash before any error-returning signing operation, and stepHash explicitly panics when json.Marshal rejects the step (internal/certify/certify.go:106-111,127). There is no recovery in SignBuild. The programming-error comment explains the panic policy but does not refute the reported NaN/Inf behavior.
confirmed by pdbethke — real, latent, low, reproduced, recheck on main b6638031: still-reproduces. A NaN/Inf rawDurationS reaches stepHash's json.Marshal and panics inside a function that returns error; no current caller can produce one (both read JSON), so it is a contract gap, not a live crash.
ref f5015cf48e540c6d94861407e91bad96750fbf6a03f496dd664cad020d7b9f63#R4
Recorded so that "found nothing here" can be told apart from "never looked here".
SignBuild is the single build-signing recipe, and it is mechanically sound: the chain hashes, the envelope signs the same canonical bytes it embeds, the decoded Steps round-trip through UnmarshalSteps/VerifyLedger without a numeric-encoding mismatch. What is wrong is what it signs. The project fixed 'durationS: 0' on the STATEMENT (review 167dd45b25cc R1) and the test for that fix says the ledger step 'omits an unmeasured duration_s on purpose'. It does not: line 60 writes rawDurationS unconditionally, so the hash-linked execution step the statement's subject digest commits to still claims the run took 0 seconds, and the golden fixture pins that (finding 1). The same shape repeats one level up: neither production caller sets StartedTS/FinishedTS and SignBuild stamps nothing, so every record it has ever signed says startedOn 0, finishedOn 0 and every step says ts 0, i.e. the build happened at the Unix epoch (finding 2); the published site records show exactly that. Finding 3 is the API accepting rec.DurationS and silently discarding it, which the recipe's own golden test exercises without noticing. Finding 4 is a panic escaping a function with an error return on a NaN duration, low because no current caller can produce one.
R1, R2, and R4 stand on code inspection. R3 mistakes explicitly documented parameter precedence for an accidental loss of data. No tests or code under review were executed.