From 821a245c68ce62540ff4658d590cf1c2f622365c Mon Sep 17 00:00:00 2001 From: Lougarou Date: Fri, 12 Jun 2026 02:07:03 +0200 Subject: [PATCH 1/2] fix: skip advisory affected entries with unrecognized ecosystems IsAffected and AffectsEcosystem called osvecosystem.MustParse on every affected[] entry's ecosystem, which panics on any string the vendored osv-schema registry doesn't recognize. Because the per-ecosystem local-db zips store the full advisory record, a single multi-ecosystem advisory that lists an ecosystem newer than the build's pinned schema crashes the entire offline scan. Parse the ecosystem with the non-panicking osvecosystem.Parse and skip the affected[] entry on error, mirroring the "skip, don't panic on unsupported ecosystems" handling already used for version parsing in rangeContainsVersion (#2837). Recognized entries are matched exactly as before. Fixes #2867. --- internal/utility/vulns/vulnerability.go | 29 ++++++++++- internal/utility/vulns/vulnerability_test.go | 51 ++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/internal/utility/vulns/vulnerability.go b/internal/utility/vulns/vulnerability.go index 27222eb41a7..2b32fcc04be 100644 --- a/internal/utility/vulns/vulnerability.go +++ b/internal/utility/vulns/vulnerability.go @@ -108,9 +108,30 @@ func rangeAffectsVersion(vulnID string, a []*osvschema.Range, pkg *extractor.Pac return false } +// parseAffectedEcosystem parses an advisory affected[] entry's ecosystem +// without panicking. The vendored osv-schema ecosystem set is pinned per +// release while osv.dev's set keeps growing, so a multi-ecosystem advisory can +// carry an ecosystem this build doesn't recognize. Skip that entry instead of +// crashing the whole scan, mirroring the "skip, don't panic on unsupported +// ecosystems" handling used for version parsing in rangeContainsVersion. +func parseAffectedEcosystem(affected *osvschema.Affected) (osvecosystem.Parsed, bool) { + ecosystem := affected.GetPackage().GetEcosystem() + parsed, err := osvecosystem.Parse(ecosystem) + if err != nil { + cmdlogger.Debugf("skipping affected entry with unrecognized ecosystem %q: %v", ecosystem, err) + return osvecosystem.Parsed{}, false + } + + return parsed, true +} + func AffectsEcosystem(v *osvschema.Vulnerability, ecosystemAffected osvecosystem.Parsed) bool { for _, affected := range v.GetAffected() { - if osvecosystem.MustParse(affected.GetPackage().GetEcosystem()).Equal(ecosystemAffected) { + affectedEcosystem, ok := parseAffectedEcosystem(affected) + if !ok { + continue + } + if affectedEcosystem.Equal(ecosystemAffected) { return true } } @@ -157,7 +178,11 @@ func IsAffected(v *osvschema.Vulnerability, pkg *extractor.Package) bool { if affected.GetPackage() == nil { continue } - if osvecosystem.MustParse(affected.GetPackage().GetEcosystem()).Equal(imodels.Ecosystem(pkg)) && + affectedEcosystem, ok := parseAffectedEcosystem(affected) + if !ok { + continue + } + if affectedEcosystem.Equal(imodels.Ecosystem(pkg)) && affected.GetPackage().GetName() == imodels.Name(pkg) { if len(affected.GetRanges()) == 0 && len(affected.GetVersions()) == 0 { cmdlogger.Warnf("%s does not have any ranges or versions - this is probably a mistake!", v.GetId()) diff --git a/internal/utility/vulns/vulnerability_test.go b/internal/utility/vulns/vulnerability_test.go index 9efeb489049..0d77fceb28b 100644 --- a/internal/utility/vulns/vulnerability_test.go +++ b/internal/utility/vulns/vulnerability_test.go @@ -86,6 +86,16 @@ func TestOSV_AffectsEcosystem(t *testing.T) { Ecosystem: "npm", Expected: true, }, + { + // an unrecognized ecosystem entry must be skipped, not panic, while a + // recognized sibling entry is still matched - see issue #2867. + Affected: []*osvschema.Affected{ + {Package: &osvschema.Package{Ecosystem: "SomeFutureEcosystem"}}, + {Package: &osvschema.Package{Ecosystem: "npm"}}, + }, + Ecosystem: "npm", + Expected: true, + }, } for i, tt := range tests { @@ -736,6 +746,47 @@ func TestOSV_EcosystemsWithSuffix(t *testing.T) { } } +// TestOSV_IsAffected_UnrecognizedEcosystemDoesNotPanic is a regression test for +// https://github.com/google/osv-scanner/issues/2867, where IsAffected panicked +// (`osvecosystem.MustParse`) on a multi-ecosystem advisory carrying an ecosystem +// the build's pinned osv-schema doesn't recognize. The unrecognized affected[] +// entry must be skipped rather than crash the scan, and a sibling entry in a +// recognized ecosystem must still match. +func TestOSV_IsAffected_UnrecognizedEcosystemDoesNotPanic(t *testing.T) { + t.Parallel() + + defer func() { + if r := recover(); r != nil { + t.Errorf("IsAffected panicked on unrecognized ecosystem: %v", r) + } + }() + + vuln := buildOSVWithAffected( + // an ecosystem this build's vendored osv-schema does not know - reached + // because the package is not in range for the recognized sibling below. + &osvschema.Affected{ + Package: &osvschema.Package{Ecosystem: "SomeFutureEcosystem", Name: "my-package"}, + Ranges: []*osvschema.Range{ + buildEcosystemAffectsRange(&osvschema.Event{Introduced: "0"}), + }, + }, + &osvschema.Affected{ + Package: &osvschema.Package{Ecosystem: string(osvconstants.EcosystemNPM), Name: "my-package"}, + Ranges: []*osvschema.Range{ + buildEcosystemAffectsRange( + &osvschema.Event{Introduced: "5.0.0"}, + &osvschema.Event{Fixed: "6.0.0"}, + ), + }, + }, + ) + + // the unrecognized entry is skipped; the npm entry does not cover 1.0.0 + expectIsAffected(t, vuln, "1.0.0", false) + // the npm entry does cover 5.1.0, proving matching still works post-skip + expectIsAffected(t, vuln, "5.1.0", true) +} + // TestOSV_IsAffected_UnsupportedEcosystemDoesNotPanic is a regression test for // https://github.com/google/osv-scanner/issues/2831, where an SBOM containing // a `pkg:github/...` component crashed the offline matcher because From f0fc7c41970ee0bc78c88cc848546099ca08b90c Mon Sep 17 00:00:00 2001 From: Lougarou Date: Fri, 12 Jun 2026 09:01:41 +0200 Subject: [PATCH 2/2] fix: skip nil-package affected entries in AffectsEcosystem Mirror the nil-package guard IsAffected already has so AffectsEcosystem skips affected[] entries without a package and does so silently, rather than relying on the nil-safe getter. Addresses review feedback on #2867. --- internal/utility/vulns/vulnerability.go | 4 ++++ internal/utility/vulns/vulnerability_test.go | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/internal/utility/vulns/vulnerability.go b/internal/utility/vulns/vulnerability.go index 2b32fcc04be..0bbca82731c 100644 --- a/internal/utility/vulns/vulnerability.go +++ b/internal/utility/vulns/vulnerability.go @@ -127,6 +127,10 @@ func parseAffectedEcosystem(affected *osvschema.Affected) (osvecosystem.Parsed, func AffectsEcosystem(v *osvschema.Vulnerability, ecosystemAffected osvecosystem.Parsed) bool { for _, affected := range v.GetAffected() { + // Assume vulnerability has already been validated + if affected.GetPackage() == nil { + continue + } affectedEcosystem, ok := parseAffectedEcosystem(affected) if !ok { continue diff --git a/internal/utility/vulns/vulnerability_test.go b/internal/utility/vulns/vulnerability_test.go index 0d77fceb28b..16750db3ff5 100644 --- a/internal/utility/vulns/vulnerability_test.go +++ b/internal/utility/vulns/vulnerability_test.go @@ -96,6 +96,16 @@ func TestOSV_AffectsEcosystem(t *testing.T) { Ecosystem: "npm", Expected: true, }, + { + // a nil package entry must be skipped (as IsAffected already does), + // while a recognized sibling entry is still matched. + Affected: []*osvschema.Affected{ + {Package: nil}, + {Package: &osvschema.Package{Ecosystem: "npm"}}, + }, + Ecosystem: "npm", + Expected: true, + }, } for i, tt := range tests {