-
Notifications
You must be signed in to change notification settings - Fork 733
feat: CPEs format decoder #4207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Most of the code is adopted from decoder of package urls in syft and the tests are taken from Grype. I don't want to make it seem like I claim authorship of that. |
|
Is there anything I can do to move this forward? |
|
Thanks for the contribution @chovanecadam, sorry it got lost in the noise 🤦 For setting the package type from the CPE target_sw, I think we could move that to the Backfill process as the very last step if we could not determine package type otherwise. This would help to improve support for SBOMs with only CPEs and no other way to determine package type, too.
|
f7cb815 to
bcb2442
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay, just a couple of asks mostly related to testing; thanks very much for working on this @chovanecadam!
syft/format/internal/backfill.go
Outdated
| backfillFromPurl(p) | ||
| } | ||
|
|
||
| if len(p.CPEs) != 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is there a reason to have this check, since the backfillFromCPE is doing the same thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good nit, I removed superfluous tests in b3da15b
|
|
||
| // CPETargetSoftwareToPackageType is derived from looking at target_software attributes in the NVD dataset | ||
| // TODO: ideally this would be driven from the store, where we can resolve ecosystem aliases directly | ||
| func CPETargetSoftwareToPackageType(tsw string) pkg.Type { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add some basic test for this function, probably in the decoder_test above -- I don't see any CPEs with targetSW set that result in execution of this logic, did I miss this or could you add a couple tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a few test cases to the cpe decoder_test as well as to the backfill_test
syft/format/cpes/decoder_test.go
Outdated
| } | ||
|
|
||
| syftPkgOpts := []cmp.Option{ | ||
| cmpopts.IgnoreFields(pkg.Package{}, "id", "Type", "Language"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably want to actually test the Type here and add at least one test exercising multiple CPEs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
Signed-off-by: Adam Chovanec <[email protected]>
Signed-off-by: Adam Chovanec <[email protected]>
Signed-off-by: Adam Chovanec <[email protected]>
Signed-off-by: Adam Chovanec <[email protected]>
cff2bcf to
7b9df15
Compare
|
@kzantow I added test cases and rebased on top of main |
| "github.com/anchore/syft/syft/sbom" | ||
| ) | ||
|
|
||
| func Test_CPEProvider(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should also test a list of cpes as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the review, I added the tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was copied from grype/internal/cpe_target_software_to_pkg_type.go , which is ok, however, now we have two sources of truth for this kind of functionality. We should probably consider moving this to syft/pkg/cataloger/common/cpe/* expose this on the public API for grype to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented. The function is used in Grype in two places. If this mergerd we should open issue in Grype to make use of this function instead its own.
Signed-off-by: Adam Chovanec <[email protected]>
to be used in Grype instead of its code Signed-off-by: Adam Chovanec <[email protected]>
7147e35 to
786fcfd
Compare
|
Once this merges, I will create MR in Grype to make use of syft/pkg/cataloger/common/cpe/target_software_to_pkg_type.go instead its own and a separate MR that enables Grype to scan a list of CPEs. |
Description
Closes:
Adds a CPE decoder to aid Grype with scanning a file of CPEs. I have a WIP commit that implements that here: chovanecadam/grype@53bd626
This is my first bigger contribution to Syft, so please double-check if I didn't make some mistake.
I didn't implement an encoder. Syft generates many possible CPEs, I don't see a use-case for this output format.
Type of change
Checklist:
I have adopted the unit tests from grype/grype/pkg/cpe_provider_test.go as this basically implements the same functionality.