Repository navigation
fix: emit valid SPDX license expressions when enriching - #151
Draft
nissessenap wants to merge 3 commits into
Draft
nissessenap wants to merge 3 commits into
nissessenap wants to merge 3 commits into
Conversation
ecosyste.ms passes package metadata through as-is, so license strings from registries that do not enforce SPDX (Maven being the usual offender) end up in the SBOM verbatim, leaving a concluded license that does not validate. The SPDX path also joined multiple licenses with a comma, which is not an operator, so any multi-license package was invalid regardless of the strings involved. Each license string is now resolved against the SPDX license list. Valid identifiers are normalized and kept; anything else becomes a LicenseRef- with the original captured as extracted licensing info, so the valid identifiers in a mixed set survive. CycloneDX keeps emitting an expression when everything is valid, and falls back to a license list naming the rest when it is not, since it cannot mix the two forms. Validation uses github/go-spdx, which carries the license list offline and so keeps enrichment working without network access. It requires cobra 1.10.2, hence the transitive bump. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
| Status | Scan Engine | Total (0) | ||||
|---|---|---|---|---|---|---|
| Open Source Security | 0 | 0 | 0 | 0 | See details | |
| Licenses | 0 | 0 | 0 | 0 | See details | |
| ✅ | Code Security | 0 | 0 | 0 | 0 | 0 issues |
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.
Review follow-up on the SPDX license work. The versioned license field is comma separated, so "The Apache Software License, Version 2.0" split into two licenses and enrichment emitted a choice between "The Apache Software License" and a license named "Version 2.0" - valid SPDX, but wrong, and it is the exact string issue snyk#80 opens with. A fragment that is just a version now rejoins the name it belongs to. CycloneDX validates license ids against a closed list that kept only six of the "or later" identifiers, while SPDX allows a "+" suffix on any of them, so a license such as Apache-2.0+ was emitted as an id no CycloneDX consumer accepts. Those are named instead. Licenses are also deduplicated on what they resolve to rather than on the incoming string, so a package listing both MIT and mit no longer concludes (MIT OR MIT), and the identifier a LicenseRef falls back to is re-derived until it is free rather than once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collapse the hand-rolled character map and "--" loop in licenseRefID into one regexp pass, drop the unreachable case in LicenseExpression, and reduce the extracted-licensing-info rehash loop to a single suffix attempt, whose ceiling is now named in a comment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Fixes #80.
Problem
ecosyste.ms passes package metadata through as-is, so license strings from registries that do not enforce SPDX (Maven being the usual offender) land in the SBOM verbatim, leaving a concluded license that does not validate.
While in there, a second instance of the same bug: the SPDX path joined multiple licenses with a comma, which is not an SPDX operator, so any multi-license package was invalid regardless of the strings involved.
Approach
Each license string is resolved against the SPDX license list, per the algorithm suggested in the issue:
apache-2.0→Apache-2.0). Valid expressions andWITHexceptions pass through.LicenseRef-<sanitized original>, with the original string captured as extracted licensing info.Validation is per license string rather than per package, so the valid identifiers in a mixed set survive:
Refs are declared once per document and shared between packages. Two different strings that sanitize to the same identifier get disambiguated, so a package is never put under a license it is not under.
CycloneDX keeps emitting an
expressionwhen everything is valid, and falls back to alicenseslist —idfor the valid,namefor the rest — when it is not, since CycloneDX cannot mix the two forms.Dependency
Validation uses github/go-spdx, per @goneall's recommendation on the issue. It carries the license list offline, so enrichment keeps working without network access. It requires cobra 1.10.2, hence the transitive bump in
go.mod.Notes for reviewers
Three deliberate limitations, flagged rather than papered over:
licenses[]entries may be read as all applying at once. AnexpressioncontainingLicenseRef-would be worse: CycloneDX has no equivalent ofhasExtractedLicensingInfos, so the ref would dangle undefined.licensesis a comma-separated field, so a single license name containing a comma splits in two —"The Apache Software License, Version 2.0"yields two refs. Pre-existing behaviour; rejoining the fragments needs a heuristic. Happy to add one if you would rather.NONE/NOASSERTIONare dropped rather than joined, since neither is legal inside an expression. The field is left untouched, as it already is when ecosyste.ms has no license data.Testing
ClassifyLicensesis table-tested over the valid, invalid, normalized, expression, exception, incoming-ref and empty cases; both enrichers have tests for the mixed-validity path, and the ref-collision case has a regression test. Existing expectations updated for the two output changes: SPDX multi-license is now anORexpression rather than comma-joined, and a single license is no longer wrapped in redundant parentheses.lib/snyktests hang onmainas well as on this branch — unrelated, they appear to want network access.🤖 Generated with Claude Code