Hold the SPDX 3 fixtures to the schema, and run the whole flow over SPDX 3 - #411
Merged
vpetersson merged 61 commits intoSep 16, 2026
Merged
Conversation
A document that validates clean against our own bundled spdx-3.0.1.schema.json came back with 12 schema errors after parse_spdx3_file plus write_spdx3_file. Every element in the output was invalid, which is why AUGMENT=true or ENRICH=true on any SPDX 3 input was an unconditional exit 1: both re-validate their own output. Four defects, all of them spdx-tools 0.8.5 modelling a pre-3.0.1 draft. CreationInfo still carries profile and data_license there. 3.0.1 moved dataLicense to SpdxDocument and replaced profile with Element.profileConformance, and CreationInfo_props is unevaluatedProperties: false, so either key invalidates every element carrying that CreationInfo. Both are now stripped on write. profileConformance was read nowhere, so every rewrite silently dropped it. Listing a profile claims every contained element meets its restrictions, so losing it weakens the document without saying so. It now rides on the payload and lands back on the SpdxDocument. A document that declared none does not gain one. The writer emitted externalReference, externalReferenceType and the type ExternalReference. 3.0.1 renamed all three to externalRef, externalRefType and ExternalRef, and rejects the old names. The parser defaults data_license to the bare id "CC0-1.0" when a document declares none. Writing that back invented a licence claim its author never made, and failed the schema besides, which wants a licence IRI rather than a short identifier. It is now read from the raw document only, from SpdxDocument first and the CreationInfo second, and is absent when the input was. Not fixed by upgrading spdx-tools: its SPDX 3 support is write-only and the maintainers say not to use it in production. The repo had no conformant SPDX 3 fixture to measure against. spdx3_minimal.json uses @id rather than spdxId, carries no @context and fails the schema eight times on its own, so a round-trip through it proved nothing. The new fixture validates before anything touches it, and the first test asserts that, because the rest are worthless without it. The mypy hook is skipped: it fails on master in two files this change does not touch, because its isolated env resolves two return types as Any.
Two more places the draft model spdx-tools 0.8.5 implements cost us data the producer wrote. The @context was hardcoded to 3.0.1 and none of the eight write call sites overrode it, so a 3.0 document came back labelled 3.0.1 while its creationInfo still said 3.0.0. Nobody wrote that document and no consumer can resolve it. 3.0 shipped April 2024 and 3.0.1 that December; syft, Microsoft sbom-tool, JFrog Xray and Zephyr all emit 3.0. The context now rides on the payload from parse to write, so every call site preserves it without knowing it has to, and a document built from nothing still gets the current release. An explicit argument still wins. SoftwarePurpose in that library is the pre-3.0.1 enum: no specification, no filesystemImage, both of which 3.0.1 defines and Yocto emits. The parser logged "Unrecognized primaryPurpose value; omitting" and dropped the field. Measured on the published Yocto 6.0.3 core-image-minimal SBOM, which validates clean against our own schema: 38 packages lost their primaryPurpose, and the one filesystemImage in the document is the image itself, the package a Yocto user cares about most. The 78 that survived did so by accident, having failed to parse for an unrelated reason and been passed through verbatim, so the elements we handled correctly were the ones that lost data. A value the producer wrote and the schema accepts is not dropped because a library is a version behind. Unmodelled purposes ride on the payload keyed by spdxId and are written back. That document now round-trips with 0 schema errors, 0 of 3049 elements lost and no field lost on any element. Before this branch it was 0 errors in and thousands out.
…pes were renamed 3.0.1 has no declaredLicense or concludedLicense property. It states a licence as a Relationship from the artifact to a licensing element. spdx-tools 0.8.5 still models both as fields, and every properties block is unevaluatedProperties: false, so the field invalidated the whole package. Invalid is the smaller half: sbomify's own reader follows the relationship, so a licence the action enriched in was one we could not read back. The same draft model costs more on the way in. Its RelationshipType enum holds 62 values and 37 of 3.0.1's 59 are not among them, both licence relationships included. The parser maps anything it does not recognise to `other`, which is itself legal, so the rewritten document passed the schema while saying something different from what the producer wrote. Measured on the published Yocto SBOMs, plain round-trip, no augment or enrich: 6.0.3 core-image-minimal 708 hasDeclaredLicense -> 708 other 6.0.3 core-image-full-cmdline 1667 hasDeclaredLicense -> 1667 other 5.1 core-image-minimal 547 hasConcludedLicense -> 547 other Every licence in the image, gone, and the file still validated. The raw value now rides on the payload beside the purposes and goes back verbatim. Two more from the same source. An artifact has one suppliedBy in 3.0.1 and originatedBy is the set; spdx-tools models both as lists, so a supplier the action worked out was written as a one-element array the schema refuses. And CreationInfo_props requires createdBy with minItems 1, while make_spdx3_creation_info left it empty, so each Tool, Organization and Person augmentation and enrichment add failed outright. Those now name a SoftwareAgent for the action, which the writer puts in the graph. A CreationInfo that arrived without a createdBy is left as it arrived: the action does not know who created that element, and naming itself there would state provenance nobody wrote. Enrichment and augmentation also stop second-guessing a package that already declared a licence. declared_license is unset on any package whose author stated one as a relationship, and reading that as "no licence" is how a contradicting second declaration got added. Overriding replaces the declaration rather than leaving two that disagree.
…ough SPDX_SCHEMAS held 2.2, 2.3 and 3.0.1. A 3.0 document matched nothing, so it validated as "skipped", logged a warning and went to the upload unchecked, while the README says generated SBOMs are validated against their JSON schemas. 3.0 is what syft, Microsoft sbom-tool, JFrog Xray and Zephyr emit, and what the published Yocto 5.1 image SBOM declares. CycloneDX has bundled every version it accepts, 1.3 through 1.7, all along. The two are separate documents rather than a version label on one, so aliasing 3.0 onto the 3.0.1 schema would have been wrong in both directions: 3.0.1 renamed File.software_contentType to contentType, added IndividualElement and renamed SpdxDocument.imports and Build.build_parameters. A document correct under either schema fails the other, and a test asserts all four cases. spdx-3.0.0.schema.json is the official shacl2code output from spdx.org/schema/3.0.0/, the same source as the 3.0.1 copy beside it. An SPDX 3.x version with no bundled schema now fails, naming the versions there are, rather than skipping. A skip is how every SPDX 3 document reached the upload unchecked in the first place, and for a format still adding versions a loud failure is the safer default. SPDX 2.x keeps the old skip: nothing claims to check 2.1, so refusing it now would be a new rejection rather than a fix. Five of the tests added here fail without the change. Two of them only discriminate because they assert `valid is False`: a skip reports valid=None, which is falsy, so assertFalse would have passed on the very behaviour they exist to rule out. Measured on the published Yocto 5.1 core-image-minimal, 2066 elements: skipped before, valid=True against the 3.0.0 schema now. Left alone: https://spdx.org/rdf/3.0/spdx-context.jsonld resolves (byte identical to 3.0.1 today) but carries no patch version, so a document using it still reports "could not detect spec version". That is a moving alias whose meaning depends on when the document was written, nothing in the corpus uses it, and guessing a version for it would be worse than refusing.
…ument states
A 3.1 document was accepted as SPDX 3 and then failed somewhere unhelpful.
`/rdf/3.1/` carries no patch number and the version regex wanted three dotted
numbers, so it found nothing and the document failed as "Could not detect spdx
spec version", naming neither SPDX nor 3.1. `/rdf/3.1.0/` fell through to the
unvalidated-skip path. Both now say so by name, in the backend's own words, so
a user who hits the action and the upload hears one answer rather than two:
SPDX 3.1 is not supported. sbomify accepts SPDX 2.2, 2.3 and 3.0.x;
send 3.0.1 to also satisfy the BSI TR-03183-2 floor.
Rejection is the right answer: 3.1 is at RC1, BSI accepts released versions
only, and the backend refuses it, so shipping it here would need a backend
change first.
extract_spdx3_version now reads the document's own specVersion before its
@context. CreationInfo_props requires specVersion and every Element requires a
creationInfo, so a conformant document always states its version, and that is
the normative claim rather than a hint. Checked on all six SPDX 3 documents in
the corpus: the two agree in every one. The context regex also accepts a
two-part version now, so an unversioned context is read rather than ignored.
The specVersion is read from a CreationInfo only, either one that names its
type or one reached as a creationInfo value. This answer chooses the schema
the whole document is held to, so a specVersion sitting on some other element
must not speak for it.
That order matters because https://spdx.org/rdf/3.0/spdx-context.jsonld is
real. It serves 200 and is byte-identical to the 3.0.1 context today, which
makes it a moving alias whose meaning depends on when the document was
written. Reading specVersion first resolves it for every document that states
one, which is every conformant document.
A document carrying only the alias still cannot validate, and that is SPDX's
own ruling rather than a choice made here: both official schemas pin @context
with a `const` to their fully qualified URL. What changes is where such a
document lands. It now gets the schema's own message naming the exact context
to write, instead of "could not detect spec version", and it is deliberately
not reported as an unsupported version, because 3.0.0 is supported and the
alias is the thing to fix. A test pins the const in both schemas so this
reasoning fails loudly if a future schema drops it.
Nine of the tests added here fail without the change. A tenth passes on the
old reader and fails on a looser one that took specVersion from anywhere,
which is the version I wrote first.
…efully
The --spec-version help read "e.g., '1.6', '2.3', '3.0.1'". Nothing here
generates SPDX 3: syft is the only SPDX generator that answers a plain version
and it caps at 2.2/2.3. So the one example that was not CycloneDX or the
default was the one that cannot work.
The config guard that catches it was already good, and pointed at SBOM_FILE
and additional-packages-only mode. Two things were wrong with it. It keyed on
the literal string "3.0.1", so SPEC_VERSION=3.0.0 fell through to the bare
"Supported: 2.2, 2.3" with no hint, and 3.0 is what syft, Microsoft sbom-tool,
JFrog Xray and Yocto 5.x emit. And it offered SBOM_FILE for any 3.x, including
versions the reader refuses, which would have sent a 3.1 user to a second
route to hear the same no. Now every version it names is one that route
accepts, and the empty-SBOM route says it writes 3.0.1 whatever was asked for,
which it does.
Below that, the registry's own failure rendered a Python dict into user-facing
text:
No generator found for input. Requested: format=spdx, version=3.0.1.
Available formats: {'cyclonedx': ['1.2', ...], 'spdx': ['2.2', '2.3',
'SPDX-2.3']}
'SPDX-2.3' is not a version anyone can set. Five generators declare it as an
internal marker meaning "reachable by converting", and three declare the plain
spelling, so the list showed one version twice and invited a value that selects
nothing. The message now names the format, the version asked for, and the
versions from the canonical tuples, which is where "what can be generated" is
already defined:
Nothing here generates SPDX 3.0.1. Generatable versions: 2.2, 2.3. SPDX 3
is read, validated and written, but no generator produces one, so supply
an existing document with SBOM_FILE instead of generating from a lock file
or an image.
The marker spelling itself is left alone and filed separately: normalising it
would make cyclonedx-cargo, gomod, maven, gradle and sbt candidates for a
plain SPDX 2.3 request, changing which tool produces the SBOM for every Rust,
Go, Java and Scala project.
action.yml gains sbom-format and spec-version as first-class inputs, so
neither has to be smuggled through a raw env: block, and a test asserts every
declared input reaches an environment variable, since one that reaches nothing
looks supported and is silently dropped. The README gains the input table it
never had; none of the existing five were in it either.
SUPPORTED_SPDX_VERSIONS gains 3.0.0, which is read, validated and written back
since the 3.0.0 schema was bundled, and validate_spdx_version's docstring no
longer claims SPDX 3.0 support has yet to be added.
…over SPDX 3
Both older SPDX 3 fixtures failed the schema this repo ships. Every Element
used `@id` where 3.0.1 wants `spdxId`, and the minimal one also carried an
empty `createdBy`, which is required with minItems 1, and a `profile` on its
CreationInfo, which 3.0.1 replaced with Element.profileConformance. The one
test that looked at a fixture's validity declined to assert it:
# Our minimal test fixture may not pass the strict 3.0.1 schema,
# but format/version detection must always succeed (valid is not None).
self.assertIsNotNone(result.valid)
A test that declines to check is why documents failing every element shipped
unnoticed. It now asserts `valid is True`, and a sibling holds all three
conformance-claiming fixtures to the same bar.
Converting the fixtures wholesale would have met the letter of that and
quietly deleted something worth keeping. `@id` on an Element is legal JSON-LD,
unprefixed software properties are what several producers emit, and the parser
reads both deliberately. So the old minimal fixture is kept verbatim as
spdx3_legacy_spellings.json, with tests that say what it is for, and one that
asserts it is still the non-conformant one, because these tests stop meaning
anything the moment somebody tidies it. spdx3_multi_type.json keeps the
blank-node `_:CreationInfo0` as `@id`, which the writer preserves on purpose
since spdxId must be an IRI.
The full-flow compliance test now runs for 3.0.0 and 3.0.1 beside 2.2 and 2.3.
It needs a sibling rather than a parametrize: the 2.x body builds a document
through spdx-tools, and nothing generates SPDX 3, so the 3.x leg starts from
the conformant fixture retargeted to the version under test. Both the context
and every specVersion move together, because the official schemas pin @context
with a const to their own fully qualified URL.
It validates the fixture before touching it, then augments, enriches, and
holds the result to the schema for the version it declares. Two further
assertions: the version it arrived as is the version it leaves as, and the
declared licence survives as a relationship, which is the writer defect in
miniature.
Reverting the writer fails it. Checked, not assumed: with spdx3.py,
enrichment.py, augmentation.py and additional_packages.py reset to
upstream/master, both legs fail on the `dataLicense` the draft model emits.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A confirmed bug in sbomify_action/spdx3.py can drop valid software_additionalPurpose values when a document mixes known and unknown purposes, due to restoring only the unknown subset and overwriting the full list.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR advances SPDX 3 support by making SPDX 3 fixtures/schema validation strict (so “conformant” fixtures must actually validate) and extending end-to-end tests to run augment+enrich across SPDX 3.0.0 and 3.0.1, while also updating the SPDX 3 reader/writer/validation paths to preserve version/context and key SPDX 3.0.1 semantics (e.g., license relationships).
Changes:
- Add/adjust SPDX 3 fixtures so conformance-claiming fixtures validate, while preserving a deliberately non-conformant “legacy spellings” fixture for parser compatibility coverage.
- Add/extend tests to validate SPDX 3 fixtures against the shipped schemas and to run a full augment→enrich→revalidate flow for SPDX 3.0.0 and 3.0.1.
- Update SPDX 3 parsing/writing/validation paths and user-facing routing messages to properly handle SPDX 3.0.0 vs 3.0.1, unsupported SPDX 3.x versions, and SPDX 3 license-as-relationship behavior.
File summaries
| File | Description |
|---|---|
| tests/test-data/spdx3_multi_type.json | Replace JSON-LD @id with spdxId for conformance and normalize list formatting. |
| tests/test-data/spdx3_minimal.json | Make the minimal SPDX 3 fixture schema-conformant (CreationInfo typing, createdBy, prefixed software fields, etc.). |
| tests/test-data/spdx3_legacy_spellings.json | Add a deliberately non-conformant SPDX 3 fixture to preserve coverage for legacy producer spellings. |
| tests/test-data/spdx3_conformant.json | Add a schema-valid SPDX 3.0.1 control fixture used for conformance/full-flow tests. |
| tests/test_validation.py | Add test coverage for SPDX 3.0.0 validation (not skipped), unsupported 3.1 messaging, and version extraction precedence. |
| tests/test_spdx3.py | Tighten schema assertions for SPDX 3 fixtures and add explicit tests around legacy spellings behavior. |
| tests/test_spdx3_conformance.py | Add extensive conformance tests around SPDX 3 writer behavior (schema validity, version/context preservation, licenses, suppliers, createdBy agent, etc.). |
| tests/test_schema_compliance.py | Add SPDX 3.0.0 and 3.0.1 to schema sweep; add full-flow compliance test for SPDX 3. |
| tests/test_generation_plugin.py | Strengthen error-message assertions for unsupported generation requests (format/version list correctness). |
| tests/test_config.py | Add tests ensuring SPDX 3 guidance/error routing remains consistent with what the code can read/write. |
| sbomify_action/validation.py | Add SPDX 3.0.0 schema support and fail-fast behavior for unrecognized SPDX 3.x (instead of skipping). |
| sbomify_action/spdx3.py | Enhance SPDX 3 version extraction and preserve unmodelled SPDX 3.0.1 values (raw relationship types/purposes, document fields), plus post-serialization normalization. |
| sbomify_action/serialization.py | Expand supported SPDX versions to include 3.0.0 for read/write (while generation remains 2.x). |
| sbomify_action/enrichment.py | Avoid adding a second declared license when the package already declares via SPDX 3.0.1 relationships. |
| sbomify_action/cli/main.py | Improve SPEC_VERSION guard messaging for SPDX 3 read/write vs generate behavior; update CLI help text. |
| sbomify_action/augmentation.py | Respect existing SPDX 3.0.1 declared-license relationships and ensure overriding replaces rather than duplicates. |
| sbomify_action/additional_packages.py | Ensure empty SPDX 3 SBOM creation uses shared CreationInfo helper (avoids invalid createdBy). |
| sbomify_action/_generation/registry.py | Centralize and improve “no generator” messaging, including SPDX 3 guidance. |
| README.md | Update supported SPDX 3 wording and document action inputs at a higher level. |
| action.yml | Expose sbom-format and spec-version as action inputs and wire them to env vars. |
Review details
- Files reviewed: 20/21 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Three from Copilot's review, all of them the same shape as the bugs this code exists to fix. A package listing both a purpose the library knows and one it does not came back with only the one it does not. Capture kept the strangers alone, restore overwrites the serialized list wholesale, so ["library", "specification"] became ["specification"]. The whole list is kept now whenever any of it is unknown. A document whose @context is a list or an object lost it. is_spdx3 and extract_spdx3_version both read every shape JSON-LD allows; only this one took the bare string, so context_url stayed unset and the writer fell back to the current release. That is the relabelling this branch added context_url to prevent, surviving in the one shape nobody tested. The minted agent stamped the wall clock, so two runs over one input differed by a line. It takes the timestamp from a CreationInfo that names it, which is both the stable answer and the honest one: the agent describes elements created at the document's own moment. Only a document stating no time at all falls back to now.
GitHub Actions passes `${{ inputs.x }}` as "" when the input is not set, so
SBOM_FORMAT arrived empty rather than unset and the CLI's click.Choice refused
it instead of falling back to cyclonedx. sbom-format declares its default the
way bom-type already does. spec-version declares an empty one on purpose,
since its real default is per format and lives in the CLI: 1.6 for CycloneDX,
2.3 for SPDX.
Separately, SPEC_VERSION=3.0 was told to pass an existing 3.0 document as
SBOM_FILE. That route fails too. "3.0" is a key in SPDX_SCHEMAS so an
alias-context document reaches a schema at all, but both official schemas pin
@context with a const to their fully qualified URL, so a document declaring
the bare line fails whichever one it is held to. It now gets the same
"nor is it read" answer as 3.1, naming the versions that do work.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
tests/test_config.py contains a mid-file if __name__ == "__main__": unittest.main() that can prevent later test classes from being defined/discovered when executed directly.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 20/21 changed files
- Comments generated: 1
- Review effort level: Lite
Two test classes were appended below `if __name__ == "__main__": unittest.main()`,
so running the file directly executed unittest at that line and exited before
they were ever defined. pytest imports the module and never notices, which is
why it stayed hidden.
python tests/test_config.py before: Ran 96 tests
python tests/test_config.py after: Ran 101 tests
Guard moved to the end, where it can see the whole file.
3.0.1 added IndividualElement; it did not drop it. I had the measurement in front of me and wrote the sentence inverted, which left a comment that would mislead the next person reading it to decide whether two schemas are interchangeable. Replaced with the differences that actually make them disagree rather than merely differ: 3.0.1 renamed SpdxDocument.imports to import, Build.build_parameters to build_parameter, and File.software_contentType to contentType. A File carrying software_contentType is valid under 3.0.0 and refused by 3.0.1, and one carrying contentType is refused by 3.0.0, which is the argument for bundling both and is already asserted in all four cells by tests/test_validation.py.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It includes substantial changes to SPDX 3 parsing/writing/validation semantics where correctness is subtle and impact is broad across the pipeline.
Review details
- Files reviewed: 20/21 changed files
- Comments generated: 1
- Review effort level: Lite
spdx-tools puts conformance on the CreationInfo as profile, real producers emit it there, and the normalisation strips profile from every CreationInfo on the way out. Nothing read it first, so a document that stated its conformance only there came back claiming none: the same shape as the dataLicense loss directly above it, and the same fix, inline or referenced. The document's own profileConformance still wins, and a document that claimed neither still claims neither.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It makes wide-ranging, correctness-sensitive changes across SPDX 3 parsing/writing/validation and the CLI pipeline, which merits final human review despite strong test coverage.
Review details
- Files reviewed: 20/21 changed files
- Comments generated: 0 new
- Review effort level: Lite
_SPDX3_CONTEXT_URL_RE accepts http and https, and the candidate was returned exactly as it arrived. The schemas pin @context to the https form with a const, so a document declaring http://spdx.org/rdf/3.0.1/spdx-context.jsonld came back declaring the same thing and failed the action's own validation step: the alias failure again, from a different direction. What an http context names is the version, not the scheme, so the version is kept and the scheme is not.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It includes wide-reaching SPDX 3 parsing/writing/validation behavior changes plus action-interface wiring updates, which need careful human verification despite strong test additions.
Review details
Suppressed comments (1)
README.md:146
- The Action inputs table omits the SPDX 3 nuance that is now documented in action.yml and the surrounding prose (SBOM_FILE ignores SBOM_FORMAT/SPEC_VERSION, and SPDX 3 is processed via SBOM_FILE but not generated). As-is, the table is inconsistent with action.yml and can mislead users who rely on the table as the authoritative description of the
with:inputs.
| `sbom-format` | `SBOM_FORMAT` | `cyclonedx` (default) or `spdx` |
| `spec-version` | `SPEC_VERSION` | Spec version to generate. CycloneDX 1.2–1.7, SPDX 2.2 or 2.3 |
- Files reviewed: 20/21 changed files
- Comments generated: 0 new
- Review effort level: Lite
SPDX binds AND tighter than OR, and set members were joined with no parentheses, so a disjunction inside a conjunction came back as "MIT OR Apache-2.0 AND GPL-2.0-only". That parses as MIT OR (Apache-2.0 AND GPL-2.0-only), which grants MIT on its own: a different licence claim from the one the producer wrote, published as though they had written it. A member is now parenthesised when it carries an operator the joiner does not, counted outside any parentheses it already has. The same operator still nests flat, because AND and OR are each associative and the parentheses would be noise. A member that is a bare expression string rather than a set is read the same way, which is the shape spdx3_license_from_string produces.
Contributor
Author
|
Reversing my earlier decline on the I declined this on the grounds that the row named generation and listed the versions, so 3.0.x being absent was correct. That was wrong, and what disproved it was my own follow-up: additional-packages-only mode does generate SPDX 3. Both the row and the action.yml description now name that route. They also name the thing neither of them said: |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
sbomify_action/spdx3.py’s _three_part_spec_version() can select the wrong patch version when resolving the 3.0 line alias because it doesn’t prefer the SpdxDocument’s own CreationInfo.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
sbomify_action/spdx3.py:1230
- _three_part_spec_version() says it uses the producer/document’s own CreationInfo first, but it currently scans all CreationInfos in the graph and returns the first matching patch version. In a merged SPDX 3 document (or any document that contains elements copied from another SPDX 3.x doc) this can let an arbitrary element’s specVersion decide the patch for the whole document when resolving a
3.0line alias, which may re-label the context and minted elements to the wrong patch version.
- Files reviewed: 20/21 changed files
- Comments generated: 0 new
- Review effort level: Lite
… version _three_part_spec_version scanned the graph for a producer-stated version, so in a merged document the element copied in from somewhere else could answer for the whole thing. That answer picks the @context the writer emits and the schema the result is then held to, so a document whose own CreationInfo says 3.0.1 could come back relabelled 3.0.0 and checked against the wrong schema. The SpdxDocument is asked first now, inline or referenced, which is the order extract_spdx3_version already uses for the same reason. A fragment with no SpdxDocument still falls back to the graph, and what the action minted is still skipped.
Contributor
Author
|
Confirmed and fixed in 29f7df6. Reproduced with a merged document: a package copied in from a 3.0.0 document, listed ahead of an SpdxDocument whose own CreationInfo says 3.0.1. Resolving the 3.0 alias returned 3.0.0, and since that value picks both the @context the writer emits and the schema the result is then held to, the document came back relabelled and checked against the wrong one. Fixed the way you describe, and it is the second time this exact shape has bitten: Covered by two new cases for the inline and referenced shapes, plus one for the no-SpdxDocument fallback. |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes are heavily covered by targeted fixture/schema and full-flow tests, and I did not find any concrete correctness or safety issues in the updated behavior.
Review details
- Files reviewed: 20/21 changed files
- Comments generated: 0 new
- Review effort level: Lite
vpetersson
approved these changes
Sep 15, 2026
27 tasks
vpetersson
approved these changes
Sep 16, 2026
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
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.
Closes #331. Phase 2 of the SPDX 3 epic, sbomify/sbomify#1324.
Stacked on #410 → #409 → #408 → #407. Only the last commit belongs to this PR. Tests and fixtures only; no change under
sbomify_action/.The test that declined to check
It did not pass. Both older SPDX 3 fixtures failed the schema this repo ships:
spdx3_minimal.json@idon every Element, emptycreatedBy,profileon CreationInfospdx3_multi_type.json@idon two ElementsA test that declines to check is why documents failing every element shipped unnoticed. It now asserts
valid is True, and a sibling holds all three conformance-claiming fixtures to the same bar.Converting them would have deleted something worth keeping
@idon an Element is legal JSON-LD. UnprefixedpackageVersionandpackageUrlare what several producers emit. The parser reads both spellings deliberately, and the only coverage of that was a fixture that happened to be invalid.So the old minimal fixture is kept verbatim as
spdx3_legacy_spellings.json, with tests that name what it is for, including one asserting it is still the non-conformant one, because the rest stop meaning anything the moment somebody tidies it.spdx3_multi_type.jsonkeeps its blank-node_:CreationInfo0as@id, which the writer preserves on purpose sincespdxIdmust be an IRI.Full flow over SPDX 3
test_spdx3_full_flow_complianceruns for 3.0.0 and 3.0.1 beside the existing 2.2 and 2.3.It is a sibling rather than a parametrize because the 2.x body builds a document through spdx-tools, and nothing generates SPDX 3. The 3.x leg starts from the conformant fixture retargeted to the version under test, moving the
@contextand everyspecVersiontogether, because the official schemas pin@contextwith aconstto their own fully qualified URL.It validates the fixture before touching it, augments, enriches, then checks three things:
hasDeclaredLicenserelationshipReverting the writer fails it
Checked rather than assumed. With
spdx3.py,enrichment.py,augmentation.pyandadditional_packages.pyreset toupstream/master, both legs fail on thedataLicensethe draft model emits:Source restored and confirmed identical to HEAD afterwards.
Tests
test_schema_compliance.py9 passed,test_spdx3.py55 passed. Full suite 3776 passed, 4 skipped, 0 failed.ruffclean.Closes #331