From c7272fd6a5417f4181f72293a169e394013e5165 Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Tue, 8 Aug 2023 15:55:50 -0400 Subject: [PATCH] fix: SPDX license values and download location (#2007) Signed-off-by: Keith Zantow --- syft/formats/common/spdxhelpers/license.go | 45 ++++++---- .../common/spdxhelpers/license_test.go | 89 ++++++++++++++++++- .../common/spdxhelpers/to_format_model.go | 32 +++---- .../common/spdxhelpers/to_syft_model.go | 6 +- .../TestSPDXJSONDirectoryEncoder.golden | 2 +- .../snapshot/TestSPDXJSONImageEncoder.golden | 2 +- .../snapshot/TestSPDXRelationshipOrder.golden | 2 +- .../snapshot/TestSPDXJSONSPDXIDs.golden | 1 + .../snapshot/TestSPDXRelationshipOrder.golden | 1 + .../TestSPDXTagValueDirectoryEncoder.golden | 1 + .../TestSPDXTagValueImageEncoder.golden | 1 + test/cli/spdx_tooling_validation_test.go | 6 +- .../image-java-spdx-tools/Dockerfile | 2 +- test/cli/utils_test.go | 28 ------ 14 files changed, 146 insertions(+), 72 deletions(-) diff --git a/syft/formats/common/spdxhelpers/license.go b/syft/formats/common/spdxhelpers/license.go index 4b0d896a1d1..e3352f27c54 100644 --- a/syft/formats/common/spdxhelpers/license.go +++ b/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 } diff --git a/syft/formats/common/spdxhelpers/license_test.go b/syft/formats/common/spdxhelpers/license_test.go index cb4e614e445..623f43aee79 100644 --- a/syft/formats/common/spdxhelpers/license_test.go +++ b/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 +} diff --git a/syft/formats/common/spdxhelpers/to_format_model.go b/syft/formats/common/spdxhelpers/to_format_model.go index 9ad5a2d858a..258c96f8f82 100644 --- a/syft/formats/common/spdxhelpers/to_format_model.go +++ b/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 diff --git a/syft/formats/common/spdxhelpers/to_syft_model.go b/syft/formats/common/spdxhelpers/to_syft_model.go index 5ef331b22ae..c17b6367ed0 100644 --- a/syft/formats/common/spdxhelpers/to_syft_model.go +++ b/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 diff --git a/syft/formats/spdxjson/test-fixtures/snapshot/TestSPDXJSONDirectoryEncoder.golden b/syft/formats/spdxjson/test-fixtures/snapshot/TestSPDXJSONDirectoryEncoder.golden index 7107f906ded..dac57a1da9e 100644 --- a/syft/formats/spdxjson/test-fixtures/snapshot/TestSPDXJSONDirectoryEncoder.golden +++ b/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" } diff --git a/syft/formats/spdxjson/test-fixtures/snapshot/TestSPDXJSONImageEncoder.golden b/syft/formats/spdxjson/test-fixtures/snapshot/TestSPDXJSONImageEncoder.golden index 955d910a63b..73e27667e00 100644 --- a/syft/formats/spdxjson/test-fixtures/snapshot/TestSPDXJSONImageEncoder.golden +++ b/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": [ { diff --git a/syft/formats/spdxjson/test-fixtures/snapshot/TestSPDXRelationshipOrder.golden b/syft/formats/spdxjson/test-fixtures/snapshot/TestSPDXRelationshipOrder.golden index d6fff364e37..80e5b269a46 100644 --- a/syft/formats/spdxjson/test-fixtures/snapshot/TestSPDXRelationshipOrder.golden +++ b/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": [ { diff --git a/syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXJSONSPDXIDs.golden b/syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXJSONSPDXIDs.golden index 2916b240f00..84f12702502 100644 --- a/syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXJSONSPDXIDs.golden +++ b/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 diff --git a/syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXRelationshipOrder.golden b/syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXRelationshipOrder.golden index ceda8d5eaf7..764ac9830c1 100644 --- a/syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXRelationshipOrder.golden +++ b/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 diff --git a/syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXTagValueDirectoryEncoder.golden b/syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXTagValueDirectoryEncoder.golden index ff168e71fbb..e50194535d6 100644 --- a/syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXTagValueDirectoryEncoder.golden +++ b/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 diff --git a/syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXTagValueImageEncoder.golden b/syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXTagValueImageEncoder.golden index 34d428afcd9..3c640ea726e 100644 --- a/syft/formats/spdxtagvalue/test-fixtures/snapshot/TestSPDXTagValueImageEncoder.golden +++ b/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 diff --git a/test/cli/spdx_tooling_validation_test.go b/test/cli/spdx_tooling_validation_test.go index 2e37d5a5b17..17c81fa4d42 100644 --- a/test/cli/spdx_tooling_validation_test.go +++ b/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) + } }) } } diff --git a/test/cli/test-fixtures/image-java-spdx-tools/Dockerfile b/test/cli/test-fixtures/image-java-spdx-tools/Dockerfile index b32d68b9e5e..36a3da7b6ea 100644 --- a/test/cli/test-fixtures/image-java-spdx-tools/Dockerfile +++ b/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 diff --git a/test/cli/utils_test.go b/test/cli/utils_test.go index e8a2218c5ae..e7b824d7ff0 100644 --- a/test/cli/utils_test.go +++ b/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 {