fix: return ecosyste.ms license based on package version #88
Conversation
lib/ecosystems/package.go
Outdated
| // Ecosyste.ms has a purl based API, but unfortunately slower | ||
| // so we break the purl down to registry and name values locally | ||
| // params := packages.LookupPackageParams{Purl: &p} | ||
| // resp, err := client.LookupPackageWithResponse(context.Background(), ¶ms) |
There was a problem hiding this comment.
can we remove this? including the comment about the purl api? i don't find it particularly useful 😄
lib/ecosystems/enrich_cyclonedx.go
Outdated
| expression := data.NormalizedLicenses[0] | ||
| func enrichCDXLicense(comp *cdx.Component, data *packages.Version) { | ||
| if data.Licenses != nil { | ||
| if *data.Licenses != "" { |
There was a problem hiding this comment.
this looks a bit like a red flag. don't we have data.NormalizedLicenses on the package version response? this change would mean that we are potentially enriching with unsanitized data. e.g. MIT,Apache-2.0 is not a valid SPDX license expression.
There was a problem hiding this comment.
looks like the ecosyste.ms response for a specific package version doesn't have a normalised licenses field, no
There was a problem hiding this comment.
I raised an issue with ecosyste.ms about this here: ecosyste-ms/packages#960
There was a problem hiding this comment.
@paulrosca-snyk could we move forward with this approach, but strings.Split() the value and build an SPDX license expression with strings.Join()? The current behaviour is to concatenate with an OR, but we have an open issue on this and should switch to AND.
There was a problem hiding this comment.
@mcombuechen definitely, but are we sure we want to proceed with the AND change, it doesn't seem like it will fix the issue, just change it in the other direction (i.e we'll be reporting OR clauses incorrectly as AND, but maybe that's better than the current behaviour?)
There was a problem hiding this comment.
i.e we'll be reporting OR clauses incorrectly as AND
How so? Are the (unnormalized) license lists from ecosyste.ms OR or AND?
There was a problem hiding this comment.
From my understanding they can be either and we just don't get this information back from ecosyste.ms (assuming they even have it stored)
paulrosca-snyk
force-pushed
the
fix/ecosyste.ms-licenses
branch
from
a3816fb to
9006bf8
Compare
paulrosca-snyk
force-pushed
the
fix/ecosyste.ms-licenses
branch
from
9006bf8 to
e6b1d52
Compare
When enriching with ecosyste.ms, fetch the license based on the package version.
Closes #87