refactor: comprehensive code quality, security, and release readiness improvements
Some checks failed
Lint & Test / test (push) Failing after 48s
Some checks failed
Lint & Test / test (push) Failing after 48s
Security: tighten CORS defaults, add webhook rate limiting, fix XSS in automations, guard WebSocket JSON.parse, validate ADB address input, seal debug exception leak, URL-encode WS tokens, CSS.escape in selectors. Code quality: add Pydantic models for brightness/power endpoints, fix thread safety and name uniqueness in DeviceStore, immutable update pattern, split 6 oversized files into 16 focused modules, enable TypeScript strictNullChecks (741→102 errors), type state variables, add dom-utils helper, migrate 3 modules from inline onclick to event delegation, ProcessorDependencies dataclass. Performance: async store saves, health endpoint log level, command palette debounce, optimized entity-events comparison, fix service worker precache list. Testing: expand from 45 to 293 passing tests — add store tests (141), route tests (25), core logic tests (42), E2E flow tests (33), organize into tests/api/, tests/storage/, tests/core/, tests/e2e/. DevOps: CI test pipeline, pre-commit config, Dockerfile multi-stage build with non-root user and health check, docker-compose improvements, version bump to 0.2.0. Docs: rewrite CLAUDE.md (202→56 lines), server/CLAUDE.md (212→76), create contexts/server-operations.md, fix .js→.ts references, fix env var prefix in README, rewrite INSTALLATION.md, add CONTRIBUTING.md and .env.example.
This commit is contained in:
0
server/tests/api/routes/__init__.py
Normal file
0
server/tests/api/routes/__init__.py
Normal file
196
server/tests/api/routes/test_devices_routes.py
Normal file
196
server/tests/api/routes/test_devices_routes.py
Normal file
@@ -0,0 +1,196 @@
|
||||
"""Tests for device CRUD routes.
|
||||
|
||||
These tests exercise the FastAPI route handlers using dependency override
|
||||
to inject test stores, avoiding real hardware dependencies.
|
||||
"""
|
||||
|
||||
from datetime import datetime, timezone
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from wled_controller.api.routes.devices import router
|
||||
from wled_controller.storage.device_store import Device, DeviceStore
|
||||
from wled_controller.storage.output_target_store import OutputTargetStore
|
||||
from wled_controller.core.processing.processor_manager import ProcessorManager
|
||||
from wled_controller.api import dependencies as deps
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# App + fixtures (isolated from the real main app)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _make_app():
|
||||
"""Build a minimal FastAPI app with just the devices router + overrides."""
|
||||
app = FastAPI()
|
||||
app.include_router(router)
|
||||
return app
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def device_store(tmp_path):
|
||||
return DeviceStore(tmp_path / "devices.json")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def output_target_store(tmp_path):
|
||||
return OutputTargetStore(str(tmp_path / "output_targets.json"))
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def processor_manager():
|
||||
"""A mock ProcessorManager — avoids real hardware."""
|
||||
m = MagicMock(spec=ProcessorManager)
|
||||
m.add_device = MagicMock()
|
||||
m.remove_device = AsyncMock()
|
||||
m.update_device_info = MagicMock()
|
||||
m.find_device_state = MagicMock(return_value=None)
|
||||
m.get_all_device_health_dicts = MagicMock(return_value=[])
|
||||
return m
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client(device_store, output_target_store, processor_manager):
|
||||
app = _make_app()
|
||||
|
||||
# Override auth to always pass
|
||||
from wled_controller.api.auth import verify_api_key
|
||||
app.dependency_overrides[verify_api_key] = lambda: "test-user"
|
||||
|
||||
# Override stores and manager
|
||||
app.dependency_overrides[deps.get_device_store] = lambda: device_store
|
||||
app.dependency_overrides[deps.get_output_target_store] = lambda: output_target_store
|
||||
app.dependency_overrides[deps.get_processor_manager] = lambda: processor_manager
|
||||
|
||||
return TestClient(app, raise_server_exceptions=False)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helper to pre-populate a device
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _seed_device(store: DeviceStore, name="Test Device", led_count=100) -> Device:
|
||||
return store.create_device(
|
||||
name=name,
|
||||
url="http://192.168.1.100",
|
||||
led_count=led_count,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# LIST
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestListDevices:
|
||||
def test_list_empty(self, client):
|
||||
resp = client.get("/api/v1/devices")
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert data["count"] == 0
|
||||
assert data["devices"] == []
|
||||
|
||||
def test_list_with_devices(self, client, device_store):
|
||||
_seed_device(device_store, "Dev A")
|
||||
_seed_device(device_store, "Dev B")
|
||||
resp = client.get("/api/v1/devices")
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert data["count"] == 2
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# GET by ID
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestGetDevice:
|
||||
def test_get_existing(self, client, device_store):
|
||||
d = _seed_device(device_store)
|
||||
resp = client.get(f"/api/v1/devices/{d.id}")
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert data["id"] == d.id
|
||||
assert data["name"] == "Test Device"
|
||||
|
||||
def test_get_not_found(self, client):
|
||||
resp = client.get("/api/v1/devices/nonexistent")
|
||||
assert resp.status_code == 404
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# UPDATE
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestUpdateDevice:
|
||||
def test_update_name(self, client, device_store):
|
||||
d = _seed_device(device_store)
|
||||
resp = client.put(
|
||||
f"/api/v1/devices/{d.id}",
|
||||
json={"name": "Renamed"},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["name"] == "Renamed"
|
||||
|
||||
def test_update_led_count(self, client, device_store):
|
||||
d = _seed_device(device_store, led_count=100)
|
||||
resp = client.put(
|
||||
f"/api/v1/devices/{d.id}",
|
||||
json={"led_count": 300},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["led_count"] == 300
|
||||
|
||||
def test_update_not_found(self, client):
|
||||
resp = client.put(
|
||||
"/api/v1/devices/missing_id",
|
||||
json={"name": "X"},
|
||||
)
|
||||
assert resp.status_code == 404
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# DELETE
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestDeleteDevice:
|
||||
def test_delete_existing(self, client, device_store):
|
||||
d = _seed_device(device_store)
|
||||
resp = client.delete(f"/api/v1/devices/{d.id}")
|
||||
assert resp.status_code == 204
|
||||
assert device_store.count() == 0
|
||||
|
||||
def test_delete_not_found(self, client):
|
||||
resp = client.delete("/api/v1/devices/missing_id")
|
||||
assert resp.status_code == 404
|
||||
|
||||
def test_delete_referenced_by_target_returns_409(
|
||||
self, client, device_store, output_target_store
|
||||
):
|
||||
d = _seed_device(device_store)
|
||||
output_target_store.create_target(
|
||||
name="Target",
|
||||
target_type="led",
|
||||
device_id=d.id,
|
||||
)
|
||||
resp = client.delete(f"/api/v1/devices/{d.id}")
|
||||
assert resp.status_code == 409
|
||||
assert "referenced" in resp.json()["detail"].lower()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Batch states
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestBatchStates:
|
||||
def test_batch_states(self, client):
|
||||
resp = client.get("/api/v1/devices/batch/states")
|
||||
assert resp.status_code == 200
|
||||
assert "states" in resp.json()
|
||||
74
server/tests/api/routes/test_system_routes.py
Normal file
74
server/tests/api/routes/test_system_routes.py
Normal file
@@ -0,0 +1,74 @@
|
||||
"""Tests for system routes — health, version.
|
||||
|
||||
These tests use the FastAPI TestClient against the real app. The health
|
||||
and version endpoints do NOT require authentication, so we can test them
|
||||
without setting up the full dependency injection.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from wled_controller import __version__
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def client():
|
||||
"""Provide a test client for the main app.
|
||||
|
||||
The app module initializes stores from the default config on import,
|
||||
which is acceptable for read-only endpoints tested here.
|
||||
"""
|
||||
from wled_controller.main import app
|
||||
|
||||
return TestClient(app, raise_server_exceptions=False)
|
||||
|
||||
|
||||
class TestHealthEndpoint:
|
||||
def test_health_returns_200(self, client):
|
||||
resp = client.get("/health")
|
||||
assert resp.status_code == 200
|
||||
|
||||
def test_health_response_structure(self, client):
|
||||
data = client.get("/health").json()
|
||||
assert data["status"] == "healthy"
|
||||
assert data["version"] == __version__
|
||||
assert "timestamp" in data
|
||||
|
||||
def test_health_no_auth_required(self, client):
|
||||
"""Health endpoint should work without Authorization header."""
|
||||
resp = client.get("/health")
|
||||
assert resp.status_code == 200
|
||||
|
||||
|
||||
class TestVersionEndpoint:
|
||||
def test_version_returns_200(self, client):
|
||||
resp = client.get("/api/v1/version")
|
||||
assert resp.status_code == 200
|
||||
|
||||
def test_version_response_fields(self, client):
|
||||
data = client.get("/api/v1/version").json()
|
||||
assert data["version"] == __version__
|
||||
assert "python_version" in data
|
||||
assert data["api_version"] == "v1"
|
||||
assert "demo_mode" in data
|
||||
|
||||
|
||||
class TestOpenAPIEndpoint:
|
||||
def test_openapi_available(self, client):
|
||||
resp = client.get("/openapi.json")
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert "info" in data
|
||||
assert data["info"]["version"] == __version__
|
||||
|
||||
def test_swagger_ui(self, client):
|
||||
resp = client.get("/docs")
|
||||
assert resp.status_code == 200
|
||||
assert "text/html" in resp.headers["content-type"]
|
||||
|
||||
|
||||
class TestRootEndpoint:
|
||||
def test_root_returns_html(self, client):
|
||||
resp = client.get("/")
|
||||
assert resp.status_code == 200
|
||||
assert "text/html" in resp.headers["content-type"]
|
||||
67
server/tests/api/routes/test_webhooks_routes.py
Normal file
67
server/tests/api/routes/test_webhooks_routes.py
Normal file
@@ -0,0 +1,67 @@
|
||||
"""Tests for webhook routes — trigger, validation, rate limiting."""
|
||||
|
||||
import time
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from wled_controller.api.routes.webhooks import _check_rate_limit, _rate_hits
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Rate limiter unit tests (pure function, no HTTP)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestRateLimiter:
|
||||
def setup_method(self):
|
||||
"""Clear rate-limit state between tests."""
|
||||
_rate_hits.clear()
|
||||
|
||||
def test_allows_under_limit(self):
|
||||
for _ in range(29):
|
||||
_check_rate_limit("1.2.3.4") # should not raise
|
||||
|
||||
def test_rejects_at_limit(self):
|
||||
for _ in range(30):
|
||||
_check_rate_limit("1.2.3.4")
|
||||
from fastapi import HTTPException
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
_check_rate_limit("1.2.3.4")
|
||||
assert exc_info.value.status_code == 429
|
||||
|
||||
def test_separate_ips_independent(self):
|
||||
for _ in range(30):
|
||||
_check_rate_limit("10.0.0.1")
|
||||
# Different IP should still be allowed
|
||||
_check_rate_limit("10.0.0.2") # should not raise
|
||||
|
||||
def test_window_expiry(self):
|
||||
"""Timestamps outside the 60s window are pruned."""
|
||||
old_time = time.time() - 120 # 2 minutes ago
|
||||
_rate_hits["1.2.3.4"] = [old_time] * 30
|
||||
# Old entries should be pruned, allowing new requests
|
||||
_check_rate_limit("1.2.3.4") # should not raise
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Webhook payload validation
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestWebhookPayload:
|
||||
def test_valid_payload_model(self):
|
||||
from wled_controller.api.routes.webhooks import WebhookPayload
|
||||
|
||||
p = WebhookPayload(action="activate")
|
||||
assert p.action == "activate"
|
||||
|
||||
p2 = WebhookPayload(action="deactivate")
|
||||
assert p2.action == "deactivate"
|
||||
|
||||
def test_arbitrary_action_accepted_by_model(self):
|
||||
"""The model accepts any string; validation is in the route handler."""
|
||||
from wled_controller.api.routes.webhooks import WebhookPayload
|
||||
|
||||
p = WebhookPayload(action="bogus")
|
||||
assert p.action == "bogus"
|
||||
Reference in New Issue
Block a user