fix(inventory): preserve results and collect DNS settings - #154
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the inventory subsystem so it can (a) preserve partial results when collectors time out and (b) collect additional safely-normalized configuration and network data (notably AdGuard Home config and richer UniFi site/network/DHCP details). It also tightens operational bounds (SSH output caps, remote frame parsing) and updates workflow-shape tests to reflect a hardened reusable release workflow contract.
Changes:
- Add AdGuard Home config collection (local + remote), storing redacted artifacts while exposing safe normalized fields under
services[].details. - Rework UniFi inventory collection to fetch sites, devices, modern/legacy networks + network details with request concurrency limiting, deadline-aware preservation, and normalized DHCP/DNS fields under
networks[].details. - Cap/structure remote collection output (bounded SSH output, framed record parsing with malformed-frame warnings) and update workflow permission contract tests.
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/workflow_shapes.rs | Expands workflow permission assertions and validates delegation to the reusable container release workflow. |
| src/runtime/inventory_refresh.rs | Adds AdGuard paths to watched config targets to trigger refreshes on changes. |
| src/runtime/inventory_refresh_tests.rs | Updates watcher target tests to include AdGuard paths. |
| src/mcp/tools_tests.rs | Updates inventory fixtures for new InventoryService.details field. |
| src/inventory/unifi.rs | Refactors UniFi collection into deadline-aware, concurrent request pipeline with safe path encoding and detail fetching. |
| src/inventory/unifi_tests.rs | Adds coverage for modern network details, deadline behavior, path encoding, and normalization edge cases. |
| src/inventory/unifi_normalize.rs | New UniFi normalization module for sites/devices/networks with DHCP/DNS extraction and merge semantics. |
| src/inventory/ssh.rs | Introduces capped SSH command execution to bound stdout/stderr memory usage. |
| src/inventory/ssh_tests.rs | Updates SSH runner test harness for new capped runner signature. |
| src/inventory/schema.rs | Adds details to InventoryService and NetworkSegment for safe normalized provider/config data. |
| src/inventory/remote_docker.rs | Updates service/network construction for new details field defaults. |
| src/inventory/remote_configs.rs | Reworks remote config sweep into a single framed batch with typed records, bounded output, and collector deadline preservation. |
| src/inventory/remote_configs_tests.rs | Updates record parsing tests, adds malformed-frame warnings test, and adds deadline preservation coverage. |
| src/inventory/redaction.rs | Expands URL query redaction to include generic key= parameters. |
| src/inventory/redaction_tests.rs | Adds regression test for redacting key= without impacting safe query params. |
| src/inventory/raw_configs.rs | Adds AdGuard local collection, Nginx variable substitution, and local collection deadline preservation. |
| src/inventory/raw_configs_tests.rs | Adds tests for SWAG/Nginx set $var substitution and exact variable-name resolution. |
| src/inventory/raw_configs_nginx.rs | New bounded-pass exact variable substitution helper for Nginx-style $var / ${var} tokens. |
| src/inventory/process.rs | Adds run_command_capped and threads max-output bounds through command execution. |
| src/inventory/process_tests.rs | Strengthens timeout test to avoid flaky sleep behavior by using a non-terminating loop with retries. |
| src/inventory/orchestrator.rs | Plumbs new AdGuard paths and splits probe vs collector timeouts for raw config collection. |
| src/inventory/orchestrator_tests.rs | Updates orchestrator fixtures for new AdGuard paths config field. |
| src/inventory/limits.rs | Adds MAX_RAW_BATCH_OUTPUT_BYTES to bound remote batched config collection output. |
| src/inventory/docker.rs | Updates docker network segments to initialize new details field. |
| src/inventory/config.rs | Adds CORTEX_INVENTORY_ADGUARD_PATHS with sensible defaults for common AdGuard Home config locations. |
| src/inventory/cache_tests.rs | Updates cache-related fixtures for new AdGuard paths field. |
| src/inventory/adguard.rs | New AdGuard Home config collector: redacted artifact storage + safe normalized InventoryService.details + port inference. |
| src/inventory/adguard_tests.rs | Validates safe-section normalization, port extraction, and redaction behavior for AdGuard configs. |
| src/inventory.rs | Exposes new adguard module. |
| src/db/graph_inventory_tests.rs | Updates graph projection fixtures for new details on services/networks. |
| src/db/graph_findings_tests.rs | Updates findings fixtures for new details on services. |
| README.md | Updates inventory capabilities to include AdGuard and remote collection. |
| docs/mcp/TOOLS.md | Documents safe normalized provider/config fields under services[].details / networks[].details. |
| docs/INVENTORY.md | Documents AdGuard config path env var and expanded remote inventory coverage + timeout preservation semantics. |
| Cargo.toml | Adds serde_yaml_ng dependency for AdGuard YAML parsing. |
| Cargo.lock | Locks new YAML dependency and transitive crates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Summary
Findings addressed during PR review
networkrecordkey=URL query parameterspermissions: {}as explicit deny-all in workflow security contractsVerification
cargo test --all-targets: 2,702 passed, 0 failed, 2 ignoredcargo check --all-targets: passed against RMCP 3.1.0cargo clippy --all-targets --all-features -- -D warnings: passedcargo audit: passedOperational impact