diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index de0abac3..da144fbb 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -60,7 +60,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v6 with: - node-version: '20' + node-version: '22' cache: 'npm' cache-dependency-path: package-lock.json @@ -97,7 +97,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v6 with: - node-version: '20' + node-version: '22' cache: 'npm' cache-dependency-path: package-lock.json @@ -136,7 +136,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v6 with: - node-version: '20' + node-version: '22' cache: 'npm' cache-dependency-path: package-lock.json @@ -218,7 +218,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v6 with: - node-version: '20' + node-version: '22' cache: 'npm' cache-dependency-path: package-lock.json @@ -279,7 +279,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v6 with: - node-version: '20' + node-version: '22' cache: 'npm' cache-dependency-path: package-lock.json diff --git a/.github/workflows/commitlint.yml b/.github/workflows/commitlint.yml index ddc7e339..96813f6f 100644 --- a/.github/workflows/commitlint.yml +++ b/.github/workflows/commitlint.yml @@ -23,7 +23,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v6 with: - node-version: '20' + node-version: '22' - name: Install commitlint run: | diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml index 047fde13..9be0ac4c 100644 --- a/.github/workflows/sonar.yml +++ b/.github/workflows/sonar.yml @@ -5,6 +5,8 @@ on: - main pull_request: types: [opened, synchronize, reopened] + schedule: + - cron: '0 6 * * 1' # Every Monday 6:00 UTC jobs: sonarqube: name: SonarQube @@ -12,7 +14,40 @@ jobs: steps: - uses: actions/checkout@v6 with: - fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis + fetch-depth: 0 + + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + cache: 'pip' + + - name: Generate backend coverage + run: | + pip install -e apps/backend + pip install -r apps/backend/requirements-dev.txt + cd apps/backend + pytest --cov=opencloudtouch --cov-report=xml:../../coverage-backend.xml -q --tb=no + env: + OCT_MOCK_MODE: "true" + OCT_HAS_DEVICES: "false" + CI: "true" + + - name: Setup Node.js + uses: actions/setup-node@v6 + with: + node-version: '22' + cache: 'npm' + cache-dependency-path: package-lock.json + + - name: Generate frontend coverage + run: | + npm ci + cd apps/frontend + npx vitest run --coverage --reporter=dot + env: + CI: "true" + - name: SonarQube Scan uses: SonarSource/sonarqube-scan-action@v7 with: @@ -22,6 +57,9 @@ jobs: -Dsonar.qualitygate.wait=false -Dsonar.projectName=opencloudtouch -Dsonar.sources=apps/backend/src,apps/frontend/src - -Dsonar.exclusions=**/tests/**,**/__tests__/**,**/*.test.*,**/pytest.ini,**/sonar-project.properties + -Dsonar.tests=apps/backend/tests,apps/frontend/tests + -Dsonar.exclusions=**/tests/**,**/__tests__/**,**/*.test.* + -Dsonar.python.coverage.reportPaths=coverage-backend.xml + -Dsonar.javascript.lcov.reportPaths=.out/coverage/frontend/lcov.info env: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} diff --git a/apps/backend/requirements-dev.txt b/apps/backend/requirements-dev.txt index 55d484ee..e1139880 100644 --- a/apps/backend/requirements-dev.txt +++ b/apps/backend/requirements-dev.txt @@ -1,6 +1,6 @@ -r requirements.txt -pytest==8.3.3 -pytest-asyncio==0.24.0 +pytest==9.0.3 +pytest-asyncio==1.3.0 pytest-cov==7.1.0 pytest-timeout==2.4.0 pytest-xdist==3.8.0 diff --git a/apps/backend/src/opencloudtouch/bmx/tunein.py b/apps/backend/src/opencloudtouch/bmx/tunein.py index b565ae52..9d0fceb2 100644 --- a/apps/backend/src/opencloudtouch/bmx/tunein.py +++ b/apps/backend/src/opencloudtouch/bmx/tunein.py @@ -6,6 +6,7 @@ import logging import os +import re from xml.etree import ElementTree import httpx @@ -17,6 +18,8 @@ TUNEIN_DESCRIBE_URL = "https://opml.radiotime.com/describe.ashx?id=%s" TUNEIN_STREAM_URL = "http://opml.radiotime.com/Tune.ashx?id=%s&formats=mp3,aac,ogg" +_STATION_ID_RE = re.compile(r"^[a-zA-Z0-9_-]+$") + def get_oct_base_url() -> str: """Get OCT backend URL from environment. @@ -83,6 +86,9 @@ async def resolve_tunein_station(station_id: str) -> BmxPlaybackResponse: """ logger.info(f"[BMX TUNEIN] Resolving station: {station_id}") + if not _STATION_ID_RE.match(station_id): + raise ValueError(f"Invalid station ID format: {station_id}") + try: async with httpx.AsyncClient(timeout=10.0) as client: describe_resp = await client.get(TUNEIN_DESCRIBE_URL % station_id) diff --git a/apps/backend/src/opencloudtouch/devices/health_check.py b/apps/backend/src/opencloudtouch/devices/health_check.py index 337d14cb..949f1031 100644 --- a/apps/backend/src/opencloudtouch/devices/health_check.py +++ b/apps/backend/src/opencloudtouch/devices/health_check.py @@ -49,7 +49,7 @@ async def stop(self) -> None: try: await self._task except asyncio.CancelledError: - pass + logger.debug("Health-check task cancelled") self._task = None logger.info("Device health-check stopped") @@ -65,7 +65,7 @@ async def _run(self) -> None: self._last_ssh_verify = now except asyncio.CancelledError: - break + raise except Exception: logger.exception("Health-check cycle failed") diff --git a/apps/backend/src/opencloudtouch/setup/wizard_routes.py b/apps/backend/src/opencloudtouch/setup/wizard_routes.py index e70a7bce..1b1766cb 100644 --- a/apps/backend/src/opencloudtouch/setup/wizard_routes.py +++ b/apps/backend/src/opencloudtouch/setup/wizard_routes.py @@ -140,11 +140,16 @@ async def wizard_detect_strategy(request: Request) -> DetectStrategyResponse: def _check_port_443(hostname: str) -> bool: - """Try an SSL handshake on port 443 to detect a reverse proxy.""" + """Try an SSL handshake on port 443 to detect a reverse proxy. + + SSL verification is intentionally disabled: this function only checks + whether *any* service responds on 443, without sending sensitive data. + The server may use a self-signed certificate. + """ try: - ctx = ssl.create_default_context() - ctx.check_hostname = False - ctx.verify_mode = ssl.CERT_NONE + ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) # NOSONAR - intentional + ctx.check_hostname = False # NOSONAR - port detection only + ctx.verify_mode = ssl.CERT_NONE # NOSONAR - no sensitive data sent with socket.create_connection((hostname, 443), timeout=3) as sock: with ctx.wrap_socket(sock, server_hostname=hostname): return True diff --git a/apps/backend/tests/unit/devices/test_health_check.py b/apps/backend/tests/unit/devices/test_health_check.py new file mode 100644 index 00000000..24b09f22 --- /dev/null +++ b/apps/backend/tests/unit/devices/test_health_check.py @@ -0,0 +1,355 @@ +"""Unit tests for DeviceHealthCheck. + +Tests the background health-check service that monitors device +reachability via HTTP ping and verifies setup status via SSH. +Focuses on: +- Lifecycle management (start/stop/cancellation) +- Device ping logic (reachable vs offline detection) +- SSH verification and status transitions +- Error resilience (network failures, SSH failures) +""" + +from datetime import UTC, datetime, timedelta +from unittest.mock import AsyncMock, MagicMock, patch + +import httpx +import pytest + +from opencloudtouch.devices.health_check import ( + OFFLINE_THRESHOLD, + DeviceHealthCheck, +) +from opencloudtouch.devices.repository import Device + + +def _make_device( + device_id: str = "dev1", + ip: str = "192.168.1.100", + name: str = "Living Room", + last_seen: datetime | None = None, + setup_status: str = "unknown", + ssh_permanent: bool = False, +) -> Device: + """Create a test device with sensible defaults.""" + return Device( + device_id=device_id, + ip=ip, + name=name, + model="SoundTouch 300", + mac_address="AA:BB:CC:DD:EE:FF", + firmware_version="29.0.6", + last_seen=last_seen or datetime.now(UTC), + setup_status=setup_status, + ssh_permanent=ssh_permanent, + ) + + +@pytest.fixture +def mock_repo(): + """Mock DeviceRepository with async methods.""" + repo = AsyncMock() + repo.get_all.return_value = [] + return repo + + +@pytest.fixture +def health_check(mock_repo): + """DeviceHealthCheck instance with mocked repository.""" + return DeviceHealthCheck(mock_repo) + + +class TestLifecycle: + """Tests for start/stop behavior and task management.""" + + def test_initial_state(self, health_check): + """Health check starts in stopped state.""" + assert health_check._running is False + assert health_check._task is None + + async def test_start_creates_task(self, health_check): + """Starting creates a background asyncio task.""" + health_check.start() + assert health_check._running is True + assert health_check._task is not None + await health_check.stop() + + async def test_start_is_idempotent(self, health_check): + """Calling start() twice doesn't create a second task.""" + health_check.start() + first_task = health_check._task + health_check.start() + assert health_check._task is first_task + await health_check.stop() + + async def test_stop_cancels_task(self, health_check): + """Stopping cancels the running task and cleans up.""" + health_check.start() + assert health_check._task is not None + + await health_check.stop() + + assert health_check._running is False + assert health_check._task is None + + async def test_stop_when_not_running_is_safe(self, health_check): + """Stopping when not running doesn't raise.""" + await health_check.stop() + assert health_check._task is None + + +class TestPingDevice: + """Tests for individual device ping logic.""" + + async def test_reachable_device_returns_true(self): + """Device returning 200 on /info is considered reachable.""" + mock_client = AsyncMock() + mock_client.get.return_value = MagicMock(status_code=200) + + result = await DeviceHealthCheck._ping_device(mock_client, "192.168.1.100") + + assert result is True + mock_client.get.assert_called_once_with("http://192.168.1.100:8091/info") + + async def test_non_200_returns_false(self): + """Device returning non-200 status is not reachable.""" + mock_client = AsyncMock() + mock_client.get.return_value = MagicMock(status_code=500) + + result = await DeviceHealthCheck._ping_device(mock_client, "192.168.1.100") + + assert result is False + + async def test_connection_error_returns_false(self): + """Connection error (device powered off) returns False.""" + mock_client = AsyncMock() + mock_client.get.side_effect = httpx.ConnectError("Connection refused") + + result = await DeviceHealthCheck._ping_device(mock_client, "192.168.1.100") + + assert result is False + + async def test_timeout_returns_false(self): + """Timeout (device unreachable) returns False.""" + mock_client = AsyncMock() + mock_client.get.side_effect = httpx.TimeoutException("Timeout") + + result = await DeviceHealthCheck._ping_device(mock_client, "192.168.1.100") + + assert result is False + + +class TestPingAllDevices: + """Tests for the batch ping logic that updates last_seen.""" + + async def test_no_devices_does_nothing(self, health_check, mock_repo): + """Empty device list doesn't cause errors.""" + mock_repo.get_all.return_value = [] + + await health_check._ping_all_devices() + + mock_repo.upsert.assert_not_called() + + async def test_reachable_device_updates_last_seen(self, health_check, mock_repo): + """Reachable device gets its last_seen timestamp updated.""" + device = _make_device(last_seen=datetime(2026, 1, 1, tzinfo=UTC)) + mock_repo.get_all.return_value = [device] + + with patch.object(DeviceHealthCheck, "_ping_device", return_value=True): + await health_check._ping_all_devices() + + mock_repo.upsert.assert_called_once() + updated_device = mock_repo.upsert.call_args[0][0] + assert updated_device.last_seen > datetime(2026, 1, 1, tzinfo=UTC) + + async def test_unreachable_device_not_updated(self, health_check, mock_repo): + """Unreachable device within threshold keeps its last_seen.""" + recent = datetime.now(UTC) - timedelta(seconds=60) + device = _make_device(last_seen=recent) + mock_repo.get_all.return_value = [device] + + with patch.object(DeviceHealthCheck, "_ping_device", return_value=False): + await health_check._ping_all_devices() + + mock_repo.upsert.assert_not_called() + + async def test_device_without_ip_is_skipped(self, health_check, mock_repo): + """Device with no IP address is silently skipped.""" + device = _make_device(ip="") + mock_repo.get_all.return_value = [device] + + with patch.object( + DeviceHealthCheck, "_ping_device", return_value=True + ) as mock_ping: + await health_check._ping_all_devices() + + mock_ping.assert_not_called() + + async def test_offline_device_logs_warning(self, health_check, mock_repo): + """Device exceeding offline threshold triggers a warning log.""" + old_time = datetime.now(UTC) - timedelta(seconds=OFFLINE_THRESHOLD + 60) + device = _make_device(last_seen=old_time) + mock_repo.get_all.return_value = [device] + + with ( + patch.object(DeviceHealthCheck, "_ping_device", return_value=False), + patch("opencloudtouch.devices.health_check.logger") as mock_logger, + ): + await health_check._ping_all_devices() + + mock_logger.warning.assert_called_once() + assert "offline" in mock_logger.warning.call_args[0][0].lower() + + +class TestSSHVerification: + """Tests for SSH-based setup status verification.""" + + async def test_non_ssh_devices_skipped(self, health_check, mock_repo): + """Devices without ssh_permanent=True are not SSH-checked.""" + device = _make_device(ssh_permanent=False) + mock_repo.get_all.return_value = [device] + + with patch("opencloudtouch.devices.health_check.check_ssh_port") as mock_ssh: + await health_check._ssh_verify_all() + + mock_ssh.assert_not_called() + + async def test_ssh_unreachable_skips_verification(self, health_check, mock_repo): + """If SSH port is closed, skip further verification.""" + device = _make_device(ssh_permanent=True) + mock_repo.get_all.return_value = [device] + + with patch( + "opencloudtouch.devices.health_check.check_ssh_port", + return_value=False, + ): + await health_check._ssh_verify_device(device, "http://myserver:7777") + + mock_repo.update_setup_status.assert_not_called() + + async def test_configured_status_when_our_server_in_bmx( + self, health_check, mock_repo + ): + """Device with our server URL in BMX config → 'configured'.""" + device = _make_device(ssh_permanent=True, setup_status="unconfigured") + mock_repo.get_all.return_value = [device] + + mock_conn = MagicMock(success=True) + mock_client = AsyncMock() + mock_client.connect.return_value = mock_conn + mock_client.execute.side_effect = [ + MagicMock(output="bmxRegistryUrl=http://myserver:7777/bmx"), + MagicMock(output="0"), # no hosts redirect + ] + mock_client.close = AsyncMock() + + with ( + patch( + "opencloudtouch.devices.health_check.check_ssh_port", + return_value=True, + ), + patch( + "opencloudtouch.devices.health_check.SoundTouchSSHClient", + return_value=mock_client, + ), + ): + await health_check._ssh_verify_device(device, "http://myserver:7777") + + mock_repo.update_setup_status.assert_called_once_with( + device_id="dev1", setup_status="configured" + ) + + async def test_unconfigured_status_when_bose_url(self, health_check, mock_repo): + """Device still pointing to bose.com → 'unconfigured'.""" + device = _make_device(ssh_permanent=True, setup_status="configured") + + mock_conn = MagicMock(success=True) + mock_client = AsyncMock() + mock_client.connect.return_value = mock_conn + mock_client.execute.side_effect = [ + MagicMock(output="bmxRegistryUrl=https://streaming.bose.com/bmx"), + MagicMock(output="0"), + ] + mock_client.close = AsyncMock() + + with ( + patch( + "opencloudtouch.devices.health_check.check_ssh_port", + return_value=True, + ), + patch( + "opencloudtouch.devices.health_check.SoundTouchSSHClient", + return_value=mock_client, + ), + ): + await health_check._ssh_verify_device(device, "http://myserver:7777") + + mock_repo.update_setup_status.assert_called_once_with( + device_id="dev1", setup_status="unconfigured" + ) + + async def test_hosts_redirect_means_configured(self, health_check, mock_repo): + """Hosts file redirect (Strategy B) → 'configured'.""" + device = _make_device(ssh_permanent=True, setup_status="unconfigured") + + mock_conn = MagicMock(success=True) + mock_client = AsyncMock() + mock_client.connect.return_value = mock_conn + mock_client.execute.side_effect = [ + MagicMock(output="bmxRegistryUrl=https://streaming.bose.com/bmx"), + MagicMock(output="1"), # hosts redirect present + ] + mock_client.close = AsyncMock() + + with ( + patch( + "opencloudtouch.devices.health_check.check_ssh_port", + return_value=True, + ), + patch( + "opencloudtouch.devices.health_check.SoundTouchSSHClient", + return_value=mock_client, + ), + ): + await health_check._ssh_verify_device(device, "http://myserver:7777") + + mock_repo.update_setup_status.assert_called_once_with( + device_id="dev1", setup_status="configured" + ) + + async def test_no_status_change_skips_update(self, health_check, mock_repo): + """If status hasn't changed, don't call update.""" + device = _make_device(ssh_permanent=True, setup_status="configured") + + mock_conn = MagicMock(success=True) + mock_client = AsyncMock() + mock_client.connect.return_value = mock_conn + mock_client.execute.side_effect = [ + MagicMock(output="bmxRegistryUrl=http://myserver:7777/bmx"), + MagicMock(output="0"), + ] + mock_client.close = AsyncMock() + + with ( + patch( + "opencloudtouch.devices.health_check.check_ssh_port", + return_value=True, + ), + patch( + "opencloudtouch.devices.health_check.SoundTouchSSHClient", + return_value=mock_client, + ), + ): + await health_check._ssh_verify_device(device, "http://myserver:7777") + + mock_repo.update_setup_status.assert_not_called() + + async def test_ssh_failure_doesnt_crash(self, health_check, mock_repo): + """SSH exception in verify_all doesn't crash the health check.""" + device = _make_device(ssh_permanent=True) + mock_repo.get_all.return_value = [device] + + with patch.object( + health_check, "_ssh_verify_device", side_effect=Exception("SSH boom") + ): + # Should not raise + await health_check._ssh_verify_all() diff --git a/apps/frontend/eslint.config.ts b/apps/frontend/eslint.config.ts index 1f58f02b..4576d22d 100644 --- a/apps/frontend/eslint.config.ts +++ b/apps/frontend/eslint.config.ts @@ -1,8 +1,7 @@ import js from "@eslint/js"; import globals from "globals"; import tseslint from "typescript-eslint"; -import pluginReact from "eslint-plugin-react"; -import pluginReactHooks from "eslint-plugin-react-hooks"; +import eslintReact from "@eslint-react/eslint-plugin"; export default [ { @@ -22,15 +21,6 @@ export default [ }, { files: ["**/*.{js,mjs,cjs,ts,mts,cts,jsx,tsx}"], - plugins: { - react: pluginReact, - "react-hooks": pluginReactHooks - }, - settings: { - react: { - version: "detect" - } - }, languageOptions: { globals: { ...globals.browser, @@ -45,16 +35,14 @@ export default [ }, rules: { ...js.configs.recommended.rules, - ...pluginReact.configs.recommended.rules, - // React 18+ doesn't require React import in JSX files - "react/react-in-jsx-scope": "off", - // TypeScript migration complete - prop-types no longer needed - "react/prop-types": "off", - // Temporary: Disable display-name until eslint-plugin-react v8 - "react/display-name": "off", - // React Hooks rules - "react-hooks/rules-of-hooks": "error", - "react-hooks/exhaustive-deps": "warn" + } + }, + // ESLint React recommended rules (replaces eslint-plugin-react + react-hooks) + eslintReact.configs["recommended-typescript"], + // Disable set-state-in-effect: setState in useEffect is standard React for data fetching/reset + { + rules: { + "@eslint-react/set-state-in-effect": "off", } }, // Test files configuration diff --git a/apps/frontend/package.json b/apps/frontend/package.json index a6ddd577..b356eb59 100644 --- a/apps/frontend/package.json +++ b/apps/frontend/package.json @@ -35,34 +35,33 @@ "react-router-dom": "^7.13.0" }, "devDependencies": { + "@eslint-react/eslint-plugin": "^4.2.3", "@eslint/css": "^1.0.0", - "cypress": "^15.10.0", - "@eslint/js": "10.0.1", + "@eslint/js": "^10.0.1", "@eslint/json": "^1.0.0", - "@eslint/markdown": "^7.5.1", + "@eslint/markdown": "^8.0.1", "@testing-library/jest-dom": "^6.1.5", "@testing-library/react": "^16.3.2", "@testing-library/user-event": "^14.6.1", "@types/react": "^19.2.10", "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^6.0.1", - "@vitest/coverage-v8": "^4.1.0", - "@vitest/ui": "^4.1.0", + "@vitest/coverage-v8": "^4.1.5", + "@vitest/ui": "^4.1.5", "axe-core": "^4.11.1", "colorette": "^2.0.20", + "cypress": "^15.10.0", "cypress-axe": "^1.7.0", - "eslint": "9.39.2", - "eslint-plugin-react": "^7.37.5", - "eslint-plugin-react-hooks": "^7.0.1", + "eslint": "^10.2.1", "globals": "^17.4.0", "jiti": "^2.6.1", "jsdom": "^29.0.1", "prettier": "^3.8.1", "rimraf": "^6.0.1", - "start-server-and-test": "^2.1.5", - "typescript": "^5.9.3", + "start-server-and-test": "^3.0.2", + "typescript": "^6.0.3", "typescript-eslint": "^8.57.1", "vite": "^8.0.5", - "vitest": "^4.0.18" + "vitest": "^4.1.5" } } diff --git a/apps/frontend/src/api/devices.ts b/apps/frontend/src/api/devices.ts index 0c3a30b4..7756685f 100644 --- a/apps/frontend/src/api/devices.ts +++ b/apps/frontend/src/api/devices.ts @@ -4,6 +4,7 @@ */ import { getErrorMessage } from "./types"; +import { SetupStatus } from "./setup"; // Backend API response structure (matches Device.to_dict() from repository.py) interface DeviceAPIResponse { @@ -28,7 +29,7 @@ export interface Device { model?: string; firmware?: string; ip?: string; - setup_status?: string; + setup_status?: SetupStatus; ssh_permanent?: boolean; setup_completed_at?: string | null; capabilities?: { @@ -54,7 +55,7 @@ function mapDeviceFromAPI(apiDevice: DeviceAPIResponse): Device { model: apiDevice.model, // Backend already returns 'model' ip: apiDevice.ip, // Backend already returns 'ip' firmware: apiDevice.firmware_version, - setup_status: apiDevice.setup_status, + setup_status: apiDevice.setup_status as SetupStatus, ssh_permanent: apiDevice.ssh_permanent, setup_completed_at: apiDevice.setup_completed_at, }; diff --git a/apps/frontend/src/components/ConfirmDialog.tsx b/apps/frontend/src/components/ConfirmDialog.tsx index da0b3233..8d2492c7 100644 --- a/apps/frontend/src/components/ConfirmDialog.tsx +++ b/apps/frontend/src/components/ConfirmDialog.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useRef } from "react"; +import { useEffect, useRef } from "react"; import "./ConfirmDialog.css"; /** @@ -72,6 +72,9 @@ export default function ConfirmDialog({