fix(release): publish rewritten packages during RC promotion - #3795

Protect package candidates referenced by open release PRs while pruning abandoned run-specific promotion artifacts. Document the stable-candidate lifecycle and maintenance-branch rollout requirement.

Assisted-By: Claude <noreply@anthropic.com>

Assisted-By: GPT-5 <noreply@openai.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

Promotion needs the `release` label to start work, so this PR added it
to the set of labels whose `labeled` event is not discarded. That set is
stated in four places and was updated in three.

`gate-concurrency-contract.bats` caught the shape of the problem and
then became the first casualty of it. It asserted that the concurrency
key excludes exactly ONE label from the `-label` group, by counting
occurrences of `github.event.label.name != '`. A second publishing label
made the count 2, turning the suite red — and with it `checks`, then
`finalize`, then `e2e`, then the required `E2E Tests` status. Raising
the constant to 2 would re-hardcode the same brittleness one label later
and throw away the invariant the test exists for: the set of labels
excluded from the `-label` suffix is exactly the set `plan` runs for.
Both sets are now extracted from the workflow and compared, so the
assertion holds at any number of labels.

`resolve_assets` was the guard that got missed. It sits on `e2e`'s
release arm, so a label event `plan` runs for and it skips for leaves
`e2e` with both arms false — `finalize` is force-skipped for release
PRs — while `e2e-report` still runs. On a promote PR already carrying
`full-e2e`, re-applying `release` therefore superseded the green run and
posted `E2E Tests` = failure, because `full-e2e` bypasses the "e2e not
repeated" branch and `E2E_RESULT` is `skipped`. `unlabeled` is not a
trigger, so removing the label could not clear it; only a push could.

`release` must also not publish on the fork lane. e2e-fork.yaml's
`resolve` decides "this run built nothing, so it is not a verdict" by
asking whether `Plan build` skipped. Admitting `release` at `plan` made
that false: a fork run reaches `plan`, has `build`/`finalize` gated off
by its own label, succeeds having produced no pr-patch, and the fork
lane fails closed on the missing artifact — red on a suite that was
never going to run, on a PR whose only fault is a mislabel. `plan` and
`resolve_assets` now admit `release` for same-repo PRs only, and the
concurrency key carries the complementary term, so
NOT(A OR B OR (C AND NOT F)) stays exactly (NOT A) AND (NOT B) AND
(NOT C OR F). For a same-repo PR F is false and every one of them
reduces to what it was, so none of this changes non-fork behaviour.

The same shape still reaches the fork lane from a PUSH to a fork PR
somebody mislabelled `release` — `build` is gated on the label, not on
the event, so this predates promotion using the label. Left alone; the
comment in e2e-fork.yaml now records it instead of asserting an
invariant this PR would have made false.

The contract enumerated `plan` and `e2e-report` by name, which is why it
did not catch `resolve_assets`: a named-job loop cannot notice a job
nobody thought to name. It now derives the job list from the file and
requires every job that decides from a label name to decide from the
same set, and pins the fork term on both halves of the complement. The
file's existing properties are kept — extraction runs over
executable-only lines, so a commented-out key cannot satisfy the
contract, and every comparison asserts a non-empty input first, so an
empty extraction fails closed under both bats and cozytest.sh.

Verified by mutation, each caught: excluding a label `plan` skips for,
admitting one the key does not exclude, reverting `resolve_assets`,
deleting `e2e-report`'s guard, narrowing `verify-release-candidate`'s,
renaming the label in one guard only, commenting out the group key, and
dropping the fork term from either side alone. Deleting `plan`'s
step-level fork guard is caught too, by an assertion that had to stop
matching on the bare identifier once `plan`'s own `if:` named it.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

The mode check was written as a single chain:

  if { [ -x "$a" ] && [ ! -x "$r" ] || [ ! -x "$a" ] && [ -x "$r" ]; }

which reads like a symmetric difference and is not one. POSIX gives
`&&` and `||` equal precedence and left associativity, so it groups as
`((A && B) || C) && D` with `D = [ -x "$r" ]`. A file executable in the
candidate and not in the release tree makes `A && B` true but `D`
false, so the whole condition is false and the mismatch is accepted.
Only the mirror case was rejected — and it is the benign one. The
direction that got through is content the merge reviewer never saw made
runnable inside the very artifact the operator installs, which cmp(1)
cannot see because it compares bytes.

Resolve each side to a value and compare those, so no precedence rule
is load-bearing.

Both directions now have coverage. Before the fix the candidate-only
test failed against the shipped condition and the release-only test
passed, which is what makes the pair meaningful rather than a check
that only goes red when the whole block is deleted.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

