From 94a5cd4968b42035e54b2157e2fdd7fbfb7ea31a Mon Sep 17 00:00:00 2001 From: Mauricio Camayo Date: Mon, 14 Sep 2026 08:48:14 -0500 Subject: [PATCH 1/6] Add DOCKERDISC plugin: enrich existing devices with their Docker containers 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. --- server/plugins/dockerdisc/README.md | 186 ++++++ server/plugins/dockerdisc/config.json | 806 ++++++++++++++++++++++++++ server/plugins/dockerdisc/script.py | 338 +++++++++++ test/plugins/test_dockerdisc.py | 460 +++++++++++++++ 4 files changed, 1790 insertions(+) create mode 100644 server/plugins/dockerdisc/README.md create mode 100644 server/plugins/dockerdisc/config.json create mode 100644 server/plugins/dockerdisc/script.py create mode 100644 test/plugins/test_dockerdisc.py diff --git a/server/plugins/dockerdisc/README.md b/server/plugins/dockerdisc/README.md new file mode 100644 index 00000000..de76b23c --- /dev/null +++ b/server/plugins/dockerdisc/README.md @@ -0,0 +1,186 @@ +## Overview + +`DOCKERDISC` enriches Docker **hosts** NetAlertX already knows about with +the list of containers running on them - image, Compose project/service, +network driver, and (for containers on a `macvlan`/`ipvlan` network) their +own MAC/IP. + +It does **not** discover devices. NetAlertX's own ARP/Nmap scanners remain +the only source of device presence. `DOCKERDISC` never creates a device row +- not for a container, and not for the Docker host itself, which must +already exist in NetAlertX before this plugin can attach anything to it. + +Maintainer's mental model for this plugin: **Device = Docker host → List of +containers.** Every container found on a host shows up under that **host's +own** Device Details → Plugins → DOCKERDISC tab, not as a device of its +own. + +> [!TIP] +> Connects via a read-only [Docker Socket +> Proxy](https://github.com/Tecnativa/docker-socket-proxy) (e.g. +> `tecnativa/docker-socket-proxy`) - never mounts `/var/run/docker.sock` +> directly into the NetAlertX container. + +### Why a Socket Proxy, and not `docker.sock` directly? + +Mounting `/var/run/docker.sock` into a container gives that container the +same power as root on the host: anything that can reach the socket can, +for example, start a new `--privileged` container with the host +filesystem bind-mounted in - a standard, well-known way to escalate from +"container access" to "host root." It can't be scoped down to "read-only" +or "just these endpoints" - it's all or nothing. + +That's a much bigger risk to accept for NetAlertX specifically than for a +small single-purpose tool: NetAlertX is a web UI, a GraphQL API, and +dozens of other plugins pulling in data from routers, DHCP leases, and +other external sources - a large attack surface. A vulnerability anywhere +in any of that would inherit full `docker.sock` access too, even though +this plugin itself only ever needs to read three things: the container +list, host info, and the network list. + +The Socket Proxy sits between NetAlertX and the real socket and only +forwards the specific API paths this plugin actually needs +(`CONTAINERS=1`, `INFO=1`, `NETWORKS=1`) - everything else (exec, image +builds, volumes, secrets, any `POST` that creates/kills something) is +rejected by default. If NetAlertX is ever compromised, the blast radius +stops at "can list containers/networks," not "can root the host." + +### Socket Proxy compose service + +Add this as another service in the **same `docker-compose.yml` as +NetAlertX itself** - not a separate stack/file: + +```yaml +services: + netalertx: + container_name: netalertx + image: "ghcr.io/jokob-sk/netalertx" + ... # same as you already have + ... + + docker-socket-proxy: + image: tecnativa/docker-socket-proxy:latest + container_name: docker-socket-proxy + environment: + CONTAINERS: 1 + INFO: 1 + NETWORKS: 1 + POST: 0 + volumes: + - /var/run/docker.sock:/var/run/docker.sock:ro + restart: unless-stopped + # Only if netalertx uses network_mode: host - see note below + # ports: + # - "127.0.0.1:2375:2375" +``` + +> [!NOTE] +> If your `netalertx` service uses `network_mode: host` (common, since ARP +> scanning needs a real host NIC), it won't resolve `docker-socket-proxy` +> by name - host networking means it isn't on the compose network at all. +> Fix: add `ports: ["127.0.0.1:2375:2375"]` to the proxy service above, and +> use `http://127.0.0.1:2375` as the Socket Proxy URL instead. Don't give +> the proxy `network_mode: host` too - that likely exposes it to the whole +> LAN instead of just the NAS. + +Same file, on purpose: + +- Compose puts every service in one file on the same default network + automatically, so NetAlertX can reach it at `http://127.0.0.1:2375` or + `http://docker-socket-proxy:2375` for free - no extra `networks:` config, + no port published to the LAN (nothing else needs to reach it). +- Its lifecycle naturally follows NetAlertX's - one `docker compose up`/ + `down` brings both up or down together, instead of a second stack to + remember to manage separately. +- It's a dependency this plugin needs, not an unrelated service, so it + belongs with NetAlertX conceptually as well as operationally. + +(The only real reason to split it into its own stack is sharing one proxy +across several unrelated projects - e.g. Watchtower and NetAlertX both +reading from the same proxy instead of each running their own. Not needed +here.) + +`POST: 0` is already the image's default; it's listed explicitly since +it's the setting that keeps this read-only - nothing here can +create/start/stop/kill anything. + +### Quick setup guide + +1. Add the Socket Proxy service above to NetAlertX's `docker-compose.yml` + and bring it up (`docker compose up -d docker-socket-proxy`) - one per + Docker host you want tracked, if you're tracking more than one. +2. In NetAlertX, add one entry per Docker host under **Docker hosts** + (`DOCKERDISC_hosts`) with that proxy's URL (`http://127.0.0.1:2375`, + `http://docker-socket-proxy:2375`, or whatever you named the service). +3. Make sure the Docker host itself already exists as a device in + NetAlertX (it normally does, found via ARP/Nmap - Docker hosts have a + real NIC on the LAN). If host-MAC auto-detection doesn't find it (see + below), fill in its MAC manually in the same entry. + +#### Required Settings + +- When to run `DOCKERDISC_RUN` +- Docker hosts `DOCKERDISC_hosts` - at least one entry, each with: + - Docker Socket Proxy URL `DOCKERDISC_SOCKET_PROXY_URL` + - Docker Host MAC Address (Fallback) `DOCKERDISC_HOST_MAC` - optional if + auto-detection works for that host + +### Host MAC auto-detection + +If `DOCKERDISC_HOST_MAC` is filled in, it's used immediately - no Socket +Proxy call at all. Deliberate trade-off: a MAC is stable, so there's +nothing to gain by re-confirming it via `/info` on every scheduled run, +but it also means a future MAC change (e.g. a replaced NIC) won't be +auto-detected while the field stays set. + +Otherwise, the plugin calls the Socket Proxy's `GET /info` (Docker Engine +API) to read the daemon's hostname, then looks for a NetAlertX device +whose `devName` matches it. If either step fails - `/info` isn't reachable +(check the `INFO=1` permission), or no device's name matches - that host's +entry is skipped for the run (logged, not fatal to other hosts). + +### Container listing + +Every container on a host is listed, including `bridge`/overlay ones - not +only `macvlan`/`ipvlan` containers. What changes per container is only +whether it has a real LAN-visible identity to show: + +- **Network Driver** (`watchedValue3`) is always populated. +- **Container MAC** (`watchedValue4`) and **IP** (`extra`) are populated + only when the container has a `macvlan`/`ipvlan` network attached; + otherwise they show `null`. A `bridge`-only container's own IP/MAC isn't + LAN-visible, so there's nothing meaningful to show there - it still gets + a row (image, Compose project/service, driver). +- If a container is attached to **more than one** `macvlan`/`ipvlan` + network at the same time (uncommon, but possible - e.g. a dual-homed + network appliance), the network whose *name* sorts first alphabetically + is the one shown. This is a deliberate, deterministic tie-break, not an + attempt to pick the "right" one - Docker doesn't expose any ordering or + priority between a container's networks, so any rule here is arbitrary; + what matters is that it's stable (the same container always reports the + same MAC/IP) rather than depending on whatever order the Socket Proxy's + JSON happens to return them in. + +### Usage + +- Head to **Settings** → **Docker discovery** to configure Docker hosts. +- Container details appear under each host's own **Device Details** → + **Plugins** → **DOCKERDISC** tab. + +### Notes + +- This plugin never writes to `devMac`, `devLastIP`, `devFirstConnection`, + `devSourcePlugin`, or `devCustomProps` - ARP/Nmap remain authoritative + for device identity and discovery-source attribution on every device, + including the Docker host itself. +- Only Socket Proxy permissions required: `CONTAINERS=1` (list containers, + their networks and labels), `INFO=1` (host-MAC auto-detection), and + `NETWORKS=1` (network driver lookup - one batched `GET /networks` call + per run for every unique network id seen, not one call per container). + No write/exec permissions needed. +- Design history and open implementation questions in [issue #1721] + (https://github.com/netalertx/NetAlertX/issues/1721). + +- Version: 0.1.0 +- Author: [mauricio-camayo](https://github.com/mauricio-camayo/) +- Release Date: `2026-09-14` diff --git a/server/plugins/dockerdisc/config.json b/server/plugins/dockerdisc/config.json new file mode 100644 index 00000000..02a37543 --- /dev/null +++ b/server/plugins/dockerdisc/config.json @@ -0,0 +1,806 @@ +{ + "code_name": "dockerdisc", + "unique_prefix": "DOCKERDISC", + "plugin_type": "other", + "enabled": true, + "data_source": "script", + "show_ui": true, + "localized": [ + "display_name", + "description", + "icon" + ], + "display_name": [ + { + "language_code": "en_us", + "string": "Docker discovery" + }, + { + "language_code": "es_es", + "string": "Descubrimiento de Docker" + }, + { + "language_code": "de_de", + "string": "Docker-Erkennung" + } + ], + "icon": [ + { + "language_code": "en_us", + "string": "" + } + ], + "description": [ + { + "language_code": "en_us", + "string": "Enriches known Docker hosts with their running containers - image, Compose project/service, network, and MAC/IP when available. Never creates devices; connects via a read-only Docker Socket Proxy." + }, + { + "language_code": "es_es", + "string": "Enriquece los hosts Docker que NetAlertX ya conoce con la lista de contenedores que corren en ellos - imagen, proyecto/servicio de Compose, driver de red y (cuando el contenedor tiene uno) su propio MAC/IP. Nunca crea devices; se conecta vía un Docker Socket Proxy de solo lectura, nunca directo a `/var/run/docker.sock`." + } + ], + "params": [], + "database_column_definitions": [ + { + "column": "index", + "css_classes": "col-sm-2", + "show": true, + "type": "none", + "default_value": "", + "options": [], + "localized": [ + "name" + ], + "name": [ + { + "language_code": "en_us", + "string": "Index" + } + ] + }, + { + "column": "plugin", + "css_classes": "col-sm-2", + "show": false, + "type": "label", + "default_value": "", + "options": [], + "localized": [ + "name" + ], + "name": [ + { + "language_code": "en_us", + "string": "N/A" + } + ] + }, + { + "column": "objectPrimaryId", + "css_classes": "col-sm-2", + "show": true, + "type": "device_mac", + "default_value": "", + "options": [], + "localized": [ + "name" + ], + "name": [ + { + "language_code": "en_us", + "string": "Container host" + } + ] + }, + { + "column": "objectSecondaryId", + "css_classes": "col-sm-2", + "show": true, + "type": "label", + "default_value": "", + "options": [], + "localized": [ + "name" + ], + "name": [ + { + "language_code": "en_us", + "string": "Container" + } + ] + }, + { + "column": "dateTimeCreated", + "css_classes": "col-sm-2", + "show": true, + "type": "label", + "default_value": "", + "options": [], + "localized": [ + "name" + ], + "name": [ + { + "language_code": "en_us", + "string": "First seen" + } + ] + }, + { + "column": "dateTimeChanged", + "css_classes": "col-sm-2", + "show": true, + "type": "label", + "default_value": "", + "options": [], + "localized": [ + "name" + ], + "name": [ + { + "language_code": "en_us", + "string": "Changed" + } + ] + }, + { + "column": "watchedValue1", + "css_classes": "col-sm-2", + "show": true, + "type": "label", + "default_value": "", + "options": [], + "localized": [ + "name" + ], + "name": [ + { + "language_code": "en_us", + "string": "Image" + } + ] + }, + { + "column": "watchedValue2", + "css_classes": "col-sm-2", + "show": true, + "type": "label", + "default_value": "", + "options": [], + "localized": [ + "name" + ], + "name": [ + { + "language_code": "en_us", + "string": "Compose Project / Service" + } + ] + }, + { + "column": "watchedValue3", + "css_classes": "col-sm-2", + "show": true, + "type": "label", + "default_value": "", + "options": [], + "localized": [ + "name" + ], + "name": [ + { + "language_code": "en_us", + "string": "Network Driver" + } + ] + }, + { + "column": "watchedValue4", + "css_classes": "col-sm-2", + "show": true, + "type": "label", + "default_value": "", + "options": [], + "localized": [ + "name" + ], + "name": [ + { + "language_code": "en_us", + "string": "Container MAC" + } + ] + }, + { + "column": "extra", + "css_classes": "col-sm-3", + "show": true, + "type": "label", + "default_value": "", + "options": [], + "localized": [ + "name" + ], + "name": [ + { + "language_code": "en_us", + "string": "IP" + } + ] + }, + { + "column": "userData", + "css_classes": "col-sm-2", + "show": false, + "type": "textbox_save", + "default_value": "", + "options": [], + "localized": [ + "name" + ], + "name": [ + { + "language_code": "en_us", + "string": "Comments" + }, + { + "language_code": "es_es", + "string": "Comentarios" + }, + { + "language_code": "de_de", + "string": "Kommentare" + } + ] + }, + { + "column": "status", + "css_classes": "col-sm-1", + "show": false, + "type": "replace", + "default_value": "", + "options": [ + { + "equals": "watched-not-changed", + "replacement": "
" + }, + { + "equals": "watched-changed", + "replacement": "
" + }, + { + "equals": "new", + "replacement": "
" + }, + { + "equals": "missing-in-last-scan", + "replacement": "
" + } + ], + "localized": [ + "name" + ], + "name": [ + { + "language_code": "en_us", + "string": "Status" + }, + { + "language_code": "es_es", + "string": "Estado" + }, + { + "language_code": "de_de", + "string": "Status" + } + ] + } + ], + "settings": [ + { + "function": "RUN", + "events": [ + "run" + ], + "type": { + "dataType": "string", + "elements": [ + { + "elementType": "select", + "elementOptions": [], + "transformers": [] + } + ] + }, + "default_value": "disabled", + "options": [ + "disabled", + "once", + "schedule", + "always_after_scan" + ], + "localized": [ + "name", + "description" + ], + "name": [ + { + "language_code": "en_us", + "string": "When to run" + }, + { + "language_code": "es_es", + "string": "Cuando ejecuta" + }, + { + "language_code": "de_de", + "string": "Wann ausführen" + } + ], + "description": [ + { + "language_code": "en_us", + "string": "Enable a regular Docker discovery run. If you select schedule the scheduling settings from below are applied. If you select once the scan is run only once on start of the application (container) for the time specified in DOCKERDISC_RUN_TIMEOUT setting." + }, + { + "language_code": "es_es", + "string": "Habilita una ejecución periódica de descubrimiento de Docker. Si selecciona schedule se aplican las opciones de programación de abajo. Si selecciona once el escaneo se ejecuta solo una vez al iniciar la aplicación (contenedor) durante el tiempo especificado en la configuración DOCKERDISC_RUN_TIMEOUT." + } + ] + }, + { + "function": "CMD", + "type": { + "dataType": "string", + "elements": [ + { + "elementType": "input", + "elementOptions": [ + { + "readonly": "true" + } + ], + "transformers": [] + } + ] + }, + "default_value": "python3 /app/server/plugins/dockerdisc/script.py", + "options": [], + "localized": [ + "name", + "description" + ], + "name": [ + { + "language_code": "en_us", + "string": "Command" + }, + { + "language_code": "es_es", + "string": "Comando" + }, + { + "language_code": "de_de", + "string": "Befehl" + } + ], + "description": [ + { + "language_code": "en_us", + "string": "Command to run" + }, + { + "language_code": "es_es", + "string": "Comando a ejecutar" + }, + { + "language_code": "de_de", + "string": "Auszuführender Befehl" + } + ] + }, + { + "function": "RUN_SCHD", + "type": { + "dataType": "string", + "elements": [ + { + "elementType": "span", + "elementOptions": [ + { + "cssClasses": "input-group-addon validityCheck" + }, + { + "getStringKey": "Gen_ValidIcon" + } + ], + "transformers": [] + }, + { + "elementType": "input", + "elementOptions": [ + { + "focusout": "validateRegex(this)" + }, + { + "base64Regex": "Xig/OlwqfCg/OlswLTldfFsxLTVdWzAtOV18WzAtOV0rLVswLTldKyg/Oi9bMC05XSspP3xcKi9bMC05XSspKSg/OiwoPzpbMC05XXxbMS01XVswLTldfFswLTldKy1bMC05XSsoPzovWzAtOV0rKT98XCovWzAtOV0rKSkqXHMrKD86XCp8KD86WzAtOV18MVswLTldfDJbMC0zXXxbMC05XSstWzAtOV0rKD86L1swLTldKyk/fFwqL1swLTldKykpKD86LCg/OlswLTldfDFbMC05XXwyWzAtM118WzAtOV0rLVswLTldKyg/Oi9bMC05XSspP3xcKi9bMC05XSspKSpccysoPzpcKnwoPzpbMS05XXxbMTJdWzAtOV18M1swMV18WzAtOV0rLVswLTldKyg/Oi9bMC05XSspP3xcKi9bMC05XSspKSg/OiwoPzpbMS05XXxbMTJdWzAtOV18M1swMV18WzAtOV0rLVswLTldKyg/Oi9bMC05XSspP3xcKi9bMC05XSspKSpccysoPzpcKnwoPzpbMS05XXwxWzAtMl18WzAtOV0rLVswLTldKyg/Oi9bMC05XSspP3xcKi9bMC05XSspKSg/OiwoPzpbMS05XXwxWzAtMl18WzAtOV0rLVswLTldKyg/Oi9bMC05XSspP3xcKi9bMC05XSspKSpccysoPzpcKnwoPzpbMC02XXxbMC02XS1bMC02XSg/Oi9bMC05XSspP3xcKi9bMC05XSspKSg/OiwoPzpbMC02XXxbMC02XS1bMC02XSg/Oi9bMC05XSspP3xcKi9bMC05XSspKSok" + } + ], + "transformers": [] + } + ] + }, + "default_value": "*/5 * * * *", + "options": [], + "localized": [ + "name", + "description" + ], + "name": [ + { + "language_code": "en_us", + "string": "Schedule" + }, + { + "language_code": "es_es", + "string": "Schedule" + }, + { + "language_code": "de_de", + "string": "Zeitplan" + } + ], + "description": [ + { + "language_code": "en_us", + "string": "Only enabled if you select schedule in the DOCKERDISC_RUN setting. Make sure you enter the schedule in the correct cron-like format (e.g. validate at crontab.guru). For example entering 0 4 * * * will run the scan after 4 am in the TIMEZONE you set above. Will be run NEXT time the time passes." + }, + { + "language_code": "es_es", + "string": "Solo habilitado si selecciona schedule en la configuración DOCKERDISC_RUN. Asegúrese de ingresar el schedule en el formato similar a cron correcto (por ejemplo, valide en crontab.guru). Por ejemplo, ingrese 0 4 * * * ejecutará el escaneo después de las 4 am en el TIMEZONE que configuró arriba. Se ejecutará la PRÓXIMA vez que pase el tiempo." + } + ] + }, + { + "function": "RUN_TIMEOUT", + "type": { + "dataType": "integer", + "elements": [ + { + "elementType": "input", + "elementOptions": [ + { + "type": "number" + } + ], + "transformers": [] + } + ] + }, + "default_value": 60, + "options": [], + "localized": [ + "name", + "description" + ], + "name": [ + { + "language_code": "en_us", + "string": "Run timeout" + }, + { + "language_code": "es_es", + "string": "Tiempo de espera de ejecución" + }, + { + "language_code": "de_de", + "string": "Zeitlimit" + } + ], + "description": [ + { + "language_code": "en_us", + "string": "Maximum time in seconds to wait for the script to finish. If this time is exceeded the script is aborted." + }, + { + "language_code": "es_es", + "string": "Tiempo máximo en segundos para esperar a que finalice el script. Si se supera este tiempo, el script se cancela." + } + ] + }, + { + "function": "hosts", + "type": { + "dataType": "array", + "elements": [ + { + "elementType": "button", + "elementOptions": [ + { + "sourceSuffixes": [] + }, + { + "separator": "" + }, + { + "cssClasses": "col-xs-12" + }, + { + "onClick": "addViaPopupForm(this)" + }, + { + "getStringKey": "Gen_Add" + } + ], + "transformers": [] + }, + { + "elementType": "select", + "elementHasInputValue": 1, + "elementOptions": [ + { + "multiple": "true" + }, + { + "readonly": "true" + }, + { + "editable": "true" + }, + { + "popupForm": [ + { + "function": "DOCKERDISC_SOCKET_PROXY_URL", + "type": { + "dataType": "string", + "elements": [ + { + "elementType": "input", + "elementOptions": [ + { + "placeholder": "http://docker-socket-proxy:2375" + }, + { + "cssClasses": "col-sm-10" + } + ], + "transformers": [] + } + ] + }, + "default_value": "http://docker-socket-proxy:2375", + "options": [], + "localized": [ + "name", + "description" + ], + "name": [ + { + "language_code": "en_us", + "string": "Docker Socket Proxy URL" + } + ], + "description": [ + { + "language_code": "en_us", + "string": "Base URL for the read-only Docker Socket Proxy endpoint for this Docker host (requires the CONTAINERS, INFO, and NETWORKS permissions)." + } + ] + }, + { + "function": "DOCKERDISC_HOST_MAC", + "type": { + "dataType": "string", + "elements": [ + { + "elementType": "input", + "elementOptions": [ + { + "placeholder": "aa:bb:cc:dd:ee:ff" + }, + { + "cssClasses": "col-sm-10" + } + ], + "transformers": [] + } + ] + }, + "default_value": "", + "options": [], + "localized": [ + "name", + "description" + ], + "name": [ + { + "language_code": "en_us", + "string": "Docker Host MAC Address (Fallback)" + } + ], + "description": [ + { + "language_code": "en_us", + "string": "Manual fallback physical MAC address of the Docker host, used if auto-detecting it via the Socket Proxy /info endpoint fails. The host must already exist as a device in NetAlertX (found via ARP/Nmap) - this plugin never creates it." + } + ] + } + ] + } + ], + "transformers": [ + "name|base64" + ] + }, + { + "elementType": "button", + "elementOptions": [ + { + "sourceSuffixes": [] + }, + { + "separator": "" + }, + { + "cssClasses": "col-xs-6" + }, + { + "onClick": "removeFromList(this)" + }, + { + "getStringKey": "Gen_Remove_Last" + } + ], + "transformers": [] + }, + { + "elementType": "button", + "elementOptions": [ + { + "sourceSuffixes": [] + }, + { + "separator": "" + }, + { + "cssClasses": "col-xs-6" + }, + { + "onClick": "removeAllOptions(this)" + }, + { + "getStringKey": "Gen_Remove_All" + } + ], + "transformers": [] + } + ] + }, + "default_value": [], + "options": [], + "localized": [ + "name", + "description" + ], + "name": [ + { + "language_code": "en_us", + "string": "Docker hosts" + } + ], + "description": [ + { + "language_code": "en_us", + "string": "One entry per Docker host to track. Each entry pairs a read-only Docker Socket Proxy URL with that host's device (auto-detected, or entered manually as a fallback). Every container found on a host is listed under that host's own Device Details → Plugins → DOCKERDISC tab - the host device must already exist in NetAlertX (via ARP/Nmap); this plugin never creates devices." + } + ] + }, + { + "function": "WATCH", + "type": { + "dataType": "array", + "elements": [ + { + "elementType": "select", + "elementOptions": [ + { + "multiple": "true", + "orderable": "true" + } + ], + "transformers": [] + } + ] + }, + "default_value": [], + "options": [ + "watchedValue1", + "watchedValue2", + "watchedValue3", + "watchedValue4" + ], + "localized": [ + "name", + "description" + ], + "name": [ + { + "language_code": "en_us", + "string": "Watched" + }, + { + "language_code": "es_es", + "string": "Visto" + }, + { + "language_code": "de_de", + "string": "Überwacht" + } + ], + "description": [ + { + "language_code": "en_us", + "string": "Send a notification if selected values change. Use CTRL + Click to select/deselect. " + }, + { + "language_code": "es_es", + "string": "Envíe una notificación si los valores seleccionados cambian. Use CTRL + Clic para seleccionar/deseleccionar. " + } + ] + }, + { + "function": "REPORT_ON", + "type": { + "dataType": "array", + "elements": [ + { + "elementType": "select", + "elementOptions": [ + { + "multiple": "true", + "orderable": "true" + } + ], + "transformers": [] + } + ] + }, + "default_value": [], + "options": [ + "new", + "watched-changed", + "watched-not-changed", + "missing-in-last-scan" + ], + "localized": [ + "name", + "description" + ], + "name": [ + { + "language_code": "en_us", + "string": "Report on" + }, + { + "language_code": "es_es", + "string": "Informar sobre" + }, + { + "language_code": "de_de", + "string": "Benachrichtige wenn" + } + ], + "description": [ + { + "language_code": "en_us", + "string": "Send a notification only on these statuses. new means a new unique (unique combination of PrimaryId and SecondaryId) object was discovered. watched-changed means that selected watchedValueN columns changed." + }, + { + "language_code": "es_es", + "string": "Envíe una notificación solo en estos estados. new significa que se descubrió un nuevo objeto único (combinación única de PrimaryId y SecondaryId). watched-changed significa que las columnas watchedValueN seleccionadas cambiaron." + } + ] + } + ] +} diff --git a/server/plugins/dockerdisc/script.py b/server/plugins/dockerdisc/script.py new file mode 100644 index 00000000..cf02bdd9 --- /dev/null +++ b/server/plugins/dockerdisc/script.py @@ -0,0 +1,338 @@ +#!/usr/bin/env python +"""NetAlertX plugin: DOCKERDISC - Docker discovery (enrichment, not import) + +Does NOT discover devices. NetAlertX's own ARP/Nmap scanners remain the +sole source of device presence. Instead, for each configured Docker host +this plugin lists that host's containers under the *host's own* Device +Details -> Plugins -> DOCKERDISC tab. + +Design ("Device = Docker host -> List of containers", per maintainer +jokob-sk, see ../../../PLUGIN_DOCKERDISC_SPEC.md for the full history): + + - objectPrimaryId / foreignKey is always the Docker HOST's MAC - never a + container's own MAC. Every plugin object (one per container) attaches + to the host device, which must already exist in NetAlertX (found the + normal way, via ARP/Nmap). This plugin never creates a device row, for + either a host or a container. + - Because matching targets the host (persistent LAN identity), not the + container, EVERY container is listed - bridge/overlay ones included - + not only macvlan/ipvlan ones. A container only gets its own MAC/IP + shown (watched4/extra) when it has a macvlan/ipvlan network; otherwise + those fields are "null". + - One `hosts` entry = one Docker host: a read-only Docker Socket Proxy + URL, plus a manual MAC fallback for when auto-detection (via the + proxy's own /info endpoint) doesn't resolve to a known device. Never + connects to /var/run/docker.sock directly. + +Verified 2026-09-08 against a real Docker Engine + docker-socket-proxy +(see PLUGIN_DOCKERDISC_SPEC.md §9 for the open questions this closed): +`GET /containers/json`'s `NetworkSettings.Networks.` does NOT carry +a `Driver` field inline (only NetworkID/Gateway/IPAddress/MacAddress/...) - +the driver has to come from a separate `GET /networks` call, filtered by +the unique NetworkIDs seen across a host's containers in one batched +request (cacheable per run, as originally anticipated). This needs the +Socket Proxy's NETWORKS=1 permission in addition to CONTAINERS=1/INFO=1. + +Structural references: server/plugins/internet_speedtest/config.json +(plugin_type "other", no mapped_to_column - this never writes into +Devices/CurrentScan) and server/plugins/vendor_update/script.py +("resolve for a device that must already exist, skip - never create - +otherwise" logic, applied here to the host instead of the container). +""" + +import json +import os +import sys +from urllib.parse import urlencode + +import requests + +INSTALL_PATH = os.getenv('NETALERTX_APP', '/app') +sys.path.extend([f"{INSTALL_PATH}/server/plugins", f"{INSTALL_PATH}/server"]) + +from plugin_helper import ( # noqa: E402 + Plugin_Objects, + handleEmpty, + normalize_mac, + decode_settings_base64, +) +from logger import mylog, Logger # noqa: E402 +from helper import get_setting_value # noqa: E402 +from const import logPath # noqa: E402 +from database import get_temp_db_connection # noqa: E402 +import conf # noqa: E402 +from pytz import timezone # noqa: E402 + +conf.tz = timezone(get_setting_value('TIMEZONE')) +Logger(get_setting_value('LOG_LEVEL')) + +pluginName = 'DOCKERDISC' + +LOG_PATH = logPath + '/plugins' +RESULT_FILE = os.path.join(LOG_PATH, f'last_result.{pluginName}.log') + +REQUEST_TIMEOUT_DEFAULT = 30 + +# Docker network drivers with their own real LAN-visible MAC/IP - the only +# ones that can populate watched4/extra (container_mac/container_ip). Every +# other driver (bridge, overlay, host, none, ...) still gets its container +# listed, just without those two fields. +LAN_VISIBLE_DRIVERS = ('macvlan', 'ipvlan') + + +class DockerHost: + """One configured `hosts` entry: a Docker Socket Proxy endpoint plus + the manual host-MAC fallback for it. Does not connect on construction - + call get_info()/get_containers() to actually talk to the proxy. Never + raises - a failed host is logged and skipped, not fatal to the run.""" + + def __init__(self, proxy_url, manual_mac, run_timeout): + self.proxy_url = (proxy_url or '').rstrip('/') + self.manual_mac = normalize_mac(manual_mac) if manual_mac else None + self.run_timeout = run_timeout + + @property + def configured(self): + return bool(self.proxy_url) + + def _get(self, path): + """GET against this host's Socket Proxy. Returns the parsed JSON + body, or None (logging why) on any failure.""" + try: + resp = requests.get( + self.proxy_url + path, + timeout=self.run_timeout, + ) + resp.raise_for_status() + return resp.json() + except requests.exceptions.Timeout: + mylog('none', [f'[{pluginName}] {self.proxy_url}: request to {path} timed out. Try increasing the run timeout.']) + return None + except requests.exceptions.ConnectionError: + mylog('none', [f'[{pluginName}] {self.proxy_url}: connection error on {path}. Check the Socket Proxy URL and that it is reachable.']) + return None + except Exception as e: + mylog('none', [f'[{pluginName}] {self.proxy_url}: unexpected error on {path}: {e}']) + return None + + def get_info(self): + """Docker Engine API /info - used only for host-MAC auto-detection + (the daemon's `Name`, i.e. hostname). Requires the Socket Proxy's + INFO=1 permission; returns None if that's not granted or /info + otherwise fails, in which case callers fall back to manual_mac.""" + return self._get('/info') + + def get_containers(self): + """Docker Engine API /containers/json (running containers only, + matching the default `all=false`) - includes NetworkSettings and + Labels, which is all this plugin needs. Requires CONTAINERS=1.""" + return self._get('/containers/json') or [] + + def get_network_drivers(self, network_ids): + """{NetworkID: Driver} for the given network IDs, in one batched + `GET /networks?filters=...` call - NetworkSettings.Networks on a + container does NOT carry Driver inline (confirmed against a real + Socket Proxy 2026-09-08), so this is the only way to get it. + Requires NETWORKS=1. Returns {} (not per-container failure) if the + call fails - callers fall back to an empty/unknown driver rather + than aborting the whole host.""" + network_ids = sorted(set(network_ids)) + if not network_ids: + return {} + + query = urlencode({'filters': json.dumps({'id': network_ids})}) + networks = self._get(f'/networks?{query}') + if networks is None: + mylog('verbose', [f'[{pluginName}] {self.proxy_url}: could not read /networks (needs the Socket Proxy NETWORKS=1 permission) - network driver will show as empty.']) + return {} + + return {n['Id']: n.get('Driver') for n in networks if 'Id' in n} + + +def resolve_host_mac(host): + """Manually configured DOCKERDISC_HOST_MAC wins immediately, with no + Socket Proxy call at all - a MAC address is stable and doesn't need + runtime "confirmation" via hostname matching, so there's nothing to + gain from spending an /info request on it every single scheduled run. + Otherwise auto-detects via Socket Proxy /info -> Devices.devName + match. Returns a normalized MAC string, or None if neither resolves + to anything.""" + + if host.manual_mac: + return host.manual_mac + + info = host.get_info() + hostname = (info or {}).get('Name') + + if hostname: + conn = get_temp_db_connection() + cursor = conn.cursor() + cursor.execute( + "SELECT devMac FROM Devices WHERE devName = ? COLLATE NOCASE LIMIT 1", + (hostname.lstrip('/'),), + ) + row = cursor.fetchone() + conn.close() + + if row: + mylog('verbose', [f'[{pluginName}] {host.proxy_url}: auto-detected host MAC via hostname "{hostname}".']) + return normalize_mac(row[0]) + + mylog( + 'verbose', + [f'[{pluginName}] {host.proxy_url}: /info hostname "{hostname}" has no matching Devices.devName - ' + 'falling back to the manually configured host MAC, if any.'], + ) + else: + mylog( + 'verbose', + [f'[{pluginName}] {host.proxy_url}: could not read hostname via /info (needs the Socket Proxy ' + 'INFO=1 permission) - falling back to the manually configured host MAC, if any.'], + ) + + return host.manual_mac + + +def lookup_device_mac(mac): + """True if `mac` already exists as a Devices row - this plugin never + creates the host device, same rule vendor_update applies to the + devices it enriches. COLLATE NOCASE is explicit here (not just relied + on from the Devices.devMac column definition) so this still matches + correctly even if that ever changes - normalize_mac() lowercases what + we search for, but what's actually stored can come from other + discovery methods and isn't guaranteed to be lowercase.""" + conn = get_temp_db_connection() + cursor = conn.cursor() + cursor.execute("SELECT 1 FROM Devices WHERE devMac = ? COLLATE NOCASE LIMIT 1", (mac,)) + row = cursor.fetchone() + conn.close() + return row is not None + + +def pick_lan_network(networks, driver_by_id): + """Given a container's NetworkSettings.Networks dict and a + {NetworkID: Driver} lookup (from DockerHost.get_network_drivers - the + per-network Driver isn't inline on `networks`, see module docstring), + return the (name, driver, network) for its macvlan/ipvlan network if it + has one, else None. + + If a container somehow has more than one macvlan/ipvlan network at + once, the one with the alphabetically first network *name* wins - a + deliberate, deterministic tie-break (spec §9), not "whatever order the + Socket Proxy's JSON happened to list them in" (dict iteration order, + which isn't a documented/guaranteed ordering from the Docker API and + could in principle vary between runs).""" + lan_networks = sorted( + ((name, network) for name, network in (networks or {}).items() + if driver_by_id.get(network.get('NetworkID')) in LAN_VISIBLE_DRIVERS), + key=lambda item: item[0], + ) + if not lan_networks: + return None + name, network = lan_networks[0] + return name, driver_by_id[network['NetworkID']], network + + +def first_network_driver(networks, driver_by_id): + """Best-effort driver name to show when the container has no + macvlan/ipvlan network - whatever its first network reports.""" + for network in (networks or {}).values(): + driver = driver_by_id.get(network.get('NetworkID')) + if driver: + return driver + return None + + +def process_host(host_entry, run_timeout, plugin_objects): + host = DockerHost( + proxy_url=host_entry.get('DOCKERDISC_SOCKET_PROXY_URL'), + manual_mac=host_entry.get('DOCKERDISC_HOST_MAC'), + run_timeout=run_timeout, + ) + + if not host.configured: + mylog('none', [f'[{pluginName}] Skipping a configured host entry with no Socket Proxy URL.']) + return 0 + + host_mac = resolve_host_mac(host) + if not host_mac: + mylog('none', [f'[{pluginName}] {host.proxy_url}: no host MAC (auto-detect failed and no manual fallback set) - skipping.']) + return 0 + + if not lookup_device_mac(host_mac): + mylog('none', [f'[{pluginName}] {host.proxy_url}: host MAC {host_mac} is not a known device (never created by this plugin) - skipping.']) + return 0 + + containers = host.get_containers() + mylog('verbose', [f'[{pluginName}] {host.proxy_url} ({host_mac}): {len(containers)} container(s) found.']) + + # One batched /networks call for every unique NetworkID referenced by + # this host's containers, instead of one call per container/network. + network_ids = ( + network.get('NetworkID') + for container in containers + for network in ((container.get('NetworkSettings') or {}).get('Networks') or {}).values() + ) + driver_by_id = host.get_network_drivers(n for n in network_ids if n) + + added = 0 + for container in containers: + networks = (container.get('NetworkSettings') or {}).get('Networks') or {} + lan_net = pick_lan_network(networks, driver_by_id) + + if lan_net: + _, network_driver, network = lan_net + container_mac = network.get('MacAddress') or '' + container_ip = network.get('IPAddress') or '' + else: + network_driver = first_network_driver(networks, driver_by_id) or '' + container_mac = '' + container_ip = '' + + labels = container.get('Labels') or {} + compose_project = labels.get('com.docker.compose.project') + compose_service = labels.get('com.docker.compose.service') + compose = ' / '.join(p for p in (compose_project, compose_service) if p) or None + + names = container.get('Names') or [] + container_name = names[0].lstrip('/') if names else container.get('Id', '')[:12] + + plugin_objects.add_object( + primaryId=host_mac, + secondaryId=handleEmpty(container_name), + watched1=handleEmpty(container.get('Image')), + watched2=handleEmpty(compose), + watched3=handleEmpty(network_driver), + watched4=handleEmpty(container_mac), + extra=handleEmpty(container_ip), + foreignKey=host_mac, + ) + added += 1 + + return added + + +def main(): + mylog('verbose', [f'[{pluginName}] In script']) + + host_configs = get_setting_value('DOCKERDISC_hosts') or [] + run_timeout = get_setting_value('DOCKERDISC_RUN_TIMEOUT') or REQUEST_TIMEOUT_DEFAULT + + mylog('verbose', [f'[{pluginName}] number of configured hosts: {len(host_configs)}']) + + plugin_objects = Plugin_Objects(RESULT_FILE) + + total_added = 0 + for host_config in host_configs: + host_entry = decode_settings_base64(host_config) + total_added += process_host(host_entry, run_timeout, plugin_objects) + + plugin_objects.write_result_file() + + mylog('verbose', [f'[{pluginName}] Update complete - {total_added} container(s) reported across {len(host_configs)} host(s).']) + + return 0 + + +if __name__ == '__main__': + main() diff --git a/test/plugins/test_dockerdisc.py b/test/plugins/test_dockerdisc.py new file mode 100644 index 00000000..3e747ef8 --- /dev/null +++ b/test/plugins/test_dockerdisc.py @@ -0,0 +1,460 @@ +"""Tests for the dockerdisc (DOCKERDISC) plugin. + +script.py is loaded with its NetAlertX-internal dependencies +(plugin_helper, logger, helper, const, conf, pytz, database) stubbed out, +the same approach test_pihole_monitor.py uses - it keeps these tests +runnable without the full devcontainer environment and without a live +Docker Socket Proxy. `requests` itself is left real; individual HTTP calls +are mocked per test. `handleEmpty`/`normalize_mac`/`decode_settings_base64` +are reimplemented locally (same shape as plugin_helper's) rather than +imported, to avoid pulling in plugin_helper's own dependency chain. + +Layout: + - pick_lan_network() / first_network_driver(): pure-function unit tests + for the macvlan/ipvlan network-selection logic (spec §6), given a + {NetworkID: Driver} lookup - Driver isn't inline on a container's own + NetworkSettings.Networks entry (confirmed 2026-09-08 against a real + Socket Proxy - see script.py's module docstring), so these always take + that lookup as a separate argument. A container with no LAN-visible + network must still get a driver name, just no MAC/IP. A container with + *more than one* macvlan/ipvlan network at once deterministically picks + the alphabetically-first network name (spec §9's decided tie-break). + - DockerHost.get_network_drivers(): unit tests for the batched + GET /networks?filters=... call - one request for every unique + NetworkID, not one per network, and a safe {} (not a crash) when the + Socket Proxy denies it (missing NETWORKS=1). + - resolve_host_mac(): unit tests for the manual-MAC-short-circuits- + without-any-request-first, else /info -> devName match chain (spec + §3.2) - a configured DOCKERDISC_HOST_MAC wins immediately with zero + Socket Proxy calls (deliberate: no auto-re-verification once you've + told us the answer), so auto-detection only ever runs when it's + empty, and only then can hostname-unmatched or /info-unreachable + resolve to None. + - lookup_device_mac(): unit test for the "host must already exist, this + plugin never creates it" gate - explicit COLLATE NOCASE, not just + relied on from the Devices.devMac column definition. + - process_host(): integration tests with DockerHost's network-touching + methods stubbed at the object level - covers a mixed macvlan+bridge + container list (only the macvlan one gets a MAC/IP), an unconfigured + entry (no proxy URL) short-circuiting before any request, and a host + MAC that doesn't resolve to a known device short-circuiting before + /containers/json is ever called. + +All of the above was additionally run live, end to end, against a real +Docker Engine + a real tecnativa/docker-socket-proxy on 2026-09-08 (ad-hoc +harness, not part of this repo) - that run is what caught the Driver-not- +inline bug these tests now guard against. +""" + +import base64 +import importlib.util +import json +import sys +import types +from pathlib import Path +from unittest.mock import MagicMock, patch + +import requests + + +def _handle_empty(value): + """Same shape as plugin_helper.handleEmpty, without its import chain.""" + return value if value else 'null' + + +def _normalize_mac(mac): + """Same shape as plugin_helper.normalize_mac, without its import chain.""" + s = str(mac).strip().lower() + if s == "internet": + return "internet" + if ':' in s: + parts = s.split(':') + elif '-' in s: + parts = s.split('-') + else: + parts = [s[i:i + 2] for i in range(0, len(s), 2)] + return ':'.join(p if p == '*' else p.zfill(2) for p in (part.strip() for part in parts)) + + +def _decode_settings_base64(encoded_str): + """Same shape as plugin_helper.decode_settings_base64 (convert_types=True).""" + settings_list = json.loads(base64.b64decode(encoded_str).decode("utf-8")) + out = {} + for _, key, _type, value in settings_list: + t = _type.lower() + if t == "boolean": + out[key] = value.lower() == "true" + elif t == "integer": + out[key] = int(value) + elif t == "float": + out[key] = float(value) + else: + out[key] = value + return out + + +def _encode_host_entry(proxy_url, host_mac): + """Builds a base64-encoded popUpForm entry matching what NetAlertX + would send for one `DOCKERDISC_hosts` row.""" + settings_list = [ + ["DOCKERDISC_hosts", "DOCKERDISC_SOCKET_PROXY_URL", "string", proxy_url], + ["DOCKERDISC_hosts", "DOCKERDISC_HOST_MAC", "string", host_mac or ""], + ] + return base64.b64encode(json.dumps(settings_list).encode("utf-8")).decode("ascii") + + +def _load_dockerdisc_module(): + missing_module = object() + previous_modules = {} + + def stub(name, **attributes): + 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, + handleEmpty=_handle_empty, + normalize_mac=_normalize_mac, + decode_settings_base64=_decode_settings_base64, + ) + stub("logger", mylog=MagicMock(), Logger=MagicMock()) + stub("helper", get_setting_value=MagicMock(return_value="UTC")) + stub("const", logPath="/tmp") + stub("database", get_temp_db_connection=MagicMock()) + stub("conf", tz=None) + stub("pytz", timezone=MagicMock(return_value="UTC")) + + module_path = Path(__file__).resolve().parents[2] / "server" / "plugins" / "dockerdisc" / "script.py" + spec = importlib.util.spec_from_file_location("dockerdisc_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 + + +dockerdisc = _load_dockerdisc_module() + + +def _resp(json_data): + resp = MagicMock() + resp.raise_for_status = MagicMock() + resp.json = MagicMock(return_value=json_data) + return resp + + +def _db_returning(rows): + """A get_temp_db_connection() replacement whose cursor().fetchone() + yields successive `rows` entries (one per execute() call), then None.""" + conn = MagicMock() + cursor = MagicMock() + conn.cursor.return_value = cursor + cursor.fetchone.side_effect = list(rows) + [None] * 10 + return conn + + +# --------------------------------------------------------------------------- +# pick_lan_network() / first_network_driver() +# --------------------------------------------------------------------------- + + +def test_pick_lan_network_prefers_macvlan(): + networks = { + "bridge": {"NetworkID": "net-bridge", "MacAddress": "02:aa:aa:aa:aa:aa", "IPAddress": "172.17.0.2"}, + "lan": {"NetworkID": "net-lan", "MacAddress": "aa:bb:cc:dd:ee:ff", "IPAddress": "192.168.1.50"}, + } + driver_by_id = {"net-bridge": "bridge", "net-lan": "macvlan"} + name, driver, network = dockerdisc.pick_lan_network(networks, driver_by_id) + assert driver == "macvlan" + assert network["MacAddress"] == "aa:bb:cc:dd:ee:ff" + + +def test_pick_lan_network_none_for_bridge_only(): + networks = {"bridge": {"NetworkID": "net-bridge", "MacAddress": "02:aa:aa:aa:aa:aa"}} + assert dockerdisc.pick_lan_network(networks, {"net-bridge": "bridge"}) is None + + +def test_pick_lan_network_empty_networks(): + assert dockerdisc.pick_lan_network({}, {}) is None + assert dockerdisc.pick_lan_network(None, {}) is None + + +def test_pick_lan_network_multiple_lan_networks_ties_broken_alphabetically_by_name(): + """A container with two simultaneous macvlan/ipvlan networks must + deterministically pick the alphabetically-first network *name* (spec + §9's decided tie-break) - not whatever order the dict happens to + iterate in.""" + networks = { + "zzz-lan": {"NetworkID": "net-z", "MacAddress": "aa:aa:aa:aa:aa:zz"}, + "aaa-lan": {"NetworkID": "net-a", "MacAddress": "aa:aa:aa:aa:aa:aa"}, + } + driver_by_id = {"net-z": "macvlan", "net-a": "ipvlan"} + + name, driver, network = dockerdisc.pick_lan_network(networks, driver_by_id) + + assert name == "aaa-lan" + assert driver == "ipvlan" + assert network["MacAddress"] == "aa:aa:aa:aa:aa:aa" + + +def test_pick_lan_network_missing_from_driver_lookup_is_treated_as_no_match(): + """If get_network_drivers() failed (Socket Proxy denied NETWORKS=1) the + lookup is {} - every network must be treated as unknown/non-LAN, not + crash on a missing key.""" + networks = {"lan": {"NetworkID": "net-lan", "MacAddress": "aa:bb:cc:dd:ee:ff"}} + assert dockerdisc.pick_lan_network(networks, {}) is None + + +def test_first_network_driver_bridge_only(): + networks = {"bridge": {"NetworkID": "net-bridge"}} + assert dockerdisc.first_network_driver(networks, {"net-bridge": "bridge"}) == "bridge" + + +def test_first_network_driver_empty(): + assert dockerdisc.first_network_driver({}, {}) is None + + +def test_first_network_driver_missing_from_driver_lookup(): + networks = {"lan": {"NetworkID": "net-lan"}} + assert dockerdisc.first_network_driver(networks, {}) is None + + +# --------------------------------------------------------------------------- +# DockerHost.get_network_drivers() +# --------------------------------------------------------------------------- + + +def test_get_network_drivers_batches_into_one_request(): + host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + networks_resp = _resp([ + {"Id": "net-a", "Driver": "macvlan"}, + {"Id": "net-b", "Driver": "bridge"}, + ]) + with patch("requests.get", return_value=networks_resp) as mock_get: + result = host.get_network_drivers(["net-a", "net-b", "net-a"]) # duplicate on purpose + + assert result == {"net-a": "macvlan", "net-b": "bridge"} + assert mock_get.call_count == 1 # one batched call, not one per network + + +def test_get_network_drivers_empty_input_makes_no_request(): + host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + with patch("requests.get") as mock_get: + assert host.get_network_drivers([]) == {} + mock_get.assert_not_called() + + +def test_get_network_drivers_denied_permission_returns_empty_dict(): + """Socket Proxy without NETWORKS=1 - request fails, callers must fall + back to an empty lookup rather than crash.""" + host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + with patch("requests.get", side_effect=requests.exceptions.ConnectionError("denied")): + assert host.get_network_drivers(["net-a"]) == {} + + +# --------------------------------------------------------------------------- +# resolve_host_mac() +# --------------------------------------------------------------------------- + + +def test_resolve_host_mac_manual_mac_short_circuits_without_any_request(): + """A configured DOCKERDISC_HOST_MAC is used immediately, with zero + Socket Proxy calls - it's a stable value, there's nothing to gain by + spending an /info request "confirming" it every scheduled run. This + is a deliberate design choice (not an oversight): auto-detection only + ever runs when the field is left blank - filling it in trades away + the self-healing "auto-detect keeps re-verifying it" behavior for the + saved request, on purpose.""" + host = dockerdisc.DockerHost("http://proxy:2375", "11:22:33:44:55:66", 5) + with patch.object(host, "get_info") as mock_get_info: + assert dockerdisc.resolve_host_mac(host) == "11:22:33:44:55:66" + mock_get_info.assert_not_called() + + +def test_resolve_host_mac_auto_detect_success(): + host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + with patch.object(host, "get_info", return_value={"Name": "docker-host-1"}): + with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([("AA:BB:CC:DD:EE:FF",)])): + assert dockerdisc.resolve_host_mac(host) == "aa:bb:cc:dd:ee:ff" + + +def test_resolve_host_mac_none_when_hostname_unmatched_and_no_manual_mac(): + """No manual fallback configured, so an unmatched hostname resolves to + nothing - the "fall back to manual" path only exists when manual is + actually set, and when it is, it short-circuits before this branch is + ever reached (see the short-circuit test above).""" + host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + with patch.object(host, "get_info", return_value={"Name": "unknown-host"}): + with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([])): + assert dockerdisc.resolve_host_mac(host) is None + + +def test_resolve_host_mac_none_when_info_unreachable_and_no_manual_mac(): + host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + with patch.object(host, "get_info", return_value=None): + assert dockerdisc.resolve_host_mac(host) is None + + +# --------------------------------------------------------------------------- +# lookup_device_mac() +# --------------------------------------------------------------------------- + + +def test_lookup_device_mac_found(): + with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([(1,)])): + assert dockerdisc.lookup_device_mac("aa:bb:cc:dd:ee:ff") is True + + +def test_lookup_device_mac_not_found(): + with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([])): + assert dockerdisc.lookup_device_mac("aa:bb:cc:dd:ee:ff") is False + + +# --------------------------------------------------------------------------- +# DockerHost._get() error handling (never raises) +# --------------------------------------------------------------------------- + + +def test_dockerhost_get_timeout_returns_none(): + host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + with patch("requests.get", side_effect=requests.exceptions.Timeout("slow")): + assert host.get_info() is None + + +def test_dockerhost_get_connection_error_returns_none(): + host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + with patch("requests.get", side_effect=requests.exceptions.ConnectionError("no route")): + assert host.get_containers() == [] + + +def test_dockerhost_get_containers_success(): + host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + with patch("requests.get", return_value=_resp([{"Id": "abc123"}])): + assert host.get_containers() == [{"Id": "abc123"}] + + +# --------------------------------------------------------------------------- +# process_host() +# --------------------------------------------------------------------------- + + +def _container(name, image, driver, mac=None, ip=None, project=None, service=None, extra_bridge=False): + # NetworkID only - Driver is deliberately NOT set here, matching what a + # real Socket Proxy actually returns (see module docstring); tests + # supply Driver separately via a {NetworkID: Driver} lookup, same as + # process_host() gets it from DockerHost.get_network_drivers(). + networks = {} + if extra_bridge: + networks["bridge"] = {"NetworkID": "net-bridge"} + if driver: + networks["lan"] = {"NetworkID": "net-lan", "MacAddress": mac, "IPAddress": ip} + labels = {} + if project: + labels["com.docker.compose.project"] = project + if service: + labels["com.docker.compose.service"] = service + return { + "Id": "deadbeef0000", + "Names": [f"/{name}"], + "Image": image, + "Labels": labels, + "NetworkSettings": {"Networks": networks}, + } + + +def test_process_host_mixed_macvlan_and_bridge_containers(): + host_entry = { + "DOCKERDISC_SOCKET_PROXY_URL": "http://proxy:2375", + "DOCKERDISC_HOST_MAC": "aa:bb:cc:dd:ee:ff", + } + containers = [ + _container("pihole", "pihole/pihole:latest", "macvlan", mac="aa:aa:aa:aa:aa:01", ip="192.168.1.50", project="dns", service="pihole"), + _container("redis", "redis:7", None, extra_bridge=True), + ] + plugin_objects = MagicMock() + plugin_objects.add_object = MagicMock() + + host = dockerdisc.DockerHost(host_entry["DOCKERDISC_SOCKET_PROXY_URL"], host_entry["DOCKERDISC_HOST_MAC"], 5) + with patch.object(dockerdisc, "DockerHost", return_value=host): + with patch.object(host, "get_info", return_value=None): # unused - manual_mac short-circuits before this would ever be called + with patch.object(host, "get_containers", return_value=containers): + with patch.object(host, "get_network_drivers", return_value={"net-lan": "macvlan", "net-bridge": "bridge"}) as mock_drivers: + with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([(1,)])): + added = dockerdisc.process_host(host_entry, 5, plugin_objects) + + # one batched call for both containers' networks, not two + mock_drivers.assert_called_once() + assert sorted(mock_drivers.call_args.args[0]) == ["net-bridge", "net-lan"] + + assert added == 2 + assert plugin_objects.add_object.call_count == 2 + + pihole_call = plugin_objects.add_object.call_args_list[0].kwargs + assert pihole_call["primaryId"] == "aa:bb:cc:dd:ee:ff" # host MAC, not the container's + assert pihole_call["foreignKey"] == "aa:bb:cc:dd:ee:ff" + assert pihole_call["secondaryId"] == "pihole" + assert pihole_call["watched2"] == "dns / pihole" + assert pihole_call["watched3"] == "macvlan" + assert pihole_call["watched4"] == "aa:aa:aa:aa:aa:01" + assert pihole_call["extra"] == "192.168.1.50" + + redis_call = plugin_objects.add_object.call_args_list[1].kwargs + assert redis_call["primaryId"] == "aa:bb:cc:dd:ee:ff" # same host, not skipped for lacking a LAN MAC + assert redis_call["watched3"] == "bridge" + assert redis_call["watched4"] == "null" # no LAN-visible MAC for a bridge-only container + assert redis_call["extra"] == "null" + + +def test_process_host_skips_unconfigured_entry_without_any_request(): + plugin_objects = MagicMock() + with patch("requests.get") as mock_get: + added = dockerdisc.process_host({"DOCKERDISC_SOCKET_PROXY_URL": "", "DOCKERDISC_HOST_MAC": ""}, 5, plugin_objects) + assert added == 0 + mock_get.assert_not_called() + plugin_objects.add_object.assert_not_called() + + +def test_process_host_skips_when_host_mac_unresolved(): + host_entry = {"DOCKERDISC_SOCKET_PROXY_URL": "http://proxy:2375", "DOCKERDISC_HOST_MAC": ""} + plugin_objects = MagicMock() + with patch.object(dockerdisc.DockerHost, "get_info", return_value=None): + added = dockerdisc.process_host(host_entry, 5, plugin_objects) + assert added == 0 + plugin_objects.add_object.assert_not_called() + + +def test_process_host_skips_when_host_not_a_known_device_without_listing_containers(): + host_entry = {"DOCKERDISC_SOCKET_PROXY_URL": "http://proxy:2375", "DOCKERDISC_HOST_MAC": "aa:bb:cc:dd:ee:ff"} + plugin_objects = MagicMock() + with patch.object(dockerdisc.DockerHost, "get_info", return_value=None): + with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([])): # not found + with patch.object(dockerdisc.DockerHost, "get_containers") as mock_get_containers: + added = dockerdisc.process_host(host_entry, 5, plugin_objects) + assert added == 0 + mock_get_containers.assert_not_called() + plugin_objects.add_object.assert_not_called() + + +# --------------------------------------------------------------------------- +# _encode_host_entry() / decode_settings_base64 round trip (sanity check +# that the test helper matches what NetAlertX actually sends) +# --------------------------------------------------------------------------- + + +def test_encode_decode_host_entry_round_trip(): + encoded = _encode_host_entry("http://proxy:2375", "aa:bb:cc:dd:ee:ff") + decoded = _decode_settings_base64(encoded) + assert decoded == { + "DOCKERDISC_SOCKET_PROXY_URL": "http://proxy:2375", + "DOCKERDISC_HOST_MAC": "aa:bb:cc:dd:ee:ff", + } From 6a26804a5e68e1a3b7f83cc19a6e9b5c2ccb713d Mon Sep 17 00:00:00 2001 From: Mauricio Camayo Date: Mon, 14 Sep 2026 10:01:40 -0500 Subject: [PATCH 2/6] Address CodeRabbit review: request timeout budget, shape validation, ambiguous devName, README fix - 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. --- server/plugins/dockerdisc/README.md | 9 +- server/plugins/dockerdisc/config.json | 10 +- server/plugins/dockerdisc/script.py | 91 +++++++++---- test/plugins/test_dockerdisc.py | 178 ++++++++++++++++++++------ 4 files changed, 222 insertions(+), 66 deletions(-) diff --git a/server/plugins/dockerdisc/README.md b/server/plugins/dockerdisc/README.md index de76b23c..01dcbe44 100644 --- a/server/plugins/dockerdisc/README.md +++ b/server/plugins/dockerdisc/README.md @@ -86,9 +86,12 @@ services: Same file, on purpose: - Compose puts every service in one file on the same default network - automatically, so NetAlertX can reach it at `http://127.0.0.1:2375` or - `http://docker-socket-proxy:2375` for free - no extra `networks:` config, - no port published to the LAN (nothing else needs to reach it). + automatically, so NetAlertX can reach it at `http://docker-socket-proxy:2375` + for free - no extra `networks:` config, no port published to the LAN + (nothing else needs to reach it). (`http://127.0.0.1:2375` only applies + under the `network_mode: host` case above, not this default one - under + default bridge networking each container has its own loopback, so + `127.0.0.1` inside NetAlertX wouldn't reach the proxy container.) - Its lifecycle naturally follows NetAlertX's - one `docker compose up`/ `down` brings both up or down together, instead of a second stack to remember to manage separately. diff --git a/server/plugins/dockerdisc/config.json b/server/plugins/dockerdisc/config.json index 02a37543..f18e1e0d 100644 --- a/server/plugins/dockerdisc/config.json +++ b/server/plugins/dockerdisc/config.json @@ -40,7 +40,15 @@ "string": "Enriquece los hosts Docker que NetAlertX ya conoce con la lista de contenedores que corren en ellos - imagen, proyecto/servicio de Compose, driver de red y (cuando el contenedor tiene uno) su propio MAC/IP. Nunca crea devices; se conecta vía un Docker Socket Proxy de solo lectura, nunca directo a `/var/run/docker.sock`." } ], - "params": [], + "params": [ + { + "name": "hosts", + "type": "setting", + "value": "DOCKERDISC_hosts", + "base64": true, + "timeoutMultiplier": true + } + ], "database_column_definitions": [ { "column": "index", diff --git a/server/plugins/dockerdisc/script.py b/server/plugins/dockerdisc/script.py index cf02bdd9..79cb8f4e 100644 --- a/server/plugins/dockerdisc/script.py +++ b/server/plugins/dockerdisc/script.py @@ -43,6 +43,7 @@ otherwise" logic, applied here to the host instead of the container). import json import os import sys +import time from urllib.parse import urlencode import requests @@ -84,27 +85,45 @@ class DockerHost: """One configured `hosts` entry: a Docker Socket Proxy endpoint plus the manual host-MAC fallback for it. Does not connect on construction - call get_info()/get_containers() to actually talk to the proxy. Never - raises - a failed host is logged and skipped, not fatal to the run.""" + raises - a failed host is logged and skipped, not fatal to the run. - def __init__(self, proxy_url, manual_mac, run_timeout): + `deadline` is a shared `time.monotonic()` timestamp for the *whole* + run (every host, every request) - not a per-request timeout. Each + request gets whatever's left of that budget, capped at + REQUEST_TIMEOUT_DEFAULT, so one slow/hanging call can't burn the + entire RUN_TIMEOUT kill-timeout by itself and starve every other host + still queued behind it (server/plugin.py enforces RUN_TIMEOUT as the + whole subprocess's hard timeout, not a safe per-call one).""" + + def __init__(self, proxy_url, manual_mac, deadline): self.proxy_url = (proxy_url or '').rstrip('/') self.manual_mac = normalize_mac(manual_mac) if manual_mac else None - self.run_timeout = run_timeout + self.deadline = deadline @property def configured(self): return bool(self.proxy_url) - def _get(self, path): + def _get(self, path, expected_type=None): """GET against this host's Socket Proxy. Returns the parsed JSON - body, or None (logging why) on any failure.""" + body, or None (logging why) on any failure - including the run's + timeout budget already being exhausted, or a response whose shape + doesn't match `expected_type` (a malformed/unexpected payload, + e.g. from a misconfigured or incompatible Socket Proxy - a plain + `dict`/`list` mismatch here would otherwise surface as a much + less obvious AttributeError/TypeError further down in a caller).""" + remaining = self.deadline - time.monotonic() + if remaining <= 0: + mylog('none', [f'[{pluginName}] {self.proxy_url}: run timeout budget exhausted before requesting {path} - skipping.']) + return None + try: resp = requests.get( self.proxy_url + path, - timeout=self.run_timeout, + timeout=min(remaining, REQUEST_TIMEOUT_DEFAULT), ) resp.raise_for_status() - return resp.json() + data = resp.json() except requests.exceptions.Timeout: mylog('none', [f'[{pluginName}] {self.proxy_url}: request to {path} timed out. Try increasing the run timeout.']) return None @@ -115,18 +134,28 @@ class DockerHost: mylog('none', [f'[{pluginName}] {self.proxy_url}: unexpected error on {path}: {e}']) return None + if expected_type is not None and not isinstance(data, expected_type): + mylog('none', [ + f'[{pluginName}] {self.proxy_url}: unexpected response shape from {path} ' + f'(expected {expected_type.__name__}, got {type(data).__name__}) - ' + 'check the Socket Proxy version/URL.' + ]) + return None + + return data + def get_info(self): """Docker Engine API /info - used only for host-MAC auto-detection (the daemon's `Name`, i.e. hostname). Requires the Socket Proxy's INFO=1 permission; returns None if that's not granted or /info otherwise fails, in which case callers fall back to manual_mac.""" - return self._get('/info') + return self._get('/info', expected_type=dict) def get_containers(self): """Docker Engine API /containers/json (running containers only, matching the default `all=false`) - includes NetworkSettings and Labels, which is all this plugin needs. Requires CONTAINERS=1.""" - return self._get('/containers/json') or [] + return self._get('/containers/json', expected_type=list) or [] def get_network_drivers(self, network_ids): """{NetworkID: Driver} for the given network IDs, in one batched @@ -141,12 +170,12 @@ class DockerHost: return {} query = urlencode({'filters': json.dumps({'id': network_ids})}) - networks = self._get(f'/networks?{query}') + networks = self._get(f'/networks?{query}', expected_type=list) if networks is None: mylog('verbose', [f'[{pluginName}] {self.proxy_url}: could not read /networks (needs the Socket Proxy NETWORKS=1 permission) - network driver will show as empty.']) return {} - return {n['Id']: n.get('Driver') for n in networks if 'Id' in n} + return {n['Id']: n.get('Driver') for n in networks if isinstance(n, dict) and 'Id' in n} def resolve_host_mac(host): @@ -168,21 +197,31 @@ def resolve_host_mac(host): conn = get_temp_db_connection() cursor = conn.cursor() cursor.execute( - "SELECT devMac FROM Devices WHERE devName = ? COLLATE NOCASE LIMIT 1", + "SELECT devMac FROM Devices WHERE devName = ? COLLATE NOCASE", (hostname.lstrip('/'),), ) - row = cursor.fetchone() + rows = cursor.fetchall() conn.close() - if row: + if len(rows) == 1: mylog('verbose', [f'[{pluginName}] {host.proxy_url}: auto-detected host MAC via hostname "{hostname}".']) - return normalize_mac(row[0]) + return normalize_mac(rows[0][0]) - mylog( - 'verbose', - [f'[{pluginName}] {host.proxy_url}: /info hostname "{hostname}" has no matching Devices.devName - ' - 'falling back to the manually configured host MAC, if any.'], - ) + if len(rows) > 1: + # devName isn't unique across Devices - guessing which one is + # this host would risk attaching every container to the wrong + # device. Ambiguous, same as "no match": fall back to manual. + mylog( + 'verbose', + [f'[{pluginName}] {host.proxy_url}: /info hostname "{hostname}" matches {len(rows)} devices, ' + 'ambiguous - falling back to the manually configured host MAC, if any.'], + ) + else: + mylog( + 'verbose', + [f'[{pluginName}] {host.proxy_url}: /info hostname "{hostname}" has no matching Devices.devName - ' + 'falling back to the manually configured host MAC, if any.'], + ) else: mylog( 'verbose', @@ -243,11 +282,11 @@ def first_network_driver(networks, driver_by_id): return None -def process_host(host_entry, run_timeout, plugin_objects): +def process_host(host_entry, deadline, plugin_objects): host = DockerHost( proxy_url=host_entry.get('DOCKERDISC_SOCKET_PROXY_URL'), manual_mac=host_entry.get('DOCKERDISC_HOST_MAC'), - run_timeout=run_timeout, + deadline=deadline, ) if not host.configured: @@ -317,6 +356,12 @@ def main(): host_configs = get_setting_value('DOCKERDISC_hosts') or [] run_timeout = get_setting_value('DOCKERDISC_RUN_TIMEOUT') or REQUEST_TIMEOUT_DEFAULT + # One shared deadline for the whole run (every host, every request) - + # config.json's "hosts" param has timeoutMultiplier set, so the outer + # kill-timeout (server/plugin.py) already scales with host count; this + # mirrors that budget inside the script itself, so one slow host can't + # eat every other host's share of it. See DockerHost._get(). + deadline = time.monotonic() + run_timeout mylog('verbose', [f'[{pluginName}] number of configured hosts: {len(host_configs)}']) @@ -325,7 +370,7 @@ def main(): total_added = 0 for host_config in host_configs: host_entry = decode_settings_base64(host_config) - total_added += process_host(host_entry, run_timeout, plugin_objects) + total_added += process_host(host_entry, deadline, plugin_objects) plugin_objects.write_result_file() diff --git a/test/plugins/test_dockerdisc.py b/test/plugins/test_dockerdisc.py index 3e747ef8..82a92588 100644 --- a/test/plugins/test_dockerdisc.py +++ b/test/plugins/test_dockerdisc.py @@ -23,13 +23,20 @@ Layout: GET /networks?filters=... call - one request for every unique NetworkID, not one per network, and a safe {} (not a crash) when the Socket Proxy denies it (missing NETWORKS=1). + - DockerHost._get(): unit tests for the shared timeout-budget/shape- + validation logic every request goes through - a run whose deadline is + already exhausted skips the request entirely, a request's own timeout + is capped by however much budget is left, and a response whose shape + doesn't match what the caller expects (e.g. /info returning a list + instead of a dict) is rejected the same as a network failure, rather + than crashing a caller further down that assumes the expected shape. - resolve_host_mac(): unit tests for the manual-MAC-short-circuits- without-any-request-first, else /info -> devName match chain (spec §3.2) - a configured DOCKERDISC_HOST_MAC wins immediately with zero Socket Proxy calls (deliberate: no auto-re-verification once you've told us the answer), so auto-detection only ever runs when it's - empty, and only then can hostname-unmatched or /info-unreachable - resolve to None. + empty, and only then can hostname-unmatched, ambiguous (more than one + device sharing that name), or /info-unreachable resolve to None. - lookup_device_mac(): unit test for the "host must already exist, this plugin never creates it" gate - explicit COLLATE NOCASE, not just relied on from the Devices.devMac column definition. @@ -50,6 +57,7 @@ import base64 import importlib.util import json import sys +import time import types from pathlib import Path from unittest.mock import MagicMock, patch @@ -146,6 +154,14 @@ def _load_dockerdisc_module(): dockerdisc = _load_dockerdisc_module() +def _deadline(seconds=5): + """A `time.monotonic()`-based deadline `seconds` in the future - what + DockerHost's real constructor now takes (a shared run deadline, not a + per-request timeout duration). Computed fresh per call so tests never + share a slowly-expiring value.""" + return time.monotonic() + seconds + + def _resp(json_data): resp = MagicMock() resp.raise_for_status = MagicMock() @@ -154,12 +170,17 @@ def _resp(json_data): def _db_returning(rows): - """A get_temp_db_connection() replacement whose cursor().fetchone() - yields successive `rows` entries (one per execute() call), then None.""" + """A get_temp_db_connection() replacement whose cursor supports both + query styles used in this plugin: fetchone() (lookup_device_mac's + EXISTS-style check) yields successive `rows` entries (one per + execute() call), then None; fetchall() (resolve_host_mac's devName + match, which needs every matching row to detect ambiguity) returns + `rows` as-is. A given test only ever exercises one of the two.""" conn = MagicMock() cursor = MagicMock() conn.cursor.return_value = cursor cursor.fetchone.side_effect = list(rows) + [None] * 10 + cursor.fetchall.return_value = list(rows) return conn @@ -235,7 +256,7 @@ def test_first_network_driver_missing_from_driver_lookup(): def test_get_network_drivers_batches_into_one_request(): - host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) networks_resp = _resp([ {"Id": "net-a", "Driver": "macvlan"}, {"Id": "net-b", "Driver": "bridge"}, @@ -248,7 +269,7 @@ def test_get_network_drivers_batches_into_one_request(): def test_get_network_drivers_empty_input_makes_no_request(): - host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) with patch("requests.get") as mock_get: assert host.get_network_drivers([]) == {} mock_get.assert_not_called() @@ -257,11 +278,89 @@ def test_get_network_drivers_empty_input_makes_no_request(): def test_get_network_drivers_denied_permission_returns_empty_dict(): """Socket Proxy without NETWORKS=1 - request fails, callers must fall back to an empty lookup rather than crash.""" - host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) with patch("requests.get", side_effect=requests.exceptions.ConnectionError("denied")): assert host.get_network_drivers(["net-a"]) == {} +def test_get_network_drivers_skips_non_dict_entries(): + """A malformed element inside an otherwise-list /networks response + (still passes the list-shape check) must be skipped, not crash on + `'Id' in n` for a non-dict `n`.""" + host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) + with patch("requests.get", return_value=_resp(["not-a-dict", {"Id": "net-a", "Driver": "macvlan"}])): + assert host.get_network_drivers(["net-a"]) == {"net-a": "macvlan"} + + +# --------------------------------------------------------------------------- +# DockerHost._get() - shared timeout-budget and shape-validation logic +# --------------------------------------------------------------------------- + + +def test_dockerhost_get_timeout_returns_none(): + host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) + with patch("requests.get", side_effect=requests.exceptions.Timeout("slow")): + assert host.get_info() is None + + +def test_dockerhost_get_connection_error_returns_none(): + host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) + with patch("requests.get", side_effect=requests.exceptions.ConnectionError("no route")): + assert host.get_containers() == [] + + +def test_dockerhost_get_containers_success(): + host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) + with patch("requests.get", return_value=_resp([{"Id": "abc123"}])): + assert host.get_containers() == [{"Id": "abc123"}] + + +def test_dockerhost_get_skips_request_when_deadline_already_passed(): + """The whole run's timeout budget is already exhausted (e.g. an + earlier host/request ate it all) - the request must not even be + attempted, not fail after a full REQUEST_TIMEOUT_DEFAULT wait.""" + host = dockerdisc.DockerHost("http://proxy:2375", "", time.monotonic() - 1) + with patch("requests.get") as mock_get: + assert host.get_info() is None + mock_get.assert_not_called() + + +def test_dockerhost_get_caps_request_timeout_to_remaining_budget(): + """With only a little run budget left, the individual request's own + timeout must be capped to that remaining amount, not the full + REQUEST_TIMEOUT_DEFAULT - so one host near the end of the shared + deadline can't still block for the plugin's whole default timeout.""" + host = dockerdisc.DockerHost("http://proxy:2375", "", time.monotonic() + 2) + with patch("requests.get", return_value=_resp({"Name": "x"})) as mock_get: + host.get_info() + used_timeout = mock_get.call_args.kwargs["timeout"] + assert 0 < used_timeout <= 2 + + +def test_dockerhost_get_caps_request_timeout_to_request_default_when_budget_is_large(): + """The reverse: plenty of run budget left, but a single request still + shouldn't be allowed to run longer than REQUEST_TIMEOUT_DEFAULT.""" + host = dockerdisc.DockerHost("http://proxy:2375", "", time.monotonic() + 3600) + with patch("requests.get", return_value=_resp({"Name": "x"})) as mock_get: + host.get_info() + assert mock_get.call_args.kwargs["timeout"] == dockerdisc.REQUEST_TIMEOUT_DEFAULT + + +def test_dockerhost_get_rejects_wrong_shape_dict_expected_got_list(): + """/info returning a list instead of a dict (malformed/incompatible + Socket Proxy) must be rejected the same as a network failure - not + handed to a caller that assumes `.get()` works on it.""" + host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) + with patch("requests.get", return_value=_resp(["unexpected"])): + assert host.get_info() is None + + +def test_dockerhost_get_rejects_wrong_shape_list_expected_got_dict(): + host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) + with patch("requests.get", return_value=_resp({"unexpected": True})): + assert host.get_containers() == [] + + # --------------------------------------------------------------------------- # resolve_host_mac() # --------------------------------------------------------------------------- @@ -275,14 +374,14 @@ def test_resolve_host_mac_manual_mac_short_circuits_without_any_request(): ever runs when the field is left blank - filling it in trades away the self-healing "auto-detect keeps re-verifying it" behavior for the saved request, on purpose.""" - host = dockerdisc.DockerHost("http://proxy:2375", "11:22:33:44:55:66", 5) + host = dockerdisc.DockerHost("http://proxy:2375", "11:22:33:44:55:66", _deadline()) with patch.object(host, "get_info") as mock_get_info: assert dockerdisc.resolve_host_mac(host) == "11:22:33:44:55:66" mock_get_info.assert_not_called() def test_resolve_host_mac_auto_detect_success(): - host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) with patch.object(host, "get_info", return_value={"Name": "docker-host-1"}): with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([("AA:BB:CC:DD:EE:FF",)])): assert dockerdisc.resolve_host_mac(host) == "aa:bb:cc:dd:ee:ff" @@ -293,18 +392,42 @@ def test_resolve_host_mac_none_when_hostname_unmatched_and_no_manual_mac(): nothing - the "fall back to manual" path only exists when manual is actually set, and when it is, it short-circuits before this branch is ever reached (see the short-circuit test above).""" - host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) with patch.object(host, "get_info", return_value={"Name": "unknown-host"}): with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([])): assert dockerdisc.resolve_host_mac(host) is None def test_resolve_host_mac_none_when_info_unreachable_and_no_manual_mac(): - host = dockerdisc.DockerHost("http://proxy:2375", "", 5) + host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) with patch.object(host, "get_info", return_value=None): assert dockerdisc.resolve_host_mac(host) is None +def test_resolve_host_mac_ambiguous_hostname_falls_back_to_none_without_manual(): + """Two devices share the auto-detected hostname (devName isn't unique) + - picking either one arbitrarily could attach every container to the + wrong device, so this must resolve the same as "no match" rather than + guessing.""" + host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) + with patch.object(host, "get_info", return_value={"Name": "htpc"}): + with patch.object( + dockerdisc, "get_temp_db_connection", + return_value=_db_returning([("AA:BB:CC:DD:EE:01",), ("AA:BB:CC:DD:EE:02",)]), + ): + assert dockerdisc.resolve_host_mac(host) is None + + +def test_resolve_host_mac_ambiguous_hostname_falls_back_to_manual_when_set(): + """Same ambiguous-hostname case, but with a manual MAC configured - + that manual value wins immediately (see the short-circuit test), so + the ambiguous auto-detect branch is never even reached.""" + host = dockerdisc.DockerHost("http://proxy:2375", "11:22:33:44:55:66", _deadline()) + with patch.object(host, "get_info") as mock_get_info: + assert dockerdisc.resolve_host_mac(host) == "11:22:33:44:55:66" + mock_get_info.assert_not_called() + + # --------------------------------------------------------------------------- # lookup_device_mac() # --------------------------------------------------------------------------- @@ -320,29 +443,6 @@ def test_lookup_device_mac_not_found(): assert dockerdisc.lookup_device_mac("aa:bb:cc:dd:ee:ff") is False -# --------------------------------------------------------------------------- -# DockerHost._get() error handling (never raises) -# --------------------------------------------------------------------------- - - -def test_dockerhost_get_timeout_returns_none(): - host = dockerdisc.DockerHost("http://proxy:2375", "", 5) - with patch("requests.get", side_effect=requests.exceptions.Timeout("slow")): - assert host.get_info() is None - - -def test_dockerhost_get_connection_error_returns_none(): - host = dockerdisc.DockerHost("http://proxy:2375", "", 5) - with patch("requests.get", side_effect=requests.exceptions.ConnectionError("no route")): - assert host.get_containers() == [] - - -def test_dockerhost_get_containers_success(): - host = dockerdisc.DockerHost("http://proxy:2375", "", 5) - with patch("requests.get", return_value=_resp([{"Id": "abc123"}])): - assert host.get_containers() == [{"Id": "abc123"}] - - # --------------------------------------------------------------------------- # process_host() # --------------------------------------------------------------------------- @@ -384,13 +484,13 @@ def test_process_host_mixed_macvlan_and_bridge_containers(): plugin_objects = MagicMock() plugin_objects.add_object = MagicMock() - host = dockerdisc.DockerHost(host_entry["DOCKERDISC_SOCKET_PROXY_URL"], host_entry["DOCKERDISC_HOST_MAC"], 5) + host = dockerdisc.DockerHost(host_entry["DOCKERDISC_SOCKET_PROXY_URL"], host_entry["DOCKERDISC_HOST_MAC"], _deadline()) with patch.object(dockerdisc, "DockerHost", return_value=host): with patch.object(host, "get_info", return_value=None): # unused - manual_mac short-circuits before this would ever be called with patch.object(host, "get_containers", return_value=containers): with patch.object(host, "get_network_drivers", return_value={"net-lan": "macvlan", "net-bridge": "bridge"}) as mock_drivers: with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([(1,)])): - added = dockerdisc.process_host(host_entry, 5, plugin_objects) + added = dockerdisc.process_host(host_entry, _deadline(), plugin_objects) # one batched call for both containers' networks, not two mock_drivers.assert_called_once() @@ -418,7 +518,7 @@ def test_process_host_mixed_macvlan_and_bridge_containers(): def test_process_host_skips_unconfigured_entry_without_any_request(): plugin_objects = MagicMock() with patch("requests.get") as mock_get: - added = dockerdisc.process_host({"DOCKERDISC_SOCKET_PROXY_URL": "", "DOCKERDISC_HOST_MAC": ""}, 5, plugin_objects) + added = dockerdisc.process_host({"DOCKERDISC_SOCKET_PROXY_URL": "", "DOCKERDISC_HOST_MAC": ""}, _deadline(), plugin_objects) assert added == 0 mock_get.assert_not_called() plugin_objects.add_object.assert_not_called() @@ -428,7 +528,7 @@ def test_process_host_skips_when_host_mac_unresolved(): host_entry = {"DOCKERDISC_SOCKET_PROXY_URL": "http://proxy:2375", "DOCKERDISC_HOST_MAC": ""} plugin_objects = MagicMock() with patch.object(dockerdisc.DockerHost, "get_info", return_value=None): - added = dockerdisc.process_host(host_entry, 5, plugin_objects) + added = dockerdisc.process_host(host_entry, _deadline(), plugin_objects) assert added == 0 plugin_objects.add_object.assert_not_called() @@ -439,7 +539,7 @@ def test_process_host_skips_when_host_not_a_known_device_without_listing_contain with patch.object(dockerdisc.DockerHost, "get_info", return_value=None): with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([])): # not found with patch.object(dockerdisc.DockerHost, "get_containers") as mock_get_containers: - added = dockerdisc.process_host(host_entry, 5, plugin_objects) + added = dockerdisc.process_host(host_entry, _deadline(), plugin_objects) assert added == 0 mock_get_containers.assert_not_called() plugin_objects.add_object.assert_not_called() From 3b83d2403bb44a8809b37c91f4dd1b01c4524126 Mon Sep 17 00:00:00 2001 From: Mauricio Camayo Date: Tue, 15 Sep 2026 11:58:10 -0500 Subject: [PATCH 3/6] Address jokob-sk review: DeviceInstance instead of raw SQL, drop HTML entity/partial translations/dead spec-file reference - 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. --- server/plugins/dockerdisc/config.json | 106 +------------------------- server/plugins/dockerdisc/script.py | 35 +++------ test/plugins/test_dockerdisc.py | 56 +++++++------- 3 files changed, 40 insertions(+), 157 deletions(-) diff --git a/server/plugins/dockerdisc/config.json b/server/plugins/dockerdisc/config.json index f18e1e0d..3b654589 100644 --- a/server/plugins/dockerdisc/config.json +++ b/server/plugins/dockerdisc/config.json @@ -14,14 +14,6 @@ { "language_code": "en_us", "string": "Docker discovery" - }, - { - "language_code": "es_es", - "string": "Descubrimiento de Docker" - }, - { - "language_code": "de_de", - "string": "Docker-Erkennung" } ], "icon": [ @@ -34,10 +26,6 @@ { "language_code": "en_us", "string": "Enriches known Docker hosts with their running containers - image, Compose project/service, network, and MAC/IP when available. Never creates devices; connects via a read-only Docker Socket Proxy." - }, - { - "language_code": "es_es", - "string": "Enriquece los hosts Docker que NetAlertX ya conoce con la lista de contenedores que corren en ellos - imagen, proyecto/servicio de Compose, driver de red y (cuando el contenedor tiene uno) su propio MAC/IP. Nunca crea devices; se conecta vía un Docker Socket Proxy de solo lectura, nunca directo a `/var/run/docker.sock`." } ], "params": [ @@ -251,14 +239,6 @@ { "language_code": "en_us", "string": "Comments" - }, - { - "language_code": "es_es", - "string": "Comentarios" - }, - { - "language_code": "de_de", - "string": "Kommentare" } ] }, @@ -293,14 +273,6 @@ { "language_code": "en_us", "string": "Status" - }, - { - "language_code": "es_es", - "string": "Estado" - }, - { - "language_code": "de_de", - "string": "Status" } ] } @@ -336,24 +308,12 @@ { "language_code": "en_us", "string": "When to run" - }, - { - "language_code": "es_es", - "string": "Cuando ejecuta" - }, - { - "language_code": "de_de", - "string": "Wann ausführen" } ], "description": [ { "language_code": "en_us", "string": "Enable a regular Docker discovery run. If you select schedule the scheduling settings from below are applied. If you select once the scan is run only once on start of the application (container) for the time specified in DOCKERDISC_RUN_TIMEOUT setting." - }, - { - "language_code": "es_es", - "string": "Habilita una ejecución periódica de descubrimiento de Docker. Si selecciona schedule se aplican las opciones de programación de abajo. Si selecciona once el escaneo se ejecuta solo una vez al iniciar la aplicación (contenedor) durante el tiempo especificado en la configuración DOCKERDISC_RUN_TIMEOUT." } ] }, @@ -383,28 +343,12 @@ { "language_code": "en_us", "string": "Command" - }, - { - "language_code": "es_es", - "string": "Comando" - }, - { - "language_code": "de_de", - "string": "Befehl" } ], "description": [ { "language_code": "en_us", "string": "Command to run" - }, - { - "language_code": "es_es", - "string": "Comando a ejecutar" - }, - { - "language_code": "de_de", - "string": "Auszuführender Befehl" } ] }, @@ -449,24 +393,12 @@ { "language_code": "en_us", "string": "Schedule" - }, - { - "language_code": "es_es", - "string": "Schedule" - }, - { - "language_code": "de_de", - "string": "Zeitplan" } ], "description": [ { "language_code": "en_us", "string": "Only enabled if you select schedule in the DOCKERDISC_RUN setting. Make sure you enter the schedule in the correct cron-like format (e.g. validate at crontab.guru). For example entering 0 4 * * * will run the scan after 4 am in the TIMEZONE you set above. Will be run NEXT time the time passes." - }, - { - "language_code": "es_es", - "string": "Solo habilitado si selecciona schedule en la configuración DOCKERDISC_RUN. Asegúrese de ingresar el schedule en el formato similar a cron correcto (por ejemplo, valide en crontab.guru). Por ejemplo, ingrese 0 4 * * * ejecutará el escaneo después de las 4 am en el TIMEZONE que configuró arriba. Se ejecutará la PRÓXIMA vez que pase el tiempo." } ] }, @@ -496,24 +428,12 @@ { "language_code": "en_us", "string": "Run timeout" - }, - { - "language_code": "es_es", - "string": "Tiempo de espera de ejecución" - }, - { - "language_code": "de_de", - "string": "Zeitlimit" } ], "description": [ { "language_code": "en_us", "string": "Maximum time in seconds to wait for the script to finish. If this time is exceeded the script is aborted." - }, - { - "language_code": "es_es", - "string": "Tiempo máximo en segundos para esperar a que finalice el script. Si se supera este tiempo, el script se cancela." } ] }, @@ -700,7 +620,7 @@ "description": [ { "language_code": "en_us", - "string": "One entry per Docker host to track. Each entry pairs a read-only Docker Socket Proxy URL with that host's device (auto-detected, or entered manually as a fallback). Every container found on a host is listed under that host's own Device Details → Plugins → DOCKERDISC tab - the host device must already exist in NetAlertX (via ARP/Nmap); this plugin never creates devices." + "string": "One entry per Docker host to track. Each entry pairs a read-only Docker Socket Proxy URL with that host's device (auto-detected, or entered manually as a fallback). Every container found on a host is listed under that host's own Device Details -> Plugins -> DOCKERDISC tab - the host device must already exist in NetAlertX (via ARP/Nmap); this plugin never creates devices." } ] }, @@ -736,24 +656,12 @@ { "language_code": "en_us", "string": "Watched" - }, - { - "language_code": "es_es", - "string": "Visto" - }, - { - "language_code": "de_de", - "string": "Überwacht" } ], "description": [ { "language_code": "en_us", "string": "Send a notification if selected values change. Use CTRL + Click to select/deselect.
  • watchedValue1 is the container image
  • watchedValue2 is the Compose project/service
  • watchedValue3 is the network driver
  • watchedValue4 is the container's own MAC, when it has one
