SBOB wildcards: DNS + IP + path exec wildcards, sign/tamper #326
Open
entlein wants to merge 3 commits into
Open
SBOB wildcards: DNS + IP + path exec wildcards, sign/tamper #326entlein wants to merge 3 commits into
entlein wants to merge 3 commits into
Conversation
….7%/-12.4% memory) Signed-off-by: entlein <einentlein@gmail.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR introduces a ChangesCollapse Configuration and Dynamic Path Analysis
🎯 4 (Complex) | ⏱️ ~60 minutes Sequence DiagramssequenceDiagram
participant Admin as Administrator
participant ApiServer as API Server
participant Strategy as CollapseConfigurationStrategy
participant Store as etcd Store
Admin->>ApiServer: POST CollapseConfiguration
ApiServer->>Strategy: Validate
Strategy->>Strategy: Check thresholds >= 0
Strategy->>Strategy: Validate prefixes (non-empty, /*, no duplicates)
Strategy-->>ApiServer: Field errors or OK
ApiServer->>Store: Store or Reject
Store-->>ApiServer: Acknowledged
ApiServer-->>Admin: Created or Error
sequenceDiagram
participant Processor as ApplicationProfileProcessor
participant Settings as CollapseSettings
participant Analyzer as PathAnalyzer
participant Consolidate as consolidateOpens
Processor->>Settings: GetCollapseSettings()
Settings-->>Processor: Thresholds + CollapseConfigs
Processor->>Analyzer: NewPathAnalyzerWithConfigs(threshold, configs)
Processor->>Analyzer: AnalyzePath for opens
Analyzer->>Analyzer: processSegments with per-prefix thresholds
Analyzer-->>Processor: Dynamic paths + literal paths
Processor->>Consolidate: consolidateOpens(results, sbomSet)
Consolidate->>Consolidate: Check each literal against patterns
Consolidate-->>Processor: Filtered + merged results
Processor-->>Processor: PreSave completes deflation
Suggested Labels
Suggested Reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Author
|
@CodeRabbit review this one together with kubescape/node-agent#810 |
🧩 Analysis chain
🏁 Script executed: Length of output: 9559
Key Areas Introduced
I'll now run a thorough review across all changed files. Given the size and scope, I'll pay close attention to:
✅ Actions performedFull review triggered. |
🧩 Analysis chainLet me fetch both PRs so I can review them together with full context. 🏁 Script executed: Length of output: 20562
Cross-PR Context:
|
| Aspect | Detail |
|---|---|
| New CRD | CollapseConfiguration — full lifecycle (types, registry, protobuf, openapi, informers, listers, clientset) |
| Wildcard matching | DNS (leading *., mid ..., trailing *), IP/CIDR, exec-arg comparison |
| Signing/tamper | Cosign-based sign+verify for ApplicationProfile / NetworkNeighborhood in node-agent |
| Memory impact | -11.7% avg / -12.4% peak (from node-agent benchmarks) |
| Test coverage | Extensive unit + component tests across both repos |
Triggering the full review now with this cross-PR context in mind.
✅ Actions performed
Full review triggered.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/registry/file/dynamicpathdetector/tests/analyze_endpoints_test.go (1)
169-178:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a length assertion before indexed comparisons.
On Line 174, the test iterates over
resultand indexestt.expected[i]without first asserting equal lengths. Ifresultis shorter thanexpected, missing endpoints won’t be detected.Proposed fix
t.Run(tt.name, func(t *testing.T) { analyzer := dynamicpathdetector.NewPathAnalyzerWithConfigs(dynamicpathdetector.EndpointDynamicThreshold, nil) result := dynamicpathdetector.AnalyzeEndpoints(&tt.input, analyzer) + assert.Len(t, result, len(tt.expected)) ja := jsonassert.New(t) for i := range result { assert.Equal(t, tt.expected[i].Endpoint, result[i].Endpoint) assert.Equal(t, tt.expected[i].Methods, result[i].Methods) ja.Assert(string(result[i].Headers), string(tt.expected[i].Headers)) } })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/registry/file/dynamicpathdetector/tests/analyze_endpoints_test.go` around lines 169 - 178, The test iterates result and indexes tt.expected without checking lengths; before the for-loop that ranges over result in the test (the block that calls dynamicpathdetector.AnalyzeEndpoints and assigns result), add an assertion that len(result) == len(tt.expected) to ensure mismatched lengths fail the test; reference the test variable names result and tt.expected and the call to dynamicpathdetector.AnalyzeEndpoints (and the analyzer created by NewPathAnalyzerWithConfigs) so you place the length check immediately after result is computed and before the indexed comparisons.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go.mod`:
- Line 9: The diff downgraded critical modules—github.com/anchore/syft,
github.com/anchore/stereoscope, and
github.com/opencontainers/runtime-spec—reintroducing known CVE/behavior
regressions; revert these changes by restoring or pinning safe versions (e.g.,
github.com/anchore/syft at >= v1.42.3 to include CVE-2026-33481 fixes,
github.com/anchore/stereoscope to a version that patches the tar path traversal
vulnerability, and github.com/opencontainers/runtime-spec to v1.3.0+ to retain
spec features), then run go mod tidy and run the test suite/CI to ensure
compatibility; add a short PR note explaining why these exact versions are
required (security fixes and spec compatibility) so the downgrade rationale is
documented.
In `@pkg/apis/softwarecomposition/v1beta1/generated.proto`:
- Around line 134-177: The generated proto is out of sync with the Go struct:
add the missing ipAddresses field (field number 9) to the NetworkNeighbor
message in the protobuf definition so it matches the Go struct's IPAddresses
[]string protobuf tag (bytes,9,rep,name=ipAddresses), then re-run the protobuf
generation to update generated types (or regenerate the file that produced
CollapseConfiguration additions) so marshal/unmarshal uses the same schema;
ensure the NetworkNeighbor message contains "repeated string ipAddresses = 9;"
(or equivalent) and that the regenerated Go/JSON tags align with the existing
NetworkNeighbor.IPAddresses field.
In `@pkg/registry/file/applicationprofile_processor.go`:
- Line 92: The call to a.collapseSettings() inside ApplicationProfileProcessor
can panic when a is zero-valued; before using it in
deflateApplicationProfileContainer, obtain a local settings variable (e.g., cs
:= a.collapseSettings()) and if cs == nil set cs =
dynamicpathdetector.DefaultCollapseSettings, then pass cs to
deflateApplicationProfileContainer instead of calling a.collapseSettings()
inline; update the call site in the container loop (where
deflateApplicationProfileContainer is invoked) to use this guarded local
variable.
In `@pkg/registry/file/dynamicpathdetector/analyzer.go`:
- Around line 397-433: The recursive wildcard backtracking in compareSegments
can explode exponentially; add memoization keyed by the pair of positions in the
two slices to cache boolean results and short-circuit repeated subcalls.
Implement either a small wrapper (e.g., compareSegmentsMemo or change
compareSegments signature to accept indices di, ri plus a memo map) where the
memo key is a pair (di, ri) or a compact int64/struct, check memo at entry,
compute as now, store the true/false result in memo before returning, and use
that memo for the loop that tries offsets when dynamic[0] == WildcardIdentifier;
keep existing semantics for WildcardIdentifier and DynamicIdentifier unchanged.
In `@pkg/registry/file/dynamicpathdetector/tests/compare_exec_args_test.go`:
- Around line 284-313: The timing assertion in
TestCompareExecArgs_ReDoSResistance is too brittle; update the test to avoid CI
flakes by (1) skipping the timing check when testing.Short() is true and (2)
raising the wall‑clock cap from 100ms to a more generous value (e.g. 500ms) for
the remaining runs. Modify the TestCompareExecArgs_ReDoSResistance function to
wrap the elapsed > threshold check with a conditional that first returns if
testing.Short() and otherwise uses the larger threshold when asserting
performance of dynamicpathdetector.CompareExecArgs.
---
Outside diff comments:
In `@pkg/registry/file/dynamicpathdetector/tests/analyze_endpoints_test.go`:
- Around line 169-178: The test iterates result and indexes tt.expected without
checking lengths; before the for-loop that ranges over result in the test (the
block that calls dynamicpathdetector.AnalyzeEndpoints and assigns result), add
an assertion that len(result) == len(tt.expected) to ensure mismatched lengths
fail the test; reference the test variable names result and tt.expected and the
call to dynamicpathdetector.AnalyzeEndpoints (and the analyzer created by
NewPathAnalyzerWithConfigs) so you place the length check immediately after
result is computed and before the indexed comparisons.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e12e4fd2-1b66-4ed9-b497-e7bf4ca9c574
⛔ Files ignored due to path filters (18)
go.sumis excluded by!**/*.sumpkg/apis/softwarecomposition/v1beta1/generated.pb.gois excluded by!**/*.pb.gopkg/apis/softwarecomposition/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.gopkg/generated/applyconfiguration/softwarecomposition/v1beta1/collapseconfigentry.gois excluded by!**/generated/**pkg/generated/applyconfiguration/softwarecomposition/v1beta1/collapseconfiguration.gois excluded by!**/generated/**pkg/generated/applyconfiguration/softwarecomposition/v1beta1/collapseconfigurationspec.gois excluded by!**/generated/**pkg/generated/applyconfiguration/utils.gois excluded by!**/generated/**pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/collapseconfiguration.gois excluded by!**/generated/**pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/fake/fake_collapseconfiguration.gois excluded by!**/generated/**pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/fake/fake_softwarecomposition_client.gois excluded by!**/generated/**pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/generated_expansion.gois excluded by!**/generated/**pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/softwarecomposition_client.gois excluded by!**/generated/**pkg/generated/informers/externalversions/generic.gois excluded by!**/generated/**pkg/generated/informers/externalversions/softwarecomposition/v1beta1/collapseconfiguration.gois excluded by!**/generated/**pkg/generated/informers/externalversions/softwarecomposition/v1beta1/interface.gois excluded by!**/generated/**pkg/generated/listers/softwarecomposition/v1beta1/collapseconfiguration.gois excluded by!**/generated/**pkg/generated/listers/softwarecomposition/v1beta1/expansion_generated.gois excluded by!**/generated/**pkg/generated/openapi/zz_generated.openapi.gois excluded by!**/generated/**
📒 Files selected for processing (52)
.gitignoreartifacts/collapseconfiguration-default-sample.yamlgo.modpkg/apis/softwarecomposition/collapse_types.gopkg/apis/softwarecomposition/network_types.gopkg/apis/softwarecomposition/register.gopkg/apis/softwarecomposition/v1beta1/collapse_types.gopkg/apis/softwarecomposition/v1beta1/generated.protopkg/apis/softwarecomposition/v1beta1/network_types.gopkg/apis/softwarecomposition/v1beta1/network_types_protobuf_test.gopkg/apis/softwarecomposition/v1beta1/register.gopkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.gopkg/apis/softwarecomposition/v1beta1/zz_generated.deepcopy.gopkg/apis/softwarecomposition/v1beta1/zz_generated.model_name.gopkg/apis/softwarecomposition/zz_generated.deepcopy.gopkg/apiserver/apiserver.gopkg/registry/file/applicationprofile_processor.gopkg/registry/file/applicationprofile_processor_collapse_provider_test.gopkg/registry/file/applicationprofile_processor_test.gopkg/registry/file/cleanup.gopkg/registry/file/cleanup_test.gopkg/registry/file/containerprofile_processor.gopkg/registry/file/containerprofile_processor_collapse_provider_test.gopkg/registry/file/dynamicpathdetector/analyze_endpoints.gopkg/registry/file/dynamicpathdetector/analyze_endpoints_internal_test.gopkg/registry/file/dynamicpathdetector/analyze_opens.gopkg/registry/file/dynamicpathdetector/analyzer.gopkg/registry/file/dynamicpathdetector/collapse_config_from_crd.gopkg/registry/file/dynamicpathdetector/compare_exec_args.gopkg/registry/file/dynamicpathdetector/tests/analyze_endpoints_test.gopkg/registry/file/dynamicpathdetector/tests/analyze_opens_test.gopkg/registry/file/dynamicpathdetector/tests/benchmark_test.gopkg/registry/file/dynamicpathdetector/tests/collapse_config_crd_test.gopkg/registry/file/dynamicpathdetector/tests/compare_exec_args_test.gopkg/registry/file/dynamicpathdetector/tests/consolidate_opens_test.gopkg/registry/file/dynamicpathdetector/tests/coverage_test.gopkg/registry/file/dynamicpathdetector/tests/profile_test.gopkg/registry/file/dynamicpathdetector/types.gopkg/registry/file/networkmatch/README.mdpkg/registry/file/networkmatch/bench_test.gopkg/registry/file/networkmatch/doc.gopkg/registry/file/networkmatch/match_dns.gopkg/registry/file/networkmatch/match_dns_test.gopkg/registry/file/networkmatch/match_ip.gopkg/registry/file/networkmatch/match_ip_test.gopkg/registry/file/networkmatch/validate.gopkg/registry/file/networkmatch/validate_test.gopkg/registry/softwarecomposition/collapseconfiguration/etcd.gopkg/registry/softwarecomposition/collapseconfiguration/strategy.gopkg/registry/softwarecomposition/collapseconfiguration/strategy_test.gopkg/registry/softwarecomposition/networkneighborhood/strategy.gopkg/registry/softwarecomposition/networkneighborhood/strategy_test.go
4 tasks
entlein
pushed a commit
to k8sstormcenter/storage
that referenced
this pull request
May 16, 2026
… backtracking CodeRabbit upstream PR kubescape#326 finding #4 (Major). The recursive expansion at the mid-path `*` branch explored all suffix splits without caching, producing 2^n / n!-style runtime on patterns with multiple `*` segments. Adversarial inputs of 20 `*` + 20 regular segments time out the test runner at 60s on the un-memoised matcher. Approach: keep the existing recursive form (compareSegments) on the fast path for single-`*` and no-`*` patterns where backtracking can't re-enter the same (di, ri) state, and route multi-`*` patterns through a DP-memoised core (compareSegmentsMemo) that caches every (di, ri) outcome. Semantics are identical — the memoised core is a pure re-shape of the same recursion. The split is gated by multipleWildcards(), a single linear scan of the dynamic-segment slice. This keeps the common-case allocation profile at 2 allocs/op (splitPath only); the memo map is only allocated when it actually earns its keep. Local benchmark on this machine (arm64, -benchtime=2s): Shape Before After ---------------------------------- --------------- --------------- ellipsis_short 94.26 ns/op 90.08 ns/op ellipsis_deep 158.3 ns/op 171.1 ns/op trailing_star 86.21 ns/op 103.5 ns/op trailing_star_no_match_on_parent 73.02 ns/op 73.35 ns/op mid_star_zero_consumed 84.76 ns/op 82.26 ns/op mid_star_many_consumed 118.4 ns/op 117.5 ns/op anchored_root_no_match 55.66 ns/op 53.30 ns/op unanchored_star_root 49.30 ns/op 55.30 ns/op deep_literal_match 93.40 ns/op 98.41 ns/op deep_literal_mismatch 108.1 ns/op 96.43 ns/op All shapes: 2 allocs/op before and after (no heap growth). Adversarial multi-* tail-mismatch: 60s+ timeout → 0.2s wall (≥ 300× faster). Tests added in tests/compare_dynamic_memoise_test.go: - TestCompareDynamic_MemoiseGoldenAcceptance: 27 paired accept/reject cases pinning byte-for-byte semantic parity (empty inputs, literal exact, trailing `*`, mid `*`, ellipsis `⋯`, consecutive `*/*` zero-or-more semantics, busybox-style symlink shape, multi-wildcard, root edge). - TestCompareDynamic_MemoiseAdversarialReDoS: 4 multi-`*` tail-mismatch shapes with strict time budgets (50/100/200/200ms); each fails the test if the matcher takes longer than the budget, catching any future regression that re-introduces exponential backtracking. - TestCompareDynamic_MemoiseAdversarialPositive: positive-case parity — a matching adversarial input completes in <200ms. - TestCompareDynamic_MemoiseAllocCeiling: per-call allocs ≤ 12 across the common shapes (current observation: 2 allocs/op), catching unbounded heap growth in future memoisation tweaks. The adversarial tests skip under testing.Short(); the alloc-ceiling test also skips so quick local runs stay fast.
entlein
pushed a commit
to k8sstormcenter/storage
that referenced
this pull request
May 16, 2026
CodeRabbit upstream PR kubescape#326 outside-diff-range finding at analyze_endpoints_test.go:169-178. Ranging over result and indexing tt.expected[i] silently passes if result is shorter than expected, making missing endpoints invisible to the gate. Adds require.Len() before the for-loop so length divergence fails the test loudly.
entlein
pushed a commit
to k8sstormcenter/storage
that referenced
this pull request
May 16, 2026
CodeRabbit upstream PR kubescape#326 finding #5. The 100ms wall-clock assertion in TestCompareExecArgs_ReDoSResistance is sensitive to runner CPU contention and can fail flakily even when the memoised matcher is correct. Skipping under testing.Short() preserves the regression intent (the test still runs by default) but lets quick local / short-mode CI runs stay deterministic. Memoisation correctness is already covered by the explicit case-table tests above, which always run regardless of short mode.
entlein
pushed a commit
to k8sstormcenter/storage
that referenced
this pull request
May 16, 2026
…e accessor CodeRabbit upstream PR kubescape#326 finding #3 (applicationprofile_processor.go:92). A zero-valued processor — constructed via direct struct literal rather than the NewApplicationProfileProcessor factory — has a nil collapseSettings function field. The deflate path then nil-dereferences on the call inside PreSave. Adds `effectiveCollapseSettings()` as the safe accessor used by PreSave. It returns the provider's result when configured, or the compiled-in DefaultCollapseSettings when the field is nil. The direct field-call path (`a.collapseSettings()`) is intentionally left raw — callers who construct a zero-value processor and call the unsafe path explicitly are on the "I know what I'm doing" side. New regression test TestApplicationProfileProcessor_ZeroValue_NoPanicOnCollapseSettings pins both halves of the contract: - effectiveCollapseSettings() on zero-value processor does NOT panic and returns the compiled-in defaults - raw field-call on zero-value processor DOES still panic (no silent behaviour change)
entlein
pushed a commit
to k8sstormcenter/storage
that referenced
this pull request
May 16, 2026
CodeRabbit upstream PR kubescape#326 finding #2 (Major). The Go struct's protobuf tag is `bytes,9,rep,name=ipAddresses` but generated.proto did not declare field 9 on the NetworkNeighbor message. Runtime behaviour was unaffected because generated.pb.go is the actual codec and already encodes field 9 correctly (verified: wire tag 0x4a = field 9, wire type 2). But the .proto file is the source of truth for: - Any future regeneration (`make generate`) — drops the field if the .proto doesn't declare it - Cross-language consumers compiling against the .proto schema - Documentation accuracy Restores the declaration. New regression test TestNetworkNeighbor_ProtoFile_DeclaresIPAddresses parses generated.proto and asserts the NetworkNeighbor message body contains the field-9 declaration. If the .proto and the Go tag ever diverge again, the test fails loudly at unit-test time rather than at the next regen. The existing IPAddresses protobuf roundtrip tests are unaffected and continue to pass.
6 tasks
entlein
pushed a commit
to k8sstormcenter/storage
that referenced
this pull request
May 16, 2026
…time-spec v1.3.0 CodeRabbit upstream PR kubescape#326 finding #1 (Critical). Lifts syft from v1.32.0 to v1.42.4 — the lowest version that includes the CVE-2026-33481 fix (Syft was failing to clean up temporary storage when a scan errored mid-flight). v1.42.4 is the latest patch on the v1.42 line, so the upgrade gets the CVE fix without taking the v1.43/v1.44 minor bumps' larger surface-area changes. go mod tidy pulled forward two transitive bumps: - stereoscope v0.1.9 → v0.1.22 (Anchore's syft-companion lib, must move in lockstep with syft) - runtime-spec v1.2.1 → v1.3.0 (transitive via stereoscope) - modernc.org/libc v1.67.6 → v1.70.0 (transitive) - modernc.org/sqlite v1.46.1 → v1.46.2 (transitive) go.mod no longer carries a replace directive against kubescape/syft — our fork doesn't have v1.42.3+ tags and the upstream content is required for the CVE fix. Direct anchore/syft dependency is now the source of truth. Local validation: go build ./... ok go test ./pkg/... ok in 1.4s (all 16 packages green) Vendor directory is .gitignored; `go mod vendor` was rerun locally to keep build/manifest in sync.
Signed-off-by: entlein <einentlein@gmail.com>
This was referenced May 16, 2026
Author
This was referenced May 16, 2026
Open
🧩 Analysis chain🏁 Script executed: Length of output: 5716
✅ Confirmed Atomic
|
🧩 Analysis chain
🏁 Script executed: Unrecoverable error: Failed to clone repository 🏁 Script executed: Unrecoverable error: Failed to clone repository 🏁 Script executed: Unrecoverable error: Stream setup permanently failed: 14 UNAVAILABLE: Connection dropped
✅ Confirmed Atomic / Well-Scoped
|
Author
|
the syft downgrade (etc) was now addressed in storage , we should be perfectly aligned with upstream, please recheck |
entlein
added a commit
to k8sstormcenter/storage
that referenced
this pull request
May 16, 2026
…ings (#33) * ci(manual-integration-tests): disable workflow until timing-fragile tests are fixed Adds a fail-fast guard step at the start of the integration-tests job that exits with a clear error pointing to issue #32. The workflow stays visible in the UI but anyone who dispatches it gets an immediate failure rather than burning ~30 minutes of CI on a known-flaky run. Last 5 fork runs (between 2026-05-15 16:32Z and 17:59Z on a15fb5a) each failed on a different *ProfileCreate subtest at the AP-completion polling stage. Upstream main MIT also red across its last 5 runs. The failover-tests jobs are consistently green and remain enabled by default (they run in their own job, untouched by this change). Re-enable when the wall-clock learning-period assumptions are replaced with dynamic AP-status polling and 5 consecutive green runs land. * fix(dynamicpathdetector): DP-memoise compareSegments to bound multi-* backtracking CodeRabbit upstream PR kubescape#326 finding #4 (Major). The recursive expansion at the mid-path `*` branch explored all suffix splits without caching, producing 2^n / n!-style runtime on patterns with multiple `*` segments. Adversarial inputs of 20 `*` + 20 regular segments time out the test runner at 60s on the un-memoised matcher. Approach: keep the existing recursive form (compareSegments) on the fast path for single-`*` and no-`*` patterns where backtracking can't re-enter the same (di, ri) state, and route multi-`*` patterns through a DP-memoised core (compareSegmentsMemo) that caches every (di, ri) outcome. Semantics are identical — the memoised core is a pure re-shape of the same recursion. The split is gated by multipleWildcards(), a single linear scan of the dynamic-segment slice. This keeps the common-case allocation profile at 2 allocs/op (splitPath only); the memo map is only allocated when it actually earns its keep. Local benchmark on this machine (arm64, -benchtime=2s): Shape Before After ---------------------------------- --------------- --------------- ellipsis_short 94.26 ns/op 90.08 ns/op ellipsis_deep 158.3 ns/op 171.1 ns/op trailing_star 86.21 ns/op 103.5 ns/op trailing_star_no_match_on_parent 73.02 ns/op 73.35 ns/op mid_star_zero_consumed 84.76 ns/op 82.26 ns/op mid_star_many_consumed 118.4 ns/op 117.5 ns/op anchored_root_no_match 55.66 ns/op 53.30 ns/op unanchored_star_root 49.30 ns/op 55.30 ns/op deep_literal_match 93.40 ns/op 98.41 ns/op deep_literal_mismatch 108.1 ns/op 96.43 ns/op All shapes: 2 allocs/op before and after (no heap growth). Adversarial multi-* tail-mismatch: 60s+ timeout → 0.2s wall (≥ 300× faster). Tests added in tests/compare_dynamic_memoise_test.go: - TestCompareDynamic_MemoiseGoldenAcceptance: 27 paired accept/reject cases pinning byte-for-byte semantic parity (empty inputs, literal exact, trailing `*`, mid `*`, ellipsis `⋯`, consecutive `*/*` zero-or-more semantics, busybox-style symlink shape, multi-wildcard, root edge). - TestCompareDynamic_MemoiseAdversarialReDoS: 4 multi-`*` tail-mismatch shapes with strict time budgets (50/100/200/200ms); each fails the test if the matcher takes longer than the budget, catching any future regression that re-introduces exponential backtracking. - TestCompareDynamic_MemoiseAdversarialPositive: positive-case parity — a matching adversarial input completes in <200ms. - TestCompareDynamic_MemoiseAllocCeiling: per-call allocs ≤ 12 across the common shapes (current observation: 2 allocs/op), catching unbounded heap growth in future memoisation tweaks. The adversarial tests skip under testing.Short(); the alloc-ceiling test also skips so quick local runs stay fast. * test(analyze_endpoints): require equal length before indexed comparisons CodeRabbit upstream PR kubescape#326 outside-diff-range finding at analyze_endpoints_test.go:169-178. Ranging over result and indexing tt.expected[i] silently passes if result is shorter than expected, making missing endpoints invisible to the gate. Adds require.Len() before the for-loop so length divergence fails the test loudly. * test(compare_exec_args): skip ReDoS timing test under testing.Short CodeRabbit upstream PR kubescape#326 finding #5. The 100ms wall-clock assertion in TestCompareExecArgs_ReDoSResistance is sensitive to runner CPU contention and can fail flakily even when the memoised matcher is correct. Skipping under testing.Short() preserves the regression intent (the test still runs by default) but lets quick local / short-mode CI runs stay deterministic. Memoisation correctness is already covered by the explicit case-table tests above, which always run regardless of short mode. * fix(applicationprofile_processor): nil-guard collapseSettings via safe accessor CodeRabbit upstream PR kubescape#326 finding #3 (applicationprofile_processor.go:92). A zero-valued processor — constructed via direct struct literal rather than the NewApplicationProfileProcessor factory — has a nil collapseSettings function field. The deflate path then nil-dereferences on the call inside PreSave. Adds `effectiveCollapseSettings()` as the safe accessor used by PreSave. It returns the provider's result when configured, or the compiled-in DefaultCollapseSettings when the field is nil. The direct field-call path (`a.collapseSettings()`) is intentionally left raw — callers who construct a zero-value processor and call the unsafe path explicitly are on the "I know what I'm doing" side. New regression test TestApplicationProfileProcessor_ZeroValue_NoPanicOnCollapseSettings pins both halves of the contract: - effectiveCollapseSettings() on zero-value processor does NOT panic and returns the compiled-in defaults - raw field-call on zero-value processor DOES still panic (no silent behaviour change) * fix(proto): declare repeated string ipAddresses = 9 on NetworkNeighbor CodeRabbit upstream PR kubescape#326 finding #2 (Major). The Go struct's protobuf tag is `bytes,9,rep,name=ipAddresses` but generated.proto did not declare field 9 on the NetworkNeighbor message. Runtime behaviour was unaffected because generated.pb.go is the actual codec and already encodes field 9 correctly (verified: wire tag 0x4a = field 9, wire type 2). But the .proto file is the source of truth for: - Any future regeneration (`make generate`) — drops the field if the .proto doesn't declare it - Cross-language consumers compiling against the .proto schema - Documentation accuracy Restores the declaration. New regression test TestNetworkNeighbor_ProtoFile_DeclaresIPAddresses parses generated.proto and asserts the NetworkNeighbor message body contains the field-9 declaration. If the .proto and the Go tag ever diverge again, the test fails loudly at unit-test time rather than at the next regen. The existing IPAddresses protobuf roundtrip tests are unaffected and continue to pass. * fix(dynamicpathdetector): DefaultCollapseConfig accessor (defensive copy) CodeRabbit upstream PR kubescape#323 finding #3. The package-level `DefaultCollapseConfig` was an exported mutable var: any caller doing `dynamicpathdetector.DefaultCollapseConfig.Threshold = 1` would silently corrupt every analyzer constructed thereafter. The threshold surface is security-sensitive (governs how aggressively profile paths collapse into wildcards), so accidental cross-caller leakage is a real risk. Replaces the exported var with an unexported `defaultCollapseConfig` and an accessor `DefaultCollapseConfig()` that returns a value copy. Pattern mirrors the existing `defaultCollapseConfigs` / `DefaultCollapseConfigs()` defensive-copy accessor for the slice form. Updates 6 callsites across the test suite to use the accessor. Adds TestDefaultCollapseConfig_DefensiveCopy pinning the contract: a caller mutating the returned struct does not affect subsequent reads. * docs(dynamicpathdetector): comment + guard kubescape#323 nitpicks (#5, #6, #9, #10) Four small docstring / defensive-guard improvements from CodeRabbit upstream PR kubescape#323: #5 wildcardKey / getEndpointKey format coupling — extract buildEndpointKey as the single source of truth for the lookup key shape. Both call-sites now route through it, removing the "must stay in sync" risk that allowed past drift. New test TestBuildEndpointKey_SharedFormat pins end-to-end behaviour via MergeDuplicateEndpoints: a wildcard-port entry and a specific- port sibling collapse into one row with merged Methods. #6 consolidateOpens O(n²) — acknowledged in the function's docstring along with the threshold (≤ ~10k entries) above which a trie-based replacement would be worthwhile. Behaviour unchanged. #9 removeEndpoint in-place mutation — added a NOTE block in the docstring describing the backing-array shift, why the sole caller is safe today, and the swap-to-copy guidance for any future caller that stores intermediate slice references. #10 hasPrefixAtBoundary("") edge — added an explicit empty-prefix branch returning true. Without it the function fell through to pathPrefix[0] == '/', which is true for any absolute path — effectively treating "" as a root-matching prefix incidentally. The explicit branch makes the invariant load-bearing. New test TestHasPrefixAtBoundary_EmptyPrefix pins the surface via FindConfigForPath. All existing tests still pass. Local run on this arm64 machine: go test ./pkg/registry/file/... ok in 0.93s * deps: bump syft to v1.42.4 (CVE-2026-33481), stereoscope v0.1.22, runtime-spec v1.3.0 CodeRabbit upstream PR kubescape#326 finding #1 (Critical). Lifts syft from v1.32.0 to v1.42.4 — the lowest version that includes the CVE-2026-33481 fix (Syft was failing to clean up temporary storage when a scan errored mid-flight). v1.42.4 is the latest patch on the v1.42 line, so the upgrade gets the CVE fix without taking the v1.43/v1.44 minor bumps' larger surface-area changes. go mod tidy pulled forward two transitive bumps: - stereoscope v0.1.9 → v0.1.22 (Anchore's syft-companion lib, must move in lockstep with syft) - runtime-spec v1.2.1 → v1.3.0 (transitive via stereoscope) - modernc.org/libc v1.67.6 → v1.70.0 (transitive) - modernc.org/sqlite v1.46.1 → v1.46.2 (transitive) go.mod no longer carries a replace directive against kubescape/syft — our fork doesn't have v1.42.3+ tags and the upstream content is required for the CVE fix. Direct anchore/syft dependency is now the source of truth. Local validation: go build ./... ok go test ./pkg/... ok in 1.4s (all 16 packages green) Vendor directory is .gitignored; `go mod vendor` was rerun locally to keep build/manifest in sync. * fix: address 3 follow-up rabbit findings on PR #33 1. ci(manual-integration-tests): use job-level if: false instead of exit 1 — workflow shows as "skipped" not "failed red X" in the Actions UI when dispatched, removing the cosmetic noise while keeping the ~30-minute waste prevention. Issue #32 reference kept in the YAML comment above the conditional. 2. test(network_types_protobuf): also assert the Go struct's protobuf tag, not just the .proto-file text. The earlier test only checked one side of the schema/tag contract, so tag drift on the Go side would silently pass. Now both halves are pinned. 3. test(applicationprofile_processor): assert the FULL CollapseSettings struct (not just OpenDynamicThreshold). Without this, a regression that resets any field added to CollapseSettings in the future would silently pass the zero-value defensive-fallback test. All from CodeRabbit follow-up review on storage PR #33. Local validation: go test ./pkg/registry/file/ ./pkg/apis/softwarecomposition/v1beta1/ → both packages ok * docs(networkmatch): correct spec URL to billofbehavior.com --------- Co-authored-by: Entlein <eineintlein@gmail.com>
entlein
added a commit
to k8sstormcenter/storage
that referenced
this pull request
May 16, 2026
…eanup) (#31) * fix(cleanup): enhance pod deletion logic and add tests for standalone pods Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com> * feat(networkmatch): IP matcher with literal + CIDR + '*' sentinel Runtime counterpart to spec §5.7 (v0.0.2). Profile entries are matched disjunctively — any single entry hit means match. Each entry MAY be: - a literal IP (canonicalised by net.ParseIP, IPv4 ↔ ::ffff:v4 unified) - a CIDR (a.b.c.d/n, validated by net.ParseCIDR) - the '*' sentinel for any IP (sugar for 0.0.0.0/0 ∪ ::/0) Public API: CompileIP([]string) *IPMatcher // compile-once for hot paths (*IPMatcher).Match(string) bool // amortised match MatchIP([]string, string) bool // cold-path convenience The compiled-matcher pattern eliminates per-call allocs from net.ParseCIDR on the hot path — node-agent's nn.* CEL functions call this on every network event captured by the eBPF tracer. Tests cover: - literal equality (incl. IPv4/IPv6 canonicalisation, ::ffff: mapping) - CIDR membership (incl. /32 == literal, edge addresses) - '*' sentinel (and rejection of empty/garbage observations) - 0.0.0.0/0 + ::/0 RFC-aligned alternatives (do NOT cross address families) - malformed entry skipping (admission validates, runtime defends) - mixed-list disjunction Benchmarks (arm64, hot-path compiled form): Literal : 12.73 ns/op 0 allocs CIDR : 18.48 ns/op 0 allocs 10-entry miss : 59.10 ns/op 0 allocs All well under the README target of <200 ns/op for IP literal/CIDR. * feat(networkmatch): DNS matcher with leading/trailing star + mid ellipsis Runtime counterpart to spec §5.8 (v0.0.2). Profile entries are matched disjunctively against an observed FQDN. Each entry's labels are walked left-to-right with positional token semantics: literal label : byte-equality (case-insensitive) leading '*' : matches EXACTLY ONE label (RFC 4592) trailing '*' : matches ONE OR MORE labels (project extension) '⋯' anywhere : matches EXACTLY ONE label (DynamicLabel) '**' : reserved/recursive — admission rejects, runtime drops Trailing dot is normalised on both sides (entry MAY or MAY NOT have it). Empty inner labels (e.g. 'foo..bar') are treated as malformed and skipped. Public API: CompileDNS([]string) *DNSMatcher (*DNSMatcher).Match(string) bool MatchDNS([]string, string) bool // cold-path convenience Tests cover: - literal equality (case-insensitive, exact-label-count) - trailing-dot normalisation in all four combinations - RFC 4592 leading '*' (exactly one label) - mid '⋯' DynamicLabel (the kubernetes service-FQDN case) - trailing '*' extension (one or more, never zero) - disjunction across entry list - malformed input rejection ('**', empty labels, lone '*') - compiled-form reuse contract Benchmarks (arm64, hot-path compiled form): Literal : 47.68 ns/op 1 alloc (label-split) Leading wildcard : 53.20 ns/op 1 alloc Deep name (10 lbl): 144.9 ns/op 1 alloc Long mixed list : 103.3 ns/op 1 alloc All under the README target of <600 ns/op for DNS wildcards. * feat(api): NetworkNeighbor.IPAddresses field + admission validation Adds IPAddresses []string to the NetworkNeighbor schema (internal + v1beta1) as the v0.0.2 list-form replacement for the deprecated singular IPAddress field. Each entry MAY be a literal IP, a CIDR, or '*'. Hand-edited generated code (deepcopy, conversion, protobuf) because the codegen pipeline isn't run as part of build; verified by a new round-trip test that pins the protobuf wire contract for field number 9. Validation lives in two places: pkg/registry/file/networkmatch/validate.go - ValidateIPEntry : literal | CIDR | '*' - ValidateDNSEntry : literal | leading-* | trailing-* | mid-⋯ rejects '**', empty inner labels, mid-* without ⋯ pkg/registry/softwarecomposition/networkneighborhood/strategy.go REST Validate() walks every neighbor in containers/initContainers/ ephemeralContainers × egress/ingress and surfaces a field.Invalid error per malformed entry. Apiserver translates this into a 400 so the user gets immediate feedback at write time. Runtime matchers (MatchIP/MatchDNS) keep their tolerance of malformed entries — a misconfigured profile that slips past admission won't kill the detection path. New tests: - protobuf round-trip (field 9 survives Marshal → Unmarshal) - ValidateIPEntry / ValidateDNSEntry edge cases - REST strategy Validate rejects malformed entries with the right field paths * chore: gitignore .claude agent state directory * fix(networkmatch): address CodeRabbit review on PR #30 Five findings, all legit, all fixed: - ValidateUpdate now also runs validateNetworkProfileEntries, so malformed ipAddresses / dnsNames can't land via PUT after a clean POST. New TestValidateUpdate_NetworkProfileEntries pins this. - validateNeighborList now also validates the deprecated singular IPAddress field while it remains supported. Bypass via the old form is closed. - TestValidate_NetworkProfileEntries asserts each error's field path (multiset, order-insensitive) instead of just the count. Field-path contract is now pinned: if validation starts emitting errors on the wrong path, downstream tooling that surfaces these to users will catch it here first. New case for deprecated-singular path. - TestNetworkNeighbor_IPAddresses_EmptyOmitted now checks Marshal errors with t.Fatalf instead of swallowing them via _. - README links to spec-v0.0.2 (was v0.0.1) and DNS normalisation wording matches runtime: trailing dot stripped, labels lowercased (was incorrectly described as 'appended'). * fix(networkmatch): address CodeRabbit round 2 on PR #30 Three findings, two fixed, one skipped: - validateNetworkProfileEntries now iterates an ordered []struct of (groupName, items) pairs rather than a map. Go map iteration is non-deterministic and admission errors flow to clients via the apiserver — stable ordering keeps error messages reproducible. - TestValidateUpdate_NetworkProfileEntries asserts each error's field path (multiset, order-insensitive). Same pattern already applied to TestValidate_NetworkProfileEntries; this closes the parallel gap. Skipped: - README.md "canonicalisation" → "canonicalization" (Trivial nit). Project already uses British spelling consistently across other docs; changing one occurrence creates inconsistency without fixing anything. Tests pass on the new ordering — note the test uses an order-insensitive multiset comparison, so the deterministic-order fix is a separate concern from the test (the test would pass with either ordering, but the runtime is now stable for downstream consumers). * fix(networkmatch): validate deprecated DNS singular field (CR round 3) Closes a parity gap with IPAddress (which round-1 already validates). Now both deprecated singular fields receive the same admission grammar check while they remain supported. Test extended with a deprecated-DNS path case mirroring the existing IPAddress one. * style(networkmatch): use American English spelling consistently Reverses the round-2 skip on the CodeRabbit nitpick (PR #30, README line 17). Files added in this branch now use American spelling (canonicalize/normalize/behavior) matching the Go ecosystem convention that 'canonicalisation' originally diverged from. Files touched: - pkg/registry/file/networkmatch/README.md - pkg/registry/file/networkmatch/match_ip.go (comment) - pkg/registry/file/networkmatch/match_dns.go (comment) - pkg/registry/file/networkmatch/match_ip_test.go (comment) No behaviour change. * test(dynamicpathdetector): pin bare-name argv[0] contract for CompareExecArgs Storage-side recording stores execs as [resolved_path, ...argv] and returns to consumers as (Path=resolved, Args=full-argv-with-argv[0]). The matcher does strict positional compare with no special argv[0] normalisation. That means profile.Args[0] MUST equal runtime argv[0] as captured by eBPF — typically the BARE program name ("sh"), not the resolved exepath ("/bin/sh"). The realistic-patterns table earlier in this file uses 2-element vectors that skip argv[0] entirely, which doesn't pin the convention. Add TestCompareExecArgs_Argv0BareName covering all the Test_32 scenarios end-to-end against the matcher, plus the two profile-shape-mismatch cases (Args[0]=full-path vs bare argv[0], and vice versa) so future drift on either side trips the test. Pairs with the rewrite of node-agent's Test_32_UnexpectedProcessArguments that changes profile.Args from [/bin/sh, -c, *] to [sh, -c, *], so R0040 can be tested in isolation from R0001's path-resolution path. * release/sbob-rc1: TDD fixes for upstream PR kubescape#326 rabbit findings (#33) * ci(manual-integration-tests): disable workflow until timing-fragile tests are fixed Adds a fail-fast guard step at the start of the integration-tests job that exits with a clear error pointing to issue #32. The workflow stays visible in the UI but anyone who dispatches it gets an immediate failure rather than burning ~30 minutes of CI on a known-flaky run. Last 5 fork runs (between 2026-05-15 16:32Z and 17:59Z on a15fb5a) each failed on a different *ProfileCreate subtest at the AP-completion polling stage. Upstream main MIT also red across its last 5 runs. The failover-tests jobs are consistently green and remain enabled by default (they run in their own job, untouched by this change). Re-enable when the wall-clock learning-period assumptions are replaced with dynamic AP-status polling and 5 consecutive green runs land. * fix(dynamicpathdetector): DP-memoise compareSegments to bound multi-* backtracking CodeRabbit upstream PR kubescape#326 finding #4 (Major). The recursive expansion at the mid-path `*` branch explored all suffix splits without caching, producing 2^n / n!-style runtime on patterns with multiple `*` segments. Adversarial inputs of 20 `*` + 20 regular segments time out the test runner at 60s on the un-memoised matcher. Approach: keep the existing recursive form (compareSegments) on the fast path for single-`*` and no-`*` patterns where backtracking can't re-enter the same (di, ri) state, and route multi-`*` patterns through a DP-memoised core (compareSegmentsMemo) that caches every (di, ri) outcome. Semantics are identical — the memoised core is a pure re-shape of the same recursion. The split is gated by multipleWildcards(), a single linear scan of the dynamic-segment slice. This keeps the common-case allocation profile at 2 allocs/op (splitPath only); the memo map is only allocated when it actually earns its keep. Local benchmark on this machine (arm64, -benchtime=2s): Shape Before After ---------------------------------- --------------- --------------- ellipsis_short 94.26 ns/op 90.08 ns/op ellipsis_deep 158.3 ns/op 171.1 ns/op trailing_star 86.21 ns/op 103.5 ns/op trailing_star_no_match_on_parent 73.02 ns/op 73.35 ns/op mid_star_zero_consumed 84.76 ns/op 82.26 ns/op mid_star_many_consumed 118.4 ns/op 117.5 ns/op anchored_root_no_match 55.66 ns/op 53.30 ns/op unanchored_star_root 49.30 ns/op 55.30 ns/op deep_literal_match 93.40 ns/op 98.41 ns/op deep_literal_mismatch 108.1 ns/op 96.43 ns/op All shapes: 2 allocs/op before and after (no heap growth). Adversarial multi-* tail-mismatch: 60s+ timeout → 0.2s wall (≥ 300× faster). Tests added in tests/compare_dynamic_memoise_test.go: - TestCompareDynamic_MemoiseGoldenAcceptance: 27 paired accept/reject cases pinning byte-for-byte semantic parity (empty inputs, literal exact, trailing `*`, mid `*`, ellipsis `⋯`, consecutive `*/*` zero-or-more semantics, busybox-style symlink shape, multi-wildcard, root edge). - TestCompareDynamic_MemoiseAdversarialReDoS: 4 multi-`*` tail-mismatch shapes with strict time budgets (50/100/200/200ms); each fails the test if the matcher takes longer than the budget, catching any future regression that re-introduces exponential backtracking. - TestCompareDynamic_MemoiseAdversarialPositive: positive-case parity — a matching adversarial input completes in <200ms. - TestCompareDynamic_MemoiseAllocCeiling: per-call allocs ≤ 12 across the common shapes (current observation: 2 allocs/op), catching unbounded heap growth in future memoisation tweaks. The adversarial tests skip under testing.Short(); the alloc-ceiling test also skips so quick local runs stay fast. * test(analyze_endpoints): require equal length before indexed comparisons CodeRabbit upstream PR kubescape#326 outside-diff-range finding at analyze_endpoints_test.go:169-178. Ranging over result and indexing tt.expected[i] silently passes if result is shorter than expected, making missing endpoints invisible to the gate. Adds require.Len() before the for-loop so length divergence fails the test loudly. * test(compare_exec_args): skip ReDoS timing test under testing.Short CodeRabbit upstream PR kubescape#326 finding #5. The 100ms wall-clock assertion in TestCompareExecArgs_ReDoSResistance is sensitive to runner CPU contention and can fail flakily even when the memoised matcher is correct. Skipping under testing.Short() preserves the regression intent (the test still runs by default) but lets quick local / short-mode CI runs stay deterministic. Memoisation correctness is already covered by the explicit case-table tests above, which always run regardless of short mode. * fix(applicationprofile_processor): nil-guard collapseSettings via safe accessor CodeRabbit upstream PR kubescape#326 finding #3 (applicationprofile_processor.go:92). A zero-valued processor — constructed via direct struct literal rather than the NewApplicationProfileProcessor factory — has a nil collapseSettings function field. The deflate path then nil-dereferences on the call inside PreSave. Adds `effectiveCollapseSettings()` as the safe accessor used by PreSave. It returns the provider's result when configured, or the compiled-in DefaultCollapseSettings when the field is nil. The direct field-call path (`a.collapseSettings()`) is intentionally left raw — callers who construct a zero-value processor and call the unsafe path explicitly are on the "I know what I'm doing" side. New regression test TestApplicationProfileProcessor_ZeroValue_NoPanicOnCollapseSettings pins both halves of the contract: - effectiveCollapseSettings() on zero-value processor does NOT panic and returns the compiled-in defaults - raw field-call on zero-value processor DOES still panic (no silent behaviour change) * fix(proto): declare repeated string ipAddresses = 9 on NetworkNeighbor CodeRabbit upstream PR kubescape#326 finding #2 (Major). The Go struct's protobuf tag is `bytes,9,rep,name=ipAddresses` but generated.proto did not declare field 9 on the NetworkNeighbor message. Runtime behaviour was unaffected because generated.pb.go is the actual codec and already encodes field 9 correctly (verified: wire tag 0x4a = field 9, wire type 2). But the .proto file is the source of truth for: - Any future regeneration (`make generate`) — drops the field if the .proto doesn't declare it - Cross-language consumers compiling against the .proto schema - Documentation accuracy Restores the declaration. New regression test TestNetworkNeighbor_ProtoFile_DeclaresIPAddresses parses generated.proto and asserts the NetworkNeighbor message body contains the field-9 declaration. If the .proto and the Go tag ever diverge again, the test fails loudly at unit-test time rather than at the next regen. The existing IPAddresses protobuf roundtrip tests are unaffected and continue to pass. * fix(dynamicpathdetector): DefaultCollapseConfig accessor (defensive copy) CodeRabbit upstream PR kubescape#323 finding #3. The package-level `DefaultCollapseConfig` was an exported mutable var: any caller doing `dynamicpathdetector.DefaultCollapseConfig.Threshold = 1` would silently corrupt every analyzer constructed thereafter. The threshold surface is security-sensitive (governs how aggressively profile paths collapse into wildcards), so accidental cross-caller leakage is a real risk. Replaces the exported var with an unexported `defaultCollapseConfig` and an accessor `DefaultCollapseConfig()` that returns a value copy. Pattern mirrors the existing `defaultCollapseConfigs` / `DefaultCollapseConfigs()` defensive-copy accessor for the slice form. Updates 6 callsites across the test suite to use the accessor. Adds TestDefaultCollapseConfig_DefensiveCopy pinning the contract: a caller mutating the returned struct does not affect subsequent reads. * docs(dynamicpathdetector): comment + guard kubescape#323 nitpicks (#5, #6, #9, #10) Four small docstring / defensive-guard improvements from CodeRabbit upstream PR kubescape#323: #5 wildcardKey / getEndpointKey format coupling — extract buildEndpointKey as the single source of truth for the lookup key shape. Both call-sites now route through it, removing the "must stay in sync" risk that allowed past drift. New test TestBuildEndpointKey_SharedFormat pins end-to-end behaviour via MergeDuplicateEndpoints: a wildcard-port entry and a specific- port sibling collapse into one row with merged Methods. #6 consolidateOpens O(n²) — acknowledged in the function's docstring along with the threshold (≤ ~10k entries) above which a trie-based replacement would be worthwhile. Behaviour unchanged. #9 removeEndpoint in-place mutation — added a NOTE block in the docstring describing the backing-array shift, why the sole caller is safe today, and the swap-to-copy guidance for any future caller that stores intermediate slice references. #10 hasPrefixAtBoundary("") edge — added an explicit empty-prefix branch returning true. Without it the function fell through to pathPrefix[0] == '/', which is true for any absolute path — effectively treating "" as a root-matching prefix incidentally. The explicit branch makes the invariant load-bearing. New test TestHasPrefixAtBoundary_EmptyPrefix pins the surface via FindConfigForPath. All existing tests still pass. Local run on this arm64 machine: go test ./pkg/registry/file/... ok in 0.93s * deps: bump syft to v1.42.4 (CVE-2026-33481), stereoscope v0.1.22, runtime-spec v1.3.0 CodeRabbit upstream PR kubescape#326 finding #1 (Critical). Lifts syft from v1.32.0 to v1.42.4 — the lowest version that includes the CVE-2026-33481 fix (Syft was failing to clean up temporary storage when a scan errored mid-flight). v1.42.4 is the latest patch on the v1.42 line, so the upgrade gets the CVE fix without taking the v1.43/v1.44 minor bumps' larger surface-area changes. go mod tidy pulled forward two transitive bumps: - stereoscope v0.1.9 → v0.1.22 (Anchore's syft-companion lib, must move in lockstep with syft) - runtime-spec v1.2.1 → v1.3.0 (transitive via stereoscope) - modernc.org/libc v1.67.6 → v1.70.0 (transitive) - modernc.org/sqlite v1.46.1 → v1.46.2 (transitive) go.mod no longer carries a replace directive against kubescape/syft — our fork doesn't have v1.42.3+ tags and the upstream content is required for the CVE fix. Direct anchore/syft dependency is now the source of truth. Local validation: go build ./... ok go test ./pkg/... ok in 1.4s (all 16 packages green) Vendor directory is .gitignored; `go mod vendor` was rerun locally to keep build/manifest in sync. * fix: address 3 follow-up rabbit findings on PR #33 1. ci(manual-integration-tests): use job-level if: false instead of exit 1 — workflow shows as "skipped" not "failed red X" in the Actions UI when dispatched, removing the cosmetic noise while keeping the ~30-minute waste prevention. Issue #32 reference kept in the YAML comment above the conditional. 2. test(network_types_protobuf): also assert the Go struct's protobuf tag, not just the .proto-file text. The earlier test only checked one side of the schema/tag contract, so tag drift on the Go side would silently pass. Now both halves are pinned. 3. test(applicationprofile_processor): assert the FULL CollapseSettings struct (not just OpenDynamicThreshold). Without this, a regression that resets any field added to CollapseSettings in the future would silently pass the zero-value defensive-fallback test. All from CodeRabbit follow-up review on storage PR #33. Local validation: go test ./pkg/registry/file/ ./pkg/apis/softwarecomposition/v1beta1/ → both packages ok * docs(networkmatch): correct spec URL to billofbehavior.com --------- Co-authored-by: Entlein <eineintlein@gmail.com> --------- Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com> Co-authored-by: Matthias Bertschy <matthias.bertschy@gmail.com> Co-authored-by: Entlein <eineintlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Umbrella PR consolidating the SBOB-wildcards storage surface. Tier-by-tier breakdown per CodeRabbit's atomic-split advisory on
kubescape/storage#323:kubescape/storage#323(path-wildcards core: anchored trailing-*, per-endpoint port key, CollapseConfig types)kubescape/storage#322(CompareExecArgs),#325(CollapseConfig CRD + processors)kubescape/storage#324(networkmatch package: DNS leading-*+ trailing-*+ mid-⋯+ CIDR + IPAddresses +*sentinel)This umbrella carries the union of the above. Once the per-tier PRs land in order, the umbrella shrinks to empty.
Performance evidence (node-agent side)
The bench is run on the node-agent build that links against this storage version. Validated on workflow run 25959674209:
Companion node-agent umbrella
kubescape/node-agent#810. Storage and node-agent run on different syft chains by design — storage onanchore/syft v1.42.4(matches storage upstream main), node-agent onkubescape/syft v1.32.0-ks.2via replace (matches node-agent upstream main).Notes
v1.42.4for CVE-2026-33481 (matches the storage upstream main bump in commitcecb767f).CompareDynamicperf: thesplitPathallocation pattern called out in matthyx's earlier review of PR CollapsConfigs and Wildcard Matching for Opens (and Paths in Endpoints) while reproducing default storage behavior if not used #316 line 358 is unchanged here. We've added DP-memoisation incompareSegments(pkg/registry/file/dynamicpathdetector/analyzer.go) to bound the worst-case for multi-*patterns, butsplitPathitself still allocates two slices per call. Happy to rebase onto the zero-alloc rewrite when it lands.pkg/registry/file/networkmatch/corrected tobillofbehavior.com.