ROX-33034: Track xattr changes by JoukoVirtanen · Pull Request #790 · stackrox/fact

JoukoVirtanen · 2026-06-14T17:56:44Z

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.

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

…vant code

coderabbitai · 2026-06-14T17:56:57Z

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 covers the main purpose and includes a complete checklist, but lacks detailed testing information beyond 'CI is sufficient'.

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

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 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 and usage tips.}

codecov-commenter · 2026-06-14T17:59:53Z

Codecov Report

❌ Patch coverage is 0% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.52%. Comparing base (d4bf87c) to head (7b389c6).

Files with missing lines	Patch %	Lines
fact/src/event/mod.rs	0.00%	30 Missing ⚠️
fact/src/metrics/kernel_metrics.rs	0.00%	10 Missing ⚠️
fact-ebpf/src/lib.rs	0.00%	2 Missing ⚠️
fact/src/host_scanner.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #790      +/-   ##
==========================================
- Coverage   27.96%   27.52%   -0.45%     
==========================================
  Files          21       21              
  Lines        2596     2638      +42     
  Branches     2596     2638      +42     
==========================================
  Hits          726      726              
- Misses       1867     1909      +42     
  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 · 2026-06-15T14:43:43Z

+  // inode hooks don't provide a struct path, so filename is left empty.
+  // __submit_event requires a valid pointer for bpf_probe_read_str.
+  struct bound_path_t* bound_path = get_bound_path(BOUND_PATH_MAIN);
+  if (bound_path == NULL) {
+    args.metrics->error++;
+    return 0;
+  }
+  bound_path->path[0] = '\0';
+  args.filename = bound_path->path;


Would it be too complicated to change the behavior of __submit_event to check whether the path is valid?

Molter73 · 2026-06-15T14:45:08Z


 #define LINEAGE_MAX 2

+// Matches Linux kernel XATTR_NAME_MAX (255) + null terminator


Can you get a github or elixir.bootlin to where this definition is?

JoukoVirtanen added 7 commits June 12, 2026 15:45

X-Smart-Branch-Parent: main

f27ef4d

Added tests

0db0d9d

Added tracking for xattr changes

1aa7ed0

Combined calls to BPF_CORE_READ

1a2bbdd

Clarified comment that the path is not available. Also clarified rele…

ceb5727

…vant code

Added helper to reduce dry

0d13bc1

Added events.h which had been forgotten

9a0b65e

JoukoVirtanen requested a review from a team as a code owner June 14, 2026 17:56

coderabbitai Bot reviewed Jun 14, 2026

View reviewed changes

Comment thread fact-ebpf/src/bpf/main.c

JoukoVirtanen added 4 commits June 14, 2026 11:06

Fix format

b286346

Removed unreachable return

bd38c3d

Fixed format again

7743c69

Renamed test_inode_xattr.py to test_xattr.py

7b389c6

JoukoVirtanen requested a review from Molter73 June 14, 2026 21:18

Molter73 reviewed Jun 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ROX-33034: Track xattr changes#790

ROX-33034: Track xattr changes#790
JoukoVirtanen wants to merge 11 commits into
mainfrom
jv-ROX-33034-track-xattr-changes

JoukoVirtanen commented Jun 14, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jun 14, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

codecov-commenter commented Jun 14, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Molter73 left a comment

Uh oh!

Molter73 Jun 15, 2026

Uh oh!

Molter73 Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		#define LINEAGE_MAX 2

		// Matches Linux kernel XATTR_NAME_MAX (255) + null terminator

Conversation

JoukoVirtanen commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Testing Performed

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

codecov-commenter commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Molter73 left a comment

Choose a reason for hiding this comment

Uh oh!

Molter73 Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

Molter73 Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JoukoVirtanen commented Jun 14, 2026 •

edited

Loading

coderabbitai Bot commented Jun 14, 2026 •

edited

Loading

codecov-commenter commented Jun 14, 2026 •

edited

Loading