add process-name exclude prefilter options#784
Conversation
📝 WalkthroughWalkthroughAdds parent-process context fields to exec events, extends extraction to populate them, updates prefilter to parse and match excludeProcesses/excludeParentProcesses maps (with path normalization and invalid-entry handling), changes ShouldSkip to accept event pointers and applies process/parent-process skip checks, and adds tests for these behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant Datasource
participant RuleManager
participant Prefilter
participant RuleEngine
Datasource->>RuleManager: deliver event (execve)
RuleManager->>RuleManager: extractEventFields -> populate Path, Comm, Pcomm, ParentExePath
RuleManager->>Prefilter: ShouldSkip(&EventFields)
Prefilter-->>Prefilter: normalize/lookup processKey in ExcludeProcesses / ExcludeParentProcesses
Prefilter-->>RuleManager: return skip decision
alt not skipped
RuleManager->>RuleEngine: ReportEnrichedEvent (process event)
else skipped
RuleManager-->>Datasource: drop/ignore event
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup EffectivenessNo data available. |
Signed-off-by: Yakir Oren <yakiroren@gmail.com>
Signed-off-by: Yakir Oren <yakiroren@gmail.com>
548d539 to
ea3b48a
Compare
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup EffectivenessNo data available. Event Counters
|
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup Effectiveness (AFTER only)
Event Counters
|
matthyx
left a comment
There was a problem hiding this comment.
One suggestion on the prefilter parsing: normalize operator-supplied paths to match what the event layer produces, so subtle config mismatches (trailing slash, missing leading slash) don't silently fail to match.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/rulemanager/prefilter/prefilter.go (1)
73-75: Refresh the staleEventFieldscomment.
ShouldSkipnow receives*EventFields, so “Passed by value” is no longer accurate.Suggested comment update
// EventFields holds event data extracted once per event for pre-filtering. -// Passed by value, stack-allocated, extracted once before the rule loop and -// reused across all rules. +// Extracted once before the rule loop and passed by pointer to avoid per-rule +// copies while being reused across all rules. type EventFields struct {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/rulemanager/prefilter/prefilter.go` around lines 73 - 75, The EventFields struct comment is stale: it says "Passed by value" but ShouldSkip now takes *EventFields; update the comment for EventFields to remove "Passed by value" and instead state that EventFields is extracted once per event and passed by pointer (e.g., to ShouldSkip) so it is reused across all rules and allocated on the stack or heap as appropriate; reference EventFields and the ShouldSkip method in the updated comment so readers know it is passed as *EventFields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/rulemanager/prefilter/prefilter.go`:
- Around line 73-75: The EventFields struct comment is stale: it says "Passed by
value" but ShouldSkip now takes *EventFields; update the comment for EventFields
to remove "Passed by value" and instead state that EventFields is extracted once
per event and passed by pointer (e.g., to ShouldSkip) so it is reused across all
rules and allocated on the stack or heap as appropriate; reference EventFields
and the ShouldSkip method in the updated comment so readers know it is passed as
*EventFields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 61eded93-330c-4f6b-860c-37d3185453df
📒 Files selected for processing (7)
pkg/rulemanager/extract_event_fields_test.gopkg/rulemanager/prefilter/prefilter.gopkg/rulemanager/prefilter/prefilter_test.gopkg/rulemanager/rule_manager.gopkg/utils/datasource_event.gopkg/utils/events.gopkg/utils/struct_event.go
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup Effectiveness (AFTER only)
Event Counters
|
Signed-off-by: Yakir Oren <yakiroren@gmail.com>
f9ff2a7 to
66b0715
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/rulemanager/prefilter/prefilter.go (1)
249-254:⚠️ Potential issue | 🟡 MinorAdd nil guard for the
eparameter inShouldSkip.Line 254 dereferences
ewithout a nil check. Although all current call sites pass valid addresses, the function should defend against nil to prevent panics if callers change or in untrusted contexts.func (p *Params) ShouldSkip(e *EventFields) bool { - if p == nil { + if p == nil || e == nil { return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/rulemanager/prefilter/prefilter.go` around lines 249 - 254, The ShouldSkip method on Params dereferences e.Dir without guarding e; add a nil-check for the EventFields parameter (e) at the start of Params.ShouldSkip (after the existing p == nil guard) and return false if e == nil so you do not dereference e when evaluating p.Dir != DirNone && e.Dir != DirNone && e.Dir != p.Dir; update any related comments if present.
🤖 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/prefilter/prefilter.go`:
- Around line 74-75: The comment about value-copy is stale because ShouldSkip
now accepts *EventFields; update the comment in prefilter.go to reflect that
EventFields is passed by pointer (e.g., "Passed by pointer, extracted once
before the rule loop and reused across all rules") and ensure it mentions
*EventFields and the reuse pattern around ShouldSkip to avoid implying
stack-allocated value copies.
---
Outside diff comments:
In `@pkg/rulemanager/prefilter/prefilter.go`:
- Around line 249-254: The ShouldSkip method on Params dereferences e.Dir
without guarding e; add a nil-check for the EventFields parameter (e) at the
start of Params.ShouldSkip (after the existing p == nil guard) and return false
if e == nil so you do not dereference e when evaluating p.Dir != DirNone &&
e.Dir != DirNone && e.Dir != p.Dir; update any related comments if present.
🪄 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: 496d4909-c9f2-4cf3-8212-7027fd137999
📒 Files selected for processing (2)
pkg/rulemanager/prefilter/prefilter.gopkg/rulemanager/prefilter/prefilter_test.go
✅ Files skipped from review due to trivial changes (1)
- pkg/rulemanager/prefilter/prefilter_test.go
| // Passed by value, stack-allocated, extracted once before the rule loop and | ||
| // reused across all rules. |
There was a problem hiding this comment.
Update the stale value-copy comment.
ShouldSkip now takes *EventFields, so this comment contradicts the current API.
Proposed comment update
-// Passed by value, stack-allocated, extracted once before the rule loop and
-// reused across all rules.
+// Passed by pointer on the prefilter hot path to avoid copying; extracted once
+// before the rule loop and reused across all rules.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Passed by value, stack-allocated, extracted once before the rule loop and | |
| // reused across all rules. | |
| // Passed by pointer on the prefilter hot path to avoid copying; extracted once | |
| // before the rule loop and reused across all rules. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/rulemanager/prefilter/prefilter.go` around lines 74 - 75, The comment
about value-copy is stale because ShouldSkip now accepts *EventFields; update
the comment in prefilter.go to reflect that EventFields is passed by pointer
(e.g., "Passed by pointer, extracted once before the rule loop and reused across
all rules") and ensure it mentions *EventFields and the reuse pattern around
ShouldSkip to avoid implying stack-allocated value copies.
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup Effectiveness (AFTER only)
Event Counters
|
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup Effectiveness (AFTER only)
Event Counters
|
Summary by CodeRabbit