fixes PIHOLEAPI's ignored VERIFY_SSL setting and checks setting keys

main() read the setting 'PIHOLEAPI_SSL_VERIFY', but config.json defines
the function VERIFY_SSL, so the real key is PIHOLEAPI_VERIFY_SSL.
get_setting_value() returns "" for a key nobody defined, so every Pi-hole
API request got verify="": certificate checks were always off and the
InsecureRequestWarning was suppressed, even when a user had turned on
"Verify SSL". The login request sends the Pi-hole web password to
api/auth. The plugin README and docs/PIHOLE_GUIDE.md used the wrong name
too. test_plugin_conventions.py did not catch it: it only looks at
get_setting_value("X") or <literal>, and this line had no fallback.

Changes:
- test/plugins/test_plugin_conventions.py: new
  test_own_setting_keys_exist_in_config. Every literal
  get_setting_value('<PREFIX>_X') in a plugin's scripts must name a
  setting its own config.json defines; core settings and other plugins'
  keys are not checked. Before the fix it fails for exactly one of the 54
  plugins: pihole_api_scan.py, get_setting_value('PIHOLEAPI_SSL_VERIFY').
- New test/plugins/test_pihole_api_scan_verify_ssl.py: runs main() with
  VERIFY_SSL true and false and checks the verify= value that reaches
  requests.post() for api/auth. Only the HTTP call is mocked. It fails
  before the fix (verify=None).
- server/plugins/pihole_api_scan/pihole_api_scan.py: main() reads
  PIHOLEAPI_VERIFY_SSL.
- server/plugins/pihole_api_scan/README.md and docs/PIHOLE_GUIDE.md name
  the setting PIHOLEAPI_VERIFY_SSL.
- docs/PLUGINS_DEV.md, Conventions Checklist: read a setting under the
  exact key config.json defines; names the new check.

Impact:
"Verify SSL" now works. The default stays false, so default installs
behave as before. A user who turned it on while Pi-hole uses a
self-signed certificate will now see the login fail, as the setting
description says ("Disable if you are using a self-signed certificate").
Live check against Pi-hole v6.8 behind a proxy with an untrusted
certificate: with "Verify SSL" on the fixed code refused to log in, the
old code logged in and imported 33 entries.
This commit is contained in:
sebingel committed 2026-11-01 09:32:55 +00:00
1 parent eeb7d070b4
commit 71efcd0ba0
6 files changed
+125 -3

No files matched your search

