From d438521f8f556fde27c4632a4b97f4f21d4bfd7e Mon Sep 17 00:00:00 2001 From: Dejan Bosanac Date: Wed, 19 Jul 2023 16:52:58 +0200 Subject: [PATCH] feat: allow filtering of CertifyVuln query results based on whether they have vulnerabilities (#1073) Signed-off-by: Dejan Bosanac --- pkg/assembler/backends/helper/validation.go | 2 +- pkg/assembler/backends/inmem/certifyVuln.go | 55 ++++++++++++------- .../graphql/generated/root_.generated.go | 10 ++-- pkg/assembler/graphql/model/nodes.go | 10 ++-- .../graphql/schema/certifyVuln.graphql | 10 ++-- 5 files changed, 50 insertions(+), 37 deletions(-) diff --git a/pkg/assembler/backends/helper/validation.go b/pkg/assembler/backends/helper/validation.go index 09bf30ce1e..98427e7098 100644 --- a/pkg/assembler/backends/helper/validation.go +++ b/pkg/assembler/backends/helper/validation.go @@ -57,7 +57,7 @@ func ValidateVulnerabilityQueryFilter(vulnerability *model.VulnerabilitySpec, no if vulnerability.Cve != nil { vulnDefined = vulnDefined + 1 } - if noVulnAllowed && vulnerability.NoVuln != nil && *vulnerability.NoVuln { + if noVulnAllowed && vulnerability.NoVuln != nil { if vulnDefined != 0 { return gqlerror.Errorf("Since NoVuln is set, no other vulnerability type is allowed") } diff --git a/pkg/assembler/backends/inmem/certifyVuln.go b/pkg/assembler/backends/inmem/certifyVuln.go index ae787ba58a..3314b82827 100644 --- a/pkg/assembler/backends/inmem/certifyVuln.go +++ b/pkg/assembler/backends/inmem/certifyVuln.go @@ -17,6 +17,7 @@ package inmem import ( "context" + "reflect" "strconv" "time" @@ -299,7 +300,6 @@ func (c *demoClient) CertifyVuln(ctx context.Context, filter *model.CertifyVulnS } if !foundOne && filter != nil && filter.Vulnerability != nil && filter.Vulnerability.NoVuln != nil && *filter.Vulnerability.NoVuln { - search = append(search, c.noKnownVulnNode.certifyVulnLinks...) foundOne = true } @@ -358,7 +358,7 @@ func (c *demoClient) addCVIfMatch(out []*model.CertifyVuln, if err != nil { return nil, err } - if foundCertifyVuln == nil { + if foundCertifyVuln == nil || reflect.ValueOf(foundCertifyVuln.Vulnerability).IsNil() { return out, nil } return append(out, foundCertifyVuln), nil @@ -383,7 +383,7 @@ func (c *demoClient) buildCertifyVulnerability(link *vulnerabilityLink, filter * } } - if filter != nil && filter.Vulnerability != nil { + if filter != nil && filter.Vulnerability != nil && filter.Vulnerability.NoVuln == nil { if filter.Vulnerability.Osv != nil && link.osvID != 0 { osv, err = c.buildOsvResponse(link.osvID, filter.Vulnerability.Osv) if err != nil { @@ -409,28 +409,32 @@ func (c *demoClient) buildCertifyVulnerability(link *vulnerabilityLink, filter * } } } else { - if link.osvID != 0 { - osv, err = c.buildOsvResponse(link.osvID, nil) - if err != nil { - return nil, err + if checkNoVulnFilter(filter, false) { + if link.osvID != 0 { + osv, err = c.buildOsvResponse(link.osvID, nil) + if err != nil { + return nil, err + } } - } - if link.cveID != 0 { - cve, err = c.buildCveResponse(link.cveID, nil) - if err != nil { - return nil, err + if link.cveID != 0 { + cve, err = c.buildCveResponse(link.cveID, nil) + if err != nil { + return nil, err + } } - } - if link.ghsaID != 0 { - ghsa, err = c.buildGhsaResponse(link.ghsaID, nil) - if err != nil { - return nil, err + if link.ghsaID != 0 { + ghsa, err = c.buildGhsaResponse(link.ghsaID, nil) + if err != nil { + return nil, err + } } } - if link.noKnownVulnID != 0 { - noVuln, err = c.buildNoVulnResponse() - if err != nil { - return nil, err + if checkNoVulnFilter(filter, true) { + if link.noKnownVulnID != 0 { + noVuln, err = c.buildNoVulnResponse() + if err != nil { + return nil, err + } } } } @@ -488,3 +492,12 @@ func (c *demoClient) buildCertifyVulnerability(link *vulnerabilityLink, filter * } return &certifyVuln, nil } + +// Checks if the given filter satisfies the condition for NoVuln in the CertifyVulnSpec. +// It returns true if any of the following conditions are met: +// 1. The filter is nil. +// 2. The filter.Vulnerability is nil. +// 3. The value of filter.Vulnerability.NoVuln matches the expected value. +func checkNoVulnFilter(filter *model.CertifyVulnSpec, expected bool) bool { + return filter == nil || filter.Vulnerability == nil || *filter.Vulnerability.NoVuln == expected +} diff --git a/pkg/assembler/graphql/generated/root_.generated.go b/pkg/assembler/graphql/generated/root_.generated.go index 98690fdfa1..194e5607a3 100644 --- a/pkg/assembler/graphql/generated/root_.generated.go +++ b/pkg/assembler/graphql/generated/root_.generated.go @@ -2976,11 +2976,11 @@ union Vulnerability = OSV | CVE | GHSA | NoVuln VulnerabilitySpec allows using Vulnerability union as input type to be used in read queries. -Either noVuln must be set to true or exactly one of osv, cve or ghsa -must be set to non-nil. Setting noVuln to true means retrieving nodes where -there is no vulnerability attached (thus, the special NoVuln node). Setting one -of the other fields means retrieving certifications for the corresponding -vulnerability types. +Either noVuln must be set or exactly one of osv, cve or ghsa +must be set to non-nil. Setting noVuln to true means retrieving only nodes where +there is no vulnerability attached. Setting it to false means retrieving only nodes +with identified vulnerabilities. Setting one of the other fields means retrieving +certifications for the corresponding vulnerability types. """ input VulnerabilitySpec { osv: OSVSpec diff --git a/pkg/assembler/graphql/model/nodes.go b/pkg/assembler/graphql/model/nodes.go index 0e629931c5..97470c19eb 100644 --- a/pkg/assembler/graphql/model/nodes.go +++ b/pkg/assembler/graphql/model/nodes.go @@ -1358,11 +1358,11 @@ type VulnerabilityMetaDataInput struct { // VulnerabilitySpec allows using Vulnerability union as input type to be used in // read queries. // -// Either noVuln must be set to true or exactly one of osv, cve or ghsa -// must be set to non-nil. Setting noVuln to true means retrieving nodes where -// there is no vulnerability attached (thus, the special NoVuln node). Setting one -// of the other fields means retrieving certifications for the corresponding -// vulnerability types. +// Either noVuln must be set or exactly one of osv, cve or ghsa +// must be set to non-nil. Setting noVuln to true means retrieving only nodes where +// there is no vulnerability attached. Setting it to false means retrieving only nodes +// with identified vulnerabilities. Setting one of the other fields means retrieving +// certifications for the corresponding vulnerability types. type VulnerabilitySpec struct { Osv *OSVSpec `json:"osv,omitempty"` Cve *CVESpec `json:"cve,omitempty"` diff --git a/pkg/assembler/graphql/schema/certifyVuln.graphql b/pkg/assembler/graphql/schema/certifyVuln.graphql index 09ce427425..9a71cc9c16 100644 --- a/pkg/assembler/graphql/schema/certifyVuln.graphql +++ b/pkg/assembler/graphql/schema/certifyVuln.graphql @@ -34,11 +34,11 @@ union Vulnerability = OSV | CVE | GHSA | NoVuln VulnerabilitySpec allows using Vulnerability union as input type to be used in read queries. -Either noVuln must be set to true or exactly one of osv, cve or ghsa -must be set to non-nil. Setting noVuln to true means retrieving nodes where -there is no vulnerability attached (thus, the special NoVuln node). Setting one -of the other fields means retrieving certifications for the corresponding -vulnerability types. +Either noVuln must be set or exactly one of osv, cve or ghsa +must be set to non-nil. Setting noVuln to true means retrieving only nodes where +there is no vulnerability attached. Setting it to false means retrieving only nodes +with identified vulnerabilities. Setting one of the other fields means retrieving +certifications for the corresponding vulnerability types. """ input VulnerabilitySpec { osv: OSVSpec