release/sbob-rc1: TDD fixes for upstream NA rabbit findings (#807, #808)#44
release/sbob-rc1: TDD fixes for upstream NA rabbit findings (#807, #808)#44entlein wants to merge 3 commits into
Conversation
…verlay CodeRabbit upstream PR kubescape#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.
…prefix + exec-args Two doc-only fixes for CodeRabbit cross-PR advisory: #7 (NA kubescape#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 kubescape#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.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Status update on syft CVE bump: Attempted to bump syft v1.32.0 → v1.42.4 (CVE-2026-33481 fix) and remove the kubescape/syft replace. Storage went through cleanly (k8sstormcenter/storage#33 commit bf7ce36c). NA is blocked by a transitive Go modules ambiguity: stereoscope v0.1.22 (pulled by syft v1.42.x) uses moby/moby/client v0.4.1; inspektor-gadget pulls the umbrella moby/moby for pkg/stringid. Go cannot disambiguate — replace / exclude / explicit require all fail. Filed k8sstormcenter/node-agent#45 tracking the blocker with three fix paths. NA stays on syft v1.32.0 via existing kubescape/syft replace until the upstream dep graph resolves (most likely path: inspektor-gadget migrating off the moby/moby umbrella). Exposure assessment: NA imports syft only for ApplicationProfile schema types (license/pkg/sbom/source/file) — it never invokes the vulnerable cleanup path. CVE practical risk is essentially zero on this binary. The declaration is still wrong-version which would surface in any SBOM scan of the NA binary; that's the cosmetic / process-hygiene concern. @coderabbitai review |
|
CVE-2026-33481 exposure assessment — confirmed sound. CVE-2026-33481 is a resource-exhaustion DoS where Syft fails to clean up temporary storage after scanning archives — it unpacks archives into temp storage and exits without cleanup when that storage is exhausted. The CVSS vector is The SBOM declaration concern is real but purely a cosmetic/compliance issue — the binary isn't affected at runtime.
The split between Proceeding with the review of the current PR changes. ✅ Actions performedReview triggered.
|
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup Effectiveness (AFTER only)
Event Counters
|
…cape/syft fork) 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
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup Effectiveness (AFTER only)
Event Counters
|
1 participant
Iterative branch addressing CodeRabbit findings across the atomic upstream NA PRs (kubescape#805, kubescape#806, kubescape#807, kubescape#808, kubescape#810). Each fix lands TDD: failing regression test → minimal code change → full local test suite.
Findings tracked
containerprofilecache.go:414— honor verify result before merging user overlay (commit e58a068). Strict-mode (EnableSignatureVerification=true) tampered overlays now nilled out before merge; permissive mode preserved.exec.go— document ExecsByPath absent-vs-empty asymmetry (commit f310091)open.go— document was_path_opened_with_suffix/_prefix wildcard-pattern false-negative gap (commit f310091)CodeRabbit self-trigger pattern
Per
feedback_coderabbit_always_trigger.md:@coderabbitai reviewafter each push so the rabbit has a chance to flag anything missed by the targeted tests.@coderabbitai review