[auditbeat][fim] Revert "Update Auditbeat file_integrity module to default to auto backend with intelligent fallback"#50166
Open
[auditbeat][fim] Revert "Update Auditbeat file_integrity module to default to auto backend with intelligent fallback"#50166
Conversation
…fault to…" This reverts commit f2a90c5.
marc-gr
added
Auditbeat
Team:Security-Windows Platform
botelastic
bot
added
the
needs_team
Contributor
|
Pinging @elastic/sec-windows-platform (Team:Security-Windows Platform) |
botelastic
bot
removed
the
needs_team
Contributor
🤖 GitHub commentsJust comment with:
|
Contributor
Contributor
Vale Linting ResultsSummary: 9 suggestions found 💡 Suggestions (9)
The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale. |
Contributor
📝 WalkthroughWalkthroughThe PR refactors the file_integrity module's backend selection mechanism from a dynamic auto-fallback system to explicit, OS-specific defaults. The Possibly related PRs
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@auditbeat/auditbeat.reference.yml`:
- Around line 102-106: The reference config hard-codes backend: fsnotify which
is rejected by Config.Validate on Windows; update the top-level backend entry
(the "backend" field in auditbeat.reference.yml) to be platform-neutral by
either removing the explicit backend line or setting it to "auto" (or generate
OS-specific variants) so Windows users copying the reference do not get a
validation error; ensure any documentation/x-pack copies are updated the same
way.
In `@auditbeat/module/file_integrity/config.go`:
- Around line 179-198: Validation currently only checks runtime.GOOS and allows
backends that won't be available on the built architecture and doesn't enforce
ETW rules for resolved-backend auto/unset; update the checks around c.Backend
(BackendETW, BackendAuto) and the flush_interval validation so you first resolve
what backend will actually be used given runtime.GOOS and runtime.GOARCH (treat
BackendAuto or empty as the platform default), then validate: 1) reject ebpf on
linux when runtime.GOARCH is not amd64/arm64, 2) reject etw on non-windows, 3)
ensure flush_interval >= 1s whenever the resolved backend is ETW (i.e. c.Backend
== BackendETW or resolved backend == ETW for BackendAuto/empty on Windows), and
append errors to errs accordingly using the existing symbols (c.Backend,
BackendETW, BackendAuto, c.FlushInterval).
🪄 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: CHILL
Plan: Pro Plus
Run ID: 7d97c923-b415-4bc9-a09e-65cc7aa05700
📒 Files selected for processing (20)
auditbeat/auditbeat.reference.ymlauditbeat/module/file_integrity/_meta/config.yml.tmplauditbeat/module/file_integrity/_meta/docs.mdauditbeat/module/file_integrity/config.goauditbeat/module/file_integrity/ebpfreader_supported.goauditbeat/module/file_integrity/eventreader.goauditbeat/module/file_integrity/eventreader_fsevents.goauditbeat/module/file_integrity/eventreader_fsnotify.goauditbeat/module/file_integrity/eventreader_kprobes.goauditbeat/module/file_integrity/eventreader_linux.goauditbeat/module/file_integrity/eventreader_other.goauditbeat/module/file_integrity/eventreader_test.goauditbeat/module/file_integrity/eventreader_windows.goauditbeat/module/file_integrity/eventreader_windows_test.goauditbeat/module/file_integrity/metricset_test.goauditbeat/tests/system/test_file_integrity.pychangelog/fragments/1730894400-auditbeat-fim-backend-auto-fallback.yamldocs/reference/auditbeat/auditbeat-module-file_integrity.mddocs/reference/auditbeat/auditbeat-reference-yml.mdx-pack/auditbeat/auditbeat.reference.yml
💤 Files with no reviewable changes (7)
- changelog/fragments/1730894400-auditbeat-fim-backend-auto-fallback.yaml
- auditbeat/tests/system/test_file_integrity.py
- auditbeat/module/file_integrity/eventreader_windows_test.go
- auditbeat/module/file_integrity/metricset_test.go
- auditbeat/module/file_integrity/eventreader_test.go
- auditbeat/module/file_integrity/eventreader.go
- auditbeat/module/file_integrity/eventreader_fsnotify.go
Comment on lines
102
to
+106
| # Select the backend which will be used to source events. | ||
| # "fsnotify" doesn't have the ability to associate user data to file events. | ||
| # Valid values: auto, fsnotify, kprobes, ebpf. | ||
| # When "auto" is used, the module will try backends in order of preference: | ||
| # - Linux: ebpf -> kprobes -> fsnotify | ||
| # - Windows: etw -> fsnotify | ||
| # Default: auto. | ||
| backend: auto | ||
| # Default: fsnotify. | ||
| backend: fsnotify |
Contributor
There was a problem hiding this comment.
This reference config is now invalid on Windows.
Config.Validate rejects fsnotify on Windows, but this top-level reference file hard-codes backend: fsnotify unconditionally. A Windows user copying this block from the shipped reference config will get a config error immediately. Keep this reference platform-neutral (auto/omitted) or generate OS-specific variants; the docs and x-pack copies need the same correction.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@auditbeat/auditbeat.reference.yml` around lines 102 - 106, The reference
config hard-codes backend: fsnotify which is rejected by Config.Validate on
Windows; update the top-level backend entry (the "backend" field in
auditbeat.reference.yml) to be platform-neutral by either removing the explicit
backend line or setting it to "auto" (or generate OS-specific variants) so
Windows users copying the reference do not get a validation error; ensure any
documentation/x-pack copies are updated the same way.
Comment on lines
+179
to
198
| if c.Backend != "" { | ||
| switch runtime.GOOS { | ||
| case "linux": | ||
| if c.Backend == BackendETW { | ||
| errs = append(errs, errors.New("backend etw is not supported on linux")) | ||
| } | ||
| case "windows": | ||
| if c.Backend != BackendETW && c.Backend != BackendAuto { | ||
| errs = append(errs, errors.New("windows only supports etw or auto backend")) | ||
| } | ||
| default: | ||
| if c.Backend != BackendAuto { | ||
| errs = append(errs, errors.New("backend can only be specified on linux or windows")) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Validate flush_interval for ETW backend | ||
| if runtime.GOOS == "windows" && (c.Backend == BackendETW || c.Backend == "auto" || c.Backend == "") && c.FlushInterval < time.Second { | ||
| if c.Backend == BackendETW && c.FlushInterval < time.Second { | ||
| errs = append(errs, fmt.Errorf("flush_interval must be at least 1 second, got %v", c.FlushInterval)) |
Contributor
There was a problem hiding this comment.
Backend validation regressed for real runtime support.
This now validates by GOOS only, so Linux accepts backend: ebpf even on non-amd64/arm64 builds where the eBPF reader is not compiled, and Windows no longer enforces the ETW flush_interval >= 1s rule for backend: auto/unset even though those values still resolve to ETW. That turns invalid configs into late startup failures instead of rejecting them up front.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@auditbeat/module/file_integrity/config.go` around lines 179 - 198, Validation
currently only checks runtime.GOOS and allows backends that won't be available
on the built architecture and doesn't enforce ETW rules for resolved-backend
auto/unset; update the checks around c.Backend (BackendETW, BackendAuto) and the
flush_interval validation so you first resolve what backend will actually be
used given runtime.GOOS and runtime.GOARCH (treat BackendAuto or empty as the
platform default), then validate: 1) reject ebpf on linux when runtime.GOARCH is
not amd64/arm64, 2) reject etw on non-windows, 3) ensure flush_interval >= 1s
whenever the resolved backend is ETW (i.e. c.Backend == BackendETW or resolved
backend == ETW for BackendAuto/empty on Windows), and append errors to errs
accordingly using the existing symbols (c.Backend, BackendETW, BackendAuto,
c.FlushInterval).
This comment has been minimized.
This comment has been minimized.
Contributor
TL;DR
Remediation
Investigation detailsRoot CauseThe failing step is Compared with the previous detective report, the failure is not materially the same anymore: this run includes additional Evidence
Validation
Follow-up
What is this? | From workflow: PR Actions Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
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
2 participants
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.
Reverts #47498
After some consideration we decided to revert this even after going through the breaking change committee as we need to figure out what the real impact it might have on current users.