Files
LEDMatrix/test/test_api_v3_wifi_endpoints.py
T
ChuckandClaude Opus 5 bdb9a94033 refactor(api-v3): split the 10,469-line blueprint into a package (#553)
* refactor(api-v3): split the 10,469-line blueprint into a package

web_interface/blueprints/api_v3.py held 111 routes, 56 helpers and 181
functions in one module -- 9% of the core by line count and three times the
next largest file. It becomes a package of nine route modules grouped by path
segment, plus __init__.py for the shared imports, constants, Blueprint and
helpers.

Every route module decorates the SAME api_v3 Blueprint object, so endpoint
names stay api_v3.<function>, the URL map is unchanged and app.py is untouched.
Verified: 111 routes before, 111 after, byte-identical rules, endpoints and
methods, and every endpoint still on the one blueprint.

  plugins   3,867   config    1,178   starlark  692   system  619
  fonts       452   misc        398   wifi      361   display 326   backup 212
  __init__  1,787 (imports, constants, Blueprint, 56 helpers)

Two things the URL-map check could not catch, both found by running the suite:

1. PROJECT_ROOT = Path(__file__).parent.parent.parent. Moving the code one
   directory deeper made that resolve to web_interface/ instead of the project
   root. Nothing failed at import; it surfaced as ~110 tests failing with 404s
   and "installation script not found", because every path built from it was
   one level too shallow. Now parents[3], and test_api_v3_url_map.py asserts
   PROJECT_ROOT/run.py exists so the next move cannot repeat it.

2. Module-attribute patching. Tests do
   monkeypatch.setattr(api_v3_module, "_BACKUP_EXPORT_DIR", ...) and a route
   module that binds such a name by value never sees the patch. The shared code
   therefore stays in __init__.py rather than moving to a _common submodule --
   it has to live on the module the tests patch -- and the eleven names tests
   patch are read back through the package (_pkg.X) instead of bound by value.
   Those eleven were found by AST-scanning every setattr in the test tree, not
   by guessing; "time" is among them, used to drive a fake clock through the
   second-resolution credential-backup filenames.

Test changes are confined to what genuinely moved: patch targets that now name
the owning route module, imports of helpers, and six tests that scan the api_v3
source as a file and now read the package directory.

Full suite: 4,278 passed, 68 skipped, 0 failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9

* fix(api-v3): address CodeRabbit findings from the blueprint-split review

Fixes to the api_v3 package split (PR #553), one per finding verified
against the actual code:

- __init__.py: _redact_credentials only blanked scalar values under a
  credential-named key; a bare list of secrets under such a key (e.g.
  tokens: ["a", "b"]) passed through untouched, since the list branch
  recursed with no memory that its key looked like a credential. Nested
  dicts still walk normally (a documented, tested behaviour -- a container
  like secrets: {api_key: ..., note: ...} is a section name, not a value to
  blank outright), but any value reached under a credential-shaped key is
  now actually blanked.

- __init__.py: the OAuth helper script's raw stderr/stdout went to
  logger.error unredacted (CWE-532) right next to a comment claiming this
  was deliberate; the HTTP response already used the existing redact_text
  helper. Routed the log line through the same helper.

- __init__.py / starlark.py: the standalone Starlark manifest fallback
  (used when the plugin instance isn't loaded) read-modified-wrote
  manifest.json with no lock, unlike StarlarkAppsPlugin._update_manifest_safe
  (plugin-repos/starlark-apps/manager.py), which already holds an flock for
  the same file when the plugin is loaded. Added _starlark_manifest_lock,
  mirroring that pattern, and wrapped every standalone read-modify-write
  call site in it. The app-config update route also wrote config.json and
  the manifest as two separate, non-transactional writes (a second,
  distinct finding at the same call site); config.json is now rolled back
  if the manifest write that follows it fails.

- backup.py: restore options used bare bool() on values from the request,
  so {"restore_secrets": "false"} restored secrets anyway (bool("false") is
  True). Switched to the existing _coerce_to_bool helper already used for
  this exact purpose elsewhere in the package.

- config.py: an automated import-rewrite mangled four user-facing
  validation strings and their neighbouring comments -- "Invalid start
  time" had become "Invalid start _pkg.time" (and likewise for "end time")
  in both the schedule and dim-schedule per-day validation paths.

- display.py: `import _pkg.time as time_module` -- _pkg is a local alias
  for the package, not a real importable module, so this raised
  ModuleNotFoundError whenever a caller restarted an already-running
  display service via /display/on-demand/start, after the on-demand
  request was already written to cache. Fixed to `import time`. Audited
  the rest of the package for the same `_pkg.<module>` import mistake;
  every other `_pkg.` reference is a legitimate attribute read-through
  (`_pkg.time.time()`, `_pkg._get_starlark_plugin()`, ...), not a broken
  import statement.

- fonts.py: validate_file_upload's max_size_mb parameter is silently
  unused by that helper (it only checks filename/extension) -- the font
  upload route saved arbitrarily large files as a result. Added the same
  seek-and-check pattern already used for the sibling .star upload.

- wifi.py: two ad hoc, inconsistent bool coercions. POST
  /wifi/ap/auto-enable used bare bool(), so a JSON string "false" enabled
  it. POST /wifi/radio's enabled/force parsing recognized real bool and
  some strings but not int 1/0 (1 is True is False in Python). Factored one
  small _parse_bool_ish helper local to this file and used it at all three
  sites.

Not changed: the "unknown/misspelled restore option keys default to True"
half of the backup.py finding -- the file's own comment documents that a
missing key deliberately means "restore everything," matching the
already-existing JSON-parse-failure guard a few lines above it; only the
bool-coercion defect was a real bug.

Added or extended regression tests for every fix, following each area's
existing test conventions. Full suite: 4328 passed, 62 skipped, 2 failed
on both this branch and origin/main (missing tzdata package breaks two
timezone-alias tests in test_onboarding_checklist.py, unrelated to this
change) -- no new failures.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S3bPMESe2TfrGvbs1ef9c5

* fix(api-v3): reject unknown restore option keys

CodeRabbit's review of the blueprint split (#553) asked that
POST /backup/restore reject option keys outside RestoreOptions'
known set. The follow-up commit fixed the bool("false")-is-True
bug with _coerce_to_bool but never added the key check: a typo'd
or renamed key (e.g. "restoreSecrets") is silently ignored by
opts_dict.get(key, True), so the flag stays at its True default
and secrets get restored despite the caller's request saying
otherwise -- with no indication anything was wrong.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vmcwf5vMgYqdt8bJTZtiwb

* fix(api-v3): address CodeRabbit findings on the blueprint split

- _redact_credentials: blank scalar descendants of objects reached
  through a credential-owned list (e.g. tokens: [{"value": "secret"}])
  regardless of field name -- the existing name-based walk only
  protected direct dict values under a credential key, not list items.
- wifi.py: reject enabled/force/auto_enable_ap_mode values
  _parse_bool_ish can't recognize (400) instead of silently treating
  them as False, which could disable Wi-Fi or the radio itself.
- Starlark manifest locking: lock a stable manifest.json.lock sidecar
  instead of manifest.json itself, in both the standalone route path
  (_starlark_manifest_lock) and the plugin path
  (StarlarkAppsPlugin._save_manifest / _update_manifest_safe).
  manifest.json is replaced by an atomic rename on every write, which
  swaps in a fresh inode; a lock held on the old inode does not
  exclude a second locker that opens the path afresh right after the
  rename and gets the new inode, so two writers could race despite
  each holding "a lock". A sidecar that no write ever touches always
  resolves to the same inode for every locker.

Skipped as stale: the "serialize the complete manifest
read-modify-write" finding at api_v3/__init__.py -- every standalone
handler that calls _write_starlark_manifest is already wrapped in
_starlark_manifest_lock() on this branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(api-v3): re-check reconciliation findings by the reconciler's own rules

Both CodeRabbit findings on the merge commit, verified against the code first.

Major, plugins.py: the stale-findings filter derived its own notion of "in
config" and "on disk", and both were looser than the reconciliation module's.
set(load_config()) also contains system keys, the secrets-file keys load_config()
merges in, and non-dict values; and any directory holding a manifest.json
counted as installed even when that manifest does not parse. Either looseness
clears a finding that is still true -- and a secrets key read as a plugin is the
precise bug the filter exists to stop reporting, so reintroducing that asymmetry
while re-checking was the wrong way round.

The two extractions now live in state_reconciliation.py as config_plugin_ids()
and disk_plugin_ids(), with ignored_config_keys() and secrets_top_level_keys()
alongside. _get_config_state() and _get_disk_state() use them too, so there is
one definition rather than two that can drift. _get_disk_state() re-reads each
manifest for version/name after taking membership from the shared extractor;
that costs one extra small read per plugin on a path that runs once per boot.

Minor, the new test: the fixture assigned api_v3.config_manager and
api_v3.plugin_manager directly. Those live on a module-level blueprint
singleton, so the mocks leaked into every later test that imports api_v3 --
pointing at a tmp_path already deleted. Both now go through monkeypatch.setattr,
which restores them. This is the same pollution class that made an earlier test
in this session break seven unrelated ones, so it is worth getting right.

Five cases added for the parity itself: a secrets key, a system key and a
non-dict value must not clear an "installed but missing from config" finding,
and neither an unparseable manifest nor a .standalone-backup- directory may
count as installed. All five fail against the looser version.

Linux CI on the preceding commit: Core unit tests, plugin harness, CodeQL and
CodeRabbit all pass. Codacy reads action_required on every commit of this
branch including the first, so it is pre-existing and not from this work.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-11 10:07:38 -04:00

319 lines
15 KiB
Python

"""
Endpoint tests for the /wifi/* routes in api_v3.
These routes drive the host's actual networking — connecting, dropping a
connection, switching the radio off — and had no endpoint-level tests at
all. WiFiManager is mocked throughout; nothing here may touch real
networking.
Each handler does `from src.wifi_manager import WiFiManager` inside the
function body, so the patch target is the class at its definition site.
"""
import sys
from pathlib import Path
from unittest.mock import MagicMock, patch
import pytest
sys.path.insert(0, str(Path(__file__).parent.parent))
from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401,E402
@pytest.fixture
def wifi_manager():
"""Patch WiFiManager where it is defined; yield the instance mock."""
with patch("src.wifi_manager.WiFiManager") as cls:
instance = MagicMock()
cls.return_value = instance
yield instance
class TestConnect:
URL = "/api/v3/wifi/connect"
def test_success(self, api_v3_client, wifi_manager):
wifi_manager.connect_to_network.return_value = (True, "Connected to HomeNet")
response = api_v3_client.post(self.URL, json={"ssid": "HomeNet", "password": "pw"})
assert response.status_code == 200
assert response.get_json()["message"] == "Connected to HomeNet"
wifi_manager.connect_to_network.assert_called_once_with("HomeNet", "pw")
def test_missing_body_rejected(self, api_v3_client, wifi_manager):
response = api_v3_client.post(self.URL, json={})
assert response.status_code == 400
wifi_manager.connect_to_network.assert_not_called()
def test_missing_ssid_rejected(self, api_v3_client, wifi_manager):
response = api_v3_client.post(self.URL, json={"password": "pw"})
assert response.status_code == 400
assert "SSID is required" in response.get_json()["message"]
wifi_manager.connect_to_network.assert_not_called()
@pytest.mark.parametrize("ssid", ["", " ", "\t"])
def test_blank_ssid_rejected(self, api_v3_client, wifi_manager, ssid):
response = api_v3_client.post(self.URL, json={"ssid": ssid})
assert response.status_code == 400
wifi_manager.connect_to_network.assert_not_called()
def test_ssid_is_trimmed(self, api_v3_client, wifi_manager):
wifi_manager.connect_to_network.return_value = (True, "ok")
api_v3_client.post(self.URL, json={"ssid": " HomeNet "})
wifi_manager.connect_to_network.assert_called_once_with("HomeNet", "")
def test_missing_password_becomes_empty_string(self, api_v3_client, wifi_manager):
wifi_manager.connect_to_network.return_value = (True, "ok")
api_v3_client.post(self.URL, json={"ssid": "OpenNet"})
wifi_manager.connect_to_network.assert_called_once_with("OpenNet", "")
def test_null_password_becomes_empty_string(self, api_v3_client, wifi_manager):
wifi_manager.connect_to_network.return_value = (True, "ok")
api_v3_client.post(self.URL, json={"ssid": "OpenNet", "password": None})
wifi_manager.connect_to_network.assert_called_once_with("OpenNet", "")
def test_failure_reports_the_managers_reason(self, api_v3_client, wifi_manager):
wifi_manager.connect_to_network.return_value = (False, "Bad password")
response = api_v3_client.post(self.URL, json={"ssid": "HomeNet"})
assert response.status_code == 400
assert response.get_json()["message"] == "Bad password"
def test_failure_without_reason_uses_fallback_text(self, api_v3_client, wifi_manager):
wifi_manager.connect_to_network.return_value = (False, None)
response = api_v3_client.post(self.URL, json={"ssid": "HomeNet"})
assert response.status_code == 400
assert response.get_json()["message"] == "Failed to connect to network"
def test_manager_exception_is_a_500_without_leaking_internals(
self, api_v3_client, wifi_manager):
wifi_manager.connect_to_network.side_effect = RuntimeError(
"/usr/lib/secret/path blew up")
response = api_v3_client.post(self.URL, json={"ssid": "HomeNet"})
assert response.status_code == 500
body = response.get_json()
assert body["message"] == "An error occurred; see logs for details"
# `details` comes from describe_exception, which is deliberately
# safe to return (redacted, capped) — it names the type.
assert "RuntimeError" in body["details"]
class TestDisconnect:
URL = "/api/v3/wifi/disconnect"
def test_success(self, api_v3_client, wifi_manager):
wifi_manager.disconnect_from_network.return_value = (True, "Disconnected")
response = api_v3_client.post(self.URL)
assert response.status_code == 200
assert response.get_json()["message"] == "Disconnected"
def test_failure(self, api_v3_client, wifi_manager):
wifi_manager.disconnect_from_network.return_value = (False, "Not connected")
response = api_v3_client.post(self.URL)
assert response.status_code == 400
assert response.get_json()["message"] == "Not connected"
def test_failure_without_reason_uses_fallback(self, api_v3_client, wifi_manager):
wifi_manager.disconnect_from_network.return_value = (False, "")
response = api_v3_client.post(self.URL)
assert response.get_json()["message"] == "Failed to disconnect from network"
def test_exception_is_a_500(self, api_v3_client, wifi_manager):
wifi_manager.disconnect_from_network.side_effect = OSError("nmcli missing")
assert api_v3_client.post(self.URL).status_code == 500
class TestApMode:
ENABLE = "/api/v3/wifi/ap/enable"
DISABLE = "/api/v3/wifi/ap/disable"
def test_enable_success(self, api_v3_client, wifi_manager):
wifi_manager.enable_ap_mode.return_value = (True, "AP enabled")
response = api_v3_client.post(self.ENABLE, json={})
assert response.status_code == 200
wifi_manager.enable_ap_mode.assert_called_once_with(force=False)
@pytest.mark.parametrize("raw,expected", [
(True, True), (False, False),
("true", True), ("TRUE", True), ("1", True),
("false", False), ("no", False), ("yes", False),
(1, False), # only real True or the listed strings count
])
def test_force_coercion(self, api_v3_client, wifi_manager, raw, expected):
wifi_manager.enable_ap_mode.return_value = (True, "ok")
api_v3_client.post(self.ENABLE, json={"force": raw})
wifi_manager.enable_ap_mode.assert_called_once_with(force=expected)
def test_enable_without_body(self, api_v3_client, wifi_manager):
wifi_manager.enable_ap_mode.return_value = (True, "ok")
assert api_v3_client.post(self.ENABLE).status_code == 200
def test_enable_failure(self, api_v3_client, wifi_manager):
wifi_manager.enable_ap_mode.return_value = (False, "hostapd missing")
response = api_v3_client.post(self.ENABLE, json={})
assert response.status_code == 400
assert response.get_json()["message"] == "hostapd missing"
def test_disable_success(self, api_v3_client, wifi_manager):
wifi_manager.disable_ap_mode.return_value = (True, "AP disabled")
assert api_v3_client.post(self.DISABLE).status_code == 200
def test_disable_failure(self, api_v3_client, wifi_manager):
wifi_manager.disable_ap_mode.return_value = (False, "not running")
assert api_v3_client.post(self.DISABLE).status_code == 400
def test_enable_exception_is_a_500(self, api_v3_client, wifi_manager):
wifi_manager.enable_ap_mode.side_effect = RuntimeError("boom")
assert api_v3_client.post(self.ENABLE, json={}).status_code == 500
class TestRadio:
URL = "/api/v3/wifi/radio"
def test_get_state(self, api_v3_client, wifi_manager):
wifi_manager.get_wifi_radio_state.return_value = {
"enabled": True, "ethernet_connected": False}
response = api_v3_client.get(self.URL)
assert response.status_code == 200
assert response.get_json()["data"]["enabled"] is True
def test_get_state_exception_is_a_500(self, api_v3_client, wifi_manager):
wifi_manager.get_wifi_radio_state.side_effect = OSError("rfkill missing")
assert api_v3_client.get(self.URL).status_code == 500
def test_enabled_is_required(self, api_v3_client, wifi_manager):
response = api_v3_client.post(self.URL, json={})
assert response.status_code == 400
assert "enabled is required" in response.get_json()["message"]
wifi_manager.set_wifi_radio.assert_not_called()
def test_enable_success(self, api_v3_client, wifi_manager):
wifi_manager.set_wifi_radio.return_value = (True, "Radio on", None)
wifi_manager.get_wifi_radio_state.return_value = {"enabled": True}
response = api_v3_client.post(self.URL, json={"enabled": True})
assert response.status_code == 200
wifi_manager.set_wifi_radio.assert_called_once_with(True, force=False)
@pytest.mark.parametrize("raw,expected", [
(True, True), ("true", True), ("1", True), ("yes", True),
(False, False), ("false", False), ("no", False), (0, False),
])
def test_enabled_coercion_is_string_aware(
self, api_v3_client, wifi_manager, raw, expected):
# bool("false") is True, so the endpoint parses strings explicitly
# rather than trusting truthiness — it is a public contract, not
# only the shipped UI which always sends real JSON booleans.
wifi_manager.set_wifi_radio.return_value = (True, "ok", None)
wifi_manager.get_wifi_radio_state.return_value = {}
api_v3_client.post(self.URL, json={"enabled": raw})
wifi_manager.set_wifi_radio.assert_called_once_with(expected, force=False)
def test_force_passed_through(self, api_v3_client, wifi_manager):
wifi_manager.set_wifi_radio.return_value = (True, "ok", None)
wifi_manager.get_wifi_radio_state.return_value = {}
api_v3_client.post(self.URL, json={"enabled": False, "force": "true"})
wifi_manager.set_wifi_radio.assert_called_once_with(False, force=True)
def test_refusal_reports_reason(self, api_v3_client, wifi_manager):
# Disabling the radio without Ethernet would lock the user out of
# this very interface, so the manager can refuse with a reason.
wifi_manager.set_wifi_radio.return_value = (
False, "Refusing: no wired fallback", "no_ethernet")
response = api_v3_client.post(self.URL, json={"enabled": False})
assert response.status_code == 400
body = response.get_json()
assert body["reason"] == "no_ethernet"
assert "Refusing" in body["message"]
def test_exception_is_a_500(self, api_v3_client, wifi_manager):
wifi_manager.set_wifi_radio.side_effect = RuntimeError("boom")
assert api_v3_client.post(self.URL, json={"enabled": True}).status_code == 500
def test_unrecognized_enabled_value_is_rejected_not_treated_as_false(
self, api_v3_client, wifi_manager):
# An invalid `enabled` used to silently fall back to False, which
# can disconnect Wi-Fi (or, with force=true, drop the caller's own
# connection to this interface) even though nothing asked for that.
response = api_v3_client.post(self.URL, json={"enabled": "typo"})
assert response.status_code == 400
wifi_manager.set_wifi_radio.assert_not_called()
def test_unrecognized_force_value_is_rejected_not_treated_as_false(
self, api_v3_client, wifi_manager):
response = api_v3_client.post(
self.URL, json={"enabled": False, "force": "typo"})
assert response.status_code == 400
wifi_manager.set_wifi_radio.assert_not_called()
class TestAutoEnableApMode:
URL = "/api/v3/wifi/ap/auto-enable"
def test_requires_the_field(self, api_v3_client, wifi_manager):
response = api_v3_client.post(self.URL, json={})
assert response.status_code == 400
@pytest.mark.parametrize("raw,expected", [
(True, True), (False, False),
("true", True), ("True", True), ("1", True), ("yes", True),
("false", False), ("False", False), ("0", False), ("no", False),
(1, True), (0, False),
])
def test_value_is_coerced_not_just_truthy(
self, api_v3_client, wifi_manager, raw, expected):
# Regression: bool(data['auto_enable_ap_mode']) meant a caller who
# sent the JSON string "false" got it stored as True — bool("false")
# is True, since any non-empty string is truthy.
wifi_manager.config = {}
response = api_v3_client.post(self.URL, json={"auto_enable_ap_mode": raw})
assert response.status_code == 200, response.get_json()
assert response.get_json()["data"]["auto_enable_ap_mode"] is expected
assert wifi_manager.config["auto_enable_ap_mode"] is expected
def test_a_string_false_does_not_enable_it(self, api_v3_client, wifi_manager):
# The exact shape of the bug.
wifi_manager.config = {}
api_v3_client.post(self.URL, json={"auto_enable_ap_mode": "false"})
assert wifi_manager.config["auto_enable_ap_mode"] is False
def test_unrecognized_value_is_rejected_not_treated_as_false(
self, api_v3_client, wifi_manager):
wifi_manager.config = {}
response = api_v3_client.post(self.URL, json={"auto_enable_ap_mode": "typo"})
assert response.status_code == 400
assert "auto_enable_ap_mode" not in wifi_manager.config
class TestRadioEnabledAndForceAcceptIntegers:
"""`{"enabled": 1}` / `{"enabled": 0}` used to be mishandled: the old
coercion was `raw is True or (isinstance(raw, str) and ...)`, and
`1 is True` is False in Python -- an int is never the `True` singleton
even though it equals it -- so a plain integer fell through to False
regardless of its value.
"""
URL = "/api/v3/wifi/radio"
@pytest.mark.parametrize("raw,expected", [(1, True), (0, False)])
def test_enabled_as_an_integer(self, api_v3_client, wifi_manager, raw, expected):
wifi_manager.set_wifi_radio.return_value = (True, "ok", None)
wifi_manager.get_wifi_radio_state.return_value = {}
api_v3_client.post(self.URL, json={"enabled": raw})
wifi_manager.set_wifi_radio.assert_called_once_with(expected, force=False)
@pytest.mark.parametrize("raw,expected", [(1, True), (0, False)])
def test_force_as_an_integer(self, api_v3_client, wifi_manager, raw, expected):
wifi_manager.set_wifi_radio.return_value = (True, "ok", None)
wifi_manager.get_wifi_radio_state.return_value = {}
api_v3_client.post(self.URL, json={"enabled": True, "force": raw})
wifi_manager.set_wifi_radio.assert_called_once_with(True, force=expected)
class TestNoRealNetworking:
def test_wifi_manager_is_never_constructed_for_real(self, api_v3_client):
# Guard against a future refactor moving the import to module level,
# where the fixture's patch of the definition site would stop
# applying and the tests would start driving real networking.
with patch("src.wifi_manager.WiFiManager") as cls:
cls.return_value.disconnect_from_network.return_value = (True, "ok")
api_v3_client.post("/api/v3/wifi/disconnect")
assert cls.called