Repository navigation
ci: publish SBOMs per release and verify published provenance - #411
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a GitHub Actions workflow that generates SPDX and CycloneDX SBOMs, uploads them to published releases in a separate job, and verifies npm package signatures and provenance attestations with retries. ChangesSupply chain evidence
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubRelease
participant GitHubActions
participant AnchoreSBOMAction
GitHubRelease->>GitHubActions: trigger published release workflow
GitHubActions->>AnchoreSBOMAction: generate SPDX and CycloneDX SBOM artifacts
GitHubActions->>GitHubRelease: upload SBOM artifacts with gh release upload
sequenceDiagram
participant GitHubActions
participant NpmCLI
participant NpmRegistry
GitHubActions->>NpmCLI: install published packages without scripts
NpmCLI->>NpmRegistry: query package signatures and attestations
GitHubActions->>NpmCLI: run npm audit signatures
GitHubActions->>NpmRegistry: query JSON provenance predicates
GitHubActions->>GitHubActions: retry and fail when no string predicate is found
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Preview DeploymentPreview URL: https://7f3d4871.bestax.pages.dev |
AI triage — issues this PR may resolve
If this PR resolves one of these, add the line below to the PR description |
Two supply-chain evidence artifacts we previously only claimed: - SPDX + CycloneDX SBOMs generated with Syft, uploaded as artifacts and attached to each GitHub release as durable, citable assets. - npm audit signatures run against the real published tarballs, closing the loop on the verification SECURITY.md tells consumers to perform. Deliberately decoupled from ci.yml's publish job so nothing here can fail a release. Refs #404
checkout, setup-node and pnpm/action-setup were pinned from a stale feature branch. origin/main uses checkout v7 (3d3c42e5), setup-node v7 (82076278) and pnpm/action-setup v6.0.9 (0ebf4713).
7904656 to
f642579
Compare
Preview DeploymentPreview URL: https://b454ef49.bestax.pages.dev |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Deep review — 0 blocking · 2 advisory
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🔵 Advisory | Security | npm audit signatures fails only on invalid signatures/attestations — it stays green when our own packages publish with missing provenance, so the job can't actually catch a future release that drops attestations |
.github/workflows/supply-chain.yml:107 |
| 2 | 🔵 Advisory | Robustness | The SBOM is generated from the full pnpm install dev workspace and path: ., so it enumerates the entire monorepo dev toolchain (storybook, playwright, docusaurus, jest, turbo…) rather than the published-package dependency closure |
.github/workflows/supply-chain.yml:48 |
Overall: The change is sound and well-scoped: it's decoupled from the release path (nothing here can fail a publish), all three actions are SHA-pinned in line with the rest of the repo, permissions are least-privilege (contents: write confined to the sbom job, verify-provenance inheriting read), and the release-upload step correctly passes tag_name through an env var rather than inline ${{ }}, so a hostile tag can't inject shell. I found no blocking defects. The riskiest part is not a bug but a strength-of-evidence gap: both artifacts promise slightly more than they enforce (see advisories). The human should focus on advisory #1 — decide whether "Verify published provenance" needs to actually assert attestation presence, or whether registry-signature verification is the intended (weaker) guarantee.
Residual risk:
- Silent-pass on dropped provenance:
npm audit signaturesverifies attestations that exist (the PR's own run shows "4 packages have verified attestations") but does not fail on their absence. If a future release losespublishConfig.provenance: trueor OIDC misconfigures, the packages keep valid registry signatures and this job stays green — provenance regressions go uncaught. Not blocking; the registry-signature check is still meaningful. - Transitive
workspace:^reintroduction: confirmed refuted —create-bestax/package.jsondepends only onchalk/commander/figures/fs-extra/prompts, and does not depend on@allxsmith/bestax-bulmaorbestax-migrate, so the excluded broken package can't sneak back in throughnpm install create-bestax. - Release-event propagation:
verify-provenanceinstalls thelatestdist-tag rather than the just-released version; semantic-release publishes to npm before creating the GitHub release, solatestshould already be current, but a registry-propagation lag would audit the priorlatest— harmless for a signature audit, and re-runnable viaworkflow_dispatch/weekly schedule.
🏄 Chill, clean set of evidence waves here, dude — pinned, decoupled, no injection undertow, totally safe to paddle out and merge. Just know the provenance check is more of a longboard cruiser than a big-wave charger: it'll spot a busted signature but won't yell if the attestation quietly ghosts. Ride it, but don't mistake green for a guarantee. 🌊
npm audit signatures only fails on invalid signatures; it stays green when provenance is absent entirely, so it could never catch a release that lost publishConfig.provenance or had OIDC misconfigured. Adds an explicit presence assertion (verified against a positive and negative control). Also documents the SBOM's scope: it is the repository/dev-toolchain SBOM, not the published-package closure. Addresses both advisories from the deep review on #411.
|
Both advisories addressed in 3f123d5 — #1 was a real weakness in the job's core promise. Advisory 1 (provenance absence vs. invalidity) — fixedCorrect and the sharpest finding here: a step named "Verify published provenance" that stays green when provenance disappears is the same failure class as #405 — claiming a gate that isn't enforced. Added an explicit presence assertion: predicate=$(npm view "$pkg" dist.attestations.provenance.predicateType)
[ -z "$predicate" ] && fail=1Verified against both controls before committing:
Advisory 2 (SBOM scope) — documented, not changedAlso correct: I've scoped the claim rather than the artifact. A repository SBOM is legitimately useful — it's the audit trail for what builds our releases — the error would be letting it be read as the consumer closure. Added an explicit On the residual-risk notes
|
Preview DeploymentPreview URL: https://39f75eb0.bestax.pages.dev |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 @.github/workflows/supply-chain.yml:
- Around line 54-60: Disable implicit release uploads by adding
upload-release-assets: false to both SBOM action configurations in
.github/workflows/supply-chain.yml at lines 54-60 and 64-70; leave the separate
gh release upload step 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5bc755a7-f7c6-4630-9ea0-4c099d939a34
📒 Files selected for processing (1)
.github/workflows/supply-chain.yml
anchore/sbom-action defaults upload-release-assets to true, so on a release event each of the two format steps would attach its own asset alongside the explicit gh release upload — three uploads with two naming conventions. Verified the default against action.yml at the pinned SHA. Addresses the CodeRabbit finding on #411.
|
Valid finding — fixed in f498320. Verified the claim against Confirmed. On a Worth noting this was latent — nothing on a PR branch can fire a |
Preview DeploymentPreview URL: https://3790f6c1.bestax.pages.dev |
|
@claude deep review again |
|
Claude finished @allxsmith's task in 2m 20s —— View job Deep review again
Deep review — 0 blocking · 0 new advisoriesRe-reviewed the file at HEAD ( Both prior advisories and the CodeRabbit actionable comment are correctly resolved:
Fresh pass over the rest of the file found nothing new:
Nothing here can fail a release (triggers are Note: I don't have working Bash tool access in this environment run (sandbox approval was denied for both Verdict: No blocking issues. This is ready to merge from a review standpoint; the only genuinely untestable path (SBOM attachment on a real |
There was a problem hiding this comment.
Pull request overview
Adds a new GitHub Actions workflow to generate auditable supply-chain evidence for the Bestax monorepo and to continuously verify the provenance/signature status of the published npm packages.
Changes:
- Introduces a
sbomjob that generates SPDX and CycloneDX SBOM artifacts, and (onrelease: published) uploads them as GitHub Release assets. - Introduces a
verify-provenancejob that installs published tarballs with npm and runsnpm audit signatures, plus an explicit provenance-presence assertion. - Runs on
release: published, weekly schedule, and manual dispatch (intentionally decoupled from the release publish pipeline).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…heck Copilot review on #411: - The sbom job ran pnpm install and third-party actions while holding contents:write on every trigger. Split generation (read-only) from the release upload (contents:write, gated on the release event), so a compromised dependency never inherits a write-scoped token. - The provenance assertion accepted any non-empty npm view output. Now uses --json with a string type guard, so null/undefined/non-string all fail closed. Verified against positive, negative and 404 cases.
|
Both Copilot findings were valid and are fixed in 82c062b.
|
| job | permissions | when |
|---|---|---|
sbom |
contents: read |
every trigger |
attach-sbom |
contents: write |
needs: sbom, if: github.event_name == 'release' |
verify-provenance |
inherits read | every trigger |
The upload job installs nothing — it only downloads the artifacts and calls gh release upload.
npm view non-empty ≠ present — fixed, with a caveat on the mechanism
Adopted. Now --json with a type guard:
predicate=$(npm view "$pkg" dist.attestations.provenance.predicateType --json 2>/dev/null \
| jq -r 'select(type=="string")' 2>/dev/null || true)Being straight about what I could and couldn't confirm: I could not reproduce the specific null/undefined output. Bare npm view printed empty (length 0) for every package I tried without attestations — fs-extra, lodash, chalk — so the original -z test wasn't actually producing a false positive.
That said the recommendation is right regardless: the type guard makes presence explicit rather than inferred, and it costs nothing. Verified across all three cases:
@allxsmith/bestax-bulma -> [https://slsa.dev/provenance/v1] ok
create-bestax -> [https://slsa.dev/provenance/v1] ok
fs-extra -> [] FAIL
lodash -> [] FAIL
nonexistent-pkg-xyz-9987 -> [] FAIL
The 404 case matters most: any failure — network, missing package, missing dist-tag — yields empty and fails closed, which is the correct default for a security assertion.
Also caught in the process
Writing the attach-sbom job I initially used a download-artifact SHA from memory. It was fabricated — gh api returned "No commit found for SHA." The real pin on main is 3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8. I've now verified every one of the five action pins in this file resolves to a real commit via the API, rather than trusting recall.
Preview DeploymentPreview URL: https://ba6d0ec4.bestax.pages.dev |
|
deep-review: RE-REVIEW — verify prior review feedback was properly addressed. Head is 82c062b; no reviewer has examined this commit. Three reviewers raised five findings and the author claims all are fixed. Verify each INDEPENDENTLY against the code, do not trust the author’s reply comments:
CRITICAL — the author made TWO action-pin errors on these PRs: first citing a pin sourced from a stale branch, then writing a Also check: does |
There was a problem hiding this comment.
Deep review — 0 blocking · 2 advisory
Re-review of head 82c062b. I verified each of the five prior findings independently against the code and the live GitHub/npm/action sources, without trusting the author's replies, and re-resolved every action pin through the GitHub API.
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🔵 Advisory | Robustness | Provenance assertion fails closed on any transient npm view/registry error — correct for a security gate, but the weekly schedule run can go red on network flake, not just on a real dropped attestation. |
.github/workflows/supply-chain.yml:145 |
| 2 | 🔵 Advisory | Security | SBOM is the dev-monorepo closure, not the published-package closure — honestly documented in-file and the artifact name is generic, so no mislabelling. On the record as an accepted scope limit. | .github/workflows/supply-chain.yml:50 |
Verification of the five prior findings
1. npm audit signatures passes when provenance is absent → explicit presence assertion. Fixed and fails closed. npm view … predicateType --json | jq -r 'select(type=="string")' then [ -z "$predicate" ]:
- Absent field →
npm viewemits empty/null/undefined;jqtype-guard yields empty →fail=1. - Network / 404 / missing dist-tag → empty stdout (stderr suppressed),
|| truekeeps the assignment empty →fail=1. - Present → JSON string passes
type=="string"→ok. GH Actions default shell isbash -eo pipefail;|| truecorrectly absorbs the pipe failure. Confirmed both packages setpublishConfig.provenance: true(bulma-ui/package.json:205,create-bestax/package.json:77), so asserting on both is right.
2. SBOM covers dev monorepo, not published closure. Documented, not silently mislabelled — see advisory #2. Comment is accurate.
3. anchore/sbom-action defaults upload-release-assets: true. Fixed. Fetched action.yml at the pinned SHA e22c389: upload-release-assets default: "true" confirmed. Both generate steps set it false. The single deterministic uploader is attach-sbom; no duplicate path remains.
4. contents: write held on non-release runs. Fixed. Top-level and sbom are contents: read; verify-provenance inherits read. Write lives only on attach-sbom, gated if: github.event_name == 'release' — the job (and its token) never materialises on schedule/workflow_dispatch. needs: sbom still lets it pull the workflow artifacts.
5. npm view non-empty treated as present. Fixed via --json + jq 'select(type=="string")' (same code as #1).
Action-pin audit (every pin re-resolved via GitHub API)
| Action | Pinned SHA | Real commit? | Tag resolves | Matches main? |
|---|---|---|---|---|
actions/checkout |
3d3c42e |
yes | v7 → 3d3c42e |
yes |
pnpm/action-setup |
0ebf471 |
yes | v6.0.9 (annotated) → 0ebf471 |
yes |
actions/setup-node |
8207627 |
yes | v7 → 8207627 |
yes |
actions/download-artifact |
3e5f45b |
yes | v8 → 3e5f45b |
yes |
anchore/sbom-action |
e22c389 |
yes | v0.24.0 → e22c389 |
n/a (new) |
Every pin is a real commit whose version comment resolves correctly (annotated pnpm and lightweight sbom-action tags both dereferenced), and matches main. The two prior pin errors are corrected.
attach-sbom artifact retrieval. Correct. In SyftGithubAction.ts@e22c389, an explicit artifact-name becomes both the workflow-artifact name and the internal filename. So the two artifacts are bestax-sbom.spdx.json / bestax-sbom.cdx.json, each containing an identically-named file. download-artifact pattern: bestax-sbom.* + merge-multiple: true lands both in the CWD, exactly the two names gh release upload references. No silent mismatch that would surface only on a real release.
Overall: The change is sound and every prior finding is genuinely resolved — I confirmed each against the live source, not the author's replies. The riskiest surface was the action pins given the two earlier pin errors; all five now resolve to real commits matching their tags and main, and the attach-sbom artifact names line up end-to-end. A human need only sanity-check the one thing no CI here can exercise: the actual release: published upload path, which by design only runs on a real release.
Residual risk:
- False-pass on a dropped attestation — refuted: absent/
null/non-string all collapse to empty via thejqtype guard →fail=1; only a JSON string passes. - Release upload fails on a real tag due to name mismatch — refuted: artifact names, internal filenames, download pattern, and
gh release uploadargs all agree onbestax-sbom.spdx.json/bestax-sbom.cdx.json. - Write token leaking to dependency/action code — refuted: write is confined to
attach-sbom, which installs nothing and runs only the artifact download, and never instantiates outside areleaseevent. - Minor: if
npm viewever returned the predicate as a single-element JSON array (bare-spec aggregation) the type guard would false-fail; latest-tag resolution returns a scalar, so not expected — the weekly schedule would surface it loudly if it ever happened.
🏄 Paddled out on every pin and every pipe, dude — the whole set holds, the write token is locked in the release-only barrel, and the SBOMs surf straight onto the release with matching names. Clean wave, no wipeouts. Good to go.
…station The assertion fails closed, which is right for a security gate, but on the weekly schedule a registry blip produced the same red as provenance genuinely disappearing. A gate that cries wolf gets ignored, so retry three times with backoff before believing the attestation is gone. Also handles npm returning the predicate as a single-element array. Addresses advisory 1 from the deep re-review of 82c062b.
Preview DeploymentPreview URL: https://d1018982.bestax.pages.dev |
|
Advisory 1 is fixed in 32f2f89. Advisory 2 I'm deliberately not fixing here — filed as #424. Advisory 1 (fails closed on transient registry error) — fixedRight, and worth acting on: a weekly gate that goes red on a network blip is one people learn to ignore, which defeats the point. The lookup now retries three times with backoff before concluding provenance is gone, so flake and a genuinely dropped attestation no longer produce the same red. Also handled the residual-risk note about npm returning the predicate as a single-element array — the guard is now Verified by extracting the step's Negative cases still fail closed — Advisory 2 (SBOM is the dev closure, not the consumer closure) — deferred to #424Fixable, and worth fixing: I'm not bolting it on here because it changes the artifact set and the job's purpose, and there are real design questions to settle first (per-package vs. combined; Note for anyone reading the review trailThe re-review reported "PR is already merged" for #406/#407 — correct, both merged at 00:01Z. That is also the real reason the re-triggered deep reviews on those two skipped: |
|
🎉 This PR is included in version 4.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 5.8.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Adds
.github/workflows/supply-chain.yml— two evidence artifacts our posture previously only claimed.What
sbomjob — Syft generates SPDX and CycloneDX SBOMs (SPDX is the ISO standard procurement asks for; CycloneDX is what most scanners ingest natively). Uploaded as workflow artifacts, and on arelease: publishedevent attached to the GitHub release as durable, citable assets. Workflow artifacts expire; release assets are what an auditor can link to.verify-provenancejob — installs the real published tarballs from npm into a scratch tree and runsnpm audit signatures.SECURITY.mdtells consumers to run this; until now we never ran it ourselves.Why it's decoupled from the release
Triggers are
release: published, weekly schedule, andworkflow_dispatch— neverci.yml's publish job. Nothing in this workflow can fail a release.Verified locally, not just written
prettier --checkpassescontents: writeonsbomonly, for the release upload;verify-provenanceinherits read)anchore/sbom-actionv0.24.0 dereferenced from its annotated tag)release: publishedevent — verify on the next releasebestax-migrateis broken on npmverify-provenanceoriginally installed all three published packages. It failed — and not because of this workflow:bestax-migrate@1.0.0's published manifest contains"@allxsmith/bestax-bulma": "workspace:^"— an unresolved pnpm workspace specifier.npm install bestax-migratefails for every consumer, on every package manager. Filed separately; this PR excludes that package from the audit (with a comment) so the check measures signatures rather than that bug. Add it back once fixed.Summary by CodeRabbit