hack/promote-packages-artifact.sh pushes the candidate BEFORE rewriting
installer values, so the artifact carries the rc tree's
platformSourceUrl/platformSourceRef verbatim — and
hack/verify-promoted-packages.sh reads exactly those two fields back out
of it as the candidate's rc baseline, requiring a trusted repository and
an immutable `digest=sha256:<64 hex>` pin.

The publisher checked neither. A tag-form ref or an untrusted URL in the
rc tree published a candidate and pushed two commits regardless; the
refusal arrived later, in the promote PR gate or in finalize, once the
staging branch and the draft release already existed. Check both at
dispatch time instead, while nothing has been written.

A copied regex would drift from the guard it is bringing forward and
quietly restore the late failure, so the trusted repository and the
digest pattern move into hack/lib/promoted-packages.sh and both halves
source it. That makes the library part of what the verifier needs at run
time, so promote's base-branch precondition gains it: the list exists
because the base's own workflows run after merge, and a base carrying
the verifier without its library fails at the same two late points.

Tests: both preflights are covered, each asserting the mock registry
client was never invoked — "exited non-zero" alone does not say nothing
was written. Both fail with the preflight removed. The publisher
fixtures now pin the trusted repository the way a real rc tree does, and
still send the candidate elsewhere via REGISTRY so the rewrite
assertions keep their teeth.

Also covers a malformed PROMOTION_ID. It is not attacker-influenced, but
it is the run-uniqueness suffix of the candidate tag and retention.yaml
deletes a candidate only when every tag on the manifest matches
`promotion-v…-run-[0-9]+-[0-9]+`; a value outside that shape publishes
an artifact no cleanup pass can ever collect.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

Three review findings on the new promotion steps.

`yq` was fetched from `releases/latest/download` in all three steps this
pipeline installs it in — the promote dispatch, the promote PR gate and
finalize. That makes an unpinned moving dependency part of what decides
whether a stable release may be created, and part of what rewrites the
installer pin: two promotions of the same rc can be served different
binaries. Pin it the way FLUX_VERSION already is, version-gated so a
matching preinstalled yq is reused, on the version the e2e sandbox image
already tracks.

A version authenticates nothing about the bytes it names, though, and
these are the workflows that mint write-once stable names. yq is
therefore also pinned BY CONTENT: its SHA-256 is committed beside its
version and verified before install.

Scoped to yq deliberately. It is the one downloaded binary with write
authority over what a release contains — it stamps the candidate digest
into installer values at promote time and platformVersion into the
stable chart at finalize. skopeo, helm and oras only move bytes that are
already digest-addressed, so a substituted one is caught downstream; a
substituted yq writes the wrong pin and every later check verifies that
pin faithfully.

The Flux installer is left as it is, on purpose. It fetches
flux_${FLUX_VERSION}_checksums.txt from the pinned release and verifies
the binary against it, so the binary is not the exposure — the ~6 KB
bootstrap script from fluxcd.io is, and it is mutable by design and
fetched identically in ten other workflows here. Pinning its bytes in
these three files would break on the next regeneration of a script the
project does not control while leaving the same fetch unpinned
everywhere else. The comment names the residual risk instead.

No precedent existed either way: nothing under .github/workflows/ or
hack/ verified a downloaded tool before this, and every version-pinned
download in the tree (oras 1.2.0, cozyvalues-gen 1.6.0) is unverified.
This is a new convention rather than an application of one, confined to
the release path and justified by write authority, and the comment says
so where the next reader will be standing. The three unpinned fetches
outside this pipeline (nightly.yaml, e2e-fork.yaml) are left alone.

The checksum is trust-on-first-use, which is the honest description: it
fixes the bytes to what was checked when the version was chosen and
makes any later substitution loud. Provenance is the release's own
published checksums list cross-checked against the downloaded asset;
both agree on
fa52a4e758c63d38299163fbdd1edfb4c4963247918bf9c1c5d31d84789eded4, and
that binary self-reports v4.53.3.

Verified locally against the real asset: the version gate skips a
matching preinstalled yq and re-installs a mismatched one, the checksum
line reports OK for the genuine binary and exits 1 for a tampered copy,
which aborts the step under the runner's `bash -e`.

`verify-release-candidate` had no `timeout-minutes`, so it inherited the
6-hour default while `e2e-report` waits on it through `needs` — a
stalled `flux pull artifact` would hold the required `E2E Tests` status
pending for the whole of it. 30 minutes, matching `checks` and
`finalize`.

The contract pins that all three install steps carry both versions and
the checksum, run the checksum check, name no `latest/download`, and
agree on one yq version and one checksum. So the unpinned one-line idiom
cannot come back into a release step by copy-paste from the workflows
that still use it, and bumping the version in one file without its
checksum cannot pass. Both mutations are caught.

Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>

Three gaps in the guards that decide whether the published candidate is
still the tree being tagged. All three passed silently rather than
failing, which is the worst shape for a check that runs immediately
before a write-once name is created.

