test(sensor): prove Wave A collectors against a real live Linux host - #1857
Conversation
Adds bins/aegis-node-sensor/tests/real_host_integration.rs, closing the "not yet proven against a real live host" caveat named in docs/current-vs-roadmap.md and docs/Implementation_Status.md for the node sensor's process/net/fs/secret collectors -- previously every test only exercised the *_in(root: &Path) helpers against a synthetic tempdir /proc tree, never the real production entrypoints against the actual /proc. Five tests, verified against a real rust:1.96-bookworm Linux container (not just compiled -- actually run): - process_collector_discovers_a_real_child_via_the_actual_proc_filesystem - process_collector_poll_registers_with_enforcer_and_can_kill_a_real_child (real SIGTERM/SIGKILL against a real live process) - net_collector_reports_a_real_established_tcp_connection (real TCP socket via bash's /dev/tcp) - fs_collector_reports_a_real_open_file_descriptor - secret_collector_reports_only_the_env_name_never_the_value_for_a_real_child (asserts the secret value never appears in the emitted event) Honestly scoped in the docs: this proves correctness on a real Linux VM (CI's ubuntu-latest runner), not a long-running production host under sustained load -- that soak testing remains a separate follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new integration test suite (real_host_integration.rs) to validate the Aegis node sensor collectors against a real Linux host instead of synthetic /proc fixtures, and updates the implementation status documentation accordingly. The feedback recommends enhancing test robustness by introducing a KillOnDrop guard to prevent orphaned child processes on test failures, and replacing hardcoded sleeps with polling retry loops to avoid flaky tests in busy CI environments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| fn spawn_sleep_with_run_id(run_id: &str) -> Child { | ||
| Command::new("sleep") | ||
| .arg("20") | ||
| .env("AEGIS_RUN_ID", run_id) | ||
| .stdout(Stdio::null()) | ||
| .stderr(Stdio::null()) | ||
| .spawn() | ||
| .expect("spawn real sleep child") | ||
| } |
There was a problem hiding this comment.
Spawning child processes in tests without a drop guard can lead to orphaned background processes if an assertion panics before child.kill() is reached. Introducing a simple KillOnDrop helper ensures that child processes are always cleaned up, even during test failures.
struct KillOnDrop(Child);
impl std::ops::Deref for KillOnDrop {
type Target = Child;
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl std::ops::DerefMut for KillOnDrop {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}
impl Drop for KillOnDrop {
fn drop(&mut self) {
let _ = self.0.kill();
let _ = self.0.wait();
}
}
fn spawn_sleep_with_run_id(run_id: &str) -> KillOnDrop {
KillOnDrop(
Command::new("sleep")
.arg("20")
.env("AEGIS_RUN_ID", run_id)
.stdout(Stdio::null())
.stderr(Stdio::null())
.spawn()
.expect("spawn real sleep child"),
)
}| // Give the connection a moment to reach ESTABLISHED in /proc/net/tcp. | ||
| std::thread::sleep(Duration::from_millis(500)); | ||
|
|
||
| let collector = NetCollector::new(); | ||
| let spool_dir = tempfile::tempdir().unwrap(); | ||
| let spool = SpoolQueue::open(spool_dir.path(), 1_000_000).unwrap(); | ||
| collector.poll(&spool); | ||
|
|
||
| let rec = spool | ||
| .read_next(Lane::Normal) | ||
| .unwrap() | ||
| .expect("network_connection event spooled for the real socket"); | ||
| let payload: RuntimeEventPayload = serde_json::from_slice(&rec.payload).unwrap(); |
There was a problem hiding this comment.
Using a hardcoded sleep of 500ms to wait for the connection to reach ESTABLISHED can lead to flaky tests in busy CI environments. Since bash executes asynchronously, it is much more robust to poll the collector in a retry loop with a timeout.
let collector = NetCollector::new();
let spool_dir = tempfile::tempdir().unwrap();
let spool = SpoolQueue::open(spool_dir.path(), 1_000_000).unwrap();
let mut payload = None;
for _ in 0..50 {
collector.poll(&spool);
if let Ok(Some(rec)) = spool.read_next(Lane::Normal) {
let p: RuntimeEventPayload = serde_json::from_slice(&rec.payload).unwrap();
if p.event_type == "network_connection" {
payload = Some(p);
break;
}
}
std::thread::sleep(Duration::from_millis(100));
}
let payload = payload.expect("network_connection event spooled for the real socket");| std::thread::sleep(Duration::from_millis(300)); | ||
|
|
||
| let collector = FsCollector::new(); | ||
| let spool_dir = tempfile::tempdir().unwrap(); | ||
| let spool = SpoolQueue::open(spool_dir.path(), 1_000_000).unwrap(); | ||
| collector.poll(&spool); | ||
|
|
||
| let rec = spool | ||
| .read_next(Lane::Normal) | ||
| .unwrap() | ||
| .expect("fs_open event spooled for the real file descriptor"); | ||
| let payload: RuntimeEventPayload = serde_json::from_slice(&rec.payload).unwrap(); |
There was a problem hiding this comment.
Using a hardcoded sleep of 300ms to wait for the file descriptor to be opened can lead to flaky tests in busy CI environments. Since bash executes asynchronously, it is much more robust to poll the collector in a retry loop with a timeout.
| std::thread::sleep(Duration::from_millis(300)); | |
| let collector = FsCollector::new(); | |
| let spool_dir = tempfile::tempdir().unwrap(); | |
| let spool = SpoolQueue::open(spool_dir.path(), 1_000_000).unwrap(); | |
| collector.poll(&spool); | |
| let rec = spool | |
| .read_next(Lane::Normal) | |
| .unwrap() | |
| .expect("fs_open event spooled for the real file descriptor"); | |
| let payload: RuntimeEventPayload = serde_json::from_slice(&rec.payload).unwrap(); | |
| let collector = FsCollector::new(); | |
| let spool_dir = tempfile::tempdir().unwrap(); | |
| let spool = SpoolQueue::open(spool_dir.path(), 1_000_000).unwrap(); | |
| let mut payload = None; | |
| for _ in 0..50 { | |
| collector.poll(&spool); | |
| if let Ok(Some(rec)) = spool.read_next(Lane::Normal) { | |
| let p: RuntimeEventPayload = serde_json::from_slice(&rec.payload).unwrap(); | |
| if p.event_type == "fs_open" { | |
| payload = Some(p); | |
| break; | |
| } | |
| } | |
| std::thread::sleep(Duration::from_millis(100)); | |
| } | |
| let payload = payload.expect("fs_open event spooled for the real file descriptor"); |
Summary
Closes the "not yet proven against a real live host (tests use synthetic
/proctrees)" caveat named for the node sensor indocs/current-vs-roadmap.md/docs/Implementation_Status.md(P0 roadmap item).Every existing test for
process_collector.rs/net_collector.rs/fs_collector.rs/secret_collector.rsonly exercised the*_in(root: &Path)helper functions against a synthetic tempdir standing in for/proc— the actual production entrypoints (scan_host_aegis_processes,NetCollector::poll,FsCollector::poll,SecretCollector::poll, all of which hit the real/procon Linux) were never called by any test.New
bins/aegis-node-sensor/tests/real_host_integration.rs(Linux-only,#[cfg(target_os = "linux")]) closes that gap with 5 tests against real host state:process_collector_discovers_a_real_child_via_the_actual_proc_filesystem— spawns a realsleepchild withAEGIS_RUN_IDset, discovers it via real/proc/<pid>/environ.process_collector_poll_registers_with_enforcer_and_can_kill_a_real_child— full pipeline: real discovery →ProcessEnforcerregistration → real SIGTERM/SIGKILL against a real live process.net_collector_reports_a_real_established_tcp_connection— a real child opens a real TCP socket (via bash's/dev/tcp) to a listener bound in the test process; asserts the collector reports the real remoteip:portfrom/proc/net/tcp.fs_collector_reports_a_real_open_file_descriptor— a real child holds a real file open; asserts the collector reports the real path from/proc/<pid>/fd.secret_collector_reports_only_the_env_name_never_the_value_for_a_real_child— a real child carries a secret-shaped env var; asserts the emitted event names the variable but the value never appears anywhere in it.Honest scope: this proves correctness on a real Linux VM — CI's
ubuntu-latestrunner, and locally verified against a realrust:1.96-bookwormDocker container — not a long-running production host under sustained load. That soak-testing gap is called out explicitly in the test file's doc comment and in the updated docs, not glossed over.Test plan
rust:1.96-bookwormLinux container (not just macOS compile-check, since the tests arecfg(target_os = "linux")-gated): all 5 tests pass.cargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warningscargo check -p aegis-node-sensor --testsGateway (stable/beta/1.88)jobs (ubuntu-latest, real Linux) will run this file as part ofcargo test --workspace🤖 Generated with Claude Code