Pick up CycloneDX BOM components from metadata as well

[dervoeti]

I'm building a docker image containing multiple Java applications. For those applications, I generate CycloneDX SBOMs at build time using cyclonedx-maven-plugin, which are placed in the docker image alongside the applications.

cyclonedx-maven-plugin stores the application component itself (e.g. my-java-app@1.2.3) inside the .metadata.component attribute of the SBOM. It does not provide it again in the .components attribute of the SBOM. This seems to be correct.
When I scan the image using Syft, the sbom-cataloger picks up the SBOMs of the Java applications, but it does not recognize the component in .metadata.component. So the main component and its direct dependency relations to sub-components are lost in the SBOM Syft produces for the image.

This PR fixes that by collecting .metadata.component as a package as well, additionally to .components. I tested this for my use case and it provided the expected result.

I'm not an expert on CycloneDX or Syft, so I have no idea whether this is an idiomatic way of implementing it or it has any side effects, hence I made this a draft PR. As mentioned, it works fine for my use case.

[kzantow]

Hey @dervoeti , this looks like a good change but I'd mention two things:

• we'd definitely like to see some tests for this, maybe using handcrafted cyclonedx objects
• the metadata.component is output by syft itself to hold certain types of information, so we need to be sure to only include this as a package when it's determined not to be the type of metadata syft uses to recreate the source object

[dervoeti]

Thanks for the quick reply! I added an integration test with a handcrafted CycloneDX SBOM.

Regarding the Syft generated .metadata.component, I think the question is how to determine this. Do you think it's sufficient to check .metadata.tools.components for the presence of an entry with .name == 'syft'? As an alternative, a property for the component (in .metadata.component.properties) could be introduced as a hint that this .metadata.component was generated by Syft.

[kzantow]

Hi @dervoeti , Syft outputs appropriate component types for the metadata.component: container and file.

For example:

    "component": {
      "bom-ref": "5cb25b1aaa91f815",
      "type": "container",
      "name": "alpine",
      "version": "sha256:844cde635b9cfad87b1fbe17d0659615264eaac1d00dda8320cba71d218421bb"
    }

or

    "component": {
      "bom-ref": "67aed86799793a90",
      "type": "file",
      "name": "/home/downloads"
    }

I think this should could probably check for the CycloneDX component type being only those appropriate to create packages. My feeling looking at the list is, this would include: application, framework, and library... maybe platform. What do you think?

[kzantow]

It looks like when Syft constructs the source object it is explicitly checking for container and file: https://github.com/anchore/syft/blob/main/syft/format/internal/cyclonedxutil/helpers/decoder.go#L232 and there is still some uncertainty when a file is found. For now, we could probably omit the file, but this probably would be good to create a file in the file section when it is determined not to be the "source" type of a file. Sorry I don't have an exact answer here. For your use case you could probably ignore files in this PR.

[dervoeti]

Yeah I think your list sounds reasonable, I implemented it that way now 🙂 For my use case only application and library are really relevant, but framework and platform make sense too.

[kzantow]

I think this change could be simplified to just adding this block:

	if bom.Metadata.Component != nil
		collectPackages(bom.Metadata.Component, s, idMap)
	}

... and in the collectPackages, we should be recursively adding sub-components, and checking for the type being one of the right types, which it looks like it's doing already

[kzantow]

Sorry I should have looked at exactly what the code was doing there before sending you down a path to implement an unnecessary thing 🤦

[dervoeti]

No worries, I also didn't see the logic was already present. Good thing you spotted it, I refactored it, the change is much simpler now 👍

[kzantow]

I may have missed suggesting a bom.Metadata != nil bit

[dervoeti]

Ah right, fixed

Done

[kzantow]

Thanks for the contribution, @dervoeti !