release: make a failed tag release recoverable, and fix the generated Homebrew formula #233
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/release-job-artifacts"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
brew install fjserves 0.3.0.v0.4.1is a published, non-draft release with zero assets, because release run 391 onrefs/tags/v0.4.1never got apublishjob. Per-job status from the Actions API:The first failure was the transient S3 upload timeout described in #230. The permanent damage came from the re-run. Artifact object keys are
<prefix>/<repo>/artifacts/<runId>/<name>, so a re-run of run 391 reuses run 391's keys, andupload-artifact-actionthrowsAn artifact named "..." already exists for run Nfor exactly the legs that succeeded the first time. Every attempt fails a different subset,publishneeds the whole matrix, so the run can never go green. #230 setoverwrite: truefor this, but it landed three days after the tag, and a tag push reads its workflow from the tag. There is currently no way to apply it tov0.4.1short of moving the tag.Even if
publishhad run, the generated formula could not have installed. It does:Homebrew has already chdir'd into the tarball's single top-level directory before
installruns, and that directory isfj-v0.4.1-darwin-aarch64, carrying the tag'svthat#{version}drops. Built the real 0.4.1 tarball locally, rendered the formula with this workflow's own heredoc, and installed it from a scratch tap:This is why
rasterstate/homebrew-tapcarries a hand-correctedinstallblock rather than the generated one, and why bumping the tap has meant editing it by hand. The generator has been wrong since it was written; no release has ever shipped a usablefj.rb.Fix
Re-releasing a tag no longer requires moving it.
workflow_dispatchwith a requiredtaginput, resolved once in apreparejob. Bothbuildandpublishcheck out that tag and use it for stage naming and the release target, so dispatching frommainbuilds the tag's code withmain's workflow. On a tag push nothing changes:inputs.tagis empty andgithub.ref_nameis used.The dispatched ref is pinned, not pattern-matched. Two review findings, both fixed here, both the same mistake in different clothes: a guard in the right place operating on the wrong thing.
env:, never interpolated into a script body. A${{ }}expression is substituted textually before the shell parses the line, soTAG="${{ inputs.tag }}"with an input ofv1$(id)executes at the assignment, ahead of any validation below it. Quoting does not help; the expansion happens first.preparethen requires the name to exist in the repository's tags collection, which a branch cannot satisfy, and exports the commit sha it resolves to. Both jobs check outrefs/tags/<tag>and assertgit rev-parse HEADmatches that sha. Shape validation alone was not enough: a branch namedv0.4.1passes a version-shaped glob, version-shaped branches already exist here (release/v0.3.0), and git resolves a bare ambiguous name inconsistently (rev-parsetakes the tag,checkouttakes the branch and only warns). A recovery path that can publish branch contents as a tag's artifacts defeats its own purpose, so the guarantee is now structural. The charset allowlist stays, since the tag also reaches tarball names, the formula, and the release API, and excluding/keeps it from altering the API path it is interpolated into.The formula generator emits what the tap actually needs. Dropped the
cd, with a comment explaining both reasons it was wrong. The rendered output now matches the tap's working formula, so a tap bump is a copy rather than an edit.Three quiet failures made loud:
sha256 ""and publish the formula anyway. Now checks all three digests and refuses.v0.4.1included. Now extracted from the tag's own## [x.y.z]CHANGELOG section; a missing section fails the run rather than shipping blank.fail_on_unmatched_fileswas at its permissive default, so an asset glob matching nothing passed silently. Nowtrue.A
guardjob. Forgejo surfaces a pushed tag as a published release immediately, so a failed build leaves something that reads as installable, which is howv0.4.1sat unnoticed from 2026-07-22 to #232. The guard runs whenbuildorpublishdid not succeed, marks an asset-less release draft with a "do not install this version" body, and files an issue naming the tag, the job results, and the dispatch command to re-release. It checks for an existing open issue with the same title so repeated attempts do not spam.Test
Reproduced the release build locally at
v0.4.1in a worktree:cargo build --release --lockedforaarch64-apple-darwinandx86_64-apple-darwin: both clean. The compile was never the problem, which is consistent with #230's read that the failure was in the upload.packagestep verbatim, producing real tarballs andSHA256SUMS.render homebrew formulastep, extracted from this file withyqand run against thatSHA256SUMS, so what was tested is what ships. Confirmed the digest guard fires with a leg missing, and that the rendered formula is byte-equivalent to the tap's working one apart from the version and URLs.brew installof the rendered formula from a scratch tap:fj --versionreportsfj 0.4.1,--helpmatches the formula's assertion, andREADME.md,LICENSE,CHANGELOG.mdland inpkgshare. The same install against the pre-fix formula fails with thechdir_patherror above.awkagainst the realCHANGELOG.md: extracts the 12-line[0.4.1]section and stops at## [0.4.0]; a version with no section yields empty output and trips the guard. A missingCHANGELOG.mdnow fails with an error that names the cause rather than a bareawkstderr line.Review follow-ups were tested the same way:
alpine:3.20against a stub tags API, over ten inputs. Acceptsv0.4.1andv0.3.0. Rejectsv1$(id),main,v1.0, a shape-valid name with no such tag (v0.4.0, which is the real case in this repo), a name that exists only as a branch, and malformed sha responses. The injection cases were checked against a filesystem marker rather than output matching, and the pre-fix expanded script was generated alongside to confirm it does execute where this one does not.v1.2.3and a branchv1.2.3at different commits: the bare name checks out the branch and the sha assertion refuses the build,refs/tags/v1.2.3checks out the tag and passes.set -o pipefailand theprintf-built issue body were checked under busybox ash 1.36.1, which is what thealpine:3.20jobs actually run.read -d ''does not exist there, which is why theguardjob no longer uses a heredoc.The
guardjob, the signing and notarization steps, and the artifact round-trip need CI and cannot be exercised locally.Not fixed here
The initial darwin-aarch64 upload timeout itself. #230 identified it as transient and I have no reason to doubt that, but I could not read run 391's logs to confirm independently: the Actions log route is cookie-gated and rejects a PAT (#103), and the API exposes no log endpoint. This PR does not try to make that upload more reliable; it makes one flaky leg cost a re-dispatch instead of a wedged release.
To get v0.4.1 artifacts out
After this merges, dispatch
releasefrommainwithtag: v0.4.1. The tag does not need to move andv0.4.1keeps its date. Once assets exist I will bump the tap from the generatedfj.rband verify a cleanbrew tap && brew install fjreportsfj 0.4.1, which closes items 1, 2, and 4 of #232.Refs #232
Forseti review
1 blocking finding(s) from the lead reviewer (advisory: not gating merges yet).
Findings
P1
.forgejo/workflows/release.yml:386— Line 377 definesauth="Authorization: ***REDACTED***as a multiline variable using a bare". Line 384 closes with a single"but the assignment spans multiple lines without proper quoting or continuation. More critically, lines 388–391 and 397–400 usecurl -H "$auth", but because$authis not quoted, word-splitting and glob expansion will break it. The variable should be"$auth"(quoted) or better, split into individual header arguments. This will cause curl to receive malformed headers.P1
.forgejo/workflows/release.yml:36— The workflow expression is expanded into the shell script before the shell runs, so a manually supplied tag such asv1$(...)would execute command substitution during this assignment, before thecasevalidation below can reject anything. This job belongs to the release workflow and the workflow grants write permissions, so the tag/ref value should be treated as untrusted input at the shell boundary.P2
.forgejo/workflows/release.yml:261— The awk script (lines 263–268) extracts the version section from CHANGELOG.md. If CHANGELOG.md does not exist or is not found in the checkout,awkexits 0 and outputs nothing. The check on lines 272–276 catches the case where the section is missing after the awk runs, which is good. However, if theawkcommand itself fails (e.g., file not found, permission denied), the script will not catch it because there is no explicit error check on the awk output. Theset -euo pipefailon line 262 should catch a nonzero exit from awk, but only if it actually exits nonzero;awkwith a missing file may fail differently. Consider addingtest -f CHANGELOG.mdbefore awk or using|| exit 1to be explicit.PR:
rasterstate/fj#233Head SHA:
754987f4ee3dReview job:
sha256:75b7e5574afa9b355d8abc38baf91daf08dc60215b57b73940e017a7cba911bcProvider pair:
openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001Blocking findings:
1Inline findings:
3Token source:
GITHUB_TOKENRunner:
b4dbfdf95573Run: https://rasterhub.com/rasterstate/fj/actions/runs/400
Forseti review
1 blocking finding(s) from the lead reviewer (advisory: not gating merges yet).
Findings
P1
.forgejo/workflows/release.yml:386— Line 377 definesauth="Authorization: ***REDACTED***as a multiline variable using a bare". Line 384 closes with a single"but the assignment spans multiple lines without proper quoting or continuation. More critically, lines 388–391 and 397–400 usecurl -H "$auth", but because$authis not quoted, word-splitting and glob expansion will break it. The variable should be"$auth"(quoted) or better, split into individual header arguments. This will cause curl to receive malformed headers.P1
.forgejo/workflows/release.yml:36— The workflow expression is expanded into the shell script before the shell runs, so a manually supplied tag such asv1$(...)would execute command substitution during this assignment, before thecasevalidation below can reject anything. This job belongs to the release workflow and the workflow grants write permissions, so the tag/ref value should be treated as untrusted input at the shell boundary.P2
.forgejo/workflows/release.yml:261— The awk script (lines 263–268) extracts the version section from CHANGELOG.md. If CHANGELOG.md does not exist or is not found in the checkout,awkexits 0 and outputs nothing. The check on lines 272–276 catches the case where the section is missing after the awk runs, which is good. However, if theawkcommand itself fails (e.g., file not found, permission denied), the script will not catch it because there is no explicit error check on the awk output. Theset -euo pipefailon line 262 should catch a nonzero exit from awk, but only if it actually exits nonzero;awkwith a missing file may fail differently. Consider addingtest -f CHANGELOG.mdbefore awk or using|| exit 1to be explicit.PR:
rasterstate/fj#233Head SHA:
754987f4ee3dReview job:
sha256:75b7e5574afa9b355d8abc38baf91daf08dc60215b57b73940e017a7cba911bcProvider pair:
openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001Blocking findings:
1Inline findings:
3Token source:
GITHUB_TOKENRunner:
b4dbfdf95573Run: https://rasterhub.com/rasterstate/fj/actions/runs/400
The tag was interpolated into the resolve step with a ${{ }} expression, which the Actions engine substitutes textually before the shell parses the line. A dispatched input of v1$(...) or v1";...;" therefore executed at the assignment, ahead of the case guard meant to reject it, in a workflow holding contents:write and issues:write. The guard was running after the thing it guarded. The value now arrives as RAW_TAG in the environment, so the shell receives it as data and the checks are the first code that sees it. Validation is an allowlist on the charset plus a shape check, because the tag also reaches git checkout, tarball names, the formula, and the release API. Also in the guard job, which only ever runs when a release has already failed: it installs curl and jq but not bash, so read -d did not exist and the indented EOF never closed its heredoc, silently swallowing the rest of the script. Built with printf instead. And an explicit test -f CHANGELOG.md so a missing file names itself rather than reading as a missing section.Forseti response,
69697acTwo of three actioned, one declined with reasoning. Taking them in order of how much they mattered.
P1,
release.yml:36, tag injection: real, fixedCorrect, and it was the more serious of the two because the
caseguard reads like protection while running strictly after the thing it protects.${{ }}is substituted textually by the Actions expression engine before the shell parses the line, so the pre-fix step compiled to:Either way the payload ran before line 37, in a workflow holding
contents: writeandissues: write.Fixed at the boundary rather than with quoting, since quoting
"${{ ... }}"does nothing here: the engine has already produced the shell text, and no amount of shell quoting in the template changes what a"in the input closes. The value now arrives asRAW_TAGinenv:, which the runner passes as an environment value, so the shell receives it as data and the validation is the first code to see it.Validation got stricter than the
v[0-9]*it replaced, because the tag also flows intogit checkout, tarball names, the rendered formula, and the release API. Allowlist on the charset first, then shape:Verified in
alpine:3.20, which is what the job runs on, by generating the pre-fix step's expanded text and running both versions against payloads that touch a marker file, so "executed" is a filesystem fact rather than a string match:Plus 16 accept/reject cases: accepts
v0.4.1,v10.20.30,v1.0.0-rc.1,v1.0.0+build.5; rejects newline injection,../../etc/passwd,v1${HOME},main,0.4.1,v1.0,v0..0, and empty.Same treatment applied to the
packagestep'sSTAGE=, whichpreparehad already made safe but which kept a tag in a script body. The rule for this file is now uniform and greppable: no tag is ever written into a script.P1,
release.yml:386,$authword-splitting: declining, false positiveThis one contradicts itself. It states
$authis unquoted, and quotescurl -H "$auth"from the same file as evidence. Quoted is what it already is, at every one of the four call sites.It also describes the assignment as spanning multiple lines. It is one line:
The multiline reading comes from the finding's own excerpt:
auth="Authorization: ***REDACTED***with no closing quote. That is the secret scrubber eating$TOKENand the"after it, which makes a complete line look unterminated to a reviewer working from scrubbed text. The conclusion is drawn from the redaction artifact rather than from the source.No change pushed. If the review pass can suppress on redaction-adjacent lines, that would kill this shape of finding generally, which seems more valuable than a per-instance dismissal.
P2,
release.yml:261, awk and a missing CHANGELOG: fixed, with a correctionAdded
test -f CHANGELOG.md. Worth correcting the reasoning though, because the finding hedges on something checkable: awk exits 2 on a missing file, andpipefaildoes propagate that out ofawk | sed. Soset -euo pipefailalready caught this, and the step was not silently passing.The check earns its place for two other reasons, which is what the comment now says:
pipefailbeing honored by whichever shell runs the step.All three paths verified under both bash and ash:
Two defects in the
guardjob, not flagged, fixed hereFound while re-reading that job for the
$authclaim. It installs onlycurlandjq, so it runs under ash, where the issue body was built with two bashisms:read -r -d ''does not exist in ash.<<EOFwith an indentedEOFnever terminates, so the terminator and everything after it were swallowed into the heredoc.Under ash the whole block produced nothing and exited 0:
Built with
printfnow, andneeds.*.resultmoved intoenv:for the same no-tags-in-script rule. This is the job whose only purpose is to speak up after a release has already failed, so a silent failure inside it defeats the point of adding it.Forseti review
1 blocking finding(s) from the lead reviewer (advisory: not gating merges yet).
Findings
P1
.forgejo/workflows/release.yml:57— The prepare job only validates the raw ref name by character set and a loose version-shaped glob. A branch named likev1.2.3passes this check, and the later checkout uses the bare ref name rather than the tag namespace, so a manual dispatch can build branch contents while the publish step targets that name as the release tag. That breaks the stated guarantee that dispatch builds an existing tag's code.PR:
rasterstate/fj#233Head SHA:
69697ac7da05Review job:
sha256:dabc8f7bbe6c8334dcdcde4e934f0e24a5fc219eabf99500ed4d2667f3467e42Provider pair:
openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001Blocking findings:
1Inline findings:
1Token source:
GITHUB_TOKENRunner:
8df7d6b0bbfbRun: https://rasterhub.com/rasterstate/fj/actions/runs/402
Forseti review
1 blocking finding(s) from the lead reviewer (advisory: not gating merges yet).
Findings
P1
.forgejo/workflows/release.yml:57— The prepare job only validates the raw ref name by character set and a loose version-shaped glob. A branch named likev1.2.3passes this check, and the later checkout uses the bare ref name rather than the tag namespace, so a manual dispatch can build branch contents while the publish step targets that name as the release tag. That breaks the stated guarantee that dispatch builds an existing tag's code.PR:
rasterstate/fj#233Head SHA:
69697ac7da05Review job:
sha256:dabc8f7bbe6c8334dcdcde4e934f0e24a5fc219eabf99500ed4d2667f3467e42Provider pair:
openai:gpt-5.5+anthropic:claude-haiku-4-5-20251001Blocking findings:
1Inline findings:
1Token source:
GITHUB_TOKENRunner:
8df7d6b0bbfbRun: https://rasterhub.com/rasterstate/fj/actions/runs/402
Confirmed and fixed in
a03aa35, not justified. The finding is correct, and it is the residue of my own previous fix:69697acmoved the guard early enough to run before a shell parsed the tag, but what it validated was the shape of a string, and the checkout still resolved a bare name. A shape is not an identity.This is not theoretical in this repository.
release/v0.3.0already exists as a branch, so version-shaped branch names are normal here, and a branch namedv0.4.1would satisfy both the charset allowlist and thev[0-9]*.[0-9]*.[0-9]*glob.Worse, git does not agree with itself about what a bare name means when a tag and a branch share it. Against a fixture holding tag
v1.2.3and branchv1.2.3at different commits:So the validation and the checkout can disagree about which commit the name refers to, and the disagreement surfaces as a warning rather than a failure. In a workflow that exists specifically to make a failed tag release recoverable, publishing branch contents as that tag's artifacts is the one outcome that would make the recovery path worse than useless.
The fix makes the guarantee structural rather than a pattern match, in three parts:
preparerequires the name to exist in the repository's tags collection, which a branch cannot satisfy, and exports the commit sha it resolves to. Verified against this repo's API:repos/rasterstate/fj/tags/v0.4.1returnscommit.sha 62d4d7f6..., whilerepos/rasterstate/fj/tags/mainerrors, so the endpoint does not fall back to branches. The lookup uses the same API base and token theguardjob already uses.buildandpublishcheck outrefs/tags/<tag>instead of<tag>, so there is no ambiguity left for git to resolve in either direction.git rev-parse HEADequals the shaprepareresolved. The namespace alone does not cover a tag that moves betweenprepareand a build, and this also makes the two jobs provably the same tree rather than two independent resolutions.Evidence for the fix, same fixture: the bare name checks out
2530fecand the assertion refuses the build;refs/tags/v1.2.3checks out7632c1fand the assertion passes. The resolver itself was exercised against a stub tags API over ten inputs and rejected, among others,v9.9.9andv0.4.0: both are shape-valid, andv0.4.0is a name with no tag in this repository, which is exactly the class the old guard waved through.The shape checks stay. They still bound the charset for the tarball names, the rendered formula, and the release API, and
/remains excluded so the tag cannot alter the API path it is interpolated into.Noting the pattern, since it is the same one twice: both times the guard was in the right place and operating on the wrong thing. First it validated a string after the shell had already expanded it; then it validated a string instead of pinning the ref. The correction in both cases was to make the check structural rather than textual.
Forseti review
No blocking findings from the lead reviewer.
No inline findings.
rasterstate/fj#233a03aa3559691sha256:09217eb0db3dca93c2b5cf81de03785981579698832cfa04be8028c247ea3911openai:gpt-5.5+anthropic:claude-haiku-4-5-2025100100GITHUB_TOKEN5c5e0f84c3d4Forseti review
No blocking findings from the lead reviewer.
No inline findings.
rasterstate/fj#233a03aa3559691sha256:09217eb0db3dca93c2b5cf81de03785981579698832cfa04be8028c247ea3911openai:gpt-5.5+anthropic:claude-haiku-4-5-2025100100GITHUB_TOKEN5c5e0f84c3d4