Jelajahi Sumber

fix(vp): populate bind-interface list on macOS (route non-Linux to psutil)

get_network_interfaces() only sent Windows to the psutil path; macOS fell into
the Linux ioctl branch, whose SIOCGIFADDR/SIOCGIFNETMASK ioctls are Linux-only.
macOS/BSD have fcntl but different ioctl numbers, so every call raised OSError
and the function returned an empty list — the VP bind-interface dropdown showed
nothing. Route all non-Linux platforms through the cross-platform psutil path.
maziggy 2 bulan lalu
induk
melakukan
af867c0392

+ 1 - 0
CHANGELOG.md

@@ -5,6 +5,7 @@ All notable changes to Bambuddy will be documented in this file.
 ## [0.2.5b2] - Unreleased
 
 ### Fixed
+- **Virtual Printer "bind interface" dropdown is empty on macOS** — Adding a Virtual Printer on macOS showed no interfaces to bind to. `get_network_interfaces()` only routed Windows to the cross-platform psutil path; macOS fell into the Linux branch, which uses the Linux-only `SIOCGIFADDR`/`SIOCGIFNETMASK` ioctls (`0x8915`/`0x891B`). macOS/BSD have `fcntl` but different ioctl numbers and sockaddr layout, so every per-interface ioctl raised `OSError` and the function silently returned an empty list (and `get_all_interface_ips()`, which has no `ip` binary to fall back to on macOS, inherited the empty result). Interface enumeration now routes **all** non-Linux platforms (macOS, BSD, Windows) through psutil, which returns each interface's name + IPv4 + netmask and filters loopback/link-local/down adapters while keeping real LAN and VPN (utun/Tailscale) interfaces bindable. Linux keeps its existing ioctl path unchanged.
 - **Native install script fails on macOS with Homebrew/venv permission errors** — On macOS the installer mixed root-only steps (defaulting to `/opt/bambuddy`, which nudged users into `sudo ./install.sh`) with steps that must **not** run as root: `brew install` hard-refuses to run as root (aborting the script mid-way), and any venv / `node_modules` created by root can't be managed by the launchd agent (which runs as the user), producing permission errors on `pip`/`npm`. The macOS path is now fully rootless: the script refuses to run under `sudo` on macOS with an actionable message, defaults the install directory to `~/bambuddy` (user-owned, no `/opt` write), and the download/venv/frontend/env/directory steps no longer shell out to `sudo` on macOS. A custom `--path` under a root-owned parent still works — the script elevates only to create+chown that one directory to the user, then continues rootless. Linux behaviour (service user + systemd) is unchanged.
 - **External camera "connection lost" when the snapshot URL serves a non-JPEG image (#1902)** — An external camera configured in HTTP-snapshot mode failed to load in Bambuddy with a repeating "connection lost", even though the camera URL rendered fine when opened directly in a browser. The log showed `Snapshot does not appear to be JPEG` on every polled frame followed by the stream ending. Root cause: `_capture_snapshot` returned the fetched bytes even when they weren't JPEG, and the MJPEG stream wraps every part with a hard-coded `Content-Type: image/jpeg` boundary — so a camera serving PNG/WebP/BMP stills (common on IP cameras and reverse-proxied snapshot endpoints) sent the browser a non-JPEG payload labelled as JPEG, which the browser rejected, tearing down the whole `multipart/x-mixed-replace` stream. `_capture_snapshot` now transcodes non-JPEG stills to JPEG (via OpenCV, already a dependency) before streaming; genuine JPEG snapshots keep their byte-for-byte fast path, and truly undecodable responses (HTML error pages, auth redirects) fall back to the previous raw-return behaviour with a clearer one-off warning instead of a per-frame log flood. Fixes browser playback and keeps the JPEG-only downstream (plate detection, Obico, finish photo) working for these cameras.
 - **Per-user Notifications page unreachable from the sidebar (#1901, reporter @JmanB52D)** — The Notifications entry (where each user opts in/out of their own print email notifications) disappeared from the left navigation. The page (`/notifications`) and its API were both intact — only the sidebar link was gone, so the screen was reachable only by typing the URL. Root cause: the sidebar-ordering refactor in #1673 accidentally deleted the `notifications` item from `defaultNavItems` (and its `notifications:user_email` permission mapping) while extracting the ordering helpers, but left the advanced-auth visibility gate that references that id — so the gate had nothing to gate and the item could never render. Restored both the `defaultNavItems` entry and the permission gate; the item now shows for any user holding `notifications:user_email` (both default groups, Administrators and Operators, do) when advanced auth and user email notifications are enabled, exactly as before #1673.

+ 17 - 11
backend/app/services/network_utils.py

@@ -27,17 +27,21 @@ def _is_excluded(name: str) -> bool:
 
 
 def _get_network_interfaces_psutil() -> list[dict]:
-    """Windows path: enumerate interfaces via psutil.
+    """Non-Linux path (Windows, macOS, BSD): enumerate interfaces via psutil.
 
-    fcntl + ioctl is Linux-only, and the ``ip`` command isn't available
-    on Windows either, so both Linux code paths return empty here. psutil
-    is already a Bambuddy dep (``psutil>=6.0.0``) and gives us cross-
-    platform name + IPv4 + netmask in one call.
+    The ioctl request numbers in the Linux path (SIOCGIFADDR 0x8915,
+    SIOCGIFNETMASK 0x891B) and the sockaddr layout they return are
+    Linux-specific. On macOS/BSD ``fcntl`` still imports, so those ioctls
+    don't raise ImportError — they raise ``OSError`` per interface and the
+    Linux path silently returns an empty list (no VP bind interfaces).
+    Windows has no ``fcntl``/``ip`` at all. psutil is already a Bambuddy dep
+    (``psutil>=6.0.0``) and gives cross-platform name + IPv4 + netmask in one
+    call, so we use it for everything that isn't Linux.
 
     Filters: IPv4 only (matches the Linux path), skip loopback and
     link-local (169.254.0.0/16), skip interfaces psutil reports as down.
-    No name-based exclusion — users on Windows may legitimately want to
-    bind a VP to a Hyper-V / WSL / Tailscale virtual adapter.
+    No name-based exclusion — users may legitimately want to bind a VP to a
+    Hyper-V / WSL / Tailscale / utun virtual adapter.
     """
     try:
         import psutil
@@ -100,10 +104,12 @@ def get_network_interfaces() -> list[dict]:
     Returns:
         List of dicts with name, ip, netmask, subnet, broadcast
     """
-    # Windows has no fcntl and no `ip` binary; the Linux ioctl path below
-    # raises ImportError on import fcntl. Route to the psutil-based path
-    # instead. The Linux path stays as-is for behavioural parity.
-    if sys.platform == "win32":
+    # Only Linux has the SIOCGIFADDR/SIOCGIFNETMASK ioctls + sockaddr layout the
+    # path below relies on. Windows lacks fcntl entirely; macOS/BSD have fcntl but
+    # different ioctl numbers, so the ioctl path there fails per-interface and
+    # returns an empty list (breaking the VP bind-interface dropdown on macOS).
+    # Route everything non-Linux to the cross-platform psutil path.
+    if not sys.platform.startswith("linux"):
         return _get_network_interfaces_psutil()
 
     interfaces = []

+ 83 - 0
backend/tests/unit/services/test_network_utils.py

@@ -0,0 +1,83 @@
+"""Tests for network interface enumeration.
+
+Focus: the platform routing in get_network_interfaces(). macOS/BSD have fcntl
+but not the Linux SIOCGIFADDR/SIOCGIFNETMASK ioctls, so the ioctl path there
+silently returns nothing and the VP bind-interface dropdown comes up empty.
+Everything that isn't Linux must go through the cross-platform psutil path.
+"""
+
+import socket
+from collections import namedtuple
+from unittest.mock import patch
+
+from backend.app.services import network_utils
+
+# Mimic the shape of psutil.net_if_addrs() / net_if_stats() entries we read.
+_Addr = namedtuple("snicaddr", ["family", "address", "netmask", "broadcast", "ptp"])
+_Stats = namedtuple("snicstats", ["isup", "duplex", "speed", "mtu", "flags"])
+
+
+def _fake_psutil():
+    addrs = {
+        "en0": [_Addr(socket.AF_INET, "192.168.1.50", "255.255.255.0", None, None)],
+        "lo0": [_Addr(socket.AF_INET, "127.0.0.1", "255.0.0.0", None, None)],
+        "awdl0": [_Addr(socket.AF_INET, "169.254.10.20", "255.255.0.0", None, None)],
+        "utun3": [_Addr(socket.AF_INET, "100.64.0.7", "255.255.255.255", None, None)],
+        "en5": [_Addr(socket.AF_INET, "10.0.0.9", "255.255.255.0", None, None)],
+    }
+    stats = {
+        "en0": _Stats(True, 0, 0, 1500, 0),
+        "lo0": _Stats(True, 0, 0, 16384, 0),
+        "awdl0": _Stats(True, 0, 0, 1500, 0),
+        "utun3": _Stats(True, 0, 0, 1500, 0),
+        "en5": _Stats(False, 0, 0, 1500, 0),  # down → skipped
+    }
+    return addrs, stats
+
+
+@patch("backend.app.services.network_utils.sys")
+def test_macos_routes_to_psutil(mock_sys):
+    """On darwin, get_network_interfaces() must use psutil, not the ioctl path."""
+    mock_sys.platform = "darwin"
+    with patch.object(network_utils, "_get_network_interfaces_psutil", return_value=[{"name": "en0"}]) as psutil_path:
+        result = network_utils.get_network_interfaces()
+    psutil_path.assert_called_once()
+    assert result == [{"name": "en0"}]
+
+
+@patch("backend.app.services.network_utils.sys")
+def test_windows_routes_to_psutil(mock_sys):
+    mock_sys.platform = "win32"
+    with patch.object(network_utils, "_get_network_interfaces_psutil", return_value=[]) as psutil_path:
+        network_utils.get_network_interfaces()
+    psutil_path.assert_called_once()
+
+
+@patch("backend.app.services.network_utils.sys")
+def test_linux_does_not_use_psutil(mock_sys):
+    """Linux keeps the ioctl path — psutil helper must not be invoked."""
+    mock_sys.platform = "linux"
+    with patch.object(network_utils, "_get_network_interfaces_psutil") as psutil_path:
+        # The ioctl path runs for real here; we only assert it wasn't short-circuited
+        # to psutil. Its actual return depends on the host, so we don't assert on it.
+        network_utils.get_network_interfaces()
+    psutil_path.assert_not_called()
+
+
+def test_psutil_path_filters_and_returns_bindable_ips():
+    """The psutil path drops loopback/link-local/down ifaces, keeps real + VPN ones."""
+    addrs, stats = _fake_psutil()
+    with (
+        patch("psutil.net_if_addrs", return_value=addrs),
+        patch("psutil.net_if_stats", return_value=stats),
+    ):
+        result = network_utils._get_network_interfaces_psutil()
+
+    by_name = {i["name"]: i for i in result}
+    assert "en0" in by_name  # normal LAN interface
+    assert by_name["en0"]["ip"] == "192.168.1.50"
+    assert by_name["en0"]["subnet"] == "192.168.1.0/24"
+    assert "utun3" in by_name  # Tailscale/VPN — legitimately bindable
+    assert "lo0" not in by_name  # loopback filtered
+    assert "awdl0" not in by_name  # link-local (169.254) filtered
+    assert "en5" not in by_name  # interface down, skipped