feat(vulnerability): bulk suppression#440
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new bulk GraphQL mutation for suppressing a CVE across an explicit set of workloads, wiring it through the GraphQL schema/resolvers into the vulnerability (v13s) client, and updating the v13s dependency to return image details for suppressed workloads.
Changes:
- Introduces
suppressVulnerabilities(input: SuppressVulnerabilitiesInput!): SuppressVulnerabilitiesPayload!and associated input/payload types. - Implements resolver authz checks per unique
teamSlugand adds backend grouping + call(s) to v13s. - Bumps
github.com/nais/v13s/pkg/apito pick up the updated suppress response including image name/tag.
Reviewed changes
Copilot reviewed 5 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/vulnerability/queries.go | Implements bulk suppression logic and maps v13s response back to API payload. |
| internal/vulnerability/models.go | Adds Go models for the new mutation input/payload types. |
| internal/graph/vulnerability.resolvers.go | Adds the new mutation resolver and per-team authorization checks. |
| internal/graph/schema/vulnerability.graphqls | Extends the public GraphQL schema with the new mutation and types. |
| internal/graph/gengql/vulnerability.generated.go | gqlgen output for marshaling/unmarshaling the new schema types. |
| internal/graph/gengql/schema.generated.go | gqlgen output wiring the new mutation into the root schema/resolvers. |
| internal/graph/gengql/root_.generated.go | gqlgen output adding complexity hooks + input unmarshaling registrations. |
| go.mod | Updates v13s dependency version. |
| go.sum | Updates checksums for the new v13s dependency version. |
Files not reviewed (3)
- internal/graph/gengql/root_.generated.go: Language not supported
- internal/graph/gengql/schema.generated.go: Language not supported
- internal/graph/gengql/vulnerability.generated.go: Language not supported
Comments suppressed due to low confidence (3)
internal/graph/schema/vulnerability.graphqls:645
- Missing type-level description for
SuppressVulnerabilitiesWorkloadInput. Add a short description for the input object itself (not only the fields) to meet the schema documentation requirements.
input SuppressVulnerabilitiesWorkloadInput {
internal/graph/schema/vulnerability.graphqls:656
- Missing type-level description for
SuppressVulnerabilitiesPayload. Add a description for the payload type itself to satisfy the schema documentation requirements.
type SuppressVulnerabilitiesPayload {
internal/graph/schema/vulnerability.graphqls:661
- Missing type-level description for
SuppressedVulnerabilityWorkload. Add a brief description for the object type itself (not only its fields) to satisfy the schema documentation requirements.
type SuppressedVulnerabilityWorkload {
|
Hadde ikke sett initiativet før nå, men hva er fordelen av å gjøre dette som en egen mutasjon fremfor at klientene kaller mutasjonen som allerede eksisterer flere ganger? F.eks. i Console så kunne det vært en "Vis CVE på tvers av team", og så kan man huke av for workloads som man skal suppresse. Console kan så gjøre kall mot |
|
Ja, men er ikke use-caset litt forskjellig? For å bruke updateImageVulnerability på tvers av mange workloads for en CVE, må klienten først hente alle vulnerabilities per workload/image for å få tak i de interne vulnerabilityIDene, og deretter gjøre ett kall per workload. suppressVulnerabilities gå rett på cveID + workload-referanser uten de forutgående oppslagene. |
|
Tja, det må jo hentes data om både CVE og workloads uansett. Du kan kjøre denne: query {
cve(identifier: "CVE-2018-14721") {
identifier
workloads {
nodes {
vulnerability {
id
}
workload {
team {
slug
viewerIsMember
viewerIsOwner
}
name
}
}
}
}
}Da vil du hente info om CVE, hvilke workloads som treffes og hvorvidt du kan modifisere de. |
Du har et poeng der 👍🏾 |
|
Er ikke dette mer riktig: å hente workloads kun for det aktuelle teamet brukeren jobber i? |
|
Jo, var det jeg mente :) Vi har vel bare ikke det filteret enda. Og så bør vi kanskje ha det som en liste med team så det kan brukes til litt mer :) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 9 changed files in this pull request and generated 1 comment.
Files not reviewed (3)
- internal/graph/gengql/complexity.go: Language not supported
- internal/graph/gengql/root_.generated.go: Language not supported
- internal/graph/gengql/vulnerability.generated.go: Language not supported
Comments suppressed due to low confidence (6)
internal/vulnerability/queries.go:711
- In the teamSlugs filter path, pagination is effectively broken: the code applies
Limit(page.Limit())per team and never appliesOffset(page.Offset()), then concatenates all nodes and builds a connection using the original page. This can return more than the requested page size, produce incorrect cursors/hasNextPage, and sets totalCount to the number of fetched nodes rather than the real total across teams. Consider computingtotalCountas the sum of per-team totals, fetching enough rows per team to satisfyoffset+limit, then sorting + slicing the merged result to the requested page before building the connection (or implement proper server-side pagination in v13s).
func getWorkloadsByCVEForTeams(ctx context.Context, cve string, page *pagination.Pagination, teamSlugs []slug.Slug) (*WorkloadWithVulnerabilityConnection, error) {
var allNodes []*vulnerabilities.WorkloadForVulnerability
for _, s := range teamSlugs {
ns := string(s)
opts := []vulnerabilities.Option{
vulnerabilities.Limit(page.Limit()),
vulnerabilities.IncludeSuppressed(),
vulnerabilities.ExcludeClustersFilter("management"),
vulnerabilities.NamespaceFilter(ns),
vulnerabilities.Order(vulnerabilities.OrderByNamespace, vulnerabilities.Direction_ASC),
}
resp, err := fromContext(ctx).Client.ListWorkloadsForVulnerability(ctx, vulnerabilities.VulnerabilityFilter{
CveIds: []string{cve},
}, opts...)
if err != nil {
return nil, apierror.Errorf("list workloads for vulnerability by CVE (team %s): %v", s, err)
}
allNodes = append(allNodes, resp.GetNodes()...)
}
return convertWorkloadNodes(ctx, allNodes, page, int32(min(len(allNodes), math.MaxInt32))) //nolint:gosec
}
internal/vulnerability/queries.go:708
teamSlugsis iterated as-is; duplicates will cause duplicate v13s calls and duplicate workloads in the merged result. Also, the merged ordering currently depends on the caller-providedteamSlugsorder, which can make cursor pagination unstable. Deduplicate and apply a deterministic ordering (e.g., sort slugs and/or sort merged nodes by namespace + workload name/type) before building cursors.
func getWorkloadsByCVEForTeams(ctx context.Context, cve string, page *pagination.Pagination, teamSlugs []slug.Slug) (*WorkloadWithVulnerabilityConnection, error) {
var allNodes []*vulnerabilities.WorkloadForVulnerability
for _, s := range teamSlugs {
ns := string(s)
opts := []vulnerabilities.Option{
vulnerabilities.Limit(page.Limit()),
vulnerabilities.IncludeSuppressed(),
vulnerabilities.ExcludeClustersFilter("management"),
vulnerabilities.NamespaceFilter(ns),
vulnerabilities.Order(vulnerabilities.OrderByNamespace, vulnerabilities.Direction_ASC),
}
resp, err := fromContext(ctx).Client.ListWorkloadsForVulnerability(ctx, vulnerabilities.VulnerabilityFilter{
CveIds: []string{cve},
}, opts...)
if err != nil {
return nil, apierror.Errorf("list workloads for vulnerability by CVE (team %s): %v", s, err)
}
allNodes = append(allNodes, resp.GetNodes()...)
}
internal/vulnerability/queries.go:700
- The team-filtered v13s query options omit
ExcludeNamespacesFilter("nais-system"), which is applied in the non-filtered path and inListCVEs. If a caller includesteamSlugs: ["nais-system"]this may expose internal workloads that are otherwise intentionally filtered out. Consider keeping the same namespace exclusion in this code path as well (or explicitly justify/guard it).
opts := []vulnerabilities.Option{
vulnerabilities.Limit(page.Limit()),
vulnerabilities.IncludeSuppressed(),
vulnerabilities.ExcludeClustersFilter("management"),
vulnerabilities.NamespaceFilter(ns),
vulnerabilities.Order(vulnerabilities.OrderByNamespace, vulnerabilities.Direction_ASC),
}
internal/graph/schema/vulnerability.graphqls:366
- The
CVEtype and itsworkloadsfield lost all field/argument descriptions. This schema file otherwise consistently documents fields and args, and these descriptions are public-facing API docs. Please restore descriptions for theCVEfields and theworkloadspagination args, and add a description for the newfilterargument as well.
This issue also appears on line 369 of the same file.
type CVE implements Node {
"The globally unique ID of the CVE."
id: ID!
"The unique identifier of the CVE. E.g. CVE-****-****."
identifier: String!
"Severity of the CVE."
severity: ImageVulnerabilitySeverity!
"Title of the CVE"
title: String!
"Description of the CVE."
description: String!
internal/graph/schema/vulnerability.graphqls:372
CVEWorkloadsFilter.teamSlugsis missing a field description. The schema elsewhere documents input fields, and this filter is part of the public API surface; please add a description explaining expected values and how multiple slugs are combined.
detailsLink: String!
"CVSS score of the CVE."
cvssScore: Float
internal/graph/schema/vulnerability.graphqls:366
- PR description says it adds a new
suppressVulnerabilitiesGraphQL mutation, but there are no references tosuppressVulnerabilitiesanywhere in the repo (schema or resolvers) in this change set. Either the mutation/schema changes are missing from this PR, or the PR description needs to be updated to match what is actually being delivered.
type CVE implements Node {
"The globally unique ID of the CVE."
id: ID!
"The unique identifier of the CVE. E.g. CVE-****-****."
identifier: String!
"Severity of the CVE."
severity: ImageVulnerabilitySeverity!
"Title of the CVE"
title: String!
"Description of the CVE."
description: String!
…ix env/cluster mapping
…r on cve.workloads - Remove suppressVulnerabilities mutation from schema, models, queries, and resolver - Keep SuppressVulnerabilities stub in fake client to satisfy v13s Client interface - Add CVEWorkloadsFilter input type with teamSlugs: [Slug!] field - Add filter arg to cve.workloads; calls v13s once per team slug when set - Extract convertWorkloadNodes helper to avoid duplication between filtered/unfiltered paths - Fix G115 gosec overflow: use min(..., math.MaxInt32) for int->int32 cast - Regenerate GraphQL code
… queries Bump github.com/nais/v13s/pkg/api to v0.0.0-20260519115145-3193309b1b36 which adds NamespacesFilter (nais/v13s#303). Replace the per-slug loop in GetWorkloadsByCVE with a single ListWorkloadsForVulnerability call using NamespacesFilter(slugs...). This eliminates the fetchWorkloadsByCVEForTeams and getWorkloadsByCVEForTeams helpers, adds Offset to the team-filter path (was missing before), and derives TotalCount from PageInfo rather than len(nodes). Update cve_workloads_test.go to test GetWorkloadsByCVE directly: - single upstream call with correct Namespaces filter - Filter.Namespace is empty (NamespacesFilter used exclusively) - TotalCount sourced from PageInfo - empty TeamSlugs falls through to the unfiltered path
8b87bbb to
d70941b
Compare
- Remove cve_workloads_test.go (covered by lua integration tests) - Export FakeVulnerabilitiesClient and add call-recording + configurable response to ListWorkloadsForVulnerability - Remove stale comment from GetWorkloadsByCVE
- Unexport FakeVulnerabilitiesClient -> fakeVulnerabilitiesClient - Drop WorkloadsForVulnerabilityCall and configurable response field - ListWorkloadsForVulnerability returns default app-with-vulnerabilities node - Fix GetCve fake to return synthetic CVE to avoid nil panic in toCVE - Add lua tests for cve.workloads without filter and with teamSlugs filter
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 11 changed files in this pull request and generated 3 comments.
Files not reviewed (3)
- internal/graph/gengql/complexity.go: Language not supported
- internal/graph/gengql/root_.generated.go: Language not supported
- internal/graph/gengql/vulnerability.generated.go: Language not supported
…system in filtered path
- Add description to CVEWorkloadsFilter.teamSlugs per schema conventions
- Apply ExcludeNamespacesFilter("nais-system") in the teamSlugs-filtered path
to prevent callers from exposing system workloads via user-provided slugs
615b7b8 to
c21a32c
Compare
Adds
cve.workloads(filter: { teamSlugs: [...] })to scope affected workloads by team, enabling a bulk-suppress UX in the frontend where clients callupdateImageVulnerabilityper selected workload.Changes
CVEWorkloadsFilterinput type withteamSlugsfield wired through theCVE.workloadsresolverGetWorkloadsByCVEuses a singleNamespacesFilter(slugs...)call (team slug = namespace) instead of per-slug loopsfakeVulnerabilitiesClient; drop configurableWorkloadsForVulnerabilityResponsefield — fake now returns a defaultapp-with-vulnerabilitiesnodeGetCvefake to return a syntheticCvestruct (avoids nil panic intoCVE)cve.workloadswithout filter and withteamSlugsfilterDepends on
repeated string namespacesto theFilterproto andNamespacesFilteroption