Open
Conversation
This was referenced Apr 30, 2026
…ompareExecArgs Routes ap.was_executed_with_args(...) through the new dynamicpathdetector.CompareExecArgs matcher so user-defined ApplicationProfile entries can express argument-vector patterns: '⋯' (DynamicIdentifier) — matches exactly one arg position. '*' (WildcardIdentifier) — matches zero or more consecutive args. Previously the function used slices.Compare for exact equality, which silently ignored wildcard tokens in the profile (a profile entry of [--user, ⋯] would not match runtime args [--user, alice]). The new matcher anchors both ends, treats '⋯' / '*' as wildcards, and falls back to literal-equality otherwise. 13 new unit subtests under TestExecWithArgsWildcardInProfile cover curl-with-any-URL, sh -c with arbitrary payload, ls -l with any directory, and echo with arbitrary trailing args, plus negative cases (literal-anchor mismatch, ⋯ over- and under-consumed, * mid-profile that fails to re-anchor on a literal). Storage dep bumped to k8sstormcenter/storage@3fc287210729 which adds the matcher (k8sstormcenter/storage#23). Note: default-rules.yaml R0001 'Unexpected process launched' currently uses path-only ap.was_executed. End-to-end exercise of the wildcard path requires either a rule update or a custom RuleSet — deferred to a follow-up so this PR stays scoped to the matcher wiring.
entlein
force-pushed
the
feat/exec-arg-wildcards
branch
from
f4c1328 to
c20e29b
Compare
📝 WalkthroughWalkthroughUpdated storage dependency to a newer pseudo-version in ChangesWildcard Exec Argument Matching
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/rulemanager/cel/libraries/applicationprofile/exec_test.go`:
- Around line 329-350: Add a positive+negative test pair that exercises
mid-profile "*" re-anchoring in the Execs test table: within the Execs slice
(type v1beta1.ExecCalls) add an entry where Args contains a literal token, then
"*" and then another literal token (e.g., Args: []string{"cmd", "*", "end"}) and
assert a match for a command whose args have arbitrary middle tokens but end
with the trailing literal (positive), and a non-match for a command where the
later literal is missing or not anchored (negative). Update the corresponding
expectations in the test (same table near the existing curl/sh/ls/echo entries
in exec_test.go) so the test validates that "*" can re-anchor to a later literal
token between anchors.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 469d529c-e5b1-49ab-b030-d6d5c0583d21
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
go.modpkg/rulemanager/cel/libraries/applicationprofile/exec.gopkg/rulemanager/cel/libraries/applicationprofile/exec_test.go
Comment on lines
+329
to
+350
| Execs: []v1beta1.ExecCalls{ | ||
| // curl any URL: --user must be literal, value is one position. | ||
| { | ||
| Path: "/usr/bin/curl", | ||
| Args: []string{"--user", "⋯"}, | ||
| }, | ||
| // sh -c with any trailing payload (zero or more args). | ||
| { | ||
| Path: "/bin/sh", | ||
| Args: []string{"-c", "*"}, | ||
| }, | ||
| // ls -l in any directory — single trailing position. | ||
| { | ||
| Path: "/bin/ls", | ||
| Args: []string{"-l", "⋯"}, | ||
| }, | ||
| // echo with any number of greeting words after a literal anchor. | ||
| { | ||
| Path: "/bin/echo", | ||
| Args: []string{"hello", "*"}, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find profile arg patterns where "*" appears before a trailing literal anchor.
rg -nP 'Args:\s*\[]string\{[^}]*"\*"\s*,\s*"[^"]+"' pkg/rulemanager/cel/libraries/applicationprofile/exec_test.goRepository: k8sstormcenter/node-agent
Length of output: 51
🏁 Script executed:
# Check if the file exists and get basic info
wc -l pkg/rulemanager/cel/libraries/applicationprofile/exec_test.goRepository: k8sstormcenter/node-agent
Length of output: 135
🏁 Script executed:
# Read the relevant test sections mentioned in the review (lines 329-350 and 364-390)
sed -n '320,420p' pkg/rulemanager/cel/libraries/applicationprofile/exec_test.goRepository: k8sstormcenter/node-agent
Length of output: 3418
🏁 Script executed:
# Find the implementation of the matcher to understand wildcard behavior
fd -i "exec" pkg/rulemanager/cel/libraries/applicationprofile/ -type f | grep -v test | head -5Repository: k8sstormcenter/node-agent
Length of output: 240
🏁 Script executed:
# Find the implementation file that contains the matching logic
fd "exec" pkg/rulemanager/cel/libraries/applicationprofile/ -not -name "*test*" -type fRepository: k8sstormcenter/node-agent
Length of output: 300
🏁 Script executed:
# Check the implementation to understand wildcard matching behavior
find pkg/rulemanager/cel/libraries/applicationprofile/ -name "*.go" -not -name "*test*" | head -5Repository: k8sstormcenter/node-agent
Length of output: 358
🏁 Script executed:
# Read the exec.go implementation to understand wildcard matching semantics
cat -n pkg/rulemanager/cel/libraries/applicationprofile/exec.goRepository: k8sstormcenter/node-agent
Length of output: 8561
🏁 Script executed:
# Find the dynamicpathdetector implementation
fd "dynamicpathdetector" --type fRepository: k8sstormcenter/node-agent
Length of output: 51
🏁 Script executed:
# Search for CompareExecArgs function
rg "func.*CompareExecArgs" -A 30Repository: k8sstormcenter/node-agent
Length of output: 51
🏁 Script executed:
# Check if there's any documentation or references to wildcard behavior
rg "wildcard|re-anchor|\*" pkg/rulemanager/cel/libraries/applicationprofile/exec_test.goRepository: k8sstormcenter/node-agent
Length of output: 1114
🏁 Script executed:
# Read the full test structure more carefully around the test cases
sed -n '302,419p' pkg/rulemanager/cel/libraries/applicationprofile/exec_test.goRepository: k8sstormcenter/node-agent
Length of output: 4229
🏁 Script executed:
# Search for any documentation about wildcard re-anchoring behavior
rg "re-anchor|anchor" pkg/rulemanager/cel/libraries/applicationprofile/Repository: k8sstormcenter/node-agent
Length of output: 361
🏁 Script executed:
# Look for test comments explaining the matching contract
rg -B5 -A5 "contract|behavior|semantics" pkg/rulemanager/cel/libraries/applicationprofile/exec_test.goRepository: k8sstormcenter/node-agent
Length of output: 51
🏁 Script executed:
# Check if there are any other test files that might cover this pattern
find . -name "*test.go" -path "*/applicationprofile/*" | xargs grep -l "wildcard\|\*"Repository: k8sstormcenter/node-agent
Length of output: 543
🏁 Script executed:
# Check the entire exec_test.go to understand all test cases and verify the structure
wc -l pkg/rulemanager/cel/libraries/applicationprofile/exec_test.go && sed -n '385,410p' pkg/rulemanager/cel/libraries/applicationprofile/exec_test.goRepository: k8sstormcenter/node-agent
Length of output: 1012
🏁 Script executed:
# Verify if there's any evidence of CompareExecArgs implementation elsewhere
rg "CompareExecArgs" --type go -A 50 | head -100Repository: k8sstormcenter/node-agent
Length of output: 8958
🏁 Script executed:
# Look for the dynamicpathdetector package location
find . -name "dynamicpathdetector*" -o -name "*pathdetector*" 2>/dev/null | head -20Repository: k8sstormcenter/node-agent
Length of output: 51
🏁 Script executed:
# Check imports in exec.go to find the full import path for dynamicpathdetector
grep -n "import\|dynamicpathdetector" pkg/rulemanager/cel/libraries/applicationprofile/exec.go | head -20Repository: k8sstormcenter/node-agent
Length of output: 243
🏁 Script executed:
# Search for any references to how wildcard matching should work
rg "WildcardIdentifier|DynamicIdentifier" -B2 -A2 pkg/rulemanager/cel/libraries/applicationprofile/Repository: k8sstormcenter/node-agent
Length of output: 705
🏁 Script executed:
# Verify the exact line numbers mentioned in the review
sed -n '329,350p' pkg/rulemanager/cel/libraries/applicationprofile/exec_test.goRepository: k8sstormcenter/node-agent
Length of output: 623
🏁 Script executed:
# Check the "Also applies to" lines 364-390
sed -n '364,390p' pkg/rulemanager/cel/libraries/applicationprofile/exec_test.goRepository: k8sstormcenter/node-agent
Length of output: 1502
Add an explicit mid-profile * re-anchoring test case.
Current test coverage validates trailing * patterns well, but there's no explicit scenario where * must re-anchor to a later literal token. Adding one positive+negative pair will lock in the anchored behavior contract.
Proposed test additions
@@
Execs: []v1beta1.ExecCalls{
@@
// echo with any number of greeting words after a literal anchor.
{
Path: "/bin/echo",
Args: []string{"hello", "*"},
},
+ // wildcard in the middle must re-anchor to trailing literal.
+ {
+ Path: "/usr/bin/env",
+ Args: []string{"A", "*", "Z"},
+ },
},
})
@@
// echo hello *
{"echo hello world from test", "/bin/echo", []string{"hello", "world", "from", "test"}, true},
{"echo hello (no trailing args)", "/bin/echo", []string{"hello"}, true},
{"echo goodbye world — wrong literal anchor", "/bin/echo", []string{"goodbye", "world"}, false},
+
+ // mid-profile * re-anchoring
+ {"env A x y Z — wildcard re-anchors to trailing literal", "/usr/bin/env", []string{"A", "x", "y", "Z"}, true},
+ {"env A x y — missing trailing literal anchor", "/usr/bin/env", []string{"A", "x", "y"}, false},
}Also applies to: 364-390
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/rulemanager/cel/libraries/applicationprofile/exec_test.go` around lines
329 - 350, Add a positive+negative test pair that exercises mid-profile "*"
re-anchoring in the Execs test table: within the Execs slice (type
v1beta1.ExecCalls) add an entry where Args contains a literal token, then "*"
and then another literal token (e.g., Args: []string{"cmd", "*", "end"}) and
assert a match for a command whose args have arbitrary middle tokens but end
with the trailing literal (positive), and a non-match for a command where the
later literal is missing or not anchored (negative). Update the corresponding
expectations in the test (same table near the existing curl/sh/ls/echo entries
in exec_test.go) so the test validates that "*" can re-anchor to a later literal
token between anchors.
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
1 participant
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.
Summary
Routes
ap.was_executed_with_args(containerID, path, args)through the newdynamicpathdetector.CompareExecArgsmatcher (k8sstormcenter/storage#23) souser-defined ApplicationProfile entries can express argument-vector patterns:
⋯DynamicIdentifier*WildcardIdentifierAnything else is literal-equality. Both ends are anchored — runtime args must
be fully consumed by the profile vector (literal /
⋯/*absorbing run).Previously the function used
slices.Comparefor exact equality, so a profileentry like
Args: [\"--user\", \"⋯\"]would never match runtime args[\"--user\", \"alice\"]— wildcard tokens were silently treated as literals.Why
Sibling work to the storage wildcard-port endpoint fix (#33). User-defined
profiles already store path + args; the path side has had wildcard support for a
while, the arg side did not.
Tests
TestExecWithArgsWildcardInProfile— 13 subtests covering:⋯value)sh -cwith arbitrary payload (*)ls -lwith any directory (⋯requires exactly one)echo hello *(zero or more trailing words)⋯over- and under-consumed,*mid-profile that fails to re-anchor on a literal.TestExecWithArgsInProfilestill passes — exact-match behaviorpreserved when no wildcards are present (literal vectors are a strict
subset of the wildcard grammar).
pkg/rulemanager/...andpkg/storage/...suites green; vet clean.Out of scope
tests/chart/templates/node-agent/default-rules.yamlrule R0001 ("Unexpectedprocess launched") currently uses path-only
ap.was_executed. Exercising thewildcard arg path end-to-end requires either updating R0001 to also call
ap.was_executed_with_argsor shipping a separate "Unexpected processarguments" rule. Both are larger semantic decisions — kept out of this PR so
the matcher wiring lands cleanly first. Component-test for the wildcard path
will follow once the rule shape is decided.
Test plan