ci: drop the SBOM release-asset job, keep the attestation (LAB-983) - #64
Merged
Conversation
The Generate SBOM job had no permissions: block, so it inherited the repo default of read-only and every release: published run died on "Resource not accessible by integration" at the asset upload. cachekit-core-v0.4.0 shipped with zero release assets. Adding contents: write to that job would have been the wrong fix: it runs cargo sbom, which compiles and executes third-party build scripts and proc-macros, and a release-write token must not share a runner with untrusted code. Split instead, matching cachekit-py's sbom job — generation stays credential-less at contents: read, the SBOM crosses as a workflow artifact, and a separate sbom-upload job holds the only write token while running nothing but first-party actions. Also in this path: - Replace actions/upload-release-asset (archived by GitHub 2021-03-03, and running Node 20 under a forced Node 24 runtime) with actions/github-script. gh is not an option: the cachekit ARC runner has no gh CLI (LAB-899). - Delete the cargo install cargo-sbom, rust-toolchain and cache steps. The runner image bakes Rust stable and cargo-sbom, the plain install exits 101 against the runner's persistent CARGO_HOME, and this job compiles nothing. - Guard against an empty SBOM. An empty-but-valid CycloneDX document looks like a verified supply chain and isn't. The guard also fails closed on an unparseable file, which the naive form does not: a failed command substitution leaves count empty, [ "" -lt 1 ] errors instead of comparing, and the step exits 0. - Switch to cyclone_dx_json_1_6 so the asset matches the format release.yml attests, and so the guard can count .components. - Add a release_tag dispatch input. Without it this path could only ever be exercised by cutting a release, which is how v0.4.0 stayed broken. It also backfills v0.4.0. An SBOM dispatch no longer drags Kani onto the ARC pool. - Check out the released tag rather than main: an SBOM describing the wrong tree is worse than a missing one.
Three findings from the panel: - The workflow concurrency group did not distinguish backfill dispatches, so a second dispatch (or a scheduled run on the same ref) could cancel one mid-flight. Because sbom-upload deletes a pre-existing asset before re-uploading, a cancel landing in that window would leave a release with its SBOM deleted and never replaced — worse than the missing asset this fixes, and hidden behind a "cancelled" status. release_tag is now part of the key. Uses github.event.inputs rather than inputs: the group is evaluated on every event and the inputs context is only documented for dispatch/reusable calls. - The target tag was not checked against a real release until the upload job, so `cargo sbom` would compile and execute third-party build scripts for any ref a dispatch named, and a typo cost a full generation run before failing. Resolution moved ahead of the checkout, while the workspace is still empty. - The comment justifying the deleted install steps cited release.yml's persistent-CARGO_HOME rationale, which is itself stale there — that job moved to ubuntu-latest. Reworded to stand on the runner image instead. release.yml is out of scope for this ticket, so its stale comment is left alone. Resolving the release id in the generate job and passing it to the upload job also drops a redundant getReleaseByTag and closes the window where a repointed tag could put this SBOM on a different release than it was generated from. Rejected: extending the backfill exclusion to deep-fuzz. run_deep_fuzz is already an explicit opt-in, and silently ignoring an input the operator ticked is worse than honouring it. The kani exclusion is not the same case — kani runs on any dispatch with no opt-in at all.
Live verification of the split found the wall behind the permissions bug: immutable releases are enabled on this repository, and GitHub applies immutability at publish. `release: published` therefore fires only ever against a sealed release, and the upload returns "Cannot upload assets to an immutable release". Verified in run 30359945625 - Generate SBOM green in 19s, Attach SBOM to Release red on that error. Every release in this repo reports immutable: true and none has ever carried an asset; cachekit-rs is the same, cachekit-py and cachekit-ts are not. That is a policy decision, not a retry: publishing an SBOM asset needs either immutable releases turned off for this repo, or releases created as drafts with assets attached before publish (which lives in release.yml, out of scope here). So the target release is now checked before generation runs, and the job fails with that choice spelled out instead of burning a generation run and dying in 48KB of octokit output. SECURITY.md documents what the repo actually ships: the attested SBOM, which is real and verifiable today, plus an explicit statement that releases carry no SBOM asset and why. The earlier wording in this branch promised an asset that cannot exist - an unverifiable claim in a security document is a trust bug, and the expert panel flagged it before this run confirmed it.
Contributor
WalkthroughThe release security workflow no longer triggers on published releases or uploads SBOM assets. ChangesRelease SBOM handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Immutable releases stay enabled on this repository, so the SBOM release asset cannot exist: GitHub seals a release's assets at publish, and the upload returns "Cannot upload assets to an immutable release". Verified live in run 30359945625. Every release since v0.1.0 has zero assets - this job has never once succeeded. So the job goes, rather than being rewritten to fail more elegantly. The SBOM is already generated by cargo-sbom and attested against the packaged crate via actions/attest-sbom in release.yml. That attestation is the cryptographically verifiable artifact; a plain release asset would have been an unsigned weaker copy of the same document, which is not worth trading tamper-resistance for. The `release:` trigger goes with it. `sbom` was the only job gated on it, so leaving it would produce an empty run on every release. The reason both were removed is recorded where the trigger used to be, including what re-adding the asset would actually require (draft release, attach, then publish), so the next person does not rediscover the wall. This also removes the archived actions/upload-release-asset (unmaintained since 2021-03-03) from the repository. SECURITY.md documents what ships: the attested SBOM and how to verify it, plus an explicit statement that releases carry no SBOM asset and why. A security document that promises a downloadable SBOM nobody can download is a trust bug.
The predicate type was wrong. actions/attest-sbom records CycloneDX as `https://cyclonedx.org/bom` with no version suffix - the spec version lives in the document's own specVersion field. Only SPDX versions its predicate type. The documented command therefore 404'd for every consumer who ran it: HTTP 404: .../attestations/sha256:6aba15...?predicate_type=...%2Fbom%2Fv1.6 Verified end to end against the real published artifact rather than from reasoning: downloaded cachekit-core-0.4.0.crate from static.crates.io, confirmed the recorded attestations for its sha256 are `https://cyclonedx.org/bom` and `https://slsa.dev/provenance/v1` with subject cachekit-core-0.4.0.crate, and confirmed the corrected `gh attestation verify` exits 0 with a signature from release.yml@refs/heads/main. The wrong form errors, the corrected form passes. The example is now concrete and copy-pasteable - it names the actual crate and includes the download - because a placeholder like <crate-file> left the reader guessing which artifact to verify. Also notes that provenance is attested separately under the SLSA predicate. The wrong string was copied from attestation-check.yml, which carries the same bug in both its file-based and list-based SBOM checks. That file is LAB-984's fail-open territory and out of scope here; noted on that ticket.
27Bslash6
changed the title
27Bslash6
marked this pull request as ready for review
August 5, 2026 03:54
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
![kodus-27b[bot]](https://avatars.githubusercontent.com/u/223823690?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@SECURITY.md`:
- Around line 86-89: Replace the unsupported gh attestation list invocation in
the no-asset attestation lookup with GET
/repos/{owner}/{repo}/attestations/{subject_digest} via gh api, passing the
subject digest and CycloneDX/provenance predicate filters and preserving real
command failures. Update the asset-path CycloneDX predicate to
https://cyclonedx.org/bom.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 77b5edfa-c4b0-4f85-8214-d141a6cfd65e
📒 Files selected for processing (2)
.github/workflows/security.ymlSECURITY.md
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.
Summary
Removes the SBOM release-asset upload job from the security workflow, which was fundamentally broken due to immutable releases being enabled on this repository.
Changes
.github/workflows/security.ymlsbomjob that generated an SBOM and attempted to upload it as a release asset. This job always failed with "Cannot upload assets to an immutable release" because immutable releases seal a release's assets at publish time — every release since v0.1.0 has had zero assets.release: [published]trigger, which existed solely for that now-deleted job. A detailed comment explains why it was dropped and what would be required to re-enable asset uploads (creating the release as a draft, attaching, then publishing).SECURITY.mdcargo-sbomand attested viaactions/attest-sbomduring publish.gh attestation verify, including a note about the predicate type (https://cyclonedx.org/bom) carrying no version suffix.Purpose
The SBOM remains generated and attested in
release.yml— that attestation is the verifiable, signed artifact. The removed release asset would have been an unsigned, weaker copy that could never be uploaded anyway. This change eliminates empty/failing workflow runs and documents the correct way to obtain and verify the SBOM.