Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 34 additions & 17 deletions grype/matcher/apk/matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,33 @@ func (m *Matcher) Type() match.MatcherType {

func (m *Matcher) Match(store vulnerability.Provider, p pkg.Package) ([]match.Match, []match.IgnoreFilter, error) {
var matches []match.Match
var ignoreFilters []match.IgnoreFilter

// direct matches with package itself
directMatches, err := m.findMatchesForPackage(store, p, nil)
// direct matches with package itself (+ distro-fixed ignore rules when metadata implements FileOwner)
directMatches, directIgnores, err := m.findMatchesForPackage(store, p, nil)
if err != nil {
return nil, nil, err
}
matches = append(matches, directMatches...)
ignoreFilters = append(ignoreFilters, directIgnores...)

// indirect matches, via package's origin package
indirectMatches, err := m.findMatchesForOriginPackage(store, p)
indirectMatches, indirectIgnores, err := m.findMatchesForOriginPackage(store, p)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was the last outstanding question -- do we keep the indirect package ignores? I could see either side of the argument. I think we have cases where the version is wrong between the found package and the upstream -- where the upstream is different and we have incorrect matches both FP and FN due to this version mismatch. I guess what we're doing here is aligning the owned packages with the related ones, so it seems reasonable to surface these ignores, probably. I just wanted to note this here, because I don't think we came to a conclusion on Zoom.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we consider upstream fixed for a given vuln, we consider it fixed. Saying we consider it fixed, but then hedging by letting GHSA findings stand about the files the relevant package owns seems like an inconsistent position and I don't see any evidence that it's more correct than this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree; if there is some evidence it is causing FNs, we can always address it later

if err != nil {
return nil, nil, err
}
matches = append(matches, indirectMatches...)
ignoreFilters = append(ignoreFilters, indirectIgnores...)

// APK sources are also able to NAK vulnerabilities, so we want to return these as explicit ignores in order
// to allow rules later to use these to ignore "the same" vulnerability found in "the same" locations
naks, err := m.findNaksForPackage(store, p)
if err != nil {
return nil, nil, err
}
ignoreFilters = append(ignoreFilters, naks...)

return matches, naks, err
return matches, ignoreFilters, nil
}

//nolint:funlen,gocognit
Expand Down Expand Up @@ -169,18 +176,18 @@ func vulnerabilitiesByID(vulns []vulnerability.Vulnerability) map[string][]vulne
return results
}

