filterPackageVulns: orphan vulnerabilities silently dropped when all groups are ignored

[hyhmrright]

Summary

filterPackageVulns in pkg/osvscanner/filter.go contains a logic guard that silently drops vulnerabilities when all Groups entries are filtered out by the ignore configuration — even if those vulnerabilities were not explicitly ignored.

Root Cause

var newVulns []*osvschema.Vulnerability
if len(newGroups) > 0 { // ← problematic guard
    for _, vuln := range pkgVulns.Vulnerabilities {
        if _, filtered := ignoredVulns[vuln.GetId()]; !filtered {
            newVulns = append(newVulns, vuln)
        }
    }
}

The comment reads: "If there are no groups left then there would be no vulnerabilities." This is an assumption, not an invariant. When all groups are ignored, len(newGroups) == 0 causes the entire vulnerability loop to be skipped. Any vulnerability whose ID is not present in ignoredVulns (i.e., not referenced by any group's Aliases) is silently dropped rather than preserved.

Reproduction (pseudo-code)

Input:
  Groups:          [ G1{ Aliases: ["CVE-2024-001"] } ]
  Vulnerabilities: [ {ID: "CVE-2024-001"}, {ID: "CVE-2024-999"} ]
  Ignore config:   ignore "CVE-2024-001"

After group filtering:
  newGroups    = []                    ← all groups ignored
  ignoredVulns = {"CVE-2024-001": {}} ← only the explicitly ignored IDs

Vulnerability loop:
  len(newGroups) > 0 → false → loop skipped entirely

Actual result:   Vulnerabilities = []            ❌
Expected result: Vulnerabilities = [CVE-2024-999] ✅

Fix

Remove the guard — the ignoredVulns map already handles filtering correctly on its own, and the loop is cheap:

var newVulns []*osvschema.Vulnerability
for _, vuln := range pkgVulns.Vulnerabilities {
    if _, filtered := ignoredVulns[vuln.GetId()]; !filtered {
        newVulns = append(newVulns, vuln)
    }
}

This change is strictly safer: if the groups-always-cover-all-vulns invariant holds in practice, the output is identical. If it ever breaks (data inconsistency, future schema changes), vulnerabilities are correctly preserved rather than silently lost.

---

Found using Logic-Lens — a semi-formal logic bug detection skill for Claude Code / Gemini CLI / Codex CLI.

[Ly-Joey]

Hi @hyhmrright.
I am under the impression that vulnerabilities should still belong to a group even in the absence of aliases, so the input in your pseudo-code reproduction might be a state that doesn't actually exist.
To make sure we’re not chasing ghosts here, could you provide a concrete example (with real OSV records) where you’ve observed this behaviour? It’ll be much easier for us to verify the issue and the proposed fix that way.
Thanks!

[hyhmrright]

Thanks for the response, @Ly-Joey!I understand the concern — I don't have a concrete OSV record at hand where this exact state is observed. The issue was identified through static code analysis rather than a live reproduction.

That said, I'd argue the fix is still worthwhile from a defensive programming standpoint: the comment "If there are no groups left then there would be no vulnerabilities" treats an assumption as an invariant. If the data ever diverges from this assumption (schema change, data inconsistency, edge case in ingestion), the current code silently drops vulnerabilities without any warning or error.

Removing the guard doesn't change behavior when the invariant holds, but prevents silent data loss when it doesn't. Given that the inner loop is cheap and the ignoredVulns map already handles filtering correctly, I think the safer behavior is worth the trivial cost.

Happy to close this if the team is confident the invariant is enforced at the data layer and is unlikely to change. Alternatively, would it be acceptable to at least add an assertion/log when len(newGroups) == 0 but len(newVulns) > 0?