`normalized_refs` piped `collect_image_refs` straight into its parsing
loop, so the pipeline reported `sort -u`'s status and the collector's
was discarded. POSIX sh has no `pipefail` and this script has to stay
POSIX, so `set -e` never saw it: a failed or truncated collection became
a smaller set and the rc-vs-candidate comparison matched it happily. The
verifier then reported that no container bytes changed across promotion
having proven nothing — on the guard that carries that entire claim.
Collect into a file first, so a nonzero collector aborts before anything
is compared. An empty collection is the same failure wearing a zero exit
status and needs its own check: two empty sets compare equal, so the
proof passes having examined nothing, and every packages tree carries
image references — none means the collector understood nothing it was
handed.

The rc-leftovers scan swallowed grep's stderr, turning "could not read
this file" into "this file is clean" — the silent-skip shape
hack/promote-rewrite-tags.sh refuses by name. The `|| true` has to stay,
because `-exec … +` reports the legitimate "nothing matched" as a
failure too, so the diagnostics are the only remaining signal that a
file went unscanned. Fail on any of them. Defence in depth rather than a
live hole: a surviving rc string would also fail the per-file compare,
since the release tree has none.

The per-file content comparison had no test at all. It is the only leg
that catches a file present on BOTH sides with different bytes — exactly
the threat the job exists for, a maintainer editing packages/ on the
release PR after promotion published the candidate. The file-set compare
sees the same names, the installer compare reads one file, and the
rc-refs compare never looks at the release tree. Replacing the
comparison with a no-op left all nine other tests green.

Red phases, all observed before the fixes. The content-compare case
changes one non-image line on one side, so no other leg can fire and
claim the credit; it fails with the comparison no-op'd and passes with
it restored. The two collector cases run the verifier from a copy whose
sibling lib/ holds a stub — which is how the workflows already invoke
it, `.release-tooling/hack/…` with libraries resolved next to the
script, so the failure is reached the way production would reach it.
Against the piped version both fail on `[ "$rc" -ne 0 ]`: the verifier
exits 0 and prints its success line whether the collector returns 3 or
returns nothing.

Also pins `e2e`'s dependency on the candidate gate. The contract pinned
`e2e-report`'s, so removing `verify-release-candidate` from `e2e`'s
`needs` together with the `success` clause on its release arm left all
six tests green — and running a full-e2e promotion suite against a
candidate that never verified is the worse of the two failures.

Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>

Retention protected a packages candidate only while an OPEN bot release
PR pinned its digest. The digest stops being eligible for deletion only
when finalize's retag adds a stable tag to the manifest, because that is
what makes the `all(tags match promotion-*)` predicate stop matching —
and finalize runs after the merge that closes the PR.

The window between them is where a half-finished release sits. Finalize
creates the write-once stable git tag, then dies before the retag: a
lost runner, a 5xx publishing the draft, a non-fast-forward on the
maintenance branch. What is left pins the candidate from a merge commit
and a tag nobody can move, with no open PR and only a temporary tag on
the manifest. Thirty days later retention deletes it, after which
re-running finalize 404s at the candidate check and re-dispatching
promote is refused because the stable tag exists. The release can then
only be finished by hand — the exact class of manual cleanup this design
exists to remove.

Protect the tips of the branches a promote PR can merge into as well.
The two sources are read differently on purpose. An open promote PR's
head is supposed to carry exactly one candidate pin, so anything else is
an anomaly and blocks. A branch tip is an additional source: every
digest pin found is protected and a branch without one contributes
nothing, because a maintenance line cut before this pipeline existed
cannot pin a candidate and must not wedge retention forever.

Blocking now stops the promotion sweep alone. It used to `exit 1` inside
the resolution loop, which also killed the unrelated nightly prune
further down the same step — so a WIP commit adding a trailing comment
to a pin on a release branch would silently stop GHCR nightly pruning.
The job still fails; it fails after the nightly sweep has run.

Verified by running the step's script against a stubbed `gh`: a digest
pinned by main's tip and referenced by no open PR is now excluded while
a genuinely abandoned one is still listed; a manifest that also carries
a stable tag stays excluded by the existing predicate; and a malformed
pin on a PR head leaves the nightly sweep running, skips the promotion
sweep and exits 1.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

Three claims that were stronger than the code, plus one legibility fix.

The PR gate's comment said a `packages/` edit "on the PR or its base"
fails the status before merge. The base half is not true: GitHub fires
no `pull_request` event when the base branch advances, it only
recomputes refs/pull/N/merge, so a change landing on the base after this
went green is never re-verified and the PR merges on the stale result.
Checking the merge tree does make base drift visible WHEN the job runs,
which is worth keeping — but finalize is what actually catches the drift
case, after the merge and before any stable name. Say both, in the
workflow and in the reviewer checklist, so nobody reads the green as
covering more than it does.

