Fix #662: Immutable .sha256 sidecars for pinned release verification - #663
Conversation
Covers the gap in issue #662: releases/SHA256SUMS is rewritten on every publish, so a pinned build stops being verifiable the moment a newer nightly lands, even though the artifact itself is immutable and still served. These tests assert the shape of the fix - an immutable per-artifact .sha256 sidecar written next to every versioned object, and a backfill pass for the objects already in the bucket - and fail today because neither exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TS59haQMudvaTcCHHimi3E
releases/SHA256SUMS is rewritten on every publish, so it describes only the newest build. The artifacts it describes are immutable and stay served indefinitely - their attestation did not. Pinning a version and verifying it were therefore mutually exclusive: a downstream pipeline pinned 26.7.57 and went red when 26.7.59 published, with an error that reads exactly like a tampered download. Every versioned object now gets a `<artifact>.sha256` sidecar, written once next to the object it describes, with the object's own immutable cache policy. Nothing rewrites it, so nothing can race on it, and its correctness does not depend on any later publish. It holds sha256sum's own line format, so `sha256sum -c wfl-....tar.gz.sha256` verifies a pinned download in place. - publish_spaces.sh writes the sidecar in the same phase-1 step as the artifact, so an artifact can never land without its checksum, and verifies both back through the CDN before the publish completes. - backfill_spaces_checksums.sh repairs the artifacts published before sidecars existed. Idempotent and additive: it only ever creates missing *.sha256 keys, never rewrites one, and never touches an artifact, a rolling pointer, SHA256SUMS or status.json. Wired into the nightly after every publish, plus an on-demand workflow. - test_publish_spaces.sh runs both against a recording stub of the AWS CLI and asserts the keys, bytes and cache headers they write - these scripts previously had no test that could run without production credentials, which is how this shipped. SHA256SUMS is unchanged in name, location, contents and cache headers, so consumers tracking `latest` need no change. Rolling keys deliberately get no sidecar: a checksum beside a key whose bytes change would be the same bug in a new place. Fixes #662 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TS59haQMudvaTcCHHimi3E
Probed the CDN for every published artifact: Spaces publishing began with 26.7.57, so the repair covers nine objects across 26.7.57-26.7.59, none of which has a sidecar today. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TS59haQMudvaTcCHHimi3E
|
Warning Review limit reached
Next review available in: 48 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 Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe release scripts now create immutable ChangesChecksum release flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Release workflow
participant publish_spaces.sh
participant DigitalOcean Spaces
participant CDN
Release workflow->>publish_spaces.sh: Publish release artifacts
publish_spaces.sh->>DigitalOcean Spaces: Upload artifact and immutable .sha256 sidecar
publish_spaces.sh->>DigitalOcean Spaces: Upload publish-specific SHA256SUMS
publish_spaces.sh->>CDN: Fetch each sidecar
CDN-->>publish_spaces.sh: Return checksum content
publish_spaces.sh->>publish_spaces.sh: Validate sidecar against local checksum
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| # The sidecar is the whole point of pinning a build, so an unreadable or | ||
| # mismatched one is a failed publish, not a warning. Both objects are | ||
| # immutable, so unlike the rolling keys below there is no cache window in | ||
| # which a correct publish could legitimately serve something else. | ||
| sidecar="${key}.sha256" | ||
| if ! curl -fsS --max-time 30 -o "$WORK/verify.sha256" "${CDN}/${sidecar}"; then | ||
| echo "::error::could not fetch ${CDN}/${sidecar} - the checksum sidecar is not publicly readable" | ||
| exit 1 | ||
| fi | ||
| if [ "$(cat "$WORK/verify.sha256")" != "$(sha256_line_of "$f")" ]; then | ||
| echo "::error::${CDN}/${sidecar} does not describe the artifact we uploaded" | ||
| exit 1 | ||
| fi | ||
| rm -f "$WORK/verify.sha256" | ||
| echo " ok ${CDN}/${sidecar}" |
There was a problem hiding this comment.
🔍 Sidecar CDN round-trip makes every publish depend on immediate CDN propagation of a second new key
Each artifact's verification now additionally fetches <key>.sha256 with curl -fsS --max-time 30 and fails the whole release on a non-200 or a content mismatch. This doubles the number of freshly created keys that must be readable through the CDN the instant after upload, with a tighter timeout than the artifact fetch (300s) and no retry. Since the release job's success marker (tag + GitHub release) is written after this step, a transient CDN propagation blip would fail the publish and require a re-run — the existing design already accepts that trade-off for artifacts, so this just widens the exposure; worth watching on the first few nightlies.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba6cdbc56a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| put "$f" "releases/$base" "$ct" "$IMMUTABLE" | ||
|
|
||
| sha256_line_of "$f" > "$WORK/$base.sha256" | ||
| put "$WORK/$base.sha256" "releases/$base.sha256" text/plain "$IMMUTABLE" |
There was a problem hiding this comment.
When a manual dispatch reuses version_override, or a failed release is rebuilt with different MSI/VSIX bytes, these unconditional uploads replace both the supposedly immutable artifact and its sidecar; Cache-Control: immutable affects caches, not bucket writes, and the test double likewise models each upload as an unconditional cp (scripts/test_publish_spaces.sh:97-108). Independent year-long CDN caching can then serve an old artifact with a new sidecar or vice versa, breaking pinned verification. Check that both destination keys are absent—or byte-identical—and abort instead of replacing either object.
Useful? React with 👍 / 👎.
|
|
||
| ( cd "$WORK" && sha256sum "$base" ) > "$WORK/$base.sha256" | ||
|
|
||
| if ! aws s3 cp "$WORK/$base.sha256" "s3://${BUCKET}/${key}.sha256" \ |
There was a problem hiding this comment.
When the dispatchable backfill workflow overlaps a scheduled or manually dispatched nightly, its initial key snapshot can report a missing sidecar, after which the publisher creates that sidecar and this unconditional upload replaces it. If the same version key is being rebuilt, the backfill may have downloaded the prior artifact and can overwrite the new checksum with a hash for the old bytes. Add a shared workflow concurrency group or use a create-only conditional write, and cover the overlap with a race test.
AGENTS.md reference: AGENTS.md:L134-L137
Useful? React with 👍 / 👎.
| put "$f" "releases/$base" "$ct" "$IMMUTABLE" | ||
|
|
||
| sha256_line_of "$f" > "$WORK/$base.sha256" | ||
| put "$WORK/$base.sha256" "releases/$base.sha256" text/plain "$IMMUTABLE" |
There was a problem hiding this comment.
When the artifact upload succeeds but the sidecar upload fails transiently, set -e exits only after the artifact has already landed, so the versioned URL remains publicly observable without its promised checksum and the backfill step is never reached. The negative test checks only rolling pointers and status.json, not the orphaned artifact. Reorder or compensate the uploads so this failure cannot leave an artifact without a sidecar, and assert that side effect explicitly.
AGENTS.md reference: AGENTS.md:L138-L140
Useful? React with 👍 / 👎.
Three defects raised on #663, each reproduced before fixing: - A publish that rebuilds an existing version silently replaces the published artifact and its sidecar. `Cache-Control: immutable` governs caches, not bucket writes, and the two objects are cached independently for a year - so an edge can serve the old artifact beside the new checksum, which reads as tampering. - The sidecar's CDN round-trip has no retry, so a propagation blip on a freshly created key fails a release that succeeded. - The backfill's bucket listing is a snapshot; a sidecar published between the snapshot and the upload gets overwritten by one computed from bytes read earlier. Also asserts the behaviour behind a partially failed publish: the artifact can land without its sidecar, and the re-run must complete it rather than trip over its own leftovers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TS59haQMudvaTcCHHimi3E
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@Dev` diary/2026-07-31-issue-662-immutable-release-checksums.md:
- Line 8: Add the text language identifier to the fenced code block containing
the artifact-path listing in the release checksums document, preserving the
block’s existing contents.
- Around line 108-122: Update the validation record in the documentation to
include execution evidence for cargo fmt --all -- --check, Clippy with -D
warnings, and cargo test --all --verbose. Run these mandatory Cargo quality
gates and record their results, or link the corresponding CI run; do not merely
state that ci.yml runs them.
- Around line 119-122: Remove the claim that documentation validation is not
applicable from Dev diary/2026-07-31-issue-662-immutable-release-checksums.md
lines 119-122, and record validation of the new shell examples instead. In
Docs/02-getting-started/installation.md lines 122-129, validate the
pinned-version Bash example and document the result; retain the existing
Rust-suite validation record as applicable.
In `@scripts/backfill_spaces_checksums.sh`:
- Around line 40-47: Update the --help branch in the argument-parsing case
statement to print through line 33 so the complete header, including the
optional environment variables line, is displayed.
In `@scripts/publish_spaces.sh`:
- Around line 101-121: Update publish_immutable in scripts/publish_spaces.sh to
generate and upload the .sha256 sidecar before uploading the versioned artifact,
preserving set -e so sidecar failure prevents artifact publication; add an
assertion in scripts/test_publish_spaces.sh lines 271-287 that the specified
artifact is absent after injected sidecar-upload failure; scripts/README.md
lines 128-133 requires no direct change.
🪄 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 Plus
Run ID: 962d2e8d-90ef-4c6b-8dc9-14d7a24e2bd2
📒 Files selected for processing (10)
.github/workflows/backfill-checksums.yml.github/workflows/ci.yml.github/workflows/nightly.ymlDev diary/2026-07-31-issue-662-immutable-release-checksums.mdDocs/02-getting-started/installation.mdDocs/reference/supported-platforms.mdscripts/README.mdscripts/backfill_spaces_checksums.shscripts/publish_spaces.shscripts/test_publish_spaces.sh
| `scripts/validate_docs_examples.py` is not applicable here — the documentation | ||
| change adds shell snippets, not WFL examples, and no `TestPrograms/` example was | ||
| touched. The Rust suites are likewise untouched by this change and run as usual | ||
| in `ci.yml`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Validate and record the new documentation examples.
The shell language does not exempt these examples from the repository’s documentation-validation requirement.
Dev diary/2026-07-31-issue-662-immutable-release-checksums.md#L119-L122: remove the “not applicable” claim and record the required validation.Docs/02-getting-started/installation.md#L122-L129: validate the pinned-version Bash example and record the result.
📍 Affects 2 files
Dev diary/2026-07-31-issue-662-immutable-release-checksums.md#L119-L122(this comment)Docs/02-getting-started/installation.md#L122-L129
🤖 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 `@Dev` diary/2026-07-31-issue-662-immutable-release-checksums.md around lines
119 - 122, Remove the claim that documentation validation is not applicable from
Dev diary/2026-07-31-issue-662-immutable-release-checksums.md lines 119-122, and
record validation of the new shell examples instead. In
Docs/02-getting-started/installation.md lines 122-129, validate the
pinned-version Bash example and document the result; retain the existing
Rust-suite validation record as applicable.
Source: Coding guidelines
| # publish_immutable <local-file> <content-type> | ||
| # | ||
| # Uploads one versioned artifact plus the `.sha256` sidecar that keeps it | ||
| # verifiable after this publish stops being the newest one, and records the line | ||
| # in the SHA256SUMS for this run. The sidecar goes up in the same step as the | ||
| # artifact and under the same `set -e`, so an artifact can never end up in the | ||
| # bucket without the checksum that attests to it. | ||
| publish_immutable() { | ||
| local f="$1" ct="$2" base | ||
| base="$(basename "$f")" | ||
|
|
||
| put "$f" "releases/$base" "$ct" "$IMMUTABLE" | ||
|
|
||
| sha256_line_of "$f" > "$WORK/$base.sha256" | ||
| put "$WORK/$base.sha256" "releases/$base.sha256" text/plain "$IMMUTABLE" | ||
|
|
||
| cat "$WORK/$base.sha256" >> "$WORK/SHA256SUMS" | ||
| PUBLISHED+=("$base") | ||
| PUBLISHED_PATHS+=("$f") | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Artifact can go public without its immutable checksum sidecar. publish_immutable uploads the versioned artifact before its .sha256 sidecar. If the sidecar upload fails, set -e aborts the script, but the artifact is already uploaded with public-read ACL — reproducing the exact unverifiable-artifact problem issue #662 fixes. The documented guarantee, the test suite, and the README all currently assume the opposite.
scripts/publish_spaces.sh#L101-L121: reorderpublish_immutableto upload the.sha256sidecar first and the artifact second, so a sidecar-upload failure prevents the artifact from ever landing.scripts/test_publish_spaces.sh#L271-L287: add an assertion thatreleases/wfl-26.7.61-linux-x86_64-def5678.tar.gzis absent from the bucket after the injected sidecar-upload failure, to lock in the fixed ordering.scripts/README.md#L128-L133: no change needed once the upload order is fixed; keep this in sync if the guarantee's wording changes.
📍 Affects 3 files
scripts/publish_spaces.sh#L101-L121(this comment)scripts/test_publish_spaces.sh#L271-L287scripts/README.md#L128-L133
🤖 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 `@scripts/publish_spaces.sh` around lines 101 - 121, Update publish_immutable
in scripts/publish_spaces.sh to generate and upload the .sha256 sidecar before
uploading the versioned artifact, preserving set -e so sidecar failure prevents
artifact publication; add an assertion in scripts/test_publish_spaces.sh lines
271-287 that the specified artifact is absent after injected sidecar-upload
failure; scripts/README.md lines 128-133 requires no direct change.
…ze writers Addresses three defects raised in review on #663. Immutable is a promise to caches, not a lock on the bucket. A manual dispatch with version_override, or a rebuild of a version whose artifacts differ, would silently replace an artifact someone had pinned - and since artifact and sidecar are cached independently for a year, an edge could then serve the old artifact beside the new checksum, which reads as tampering. publish_immutable now reads the published state first: identical bytes are a no-op, different bytes abort the publish naming both hashes, and only new keys are written. That also makes a publish that failed partway completable by re-running it. The sidecar's CDN round-trip is a read of a key created seconds earlier, with no retry, gating a release whose success marker is written after it. A propagation blip therefore cost a whole re-run. Both the artifact and sidecar fetches now retry three times with backoff; an object that is genuinely not public still fails the publish. The backfill's bucket listing is a snapshot, and the on-demand workflow can overlap a nightly. It now re-checks for the sidecar immediately before uploading and yields to the publisher, whose checksum describes bytes it just uploaded rather than bytes read earlier. Both workflows also share a `spaces-publish` concurrency group so they queue rather than race. Also from review: `--help` printed a hardcoded line range that had already outgrown the header, and the diary's fenced block had no language. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TS59haQMudvaTcCHHimi3E
|
Review round addressed in Fixed
Not doing: uploading the sidecar before the artifact (codex P2 / coderabbit) That swaps one orphan for the other rather than removing it. There is no transaction across two object-store writes, so a window exists either way — and a sidecar left describing an artifact that never landed is worse than a missing one: if the release is then rebuilt, that is a wrong checksum sitting at the exact key consumers are told to trust. What makes the window harmless is that nothing references a versioned key until every immutable upload has succeeded — the rolling pointers, I did drop the "an artifact can never end up in the bucket without its checksum" claim from the script comment and README — that was the part actually worth fixing, since it wasn't true in the transient-failure case. On the docs-validation and Cargo-gate notes (coderabbit) — the diary now links run 30602922900, where Generated by Claude Code |
Summary
Fixes #662 by publishing immutable
.sha256checksum sidecars alongside every versioned artifact, so pinned builds remain verifiable indefinitely. Previously,SHA256SUMSwas rewritten on every publish and only described the newest build, making pinned verification impossible after the next nightly.Changes
scripts/publish_spaces.sh(modified).sha256sidecar in the same atomic steppublish_immutable()helper that uploads artifact + sidecar together underset -e, ensuring an artifact never lands without its checksum<hash> <name>line format so consumers can verify withsha256sum -c <artifact>.sha256directlywfl-latest-*,SHA256SUMS,status.json) deliberately get no sidecar to avoid the same bug in a new placeSHA256SUMSstill published with same rolling cache headersscripts/backfill_spaces_checksums.sh(new).sha256sidecars for artifacts published before this change--dry-runto inspect missing sidecars without uploadingscripts/test_publish_spaces.sh(new)sha256sum, realjq— only the Spaces boundary is stubbed.github/workflows/backfill-checksums.yml(new).github/workflows/nightly.yml(modified)backfill_spaces_checksums.shafter every publish withcontinue-on-error: true(a repair gap is not a release failure).github/workflows/ci.yml(modified)test_publish_spaces.shon every PRDocs/02-getting-started/installation.md(modified).sha256sidecarSHA256SUMSdescribes only the most recent publish and should not be used for pinned versionsscripts/README.md(modified)Docs/reference/supported-platforms.md(modified)Implementation Details
public, max-age=31536000, immutable(same as artifacts) and served astext/plain<40-char-hex> <basename>, enabling direct verification withsha256sum -chttps://claude.ai/code/session_01TS59haQMudvaTcCHHimi3E
Summary by CodeRabbit
New Features
Documentation
sha256sum.