perf(detector): derive verified-product suppression from the detection result#2592
Open
MaineK00n wants to merge 13 commits into
Open
perf(detector): derive verified-product suppression from the detection result#2592MaineK00n wants to merge 13 commits into
MaineK00n wants to merge 13 commits into
Conversation
adaa539 to
28d61cb
Compare
37c078a to
7f6053b
Compare
…n result collectVerifiedProducts re-fetched each detected VulnCheck/JVN CVE's verified-source data with a per-CVE GetVulnerabilityDataByVulnerabilityID. On a pathological kernel-as-CPE scan (7268 CVEs) this dominated DetectCPEs at ~36s of a ~42s warm run (did not finish within 2 min cold). The re-fetch is redundant: cpe.Detect finds candidate roots through a part:vendor:product index (version-agnostic) and util.Detect returns every source's full criteria for each such root unconditionally, so the verified sources' defined products are already in the in-memory detection result. A verified root absent from the result can only define non-scanned products (else the index would have surfaced it), and suppress() only looks up scanned products, so the derived set is identical for suppression purposes. Build the set from detected instead: collectVerifiedProducts drops its *session.Session, unions each detected root's verified-source defined products per CVE (collectDefinedCPEProducts now walks a FilteredCriteria via FilteredCriterion.Criterion, which cond.Accept keeps in full), then attaches them to the suppressed roots. collectVerifiedProducts ~36s -> ~1.3s (I/O-free, cache-independent); DetectCPEs ~42s -> ~8.6s; detection output unchanged (kernel:6.1 -> 7268 CVEs, 1162 VulnCheck / 109 JVN, identical before/after). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
7f6053b to
11d7dac
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes per-CVE DB re-fetching for verified-product suppression during vuls2 CPE detection by deriving the verified-source defined product set directly from the in-memory detection result, substantially reducing runtime overhead in pathological high-CVE scans.
Changes:
- Reworked
collectVerifiedProductsto be I/O-free by deriving verified-source defined products fromdetectTypes.DetectResult. - Added
isVerifiedCPESourceto centralize the “verified CPE source” predicate. - Updated
collectDefinedCPEProductsto traverseFilteredCriteriaviaFilteredCriterion.Criterion.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ment The old header still described a raw, unfiltered criteria tree after the signature moved to FilteredCriteria, contradicting the accurate block directly below it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This branch removed the go-cve-dictionary dependency from go.mod, so its entries in the vuls group and others exclude-patterns are now stale (same cleanup already applied to go-cti). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…on result Add focused table-driven tests for the I/O-free verified-product derivation: same-root suppression, cross-root aliasing (NVD under the CVE root feeding JVN under a JVNDB-* root), per-CVE separation on a multi-CVE suppressed root, empty result when no verified product exists, CPEMatches folding to the product key with non-CPE ecosystems ignored, and that a suppressed source's own products are not treated as verified. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the cpeCond/cpeCondMatches/cpeDetection/vulns/set helper closures and spell out each DetectResult literal in full, so individual cases can be tuned independently without threading changes through shared builders. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pass 1 allocated a products map for every detected root, including the majority that carry no verified CPE source. Allocate only once a verified CPE source is actually encountered, cutting avoidable allocations/GC on large detection results. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Single-use one-liner over slices.Contains(verifiedCPESources, ...); the verifiedCPESources var already documents the set, so the call site reads clearly inline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Revert the lazy allocation: the avoided per-root heap header is negligible (empty maps allocate no buckets, and this is dwarfed by the DB-I/O win the rewrite delivered), so favor the simpler unconditional make. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reference fcn.Criterion directly instead of aliasing it, removing the intermediate binding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
postConvert already has the DetectResult, so compute the verified-product sets from it directly (inlined into the walkVulnerabilityDetections call) instead of threading a separate parameter through detectWith. Removes a redundant argument that had to be kept consistent with detected. Test_postConvert no longer injects an independent verified map; the multi-CVE JVN case now carries a real (unmatched) NVD root so the product set is derived, exercising the production path end-to-end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
postConvert threaded noJVNCPEs through but every Test_postConvert case passed nil, so the parameter never rode the execution path. Add a case where the scanned CPE is in noJVNCPEs and the sole (JVN) detection is therefore suppressed, asserting the CVE drops from the result — locking the wiring end-to-end (per-CPE behaviour stays unit-tested in Test_walkCPECriteria). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cts case VulnCheck (like NVD) is rooted per CVE ID, so a single VulnCheck root carrying two CVEs is unrealistic. Switch the multi-CVE suppressed root to JVN's JVNDB-* form, which genuinely groups multiple CVEs under one root. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
VulnCheck is rooted per CVE ID, so the no-verified-product case's root is renamed VC-only -> CVE-2024-0004. Also switch the placeholder CVE-2024-000A/ 000B (letters are not a valid CVE ID) to numeric CVE-2024-0011/0012. Co-Authored-By: Claude Opus 4.8 (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.
Stacked on #2590 (depends on
collectVerifiedProducts, introduced there). Base will retarget tomasteronce #2590 merges.Problem
collectVerifiedProducts(the verified-source suppression mirroring go-cve-dictionary'sisCpeURIAlsoDefinedInVerifiedDataSource) re-fetched each detected VulnCheck/JVN CVE's verified-source data with a per-CVEGetVulnerabilityDataByVulnerabilityID. On a pathological kernel-as-CPE scan (cpe:/o:linux:linux_kernel:6.1→ 7268 CVEs) this dominatesDetectCPEs— ~36s of a ~42s warm run (did not finish within 2 min cold).Why the re-fetch is redundant
cpe.Detect→util.Detectfinds candidate roots through apart:vendor:productindex (version-agnostic) and returns every source's full criteria for each such root unconditionally (FilteredCriterionkeeps the underlyingCriterion, so non-matching definitions survive), so the verified sources' defined products are already present in the in-memory detection result. A verified root absent from the result can only define products that were not scanned (else the index would have surfaced it), andsuppress()only looks up scanned products — so the derived set is identical for suppression purposes.Change
collectVerifiedProductsdrops its*session.Sessionand derives the set fromdetectedin two passes (union verified-source defined products per CVE, then attach to suppressed roots).collectDefinedCPEProductsnow walks aFilteredCriteriaviaFilteredCriterion.Criterion.isVerifiedCPESourcepredicate.Result (measured, same DB)
collectVerifiedProductsDetectCPEstotal (warm)DetectCPEs(cold)Verification
-tags scanner ./detector/.../GOOS=windows),go vet,gofmt,go test ./detector/..., golangci-lint (0 issues) green.🤖 Generated with Claude Code