fix: SPDX license values and download location (#2007)

Signed-off-by: Keith Zantow <kzantow@gmail.com>

syft/formats/common/spdxhelpers/license.go

@@ -1,6 +1,8 @@
 package spdxhelpers
 
 import (
+	"crypto/sha256"
+	"fmt"
 	"strings"
 
 	"github.com/anchore/syft/internal/spdxlicense"
@@ -27,29 +29,18 @@ func License(p pkg.Package) (concluded, declared string) {
 	// https://spdx.github.io/spdx-spec/v2.3/SPDX-license-expressions/
 	pc, pd := parseLicenses(p.Licenses.ToSlice())
 
-	for i, v := range pc {
-		if strings.HasPrefix(v, spdxlicense.LicenseRefPrefix) {
-			pc[i] = SanitizeElementID(v)
-		}
-	}
-
-	for i, v := range pd {
-		if strings.HasPrefix(v, spdxlicense.LicenseRefPrefix) {
-			pd[i] = SanitizeElementID(v)
-		}
-	}
-
 	return joinLicenses(pc), joinLicenses(pd)
 }
 
-func joinLicenses(licenses []string) string {
+func joinLicenses(licenses []spdxLicense) string {
 	if len(licenses) == 0 {
 		return NOASSERTION
 	}
 
 	var newLicenses []string
 
-	for _, v := range licenses {
+	for _, l := range licenses {
+		v := l.id
 		// check if license does not start or end with parens
 		if !strings.HasPrefix(v, "(") && !strings.HasSuffix(v, ")") {
 			// if license contains AND, OR, or WITH, then wrap in parens
@@ -66,14 +57,31 @@ func joinLicenses(licenses []string) string {
 	return strings.Join(newLicenses, " AND ")
 }
 
-func parseLicenses(raw []pkg.License) (concluded, declared []string) {
+type spdxLicense struct {
+	id    string
+	value string
+}
+
+func parseLicenses(raw []pkg.License) (concluded, declared []spdxLicense) {
 	for _, l := range raw {
-		var candidate string
+		if l.Value == "" {
+			continue
+		}
+
+		candidate := spdxLicense{}
 		if l.SPDXExpression != "" {
-			candidate = l.SPDXExpression
+			candidate.id = l.SPDXExpression
 		} else {
 			// we did not find a valid SPDX license ID so treat as separate license
-			candidate = spdxlicense.LicenseRefPrefix + l.Value
+			if len(l.Value) <= 64 {
+				// if the license text is less than the size of the hash,
+				// just use it directly so the id is more readable
+				candidate.id = spdxlicense.LicenseRefPrefix + SanitizeElementID(l.Value)
+			} else {
+				hash := sha256.Sum256([]byte(l.Value))
+				candidate.id = fmt.Sprintf("%s%x", spdxlicense.LicenseRefPrefix, hash)
+			}
+			candidate.value = l.Value
 		}
 
 		switch l.Type {
@@ -83,5 +91,6 @@ func parseLicenses(raw []pkg.License) (concluded, declared []string) {
 			declared = append(declared, candidate)
 		}
 	}
+
 	return concluded, declared
 }

syft/formats/common/spdxhelpers/license_test.go

@@ -1,11 +1,16 @@
 package spdxhelpers
 
 import (
+	"strings"
 	"testing"
 
+	"github.com/spdx/tools-golang/spdx"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
+	"github.com/anchore/syft/internal/spdxlicense"
 	"github.com/anchore/syft/syft/pkg"
+	"github.com/anchore/syft/syft/sbom"
 )
 
 func Test_License(t *testing.T) {
@@ -103,6 +108,77 @@ func Test_License(t *testing.T) {
 	}
 }
 
+func Test_otherLicenses(t *testing.T) {
+	pkg1 := pkg.Package{
+		Name:    "first-pkg",
+		Version: "1.1",
+		Licenses: pkg.NewLicenseSet(
+			pkg.NewLicense("MIT"),
+		),
+	}
+	pkg2 := pkg.Package{
+		Name:    "second-pkg",
+		Version: "2.2",
+		Licenses: pkg.NewLicenseSet(
+			pkg.NewLicense("non spdx license"),
+		),
+	}
+	bigText := `
+                                 Apache License
+                           Version 2.0, January 2004`
+	pkg3 := pkg.Package{
+		Name:    "third-pkg",
+		Version: "3.3",
+		Licenses: pkg.NewLicenseSet(
+			pkg.NewLicense(bigText),
+		),
+	}
+
+	tests := []struct {
+		name     string
+		packages []pkg.Package
+		expected []*spdx.OtherLicense
+	}{
+		{
+			name:     "no other licenses when all valid spdx expressions",
+			packages: []pkg.Package{pkg1},
+			expected: nil,
+		},
+		{
+			name:     "other licenses includes original text",
+			packages: []pkg.Package{pkg2},
+			expected: []*spdx.OtherLicense{
+				{
+					LicenseIdentifier: "LicenseRef-non-spdx-license",
+					ExtractedText:     "non spdx license",
+				},
+			},
+		},
+		{
+			name:     "big licenses get hashed",
+			packages: []pkg.Package{pkg3},
+			expected: []*spdx.OtherLicense{
+				{
+					LicenseIdentifier: "LicenseRef-e9a1e42833d3e456f147052f4d312101bd171a0798893169fe596ca6b55c049e",
+					ExtractedText:     bigText,
+				},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			s := sbom.SBOM{
+				Artifacts: sbom.Artifacts{
+					Packages: pkg.NewCollection(test.packages...),
+				},
+			}
+			got := ToFormatModel(s)
+			require.Equal(t, test.expected, got.OtherLicenses)
+		})
+	}
+}
+
 func Test_joinLicenses(t *testing.T) {
 	tests := []struct {
 		name string
@@ -122,7 +198,18 @@ func Test_joinLicenses(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			assert.Equalf(t, tt.want, joinLicenses(tt.args), "joinLicenses(%v)", tt.args)
+			assert.Equalf(t, tt.want, joinLicenses(toSpdxLicenses(tt.args)), "joinLicenses(%v)", tt.args)
 		})
 	}
 }
+
+func toSpdxLicenses(ids []string) (licenses []spdxLicense) {
+	for _, l := range ids {
+		license := spdxLicense{id: l}
+		if strings.HasPrefix(l, spdxlicense.LicenseRefPrefix) {
+			license.value = l
+		}
+		licenses = append(licenses, license)
+	}
+	return licenses
+}

syft/formats/common/spdxhelpers/to_format_model.go

@@ -245,6 +245,7 @@ func toRootPackage(s source.Description) *spdx.Package {
 		PackageSupplier: &spdx.Supplier{
 			Supplier: NOASSERTION,
 		},
+		PackageDownloadLocation: NOASSERTION,
 	}
 
 	if purl != nil {
@@ -703,32 +704,31 @@ func toFileTypes(metadata *file.Metadata) (ty []string) {
 // other licenses are for licenses from the pkg.Package that do not have an SPDXExpression
 // field. The spdxexpression field is only filled given a validated Value field.
 func toOtherLicenses(catalog *pkg.Collection) []*spdx.OtherLicense {
-	licenses := map[string]bool{}
-	for _, p := range catalog.Sorted() {
+	licenses := map[string]spdxLicense{}
+
+	for p := range catalog.Enumerate() {
 		declaredLicenses, concludedLicenses := parseLicenses(p.Licenses.ToSlice())
-		for _, license := range declaredLicenses {
-			if strings.HasPrefix(license, spdxlicense.LicenseRefPrefix) {
-				licenses[license] = true
+		for _, l := range declaredLicenses {
+			if l.value != "" {
+				licenses[l.id] = l
 			}
 		}
-		for _, license := range concludedLicenses {
-			if strings.HasPrefix(license, spdxlicense.LicenseRefPrefix) {
-				licenses[license] = true
+		for _, l := range concludedLicenses {
+			if l.value != "" {
+				licenses[l.id] = l
 			}
 		}
 	}
 
 	var result []*spdx.OtherLicense
 
-	sorted := maps.Keys(licenses)
-	slices.Sort(sorted)
-	for _, license := range sorted {
-		// separate the found value from the prefix
-		// this only contains licenses that are not found on the SPDX License List
-		name := strings.TrimPrefix(license, spdxlicense.LicenseRefPrefix)
+	ids := maps.Keys(licenses)
+	slices.Sort(ids)
+	for _, id := range ids {
+		license := licenses[id]
 		result = append(result, &spdx.OtherLicense{
-			LicenseIdentifier: SanitizeElementID(license),
-			ExtractedText:     name,
+			LicenseIdentifier: license.id,
+			ExtractedText:     license.value,
 		})
 	}
 	return result

syft/formats/common/spdxhelpers/to_syft_model.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/anchore/packageurl-go"
 	"github.com/anchore/syft/internal/log"
+	"github.com/anchore/syft/internal/spdxlicense"
 	"github.com/anchore/syft/syft/artifact"
 	"github.com/anchore/syft/syft/cpe"
 	"github.com/anchore/syft/syft/file"
@@ -495,10 +496,7 @@ func parseSPDXLicenses(p *spdx.Package) []pkg.License {
 }
 
 func cleanSPDXID(id string) string {
-	if strings.HasPrefix(id, "LicenseRef-") {
-		return strings.TrimPrefix(id, "LicenseRef-")
-	}
-	return id
+	return strings.TrimPrefix(id, spdxlicense.LicenseRefPrefix)
 }
 
 //nolint:funlen

syft/formats/spdxjson/test-fixtures/snapshot/TestSPDXJSONDirectoryEncoder.golden

@@ -65,7 +65,7 @@
    "name": "some/path",
    "SPDXID": "SPDXRef-DocumentRoot-Directory-some-path",
    "supplier": "NOASSERTION",
-   "downloadLocation": "",
+   "downloadLocation": "NOASSERTION",
    "filesAnalyzed": false,
    "primaryPackagePurpose": "FILE"
   }

syft/formats/spdxjson/test-fixtures/snapshot/TestSPDXJSONImageEncoder.golden

@@ -66,7 +66,7 @@
    "SPDXID": "SPDXRef-DocumentRoot-Image-user-image-input",
    "versionInfo": "sha256:2731251dc34951c0e50fcc643b4c5f74922dad1a5d98f302b504cf46cd5d9368",
    "supplier": "NOASSERTION",
-   "downloadLocation": "",
+   "downloadLocation": "NOASSERTION",
    "filesAnalyzed": false,
    "checksums": [
     {

syft/formats/spdxjson/test-fixtures/snapshot/TestSPDXRelationshipOrder.golden

@@ -66,7 +66,7 @@
    "SPDXID": "SPDXRef-DocumentRoot-Image-user-image-input",
    "versionInfo": "sha256:2731251dc34951c0e50fcc643b4c5f74922dad1a5d98f302b504cf46cd5d9368",
    "supplier": "NOASSERTION",
-   "downloadLocation": "",
+   "downloadLocation": "NOASSERTION",
    "filesAnalyzed": false,
    "checksums": [
     {

syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXJSONSPDXIDs.golden

@@ -13,6 +13,7 @@ Created: redacted
 PackageName: foobar/baz
 SPDXID: SPDXRef-DocumentRoot-Directory-foobar-baz
 PackageSupplier: NOASSERTION
+PackageDownloadLocation: NOASSERTION
 PrimaryPackagePurpose: FILE
 FilesAnalyzed: false
 

syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXRelationshipOrder.golden

@@ -52,6 +52,7 @@ PackageName: user-image-input
 SPDXID: SPDXRef-DocumentRoot-Image-user-image-input
 PackageVersion: sha256:2731251dc34951c0e50fcc643b4c5f74922dad1a5d98f302b504cf46cd5d9368
 PackageSupplier: NOASSERTION
+PackageDownloadLocation: NOASSERTION
 PrimaryPackagePurpose: CONTAINER
 FilesAnalyzed: false
 PackageChecksum: SHA256: 2731251dc34951c0e50fcc643b4c5f74922dad1a5d98f302b504cf46cd5d9368

syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXTagValueDirectoryEncoder.golden

@@ -13,6 +13,7 @@ Created: redacted
 PackageName: some/path
 SPDXID: SPDXRef-DocumentRoot-Directory-some-path
 PackageSupplier: NOASSERTION
+PackageDownloadLocation: NOASSERTION
 PrimaryPackagePurpose: FILE
 FilesAnalyzed: false
 

syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXTagValueImageEncoder.golden

@@ -14,6 +14,7 @@ PackageName: user-image-input
 SPDXID: SPDXRef-DocumentRoot-Image-user-image-input
 PackageVersion: sha256:2731251dc34951c0e50fcc643b4c5f74922dad1a5d98f302b504cf46cd5d9368
 PackageSupplier: NOASSERTION
+PackageDownloadLocation: NOASSERTION
 PrimaryPackagePurpose: CONTAINER
 FilesAnalyzed: false
 PackageChecksum: SHA256: 2731251dc34951c0e50fcc643b4c5f74922dad1a5d98f302b504cf46cd5d9368

test/cli/spdx_tooling_validation_test.go

@@ -85,7 +85,11 @@ func TestSpdxValidationTooling(t *testing.T) {
 
 				validateCmd := exec.Command("make", "validate", fileArg, mountArg, imageArg)
 				validateCmd.Dir = filepath.Join(cwd, "test-fixtures", "image-java-spdx-tools")
-				runAndShow(t, validateCmd)
+
+				stdout, stderr, err := runCommand(validateCmd, map[string]string{})
+				if err != nil {
+					t.Fatalf("invalid SPDX document:%v\nSTDOUT:\n%s\nSTDERR:\n%s", err, stdout, stderr)
+				}
 			})
 		}
 	}

test/cli/test-fixtures/image-java-spdx-tools/Dockerfile

@@ -1,6 +1,6 @@
 FROM openjdk:11
 
-RUN wget https://github.com/spdx/tools-java/releases/download/v1.1.3/tools-java-1.1.3.zip && \
+RUN wget --no-verbose https://github.com/spdx/tools-java/releases/download/v1.1.3/tools-java-1.1.3.zip && \
 	unzip tools-java-1.1.3.zip && \
 	rm tools-java-1.1.3.zip
 

test/cli/utils_test.go

@@ -1,11 +1,9 @@
 package cli
 
 import (
-	"bufio"
 	"bytes"
 	"flag"
 	"fmt"
-	"io"
 	"math"
 	"os"
 	"os/exec"
@@ -17,8 +15,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/require"
-
 	"github.com/anchore/stereoscope/pkg/imagetest"
 )
 
@@ -32,30 +28,6 @@ func logOutputOnFailure(t testing.TB, cmd *exec.Cmd, stdout, stderr string) {
 	}
 }
 
-func runAndShow(t *testing.T, cmd *exec.Cmd) {
-	t.Helper()
-
-	stderr, err := cmd.StderrPipe()
-	require.NoErrorf(t, err, "could not get stderr: +v", err)
-
-	stdout, err := cmd.StdoutPipe()
-	require.NoErrorf(t, err, "could not get stdout: +v", err)
-
-	err = cmd.Start()
-	require.NoErrorf(t, err, "failed to start cmd: %+v", err)
-
-	show := func(label string, reader io.ReadCloser) {
-		scanner := bufio.NewScanner(reader)
-		scanner.Split(bufio.ScanLines)
-		for scanner.Scan() {
-			t.Logf("%s: %s", label, scanner.Text())
-		}
-	}
-
-	show("out", stdout)
-	show("err", stderr)
-}
-
 func setupPKI(t *testing.T, pw string) func() {
 	err := os.Setenv("COSIGN_PASSWORD", pw)
 	if err != nil {