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
Does two jobs against the same Pi-hole connection(s), instead of two
separately configured plugins:
1. Device import - same job as the official PIHOLEAPI (pihole_api_scan)
plugin, but supports an optional secondary/failover Pi-hole natively
(accepts two sets of credentials instead of forking the official
plugin, which hardcodes its settings-key prefix and doesn't support
multiple instances).
2. Query anomaly detection - flags a device whose blocked-query count
spikes well above its own recent rolling average (signature of
malware/a compromised device beaconing out), keyed by MAC address
(not IP, which changes under DHCP) and combined across both Pi-hole
instances so a compromised device can't evade detection by switching
resolvers.
Notifications are delegated entirely to NetAlertX's own Watched/Report
on mechanism - the plugin never calls a notification service directly.
Live-tested against a two-Pi-hole home setup (v26.8.5) for several days,
including two real bugs found and fixed during that testing (an
offline-filtered device losing its MAC and falling back to a bare-IP
identifier, and a boolean-expression flake8 style fix).
The whitespace note contradicted itself: it said surrounding whitespace is
trimmed and then that leading or trailing whitespace is invalid. The parser
strips, so trimming is what actually happens.
Also lists the new non-ASCII/newline skip reason and states that skipping is
per entry, and adds the language identifier markdownlint wants on the two
fenced blocks (MD040).
A custom header carrying a non-ASCII character raised UnicodeEncodeError from
inside http.client. That is a ValueError, not a RequestException, so it escaped
both handlers in send() and took the whole publisher down - every notification
lost because of one typo in one header.
build_custom_headers now rejects newlines and non-ASCII the same way it already
rejects malformed and colliding entries: warn, skip that entry, keep the rest.
UnicodeEncodeError is still caught at the request, as a backstop for the
plugin's own headers, since REPORT_DASHBOARD_URL feeds one of them.
Verified against real requests: before, send() raised UnicodeEncodeError; after,
the bad header is dropped and the notification is still posted.
The custom header added in #1695 was a single name/value pair, which is
enough for a proxy that authenticates with one token but not for Pangolin,
which expects both P-Access-Token-Id and P-Access-Token.
NTFY_CUSTOMHEADER_NAME and NTFY_CUSTOMHEADER_VALUE are replaced by a single
list setting, NTFY_CUSTOM_HEADERS, holding one "Name: Value" entry per
header. The list widget is the same one the other list settings use.
Only the first colon separates the name from the value, so values may
contain colons. An entry is skipped and logged when it is malformed, when
the name repeats, or when it collides with a header the plugin already set,
so a custom header still cannot clobber the ntfy credentials.
Values are never written to the log, since they are usually secrets. That
also applies to the invalid-header error, which now names the headers that
were applied without quoting any of them.
Adds README examples for the custom header and URL query string settings,
and handles requests' InvalidHeader separately: its message embeds the
offending header value, so logging it leaked NTFY_CUSTOMHEADER_VALUE into
the plugin result file and the UI.
Lets users authenticate ntfy notifications through a reverse proxy or tunnel
(Pangolin, Tailscale, ...) in front of the ntfy instance. Adds three optional,
backward-compatible settings that default to empty and are no-ops when unset:
- NTFY_URL_QUERY_STRING: appended to the request URL (e.g. p_token=...). A
leading '?' is tolerated, and the value is redacted from error logs / the
plugin result file since the request URL can carry a secret token.
- NTFY_CUSTOMHEADER_NAME / NTFY_CUSTOMHEADER_VALUE: a custom request header,
skipped with a warning if it would clobber a built-in header (e.g.
Authorization) so ntfy's own auth stays intact.
Secret-bearing fields are password-masked in the UI. Addresses #1663.
- Introduced a new plugin for monitoring website health, including functionality to check URLs and log results.
- Created README and configuration files for the workflows plugin, detailing its purpose and settings.
- Updated import paths in various test files to reflect the new directory structure.
- Ensured compatibility of test cases with the updated plugin architecture.
jokob-sk