Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add ability to ignore packages on pom properties #1345

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
Expand Up @@ -316,6 +316,9 @@ Each rule can specify any combination of the following criteria:
- package language (e.g. `"python"`; these values are defined [here](https://github.com/anchore/syft/blob/main/syft/pkg/language.go#L14-L23))
- package type (e.g. `"npm"`; these values are defined [here](https://github.com/anchore/syft/blob/main/syft/pkg/type.go#L10-L24))
- package location (e.g. `"/usr/local/lib/node_modules/**"`; supports glob patterns)
- pom scope (e.g. `"test"`)
- pom group id (e.g. `"org.springframework.boot"`)
- pom artifact id (e.g. `"spring-boot-starter-webflux"`)

Here's an example `~/.grype.yaml` that demonstrates the expected format for ignore rules:

Expand All @@ -336,6 +339,10 @@ ignore:
# ...or just by a single package field:
- package:
type: gem

# ...or just by a single pom field:
- pom:
scope: test
```

Vulnerability matches will be ignored if **any** rules apply to the match. A rule is considered to apply to a given vulnerability match only if **all** fields specified in the rule apply to the vulnerability match.
Expand Down
49 changes: 49 additions & 0 deletions grype/match/ignore.go
Expand Up @@ -2,6 +2,8 @@ package match

import (
"github.com/bmatcuk/doublestar/v2"

"github.com/anchore/grype/grype/pkg"
)

// An IgnoredMatch is a vulnerability Match that has been ignored because one or more IgnoreRules applied to the match.
Expand All @@ -21,6 +23,7 @@ type IgnoreRule struct {
Namespace string `yaml:"namespace" json:"namespace" mapstructure:"namespace"`
FixState string `yaml:"fix-state" json:"fix-state" mapstructure:"fix-state"`
Package IgnoreRulePackage `yaml:"package" json:"package" mapstructure:"package"`
Pom IgnoreRulePom `yaml:"pom" json:"pom" mapstructure:"pom"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this should be nested under a Java ignore rule container? E.g. the configuration would be:

  # ...or just by a single pom field:
  - java:
      pom:
        scope: test

Copy link
Contributor

Choose a reason for hiding this comment

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

@jneate the team talked about this PR a bit and had a few comments:

The first thing to note is

The simplest change to get this to the finish line: since this is for a package rather than something else we believe this should be nested under the IgnoreRulePackage, probably for now as java (since the metadata is pkg.JavaMetadata rather than Maven POM specific), so the configuration would look something like:

- package:
    java:
      scope: 'test'
      artifact-id: 'log4j'
      group-id: 'org.apache.logging.log4j'

... I think this would basically just involve moving the

Pom           IgnoreRulePom     `yaml:"pom" json:"pom" mapstructure:"pom"`

under the IgnoreRulePackage, as something more like:

Java           IgnoreRuleJava     `yaml:"java" json:"java" mapstructure:"java"`

--

But... I'd also mention there was another thought that we might like to make this more generic, where we could potentially specify any metadata information, something like:

- package:
    metadata:
      - field : .PomScope
        value: test
      - field :  .PomArtifactID
        value: log4j
      - field :  .PomGroupID
        value: 'org.apache.logging.log4j'

Do you think either of these options would be something you'd like to do? If not, we could probably help at least get the first change done to get this to the finish line.

}

// IgnoreRulePackage describes the Package-specific fields that comprise the IgnoreRule.
Expand All @@ -32,6 +35,13 @@ type IgnoreRulePackage struct {
Location string `yaml:"location" json:"location" mapstructure:"location"`
}

// IgnoreRulePom describes the Pom-specific fields that comprise the IgnoreRule.
type IgnoreRulePom struct {
Scope string `yaml:"scope" json:"scope" mapstructure:"scope"`
GroupID string `yaml:"group-id" json:"group-id" mapstructure:"group-id"`
ArtifactID string `yaml:"artifact-id" json:"artifact-id" mapstructure:"artifact-id"`
}

// ApplyIgnoreRules iterates through the provided matches and, for each match,
// determines if the match should be ignored, by evaluating if any of the
// provided IgnoreRules apply to the match. If any rules apply to the match, all
Expand Down Expand Up @@ -123,6 +133,18 @@ func getIgnoreConditionsForRule(rule IgnoreRule) []ignoreCondition {
ignoreConditions = append(ignoreConditions, ifFixStateApplies(fs))
}

if s := rule.Pom.Scope; s != "" {
ignoreConditions = append(ignoreConditions, ifPomScopeApplies(s))
}

if gi := rule.Pom.GroupID; gi != "" {
ignoreConditions = append(ignoreConditions, ifPomGroupIDApplies(gi))
}

if ai := rule.Pom.ArtifactID; ai != "" {
ignoreConditions = append(ignoreConditions, ifPomArtifactIDApplies(ai))
}

return ignoreConditions
}

Expand Down Expand Up @@ -174,6 +196,33 @@ func ifPackageLocationApplies(location string) ignoreCondition {
}
}

func ifPomScopeApplies(scope string) ignoreCondition {
return func(match Match) bool {
if metadata, ok := match.Package.Metadata.(pkg.JavaMetadata); ok {
return scope == metadata.PomScope
}
return false
}
}

func ifPomGroupIDApplies(groupID string) ignoreCondition {
return func(match Match) bool {
if metadata, ok := match.Package.Metadata.(pkg.JavaMetadata); ok {
return groupID == metadata.PomGroupID
}
return false
}
}

func ifPomArtifactIDApplies(artifactID string) ignoreCondition {
return func(match Match) bool {
if metadata, ok := match.Package.Metadata.(pkg.JavaMetadata); ok {
return artifactID == metadata.PomArtifactID
}
return false
}
}

func ruleLocationAppliesToMatch(location string, match Match) bool {
for _, packageLocation := range match.Package.Locations.ToSlice() {
if ruleLocationAppliesToPath(location, packageLocation.RealPath) {
Expand Down
154 changes: 137 additions & 17 deletions grype/match/ignore_test.go
Expand Up @@ -70,19 +70,22 @@ var (
{
Vulnerability: vulnerability.Vulnerability{
ID: "CVE-458",
Namespace: "ruby-vulns",
Namespace: "java-vulns",
Copy link
Contributor

Choose a reason for hiding this comment

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

Has the change to this vulnerability test removed the ruby case? Unclear why it should be removed, should this be added back in as another test case?

Fix: vulnerability.Fix{
State: grypeDb.UnknownFixState,
},
},
Package: pkg.Package{
ID: pkg.ID(uuid.NewString()),
Name: "speach",
Version: "100.0.52",
Language: syftPkg.Ruby,
Type: syftPkg.GemPkg,
Locations: source.NewLocationSet(source.NewVirtualLocation("/real/path/with/speach",
"/virtual/path/that/has/speach")),
Name: "log4j",
Version: "1.1.1",
Language: syftPkg.Java,
Type: syftPkg.JavaPkg,
Metadata: pkg.JavaMetadata{
PomGroupID: "log4j",
PomArtifactID: "log4j-core",
PomScope: "test",
},
},
},
}
Expand Down Expand Up @@ -235,6 +238,7 @@ func TestApplyIgnoreRules(t *testing.T) {
},
expectedRemainingMatches: []Match{
allMatches[0],
allMatches[3],
},
expectedIgnoredMatches: []IgnoredMatch{
{
Expand All @@ -253,14 +257,6 @@ func TestApplyIgnoreRules(t *testing.T) {
},
},
},
{
Match: allMatches[3],
AppliedIgnoreRules: []IgnoreRule{
{
Namespace: "ruby-vulns",
},
},
},
},
},
{
Expand All @@ -275,6 +271,7 @@ func TestApplyIgnoreRules(t *testing.T) {
},
expectedRemainingMatches: []Match{
allMatches[0],
allMatches[3],
},
expectedIgnoredMatches: []IgnoredMatch{
{
Expand All @@ -297,12 +294,86 @@ func TestApplyIgnoreRules(t *testing.T) {
},
},
},
},
},
{
name: "ignore matches on pom scope",
allMatches: allMatches,
ignoreRules: []IgnoreRule{
{
Pom: IgnoreRulePom{
Scope: "test",
},
},
},
expectedRemainingMatches: []Match{
allMatches[0],
allMatches[1],
allMatches[2],
},
expectedIgnoredMatches: []IgnoredMatch{
{
Match: allMatches[3],
AppliedIgnoreRules: []IgnoreRule{
{
Package: IgnoreRulePackage{
Language: string(syftPkg.Ruby),
Pom: IgnoreRulePom{
Scope: "test",
},
},
},
},
},
},
{
name: "ignore matches on pom group id",
allMatches: allMatches,
ignoreRules: []IgnoreRule{
{
Pom: IgnoreRulePom{
GroupID: "log4j",
},
},
},
expectedRemainingMatches: []Match{
allMatches[0],
allMatches[1],
allMatches[2],
},
expectedIgnoredMatches: []IgnoredMatch{
{
Match: allMatches[3],
AppliedIgnoreRules: []IgnoreRule{
{
Pom: IgnoreRulePom{
GroupID: "log4j",
},
},
},
},
},
},
{
name: "ignore matches on pom artifact id",
allMatches: allMatches,
ignoreRules: []IgnoreRule{
{
Pom: IgnoreRulePom{
ArtifactID: "log4j-core",
},
},
},
expectedRemainingMatches: []Match{
allMatches[0],
allMatches[1],
allMatches[2],
},
expectedIgnoredMatches: []IgnoredMatch{
{
Match: allMatches[3],
AppliedIgnoreRules: []IgnoreRule{
{
Pom: IgnoreRulePom{
ArtifactID: "log4j-core",
},
},
},
Expand Down Expand Up @@ -346,6 +417,25 @@ var (
}
)

var (
exampleJavaMatch = Match{
Vulnerability: vulnerability.Vulnerability{
ID: "CVE-2000-1234",
},
Package: pkg.Package{
ID: pkg.ID(uuid.NewString()),
Name: "a-pkg",
Version: "1.0",
Type: "java-archive",
Metadata: pkg.JavaMetadata{
PomGroupID: "example-group",
PomArtifactID: "example-artifact",
PomScope: "test",
},
},
}
)

func TestShouldIgnore(t *testing.T) {
cases := []struct {
name string
Expand Down Expand Up @@ -450,6 +540,36 @@ func TestShouldIgnore(t *testing.T) {
},
expected: false,
},
{
name: "rule applies via pom scope",
match: exampleJavaMatch,
rule: IgnoreRule{
Pom: IgnoreRulePom{
Scope: exampleJavaMatch.Package.Metadata.(pkg.JavaMetadata).PomScope,
},
},
expected: true,
},
{
name: "rule applies via pom group id",
match: exampleJavaMatch,
rule: IgnoreRule{
Pom: IgnoreRulePom{
GroupID: exampleJavaMatch.Package.Metadata.(pkg.JavaMetadata).PomGroupID,
},
},
expected: true,
},
{
name: "rule applies via pom artifact id",
match: exampleJavaMatch,
rule: IgnoreRule{
Pom: IgnoreRulePom{
ArtifactID: exampleJavaMatch.Package.Metadata.(pkg.JavaMetadata).PomArtifactID,
},
},
expected: true,
},
}

for _, testCase := range cases {
Expand Down
1 change: 1 addition & 0 deletions grype/pkg/java_metadata.go
Expand Up @@ -4,6 +4,7 @@ type JavaMetadata struct {
VirtualPath string `json:"virtualPath"`
PomArtifactID string `json:"pomArtifactID"`
PomGroupID string `json:"pomGroupID"`
PomScope string `json:"pomScope"`
ManifestName string `json:"manifestName"`
ArchiveDigests []Digest `json:"archiveDigests"`
}
Expand Down
4 changes: 3 additions & 1 deletion grype/pkg/package.go
Expand Up @@ -238,10 +238,11 @@ func getNameAndELVersion(sourceRpm string) (string, string) {

func javaDataFromPkg(p pkg.Package) (metadata *JavaMetadata) {
if value, ok := p.Metadata.(pkg.JavaMetadata); ok {
var artifact, group, name string
var artifact, group, name, scope string
if value.PomProperties != nil {
artifact = value.PomProperties.ArtifactID
group = value.PomProperties.GroupID
scope = value.PomProperties.Scope
}
if value.Manifest != nil {
if n, ok := value.Manifest.Main["Name"]; ok {
Expand All @@ -263,6 +264,7 @@ func javaDataFromPkg(p pkg.Package) (metadata *JavaMetadata) {
VirtualPath: value.VirtualPath,
PomArtifactID: artifact,
PomGroupID: group,
PomScope: scope,
ManifestName: name,
ArchiveDigests: archiveDigests,
}
Expand Down