Only take top-level binary dependencies into account for BOM generation

[hertrste]

The current sbom generation for a rust project like e.g. the cloud hypervisor also contains dependencies that are test or development only. In general, we do not care about those dev dependencies. These dependencies get included because bombon collects the *.cdx.json files in all subfolders and not only of relevant binaries.

A potential fix is to change the cyclone dx invocation to only generate the bom for top-level binaries that will be a result of the rust crate.

This MR is more of a discussion basis and there are some questions to resolve:

• Is this a valid solution?
• Should the behavior be the default or does the user have to configure it?
• What about library crates?

Following is an example bom for the cloud hypervisor project before and after the change:

• ch-deps-old.json
• ch-deps-new.json

@blitz

[nikstur]

cc @thillux

[nikstur]

This might also be related/relevant for the discussion: CycloneDX/cyclonedx-rust-cargo#755

[thillux]

I guess this is only sufficient if a Rust workspace has lets say 3 binary crates in its repository and you use all of them. What about the 1 out of 3 or 2 out of 3 case?

[hertrste]

I guess this is only sufficient if a Rust workspace has lets say 3 binary crates in its repository and you use all of them. What about the 1 out of 3 or 2 out of 3 case?

That is true. But it's the same in the current state of bombon + you get all the dev dependencies currently?

I think handling your 1/3 or 2/3 use cases is only possible if the user is able to specify the names of the binary dependencies explicitly e.g. as arguments to the passthruVendoredSbom.rust function.

[blitz]

I think handling your 1/3 or 2/3 use cases is only possible if the user is able to specify the names of the binary dependencies explicitly e.g. as arguments to the passthruVendoredSbom.rust function.

In general, the BOM should only include the dependencies of the binaries that are actually installed, shouldn't it? This coincides with all the top-level binary BOMs that cargo-cyclonedx produces. If people select a different set of binaries to install that will require manual configuration. Also if someone installs libraries.

I'm also ok with overestimating the SBOM by including everything and adding a config option. YMMV.

@thillux @nikstur Do you have a preference? We (Cyberus) can provide a patch either way.

[nikstur]

I'm fine with describing just the binaries since that's the thing you're usually concerned with. Collecting SBOMs for libs doesn't really make sense for Rust dependencies in Nix as cargo usually takes care of handling the libraries (and thus cargo-cyclonedx will find them correctly without help from Nix).

bombon makes a configurable distinction between runtime (the things that end up in the Nix closure) and buildtime dependencies (everything else). We should uphold this if possible. This might mean that we should run cargo-cyclonedx twice: once without --describe, to collect builtime dependencies and once with --describe binaries to collect the runtime dependenies. The transformer can the decide (based on the passed parameter) which SBOMs to merge into the final SBOM.

[hertrste]

bombon makes a configurable distinction between runtime (the things that end up in the Nix closure) and buildtime dependencies (everything else). We should uphold this if possible. This might mean that we should run cargo-cyclonedx twice: once without --describe, to collect builtime dependencies and once with --describe binaries to collect the runtime dependenies. The transformer can the decide (based on the passed parameter) which SBOMs to merge into the final SBOM.

It looks like currently the passthruVendoredSbom does not make a difference between runtime and buildtime. How would you like to have those reported separately?

[blitz]

@nikstur Not passing --describe binaries doesn't give you the build time dependencies. It gives you all dependencies, even those not used for build or runtime. So I don't see how this is relevant.

Do you think we should set up a quick call to discuss this?

[blitz]

@nikstur The PR is updated!