From a6f69d47a694b1095a36b0a095fa953e3709ef06 Mon Sep 17 00:00:00 2001 From: Ben Date: Wed, 2 Sep 2026 10:05:21 +0300 Subject: [PATCH] fix(kubescape): stop reporting vulnerability_count 0 for every image list_vulnerability_manifests reported "vulnerability_count": 0 for every manifest in every cluster, always. The count came from len(manifest.Spec.Payload.Matches) but the Kubescape aggregated API strips spec.payload.matches on LIST and serves it only on GET -- sensibly, the payloads are megabytes. Matches is therefore nil on every listed object, and len(nil) is 0. This is a silent false negative on a security question. Measured on a live cluster, docker.io/library/nginx:1.14.0 has 466 CVE matches (76 Critical, 133 High, 99 Medium, 56 Low, 102 Negligible, confirmed by GET on the same manifest), and the tool returned: {"image_tag": "docker.io/library/nginx:1.14.0", "manifest_name": "docker.io-library-nginx-1.14.0-e34030", "vulnerability_count": 0} An agent asked for the cluster's most critical CVEs answered "No CVEs detected across all container images ... your cluster's container images currently have no known CVEs". It behaved correctly given its input: it listed the manifests, saw every count at 0, and had no reason to drill in. The data lied to it. The count is not knowable from a LIST response, so it is removed rather than corrected. An absent field cannot be mistaken for a measured zero. Removing it is only safe if the agent is told where counts come from, so the tool description now states that this is an index which does not report vulnerability counts, that an entry appearing here says nothing about whether the image is clean, and that kubescape_list_vulnerabilities returns counts and severities for a given manifest. Without that, absence of data reads as absence of risk -- the same false negative in a different costume. Signed-off-by: Ben Hirschberg Docs-exempt: bug fix; no existing doc describes the list response shape Signed-off-by: Ben --- pkg/kubescape/kubescape.go | 10 ++++-- pkg/kubescape/kubescape_test.go | 58 +++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/pkg/kubescape/kubescape.go b/pkg/kubescape/kubescape.go index e2227fa..9e070c4 100644 --- a/pkg/kubescape/kubescape.go +++ b/pkg/kubescape/kubescape.go @@ -505,6 +505,10 @@ func (k *KubescapeTool) handleListVulnerabilityManifests(ctx context.Context, re vulnerabilityManifests := []map[string]interface{}{} for _, manifest := range manifests.Items { isImageLevel := manifest.Annotations[helpersv1.WlidMetadataKey] == "" + // No vulnerability count is reported here. The aggregated API strips + // spec.payload.matches on LIST and serves it only on GET, so it is nil + // on every listed object and len() would report 0 for every image in + // every cluster. Use kubescape_list_vulnerabilities for real counts. manifestMap := map[string]interface{}{ "namespace": manifest.Namespace, "manifest_name": manifest.Name, @@ -514,7 +518,6 @@ func (k *KubescapeTool) handleListVulnerabilityManifests(ctx context.Context, re "image_tag": manifest.Annotations[helpersv1.ImageTagMetadataKey], "workload_id": manifest.Annotations[helpersv1.WlidMetadataKey], "workload_container_name": manifest.Annotations[helpersv1.ContainerNameMetadataKey], - "vulnerability_count": len(manifest.Spec.Payload.Matches), } vulnerabilityManifests = append(vulnerabilityManifests, manifestMap) } @@ -1058,7 +1061,10 @@ func RegisterTools(s *server.MCPServer, kubeconfig string, readOnly bool) { // 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."), + mcp.WithDescription("List vulnerability manifests from Kubescape operator, at image or workload level. "+ + "This is an index only: it does NOT report how many vulnerabilities each manifest contains, "+ + "and an entry appearing here says nothing about whether that image is clean. "+ + "To get vulnerability counts and severities for a manifest, call kubescape_list_vulnerabilities with its manifest_name."), mcp.WithString("namespace", mcp.Description("Filter by namespace (optional, defaults to all namespaces)")), mcp.WithString("level", mcp.Description("Type of manifests to list: 'image', 'workload', or 'both' (default: both)")), ), telemetry.AdaptToolHandler(telemetry.WithTracing("kubescape_list_vulnerability_manifests", tool.handleListVulnerabilityManifests))) diff --git a/pkg/kubescape/kubescape_test.go b/pkg/kubescape/kubescape_test.go index 2b0bcaf..6218e5b 100644 --- a/pkg/kubescape/kubescape_test.go +++ b/pkg/kubescape/kubescape_test.go @@ -1191,3 +1191,61 @@ func TestHandleGetNetworkNeighborhood_NotFound(t *testing.T) { // func TestHandleGetSBOM_MissingName(t *testing.T) { ... } // func TestHandleGetSBOM_MissingNamespace(t *testing.T) { ... } // func TestHandleGetSBOM_NotFound(t *testing.T) { ... } + +// P0: the aggregated API strips spec.payload.matches on LIST and serves it only +// on GET, so len(Matches) was 0 for every manifest in every cluster, always. +// The tool reported "vulnerability_count": 0 for images with hundreds of CVEs, +// and agents correctly concluded from that data that the cluster was clean. +// The count must not appear in the list response at all: an absent field cannot +// be mistaken for a measured zero. +func TestHandleListVulnerabilityManifests_OmitsVulnerabilityCount(t *testing.T) { + // Matches is nil here exactly as the aggregated API returns it on LIST, + // even though this image really has 466 CVEs. + spdxClient := kubescapefake.NewClientset( + &v1beta1.VulnerabilityManifest{ + ObjectMeta: metav1.ObjectMeta{ + Name: "docker.io-library-nginx-1.14.0-e34030", + Namespace: "kubescape", + Annotations: map[string]string{ + "kubescape.io/image-tag": "docker.io/library/nginx:1.14.0", + }, + }, + }, + ) + + tool := NewKubescapeToolWithClients(nil, nil, spdxClient.SpdxV1beta1()) + + result, err := tool.HandleListVulnerabilityManifests(context.Background(), makeRequest(nil)) + require.NoError(t, err) + require.False(t, result.IsError) + + var response map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(getResultText(result)), &response)) + + manifests := response["vulnerability_manifests"].([]interface{}) + require.Len(t, manifests, 1) + + entry := manifests[0].(map[string]interface{}) + _, present := entry["vulnerability_count"] + assert.False(t, present, + "vulnerability_count must be absent from the list response: it is unknowable on LIST, and reporting 0 tells an agent the image is clean") + + // The fields that identify the manifest must survive, so an agent can still + // drill into it. + assert.Equal(t, "docker.io-library-nginx-1.14.0-e34030", entry["manifest_name"]) + assert.Equal(t, "docker.io/library/nginx:1.14.0", entry["image_tag"]) +} + +// Removing the count is only safe if the agent is told where counts come from. +// Without that, absence of data reads as absence of risk -- the same false +// negative in a different costume. +func TestListVulnerabilityManifestsToolDescribesHowToGetCounts(t *testing.T) { + s := server.NewMCPServer("test", "1.0.0") + RegisterTools(s, "", false) + + tool, ok := s.ListTools()["kubescape_list_vulnerability_manifests"] + require.True(t, ok) + + assert.Contains(t, tool.Tool.Description, "kubescape_list_vulnerabilities", + "the tool must name the tool that returns real counts") +}