-
Notifications
You must be signed in to change notification settings - Fork 575
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: James Neate <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jneate -- my main question is if the pom should be a top-level field on the IgnoreRule (I don't know, just posing the question)
Namespace: "ruby-vulns", | ||
Namespace: "java-vulns", |
There was a problem hiding this comment.
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?
grype/match/ignore.go
Outdated
@@ -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"` |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
This to me feels more like a matcher configuration and not additional fields on an ignore rule. The reason being is that matchers are already ecosystem specific and can handle cases where we're adding new attributes to consider for that entire ecosystem. |
* main: (224 commits) fix: only warn missing CPEs if CPEs wanted (anchore#1710) fix: ensure version output to stdout (anchore#1709) chore(deps): update bootstrap tools to latest versions (anchore#1706) chore(deps): update Syft to v0.104.0 (anchore#1704) Bump Syft in Grype to pull in unmarshaling fix (anchore#1703) chore(deps): bump github.com/docker/docker (anchore#1702) chore(deps): bump gorm.io/gorm from 1.25.6 to 1.25.7 (anchore#1700) chore(deps): update bootstrap tools to latest versions (anchore#1698) chore(deps): bump actions/upload-artifact from 4.3.0 to 4.3.1 (anchore#1699) chore(deps): bump github.com/gkampitakis/go-snaps from 0.5.0 to 0.5.2 (anchore#1697) chore(deps): bump peter-evans/create-pull-request from 5.0.2 to 6.0.0 (anchore#1687) chore(deps): bump anchore/sbom-action from 0.15.6 to 0.15.8 (anchore#1690) chore(deps): bump sigstore/cosign-installer from 3.3.0 to 3.4.0 (anchore#1691) chore(deps): bump github.com/docker/docker (anchore#1692) chore(deps): bump github.com/opencontainers/runc from 1.1.5 to 1.1.12 (anchore#1689) Upgrade syft to v0.103.1 (anchore#1688) chore(deps): bump github.com/google/go-containerregistry (anchore#1685) chore(deps): bump anchore/sbom-action from 0.15.5 to 0.15.6 (anchore#1684) ensure releases only use released versions of syft (anchore#1680) chore(deps): bump gorm.io/gorm from 1.25.5 to 1.25.6 (anchore#1683) ... Signed-off-by: Christopher Phillips <[email protected]>
@jneate thanks for the contribution and apologies that this got stale. I've rebased the changes here and will work on getting the comments from @wagoodman and @kzantow addressed! |
* main: chore(deps): update bootstrap tools to latest versions (anchore#1707) test(quality): bump label dataset and images (anchore#1712)
Signed-off-by: Christopher Phillips <[email protected]>
We're back to passing on this PR - I'm now taking it in the direction suggested by @wagoodman where we update the matcher configuration rather than the ignore rules |
This resolves #985
I was 50/50 about adding GroupID and ArtifactID, happy to remove artifact ID as you could use package-name in the package ignore rules but opened the PR anyway just in case it added value.
The artifact ID ignore rules are dependant on anchore/syft#1870 as currently it's blank
Group ID filter
Scope filter
Pom used for testing: