fix(spdx): use the `hasExtractedLicensingInfos` field for licenses that are not listed in the SPDX #8077

DmitriyLewen · 2024-12-10T13:14:58Z

Description

For cases where the SPDX license list does not contain the detected license, we should use the hasExtractedLicensingInfos field.
See #7721 for more details.

Before:

➜ trivy -q image bitnami/wordpress:6.6.2-debian-12-r14 --format spdx-json --cache-backend memory | grep '"name": "libsasl2-2",' -A 8 
      "name": "libsasl2-2",
      "SPDXID": "SPDXRef-Package-eb685f3647d2fa92",
...
      "licenseConcluded": "BSD-4-Clause AND OpenSSL AND SSLeay AND BSD-3-Clause AND BSD-2-Clause AND GPL-3.0-or-later AND GPL-3.0-only AND BSD-4-Clause-UC AND RSA-MD AND BSD-4-clause-KTH AND text:--BSD-4-clause AND IBM-as-is AND BSD-3-clause-JANET AND BSD-3-clause-PADL AND MIT-OpenVision AND OpenLDAP AND FSFULLR AND MIT-CMU AND MIT-Export AND BSD-2.2-clause AND text:--IBM-as-is",
      "licenseDeclared": "BSD-4-Clause AND OpenSSL AND SSLeay AND BSD-3-Clause AND BSD-2-Clause AND GPL-3.0-or-later AND GPL-3.0-only AND BSD-4-Clause-UC AND RSA-MD AND BSD-4-clause-KTH AND text:--BSD-4-clause AND IBM-as-is AND BSD-3-clause-JANET AND BSD-3-clause-PADL AND MIT-OpenVision AND OpenLDAP AND FSFULLR AND MIT-CMU AND MIT-Export AND BSD-2.2-clause AND text:--IBM-as-is",

After:

➜  ./trivy -q image bitnami/wordpress:6.6.2-debian-12-r14 --format spdx-json --cache-backend memory | grep '"name": "libsasl2-2",' -A 8
      "name": "libsasl2-2",
      "SPDXID": "SPDXRef-Package-eb685f3647d2fa92",
...
      "licenseConcluded": "BSD-4-Clause AND OpenSSL AND LicenseRef-ea55a018594e6bf5 AND BSD-3-Clause AND BSD-2-Clause AND GPL-3.0-or-later AND GPL-3.0-only AND BSD-4-Clause-UC AND LicenseRef-65f52c543487922d AND LicenseRef-a02a4c35e67d69da AND LicenseRef-148e27c4af509b1f AND LicenseRef-6f1acaa4d7f6c64a AND LicenseRef-f3daae6e2ead6f1 AND LicenseRef-90cfa1054f767716 AND LicenseRef-d336b55b7573f941 AND LicenseRef-7955e7acaf9b5cc3 AND LicenseRef-17a620271d638d1c AND LicenseRef-157e923866327e93 AND LicenseRef-5a72bdaf3db99c53 AND LicenseRef-d5845310a6a8e792",
      "licenseDeclared": "BSD-4-Clause AND OpenSSL AND LicenseRef-ea55a018594e6bf5 AND BSD-3-Clause AND BSD-2-Clause AND GPL-3.0-or-later AND GPL-3.0-only AND BSD-4-Clause-UC AND LicenseRef-65f52c543487922d AND LicenseRef-a02a4c35e67d69da AND LicenseRef-148e27c4af509b1f AND LicenseRef-6f1acaa4d7f6c64a AND LicenseRef-f3daae6e2ead6f1 AND LicenseRef-90cfa1054f767716 AND LicenseRef-d336b55b7573f941 AND LicenseRef-7955e7acaf9b5cc3 AND LicenseRef-17a620271d638d1c AND LicenseRef-157e923866327e93 AND LicenseRef-5a72bdaf3db99c53 AND LicenseRef-d5845310a6a8e792",
➜  ./trivy -q image bitnami/wordpress:6.6.2-debian-12-r14 --format spdx-json --cache-backend memory | grep '"licenseId": "LicenseRef-a02a4c35e67d69da",' -A 2
      "licenseId": "LicenseRef-a02a4c35e67d69da",
      "extractedText": "NOASSERTION",
      "name": "BSD-4-clause-KTH"
