snyk / parlay

Enrich SBOMs with data from third party services
Apache License 2.0
117 stars 18 forks source link

SPDX Supplier is missing #76

Closed BuriKizilkaya closed 3 weeks ago

BuriKizilkaya commented 2 months ago

Hello

I tested parlay with a SPDX2.3 generated from syft. The vendor property would not be added. When I generate for CycloneDx it does. I guess on the enrich_spdx.go file the supplier is missing, I found the following code snippet

enrichSPDXDescription(pkg, pkgData)
enrichSPDXLicense(pkg, pkgData)
enrichSPDXHomepage(pkg, pkgData)
mcombuechen commented 2 months ago

Hey @BuriKizilkaya

thanks for opening the issue. Could you please state exactly what commands you ran, what the outcome was and which behaviour you would expect instead?

Thank you!

BuriKizilkaya commented 2 months ago

@mcombuechen Sure!

I ran first this command to generate a sbom with syft:

syft ./ -o spdx-json | jq > sbom.json

Below is a screenshot of the SBOM. Here you can see that the licence and supplier are filled with NOASSERTION. image

Below is a screenshot of the enhanced SBOM. You can see that the licence has been populated with the information, but the supplier remains the same. I ran this command to enrich the sbom:

parlay ecosystems enrich sbom.json | jq > sbom.enriched.json

image

If I create a cyclonedx sbom and fill it with parlay, it will also fill the supplier field.

Is this helpful?

goneall commented 1 month ago

@mcombuechen - Any chance of getting this added? I'd like to use Parlay to augment the SPDX code for some of the LF projects.

It looks like we can add a function enrichSPDXSupplier(pkg *v2_3.Package, data *packages.Package) to enrich_spdx.go setting the pkg.PackageSupplier value.

similar to the CDX supplier enrichment function.

I'd offer to do a pull request, but it would be the first go code I've ever written ;)

goneall commented 1 month ago

I created a fix in https://github.com/goneall/parlay/tree/upstreamissue76

@mcombuechen Please let me know if you'd like me to create a pull request.

Note that the fix has been manually tested, but I did not create a specific unit test.

Although it is the first Golang code I've written, it was pretty straightforward so it should be relatively safe.

mcombuechen commented 3 weeks ago

@goneall @BuriKizilkaya

Gary, Burak, apologies for the radio silence. My skewed-up notification settings failed to inform me about your messages on this thread.

Gary I took a look at your changes and they look good to me, seems to be pretty much in line what the CycloneDX enrichment is doing. If you could open a PR, that would be grand.

I'd like to see a unit test added for this as well, happy to take care of that so you don't have to 😄

Thanks for the work you did on this!

goneall commented 3 weeks ago

@mcombuechen - Just opened a PR

I'd like to see a unit test added for this as well, happy to take care of that so you don't have to 😄

If you don't mind adding the unit test - it would take me a while to understand enough of the Golang and the existing codebase to write it myself. Thanks.

mcombuechen commented 3 weeks ago

@goneall @BuriKizilkaya This is now available in v0.6.0.

goneall commented 3 weeks ago

Thanks @mcombuechen