+1 -1
View File
@@ -15,7 +15,7 @@ To use this approach, make sure a Web UI password is configured in **Pi-hole**.
| `PIHOLEAPI_URL` | Your Pi-hole base URL, including the port. | `http://192.168.1.82:9880/` |
| `PIHOLEAPI_RUN_SCHD` | If you run multiple device scanner plugins, configure them to use the same schedule. | `*/5 * * * *` |
| `PIHOLEAPI_PASSWORD` | The Pi-hole Web UI admin password. NetAlertX automatically handles Base64 encoding and decoding. | `passw0rd` |
| `PIHOLEAPI_SSL_VERIFY` | Whether to verify HTTPS certificates. Disable only when using self-signed certificates. | `False` |
| `PIHOLEAPI_VERIFY_SSL` | Whether to verify HTTPS certificates. Disable only when using self-signed certificates. | `False` |
| `PIHOLEAPI_API_MAXCLIENTS` | Maximum number of devices to request from Pi-hole. The default value is usually sufficient. | `500` |
| `PIHOLEAPI_FAKE_MAC` | Generate a deterministic fake MAC address from the IP address. | `False` |
+1
View File
@@ -246,6 +246,7 @@ Check your plugin against these repo-wide conventions before opening a PR (verif
- **Persist plugin state under `dbFolderPath`, config artifacts under `configPath`** — see [Persisting Plugin Data](#persisting-plugin-data-state--config-files) below.
- **A plugin mapped to `mapped_to_table: "CurrentScan"` must also map `scanSourcePlugin`** (a static value via `mapped_to_column_data`, see [Static Value Mapping](#static-value-mapping) below), **and that value must equal the plugin's own `unique_prefix` exactly.** Whether the mapping is present at all isn't mechanically enforced by `test_plugin_conventions.py`, so review that by eye - but `test_scan_source_plugin_matches_unique_prefix` does mechanically check the value matches `unique_prefix` whenever the mapping exists, since `update_devices_data_from_scan()` (`server/scan/device_handling.py`) reads it verbatim to build the `<value>_SET_ALWAYS`/`<value>_SET_EMPTY` settings keys (`server/db/authoritative_handler.py`) - any mismatch (wrong case, a friendly display name, punctuation) means those settings silently never resolve, so the plugin's SET_ALWAYS/SET_EMPTY overrides get quietly ignored (this shipped for real: `freebox` used `"Freebox"` against `unique_prefix: "FREEBOX"`, `fritzbox` used `"Fritz!Box"` against `"FRITZBOX"`). Omitting the mapping entirely leaves `scanSourcePlugin` `NULL` on every row this plugin inserts, which silently breaks two other things: `create_new_devices()`'s `plugin_prefix = str(scanSourcePlugin).strip() if scanSourcePlugin else "NEWDEV"` mislabels devices this plugin creates as source `NEWDEV`; and `update_devices_data_from_scan()`'s `SELECT DISTINCT scanSourcePlugin FROM CurrentScan` + `[row[0] for row in plugin_rows if row[0]] or [None]` drops the `NULL` rows entirely (the `or [None]` fallback never triggers once any other plugin contributes a non-null prefix), so this plugin's `CurrentScan` rows never get picked up by the per-plugin device-update loop at all - the plugin can *insert* into `CurrentScan` but never actually confirm/update a device's presence.
- **A setting's `dataType` and `default_value` must actually agree.** `dataType: "array"` (or `"object"`) means `default_value` must be a real JSON literal for that shape — `'["default"]'`, not the bare string `"default"`. `setting_value_to_python_type()` (`server/helper.py`) `json.loads()`s the default at runtime; a bare string fails that parse, silently logs a decode error, and returns `[]` instead of your intended default — this shipped for real in `devParentRelType`/`UI_theme`/`UI_TOPOLOGY_ORDER` before being caught. If `elementOptions` already sets `multiple`/`orderable: "false"`, that's a strong signal the setting is actually scalar and `dataType` should be `"string"`, not `"array"`, regardless of what UI widget (`select`, etc.) renders it.
- **Read a setting under the exact key `config.json` defines** (`<unique_prefix>_<function>`, e.g. `PIHOLEAPI_VERIFY_SSL` for the `VERIFY_SSL` setting of `PIHOLEAPI`). `get_setting_value()` returns `""` for an unknown key, so a misspelled key silently turns the setting off. `test_own_setting_keys_exist_in_config` (`test/plugins/test_plugin_conventions.py`) checks every literal key that starts with the plugin's own prefix.
- **Only set `"allow_raw_text": true` on a display-only column type (`textarea_readonly`).** Every plugin-sourced field is HTML/control-char-stripped by default before it reaches the DB; see [Field Sanitization](#field-sanitization) below. `test_allow_raw_text_only_on_safe_types` (`test/plugins/test_plugin_conventions.py`) enforces the type restriction; it can't catch a column that legitimately needs the opt-out but is rendered somewhere unsafe, so use it only for values that are never interpreted as HTML.
- **A new system package or Python dependency needs mirroring across every build target it should work on, not just the root `Dockerfile`.** `.devcontainer/Dockerfile` is auto-generated (its own header says so) by `.devcontainer/scripts/generate-configs.sh`, which concatenates the root `Dockerfile` with `.devcontainer/resources/devcontainer-Dockerfile`. It's a committed, git-tracked file, not something read fresh at build time - a package added to the root `Dockerfile` without re-running that script leaves the devcontainer's checked-in Dockerfile stale, so the package is silently missing there until someone regenerates it. `Dockerfile.debian` (`docs/BUILDS.md`) is a second, separately-maintained build target with its own `apt-get install` package list and `setcap` calls - a system package (and its `setcap` grant, if it needs one) has to be added there too, not just to the Alpine `Dockerfile`. For a new Python dependency, the root `requirements.txt` is not the only one either: `install/ubuntu24/requirements.txt` and `install/proxmox/requirements.txt` are separate lists for their respective non-Docker install methods, not mechanically kept in sync with the root file (no CI check covers this) - check whether the new dependency is actually needed by those install methods too rather than assuming the root file alone is enough.
+1 -1
View File
@@ -31,7 +31,7 @@ No additional Pi-hole configuration is required.
| ---------------------------- | -------------------------------------------------------------------------------- |
| **PIHOLEAPI_URL** | Your Pi-hole base URL. |
| **PIHOLEAPI_PASSWORD** | The Web UI base64 encoded (en-/decoding handled by the app) admin password. |
| **PIHOLEAPI_SSL_VERIFY** | Whether to verify HTTPS certificates. Disable only for self-signed certificates. |
| **PIHOLEAPI_VERIFY_SSL** | Whether to verify HTTPS certificates. Disable only for self-signed certificates. |
| **PIHOLEAPI_RUN_TIMEOUT** | Request timeout in seconds. |
| **PIHOLEAPI_API_MAXCLIENTS** | Maximum number of devices to request from Pi-hole. Defaults are usually fine. |
| **PIHOLEAPI_FAKE_MAC** | Generate FAKE MAC from IP. |
@@ -281,7 +281,7 @@ def main():
PIHOLEAPI_PASSWORD = get_setting_value('PIHOLEAPI_PASSWORD')
PIHOLEAPI_API_MAXCLIENTS = get_setting_value('PIHOLEAPI_API_MAXCLIENTS')
# Accept boolean or string "True"/"False"
PIHOLEAPI_VERIFY_SSL = get_setting_value('PIHOLEAPI_SSL_VERIFY')
PIHOLEAPI_VERIFY_SSL = get_setting_value('PIHOLEAPI_VERIFY_SSL')
PIHOLEAPI_RUN_TIMEOUT = get_setting_value('PIHOLEAPI_RUN_TIMEOUT')
PIHOLEAPI_GET_OFFLINE = get_setting_value('PIHOLEAPI_GET_OFFLINE')
PIHOLEAPI_CONSIDER_ONLINE = get_setting_value('PIHOLEAPI_CONSIDER_ONLINE')
@@ -0,0 +1,89 @@
"""Tests for the "Verify SSL" setting of the pihole_api_scan (PIHOLEAPI) plugin.
pihole_api_scan.py is loaded with its NetAlertX-internal dependencies stubbed
out (same approach as test_adguard_import.py), so these tests run without a
Pi-hole instance. Only the HTTP call to Pi-hole is mocked; main() and
pihole_api_auth() run for real.
"""
import importlib.util
import os
import sys
import types
from pathlib import Path
from unittest.mock import MagicMock
import pytest
INSTALL_PATH = os.getenv("NETALERTX_APP", "/app")
sys.path.extend([f"{INSTALL_PATH}/server/plugins", f"{INSTALL_PATH}/server"])
def _load_pihole_api_scan_module():
"""Import pihole_api_scan.py with NetAlertX modules stubbed, then restore sys.modules."""
missing_module = object()
previous_modules = {}
def stub(name, **attributes):
"""Put a fake module with the given attributes into sys.modules, remembering the previous entry."""
previous_modules[name] = sys.modules.get(name, missing_module)
module = types.ModuleType(name)
for attribute, value in attributes.items():
setattr(module, attribute, value)
sys.modules[name] = module
stub("plugin_helper", Plugin_Objects=MagicMock, is_mac=MagicMock(return_value=True))
stub("utils.datetime_utils", timeNowUTC=MagicMock())
stub("logger", mylog=MagicMock(), Logger=MagicMock())
stub("helper", get_setting_value=MagicMock(return_value="UTC"))
stub("const", logPath="/tmp")
stub("conf", tz=None)
stub("pytz", timezone=MagicMock(return_value="UTC"))
stub("utils.crypto_utils", string_to_fake_mac=MagicMock(return_value="fa:ce:00:00:00:01"))
module_path = Path(__file__).resolve().parents[2] / "server" / "plugins" / "pihole_api_scan" / "pihole_api_scan.py"
spec = importlib.util.spec_from_file_location("pihole_api_scan_verify_ssl_script", module_path)
module = importlib.util.module_from_spec(spec)
try:
spec.loader.exec_module(module)
finally:
for name, previous_module in previous_modules.items():
if previous_module is missing_module:
sys.modules.pop(name, None)
else:
sys.modules[name] = previous_module
return module
pihole_api_scan = _load_pihole_api_scan_module()
def _use_settings(monkeypatch, settings):
"""Serve settings to main() via get_setting_value, and restore the module globals main() rebinds after the test."""
# raising=False: a global may only come into existence when main() runs.
for name in ("PIHOLEAPI_URL", "PIHOLEAPI_PASSWORD", "PIHOLEAPI_API_MAXCLIENTS", "PIHOLEAPI_VERIFY_SSL",
"PIHOLEAPI_RUN_TIMEOUT", "PIHOLEAPI_GET_OFFLINE", "PIHOLEAPI_CONSIDER_ONLINE"):
monkeypatch.setattr(pihole_api_scan, name, getattr(pihole_api_scan, name, None), raising=False)
monkeypatch.setattr(pihole_api_scan, "get_setting_value", settings.get)
class TestVerifySsl:
@pytest.mark.parametrize("verify_ssl", [True, False])
def test_verify_ssl_setting_reaches_pihole_request(self, monkeypatch, verify_ssl):
"""The VERIFY_SSL setting (key PIHOLEAPI_VERIFY_SSL) is what main() passes as verify= to the Pi-hole API."""
_use_settings(monkeypatch, {
"PIHOLEAPI_URL": "https://pi.hole/",
"PIHOLEAPI_PASSWORD": "secret",
"PIHOLEAPI_RUN_TIMEOUT": 30,
"PIHOLEAPI_VERIFY_SSL": verify_ssl,
})
auth_response = MagicMock()
auth_response.json.return_value = {"session": {"valid": False}}
post = MagicMock(return_value=auth_response)
monkeypatch.setattr(pihole_api_scan.requests, "post", post)
monkeypatch.setattr(pihole_api_scan.requests.packages.urllib3, "disable_warnings", MagicMock())
pihole_api_scan.main()
assert post.call_args.kwargs["verify"] is verify_ssl
+32
View File
@@ -235,6 +235,38 @@ def test_hardcoded_default_matches_config(plugin_name):
)
# ---------------------------------------------------------------------------
# A plugin reading one of its own settings under a key config.json never defines
#
# get_setting_value() returns "" for a key nobody defined, so a misspelled key
# silently turns the setting off. Only literal keys that start with the
# plugin's own unique_prefix are checked; core settings and other plugins'
# keys are out of scope.
# ---------------------------------------------------------------------------
_SETTING_KEY_RE = re.compile(r'''get_setting_value\(\s*["']([A-Za-z0-9_]+)["']''')
@pytest.mark.parametrize('plugin_name', _PLUGIN_NAMES)
def test_own_setting_keys_exist_in_config(plugin_name):
"""Every literal get_setting_value('<PREFIX>_X') in a plugin's scripts names a setting its config.json defines."""
config = _load_config(plugin_name)
own_prefix = config.get('unique_prefix', '') + '_'
defined = set(_setting_defaults(config))
unknown = []
for py_file in _plugin_py_files(plugin_name):
with open(py_file) as f:
source = f.read()
for key in _SETTING_KEY_RE.findall(source):
if key.startswith(own_prefix) and key not in defined:
unknown.append(f"{os.path.basename(py_file)}: get_setting_value('{key}')")
assert not unknown, (
f"{plugin_name}: setting key(s) not defined in config.json - get_setting_value() returns '' for them, "
"so the setting is silently ignored:\n" + "\n".join(unknown)
)
# ---------------------------------------------------------------------------
# RUN_TIMEOUT reused as a per-call timeout inside a loop
#