➜  ./trivy -q image bitnami/wordpress:6.6.2-debian-12-r14 --format spdx-json --cache-backend memory | grep '"licenseId": "LicenseRef-148e27c4af509b1f",' -A 2
      "licenseId": "LicenseRef-148e27c4af509b1f",
      "extractedText": "BSD-4-clause and IBM-as-is",
      "name": "NOASSERTION"

test runs for cron action:

Related issues

Close fix(spdx): use hasExtractedLicensingInfos for licenses not in the SPDX license list #7721

Checklist

I've read the guidelines for contributing to this repository.
I've followed the conventions in the PR title.
I've added tests that prove my fix is effective or that my feature works.
I've updated the documentation with the relevant information (if needed).
I've added usage information (if the PR introduces new options)
I've included a "before" and "after" example to the description (if the PR is a user interface change).

DmitriyLewen · 2024-12-11T06:43:44Z

pkg/sbom/spdx/marshal.go

+		return fmt.Sprintf("(%s)", license)
+	}), " AND ")
+
+	normalizedLicense, err := expression.Normalize(license, licensing.NormalizeLicense, expression.NormalizeForSPDX)


I wanted to override the expression.NormalizeForSPDX function, but we use With as a delimiter to parse licenses:

trivy/pkg/licensing/expression/parser.go.y

Line 14 in 6472e3c

%token<token> IDENT OR AND WITH

So there are problems with overwriting licenses with the wrong exception

DmitriyLewen · 2024-12-11T07:03:37Z

@goneall I created this PR to use the hasExtractedLicensingInfos field (as you wrote in #7716).
It would be great if you took a look if you have time

goneall

It looks like this code will fix the issue and provide compliance SPDX license expressions.

I had a few suggestions to produce a bit more understandable results and a few coding suggestions.

goneall · 2024-12-11T22:48:01Z

pkg/sbom/spdx/marshal.go

+	}
+	if text {
+		otherLicense.ExtractedText = license
+	} else {


Suggested change

} else {

I would always include the license as the license name - not just when the text is present

Second thought, this may be appropriate if the license text is the actual text of the license. In most cases, the metadata for packages includes text that are supposed to be a license name or identifier in which case it should also be in the name. If we know the license text is really the text, then the existing code is OK.

If, however, the license text is what is found in the package metadata files and they are not the actual text, I would add the same field as the name PLUS add a LicenseComment to explain - something like `otherLicense.LicenseComment = "The license text represents text found in package metadata and may not represent the full text of the license"

There are cases (e.g. license field from python METADATA files) when we can't understand that license is name/id/text.
We tried to detect text of license:

trivy/pkg/licensing/normalize.go

Lines 596 to 617 in f6acdf7

var licenseTextKeywords = []string{

"http://",

"https://",

"(c)",

"as-is",

";",

"hereby",

"permission to use",

"permission is",

"use in source",

"use, copy, modify",

"using",

}

func isLicenseText(str string) bool {

for _, keyword := range licenseTextKeywords {

if strings.Contains(str, keyword) {

return true

}

}

return false

}

So Trivy split license name/id and license text(text:// prefix).
That is why i used both LicenseName and ExtractedText fields:
https://github.com/DmitriyLewen/trivy/blob/f851f9bb18411db838fc65e0c6c4351b04953f8f/pkg/sbom/spdx/marshal_private_test.go#L92-L112

Another question - i used NOASSERTION for LicenseName and ExtractedText fields (see link above), because these fields are mandatory (https://github.com/spdx/tools-golang/blob/f6e45fdb9e4e0c993105f798bee5f8aa8ea70f84/spdx/v2/v2_3/other_license.go#L12-L20).
Is this correct?

add a LicenseComment to explain - something like `otherLicense.LicenseComment = "The license text represents text found in package metadata and may not represent the full text of the license"

I liked this idea 👍
Added in 041ab21

So Trivy split license name/id and license text(text:// prefix).
That is why i used both LicenseName and ExtractedText fields

Makes sense - I wasn't sure how the text was captured. Since you are capturing the text and name distinctly, your approach should work. BTW, it's a bit tricky to find the start and end of the license text and even trickier to match it to know licenses - something we've been working on in the SPDX java libraries for about 10 years and still don't have it perfected ;).

i used NOASSERTION for LicenseName and ExtractedText fields (see link above), because these fields are mandatory (https://github.com/spdx/tools-golang/blob/f6e45fdb9e4e0c993105f798bee5f8aa8ea70f84/spdx/v2/v2_3/other_license.go#L12-L20).
Is this correct?

For the license name, this is OK. The spec isn't very clear on how these should be treated, so many people make up a name based on the text. Unlike other parts of the spec, NOASSERTION is not required if you don't know - but in this case I think it would be fine to use NOASSERTION - for the name.

For the text, I would put in whatever string was actually found - even if it is the name. The definition of the field is the license text found - so if someone puts in "This software is licensed under mylicense" - you can put that exact string in the text field even though it is not technically the text of "mylicense". I wouldn't use "NOASSERTION" for the text field.

Got it! Thanks for your opinion!
Updated text field in 659f992

goneall · 2024-12-11T22:59:40Z

pkg/licensing/expression/category.go

+		"VSFTPD-OPENSSL-EXCEPTION",
+		"WXWINDOWS-EXCEPTION-3.1",
+		"X11VNC-OPENSSL-EXCEPTION",
+	}


This comment may deserve to be a separate suggestion, but in reading the code I would recommend building the license and exception IDs from the JSON files maintained by the SPDX legal team. The license list is updated every 3 months with new IDs and maintaining these in code can be a challenge to keep up and maintain. What I do in the code I maintain is attempt to access the current JSON files on the website https://spdx.org/licenses/licenses.json and https://spdx.org/licenses/exceptions.json. If I can not access the website or if the user specified not to use the online version, I'll use a cached version of the file.

There are cases when users run multiple times.
Downloading these files for each run is not good.
But we can save licenses.json and exceptions.json files in the cache dir and use them.
The files contain releaseDate field, so we can update this file only when releaseDate + 3 months has expired.

The license list is updated every 3 months

How strictly is this rule followed?

Anyway let's move this discussion into another issue/pr.

What I do in the code I maintain is attempt to access the current JSON files on the website https://spdx.org/licenses/licenses.json and https://spdx.org/licenses/exceptions.json

I found that https://spdx.org/licenses/exceptions.json and https://github.com/spdx/license-list-data/blob/592c2dcb8497c6fe829eea604045f77d3bce770b/json/exceptions.json are different (see harbour-exception).
Which file would be more correct to use?

How strictly is this rule followed?

Not very strictly. There is, however, a license list version field which is reliably incremented on release.

I found that https://spdx.org/licenses/exceptions.json and https://github.com/spdx/license-list-data/blob/592c2dcb8497c6fe829eea604045f77d3bce770b/json/exceptions.json are different (see harbour-exception).
Which file would be more correct to use?

The lists at https://spdx.org/licenses - these will always be the latest released version. The github repo master will have the latest in development version which may not be stable. The github repo is tagged with release versions, so if you go to the tag for the latest release in github, it will match what is on the website.

Okay, update exception list in 659f992

goneall · 2024-12-11T23:02:48Z

pkg/sbom/spdx/marshal.go

 	}
