Files
NetAlertX/server/plugins
Mauricio CamayoandClaude Sonnet 5 f564448617 fix: address CodeRabbit review findings on pihole_monitor plugin
Addresses 5 of the 6 actionable comments from CodeRabbit's review of
PR #1765 (netalertx/NetAlertX#1765), plus adds test coverage:

- fetch_top_blocked_clients() returns None on failure instead of {},
  so a failed request can no longer be mistaken for "genuinely zero
  blocked queries this run" and silently write a false 0 into a
  device's rolling history baseline. main() now tracks a
  stats_complete flag and skips anomaly evaluation + history
  persistence entirely for a run with incomplete blocked-query data.
- fetch_top_blocked_clients() is now called with count=max_clients
  (the existing PIHOLEMON_API_MAXCLIENTS setting) instead of a
  hardcoded default of 50, so clients beyond the top 50 are no longer
  silently dropped from anomaly detection.
- New build_ip_to_mac() derives the IP->MAC identity map from every
  gathered device entry instead of from merge_device_entries()'s
  by-MAC-deduplicated output, which only kept one IP per device and
  silently lost a multi-IP device's other IPs (misattributing their
  blocked-query traffic to a bare IP instead of the real MAC).
- PIHOLEMON_HISTORY_LENGTH is clamped to at least 1, so a negative
  setting can no longer reach the history[-history_length:] slice
  with a nonsensical negative-of-negative length.
- PIHOLEMON_VERIFY_SSL now defaults to true (was false, matching the
  official PIHOLEAPI plugin's convention). README documents the
  http:// vs https:// credentials trade-off explicitly rather than
  forcing https:// - most home Pi-hole setups, including the one this
  plugin targets, run over plain HTTP on a trusted LAN.
- Added test/plugins/test_pihole_monitor.py (37 tests, 99% line and
  branch coverage of pihole_monitor.py per pytest-cov - only the
  `if __name__ == '__main__':` entry-point guard is unreached):
  auth and deauth success/failure paths, the None-sentinel-on-failure
  contract, fetch_devices()'s own failure path, build_ip_to_mac()'s
  multi-IP fix, gather_device_entries()'s skip branches and fake-MAC
  fallback, netalertx_device_owner()'s success/failure/no-URL paths,
  and main()-level coverage for source aggregation, the
  stats_complete gate, the history_length boundary clamp, the
  CONSIDER_ONLINE fallback, an unconfigured-sources run, and the
  offline-device / invalid-MAC / unknown-IP / owner-lookup branches
  together in one run.

Not addressed: CodeRabbit's suggestion to hard-reject http:// URLs in
auth(). Diverges deliberately - it would break the plugin's majority
use case (Pi-hole admin API on a trusted home LAN without TLS), which
this repo's own PIHOLEAPI plugin also targets over plain HTTP.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CHJAArRiet4GmXUsxnNLdW
2026-08-30 11:03:15 -05:00
..