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

feat: add validation script for nvd json files #3723

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ayushthe1
Copy link
Contributor

fixes #3275 .
This PR attempts to provide a validation script that validates all the json files in a directory based on attributes like size, zipSize, gzSize ,sha256 provided in the meta file.
It also provides the feature to validate the jsonschema

@ayushthe1
Copy link
Contributor Author

ayushthe1 commented Jan 13, 2024

@terriko The validation_json function gets stuck at the validate step. Thats why I have commented that function. Can you please take a look at what is wrong with the function.
The script works for validating on the information in meta files.
This is the initial draft so i expect there will be a lot of further changes to be made 😅

@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f835f2c) 79.02% compared to head (615dd38) 80.02%.
Report is 19 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3723      +/-   ##
==========================================
+ Coverage   79.02%   80.02%   +1.00%     
==========================================
  Files         799      803       +4     
  Lines       11954    11967      +13     
  Branches     1603     1603              
==========================================
+ Hits         9447     9577     +130     
+ Misses       2065     1959     -106     
+ Partials      442      431      -11     
Flag Coverage Δ
longtests 75.01% <ø> (+1.11%) ⬆️
win-longtests 78.04% <ø> (+1.00%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

Is it really stuck or just very slow? json validation can take quite a while (that's why we don't do it as a matter of course). If it's not just slow, I'd guess that the hang is due to something about decompressing and reading the file at the same time -- I've seen other functions behave weirdly when you try to do it in a single step like this. You may need to decompress to a temp file and then read it in separately to validate.

That said, json validation is the least important of the validation functions for now because we know that NVD gives us invalid json somewhat regularly at the moment. In theory, they should never give us bad signatures, though.

So rather than fixing the json part, I'd say leave that out of this PR entirely (it can come in a separate one when you're ready to work on it more) and focus on getting the rest ready to merge:

  1. Fix the linter errors
  2. Add tests for each function
  3. Add docstrings to each function.
  4. Add documentation (probably in https://github.com/intel/cve-bin-tool/tree/main/doc/how_to_guides ) on how to use the validation.
  5. Move the file. It's currently checked in to the root directory but if we want to distribute as part of cve-bin-tool it should probably go in the cve_bin_tool directory because that's where python packaging expects files we distribute to be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Add validation script for NVD JSON files
3 participants