From a3e263195d94b7bd5c4ab799421871e1c55c1b67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Meira?= Date: Wed, 8 Jan 2025 12:44:54 +0000 Subject: [PATCH 1/3] clean: Unencode BOM ref and PURL for components in SBOM files [TAROT-3083] --- internal/tool/tool.go | 35 ++++++++++++++- internal/tool/tool_test.go | 88 +++++++++++++++++++++++++++++++++----- 2 files changed, 110 insertions(+), 13 deletions(-) diff --git a/internal/tool/tool.go b/internal/tool/tool.go index bf6273d..0e732b2 100644 --- a/internal/tool/tool.go +++ b/internal/tool/tool.go @@ -11,13 +11,14 @@ import ( "path/filepath" "strings" + cdx "github.com/CycloneDX/cyclonedx-go" dbTypes "github.com/aquasecurity/trivy-db/pkg/types" "github.com/aquasecurity/trivy/pkg/fanal/secret" ftypes "github.com/aquasecurity/trivy/pkg/fanal/types" "github.com/aquasecurity/trivy/pkg/flag" "github.com/aquasecurity/trivy/pkg/log" tresult "github.com/aquasecurity/trivy/pkg/result" - "github.com/aquasecurity/trivy/pkg/sbom/cyclonedx" + tcdx "github.com/aquasecurity/trivy/pkg/sbom/cyclonedx" ptypes "github.com/aquasecurity/trivy/pkg/types" codacy "github.com/codacy/codacy-engine-golang-seed/v6" "github.com/codacy/codacy-trivy/internal" @@ -219,12 +220,13 @@ func (t codacyTrivy) getVulnerabilities(ctx context.Context, report ptypes.Repor // getSBOM produces a SBOM result from `report`. func (t codacyTrivy) getSBOM(ctx context.Context, report ptypes.Report) (codacy.SBOM, error) { - marshaler := cyclonedx.NewMarshaler(internal.TrivyVersion()) + marshaler := tcdx.NewMarshaler(internal.TrivyVersion()) bom, err := marshaler.MarshalReport(ctx, report) if err != nil { return codacy.SBOM{}, &ToolError{msg: "Failed to run Codacy Trivy", w: err} } + unencodeComponents(bom) return codacy.SBOM{BOM: *bom}, nil } @@ -400,6 +402,35 @@ func findLeastDisruptiveFixedVersion(vuln ptypes.DetectedVulnerability) string { return vuln.FixedVersion } +// unencodeComponentes uses URL unencoding on component PURLs and BOMRefs to helps downstream consumers of the SBOM file. +// +// This function mutates the provided BOM. +func unencodeComponents(bom *cdx.BOM) { + components := *bom.Components + for i, component := range components { + if purl, err := url.PathUnescape(component.PackageURL); err == nil { + components[i].PackageURL = purl + } + if bomRef, err := url.PathUnescape(component.BOMRef); err == nil { + components[i].BOMRef = bomRef + } + } + + dependencies := *bom.Dependencies + for i, dependency := range dependencies { + if ref, err := url.PathUnescape(dependency.Ref); err == nil { + dependencies[i].Ref = ref + } + + dDependencies := *dependency.Dependencies + for j, dDependency := range dDependencies { + if d, err := url.PathUnescape(dDependency); err == nil { + dDependencies[j] = d + } + } + } +} + // Remove the pkg: prefix and url-decode the PURL for display purposes. func purlPrettyPrint(purl packageurl.PackageURL) string { purlStripPkg := strings.TrimPrefix(purl.ToString(), "pkg:") diff --git a/internal/tool/tool_test.go b/internal/tool/tool_test.go index 86ab077..2315042 100644 --- a/internal/tool/tool_test.go +++ b/internal/tool/tool_test.go @@ -37,8 +37,8 @@ func TestRun(t *testing.T) { ctx := context.Background() ctrl := gomock.NewController(t) - package1Purl := packageurl.NewPackageURL("type", "namespace", "package-1", "version", nil, "") - package2Purl := packageurl.NewPackageURL("type", "namespace", "package-2", "version", nil, "") + package1Purl := packageurl.NewPackageURL("type", "@namespace", "package-1", "version+incompatible", nil, "") + package2Purl := packageurl.NewPackageURL("type", "@namespace", "package-2", "version+RC", nil, "") // Create a temporary file with a secret srcDir, err := os.MkdirTemp("", "") @@ -111,15 +111,22 @@ func TestRun(t *testing.T) { }, }, Identifier: ftypes.PkgIdentifier{ - PURL: package1Purl, + BOMRef: package1Purl.String(), + PURL: package1Purl, + UID: package1Purl.String(), }, + Relationship: ftypes.RelationshipDirect, }, { Identifier: ftypes.PkgIdentifier{ - PURL: package2Purl, + BOMRef: package2Purl.String(), + PURL: package2Purl, + UID: package2Purl.String(), }, + Relationship: ftypes.RelationshipDirect, }, }, + Class: ptypes.ClassLangPkg, Vulnerabilities: []ptypes.DetectedVulnerability{ // Will generate an issue { @@ -214,13 +221,13 @@ func TestRun(t *testing.T) { File: fileName, Line: 1, PatternID: ruleIDVulnerability, - Message: "Insecure dependency type/namespace/package-1@version (vuln id: vuln title) (update to vuln fixed)", + Message: "Insecure dependency type/@namespace/package-1@version+incompatible (vuln id: vuln title) (update to vuln fixed)", }, { File: fileName, Line: 1, PatternID: ruleIDVulnerability, - Message: "Insecure dependency type/namespace/package-1@version (vuln id no fixed version: vuln no fixed version) (no fix available)", + Message: "Insecure dependency type/@namespace/package-1@version+incompatible (vuln id no fixed version: vuln no fixed version) (no fix available)", }, { File: fileName, @@ -259,6 +266,9 @@ func TestRun(t *testing.T) { }) assert.ElementsMatch(t, expectedFileErrors, fileErrors) + expectedMetadataComponentBOMRef := "b804b498-f626-41c5-a47f-45e1471acf33" + expectedRootComponentBOMRef := "d16d6083-4370-442f-a6ab-c5146a215dbe" + expectedRooComponentName := "file-802713450" expectedSBOM := codacy.SBOM{ BOM: cyclonedx.BOM{ XMLNS: "http://cyclonedx.org/schema/bom/1.6", @@ -280,7 +290,7 @@ func TestRun(t *testing.T) { }, }, Component: &cyclonedx.Component{ - BOMRef: "b804b498-f626-41c5-a47f-45e1471acf33", // different every run + BOMRef: expectedMetadataComponentBOMRef, Type: "application", Properties: &[]cyclonedx.Property{ { @@ -290,10 +300,56 @@ func TestRun(t *testing.T) { }, }, }, - Components: &[]cyclonedx.Component{}, + Components: &[]cyclonedx.Component{ + { + BOMRef: expectedRootComponentBOMRef, + Type: "application", + Name: "file-802713450", + Properties: &[]cyclonedx.Property{ + { + Name: "aquasecurity:trivy:Class", + Value: "lang-pkgs", + }, + { + Name: "aquasecurity:trivy:Type", + }, + }, + }, + { + BOMRef: "pkg:type/@namespace/package-1@version+incompatible", + Type: "library", + Properties: &[]cyclonedx.Property{}, + PackageURL: "pkg:type/@namespace/package-1@version+incompatible", + Version: "version+incompatible", + }, + { + BOMRef: "pkg:type/@namespace/package-2@version+RC", + Type: "library", + Properties: &[]cyclonedx.Property{}, + PackageURL: "pkg:type/@namespace/package-2@version+RC", + Version: "version+RC", + }, + }, Dependencies: &[]cyclonedx.Dependency{ { - Ref: "b804b498-f626-41c5-a47f-45e1471acf33", + Ref: expectedMetadataComponentBOMRef, + Dependencies: &[]string{ + expectedRootComponentBOMRef, + }, + }, + { + Ref: expectedRootComponentBOMRef, + Dependencies: &[]string{ + "pkg:type/@namespace/package-1@version+incompatible", + "pkg:type/@namespace/package-2@version+RC", + }, + }, + { + Ref: "pkg:type/@namespace/package-1@version+incompatible", + Dependencies: &[]string{}, + }, + { + Ref: "pkg:type/@namespace/package-2@version+RC", Dependencies: &[]string{}, }, }, @@ -308,6 +364,18 @@ func TestRun(t *testing.T) { return false } }) + + // Set values that change on every run to known values.`` + // This allows us to test the relationship between components. + sboms[0].(codacy.SBOM).Metadata.Component.BOMRef = expectedMetadataComponentBOMRef + cs := *sboms[0].(codacy.SBOM).Components + cs[0].BOMRef = expectedRootComponentBOMRef + cs[0].Name = expectedRooComponentName + ds := *sboms[0].(codacy.SBOM).Dependencies + ds[0].Ref = expectedMetadataComponentBOMRef + ds[0].Dependencies = &[]string{expectedRootComponentBOMRef} + ds[1].Ref = expectedRootComponentBOMRef + // Only one SBOM result is produced assert.Len(t, sboms, 1) assert.True( @@ -319,8 +387,6 @@ func TestRun(t *testing.T) { // Ignore fields that change each run cmpopts.IgnoreFields(codacy.SBOM{}, "SerialNumber"), cmpopts.IgnoreFields(cyclonedx.Metadata{}, "Timestamp"), - cmpopts.IgnoreFields(cyclonedx.Component{}, "BOMRef"), - cmpopts.IgnoreFields(cyclonedx.Dependency{}, "Ref"), }, ), ) From 92d96af3eea95bff347eda888753f2e7d6444813 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Meira?= Date: Wed, 8 Jan 2025 13:43:25 +0000 Subject: [PATCH 2/3] clean: Address review comments [TAROT-3083] --- internal/tool/tool.go | 3 ++- internal/tool/tool_test.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/tool/tool.go b/internal/tool/tool.go index 0e732b2..d2efbce 100644 --- a/internal/tool/tool.go +++ b/internal/tool/tool.go @@ -402,7 +402,8 @@ func findLeastDisruptiveFixedVersion(vuln ptypes.DetectedVulnerability) string { return vuln.FixedVersion } -// unencodeComponentes uses URL unencoding on component PURLs and BOMRefs to helps downstream consumers of the SBOM file. +// unencodeComponents decodes URL-encoded fields (`PackageURL`, `BOMRef`) in components and dependencies +// to help downstream consumers of the SBOM file. // // This function mutates the provided BOM. func unencodeComponents(bom *cdx.BOM) { diff --git a/internal/tool/tool_test.go b/internal/tool/tool_test.go index 2315042..fe597c0 100644 --- a/internal/tool/tool_test.go +++ b/internal/tool/tool_test.go @@ -365,7 +365,7 @@ func TestRun(t *testing.T) { } }) - // Set values that change on every run to known values.`` + // Set values that change on every run to known values. // This allows us to test the relationship between components. sboms[0].(codacy.SBOM).Metadata.Component.BOMRef = expectedMetadataComponentBOMRef cs := *sboms[0].(codacy.SBOM).Components From ff4efd3902d6d092a9a8e4ab4aa8bfe86af660aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Meira?= Date: Wed, 8 Jan 2025 16:42:54 +0000 Subject: [PATCH 3/3] test: Ensure correct order and values before comparison [TAROT-3083] Without this the test was flaky. --- internal/tool/tool_test.go | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/internal/tool/tool_test.go b/internal/tool/tool_test.go index fe597c0..ddcd68d 100644 --- a/internal/tool/tool_test.go +++ b/internal/tool/tool_test.go @@ -6,6 +6,8 @@ import ( "context" "os" "path/filepath" + "slices" + "strings" "testing" "github.com/CycloneDX/cyclonedx-go" @@ -367,14 +369,28 @@ func TestRun(t *testing.T) { // Set values that change on every run to known values. // This allows us to test the relationship between components. + oldMetadataComponentBOMRef := sboms[0].(codacy.SBOM).Metadata.Component.BOMRef sboms[0].(codacy.SBOM).Metadata.Component.BOMRef = expectedMetadataComponentBOMRef + // Components are always in declaration order, with the root component (created automatically) coming first cs := *sboms[0].(codacy.SBOM).Components + oldRootComponentBOMRef := cs[0].BOMRef cs[0].BOMRef = expectedRootComponentBOMRef cs[0].Name = expectedRooComponentName + // Dependencies are not always in order we must take care to change the correct value ds := *sboms[0].(codacy.SBOM).Dependencies - ds[0].Ref = expectedMetadataComponentBOMRef - ds[0].Dependencies = &[]string{expectedRootComponentBOMRef} - ds[1].Ref = expectedRootComponentBOMRef + for i, d := range ds { + if d.Ref == oldMetadataComponentBOMRef { + ds[i].Ref = expectedMetadataComponentBOMRef + ds[i].Dependencies = &[]string{expectedRootComponentBOMRef} + } + if d.Ref == oldRootComponentBOMRef { + ds[i].Ref = expectedRootComponentBOMRef + } + } + // Ensure dependencies array is as we expect it, otherwise comparison fails + slices.SortFunc(ds, func(a cyclonedx.Dependency, b cyclonedx.Dependency) int { + return strings.Compare(a.Ref, b.Ref) + }) // Only one SBOM result is produced assert.Len(t, sboms, 1)