func (m *Matcher) findMatchesForPackage(store vulnerability.Provider, p pkg.Package, catalogPkg *pkg.Package) ([]match.Match, error) {
func (m *Matcher) findMatchesForPackage(store vulnerability.Provider, p pkg.Package, catalogPkg *pkg.Package) ([]match.Match, []match.IgnoreFilter, error) {
// find SecDB matches for the given package name and version
// APK doesn't use epochs, so pass nil for the config
secDBMatches, _, err := internal.MatchPackageByDistro(store, p, catalogPkg, m.Type(), nil)
secDBMatches, secDBIgnores, err := internal.MatchPackageByDistroWithOwnedFiles(store, p, catalogPkg, m.Type(), nil)
if err != nil {
return nil, err
return nil, nil, err
}

// TODO: are there other errors that we should handle here that causes this to short circuit
cpeMatches, err := m.cpeMatchesWithoutSecDBFixes(store, p)
if err != nil && !errors.Is(err, internal.ErrEmptyCPEMatch) {
return nil, err
return nil, nil, err
}

var matches []match.Match
Expand All @@ -191,25 +198,35 @@ func (m *Matcher) findMatchesForPackage(store vulnerability.Provider, p pkg.Pack
// keep only unique CPE matches
matches = append(matches, deduplicateMatches(secDBMatches, cpeMatches)...)

return matches, nil
return matches, secDBIgnores, nil
}

func (m *Matcher) findMatchesForOriginPackage(store vulnerability.Provider, catalogPkg pkg.Package) ([]match.Match, error) {
func (m *Matcher) findMatchesForOriginPackage(store vulnerability.Provider, catalogPkg pkg.Package) ([]match.Match, []match.IgnoreFilter, error) {
var matches []match.Match
var ignores []match.IgnoreFilter

for _, indirectPackage := range pkg.UpstreamPackages(catalogPkg) {
indirectMatches, err := m.findMatchesForPackage(store, indirectPackage, &catalogPkg)
indirectMatches, indirectIgnores, err := m.findMatchesForPackage(store, indirectPackage, &catalogPkg)
if err != nil {
return nil, fmt.Errorf("failed to find vulnerabilities for apk upstream source package: %w", err)
return nil, nil, fmt.Errorf("failed to find vulnerabilities for apk upstream source package: %w", err)
}
matches = append(matches, indirectMatches...)
ignores = append(ignores, indirectIgnores...)
}

// we want to make certain that we are tracking the match based on the package from the SBOM (not the indirect package)
// however, we also want to keep the indirect package around for future reference
match.ConvertToIndirectMatches(matches, catalogPkg)

return matches, nil
return matches, ignores, nil
}

// ownedFilesFromMetadata returns the files owned by a package if its metadata implements pkg.FileOwner.
func ownedFilesFromMetadata(p pkg.Package) []string {
if fo, ok := p.Metadata.(pkg.FileOwner); ok {
return fo.OwnedFiles()
}
return nil
}

// NAK entries are those reported as explicitly not vulnerable by the upstream provider,
Expand Down Expand Up @@ -248,21 +265,21 @@ func (m *Matcher) findNaksForPackage(provider vulnerability.Provider, p pkg.Pack
naks = append(naks, upstreamNaks...)
}

meta, ok := p.Metadata.(pkg.ApkMetadata)
if !ok {
paths := ownedFilesFromMetadata(p)
if len(paths) == 0 {
return nil, nil
}

var ignores []match.IgnoreFilter
for _, nak := range naks {
for _, f := range meta.Files {
for _, path := range paths {
ignores = append(ignores,
match.IgnoreRule{
Vulnerability: nak.ID,
IncludeAliases: true,
Reason: "Explicit APK NAK",
Package: match.IgnoreRulePackage{
Location: f.Path,
Location: path,
},
})
}
Expand Down
189 changes: 189 additions & 0 deletions grype/matcher/apk/matcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1091,3 +1091,192 @@ func Test_nakIgnoreRules(t *testing.T) {
})
}
}

func TestMatcherApk_DistroFixedIgnoreRules(t *testing.T) {
apkNamespace := "secdb:distro:wolfi:rolling"

apkFiles := pkg.ApkMetadata{Files: []pkg.ApkFileRecord{
{Path: "/usr/bin/kyverno"},
{Path: "/usr/lib/kyverno/config"},
}}

tests := []struct {
name string
p pkg.Package
vulnerabilities []vulnerability.Vulnerability
expectedIgnoreVulnIDs []string
expectedMatchIDs []string
}{
{
name: "package already at fixed version - should produce location-scoped ignore rules but no matches",
p: pkg.Package{
ID: pkg.ID(uuid.NewString()),
Name: "kyverno",
Version: "1.15.3-r0",
Type: syftPkg.ApkPkg,
Distro: distro.New(distro.Wolfi, "", ""),
Metadata: apkFiles,
},
vulnerabilities: []vulnerability.Vulnerability{
{
PackageName: "kyverno",
Constraint: version.MustGetConstraint("< 1.15.3-r0", version.ApkFormat),
Reference: vulnerability.Reference{ID: "CVE-2026-22039", Namespace: apkNamespace},
},
},
// one rule per owned path
expectedIgnoreVulnIDs: []string{"CVE-2026-22039", "CVE-2026-22039"},
expectedMatchIDs: nil,
},
{
name: "package still vulnerable - should produce matches but no ignore rules",
p: pkg.Package{
ID: pkg.ID(uuid.NewString()),
Name: "kyverno",
Version: "1.14.5-r0",
Type: syftPkg.ApkPkg,
Distro: distro.New(distro.Wolfi, "", ""),
Metadata: apkFiles,
},
vulnerabilities: []vulnerability.Vulnerability{
{
PackageName: "kyverno",
Constraint: version.MustGetConstraint("< 1.15.3-r0", version.ApkFormat),
Reference: vulnerability.Reference{ID: "CVE-2026-22039", Namespace: apkNamespace},
},
},
expectedIgnoreVulnIDs: nil,
expectedMatchIDs: []string{"CVE-2026-22039"},
},
{
name: "no distro data for the package - no ignore rules (search miss allows GHSA to stand)",
p: pkg.Package{
ID: pkg.ID(uuid.NewString()),
Name: "something-obscure",
Version: "1.0.0-r0",
Type: syftPkg.ApkPkg,
Distro: distro.New(distro.Wolfi, "", ""),
Metadata: apkFiles,
},
vulnerabilities: []vulnerability.Vulnerability{
{
PackageName: "kyverno",
Constraint: version.MustGetConstraint("< 1.15.3-r0", version.ApkFormat),
Reference: vulnerability.Reference{ID: "CVE-2026-22039", Namespace: apkNamespace},
},
},
expectedIgnoreVulnIDs: nil,
expectedMatchIDs: nil,
},
{
name: "upstream package is fixed - should produce location-scoped ignore rules",
p: pkg.Package{
ID: pkg.ID(uuid.NewString()),
Name: "kyverno-cli",
Version: "1.15.3-r0",
Type: syftPkg.ApkPkg,
Distro: distro.New(distro.Wolfi, "", ""),
Metadata: apkFiles,
Upstreams: []pkg.UpstreamPackage{
{
Name: "kyverno",
Version: "1.15.3-r0",
},
},
},
vulnerabilities: []vulnerability.Vulnerability{
{
PackageName: "kyverno",
Constraint: version.MustGetConstraint("< 1.15.3-r0", version.ApkFormat),
Reference: vulnerability.Reference{ID: "CVE-2026-22039", Namespace: apkNamespace},
},
},
// one rule per owned path
expectedIgnoreVulnIDs: []string{"CVE-2026-22039", "CVE-2026-22039"},
expectedMatchIDs: nil,
},
{
name: "fixed CVE with related GHSA - ignore rules include both IDs at all paths",
p: pkg.Package{
ID: pkg.ID(uuid.NewString()),
Name: "kyverno",
Version: "1.15.3-r0",
Type: syftPkg.ApkPkg,
Distro: distro.New(distro.Wolfi, "", ""),
Metadata: apkFiles,
},
vulnerabilities: []vulnerability.Vulnerability{
{
PackageName: "kyverno",
Constraint: version.MustGetConstraint("< 1.15.3-r0", version.ApkFormat),
Reference: vulnerability.Reference{ID: "CVE-2026-22039", Namespace: apkNamespace},
RelatedVulnerabilities: []vulnerability.Reference{
{ID: "GHSA-8p9x-46gm-qfx2", Namespace: "github:language:go"},
},
},
},
// 2 IDs × 2 paths = 4 rules
expectedIgnoreVulnIDs: []string{"CVE-2026-22039", "CVE-2026-22039", "GHSA-8p9x-46gm-qfx2", "GHSA-8p9x-46gm-qfx2"},
expectedMatchIDs: nil,
},
{
name: "no APK metadata (no file list) - should NOT produce ignore rules even if fixed",
p: pkg.Package{
ID: pkg.ID(uuid.NewString()),
Name: "kyverno",
Version: "1.15.3-r0",
Type: syftPkg.ApkPkg,
Distro: distro.New(distro.Wolfi, "", ""),
// no Metadata
},
vulnerabilities: []vulnerability.Vulnerability{
{
PackageName: "kyverno",
Constraint: version.MustGetConstraint("< 1.15.3-r0", version.ApkFormat),
Reference: vulnerability.Reference{ID: "CVE-2026-22039", Namespace: apkNamespace},
},
},
expectedIgnoreVulnIDs: nil,
expectedMatchIDs: nil,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
matcher := Matcher{}

store := mock.VulnerabilityProvider(test.vulnerabilities...)
matches, ignoreFilters, err := matcher.Match(store, test.p)
require.NoError(t, err)

// verify matches
var gotMatchIDs []string
for _, m := range matches {
gotMatchIDs = append(gotMatchIDs, m.Vulnerability.ID)
}
if test.expectedMatchIDs == nil {
assert.Empty(t, gotMatchIDs, "expected no matches")
} else {
assert.ElementsMatch(t, test.expectedMatchIDs, gotMatchIDs, "unexpected match IDs")
}

// verify ignore rules - filter to only DistroPackageFixed rules (not NAK rules)
var gotIgnoreIDs []string
for _, filter := range ignoreFilters {
rule, ok := filter.(match.IgnoreRule)
require.True(t, ok, "expected IgnoreRule type")
if rule.Reason != "DistroPackageFixed" {
continue
}
gotIgnoreIDs = append(gotIgnoreIDs, rule.Vulnerability)
assert.True(t, rule.IncludeAliases, "expected IncludeAliases to be true")
assert.NotEmpty(t, rule.Package.Location, "expected location to be set on DistroPackageFixed rule")
}
if test.expectedIgnoreVulnIDs == nil {
assert.Empty(t, gotIgnoreIDs, "expected no ignore rules")
} else {
assert.ElementsMatch(t, test.expectedIgnoreVulnIDs, gotIgnoreIDs, "unexpected ignore rule vulnerability IDs")
}
})
}
}
Loading
Loading