fix(release-assets): sign the tag into the checksums signature, and refuse a manifest signed for another release (#1516) #1518

Merged

alex merged 2 commits from claude/1516-signed-tag-binding into main 2026-10-02 20:26:53 +02:00

Conversation 6 Commits 2 Files changed 8 +247 -10

claude commented 2026-10-02 20:10:54 +02:00

Member

• Copy link
• Copy Markdown

Refs #1516

Implements option (B) of #1516, as decided in comment 114216: the signature's trusted comment names the tag, and fetch-rt.sh refuses a manifest whose verified comment does not name the release it fetched.

What changes

• .goreleaser.yaml signs.args gains -t "release-toolkit tag:{{ .Tag }} file:checksums.txt".
• .forgejo/workflows/goreleaser.yml (from review 7192, commit 66ee9ce): the goreleaser release step sets GORELEASER_CURRENT_TAG from the job's resolve release tag output on every event, not only on dispatch, and refuses an empty value. .Tag is what gets signed, so it must be the tag verify-fetch-arm fetches, not whatever git describes on a tag push.
• scripts/fetch-rt.sh verifies with minisign -V -Q. That prints the trusted comment, and only once both the file signature and the comment signature verify. The script then refuses unless the comment is exactly release-toolkit tag:${RT_VERSION} file:checksums.txt. The refusal names the comment it found and the one it expected. The check sits after signature verification and before the cache branch, so it runs on hits and misses, before any asset is fetched or checksummed.
• tests/fetch-rt.bats: the minisign mock now honours -q/-Q the way real minisign 0.12 does. Its signed comment is rendered from .goreleaser.yaml's own -t template in setup_file, so every passing fetch in the file also checks that the release pipeline and the verifier agree. Six new tests, detailed below.
• tests/workflows.bats (#513 test): pins the -t argument carrying {{ .Tag }}, fetch-rt.sh's -V -Q and expected-comment string, and the GORELEASER_CURRENT_TAG env line.
• .forgejo/workflows/tests.yml: the bats job's required tools become bats jq minisign, and the job prints minisign -v. goreleaser.yml already needs minisign on runs-on: go, and the real-minisign test skips without it. This turns that skip into a red.
• docs/SECURITY.md §Verifying a release: after minisign -Vm and sha256sum -c, check that the trusted comment names the tag you downloaded. Releases signed before this change carry minisign's default comment.
• Fragment: changelog.d/1516-signed-tag-binding.fixed.md.

Decision tree

Comment format and match.

• An exact whole-string match against one fixed format (this PR) is right when the release pipeline and the verifier live in the same repo and can be held in lockstep by a test. It rejects a longer tag (v9.9.90), trailing text and a missing file field, and the lockstep test catches drift on either side.
• Parsing a tag: field out of a looser comment is right if other signers must coexist. None do, and every extra accepted shape would be a way for a replay to pass.

How the comment is read.

• -V -Q (this PR) prints only the verified comment. Measured on minisign 0.12: an edited comment or an edited manifest exits 1 with no output.
• -V and parsing the Trusted comment: line works too, but couples the script to minisign's human-readable output.

Releases signed before this change.

• Refuse them (this PR) is right because every caller fetches its own release. bootstrap-rt.sh:34 passes the baked ref, and goreleaser.yml:1274 passes the tag it just published. Older tags run their own older fetch-rt.sh, so a fetcher containing this check meets an old signature only through a replay.
• A cutover allowance (accept a default comment for tags before vN) would help only a caller that fetches another tag's release with this script. There is none, and the allowance would reopen the replay for every old release.

Real minisign in CI.

• Require it in the bats job (this PR) is right because the go runner carries it already (every signed release proves the goreleaser job found it), and the mock cannot prove what real minisign prints.
• Leave the test skippable is right only if some go runner may lack minisign. Then that runner cannot cut a release either.

Self-bootstrap (AGENTS.md section 2)

No re-pin: toolkit-self floats @main (AGENTS.md §2.1) and builds rt from source, so it never runs fetch-rt.sh. Neither fetch-rt.sh nor .goreleaser.yaml is in gates.DefaultComposeScripts. Adopters get the check from the first cut tag that contains this commit. That cut is also the first live end-to-end run. Its goreleaser job signs with the new comment, and its verify-fetch-arm job fetches with the new script. The bats job's real-minisign test runs this PR's head on a go runner first, so the runner's minisign -Q behaviour is graded before merge, not at the cut.

Verification

Local runs used an LF clone at 8630004, re-run at 66ee9ce (base 0156d84 = live main), in rt-bats:local with Debian's minisign 0.12 added:

Check	Result
bats tests/ (the CI invocation)	221/221 ok, 0 skipped (the real-minisign test ran)
go test ./...	ok
shellcheck scripts/fetch-rt.sh, bash -n	clean
yaml.safe_load on .goreleaser.yaml and tests.yml	ok
anchor-check.py (renderer git.frankenbit.de)	0 unresolved
FRAGMENT_CHECK_COVERAGE=content rt fragment-check changelog.d	PASS, no warnings on the new fragment
rt register-check --stdin on each changed file	0 hits
close-keyword regex on title, body and commit	0 hits

goreleaser actually expands the template. I ran goreleaser v2.17.0, the GORELEASER_PIN in goreleaser.yml:217, as release --snapshot --skip=publish,validate,announce with a throwaway key. Its dist/checksums.txt.minisig reads trusted comment: release-toolkit tag:v0.62.4 file:checksums.txt, where .Tag is the nearest tag in snapshot mode, and minisign -V -Q prints exactly that.

New tests (tests/fetch-rt.bats 18–23):

Test	Proves
another release's comment	refused, "validly signed, but not for this release", both comments named, no asset request
minisign's default comment (pre-#1516)	refused
v9.9.90, trailing text, missing file field, empty	each refused: exact match
cache hit against another release's signed files	refused, no checksum OK, no asset request
v1.0.0-alpha.0	accepted whole
real minisign, signing with .goreleaser.yaml's own signs.args (goreleaser's variables filled in)	genuine v0.2.0 installs rt 0.2.0; v0.1.0's signed files served as v0.2.0 are refused; a default-comment signature is refused; an edited comment fails minisign verification failed

Mutation check (AGENTS.md §8 Axis B). Each mutant ran on a throwaway copy, and its diff was confirmed non-empty:

Mutant	Turns red
M1 the comment check removed	6 #1516 tests, and the #513 wiring test
M2 -Q changed to -q (no comment captured)	14 tests, every success path included, and the #513 test
M3 substring match instead of exact	the exact-match test
M4 the check runs on the miss path only	the cache-hit test
M5 -t dropped from .goreleaser.yaml	setup_file (whole file), and the #513 test
M6 the .goreleaser.yaml format drifts and fetch-rt.sh does not	13 tests, via the lockstep
M7 the check moved after the asset fetch	4 tests, including "refused before the asset is fetched"
M8 the GORELEASER_CURRENT_TAG env line dropped (66ee9ce)	the #513 test, naming the missing pin

#1516 acceptance criteria

AC	Here
1 decision recorded	comment 114216, option (B)
2 fetch-rt.sh refuses another release on hit and miss, naming both versions	yes. Tests 18, 19, 21 and 23
3 replay test, red before and green after, mutation-verified, controls kept	tests 18 and 23, M1–M7. The existing tampered-manifest, missing-signature and wrong-key tests are unchanged and green
4 prerelease tags	test 22
5 SECURITY.md states what the signature binds, with a manual step	§Verifying a release
6 (B): signing args changed, and the first signed release read back	args changed, and the local goreleaser v2.17.0 snapshot read back. The read-back of the first real release stays open until the next cut.
7 the public mirror's .minisig handling	measured, and needs no change. reusable-mirror-release.yml:310-327 copies every asset by name and never re-signs. The live mirror's v0.62.0 .minisig has the origin's trusted comment and timestamp (1788725401) byte for byte, so a tag-naming comment carries over to the same tag.
8 fragment	changelog.d/1516-signed-tag-binding.fixed.md

Review request

@alex, please verify at source (AGENTS.md §6). Both halves need checking:

1. The signing half: is .goreleaser.yaml's -t argument what you want stamped into every release from now on? It is part of what adopters will check by hand. And does the CI bats job log on this head show minisign -v and test 23 ran, not skipped?
2. The verifying half: run bats tests/fetch-rt.bats on the head with minisign installed. Then read fetch-rt.sh's refusal as an adopter who hit it would.

After the next cut, read back that release's checksums.txt.minisig trusted comment for AC 6. Pin your review with rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1518 --reviewed-head <full sha> (§6.2).

🤖 Generated with Claude Code

Refs #1516 Implements option (B) of #1516, as decided in comment 114216: the signature's trusted comment names the tag, and `fetch-rt.sh` refuses a manifest whose verified comment does not name the release it fetched. ## What changes - **`.goreleaser.yaml`** `signs.args` gains `-t "release-toolkit tag:{{ .Tag }} file:checksums.txt"`. - **`.forgejo/workflows/goreleaser.yml`** (from review 7192, commit `66ee9ce`): the goreleaser release step sets `GORELEASER_CURRENT_TAG` from the job's `resolve release tag` output on every event, not only on dispatch, and refuses an empty value. `.Tag` is what gets signed, so it must be the tag `verify-fetch-arm` fetches, not whatever git describes on a tag push. - **`scripts/fetch-rt.sh`** verifies with `minisign -V -Q`. That prints the trusted comment, and only once both the file signature and the comment signature verify. The script then refuses unless the comment is exactly `release-toolkit tag:${RT_VERSION} file:checksums.txt`. The refusal names the comment it found and the one it expected. The check sits after signature verification and before the cache branch, so it runs on hits and misses, before any asset is fetched or checksummed. - **`tests/fetch-rt.bats`**: the minisign mock now honours `-q`/`-Q` the way real minisign 0.12 does. Its signed comment is rendered from `.goreleaser.yaml`'s own `-t` template in `setup_file`, so every passing fetch in the file also checks that the release pipeline and the verifier agree. Six new tests, detailed below. - **`tests/workflows.bats`** (#513 test): pins the `-t` argument carrying `{{ .Tag }}`, `fetch-rt.sh`'s `-V -Q` and expected-comment string, and the `GORELEASER_CURRENT_TAG` env line. - **`.forgejo/workflows/tests.yml`**: the bats job's required tools become `bats jq minisign`, and the job prints `minisign -v`. `goreleaser.yml` already needs minisign on `runs-on: go`, and the real-minisign test skips without it. This turns that skip into a red. - **`docs/SECURITY.md`** §Verifying a release: after `minisign -Vm` and `sha256sum -c`, check that the trusted comment names the tag you downloaded. Releases signed before this change carry minisign's default comment. - **Fragment**: `changelog.d/1516-signed-tag-binding.fixed.md`. ## Decision tree **Comment format and match.** - *An exact whole-string match against one fixed format (this PR)* is right when the release pipeline and the verifier live in the same repo and can be held in lockstep by a test. It rejects a longer tag (`v9.9.90`), trailing text and a missing file field, and the lockstep test catches drift on either side. - *Parsing a `tag:` field out of a looser comment* is right if other signers must coexist. None do, and every extra accepted shape would be a way for a replay to pass. **How the comment is read.** - *`-V -Q` (this PR)* prints only the verified comment. Measured on minisign 0.12: an edited comment or an edited manifest exits 1 with no output. - *`-V` and parsing the `Trusted comment:` line* works too, but couples the script to minisign's human-readable output. **Releases signed before this change.** - *Refuse them (this PR)* is right because every caller fetches its own release. `bootstrap-rt.sh:34` passes the baked ref, and `goreleaser.yml:1274` passes the tag it just published. Older tags run their own older `fetch-rt.sh`, so a fetcher containing this check meets an old signature only through a replay. - *A cutover allowance* (accept a default comment for tags before vN) would help only a caller that fetches another tag's release with this script. There is none, and the allowance would reopen the replay for every old release. **Real minisign in CI.** - *Require it in the bats job (this PR)* is right because the go runner carries it already (every signed release proves the goreleaser job found it), and the mock cannot prove what real minisign prints. - *Leave the test skippable* is right only if some go runner may lack minisign. Then that runner cannot cut a release either. ## Self-bootstrap (AGENTS.md section 2) No re-pin: toolkit-self floats `@main` (AGENTS.md §2.1) and builds `rt` from source, so it never runs `fetch-rt.sh`. Neither `fetch-rt.sh` nor `.goreleaser.yaml` is in `gates.DefaultComposeScripts`. Adopters get the check from the first cut tag that contains this commit. **That cut is also the first live end-to-end run.** Its goreleaser job signs with the new comment, and its `verify-fetch-arm` job fetches with the new script. The bats job's real-minisign test runs this PR's head on a `go` runner first, so the runner's minisign `-Q` behaviour is graded before merge, not at the cut. ## Verification Local runs used an LF clone at `8630004`, re-run at `66ee9ce` (base `0156d84` = live `main`), in `rt-bats:local` with Debian's `minisign 0.12` added: | Check | Result | |---|---| | `bats tests/` (the CI invocation) | 221/221 ok, 0 skipped (the real-minisign test ran) | | `go test ./...` | ok | | `shellcheck scripts/fetch-rt.sh`, `bash -n` | clean | | `yaml.safe_load` on `.goreleaser.yaml` and `tests.yml` | ok | | `anchor-check.py` (renderer `git.frankenbit.de`) | 0 unresolved | | `FRAGMENT_CHECK_COVERAGE=content rt fragment-check changelog.d` | PASS, no warnings on the new fragment | | `rt register-check --stdin` on each changed file | 0 hits | | close-keyword regex on title, body and commit | 0 hits | **goreleaser actually expands the template.** I ran goreleaser **v2.17.0**, the `GORELEASER_PIN` in `goreleaser.yml:217`, as `release --snapshot --skip=publish,validate,announce` with a throwaway key. Its `dist/checksums.txt.minisig` reads `trusted comment: release-toolkit tag:v0.62.4 file:checksums.txt`, where `.Tag` is the nearest tag in snapshot mode, and `minisign -V -Q` prints exactly that. **New tests** (`tests/fetch-rt.bats` 18–23): | Test | Proves | |---|---| | another release's comment | refused, *"validly signed, but not for this release"*, both comments named, no asset request | | minisign's default comment (pre-#1516) | refused | | `v9.9.90`, trailing text, missing file field, empty | each refused: exact match | | cache hit against another release's signed files | refused, no `checksum OK`, no asset request | | `v1.0.0-alpha.0` | accepted whole | | **real minisign**, signing with `.goreleaser.yaml`'s own `signs.args` (goreleaser's variables filled in) | genuine v0.2.0 installs `rt 0.2.0`; v0.1.0's signed files served as v0.2.0 are refused; a default-comment signature is refused; an edited comment fails `minisign verification failed` | **Mutation check** (AGENTS.md §8 Axis B). Each mutant ran on a throwaway copy, and its diff was confirmed non-empty: | Mutant | Turns red | |---|---| | M1 the comment check removed | 6 #1516 tests, and the #513 wiring test | | M2 `-Q` changed to `-q` (no comment captured) | 14 tests, every success path included, and the #513 test | | M3 substring match instead of exact | the exact-match test | | M4 the check runs on the miss path only | the cache-hit test | | M5 `-t` dropped from `.goreleaser.yaml` | `setup_file` (whole file), and the #513 test | | M6 the `.goreleaser.yaml` format drifts and `fetch-rt.sh` does not | 13 tests, via the lockstep | | M7 the check moved after the asset fetch | 4 tests, including "refused before the asset is fetched" | | M8 the `GORELEASER_CURRENT_TAG` env line dropped (`66ee9ce`) | the #513 test, naming the missing pin | ## #1516 acceptance criteria | AC | Here | |---|---| | 1 decision recorded | comment 114216, option (B) | | 2 `fetch-rt.sh` refuses another release on hit and miss, naming both versions | yes. Tests 18, 19, 21 and 23 | | 3 replay test, red before and green after, mutation-verified, controls kept | tests 18 and 23, M1–M7. The existing tampered-manifest, missing-signature and wrong-key tests are unchanged and green | | 4 prerelease tags | test 22 | | 5 SECURITY.md states what the signature binds, with a manual step | §Verifying a release | | 6 (B): signing args changed, and the first signed release read back | args changed, and the local goreleaser v2.17.0 snapshot read back. **The read-back of the first real release stays open until the next cut.** | | 7 the public mirror's `.minisig` handling | measured, and needs no change. `reusable-mirror-release.yml:310-327` copies every asset by name and never re-signs. The live mirror's v0.62.0 `.minisig` has the origin's trusted comment and timestamp (`1788725401`) byte for byte, so a tag-naming comment carries over to the same tag. | | 8 fragment | `changelog.d/1516-signed-tag-binding.fixed.md` | ## Review request @alex, please **verify at source** (AGENTS.md §6). Both halves need checking: 1. **The signing half**: is `.goreleaser.yaml`'s `-t` argument what you want stamped into every release from now on? It is part of what adopters will check by hand. And does the CI bats job log on this head show `minisign -v` and test 23 *ran*, not skipped? 2. **The verifying half**: run `bats tests/fetch-rt.bats` on the head with minisign installed. Then read `fetch-rt.sh`'s refusal as an adopter who hit it would. After the next cut, read back that release's `checksums.txt.minisig` trusted comment for AC 6. Pin your review with `rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1518 --reviewed-head <full sha>` (§6.2). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude added 1 commit 2026-10-02 20:10:54 +02:00

fix(release-assets): sign the tag into the checksums signature, and refuse a manifest signed for another release (#1516) ... 863000402f

checksums.txt lists rt-linux-amd64 with no version, and minisign's
default trusted comment is a timestamp and a file name. A valid
signature therefore proved only that the key signed some release. An
older release's signed checksums.txt, .minisig and binary, served under
a newer tag, passed every check in fetch-rt.sh, and the older rt ran.

.goreleaser.yaml now signs with -t "release-toolkit tag:{{ .Tag }}
file:checksums.txt". goreleaser v2.17.0, the version the cut pins,
expands the template in signs.args: a local snapshot signed
"release-toolkit tag:v0.62.4 file:checksums.txt". fetch-rt.sh verifies
with -V -Q, which prints the comment only once both signatures verify.
It then refuses unless the comment is exactly that string with
RT_VERSION. The check sits before the cache branch, so it runs on hits
and misses, and before any asset is fetched or checksummed. Every
caller fetches its own release, so a release signed before this change
is refused only when it is replayed.

tests/fetch-rt.bats renders the mock's comment from .goreleaser.yaml's
own -t template, so every passing fetch also checks that the two agree.
A new test signs with the real minisign, using the signs.args from
.goreleaser.yaml, and covers a genuine fetch, a replay, a default
comment and an edited comment. The bats job now requires minisign on
the go runner, which goreleaser.yml already needs, so that test cannot
skip in CI. docs/SECURITY.md's manual recipe gains the trusted-comment
check.

Refs #1516

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude requested review from alex 2026-10-02 20:10:55 +02:00

claude reviewed 2026-10-02 20:16:48 +02:00

claude left a comment

• Copy link
• Copy Markdown

Verdict: APPROVE at 863000402fa170a50eade11257cc228fb585fca7. This review is a COMMENT because the forge identity here is claude, the PR's author, and Forgejo refuses self-approval (AGENTS.md section 8, instance 7). @alex has to click Approve.

rt review-submit-check --pr 1518 --reviewed-head 863000402fa170a50eade11257cc228fb585fca7 gave READY, rc=0, immediately before this review was posted.

Verified at source

The signing half

• .goreleaser.yaml signs.args adds -t "release-toolkit tag:{{ .Tag }} file:checksums.txt" after -s <keyfile> and before -x/-m. That fits the option (B) decision in issue comment 114216.
• On tag push, .Tag is the pushed tag. On dispatch, GORELEASER_CURRENT_TAG is pinned to inputs.tag (goreleaser.yml:426-428).
• The goreleaser expansion itself I did not re-run. I rely on the author's v2.17.0 snapshot read-back.

The verifying half

• fetch-rt.sh captures the output of -V -Q and compares it whole-string against release-toolkit tag:${RT_VERSION} file:checksums.txt.
• The check runs after verify_manifest_signature and before the cached branch, so it covers both hit and miss, and runs before any asset request.
• The refusal names the comment it found, the comment it expected, and what probably happened. An adopter can act on it.

Real minisign 0.12 (Debian), probed directly

• -V -Q prints exactly the trusted comment on stdout, with nothing on stderr, rc=0.
• -V alone prints Signature and comment signature verified and Trusted comment: …. That matches the mock and the docs/SECURITY.md text.

Callers. Every caller fetches its own tag:

• bootstrap-rt.sh uses its v[0-9]* arm with RT_VERSION=$ref.
• goreleaser.yml's verify-fetch-arm checks out the tag, then runs that tag's scripts/fetch-rt.sh.
• rc tags go through the same goreleaser trigger (v*), so their .Tag is the rc tag and they match too.
• I agree the "refuse pre-#1516 signatures" branch breaks no caller.

Tests

Local run. An LF clone at the head, in rt-bats:local plus minisign 0.12:

• bats tests/fetch-rt.bats is 23/23 ok, and test 23 (real minisign) ran.
• bats -f 513 tests/workflows.bats is ok.

CI on this head (run 19128, job bats, read in the job log):

• The tool check printed minisign=/usr/bin/minisign, then minisign 0.12.
• The suite reported 1..221.
• ok 48 #1516: real minisign: … has no # skip. The runner-side -Q behaviour is graded before merge, as the PR body claims.
• Combined status is success (30 contexts).

Reviewer-side mutants (AGENTS.md section 8 Axis B), separate from the author's M1–M7. Each ran on a throwaway copy, with a non-empty diff confirmed:

Mutant	Killed by
R1 prefix match ([[ "$c" != "$e"* ]])	test 20
R2 skip the check on a cache hit	test 21
R3 -Q → -q (empty comment)	14 tests, including 18, 21, 22, 23

Other checks

• ASCII-only (section 11): no non-ASCII in the added lines under scripts/, tests/, .forgejo/ or .goreleaser.yaml.
• Re-pin (section 2.1): none needed. check-self-bootstrap is green.

Non-blocking

1. Two v* tags on one commit. On tag push GORELEASER_CURRENT_TAG is unset, so goreleaser picks the tag from git.
   • If two v* tags ever point at the same commit, .Tag might name the other one. Every fetch of that release would then be refused, and verify-fetch-arm would fail at the cut.
   • That failure is loud, not silent. Today no two tags share a commit (measured on a fresh clone).
   • Which tag goreleaser picks in that case was not measured.
   • Optional hardening, here or as a follow-up: export GORELEASER_CURRENT_TAG="${{ github.ref_name }}" on the push path too, so the signed tag is the pushed tag by construction.
2. Typo. tests/workflows.bats, the new Python comment: "any release s signed checksums.txt". The apostrophe was lost.
3. Fragment wording. "refuse a signed checksums.txt naming another release" understates the change. It also refuses one that names no release (the pre-#1516 default comment). Optional: "…whose signature does not name this release".

AC 6 (read back the first real release's trusted comment) stays open until the next cut, as the PR body and comment 114216 say. Please read the comment back from the release's checksums.txt.minisig itself, not from the goreleaser log.

**Verdict: APPROVE at `863000402fa170a50eade11257cc228fb585fca7`.** This review is a COMMENT because the forge identity here is `claude`, the PR's author, and Forgejo refuses self-approval (AGENTS.md section 8, instance 7). @alex has to click Approve. `rt review-submit-check --pr 1518 --reviewed-head 863000402fa170a50eade11257cc228fb585fca7` gave `READY`, rc=0, immediately before this review was posted. ## Verified at source **The signing half** - `.goreleaser.yaml` `signs.args` adds `-t "release-toolkit tag:{{ .Tag }} file:checksums.txt"` after `-s <keyfile>` and before `-x`/`-m`. That fits the option (B) decision in issue comment 114216. - On tag push, `.Tag` is the pushed tag. On dispatch, `GORELEASER_CURRENT_TAG` is pinned to `inputs.tag` (`goreleaser.yml:426-428`). - The goreleaser expansion itself I did not re-run. I rely on the author's v2.17.0 snapshot read-back. **The verifying half** - `fetch-rt.sh` captures the output of `-V -Q` and compares it whole-string against `release-toolkit tag:${RT_VERSION} file:checksums.txt`. - The check runs after `verify_manifest_signature` and before the `cached` branch, so it covers both hit and miss, and runs before any asset request. - The refusal names the comment it found, the comment it expected, and what probably happened. An adopter can act on it. **Real minisign 0.12 (Debian), probed directly** - `-V -Q` prints exactly the trusted comment on stdout, with nothing on stderr, rc=0. - `-V` alone prints `Signature and comment signature verified` and `Trusted comment: …`. That matches the mock and the `docs/SECURITY.md` text. **Callers.** Every caller fetches its own tag: - `bootstrap-rt.sh` uses its `v[0-9]*` arm with `RT_VERSION=$ref`. - `goreleaser.yml`'s verify-fetch-arm checks out the tag, then runs that tag's `scripts/fetch-rt.sh`. - rc tags go through the same goreleaser trigger (`v*`), so their `.Tag` is the rc tag and they match too. - I agree the "refuse pre-#1516 signatures" branch breaks no caller. ## Tests **Local run.** An LF clone at the head, in `rt-bats:local` plus `minisign 0.12`: - `bats tests/fetch-rt.bats` is 23/23 ok, and test 23 (real minisign) ran. - `bats -f 513 tests/workflows.bats` is ok. **CI on this head** (run 19128, job `bats`, read in the job log): - The tool check printed `minisign=/usr/bin/minisign`, then `minisign 0.12`. - The suite reported `1..221`. - `ok 48 #1516: real minisign: …` has no `# skip`. **The runner-side `-Q` behaviour is graded before merge, as the PR body claims.** - Combined status is `success` (30 contexts). **Reviewer-side mutants** (AGENTS.md section 8 Axis B), separate from the author's M1–M7. Each ran on a throwaway copy, with a non-empty diff confirmed: | Mutant | Killed by | |---|---| | R1 prefix match (`[[ "$c" != "$e"* ]]`) | test 20 | | R2 skip the check on a cache hit | test 21 | | R3 `-Q` → `-q` (empty comment) | 14 tests, including 18, 21, 22, 23 | **Other checks** - ASCII-only (section 11): no non-ASCII in the added lines under `scripts/`, `tests/`, `.forgejo/` or `.goreleaser.yaml`. - Re-pin (section 2.1): none needed. `check-self-bootstrap` is green. ## Non-blocking 1. **Two `v*` tags on one commit.** On tag push `GORELEASER_CURRENT_TAG` is unset, so goreleaser picks the tag from git. - If two `v*` tags ever point at the same commit, `.Tag` might name the other one. Every fetch of that release would then be refused, and verify-fetch-arm would fail at the cut. - That failure is loud, not silent. Today no two tags share a commit (measured on a fresh clone). - Which tag goreleaser picks in that case was not measured. - Optional hardening, here or as a follow-up: export `GORELEASER_CURRENT_TAG="${{ github.ref_name }}"` on the push path too, so the signed tag is the pushed tag by construction. 2. **Typo.** `tests/workflows.bats`, the new Python comment: "any release s signed checksums.txt". The apostrophe was lost. 3. **Fragment wording.** "refuse a signed `checksums.txt` naming another release" understates the change. It also refuses one that names *no* release (the pre-#1516 default comment). Optional: "…whose signature does not name this release". AC 6 (read back the first real release's trusted comment) stays open until the next cut, as the PR body and comment 114216 say. Please read the comment back from the release's `checksums.txt.minisig` itself, not from the goreleaser log.

claude commented 2026-10-02 20:18:00 +02:00

Author

Member

• Copy link
• Copy Markdown

HOLD. A new commit is coming on top of 863000402fa170a50eade11257cc228fb585fca7. It takes all three non-blocking items from review 7192:

1. goreleaser.yml: GORELEASER_CURRENT_TAG comes from the job's resolve release tag output on tag push too, not only on dispatch. The trusted comment then always names the tag verify-fetch-arm fetches.
2. tests/workflows.bats: the comment typo.
3. The fragment summary also covers a signature that names no release.

Please don't approve 8630004. I'll post the new full head SHA here once it's pushed.

**HOLD.** A new commit is coming on top of `863000402fa170a50eade11257cc228fb585fca7`. It takes all three non-blocking items from review 7192: 1. `goreleaser.yml`: `GORELEASER_CURRENT_TAG` comes from the job's `resolve release tag` output on tag push too, not only on dispatch. The trusted comment then always names the tag `verify-fetch-arm` fetches. 2. `tests/workflows.bats`: the comment typo. 3. The fragment summary also covers a signature that names no release. Please don't approve `8630004`. I'll post the new full head SHA here once it's pushed.

claude referenced this pull request from a commit 2026-10-02 20:20:52 +02:00

fix(release-assets): pin GORELEASER_CURRENT_TAG on tag push too, and tighten the #1516 wording

claude added 1 commit 2026-10-02 20:20:52 +02:00

fix(release-assets): pin GORELEASER_CURRENT_TAG on tag push too, and tighten the #1516 wording ... 66ee9ce1c4

Review 7192 on #1518 found three things.

GORELEASER_CURRENT_TAG was set only on workflow_dispatch. On a tag push
goreleaser reads the tag from git, and .Tag is now signed into the
trusted comment. If two v* tags shared a commit, it could sign the
other tag's name, and verify-fetch-arm would refuse the release only
after it was published. The goreleaser release step now takes the tag
on every event from the job's resolve release tag step, which resolves
it the same way as verify-fetch-arm. The step refuses an empty value,
and the #513 wiring test pins the env line.

The fragment summary now also covers a signature that names no release,
and a comment typo in tests/workflows.bats is gone.

Refs #1516

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude commented 2026-10-02 20:21:20 +02:00

Author

Member

• Copy link
• Copy Markdown

HOLD released. The new head is 66ee9ce1c4a2df8fac1b127e548f1ca66f20751d, one new commit on top of the reviewed 8630004, with no amend and no force-push. It takes all three items from review 7192:

1. goreleaser.yml: the goreleaser release step sets GORELEASER_CURRENT_TAG: ${{ steps.tag.outputs.tag }} on every event. That is the job's resolve release tag output, resolved the same way as verify-fetch-arm: the dispatch input, else github.ref_name. The dispatch-only export is gone, and the step refuses an empty value. The #513 wiring test pins the env line, and dropping it turns that test red with "publish step does not pin GORELEASER_CURRENT_TAG to the resolved release tag".
2. tests/workflows.bats: the comment typo is fixed.
3. Fragment summary: "the reusables refuse one naming another tag or none".

Re-run locally at 66ee9ce: bats tests/ 221/221 (0 skipped, real minisign present), YAML parse ok, fragment-check with no warnings, register-check 0 hits. The PR body was updated under this HOLD and read back byte for byte.

Please take a fresh read at 66ee9ce. The 8630004 verdict does not carry over. Pin it with rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1518 --reviewed-head 66ee9ce1c4a2df8fac1b127e548f1ca66f20751d.

**HOLD released.** The new head is `66ee9ce1c4a2df8fac1b127e548f1ca66f20751d`, one new commit on top of the reviewed `8630004`, with no amend and no force-push. It takes all three items from review 7192: 1. **`goreleaser.yml`**: the goreleaser release step sets `GORELEASER_CURRENT_TAG: ${{ steps.tag.outputs.tag }}` on every event. That is the job's `resolve release tag` output, resolved the same way as `verify-fetch-arm`: the dispatch input, else `github.ref_name`. The dispatch-only `export` is gone, and the step refuses an empty value. The #513 wiring test pins the env line, and dropping it turns that test red with "publish step does not pin GORELEASER_CURRENT_TAG to the resolved release tag". 2. **`tests/workflows.bats`**: the comment typo is fixed. 3. **Fragment summary**: "the reusables refuse one naming another tag or none". Re-run locally at `66ee9ce`: `bats tests/` 221/221 (0 skipped, real minisign present), YAML parse ok, fragment-check with no warnings, register-check 0 hits. The PR body was updated under this HOLD and read back byte for byte. Please take a fresh read at `66ee9ce`. The `8630004` verdict does not carry over. Pin it with `rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1518 --reviewed-head 66ee9ce1c4a2df8fac1b127e548f1ca66f20751d`.

claude reviewed 2026-10-02 20:25:52 +02:00

claude left a comment

• Copy link
• Copy Markdown

Verdict: APPROVE at 66ee9ce1c4a2df8fac1b127e548f1ca66f20751d. This review replaces review 7192, which was at 8630004 and does not carry over. It is a COMMENT for the same reason as before: this forge identity authored the PR, and self-approval is refused. @alex has to click Approve.

rt review-submit-check --pr 1518 --reviewed-head 66ee9ce1c4a2df8fac1b127e548f1ca66f20751d gave READY, rc=0, immediately before this review was posted.

Process (AGENTS.md section 6.2)

• HOLD is comment 114230 and the release is comment 114235.
• 66ee9ce is one commit whose parent is the reviewed 8630004. There was no amend and no force-push.
• The incremental diff 8630004..66ee9ce touches only goreleaser.yml, tests/workflows.bats and the fragment.

The three items from review 7192

1. GORELEASER_CURRENT_TAG on every event. Taken, and correct.
   • The step's env now reads ${{ steps.tag.outputs.tag }}. That step (resolve release tag, id: tag, goreleaser.yml:138-154) is in the same goreleaser job, and it already feeds RELEASE_ASSET_TAG.
   • It resolves the dispatch input, else github.ref_name, and refuses an empty value. That is the same resolution verify-fetch-arm uses (:1222-1232). So the tag signed into the comment and the tag the verifier fetches come from one rule.
   • The step's own empty-value guard is redundant with the resolve step's guard, which is harmless.
   • The dispatch-only export is gone, so the old and new paths cannot disagree.
   • Mutants on the new #513 assertion: deleting the env line, and pointing it at ${{ inputs.tag }}. Both turn workflows: #513 … red with "publish step does not pin GORELEASER_CURRENT_TAG to the resolved release tag".
2. Typo. Fixed.
3. Fragment. It now covers "another tag or none".

Re-verified at this head

• Local run. An LF clone in rt-bats:local plus minisign 0.12:
  • bats tests/fetch-rt.bats is 23/23 ok, and the real-minisign test ran.
  • bats -f 513 tests/workflows.bats is ok.
• CI. Combined status is success (30 contexts). The bats job log (run 19146) shows minisign=/usr/bin/minisign and minisign 0.12, then 1..221. ok 48 #1516: real minisign: … has no # skip, and ok 128 workflows: #513 … passes.
• Unchanged since review 7192: fetch-rt.sh, .goreleaser.yaml, tests/fetch-rt.bats and docs/SECURITY.md. The verification in 7192 still applies to them, including mutants R1–R3.

No open findings. AC 6 (read back the first real release's trusted comment from its checksums.txt.minisig) stays open until the next cut, as agreed.

**Verdict: APPROVE at `66ee9ce1c4a2df8fac1b127e548f1ca66f20751d`.** This review replaces review 7192, which was at `8630004` and does not carry over. It is a COMMENT for the same reason as before: this forge identity authored the PR, and self-approval is refused. @alex has to click Approve. `rt review-submit-check --pr 1518 --reviewed-head 66ee9ce1c4a2df8fac1b127e548f1ca66f20751d` gave `READY`, rc=0, immediately before this review was posted. ## Process (AGENTS.md section 6.2) - HOLD is comment 114230 and the release is comment 114235. - `66ee9ce` is one commit whose parent is the reviewed `8630004`. There was no amend and no force-push. - The incremental diff `8630004..66ee9ce` touches only `goreleaser.yml`, `tests/workflows.bats` and the fragment. ## The three items from review 7192 1. **`GORELEASER_CURRENT_TAG` on every event.** Taken, and correct. - The step's env now reads `${{ steps.tag.outputs.tag }}`. That step (`resolve release tag`, `id: tag`, `goreleaser.yml:138-154`) is in the same `goreleaser` job, and it already feeds `RELEASE_ASSET_TAG`. - It resolves the dispatch input, else `github.ref_name`, and refuses an empty value. That is the same resolution `verify-fetch-arm` uses (`:1222-1232`). So the tag signed into the comment and the tag the verifier fetches come from one rule. - The step's own empty-value guard is redundant with the resolve step's guard, which is harmless. - The dispatch-only `export` is gone, so the old and new paths cannot disagree. - **Mutants** on the new #513 assertion: deleting the env line, and pointing it at `${{ inputs.tag }}`. Both turn `workflows: #513 …` red with "publish step does not pin GORELEASER_CURRENT_TAG to the resolved release tag". 2. **Typo.** Fixed. 3. **Fragment.** It now covers "another tag or none". ## Re-verified at this head - **Local run.** An LF clone in `rt-bats:local` plus `minisign 0.12`: - `bats tests/fetch-rt.bats` is 23/23 ok, and the real-minisign test ran. - `bats -f 513 tests/workflows.bats` is ok. - **CI.** Combined status is `success` (30 contexts). The bats job log (run 19146) shows `minisign=/usr/bin/minisign` and `minisign 0.12`, then `1..221`. `ok 48 #1516: real minisign: …` has no `# skip`, and `ok 128 workflows: #513 …` passes. - **Unchanged since review 7192:** `fetch-rt.sh`, `.goreleaser.yaml`, `tests/fetch-rt.bats` and `docs/SECURITY.md`. The verification in 7192 still applies to them, including mutants R1–R3. No open findings. AC 6 (read back the first real release's trusted comment from its `checksums.txt.minisig`) stays open until the next cut, as agreed.

alex approved these changes 2026-10-02 20:26:49 +02:00

alex merged commit 66ee9ce1c4 into main 2026-10-02 20:26:53 +02:00

alex deleted branch claude/1516-signed-tag-binding 2026-10-02 20:26:53 +02:00

forgejo-actions commented 2026-10-02 20:27:40 +02:00

• Copy link
• Copy Markdown

Landing identity record

• PR: #1518
• landed commit (server merge_commit_sha): 66ee9ce1c4a2df8fac1b127e548f1ca66f20751d
• effective official approval(s):
  • @alex, review #7195, stamped commit: 66ee9ce1c4a2df8fac1b127e548f1ca66f20751d
• replay comparison: no identity change (stamped SHA equals landed SHA)

This is a post-merge identity record. It does not retroactively review the landed object; it records whether the server landed the object that an official approval named.

<!-- release-toolkit:landing-review-record-v1 landed=66ee9ce1c4a2df8fac1b127e548f1ca66f20751d --> ## Landing identity record - PR: #1518 - landed commit (server merge_commit_sha): `66ee9ce1c4a2df8fac1b127e548f1ca66f20751d` - effective official approval(s): - @alex, review #7195, stamped commit: `66ee9ce1c4a2df8fac1b127e548f1ca66f20751d` - replay comparison: no identity change (stamped SHA equals landed SHA) This is a post-merge identity record. It does not retroactively review the landed object; it records whether the server landed the object that an official approval named.

claude referenced this pull request 2026-10-02 20:27:42 +02:00

bug(release-assets): the minisign signature does not bind a release to its tag, so an older signed rt passes verification under a newer tag #1516

claude referenced this pull request 2026-10-02 20:27:55 +02:00

bug(release-assets): the minisign signature does not bind a release to its tag, so an older signed rt passes verification under a newer tag #1516

claude referenced this pull request 2026-10-02 20:43:10 +02:00

bug(release-assets): the minisign signature does not bind a release to its tag, so an older signed rt passes verification under a newer tag #1516

Sign in to join this conversation.

Reviewers

No reviewers

alex

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind

bug

Something is broken

  

kind

chore

Maintenance, tooling, refactor

  

kind

docs

Documentation work

  

kind

epic

Tracker: its acceptance criteria are sub-issues; closed by hand when all are ticked

  

kind

feature

New feature or capability

  

loop/paused

rt-bugfix-loop kill switch: while this is on the loop log issue, no round claims or merges

  

mirror-drift

Automated: a release here has no asset-bearing counterpart on the mirror. One tracker per drift episode; the check never reopens a closed one (#1586)

  

priority

critical

Critical: must be addressed immediately

  

priority

high

Important, should be addressed soon

  

priority

low

Low priority, address when convenient

  

priority

medium

Normal priority

  

size

L

Large: significant feature, ~6-16h

  

size

M

Medium: moderate feature, ~2-6h

  

size

S

Small: simple change, ~1-2h

  

size

XL

Extra large: major feature or refactor, 16h+

  

status

blocked

Cannot start until a named dependency resolves

No labels

bump

major

bump

minor

bump

patch

kind

bug

kind

chore

kind

docs

kind

epic

kind

feature

loop/paused

mirror-drift

priority

critical

priority

high

priority

low

priority

medium

size

L

size

M

size

S

size

XL

status

blocked

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

frankenbit/release-toolkit!1518

No description provided.