Skip to content

ci (release): Rework the SBOM output verification step - #383

Draft
turbobobbytraykov wants to merge 1 commit into
btraykov/release-workflow-refactoringfrom
btraykov/release-sbom-output-verification
Draft

ci (release): Rework the SBOM output verification step#383
turbobobbytraykov wants to merge 1 commit into
btraykov/release-workflow-refactoringfrom
btraykov/release-sbom-output-verification

Conversation

@turbobobbytraykov

Copy link
Copy Markdown
Contributor

Description

Warning

Draft — this needs reworking completely. Do not review it as a proposal; review it as a problem statement.

Reinstates the Verify SBOM output step and the two SBOM_* budget variables it reads, unchanged, on a branch of its own. It was removed from #371 in 4c748a7 so the rest of the release workflow is not held behind this rework. Restoring it verbatim here gives the rework a baseline to diff against rather than a deleted step to reconstruct from memory.

Base branch is btraykov/release-workflow-refactoring, not master, because the step only makes sense on top of the sbom job that #371 introduces.

Motivation / Context

The sbom job generates SPDX 2.2 and 3.0 documents and attests them against the package digest. An SBOM that is generated from the wrong dependency graph — the whole dev tree instead of the shipped set — is worse than no SBOM, because it is signed, attested, and wrong. The alpha.1 run produced 583 components for a package that ships around 60. Something has to fail the release when that happens again.

The step in this PR is that something, and it does catch the alpha.1 failure. But it establishes the property it wants through three proxies, and each fails in a different direction:

  • Hand-maintained blocklist of build-only package names. Catches webpack, xunit, prettier and the dozen-odd others somebody thought to list. A new devDependency, a renamed one, or a transitive build-only package nobody enumerated passes silently. The list needs updating every time the toolchain changes, and nothing makes that failure visible.
  • Component ceiling of 150. A single number spanning two ecosystems. It is the backstop for everything the blocklist misses, which is a lot of load for a constant with no derivation behind it — it sits at roughly 2.5× the current count purely so it does not trip on ordinary growth. A regression that leaks 40 dev packages passes.
  • Declared-license floor of 50%. Set below the measured 73.3% rather than at it, because only NuGet components declare a license locally. It therefore cannot detect a drop from 73% to 55%, which is the shape of a real regression.

The underlying issue is that all three try to infer "was this generated from a production-only graph?" from the shape of the output. That question has a direct answer — the restore and install that fed the generator either excluded dev dependencies or did not — and asserting it directly would be both stronger and maintenance-free.

What the rework should probably look like

Not committing to a design in the PR description, but the directions worth evaluating:

  1. Assert the input, not the output. The sbom job already runs dotnet restore on the library project alone and npm ci --omit=dev --ignore-scripts followed by deleting the lockfile. Those are the invariants that matter. Verifying them directly — that no other project.assets.json was written, that the lockfile is gone before the generator runs — replaces all three proxies with checks that cannot silently rot.
  2. Compare against the package, not against a constant. The nupkg is right there in artifacts. Its dependency group and its bundled static assets are the shipped set by definition, so the SBOM's package list can be diffed against them instead of against a number somebody picked.
  3. If a threshold survives, derive and record it. A floor set below the measured value cannot detect regression. If the license percentage is worth checking at all, it should be pinned at the measured value and raised deliberately, the way eng/bundle-budgets.json handles bundle sizes.
  4. Keep the run-summary output. The ### SBOM block with component and license counts is the one part of this step that is unambiguously worth keeping regardless of how the assertions change.

Type of Change (check all that apply):

  • Bug fix
  • New functionality
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (no functional changes)
  • Documentation
  • Demos
  • CI/CD
  • Tests
  • Changelog

Component(s) / Area(s) Affected:

Release workflow, sbom job only. No product code.

How Has This Been Tested?

  • Unit tests
  • Manual testing
  • Automated e2e tests

Not tested, deliberately. The content is byte-identical to what was on btraykov/release-workflow-refactoring at cde2b0agit diff against the base is 57 additions and 0 deletions, and the YAML was parsed to confirm Verify SBOM output sits back between Generate SBOMs and Reverify package before attestation. The step's behaviour was exercised by the 0.1.2-alpha.* release runs; the objection here is to what it asserts, not to whether it runs.

Test Configuration:

  • .NET version: n/a
  • Hosting model: n/a
  • Browser(s): n/a
  • OS: windows-latest runner

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code
  • This PR includes API docs for newly added methods/properties
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes behavioral changes and the feature specification has been updated with them
  • Accessibility (ARIA, keyboard navigation, focus management) has been verified

Merge order

Blocked on #371. Rebase onto master and retarget once that merges.

Restores the 'Verify SBOM output' step and the two SBOM_* budget variables it reads, unchanged, so the rework has a baseline to diff against rather than starting from a deleted step.

The check is not in a state to merge. It asserts SBOM quality through a hand-maintained blocklist of build-only package names, a component ceiling and a declared-license floor - three proxies that each fail in a different direction: the blocklist only catches names somebody thought to add, the ceiling is a single number covering two ecosystems, and the license floor is set below the measured value rather than at it. Draft until that is replaced with something that checks the property we actually care about.
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