From af867c039290af29fa83652249157d9bfdb87b49 Mon Sep 17 00:00:00 2001 From: maziggy Date: Mon, 6 Jul 2026 13:03:22 +0200 Subject: [PATCH] fix(vp): populate bind-interface list on macOS (route non-Linux to psutil) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CHANGELOG.md | 1 + backend/app/services/network_utils.py | 28 ++++--- .../tests/unit/services/test_network_utils.py | 83 +++++++++++++++++++ 3 files changed, 101 insertions(+), 11 deletions(-) create mode 100644 backend/tests/unit/services/test_network_utils.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 477f33ce5..d7a370f14 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/backend/app/services/network_utils.py b/backend/app/services/network_utils.py index 6ef9293cf..f62932076 100644 --- a/backend/app/services/network_utils.py +++ b/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 = [] diff --git a/backend/tests/unit/services/test_network_utils.py b/backend/tests/unit/services/test_network_utils.py new file mode 100644 index 000000000..7ad608e2c --- /dev/null +++ b/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