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): duplicate dependson #3261

Merged
merged 2 commits into from
Dec 4, 2022
Merged

fix(sbom): duplicate dependson #3261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ac9a4d4
fix(sbom): skip seen package dependency
masahiro331 Dec 3, 2022
2e8d4fc
fix(sbom): changed not to aggregate
masahiro331 Dec 3, 2022
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
7 changes: 3 additions & 4 deletions pkg/sbom/cyclonedx/marshal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ func (e *Marshaler) marshalComponents(r types.Report, bomRef string) (*[]cdx.Com
pkgID := packageID(result.Target, pkg.Name, utils.FormatVersion(pkg), pkg.FilePath)
if _, ok := bomRefMap[pkgID]; !ok {
bomRefMap[pkgID] = pkgComponent.BOMRef
componentDependencies = append(componentDependencies, cdx.Dependency{Ref: pkgComponent.BOMRef})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why could it be duplicated? componentDependencies is defined per types.Result. The duplicate should not exist in Result.Packages.

}

// When multiple lock files have the same dependency with the same name and version,
Expand All @@ -227,8 +228,6 @@ func (e *Marshaler) marshalComponents(r types.Report, bomRef string) (*[]cdx.Com
// TODO: All packages are flattened at the moment. We should construct dependency tree.
components = append(components, pkgComponent)
}

componentDependencies = append(componentDependencies, cdx.Dependency{Ref: pkgComponent.BOMRef})
}

for _, vuln := range result.Vulnerabilities {
Expand All @@ -247,8 +246,8 @@ func (e *Marshaler) marshalComponents(r types.Report, bomRef string) (*[]cdx.Com
}
}

if result.Type == ftypes.NodePkg || result.Type == ftypes.PythonPkg || result.Type == ftypes.GoBinary ||
result.Type == ftypes.GemSpec || result.Type == ftypes.Jar || result.Type == ftypes.RustBinary {
if result.Type == ftypes.NodePkg || result.Type == ftypes.PythonPkg ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this is the root. We aggregate Go/Rust binaries by mistake.

Copy link

Choose a reason for hiding this comment

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

Are we no longer aggregating these components with this change, or would this only filter duplicates?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We no longer aggregate dependencies in Go/Rust binaries. It should have been a mistake.

Copy link

Choose a reason for hiding this comment

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

Oh no. I was relying on this feature in my project. Any ideas for enabling go/rust binary parsing while generating SBoM with the new release?

Copy link
Contributor Author

@masahiro331 masahiro331 Dec 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

No problem.

This feature modifies packages that depend on binaries so that they are not aggregated.

Before. (Aggregate package with the golang)

golang
 -  package A
 -  package B
 -  package C

After (Aggregate package with the package)

Binary A 
- package A
- package B

Binary B 
- package C

Copy link
Collaborator

Choose a reason for hiding this comment

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

We just don't aggregate those dependencies. You are still able to see those packages in SBOM.

Copy link

Choose a reason for hiding this comment

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

Thank you, @masahiro331 and @knqyf263, for the detailed explanation!

result.Type == ftypes.GemSpec || result.Type == ftypes.Jar {
// If a package is language-specific package that isn't associated with a lock file,
// it will be a dependency of a component under "metadata".
// e.g.
Expand Down
51 changes: 51 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 @@ -153,6 +153,17 @@ func TestMarshaler_Marshal(t *testing.T) {
},
},
},
{
Target: "usr/local/bin/tfsec",
Class: types.ClassLangPkg,
Type: ftypes.GoBinary,
Packages: []ftypes.Package{
{
Name: "golang.org/x/crypto",
Version: "v0.0.0-20210421170649-83a5a9bb288b",
},
},
},
},
},
want: &cdx.BOM{
Expand Down Expand Up @@ -335,6 +346,35 @@ func TestMarshaler_Marshal(t *testing.T) {
},
},
},
{
BOMRef: "pkg:golang/golang.org/x/crypto@v0.0.0-20210421170649-83a5a9bb288b",
Type: cdx.ComponentTypeLibrary,
Name: "golang.org/x/crypto",
Version: "v0.0.0-20210421170649-83a5a9bb288b",
PackageURL: "pkg:golang/golang.org/x/crypto@v0.0.0-20210421170649-83a5a9bb288b",
Properties: &[]cdx.Property{
{
Name: "aquasecurity:trivy:PkgType",
Value: "gobinary",
},
},
},
{
BOMRef: "3ff14136-e09f-4df9-80ea-000000000006",
Type: cdx.ComponentTypeApplication,
Name: "usr/local/bin/tfsec",
Version: "",
Properties: &[]cdx.Property{
{
Name: "aquasecurity:trivy:Type",
Value: "gobinary",
},
{
Name: "aquasecurity:trivy:Class",
Value: "lang-pkgs",
},
},
},
},
Dependencies: &[]cdx.Dependency{
{
Expand Down Expand Up @@ -372,6 +412,14 @@ func TestMarshaler_Marshal(t *testing.T) {
},
},
},
{
Ref: "3ff14136-e09f-4df9-80ea-000000000006",
Dependencies: &[]cdx.Dependency{
{
Ref: "pkg:golang/golang.org/x/crypto@v0.0.0-20210421170649-83a5a9bb288b",
},
},
},
{
Ref: "pkg:oci/rails@sha256:a27fd8080b517143cbbbab9dfb7c8571c40d67d534bbdee55bd6c473f432b177?repository_url=index.docker.io%2Flibrary%2Frails&arch=arm64",
Dependencies: &[]cdx.Dependency{
Expand All @@ -387,6 +435,9 @@ func TestMarshaler_Marshal(t *testing.T) {
{
Ref: "3ff14136-e09f-4df9-80ea-000000000005",
},
{
Ref: "3ff14136-e09f-4df9-80ea-000000000006",
},
},
},
},
Expand Down