"Transactional at every consumer-visible name" is a judgement call
dressed as an absolute, inherited from the "this dispatch performs NO
registry mutation" it replaced. The dispatch now does write to the
registry: the `promotion-*` candidate is a real, publicly listable tag.
What it creates is no *stable* name — no vX.Y.Z, no :latest, no stable
installer. Stated that way the sentence stops needing the reader to
already know which writes were meant.

The retention paragraph listed two of the conditions the workflow
enforces. It now lists all of them, including the ones the workflow
gained here: same-repository, bot-authored, `release-X.Y.Z` branch, the
`release` label, exactly one valid digest pin, and protection from a
base-branch tip as well as an open PR.

Also: `flux pull artifact` reads its credentials from
~/.docker/config.json, which is why finalize now runs `docker login` —
but its runner class is not one of the docker-using build ones and the
setup step installs everything except docker. Check for it there, where
the message can name what is missing, instead of failing with "command
not found" halfway down the release path. And the verifier's exclusion
list is a restatement of flux's `excludeOCI`, not a derivation — the CLI
does not expose it — so the coupling and the pinned version are now
written down next to the list.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

The scan treats any grep diagnostic as a failed scan, because
`find ... -exec ... +` reports the legitimate "nothing matched" as a
failure too and the `|| true` absorbing that also absorbs a file
that went unscanned. Deleting the whole guard left the suite green:
its two siblings -- a failing collector and an empty collection --
each have a dedicated test, and this one had none.

Reach the state the guard exists for by putting a grep on PATH that
answers the scan's invocation the way the real one answers a file it
cannot open. A chmod 000 fixture would stop proving anything
wherever the suite runs as root.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

Both candidate-protection sources open with a `gh api --paginate`
listing and neither handled a failure. Under `set -euo pipefail` a
transient /pulls or /branches blip therefore exited the step before
the unrelated nightly GHCR sweep had run -- the exact coupling the
deferred guard at the bottom exists to remove, and which
hack/promotion-retention-contract.bats already asserts positionally.

Route both listings into that guard instead: raise the blocked flag
and empty the list, so the promotion sweep is skipped and the job
still fails while the nightly sweep finishes. Emptying is not
cosmetic -- a half-paginated list left in place is a smaller
protected set, and a smaller protected set is the one shape that can
authorise a delete.

Replaying the step's real shell against a stub gh: with /pulls
failing it exited 5 and never reached the nightly line; it now exits
1 with the nightly sweep done and the promotion sweep skipped.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

The library header still claimed three call sites. There are four:
this PR made hack/verify-promoted-packages.sh source it to compare
the candidate's refs against the rc artifact's. The historical
drift story stays with the original three, since the verifier has
only ever read the shared enumeration.

The consumer table gained hack/promote-packages-artifact.sh under a
bullet requiring every consumer of references to read all three
shapes through the library, while the exception below it still named
only hack/overlay-main-images.sh. The publisher sources nothing and
needs nothing: it hands the whole tree to `flux push artifact` and
touches one reference, the installer's own self-pin, by key with yq.
Say that, rather than implying a gap it does not have -- the real
exception remains overlay-main-images.sh, which does read references
and does walk the tree itself.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

The nightly sweep's own listing shared one `|| true` with the grep
that filters it, and grep legitimately exits 1 when no cozystack/*
package matches. A failed /orgs/.../packages read therefore arrived
as an empty list, took the "No cozystack/* container packages found"
branch and exited 0 -- below the nightly sweep but ABOVE the deferred
guard, so a protection set that could not be resolved reported
success having swept nothing. That bypasses the guard rather than
varying it: replaying the step against a stub gh, a failed /pulls
combined with a legitimately empty org listing exited 0 after
printing its own ::error::.

List and filter as two statements so one `|| true` stops meaning two
things, handle the listing failure like the other two (::error::,
blocked flag, emptied list), and consult the flag before the early
exit. That exit is non-zero, matching the other two listing
failures: no sweep is left to carry the run down to the guard, so
this is the last place the flag can be honoured, and an unresolved
protection set is exactly what fail-closed means. The legitimate
empty-org case still prints the same line and exits 0.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

The routing bullet called hack/overlay-main-images.sh "a fourth
consumer" from a time when three scripts sourced the enumeration.
Four do now, so the ordinal names the wrong thing in the file every
agent reads first.

State the distinction instead of a count: overlay is the exception
because it reads refs and walks the tree itself, so a newly declared
file never reaches it, while hack/promote-packages-artifact.sh
sources nothing because it publishes the whole tree and consumes no
refs. One clause each, with the existing pointer to
docs/agents/image-refs.md carrying the rest.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

This reverts commit 302b55c.

Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>