cmd/corral/certify_repo_transparency.go at b663803169, reviewed by claude-code:claude-fable-5-1, verified by codex:gpt-6-astra.
loadLocalCertifyKeyIfConfigured calls loadLocalCertifyKey unconditionally whenever CORRALAI_CERTIFY_KEY_FILE is non-empty, so a configured-but-missing path (a typo, an unprovisioned CI secret) silently mints a fresh Ed25519 identity, signs the statement with it, and --transparency logs that entry permanently — directly contradicting the --transparency help text's "this never mints a fresh key just to have one" and the guard comment's "rather than silently minting one".
#!/bin/sh
set -u
cat > cmd/corral/zz_coldreview_mint_test.go <<'EOF'
package main
import (
"os"
"path/filepath"
"testing"
)
// Cold-review reproduction: the --transparency flag help promises the
// feature "never mints a fresh key just to have one". With
// CORRALAI_CERTIFY_KEY_FILE naming a path that does not exist, the guard's
// own key loader creates one and returns success.
func TestColdReviewTransparencyGuardMintsAKey(t *testing.T) {
t.Setenv("CORRALAI_CERTIFY_KEY", "")
t.Setenv("HOME", t.TempDir())
missing := filepath.Join(t.TempDir(), "typo", "corral_key.txt")
t.Setenv("CORRALAI_CERTIFY_KEY_FILE", missing)
if _, err := os.Stat(missing); err == nil {
t.Fatal("precondition: the configured key file must not exist")
}
priv, err := loadLocalCertifyKeyIfConfigured()
if err != nil {
t.Fatalf("refused, as the --transparency help promises: %v", err)
}
st, serr := os.Stat(missing)
if serr != nil {
t.Fatalf("no key file appeared: %v", serr)
}
t.Logf("DEFECT: loadLocalCertifyKeyIfConfigured returned a %d-byte private key with err=nil and MINTED %s (%d bytes, mode %v) although the configured file did not exist; the --transparency help says it \"never mints a fresh key just to have one\"", len(priv), missing, st.Size(), st.Mode())
}
EOF
timeout 58 go test ./cmd/corral -run '^TestColdReviewTransparencyGuardMintsAKey$' -count=1 -v > ./coldreview_mint_out.txt 2>&1
cat ./coldreview_mint_out.txt
grep -q 'DEFECT: loadLocalCertifyKeyIfConfigured' ./coldreview_mint_out.txt && grep -q -- '--- PASS: TestColdReviewTransparencyGuardMintsAKey' ./coldreview_mint_out.txt=== RUN TestColdReviewTransparencyGuardMintsAKey
zz_coldreview_mint_test.go:29: DEFECT: loadLocalCertifyKeyIfConfigured returned a 64-byte private key with err=nil and MINTED /tmp/TestColdReviewTransparencyGuardMintsAKey2937866793/002/typo/corral_key.txt (64 bytes, mode -rw-------) although the configured file did not exist; the --transparency help says it "never mints a fresh key just to have one"
--- PASS: TestColdReviewTransparencyGuardMintsAKey (0.00s)
PASS
ok github.com/pdbethke/corralai/cmd/corral 0.011sI checked whether explicit configuration or downstream key loading prevents creation at a missing path. cmd/corral/certify_repo_transparency.go:82–83 delegates directly to loadLocalCertifyKey; cmd/corral/certify_change.go:86–100 passes the configured path to buildstore.LoadOrCreateSigningKey. internal/buildstore/store.go:475–502 generates and persists a key when that path is missing. The deliberate-bootstrap explanation at certify_repo_transparency.go:60–77 establishes intent but does not reconcile the unconditional wording in the flag help at certify_repo.go:90. The supplied reproduction demonstrates key creation, not an actual public upload, but the subsequent signing and upload wiring supports that consequence when upload succeeds.
confirmed by pdbethke — real, reproduced, recheck on main b6638031: still-reproduces. A CORRALAI_CERTIFY_KEY_FILE naming a missing path goes straight to LoadOrCreateSigningKey and mints a key; the doc comment calls that a deliberate bootstrap, but the --transparency help (certify_repo.go:90) promises it 'never mints a fresh key just to have one', and on that flag the minted identity signs into a public, permanent log. The promise and the code disagree; for a permanent log the code should change, not the text.
ref e1608f9712359ac8221867374d47accbd409b6c456cc2e0ae8b3f76e1bfd7819#R1
uploadToTransparencyLog (and the transparencyPublicKeyPEM failure branch at its call site) prints only a plain stderr line on failure, while the sibling failures in the same end-of-scan sequence — a failed statement write and a failed --push — both emit a `::warning` annotation when GITHUB_ACTIONS=true; on a runner a requested-but-failed public log entry therefore leaves a green job, NULL rekor columns in the ledger, and no annotation, the exact silent-stderr-on-a-green-job outcome the push path's own comment and test reject.
I traced both failure branches and their caller for annotation handling. certify_repo_transparency.go:180–188 prints ordinary stderr and returns false; certify_repo.go:1465–1477 likewise prints the public-key error without an annotation and only populates receipt fields on successful upload. Neither branch changes the verdict's exit code. The statement and push failures explicitly emit workflow warnings at certify_repo.go:1450 and :1549. The documented fail-open contract explains the unchanged exit status, but does not refute the missing workflow annotation.
confirmed by pdbethke — real, checked by reading main b6638031: a failed statement write (certify_repo.go:1450) and a failed push (:1549) raise ::warning under GITHUB_ACTIONS; the --transparency failures (the transparencyPublicKeyPEM branch at :1466 and uploadToTransparencyLog) print one plain stderr line, so a requested-but-failed public log entry leaves a green job and NULL receipt columns. A rule at two doors and not the third.
ref e1608f9712359ac8221867374d47accbd409b6c456cc2e0ae8b3f76e1bfd7819#R2
writeStatement writes the plain statement file BEFORE attempting to remove the stale envelope, so when that removal fails (e.g. the envelope path is a non-empty directory or an unremovable file) it returns an error with sha="" although the new plain file is already on disk; runCertifyRepo then withholds the push and, on a runner, prints "nothing will be attested and no rows were pushed" while action.yml finds the statement present and attests it — an attested statement whose rows were never pushed, plus a stale envelope for a previous statement sitting beside it.
I checked for cleanup after envelope-removal failure and whether the Action requires successful statement generation. certify_repo_transparency.go:156–160 writes the plain file before removing the envelope and returns an empty hash on removal failure without deleting the plain file. certify_repo.go:1450 then says nothing will be attested; the empty hash prevents the required statement linkage for --push. However, action.yml:545–568 runs predicate extraction regardless of the preceding step's result and gates attestation on the existing nonempty statement and extracted predicate. Thus the new plain statement can still be attested despite the reported failure and withheld push.
confirmed by pdbethke — real, low, checked by reading main b6638031: writeStatement writes the plain file before removing the stale envelope (certify_repo_transparency.go:156-160) and returns sha='' on a removal failure, so the runner is told 'nothing will be attested and no rows were pushed' while action.yml's predicate step finds a non-empty statement and attests it. Needs an unremovable envelope path to trigger.
ref e1608f9712359ac8221867374d47accbd409b6c456cc2e0ae8b3f76e1bfd7819#R3
writeAuditStatement discards writeStatement's signErr (`sha, _, _, err := writeStatement(...)`), so on a --transparency run where signing or the envelope write fails after the early guard passed, the real cause is lost and uploadToTransparencyLog reports only "reading the signed envelope ... no such file" — a diagnostic computed and dropped on the floor, the bug shape AGENTS.md names, on the one path where the sign error is not benign.
I checked whether the early key guard or later public-key reload preserves the envelope creation error. certify_repo_transparency.go:129–130 produces a specific envelope-write error, and :163–164 returns it separately as signErr with a successful plain-file hash. certify_repo.go:5103 discards signErr. The early key check cannot guarantee a later filesystem write succeeds; when the key remains usable but envelope creation fails without leaving a file, the public-key reload succeeds and certify_repo_transparency.go:180–182 reports only the missing envelope. Some key-loading failures can be reported again by the public-key reload, but that does not refute the lost diagnostic on the envelope-write failure path.
confirmed by pdbethke — real, low, checked by reading main b6638031: writeAuditStatement discards signErr (certify_repo.go:5103, sha, _, _, err), which is right for a plain --attest run but loses the real cause on --transparency, where the operator then sees only 'reading the signed envelope … no such file'. The computed-then-dropped diagnostic shape AGENTS.md names.
ref e1608f9712359ac8221867374d47accbd409b6c456cc2e0ae8b3f76e1bfd7819#R4
Recorded so that "found nothing here" can be told apart from "never looked here".
The file is small and mostly careful: the uploaded bytes are read back off disk, the upload fails open without touching the exit code, the stale envelope is removed before a new one is signed, and the Rekor URL is resolved by the same function on the upload and verify doors. The one claim the file makes that execution refutes is in finding 1: the --transparency help text promises the feature "never mints a fresh key just to have one", yet loadLocalCertifyKeyIfConfigured (lines 82-84) hands any non-empty CORRALAI_CERTIFY_KEY_FILE straight to LoadOrCreateSigningKey with no existence check, so a mistyped path mints a brand-new identity, signs the statement with it, and logs the result into a public, permanent log. The function's doc comment calls this a deliberate bootstrap, which is fine for `corral certify`'s own signing but is exactly the behavior the user-facing flag text says cannot happen. Finding 2 is the repository's own recurring shape — a rule at one door and not the others: a failed push and a failed statement write both raise a ::warning annotation on a runner, a failed transparency upload prints one plain stderr line and the job goes green with NULL receipt columns. Findings 3 and 4 are low: writeStatement can return an error after the plain file is already on disk (so the runner message "nothing will be attested" is false), and the real sign error is discarded on the --transparency path, so the operator is told the envelope could not be read rather than why it was never written.
I could not refute R1–R4. R1's bootstrap is explicitly intentional, but the help text still promises otherwise. R2's fail-open behavior is documented, but that does not resolve the missing annotation. R3's partial-write sequence and Action consumer support the finding. R4's signing diagnostic is discarded despite the early guard not guaranteeing successful envelope creation. These conclusions are from code inspection only; I ran no tests or repository code.