Skip to content

ci: harden release SBOM job per security review (follow-up to #201)#202

Merged
27Bslash6 merged 1 commit into
mainfrom
fix/release-sbom-hardening
Jun 21, 2026
Merged

ci: harden release SBOM job per security review (follow-up to #201)#202
27Bslash6 merged 1 commit into
mainfrom
fix/release-sbom-hardening

Conversation

@27Bslash6

@27Bslash6 27Bslash6 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #201 — security-review hardening

#201 fixed the silently-broken SBOM attestation (syft replacing the dead cyclonedx-py pipeline) and is merged. A security-specialist review of #201 returned APPROVE-WITH-NITS: the SBOM was generated correctly but with four hardening gaps. This PR closes all four (plus a gating fix the job-split makes mandatory).

Note

None of these are regressions — #201 is already a strict improvement over the prior fake-green/no-SBOM state. This is defense-in-depth for a product whose SBOM is the compliance artifact.

Changes

# Severity Fix
001 Medium Accurate BOM. .github/syft.yaml excludes tests/** (+rust/fuzz/**) — syft was cataloging tests/integration/saas/requirements.txt, so the attested SBOM falsely claimed the wheel depends on SaaS integration-test packages.
002 Medium Credential isolation. syft (a third-party action that fetches+executes an installer from anchore/syft@main at runtime) is moved into its own sbom job with no id-token/attestations/contents:write. The attest job now runs first-party actions only and consumes the SBOM as an artifact, so the OIDC signing token never shares a runner with third-party code.
003 Low Fail closed on empty SBOM. A jq guard rejects a valid-but-empty SBOM (syft can exit 0 yet catalog nothing) before it gets attested.
004 Low No stray uploads in the gating path. Disabled the action's upload-artifact/upload-release-assets; the SBOM is handed to attest via an explicit pinned actions/upload-artifact.
(required by 002) publish gates on needs.attest.result == 'success' instead of !failure(). With attest now depending on sbom, a failed sbom skips attest (not fails), and a plain !failure() would let publish ship unattested.

Verification

  • actionlint clean
  • syft uv.lock + Cargo.lock cataloging confirmed (bundled syft v1.42.3; uv.lock cataloger since v1.18.0)
  • All actions SHA-pinned (anchore/sbom-action@…v0.24.0, actions/upload-artifact@…v7.0.1)
  • Gating reasoned through: sbom fail → attest skipped → publish skipped (fail-closed)
  • End-to-end SBOM runs only on a real release / workflow_dispatch — by design.

ci: typed — workflow only, no release.

Summary by CodeRabbit

Release Notes

Chores

  • Enhanced SBOM (Software Bill of Materials) generation for improved supply chain security verification
  • Strengthened build artifact attestation requirements to ensure published releases maintain higher integrity standards
  • Improved verification processes for build artifacts and attestations

Security review of #201 found the SBOM was generated correctly but with hardening gaps. Apply all four findings: (1) isolate syft (third-party) into a credential-less 'sbom' job so the OIDC signing token in 'attest' is never exposed to untrusted code; (2) exclude tests/** via .github/syft.yaml so the BOM describes shipped deps, not the test harness (syft was cataloging tests/integration/saas/requirements.txt); (3) jq guard fails closed on a valid-but-empty SBOM; (4) disable the action's upload-artifact/upload-release-assets side-effects, handing the SBOM to attest via an explicit artifact. Also gate publish on needs.attest.result=='success': required now that attest is *skipped* (not failed) when sbom fails, which a plain !failure() check would let publish through unattested.
@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 092a73eb-34ee-4e10-afb3-b923b9e203a4

📥 Commits

Reviewing files that changed from the base of the PR and between e09701a and c809ac9.

📒 Files selected for processing (2)
  • .github/syft.yaml
  • .github/workflows/release-please.yml

Walkthrough

A new .github/syft.yaml excludes test and fuzz directories from Syft scans. The release workflow gains a dedicated sbom job that generates and validates a CycloneDX SBOM before the attest job consumes it. The publish job gate is tightened from !failure() to an explicit attestation success check.

Changes

SBOM Pipeline Restructure

Layer / File(s) Summary
Syft scan exclusion config
.github/syft.yaml
Adds a Syft configuration excluding ./tests/** and ./rust/fuzz/** so generated SBOMs omit non-shipping dependencies.
Dedicated sbom job and attest wiring
.github/workflows/release-please.yml
Introduces a new sbom job with restricted permissions and credential-less checkout that runs anchore/sbom-action to produce sbom.cdx.json, verifies a minimum component count, and uploads the result as an artifact. Updates the attest job to declare sbom as a dependency and download the pre-generated SBOM artifact rather than producing it inline.
Publish job success gate
.github/workflows/release-please.yml
Changes the publish job condition from !failure() to needs.attest.result == 'success', ensuring publishing is blocked when attestation is skipped or fails.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • cachekit-io/cachekit-py#101: Modifies the same attest and publish job logic and dependencies in release-please.yml, directly overlapping with the attestation/publish gating changes here.
  • cachekit-io/cachekit-py#116: Alters the same release-please.yml control flow around the attest job's failure handling and how publish blocks on attest.
  • cachekit-io/cachekit-py#201: Switches to anchore/sbom-action producing sbom.cdx.json and wires it into the attest job — the same SBOM workflow refactor extended by this PR.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description covers the key sections: a brief overview of changes, motivation (security review findings), type of change (CI/CD), and comprehensive details of the five fixes. However, it does not follow the provided template structure with explicit checklist sections. Consider following the repository's PR template more closely by using the explicit template sections (Description, Motivation, Type of Change checkboxes, Security Checklist, etc.) rather than a custom format, even though the substantive content is thorough and complete.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically identifies the primary change: hardening the release SBOM job following a security review. It references the follow-up context (#201) and uses clear, action-oriented language without noise or vague terms.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/release-sbom-hardening

Comment @coderabbitai help to get the list of available commands and usage tips.

@27Bslash6 27Bslash6 merged commit 0d46ab5 into main Jun 21, 2026
32 checks passed
@27Bslash6 27Bslash6 deleted the fix/release-sbom-hardening branch June 21, 2026 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant