Uh oh!
There was an error while loading. Please reload this page.
ci(release): sign and propagate in the publish stage - #284
Conversation
a50209b to
766d2f4Compare
LeeroyHannigan
left a comment
There was a problem hiding this comment.
Reviewed in depth. This is merge-safe once rebased, and the design is right: signing and propagation ride the existing dockerhub approval, permissions are job-scoped to exactly the publishing job, the INDEX digest is what gets signed, and nothing here touches the latest/no-overwrite guards in promote-image.
First, rebase onto main: five of the six commits are the #279 signing infra, now byte-identical to main, so they collapse to nothing and the reviewable delta becomes just 4f640ba (release-image.yml, +104/-8). I'm holding approval until then only because a rebase push would dismiss it anyway.
One thing to fix before this is relied on operationally:
- The no-overwrite guard exits 1 whenever the candidate tag exists, but this PR appends the failure-prone tail (two OIDC assumes, cosign+Rekor, two cross-registry copies) after the push. Any post-push failure leaves the candidate published-but-unsigned and blocks every re-dispatch at the guard, so recovery means manually deleting the Docker Hub tag. The guard's own comment says identical-artifact re-runs are permitted; make the code do that: if the candidate exists and its digest matches the built artifact, skip the push and continue. The propagate loops are already idempotent, they're just unreachable on a retry today.
Smaller items, none blocking: copy the image leg by @digest rather than tag (promote-image already does; the .sig leg stays tag-addressed), route ${{ github.actor }} (L375) through env like everything else, capture the digest from the create output instead of re-inspecting the tag, and the checkout pin comment on L275 says v4.2.2 for a SHA that's v4.4.0 elsewhere.
Ping me after the rebase and I'll approve.
There was a problem hiding this comment.
Signing inside the publish approval is the right move. One blocker inline, one below since the code is unchanged.
Blocking: the push guard at :311-314 exits 1 whenever the candidate tag exists, and that tag is created at :321 before sign, GHCR and ECR. So any failure past :321 strands a pushed-but-unsigned candidate that no re-dispatch can finish, and it makes the tolerate-existing branches at :382-383 dead code. The comment at :309-310 promises the opposite. Fix: if the tag exists, compare its digest to what was just built and fall through to sign/propagate when identical; only a different digest should error. Same gap you flagged as should-fix 2 on #281.
- Should-fix: this duplicates #283's
promote-registrycomposite, and they already disagree (digest vs tag, unsigned-refusal vs not). They'll drift. - Nit: headers at :4-6 and :29-33 still say the workflow stops at the candidate.
- Nit: summarise step at :430 has no
if:, so a partial-failure run produces no summary.if: always(). - Nit: "stacked on #279" is stale, merge base is main's tip.
Checked and fine: the two sequential OIDC assumptions are correct, and sig-tag/sig-digest do exist on the composite.
| set -euo pipefail | ||
| # The pushed index, fetched as any user would fetch it, must be the | ||
| # digest we just recorded and contain both platforms. | ||
| RAW=$(skopeo inspect --raw "docker://${IMAGE_REPO}@${DIGEST}") |
There was a problem hiding this comment.
Blocking: this isn't anonymous and isn't a digest check.
docker/login-action at :293-297 already wrote creds to ~/.docker/config.json, which skopeo reads by documented fallback, so this fetches as the maintainer PAT. And inspecting @${DIGEST} is content-addressed, so it can't detect the tag resolving elsewhere: it never looks at the tag. Since DIGEST came from this same index at :324, nothing new is proven.
That matters because it's the only gate between publish and "users can pull this", and it'd pass on a repo that isn't anonymously pullable.
GOT=$(skopeo inspect --no-creds --format '{{.Digest}}'"docker://${IMAGE_REPO}:${CANDIDATE}")
[[ "$GOT"=="$DIGEST" ]] || { echo"::error::tag resolves to $GOT";exit 1; }(needs CANDIDATE in the step env)
| for pair in "${CANDIDATE}|${DIGEST}" "${SIG_TAG}|${SIG_DIGEST}"; do | ||
| TAG="${pair%%|*}" | ||
| WANT="${pair##*|}" | ||
| if EXISTING=$(skopeo inspect --format '{{.Digest}}' "docker://${GHCR_REPO}:${TAG}" 2>/dev/null); then |
There was a problem hiding this comment.
Should-fix: branches on exit code with stderr discarded, and skopeo inspect exits nonzero for blips and auth errors as well as 404. Any nonzero skips the different-digest check and copies; the post-copy assert then compares what was just written, so it always passes and hides that the guard was bypassed. Same at :413, and same pattern in #283's composite.
| # automated gates between are the safety net). | ||
| permissions: | ||
| contents: read | ||
| packages: write # GHCR propagation |
There was a problem hiding this comment.
Should-fix: job-scoped, so every step carries these including summarise. Bigger point: a job holds one environment, this one is dockerhub (:265), but ghcr-mirror.yml:69 gates GHCR on ghcr, so this path bypasses it and ECR gets its first automated write path with no environment of its own. The comment on :265 also still says "the only job with secrets".
| id-token: write # KMS signing + ECR propagation (OIDC roles) | ||
| steps: | ||
| - name: Check out (composite signing action lives in-repo) | ||
| uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.2.2 |
There was a problem hiding this comment.
Nit: this SHA is v4.4.0, not v4.2.2 (that's 11bd7190...). The repo's other four call sites say # v4.4.0, and #290 is rewording a comment on this same SHA, so worth converging.
| # Constant username: GHCR ignores it with a GitHub token, and it keeps | ||
| # the operator identity out of the run block. | ||
| skopeo login ghcr.io -u x-access-token --password-stdin <<< "$GHCR_TOKEN" | ||
| for pair in "${CANDIDATE}|${DIGEST}" "${SIG_TAG}|${SIG_DIGEST}"; do |
There was a problem hiding this comment.
Nit: latent only, since I checked both sign outputs do exist. But if either were ever empty the pair becomes "|", skopeo gets an empty tag and defaults to latest, which is the one tag this workflow says it never writes. [[ -n "$TAG" && -n "$WANT" ]] || exit 1 covers it.
robinnsc
commented
Aug 20, 2026
All addressed in the latest commit: Blockers:
Should-fixes:
Nits: headers rewritten (the workflow no longer "stops at the candidate"), summary is The operator picture across #283+#284 is now symmetric: release = dockerhub approval (publish+sign) → ghcr approval (propagate); promotion = dockerhub approval (canary) → ghcr approval (mirrors). Two environments, each gating its own writes, four clicks per release end to end. |
There was a problem hiding this comment.
Re-reviewed at 954748b. Everything from the last round is addressed, and a couple of the fixes are better than what I suggested:
- The anonymous verify is now genuinely anonymous (
--no-creds) and tag-addressed with a digest assertion - exactly the property the step name claims. - The re-entrancy fix is the stronger option: instead of just tolerating an existing tag, it compares per-arch image config digests against what this run built and tested, with no registry mutation, and only then falls through to sign/propagate. A different artifact still hard-errors. Nice.
- Propagation moved to its own job under the
ghcrgate, thecopy-artifactcomposite carries the fail-closed three-way probe and the empty-input guard, headers are current, all five checkout pins read# v4.4.0, and the summary runs on failure. - The cosign concern I raised on #283 doesn't apply here:
sign-imageinstalls a pinned, checksum-verified cosign itself. That's the pattern #283 should borrow.
Two tiny things, neither blocking:
- Nit: the PR title still says "(stacked on #279)" - the body was fixed but the title wasn't, and #279 has been on main for a day.
- Note:
probe()is now copy-pasted betweencopy-artifactand #283'spromote-registry. Fine for now; worth folding into one place whenever one of them next changes.
Good to go from my side.
After the candidate publishes and verifies anonymously (tag-addressed, --no-creds), the same approved job signs it via the shared composite action (KMS via OIDC, Rekor, tag-based .sig); a second job under the ghcr environment then propagates image + signature to GHCR and ECR through the new copy-artifact composite action (idempotent, three-way fail-closed probes, digest-asserted). The candidate push guard is re-entrant: identical artifacts (compared by per-arch image config digest, no registry mutation) fall through so a tail failure is finishable by re-dispatch; different artifacts hard-error. Squash of the reviewed branch (tree identical to ffe58fd); review history on PR #284.
ffe58fd to
c867956CompareUh oh!
There was an error while loading. Please reload this page.
What
Extends
release-image'spublish-candidatejob so the one publish approval carries the release tofully-signed-everywhere:
both platforms, no credential)
tag-based
.sig, post-sign tag gate)GITHUB_TOKEN) and ECR Public(OIDC publisher role) by digest,
--preserve-digests, per-tagno-overwrite gates, per-copy digest verification
The job gains
packages: write+id-token: write(job-scoped), acheckout step (the composite action is in-repo), and updated summary /
footer text. Idempotent re-runs: every leg no-ops on identical digests and
fails on divergent ones.
Why
Phase 1 completion (runbook §7): with this plus #283, a release is two
approved runs — this workflow (publish→sign→propagate) and
promote-image(all-registry promotion). v0.1.6 needed those two approvals plus a
manual laptop signing session, a manual Isengard/crane ECR mirror, and
three separate
ghcr-mirrordispatches with approvals. Atomicity alsocloses the ordering hazards we hit live: signing can no longer happen
after promotion (v0.1.5) and no registry ever serves an unsigned
candidate that a
.sigdispatch could miss.Testing done
yaml.safe_loadpasses.propagation gates replicate the ghcr-mirror gates that have now passed
live runs for image, signature, and release-tag artifact kinds.
0.1.7 release, or a re-dispatch of v0.1.6's tag — every leg no-ops on the
already-current digests except signing, which adds a Rekor-logged second
signature to the existing
.sigtag (harmless, and proves the OIDC + KMSChecklist
cargo test --workspace) — not applicable, workflow-only changecargo fmt --check) — not applicablecargo clippy -- -W clippy::pedantic) — not applicableADR / RFC: n/a — CI tooling only.
Breaking changes
None. Tag-push trigger, gate, builds, and the publish approval are unchanged.
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.