feat!(detector): route Cisco to vuls2#2581
Open
shino wants to merge 8 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Routes Cisco-derived CPE detection and Cisco CveContent enrichment through the vuls2 DB path (matching prior NVD/JVN migrations), and removes Cisco filling/synthesis from the go-cve-dictionary path so Cisco detection/enrichment is sourced consistently from vuls2.
Changes:
- Drops Cisco-derived
CveContentbuilt from the generic CPE vulnerability stub and enriches Cisco from advisory roots via a newenrichCisco. - Expands vuls2 enrichment datasource filtering to include Cisco advisory sources (CSAF/CVRF/JSON) and adds a Cisco enrichment test + fixtures.
- Removes Cisco
CveContentfill and go-cve-dictionary-side Cisco advisory synthesis, and ensures Cisco-only dictionary detections are skipped so vuls2 re-detects them.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| detector/vuls2/vuls2.go | Drops sparse Cisco CveContent created during vulninfo creation; adds Cisco sources to the vuls2 enrich datasource filter. |
| detector/vuls2/vendor.go | Wires Cisco advisory sources into advisory enrichment via enrichCisco. |
| detector/vuls2/vuls2_test.go | Adds a Cisco enrichment test case asserting advisory-sourced content and no CVSS. |
| detector/vuls2/testdata/fixtures/enrich/cisco-json/datasource.json | Adds Cisco datasource fixture metadata for enrichment tests. |
| detector/vuls2/testdata/fixtures/enrich/cisco-json/data/2099/cisco-sa-test-abc.json | Adds Cisco advisory+vulnerability fixture to drive enrichCisco test coverage. |
| detector/detector.go | Removes Cisco CveContent fill and dictionary-side Cisco advisory synthesis; adjusts dictionary CPE detection gating/stripping for Cisco migration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
shino
added a commit
that referenced
this pull request
Jun 23, 2026
Address Copilot review on PR #2581: - enrichAdvisories selected the Cisco source via a.Contents map iteration, which Go randomizes; when an advisory carries multiple Cisco source representations (CSAF/CVRF/JSON) the winner was non-deterministic. Iterate a fixed priority order (CSAF > CVRF > JSON, mirroring compareSourceID) instead. - Correct the FillCvesWithGoCVEDictionary doc comment: NVD/US-CERT alerts come from the vuls2 enrich path; only JP-CERT (from JVN) still comes from here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
shino
added a commit
that referenced
this pull request
Jun 23, 2026
Address Copilot review on PR #2581: the priority loop kept calling enrichCisco for every Cisco source present even though it no-ops once the cisco CveContent exists. Break after the first source that populates it — fewer redundant calls and clearer "first by priority" intent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
shino
added a commit
that referenced
this pull request
Jun 23, 2026
Address Copilot review on PR #2581: - enrichAdvisories doc comment now notes it enriches Cisco CveContent too, not just KEV. - enrichCisco's "already present" note no longer cites the CPE detection path as the example (that path's sparse Cisco content is now deleted); make it generic (a higher-priority Cisco source on a prior call). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
shino
added a commit
that referenced
this pull request
Jun 23, 2026
Address Copilot review on PR #2581: - enrich() now stamps vi.CveID from the map key before running enrichers, so enrichers that derive CveContent.CveID from vi.CveID (enrichCisco) never emit an empty CveID when a caller relied on the map key. - enrichCisco tags *.cisco.com reference URLs with Source "CISCO" instead of toReference's default "MISC", matching go-cve-dictionary's ConvertCiscoToModel and the Cisco advisoryReference so reference-source grouping stays stable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
shino
added a commit
that referenced
this pull request
Jun 23, 2026
…empty Address Copilot review on PR #2581: - postConvert: look up advContent[src] with the comma-ok form; only build the Cisco CveContent from it when present, and reuse the already-computed cc.Optional instead of recomputing cveContentOptional. If a Cisco detection has no matching advisory content (anomalous for cisco-json, whose advisory shares the root), emit no cisco content so the enrich path backfills it by CVE rather than shipping an empty entry. - enrichCisco: skip only when a non-empty cisco content is already present (the detection-built, provenance-bearing one); an empty/placeholder entry is still backfilled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
shino
added a commit
that referenced
this pull request
Jun 23, 2026
Address Copilot review on PR #2581: Test_enrich only exercised enrichCisco (the backfill), not the postConvert detection branch. Add a Test_postConvert case with a minimal CiscoJSON advisory + CVE-stub vulnerability + CPE detection that asserts the Cisco CveContent is built from the advisory (title/summary/ source link/CISCO references), carries Optional["vuls2-sources"] provenance, and that the empty vulnerability stub does not leak into the content. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
shino
added a commit
that referenced
this pull request
Jun 24, 2026
…raws path Address Copilot review on PR #2581: - enrich() doc comment now lists Cisco among the enrichment sources (it filters CiscoJSON in DataSources and backfills Cisco advisory content). - cisco-json enrich fixture: data_source.raws uses the canonical vuls-data-raw-cisco-json/... path, matching the other vuls2 fixtures, instead of the extractor's fixtures/... input path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
shino
added a commit
that referenced
this pull request
Jun 24, 2026
…re raw url Address Copilot review on PR #2581: - postConvert: only stash advContent[src] when the source maps to models.Cisco (it's consumed solely when building Cisco CveContent), avoiding copying other sources' large advisory descriptions; keying on the CveContent type also covers any future Cisco source. - cisco-json fixture datasource.json: the raw image tag is vuls-data-raw-cisco-json, matching the data file's data_source.raws prefix and the other fixtures. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
shino
added a commit
that referenced
this pull request
Jun 24, 2026
…nk in merge Address Copilot review on PR #2581: mergeVulnInfo keyed CveContents by type alone, collapsing same-type entries to one winner. When a CVE is detected by multiple vuls2 sources (e.g. Cisco + NVD) and carries several Cisco advisory-sourced contents (distinct advisories), all but one were dropped, defeating the per-advisory build in postConvert. Key the merge map by (type, source link) instead — mirroring mergeIntoScannedCves' (type, cve, source link) dedup — so distinct advisories survive while same-link entries still merge (CVSS/refs/sources unioned). Sort each type's contents by source link for a stable order. Single-content types (NVD/RedHat/...) have one source link per type, so their behavior is unchanged (all existing tests pass). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
shino
added a commit
that referenced
this pull request
Jun 24, 2026
Address Copilot review on PR #2581: enrichCisco appended Cisco CveContents while ranging the rootMap (a map), so the emitted order was nondeterministic when a CVE has multiple Cisco advisories. Sort the resulting slice by source link, matching the stable ordering mergeVulnInfo now applies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
shino
added a commit
that referenced
this pull request
Jun 24, 2026
…bstring Address Copilot review on PR #2581: the Cisco reference-source tagging matched the substring ".cisco.com/", missing apex URLs like https://cisco.com/... and leaving them Source="MISC". Parse the URL and tag when the host is cisco.com or *.cisco.com, so both apex and subdomains (sec.cloudapps.cisco.com, ...) qualify while lookalikes (evilcisco.com) do not. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
6ca95e4 to
1eeca5f
Compare
shino
added a commit
that referenced
this pull request
Jun 25, 2026
Migrate Cisco (cisco-json/csaf/cvrf) CPE vulnerability detection and content from go-cve-dictionary to the vuls2 boltdb. - postConvert builds Cisco CveContent from the advisory (vulnerability is a thin CVE stub), carrying the vuls2-sources provenance; Cisco has no CVSS (vendor SIR -> DistroAdvisory severity), references to *.cisco.com are tagged "CISCO". - enrichAdvisories backfills Cisco content for CVEs detected by other sources, selecting the source deterministically by priority (CSAF > CVRF > JSON); enrich DataSources include the Cisco sources. - mergeVulnInfo keys CveContents by (type, source link) so a CVE under multiple Cisco advisories keeps all of them. - detector.go drops Cisco from the go-cve-dictionary admission gate / content fill and strips it before getMaxConfidence, so Cisco is reported by vuls2 only. - tui: treat gocui ErrQuit as a clean exit (staticcheck SA4023). Squashed from the shino/cpe-detect-cisco review history (was PR #2581). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
c52f56b to
7759a00
Compare
Migrate Cisco (cisco-json/csaf/cvrf) CPE vulnerability detection and content from go-cve-dictionary to the vuls2 boltdb. - postConvert builds Cisco CveContent from the advisory (vulnerability is a thin CVE stub), carrying the vuls2-sources provenance; Cisco has no CVSS (vendor SIR -> DistroAdvisory severity), references to *.cisco.com are tagged "CISCO". - enrichAdvisories backfills Cisco content for CVEs detected by other sources, selecting the source deterministically by priority (CSAF > CVRF > JSON); enrich DataSources include the Cisco sources. - mergeVulnInfo keys CveContents by (type, source link) so a CVE under multiple Cisco advisories keeps all of them. - detector.go drops Cisco from the go-cve-dictionary admission gate / content fill and strips it before getMaxConfidence, so Cisco is reported by vuls2 only. - tui: treat gocui ErrQuit as a clean exit (staticcheck SA4023). Squashed from the shino/cpe-detect-cisco review history (was PR #2581). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…chCisco enrichCisco guarded on a non-empty cisco entry and then deleted the cisco key before backfilling. Since CveContent.Empty() is Summary=="", a detection-built Cisco content for an advisory with an empty description was treated as empty, deleted, and rebuilt with nil Optional — silently dropping the vuls2-sources provenance that only the detection path can attach. Switch to a presence-based skip (matching enrichNVD / enrichRedHatCVE): if any cisco content is already present, leave it untouched. The detection path never writes empty placeholders (it emits no cisco key when it has nothing), so the backfill still runs for CVEs the Cisco CPE path did not detect. Add regression tests: - Test_mergeVulnInfo: a CVE with two Cisco advisories keeps both CveContents (the (type, source link) merge key), instead of collapsing by type. - Test_enrich case: enrichCisco skips a CVE that already carries detection-built cisco content, preserving its vuls2-sources provenance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s/fixtures vulsio/vuls-data-update#844 (cisco-json extractor) is merged. Replace the synthetic Test_postConvert Cisco case (cisco-sa-test / cisco:test_product) with a real extracted advisory from the merged golden: cisco-sa-3100_4200_tlsdos (CVE-2025-20127, cpe:2.3:a:cisco:firepower_threat_defense, vendor SIR High, CWE-404, cisco.com references, distinct published/modified dates). The product version (7.4.0.0) is paren-free, keeping CPE strings clean; the long advisory description is trimmed to one real sentence. Align the cisco-json fixture datasource name with the extractor's output ("Cisco Security Advisories: openVuln API"). The n39k enrich fixture content already matches the merged golden (only the canonical raws path differs). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ontentsFromAdvs The vulnerability-stub CveContent literal lived inline in postConvert's vuln loop, deeply indented and long. Move it into cveContentsFromVulns in vendor.go so the source-origin switch reads symmetrically: each arm is a one-line call (cveContentsFromVulns for stub-sourced, cveContentsFromAdvs for advisory-sourced). The vuls2-sources provenance (optional) is now computed once before the switch and passed to both. References are still merged in the caller (the advisory-ref merge can fail) and handed in, so cveContentsFromVulns stays error-free and symmetric with cveContentsFromAdvs. No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…romVulns Follow-up to the cveContentsFromVulns extraction: also move the CVSS computation (toCvss) and the reference merge (vulnerability refs + the source's advisory references, the part that can fail) into cveContentsFromVulns — both are used only on the vulnerability-sourced path. It now takes (src, cctype, v, fdas, optional) and returns (CveContents, error); cveContentsFromAdvs gains a matching (CveContents, error) signature (nil error today) so the two read symmetrically. The postConvert vuln loop now builds the VulnInfo first and assigns vinfo.CveContents from the switch, dropping the local cvss/rs/ccs vars and the inline reference-merge loop; a single error check follows the switch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…switch arm Move the error check into each switch arm (a local ccs, err := ... then assign vinfo.CveContents) instead of a single post-switch check that reassigned the outer err. Slightly more verbose (one temp per arm) but keeps each arm's error handling self-contained and the failure message specific. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
RunTui logged gocui.ErrQuit (the normal-quit sentinel) as an error and called os.Exit(1), so a normal TUI quit looked like a failure and bypassed the deferred g.Close() cleanup. Gate on errors.Is(err, gocui.ErrQuit): return ExitSuccess on a normal quit and ExitFailure (logged) only for a real error, letting the deferred Close run in every case. Dropping the always-true `err != nil` also removes the staticcheck SA4023 suppression. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lean exit Refine the previous tui change: still treat gocui.ErrQuit (normal quit) as a clean exit so a normal quit no longer logs an error or exits 1 (and the deferred g.Close runs), but for a real error keep the original behavior — explicit g.Close, log, os.Exit(1) — rather than returning ExitFailure, to avoid changing the error-path exit semantics. Using errors.Is means there is no always-true `err != nil` comparison, so no staticcheck SA4023 to suppress. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
9bb8889 to
b27797d
Compare
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.
What
Move Cisco-derived data from go-cve-dictionary to the vuls2 path, mirroring the NVD (#2575) and JVN migrations. Cisco CPE detection and the Cisco
CveContentnow come from vuls2; go-cve-dictionary no longer detects or fills Cisco.Why
Continue routing CPE-ecosystem sources through vuls2 (cpematch-expanded criteria, unified match-quality projection). Cisco is the next source after NVD/JVN.
How
Detection
detectCpeURIsCvesWithGoCVEDictionarydrops Cisco from the admission gate and nilsdetail.Ciscosbefore confidence selection (same treatment as NVD), so Cisco-only details are skipped and vuls2 re-detects them.postConvertdeletes the sparseciscoCveContentthe generic CPE path builds — Cisco's rich content lives in the advisory, not the vulnerability stub (same shape as JVN).Enrich
enrichCiscoreads Cisco advisory roots and builds theCiscoCveContent(title/summary/CWE/references). Cisco carries no CVSS — only a vendor SIR, which the detection path already surfaces as theDistroAdvisoryseverity — so no CVSS is set, matchingConvertCiscoToModel. Wired viaenrichAdvisories; Cisco sources added toenrich()'sDataSourcesfilter.go-cve-dictionary cleanup
FillCvesWithGoCVEDictionaryno longer fills CiscoCveContent.DistroAdvisory(AdvisoryID + SIR) is now produced by the vuls2 detection path, so the go-cve-dictionary synthesis is removed. No NVD-coverage gating is needed for Cisco (it was always unconditional).Dependencies
db-nightly.mk). Until then the Cisco data must bevuls db add-ed locally.Testing
Test_enrichgains a cisco case (advisory-sourced content, no CVSS).:nightlyDB: NEW is a strict superset of OLD (zero detection losses); 6/10 CPEs exact-match, the rest gain detections where the extract maps product names the legacy fetcher missed. Match tier,CveContent, and the SIRDistroAdvisorymatch.Note
Stacked on #2575 (
shino/cpe-detect-nvd); rebase onto master once that merges.🤖 Generated with Claude Code