Skip to content

Commit

Permalink
fix!: invalid spdx licenses
Browse files Browse the repository at this point in the history
BREAKING CHANGE:
* Enriching a CycloneDX document with licenses will no longer generate SPDX license expressions, instead parlay will add individual license entries with the "id" field populated if the licenses are valid SPDX identifiers, or with the "name" field populated otherwise
* When enriching an SPDX document with licenses, if any of the licenses aren't valid SPDX identifiers parlay will create "OtherLicense" entries in the SBOM and reference those in the "licenseConcluded" expressions
  • Loading branch information
paulrosca-snyk committed Dec 19, 2024
1 parent bc55e4d commit e676204
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 30 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ require (
github.com/avast/retry-go v3.0.0+incompatible // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/fsnotify/fsnotify v1.6.0 // indirect
github.com/github/go-spdx/v2 v2.3.2 // indirect
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
github.com/hashicorp/hcl v1.0.0 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ github.com/frankban/quicktest v1.14.3 h1:FJKSZTDHjyhriyC81FLQ0LY93eSai0ZyR/ZIkd3
github.com/frankban/quicktest v1.14.3/go.mod h1:mgiwOwqx65TmIk1wJ6Q7wvnVMocbUorkibMOrVTHZps=
github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY=
github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw=
github.com/github/go-spdx/v2 v2.3.2 h1:IfdyNHTqzs4zAJjXdVQfRnxt1XMfycXoHBE2Vsm1bjs=
github.com/github/go-spdx/v2 v2.3.2/go.mod h1:2ZxKsOhvBp+OYBDlsGnUMcchLeo2mrpEBn2L1C+U3IQ=
github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
Expand Down
15 changes: 12 additions & 3 deletions internal/utils/spdx.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package utils

import (
"fmt"
"slices"
"strings"

"github.com/github/go-spdx/v2/spdxexp"
"github.com/package-url/packageurl-go"
spdx_2_3 "github.com/spdx/tools-golang/spdx/v2/v2_3"

Expand Down Expand Up @@ -32,9 +34,16 @@ func GetPurlFromSPDXPackage(pkg *spdx_2_3.Package) (*packageurl.PackageURL, erro
return &purl, nil
}

func GetSPDXLicenseExpressionFromEcosystemsLicense(data *packages.Version) string {
func GetSPDXLicensesFromEcosystemsLicense(data *packages.Version) (valid []string, invalid []string) {
if data == nil || data.Licenses == nil || *data.Licenses == "" {
return ""
return nil, nil
}
return fmt.Sprintf("(%s)", strings.Join(strings.Split(*data.Licenses, ","), " OR "))
licenses := strings.Split(*data.Licenses, ",")
_, invalid = spdxexp.ValidateLicenses(licenses)
for _, lic := range licenses {
if !slices.Contains(invalid, lic) {
valid = append(valid, lic)
}
}
return valid, invalid
}
45 changes: 31 additions & 14 deletions internal/utils/spdx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,48 @@ import (
"github.com/stretchr/testify/assert"
)

func TestGetSPDXLicenseExpressionFromEcosystemsLicense(t *testing.T) {
func TestGetSPDXLicensesFromEcosystemsLicense(t *testing.T) {
assert := assert.New(t)
licenses := "GPLv2,MIT"
licenses := "MIT,AGPL-3.0-or-later,Unknown,AGPL-1.0"
data := packages.Version{Licenses: &licenses}
expression := utils.GetSPDXLicenseExpressionFromEcosystemsLicense(&data)
assert.Equal("(GPLv2 OR MIT)", expression)

validLics, invalidLics := utils.GetSPDXLicensesFromEcosystemsLicense(&data)

assert.Len(validLics, 3)
assert.Equal(validLics[0], "MIT")
assert.Equal(validLics[1], "AGPL-3.0-or-later")
assert.Equal(validLics[2], "AGPL-1.0")

assert.Len(invalidLics, 1)
assert.Equal(invalidLics[0], "Unknown")
}

func TestGetSPDXLicenseExpressionFromEcosystemsLicense_NoData(t *testing.T) {
func TestGetSPDXLicensesFromEcosystemsLicense_NoData(t *testing.T) {
assert := assert.New(t)
expression := utils.GetSPDXLicenseExpressionFromEcosystemsLicense(nil)
assert.Equal("", expression)

validLics, invalidLics := utils.GetSPDXLicensesFromEcosystemsLicense(nil)

assert.Len(validLics, 0)
assert.Len(invalidLics, 0)
}

func TestGetSPDXLicenseExpressionFromEcosystemsLicense_NoLicenses(t *testing.T) {
func TestGetSPDXLicensesFromEcosystemsLicense_NoLicenses(t *testing.T) {
assert := assert.New(t)
data := packages.Version{}
expression := utils.GetSPDXLicenseExpressionFromEcosystemsLicense(&data)
assert.Equal("", expression)
data := packages.Version{Licenses: nil}

validLics, invalidLics := utils.GetSPDXLicensesFromEcosystemsLicense(&data)

assert.Len(validLics, 0)
assert.Len(invalidLics, 0)
}

func TestGetSPDXLicenseExpressionFromEcosystemsLicense_EmptyLicenses(t *testing.T) {
func TestGetSPDXLicensesFromEcosystemsLicense_EmptyLicenses(t *testing.T) {
assert := assert.New(t)
licenses := ""
data := packages.Version{Licenses: &licenses}
expression := utils.GetSPDXLicenseExpressionFromEcosystemsLicense(&data)
assert.Equal("", expression)

validLics, invalidLics := utils.GetSPDXLicensesFromEcosystemsLicense(&data)

assert.Len(validLics, 0)
assert.Len(invalidLics, 0)
}
14 changes: 10 additions & 4 deletions lib/ecosystems/enrich_cyclonedx.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,17 @@ func enrichCDXDescription(comp *cdx.Component, data *packages.Package) {
}

func enrichCDXLicense(comp *cdx.Component, data *packages.Version) {
expression := utils.GetSPDXLicenseExpressionFromEcosystemsLicense(data)
if expression != "" {
licenses := cdx.LicenseChoice{Expression: expression}
comp.Licenses = &cdx.Licenses{licenses}
validLics, invalidLics := utils.GetSPDXLicensesFromEcosystemsLicense(data)
var licenses cdx.Licenses
for _, licenseID := range validLics {
license := cdx.License{ID: licenseID}
licenses = append(licenses, cdx.LicenseChoice{License: &license})
}
for _, licenseName := range invalidLics {
license := cdx.License{Name: licenseName}
licenses = append(licenses, cdx.LicenseChoice{License: &license})
}
comp.Licenses = &licenses
}

func enrichExternalReference(comp *cdx.Component, url *string, refType cdx.ExternalReferenceType) {
Expand Down
6 changes: 4 additions & 2 deletions lib/ecosystems/enrich_cyclonedx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ func TestEnrichSBOM_CycloneDX(t *testing.T) {
component := components[0]
licenses := *component.Licenses

comp := cdx.LicenseChoice(cdx.LicenseChoice{Expression: "(MIT)"})
cdxLic := cdx.License{ID: "MIT"}
comp := cdx.LicenseChoice(cdx.LicenseChoice{License: &cdxLic})

assert.Equal(t, "description", components[0].Description)
assert.Equal(t, comp, licenses[0])
Expand Down Expand Up @@ -198,7 +199,8 @@ func TestEnrichLicense(t *testing.T) {
enrichCDXLicense(component, pack)

licenses := *component.Licenses
comp := cdx.LicenseChoice(cdx.LicenseChoice{Expression: "(BSD-3-Clause)"})
cdxLic := cdx.License{ID: "BSD-3-Clause"}
comp := cdx.LicenseChoice(cdx.LicenseChoice{License: &cdxLic})
assert.Equal(t, comp, licenses[0])
}

Expand Down
28 changes: 23 additions & 5 deletions lib/ecosystems/enrich_spdx.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ package ecosystems

import (
"errors"
"fmt"
"slices"
"strings"

"github.com/google/uuid"
"github.com/package-url/packageurl-go"
"github.com/rs/zerolog"
"github.com/spdx/tools-golang/spdx"
Expand All @@ -31,6 +35,7 @@ import (

func enrichSPDX(bom *spdx.Document, logger *zerolog.Logger) {
packages := bom.Packages
licenses := &bom.OtherLicenses

logger.Debug().Msgf("Detected %d packages", len(packages))

Expand Down Expand Up @@ -64,7 +69,7 @@ func enrichSPDX(bom *spdx.Document, logger *zerolog.Logger) {
continue
}

enrichSPDXLicense(pkg, pkgVersionData)
enrichSPDXLicense(pkg, pkgVersionData, licenses)
}
}

Expand Down Expand Up @@ -96,11 +101,24 @@ func enrichSPDXSupplier(pkg *v2_3.Package, data *packages.Package) {
}
}

func enrichSPDXLicense(pkg *v2_3.Package, data *packages.Version) {
expression := utils.GetSPDXLicenseExpressionFromEcosystemsLicense(data)
if expression != "" {
pkg.PackageLicenseConcluded = *data.Licenses
func enrichSPDXLicense(pkg *v2_3.Package, data *packages.Version, licenses *[]*v2_3.OtherLicense) {
validLics, invalidLics := utils.GetSPDXLicensesFromEcosystemsLicense(data)
for _, lic := range invalidLics {
var spdxOtherLic *v2_3.OtherLicense
idx := slices.IndexFunc(*licenses, func(l *v2_3.OtherLicense) bool { return l.LicenseName == lic })
if idx == -1 {
spdxOtherLic = &v2_3.OtherLicense{
LicenseIdentifier: fmt.Sprintf("LicenseRef-%s", uuid.NewString()),
LicenseName: lic,
ExtractedText: lic,
}
*licenses = append(*licenses, spdxOtherLic)
} else {
spdxOtherLic = (*licenses)[idx]
}
validLics = append(validLics, spdxOtherLic.LicenseIdentifier)
}
pkg.PackageLicenseConcluded = fmt.Sprintf("(%s)", strings.Join(validLics, " OR "))
}

func enrichSPDXHomepage(pkg *v2_3.Package, data *packages.Package) {
Expand Down
8 changes: 6 additions & 2 deletions lib/ecosystems/enrich_spdx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package ecosystems

import (
"bytes"
"fmt"
"net/http"
"testing"

Expand All @@ -39,7 +40,7 @@ func TestEnrichSBOM_SPDX(t *testing.T) {
func(r *http.Request) (*http.Response, error) {
return httpmock.NewJsonResponse(200, map[string]interface{}{
// This is the license we expect to see for the specific package version
"licenses": "MIT",
"licenses": "MIT,Unknown",
})
},
)
Expand Down Expand Up @@ -86,8 +87,11 @@ func TestEnrichSBOM_SPDX(t *testing.T) {

pkgs := bom.Packages

lics := bom.OtherLicenses
assert.Len(t, lics, 1)

assert.Equal(t, "description", pkgs[0].PackageDescription)
assert.Equal(t, "MIT", pkgs[0].PackageLicenseConcluded)
assert.Equal(t, fmt.Sprintf("(MIT OR %s)", lics[0].LicenseIdentifier), pkgs[0].PackageLicenseConcluded)
assert.Equal(t, "https://github.com/spdx/tools-golang", pkgs[0].PackageHomePage)
assert.Equal(t, "Organization", pkgs[0].PackageSupplier.SupplierType)
assert.Equal(t, "Acme Corp", pkgs[0].PackageSupplier.Supplier)
Expand Down

0 comments on commit e676204

Please sign in to comment.