Skip to content
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

fix: handling of product versions and vendor name guessing #3225

Merged
merged 3 commits into from
Aug 14, 2023

Conversation

rhythmrx9
Copy link
Contributor

@rhythmrx9 rhythmrx9 commented Aug 10, 2023

removed use of ECOSYSTEM versions, and corrected vendor name guessing from package name.

fixes #3200
partial work on #3201

cve_bin_tool/data_sources/osv_source.py Dismissed Show resolved Hide resolved
Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this breaks one test:

=========================== short test summary info ============================
FAILED test/test_source_osv.py::TestSourceOSV::test_format_data[cve_entries0] - AssertionError: assert [{'cve_id': '...n': '*', ...}] == [{'cve_id': '...n': '*', ...}]
  At index 0 diff: {'cve_id': 'CVE-2014-5461', 'vendor': 'unknown', 'product': 'lua5.1', 'version': '*', 'versionStartIncluding': '5.1.4-5', 'versionStartExcluding': '', 'versionEndIncluding': '5.1.4-5', 'versionEndExcluding': ''} != {'cve_id': 'CVE-2014-5461', 'vendor': 'unknown', 'product': 'lua5.1', 'version': '*', 'versionStartIncluding': '0', 'versionStartExcluding': '', 'versionEndIncluding': '', 'versionEndExcluding': '5.1.4-5+deb6u1'}
  Full diff:
    [
     {'cve_id': 'CVE-2014-5461',
      'product': 'lua5.1',
      'vendor': 'unknown',
      'version': '*',
  -   'versionEndExcluding': '5.1.4-5+deb6u1',
  -   'versionEndIncluding': '',
  ?              ^^
  +   'versionEndExcluding': '',
  ?              ^^
  +   'versionEndIncluding': '5.1.4-5',
      'versionStartExcluding': '',
  -   'versionStartIncluding': '0'},
  ?                             ^
  +   'versionStartIncluding': '5.1.4-5'},
  ?                             ^^^^^^^
    ]
FAILED test/test_source_osv.py::TestSourceOSV::test_format_data[cve_entries1] - AssertionError: assert [{'cve_id': '...n': '*', ...}] == []
  Left contains one more item: {'cve_id': 'CVE-2018-20133', 'product': 'ymlref', 'vendor': 'unknown', 'version': '*', ...}
  Full diff:
    [
  -  ,
  +  {'cve_id': 'CVE-2018-20133',
  +   'product': 'ymlref',
  +   'vendor': 'unknown',
  +   'version': '*',
  +   'versionEndExcluding': '',
  +   'versionEndIncluding': '0.1.1',
  +   'versionStartExcluding': '',
  +   'versionStartIncluding': '0.1.0'},
    ]
===== 2 failed, 1751 passed, 28 skipped, 25 warnings in 1374.70s (0:22:54) =====

I'm guessing it just needs an update but I admit I didn't dig into the numbers to see if the new ones made sense.

@gluesmith2021
Copy link
Contributor

removed use of ECOSYSTEM versions, and corrected vendor name guessing from package name.

fixes #3200 fixes (#)3201

@rhythmrx9 @terriko Current code change does fix #3200, but not 3201 (that could be for another PR). Should the link to 3201 be removed from the description to avoid confusion?

@rhythmrx9
Copy link
Contributor Author

rhythmrx9 commented Aug 14, 2023

@rhythmrx9 @terriko Current code change does fix #3200, but not 3201 (that could be for another PR). Should the link to 3201 be removed from the description to avoid confusion?

@gluesmith2021 removing ECOSYSTEM versions fixes a part of #3201, the other part is duplicated ranges which is now fixed by 4c59c11.

@codecov-commenter
Copy link

Codecov Report

Merging #3225 (4c59c11) into main (338e0cf) will decrease coverage by 5.31%.
Report is 14 commits behind head on main.
The diff coverage is 40.00%.

@@            Coverage Diff             @@
##             main    #3225      +/-   ##
==========================================
- Coverage   80.82%   75.51%   -5.31%     
==========================================
  Files         716      722       +6     
  Lines       11163    11211      +48     
  Branches     1497     1502       +5     
==========================================
- Hits         9022     8466     -556     
- Misses       1738     2415     +677     
+ Partials      403      330      -73     
Flag Coverage Δ
longtests 75.51% <40.00%> (+0.02%) ⬆️
win-longtests ?

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

Files Changed Coverage Δ
test/test_source_osv.py 66.66% <ø> (ø)
cve_bin_tool/data_sources/osv_source.py 30.41% <40.00%> (-32.38%) ⬇️

... and 33 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failing CVE scan test seems to be unrelated, so this looks good to go.

I've removed the thing for #3201 so it shouldn't auto-close (we'll have to do that manually later)

@terriko terriko merged commit 1cd48a2 into intel:main Aug 14, 2023
20 of 21 checks passed
@rhythmrx9 rhythmrx9 deleted the fix_osv branch August 16, 2023 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dubious code for vendor guessing in OSV source
4 participants