feat: Added CI-PRE-CHECKER for VENDOR_PRODUCT #3840
Conversation
Changes
Changes-2
Changes
Changes
Changes
Codecov ReportAttention: Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
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:
cve-bin-tool/.github/workflows/testing.yml
Lines 91 to 111 in 34784dc
(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 , 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. |
You have to load it in the pipeline for it to be accessible to your code |
|
Hello @terriko , ( I did a PR for this, please check) |
|
Hello @terriko @ffontaine @anthonyharrison @Rexbeast2, Edit: The windows long tests are failing because i provided an empty VENDOR_PRODUCT to test the checker action. It should be ignored. |
|
The location of the database file in the environment of the repo is coming out to be |
|
Location here is correct. |
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. |
| - name: Install cve-bin-tool | ||
| if: env.sbom != 'true' | ||
| run: | | ||
| python -m pip install --upgrade pip | ||
| python -m pip install --upgrade setuptools | ||
| python -m pip install --upgrade wheel | ||
| python -m pip install --upgrade -r dev-requirements.txt | ||
| python -m pip install --upgrade . | ||
|
|
There was a problem hiding this comment.
I think you were given some bad advice above: since your script doesn't actually call anything out of cve-bin-tool you shouldn't need to install it or run it, merely copy the database to the expected location. If you had any dependencies in your script that weren't python packages you could install them here (example below), but probably you just want to remove this section.
| - name: Install cve-bin-tool | |
| if: env.sbom != 'true' | |
| run: | | |
| python -m pip install --upgrade pip | |
| python -m pip install --upgrade setuptools | |
| python -m pip install --upgrade wheel | |
| python -m pip install --upgrade -r dev-requirements.txt | |
| python -m pip install --upgrade . | |
| - name: Install requirements for vendor product pre-check | |
| run: | | |
| python -m pip install YOUR_LIST_OF_REQUIREMENTS |
| - name: Try single CLI run of tool | ||
| if: env.sbom != 'true' | ||
| run: | | ||
| [[ -e cache ]] && mkdir -p .cache && mv cache ~/.cache/cve-bin-tool | ||
| NO_EXIT_CVE_NUM=1 python -m cve_bin_tool.cli test/assets/test-kerberos-5-1.15.1.out | ||
| cp -r ~/.cache/cve-bin-tool cache | ||
|
|
There was a problem hiding this comment.
| - name: Try single CLI run of tool | |
| if: env.sbom != 'true' | |
| run: | | |
| [[ -e cache ]] && mkdir -p .cache && mv cache ~/.cache/cve-bin-tool | |
| NO_EXIT_CVE_NUM=1 python -m cve_bin_tool.cli test/assets/test-kerberos-5-1.15.1.out | |
| cp -r ~/.cache/cve-bin-tool cache | |
| - name: Copy cache to expected location | |
| run: | | |
| [[ -e cache ]] && mkdir -p .cache && mv cache ~/.cache/cve-bin-tool | |
We only need to keep the cache copy line here.
| - name: Get changed files in checkers directory | ||
| id: changed-files | ||
| run: | | ||
| files=$(git diff --name-only ${{ github.sha }} ${{ github.event.before }} | grep '^cve_bin_tool/checkers/' | xargs) |
There was a problem hiding this comment.
This is generating an error when run and I don't know offhand what to do with it, so you're on your own for fixing it:
Run files=$(git diff --name-only 6abca452cae5546029a21f9b8db03a2a6ee5c822 9bf7a8e572bc4ab05248b861c5d271b48792631a | grep '^cve_bin_tool/checkers/' | xargs)
fatal: bad object 9bf7a8e572bc4ab05248b861c5d271b48792631a
There was a problem hiding this comment.
Yepp. I tried changing the depth of the checkout code to 0 and it fetched all the checker files. I'm not able to fetch only the changed .py files in checkers.
There was a problem hiding this comment.
We're using https://github.com/technote-space/get-diff-action on some of our other workflows; don't know if it'll be better but maybe worth a shot?
There was a problem hiding this comment.
@joydeep049 I am not sure how comparing commits is better then just comparing branches, i think you can give it a try, not sure of the top its best way to do it but it should work, alternatively you can try get-diff-action as @terriko mentioned but on my first glance i think it only check changes between commits so you have to read a little, hope it helps:
- name: Fetch Main Branch
run: git fetch origin main
- name: Get changed files in checkers directory
id: changed-files
run: |
files=$(git diff --name-only origin/main | grep '^cve_bin_tool/checkers/' | xargs)
echo "::set-output name=files::$files"
- name: Print changed files in checkers directory
run: |
echo "Changed files in checkers directory:"
echo "${{ steps.changed-files.outputs.files }}"
There was a problem hiding this comment.
@mastersans Thanx! I'll try this!
Actually that is basically the problem I'm facing. I just want to read the difference between the last commit and this one!
But it actually gives me the difference between current and some very old commit!
If it doesn't work then @terriko anyways said in the meet she would give it a look next week.
Thanx!
There was a problem hiding this comment.
Yeah, but one branch can have multiple commit so why not just check branches for the difference.
|
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. |
There was a problem hiding this comment.
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 9bf7a8e572bc4ab05248b861c5d271b48792631aAnd 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! |
Closes #3628