-	return NormalizeLicense(c.Licenses)
+	otherLicense.LicenseIdentifier = LicenseRefPrefix + "-" + licenseID


Minor suggestion - you could simplify this by including the "-" in the LicenseRefPrefix definition and just use LicenseRefPrefix + licenseID The string "LicenseRef" will likely never be used without the trailing "-".

This is our internal decision - we don't use - suffixes in constants :)

knqyf263 · 2024-12-17T06:23:29Z

pkg/licensing/expression/category.go

+	// SpdxLicenseExceptions contains all supported SPDX Exceptions
+	// cf. https://spdx.org/licenses/exceptions.json
+	// used `awk -F'"' '/"licenseExceptionId":/ {print toupper("\"" $4 "\"," )}' exceptions.json ` command


Since we need to keep it up-to-date, it should be done by mage spdx or something like that. I think we should create a separate file for the list and add

// Code generated by "mage spdx", DO NOT EDIT. // source: https://spdx.org/licenses/exceptions.json

to the header.

Do you want to check exceptions.json file in tests?
I mean the same as for mage docs:generate

This will help keep the file up-to-date, but can be noisy for PRs when a new version of file is released.
On the other hand, we can add a separate action to check the file's relevance once a week.

Using curl and awk through go generate is the easiest way, but some environments don't have curl, and CLI flags might be different. Ideally, we should do that in Go.

Do you want to check exceptions.json file in tests?

No, we don't need it for now. We can update the file when we notice that.

