Separate what the action generates from what it checks - #412
Merged
vpetersson merged 79 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.
…standard The format list said "generate and process" per format, which reads as one capability and is two. Everything here is read, validated, augmented, enriched and written back; only some of it can be produced from a lock file or an image in the first place, and the two lists do not match in either direction. SPDX 3 is the loud gap: nothing generates it, syft being the only SPDX generator and stopping at 2.3. CycloneDX 1.2 is the quiet one in the other direction, generatable with no bundled schema, so it reaches the upload unchecked. The table now has a Generate column and a Validate column and they disagree in both rows, which is the point. Added a short standards note, because a procurement clause citing "the ISO SBOM standard" is a real way to end up on the wrong version, and this repo mentioned 5962 nowhere. Every claim in it is from a primary source, not from the issue text. ISO/IEC 5962:2021 is SPDX 2.2.1: the SPDX project's own v2.2.1 release notes say the release "includes all updates for the final ISO/IEC 5962:2021 SPDX specification", and spdx.dev dates it 2021/08. SPDX 3.0 is OMG formal/24-11-01, March 2025, read off omg.org/spec/SPDX. BSI TR-03183-2 v2.1.0 asks for SPDX 3.0.1 or CycloneDX 1.6 and up, which this repo's own BSI plugin already enforces. The issue also asserted that ISO/IEC DIS 5962 edition 2 remains unpublished. iso.org sits behind Cloudflare and returns 403, so that could not be checked and is not claimed here. What is said instead is that SPDX 3.0 is not covered by 5962:2021, which follows from what was verified. omg.org/spec/SPDX/3.0.1/ is a 404 and is not linked; the versionless page is.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new sbom-format action input has no default (so SBOM_FORMAT becomes empty and the CLI fails), and the minted SPDX SoftwareAgent uses a run-time timestamp that makes outputs unnecessarily non-reproducible.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR clarifies (and enforces in code/tests) the distinction between SBOM generation capabilities vs validation/read/write capabilities, with a particular focus on SPDX 3.x handling and the user-facing guidance/errors around unsupported versions.
Changes:
- Document and enforce the separate “generatable” vs “validatable” version sets (notably: SPDX 3 is ingest/validate/write, not generatable).
- Improve SPDX 3 parsing/writing/normalization to preserve semantically important fields and validate against the correct schema version (3.0.0 vs 3.0.1).
- Add/extend fixtures and tests to prevent regressions in SPDX 3 conformance, schema validation, and user-facing error messaging.
File summaries
| File | Description |
|---|---|
action.yml |
Adds sbom-format / spec-version inputs and wires them to env vars for the container runtime. |
README.md |
Updates “Format support” section to split Generate vs Validate capabilities and adds standards/version notes. |
sbomify_action/_generation/registry.py |
Reworks “no generator” error messaging to be actionable and avoid leaking internal version markers. |
sbomify_action/validation.py |
Adds SPDX 3.0.0 schema support and makes unknown SPDX 3.x versions fail loudly (not skip). |
sbomify_action/spdx3.py |
Preserves/normalizes SPDX 3 fields across parse→write, restores unmodelled enum values, and post-processes output for schema conformance. |
sbomify_action/serialization.py |
Expands supported SPDX read/write versions to include 3.0.0 and clarifies docs around “supported vs generatable”. |
sbomify_action/enrichment.py |
Avoids adding a second declared license when SPDX 3.0.1 expresses it via relationships. |
sbomify_action/augmentation.py |
Same license-relationship awareness as enrichment; override mode removes prior declarations before restating. |
sbomify_action/additional_packages.py |
Uses shared SPDX 3 CreationInfo builder for empty SPDX 3 docs. |
sbomify_action/cli/main.py |
Improves CLI help text and config validation messaging around SPDX 3 non-generatability. |
tests/test_generation_plugin.py |
Updates expectations for improved “no generator” error message. |
tests/test_config.py |
Adds tests around routable SPDX 3 advice and validates action.yml input wiring. |
tests/test_validation.py |
Adds extensive tests for SPDX 3.0.0 validation, unsupported 3.x behavior, and version detection rules. |
tests/test_schema_compliance.py |
Adds full-flow schema compliance checks for SPDX 3.0.0 and 3.0.1. |
tests/test_spdx3.py |
Tightens SPDX 3 fixture validation expectations and introduces legacy-spellings fixture coverage. |
tests/test_spdx3_conformance.py |
New deep conformance suite ensuring writer output remains schema-valid and semantically stable. |
tests/test-data/spdx3_multi_type.json |
Adjusts fixture IDs/spellings to match conformant expectations. |
tests/test-data/spdx3_minimal.json |
Updates minimal fixture toward conformance (profileConformance, typed CreationInfo, etc.). |
tests/test-data/spdx3_legacy_spellings.json |
New fixture capturing “real producer” legacy spellings (intentionally non-conformant). |
tests/test-data/spdx3_conformant.json |
New known-good SPDX 3.0.1 conformant control fixture used across tests. |
Review details
- Files reviewed: 20/21 changed files
- Comments generated: 2
- 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.
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.
"No tool generates it" and a Generate column reading "none" are both wrong: additional-packages-only mode builds a 3.0.1 document from a PURL list, and the error you get for asking a generator plugin for 3.0.1 already points at that route. Verified with LOCK_FILE=none, SPEC_VERSION=3.0.1 and ADDITIONAL_PACKAGES, which writes a document the action's own validation step then passes against the 3.0.1 schema.
Contributor
Author
|
The profileConformance finding is confirmed and fixed in 0345a79 on #407. Reproduced exactly as described: a document that states its conformance only as The document own |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The SPDX3 context preservation logic can write http://spdx.org/... contexts verbatim even though the bundled schemas pin @context to https://spdx.org/..., which risks emitting schema-invalid output.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
sbomify_action/spdx3.py:791
_declared_context()accepts both http and https context URLs via_SPDX3_CONTEXT_URL_RE(https?://...) and then returns the original candidate verbatim. The shipped SPDX 3 schemas pin@contextto an https URL (see tests assertingschema["properties"]["@context"]["const"] == "https://spdx.org/..."), so preserving anhttp://spdx.org/...context would cause the action to write an SBOM that fails schema validation even after other normalizations. Consider normalizinghttp://→https://at capture time so output always uses the schema-pinned form while still preserving the version.
- 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
Author
|
Confirmed and fixed in 4b7eeac on #411. Reproduced: a document declaring Took the normalising route you suggested rather than narrowing the regex: what an http context names is the version, and dropping the whole candidate would relabel an Worth flagging for whoever reviews: this defect is on master now, since it came in with #407. The fix rides on #411 because that is the lowest open branch in the chain, not because it belongs to that PR scope. The same is true of the |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It makes broad, high-impact changes across SPDX 3 parsing/serialization/validation paths and action surface area that warrant final human review despite expanded test coverage.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
README.md:128
- The PR description says “Only the last commit belongs to this PR. README only.”, but the PR diff includes substantial Python code, tests, and action.yml changes. If this is a stacked-PR diff artifact, consider retargeting/rebasing or adjusting the PR description so reviewers understand the true scope of changes being reviewed.
action.yml:26 - The
spec-versioninput description says SPDX 3 is “not generated” and directs users to SBOM_FILE only, but this action can still produce an SPDX 3.0.1 SBOM in additional-packages-only mode (LOCK_FILE=none or SBOM_FILE=none) when SPEC_VERSION starts with 3. This mismatch can mislead users who rely on the input descriptions rather than the README.
- Files reviewed: 20/21 changed files
- Comments generated: 2
- 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.
…ther Both the action.yml description and the README input row said SPDX 3 is not generated. Additional-packages-only mode does generate it: LOCK_FILE=none with SPEC_VERSION=3.0.1 builds a 3.0.1 document from ADDITIONAL_PACKAGES, and the error you get for asking a generator plugin for 3.0.1 already points there. Neither said what happens on the path most SPDX 3 users take. SPEC_VERSION is ignored outright when SBOM_FILE names a real file: the document is read, checked and written at the version it declares. Verified by running the same input at SPEC_VERSION unset, 2.2 and 1.6, which produce identical output.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It makes broad, behavior-changing updates to core SPDX 3 parsing/writing and validation logic across multiple pipeline entry points, so a final human review is warranted despite the strong added tests.
Review details
- 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
There was a problem hiding this comment.
🔵 Needs a closer look
It makes broad, behavior-defining changes to SPDX 3 parsing/writing and validation semantics across multiple code paths, which warrants final human review despite strong test coverage.
Review details
- Files reviewed: 20/21 changed files
- Comments generated: 0 new
- Review effort level: Lite
The help named only SBOM_FILE, while the README documents two routes and its input table says "SPDX 3.0.1 with LOCK_FILE: none". LOCK_FILE=none with ADDITIONAL_PACKAGES builds a 3.0.1 document, so the help was telling users one of the two ways was the only way.
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 #333. The last of phase 2 of the SPDX 3 epic, sbomify/sbomify#1324.
Stacked on #411. #407, #408, #409 and #410 have merged, so the diff against master is now this PR plus #411.
Three commits here are not about docs, and they are not about #411 either. They fix defects that shipped with #407 and are on master now, found by review after #407 merged:
0345a79a profileConformance stated the draft way, ascreationInfo.profile, was dropped on rewrite4b7eeacanhttp://context was written back with its scheme, which the schemas reject1f4a2daa nested licence set lost its precedence, so(MIT OR Apache-2.0) AND GPL-2.0-onlycame back granting MIT aloneThey ride on the open branches because that is the shortest path back to master, not because they belong to this scope. The first of them belongs to #327, which is already closed.
Generating and checking are two lists, and they disagree in both directions
The old section said "generate and process" per format, which reads as one capability.
SPDX 3 is the loud gap: nothing generates it, syft being the only SPDX generator and stopping at 2.3.
CycloneDX 1.2 is the quiet one in the other direction. It can be generated and no 1.2 schema ships here, so a 1.2 document reaches the upload unchecked. That is the same gap #329 just closed for SPDX 3.0, on the other format. Found while writing this table; the README now says so rather than implying 1.2 is checked like the rest. Not fixed here, because bundling the schema and dropping 1.2 from the supported set are different decisions with different blast radii.
The standards note
A procurement clause citing "the ISO SBOM standard" is a real way to end up on the wrong version, and "5962" appeared nowhere in this repo.
Every claim is from a primary source rather than from the issue text:
formal/24-11-01, March 2025 — read off omg.org/spec/SPDX.What is deliberately not claimed
The issue also asserted that ISO/IEC DIS 5962 edition 2 remains unpublished.
iso.orgsits behind Cloudflare and returns 403, so that could not be checked and is not stated. The README says SPDX 3.0 is not covered by ISO/IEC 5962:2021, which follows from what was verified.https://www.omg.org/spec/SPDX/3.0.1/is a 404, so the versionless page is linked instead. All four URLs added here return 200.Checks
markdownlintclean on the new section; the file's only remaining findings are the pre-existing HTML banner and line lengths that predate this change.Closes #333