From 1a9e744d485c4cce6f265e5d3d37c5c93143225c Mon Sep 17 00:00:00 2001 From: Ben Date: Wed, 2 Sep 2026 14:51:20 +0300 Subject: [PATCH 1/2] feat(kubescape): add vulnerability drill-down tools and bound CVE output An agent could not answer "what are the worst vulnerabilities in my cluster?" without either being lied to or blowing its context window. The only route to any severity information was list_vulnerabilities, which returned 200,793 B (~50k tokens) for a single image, and nothing aggregated across manifests at all, so cluster-scope questions had no answer at any price. The Kubescape storage API already exposes a full aggregation ladder that this provider never used: cluster -> namespace -> workload -> manifest -> CVE, where every level except the last is a handful of integers plus a reference. Two new tools walk it: kubescape_vulnerability_overview Severity totals per namespace from the server-side aggregates, worst first. One call, ~1 KB, from vulnerabilitysummaries (cluster-scoped, one object per namespace). kubescape_list_vulnerable_workloads Workloads ranked by severity, each row carrying the manifest_name that kubescape_list_vulnerabilities takes, read from the summary's spec.vulnerabilitiesRef. One call, ~1.9 KB per workload. Both take ListOptions.ResourceVersion = "fullSpec". That sentinel is load-bearing and not obvious. The storage server strips spec from EVERY list response by default, so severity counters come back zero and vulnerabilitiesRef comes back empty -- not an error, and indistinguishable from a clean cluster. Measured on storage v0.0.298, nginx:1.14.0 has 76 critical CVEs and a default LIST of its summary reports 0. This is the same failure shape as the vulnerability_count bug, one level up, so listing these resources without the sentinel would report a vulnerable cluster as clean. Verified against a live cluster that the typed client transmits it: a wire capture shows resourceVersion=fullSpec on the request, and the returned counts match the values an individual GET reports. Because that correctness rests on a vendor sentinel, the overview also refuses to present an all-zero cluster as good news: it says the result could not be confirmed and points at a per-manifest check. A wrong "no vulnerabilities" is the most expensive answer this provider can give. kubescape_list_vulnerabilities is reshaped to fit the ladder: - severity_summary always describes the WHOLE manifest, even when the array is filtered or truncated, so the aggregate survives a bounded response. - The array is capped by limit (default 20), ordered worst severity first with id as a tiebreak for stable output, and reports total_count, returned_count and truncated so the agent knows it holds partial data. - Records carry only id, severity and fix_state. Measured field shares of the old payload: description 56.1%, data_source 19.5%, fix_versions 7.9%. The description was truncated at 200 chars mid-word, so it was lossy prose that invited reasoning from a fragment; the full text, data source and fix versions are all in get_vulnerability_details. - New severity and fixable_only filters for drill-down. - Grype emits one match per affected package, so the same CVE recurs. With the package fields trimmed away those rows are byte-identical, and duplicates would consume the limit budget: the first 20 records for nginx:1.14.0 held CVE-2017-12424 and CVE-2017-15670 twice each. Matches are now collapsed by CVE id with an affected_artifacts count, and a CVE left unfixed in any package is never reported as fixed. 466 matches become 293 distinct CVEs. Measured against the same image on a live cluster: 3,013 B, against 200,793 B before -- 67x smaller, with a complete and correct severity summary. Also fixes a miscount in that summary: severityCounts had no Negligible bucket, so the 102 Negligible CVEs in nginx:1.14.0 were reported as "Unknown": 102. Severity handling is now driven by one ordered list used for both bucketing and ranking, so a severity cannot be dropped into the wrong bucket again. Relevancy is surfaced where the API provides it -- "relevant" counts CVEs whose code node-agent observed loaded at runtime, e.g. 76 critical of which 30 are reachable. It is emitted only when non-zero: the field is `json:"relevant,omitempty"` upstream, so a zero is indistinguishable from "relevancy was never computed", and node-agent needs a learning period before it reports anything. Rendering absent as 0 would claim nothing is reachable when nobody has looked yet. One value from the summaries cannot be trusted. vulnerabilitiesRef reports the WORKLOAD's namespace, but the manifests live in the Kubescape namespace, so following it verbatim yields NotFound -- confirmed on a live cluster, where a GET of docker.io-library-nginx-1.14.0-e34030 in the referenced namespace fails while the same GET in `kubescape` succeeds. Only the name is taken from the ref; the namespace comes from the operator namespace, overridable with kubescape_namespace. Handing the agent a pointer that 404s would defeat the point of the chain. Tests are fake-client based and need no cluster. Note that client-go's fake drops ResourceVersion from recorded actions, so a reactor cannot observe the sentinel; the tests wrap the typed client to record the ListOptions each handler actually passes, which pins the invariant at the real call site. Signed-off-by: Ben Hirschberg Docs-exempt: no existing doc describes the vulnerability tool surface Signed-off-by: Ben --- pkg/kubescape/kubescape.go | 426 ++++++++++++++++++++++++-- pkg/kubescape/kubescape_test.go | 527 +++++++++++++++++++++++++++++++- 2 files changed, 926 insertions(+), 27 deletions(-) diff --git a/pkg/kubescape/kubescape.go b/pkg/kubescape/kubescape.go index e2227fa..4523862 100644 --- a/pkg/kubescape/kubescape.go +++ b/pkg/kubescape/kubescape.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "sort" "strings" "time" @@ -36,8 +37,58 @@ const ( // Pod labels operatorPodLabel = "app.kubernetes.io/name=kubescape-operator" storagePodLabel = "app.kubernetes.io/name=storage" + + // defaultVulnerabilityLimit bounds kubescape_list_vulnerabilities. A single + // image measured 466 CVEs / 200 KB unbounded, which no agent can consume; + // the severity summary carries the aggregate answer instead. + defaultVulnerabilityLimit = 20 + + // defaultWorkloadLimit bounds kubescape_list_vulnerable_workloads. Each + // workload summary renders small, but a large cluster has many of them. + defaultWorkloadLimit = 20 + + // fixStateFixed is the Grype fix state meaning an upgrade is available. + fixStateFixed = "fixed" + + severityUnknown = "Unknown" ) +// severityOrder lists Grype severities worst first. It is both the ranking used +// to sort results and the set of buckets the severity summary reports, so a +// severity can never be silently dropped into the wrong bucket. +var severityOrder = []string{"Critical", "High", "Medium", "Low", "Negligible", severityUnknown} + +// normaliseSeverity maps a Grype severity onto a known bucket, case-insensitively. +// Anything unrecognised becomes Unknown -- but Negligible is a real severity and +// must not land there: 102 Negligible CVEs were previously counted as Unknown. +func normaliseSeverity(s string) string { + for _, known := range severityOrder { + if strings.EqualFold(s, known) { + return known + } + } + return severityUnknown +} + +func isKnownSeverity(s string) bool { + for _, known := range severityOrder { + if strings.EqualFold(s, known) { + return true + } + } + return false +} + +// severityRank returns the sort position of a severity, worst first. +func severityRank(s string) int { + for i, known := range severityOrder { + if s == known { + return i + } + } + return len(severityOrder) +} + // KubescapeTool holds the clients for Kubescape and Kubernetes APIs type KubescapeTool struct { spdxClient spdxv1beta1.SpdxV1beta1Interface @@ -462,6 +513,244 @@ func (k *KubescapeTool) handleCheckHealth(ctx context.Context, request mcp.CallT return mcp.NewToolResultText(string(content)), nil } +// storageFullSpec is the sentinel the Kubescape storage API server requires in +// ListOptions.ResourceVersion to return object specs on LIST. +// +// By default the server strips spec from EVERY list response, so severity +// counters come back as zero and vulnerabilitiesRef comes back empty. That is +// not an error and is indistinguishable from a healthy cluster with no +// vulnerabilities -- listing the summary resources without this sentinel +// reports a vulnerable cluster as clean. Measured on storage v0.0.298: +// nginx:1.14.0 has 76 critical CVEs and a default LIST reports 0. +// +// Defined as ResourceVersionFullSpec in +// github.com/kubescape/storage/pkg/apis/softwarecomposition/register.go. +const storageFullSpec = "fullSpec" + +// fullSpecList returns the ListOptions required to read specs from the storage +// API. Use it for every list of an aggregate resource. +func fullSpecList() metav1.ListOptions { + return metav1.ListOptions{ResourceVersion: storageFullSpec} +} + +// severityCounts renders a SeveritySummary for output. +// +// `relevant` is `json:"relevant,omitempty"` upstream, so a zero is +// indistinguishable from "relevancy was never computed" -- node-agent needs a +// learning period before it reports anything. Emitting a zero there would tell +// an agent that no vulnerability is runtime-reachable when the truth is that +// nobody has looked yet, so the key is omitted unless it holds a real value. +func severityCounts(s v1beta1.SeveritySummary) map[string]map[string]int64 { + out := map[string]map[string]int64{} + for name, c := range map[string]v1beta1.VulnerabilityCounters{ + "Critical": s.Critical, + "High": s.High, + "Medium": s.Medium, + "Low": s.Low, + "Negligible": s.Negligible, + severityUnknown: s.Unknown, + } { + entry := map[string]int64{"all": c.All} + if c.Relevant > 0 { + entry["relevant"] = c.Relevant + } + out[name] = entry + } + return out +} + +// addCounters accumulates src into dst, preserving the omit-when-absent rule. +func addCounters(dst map[string]map[string]int64, src v1beta1.SeveritySummary) { + for name, entry := range severityCounts(src) { + if dst[name] == nil { + dst[name] = map[string]int64{"all": 0} + } + dst[name]["all"] += entry["all"] + if rel, ok := entry["relevant"]; ok { + dst[name]["relevant"] += rel + } + } +} + +func totalAll(counts map[string]map[string]int64) int64 { + var t int64 + for _, c := range counts { + t += c["all"] + } + return t +} + +// handleVulnerabilityOverview reports cluster or namespace vulnerability posture +// from the server-side aggregated summaries. +func (k *KubescapeTool) handleVulnerabilityOverview(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + if k.initError != nil { + toolErr := errors.NewKubescapeError("vulnerability_overview", k.initError) + return toolErr.ToMCPResult(), nil + } + + namespace := mcp.ParseString(request, "namespace", "") + + // VulnerabilitySummary is cluster-scoped and each object's NAME is a + // namespace, so listing returns one aggregate per namespace. + summaries, err := k.spdxClient.VulnerabilitySummaries(metav1.NamespaceAll).List(ctx, fullSpecList()) + if err != nil { + toolErr := errors.NewKubescapeError("vulnerability_overview", err). + WithContext("namespace", namespace) + return toolErr.ToMCPResult(), nil + } + + clusterTotals := map[string]map[string]int64{} + namespaces := []map[string]interface{}{} + for _, summary := range summaries.Items { + if namespace != "" && summary.Name != namespace { + continue + } + addCounters(clusterTotals, summary.Spec.Severities) + namespaces = append(namespaces, map[string]interface{}{ + "namespace": summary.Name, + "workload_count": len(summary.Spec.WorkloadVulnerabilitiesObj), + "severities": severityCounts(summary.Spec.Severities), + }) + } + + // Worst namespace first, so the agent's next call is obvious. + sort.SliceStable(namespaces, func(i, j int) bool { + si := namespaces[i]["severities"].(map[string]map[string]int64) + sj := namespaces[j]["severities"].(map[string]map[string]int64) + if si["Critical"]["all"] != sj["Critical"]["all"] { + return si["Critical"]["all"] > sj["Critical"]["all"] + } + return totalAll(si) > totalAll(sj) + }) + + scope := "cluster" + if namespace != "" { + scope = "namespace" + } + + result := map[string]interface{}{ + "scope": scope, + "cluster_totals": clusterTotals, + "namespaces": namespaces, + "next_step": "call kubescape_list_vulnerable_workloads with a namespace to rank its workloads", + } + + // A cluster with summaries but no counts anywhere is far more likely to mean + // the spec was stripped than that every image is clean. Never present that + // as good news. + if len(namespaces) > 0 && totalAll(clusterTotals) == 0 { + result["warning"] = "Every namespace reported zero vulnerabilities, which could not be confirmed as a genuinely clean cluster: " + + "the Kubescape storage API returns zeroed counts when it strips object specs from list responses. " + + "Verify with kubescape_list_vulnerabilities on a specific manifest before concluding there are no vulnerabilities." + } + + content, err := json.MarshalIndent(result, "", " ") + if err != nil { + return mcp.NewToolResultError(fmt.Sprintf("failed to marshal result: %v", err)), nil + } + return mcp.NewToolResultText(string(content)), nil +} + +// handleListVulnerableWorkloads ranks workloads by vulnerability severity and +// hands back the manifest name needed to drill into each one. +func (k *KubescapeTool) handleListVulnerableWorkloads(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + if k.initError != nil { + toolErr := errors.NewKubescapeError("list_vulnerable_workloads", k.initError) + return toolErr.ToMCPResult(), nil + } + + namespace := mcp.ParseString(request, "namespace", "") + // Where the VulnerabilityManifest objects live, which is not what the + // summaries' vulnerabilitiesRef reports -- see the comment at its use below. + manifestNamespace := mcp.ParseString(request, "kubescape_namespace", defaultKubescapeNamespace) + limit := int(mcp.ParseFloat64(request, "limit", defaultWorkloadLimit)) + if limit < 0 { + limit = 0 + } + + queryNamespace := metav1.NamespaceAll + if namespace != "" { + queryNamespace = namespace + } + + summaries, err := k.spdxClient.VulnerabilityManifestSummaries(queryNamespace).List(ctx, fullSpecList()) + if err != nil { + toolErr := errors.NewKubescapeError("list_vulnerable_workloads", err). + WithContext("namespace", namespace) + return toolErr.ToMCPResult(), nil + } + + workloads := []map[string]interface{}{} + for _, summary := range summaries.Items { + counts := severityCounts(summary.Spec.Severities) + + entry := map[string]interface{}{ + "namespace": summary.Namespace, + "workload_summary": summary.Name, + "severities": counts, + "scan_status": summary.Annotations["kubescape.io/status"], + } + if kind, name := summary.Labels["kubescape.io/workload-kind"], summary.Labels["kubescape.io/workload-name"]; kind != "" && name != "" { + entry["workload"] = kind + "/" + name + } + if container := summary.Labels["kubescape.io/workload-container-name"]; container != "" { + entry["container"] = container + } + if image := summary.Annotations["kubescape.io/image-tag"]; image != "" { + entry["image"] = image + } + // vulnerabilitiesRef points straight at the manifests holding the CVEs: + // `all` is the image-level manifest, `relevant` the workload-filtered + // one. These names are exactly what kubescape_list_vulnerabilities takes. + // + // Only the NAME from the ref is usable. The server fills the ref's + // namespace with the workload's namespace, but the manifests live in the + // Kubescape namespace -- verified on a live cluster, where a GET of + // docker.io-library-nginx-1.14.0-e34030 in the referenced namespace + // returns NotFound while the same GET in `kubescape` succeeds. Passing + // the ref's namespace on would hand the agent a pointer that 404s. + if ref := summary.Spec.Vulnerabilities.ImageVulnerabilitiesObj; ref.Name != "" { + entry["manifest_name"] = ref.Name + entry["manifest_namespace"] = manifestNamespace + } + if ref := summary.Spec.Vulnerabilities.WorkloadVulnerabilitiesObj; ref.Name != "" { + entry["workload_manifest_name"] = ref.Name + } + workloads = append(workloads, entry) + } + + sort.SliceStable(workloads, func(i, j int) bool { + si := workloads[i]["severities"].(map[string]map[string]int64) + sj := workloads[j]["severities"].(map[string]map[string]int64) + if si["Critical"]["all"] != sj["Critical"]["all"] { + return si["Critical"]["all"] > sj["Critical"]["all"] + } + return totalAll(si) > totalAll(sj) + }) + + total := len(workloads) + truncated := false + if limit > 0 && total > limit { + workloads = workloads[:limit] + truncated = true + } + + result := map[string]interface{}{ + "namespace": namespace, + "total_workloads": total, + "returned": len(workloads), + "truncated": truncated, + "workloads": workloads, + "next_step": "call kubescape_list_vulnerabilities with a workload's manifest_name to see its CVEs", + } + + content, err := json.MarshalIndent(result, "", " ") + if err != nil { + return mcp.NewToolResultError(fmt.Sprintf("failed to marshal result: %v", err)), nil + } + return mcp.NewToolResultText(string(content)), nil +} + // handleListVulnerabilityManifests lists vulnerability manifests at image and workload levels func (k *KubescapeTool) handleListVulnerabilityManifests(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { if k.initError != nil { @@ -546,6 +835,21 @@ func (k *KubescapeTool) handleListVulnerabilitiesInManifest(ctx context.Context, return mcp.NewToolResultError("manifest_name parameter is required"), nil } + severityFilter := mcp.ParseString(request, "severity", "") + if severityFilter != "" { + if !isKnownSeverity(severityFilter) { + toolErr := errors.NewKubescapeError("list_vulnerabilities", + fmt.Errorf("invalid severity %q: must be one of %v", severityFilter, severityOrder)) + return toolErr.ToMCPResult(), nil + } + severityFilter = normaliseSeverity(severityFilter) + } + fixableOnly := mcp.ParseBoolean(request, "fixable_only", false) + limit := int(mcp.ParseFloat64(request, "limit", defaultVulnerabilityLimit)) + if limit < 0 { + limit = 0 + } + manifest, err := k.spdxClient.VulnerabilityManifests(namespace).Get(ctx, manifestName, metav1.GetOptions{}) if err != nil { toolErr := errors.NewKubescapeError("get_vulnerability_manifest", err). @@ -554,46 +858,84 @@ func (k *KubescapeTool) handleListVulnerabilitiesInManifest(ctx context.Context, return toolErr.ToMCPResult(), nil } - // Extract vulnerabilities with summary info - vulnerabilities := []map[string]interface{}{} - severityCounts := map[string]int{ - "Critical": 0, - "High": 0, - "Medium": 0, - "Low": 0, - "Unknown": 0, + // Count every match by severity first. The summary describes the WHOLE + // manifest even when the returned array is filtered or truncated, so a + // bounded response still carries the aggregate answer. + severityCounts := map[string]int{} + for _, name := range severityOrder { + severityCounts[name] = 0 } + // Grype reports one Match per affected package, so the same CVE recurs. + // Collapse them by ID -- without the package fields the rows would be + // indistinguishable and duplicates would consume the limit budget -- while + // keeping the number of affected artifacts visible. + matched := []map[string]interface{}{} + byID := map[string]map[string]interface{}{} for _, match := range manifest.Spec.Payload.Matches { vuln := match.Vulnerability - severity := string(vuln.Severity) - if _, exists := severityCounts[severity]; exists { - severityCounts[severity]++ - } else { - severityCounts["Unknown"]++ + severity := normaliseSeverity(string(vuln.Severity)) + severityCounts[severity]++ + + if severityFilter != "" && severity != severityFilter { + continue + } + if fixableOnly && vuln.Fix.State != fixStateFixed { + continue } - vulnInfo := map[string]interface{}{ - "id": vuln.ID, - "severity": severity, - "description": truncateString(vuln.Description, 200), - "data_source": vuln.DataSource, + if existing, seen := byID[vuln.ID]; seen { + existing["affected_artifacts"] = existing["affected_artifacts"].(int) + 1 + // A CVE unfixed in any package is not actionable as "fixed". + if vuln.Fix.State != fixStateFixed { + existing["fix_state"] = vuln.Fix.State + } + continue } - if vuln.Fix.State != "" { - vulnInfo["fix_state"] = vuln.Fix.State - vulnInfo["fix_versions"] = vuln.Fix.Versions + // Only the fields an agent needs to decide what to look at next. + // description (56% of the old payload) is truncated mid-word and is + // available in full from kubescape_get_vulnerability_details, which is + // also where data_source and fix_versions live. + entry := map[string]interface{}{ + "id": vuln.ID, + "severity": severity, + "fix_state": vuln.Fix.State, + "affected_artifacts": 1, } + byID[vuln.ID] = entry + matched = append(matched, entry) + } - vulnerabilities = append(vulnerabilities, vulnInfo) + // Worst first, so a truncated array is the half that matters. Ties break on + // id to keep responses stable between calls. + sort.SliceStable(matched, func(i, j int) bool { + ri, rj := severityRank(matched[i]["severity"].(string)), severityRank(matched[j]["severity"].(string)) + if ri != rj { + return ri < rj + } + return matched[i]["id"].(string) < matched[j]["id"].(string) + }) + + totalMatching := len(matched) + truncated := false + if limit > 0 && totalMatching > limit { + matched = matched[:limit] + truncated = true } result := map[string]interface{}{ "manifest_name": manifestName, "namespace": namespace, - "total_count": len(vulnerabilities), "severity_summary": severityCounts, - "vulnerabilities": vulnerabilities, + "total_count": totalMatching, + "returned_count": len(matched), + "truncated": truncated, + "filters": map[string]interface{}{ + "severity": severityFilter, + "fixable_only": fixableOnly, + }, + "vulnerabilities": matched, } content, err := json.MarshalIndent(result, "", " ") @@ -1056,6 +1398,25 @@ func RegisterTools(s *server.MCPServer, kubeconfig string, readOnly bool) { mcp.WithString("namespace", mcp.Description("Namespace to check (default: kubescape)")), ), telemetry.AdaptToolHandler(telemetry.WithTracing("kubescape_check_health", tool.handleCheckHealth))) + // Cluster / namespace vulnerability posture -- the cheapest entry point + s.AddTool(mcp.NewTool("kubescape_vulnerability_overview", + mcp.WithDescription("START HERE for any question about cluster-wide or namespace-wide vulnerabilities. "+ + "Returns severity totals per namespace from Kubescape's server-side aggregates in a single cheap call, worst namespace first. "+ + "Counts are 'all' plus, where computed, 'relevant' (the vulnerable code was observed loaded at runtime). "+ + "A missing 'relevant' means it was not computed, NOT that the count is zero. "+ + "Then narrow with kubescape_list_vulnerable_workloads."), + mcp.WithString("namespace", mcp.Description("Restrict to one namespace (optional; omit for the whole cluster)")), + ), telemetry.AdaptToolHandler(telemetry.WithTracing("kubescape_vulnerability_overview", tool.handleVulnerabilityOverview))) + + // Rank workloads and hand back the manifest to drill into + s.AddTool(mcp.NewTool("kubescape_list_vulnerable_workloads", + mcp.WithDescription("Rank workloads by vulnerability severity, worst first, and return the 'manifest_name' needed to inspect each one's CVEs. "+ + "Use after kubescape_vulnerability_overview to find which workloads matter, then pass a returned manifest_name to kubescape_list_vulnerabilities."), + mcp.WithString("namespace", mcp.Description("Restrict to one namespace (optional, defaults to all namespaces)")), + mcp.WithNumber("limit", mcp.Description("Maximum workloads to return (default: 20). Use 0 for no limit.")), + mcp.WithString("kubescape_namespace", mcp.Description("Namespace the Kubescape operator is installed in, where vulnerability manifests are stored (default: kubescape)")), + ), telemetry.AdaptToolHandler(telemetry.WithTracing("kubescape_list_vulnerable_workloads", tool.handleListVulnerableWorkloads))) + // List vulnerability manifests s.AddTool(mcp.NewTool("kubescape_list_vulnerability_manifests", mcp.WithDescription("List vulnerability manifests from Kubescape operator. Returns vulnerability scan results at image or workload level."), @@ -1065,9 +1426,14 @@ func RegisterTools(s *server.MCPServer, kubeconfig string, readOnly bool) { // List vulnerabilities in a manifest s.AddTool(mcp.NewTool("kubescape_list_vulnerabilities", - mcp.WithDescription("List all CVEs/vulnerabilities found in a specific vulnerability manifest. Returns severity summary and vulnerability details."), + mcp.WithDescription("List CVEs in a specific vulnerability manifest. Always returns severity_summary, which counts EVERY CVE in the manifest. "+ + "The 'vulnerabilities' array is capped at 'limit' (default 20), ordered worst severity first; check 'truncated' and 'total_count' to see whether more exist. "+ + "Each entry carries only id, severity and fix_state -- use kubescape_get_vulnerability_details for the description, data source and fix versions of one CVE."), mcp.WithString("namespace", mcp.Description("Namespace of the manifest (default: kubescape)")), mcp.WithString("manifest_name", mcp.Description("Name of the vulnerability manifest"), mcp.Required()), + mcp.WithString("severity", mcp.Description("Only return CVEs of this severity: 'Critical', 'High', 'Medium', 'Low', 'Negligible' or 'Unknown'. severity_summary still covers the whole manifest.")), + mcp.WithBoolean("fixable_only", mcp.Description("Only return CVEs that have a fix available (default: false)")), + mcp.WithNumber("limit", mcp.Description("Maximum CVEs to return (default: 20). Use 0 for no limit -- a single image can hold hundreds of CVEs and overflow the context window.")), ), telemetry.AdaptToolHandler(telemetry.WithTracing("kubescape_list_vulnerabilities", tool.handleListVulnerabilitiesInManifest))) // Get detailed vulnerability info @@ -1138,6 +1504,8 @@ func RegisterTools(s *server.MCPServer, kubeconfig string, readOnly bool) { // Interfaces for testing - allows mocking the Kubernetes clients type KubescapeToolInterface interface { HandleCheckHealth(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) + HandleVulnerabilityOverview(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) + HandleListVulnerableWorkloads(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) HandleListVulnerabilityManifests(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) HandleListVulnerabilitiesInManifest(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) HandleGetVulnerabilityDetails(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) @@ -1160,6 +1528,14 @@ func (k *KubescapeTool) HandleCheckHealth(ctx context.Context, request mcp.CallT return k.handleCheckHealth(ctx, request) } +func (k *KubescapeTool) HandleVulnerabilityOverview(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + return k.handleVulnerabilityOverview(ctx, request) +} + +func (k *KubescapeTool) HandleListVulnerableWorkloads(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + return k.handleListVulnerableWorkloads(ctx, request) +} + func (k *KubescapeTool) HandleListVulnerabilityManifests(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { return k.handleListVulnerabilityManifests(ctx, request) } diff --git a/pkg/kubescape/kubescape_test.go b/pkg/kubescape/kubescape_test.go index 2b0bcaf..971c2a0 100644 --- a/pkg/kubescape/kubescape_test.go +++ b/pkg/kubescape/kubescape_test.go @@ -4,10 +4,12 @@ import ( "context" "encoding/json" "errors" + "fmt" "testing" "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" kubescapefake "github.com/kubescape/storage/pkg/generated/clientset/versioned/fake" + spdxv1beta1 "github.com/kubescape/storage/pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" "github.com/stretchr/testify/assert" @@ -46,12 +48,14 @@ func TestRegisterTools(t *testing.T) { }) // Verify tools are registered by checking the server has tools - // NOTE: SBOM tools are disabled (too large for LLM context), so we expect 10 tools + // NOTE: SBOM tools are disabled (too large for LLM context), so we expect 12 tools tools := s.ListTools() - assert.Len(t, tools, 10) + assert.Len(t, tools, 12) expectedTools := map[string]bool{ "kubescape_check_health": false, + "kubescape_vulnerability_overview": false, + "kubescape_list_vulnerable_workloads": false, "kubescape_list_vulnerability_manifests": false, "kubescape_list_vulnerabilities": false, "kubescape_get_vulnerability_details": false, @@ -1191,3 +1195,522 @@ func TestHandleGetNetworkNeighborhood_NotFound(t *testing.T) { // func TestHandleGetSBOM_MissingName(t *testing.T) { ... } // func TestHandleGetSBOM_MissingNamespace(t *testing.T) { ... } // func TestHandleGetSBOM_NotFound(t *testing.T) { ... } + +// --------------------------------------------------------------------------- +// T3: list_vulnerabilities -- bounded output, truthful summary (design 01b) +// --------------------------------------------------------------------------- + +// manifestWithMatches builds a manifest holding n CVEs of the given severity, +// alternating fix state so fixable filtering can be exercised. +func manifestWithMatches(name string, perSeverity map[string]int) *v1beta1.VulnerabilityManifest { + matches := []v1beta1.Match{} + i := 0 + for severity, n := range perSeverity { + for j := 0; j < n; j++ { + fixState := "not-fixed" + if i%2 == 0 { + fixState = "fixed" + } + matches = append(matches, v1beta1.Match{ + Vulnerability: v1beta1.Vulnerability{ + VulnerabilityMetadata: v1beta1.VulnerabilityMetadata{ + ID: fmt.Sprintf("CVE-2024-%s-%04d", severity, j), + Severity: severity, + Description: "a description long enough to matter for payload size, repeated over and over", + DataSource: "https://security-tracker.debian.org/tracker/CVE-2024-0000", + }, + Fix: v1beta1.Fix{State: fixState, Versions: []string{"1.2.3"}}, + }, + }) + i++ + } + } + return &v1beta1.VulnerabilityManifest{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "kubescape"}, + Spec: v1beta1.VulnerabilityManifestSpec{Payload: v1beta1.GrypeDocument{Matches: matches}}, + } +} + +type vulnListResponse struct { + ManifestName string `json:"manifest_name"` + SeveritySummary map[string]int `json:"severity_summary"` + TotalCount int `json:"total_count"` + ReturnedCount int `json:"returned_count"` + Truncated bool `json:"truncated"` + Vulnerabilities []map[string]interface{} `json:"vulnerabilities"` +} + +func listVulns(t *testing.T, manifest *v1beta1.VulnerabilityManifest, args map[string]interface{}) vulnListResponse { + t.Helper() + spdxClient := kubescapefake.NewClientset(manifest) + tool := NewKubescapeToolWithClients(nil, nil, spdxClient.SpdxV1beta1()) + + if args == nil { + args = map[string]interface{}{} + } + args["manifest_name"] = manifest.Name + + result, err := tool.HandleListVulnerabilitiesInManifest(context.Background(), makeRequest(args)) + require.NoError(t, err) + require.False(t, result.IsError, getResultText(result)) + + var resp vulnListResponse + require.NoError(t, json.Unmarshal([]byte(getResultText(result)), &resp)) + return resp +} + +// The whole point of P4: a manifest with hundreds of CVEs must not return +// hundreds of records. nginx:1.14.0 measured 200,793 B / 466 matches. +func TestHandleListVulnerabilities_BoundsOutputByDefault(t *testing.T) { + m := manifestWithMatches("big", map[string]int{ + "Critical": 76, "High": 133, "Medium": 99, "Low": 56, "Negligible": 102, + }) + + resp := listVulns(t, m, nil) + + assert.Equal(t, 466, resp.TotalCount, "total_count must report every CVE, not just the returned ones") + assert.Equal(t, 20, resp.ReturnedCount, "default limit should bound the array") + assert.Len(t, resp.Vulnerabilities, 20) + assert.True(t, resp.Truncated, "the agent must be told it received partial data") +} + +// The severity summary is the cheap answer and must describe the WHOLE manifest +// even when the array is truncated. +func TestHandleListVulnerabilities_SummaryCoversAllMatchesNotJustReturned(t *testing.T) { + m := manifestWithMatches("big", map[string]int{ + "Critical": 76, "High": 133, "Medium": 99, "Low": 56, "Negligible": 102, + }) + + resp := listVulns(t, m, nil) + + assert.Equal(t, 76, resp.SeveritySummary["Critical"]) + assert.Equal(t, 133, resp.SeveritySummary["High"]) + assert.Equal(t, 99, resp.SeveritySummary["Medium"]) + assert.Equal(t, 56, resp.SeveritySummary["Low"]) +} + +// Measured live: 102 Negligible CVEs were reported as "Unknown": 102 because +// severityCounts had no Negligible bucket. +func TestHandleListVulnerabilities_CountsNegligibleNotUnknown(t *testing.T) { + m := manifestWithMatches("negl", map[string]int{"Negligible": 102}) + + resp := listVulns(t, m, nil) + + assert.Equal(t, 102, resp.SeveritySummary["Negligible"], "Negligible must have its own bucket") + assert.Equal(t, 0, resp.SeveritySummary["Unknown"], "Negligible must not be miscounted as Unknown") +} + +// A genuinely unrecognised severity still lands in Unknown. +func TestHandleListVulnerabilities_UnrecognisedSeverityCountsAsUnknown(t *testing.T) { + m := manifestWithMatches("weird", map[string]int{"Bogus": 3}) + + resp := listVulns(t, m, nil) + + assert.Equal(t, 3, resp.SeveritySummary["Unknown"]) +} + +// Per-record fields: description (56.1% of the payload) and data_source (19.5%) +// are dropped; description is truncated mid-word anyway and full text lives in +// get_vulnerability_details. +func TestHandleListVulnerabilities_RecordCarriesOnlyIdSeverityFixState(t *testing.T) { + m := manifestWithMatches("fields", map[string]int{"Critical": 1}) + + resp := listVulns(t, m, nil) + require.Len(t, resp.Vulnerabilities, 1) + + entry := resp.Vulnerabilities[0] + assert.ElementsMatch(t, []string{"id", "severity", "fix_state", "affected_artifacts"}, keysOf(entry)) +} + +func keysOf(m map[string]interface{}) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + return out +} + +// Worst-first, so a truncated array is the part that matters. +func TestHandleListVulnerabilities_SortsBySeverityDescending(t *testing.T) { + m := manifestWithMatches("order", map[string]int{ + "Negligible": 5, "Critical": 2, "Medium": 3, "High": 4, "Low": 1, + }) + + resp := listVulns(t, m, map[string]interface{}{"limit": float64(6)}) + + got := []string{} + for _, v := range resp.Vulnerabilities { + got = append(got, v["severity"].(string)) + } + assert.Equal(t, []string{"Critical", "Critical", "High", "High", "High", "High"}, got) +} + +func TestHandleListVulnerabilities_FiltersBySeverity(t *testing.T) { + m := manifestWithMatches("filter", map[string]int{"Critical": 3, "Low": 7}) + + resp := listVulns(t, m, map[string]interface{}{"severity": "Critical"}) + + assert.Equal(t, 3, resp.TotalCount, "total_count reflects the filter") + assert.Len(t, resp.Vulnerabilities, 3) + for _, v := range resp.Vulnerabilities { + assert.Equal(t, "Critical", v["severity"]) + } + // The summary still describes the whole manifest, so the agent keeps context. + assert.Equal(t, 7, resp.SeveritySummary["Low"]) +} + +func TestHandleListVulnerabilities_FiltersFixableOnly(t *testing.T) { + m := manifestWithMatches("fixable", map[string]int{"Critical": 10}) + + resp := listVulns(t, m, map[string]interface{}{"fixable_only": true}) + + assert.NotZero(t, len(resp.Vulnerabilities)) + for _, v := range resp.Vulnerabilities { + assert.Equal(t, "fixed", v["fix_state"]) + } + assert.Less(t, resp.TotalCount, 10, "fixable_only must actually exclude the unfixed ones") +} + +// An explicit limit above the match count must not claim truncation. +func TestHandleListVulnerabilities_NotTruncatedWhenLimitExceedsMatches(t *testing.T) { + m := manifestWithMatches("small", map[string]int{"High": 3}) + + resp := listVulns(t, m, map[string]interface{}{"limit": float64(100)}) + + assert.Equal(t, 3, resp.TotalCount) + assert.Equal(t, 3, resp.ReturnedCount) + assert.False(t, resp.Truncated) +} + +func TestHandleListVulnerabilities_RejectsInvalidSeverity(t *testing.T) { + m := manifestWithMatches("bad", map[string]int{"High": 1}) + spdxClient := kubescapefake.NewClientset(m) + tool := NewKubescapeToolWithClients(nil, nil, spdxClient.SpdxV1beta1()) + + result, err := tool.HandleListVulnerabilitiesInManifest(context.Background(), makeRequest(map[string]interface{}{ + "manifest_name": "bad", + "severity": "VeryBad", + })) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, getResultText(result), "severity") +} + +// --------------------------------------------------------------------------- +// T1/T2: overview and workload ranking (design 01b) +// +// These read the aggregate resources through the storage API. That API strips +// spec from every LIST unless ResourceVersion is the "fullSpec" sentinel, so a +// default LIST returns all-zero counts -- the P0 failure shape at cluster +// scale. The fake clientset ignores ResourceVersion and returns whatever was +// seeded, so it CANNOT reproduce the stripping. The invariant is therefore +// pinned by asserting the outgoing ListOptions, not by observing the response. +// --------------------------------------------------------------------------- + +func counters(all, relevant int64, withRelevant bool) v1beta1.VulnerabilityCounters { + c := v1beta1.VulnerabilityCounters{All: all} + if withRelevant { + c.Relevant = relevant + } + return c +} + +func nsSummary(namespace string, crit, high int64, refs ...string) *v1beta1.VulnerabilitySummary { + objRefs := []v1beta1.VulnerabilitiesObjScope{} + for _, r := range refs { + objRefs = append(objRefs, v1beta1.VulnerabilitiesObjScope{ + Namespace: namespace, Name: r, Kind: "vulnerabilitymanifestsummary", + }) + } + return &v1beta1.VulnerabilitySummary{ + ObjectMeta: metav1.ObjectMeta{Name: namespace}, + Spec: v1beta1.VulnerabilitySummarySpec{ + Severities: v1beta1.SeveritySummary{ + Critical: counters(crit, crit/2, true), + High: counters(high, 0, false), + }, + WorkloadVulnerabilitiesObj: objRefs, + }, + } +} + +func workloadSummary(namespace, name, imageTag, manifestName string, crit, high int64) *v1beta1.VulnerabilityManifestSummary { + return &v1beta1.VulnerabilityManifestSummary{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + "kubescape.io/image-tag": imageTag, + "kubescape.io/status": "ready", + }, + Labels: map[string]string{ + "kubescape.io/workload-kind": "deployment", + "kubescape.io/workload-name": "app", + "kubescape.io/workload-container-name": "main", + }, + }, + Spec: v1beta1.VulnerabilityManifestSummarySpec{ + Severities: v1beta1.SeveritySummary{ + Critical: counters(crit, crit/2, true), + High: counters(high, 0, false), + }, + Vulnerabilities: v1beta1.VulnerabilitiesComponents{ + ImageVulnerabilitiesObj: v1beta1.VulnerabilitiesObjScope{ + // The server reports the WORKLOAD's namespace here, but the + // manifest actually lives in the Kubescape namespace, so + // this value is unusable. Verified on a live cluster: a GET + // in this namespace returns NotFound. + Namespace: namespace, Name: manifestName, Kind: "vulnerabilitymanifests", + }, + }, + }, + } +} + +// The fake clientset's recorded actions drop ResourceVersion entirely, so a +// reactor cannot see it. These thin decorators wrap the typed client and record +// the ListOptions the handler actually passes, which is the thing under test. + +type recordingSpdx struct { + spdxv1beta1.SpdxV1beta1Interface + recorded *[]metav1.ListOptions +} + +func (r recordingSpdx) VulnerabilitySummaries(ns string) spdxv1beta1.VulnerabilitySummaryInterface { + return recordingVulnSummaries{r.SpdxV1beta1Interface.VulnerabilitySummaries(ns), r.recorded} +} + +func (r recordingSpdx) VulnerabilityManifestSummaries(ns string) spdxv1beta1.VulnerabilityManifestSummaryInterface { + return recordingManifestSummaries{r.SpdxV1beta1Interface.VulnerabilityManifestSummaries(ns), r.recorded} +} + +type recordingVulnSummaries struct { + spdxv1beta1.VulnerabilitySummaryInterface + recorded *[]metav1.ListOptions +} + +func (r recordingVulnSummaries) List(ctx context.Context, opts metav1.ListOptions) (*v1beta1.VulnerabilitySummaryList, error) { + *r.recorded = append(*r.recorded, opts) + return r.VulnerabilitySummaryInterface.List(ctx, opts) +} + +type recordingManifestSummaries struct { + spdxv1beta1.VulnerabilityManifestSummaryInterface + recorded *[]metav1.ListOptions +} + +func (r recordingManifestSummaries) List(ctx context.Context, opts metav1.ListOptions) (*v1beta1.VulnerabilityManifestSummaryList, error) { + *r.recorded = append(*r.recorded, opts) + return r.VulnerabilityManifestSummaryInterface.List(ctx, opts) +} + +func recordingClient(c *kubescapefake.Clientset, sink *[]metav1.ListOptions) spdxv1beta1.SpdxV1beta1Interface { + return recordingSpdx{c.SpdxV1beta1(), sink} +} + +// THE LINCHPIN TEST. Without ResourceVersion="fullSpec" the storage server +// returns spec-stripped objects and every count reads zero -- an agent would be +// told a vulnerable cluster is clean. A fake cannot show that, so assert the +// request instead. +func TestHandleVulnerabilityOverview_RequestsFullSpec(t *testing.T) { + spdxClient := kubescapefake.NewClientset(nsSummary("verify-targets", 99, 242, "deployment-vuln-nginx-nginx")) + var seen []metav1.ListOptions + + tool := NewKubescapeToolWithClients(nil, nil, recordingClient(spdxClient, &seen)) + _, err := tool.HandleVulnerabilityOverview(context.Background(), makeRequest(nil)) + require.NoError(t, err) + + require.Len(t, seen, 1) + assert.Equal(t, storageFullSpec, seen[0].ResourceVersion, + "must request fullSpec: a default LIST returns all-zero counts and would report a vulnerable cluster as clean") +} + +func TestHandleListVulnerableWorkloads_RequestsFullSpec(t *testing.T) { + spdxClient := kubescapefake.NewClientset( + workloadSummary("verify-targets", "deployment-vuln-nginx-nginx", + "docker.io/library/nginx:1.14.0", "docker.io-library-nginx-1.14.0-e34030", 76, 133)) + var seen []metav1.ListOptions + + tool := NewKubescapeToolWithClients(nil, nil, recordingClient(spdxClient, &seen)) + _, err := tool.HandleListVulnerableWorkloads(context.Background(), makeRequest(nil)) + require.NoError(t, err) + + require.Len(t, seen, 1) + assert.Equal(t, storageFullSpec, seen[0].ResourceVersion) +} + +func TestHandleVulnerabilityOverview_AggregatesNamespaces(t *testing.T) { + spdxClient := kubescapefake.NewClientset( + nsSummary("verify-targets", 99, 242, "a", "b"), + nsSummary("other", 1, 2, "c"), + ) + tool := NewKubescapeToolWithClients(nil, nil, spdxClient.SpdxV1beta1()) + + result, err := tool.HandleVulnerabilityOverview(context.Background(), makeRequest(nil)) + require.NoError(t, err) + require.False(t, result.IsError, getResultText(result)) + + var resp struct { + Scope string `json:"scope"` + ClusterTotals map[string]map[string]interface{} `json:"cluster_totals"` + Namespaces []struct { + Namespace string `json:"namespace"` + WorkloadCount int `json:"workload_count"` + } `json:"namespaces"` + } + require.NoError(t, json.Unmarshal([]byte(getResultText(result)), &resp)) + + assert.Equal(t, "cluster", resp.Scope) + assert.Equal(t, float64(100), resp.ClusterTotals["Critical"]["all"], "cluster total sums namespaces") + require.Len(t, resp.Namespaces, 2) + assert.Equal(t, "verify-targets", resp.Namespaces[0].Namespace, "worst namespace first") + assert.Equal(t, 2, resp.Namespaces[0].WorkloadCount) +} + +// relevant is `json:"relevant,omitempty"`, so a zero is indistinguishable from +// "relevancy not computed". Never render it as a measured zero. +func TestHandleVulnerabilityOverview_OmitsRelevantWhenAbsent(t *testing.T) { + spdxClient := kubescapefake.NewClientset(nsSummary("verify-targets", 99, 242, "a")) + tool := NewKubescapeToolWithClients(nil, nil, spdxClient.SpdxV1beta1()) + + result, err := tool.HandleVulnerabilityOverview(context.Background(), makeRequest(nil)) + require.NoError(t, err) + + var resp struct { + ClusterTotals map[string]map[string]interface{} `json:"cluster_totals"` + } + require.NoError(t, json.Unmarshal([]byte(getResultText(result)), &resp)) + + // High was built with no relevant value at all. + _, present := resp.ClusterTotals["High"]["relevant"] + assert.False(t, present, "an absent relevant count must not be reported as 0") + // Critical had one, so it survives. + assert.Equal(t, float64(49), resp.ClusterTotals["Critical"]["relevant"]) +} + +func TestHandleListVulnerableWorkloads_CarriesManifestPointer(t *testing.T) { + spdxClient := kubescapefake.NewClientset( + workloadSummary("verify-targets", "deployment-vuln-nginx-nginx", + "docker.io/library/nginx:1.14.0", "docker.io-library-nginx-1.14.0-e34030", 76, 133)) + tool := NewKubescapeToolWithClients(nil, nil, spdxClient.SpdxV1beta1()) + + result, err := tool.HandleListVulnerableWorkloads(context.Background(), makeRequest(nil)) + require.NoError(t, err) + require.False(t, result.IsError, getResultText(result)) + + var resp struct { + Workloads []map[string]interface{} `json:"workloads"` + } + require.NoError(t, json.Unmarshal([]byte(getResultText(result)), &resp)) + require.Len(t, resp.Workloads, 1) + + w := resp.Workloads[0] + // The whole point of the ladder: the next call is handed over, never guessed. + assert.Equal(t, "docker.io-library-nginx-1.14.0-e34030", w["manifest_name"]) + assert.Equal(t, "docker.io/library/nginx:1.14.0", w["image"]) + // The summary was built with the workload's namespace in the ref, which is + // what the real server sends and where the manifest is NOT. Reporting it + // verbatim hands the agent a pointer that 404s. + assert.Equal(t, "kubescape", w["manifest_namespace"], + "manifest_namespace must be where manifests actually live, not the unusable value in vulnerabilitiesRef") +} + +func TestHandleListVulnerableWorkloads_SortsAndLimits(t *testing.T) { + spdxClient := kubescapefake.NewClientset( + workloadSummary("ns", "low-risk", "img:a", "manifest-a", 1, 0), + workloadSummary("ns", "high-risk", "img:b", "manifest-b", 76, 0), + workloadSummary("ns", "mid-risk", "img:c", "manifest-c", 12, 0), + ) + tool := NewKubescapeToolWithClients(nil, nil, spdxClient.SpdxV1beta1()) + + result, err := tool.HandleListVulnerableWorkloads(context.Background(), makeRequest(map[string]interface{}{ + "limit": float64(2), + })) + require.NoError(t, err) + + var resp struct { + TotalWorkloads int `json:"total_workloads"` + Returned int `json:"returned"` + Truncated bool `json:"truncated"` + Workloads []map[string]interface{} `json:"workloads"` + } + require.NoError(t, json.Unmarshal([]byte(getResultText(result)), &resp)) + + assert.Equal(t, 3, resp.TotalWorkloads) + assert.Equal(t, 2, resp.Returned) + assert.True(t, resp.Truncated) + require.Len(t, resp.Workloads, 2) + assert.Equal(t, "high-risk", resp.Workloads[0]["workload_summary"]) + assert.Equal(t, "mid-risk", resp.Workloads[1]["workload_summary"]) +} + +// If every namespace reports zero, we cannot tell "clean cluster" from +// "fullSpec stopped working". Say so rather than reporting good news. +func TestHandleVulnerabilityOverview_FlagsAllZeroAsUnverified(t *testing.T) { + spdxClient := kubescapefake.NewClientset(nsSummary("verify-targets", 0, 0, "a")) + tool := NewKubescapeToolWithClients(nil, nil, spdxClient.SpdxV1beta1()) + + result, err := tool.HandleVulnerabilityOverview(context.Background(), makeRequest(nil)) + require.NoError(t, err) + + assert.Contains(t, getResultText(result), "could not be confirmed", + "an all-zero result must be reported as unconfirmed, never as a clean cluster") +} + +// Grype emits one Match per affected package, so the same CVE ID recurs. Once +// the package fields are trimmed away those rows are byte-identical and would +// burn the limit budget on duplicates -- measured live, the first 20 records for +// nginx:1.14.0 contained CVE-2017-12424 and CVE-2017-15670 twice each. +func TestHandleListVulnerabilities_DeduplicatesByCVE(t *testing.T) { + dup := func(id, severity, fixState string) v1beta1.Match { + return v1beta1.Match{Vulnerability: v1beta1.Vulnerability{ + VulnerabilityMetadata: v1beta1.VulnerabilityMetadata{ID: id, Severity: severity}, + Fix: v1beta1.Fix{State: fixState}, + }} + } + m := &v1beta1.VulnerabilityManifest{ + ObjectMeta: metav1.ObjectMeta{Name: "dupes", Namespace: "kubescape"}, + Spec: v1beta1.VulnerabilityManifestSpec{Payload: v1beta1.GrypeDocument{Matches: []v1beta1.Match{ + dup("CVE-2017-12424", "Critical", "fixed"), + dup("CVE-2017-12424", "Critical", "fixed"), + dup("CVE-2017-12424", "Critical", "fixed"), + dup("CVE-2020-0001", "High", "not-fixed"), + }}}, + } + + resp := listVulns(t, m, nil) + + // The summary counts matches, matching the numbers Kubescape itself reports. + assert.Equal(t, 3, resp.SeveritySummary["Critical"]) + assert.Equal(t, 1, resp.SeveritySummary["High"]) + + // The array carries distinct CVEs. + require.Len(t, resp.Vulnerabilities, 2) + assert.Equal(t, 2, resp.TotalCount, "total_count counts distinct CVEs in the array") + + assert.Equal(t, "CVE-2017-12424", resp.Vulnerabilities[0]["id"]) + assert.Equal(t, float64(3), resp.Vulnerabilities[0]["affected_artifacts"], + "the collapsed matches must still be visible as a count") + assert.Equal(t, float64(1), resp.Vulnerabilities[1]["affected_artifacts"]) +} + +// A CVE fixed in one package but not another is only actionable if the list says +// so, and "not-fixed" is the safer thing to surface. +func TestHandleListVulnerabilities_DedupeKeepsWorstFixState(t *testing.T) { + mk := func(fixState string) v1beta1.Match { + return v1beta1.Match{Vulnerability: v1beta1.Vulnerability{ + VulnerabilityMetadata: v1beta1.VulnerabilityMetadata{ID: "CVE-2021-1", Severity: "High"}, + Fix: v1beta1.Fix{State: fixState}, + }} + } + m := &v1beta1.VulnerabilityManifest{ + ObjectMeta: metav1.ObjectMeta{Name: "mixed", Namespace: "kubescape"}, + Spec: v1beta1.VulnerabilityManifestSpec{Payload: v1beta1.GrypeDocument{Matches: []v1beta1.Match{mk("fixed"), mk("not-fixed")}}}, + } + + resp := listVulns(t, m, nil) + + require.Len(t, resp.Vulnerabilities, 1) + assert.Equal(t, "not-fixed", resp.Vulnerabilities[0]["fix_state"], + "a CVE unfixed in any package must not be reported as fixed") +} From f53d586a8978a58c2d68bc58e2503771ce3f10b6 Mon Sep 17 00:00:00 2001 From: Ben Date: Wed, 2 Sep 2026 15:07:00 +0300 Subject: [PATCH 2/2] fix(kubescape): correct the relevancy caveat in the overview tool description The description asserted that a missing 'relevant' count means relevancy was not computed. That overstates what can be known: the upstream field is `json:"relevant,omitempty"`, so an absent key is equally a genuine zero, and the two are indistinguishable in the source data. The design proposed deriving a relevancy availability verdict from the summary's kubescape.io/status annotation. Measurement rules that out -- workloads with status=ready were observed with 'relevant' absent -- so no such field is emitted. The description now states the ambiguity in both directions and tells the caller not to read an absent value as a measured zero. Signed-off-by: Ben Hirschberg Docs-exempt: tool description wording, no behavioral change Signed-off-by: Ben --- pkg/kubescape/kubescape.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/pkg/kubescape/kubescape.go b/pkg/kubescape/kubescape.go index 4523862..9ce1b16 100644 --- a/pkg/kubescape/kubescape.go +++ b/pkg/kubescape/kubescape.go @@ -538,8 +538,15 @@ func fullSpecList() metav1.ListOptions { // `relevant` is `json:"relevant,omitempty"` upstream, so a zero is // indistinguishable from "relevancy was never computed" -- node-agent needs a // learning period before it reports anything. Emitting a zero there would tell -// an agent that no vulnerability is runtime-reachable when the truth is that -// nobody has looked yet, so the key is omitted unless it holds a real value. +// an agent that no vulnerability is runtime-reachable when the truth may be that +// nobody has looked yet, so the key is omitted unless it holds a real value and +// the tool description spells out that its absence is ambiguous. +// +// There is deliberately no derived "relevancy: available|unavailable" field. +// The obvious signal for one does not work: workloads carrying +// kubescape.io/status=ready were measured with `relevant` absent, so the +// annotation says nothing about whether relevancy was computed. Reporting a +// confident availability verdict from it would be a guess dressed as a fact. func severityCounts(s v1beta1.SeveritySummary) map[string]map[string]int64 { out := map[string]map[string]int64{} for name, c := range map[string]v1beta1.VulnerabilityCounters{ @@ -1402,8 +1409,9 @@ func RegisterTools(s *server.MCPServer, kubeconfig string, readOnly bool) { s.AddTool(mcp.NewTool("kubescape_vulnerability_overview", mcp.WithDescription("START HERE for any question about cluster-wide or namespace-wide vulnerabilities. "+ "Returns severity totals per namespace from Kubescape's server-side aggregates in a single cheap call, worst namespace first. "+ - "Counts are 'all' plus, where computed, 'relevant' (the vulnerable code was observed loaded at runtime). "+ - "A missing 'relevant' means it was not computed, NOT that the count is zero. "+ + "Counts are 'all' plus 'relevant' (the vulnerable code was observed loaded at runtime). "+ + "'relevant' is reported only when greater than zero; when it is absent that means EITHER no runtime-relevant CVEs OR that relevancy "+ + "has not been computed for those workloads yet, and the two cannot be distinguished here -- so do not report an absent 'relevant' as a measured zero. "+ "Then narrow with kubescape_list_vulnerable_workloads."), mcp.WithString("namespace", mcp.Description("Restrict to one namespace (optional; omit for the whole cluster)")), ), telemetry.AdaptToolHandler(telemetry.WithTracing("kubescape_vulnerability_overview", tool.handleVulnerabilityOverview)))