Update Falco with event parsing fix - #3681
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe ChangesFalco libraries update
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@collector/lib/system-inspector/Service.cpp`:
- Around line 130-136: Update the sinsp_exception handling in GetNext() so only
genuinely recoverable parse failures return nullptr for Run() to retry;
propagate fatal or invalid-state exceptions or reinitialize the capture instead.
Replace the unthrottled CLOG warning with CLOG_THROTTLED for repeated failures,
preserving the existing successful-event path.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 50f110a2-060a-44bb-857d-5889c44504de
📒 Files selected for processing (2)
collector/lib/system-inspector/Service.cppfalcosecurity-libs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3681 +/- ##
=======================================
Coverage 27.43% 27.43%
=======================================
Files 94 94
Lines 5406 5406
Branches 2535 2535
=======================================
Hits 1483 1483
Misses 3198 3198
Partials 725 725
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
7219d7e to
701263d
Compare
Stringy
changed the title
Updates the falcosecurity-libs submodule to include a fix for mismatched event parameter lengths that caused collector to crash via sinsp_exception -> std::unexpected() -> abort(). The string extraction templates now tolerate parameter lengths that don't match the null-terminated string length, which has been observed on some kernels for both string and integer parameters.
701263d to
24427d1
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Update submodule to include enhanced diagnostic logging in throw_invalid_len_error(). This reverts the param_len-1 workaround so the crash still occurs on Fedora CoreOS CI, but now with full raw event dumps (header fields, length array, data region) to identify the root cause of the parameter corruption.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@falcosecurity-libs`:
- Line 1: Use a revision that includes the lenient filename extraction fix and
passes the Fedora CoreOS x86 reproducer; do not use revision
713d313c67b9ce66690f39e31b9ec2e936eb0ccd, which only restores throwing behavior
and diagnostics. Ensure diagnostic handling bounds all raw event access by
raw->len, even when raw->nparams is corrupted.
🪄 Autofix
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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 0c5f8578-f7ec-4852-964f-3c00ae7adde8
📒 Files selected for processing (1)
falcosecurity-libs
2 participants
Description
Updates the falcosecurity-libs submodule to include the lenient filename parameter extraction fix.
stackrox/falcosecurity-libs#98
Checklist
Automated testing
If any of these don't apply, please comment below.
Testing Performed
CI shuold be enough. Fedora CoreOS x86 was crashing, so this is the priority.