" - }, - { - "language_code": "es_es", - "string": "Envíe una notificación si los valores seleccionados cambian. Use CTRL + Clic para seleccionar/deseleccionar.
  • watchedValue1 es la imagen del contenedor
  • watchedValue2 es el proyecto/servicio de Compose
  • watchedValue3 es el driver de red
  • watchedValue4 es el MAC propio del contenedor, cuando tiene uno
" } ] }, @@ -789,24 +697,12 @@ { "language_code": "en_us", "string": "Report on" - }, - { - "language_code": "es_es", - "string": "Informar sobre" - }, - { - "language_code": "de_de", - "string": "Benachrichtige wenn" } ], "description": [ { "language_code": "en_us", "string": "Send a notification only on these statuses. new means a new unique (unique combination of PrimaryId and SecondaryId) object was discovered. watched-changed means that selected watchedValueN columns changed." - }, - { - "language_code": "es_es", - "string": "Envíe una notificación solo en estos estados. new significa que se descubrió un nuevo objeto único (combinación única de PrimaryId y SecondaryId). watched-changed significa que las columnas watchedValueN seleccionadas cambiaron." } ] } diff --git a/server/plugins/dockerdisc/script.py b/server/plugins/dockerdisc/script.py index 79cb8f4e..4f270e9a 100644 --- a/server/plugins/dockerdisc/script.py +++ b/server/plugins/dockerdisc/script.py @@ -6,8 +6,8 @@ sole source of device presence. Instead, for each configured Docker host this plugin lists that host's containers under the *host's own* Device Details -> Plugins -> DOCKERDISC tab. -Design ("Device = Docker host -> List of containers", per maintainer -jokob-sk, see ../../../PLUGIN_DOCKERDISC_SPEC.md for the full history): +Design ("Device = Docker host -> List of containers", agreed with maintainer +jokob-sk across several rounds on issue #1721): - objectPrimaryId / foreignKey is always the Docker HOST's MAC - never a container's own MAC. Every plugin object (one per container) attaches @@ -24,8 +24,7 @@ jokob-sk, see ../../../PLUGIN_DOCKERDISC_SPEC.md for the full history): proxy's own /info endpoint) doesn't resolve to a known device. Never connects to /var/run/docker.sock directly. -Verified 2026-09-08 against a real Docker Engine + docker-socket-proxy -(see PLUGIN_DOCKERDISC_SPEC.md §9 for the open questions this closed): +Verified 2026-09-08 against a real Docker Engine + docker-socket-proxy: `GET /containers/json`'s `NetworkSettings.Networks.` does NOT carry a `Driver` field inline (only NetworkID/Gateway/IPAddress/MacAddress/...) - the driver has to come from a separate `GET /networks` call, filtered by @@ -60,7 +59,7 @@ from plugin_helper import ( # noqa: E402 from logger import mylog, Logger # noqa: E402 from helper import get_setting_value # noqa: E402 from const import logPath # noqa: E402 -from database import get_temp_db_connection # noqa: E402 +from models.device_instance import DeviceInstance # noqa: E402 import conf # noqa: E402 from pytz import timezone # noqa: E402 @@ -194,18 +193,11 @@ def resolve_host_mac(host): hostname = (info or {}).get('Name') if hostname: - conn = get_temp_db_connection() - cursor = conn.cursor() - cursor.execute( - "SELECT devMac FROM Devices WHERE devName = ? COLLATE NOCASE", - (hostname.lstrip('/'),), - ) - rows = cursor.fetchall() - conn.close() + rows = DeviceInstance().getAllByName(hostname.lstrip('/')) if len(rows) == 1: mylog('verbose', [f'[{pluginName}] {host.proxy_url}: auto-detected host MAC via hostname "{hostname}".']) - return normalize_mac(rows[0][0]) + return normalize_mac(rows[0]['devMac']) if len(rows) > 1: # devName isn't unique across Devices - guessing which one is @@ -235,17 +227,10 @@ def resolve_host_mac(host): def lookup_device_mac(mac): """True if `mac` already exists as a Devices row - this plugin never creates the host device, same rule vendor_update applies to the - devices it enriches. COLLATE NOCASE is explicit here (not just relied - on from the Devices.devMac column definition) so this still matches - correctly even if that ever changes - normalize_mac() lowercases what - we search for, but what's actually stored can come from other - discovery methods and isn't guaranteed to be lowercase.""" - conn = get_temp_db_connection() - cursor = conn.cursor() - cursor.execute("SELECT 1 FROM Devices WHERE devMac = ? COLLATE NOCASE LIMIT 1", (mac,)) - row = cursor.fetchone() - conn.close() - return row is not None + devices it enriches. Delegates the actual matching (case sensitivity + included) to DeviceInstance.getByMac() - the core's own contract for + what "the same MAC" means, not something this plugin second-guesses.""" + return DeviceInstance().getByMac(mac) is not None def pick_lan_network(networks, driver_by_id): diff --git a/test/plugins/test_dockerdisc.py b/test/plugins/test_dockerdisc.py index 82a92588..5f32d2ad 100644 --- a/test/plugins/test_dockerdisc.py +++ b/test/plugins/test_dockerdisc.py @@ -1,13 +1,17 @@ """Tests for the dockerdisc (DOCKERDISC) plugin. script.py is loaded with its NetAlertX-internal dependencies -(plugin_helper, logger, helper, const, conf, pytz, database) stubbed out, -the same approach test_pihole_monitor.py uses - it keeps these tests -runnable without the full devcontainer environment and without a live +(plugin_helper, logger, helper, const, models.device_instance, conf, pytz) +stubbed out, the same approach test_pihole_monitor.py uses - it keeps these +tests runnable without the full devcontainer environment and without a live Docker Socket Proxy. `requests` itself is left real; individual HTTP calls are mocked per test. `handleEmpty`/`normalize_mac`/`decode_settings_base64` are reimplemented locally (same shape as plugin_helper's) rather than imported, to avoid pulling in plugin_helper's own dependency chain. +`models.device_instance` is stubbed with a fake `DeviceInstance` rather than +letting the real one load, since the real module pulls in its own separate +dependency chain (db.db_helper, workflows.constants, ...) this harness +doesn't otherwise need. Layout: - pick_lan_network() / first_network_driver(): pure-function unit tests @@ -37,9 +41,9 @@ Layout: told us the answer), so auto-detection only ever runs when it's empty, and only then can hostname-unmatched, ambiguous (more than one device sharing that name), or /info-unreachable resolve to None. + Delegates the actual devName lookup to DeviceInstance.getAllByName(). - lookup_device_mac(): unit test for the "host must already exist, this - plugin never creates it" gate - explicit COLLATE NOCASE, not just - relied on from the Devices.devMac column definition. + plugin never creates it" gate - delegates to DeviceInstance.getByMac(). - process_host(): integration tests with DockerHost's network-touching methods stubbed at the object level - covers a mixed macvlan+bridge container list (only the macvlan one gets a MAC/IP), an unconfigured @@ -132,7 +136,7 @@ def _load_dockerdisc_module(): stub("logger", mylog=MagicMock(), Logger=MagicMock()) stub("helper", get_setting_value=MagicMock(return_value="UTC")) stub("const", logPath="/tmp") - stub("database", get_temp_db_connection=MagicMock()) + stub("models.device_instance", DeviceInstance=MagicMock) stub("conf", tz=None) stub("pytz", timezone=MagicMock(return_value="UTC")) @@ -169,19 +173,17 @@ def _resp(json_data): return resp -def _db_returning(rows): - """A get_temp_db_connection() replacement whose cursor supports both - query styles used in this plugin: fetchone() (lookup_device_mac's - EXISTS-style check) yields successive `rows` entries (one per - execute() call), then None; fetchall() (resolve_host_mac's devName - match, which needs every matching row to detect ambiguity) returns - `rows` as-is. A given test only ever exercises one of the two.""" - conn = MagicMock() - cursor = MagicMock() - conn.cursor.return_value = cursor - cursor.fetchone.side_effect = list(rows) + [None] * 10 - cursor.fetchall.return_value = list(rows) - return conn +def _stub_device_instance(get_all_by_name=None, get_by_mac=None): + """A DeviceInstance *class* replacement, for `patch.object(dockerdisc, + "DeviceInstance", ...)` - calling it (as resolve_host_mac()/ + lookup_device_mac() do: `DeviceInstance()`) returns a fixed instance + whose getAllByName()/getByMac() return the given values, mirroring the + real model's method names/shapes (getAllByName -> list of device + dicts, getByMac -> a device dict or None).""" + instance = MagicMock() + instance.getAllByName.return_value = get_all_by_name if get_all_by_name is not None else [] + instance.getByMac.return_value = get_by_mac + return MagicMock(return_value=instance) # --------------------------------------------------------------------------- @@ -383,7 +385,7 @@ def test_resolve_host_mac_manual_mac_short_circuits_without_any_request(): def test_resolve_host_mac_auto_detect_success(): host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) with patch.object(host, "get_info", return_value={"Name": "docker-host-1"}): - with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([("AA:BB:CC:DD:EE:FF",)])): + with patch.object(dockerdisc, "DeviceInstance", _stub_device_instance(get_all_by_name=[{"devMac": "AA:BB:CC:DD:EE:FF"}])): assert dockerdisc.resolve_host_mac(host) == "aa:bb:cc:dd:ee:ff" @@ -394,7 +396,7 @@ def test_resolve_host_mac_none_when_hostname_unmatched_and_no_manual_mac(): ever reached (see the short-circuit test above).""" host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) with patch.object(host, "get_info", return_value={"Name": "unknown-host"}): - with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([])): + with patch.object(dockerdisc, "DeviceInstance", _stub_device_instance(get_all_by_name=[])): assert dockerdisc.resolve_host_mac(host) is None @@ -412,8 +414,8 @@ def test_resolve_host_mac_ambiguous_hostname_falls_back_to_none_without_manual() host = dockerdisc.DockerHost("http://proxy:2375", "", _deadline()) with patch.object(host, "get_info", return_value={"Name": "htpc"}): with patch.object( - dockerdisc, "get_temp_db_connection", - return_value=_db_returning([("AA:BB:CC:DD:EE:01",), ("AA:BB:CC:DD:EE:02",)]), + dockerdisc, "DeviceInstance", + _stub_device_instance(get_all_by_name=[{"devMac": "AA:BB:CC:DD:EE:01"}, {"devMac": "AA:BB:CC:DD:EE:02"}]), ): assert dockerdisc.resolve_host_mac(host) is None @@ -434,12 +436,12 @@ def test_resolve_host_mac_ambiguous_hostname_falls_back_to_manual_when_set(): def test_lookup_device_mac_found(): - with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([(1,)])): + with patch.object(dockerdisc, "DeviceInstance", _stub_device_instance(get_by_mac={"devMac": "aa:bb:cc:dd:ee:ff"})): assert dockerdisc.lookup_device_mac("aa:bb:cc:dd:ee:ff") is True def test_lookup_device_mac_not_found(): - with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([])): + with patch.object(dockerdisc, "DeviceInstance", _stub_device_instance(get_by_mac=None)): assert dockerdisc.lookup_device_mac("aa:bb:cc:dd:ee:ff") is False @@ -489,7 +491,7 @@ def test_process_host_mixed_macvlan_and_bridge_containers(): with patch.object(host, "get_info", return_value=None): # unused - manual_mac short-circuits before this would ever be called with patch.object(host, "get_containers", return_value=containers): with patch.object(host, "get_network_drivers", return_value={"net-lan": "macvlan", "net-bridge": "bridge"}) as mock_drivers: - with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([(1,)])): + with patch.object(dockerdisc, "DeviceInstance", _stub_device_instance(get_by_mac={"devMac": host_entry["DOCKERDISC_HOST_MAC"]})): added = dockerdisc.process_host(host_entry, _deadline(), plugin_objects) # one batched call for both containers' networks, not two @@ -537,7 +539,7 @@ def test_process_host_skips_when_host_not_a_known_device_without_listing_contain host_entry = {"DOCKERDISC_SOCKET_PROXY_URL": "http://proxy:2375", "DOCKERDISC_HOST_MAC": "aa:bb:cc:dd:ee:ff"} plugin_objects = MagicMock() with patch.object(dockerdisc.DockerHost, "get_info", return_value=None): - with patch.object(dockerdisc, "get_temp_db_connection", return_value=_db_returning([])): # not found + with patch.object(dockerdisc, "DeviceInstance", _stub_device_instance(get_by_mac=None)): # not found with patch.object(dockerdisc.DockerHost, "get_containers") as mock_get_containers: added = dockerdisc.process_host(host_entry, _deadline(), plugin_objects) assert added == 0 From b0d1776221e74189eb4490247565502a3f3b8110 Mon Sep 17 00:00:00 2001 From: Mauricio Camayo Date: Tue, 15 Sep 2026 12:06:04 -0500 Subject: [PATCH 4/6] Trim module docstring: drop design-history attribution and verification date 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). --- server/plugins/dockerdisc/script.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/server/plugins/dockerdisc/script.py b/server/plugins/dockerdisc/script.py index 4f270e9a..d7c1c4da 100644 --- a/server/plugins/dockerdisc/script.py +++ b/server/plugins/dockerdisc/script.py @@ -6,9 +6,6 @@ sole source of device presence. Instead, for each configured Docker host this plugin lists that host's containers under the *host's own* Device Details -> Plugins -> DOCKERDISC tab. -Design ("Device = Docker host -> List of containers", agreed with maintainer -jokob-sk across several rounds on issue #1721): - - objectPrimaryId / foreignKey is always the Docker HOST's MAC - never a container's own MAC. Every plugin object (one per container) attaches to the host device, which must already exist in NetAlertX (found the @@ -24,13 +21,12 @@ jokob-sk across several rounds on issue #1721): proxy's own /info endpoint) doesn't resolve to a known device. Never connects to /var/run/docker.sock directly. -Verified 2026-09-08 against a real Docker Engine + docker-socket-proxy: `GET /containers/json`'s `NetworkSettings.Networks.` does NOT carry a `Driver` field inline (only NetworkID/Gateway/IPAddress/MacAddress/...) - the driver has to come from a separate `GET /networks` call, filtered by the unique NetworkIDs seen across a host's containers in one batched -request (cacheable per run, as originally anticipated). This needs the -Socket Proxy's NETWORKS=1 permission in addition to CONTAINERS=1/INFO=1. +request. This needs the Socket Proxy's NETWORKS=1 permission in addition +to CONTAINERS=1/INFO=1. Structural references: server/plugins/internet_speedtest/config.json (plugin_type "other", no mapped_to_column - this never writes into From d512e5d84ed4b20ac9068f3c04756d5e960be7ba Mon Sep 17 00:00:00 2001 From: Mauricio Camayo Date: Tue, 15 Sep 2026 12:19:30 -0500 Subject: [PATCH 5/6] Add regression test for lookup_device_mac() case handling CodeRabbit flagged a possible case-sensitivity gap in getByMac() usage. No functional change needed - Devices.devMac is COLLATE NOCASE at the schema level, so getByMac()'s plain equality lookup is already case-insensitive (that's exactly why getAllByName() has to apply it explicitly and getByMac() doesn't - devName has no column collation). This test guards that lookup_device_mac() doesn't do anything of its own that would undo that. --- test/plugins/test_dockerdisc.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/plugins/test_dockerdisc.py b/test/plugins/test_dockerdisc.py index 5f32d2ad..7f425091 100644 --- a/test/plugins/test_dockerdisc.py +++ b/test/plugins/test_dockerdisc.py @@ -445,6 +445,22 @@ def test_lookup_device_mac_not_found(): assert dockerdisc.lookup_device_mac("aa:bb:cc:dd:ee:ff") is False +def test_lookup_device_mac_found_regardless_of_stored_mac_case(): + """The plugin always passes a lowercase, normalize_mac()'d value in - + this only guards that lookup_device_mac() doesn't do anything of its + own (e.g. an exact-string comparison) that would undo whatever + case-insensitivity DeviceInstance.getByMac() provides. The real + guarantee is schema-level - Devices.devMac is declared + `COLLATE NOCASE` (server/db/schema/app.sql), which is exactly why + getByMac() itself doesn't need to apply it explicitly (unlike + getAllByName(), which does - devName has no such column collation; + see that method's docstring and test/backend/test_device_instance.py). + This test's stub can't exercise real SQLite collation, only that this + function's own logic is agnostic to it.""" + with patch.object(dockerdisc, "DeviceInstance", _stub_device_instance(get_by_mac={"devMac": "AA:BB:CC:DD:EE:FF"})): + assert dockerdisc.lookup_device_mac("aa:bb:cc:dd:ee:ff") is True + + # --------------------------------------------------------------------------- # process_host() # --------------------------------------------------------------------------- From 091e648e882bb197760090dc3ac73a0a087ea9cb Mon Sep 17 00:00:00 2001 From: Mauricio Camayo Date: Tue, 15 Sep 2026 12:57:04 -0500 Subject: [PATCH 6/6] Fix DOCKERDISC_HOST_MAC docs and strengthen case-insensitivity test 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 Claude-Session: https://claude.ai/code/session_011meLPKCzVpdZyAUfv5U6mm --- server/plugins/dockerdisc/config.json | 6 ++--- test/backend/test_device_instance.py | 32 +++++++++++++++++++++++++++ test/db_test_helpers.py | 2 +- test/plugins/test_dockerdisc.py | 26 ++++++++++------------ 4 files changed, 48 insertions(+), 18 deletions(-) diff --git a/server/plugins/dockerdisc/config.json b/server/plugins/dockerdisc/config.json index 3b654589..b3182998 100644 --- a/server/plugins/dockerdisc/config.json +++ b/server/plugins/dockerdisc/config.json @@ -544,13 +544,13 @@ "name": [ { "language_code": "en_us", - "string": "Docker Host MAC Address (Fallback)" + "string": "Docker Host MAC Address (Manual)" } ], "description": [ { "language_code": "en_us", - "string": "Manual fallback physical MAC address of the Docker host, used if auto-detecting it via the Socket Proxy /info endpoint fails. The host must already exist as a device in NetAlertX (found via ARP/Nmap) - this plugin never creates it." + "string": "Manually configured physical MAC address of the Docker host. When set, it is used immediately - no Socket Proxy /info call is made. Leave blank to auto-detect the host via hostname matching instead. The host must already exist as a device in NetAlertX (found via ARP/Nmap) - this plugin never creates it." } ] } @@ -620,7 +620,7 @@ "description": [ { "language_code": "en_us", - "string": "One entry per Docker host to track. Each entry pairs a read-only Docker Socket Proxy URL with that host's device (auto-detected, or entered manually as a fallback). Every container found on a host is listed under that host's own Device Details -> Plugins -> DOCKERDISC tab - the host device must already exist in NetAlertX (via ARP/Nmap); this plugin never creates devices." + "string": "One entry per Docker host to track. Each entry pairs a read-only Docker Socket Proxy URL with that host's device, matched either by a manually configured MAC (used immediately when set) or by auto-detected hostname. Every container found on a host is listed under that host's own Device Details -> Plugins -> DOCKERDISC tab - the host device must already exist in NetAlertX (via ARP/Nmap); this plugin never creates devices." } ] }, diff --git a/test/backend/test_device_instance.py b/test/backend/test_device_instance.py index 8ad4cf88..6ddd2c1c 100644 --- a/test/backend/test_device_instance.py +++ b/test/backend/test_device_instance.py @@ -60,5 +60,37 @@ class TestGetAllByName(unittest.TestCase): self.assertEqual(results, []) +class TestGetByMac(unittest.TestCase): + """devMac is declared COLLATE NOCASE at the column level (unlike + devName), so getByMac() relies on the schema rather than applying its + own COLLATE clause - this exercises that guarantee against a real + SQLite connection, not a mock.""" + + def setUp(self): + self.conn = make_db() + insert_device_from_dict(self.conn, make_device_dict("aa:bb:cc:dd:ee:ff")) + self.conn.commit() + + def _instance(self): + from models.device_instance import DeviceInstance + inst = DeviceInstance() + + def _fetchone(q, p=()): + row = self.conn.execute(q, p).fetchone() + return dict(row) if row else None + inst._fetchone = _fetchone + return inst + + def test_case_insensitive_match(self): + inst = self._instance() + result = inst.getByMac("AA:BB:CC:DD:EE:FF") + self.assertIsNotNone(result) + self.assertEqual(result["devMac"], "aa:bb:cc:dd:ee:ff") + + def test_no_match_returns_none(self): + inst = self._instance() + self.assertIsNone(inst.getByMac("00:00:00:00:00:00")) + + if __name__ == "__main__": unittest.main() diff --git a/test/db_test_helpers.py b/test/db_test_helpers.py index 646f8526..66763ac2 100644 --- a/test/db_test_helpers.py +++ b/test/db_test_helpers.py @@ -28,7 +28,7 @@ from db.db_history import ensure_deviceshistory_table, ensure_deviceshistory_tri CREATE_DEVICES = """ CREATE TABLE IF NOT EXISTS Devices ( - devMac TEXT PRIMARY KEY, + devMac TEXT PRIMARY KEY COLLATE NOCASE, devName TEXT, devOwner TEXT, devType TEXT, diff --git a/test/plugins/test_dockerdisc.py b/test/plugins/test_dockerdisc.py index 7f425091..582e834a 100644 --- a/test/plugins/test_dockerdisc.py +++ b/test/plugins/test_dockerdisc.py @@ -445,20 +445,18 @@ def test_lookup_device_mac_not_found(): assert dockerdisc.lookup_device_mac("aa:bb:cc:dd:ee:ff") is False -def test_lookup_device_mac_found_regardless_of_stored_mac_case(): - """The plugin always passes a lowercase, normalize_mac()'d value in - - this only guards that lookup_device_mac() doesn't do anything of its - own (e.g. an exact-string comparison) that would undo whatever - case-insensitivity DeviceInstance.getByMac() provides. The real - guarantee is schema-level - Devices.devMac is declared - `COLLATE NOCASE` (server/db/schema/app.sql), which is exactly why - getByMac() itself doesn't need to apply it explicitly (unlike - getAllByName(), which does - devName has no such column collation; - see that method's docstring and test/backend/test_device_instance.py). - This test's stub can't exercise real SQLite collation, only that this - function's own logic is agnostic to it.""" - with patch.object(dockerdisc, "DeviceInstance", _stub_device_instance(get_by_mac={"devMac": "AA:BB:CC:DD:EE:FF"})): - assert dockerdisc.lookup_device_mac("aa:bb:cc:dd:ee:ff") is True +def test_lookup_device_mac_passes_mac_through_unchanged(): + """lookup_device_mac() must not do any of its own case massaging - it + delegates entirely to DeviceInstance.getByMac(), which relies on + Devices.devMac's schema-level `COLLATE NOCASE` (server/db/schema/app.sql) + for case-insensitive matching. That guarantee is exercised against a + real SQLite connection in test/backend/test_device_instance.py's + TestGetByMac; a mocked DeviceInstance can't exercise real collation, so + this test only checks that the mac argument reaches getByMac() as-is.""" + stub = _stub_device_instance(get_by_mac={"devMac": "aa:bb:cc:dd:ee:ff"}) + with patch.object(dockerdisc, "DeviceInstance", stub): + dockerdisc.lookup_device_mac("AA:BB:CC:DD:EE:FF") + stub.return_value.getByMac.assert_called_once_with("AA:BB:CC:DD:EE:FF") # ---------------------------------------------------------------------------