diff --git a/syft/format/common/spdxhelpers/to_format_model.go b/syft/format/common/spdxhelpers/to_format_model.go index 426f9e10c..2a175ce9d 100644 --- a/syft/format/common/spdxhelpers/to_format_model.go +++ b/syft/format/common/spdxhelpers/to_format_model.go @@ -2,6 +2,7 @@ package spdxhelpers import ( + "cmp" "crypto/sha1" "fmt" "path" @@ -179,7 +180,8 @@ func toRootRelationships(rootPackage *spdx.Package, packages []*spdx.Package) (o func toRootPackage(s source.Description) *spdx.Package { var prefix string - name := s.Name + // the SPDX package name is mandatory, and a source decoded from another SBOM may not carry one + name := cmp.Or(s.Name, imageRepoName(s.Metadata), s.ID, helpers.DocumentName(s)) version := s.Version var purl *packageurl.PackageURL @@ -210,7 +212,7 @@ func toRootPackage(s source.Description) *spdx.Package { checksums = append(checksums, *c) purl = &packageurl.PackageURL{ Type: "oci", - Name: s.Name, + Name: name, Version: m.ManifestDigest, Qualifiers: qualifiers, } @@ -240,7 +242,7 @@ func toRootPackage(s source.Description) *spdx.Package { checksums = append(checksums, *c) purl = &packageurl.PackageURL{ Type: "oci", - Name: s.Name, + Name: name, Version: m.ManifestDigest, Qualifiers: qualifiers, } @@ -275,10 +277,6 @@ func toRootPackage(s source.Description) *spdx.Package { default: prefix = prefixUnknown purpose = spdxPrimaryPurposeOther - - if name == "" { - name = s.ID - } } p := &spdx.Package{ @@ -308,6 +306,25 @@ func toRootPackage(s source.Description) *spdx.Package { return p } +// imageRepoName returns the repository name from an image or model user input, the same way the image source names +// itself, so a source decoded from another SBOM (which may only carry the user input) gets a name without the tag. +func imageRepoName(metadata any) string { + var userInput string + switch m := metadata.(type) { + case source.ImageMetadata: + userInput = m.UserInput + case source.OCIModelMetadata: + userInput = m.UserInput + default: + return "" + } + ref, _ := reference.Parse(userInput) + if named, ok := ref.(reference.Named); ok { + return named.Name() + } + return "" +} + func toSPDXID(identifiable artifact.Identifiable) spdx.ElementID { id := string(identifiable.ID()) if after, ok := strings.CutPrefix(id, "SPDXRef-"); ok { diff --git a/syft/format/common/spdxhelpers/to_format_model_test.go b/syft/format/common/spdxhelpers/to_format_model_test.go index 302fe81ae..aede4dbee 100644 --- a/syft/format/common/spdxhelpers/to_format_model_test.go +++ b/syft/format/common/spdxhelpers/to_format_model_test.go @@ -1145,3 +1145,49 @@ func Test_otherLicenses(t *testing.T) { }) } } + +func Test_toRootPackage_name(t *testing.T) { + tests := []struct { + name string + src source.Description + wantName string + wantPurl string + }{ + { + name: "source name wins", + src: source.Description{Name: "named", ID: "id", Metadata: source.ImageMetadata{UserInput: "alpine:3.18"}}, + wantName: "named", + }, + { + // a CycloneDX container component decodes to an image source with only the user input set + name: "image user input without a source name", + src: source.Description{Metadata: source.ImageMetadata{ + UserInput: "alpine:3.18", + ManifestDigest: "sha256:0e4b2e0d6b2f3c2b1d3bb3a1c8f1c4d9e6a9b1e8c6f4d2b3a1f0e9d8c7b6a5f4", + }}, + wantName: "alpine", + wantPurl: "pkg:oci/alpine@sha256%3A0e4b2e0d6b2f3c2b1d3bb3a1c8f1c4d9e6a9b1e8c6f4d2b3a1f0e9d8c7b6a5f4?arch=&tag=3.18", + }, + { + name: "id without a source name", + src: source.Description{ID: "some-id"}, + wantName: "some-id", + }, + { + name: "nothing to name it by", + src: source.Description{Metadata: source.DirectoryMetadata{}}, + wantName: "unknown", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := toRootPackage(tt.src) + assert.Equal(t, tt.wantName, p.PackageName) + var purl string + if len(p.PackageExternalReferences) > 0 { + purl = p.PackageExternalReferences[0].Locator + } + assert.Equal(t, tt.wantPurl, purl) + }) + } +} diff --git a/syft/format/common/spdxhelpers/to_syft_model.go b/syft/format/common/spdxhelpers/to_syft_model.go index e8b3e2bfb..eaddc9650 100644 --- a/syft/format/common/spdxhelpers/to_syft_model.go +++ b/syft/format/common/spdxhelpers/to_syft_model.go @@ -147,12 +147,24 @@ func extractSource(spdxIDMap map[string]any, doc *spdx.Document) source.Descript } p := rootPackages[0] + rootType := syftRootType(p.PackageSPDXIdentifier) - switch p.PrimaryPackagePurpose { + purpose := p.PrimaryPackagePurpose + if purpose == "" { + // SPDX 2.1 and 2.2 have no primary package purpose, so fall back to the source type syft puts in the root's ID + purpose = syftRootPurpose(rootType) + } + + switch purpose { case spdxPrimaryPurposeContainer: src = containerSource(p) case spdxPrimaryPurposeFile: src = fileSource(p) + case spdxPrimaryPurposeOther: + // syft writes a root package even for a source it knows nothing about, which is not a package in the SBOM + if rootType != prefixUnknown { + return src + } default: return src } @@ -165,6 +177,30 @@ func extractSource(spdxIDMap map[string]any, doc *spdx.Document) source.Descript return src } +// syftRootType returns the source type syft encodes in a root package ID ("DocumentRoot--"), or "" for +// any other ID. +func syftRootType(id spdx.ElementID) string { + rest, ok := strings.CutPrefix(string(id), "DocumentRoot-") + if !ok { + return "" + } + rootType, _, _ := strings.Cut(rest, "-") + return rootType +} + +// syftRootPurpose returns the primary package purpose syft writes for a root package of the given source type. +func syftRootPurpose(rootType string) string { + switch rootType { + case prefixImage, prefixOCIModel, prefixSnap: + return spdxPrimaryPurposeContainer + case prefixDirectory, prefixFile: + return spdxPrimaryPurposeFile + case prefixUnknown: + return spdxPrimaryPurposeOther + } + return "" +} + func containerSource(p *spdx.Package) source.Description { id := string(p.PackageSPDXIdentifier) diff --git a/syft/format/internal/spdxutil/helpers/document_name.go b/syft/format/internal/spdxutil/helpers/document_name.go index b34c7a445..afba5a5a7 100644 --- a/syft/format/internal/spdxutil/helpers/document_name.go +++ b/syft/format/internal/spdxutil/helpers/document_name.go @@ -9,16 +9,21 @@ func DocumentName(src source.Description) string { return src.Name } + var name string switch metadata := src.Metadata.(type) { case source.ImageMetadata: - return metadata.UserInput + name = metadata.UserInput case source.OCIModelMetadata: - return metadata.UserInput + name = metadata.UserInput case source.DirectoryMetadata: - return metadata.Path + name = metadata.Path case source.FileMetadata: - return metadata.Path - default: + name = metadata.Path + } + + // the SPDX document name is mandatory, and a source read from another SBOM may not have one + if name == "" { return "unknown" } + return name } diff --git a/syft/format/internal/spdxutil/helpers/document_name_test.go b/syft/format/internal/spdxutil/helpers/document_name_test.go index 26dbf1ebf..eab8fc18f 100644 --- a/syft/format/internal/spdxutil/helpers/document_name_test.go +++ b/syft/format/internal/spdxutil/helpers/document_name_test.go @@ -1,8 +1,6 @@ package helpers import ( - "fmt" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -73,11 +71,23 @@ func Test_DocumentName(t *testing.T) { }, expected: "some/name", }, + { + name: "no name or metadata", + srcMetadata: source.Description{}, + expected: "unknown", + }, + { + name: "metadata without a name", + srcMetadata: source.Description{ + Metadata: source.ImageMetadata{}, + }, + expected: "unknown", + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { actual := DocumentName(test.srcMetadata) - assert.True(t, strings.HasPrefix(actual, test.expected), fmt.Sprintf("actual name %q", actual)) + assert.Equal(t, test.expected, actual) // track each scheme tested (passed or not) tracker.Tested(t, test.srcMetadata.Metadata) diff --git a/syft/format/spdxjson/encoder_test.go b/syft/format/spdxjson/encoder_test.go index be4680942..42912d5d5 100644 --- a/syft/format/spdxjson/encoder_test.go +++ b/syft/format/spdxjson/encoder_test.go @@ -236,23 +236,6 @@ func TestSupportedVersions(t *testing.T) { subject := testutil.DirectoryInput(t, t.TempDir()) dec := NewFormatDecoder() - relationshipOffsetPerVersion := map[string]int{ - // the package representing the source gets a relationship from the source package to all other packages found - // these relationships cannot be removed until the primaryPackagePurpose info is available in 2.3 - "2.1": 2, - "2.2": 2, - // the source-to-package relationships can be removed since the primaryPackagePurpose info is available in 2.3 - "2.3": 0, - "3.0": 0, - } - - pkgCountOffsetPerVersion := map[string]int{ - "2.1": 1, // the source is mapped as a package, but cannot distinguish it since the primaryPackagePurpose info is not available until 2.3 - "2.2": 1, // the source is mapped as a package, but cannot distinguish it since the primaryPackagePurpose info is not available until 2.3 - "2.3": 0, // the source package can be removed since the primaryPackagePurpose info is available - "3.0": 0, - } - for _, enc := range encs { t.Run(enc.Version(), func(t *testing.T) { require.Contains(t, versions, enc.Version()) @@ -274,13 +257,9 @@ func TestSupportedVersions(t *testing.T) { require.NotEmpty(t, s.Artifacts.Packages.PackageCount()) - offset := relationshipOffsetPerVersion[enc.Version()] + assert.Equal(t, len(subject.Relationships), len(s.Relationships), "mismatched relationship count") - assert.Equal(t, len(subject.Relationships)+offset, len(s.Relationships), "mismatched relationship count") - - offset = pkgCountOffsetPerVersion[enc.Version()] - - if !assert.Equal(t, subject.Artifacts.Packages.PackageCount()+offset, s.Artifacts.Packages.PackageCount(), "mismatched package count") { + if !assert.Equal(t, subject.Artifacts.Packages.PackageCount(), s.Artifacts.Packages.PackageCount(), "mismatched package count") { t.Logf("expected: %d", subject.Artifacts.Packages.PackageCount()) for _, p := range subject.Artifacts.Packages.Sorted() { t.Logf(" - %s", p.String()) diff --git a/syft/format/spdxjson/testdata/snapshot/TestSPDX22JSONRequredProperties.golden b/syft/format/spdxjson/testdata/snapshot/TestSPDX22JSONRequredProperties.golden index 8587d7bc0..5f2b09b5b 100644 --- a/syft/format/spdxjson/testdata/snapshot/TestSPDX22JSONRequredProperties.golden +++ b/syft/format/spdxjson/testdata/snapshot/TestSPDX22JSONRequredProperties.golden @@ -47,13 +47,13 @@ "copyrightText": "NOASSERTION" }, { - "SPDXID": "SPDXRef-DocumentRoot-Unknown-", + "SPDXID": "SPDXRef-DocumentRoot-Unknown-unknown", "copyrightText": "NOASSERTION", "downloadLocation": "NOASSERTION", "filesAnalyzed": false, "licenseConcluded": "NOASSERTION", "licenseDeclared": "NOASSERTION", - "name": "", + "name": "unknown", "supplier": "NOASSERTION" } ], @@ -82,18 +82,18 @@ "relationshipType": "CONTAINS" }, { - "spdxElementId": "SPDXRef-DocumentRoot-Unknown-", + "spdxElementId": "SPDXRef-DocumentRoot-Unknown-unknown", "relatedSpdxElement": "SPDXRef-Package-files-analyzed-false-7d37ba9d2f7c574b", "relationshipType": "CONTAINS" }, { - "spdxElementId": "SPDXRef-DocumentRoot-Unknown-", + "spdxElementId": "SPDXRef-DocumentRoot-Unknown-unknown", "relatedSpdxElement": "SPDXRef-Package-files-analyzed-true-035066c2086b8bb4", "relationshipType": "CONTAINS" }, { "spdxElementId": "SPDXRef-DOCUMENT", - "relatedSpdxElement": "SPDXRef-DocumentRoot-Unknown-", + "relatedSpdxElement": "SPDXRef-DocumentRoot-Unknown-unknown", "relationshipType": "DESCRIBES" } ] diff --git a/syft/format/spdxtagvalue/encoder_test.go b/syft/format/spdxtagvalue/encoder_test.go index e5d470ec8..991d1ad1e 100644 --- a/syft/format/spdxtagvalue/encoder_test.go +++ b/syft/format/spdxtagvalue/encoder_test.go @@ -3,6 +3,7 @@ package spdxtagvalue import ( "bytes" "flag" + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -143,21 +144,6 @@ func TestSupportedVersions(t *testing.T) { subject := testutil.DirectoryInput(t, t.TempDir()) dec := NewFormatDecoder() - relationshipOffsetPerVersion := map[string]int{ - // the package representing the source gets a relationship from the source package to all other packages found - // these relationships cannot be removed until the primaryPackagePurpose info is available in 2.3 - "2.1": 2, - "2.2": 2, - // the source-to-package relationships can be removed since the primaryPackagePurpose info is available in 2.3 - "2.3": 0, - } - - pkgCountOffsetPerVersion := map[string]int{ - "2.1": 1, // the source is mapped as a package, but cannot distinguish it since the primaryPackagePurpose info is not available until 2.3 - "2.2": 1, // the source is mapped as a package, but cannot distinguish it since the primaryPackagePurpose info is not available until 2.3 - "2.3": 0, // the source package can be removed since the primaryPackagePurpose info is available - } - for _, enc := range encs { t.Run(enc.Version(), func(t *testing.T) { require.Contains(t, versions, enc.Version()) @@ -178,13 +164,9 @@ func TestSupportedVersions(t *testing.T) { require.NotEmpty(t, s.Artifacts.Packages.PackageCount()) - offset := relationshipOffsetPerVersion[enc.Version()] + assert.Equal(t, len(subject.Relationships), len(s.Relationships), "mismatched relationship count") - assert.Equal(t, len(subject.Relationships)+offset, len(s.Relationships), "mismatched relationship count") - - offset = pkgCountOffsetPerVersion[enc.Version()] - - if !assert.Equal(t, subject.Artifacts.Packages.PackageCount()+offset, s.Artifacts.Packages.PackageCount(), "mismatched package count") { + if !assert.Equal(t, subject.Artifacts.Packages.PackageCount(), s.Artifacts.Packages.PackageCount(), "mismatched package count") { t.Logf("expected: %d", subject.Artifacts.Packages.PackageCount()) for _, p := range subject.Artifacts.Packages.Sorted() { t.Logf(" - %s", p.String()) @@ -209,3 +191,39 @@ func defaultFormatEncoders() []sbom.FormatEncoder { } return encs } + +func TestEncodeDecodeSourceWithoutName(t *testing.T) { + // a source read from another SBOM (a purl list, a CycloneDX document without metadata.component) may have no name, + // but the SPDX package name is mandatory and the tag-value decoder needs it to start the root package section + p := pkg.Package{Name: "left-pad", Version: "1.3.0", Type: pkg.NpmPkg} + p.SetID() + + for _, src := range []source.Description{ + {}, + {Metadata: source.FileMetadata{}}, + {Metadata: source.DirectoryMetadata{}}, + {Metadata: source.ImageMetadata{}}, + {Metadata: source.OCIModelMetadata{}}, + {Metadata: source.SnapMetadata{}}, + } { + for _, enc := range defaultFormatEncoders() { + t.Run(fmt.Sprintf("%T/%s", src.Metadata, enc.Version()), func(t *testing.T) { + var buf bytes.Buffer + require.NoError(t, enc.Encode(&buf, sbom.SBOM{ + Source: src, + Artifacts: sbom.Artifacts{Packages: pkg.NewCollection(p)}, + })) + + s, _, _, err := NewFormatDecoder().Decode(bytes.NewReader(buf.Bytes())) + require.NoError(t, err, "encoded document:\n%s", buf.String()) + + var names []string + for _, decoded := range s.Artifacts.Packages.Sorted() { + names = append(names, decoded.Name) + } + // the root package describes the source, so it must not come back as a package of the SBOM + assert.Equal(t, []string{"left-pad"}, names) + }) + } + } +}