0e3ae78de7
Closes the issues surfaced by the pre-merge code review of the expand-device-support branch. CRITICAL #2 -- update_device double-encrypts secrets in memory. storage/device_store.py round-tripped through device.to_dict() which encrypts hue_username / hue_client_key / ble_govee_key / nanoleaf_token via _enc(), but Device.__init__ does not decrypt. The cached self._items[device_id] thus held ciphertext where plaintext belonged, breaking runtime auth for paired devices on any update -- even an innocuous rename. Sourcing kwargs from vars(device) directly avoids the round-trip. Regression tests cover Nanoleaf and Hue. HIGH #3 -- secrets leaked in GET /api/v1/devices response. DeviceResponse previously returned nanoleaf_token / hue_username / hue_client_key in plaintext (decrypted server-side from storage), defeating the encryption-at-rest. Replaced with nanoleaf_paired and hue_paired booleans. ble_govee_key intentionally stays -- it's a user-managed value pasted from a third-party tool, must remain visible for edit. Frontend types.ts + the one nanoleaf_token reader updated to the boolean. HIGH #4 -- SSRF surface. validate_lan_host() added to net_classify.py; called from each new driver's validate_device (DDP / Yeelight / WiZ / LIFX / Govee / OPC / Nanoleaf) and from pair_device. Rejects literal public IPs with a descriptive ValueError; non-IP hostnames pass through (mDNS labels, bare hostnames). RFC6890 ranges (documentation, former class E) are accepted as LAN-like since Python's ipaddress.is_private treats them so -- correct policy for LedGrab. HIGH #5 -- decrypt failure deletes the device row. _dec() now catches the exception, logs an error, and returns "" instead of propagating. Without the fix, a regenerated data/.secret_key would silently make every Hue / Nanoleaf / BLE-Govee device disappear from the device list on next startup. Regression test asserts a corrupt envelope leaves the device hydratable. HIGH #6 -- update_device route does not rstrip("/") for non-WLED. Moved the trim before the WLED-specific scheme inference so every device type gets consistent URL normalization between create and update. MEDIUM #7 -- Govee discovery port 4002 collision. Added a lazily- initialized module-level asyncio.Lock that serializes concurrent discover_govee_devices() calls; the previous behavior had the second parallel scan silently return [] when the first still held port 4002. Error message also clarified to mention another Govee tool. MEDIUM #8 -- Nanoleaf discover() leaked browser tasks on cancellation. Moved the browser cancel loop into the finally block so an interrupted mDNS scan still tears them down. MEDIUM #9 -- pair endpoint logged user-supplied URL with exc_info=True. Added _sanitize_url_for_log() that strips userinfo + fragment, and demoted the log from exc_info to type(exc).__name__ + str(exc) so a hostile receiver's response body can't end up in the log file. LOW -- Nanoleaf was the only client without a .port property. Added one (returns NANOLEAF_PORT, fixed) for cross-driver symmetry. LOW -- no end-to-end pair-then-create coverage. Added TestPairThenCreateFlow.test_pair_then_create_persists_encrypted_token which exercises the full path: POST /api/v1/devices/pair returned fields, store.create_device, then asserts (a) in-memory plaintext, (b) to_config() plaintext, (c) persisted ciphertext, (d) API response strip + paired-boolean. Tests: 1379 pass (was 1358 -- 21 new regression tests added). ruff clean. TypeScript clean.
190 lines
5.8 KiB
Python
190 lines
5.8 KiB
Python
"""Tests for ledgrab.utils.net_classify."""
|
|
|
|
import pytest
|
|
|
|
from ledgrab.utils.net_classify import (
|
|
HostCategory,
|
|
classify_ip,
|
|
is_blocked_for_ssrf,
|
|
is_local_for_http_default,
|
|
is_loopback,
|
|
)
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"host,expected",
|
|
[
|
|
("127.0.0.1", HostCategory.LOOPBACK),
|
|
("::1", HostCategory.LOOPBACK),
|
|
("10.0.0.5", HostCategory.PRIVATE),
|
|
("192.168.1.1", HostCategory.PRIVATE),
|
|
("172.16.0.5", HostCategory.PRIVATE),
|
|
("fd00::1", HostCategory.PRIVATE), # ULA
|
|
("169.254.1.1", HostCategory.LINK_LOCAL),
|
|
("fe80::1", HostCategory.LINK_LOCAL),
|
|
("0.0.0.0", HostCategory.UNSPECIFIED),
|
|
("::", HostCategory.UNSPECIFIED),
|
|
("224.0.0.1", HostCategory.MULTICAST),
|
|
("ff00::1", HostCategory.MULTICAST),
|
|
# ``240.0.0.0/4`` (class E) is labelled both ``is_private`` and
|
|
# ``is_reserved`` by Python; we keep PRIVATE first which is the
|
|
# stricter SSRF policy.
|
|
("240.0.0.1", HostCategory.PRIVATE),
|
|
("8.8.8.8", HostCategory.PUBLIC),
|
|
("1.1.1.1", HostCategory.PUBLIC),
|
|
("2606:4700:4700::1111", HostCategory.PUBLIC),
|
|
("not-an-ip", HostCategory.UNPARSEABLE),
|
|
("", HostCategory.UNPARSEABLE),
|
|
("example.com", HostCategory.UNPARSEABLE),
|
|
],
|
|
)
|
|
def test_classify_ip(host: str, expected: HostCategory) -> None:
|
|
assert classify_ip(host) is expected
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# SSRF block list — must include EVERY non-public category, including
|
|
# unparseable inputs. Regression guard: if anyone narrows this set we lose
|
|
# SSRF protection.
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"host",
|
|
[
|
|
"127.0.0.1",
|
|
"::1",
|
|
"10.0.0.5",
|
|
"192.168.1.1",
|
|
"172.16.0.5",
|
|
"fd00::1",
|
|
"169.254.1.1",
|
|
"fe80::1",
|
|
"0.0.0.0",
|
|
"::",
|
|
"224.0.0.1",
|
|
"ff00::1",
|
|
"240.0.0.1",
|
|
"not-an-ip", # unparseable → blocked
|
|
"", # unparseable → blocked
|
|
],
|
|
)
|
|
def test_is_blocked_for_ssrf_blocks_non_public(host: str) -> None:
|
|
assert is_blocked_for_ssrf(host) is True
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"host",
|
|
["8.8.8.8", "1.1.1.1", "2606:4700:4700::1111"],
|
|
)
|
|
def test_is_blocked_for_ssrf_allows_public(host: str) -> None:
|
|
assert is_blocked_for_ssrf(host) is False
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# LAN-default policy — narrower than SSRF: we infer ``http://`` for loopback
|
|
# / private / link-local / unspecified, NOT for multicast / reserved /
|
|
# unparseable (those should fall through to ``https://`` or to caller-side
|
|
# heuristics like mDNS suffix matching).
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"host",
|
|
["127.0.0.1", "::1", "10.0.0.5", "192.168.1.1", "fe80::1", "0.0.0.0"],
|
|
)
|
|
def test_is_local_for_http_default_true(host: str) -> None:
|
|
assert is_local_for_http_default(host) is True
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"host",
|
|
["8.8.8.8", "224.0.0.1", "not-an-ip", ""],
|
|
)
|
|
def test_is_local_for_http_default_false(host: str) -> None:
|
|
assert is_local_for_http_default(host) is False
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Loopback predicate — accepts both literals and the auth module's textual
|
|
# placeholders (localhost, testclient).
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"host",
|
|
[
|
|
"127.0.0.1",
|
|
"127.0.0.5",
|
|
"::1",
|
|
"localhost",
|
|
"LOCALHOST",
|
|
"testclient",
|
|
"[::1]",
|
|
"fe80::1%eth0", # ← link-local with zone — must NOT match
|
|
],
|
|
)
|
|
def test_is_loopback_recognises_loopback(host: str) -> None:
|
|
expected = host != "fe80::1%eth0"
|
|
assert is_loopback(host) is expected
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"host",
|
|
["8.8.8.8", "10.0.0.5", "example.com", "", None],
|
|
)
|
|
def test_is_loopback_rejects_other(host) -> None:
|
|
assert is_loopback(host) is False
|
|
|
|
|
|
# ─── validate_lan_host ────────────────────────────────────────────
|
|
|
|
|
|
from ledgrab.utils.net_classify import validate_lan_host # noqa: E402
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"host",
|
|
[
|
|
"127.0.0.1",
|
|
"10.0.0.5",
|
|
"192.168.1.50",
|
|
"172.16.0.10",
|
|
"fe80::1",
|
|
"fc00::1",
|
|
"0.0.0.0",
|
|
# IPv6 with brackets and zone id
|
|
"[fe80::1%eth0]",
|
|
# Plain hostnames pass through (mDNS / bare-label)
|
|
"bulb.local",
|
|
"office-strip",
|
|
"controller.lan",
|
|
"", # empty also passes (caller handles validation upstream)
|
|
],
|
|
)
|
|
def test_validate_lan_host_accepts_local(host) -> None:
|
|
"""Loopback / private / link-local / hostnames must not raise."""
|
|
validate_lan_host(host)
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"host",
|
|
[
|
|
"1.1.1.1", # Cloudflare DNS (routable public)
|
|
"8.8.8.8", # Google DNS
|
|
"224.0.0.1", # multicast — not a LAN device address
|
|
"2606:4700:4700::1111", # Cloudflare DNS IPv6
|
|
],
|
|
)
|
|
def test_validate_lan_host_rejects_public_or_multicast(host) -> None:
|
|
"""Reject genuinely-public addresses and multicast targets.
|
|
|
|
Note: Python's :func:`ipaddress.ip_address(...).is_private` treats
|
|
RFC6890 ranges (documentation 192.0.2/24, former class E 240/4, etc.)
|
|
as private, so those are accepted as "local-ish" by our classifier --
|
|
which is the correct policy for LedGrab: those ranges aren't routable
|
|
public addresses, and refusing them would be unhelpful theatre.
|
|
"""
|
|
with pytest.raises(ValueError, match="LedGrab is LAN-only"):
|
|
validate_lan_host(host)
|