Skip to content

feat(vulnerability): bulk suppression#440

Open
ybelMekk wants to merge 13 commits into
mainfrom
feat/bulk-suppression
Open

feat(vulnerability): bulk suppression#440
ybelMekk wants to merge 13 commits into
mainfrom
feat/bulk-suppression

Conversation

@ybelMekk
Copy link
Copy Markdown
Contributor

@ybelMekk ybelMekk commented May 18, 2026

Adds cve.workloads(filter: { teamSlugs: [...] }) to scope affected workloads by team, enabling a bulk-suppress UX in the frontend where clients call updateImageVulnerability per selected workload.

Changes

  • New CVEWorkloadsFilter input type with teamSlugs field wired through the CVE.workloads resolver
  • GetWorkloadsByCVE uses a single NamespacesFilter(slugs...) call (team slug = namespace) instead of per-slug loops
  • Unexport fakeVulnerabilitiesClient; drop configurable WorkloadsForVulnerabilityResponse field — fake now returns a default app-with-vulnerabilities node
  • Fix GetCve fake to return a synthetic Cve struct (avoids nil panic in toCVE)
  • Lua integration tests for cve.workloads without filter and with teamSlugs filter

Depends on

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 teamSlug and adds backend grouping + call(s) to v13s.
  • Bumps github.com/nais/v13s/pkg/api to 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 {

Comment thread internal/vulnerability/queries.go Outdated
Comment thread internal/vulnerability/queries.go Outdated
Comment thread internal/vulnerability/queries.go Outdated
Comment thread internal/graph/schema/vulnerability.graphqls Outdated
@thokra-nav
Copy link
Copy Markdown
Contributor

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 updateImageVulnerability på samme måte som eksisterer i dag.

@ybelMekk
Copy link
Copy Markdown
Contributor Author

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.

@thokra-nav
Copy link
Copy Markdown
Contributor

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.
Vi kan utvide cve.workloads med et filter for å kun hente for team brukeren er medlem av f.eks.

@ybelMekk
Copy link
Copy Markdown
Contributor Author

Da vil du hente info om CVE, hvilke workloads som treffes og hvorvidt du kan modifisere de.
Vi kan utvide cve.workloads med et filter for å kun hente for team brukeren er medlem av f.eks.

Du har et poeng der 👍🏾

@ybelMekk
Copy link
Copy Markdown
Contributor Author

Er ikke dette mer riktig:

{
  cve(identifier: "CVE-2018-14721") {
    identifier
    workloads(filter: { teamSlug: "nais-team" }) {
      nodes {
        vulnerability {
          id
        }
        workload {
          name
        }
      }
    }
  }
}

å hente workloads kun for det aktuelle teamet brukeren jobber i?

@thokra-nav
Copy link
Copy Markdown
Contributor

thokra-nav commented May 19, 2026

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 :)

@ybelMekk ybelMekk changed the title feat(vulnerability): bulk suppression mutation feat(vulnerability): bulk suppression May 19, 2026
@ybelMekk ybelMekk requested a review from Copilot May 19, 2026 10:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 applies Offset(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 computing totalCount as the sum of per-team totals, fetching enough rows per team to satisfy offset+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

  • teamSlugs is 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-provided teamSlugs order, 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 in ListCVEs. If a caller includes teamSlugs: ["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 CVE type and its workloads field 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 the CVE fields and the workloads pagination args, and add a description for the new filter argument 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.teamSlugs is 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 suppressVulnerabilities GraphQL mutation, but there are no references to suppressVulnerabilities anywhere 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!

Comment thread internal/vulnerability/queries.go
ybelMekk added 10 commits May 19, 2026 13:57
…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
@ybelMekk ybelMekk force-pushed the feat/bulk-suppression branch from 8b87bbb to d70941b Compare May 19, 2026 11:57
ybelMekk added 2 commits May 19, 2026 14:10
- 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
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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

Comment thread internal/vulnerability/queries.go
Comment thread internal/graph/schema/vulnerability.graphqls
Comment thread internal/graph/schema/vulnerability.graphqls
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants