From 9727d197766070a25a7d44f06ae1e14fd78f96c8 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Fri, 28 Aug 2026 23:58:28 +0200 Subject: [PATCH 1/4] release: the macOS signing script stops changing directory Found by the first real release, which it stopped twice in two consecutive steps. Phase B refused on tfg-gui.app with "was signed by a DIFFERENT certificate, expected C656..., got none". The signature was correct: codesign -dv on that same bundle shows Developer ID Application, a chain to the Apple Root CA and a timestamp. What failed was reading it back. certificate_of cd'd into a temporary directory and then asked codesign about the bundle, because --extract-certificates looked like a file name rather than a path. The bundle path is relative - sign_release.py hands this script a directory under the home directory - so after the cd it named nothing, codesign answered "No such file or directory", and no certificate came out. A refusal that is right about there being a problem and wrong about what it is costs more than a silent one: it sends somebody to look at the card. Measured on the Mac, all three ways: relative after a cd gives nothing, absolute gives exactly the pinned digest, and relative with the prefix written as a path and no cd gives the same. So the cd goes. The same trap sat one step further on and was measured before anybody reached it: the zip for notarisation cd'd into the unpacked directory and wrote to a path relative to where the script started, so the file was never created. Bare ditto with both paths as they arrive works, and --keepParent still puts the .app at the top of the archive - read back out of the zip rather than assumed. Why the rehearsal did not show this: it ran the script by hand with an absolute directory, and the real caller passes a relative one. The command was proven, the call was not. The guard asks whether there is any cd at all rather than whether the paths are absolute, because that is the property that can be read off the file. If a directory change is ever genuinely needed here, the guard is the conversation about it. Co-Authored-By: Claude Opus 5 --- .github/scripts/sign_macos.sh | 27 ++++++++++++++++++-- internal/guard/macossigning_test.go | 39 +++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/.github/scripts/sign_macos.sh b/.github/scripts/sign_macos.sh index 58c92e7..0bdd9d1 100755 --- a/.github/scripts/sign_macos.sh +++ b/.github/scripts/sign_macos.sh @@ -120,10 +120,26 @@ A renewal is a DIFFERENT certificate and internal/legal has to move with it." } # The certificate that ACTUALLY signed a bundle, read back out of it. +# 🔴 Nothing here changes directory, and that is the whole of what this function +# learned on 2026-08-28, on the first real release. +# +# It used to cd into the temporary directory and pass the bundle path after +# that, because --extract-certificates looked like a file NAME. The path it was +# given is relative - sign_release.py passes a directory under the home +# directory - so after the cd it named nothing. codesign answered "No such file +# or directory", no certificate came out, and the script stopped the release +# saying the bundle "was signed by a DIFFERENT certificate, got none" about a +# bundle that was correctly signed, by the pinned certificate, with a timestamp. +# A refusal that is right about there being a problem and wrong about what it is +# costs more than a silent one, because it sends somebody to look at the card. +# +# The prefix takes a path. Measured rather than assumed: with the prefix written +# as "$tmp/cert" and the bundle path left exactly as it arrives, cert0 appears +# and hashes to the pinned digest. certificate_of() { local bundle="$1" tmp tmp="$(mktemp -d)" - ( cd "$tmp" && codesign -d --extract-certificates=cert "$bundle" >/dev/null 2>&1 ) + codesign -d --extract-certificates="$tmp/cert" "$bundle" >/dev/null 2>&1 if [ ! -f "$tmp/cert0" ]; then rm -rf "$tmp" echo "none" @@ -172,8 +188,15 @@ Nothing has been handed back." # Apple takes a zip, a pkg or a dmg, and will not take the tar.gz we publish. # So the zip exists only to carry the bundle there. What gets stapled and # republished is the bundle itself. + # The same trap as certificate_of, one step further on, and it was measured + # the same day rather than met later: this cd'd into the unpacked directory + # and then wrote to a path that was relative to where the script STARTED, so + # the zip was never created and the next line would have stopped the release + # with "could not zip". Without the cd, ditto is given both paths as they + # arrive and --keepParent still puts tfg-gui.app at the top of the archive, + # which is the shape notarisation wants - read back out of the zip. local zip="${work}/notarise.zip" - ( cd "$work" && ditto -c -k --keepParent "$(basename "$app")" "$zip" ) || + ditto -c -k --keepParent "$app" "$zip" || die "could not zip $(basename "$app") for notarisation" local out diff --git a/internal/guard/macossigning_test.go b/internal/guard/macossigning_test.go index ff4ea6c..7e6bd1f 100644 --- a/internal/guard/macossigning_test.go +++ b/internal/guard/macossigning_test.go @@ -204,6 +204,45 @@ func TestTheIconMacOSReadsCarriesEverySizeItIsAskedFor(t *testing.T) { t.Logf("%d entries, every size macOS asks for, %d bytes", len(found), len(body)) } +// The macOS signing script never changes directory. +// +// Found by the first real release, on 2026-08-28, and it stopped that release +// twice in two consecutive steps. Every path this script works with is derived +// from the directory it is handed, and sign_release.py hands it one relative to +// the home directory. Two commands ran inside a subshell that had cd'd +// somewhere else first, so the paths they were given stopped meaning anything: +// +// - certificate_of cd'd into a temporary directory and then asked codesign +// about the bundle. codesign answered "No such file or directory", no +// certificate came out, and the script refused the release saying the +// bundle was "signed by a DIFFERENT certificate, got none" - about a bundle +// that was correctly signed by the pinned certificate, with a timestamp, +// chaining to the Apple Root CA. Measured after the fact: the same command +// with the path resolved gives exactly the pinned digest; +// - the zip for notarisation cd'd into the unpacked bundle and wrote to a +// path relative to where the script started, so the file was never created. +// That one had not been reached yet and would have stopped the next step. +// +// Asked as "no cd at all" rather than "the paths are absolute", because that is +// the property that can be read off the file. If a directory change is ever +// genuinely needed here, make every path absolute first and this guard is the +// conversation about it. +// +// 🔴 Why nothing caught this before: the rehearsal of 2026-08-28 ran the script +// by hand with an absolute directory, and sign_release.py passes a relative one. +// The command was proven, the call was not. +func TestTheMacSigningScriptDoesNotDependOnWhereItIsRunFrom(t *testing.T) { + script := macSigningScript(t) + + if strings.Contains(script, "cd \"") { + t.Error("sign_macos.sh changes directory somewhere. Every path in it comes from the " + + "directory it is handed, and sign_release.py hands it a relative one - so a cd " + + "silently changes what those paths mean.\n" + + "What happened when this was last true: codesign reported no certificate for a " + + "correctly signed bundle, and the release stopped saying the wrong thing about why.") + } +} + // The pin is a digest with a date, and the script derives its selector from it. // // Same shape as the Windows pin and for the same reason: codesign selects by From 7917adb782c96f23df73e8bf999572f3afab6073 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sat, 29 Aug 2026 00:17:11 +0200 Subject: [PATCH 2/4] release: the attesting half fetches what the checksums name The second thing the first real release found, and this one landed after the signing was already done - which is the expensive moment for a release step to stop. attest-release.yml fetched the draft with its own list of patterns - archives, the SBOM, the checksums - and then ran sha256sum -c over the checksums. The checksums describe ten files, including the provenance bundle, and no pattern matched that one. sha256sum said "No such file or directory" about a file that was on the release page the whole time, and the run failed. The failure mode is not a missing file. It is a check reporting a problem with the release when the problem is in the checker, at the one moment when somebody is holding a card and half a release. So the second list goes rather than growing a pattern: the names come out of verify-SHA256SUMS.txt, which is the same document that is then verified. An asset added later cannot make the two disagree, because there is one list. The names are collected before anything is fetched, because gh reads stdin and a download inside the read loop eats the rest of the list. Co-Authored-By: Claude Opus 5 --- .github/workflows/attest-release.yml | 31 ++++++++++++++++++++++++-- internal/guard/attestation_test.go | 33 ++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/.github/workflows/attest-release.yml b/.github/workflows/attest-release.yml index 675f16c..c2cd1f9 100644 --- a/.github/workflows/attest-release.yml +++ b/.github/workflows/attest-release.yml @@ -56,8 +56,21 @@ jobs: set -euo pipefail mkdir -p signed cd signed - gh release download "$TAG" --pattern '*.zip' --pattern '*.tar.gz' \ - --pattern '*.spdx.json' --pattern 'verify-SHA256SUMS.txt' + # The checksums first, because they are the list of what to fetch. + # + # 🔴 This used to be a hand written list of patterns beside them, and + # the first real release is what said why that cannot work: the + # patterns fetched the archives, the SBOM and the checksums, and the + # checksums also describe the provenance bundle - which nothing in + # that list matched. sha256sum then reported "No such file or + # directory" for a file that was on the release page all along, and + # the run stopped after the signing was already done. + # + # Two lists of what a release holds is one list too many. The names + # come out of the checksums now, so the thing being verified is also + # the thing that says what to verify, and adding an asset cannot make + # them disagree again. + gh release download "$TAG" --pattern 'verify-SHA256SUMS.txt' actual="$(sha256sum verify-SHA256SUMS.txt | cut -d' ' -f1)" echo "the release carries: $actual" if [ "$actual" != "$CLAIMED" ]; then @@ -65,6 +78,20 @@ jobs: echo "::error::was dispatched for $CLAIMED - something changed in between" exit 1 fi + # Collected before anything is fetched, because gh reads stdin and a + # download inside the read loop eats the rest of the list. + names=() + while read -r _ name; do + [ -n "${name:-}" ] && names+=("$name") + done < verify-SHA256SUMS.txt + if [ "${#names[@]}" -eq 0 ]; then + echo "::error::verify-SHA256SUMS.txt names no files, so there is nothing to attest" + exit 1 + fi + echo "the checksums describe ${#names[@]} file(s)" + for name in "${names[@]}"; do + gh release download "$TAG" --pattern "$name" + done # And the checksums have to describe the files that came with them, # because everything below is a statement about that list. sha256sum -c verify-SHA256SUMS.txt diff --git a/internal/guard/attestation_test.go b/internal/guard/attestation_test.go index 6bff93f..118bf29 100644 --- a/internal/guard/attestation_test.go +++ b/internal/guard/attestation_test.go @@ -138,6 +138,39 @@ func TestTheReleaseMakesItsDocumentAndHandsItOver(t *testing.T) { // And the notes tell somebody how to use any of it. A statement nobody knows // about is a statement nobody checks. +// What the attesting half downloads is named BY the checksums, not beside them. +// +// Found by the first real release, on 2026-08-28, after the signing was already +// done. That job fetched the release with a hand written list of patterns - +// archives, the SBOM, the checksums - and then ran sha256sum -c over the +// checksums. The checksums also describe the provenance bundle, which no +// pattern matched, so sha256sum said "No such file or directory" about a file +// that was sitting on the release page the whole time, and the run stopped. +// +// Two lists of what a release holds is one list too many, and the failure mode +// is not a missing file: it is a check that reports a problem with the release +// when the problem is in the checker. That is expensive here, because it lands +// at the one moment when somebody is holding a card and half a release. +// +// So the names come out of the file being verified. Adding an asset cannot make +// the two disagree again, because there is only one list. +func TestTheAttestingHalfFetchesWhatTheChecksumsName(t *testing.T) { + attest := workflowText(t, "attest-release.yml") + + if strings.Contains(attest, "--pattern '*.zip'") { + t.Error("attest-release.yml fetches the release with its own list of patterns. " + + "That list is a second description of what a release holds, and when it " + + "disagrees with the checksums the run stops with a message about the release " + + "rather than about itself.\n" + + "What to do: read the names out of verify-SHA256SUMS.txt and fetch those.") + } + if !strings.Contains(attest, "done < verify-SHA256SUMS.txt") { + t.Error("attest-release.yml does not take the list of files to fetch from " + + "verify-SHA256SUMS.txt, so nothing keeps what it downloads and what it " + + "verifies in step") + } +} + func TestTheReleaseNotesSayHowToCheckWhatWasDownloaded(t *testing.T) { job := releasePublishJob(t) var notes string From c4f73e71ba8484f73edd419fc6f7e27786443660 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sat, 29 Aug 2026 00:21:38 +0200 Subject: [PATCH 3/4] release: every workflow names the predicate type the statement actually carries The third thing the first real release found, and this refusal was correct. Phase C stopped with "the release notes tell people to pass --predicate-type https://spdx.dev/Document and the statement that was just made is https://spdx.dev/Document/v2.3". That check exists precisely because the provenance plan wrote the URI down as a promise taken from the action's documentation rather than from an attestation, and said out loud it was unmeasured. It has now been measured: the statement carries the versioned one. The value was written in five places across three files and only one of them was being checked. Three of the five are not checks at all - they are the commands a person copies out of the release notes, and with the wrong URI gh answers "no attestation found", which reads exactly like a release nobody attested. It cannot be written once: three workflows are three files and a workflow cannot read a constant out of another one. So the guard asks for agreement rather than for a single home - it collects every spdx.dev URI in .github/workflows and refuses when there are two different ones. The lesson worth keeping: read a predicate type out of a bundle, not out of documentation. base64 -d on dsseEnvelope.payload and predicateType is inside. Co-Authored-By: Claude Opus 5 --- .github/workflows/attest-release.yml | 2 +- .github/workflows/release.yml | 2 +- .github/workflows/verify-release.yml | 6 +-- internal/guard/attestation_test.go | 58 ++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 5 deletions(-) diff --git a/.github/workflows/attest-release.yml b/.github/workflows/attest-release.yml index c2cd1f9..2bf7710 100644 --- a/.github/workflows/attest-release.yml +++ b/.github/workflows/attest-release.yml @@ -123,7 +123,7 @@ jobs: # missing file rather than a wrong flag. run: | set -euo pipefail - promised="https://spdx.dev/Document" + promised="https://spdx.dev/Document/v2.3" actual="$(python3 - "$BUNDLE" <<'PY' import base64, json, sys bundle = json.load(open(sys.argv[1], encoding="utf-8")) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0d1ddb4..f8b7352 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -465,7 +465,7 @@ jobs: echo echo "\`\`\`" echo "gh attestation verify -R ${GITHUB_REPOSITORY} \\" - echo " --predicate-type https://spdx.dev/Document" + echo " --predicate-type https://spdx.dev/Document/v2.3" echo "\`\`\`" echo echo "The predicate type has to be given. \`gh\` asks for build provenance unless told" diff --git a/.github/workflows/verify-release.yml b/.github/workflows/verify-release.yml index 19bbe26..d52732e 100644 --- a/.github/workflows/verify-release.yml +++ b/.github/workflows/verify-release.yml @@ -222,7 +222,7 @@ jobs: gh attestation verify "$linux" -R "$GITHUB_REPOSITORY" echo "what is inside, on a signed file: $windows" gh attestation verify "$windows" -R "$GITHUB_REPOSITORY" \ - --predicate-type https://spdx.dev/Document + --predicate-type https://spdx.dev/Document/v2.3 - name: the same two commands with no network to the API shell: bash @@ -237,7 +237,7 @@ jobs: --bundle ./*.provenance.sigstore.json gh attestation verify "$windows" -R "$GITHUB_REPOSITORY" \ --bundle ./*.sbom.sigstore.json \ - --predicate-type https://spdx.dev/Document + --predicate-type https://spdx.dev/Document/v2.3 - name: every Windows program is signed, stamped, and by OUR certificate shell: pwsh @@ -289,7 +289,7 @@ jobs: run: | set -euo pipefail gh release view "$TAG" --repo "$GITHUB_REPOSITORY" --json body --jq .body > notes.md - for promised in "gh attestation verify" "--predicate-type https://spdx.dev/Document" "verify-SHA256SUMS.txt"; do + for promised in "gh attestation verify" "--predicate-type https://spdx.dev/Document/v2.3" "verify-SHA256SUMS.txt"; do grep -qF -- "$promised" notes.md || { echo "the release notes never mention: $promised" echo "This job just ran it, so the page and the check disagree about what a person should do." diff --git a/internal/guard/attestation_test.go b/internal/guard/attestation_test.go index 118bf29..1f89d8b 100644 --- a/internal/guard/attestation_test.go +++ b/internal/guard/attestation_test.go @@ -3,6 +3,7 @@ package guard import ( "os" "path/filepath" + "regexp" "strings" "testing" @@ -171,6 +172,63 @@ func TestTheAttestingHalfFetchesWhatTheChecksumsName(t *testing.T) { } } +// Every workflow that names the SBOM predicate type names the SAME one. +// +// This fact is written five times across three files, and it cannot be written +// once: the notes tell a person what to type, the attesting half checks that +// promise against the statement it just made, and the verifying half runs the +// command for real. They are three different jobs in three different files and +// a workflow cannot read a constant out of another one. +// +// 🔴 What it cost when they disagreed, on the first real release, on +// 2026-08-28: the notes promised https://spdx.dev/Document, the statement +// carried https://spdx.dev/Document/v2.3, and the attesting half stopped the +// release. It was RIGHT to - the promise was written from the action's +// documentation rather than from an attestation, and the provenance plan said +// out loud that it was unmeasured - but the value that was wrong was written in +// five places and only one of them was checked. +// +// The one that matters most is not the check. It is the pair of commands a +// person types out of the release notes: with the wrong URI, gh answers "no +// attestation found", which reads exactly like a release nobody attested. +func TestEveryWorkflowNamesTheSamePredicateType(t *testing.T) { + dir := filepath.Join(repoRoot(t), ".github", "workflows") + entries, err := os.ReadDir(dir) + if err != nil { + t.Skipf("no workflows here: %v", err) + } + + uri := regexp.MustCompile(`https://spdx\.dev/[A-Za-z0-9./-]*`) + found := map[string][]string{} + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".yml") { + continue + } + body, err := os.ReadFile(filepath.Join(dir, entry.Name())) + if err != nil { + t.Fatalf("reading %s: %v", entry.Name(), err) + } + for _, match := range uri.FindAllString(string(body), -1) { + found[match] = append(found[match], entry.Name()) + } + } + + if len(found) == 0 { + t.Fatal("no workflow names the SBOM predicate type, so this guard checked nothing") + } + if len(found) > 1 { + for value, files := range found { + t.Errorf("%q is named in %v", value, files) + } + t.Error("the workflows disagree about the SBOM predicate type. One of them tells a " + + "person what to type, one checks that promise against the statement, and one " + + "runs the command for real - so a disagreement here is a release page whose " + + "own instructions answer \"no attestation found\".\n" + + "What to do: the value is whatever the statement actually carries. Read it out " + + "of a bundle rather than out of documentation.") + } +} + func TestTheReleaseNotesSayHowToCheckWhatWasDownloaded(t *testing.T) { job := releasePublishJob(t) var notes string From 270a0ccb3cbe2c7cf3af004073c03f92465c3512 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sat, 29 Aug 2026 00:28:12 +0200 Subject: [PATCH 4/4] guard: no symbols in the comments that ship Rule 13 again, and by me again - the same mistake this file already records twice. The markers belong in the project's notes, which are not in the repository. The workflows and the shell script keep theirs: the punctuation guard reads .go files, and those files have carried that style since they were written. Co-Authored-By: Claude Opus 5 --- internal/guard/attestation_test.go | 2 +- internal/guard/macossigning_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/guard/attestation_test.go b/internal/guard/attestation_test.go index 1f89d8b..e427ba0 100644 --- a/internal/guard/attestation_test.go +++ b/internal/guard/attestation_test.go @@ -180,7 +180,7 @@ func TestTheAttestingHalfFetchesWhatTheChecksumsName(t *testing.T) { // command for real. They are three different jobs in three different files and // a workflow cannot read a constant out of another one. // -// 🔴 What it cost when they disagreed, on the first real release, on +// What it cost when they disagreed, on the first real release, on // 2026-08-28: the notes promised https://spdx.dev/Document, the statement // carried https://spdx.dev/Document/v2.3, and the attesting half stopped the // release. It was RIGHT to - the promise was written from the action's diff --git a/internal/guard/macossigning_test.go b/internal/guard/macossigning_test.go index 7e6bd1f..ead7b7a 100644 --- a/internal/guard/macossigning_test.go +++ b/internal/guard/macossigning_test.go @@ -228,7 +228,7 @@ func TestTheIconMacOSReadsCarriesEverySizeItIsAskedFor(t *testing.T) { // genuinely needed here, make every path absolute first and this guard is the // conversation about it. // -// 🔴 Why nothing caught this before: the rehearsal of 2026-08-28 ran the script +// Why nothing caught this before: the rehearsal of 2026-08-28 ran the script // by hand with an absolute directory, and sign_release.py passes a relative one. // The command was proven, the call was not. func TestTheMacSigningScriptDoesNotDependOnWhereItIsRunFrom(t *testing.T) {