Skip to content

OperatorCommand can falsely mark configuration scans successful #453

Open

Description

@ANAMASGARD

Description

An OperatorCommand running kubescapeScan or kubescapeScanJob can be marked status.completed=true with no status.error before the scanner finishes. It can also receive that successful status after the scanner fails.

We found this while tracing a CRD-triggered configuration scan from dispatch through the scanner status poll and the Kubernetes status patch. There are two false-success paths in the pinned upstream source:

  1. After the scanner accepts an asynchronous POST /v1/scan, kubescapeScan schedules a poll and returns without a terminal result. The generic command handler then writes success.
  2. On a later poll, every response other than busy is treated as success. But the scanner's /v1/status response only establishes whether that scan ID is still busy. Both successful and failed executions can return notBusy.

This matters to automation and dashboards that use OperatorCommand.status.completed and status.error to decide whether a configuration scan ran successfully. A failed scan can appear green. This concerns scan execution status, not whether a completed scan found failing security controls.

Environment

  • Operator source: 9f88e2e48b4dfd6cac9daf2605300ca2161937ff
  • Scanner source: 32da7c733db298b2f3af4163772f1514ab2d0685
  • Go version required by the operator baseline: 1.26.9
  • The deterministic reproduction below uses a local HTTP server and fake Kubernetes dynamic client; no cluster is required.

Steps To Reproduce

  1. Check out the affected operator revision:

    git clone https://github.com/kubescape/operator.git
    cd operator
    git checkout --detach 9f88e2e48b4dfd6cac9daf2605300ca2161937ff
  2. Save the following file as mainhandler/false_success_compact_repro_test.go:

    package mainhandler
    
    import (
    	"context"
    	"encoding/json"
    	"net/http"
    	"net/http/httptest"
    	"strings"
    	"testing"
    	"time"
    
    	utilsmetadata "github.com/armosec/utils-k8s-go/armometadata"
    	"github.com/kubescape/backend/pkg/command/types/v1alpha1"
    	"github.com/kubescape/k8s-interface/k8sinterface"
    	"github.com/kubescape/operator/config"
    	"github.com/kubescape/operator/utils"
    	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
    	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
    	"k8s.io/apimachinery/pkg/runtime"
    	"k8s.io/client-go/dynamic/fake"
    	ktesting "k8s.io/client-go/testing"
    )
    
    // notBusy also follows a failed scan; it is not proof of success.
    func TestNotBusyMustNotCompleteOperatorCommandAsSuccess(t *testing.T) {
    	const scanID = "99a48a8c-0795-42f3-b009-75f7eab1df75"
    	scanner := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
    		if r.Method != http.MethodGet || r.URL.Path != "/v1/status" || r.URL.Query().Get("ID") != scanID {
    			t.Errorf("unexpected scanner request: %s %s", r.Method, r.URL)
    		}
    		_, _ = w.Write([]byte(`{"type":"notBusy"}`))
    	}))
    	defer scanner.Close()
    	previousClient := KubescapeHttpClient
    	KubescapeHttpClient = scanner.Client()
    	defer func() { KubescapeHttpClient = previousClient }()
    
    	cfg, err := config.NewOperatorConfig(config.CapabilitiesConfig{},
    		utilsmetadata.ClusterConfig{KubescapeURL: strings.TrimPrefix(scanner.URL, "http://")},
    		nil, config.Config{Namespace: "kubescape"})
    	if err != nil {
    		t.Fatal(err)
    	}
    
    	dynamic := fake.NewSimpleDynamicClient(runtime.NewScheme())
    	var patched bool
    	var got struct {
    		Status v1alpha1.OperatorCommandStatus `json:"status"`
    	}
    	dynamic.PrependReactor("patch", v1alpha1.SchemaGroupVersionResource.Resource,
    		func(action ktesting.Action) (bool, runtime.Object, error) {
    			patched = true
    			err := json.Unmarshal(action.(ktesting.PatchAction).GetPatch(), &got)
    			return true, &unstructured.Unstructured{}, err
    		})
    	session := &utils.SessionObj{ParentCommandDetails: &utils.OperatorCommandDetails{
    		Command:   &v1alpha1.OperatorCommand{ObjectMeta: metav1.ObjectMeta{Namespace: "kubescape", Name: "scan-request"}},
    		StartedAt: time.Now(), Client: &k8sinterface.KubernetesApi{DynamicClient: dynamic},
    	}}
    	HandleKubescapeResponse(context.Background(), cfg, &kubescapeResponseData{scanID: scanID, sessionObj: session})
    	if !patched {
    		t.Fatal("no OperatorCommand status patch observed")
    	}
    	if got.Status.Completed && got.Status.Error == nil {
    		t.Fatal("notBusy alone completed the OperatorCommand successfully")
    	}
    }
  3. Run:

    go test ./mainhandler -run '^TestNotBusyMustNotCompleteOperatorCommandAsSuccess$' -count=1 -v
  4. The test fails on the unchanged baseline with:

    notBusy alone completed the OperatorCommand successfully
    

The test intercepts the actual Kubernetes OperatorCommand status patch. Its HTTP server reproduces the scanner's baseline notBusy response; it is a local boundary reproduction, not a claim of a linked live-cluster run.

Expected behavior

  • Acceptance of an asynchronous scan leaves the command pending and records its scan ID.
  • The command becomes successful only after a matching scan ID has a confirmed successful terminal outcome.
  • Scanner failure or cancellation completes the command with an error. An unknown outcome must not be reported as success.
  • Restart recovery resumes an unfinished scan by its recorded ID rather than dispatching it again.

Actual Behavior

The command is marked completed without an error immediately after scan acceptance. The later notBusy poll can write the same successful status even when scanner execution failed.

Additional context

A coordinated fix would add a scan-ID-correlated terminal outcome to the scanner's /v1/status response while retaining its existing busy/notBusy field. The operator would keep accepted commands pending, require a matching successful outcome, and fail closed for failed, cancelled, or unknown outcomes. The scanner change should roll out first.

Candidate signed-off implementations and regression tests are available in the scanner commit and the operator commit. No live-cluster end-to-end verification is claimed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions