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

bug: Epss_Source.update_epss() missing 1 required positional argument: 'cursor' #4473

Closed
weichslgartner opened this issue Sep 24, 2024 · 6 comments
Labels
bug Something isn't working
Milestone

Comments

@weichslgartner
Copy link
Contributor

weichslgartner commented Sep 24, 2024

Description

In version 3.4 the download of EPSS data does not work. It seems

await self.update_epss()
calls
async def update_epss(self, cursor):
but does not provide the cursor argument.
With debug logs activated the log message in
self.LOGGER.debug(f"Error while fetching EPSS data: {e}")
does not get activated for me, but if I set a breakpoint I can see the Epss_Source.update_epss() missing 1 required positional argument: 'cursor' Exception. (also when I change the log message to error).

To reproduce

Steps to reproduce the behavior:

  1. cve-bin-tool -l debug --update now

Expected behaviour: epss data should be downloaded, no error
Actual behaviour: ERROR CVEDB - Unable to fetch EPSS, skipping EPSS. epss_source.py:158

Version/platform info

Version of CVE-bin-tool( e.g. output of cve-bin-tool --version): 3.4
Installed from pypi or github?: pypi
Operating system: "5.4.0-193-generic #213-Ubuntu SMP Fri Aug 2 19:14:16 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux"
Python version (e.g. python3 --version): 3.11.10
Running in any particular CI environment we should know about? (e.g. Github Actions)

@weichslgartner weichslgartner added the bug Something isn't working label Sep 24, 2024
@weichslgartner weichslgartner changed the title fix: Epss_Source.update_epss() missing 1 required positional argument: 'cursor' bug: Epss_Source.update_epss() missing 1 required positional argument: 'cursor' Sep 24, 2024
@terriko
Copy link
Contributor

terriko commented Sep 25, 2024

I think I see what happened here: when I moved epss into its own data source (so that it could be disabled correctly as the other data sources are) I missed the cursor setup that was previously happening just in cvedb. I need to think a bit about the best way to fix it (i.e. do we pass the cursor from cvedb or get it anew) but it's definitely a bug. And the fact that our own tests didn't find it is obnoxious so I need to think about how to ensure that this doesn't happen again. Thanks for finding the bug!

@terriko terriko added this to the 3.4.1 milestone Sep 25, 2024
@weichslgartner
Copy link
Contributor Author

Think the cursor is only need for EPSS_id_finder and this sets self.epss_metric_id and doesn't store directly in the db. The data only populated later and would be enough to add the epss_metric_id in

def store_epss_data(self, epss_data):
as there the cursor is already present.
Regarding catching such errors, maybe the possible exceptions (timeout etc) could be handled different than the generic exceptions (which can be displayed as error log message).

weichslgartner added a commit to weichslgartner/cve-bin-tool that referenced this issue Sep 26, 2024
@weichslgartner
Copy link
Contributor Author

I implemented a proof of concept fix.
I initialized epps_metric_id to 1: https://github.com/weichslgartner/cve-bin-tool/blob/53bc4fab4d095da3a9b14051378b1fde2666d4b3/cve_bin_tool/data_sources/epss_source.py#L39
and then check later if it is really 1 in the database:
https://github.com/weichslgartner/cve-bin-tool/blob/53bc4fab4d095da3a9b14051378b1fde2666d4b3/cve_bin_tool/cvedb.py#L860

But while implementing I saw that it must always be 1:

(1, "EPSS"),

An idea would to introduce a constant for the metric ids and use then the constant in epss_source

weichslgartner added a commit to weichslgartner/cve-bin-tool that referenced this issue Sep 26, 2024
@weichslgartner
Copy link
Contributor Author

@terriko
Copy link
Contributor

terriko commented Sep 27, 2024

That sounds great! Did you want to do a pull request?

@weichslgartner
Copy link
Contributor Author

Sure. :)

weichslgartner added a commit to weichslgartner/cve-bin-tool that referenced this issue Oct 1, 2024
weichslgartner added a commit to weichslgartner/cve-bin-tool that referenced this issue Oct 1, 2024
weichslgartner added a commit to weichslgartner/cve-bin-tool that referenced this issue Oct 14, 2024
Add a test to the cli tests to check the EPSS functionality:
It first tests if the the update of EPSS source runs without errors
(regression test for intel#4473).
Then checks for an example SBOM if EPSS values are written to csv report.

test: fix Delete epss testfile if exists
weichslgartner added a commit to weichslgartner/cve-bin-tool that referenced this issue Oct 14, 2024
Add a test to the cli tests to check the EPSS functionality:
It first tests if the the update of EPSS source runs without errors
(regression test for intel#4473).
Then checks for an example SBOM if EPSS values are written to csv report.
weichslgartner added a commit to weichslgartner/cve-bin-tool that referenced this issue Oct 14, 2024
Add a test to the cli tests to check the EPSS functionality:
It first tests if the the update of EPSS source runs without errors
(regression test for intel#4473).
Then checks for an example SBOM if EPSS values are written to csv report.
weichslgartner added a commit to weichslgartner/cve-bin-tool that referenced this issue Oct 14, 2024
Add a test to the cli tests to check the EPSS functionality:
It first tests if the the update of EPSS source runs without errors
(regression test for intel#4473).
Then checks for an example SBOM if EPSS values are written to csv report.
weichslgartner added a commit to weichslgartner/cve-bin-tool that referenced this issue Oct 14, 2024
Add a test to the cli tests to check the EPSS functionality:
It first tests if the update of EPSS source runs without errors
(regression test for intel#4473).
Then checks for an example SBOM if EPSS values are written to csv report.
terriko added a commit that referenced this issue Dec 18, 2024
* test: basic execution test for EPSS #4484

Add a test to the cli tests to check the EPSS functionality:
It first tests if the update of EPSS source runs without errors
(regression test for #4473).
Then checks for an example SBOM if EPSS values are written to csv report.

* test: Added sugestion to use -u never instead of -u now

* Adds better assert messages on failure and filters out empty lines in windows csv files cause by double newlines in csv file

---------

Co-authored-by: Terri Oda <terri.oda@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants