feat(cel/parse): add 3-arg get_exec_path(args, comm, exepath) overload#843
Conversation
Adds an exepath-aware overload of parse.get_exec_path to the CEL parse library. The existing 2-arg form resolves argv[0] -> comm; the new 3-arg form resolves the exec path symmetrically with the recording side's resolveExecPath in pkg/containerprofilemanager/v1/event_reporting.go: 1. exepath (kernel-authoritative, spoof-resistant) 2. argv[0] when non-empty AND exepath empty (fexecve / AT_EMPTY_PATH) 3. comm This lets rules query the same identity the recorder stored, closing the fork-shell mismatch (`sh -c ...` records /bin/sh and the rule side now queries /bin/sh) while preserving argv[0]-spoofing protection. The 2-arg overload is unchanged, so existing rules keep working. The overload is consumed by the OSS R0040 rule in kubescape/rulelibrary (the path-resolution half of the get_exec_path migration). It replaces the engine portion of node-agent#805; the rule YAML and component tests from that PR move to rulelibrary instead. Adds TestGetExecPath_SymmetryWithRecordingSide covering exepath-wins, fexecve fallback, comm fallback, busybox symlink, and absolute-argv[0] spoof cases. Signed-off-by: matthyx <matthias.bertschy@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Docs-exempt: additive CEL helper overload, no node-agent config/API/behavioral change; consumer rule R0040 is documented in kubescape/rulelibrary
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe parse CEL library now accepts a 3-argument ChangesExec path overload
Sequence Diagram(s)sequenceDiagram
participant CEL evaluator
participant parseLibrary.Declarations
participant getExecPathWithExePath
participant getExecPath
CEL evaluator->>parseLibrary.Declarations: invoke parse.get_exec_path(args, comm, exepath)
parseLibrary.Declarations->>getExecPathWithExePath: dispatch 3-argument overload
getExecPathWithExePath->>getExecPath: fallback when exepath is empty
getExecPathWithExePath-->>CEL evaluator: resolved exec path string
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…igrate rule tests to new mock API Pins node-agent to the pseudo-version of kubescape/node-agent#843 (feat(cel/parse): add 3-arg get_exec_path overload) so the R0040 rule and its test can resolve parse.get_exec_path(args, comm, exepath). Also bumps storage v0.0.258 -> v0.0.278 (MatchExecArgs / CompareExecArgs) and aligns the replace block with node-agent main (inspektor-gadget fork, runtime-spec v1.2.1, syft, stereoscope) so the module graph builds. The node-agent bump (Apr -> Jun main) drifted the test-facing mock API; migrates all rule tests accordingly: * objCache.ApplicationProfileCache().GetApplicationProfile(id) -> objCache.GetApplicationProfile(id) (12 files) * objCache.NetworkNeighborhoodCache().GetNetworkNeighborhood(id) -> objCache.GetNetworkNeighborhood(id) (4 files) * RulePolicyValidator.Validate(ruleId, comm, &profile.Spec.Containers[0]) -> Validate(ruleId, comm, &objectcache.ProjectedContainerProfile{ PolicyByRuleId: profile.Spec.Containers[0].PolicyByRuleId}) (r1011, r1012, r1030 — Validate now takes a ProjectedContainerProfile) Full suite, lint-projection, and the README check pass except one pre-existing R1001 subtest (different_container_name), whose premise (name-based profile isolation) the new RuleObjectCacheMock no longer models — left for an owner decision; it is already covered by the sibling "no application profile" case. Signed-off-by: matthyx <matthias.bertschy@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup EffectivenessNo data available. |
* feat(R0040): add Unexpected Process Arguments rule Adds pkg/rules/r0040-unexpected-process-arguments/ (yaml + README + tests). R0040 fires when an allowed binary is exec'd with an argv vector that was never recorded for that path, consuming ap.was_executed_with_args over the ExecsByPath projection surface and storage's dynamicpathdetector.MatchExecArgs. It stays silent on unknown paths (R0001's domain) and on any recorded argv. Blocked on node-agent#805: the expression uses the 3-arg parse.get_exec_path(args, comm, exepath) overload and ap.was_executed_with_args, neither of which exists in the pinned node-agent (v0.3.98-...-4aefa6517e0a) or in storage v0.0.258. go.mod must bump node-agent to a release carrying the #805 engine and storage to >= v0.0.278 before `go test` can pass. lint-projection, the README check, and `go vet` are green today; `go test` fails only on the missing CEL overload. Signed-off-by: matthyx <matthias.bertschy@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(deps): bump node-agent to 3-arg get_exec_path pseudo-version; migrate rule tests to new mock API Pins node-agent to the pseudo-version of kubescape/node-agent#843 (feat(cel/parse): add 3-arg get_exec_path overload) so the R0040 rule and its test can resolve parse.get_exec_path(args, comm, exepath). Also bumps storage v0.0.258 -> v0.0.278 (MatchExecArgs / CompareExecArgs) and aligns the replace block with node-agent main (inspektor-gadget fork, runtime-spec v1.2.1, syft, stereoscope) so the module graph builds. The node-agent bump (Apr -> Jun main) drifted the test-facing mock API; migrates all rule tests accordingly: * objCache.ApplicationProfileCache().GetApplicationProfile(id) -> objCache.GetApplicationProfile(id) (12 files) * objCache.NetworkNeighborhoodCache().GetNetworkNeighborhood(id) -> objCache.GetNetworkNeighborhood(id) (4 files) * RulePolicyValidator.Validate(ruleId, comm, &profile.Spec.Containers[0]) -> Validate(ruleId, comm, &objectcache.ProjectedContainerProfile{ PolicyByRuleId: profile.Spec.Containers[0].PolicyByRuleId}) (r1011, r1012, r1030 — Validate now takes a ProjectedContainerProfile) Full suite, lint-projection, and the README check pass except one pre-existing R1001 subtest (different_container_name), whose premise (name-based profile isolation) the new RuleObjectCacheMock no longer models — left for an owner decision; it is already covered by the sibling "no application profile" case. Signed-off-by: matthyx <matthias.bertschy@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(rules): migrate exec rules to 3-arg get_exec_path; drop redundant R1001 subtest Aligns the sibling exec rules with the exepath-authoritative parse.get_exec_path(args, comm, exepath) overload (node-agent#843), matching the reviewed forms from node-agent#805's default-rules.yaml: R0001, R0007, R1001 — replace the 2-arg get_exec_path + the now-redundant (exepath == "" || !ap.was_executed(..., exepath)) fallback with the single 3-arg call. The overload subsumes the fallback: exepath wins (kernel-authoritative, spoof-resistant), argv[0] only when exepath is empty, then comm. R1004 — both get_exec_path call sites bumped to 3-arg; fallback dropped. R1000 (/dev/shm) — replace get_exec_path(...).startsWith('/dev/shm/') with a direct event.args[0] check and fix the clause grouping. Test fixtures for R0001/R0007 now store the kernel-resolved exepath as the profile Exec.Path (the recording side is exepath-authoritative), so the "allowed" cases still suppress under the new resolution. READMEs updated per the rule-yaml change. Drops the redundant R1001 "different container name" subtest: its premise (name-based profile isolation) is no longer modeled by RuleObjectCacheMock, and the case it intended is already covered by the sibling "no application profile" case. Signed-off-by: matthyx <matthias.bertschy@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(crd): regenerate rules-crd.yaml (make generate-rules-crd) Regenerates the bundled CRD from pkg/rules/. Picks up the R0040 addition and the 3-arg get_exec_path migration (R0001/R0007/R1000/R1001/R1004), plus pre-existing source drift in rules whose per-rule yaml had been updated without regenerating the CRD (R1003/R1005/R1006/R1011/R1015 name/description/ message). Generated artifact only — no source rule changes here. Signed-off-by: matthyx <matthias.bertschy@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(deps): pin node-agent v0.3.147 (released; replaces branch pseudo-version) node-agent#843 (the 3-arg get_exec_path overload) merged and was released as v0.3.147. Re-pins node-agent from the #843 branch pseudo-version to the release tag; storage stays at v0.0.278. go test ./..., make lint-projection, and the README check are green against the release. Signed-off-by: matthyx <matthias.bertschy@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(rules): inline-ternary exepath resolution for the migrated exec rules Replaces the 3-arg parse.get_exec_path(args, comm, exepath) overload in R0001/R0007/R1001/R1004 with a standard-CEL ternary: event.exepath != "" ? event.exepath : parse.get_exec_path(args, comm) Behaviorally identical (exepath wins, else argv[0]/comm) but uses only the 2-arg overload + a ternary, both present on every agent version. So these four rules no longer require node-agent v0.3.147 and need no agentVersionRequirement gate or dual-variant: they keep working on older agents and on the private-node-agent fork (which still imports kubescape/node-agent v0.3.146) with the same spoof-resistant behavior. R0040 is unchanged (it genuinely needs the 3-arg overload + ap.was_executed_with_args from v0.3.147). READMEs and rules-crd.yaml regenerated. go test ./..., lint-projection, and the README check pass. Signed-off-by: matthyx <matthias.bertschy@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Signed-off-by: matthyx <matthias.bertschy@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…st_32 + lint guard (#844) * test(rules): sync default-rules.yaml from rulelibrary; add R0040, Test_32, lint guard Syncs the bundled tests/chart/templates/node-agent/default-rules.yaml from kubescape/rulelibrary's rules-crd.yaml (the source of truth after rulelibrary#39): - adds R0040 "Unexpected process arguments" (3-arg get_exec_path overload + ap.was_executed_with_args, both in node-agent v0.3.147) - the migrated exec rules (R0001/R0007/R1001/R1004) use the backward-compatible exepath-first ternary rather than the 3-arg overload - adds R2000/R2001 and picks up the rule renames already in rulelibrary - reconciles default-rule-binding.yaml: enables R0040 and updates the renamed rule names (R1000/R1003/R1015/R2001) so every binding still resolves Adds Test_32_UnexpectedProcessArguments (+ fixtures) and registers it in the component-tests matrix. Test_32 uses dynamicpathdetector.WildcardIdentifier (the zero-or-more exec-args sentinel in the pinned storage v0.0.278), matching the existing exec_test.go convention. The ExecArgsWildcard ("⋯⋯") form and the "*"->literal flip arrived in storage v0.0.287 and are a separate coordinated storage bump. Adds pkg/rulemanager/cel/libraries/parse/default_rules_yaml_lint_test.go to guard the bundled rules: no UNGUARDED 2-arg get_exec_path (the exepath-first ternary or the 3-arg form is required), no string(event.args), and R1000 detects /dev/shm via argv[0]. All three pass against the synced bundle. Replaces node-agent#805 (its engine landed separately via #843). Signed-off-by: matthyx <matthias.bertschy@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Docs-exempt: test-only + rule-bundle sync; node-agent engine/behavior unchanged (R0040 engine shipped in #843, rule bodies are sourced from kubescape/rulelibrary). * test: fix Test_02 rule rename; skip storage-version-dependent Test_32 case - Test_02_AllAlertsFromMaliciousApp: update the R1000 expectation key from the old "Process executed from malicious source" to the synced name "Process Executed from /dev/shm" (rule renamed in rulelibrary). - Test_32/echo_literal_star_does_not_broaden_R0040: t.Skip — it asserts the "*"-is-literal semantics introduced in storage v0.0.287, but main pins v0.0.278 where "*" (WildcardIdentifier) is a zero-or-more wildcard, so a literal "*" does broaden. Re-enable with the coordinated storage bump. The other 9 Test_32 subtests pass. Test_27 (R0002 was disabled by the wholesale bundle sync) is still pending a decision on whether to keep R0002 enabled in node-agent's bundle. Docs-exempt: test-only fix, no node-agent behavioral/API change. Signed-off-by: matthyx <matthias.bertschy@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(rules): bump storage v0.0.287 (ExecArgsWildcard); enable R0002 in Test_27 #3 (the original #805 intent): bump kubescape/storage v0.0.278 -> v0.0.287, which introduces ExecArgsWildcard ("⋯⋯") as the zero-or-more exec-args wildcard and makes "*" a literal in exec args. Migrate exec_test.go's "*"-as-wildcard profile entries to dynamicpathdetector.ExecArgsWildcard, revert Test_32 from the v0.0.278 WildcardIdentifier stopgap back to ExecArgsWildcard, and un-skip echo_literal_star_does_not_broaden_R0040 (it now passes — "*" is literal). Engine code is unchanged: it routes through dynamicpathdetector.MatchExecArgs (stable signature); only test fixtures supply the sentinel. #2: Test_27_ApplicationProfileOpens enables R0002 ("Files Access Anomalies"), which ships disabled in the synced bundle, for its own cluster only — by applying tests/resources/r0002-files-access-enabled.yaml (an override Rules CRD with R0002 enabled). The rules-watcher filters disabled rules before its per-ID merge, so it uses the enabled override; default-rules.yaml is untouched. Component-test matrix jobs run on isolated clusters. (Test_33 also exercises R0002 but is not in the matrix.) Unit tests (applicationprofile, parse, objectcache) pass under v0.0.287; the component tests compile. Docs-exempt: dependency bump + test updates; the exec-args wildcard sentinel is documented in storage and the rulelibrary R0040 rule, not in node-agent docs. Signed-off-by: matthyx <matthias.bertschy@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: also enable R0002 in Test_02; share the enable helper Test_02_AllAlertsFromMaliciousApp expects "Files Access Anomalies in container" (R0002) to fire, but R0002 ships disabled in the synced bundle — so last commit's rename fix alone left Test_02 red on R0002. Extract enableR0002ForTest (apply the override Rules CRD, return a cleanup func) and use it from both Test_02 and Test_27, replacing Test_27's inline copy. Docs-exempt: test-only change. Signed-off-by: matthyx <matthias.bertschy@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Signed-off-by: matthyx <matthias.bertschy@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Adds a 3-arg
parse.get_exec_path(args, comm, exepath)overload to the CEL parse library. This is the engine replacement for node-agent#805 — just the path-resolution helper the OSS R0040 rule needs, branched cleanly frommain. The rule YAML and component tests from #805 move tokubescape/rulelibraryinstead.Details
mainalready carries the rest of the #805 engine —ap.was_executed_with_args, theExecsByPathprojection (incl. mock population), andstorage v0.0.278. The only missing piece for R0040 was the exepath-aware path resolver, which this PR adds.The existing 2-arg form resolves
argv[0] → comm. The new 3-arg form resolves symmetrically with the recording side (pkg/containerprofilemanager/v1/event_reporting.go:resolveExecPath):argv[0]is user-controllable even when absolute, e.g.exec -a /bin/sh sleep).AT_EMPTY_PATH).This closes the fork-shell mismatch (
sh -c …records/bin/sh, and the rule side now queries/bin/shtoo) while preserving argv[0]-spoofing protection. The 2-arg overload is unchanged, so every existing rule keeps working — this is purely additive.Why a new PR instead of #805
Per the issue discussion, #805 is being retired. Its engine already landed on
mainexcept this one overload; its rule content (R0040 + theget_exec_pathmigrations + binding) belongs inkubescape/rulelibrary, not bundled in node-agent. This PR is the minimal engine delta; the rule lives in rulelibrary#39, which will pin this commit via ago.modpseudo-version.Tests
TestGetExecPath_SymmetryWithRecordingSidepins exepath-wins, argv[0]-spoof, fexecve fallback, comm fallback, and busybox-symlink cases, mirroringTestResolveExecPathon the recording side.Summary by CodeRabbit
New Features
parse.get_exec_pathnow supports an additional 3-argument form that can use an explicit executable path when available.Bug Fixes