Repository navigation
feat(verdict): keep the verdict per product, and judge over HTTP too - #59
Closed
maksimtech wants to merge 2 commits into
Closed
maksimtech wants to merge 2 commits into
maksimtech wants to merge 2 commits into
Conversation
PR-01a judged each CVE against the installed version in memory; the next
`status`, /api/cves and the web UI went back to CVEs by name, and a scan
over HTTP did not judge at all, so the two scans disagreed.
- cve_software gains verdict, verdict_reason, verdict_fixed_version and
judged_version, migrated into existing databases: per product, since
one CVE can affect one watched product and not another. The latest
scan decides it.
- get_cves shows a verdict only while the installed version is still the
one it was given for: a stale "affected" sends someone to patch what
is patched, a stale "not affected" is worse.
- affected.mark is the one place both scans judge.
- The API takes version and cpe on POST /api/watchlist/{name} with the
CLI's checks, lists entries on GET /api/watchlist, and returns the
split per product from POST /api/scan. `status` gets a Verdict column.
Tests first, seen failing; 15 of 15 mutants killed.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Merging this PR will degrade performance by 14.06%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ |
test_api_cves_filtered |
49.7 ms | 59.2 ms | -16.09% |
| ❌ |
test_api_cves |
33.6 ms | 39.2 ms | -14.13% |
| ❌ |
test_get_cves_by_software |
10.7 ms | 12.3 ms | -13.07% |
| ❌ |
test_get_cves |
11.4 ms | 13.1 ms | -12.9% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing feat/the-verdict-is-kept-per-product (284d5de) with main (2e2fdc8)
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…e watchlist CodSpeed on #59: /api/watchlist 24% slower, get_cves and the /api/cves listings 14-17%. - /api/watchlist read the names and the entries in two queries; the names now come from the entries. - get_cves picked the current verdict with a CASE per column; the version check is now part of the join, so a stale link simply does not match. Measured locally on the benchmark's 500 seeded CVEs: 1.68 ms -> 1.61 ms (main 1.45 ms). The rest is the join and four more fields per row, which is what reading the verdict costs: the same query with NULL columns was already 1.57 ms. The full listing's stale-verdict case had no test of its own: added, and it kills the mutant that drops the version check from that join. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Comment on lines
+344
to
+345
| async with db.execute(f""" | ||
| SELECT cves.*, {_CURRENT_VERDICT} FROM cves |
Comment on lines
+356
to
+357
| async with db.execute(f""" | ||
| SELECT cves.*, {_CURRENT_VERDICT} FROM cves |
|
This was referenced Oct 17, 2026
Owner
Author
|
Folded into #64, which now targets main with every commit of the stack: one review, one merge, no conflicts from squash merges. |
maksimtech
added a commit
that referenced
this pull request
Oct 17, 2026
…01) (#64) * feat(verdict): keep the verdict per product, and judge over HTTP too PR-01a judged each CVE against the installed version in memory; the next `status`, /api/cves and the web UI went back to CVEs by name, and a scan over HTTP did not judge at all, so the two scans disagreed. - cve_software gains verdict, verdict_reason, verdict_fixed_version and judged_version, migrated into existing databases: per product, since one CVE can affect one watched product and not another. The latest scan decides it. - get_cves shows a verdict only while the installed version is still the one it was given for: a stale "affected" sends someone to patch what is patched, a stale "not affected" is worse. - affected.mark is the one place both scans judge. - The API takes version and cpe on POST /api/watchlist/{name} with the CLI's checks, lists entries on GET /api/watchlist, and returns the split per product from POST /api/scan. `status` gets a Verdict column. Tests first, seen failing; 15 of 15 mutants killed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * perf(verdict): read the current verdict in the join, one query for the watchlist CodSpeed on #59: /api/watchlist 24% slower, get_cves and the /api/cves listings 14-17%. - /api/watchlist read the names and the entries in two queries; the names now come from the entries. - get_cves picked the current verdict with a CASE per column; the version check is now part of the join, so a stale link simply does not match. Measured locally on the benchmark's 500 seeded CVEs: 1.68 ms -> 1.61 ms (main 1.45 ms). The rest is the join and four more fields per row, which is what reading the verdict costs: the same query with NULL columns was already 1.57 ms. The full listing's stale-verdict case had no test of its own: added, and it kills the mutant that drops the version check from that join. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * feat(scan): a CVE from KEV or MSRC alone is judged through NVD's record KEV and MSRC carry no cpeMatch, so for an entry with an installed version a CVE that reached the scan only through them was "not evaluable" - and KEV's are the ones being exploited. - nvd.records_for asks NVD by id (cveId=), paced like every request to NVD through the pacer fetch_cves now shares. - with_records_for_the_rest merges NVD's record into each such row, configurations and score with it; the CLI and the API call it for an entry they can judge, and only then. - When NVD cannot be asked, the CVE stays not evaluable and the scan says so; the API lists it in `errors`. - The NIS2 document's NVD row says the CVE ids that are sent. Tests first, against the recorded cveId answer for CVE-2026-48092 and a KEV entry derived in KEV's shape; seen failing; 9 of 9 mutants killed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * feat(priority): store what decides a rank, and rank the stored listings CISA's exploitation facts and the CVSS v4.0 exploit maturity travelled in the scan's records and were not columns: the next `status` listed by publication date, and an exploited CVE from 2023 sat under every new 5.3. test_priority_wiring held that boundary to say "now rank the database paths too" the day the columns arrived. - cves gains known_exploited, kev_ransomware, kev_date_added, kev_due_date and cvss_exploit_maturity, migrated in place and refreshed by every scan that brings them: CISA lists a CVE long after NVD publishes it. CISA's "Unknown" is stored as no. - get_cves orders by priority.sql_order, built from the constants priority() decides with; an index on that order makes the full listing 1.0 ms on the benchmark's 500 CVEs, against 1.45 ms by date. - /api/cves rows carry priority, priority_reason and the KEV fields; status has a Priority column and lists most urgent first. - test_priority_wiring now holds the other side of the boundary. Tests first, seen failing; the SQL order is checked against sort_by_priority on rows covering every rank and edge. 15 of 16 mutants killed; the survivor drops the "scored" branch, which COALESCE on the score already orders the same way (equivalent). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(watchlist): vendor and product_id are the vendor source's, as inventory.py meant inventory.py named the watchlist columns before they existed: vendor, product_id, installed_version, channel, cpe. There a vendor source is a feed that answers by version (Chrome's, Edge's, MSRC, Mozilla's) and product_id is the product it needs; NVD by CPE is the other route. #58 filled vendor and product_id with the CPE's fields instead - a misreading of the design it rebuilt - which would have had the inventory import write one over the other. No release carried it. - The CPE's vendor and product are read from `cpe` (WatchEntry.cpe_vendor, cpe_product, installed()), not stored. - set_source records the vendor source, product and channel a person confirmed, for the import to call; it never adds a name. - DESIGN.md section 5.1 says which column is which, and why. Tests first, seen failing; the mutants that swap source and product or drop the version from installed() are killed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * feat(import): --accept writes the products a person confirmed `import` proposes and a person confirms, and what gets remembered is the confirmation (inventory.py). Until the watchlist had the columns the confirmation had nowhere to go and the command wrote nothing. - `--accept "<name>"`, once per product, any of its registry entries: a product the survey identified completely (Chrome, Edge) is watched with the registry's version and its vendor source in vendor and product_id (set_source). - A source without its product, a component, a program nothing answers for: refused with the survey's own reason, exit 1; what was confirmed beside it is still written. - Without --accept nothing is written, and the closing line says how to accept instead of naming columns this release no longer lacks. - README, the NIS2 document and DESIGN 5.6 say what import writes. Tests first, seen failing; the mutants for each refusal, the version, the exit code and the lookup by any registry entry are killed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * feat(ui): the dashboard shows what is installed and the verdict The CLI and the API learnt the installed version, the verdict on each CVE and the stored priority; the dashboard still listed every CVE found by name, ranked as if each were a job, and could not say what is installed. - The watchlist shows the version beside each name; the add form takes a version and a CPE, sent to POST /api/watchlist/{name} with the CLI's checks, and a refusal shows the server's detail (NVD's form of a CPE). - The table gains Priority (its reason on hover) and Verdict columns; a product matched by name reads "not checked" rather than blank. - The CVEs that do not affect the installed version are counted and hidden behind one button, as `scan` does without --all; with none hidden there is no button. - A scan's message gives the split per judged product. Tests first, the page's real script run in Node by the existing harness; seen failing (8 of 9); 10 of 10 mutants killed. One test's routes were reordered after the first run, as test_ui_errors.py orders them: the harness takes the first matching prefix. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(db): no statement is assembled from a formatted string Bandit's B608 flagged three: the watchlist UPDATE whose column list was joined at run time (#58, open on main as alert 61) and the two listings of get_cves whose select list and ORDER BY were interpolated (#64, alerts 64 and 65). Every interpolated piece was a module constant, so no input reached them - but the project takes no `# nosec`, and a statement written out leaves nothing to vouch for. - set_installed / set_source run fixed UPDATE statements, one per field. - get_cves' two listings and the priority index are written out; their ORDER BY is priority.sql_order() verbatim, and the new test holds the text to it, so the one-source rule stands without building from it. EXPLAIN still shows the listing scanning the priority index. Test first and seen failing: no SELECT/INSERT/UPDATE/DELETE passed to execute() may be an f-string or a concatenation. test_priority_wiring now reads the written-out ORDER BY. Bandit B608: no issues. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Audit finding PR-01, step 01b-1 (docs/DESIGN.md §5.5, §5.6). It follows #58.
What was wrong after #58
patchradar status,/api/cvesand the web UI went back to CVEs by name.POST /api/scandid not judge, so the CLI and the API gave two different answers for the same machine. That is the failuretest_collector_wiring.pyexists to prevent.What changes
cve_softwaregainsverdict,verdict_reason,verdict_fixed_versionandjudged_version, migrated in place.get_cvesshows the verdict only whilejudged_versionis still the installed version. Afteradd --version 8.9.6.4, a verdict about 8.5.0 is not shown until a scan judges the new version.affected.mark: the one place both scans judge.POST /api/watchlist/{name}?version=&cpe=runs the CLI's checks and answers 422 with NVD's form of the CPE;GET /api/watchlistaddsentries;POST /api/scanreturnsverdictsper judged product;/api/cvesrows carry the four fields, sanitised.statusgets a Verdict column when any row has a current verdict.test_priority_wiringboundary), and KEV or MSRC entries judged through NVD's record of the same CVE.Tests (strict TDD)
test_verdict_stored.py: migration from a hand-written 2026.44cve_software, round trip, stale version hidden, one verdict per product, the latest scan wins,status.test_api_versioned.py: add, 422s, update, entries, scan split{affected: 10, not evaluable: 0, not affected: 11}, rows read back, entry without a version. NVD replayed from the recording; Debian 503 is listed inerrors.Note:
feat/the-verdict-is-kepton the remote is the same commit before the rebase. The force-push was refused, so it went to this new branch; the old one can be deleted.🤖 Generated with Claude Code