check_trusted_aps() evaluated a rogue AP once per trusted_aps entry sharing
its SSID, so the documented main-AP+extender pattern (same SSID, two
entries) produced duplicate (bssid, motor) rows for a real clone - a
problem once next_release's per-plugin identity-hash dedup guard lands in
main, since it drops a plugin's entire batch on any internal duplicate.
Fixed by deduping once in main() after collecting from all check_*
functions. Also added iw + its setcap to Dockerfile.debian and
.devcontainer/Dockerfile, which the original PR missed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011meLPKCzVpdZyAUfv5U6mm
- check_trusted_aps() was evaluating every AP sharing a protected SSID
against every trusted entry for that SSID, not just its own. A main AP
requiring a stricter accepted security set (e.g. wpa3-only) than a
separately-trusted extender (e.g. wpa2) caused the extender to be
flagged as evil_twin/absent_baseline_clone - it was being judged
against the main AP's accepted set instead of its own. Fixed by
excluding, from each trusted entry's evaluation, any BSSID that has its
own separate trusted entry for the same SSID.
- README's "iw isn't in the published image yet" section was already
stale within the same PR - this branch's own Dockerfile change adds
iw + setcap, so the image ships it. Replaced with one sentence.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Periodic iw-scan-based detection of the 6 heuristics that don't need
monitor-mode hardware (see issue #1789): pwnagotchi/Pineapple signatures,
evil-twin/open clones, baseline-AP-absent-with-clone, security downgrades,
and duplicate-SSID/different-vendor - all evaluated against a user-curated
trusted-AP baseline (WIFICANARY_trusted_aps). A detection creates a
flagged Devices entry even for BSSIDs that never associate, per the
addendum on the same issue.
- WIFICANARY_TRUSTED_SECURITY is multi-select: an observed encryption
exactly matching any selected value is accepted; otherwise it's flagged
if weaker than the strongest selected value (deliberate - comparing
against the weakest would make multi-select pointless, since anything
at/above the weakest would silently pass regardless of the rest of the
selection).
- Added a "known device turned rogue" motor: escalate_known_devices()
cross-references each detection's BSSID against the Devices table via
the new DeviceInstance.getAllByMacs(). This covers the BSSID-identity
half of the issue #1789 addendum's motor 10; the deauth/probe-source-MAC
half still needs monitor-mode data this plugin doesn't have.
- Vendor is deliberately not looked up by this plugin - any device it
creates gets devVendor filled in for free by core's own vendor_update
plugin on its next pass.
43 wificanary unit tests + 10 DeviceInstance.getAllByMacs() tests, all
test_plugin_conventions.py checks pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011meLPKCzVpdZyAUfv5U6mm
Settings-UI description trimmed to one short line - implementation
detail (Socket Proxy, column mapping, CREATE_DEV behavior) already
lives in README, doesn't belong in the Settings page string.
scanSourcePlugin now maps to a static "DOCKERDISC" value (same
Dummy-column pattern arp_scan already uses), so a container device
created by this plugin gets devSourcePlugin set correctly instead of
NULL - every other CurrentScan-mapped plugin already does this.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011meLPKCzVpdZyAUfv5U6mm
Declares DOCKERDISC_IMPORT_ON (default on) so an operator can fully opt
this plugin out of CurrentScan promotion. Needed because
DOCKERDISC_CREATE_DEV alone doesn't cover it: a macvlan/ipvlan
container's row always carries a real scanMac, so even with
CREATE_DEV off, an already-existing device for that MAC (found
independently by ARP/Nmap) still gets its presence/devLastIP/
devParentMAC updated by this plugin on every run - only IMPORT_ON can
turn that off. The two settings are independent, per jokob-sk's PR
feedback - IMPORT_ON gates promotion for the whole run, CREATE_DEV
gates device creation per row.
Also adds missing docstrings to process_host()/main() (CodeRabbit
docstring-coverage check), matching the style already used elsewhere
in this file.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011meLPKCzVpdZyAUfv5U6mm
Maps DOCKERDISC to CurrentScan (scanMac/scanCreatesDevice/scanParentMAC/
scanLastIP) so a container on a macvlan/ipvlan network can opt into
creating or confirming its own device, parented to its Docker host.
Gated by a new DOCKERDISC_CREATE_DEV setting (default off). A container
without its own MAC (bridge/overlay/etc.) never creates a device either
way - the framework's blank-scanMac guard blocks the whole group
regardless of the setting.
Reuses the existing objectPrimaryId/extra column definitions (already
host MAC / container IP) to also feed scanParentMAC/scanLastIP, so every
promoted container is auto-parented to its host with no extra plugin
logic. Two new hidden columns (helpVal1/helpVal2) carry the per-container
scanMac/scanCreatesDevice values.
Tests: 33 -> 35, both DOCKERDISC_CREATE_DEV on/off paths asserted.
Live-verified end to end against a real built image (docker-socket-proxy
+ isolated macvlan/bridge test containers), since IMPORT_ON isn't in any
released NetAlertX image yet.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011meLPKCzVpdZyAUfv5U6mm
resolve_host_mac() returns the manually configured MAC immediately,
with no Socket Proxy /info call at all - the config.json text still
described it as a fallback used only when auto-detection fails.
Reworded both the setting's own description and the parent "Docker
hosts" description to match actual behavior.
The case-insensitivity regression test for lookup_device_mac() stubbed
DeviceInstance.getByMac() to return a fixed row regardless of input,
so it passed even without exercising real collation - functionally a
duplicate of test_lookup_device_mac_found. Replaced it with a
delegation check, and added real SQLite-backed coverage for
DeviceInstance.getByMac()'s case-insensitivity in
test/backend/test_device_instance.py. That surfaced a gap in the
shared db_test_helpers.py fixture: its Devices.devMac column was
missing the COLLATE NOCASE that the real schema declares, so it could
not have exercised this behavior. Fixed the fixture to match.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011meLPKCzVpdZyAUfv5U6mm
Per jokob-sk's review - unnecessary details belongs in the PR/commit
history, not the docstring (matches CLAUDE.md's own convention: a
docstring describes current behavior, not a changelog of why).
- resolve_host_mac()/lookup_device_mac() now use the new
DeviceInstance.getAllByName()/getByMac() core methods instead of
querying Devices directly - no more direct SQL access from the plugin.
- config.json: removed the → HTML entity from a description (plain
ASCII ->, matching e.g. pihole_monitor's convention), and dropped the
partial es_es/de_de translations scattered through settings/columns
(English only now, matching e.g. rest_import) instead of leaving some
strings translated and others not.
- script.py: removed the two remaining references to
PLUGIN_DOCKERDISC_SPEC.md, a file that was never included in this PR.
- DockerHost now takes a shared run deadline instead of a per-request
timeout duration - every _get() call is capped by whatever's left of
that budget (and REQUEST_TIMEOUT_DEFAULT as an upper bound), so one
slow/hanging host can't burn the whole RUN_TIMEOUT and starve every
other configured host. config.json's hosts param now also sets
timeoutMultiplier, scaling the outer kill-timeout by host count.
- _get() validates the parsed response's shape (dict for /info, list for
/containers/json and /networks) before returning it, rejecting a
malformed/unexpected payload the same as a network failure instead of
letting a caller crash on it further down.
- resolve_host_mac()'s hostname match now detects more than one device
sharing that name and treats it as ambiguous (falls back to manual),
instead of silently picking an arbitrary one via LIMIT 1.
- README: the Socket Proxy is only reachable at 127.0.0.1:2375 under the
network_mode: host case described above it, not under normal compose
networking - fixed the doc to not imply either URL works there.
Read-only enrichment plugin, not an import/discovery plugin. For each
configured Docker host (via Docker Socket Proxy, never /var/run/docker.sock
directly), lists that host's containers under the host device's own
Device Details -> Plugins -> DOCKERDISC tab.
- Never creates a device, for either a host or a container - matches
against hosts already discovered the normal way (ARP/Nmap).
- Every container is listed (bridge/overlay included), not only
macvlan/ipvlan ones - a container only gets its own MAC/IP shown when
it has a macvlan/ipvlan network.
- Host MAC auto-detected via the Socket Proxy's /info -> Devices.devName
match, with a manual fallback.
Addresses CodeRabbit review on PR #1765 (pullrequestreview-5069337680).
pihole_monitor.py:
- last_raw is now tracked per source ({"primary": N, "secondary": M}
per device) instead of one combined value. Combining raw totals
across sources before diffing let a counter reset on one instance
silently net out against real traffic on the other - e.g. primary
+2000 (a real spike) and secondary resetting 1000->5 (-995) would
combine into a raw delta of only 1005, hiding most of the primary's
actual spike behind the secondary's unrelated restart.
- New aggregate_source_deltas(): diffs each source independently via
compute_delta(), then sums only the valid deltas. A source with no
valid delta this run (bootstrapping or just reset) contributes
nothing and doesn't block the others; each source keeps its own
reference point going forward.
- State loaded from before this change (last_raw as a plain number,
not per-source) is now tolerated instead of crashing - treated as no
prior reference point, so every source just bootstraps fresh on the
next run.
README.md:
- Fixed a self-contradicting line: a less frequent schedule means
larger per-run deltas, so PIHOLEMON_MIN_BLOCKED may need *raising*,
not lowering as it previously said.
- Corrected PIHOLEMON_HISTORY_DAYS guidance: it's a retention window,
not a detection delay. A new device becomes evaluable on its 3rd
successful run (1st anchors the counter, 2nd records the first
delta, 3rd has a baseline to compare against), not after the full
retention window.
Tests: 54 (up from 48). New coverage: aggregate_source_deltas() unit
tests including the exact dual-source reset-masking scenario, a
main()-level integration test for the same, and a regression test for
tolerating pre-per-source state. Both the reset-masking fix and the
legacy-state guard verified via mutation testing (reverted each,
confirmed the relevant tests fail, restored). 99% line+branch coverage
maintained.
References PR #1765.
Docs:
- Added PIHOLEMON to docs/PLUGINS.md and a new "Approach 4" section in
docs/PIHOLE_GUIDE.md, leading with anomaly detection (the actual
differentiator vs PIHOLEAPI) and explaining when to pick each plugin.
- README/PLUGINS.md/config.json's UI-facing description all reordered
and shortened to lead with anomaly detection instead of device
import, and to drop implementation detail that belongs in the
README, not the Settings page.
- Trimmed the "Why not extend PIHOLEAPI" README section per feedback -
useful context for a maintainer, not for an end user configuring
the plugin.
config.json / pihole_monitor.py:
- RUN defaults to "disabled", matching every other non-core plugin.
- VERIFY_SSL split into PRIMARY_VERIFY_SSL / SECONDARY_VERIFY_SSL -
each instance can be http/https independently. Settings reordered so
each *_VERIFY_SSL sits right under its matching *_PASSWORD.
- GRAPHQL_TOKEN removed; graphql_token now reads the core API_TOKEN
setting instead of a plugin-specific duplicate.
- GRAPHQL_URL replaced with a GET_OWNER boolean - the endpoint is now
derived from this app's own GRAPHQL_PORT (single source of truth)
instead of a URL the user had to keep in sync by hand.
- HISTORY_LENGTH (run count) replaced with HISTORY_DAYS (a real time
window): state now stores [timestamp, delta] samples and
trim_history() drops anything older than the window, so the
baseline means the same thing regardless of schedule - a faster
schedule adds more data points instead of shrinking the window.
- STATE_FILE moved from the log folder to dbFolderPath, so the rolling
anomaly baseline survives NetAlertX upgrades instead of being wiped
with the logs.
- netalertx_device_owner() (1 GraphQL call per device) replaced by
netalertx_device_owners() (1 call per run, batched) - avoids N
blocking round-trips on a large network.
- Fixed a zero-baseline bug: `bool(... and baseline and ...)` silently
exempted a device with an all-zero blocked-query history (0.0 is
falsy in Python) from ever being flagged, even on its first real
spike. Now checks `baseline is not None`.
- Fixed the placeholder-MAC filter: only excluded the literal "ip-::",
not Pi-hole's general "ip-<address>" placeholder pattern. Caught
downstream by is_mac() either way, but now the actual placeholder
check does what it looks like it does.
- Fixed a cumulative-counter bug: Pi-hole's /api/stats/top_clients
returns a count that's cumulative since FTL last started, not a
per-interval or daily-resetting one (confirmed against FTL's own
source and long-standing user reports that it doesn't reset at
midnight). Comparing that raw total directly against a rolling
average made any device's ordinary growing traffic look like an
escalating anomaly. compute_delta() now diffs each run's raw count
against the previous run's (state gained a per-key last_raw
reference point alongside the delta history) - None (not 0) on the
first-ever run for a device or right after a counter reset, so
those runs re-anchor the reference point instead of fabricating or
swallowing a delta.
- RUN_SCHD default changed from every 6 hours to every 5 minutes now
that the baseline window is real days, not run count, so a frequent
schedule only adds data points instead of narrowing the window; also
matches the default most other device-scanner plugins use.
- RUN_SCHD gained the same live cron-validity checkmark ARPSCAN and
other scanner plugins use (a ✓/✗ icon next to the field, validated
client-side against a regex) - reuses the existing generic
validateRegex() widget, nothing plugin-specific to build.
Tests: 48 tests (up from 37), 99% line+branch coverage. Every fix
above verified via mutation testing (deliberately broken, confirmed
the relevant test fails, then restored).
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
jokob-sk