From e58a068dc35cd573b139ee3a83e63caa7ccf1e71 Mon Sep 17 00:00:00 2001 From: Entlein Date: Sat, 16 May 2026 11:05:44 +0200 Subject: [PATCH 1/3] fix(containerprofilecache): honor verify result before merging user overlay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit upstream PR #808 / containerprofilecache.go:414 (Major). The verifyUserApplicationProfile and verifyUserNetworkNeighborhood methods already return a boolean reflecting verification outcome — true when the overlay is unsigned OR when verification succeeded OR in permissive mode (EnableSignatureVerification=false); false only in strict mode on actual tamper. The two call sites in projection-load were discarding that return, so tampered overlays in strict mode silently merged anyway. The R1016 alert was emitted but the protection was advisory only. Now: when verify returns false (strict mode + tamper detected) the overlay is nilled out before the merge step so the cache never projects a known-tampered profile. Permissive mode is unchanged — verify always returns true, the overlay still merges, R1016 still fires. New tests: - TestVerifyAP_StrictMode_ReturnsFalseOnTamper — sign + tamper an ApplicationProfile, construct a cache with EnableSignatureVerification=true, and assert verifyUserApplicationProfile returns false (caller drops overlay). - TestVerifyNN_StrictMode_ReturnsFalseOnTamper — symmetric pin for the NetworkNeighborhood path. The existing legacy-permissive tamper test (TestVerifyAP_TamperedProfile_PopulatesDedupMap) continues to pass unchanged — that path still returns true with the R1016 emitted. --- .../containerprofilecache.go | 15 +++- .../tamper_alert_test.go | 70 +++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/pkg/objectcache/containerprofilecache/containerprofilecache.go b/pkg/objectcache/containerprofilecache/containerprofilecache.go index c539fad0e..eeb401586 100644 --- a/pkg/objectcache/containerprofilecache/containerprofilecache.go +++ b/pkg/objectcache/containerprofilecache/containerprofilecache.go @@ -409,8 +409,17 @@ func (c *ContainerProfileCacheImpl) tryPopulateEntry( // when a signed overlay's signature no longer matches (i.e. content // has been mutated post-sign). No-op when the overlay is unsigned or // the tamper-alert exporter has not been wired. + // CodeRabbit upstream PR #808 / containerprofilecache.go:414 (Major): + // when EnableSignatureVerification=true and the overlay fails + // verification, verifyUserApplicationProfile returns false. Drop the + // failed overlay before merging so a tampered profile does not + // silently project into the cache. In permissive mode the verifier + // always returns true and the overlay still merges (alert-only + // behaviour preserved). if userAP != nil { - c.verifyUserApplicationProfile(userAP, sharedData.Wlid) + if !c.verifyUserApplicationProfile(userAP, sharedData.Wlid) { + userAP = nil + } } var userNNErr error _ = c.refreshRPC(ctx, func(rctx context.Context) error { @@ -426,7 +435,9 @@ func (c *ContainerProfileCacheImpl) tryPopulateEntry( userNN = nil } if userNN != nil { - c.verifyUserNetworkNeighborhood(userNN, sharedData.Wlid) + if !c.verifyUserNetworkNeighborhood(userNN, sharedData.Wlid) { + userNN = nil + } } } diff --git a/pkg/objectcache/containerprofilecache/tamper_alert_test.go b/pkg/objectcache/containerprofilecache/tamper_alert_test.go index 03fa7b0a8..c8af3ce9b 100644 --- a/pkg/objectcache/containerprofilecache/tamper_alert_test.go +++ b/pkg/objectcache/containerprofilecache/tamper_alert_test.go @@ -17,6 +17,7 @@ import ( "sync" "testing" + "github.com/kubescape/node-agent/pkg/config" "github.com/kubescape/node-agent/pkg/hostfimsensor" "github.com/kubescape/node-agent/pkg/malwaremanager" rmtypes "github.com/kubescape/node-agent/pkg/rulemanager/types" @@ -279,3 +280,72 @@ func TestVerifyAP_OperationalError_DoesNotEmit(t *testing.T) { t.Errorf("unsigned AP produced %d R1016 alerts; want 0", got) } } + +// TestVerifyAP_StrictMode_ReturnsFalseOnTamper pins CodeRabbit upstream +// PR #808 / containerprofilecache.go:414 (Major). The fix wires the +// verifyUserApplicationProfile boolean result into the caller so that +// in EnableSignatureVerification=true (strict) mode a tampered overlay +// is NOT merged into the projected profile. This unit-level test pins +// the verifier's strict-mode return contract; the call-site honors the +// return value (drop tampered overlay → userAP = nil). +func TestVerifyAP_StrictMode_ReturnsFalseOnTamper(t *testing.T) { + profile := &v1beta1.ApplicationProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tampered-strict", + Namespace: "test-ns", + ResourceVersion: "1", + UID: "ap-uid-strict", + }, + Spec: v1beta1.ApplicationProfileSpec{ + Containers: []v1beta1.ApplicationProfileContainer{{Name: "test"}}, + }, + } + + adapter := profiles.NewApplicationProfileAdapter(profile) + if err := signature.SignObjectDisableKeyless(adapter); err != nil { + t.Fatalf("sign profile: %v", err) + } + profile.Spec.Containers[0].Name = "TAMPERED" + + // Strict mode: EnableSignatureVerification = true + c := &ContainerProfileCacheImpl{ + cfg: config.Config{EnableSignatureVerification: true}, + } + ok := c.verifyUserApplicationProfile(profile, "wlid://test/cluster/ns/Pod/p") + if ok { + t.Errorf("verify returned true on tampered profile in strict mode; expected false (caller drops overlay)") + } +} + +// TestVerifyNN_StrictMode_ReturnsFalseOnTamper — symmetric pin for the +// NetworkNeighborhood overlay verification path. +func TestVerifyNN_StrictMode_ReturnsFalseOnTamper(t *testing.T) { + nn := &v1beta1.NetworkNeighborhood{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tampered-strict-nn", + Namespace: "test-ns", + ResourceVersion: "1", + UID: "nn-uid-strict", + }, + Spec: v1beta1.NetworkNeighborhoodSpec{ + Containers: []v1beta1.NetworkNeighborhoodContainer{{Name: "test"}}, + }, + } + + adapter := profiles.NewNetworkNeighborhoodAdapter(nn) + if err := signature.SignObjectDisableKeyless(adapter); err != nil { + t.Fatalf("sign nn: %v", err) + } + nn.Spec.Containers[0].Name = "TAMPERED" + + c := &ContainerProfileCacheImpl{ + cfg: config.Config{EnableSignatureVerification: true}, + } + ok := c.verifyUserNetworkNeighborhood(nn, "wlid://test/cluster/ns/Pod/p") + if ok { + t.Errorf("verify returned true on tampered nn in strict mode; expected false (caller drops overlay)") + } +} + +// cfgRef is a minimal config shim for the strict-mode tests. Mirrors the +// concrete config.Config struct shape only in the field the verifier reads. From f310091064e7ef4adefe9cbd86043b3ce309f0c4 Mon Sep 17 00:00:00 2001 From: Entlein Date: Sat, 16 May 2026 11:07:19 +0200 Subject: [PATCH 2/3] docs(applicationprofile): rule-author contracts on opens-with-suffix/prefix + exec-args MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two doc-only fixes for CodeRabbit cross-PR advisory: #7 (NA #807): ap.was_path_opened_with_suffix / _prefix — explicitly document the false-negative gap when the projection is in pass-through mode (cp.Opens.All=true). Wildcard Patterns are skipped from the suffix/prefix scan because their token-bearing text doesn't safely answer suffix questions. Rule authors who need wildcard-aware coverage should either declare an Opens-projection slice (so SuffixHits/PrefixHits become authoritative for the literals they care about) or use ap.was_path_opened (which runs CompareDynamic over Patterns). #8 (NA #807): wasExecutedWithArgs — document the three states of ExecsByPath: 1. Path absent from Execs.Values → exec not allowed, fall through. 2. Path in Values, ABSENT from ExecsByPath → legacy back-compat "no argv constraint", match. 3. Path in Values, PRESENT with empty arg list [] → explicit "ran with no args" constraint, NOT a wildcard. The distinction is load-bearing for profile authors: an entry of {Path: ..., Args: []} is a constraint, not a free pass. No behavioural change. Tests pass unchanged. --- .../cel/libraries/applicationprofile/ap.go | 22 ++++++++++++++ .../cel/libraries/applicationprofile/exec.go | 30 +++++++++++++++++-- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/pkg/rulemanager/cel/libraries/applicationprofile/ap.go b/pkg/rulemanager/cel/libraries/applicationprofile/ap.go index ce86d7ab8..99ab11831 100644 --- a/pkg/rulemanager/cel/libraries/applicationprofile/ap.go +++ b/pkg/rulemanager/cel/libraries/applicationprofile/ap.go @@ -130,6 +130,28 @@ func (l *apLibrary) Declarations() map[string][]cel.FunctionOpt { }), ), }, + // ap.was_path_opened_with_suffix and ap.was_path_opened_with_prefix + // — rule-author contract (CodeRabbit upstream PR #807 finding #7): + // + // These helpers answer "did any RECORDED concrete path open match + // this suffix/prefix?". When the profile-projection cache is in + // pass-through mode (no rule declared an Opens-projection slice, + // so cp.Opens.All == true), wildcard patterns in cp.Opens.Patterns + // are NOT scanned via string-level HasSuffix/HasPrefix because the + // pattern text contains '*' / '⋯' tokens whose string shape doesn't + // safely answer suffix/prefix questions (see open.go comment). + // Concrete-only Values are scanned. + // + // False-negative gap: if a profile entry is `/var/log/pods/*/foo.log`, + // the runtime path `/var/log/pods/web-7d6f/volumes/foo.log` actually + // matches this pattern, but `was_path_opened_with_suffix("/foo.log")` + // returns FALSE because the pattern text doesn't end in `/foo.log` + // literally. Rule authors who need wildcard-aware coverage should + // either: (a) declare an Opens projection slice in the rule's + // ProfileDataRequired (then SuffixHits/PrefixHits become authoritative + // and the projector pre-computes the hit map for wildcard entries), + // or (b) use ap.was_path_opened(path) which DOES run dynamic-segment + // matching over Patterns via CompareDynamic. "ap.was_path_opened_with_suffix": { cel.Overload( "ap_was_path_opened_with_suffix", []*cel.Type{cel.StringType, cel.StringType}, cel.BoolType, diff --git a/pkg/rulemanager/cel/libraries/applicationprofile/exec.go b/pkg/rulemanager/cel/libraries/applicationprofile/exec.go index 5f5736922..10e8d4c97 100644 --- a/pkg/rulemanager/cel/libraries/applicationprofile/exec.go +++ b/pkg/rulemanager/cel/libraries/applicationprofile/exec.go @@ -92,14 +92,40 @@ func (l *apLibrary) wasExecutedWithArgs(containerID, path, args ref.Val) ref.Val // Exact path match: walk the profile's Args for that path via // CompareExecArgs (handles ⋯ single-arg and * zero-or-more tokens). + // + // ExecsByPath absent-vs-empty asymmetry — CodeRabbit upstream PR + // #807 finding #8. Three states to distinguish: + // + // 1. Path absent from cp.Execs.Values: + // Profile doesn't allow this exec at all → fall through to + // the pattern-match loop, then to false. + // + // 2. Path in Values, ABSENT from ExecsByPath (map lookup ok=false): + // Legacy / pre-args-projection profiles. Treated as + // "no argv constraint" — back-compat MATCH any args. + // This is the intentional fallback for profiles compiled + // against older storage versions that didn't populate the + // composite ExecsByPath surface. + // + // 3. Path in Values, PRESENT in ExecsByPath with an EMPTY arg + // list ([]): + // Profile explicitly captured "this path ran with no args". + // CompareExecArgs matches only when runtimeArgs is also + // empty. NOT a back-compat fallback — a deliberately tight + // constraint authored by the profile producer. + // + // The distinction matters for rule-author intuition: producing a + // signed profile that lists `{Path: /usr/bin/foo, Args: []}` is a + // CONSTRAINT, not a wildcard. Authors who want "any args" must + // omit the ExecsByPath entry (rare) or use an explicit `*` + // wildcard token in Args. if _, ok := cp.Execs.Values[pathStr]; ok { if profileArgs, ok := cp.ExecsByPath[pathStr]; ok { if dynamicpathdetector.CompareExecArgs(profileArgs, runtimeArgs) { return types.Bool(true) } } else { - // No ExecsByPath entry for this path — back-compat: treat as - // "no argv constraint", match. + // State 2: ExecsByPath absent → back-compat "no argv constraint". return types.Bool(true) } } From b4d4e895ee789837563fc185c71239fe0384d3ec Mon Sep 17 00:00:00 2001 From: Entlein Date: Sat, 16 May 2026 12:30:50 +0200 Subject: [PATCH 3/3] deps: pin stereoscope v0.1.9 + runtime-spec v1.2.1 (compat with kubescape/syft fork) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Storage rc1 bumped to syft v1.42.4 (CVE-2026-33481), which transitively requires stereoscope v0.1.22 + runtime-spec v1.3.0. Those versions use the new moby/moby/client submodule API, which is incompatible with inspektor-gadget's moby/moby umbrella requirement on the node-agent side (ambiguous-import wall — see issue #45). Node-agent stays on kubescape/syft v1.32.0-ks.2 via the existing replace, but transitive resolution from storage's go.mod pulls the newer stereoscope into the build, breaking the build with: undefined: client.New undefined: client.PingOptions Adds two replace directives to force the older transitive chain that matches kubescape/syft v1.32.0-ks.2's expectations: github.com/anchore/stereoscope => v0.1.9-0.20250826202322-... github.com/opencontainers/runtime-spec => v1.2.1 This is the minimum set needed for node-agent to build cleanly against storage rc1 (with syft v1.42.4) while still using kubescape/syft on its own side. Storage's CVE fix remains in effect at the storage binary; node-agent's syft surface is unchanged. Verified locally: go build ./... ok go test ./pkg/objectcache/... ./pkg/rulemanager/... -count=1 → 30+ packages ok --- go.mod | 4 ++++ go.sum | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 014958883..b721ec513 100644 --- a/go.mod +++ b/go.mod @@ -511,3 +511,7 @@ replace github.com/cilium/ebpf => github.com/matthyx/ebpf v0.0.0-20260421101317- replace github.com/anchore/syft => github.com/kubescape/syft v1.32.0-ks.2 replace github.com/kubescape/storage => github.com/k8sstormcenter/storage v0.0.240-0.20260513133617-b23d85f00f6a + +replace github.com/anchore/stereoscope => github.com/anchore/stereoscope v0.1.9-0.20250826202322-ef061ea78385 + +replace github.com/opencontainers/runtime-spec => github.com/opencontainers/runtime-spec v1.2.1 diff --git a/go.sum b/go.sum index b5cafe303..bb4a12f3f 100644 --- a/go.sum +++ b/go.sum @@ -198,8 +198,8 @@ github.com/anchore/go-version v1.2.2-0.20210903204242-51efa5b487c4 h1:rmZG77uXgE github.com/anchore/go-version v1.2.2-0.20210903204242-51efa5b487c4/go.mod h1:Bkc+JYWjMCF8OyZ340IMSIi2Ebf3uwByOk6ho4wne1E= github.com/anchore/packageurl-go v0.1.1-0.20250220190351-d62adb6e1115 h1:ZyRCmiEjnoGJZ1+Ah0ZZ/mKKqNhGcUZBl0s7PTTDzvY= github.com/anchore/packageurl-go v0.1.1-0.20250220190351-d62adb6e1115/go.mod h1:KoYIv7tdP5+CC9VGkeZV4/vGCKsY55VvoG+5dadg4YI= -github.com/anchore/stereoscope v0.1.9 h1:Nhvk8g6PRx9ubaJU4asAhD3fGcY5HKXZCDGkxI2e0sI= -github.com/anchore/stereoscope v0.1.9/go.mod h1:YkrCtDgz7A+w6Ggd0yxU9q58CerqQFwYARS+F2RvLQQ= +github.com/anchore/stereoscope v0.1.9-0.20250826202322-ef061ea78385 h1:icCqbvAKGZXf29lEi8JmwvHVCBCYkiyZMuSnk+5ajYo= +github.com/anchore/stereoscope v0.1.9-0.20250826202322-ef061ea78385/go.mod h1:0UCjLz5MdPNiH9F0h2tSNf3yGF6/MnK8ZCPo0YfDQVc= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8= github.com/andybalholm/brotli v1.2.0 h1:ukwgCxwYrmACq68yiUqwIWnGY0cTPox/M94sVwToPjQ= github.com/andybalholm/brotli v1.2.0/go.mod h1:rzTDkvFWvIrjDXZHkuS16NPggd91W3kUSvPlQ1pLaKY=