ROX-33034: Track xattr changes

[JoukoVirtanen]

Description

Adds tracking for setting and removing xattr. For now this only adds metrics for xattr events. In the future gRPC messages will be sent to sensor when these event occur. Sending gRPC messages to sensor will require changes to the stackrox/stackrox protobuf definitions and will be done later. The added integration tests rely on metrics. Once gRPC messages are sent the integration tests will not check the metrics and will instead check the gRPC messages.

A change was made to relax the success criteria of _wait_events used by the integration tests. Events of unexpected type are now ignored. This change was needed, because xattr changes are made by SELinux in many of the tests.

Protobuf changes are made at stackrox/stackrox#21148

Checklist

• Patch has a change log entry OR does not need one.
• Investigated and inspected CI test results
• Updated documentation accordingly

Automated testing

• Added unit tests
• Added integration tests
• Added regression tests

If any of these don't apply, please comment below.

Testing Performed

CI is sufficient

Summary by CodeRabbit

Release Notes

• New Features

  • Added monitoring and tracking of extended attribute (xattr) operations on files
  • Metrics collection for xattr set and remove operations

• Tests

  • Added comprehensive test suite validating xattr tracking via kernel metrics

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds end-to-end tracking of inode_setxattr and inode_removexattr LSM events. New eBPF ABI types, BPF programs, a Rust event model with XattrFileData, Prometheus counter wiring in KernelMetrics, and a skip condition in HostScanner are introduced together with pytest integration tests.

Changes

Xattr tracking feature

Layer / File(s)	Summary
eBPF ABI types and metrics counters fact-ebpf/src/bpf/types.h	Adds XATTR_NAME_MAX_LEN, FILE_ACTIVITY_SETXATTR/FILE_ACTIVITY_REMOVEXATTR enum values, an xattr payload variant in event_t, and inode_setxattr/inode_removexattr counter fields in metrics_t.
BPF event emission and LSM entrypoints fact-ebpf/src/bpf/events.h, fact-ebpf/src/bpf/main.c	Adds submit_xattr_event ringbuf helper, the handle_xattr shared logic (inode monitoring check, path clearing), and trace_inode_setxattr/trace_inode_removexattr BPF LSM programs.
Rust XattrFileData struct and FileData variants fact/src/event/mod.rs	Introduces XattrFileData, adds SetXattr/RemoveXattr to FileData, extends FileData::new() to parse xattr names from raw eBPF events, marks both variants unreachable in protobuf conversion, and adds test PartialEq.
Event accessors, is_xattr predicate, and host scanner skip fact/src/event/mod.rs, fact/src/host_scanner.rs	Adds is_xattr() and extends all Event accessor/mutator methods to handle xattr variants; skips xattr events in HostScanner's processing loop.
Metrics accumulation and Prometheus counter wiring fact-ebpf/src/lib.rs, fact/src/metrics/kernel_metrics.rs	Extends metrics_t::accumulate for xattr fields; adds inode_setxattr/inode_removexattr EventCounter fields to KernelMetrics, registers them with the Prometheus registry, and refreshes them in collect().
Pytest integration tests tests/test_xattr.py	Adds metric query helpers and five tests: single set/remove, multiple xattr set, ignored-directory isolation, and post-startup file creation with xattr tracking.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 Hop, hop through the kernel trace,
xattr names now leave their mark in place.
The ringbuf fills with set and remove,
Prometheus counts with every groove.
No secret attribute slips away—
the rabbit watches every day! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 61.54% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title 'Track xattr changes' clearly and concisely describes the main change: adding tracking for extended attribute operations.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description includes a detailed explanation of changes, a completed checklist with justified selections, and adequate testing rationale.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jv-ROX-33034-track-xattr-changes

---

_{Comment @coderabbitai help to get the list of available commands.}

[codecov-commenter]

Codecov Report

❌ Patch coverage is 0% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.23%. Comparing base (749dd31) to head (24c4c6a).

Files with missing lines	Patch %	Lines
fact/src/event/mod.rs	0.00%	41 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #790      +/-   ##
==========================================
- Coverage   32.72%   32.23%   -0.50%     
==========================================
  Files          21       21              
  Lines        2695     2736      +41     
  Branches     2695     2736      +41     
==========================================
  Hits          882      882              
- Misses       1810     1851      +41     
  Partials        3        3              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderabbitai]

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 `@fact-ebpf/src/bpf/main.c`:
- Around line 434-443: The trace_inode_removexattr function has an unreachable
return statement. After the return handle_xattr(...) call on line 441, there is
a duplicate return 0; statement that serves as dead code. Remove the duplicate
return 0; statement that appears after the handle_xattr return, keeping only the
return handle_xattr(...) line to ensure the function properly returns the result
of the xattr handler call.

🪄 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: Path: .coderabbit.yml

Review profile: CHILL

Plan: Enterprise

Run ID: 461c91e6-bfc7-42e7-97f6-99a4157926c8

📥 Commits

Reviewing files that changed from the base of the PR and between d4bf87c and 9a0b65e.

📒 Files selected for processing (8)

• fact-ebpf/src/bpf/events.h
• fact-ebpf/src/bpf/main.c
• fact-ebpf/src/bpf/types.h
• fact-ebpf/src/lib.rs
• fact/src/event/mod.rs
• fact/src/host_scanner.rs
• fact/src/metrics/kernel_metrics.rs
• tests/test_inode_xattr.py

[Molter73]

As discussed offline, we should make the protobuf changes for supporting xattr as part of these changes, so I expect some changes incoming.

There are also 2 small comments on the eBPF code.

[Molter73]

Overall looks good. There's a small comment on the operation field which shouldn't be there, that kind of information should be encoded in the type system.

The integration tests need a bit of work.

[Molter73]

Does this mean that now any test can just have random noise from xattr changes? Why is that? Are we certain it is random or is it that there already were xattr changes we were not seeing when the original tests were written and are now being emitted? Since the tests limit themselves to monitoring a single empty directory I find it very hard to believe events emitted are random.

In any case, we need to either:

• Check what the new events in tests are, whether they truly are random and, if they are not, add any missing events to the sequence of events on the existing tests.
• If the events truly are random, double check if we are not emitting events that we shouldn't be emitting and figure out a way to disable xattr events for tests to reduce noise.

As is, this check will probably lead to some hard to understand errors due to events being skipped when they shouldn't. strict mode means STRICT MODE, there should be no room for speculation when using this.

[JoukoVirtanen]

I have reverted the change to strict mode, but I have added the option to skip xattr events.

[JoukoVirtanen]

I will write a more complete response later.

[JoukoVirtanen]

These events are not random. It is dockerd adding the xattr security.selinux to monitored files and directories. Previously we did not see these events and now they are seen. Looking for these events may be flaky since it depends upon the environment. dockerd does not set this xattr when I run locally. I have a PR where all xattr events are expected and the tests pass in my local environment where they do not occur. The tests check if docker reports that SELinux is enabled and adjusts expected events accordingly. #864

Let me know if you prefer the method here or in the other PR.