Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions .github/scripts/sign_macos.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
33 changes: 30 additions & 3 deletions .github/workflows/attest-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,42 @@ 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
echo "::error::the checksums file on the release hashes to $actual, but this run"
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
Expand Down Expand Up @@ -96,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"))
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ jobs:
echo
echo "\`\`\`"
echo "gh attestation verify <the file you downloaded> -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"
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/verify-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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."
Expand Down
91 changes: 91 additions & 0 deletions internal/guard/attestation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package guard
import (
"os"
"path/filepath"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -138,6 +139,96 @@ 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")
}
}

// 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
Expand Down
39 changes: 39 additions & 0 deletions internal/guard/macossigning_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading