Repository navigation
fix(rulemanager): enforce required profiles for host events - #1031
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: enforcing required profiles for host events. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
e2526eb to
01ae737
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The focused fix is consistent with profile dependency semantics and has comprehensive regression coverage.
0 open findings
What changed in this PR
Ensures profile-required rules suppress host and standalone events when profiles are unavailable.
Changes:
- Applies the existing required-profile gate to every event context.
- Adds a 24-case regression matrix covering contexts, dependencies, and profile availability.
| File | Description |
|---|---|
pkg/rulemanager/rule_manager.go |
Removes the Kubernetes-only profile-gate restriction. |
pkg/rulemanager/rule_manager_profile_test.go |
Verifies evaluation, export, and suppression behavior. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup EffectivenessNo data available. |
Main gained tests after this branch was cut that predate its interface changes: the profile-gate test (#1031) stubs CreateRuleFailure with the old arity and does not implement EvaluateStringExpressionWithContext, so message rendering hit the nil embedded evaluator; the network creator test builds typesv1.Rule with a depth-0 ProfileDependency, which now lives on the embedded RuntimeRule. Also drops a duplicate GetProcessBootTimeNs on the ancestry test stub. Docs-exempt: test-only changes Signed-off-by: Ben <ben@armosec.io>
Main gained tests after this branch was cut that predate its interface changes: the profile-gate test (#1031) stubs CreateRuleFailure with the old arity and does not implement EvaluateStringExpressionWithContext, so message rendering hit the nil embedded evaluator; the network creator test builds typesv1.Rule with a depth-0 ProfileDependency, which now lives on the embedded RuntimeRule. Also drops a duplicate GetProcessBootTimeNs on the ancestry test stub. Docs-exempt: test-only changes Signed-off-by: Ben <ben@armosec.io>
Host and standalone events currently bypass
profileDependency: RequiredinReportEnrichedEvent. A host IMDS request can therefore trigger R1059 without a profile: the missing-profile lookup returns false and the rule negates it into a match.Enforce the existing profile gate for every event context before CEL evaluation and alert export. Optional and NotRequired behavior, context selection, and suppression metrics remain unchanged. Remove the comments describing the old Kubernetes-only exception.
Regression coverage
A 24-case matrix exercises host, standalone container, Kubernetes, and legacy nil-context events against all three profile dependencies with missing and available profiles. It asserts evaluation, export, and
profile_incompletesuppression. Before the fix, the host and standalone Required/missing cases failed because they evaluated and exported an alert.Validation
go vet, scoped golangci-lint (0 issues), and upstreammake binarypassed.make nodeand host rule manager/exporter tests passed. No replace directive is committed.Rollout
Private node-agent consumes this package as a dependency and needs a compatible upstream release/backport containing this fix. A blind bump to current upstream main fails against the consumer’s existing storage implementation (
ListContainerProfilesis newly required). This PR fixes the shared source; it does not change deployment configuration.Required detections stay suppressed while a profile is unavailable, for host events as well as container events, and retain the
profile_incompletesuppression metric.Independent code review: APPROVE, zero findings. Independent architecture review: CLEAR, including a passing matrix rerun and confirmation that the cache supports host profiles.
Summary by CodeRabbit