Skip to content

feat(cel/parse): add 3-arg get_exec_path(args, comm, exepath) overload#843

Merged
matthyx merged 1 commit into
mainfrom
feat/parse-get-exec-path-exepath-overload
Jun 26, 2026
Merged

feat(cel/parse): add 3-arg get_exec_path(args, comm, exepath) overload#843
matthyx merged 1 commit into
mainfrom
feat/parse-get-exec-path-exepath-overload

Conversation

@matthyx

@matthyx matthyx commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

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 from main. The rule YAML and component tests from #805 move to kubescape/rulelibrary instead.

Details

main already carries the rest of the #805 engine — ap.was_executed_with_args, the ExecsByPath projection (incl. mock population), and storage 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):

  1. exepath — kernel-authoritative, spoof-resistant (argv[0] is user-controllable even when absolute, e.g. exec -a /bin/sh sleep).
  2. argv[0] when non-empty AND exepath empty (fexecve / AT_EMPTY_PATH).
  3. comm as final fallback.

This closes the fork-shell mismatch (sh -c … records /bin/sh, and the rule side now queries /bin/sh too) 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 main except this one overload; its rule content (R0040 + the get_exec_path migrations + binding) belongs in kubescape/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 a go.mod pseudo-version.

Tests

TestGetExecPath_SymmetryWithRecordingSide pins exepath-wins, argv[0]-spoof, fexecve fallback, comm fallback, and busybox-symlink cases, mirroring TestResolveExecPath on the recording side.

ok  github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/parse
ok  github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/...   (full CEL subtree green)

Summary by CodeRabbit

  • New Features

    • parse.get_exec_path now supports an additional 3-argument form that can use an explicit executable path when available.
    • Path resolution now prefers a provided executable path, then falls back to the previous argument-based logic.
  • Bug Fixes

    • Improved consistency in path selection for cases where the executable path is empty, missing, or differs from command-name values.
    • Added validation so invalid path values are handled cleanly.

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
@matthyx matthyx added the ai-assisted Created through Armosec AI tooling (armosec-shared-rules plugin) label Jun 25, 2026
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c3756073-50c0-4120-94e4-3449a6864d82

📥 Commits

Reviewing files that changed from the base of the PR and between 6a05130 and 2ff5f05.

📒 Files selected for processing (3)
  • pkg/rulemanager/cel/libraries/parse/parse.go
  • pkg/rulemanager/cel/libraries/parse/parselib.go
  • pkg/rulemanager/cel/libraries/parse/parsing_test.go

📝 Walkthrough

Walkthrough

The parse CEL library now accepts a 3-argument parse.get_exec_path(args, comm, exepath) overload, resolves exepath before falling back to args[0] or comm, and adds tests covering precedence and fallback cases.

Changes

Exec path overload

Layer / File(s) Summary
Exec-path overload wiring
pkg/rulemanager/cel/libraries/parse/parse.go, pkg/rulemanager/cel/libraries/parse/parselib.go
getExecPath keeps the 2-argument fallback path, getExecPathWithExePath returns a non-empty exepath first, and parse.get_exec_path registers the 3-argument overload.
Exec-path resolver tests
pkg/rulemanager/cel/libraries/parse/parsing_test.go
A new CEL test suite compiles parse.get_exec_path(args, comm, exepath) expressions and verifies precedence, fallback, and spoofing-related cases.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • kubescape/node-agent#800: Updates the exec-path selection logic on the recording side with the same exepath-first, args[0]-then-comm precedence.

Poem

Hoppy paths! I leap through code so bright 🐰
exepath first, then argv takes flight
If empty, comm will guide the way
My whiskers twitch at every test today
Hop hop, resolved and snug in sight

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a 3-argument get_exec_path overload to the parse CEL library.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/parse-get-exec-path-exepath-overload

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

matthyx added a commit to kubescape/rulelibrary that referenced this pull request Jun 25, 2026
…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>
@github-actions

Copy link
Copy Markdown

Performance Benchmark Results

Node-Agent Resource Usage
Metric BEFORE AFTER Delta
Avg CPU (cores) 0.212 0.209 -1.2%
Peak CPU (cores) 0.225 0.223 -0.8%
Avg Memory (MiB) 333.964 336.442 +0.7%
Peak Memory (MiB) 335.484 346.621 +3.3%
Dedup Effectiveness

No data available.

@matthyx matthyx added the release Create release label Jun 25, 2026
@matthyx
matthyx merged commit 86de444 into main Jun 26, 2026
29 checks passed
@matthyx
matthyx deleted the feat/parse-get-exec-path-exepath-overload branch June 26, 2026 05:17
@matthyx matthyx moved this to To Archive in KS PRs tracking Jun 26, 2026
matthyx added a commit to kubescape/rulelibrary that referenced this pull request Jun 26, 2026
* 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>
matthyx added a commit that referenced this pull request Jun 26, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-assisted Created through Armosec AI tooling (armosec-shared-rules plugin) release Create release

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

1 participant