BE: review fixes

docs/docker-troubleshooting/missing-capabilities.md

@@ -20,7 +20,7 @@ NetAlertX supports exactly these capabilities; anything beyond them triggers the
 |------------|-----------|----------|
 | `NET_RAW` | Raw socket operations by the scanners | Yes - the image's Python binary fails to start without it |
 | `NET_ADMIN` | ARP scanning, and applying [ARP sysctls](arp-flux-sysctls.md) at runtime with host networking | Yes - the image's Python binary fails to start without it |
-| `NET_BIND_SERVICE` | `nbtscan` binding to privileged UDP port 137 | Recommended - without it NetBIOS discovery of legacy devices is less accurate and a startup warning is logged |
+| `NET_BIND_SERVICE` | `nbtscan` binding to privileged UDP port 137 | Yes, for `nbtscan` - its binary carries this as a file capability, so without it `nbtscan` can't execute (NetBIOS discovery of legacy devices stops working) and a startup warning is logged |
 | `CHOWN` | Root priming of `/data` and `/tmp` ownership at startup | Only with `cap_drop: [ALL]` - without it ownership is not changed and a warning is logged (see below) |
 | `SETUID`, `SETGID` | Dropping from root to `PUID:PGID` at startup | Only when starting as root with `cap_drop: [ALL]` - without them the container keeps running as root ([details](PUID_PGID_SECURITY.md)) |
 

server/plugins/nbtscan_scan/nbtscan.py

@@ -110,7 +110,7 @@ def execute_name_lookup(ip, timeout):
         output = subprocess.check_output(
             args,
             universal_newlines=True,
-            stderr=subprocess.DEVNULL,
+            stderr=subprocess.PIPE,
             timeout=(timeout),
             text=True
         )
@@ -129,7 +129,7 @@ def execute_name_lookup(ip, timeout):
             if not parts or parts[0] != ip:
                 continue
 
-            if 'failed' in line.lower():
+            if len(parts) > 2 and parts[1].lower() == 'sendto' and parts[2].lower().startswith('failed'):
                 mylog('verbose', [f'[{pluginName}] ⚠ ERROR - nbtscan reported a failure: {line}'])
                 continue
 
@@ -148,7 +148,7 @@ def execute_name_lookup(ip, timeout):
         # if "NXDOMAIN" in e.output:
         #     mylog('verbose', [f'[{pluginName}]', f"No PTR record found for IP: {ip}"])
         # else:
-        mylog('verbose', [f'[{pluginName}] ⚠ ERROR - {e.output}'])
+        mylog('verbose', [f'[{pluginName}] ⚠ ERROR - {e.output} | stderr: {e.stderr}'])
 
     except subprocess.TimeoutExpired:
         mylog('verbose', [f'[{pluginName}] TIMEOUT - the process forcefully terminated as timeout reached'])

test/plugins/test_nbtscan_parsing.py

@@ -71,6 +71,11 @@ def test_sendto_failed_line_with_spaces_is_not_a_name():
     assert _lookup(output) == ("", "")
 
 
+def test_name_containing_failed_is_kept():
+    output = HEADER + TABLE_HEADER + f"{IP}    FAILED-SRV       <server>  <unknown>        aa:bb:cc:dd:ee:ff\n"
+    assert _lookup(output) == ("FAILED-SRV", "")
+
+
 def test_no_result_row_returns_empty():
     output = HEADER + TABLE_HEADER
     assert _lookup(output) == ("", "")
@@ -89,3 +94,11 @@ def test_unknown_placeholder_name_rejected():
 def test_overlong_name_rejected():
     output = HEADER + TABLE_HEADER + f"{IP}    {'A' * 20}  <server>  <unknown>        aa:bb:cc:dd:ee:ff\n"
     assert _lookup(output) == ("", "")
+
+
+def test_called_process_error_logs_stderr():
+    error = nbtscan.subprocess.CalledProcessError(1, "nbtscan", output="out", stderr="boom-stderr")
+    with patch.object(nbtscan.subprocess, "check_output", side_effect=error), \
+            patch.object(nbtscan, "mylog") as mock_log:
+        assert nbtscan.execute_name_lookup(IP, 10) == ("", "")
+    assert any("boom-stderr" in str(call.args) for call in mock_log.call_args_list)