Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sbom): Use UUID as BomRef for packages with empty purl #5448

Merged
merged 3 commits into from
Nov 6, 2023
Merged

fix(sbom): Use UUID as BomRef for packages with empty purl #5448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c5c487
fix: skip packages with empty purl
DmitriyLewen Oct 26, 2023
b272408
refactor: update comment
DmitriyLewen Oct 26, 2023
9fdceaf
refactor: return pointer to PackageURL for NewPackageURL()
DmitriyLewen Oct 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/k8s/scanner/scanner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func clusterInfoToReportResources(allArtifact []*artifacts.Artifact) (*core.Comp
return nil, xerrors.Errorf("failed to create PURL: %w", err)
}
imageComponents = append(imageComponents, &core.Component{
PackageURL: &imagePURL,
PackageURL: imagePURL,
Type: cdx.ComponentTypeContainer,
Name: name,
Version: cDigest,
Expand Down
13 changes: 8 additions & 5 deletions pkg/purl/purl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func (p *PackageURL) BOMRef() string {
}

// nolint: gocyclo
func NewPackageURL(t ftypes.TargetType, metadata types.Metadata, pkg ftypes.Package) (PackageURL, error) {
func NewPackageURL(t ftypes.TargetType, metadata types.Metadata, pkg ftypes.Package) (*PackageURL, error) {
var qualifiers packageurl.Qualifiers
if metadata.OS != nil {
qualifiers = parseQualifier(pkg)
Expand Down Expand Up @@ -235,7 +235,7 @@ func NewPackageURL(t ftypes.TargetType, metadata types.Metadata, pkg ftypes.Pack
case packageurl.TypeGolang:
namespace, name = parseGolang(name)
if name == "" {
return PackageURL{PackageURL: *packageurl.NewPackageURL("", "", "", "", nil, "")}, nil
return nil, nil
}
case packageurl.TypeNPM:
namespace, name = parseNpm(name)
Expand All @@ -246,12 +246,15 @@ func NewPackageURL(t ftypes.TargetType, metadata types.Metadata, pkg ftypes.Pack
case packageurl.TypeOCI:
purl, err := parseOCI(metadata)
if err != nil {
return PackageURL{}, err
return nil, err
}
return PackageURL{PackageURL: purl}, nil
if purl.Type == "" {
return nil, nil
}
return &PackageURL{PackageURL: purl}, nil
}

return PackageURL{
return &PackageURL{
PackageURL: *packageurl.NewPackageURL(ptype, namespace, name, ver, qualifiers, subpath),
FilePath: pkg.FilePath,
}, nil
Expand Down
56 changes: 21 additions & 35 deletions pkg/purl/purl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestNewPackageURL(t *testing.T) {
typ ftypes.TargetType
pkg ftypes.Package
metadata types.Metadata
want purl.PackageURL
want *purl.PackageURL
wantErr string
}{
{
Expand All @@ -30,7 +30,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "org.springframework:spring-core",
Version: "5.3.14",
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeMaven,
Namespace: "org.springframework",
Expand All @@ -46,7 +46,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "org.springframework:spring-core",
Version: "5.3.14",
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeMaven,
Namespace: "org.springframework",
Expand All @@ -62,7 +62,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "@xtuc/ieee754",
Version: "1.2.0",
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeNPM,
Namespace: "@xtuc",
Expand All @@ -78,7 +78,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "lodash",
Version: "4.17.21",
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeNPM,
Name: "lodash",
Expand All @@ -93,7 +93,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "@xtuc/ieee754",
Version: "1.2.0",
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeNPM,
Namespace: "@xtuc",
Expand All @@ -109,7 +109,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "lodash",
Version: "4.17.21",
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeNPM,
Name: "lodash",
Expand All @@ -124,7 +124,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "Django_test",
Version: "1.2.0",
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypePyPi,
Name: "django-test",
Expand All @@ -139,7 +139,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "absl-py",
Version: "0.4.1",
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeConda,
Name: "absl-py",
Expand All @@ -154,7 +154,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "symfony/contracts",
Version: "v1.0.2",
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeComposer,
Namespace: "symfony",
Expand All @@ -170,7 +170,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "github.com/go-sql-driver/Mysql",
Version: "v1.5.0",
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeGolang,
Namespace: "github.com/go-sql-driver",
Expand All @@ -186,14 +186,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "./private_repos/cnrm.googlesource.com/cnrm/",
Version: "(devel)",
},
want: purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: "",
Namespace: "",
Name: "",
Version: "",
},
},
want: nil,
},
{
name: "hex package",
Expand All @@ -209,7 +202,7 @@ func TestNewPackageURL(t *testing.T) {
},
},
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeHex,
Name: "bunt",
Expand All @@ -224,7 +217,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "http",
Version: "0.13.2",
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: purl.TypeDart,
Name: "http",
Expand All @@ -240,7 +233,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "github.com/apple/swift-atomics",
Version: "1.1.0",
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeSwift,
Namespace: "github.com/apple",
Expand All @@ -257,7 +250,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "GoogleUtilities/NSData+zlib",
Version: "7.5.2",
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeCocoapods,
Name: "GoogleUtilities",
Expand All @@ -274,7 +267,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "abomination",
Version: "0.7.3",
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeCargo,
Name: "abomination",
Expand Down Expand Up @@ -304,7 +297,7 @@ func TestNewPackageURL(t *testing.T) {
Name: "8",
},
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeRPM,
Namespace: "redhat",
Expand Down Expand Up @@ -342,7 +335,7 @@ func TestNewPackageURL(t *testing.T) {
Architecture: "amd64",
},
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeOCI,
Namespace: "",
Expand Down Expand Up @@ -372,14 +365,7 @@ func TestNewPackageURL(t *testing.T) {
},
ImageID: "sha256:8fe1727132b2506c17ba0e1f6a6ed8a016bb1f5735e43b2738cd3fd1979b6260",
},
want: purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: "",
Namespace: "",
Name: "",
Version: "",
},
},
want: nil,
},
{
name: "container with implicit registry",
Expand All @@ -397,7 +383,7 @@ func TestNewPackageURL(t *testing.T) {
Architecture: "amd64",
},
},
want: purl.PackageURL{
want: &purl.PackageURL{
PackageURL: packageurl.PackageURL{
Type: packageurl.TypeOCI,
Namespace: "",
Expand Down
3 changes: 3 additions & 0 deletions pkg/report/github/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,5 +165,8 @@ func buildPurl(t ftypes.TargetType, pkg ftypes.Package) (string, error) {
if err != nil {
return "", xerrors.Errorf("purl error: %w", err)
}
if packageUrl == nil {
return "", nil
}
return packageUrl.ToString(), nil
}
28 changes: 20 additions & 8 deletions pkg/sbom/cyclonedx/marshal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,11 @@ func (e *Marshaler) marshalPackage(pkg Package, pkgs map[string]Package, compone
if err != nil {
return nil, xerrors.Errorf("failed to parse pkg: %w", err)
}

// Skip component that can't be converted from `Package`
if component == nil {
return nil, nil
}
components[pkg.ID] = component

// Iterate dependencies
Expand Down Expand Up @@ -234,8 +239,9 @@ func (e *Marshaler) rootComponent(r types.Report) (*core.Component, error) {
p, err := purl.NewPackageURL(purl.TypeOCI, r.Metadata, ftypes.Package{})
if err != nil {
return nil, xerrors.Errorf("failed to new package url for oci: %w", err)
} else if p.Type != "" {
root.PackageURL = &p
}
if p != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Is there any reason to make it an independent if-statement rather than else if?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see point of using else if, because we return error when err != nil.
And independent if seems to me more readable.

root.PackageURL = p
}

case ftypes.ArtifactVM:
Expand Down Expand Up @@ -315,11 +321,17 @@ func pkgComponent(pkg Package) (*core.Component, error) {
}

name := pkg.Name
version := pkg.Version
var group string
// use `group` field for GroupID and `name` for ArtifactID for jar files
if pkg.Type == ftypes.Jar {
name = pu.Name
group = pu.Namespace
// there are cases when we can't build purl
// e.g. local Go packages
if pu != nil {
version = pu.Version
knqyf263 marked this conversation as resolved.
Show resolved Hide resolved
// use `group` field for GroupID and `name` for ArtifactID for jar files
if pkg.Type == ftypes.Jar {
name = pu.Name
group = pu.Namespace
}
}

properties := []core.Property{
Expand Down Expand Up @@ -369,8 +381,8 @@ func pkgComponent(pkg Package) (*core.Component, error) {
Type: cdx.ComponentTypeLibrary,
Name: name,
Group: group,
Version: pu.Version,
PackageURL: &pu,
Version: version,
PackageURL: pu,
Supplier: pkg.Maintainer,
Licenses: pkg.Licenses,
Hashes: lo.Ternary(pkg.Digest == "", nil, []digest.Digest{pkg.Digest}),
Expand Down
23 changes: 23 additions & 0 deletions pkg/sbom/cyclonedx/marshal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,11 @@ func TestMarshaler_Marshal(t *testing.T) {
Name: "golang.org/x/crypto",
Version: "v0.0.0-20210421170649-83a5a9bb288b",
},
// dependency has been replaced with local directory
{
Name: "./api",
Version: "(devel)",
},
},
},
},
Expand Down Expand Up @@ -302,6 +307,19 @@ func TestMarshaler_Marshal(t *testing.T) {
},
},
},
{
// Use UUID for local Go packages
BOMRef: "3ff14136-e09f-4df9-80ea-000000000007",
Type: cdx.ComponentTypeLibrary,
Name: "./api",
Version: "(devel)",
Properties: &[]cdx.Property{
{
Name: "aquasecurity:trivy:PkgType",
Value: "gobinary",
},
},
},
{
BOMRef: "pkg:gem/actioncontroller@7.0.0",
Type: cdx.ComponentTypeLibrary,
Expand Down Expand Up @@ -441,9 +459,14 @@ func TestMarshaler_Marshal(t *testing.T) {
{
Ref: "3ff14136-e09f-4df9-80ea-000000000006",
Dependencies: &[]string{
"3ff14136-e09f-4df9-80ea-000000000007",
"pkg:golang/golang.org/x/crypto@v0.0.0-20210421170649-83a5a9bb288b",
},
},
{
Ref: "3ff14136-e09f-4df9-80ea-000000000007",
Dependencies: lo.ToPtr([]string{}),
},
{
Ref: "pkg:gem/actioncontroller@7.0.0",
Dependencies: &[]string{
Expand Down
4 changes: 2 additions & 2 deletions pkg/sbom/spdx/marshal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func (m *Marshaler) rootPackage(r types.Report, pkgDownloadLocation string) (*sp
// When the target is a container image, add PURL to the external references of the root package.
if p, err := purl.NewPackageURL(purl.TypeOCI, r.Metadata, ftypes.Package{}); err != nil {
return nil, xerrors.Errorf("failed to new package url for oci: %w", err)
} else if p.Type != "" {
} else if p != nil {
externalReferences = append(externalReferences, purlExternalReference(p.ToString()))
}

Expand Down Expand Up @@ -327,7 +327,7 @@ func (m *Marshaler) pkgToSpdxPackage(t ftypes.TargetType, pkgDownloadLocation st
}

var pkgExtRefs []*spdx.PackageExternalReference
if packageURL.Type != "" {
if packageURL != nil {
pkgExtRefs = []*spdx.PackageExternalReference{purlExternalReference(packageURL.String())}
}

Expand Down