feat: improve behaviour for -i when specified file is binary

[b31ngd3v]

closes #1861

[codecov-commenter]

Codecov Report

Merging #1885 (3a6fac4) into main (3354d63) will decrease coverage by 39.48%. The diff coverage is 0.00%.

@@             Coverage Diff             @@
##             main    #1885       +/-   ##
===========================================
- Coverage   89.54%   50.06%   -39.49%     
===========================================
  Files         316      198      -118     
  Lines        7224     6993      -231     
  Branches     1176     1177        +1     
===========================================
- Hits         6469     3501     -2968     
- Misses        490     3284     +2794     
+ Partials      265      208       -57     

Flag	Coverage Δ
longtests	50.06% <0.00%> (-28.66%)	:arrow_down:
win-longtests	?

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

Impacted Files	Coverage Δ
cve_bin_tool/cli.py	70.18% <0.00%> (-6.49%)	:arrow_down:
test/test_file.py	0.00% <0.00%> (-100.00%)	:arrow_down:
test/test_sbom.py	0.00% <0.00%> (-100.00%)	:arrow_down:
test/test_util.py	0.00% <0.00%> (-100.00%)	:arrow_down:
test/test_merge.py	0.00% <0.00%> (-100.00%)	:arrow_down:
test/test_config.py	0.00% <0.00%> (-100.00%)	:arrow_down:
test/test_csv2cve.py	0.00% <0.00%> (-100.00%)	:arrow_down:
test/test_strings.py	0.00% <0.00%> (-100.00%)	:arrow_down:
test/test_version.py	0.00% <0.00%> (-100.00%)	:arrow_down:
test/test_exploits.py	0.00% <0.00%> (-100.00%)	:arrow_down:
... and 183 more

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

[b31ngd3v]

#1889 this PR would fix the longtest CI error

[terriko]

I'm not sure I understand what's up with the longtests failure. Can you walk me through why it's happening and how the fix solves it?

[b31ngd3v]

I'm not sure I understand what's up with the longtests failure. Can you walk me through why it's happening and how the fix solves it?

sure, before the execution starts github action restores the cache dir. (note. that the cache was generated using the api method). so when the test_update_flags test runs cve-bin-tool with "-u -never" and "-n json" [snippet i], this part [snippet ii] gets triggered and it calls source_nvd.nvd_years() function [snippet iii] and this function looks for nvdcve-1.1-*.json.gz in the cache dir, but returns none, because these files get only generated when using json method to update the database. so i fixed the issue just by updating the db with the json method in the first cli run of cve-bin-tool.

test_cli.py [snippet i]

    @pytest.mark.skipif(LONG_TESTS() != 1, reason="Update flag tests are long tests")
    def test_update_flags(self):
        assert (
            main(["cve-bin-tool", "-x", "-u", "never", "-n", "json", self.tempdir]) != 0
        )

cli.py [snippet ii]

    if db_update != "never":
        cvedb_orig.get_cvelist_if_stale()
        cvedb_orig.create_exploit_db()
        cvedb_orig.update_exploits()
    else:
        if args["nvd"] == "json":
            LOGGER.warning("Not verifying CVE DB cache")
            cvedb_orig.get_db_update_date()
            if not source_nvd.nvd_years():
                with ErrorHandler(mode=error_mode, logger=LOGGER):
                    raise EmptyCache(cvedb_orig.cachedir)

nvd_source.py [snippet iii]

    def nvd_years(self) -> list[int]:
        """
        Return the years we have NVD data for.
        """
        return sorted(
            int(filename.split(".")[-3].split("-")[-1])
            for filename in glob.glob(str(Path(self.cachedir) / "nvdcve-1.1-*.json.gz"))
        )

[terriko]

Okay, that makes sense, but I think we should fix the test/code rather than working around it. (And it's probably going to have to go away entirely within a year because the json update method is going to go away, but that's another kettle of fish.)

Should we even be running this check? The goal here seems to be to warn folk if they don't have cached data. Wouldn't checking the size of the cvedb database file accomplish the same goal?

We don't actually seem to need precise years here, only "at least one year exists" Can we switch nvd_years() to check the database directly instead and if it gets any entry back on a limit 1 query, we're good enough? is nvd_years() actually used anywhere else?

[terriko]

Okay, I've opened a new issue #1894 for some future work on this. But in the meantime, I'm re-running the tests here because no one else with an open PR seems to have hit this issue so I wanted to double-check when it happens.

Once I'm done experimenting, I'm intending to merge the fix temporarily, and we can revert it when we've got a better long-term solution that doesn't rely on a json update being run.

[terriko]

Updating to get the CI fix in #1889

[terriko]

• closes #1861