From f77e9f1eb80a077c6334b0e87da747501e4f22fd Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 18 Aug 2026 13:19:50 +0200 Subject: [PATCH 1/2] fix(install.ps1): verify-blob must not inherit a stale exit code `$LASTEXITCODE` persists from the previous command. `Test-CosignRuns` runs `cosign version` immediately before the verification, and presets 255 precisely so a binary that never starts cannot leave a stale 0 behind. The `verify-blob` call that actually GATES the install had no such preset. So a cosign shim that exits 0 on `version` and then no-ops on `verify-blob` leaves `$LASTEXITCODE` at 0, the `-ne 0` gate reads that as success, and the installer prints "cosign signature valid" and installs a binary nothing verified. That is RFC-0001 R8 defeated by a stale variable, one line from the guard that exists. Reproduced before fixing, driving the real block extracted from install.ps1 under pwsh with a no-op verifier: with fix -> REFUSED (LASTEXITCODE=255) without fix -> INSTALLED-UNVERIFIED (LASTEXITCODE=0) The regression assertion is a SOURCE check, and the limitation is stated rather than hidden: install.ps1 has NO behavioural coverage -- there is no pwsh or Pester anywhere in this repo's CI, which is why a signature gate that does not gate reached a promotion PR. Case 19 asserts the preset sits inside the `$sigDownloaded` block and before the invocation, by line number, so a preset elsewhere cannot satisfy it. It closes this hole; it does not make the Windows installer tested. Worth its own ticket. Mutation-proved: removing the preset reddens case 19 with the message naming the consequence. 34 passed / 0 failed; the mutation gives 33/1. One self-inflicted detail recorded because it is the repo's own failure class: the first version of the check grepped for `verify-blob` and matched the explanatory comment written directly above the call, so it failed on correct code. Anchored on the `& $cosign` invocation instead -- prose is not wiring. Found by Bugbot on release-train promotion PR cli#528 (High). Co-Authored-By: Claude Opus 5 --- scripts/install.ps1 | 14 ++++++++++++ scripts/tests/install-verify.sh | 38 +++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 7d7266c..0ca0cf2 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -327,6 +327,20 @@ try { } if ($sigDownloaded) { + # PRESET 255, exactly as Test-CosignRuns does — and for the same reason, + # which this call site was missing (Bugbot, HIGH, cli#528). + # + # $LASTEXITCODE persists from the PREVIOUS command. Test-CosignRuns runs + # `cosign version` immediately before this, so a shim that exits 0 there + # and then never sets an exit code on verify-blob leaves $LASTEXITCODE at + # 0 — and the `-ne 0` check below reads that stale success as a valid + # signature. The installer then prints "cosign signature valid" and + # installs a binary nothing verified. + # + # 255 means "no verdict yet": a verifier that never runs cannot inherit a + # pass. Only cosign actually completing can bring it back to 0. That is + # the whole guarantee behind RFC-0001 R8, and it was one line away. + $global:LASTEXITCODE = 255 & $cosign verify-blob ` --certificate-identity-regexp "https://github.com/$GitHubRepo/.github/workflows/release.yml@refs/tags/v.*" ` --certificate-oidc-issuer 'https://token.actions.githubusercontent.com' ` diff --git a/scripts/tests/install-verify.sh b/scripts/tests/install-verify.sh index 58d30a1..b2ee2a3 100755 --- a/scripts/tests/install-verify.sh +++ b/scripts/tests/install-verify.sh @@ -529,6 +529,44 @@ else fi drop_sandbox +# -- 19. install.ps1 presets LASTEXITCODE before verify-blob ----------------- +# THE WINDOWS HALF OF R8, and until now nothing checked it. $LASTEXITCODE persists +# from the previous command, so a cosign shim that exits 0 on `version` (which +# Test-CosignRuns runs immediately before) and then no-ops on verify-blob leaves a +# stale 0 behind — and the `-ne 0` gate reads that as a valid signature. The +# installer prints "cosign signature valid" and installs a binary nothing verified +# (Bugbot, HIGH, cli#528). +# +# Test-CosignRuns already presets 255 for exactly this reason. This asserts the +# same preset guards the call that actually gates the install. +# +# A SOURCE ASSERTION, and the limitation is worth stating: install.ps1 has no +# behavioural coverage at all — there is no pwsh or Pester anywhere in this repo's +# CI, which is why the gap reached a promotion PR. This closes the specific hole; +# it does not make the Windows installer tested. Reproduced behaviourally with +# pwsh before writing it: without the preset the block accepts an unverified +# binary, with it it refuses. +PS1="$SELF_DIR/../install.ps1" +if [ ! -f "$PS1" ]; then + bad "install.ps1 not found — cannot assert the verify-blob exit preset" +else + # The preset must appear INSIDE the $sigDownloaded block and BEFORE the + # verify-blob invocation. Line numbers, so a preset elsewhere in the file + # cannot satisfy it. + # The INVOCATION, not the word: the first version grepped for `verify-blob` + # and matched the explanatory comment written directly above the call, so the + # preset looked out of order and the check failed on correct code. Prose is not + # wiring -- anchor on the `& $cosign` invocation itself. + vb_line=$(grep -n '& \$cosign verify-blob' "$PS1" | head -1 | cut -d: -f1) + pre_line=$(awk '/\$global:LASTEXITCODE = 255/ {print NR}' "$PS1" | awk -v v="${vb_line:-0}" '$1 < v {last=$1} END {print last+0}') + blk_line=$(grep -n 'if ($sigDownloaded)' "$PS1" | head -1 | cut -d: -f1) + if [ -n "$vb_line" ] && [ -n "$blk_line" ] && [ "$pre_line" -gt "$blk_line" ] && [ "$pre_line" -lt "$vb_line" ]; then + ok "install.ps1: LASTEXITCODE preset guards verify-blob (line $pre_line, before $vb_line)" + else + bad "install.ps1: no LASTEXITCODE preset between the \$sigDownloaded block (line ${blk_line:-?}) and verify-blob (line ${vb_line:-?}) — a no-op verifier would inherit a stale 0 and install unverified" + fi +fi + echo echo "install-verify: $PASS passed, $FAIL failed" [ "$FAIL" -eq 0 ] From 372e86882ca85d3aebbfb396156e9f9e41d0ecc6 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 18 Aug 2026 16:31:55 +0200 Subject: [PATCH 2/2] fix(install-verify): match the STATEMENT, not the text (shujaatTracebloc, #529) Both review findings were right, and both are the failure this PR is about -- left open on the side the PR did not anchor. A COMMENTED-OUT PRESET SATISFIED CASE 19. The check matched the substring `$global:LASTEXITCODE = 255` anywhere on a line, so `# $global:LASTEXITCODE = 255` passed it: the gate dead, the suite green. That is the likelier human mutation -- commenting the line out while debugging the installer -- and it was exactly the one not covered. The PR proved the DELETE mutation and missed this one. THE MIRROR IMAGE, ON THE SAME LINE. Hard-coded single spaces meant `$global:LASTEXITCODE=255` -- correct, equivalent PowerShell -- turned case 19 RED on a working gate, with a message asserting the installer would install unverified. A false alarm that names a supply-chain failure is worse than none. Anchoring the whole statement start-to-end, with flexible spacing, closes both. Suggestion taken as written from the review. AND THE `$` IS ESCAPED in the `if ($sigDownloaded)` grep, matching the two sibling patterns in the same block. In a POSIX BRE a `$` that is not at the end is undefined; an implementation treating it as an anchor matches nothing, `blk_line` comes back empty, and case 19 fails on correct code. Verified all three directions on this branch, reproducing the reviewer's results first: commented-out preset before 34/0 (escaped) -> after 33/1 (caught) no-space variant before 33/1 (false) -> after 34/0 (correct) preset deleted before 33/1 -> after 33/1 (still caught) Co-Authored-By: Claude Opus 5 --- scripts/tests/install-verify.sh | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/scripts/tests/install-verify.sh b/scripts/tests/install-verify.sh index b2ee2a3..018612a 100755 --- a/scripts/tests/install-verify.sh +++ b/scripts/tests/install-verify.sh @@ -558,8 +558,21 @@ else # preset looked out of order and the check failed on correct code. Prose is not # wiring -- anchor on the `& $cosign` invocation itself. vb_line=$(grep -n '& \$cosign verify-blob' "$PS1" | head -1 | cut -d: -f1) - pre_line=$(awk '/\$global:LASTEXITCODE = 255/ {print NR}' "$PS1" | awk -v v="${vb_line:-0}" '$1 < v {last=$1} END {print last+0}') - blk_line=$(grep -n 'if ($sigDownloaded)' "$PS1" | head -1 | cut -d: -f1) + # A WHOLE STATEMENT, not a substring (shujaatTracebloc, #529). Two failures in + # opposite directions came from matching text rather than code: + # * a COMMENTED-OUT preset satisfied the check -- the gate dead, case 19 green. + # That is the likelier human mutation (commenting it out while debugging the + # installer) and it was the one not covered. + # * hard-coded single spaces meant `$global:LASTEXITCODE=255` -- correct, + # equivalent PowerShell -- turned case 19 RED on a working gate, asserting the + # installer would install unverified. + # Anchoring start-to-end with flexible spacing closes both. + pre_line=$(awk '/^[[:space:]]*\$global:LASTEXITCODE[[:space:]]*=[[:space:]]*255[[:space:]]*$/ {print NR}' "$PS1" | awk -v v="${vb_line:-0}" '$1 < v {last=$1} END {print last+0}') + # `\$` ESCAPED, like the two sibling patterns in this block. In a POSIX BRE a `$` + # that is not at the end is undefined; an implementation treating it as an anchor + # matches nothing, `blk_line` comes back empty, and case 19 goes red on correct + # code with the "would install unverified" message (shujaatTracebloc, #529). + blk_line=$(grep -n 'if (\$sigDownloaded)' "$PS1" | head -1 | cut -d: -f1) if [ -n "$vb_line" ] && [ -n "$blk_line" ] && [ "$pre_line" -gt "$blk_line" ] && [ "$pre_line" -lt "$vb_line" ]; then ok "install.ps1: LASTEXITCODE preset guards verify-blob (line $pre_line, before $vb_line)" else