-
Notifications
You must be signed in to change notification settings - Fork 534
feat: added a function to utilize purl integration #4164
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
Conversation
modified python language parser, tailored purls according to the purl2cpe requirement, resolves docutils name collision Signed-off-by: Meet Soni <[email protected]>
cve_bin_tool/parsers/__init__.py
Outdated
if cpeList != []: | ||
for item in cpeList: | ||
vendor, product, version = sbom.decode_cpe23(str(item)) | ||
location = "/usr/local/bin/product" |
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'm not sure what to do about this
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.
What is location
being used for? How is the path established? As written, this won't work on Windows.
I think the location in the ProductInfo is the file location of the application which is being scanned.
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 too thought that it's the location of the product binary being scanned, but wasn't sure so I put a default string similar to find_vendor() method.
Apart from windows, it should be working on the Linux, however it's not able to find the installed database in the cache.
From your email, it sounds like we just need to add purl2cpe in as a data source. It's not quite as complicated as the other data sources because right now we aren't parsing data or anything, just grabbing the db file and going, so it's going to be a slightly awkward fit but I think we can make it work. I'd suggest you do it rather than me, but since I was just in the sources updating stuff let me lay out what I think needs to happen Here's what you'd need to do:
Writing this makes me think maybe I should also add some docs on how to write a data source, so I'll do the docs writing, but you should write the code for purl2cpe specifically! |
Signed-off-by: Meet Soni <[email protected]>
Signed-off-by: Meet Soni <[email protected]>
Signed-off-by: Meet Soni <[email protected]>
This should be working now. After removing cache manually and running the tool, it works fine locally. Failing tests too, pass. |
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.
Added some comments.
Need to create some test scripts for PURL2CPE data source.
cve_bin_tool/parsers/__init__.py
Outdated
if cpeList != []: | ||
for item in cpeList: | ||
vendor, product, version = sbom.decode_cpe23(str(item)) | ||
location = "/usr/local/bin/product" |
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.
What is location
being used for? How is the path established? As written, this won't work on Windows.
I think the location in the ProductInfo is the file location of the application which is being scanned.
Yeah, I'll be working on the tests this week, after we're satisfied with the code. |
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.
Hm, so... our repo settings won't let me merge this because the tests don't pass, but the tests aren't passing because the cache doesn't have the purl2cpe database yet.
Can we maybe split out the part that adds the database so I can merge that and then update the cache, and then we'll be able to merge the rest?
The cache update is running now. I think you can see it here if you want to track when it finishes successfully and we can re-run tests here and hopefully have them all pass: https://github.com/intel/cve-bin-tool/actions/workflows/update-cache.yml |
Whoops, didn't mean to mark that as approved before the tests run but whatever. Cache appears to have updated cleanly, I've resolved the merge conflict, and I believe the tests should pass this time. I've got to run to a meeting but I'll be back to check on them later. |
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.
Okay, tests are passing, let's review for real!
I think we're going to need to refactor decode_cp23 out of here and where it is in sbom manager and make a single function that we call, probably in util for now. Since you've probably gone to bed and this should solve at least the docutils issue, I think I'm going to err on the side of merging as is and I'll file a separate issue for a refactor.
This implementation resolves name collision for docutils #3152.
I have temporarily added the database related code in parsers directory for demonstration until we include it in cache.
For the simplicity's sake, I have only modified python parser. After this one gets green flag, I'll add the other.
@terriko @anthonyharrison