Release path: digest promotion, self-verifying publish, RC tags - #60
Merged
Conversation
The publish job has never executed -- environment approval, GHCR push under the release token, cosign OIDC, attestation upload, gh release create are all untested. Tags are meant to be immutable, so a v0.1.0 that fails halfway leaves a half-published release and a version that cannot be cleanly redone: you would burn 0.1.0 and ship 0.1.1 as the real first release. An rc number is free to burn; a version number is not. verifyReleaseVersion and just release now accept X.Y.Z and X.Y.Z-rc.N, keeping the tag<->canonical-version equality check that makes drift impossible. Deliberately NOT the full SemVer pre-release grammar: -rc.N is the only form this project ships, and an exhaustive pattern would accept -alpha/-beta.2+build and other identifiers whose publish semantics nothing implements. Build metadata stays unsupported -- it does not affect precedence, so it cannot mean anything useful on a release tag. Verified across the matrix: rc tag + rc version passes; every tag/version mismatch fails on the equality check; -beta.1 fails on grammar; a -SNAPSHOT canonical version still fails.
…hing Reworks the publish job so the release stops being the least-verified thing we ship. Digest promotion. The image is now pushed BY DIGEST with no tags, smoked, and only then tagged via imagetools create. Previously build-push-action attached X.Y.Z and latest in the same step, so the release tags named a build nothing had ever executed -- while dev sha- images got a full runtime smoke in ci.yml. The release image now gets that same deep smoke (Parquet write, DuckDB read-back, resume no-op) against the exact digest the tags will point at. imagetools create copies no bytes; it repoints tags at the smoked digest. Rolling X.Y tag for stable releases. No bare X while 0.x -- a '0' tag would imply a stability contract 0.x disclaims. RC branching. A -rc.N tag publishes its own immutable container tag only (no latest, no X.Y) and a GitHub pre-release. Everything else -- promotion, smoke, signing, attestation, self-verification -- runs identically, which is the whole point of a rehearsal. Signing now covers SHA256SUMS and the SBOM, previously omitted. SHA256SUMS is signed last and deliberately: it is the one file whose integrity transitively covers every other asset. Dropped COSIGN_EXPERIMENTAL, a no-op since cosign 2.0 went keyless-GA in 2023. Artifact attestations alongside cosign, answering the question signatures cannot: which workflow, at which commit, produced this. Assets, the image digest (push-to-registry), and the SBOM. Both actions pinned to verified v4 commits resolved via the API, not from memory. Draft, verify, then publish. gh release create publishes and THEN uploads, so a mid-upload failure left a live half-release and notified watchers. Now it creates a draft, runs the exact verification commands the docs give users -- sha256sum --check, cosign verify-blob, cosign verify, gh attestation verify -- and only un-drafts if they pass. A signature nobody can verify is worse than no signature, so that failure belongs here rather than in a user's terminal. It is also what makes the rc rehearsal assert something instead of merely executing. Verified: workflow parses; step order correct; tag selection simulated for rc/0.x/1.x; all twelve pinned action SHAs resolve via the API.
RELEASING.md: the versioning rule now admits X.Y.Z-rc.N (and still nothing else); a new section explains what a release candidate does, why it exists -- a failure costs an rc number instead of an immutable version -- and that it is NOT promoted, since its jar reports X.Y.Z-rc.N and shipping those bytes as X.Y.Z would contradict the tag. The cut steps now describe draft -> verify -> publish, including that a failed self-verification leaves the release a draft. Both docs gain the verification commands, quoting the exact certificate identity and OIDC issuer rather than just naming the flags -- an identity a user cannot construct is a verification story they will not follow. These are the same commands the pipeline runs against itself before un-drafting, so the docs cannot drift from what actually passes. Note this pins the workflow FILENAME: renaming release.yml would change the identity and invalidate every published instruction. install.md also documents the two first-run traps hit while smoke-testing the public image today: the container runs as UID 10001 so a directory you own is not writable (recommending --user over chmod 777, which also leaves output owned by you), and a short run looks silent because progress is appended every 30s off a terminal rather than redrawn. Verified: HeadlineDocsCommandSmokeTest, which reads docs/install.md, passes.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe release process now supports ChangesRelease pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseWorkflow
participant GHCR
participant GitHubRelease
participant Verification
ReleaseWorkflow->>GHCR: Push image by digest
ReleaseWorkflow->>GHCR: Smoke-test digest and apply release tags
ReleaseWorkflow->>GitHubRelease: Create draft release with assets
ReleaseWorkflow->>Verification: Verify checksums, signatures, and attestations
Verification-->>ReleaseWorkflow: Verification succeeds
ReleaseWorkflow->>GitHubRelease: Undraft release
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 173-178: Update the “Install DuckDB CLI” step to verify the
downloaded release asset’s authenticity before installation or execution. Pin
and validate the expected checksum or an upstream signature for the exact DuckDB
version and platform, fail the workflow on verification failure, then retain the
existing extraction and installation flow only after successful validation.
- Around line 284-314: Update the “Self-verify the published release” workflow
step to download the draft release assets into a fresh temporary directory using
the release identifier before verification. Run sha256sum, cosign verify-blob,
and gh attestation verify against the downloaded SHA256SUMS and signature files
there, and complete these checks before the release is un-drafted; leave image
verification unchanged.
- Line 150: Update the tag output in the release workflow to include the
documented major-version tag using VERSION%%.* for stable 1.x+ releases, while
preserving the existing full, minor, and latest tags and the intended 0.x
exception behavior.
In `@docs/install.md`:
- Around line 102-113: Update the fenced command blocks around the TAG/IDENTITY
verification commands and the additional blocks at the referenced locations to
include a shell language identifier, using ```sh or ```console, without changing
their command contents.
- Around line 95-97: Revise the release verification statement in
docs/install.md to distinguish the signed checksum manifest and OCI
image/SHA256SUMS attestations from per-asset provenance; do not claim every
release asset has its own SLSA attestation unless the release workflow adds and
verifies those attestations.
In `@justfile`:
- Around line 95-96: Update the comments and usage example near the VERSION
validation command to document that X.Y.Z-rc.N release candidates are supported,
removing the stale guidance that rejects all pre-releases. Keep the existing
validation behavior and error message unchanged.
In `@RELEASING.md`:
- Around line 104-123: Update the fenced code block containing the release
verification commands to declare the Bash language on its opening fence, while
leaving the commands unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5b005ea3-1457-4735-b434-a8161723c791
📒 Files selected for processing (5)
.github/workflows/release.ymlRELEASING.mdbuild.gradle.ktsdocs/install.mdjustfile
Comment on lines
+104
to
+123
| ``` | ||
| TAG=v0.1.0 | ||
| IDENTITY="https://github.com/varveio/swath/.github/workflows/release.yml@refs/tags/${TAG}" | ||
| ISSUER=https://token.actions.githubusercontent.com | ||
|
|
||
| # 1. Checksums cover every asset. | ||
| sha256sum --check SHA256SUMS | ||
|
|
||
| # 2. The checksum file itself is signed, so step 1 is trustworthy. | ||
| cosign verify-blob --bundle SHA256SUMS.sigstore.json \ | ||
| --certificate-identity "$IDENTITY" --certificate-oidc-issuer "$ISSUER" SHA256SUMS | ||
|
|
||
| # 3. The image, by digest. | ||
| cosign verify --certificate-identity "$IDENTITY" --certificate-oidc-issuer "$ISSUER" \ | ||
| ghcr.io/varveio/swath@sha256:<digest> | ||
|
|
||
| # 4. Build provenance — which workflow, at which commit, built this. | ||
| gh attestation verify oci://ghcr.io/varveio/swath@sha256:<digest> --repo varveio/swath | ||
| gh attestation verify SHA256SUMS --repo varveio/swath | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Declare the fenced block language.
Add bash to the opening fence at Line 104 to satisfy markdownlint MD040.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@RELEASING.md` around lines 104 - 123, Update the fenced code block containing
the release verification commands to declare the Bash language on its opening
fence, while leaving the commands unchanged.
Source: Linters/SAST tools
The deep runtime smoke was copy-pasted into both publish paths -- same bucket, same DuckDB read-back, same resume-no-op assertion -- differing only in the work directory and how the image is named. That is the wrong shape for this check in particular: its whole purpose is that the release image gets the SAME verification as dev images, and two copies drift silently. scripts/ci/smoke-container.sh <image-ref> [work-dir] is now called from ci.yml's docker-publish and release.yml's publish, matching the existing prepare/verify-container-promotion.sh pattern, and runs locally against any image reference. It refuses to reuse an existing work directory rather than pre-deleting one. The smoke deliberately does NOT pass --user, because exercising the image's own default identity (UID 10001) is part of what it verifies -- but that means outputs land in 10001-owned subdirectories the invoking user usually cannot unlink, so an 'rm -rf' here would fail on the second local run. Found by hitting exactly that while testing; it now exits 2 with the elevation hint. Verified against the live public image: 7 objects listed, 7 read back via DuckDB, resume no-op left the count unchanged, exit 0. Negative paths: wrong arity exits 2, an existing work directory exits 2, an unresolvable image exits non-zero rather than passing vacuously. bash -n and shellcheck clean.
A release publish now depends on listing a bucket we do not control. The default target is a CMAS example case for the SMOKE emissions model -- a third-party research-consortium bucket. If it is ever removed, renamed, or flipped to requester-pays, the release path breaks at the worst moment: after environment approval, with the image already pushed by digest and its tags not yet applied. SWATH_SMOKE_BUCKET lets an operator repoint it with a repository variable and a re-tag, instead of a code change, a PR and a merge while a half-published release waits. Same reasoning as PUBLIC_RELEASE_ENABLED being a runtime switch. Both workflows pass vars.SWATH_SMOKE_BUCKET through; unset leaves today's behaviour exactly as it was. Also corrects the inherited comment describing the bucket as 'a small, stable public AWS Open Data prefix'. It is public, small and us-east-1, but I could not confirm it belongs to the Registry of Open Data on AWS, and the distinction matters now that a release depends on it. Notes too that 'SMOKE' in the bucket name is Sparse Matrix Operator Kernel Emissions, not a smoke test -- the name is a coincidence. Verified all three paths against the live image: unset -> 7 objects (default), overridden -> 2 objects (a different bucket), empty string -> falls back to the default. That last case is what the workflow actually passes when the variable is unset, so the :- form is required.
…e major tag
Seven review findings, all valid.
Verify DuckDB before executing it. The install curl'd a binary and sudo
install'd it with no integrity check, in a job holding contents:write,
packages:write, id-token:write and attestations:write -- the
highest-privilege foothold in the pipeline, able to sign and publish. Now
checksum-pinned, at all three call sites (release.yml plus both in ci.yml).
Verify what GitHub STORED, not what we uploaded. The self-verification ran
sha256sum and cosign verify-blob against release-assets/ in the workspace,
re-confirming files already known good and missing the failure it exists to
catch -- a truncated or mis-uploaded asset. It now downloads the draft's
assets into a fresh directory and verifies those.
Implement the bare major tag for 1.x+. The comment promised '{{major}} at
1.0' but no code emitted it, so at 1.0 the behaviour would silently not
appear. Now conditional on the major not being 0, which is the exception the
comment described.
Correct the attestation claim in install.md. It said every asset 'carries a
SLSA build-provenance attestation'; the SBOM is signed but not separately
attested, since SHA256SUMS covers it. Reworded to state exactly what is
attested.
Also: the justfile's release comment still said stable-only after this branch
taught it -rc.N, and the fenced blocks this PR added lacked language
identifiers.
Not addressed, deliberately: unlabelled fences PREDATING this PR (5 in
install.md, 11 in packaging-and-docker.md). Mechanical and unrelated to the
release path; fixing them here would bury this diff.
Verified: workflows parse; justfile parses; shellcheck clean; docs test green;
DuckDB checksum computed from the pinned v1.1.3 asset.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The release path itself — everything that only runs on a
v*tag. After this,v0.1.0-rc.1becomes possible.Digest promotion (the structural change)
The image is now pushed by digest with no tags, smoked, and only then tagged
via
imagetools create. Previouslybuild-push-actionattachedX.Y.Zandlatestin the same step, so the release tags named a build nothing had everexecuted — while dev
sha-images got a full runtime smoke inci.yml. Therelease was the least-verified image we ship.
It now gets that same deep smoke (Parquet write → DuckDB read-back → resume
no-op) against the exact digest the tags will point at.
imagetools createcopies no bytes; it repoints tags at the smoked digest.
Signing, attestation, and a release that verifies itself
SHA256SUMSand the SBOM are now signed — previously omitted, andSHA256SUMSis the one file whose integrity transitively covers every other asset. Signed
last, deliberately.
(
push-to-registry), and the SBOM. Signatures say "these are the bytes thatwere signed"; attestations say "this workflow, at this commit, built it".
COSIGN_EXPERIMENTALremoved — a no-op since cosign 2.0 (2023).gh release createpublishes and then uploads,so a mid-upload failure left a live half-release and notified watchers. It now
creates a draft, runs the exact commands the docs give users (
sha256sum --check,cosign verify-blob,cosign verify,gh attestation verify), andonly un-drafts if they pass. A signature nobody can verify is worse than no
signature, because it invites trust — so that failure belongs in CI, not in a
user's terminal.
Release candidates
vX.Y.Z-rc.Nis now accepted. It publishes its own immutable container tag only(no
latest, no rollingX.Y) plus a GitHub pre-release; everything else runsidentically, which is the point.
Not the full SemVer pre-release grammar —
-rc.Nis the only form withimplemented publish semantics, and
-alpha/-beta.2+buildwould parse as validwhile meaning nothing here.
Also adds the rolling
X.Ytag for stable releases. No bareXwhile 0.x.Docs
RELEASING.mdgets the RC section, including that an RC is not promoted —its jar reports
X.Y.Z-rc.N, so shipping those bytes asX.Y.Zwould contradictthe tag.
install.mdgets the verification commands with the exact certificateidentity and issuer, not just flag names — an identity a user cannot construct is
a story they won't follow. Note this pins the workflow filename: renaming
release.ymlchanges the identity and invalidates every published instruction.Plus the two first-run traps found while smoke-testing the public image today:
the UID-10001 write failure (recommending
--useroverchmod 777) and why ashort containerised run looks silent.
Verification, and its limits
Done: workflow parses; publish step order confirmed post-parse; tag selection
simulated for rc/0.x/1.x; all twelve pinned action SHAs resolved against the
API (the two new attestation actions are v4 commits, dereferenced from tag
objects — I do not pin SHAs from memory);
verifyReleaseVersionexercised acrossthe rc/stable/mismatch/
-beta.1/-SNAPSHOTmatrix; docs test green.Not done, and cannot be before merge: none of the publish job runs without a
tag. Environment approval, GHCR push under the release token, cosign OIDC,
attestation upload,
imagetools create, andgh release createare all stillunexercised. That is exactly why RC support ships here —
v0.1.0-rc.1is thetest for this PR, and I would expect it to surface something.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
vX.Y.Z-rc.Nformat.latestor rolling tags.Documentation