diff --git a/CHANGELOG.md b/CHANGELOG.md index 2504d57..c9a1cd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,14 @@ what has already shipped, and for nothing else. See `RELEASING.md`. also kept when the keyword names, as a word, one of the products it lists as Known Affected: 1,057, 980 and 1,099 on the same document, in the same time. The September fixture had no ProductTree; the tests read a recorded excerpt of October's with its product names. +- **Without `VULDB_API_KEY` no scan could end in an all-clear.** VulDB answers only with a key, and + without it the collector refused every target: one "results … are incomplete" line per target, + "VulDB answered for none of the N targets scanned" at the end, and never "no CVEs found", which + needs every source to have answered; over HTTP the refusal sat in `errors` of every scan. A + fixture in conftest.py replaced the VulDB collector with one returning nothing in every test, so + the suite never saw it. A source nobody configured is now not queried, by the CLI or the API, and + a scan says once that VulDB was not asked and how to add it; the fixture now only removes the key + from the environment. With the key set, nothing changes. - **On a pull request, Security Posture called live exceptions unused.** CodeQL reports only what the change introduces on a pull request ref, so when it reported anything, an exception for a diff --git a/README.md b/README.md index 720ea7f..8abaab0 100644 --- a/README.md +++ b/README.md @@ -171,7 +171,7 @@ two is not the same as a complete scan. | [MSRC](https://msrc.microsoft.com) | Microsoft Patch Tuesday | ✅ Active | | [Debian Security](https://security-tracker.debian.org) | Linux packages | ✅ Active | | [CISA KEV](https://www.cisa.gov/known-exploited-vulnerabilities-catalog) | Known exploited vulnerabilities | ✅ Active | -| [VulDB](https://vuldb.com) | Independent vulnerability database | ✅ Active | +| [VulDB](https://vuldb.com) | Independent vulnerability database | ✅ Active with `VULDB_API_KEY`; without it a scan says once that VulDB was not asked | | [FIRST EPSS](https://www.first.org/epss/) | Exploitation forecast (enrichment) | ✅ Active | KEV answers a different question from the others. They report what has been diff --git a/src/patchradar/api/main.py b/src/patchradar/api/main.py index d1b5846..31fe0d9 100644 --- a/src/patchradar/api/main.py +++ b/src/patchradar/api/main.py @@ -20,6 +20,7 @@ from patchradar.collectors.kev import fetch_cves as kev_fetch from patchradar.collectors.msrc import fetch_cves as msrc_fetch from patchradar.collectors.nvd import fetch_cves as nvd_fetch +from patchradar.collectors.vuldb import api_key as vuldb_api_key from patchradar.collectors.vuldb import fetch_cves as vuldb_fetch from patchradar.db.database import add_to_watchlist, get_cves, get_watchlist, init_db, remove_from_watchlist from patchradar.epss import enrich as epss_enrich @@ -284,13 +285,17 @@ async def _scan_one(sw: str, days: int, errors: list[dict]) -> int: # KEV ignores `days`: it is a standing list of what is being exploited, not # a feed of recent publications, and the entries that have been on it # longest are the ones that have gone unpatched the longest. - fetches = ( + # + # VulDB only with its key, as in the CLI: without it `errors` always held its + # refusal, so a scan could never come back without errors. + fetches = [ lambda: nvd_fetch(sw, days_back=days), lambda: msrc_fetch(sw, days_back=days), lambda: debian_fetch(sw), lambda: kev_fetch(sw), - lambda: vuldb_fetch(sw, days_back=days), - ) + ] + if vuldb_api_key(): + fetches.append(lambda: vuldb_fetch(sw, days_back=days)) collected: list[dict] = [] for fetch in fetches: try: diff --git a/src/patchradar/cli.py b/src/patchradar/cli.py index e02a965..9698f16 100644 --- a/src/patchradar/cli.py +++ b/src/patchradar/cli.py @@ -24,6 +24,7 @@ from patchradar.collectors.nvd import api_key as nvd_api_key from patchradar.collectors.nvd import fetch_cves from patchradar.collectors.vuldb import SOURCE as vuldb_source +from patchradar.collectors.vuldb import api_key as vuldb_api_key from patchradar.collectors.vuldb import fetch_cves as vuldb_fetch from patchradar.db.database import add_to_watchlist, get_cves, get_watchlist, init_db, remove_from_watchlist, save_cve from patchradar.epss import enrich as epss_enrich @@ -337,7 +338,11 @@ async def _scan_target( # line and a scan over HTTP that consult different sources print # different totals for the same machine, and neither says which. # `tests/test_collector_wiring.py` holds the two lists together. - for fetch in (fetch_cves, msrc_fetch, kev_fetch, vuldb_fetch): + # VulDB only with its key: without it every target came back "incomplete" + # and no scan could ever end in an all-clear. A source nobody configured + # is not one that failed; `scan` says once that it was not asked. + fetches = (fetch_cves, msrc_fetch, kev_fetch) + ((vuldb_fetch,) if vuldb_api_key() else ()) + for fetch in fetches: try: all_cves += await fetch(target, days_back=days) except CollectorError as exc: @@ -419,11 +424,12 @@ def _report_silent_sources(failures: list[CollectorError], targets: list[str]) - # target — so a count equal to the number of targets means it failed on all # of them. failures_per_source = Counter(f.source for f in failures) - silent = [s for s in SCAN_SOURCES if failures_per_source[s] >= len(targets)] + queried = [s for s in SCAN_SOURCES if s != vuldb_source or vuldb_api_key()] + silent = [s for s in queried if failures_per_source[s] >= len(targets)] if not silent: return - answered = [source for source in SCAN_SOURCES if source not in silent] + answered = [source for source in queried if source not in silent] for source in silent: where = ( f"every CVE above comes from {', '.join(answered)}" @@ -462,6 +468,8 @@ async def _scan(): console.print(f"\n Total: [bold]{total}[/bold] CVEs found\n") # After the total, because it is the total it qualifies. _report_silent_sources(failures, list(targets)) + if not vuldb_api_key(): + console.print(f"ℹ️ {vuldb_source} was not asked: set VULDB_API_KEY to add it to the scan.") if days > msrc_max_days: # The collector caps its window without a word, so `--days 365` # looked like a year of Patch Tuesdays and was three months of them. diff --git a/src/patchradar/collectors/vuldb.py b/src/patchradar/collectors/vuldb.py index e29f2fb..f3f7934 100644 --- a/src/patchradar/collectors/vuldb.py +++ b/src/patchradar/collectors/vuldb.py @@ -168,8 +168,3 @@ async def fetch_cves(keyword: str, days_back: int = 30) -> list[dict]: records.append(parsed) return records - - -# Kept so test_vuldb.py can restore the real function after the -# session-wide vuldb_offline_by_default fixture replaces it. -_real_fetch_cves = fetch_cves diff --git a/tests/conftest.py b/tests/conftest.py index b5bb6ce..1b9d691 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -73,34 +73,19 @@ def kev_offline_by_default(): @pytest.fixture(autouse=True) -def vuldb_offline_by_default(monkeypatch): - """Give every test a VulDB collector that returns nothing, without a request. - - The same reasoning as ``kev_offline_by_default``: a scan reaches VulDB - through the CLI and the API alike, so any test that exercises a scan would - otherwise either hit vuldb.com (consuming real credits) or raise a - ``CollectorError`` for a missing API key — which breaks every test that - asserts ``errors == []`` on a clean scan. - - Patching ``fetch_cves`` to return an empty list is better than just - deleting the env var, because the latter still raises ``CollectorError`` - (caught as a scan error), while this makes VulDB behave as if it answered - with no results. - - Tests in ``test_vuldb.py`` override this with their own ``_set_api_key`` - fixture and ``respx`` mocks. +def vuldb_unconfigured_by_default(monkeypatch): + """Every test runs as most installations do: with no VULDB_API_KEY. + + This used to replace the VulDB collector with one that returned nothing, + because without the key the real one made every scan "incomplete" and broke + every test asserting ``errors == []`` — which is to say the fixture hid the + defect it was written around: the default configuration could never give an + all-clear (tests/test_scan_unconfigured_source.py). A source nobody + configured is now not queried, so the environment is all a test needs to + set. Tests in ``test_vuldb.py`` set the key themselves and answer from + recordings with ``respx``. """ - from patchradar import cli as pr_cli - from patchradar.api import main as api_main - from patchradar.collectors import vuldb - - async def _no_results(keyword, days_back=30): - return [] - monkeypatch.delenv("VULDB_API_KEY", raising=False) - monkeypatch.setattr(vuldb, "fetch_cves", _no_results) - monkeypatch.setattr(api_main, "vuldb_fetch", _no_results) - monkeypatch.setattr(pr_cli, "vuldb_fetch", _no_results) @pytest.fixture(autouse=True) diff --git a/tests/test_nis2_evidence.py b/tests/test_nis2_evidence.py index dccd20d..35e95bb 100644 --- a/tests/test_nis2_evidence.py +++ b/tests/test_nis2_evidence.py @@ -276,7 +276,9 @@ def test_the_law_check_runs_from_the_command_line_scan_only(): @reads_source def test_the_web_scan_asks_debian_and_the_command_line_scan_does_not(): assert "debian_fetch(sw)" in inspect.getsource(api_main._scan_one) - assert "for fetch in (fetch_cves, msrc_fetch, kev_fetch, vuldb_fetch):" in inspect.getsource(cli._scan_target) + command_line = inspect.getsource(cli._scan_target) + assert "fetches = (fetch_cves, msrc_fetch, kev_fetch)" in command_line + assert "debian_fetch" not in command_line @reads_source diff --git a/tests/test_scan_unconfigured_source.py b/tests/test_scan_unconfigured_source.py new file mode 100644 index 0000000..9475bdf --- /dev/null +++ b/tests/test_scan_unconfigured_source.py @@ -0,0 +1,122 @@ +""" +PatchRadar — a source nobody configured is not a source that failed + +VulDB answers only with an API key, and most installations have none. Without +it the collector refused every target, and the scan reported each refusal as a +failure: + + ⚠️ VulDB no_api_key: set VULDB_API_KEY to use VulDB — results for 7-zip are incomplete + … one line per target … + ⚠️ VulDB answered for none of the 8 targets scanned — every CVE above comes from NVD, MSRC, CISA KEV. + +and the line "no CVEs found", which needs every source to have answered, could +never be printed: `patchradar scan` of a product with no CVEs ended in a warning +instead of an all-clear (seen on 2026-10-17). The API listed the same refusal in +`errors`. A fixture in conftest.py made VulDB return nothing in every test, so +the suite never saw it. + +VulDB is now queried only when its key is set, and a scan without it says so +once, as information about what was asked, not as a failure. + +NVD answers from a recording (`nvd_no_results.json`, keywordSearch=patchradar, +2026-10-07), MSRC has no document for the months asked (404), and the KEV +catalogue is the empty one conftest.py caches. +""" +from __future__ import annotations + +import asyncio +import json +import pathlib + +import httpx +import pytest +import respx +from typer.testing import CliRunner + +from patchradar import cli +from patchradar.api import main as api_main +from patchradar.collectors import msrc, nvd, vuldb +from patchradar.db import database + +FIXTURES = pathlib.Path(__file__).parent / "fixtures" +NVD_EMPTY = json.loads((FIXTURES / "nvd_no_results.json").read_text(encoding="utf-8")) + + +@pytest.fixture +def nothing_anywhere(): + """Every source answers, with nothing; VulDB would be answered too, if asked.""" + with respx.mock(assert_all_called=False) as router: + router.get(url__startswith=nvd.NVD_API).mock(return_value=httpx.Response(200, json=NVD_EMPTY)) + router.get(url__startswith=msrc.MSRC_API).mock(return_value=httpx.Response(404)) + asked_vuldb = router.post(url__startswith=vuldb.VULDB_API).mock(return_value=httpx.Response(200, json={})) + yield asked_vuldb + + +def test_a_scan_without_a_vuldb_key_can_still_give_an_all_clear(nothing_anywhere): + result = CliRunner().invoke(cli.app, ["scan", "patchradar", "--days", "7"]) + + assert result.exit_code == 0, result.output + assert "no CVEs found" in result.output + assert "incomplete" not in result.output + + +def test_vuldb_is_not_asked_without_its_key(nothing_anywhere): + CliRunner().invoke(cli.app, ["scan", "patchradar", "--days", "7"]) + + assert not nothing_anywhere.called + + +def test_the_scan_says_once_that_vuldb_was_not_asked(nothing_anywhere): + """Information, not a warning: what was asked, and how to add VulDB.""" + result = CliRunner().invoke(cli.app, ["scan", "patchradar", "--days", "7"]) + + assert result.output.count("VULDB_API_KEY") == 1 + + +@pytest.mark.asyncio +async def test_the_api_lists_no_error_for_a_source_nobody_configured(): + """Over HTTP the scan's `errors` meant a source could not be queried; VulDB + without its key was always there. Every upstream here refuses (503), so the + other sources do appear in `errors` — VulDB must not.""" + errors: list[dict] = [] + with respx.mock(assert_all_called=False) as router: + router.route().mock(return_value=httpx.Response(503)) + await api_main._scan_one("patchradar", 7, errors) + + assert errors, "the other sources refused, and say so" + assert not [e for e in errors if e["source"] == vuldb.SOURCE], errors + + +async def test_a_source_that_was_not_asked_is_not_named_as_one_that_answered(): + """With NVD refusing every target, the summary names the sources the CVEs + came from. VulDB was not asked, so it is not one of them.""" + await database.add_to_watchlist("patchradar") + await database.add_to_watchlist("zzqxnonexistentproduct") + with respx.mock(assert_all_called=False) as router: + router.get(url__startswith=nvd.NVD_API).mock(return_value=httpx.Response(503)) + router.get(url__startswith=msrc.MSRC_API).mock(return_value=httpx.Response(404)) + result = await asyncio.to_thread(CliRunner().invoke, cli.app, ["scan", "--days", "7"]) + + text = " ".join(result.output.split()) # Rich folds long lines at its own width + assert "NVD answered for none of the 2 targets scanned" in text, result.output + assert "every CVE above comes from MSRC, CISA KEV." in text + + +async def test_with_its_key_vuldb_is_asked_by_the_cli_and_the_api(monkeypatch): + """The other side: with VULDB_API_KEY set nothing changes, from either entry + point. VulDB answers with an empty result, in the shape of its API.""" + monkeypatch.setenv(vuldb.API_KEY_ENV, "a-key") + empty = {"request": {"type": "search"}, "response": {"remaining": 10, "total": 0}, "result": []} + with respx.mock(assert_all_called=False) as router: + asked = router.post(url__startswith=vuldb.VULDB_API).mock(return_value=httpx.Response(200, json=empty)) + router.get(url__startswith=nvd.NVD_API).mock(return_value=httpx.Response(200, json=NVD_EMPTY)) + router.get(url__startswith=msrc.MSRC_API).mock(return_value=httpx.Response(404)) + router.route().mock(return_value=httpx.Response(503)) # Debian, for the API + + result = await asyncio.to_thread(CliRunner().invoke, cli.app, ["scan", "patchradar", "--days", "7"]) + after_cli = asked.call_count + await api_main._scan_one("patchradar", 7, []) + + assert after_cli == 1, result.output + assert asked.call_count == 2 + assert "VULDB_API_KEY" not in result.output diff --git a/tests/test_vuldb.py b/tests/test_vuldb.py index 03ba3d6..b7760ef 100644 --- a/tests/test_vuldb.py +++ b/tests/test_vuldb.py @@ -75,12 +75,7 @@ def _mock(payload=None, status=200, *, key="test-vuldb-key"): @pytest.fixture(autouse=True) def _set_api_key(monkeypatch): - """Every test gets an API key and the real fetch_cves back. - - The session-wide ``vuldb_offline_by_default`` fixture replaces - ``fetch_cves`` with a no-op. This file's tests need the real one. - """ - monkeypatch.setattr(vuldb, "fetch_cves", vuldb._real_fetch_cves) + """Every test gets an API key; conftest.py removes it from the environment.""" monkeypatch.setenv(vuldb.API_KEY_ENV, "test-vuldb-key")