Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The deprecated VULNERABILITY_PRIORITY_URGENT ordering behavior is inconsistent with the schema/documentation and the exposed data (urgent counts always 0 at summary/image scope).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates vulnerability priority semantics so that “URGENT” becomes workload-contextual (tied to internet exposure), while “known-exploited” (KEV count) is used for workload-level issue emission; it also updates GraphQL filters to accept multiple priorities and clarifies that URGENT is never reported at image/summary scope.
Changes:
- Updated summary/image vulnerability transforms to always return
urgent = 0at image/summary scope and removed ACT_NOW↔URGENT mapping. - Added workload internet exposure resolution and a priority resolver to gate URGENT behavior based on KEV + confirmed external exposure.
- Updated GraphQL inputs/resolvers/tests to support
priorities: [CVEPriority!]and refreshed messaging and dependency versions.
File summaries
| File | Description |
|---|---|
| internal/workload/application/ingress_class_mapping.go | Exposes ingress class → ingress type classification helper for reuse. |
| internal/vulnerability/transform.go | Stops surfacing URGENT at image/summary scope; adds list-priority mapping helper. |
| internal/vulnerability/transform_test.go | Updates tests for KEV/urgent behavior and adds priority-list mapping tests. |
| internal/vulnerability/sortfilter.go | Adjusts order-by field mapping for workload summaries (incl. URGENT). |
| internal/vulnerability/queries.go | Updates summary/image list queries to use list-based priority filtering. |
| internal/vulnerability/priority.go | Introduces centralized priority resolution (KEV + exposure-based escalation). |
| internal/vulnerability/priority_test.go | Adds coverage for priority resolution rules and reason flags. |
| internal/vulnerability/models.go | Changes filters to Priorities []CVEPriority and adds priorities to image filter model. |
| internal/vulnerability/fake/v13s.go | Updates fake v13s time-series data to new priority fields/expectations. |
| internal/vulnerability/fake/fakedata.go | Updates fake workload summaries to align with new top priority expectations. |
| internal/vulnerability/exposure.go | Adds workload internet exposure resolver from ingress class names. |
| internal/vulnerability/exposure_test.go | Adds tests for exposure resolution across ingress-class combinations. |
| internal/issue/checker/workload_v13s.go | Gates urgent ingress issue emission on KEV count + confirmed external exposure; updates messaging. |
| internal/issue/checker/workload_v13s_test.go | Updates tests to use KevCount and validates urgent gating scenarios. |
| internal/graph/schema/vulnerability.graphqls | Updates filter inputs to list priorities and deprecates URGENT-at-summary/image fields with clarifications. |
| internal/graph/gengql/vulnerability.generated.go | Regenerates gqlgen unmarshalling/marshalling for list-priority inputs. |
| internal/graph/gengql/root_.generated.go | Regenerates schema snapshot with updated inputs/deprecations. |
| integration_tests/vulnerabilities.lua | Updates integration expectations for urgent = 0 at summary scope. |
| integration_tests/issues_for_team.lua | Updates issue message expectations to “known-exploited vulnerabilities”. |
| go.mod | Bumps v13s API module and golang.org/x/crypto versions. |
| go.sum | Updates dependency checksums for the bumped modules. |
Review details
Files not reviewed (1)
- internal/graph/gengql/vulnerability.generated.go: Generated file
- Files reviewed: 19/21 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces public GraphQL API behavior changes plus non-trivial vulnerability/issue semantics updates and dependency bumps that warrant a final human review of compatibility and domain correctness.
Review details
Files not reviewed (1)
- internal/graph/gengql/vulnerability.generated.go: Generated file
- Files reviewed: 19/21 changed files
- Comments generated: 0 new
- Review effort level: Lite
886dd61 to
d3985ff
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The GraphQL schema removes an existing public field (urgent) rather than deprecating it, which is a breaking change for API consumers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- internal/graph/gengql/vulnerability.generated.go: Generated file
- Files reviewed: 20/22 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The GraphQL schema description for the deprecated urgent field is misleading (it claims urgent is “known-exploited” while it always returns 0 at image/summary scope).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- internal/graph/gengql/vulnerability.generated.go: Generated file
- Files reviewed: 20/22 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness/maintainability issues in the updated urgent gating logic and a schema documentation mismatch in generated GraphQL output that should be fixed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- internal/graph/gengql/vulnerability.generated.go: Generated file
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
internal/issue/checker/workload_v13s.go:227
ResolvePriorityis invoked with hardcoded values (HIGH+hasKevEntry=true+ unassigned severity +epssKnown=false), which makes the result depend only onexposure. This is misleading for readers and will silently diverge ifResolvePrioritylogic changes; prefer checking exposure directly here (or pass real inputs).
- Files reviewed: 20/22 changed files
- Comments generated: 1
- Review effort level: Lite
63afd89 to
a5c7a60
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It changes public GraphQL contract semantics around vulnerability priority and updates issue emission logic + dependency versions, which merits final human verification of client impact and behavior.
Review details
Files not reviewed (1)
- internal/graph/gengql/vulnerability.generated.go: Generated file
- Files reviewed: 20/22 changed files
- Comments generated: 1
- Review effort level: Lite
6178e9c to
070f4db
Compare
There was a problem hiding this comment.
🟡 Changes recommended
It introduces a breaking GraphQL schema change by removing the priority input field (replaced by priorities) without an in-schema deprecation/migration path.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- internal/graph/gengql/vulnerability.generated.go: Generated file
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
internal/graph/schema/vulnerability.graphqls:350
- The PR description says the
urgentpriority field was replaced everywhere byknownExploited, but the schema still exposesurgent(deprecated, always 0) for backwards compatibility and tests still query it. Consider updating the PR description to reflect the actual approach (deprecatedurgent+ newknownExploited) so reviewers and release notes are accurate.
- Files reviewed: 18/20 changed files
- Comments generated: 1
- Review effort level: Lite
2806538 to
c765ba0
Compare
… improve error handling
…xploited Bump github.com/nais/v13s/pkg/api to bd6d431 (renames Summary.ActNow to Summary.KevCount, wire-compatible). Remove the always-zero countsByPriority.urgent counter and expose a proper knownExploited count sourced from KevCount instead. Keep severity ints and hasSBOM as deprecated-but-present fields. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The urgent field is now deprecated and documented as always 0, but its description still says "Known-exploited vulnerabilities that require immediate action", which is misleading for API consumers. Update the field description to reflect that it is kept only for backwards compatibility and always returns 0 at image/summary scope. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…d monitor priorities in TeamVulnerabilitySummary
…del and GraphQL schema
… query for filtering
…mOrderFields and GraphQL schema
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
150286f to
8148b00
Compare
| "Risk and threat intelligence signals used to prioritize the image vulnerability." | ||
| riskAssessment: VulnerabilityRiskAssessment! |
There was a problem hiding this comment.
@ybelMekk Hvorfor bruker vi ikke @deprecated her?
| "Whether the CVE is known to be used in ransomware attacks." | ||
| knownRansomwareUse: Boolean! | ||
| "Risk and threat intelligence signals used to prioritize the CVE." | ||
| riskAssessment: VulnerabilityRiskAssessment! |
…ls with deprecated fields, this will be cleaned up when the frontend is updated
|
ok, då har jeg sett på disse :) |
|
Jeg oppdaterer å fikser på den oppfølgnings branch/ ryddig av ting etterpå |
This PR makes
URGENTan explicitly workload-contextual priority.URGENTrequires confirmed workload internet exposure, which cannot be determined at image or summary scope, so image/summary responses now expose the raw CISA KEV count through a newknownExploitedfield instead of overloadingurgent.GraphQL schema changes (additive, non-breaking)
ImageVulnerabilitySummaryCountsByPriority:knownExploited: Int!, the number of vulnerabilities with a CISA Known Exploited Vulnerabilities (KEV) entry.urgent: Int!, now@deprecatedand hardcoded to0at image and summary scope. It is retained for backwards compatibility so existing consumers do not break. Clients should migrate toknownExploited.VULNERABILITY_PRIORITY_URGENTas an order field is@deprecated. It is backed by a count that is always 0, so ordering by it has no effect and falls back to a stable name sort. The enum value is removed entirely in the stacked follow-up #513, so this PR only deprecates it.Priority filtering
TeamVulnerabilitySummaryFilter.prioritiesandImageVulnerabilityFilter.prioritiesaccept a list, replacing the previous single-value filter.TeamVulnerabilitySummaryFilter.priorityis kept and@deprecated. It is mapped internally to a single-elementprioritieslist, so existing clients keep working. If both are set,prioritieswins.URGENTthrough either field returns an error rather than silently matching nothing, since the value is not resolvable at these scopes. This is a behaviour change for any client currently filtering onURGENT.Workload internet exposure
New
internal/vulnerability/exposure.goresolves a workload's internet exposure from its ingress class names intoTRUE,FALSEorUNKNOWN. The issue checker uses this to emit external-ingress urgent/critical vulnerability issues only for workloads with confirmed external exposure and a non-zero KEV count.An earlier iteration added a
ResolvePriorityhelper. It ended up with no production caller once the issue checker gated directly on exposure, so it was removed rather than left as dead logic.Tests
integration_tests/vulnerabilities.luaquery both fields, assertingurgent = 0and a non-nullknownExploited, which locks in the deprecation contract.prioritytoprioritiesmapping, and the summary transform.Dependencies
Bumps
github.com/nais/v13s/pkg/apifor the KEV count, plus routine updates tocloud.google.com/go/*,google.golang.org/api,google.golang.org/genprotoandgoogle.golang.org/grpc.Migration note for consumers
Replace
countsByPriority.urgentwithcountsByPriority.knownExploited, andfilter.prioritywithfilter.priorities. Stop filtering and ordering byURGENTat image and summary scope. No field is removed in this PR, so no consumer breaks on deploy.Follow-up
#513 is stacked on this branch and removes the deprecated
VULNERABILITY_PRIORITY_URGENTorder-by enum value. Merge this PR first.