Skip to content

Check SPDX against its schema before uploading it - #452

Open
aurangzaib048 wants to merge 1 commit into
sbomify:masterfrom
aurangzaib048:fix/validate-every-spdx3-write
Open

aurangzaib048 wants to merge 1 commit into
sbomify:masterfrom
aurangzaib048:fix/validate-every-spdx3-write

Conversation

@aurangzaib048

Copy link
Copy Markdown
Contributor

Fixes #326, together with #436.

What was wrong

No SPDX document of any version was checked against its schema before upload. The pipeline passed validate_before_upload only for CycloneDX, and the sbomify destination only knew how to check CycloneDX. So an SPDX 3 document the action rewrote, through a component override, additional-package injection or an empty SBOM, reached the server unchecked. So did one that passed through untouched.

The fix

  • The pipeline asks for validation for SPDX as well as CycloneDX.
  • The sbomify destination validates SPDX with validate_sbom_file_auto.
  • CycloneDX handling is unchanged.

Why one check at the upload, not one after each write

Every SPDX 3 write path runs before step 5: the three overrides, both additional-package writers, the empty SBOM, augmentation and enrichment. So one check at the upload covers all of them, and any write path added later.

Checking after each write would repeat an expensive check. One SPDX 3 schema validation costs about 10 ms per element. That is 62 ms for the conformant fixture, and around 32 seconds for the same document with 3000 extra packages. A run can write the document five or more times.

This also covers the override functions' except Exception: an override that wrote an invalid document now fails at the upload, not in a swallowed warning.

What the issue's criteria already had

The acceptance criteria for enrich, augment, a COMPONENT_VERSION override and create_empty_sbom already hold, and TestTheOtherWritePaths already tests them. They hold because #325's writer is conformant, not because anything validated. This PR adds the validation the issue's work items asked for. The Yocto items are #436.

Verified

  • New tests in TestSbomifySpdxValidationGate:
    • An invalid SPDX 3 document is not uploaded.
    • A valid one is uploaded.
    • An invalid SPDX 2 document is uploaded with a warning.
    • Turning validation off still skips it.
  • test_pipeline_spdx_upload.py checks that the pipeline asks for validation for SPDX and CycloneDX.
  • Each new test except the two that pin unchanged behaviour (CycloneDX, validation off) failed before the change.
  • ruff, ruff format --check and mypy are clean. Coverage is 81.74%.
  • The full suite has 3866 passed. The 26 failures in test_runtimes.py, test_ecosystem_routing.py and test_php_composer.py are the same 26 that fail on master on macOS, because the pinned runtimes are Linux builds.

Follow-ups, not in this PR

The pipeline passed validate_before_upload only for CycloneDX, and the sbomify
destination only ever checked CycloneDX, so no SPDX document of any version
was schema-checked before upload. An SPDX 3 document the action rewrote
(component overrides, additional packages, an empty SBOM) or passed through
untouched reached the server unchecked.

The pipeline now asks for validation for SPDX too, and the destination
validates it. Every SPDX 3 write path runs before the upload, so this one
check covers all of them. An invalid SPDX 3 document is not uploaded, and the
result names the version, the path and the reason. An invalid SPDX 2 document
is uploaded with a warning, the same policy the Yocto pipeline uses, because
the fault is usually the producer's.

Validation runs once, at the upload, rather than after each write: one SPDX 3
schema check costs about 10 ms per element, around 30 seconds for a document
with 3000 packages, and a run can write the document five or more times.
Copilot AI balanced review requested due to automatic review settings October 15, 2026 08:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It changes the pre-upload validation gate behavior on the upload/delivery path, which is correctness-sensitive and warrants final human sign-off despite no concrete defects being found.

0 open findings

What changed in this PR

This PR closes a validation gap where SPDX documents (any version) reached the sbomify backend without being schema-checked, while CycloneDX documents were validated pre-upload. The pipeline now requests pre-upload validation for SPDX as well as CycloneDX, and the sbomify destination performs auto-detecting SPDX schema validation at the single upload chokepoint—chosen deliberately because every SPDX 3 write path (overrides, additional-package injection, empty SBOM, augmentation, enrichment) runs before upload, so one check covers them all without repeating an expensive per-write validation. The policy is asymmetric by design: invalid SPDX 3 is blocked (loud failure, since the action rewrote it), while invalid SPDX 2 is uploaded with a warning (treated as the producer's responsibility, aligning with the Yocto work in #436).

Changes:

  • run_pipeline now sets validate_before_upload=True for both cyclonedx and spdx formats.
  • SbomifyDestination.upload validates SPDX via validate_sbom_file_auto; SPDX 3 failures block upload, SPDX 2 failures warn-and-upload.
  • Adds TestSbomifySpdxValidationGate and a new pipeline test asserting both formats are handed to upload with validation on.
File Description
sbomify_action/​cli/​main.py Extends validate_before_upload to SPDX so SPDX documents reach the upload with validation requested.
sbomify_action/​_upload/​destinations/​sbomify.py Adds the SPDX validation branch: SPDX 3 failures block upload; SPDX 2 failures log a warning and proceed.
tests/​test_upload_module.py New TestSbomifySpdxValidationGate covering invalid/valid SPDX 3, warned SPDX 2, and validation-off cases.
tests/​test_pipeline_spdx_upload.py New test verifying the pipeline passes sbom_format and validate_before_upload=True for both SPDX and CycloneDX.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

This branch has not been deployed

No deployments
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.

Validate every SPDX 3 write path

2 participants