yes, that's what i planned
this command (using awk and other commands) is just a quick way to get all exceptions

we can add a separate action to check the file's relevance once a week.

This sounds better. Also, we don't need to fail the test. We can notify it on Microsoft Teams.

I updated PR.
Take a look, when you have time, please.

DmitriyLewen added 6 commits December 10, 2024 19:04

feat: add spdx licenses and exceptions + validation

6272fdd

feat: use otherLicense for non-spdx licenses

eb30b49

test: add/update unit tests

b1081e2

refactor: remove duplicates of otherLicenses

420c15e

refactor: always fill ExtractedText and LicenseName fields

b6c1d28

fix: tests

1bac2ca

DmitriyLewen commented Dec 11, 2024

View reviewed changes

add comment

f851f9b

DmitriyLewen marked this pull request as ready for review December 11, 2024 07:03

DmitriyLewen requested a review from knqyf263 as a code owner December 11, 2024 07:03

DmitriyLewen self-assigned this Dec 11, 2024

goneall reviewed Dec 11, 2024

View reviewed changes

DmitriyLewen added 4 commits December 12, 2024 13:26

feat: add LicenseComment field

041ab21

refactor: ExtractedText field for license with name

659f992

refactor: use exception list from spdx.org site

c25d840

test: update tests

20275c3

knqyf263 reviewed Dec 17, 2024

View reviewed changes

DmitriyLewen added 7 commits December 17, 2024 16:34

refactor: use exceptions.json file

c89c4e3

feat(mage): add command to create exceptions.json file

784db9e

feat(licensing): add exceptions.json file

ffb5067

fix(mage): fix typo

9f0f7bf

ci: add spdx-cron

c5563f5

ci: add aqua-installer

0d13108

fix: linter error

a2ebc32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(spdx): use the `hasExtractedLicensingInfos` field for licenses that are not listed in the SPDX #8077

fix(spdx): use the `hasExtractedLicensingInfos` field for licenses that are not listed in the SPDX #8077

DmitriyLewen commented Dec 10, 2024 •

edited

Loading

DmitriyLewen Dec 11, 2024

DmitriyLewen commented Dec 11, 2024

goneall left a comment

goneall Dec 11, 2024

goneall Dec 11, 2024

DmitriyLewen Dec 12, 2024

DmitriyLewen Dec 12, 2024

goneall Dec 12, 2024

DmitriyLewen Dec 13, 2024 •

edited

Loading

goneall Dec 11, 2024

DmitriyLewen Dec 12, 2024

goneall Dec 12, 2024

DmitriyLewen Dec 13, 2024

goneall Dec 11, 2024

DmitriyLewen Dec 12, 2024

knqyf263 Dec 17, 2024

DmitriyLewen Dec 17, 2024

knqyf263 Dec 17, 2024

knqyf263 Dec 17, 2024 •

edited

Loading

DmitriyLewen Dec 17, 2024

knqyf263 Dec 17, 2024

DmitriyLewen Dec 17, 2024

	var licenseTextKeywords = []string{
	"http://",
	"https://",
	"(c)",
	"as-is",
	";",
	"hereby",
	"permission to use",
	"permission is",
	"use in source",
	"use, copy, modify",
	"using",
	}

	func isLicenseText(str string) bool {
	for _, keyword := range licenseTextKeywords {
	if strings.Contains(str, keyword) {
	return true
	}
	}
	return false
	}

fix(spdx): use the hasExtractedLicensingInfos field for licenses that are not listed in the SPDX #8077

Are you sure you want to change the base?

fix(spdx): use the hasExtractedLicensingInfos field for licenses that are not listed in the SPDX #8077

Conversation

DmitriyLewen commented Dec 10, 2024 • edited Loading

Description

Related issues

Checklist

Choose a reason for hiding this comment

DmitriyLewen commented Dec 11, 2024

goneall left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DmitriyLewen Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knqyf263 Dec 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fix(spdx): use the `hasExtractedLicensingInfos` field for licenses that are not listed in the SPDX #8077

fix(spdx): use the `hasExtractedLicensingInfos` field for licenses that are not listed in the SPDX #8077

DmitriyLewen commented Dec 10, 2024 •

edited

Loading

DmitriyLewen Dec 13, 2024 •

edited

Loading

knqyf263 Dec 17, 2024 •

edited

Loading