fix(cosign-retry): stop stale-artifact fail-open and equals-form fail-closed (cli#568)

[aptracebloc]

Closes#568

What

Two Bugbot findings on scripts/cosign-retry.sh (introduced by #563), both about how the retry harness tracks --output-* paths. The script's model of "which files should exist after this attempt" was derived once and never reconciled with reality, so it failed both directions.

Defect 1 — fail OPEN on stale artifacts (Medium)

Retry attempts never cleared prior --output-* files. The fail-closed check only tests [ -s "$out" ], so a leftover non-empty .cert/.sig from an earlier failed attempt was treated as proof that a later exit 0 (which wrote nothing) had signed — the harness reporting a signature that does not exist.

Fix: remove every parsed --output-* path immediately before each cosign attempt, so [ -s ] is a claim about the current attempt alone.

Defect 2 — fail CLOSED on equals-form flags (Low)

outputs_from_args() set prev="$a" unconditionally, so after an equals-form flag (--output-x=val) prev still matched --output-* and the following token was misparsed as a phantom output path — failing a genuine equals-form sign-blob with exit 3 on a path cosign was never asked to write.

Fix: reset prev after handling the equals form; both space-separated and =value forms keep working.

Tests

Extended scripts/tests/cosign-retry-verify.sh (the harness build.yml runs), keeping its style of driving the real script through a PATH-shimmed fake cosign:

• Stale-leftover (case 15): attempt 1 writes real .cert/.sig then dies transient; attempt 2 exits 0 writing nothing → must fail closed (exit 3).
• Equals-form parse (case 16): a good sign with --output-x=path flags must succeed (exit 0), not fail closed on phantom paths.
• Equals-form still tracked (case 17): equals-form + empty output still fails closed.
• Wipe ordering (case 18): attempt 1 writes junk then dies transient; attempt 2 re-signs cleanly → still succeeds (the wipe runs before the attempt, not after).

Evidence

• Confirmed the two bug-catching cases FAIL against the pre-fix script (case 15: rc=0 want 3 — reported "signed on attempt 2/3" off attempt-1 debris; case 16: rc=3 want 0 — good sign failed closed on --output-signature=…), and all 19 pass on the fix.
• shellcheck --shell=bash --severity=error and bash -n clean on both files (the build.yml gates).

Header property #3 (FAIL CLOSED) updated to describe the per-attempt clearing so the doc matches the code.

🤖 Generated with Claude Code

---

Note

Medium Risk
Touches fail-closed signing verification used in release.yml; the change reduces false “signed” outcomes rather than weakening gates, but incorrect path parsing or wipe ordering could still break legitimate retries.

Overview
Fixes two bugs in how scripts/cosign-retry.sh tracks --output-* paths during release signing retries.

Stale artifacts (fail-open): The script now deletes every parsed --output-* path before each cosign attempt, so the post-success [ -s ] check only reflects what the current attempt wrote. Without that wipe, leftover .cert/.sig from a failed retry could make a later exit 0 with no new output look like a successful sign.

Equals-form flags (fail-closed):outputs_from_args now clears its “previous token” state after --output-x=path, so the next CLI argument is not treated as a phantom output file. Space-separated --output-x path behavior is unchanged.

The verify harness adds four cases (stale leftover, equals-form success, equals-form empty output, wipe-before-retry happy path) plus a run_under_test_equals helper. Header comments for property #3 are updated to match the per-attempt wipe.

^{Reviewed by Cursor Bugbot for commit b8ec411. Bugbot is set up for automated code reviews on this repo. Configure here.}

[saadqbal]

Approving. Two defects in opposite directions from one root cause — a model of "which files should exist" derived once and never reconciled with reality — and both fixes are placed where the property actually lives rather than patched at the symptom.

The wipe is inside the retry loop, before the attempt. That is the whole fix: it makes [ -s "$out" ] a claim about this attempt instead of about the filesystem. Clearing once before the loop would have left attempts 2..N reading attempt 1's debris, which is the original bug with a smaller window.

On the equals-form, the continue is load-bearing and not decoration.prev='' alone would be undone by prev="$a" at the loop bottom, so the flag would still match --output-* on the next iteration and swallow the following token. Skipping that assignment is what actually closes it. Worth stating because a future tidy-up that "simplifies" the continue away reopens the bug silently.

The evidence is the right kind. Confirming case 15 and 16 fail against the pre-fix script with the exact reversed codes — rc=0 where 3 was wanted, rc=3 where 0 was wanted — is a demonstration rather than a claim, and it is the one thing that distinguishes a real regression test from a test that happens to pass.

Case 18 is the one I would have asked for: the wipe could plausibly have broken the success path, so proving a transient-then-clean-resign still exits 0 guards the fix's own risk rather than only the bug it fixes.

Updating the header invariant in the same change keeps the file's stated contract true — the docstring now says the files are removed before each attempt, which is the sentence someone will rely on next.

26 checks green, no open threads.