fix: CycloneDX relationships not output or decoded properly (#1974)

Signed-off-by: Mark Galpin <[email protected]> Signed-off-by: Keith Zantow <[email protected]> Co-authored-by: Mark Galpin <[email protected]> Co-authored-by: Keith Zantow <[email protected]>
anchore · Aug 17, 2023 · 9467bd6 · 9467bd6
1 parent 5910732
commit 9467bd6
Show file tree

Hide file tree

Showing 5 changed files with 226 additions and 14 deletions.
diff --git a/syft/formats/common/cyclonedxhelpers/decoder.go b/syft/formats/common/cyclonedxhelpers/decoder.go
@@ -210,24 +210,33 @@ func collectRelationships(bom *cyclonedx.BOM, s *sbom.SBOM, idMap map[string]int
 		return
 	}
 	for _, d := range *bom.Dependencies {
-		to, fromExists := idMap[d.Ref].(artifact.Identifiable)
-		if !fromExists {
+		if d.Dependencies == nil {
 			continue
 		}
 
-		if d.Dependencies == nil {
+		toPtr, toExists := idMap[d.Ref]
+		if !toExists {
+			continue
+		}
+		to, ok := common.PtrToStruct(toPtr).(artifact.Identifiable)
+		if !ok {
 			continue
 		}
 
 		for _, t := range *d.Dependencies {
-			from, toExists := idMap[t].(artifact.Identifiable)
-			if !toExists {
+			fromPtr, fromExists := idMap[t]
+			if !fromExists {
+				continue
+			}
+			from, ok := common.PtrToStruct(fromPtr).(artifact.Identifiable)
+			if !ok {
 				continue
 			}
 			s.Relationships = append(s.Relationships, artifact.Relationship{
 				From: from,
 				To:   to,
-				Type: artifact.DependencyOfRelationship, // FIXME this information is lost
+				// match assumptions in encoding, that this is the only type of relationship captured:
+				Type: artifact.DependencyOfRelationship,
 			})
 		}
 	}

diff --git a/syft/formats/common/cyclonedxhelpers/decoder_test.go b/syft/formats/common/cyclonedxhelpers/decoder_test.go
@@ -8,8 +8,10 @@ import (
 
 	"github.com/CycloneDX/cyclonedx-go"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/anchore/syft/syft/artifact"
+	"github.com/anchore/syft/syft/pkg"
 	"github.com/anchore/syft/syft/sbom"
 )
 
@@ -336,3 +338,101 @@ func Test_missingComponentsDecode(t *testing.T) {
 
 	assert.NoError(t, err)
 }
+
+func Test_decodeDependencies(t *testing.T) {
+	c1 := cyclonedx.Component{
+		Name: "c1",
+	}
+
+	c2 := cyclonedx.Component{
+		Name: "c2",
+	}
+
+	c3 := cyclonedx.Component{
+		Name: "c3",
+	}
+
+	for _, c := range []*cyclonedx.Component{&c1, &c2, &c3} {
+		c.BOMRef = c.Name
+	}
+
+	setTypes := func(typ cyclonedx.ComponentType, components ...cyclonedx.Component) *[]cyclonedx.Component {
+		var out []cyclonedx.Component
+		for _, c := range components {
+			c.Type = typ
+			out = append(out, c)
+		}
+		return &out
+	}
+
+	tests := []struct {
+		name     string
+		sbom     cyclonedx.BOM
+		expected []string
+	}{
+		{
+			name: "dependencies decoded as dependencyOf relationships",
+			sbom: cyclonedx.BOM{
+				Components: setTypes(cyclonedx.ComponentTypeLibrary,
+					c1,
+					c2,
+					c3,
+				),
+				Dependencies: &[]cyclonedx.Dependency{
+					{
+						Ref: c1.BOMRef,
+						Dependencies: &[]string{
+							c2.BOMRef,
+							c3.BOMRef,
+						},
+					},
+				},
+			},
+			expected: []string{c2.Name, c3.Name},
+		},
+		{
+			name: "dependencies skipped with unhandled components",
+			sbom: cyclonedx.BOM{
+				Components: setTypes("",
+					c1,
+					c2,
+					c3,
+				),
+				Dependencies: &[]cyclonedx.Dependency{
+					{
+						Ref: c1.BOMRef,
+						Dependencies: &[]string{
+							c2.BOMRef,
+							c3.BOMRef,
+						},
+					},
+				},
+			},
+			expected: nil,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			s, err := ToSyftModel(&test.sbom)
+			require.NoError(t, err)
+			require.NotNil(t, s)
+
+			var deps []string
+			if s != nil {
+				for _, r := range s.Relationships {
+					if r.Type != artifact.DependencyOfRelationship {
+						continue
+					}
+					if p, ok := r.To.(pkg.Package); !ok || p.Name != c1.Name {
+						continue
+					}
+					if p, ok := r.From.(pkg.Package); ok {
+						deps = append(deps, p.Name)
+					}
+				}
+			}
+			require.Equal(t, test.expected, deps)
+		})
+	}
+}
diff --git a/syft/formats/common/cyclonedxhelpers/format.go b/syft/formats/common/cyclonedxhelpers/format.go
@@ -143,27 +143,29 @@ func toDependencies(relationships []artifact.Relationship) []cyclonedx.Dependenc
 	for _, r := range relationships {
 		exists := isExpressiblePackageRelationship(r.Type)
 		if !exists {
-			log.Debugf("unable to convert relationship from CycloneDX 1.4 JSON, dropping: %+v", r)
+			log.Debugf("unable to convert relationship type to CycloneDX JSON, dropping: %#v", r)
 			continue
 		}
 
 		// we only capture package-to-package relationships for now
-		fromPkg, ok := r.From.(*pkg.Package)
+		fromPkg, ok := r.From.(pkg.Package)
 		if !ok {
+			log.Tracef("unable to convert relationship fromPkg to CycloneDX JSON, dropping: %#v", r)
 			continue
 		}
 
-		toPkg, ok := r.To.(*pkg.Package)
+		toPkg, ok := r.To.(pkg.Package)
 		if !ok {
+			log.Tracef("unable to convert relationship toPkg to CycloneDX JSON, dropping: %#v", r)
 			continue
 		}
 
 		// ind dep
 
 		innerDeps := []string{}
-		innerDeps = append(innerDeps, deriveBomRef(*fromPkg))
+		innerDeps = append(innerDeps, deriveBomRef(fromPkg))
 		result = append(result, cyclonedx.Dependency{
-			Ref:          deriveBomRef(*toPkg),
+			Ref:          deriveBomRef(toPkg),
 			Dependencies: &innerDeps,
 		})
 	}

diff --git a/syft/formats/common/cyclonedxhelpers/format_test.go b/syft/formats/common/cyclonedxhelpers/format_test.go
@@ -3,7 +3,13 @@ package cyclonedxhelpers
 import (
 	"testing"
 
+	"github.com/CycloneDX/cyclonedx-go"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/anchore/syft/syft/artifact"
+	"github.com/anchore/syft/syft/pkg"
+	"github.com/anchore/syft/syft/sbom"
 )
 
 func Test_formatCPE(t *testing.T) {
@@ -32,3 +38,98 @@ func Test_formatCPE(t *testing.T) {
 		})
 	}
 }
+
+func Test_relationships(t *testing.T) {
+	p1 := pkg.Package{
+		Name: "p1",
+	}
+	p1.SetID()
+
+	p2 := pkg.Package{
+		Name: "p2",
+	}
+	p2.SetID()
+
+	p3 := pkg.Package{
+		Name: "p3",
+	}
+	p3.SetID()
+
+	tests := []struct {
+		name     string
+		sbom     sbom.SBOM
+		expected []string
+	}{
+		{
+			name: "package dependencyOf relationships output as dependencies",
+			sbom: sbom.SBOM{
+				Artifacts: sbom.Artifacts{
+					Packages: pkg.NewCollection(p1, p2, p3),
+				},
+				Relationships: []artifact.Relationship{
+					{
+						From: p2,
+						To:   p1,
+						Type: artifact.DependencyOfRelationship,
+					},
+					{
+						From: p3,
+						To:   p1,
+						Type: artifact.DependencyOfRelationship,
+					},
+				},
+			},
+			expected: []string{p2.Name, p3.Name},
+		},
+		{
+			name: "package contains relationships not output",
+			sbom: sbom.SBOM{
+				Artifacts: sbom.Artifacts{
+					Packages: pkg.NewCollection(p1, p2, p3),
+				},
+				Relationships: []artifact.Relationship{
+					{
+						From: p2,
+						To:   p1,
+						Type: artifact.ContainsRelationship,
+					},
+					{
+						From: p3,
+						To:   p1,
+						Type: artifact.ContainsRelationship,
+					},
+				},
+			},
+			expected: nil,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			cdx := ToFormatModel(test.sbom)
+			got := cdx.Dependencies
+
+			var deps []string
+			if got != nil {
+				for _, r := range *got {
+					for _, d := range *r.Dependencies {
+						c := findComponent(cdx, d)
+						require.NotNil(t, c)
+						deps = append(deps, c.Name)
+					}
+
+				}
+			}
+			require.Equal(t, test.expected, deps)
+		})
+	}
+}
+
+func findComponent(cdx *cyclonedx.BOM, bomRef string) *cyclonedx.Component {
+	for _, c := range *cdx.Components {
+		if c.BOMRef == bomRef {
+			return &c
+		}
+	}
+	return nil
+}
diff --git a/test/integration/encode_decode_cycle_test.go b/test/integration/encode_decode_cycle_test.go
@@ -47,8 +47,8 @@ func TestEncodeDecodeEncodeCycleComparison(t *testing.T) {
 			formatOption: cyclonedxjson.ID,
 			redactor: func(in []byte) []byte {
 				// unstable values
-				in = regexp.MustCompile(`"(timestamp|serialNumber|bom-ref)": "[^"]+",`).ReplaceAll(in, []byte{})
-
+				in = regexp.MustCompile(`"(timestamp|serialNumber|bom-ref|ref)":\s*"(\n|[^"])+"`).ReplaceAll(in, []byte(`"$1": "redacted"`))
+				in = regexp.MustCompile(`"(dependsOn)":\s*\[(?:\s|[^]])+]`).ReplaceAll(in, []byte(`"$1": []`))
 				return in
 			},
 			json: true,
@@ -57,7 +57,7 @@ func TestEncodeDecodeEncodeCycleComparison(t *testing.T) {
 			formatOption: cyclonedxxml.ID,
 			redactor: func(in []byte) []byte {
 				// unstable values
-				in = regexp.MustCompile(`(serialNumber|bom-ref)="[^"]+"`).ReplaceAll(in, []byte{})
+				in = regexp.MustCompile(`(serialNumber|bom-ref|ref)="[^"]+"`).ReplaceAll(in, []byte{})
 				in = regexp.MustCompile(`<timestamp>[^<]+</timestamp>`).ReplaceAll(in, []byte{})
 
 				return in