cve-bin-tool icon indicating copy to clipboard operation
cve-bin-tool copied to clipboard

feat: Added CI-PRE-CHECKER for VENDOR_PRODUCT

Open joydeep049 opened this issue 1 year ago • 12 comments

Closes #3628

joydeep049 avatar Feb 19 '24 20:02 joydeep049

Codecov Report

Attention: Patch coverage is 17.77778% with 37 lines in your changes are missing coverage. Please review.

Project coverage is 80.06%. Comparing base (d6cbe40) to head (9bf7a8e). Report is 68 commits behind head on main.

:exclamation: Current head 9bf7a8e differs from pull request most recent head 426bfe3. Consider uploading reports for the commit 426bfe3 to get more accurate results

Files Patch % Lines
cve_bin_tool/ci_pre_checker.py 0.00% 37 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3840      +/-   ##
==========================================
+ Coverage   75.41%   80.06%   +4.64%     
==========================================
  Files         808      814       +6     
  Lines       11983    12223     +240     
  Branches     1598     1654      +56     
==========================================
+ Hits         9037     9786     +749     
+ Misses       2593     2013     -580     
- Partials      353      424      +71     
Flag Coverage Δ
longtests 80.06% <17.77%> (+4.64%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Feb 19 '24 20:02 codecov-commenter

Looks like a good start!

In the checker action: you'll need to grab the database from the cache here. You probably want the same code as we use in the testing yaml:

https://github.com/intel/cve-bin-tool/blob/34784dc7f3833b370ee122310b9fdbc725484cbc/.github/workflows/testing.yml#L91-L111

(edit: missed some necessary lines)

I'd also like to see

  • at least 1 test
  • documentation for what's going on here added to the contributor docs so people can understand what it means if it throws an error and how to fix it.

I'm also wondering if we should just load the python code of the checker itself rather than parsing the file, but I need to think a bit about the security implications of doing so. You may have the right idea for safety's sake.

Hello @terriko , Why do we need to load the database in the checker-action. Doesn't my script already load the database to query it? I think I'm missing something here.

Also, I've been thinking about the nature of the test to write for this checker-action. Ideas and examples for that would be really appreciated.

joydeep049 avatar Feb 23 '24 20:02 joydeep049

Why do we need to load the database in the checker-action. Doesn't my script already load the database to query it?

You have to load it in the pipeline for it to be accessible to your code

inosmeet avatar Feb 26 '24 03:02 inosmeet

Hello @terriko , As an extension of the Pre-Checker concept, I was thinking of writing a script meant as an experiment which takes in all of the currently existing checkers, And prints whether each of their VENDOR_PRODUCT has associated CVEs or not. If there are checkers which have such CPEs in VENDOR_PRODUCT, then I'll file issues for those checkers. How does this sound?

( I did a PR for this, please check)

joydeep049 avatar Feb 26 '24 19:02 joydeep049

Hello @terriko @ffontaine @anthonyharrison @Rexbeast2, Most of the work here is done. But it is not being able to load the database even after i loaded it in cache . I am using the same technique as is done throughout the repo to access the database. It runs perfectly on my local as well. Can you tell me where I'm going wrong?

Edit: The windows long tests are failing because i provided an empty VENDOR_PRODUCT to test the checker action. It should be ignored.

joydeep049 avatar Mar 04 '24 11:03 joydeep049

The location of the database file in the environment of the repo is coming out to be /home/runner/.cache/cve-bin-tool/cve.db. Is this correct? Or do I have some issues in loading cve.db in the cache? @terriko @Rexbeast2 @anthonyharrison

joydeep049 avatar Mar 04 '24 14:03 joydeep049

Location here is correct. You should run the tool once after retrieving the cache, because you are retrieving it correctly but aren't moving it at desired location, that's why it can't find the file :)

inosmeet avatar Mar 11 '24 14:03 inosmeet

Location here is correct. You should run the tool once after retrieving the cache, because you are retrieving it correctly but aren't moving it at desired location, that's why it can't find the file :)

Does that mean I'll have to run the tool in the github action once after caching the database?

joydeep049 avatar Mar 11 '24 17:03 joydeep049

Location here is correct. You should run the tool once after retrieving the cache, because you are retrieving it correctly but aren't moving it at desired location, that's why it can't find the file :)

Does that mean I'll have to run the tool in the github action once after caching the database?

Yes.

anthonyharrison avatar Mar 11 '24 17:03 anthonyharrison

Location here is correct. You should run the tool once after retrieving the cache, because you are retrieving it correctly but aren't moving it at desired location, that's why it can't find the file :)

Does that mean I'll have to run the tool in the github action once after caching the database?

Yes.

Okay. Will do. @terriko mentioned above that i shud add tests. IS the current test enough to show the working of the action? I'm making the action fail to show that if it does not find any valid CVE , it will return error code. As a result of VENDOR_PRODUCT being empty in the test_checker_action file, some windows tests are failing. Is that okay or should I change it?

joydeep049 avatar Mar 11 '24 17:03 joydeep049

Just putting a note in to say that I looked at this today, but it's so low priority that I am not expecting to have time to spend on debugging the diff issue until after the 3,3 release is out. I'm going to mark this as blocked so I don't waste time looking at it again until after.

terriko avatar Apr 03 '24 21:04 terriko

Marking as unblocked now that the release is done.

It's still failing with the following message:

Run files=$(git diff --name-only 6abca452cae5546029a21f9b8db03a2a6ee5c822 9bf7a8e572bc4ab05248b861c5d271b48792631a | grep '^cve_bin_tool/checkers/' | xargs)
fatal: bad object 9bf7a8e572bc4ab05248b861c5d271b48792631a

And the suggestions have been to try comparing branches rather than commits, which may yield better results. Looking at the diff action workflow we use elsewhere is also a possible option.

I'll try that! It may take a bit since my end-semester exams are going on. (I'll keep this as my agenda for next week)

joydeep049 avatar Apr 26 '24 13:04 joydeep049