From 4736035603acc57cbbdea9bbd9db10d1e2427785 Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Fri, 10 Jul 2026 17:17:34 +0200 Subject: [PATCH 1/5] =?UTF-8?q?perf(db):=20bound=20health=5Fevents=20growt?= =?UTF-8?q?h=20=E2=80=94=20split=20retention=20+=20weekly=20VACUUM?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit health_events is dominated by check_ok/check_down uptime samples (one per service every 5 minutes), yet get_health_scores only reads a bounded window (/api/health/scores caps it at 90 days) while retention kept everything 400 days. Trim samples to HEALTH_CHECK_RETENTION_DAYS=95 (just past the max query window, so no allowed query can out-range its own samples) while keeping the rare lifecycle events (start/stop/restart/crash) the full 400 days. Add a weekly VACUUM job (commit-first, WAL-checkpoint TRUNCATE) since SQLite never shrinks the file on DELETE alone — without it the DB keeps its high-water-mark size forever. Uptime% is unchanged for every allowed query. Tests cover the split-retention boundary + a clean VACUUM run. From 42a1e494283b6b2f0d68a1d27d44015b47c36aaf Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Fri, 10 Jul 2026 17:24:25 +0200 Subject: [PATCH 2/5] test: cover orchestrator/worker_api Docker paths; expect db_vacuum job Add 61 tests (Docker SDK fully mocked) for the two lowest-covered, most privileged modules: orchestrator.py 38%->73%, worker_api.py 44%->74%. Covers _collect_stats (zero-delta CPU, missing keys, APIError), _find_container name/ label/ValueError fallback, get_status[_light] docker-unavailable + corrupted- container-skip paths, _verify_api_key negative paths, _validate_deploy_spec rejections (privileged, cap_add, network_mode, blocked volume roots incl. subpaths; host/bridge + named volumes allowed), and every command endpoint's success + ValueError->404 / RuntimeError->503 branches. Also update the lifespan-scheduler test to expect the new db_vacuum interval job (added in the previous commit). --- tests/test_main_routes.py | 3 +- tests/test_orchestrator_coverage.py | 301 +++++++++++++++++++++++++ tests/test_worker_api_coverage.py | 333 ++++++++++++++++++++++++++++ 3 files changed, 636 insertions(+), 1 deletion(-) create mode 100644 tests/test_orchestrator_coverage.py create mode 100644 tests/test_worker_api_coverage.py diff --git a/tests/test_main_routes.py b/tests/test_main_routes.py index 6dba698..c66de19 100644 --- a/tests/test_main_routes.py +++ b/tests/test_main_routes.py @@ -1800,12 +1800,13 @@ async def _run(): async with main_mod.lifespan(main_mod.app): sched = main_mod.scheduler jobs = {j.id: j for j in sched.get_jobs()} - # All five interval jobs registered. + # All interval jobs registered. assert set(jobs) == { "collect", "health_check", "stale_workers", "data_retention", + "db_vacuum", "exchange_rates", } # Every job carries the hardening kwargs (audit fix). diff --git a/tests/test_orchestrator_coverage.py b/tests/test_orchestrator_coverage.py new file mode 100644 index 0000000..8a9f23b --- /dev/null +++ b/tests/test_orchestrator_coverage.py @@ -0,0 +1,301 @@ +"""Tests targeting uncovered lines in app/orchestrator.py. + +Covers get_status / get_status_light (running / exited / missing-container / +docker-unavailable branches and the image-matched external-container path), +_collect_stats CPU/memory parsing (including zero-delta and error edge +cases), and _find_container's label-based fallback lookup. + +Mocks the Docker SDK entirely — no real Docker socket is used. +""" + +from __future__ import annotations + +import os +from unittest.mock import MagicMock, patch + +os.environ.setdefault("CASHPILOT_API_KEY", "test-fleet-key") + +import pytest # noqa: E402 + +docker = pytest.importorskip("docker") # noqa: E402 + +from docker.errors import APIError, NotFound # noqa: E402 + +from app import orchestrator # noqa: E402 +from app.constants import LABEL_MANAGED, LABEL_SERVICE # noqa: E402 + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _mock_container(*, name, status, slug, deployed_by="worker", category="bandwidth", container_id="short123"): + c = MagicMock() + c.id = f"id-{name}" + c.name = name + c.status = status + c.short_id = container_id + c.labels = { + LABEL_SERVICE: slug, + LABEL_MANAGED: "true", + "cashpilot.deployed-by": deployed_by, + "cashpilot.category": category, + } + c.image.tags = [f"{slug}:latest"] + c.image.short_id = "sha256:abcdef" + c.attrs = {"Created": "2026-01-01T00:00:00Z"} + return c + + +def _zero_stats(): + return { + "cpu_stats": {"cpu_usage": {"total_usage": 1, "percpu_usage": [1]}, "system_cpu_usage": 10}, + "precpu_stats": {"cpu_usage": {"total_usage": 0}, "system_cpu_usage": 5}, + "memory_stats": {"usage": 0}, + } + + +# --------------------------------------------------------------------------- +# _collect_stats +# --------------------------------------------------------------------------- + + +class TestCollectStats: + def test_parses_cpu_and_memory(self): + c = MagicMock() + c.stats.return_value = { + "cpu_stats": { + "cpu_usage": {"total_usage": 2_000_000_000, "percpu_usage": [1, 1]}, + "system_cpu_usage": 100_000_000_000, + "online_cpus": 2, + }, + "precpu_stats": { + "cpu_usage": {"total_usage": 1_000_000_000}, + "system_cpu_usage": 90_000_000_000, + }, + "memory_stats": {"usage": 209_715_200}, # 200 MB + } + cpu_pct, mem_mb = orchestrator._collect_stats(c) + assert cpu_pct == 20.0 + assert mem_mb == 200.0 + + def test_zero_system_delta_returns_zero_cpu(self): + c = MagicMock() + c.stats.return_value = { + "cpu_stats": {"cpu_usage": {"total_usage": 500, "percpu_usage": [1]}, "system_cpu_usage": 500}, + "precpu_stats": {"cpu_usage": {"total_usage": 500}, "system_cpu_usage": 500}, + "memory_stats": {"usage": 0}, + } + cpu_pct, mem_mb = orchestrator._collect_stats(c) + assert cpu_pct == 0.0 + assert mem_mb == 0.0 + + def test_missing_memory_usage_defaults_to_zero(self): + c = MagicMock() + c.stats.return_value = { + "cpu_stats": {"cpu_usage": {"total_usage": 100, "percpu_usage": [1]}, "system_cpu_usage": 500}, + "precpu_stats": {"cpu_usage": {"total_usage": 50}, "system_cpu_usage": 400}, + "memory_stats": {}, # no "usage" key + } + _, mem_mb = orchestrator._collect_stats(c) + assert mem_mb == 0.0 + + def test_missing_key_returns_zero_tuple(self): + c = MagicMock() + c.stats.return_value = {"cpu_stats": {}} # precpu_stats missing -> KeyError + assert orchestrator._collect_stats(c) == (0.0, 0.0) + + def test_stats_api_error_returns_zero_tuple(self): + c = MagicMock() + c.stats.side_effect = APIError("stats unavailable") + assert orchestrator._collect_stats(c) == (0.0, 0.0) + + +# --------------------------------------------------------------------------- +# _find_container +# --------------------------------------------------------------------------- + + +class TestFindContainer: + def test_finds_by_name(self): + container = MagicMock() + client = MagicMock() + client.containers.get.return_value = container + with patch.object(orchestrator, "_get_client", return_value=client): + result = orchestrator._find_container("honeygain") + assert result is container + client.containers.get.assert_called_once_with("cashpilot-honeygain") + client.containers.list.assert_not_called() + + def test_falls_back_to_label_lookup_when_renamed(self): + container = MagicMock() + client = MagicMock() + client.containers.get.side_effect = NotFound("nope") + client.containers.list.return_value = [container] + with patch.object(orchestrator, "_get_client", return_value=client): + result = orchestrator._find_container("honeygain") + assert result is container + _, kwargs = client.containers.list.call_args + assert kwargs["filters"]["label"] == [ + f"{LABEL_SERVICE}=honeygain", + f"{LABEL_MANAGED}=true", + ] + + def test_raises_value_error_when_not_found_anywhere(self): + client = MagicMock() + client.containers.get.side_effect = NotFound("nope") + client.containers.list.return_value = [] + with ( + patch.object(orchestrator, "_get_client", return_value=client), + pytest.raises(ValueError, match="honeygain"), + ): + orchestrator._find_container("honeygain") + + +# --------------------------------------------------------------------------- +# get_status (slow path, includes CPU/mem stats) +# --------------------------------------------------------------------------- + + +class TestGetStatus: + def test_docker_unavailable_returns_empty(self): + with patch.object(orchestrator, "_get_client", side_effect=RuntimeError("no socket")): + assert orchestrator.get_status() == [] + + def test_running_container_reports_stats(self): + container = _mock_container(name="cashpilot-honeygain", status="running", slug="honeygain") + container.stats.return_value = { + "cpu_stats": {"cpu_usage": {"total_usage": 100, "percpu_usage": [1]}, "system_cpu_usage": 500}, + "precpu_stats": {"cpu_usage": {"total_usage": 50}, "system_cpu_usage": 400}, + "memory_stats": {"usage": 1_048_576}, + } + client = MagicMock() + client.containers.list.return_value = [container] + with ( + patch.object(orchestrator, "_get_client", return_value=client), + patch.object(orchestrator, "_build_image_slug_map", return_value={}), + ): + results = orchestrator.get_status() + assert len(results) == 1 + row = results[0] + assert row["slug"] == "honeygain" + assert row["status"] == "running" + assert row["memory_mb"] == 1.0 + + def test_exited_container_missing_stats_handled_gracefully(self): + container = _mock_container(name="cashpilot-earnapp", status="exited", slug="earnapp") + container.stats.side_effect = APIError("exited, no stats") + client = MagicMock() + client.containers.list.return_value = [container] + with ( + patch.object(orchestrator, "_get_client", return_value=client), + patch.object(orchestrator, "_build_image_slug_map", return_value={}), + ): + results = orchestrator.get_status() + assert results[0]["status"] == "exited" + assert results[0]["cpu_percent"] == 0.0 + assert results[0]["memory_mb"] == 0.0 + + def test_corrupted_container_is_skipped_not_crashed(self): + good = _mock_container(name="cashpilot-honeygain", status="running", slug="honeygain") + good.stats.return_value = _zero_stats() + bad = MagicMock() + bad.id = "bad-id" + bad.short_id = "bad12" + bad.labels.get.side_effect = RuntimeError("corrupted container labels") + client = MagicMock() + client.containers.list.return_value = [bad, good] + with ( + patch.object(orchestrator, "_get_client", return_value=client), + patch.object(orchestrator, "_build_image_slug_map", return_value={}), + ): + results = orchestrator.get_status() + assert len(results) == 1 + assert results[0]["slug"] == "honeygain" + + def test_image_matched_external_container_included(self): + external = MagicMock() + external.id = "ext-1" + external.name = "manually-run-storj" + external.status = "running" + external.image.tags = ["storjlabs/storagenode:latest"] + external.short_id = "ext1" + external.attrs = {"Created": "2026-01-01T00:00:00Z"} + external.stats.return_value = _zero_stats() + client = MagicMock() + client.containers.list.side_effect = [[], [external]] + image_map = {"storjlabs/storagenode:latest": "storj"} + with ( + patch.object(orchestrator, "_get_client", return_value=client), + patch.object(orchestrator, "_build_image_slug_map", return_value=image_map), + ): + results = orchestrator.get_status() + assert len(results) == 1 + assert results[0]["slug"] == "storj" + assert results[0]["deployed_by"] == "external" + + def test_all_containers_listing_failure_still_returns_labeled(self): + labeled = _mock_container(name="cashpilot-honeygain", status="running", slug="honeygain") + labeled.stats.return_value = _zero_stats() + client = MagicMock() + client.containers.list.side_effect = [[labeled], Exception("docker daemon hiccup")] + image_map = {"some/image:latest": "svc"} + with ( + patch.object(orchestrator, "_get_client", return_value=client), + patch.object(orchestrator, "_build_image_slug_map", return_value=image_map), + ): + results = orchestrator.get_status() + assert len(results) == 1 + assert results[0]["slug"] == "honeygain" + + +# --------------------------------------------------------------------------- +# get_status_light (fast path, no CPU/mem stats) +# --------------------------------------------------------------------------- + + +class TestGetStatusLight: + def test_docker_unavailable_returns_empty(self): + with patch.object(orchestrator, "_get_client", side_effect=RuntimeError("no socket")): + assert orchestrator.get_status_light() == [] + + def test_no_stats_call_even_when_running(self): + container = _mock_container(name="cashpilot-honeygain", status="running", slug="honeygain") + client = MagicMock() + client.containers.list.return_value = [container] + with ( + patch.object(orchestrator, "_get_client", return_value=client), + patch.object(orchestrator, "_build_image_slug_map", return_value={}), + ): + results = orchestrator.get_status_light() + assert results[0]["cpu_percent"] == 0.0 + assert results[0]["memory_mb"] == 0.0 + container.stats.assert_not_called() + + def test_image_matched_container_skipped_when_slug_already_seen(self): + labeled = _mock_container(name="cashpilot-honeygain", status="running", slug="honeygain") + dup = MagicMock() + dup.id = "dup-id" + dup.image.tags = ["honeygain/desktop:latest"] + client = MagicMock() + client.containers.list.side_effect = [[labeled], [dup]] + image_map = {"honeygain/desktop:latest": "honeygain"} + with ( + patch.object(orchestrator, "_get_client", return_value=client), + patch.object(orchestrator, "_build_image_slug_map", return_value=image_map), + ): + results = orchestrator.get_status_light() + # The duplicate slug from the image-matched scan must not be added again. + assert len(results) == 1 + + def test_all_containers_listing_failure_handled(self): + labeled = _mock_container(name="cashpilot-honeygain", status="running", slug="honeygain") + client = MagicMock() + client.containers.list.side_effect = [[labeled], Exception("boom")] + image_map = {"x": "y"} + with ( + patch.object(orchestrator, "_get_client", return_value=client), + patch.object(orchestrator, "_build_image_slug_map", return_value=image_map), + ): + results = orchestrator.get_status_light() + assert len(results) == 1 diff --git a/tests/test_worker_api_coverage.py b/tests/test_worker_api_coverage.py new file mode 100644 index 0000000..7246e24 --- /dev/null +++ b/tests/test_worker_api_coverage.py @@ -0,0 +1,333 @@ +"""Tests targeting uncovered lines in app/worker_api.py. + +Covers _verify_api_key negative paths (missing header, wrong key, no key +configured), _validate_deploy_spec rejections not already exercised by +test_worker_resources.py (privileged, blocked capabilities, disallowed +network_mode, blocked volume roots, named-volume allow-list), and the +container command endpoints (status/list/deploy/restart/stop/start/remove/ +logs) success + docker-error paths. + +Mocks app.orchestrator entirely — no real Docker socket is used. +""" + +from __future__ import annotations + +import os +from contextlib import asynccontextmanager +from unittest.mock import MagicMock, patch + +os.environ.setdefault("CASHPILOT_API_KEY", "test-fleet-key") + +import pytest # noqa: E402 + +try: + from fastapi import HTTPException # noqa: E402 + from fastapi.testclient import TestClient # noqa: E402 + + from app import worker_api # noqa: E402 + from app.worker_api import DeploySpec, _validate_deploy_spec, _verify_api_key # noqa: E402 +except ImportError: + pytest.skip( + "Requires full app dependencies (fastapi, docker, etc.) — runs in CI", + allow_module_level=True, + ) + + +# --------------------------------------------------------------------------- +# Helpers — match the TestClient harness used in test_worker_resources.py +# --------------------------------------------------------------------------- + + +@asynccontextmanager +async def _noop_lifespan(a): + yield + + +def _client(): + # Disable the heartbeat lifespan so the TestClient stays isolated. + worker_api.app.router.lifespan_context = _noop_lifespan + return TestClient(worker_api.app, raise_server_exceptions=False) + + +def _auth(): + return {"Authorization": f"Bearer {worker_api.API_KEY}"} + + +# --------------------------------------------------------------------------- +# _verify_api_key — negative paths +# --------------------------------------------------------------------------- + + +class TestVerifyApiKeyUnit: + def test_missing_header_rejected(self): + req = MagicMock() + req.headers = {} + with pytest.raises(HTTPException) as ei: + _verify_api_key(req) + assert ei.value.status_code == 401 + + def test_wrong_key_rejected(self): + req = MagicMock() + req.headers = {"Authorization": "Bearer wrong-key"} + with pytest.raises(HTTPException) as ei: + _verify_api_key(req) + assert ei.value.status_code == 401 + + def test_no_key_configured_returns_503(self): + req = MagicMock() + req.headers = {"Authorization": "Bearer anything"} + with patch.object(worker_api, "API_KEY", ""), pytest.raises(HTTPException) as ei: + _verify_api_key(req) + assert ei.value.status_code == 503 + + def test_correct_key_passes(self): + req = MagicMock() + req.headers = {"Authorization": f"Bearer {worker_api.API_KEY}"} + _verify_api_key(req) # must not raise + + +class TestEndpointAuthRejection: + def test_endpoint_rejects_missing_header(self): + resp = _client().get("/api/status") + assert resp.status_code == 401 + + def test_endpoint_rejects_wrong_key(self): + resp = _client().get("/api/status", headers={"Authorization": "Bearer nope"}) + assert resp.status_code == 401 + + def test_health_endpoint_requires_no_auth(self): + resp = _client().get("/api/health") + assert resp.status_code == 200 + assert resp.json() == {"status": "ok", "worker": worker_api.WORKER_NAME} + + +# --------------------------------------------------------------------------- +# _validate_deploy_spec — rejections not covered by test_worker_resources.py +# --------------------------------------------------------------------------- + + +class TestValidateDeploySpecRejections: + def test_privileged_rejected(self): + spec = DeploySpec(image="x", privileged=True) + with pytest.raises(HTTPException) as ei: + _validate_deploy_spec(spec) + assert ei.value.status_code == 403 + + def test_blocked_capability_rejected(self): + spec = DeploySpec(image="x", cap_add=["SYS_ADMIN"]) + with pytest.raises(HTTPException) as ei: + _validate_deploy_spec(spec) + assert ei.value.status_code == 403 + + def test_blocked_capability_rejected_case_insensitive(self): + spec = DeploySpec(image="x", cap_add=["sys_ptrace"]) + with pytest.raises(HTTPException) as ei: + _validate_deploy_spec(spec) + assert ei.value.status_code == 403 + + def test_allowed_capability_passes(self): + _validate_deploy_spec(DeploySpec(image="x", cap_add=["NET_ADMIN"])) # must not raise + + def test_disallowed_network_mode_rejected(self): + spec = DeploySpec(image="x", network_mode="container:abc123") + with pytest.raises(HTTPException) as ei: + _validate_deploy_spec(spec) + assert ei.value.status_code == 403 + + def test_host_network_mode_allowed(self): + # mysterium legitimately needs host networking. + _validate_deploy_spec(DeploySpec(image="x", network_mode="host")) # must not raise + + def test_bridge_network_mode_allowed(self): + _validate_deploy_spec(DeploySpec(image="x", network_mode="bridge")) # must not raise + + @pytest.mark.parametrize("root", ["/etc", "/root", "/var/run", "/proc"]) + def test_blocked_volume_root_rejected(self, root): + # Patch realpath to identity: on macOS dev machines /etc and /var/run + # are themselves symlinks (-> /private/etc, /private/var/run), which + # would dodge the block by resolving to a path outside the blocklist. + # In the worker's actual Linux container this resolution is a no-op. + spec = DeploySpec(image="x", volumes={root: {"bind": "/data", "mode": "rw"}}) + with ( + patch("app.worker_api.os.path.realpath", side_effect=lambda p: p), + pytest.raises(HTTPException) as ei, + ): + _validate_deploy_spec(spec) + assert ei.value.status_code == 403 + + def test_blocked_volume_subpath_rejected(self): + spec = DeploySpec(image="x", volumes={"/etc/passwd": {"bind": "/x", "mode": "ro"}}) + with ( + patch("app.worker_api.os.path.realpath", side_effect=lambda p: p), + pytest.raises(HTTPException) as ei, + ): + _validate_deploy_spec(spec) + assert ei.value.status_code == 403 + + def test_named_volume_always_allowed(self): + # Not an absolute path -> named volume (e.g. mysterium-data), always allowed. + spec = DeploySpec(image="x", volumes={"mysterium-data": {"bind": "/data", "mode": "rw"}}) + _validate_deploy_spec(spec) # must not raise + + def test_allowed_bind_mount_passes(self): + spec = DeploySpec(image="x", volumes={"/data/honeygain": {"bind": "/data", "mode": "rw"}}) + _validate_deploy_spec(spec) # must not raise + + +# --------------------------------------------------------------------------- +# Container command endpoints — success + docker-error paths +# --------------------------------------------------------------------------- + + +class TestStatusAndListEndpoints: + def test_status_reports_docker_and_counts(self): + containers = [{"status": "running"}, {"status": "exited"}] + with ( + patch("app.worker_api.orchestrator.get_status_cached", return_value=containers), + patch("app.worker_api.orchestrator.docker_available", return_value=True), + ): + resp = _client().get("/api/status", headers=_auth()) + assert resp.status_code == 200 + body = resp.json() + assert body["docker_available"] is True + assert body["container_count"] == 2 + assert body["running_count"] == 1 + + def test_status_handles_status_fetch_exception(self): + with ( + patch("app.worker_api.orchestrator.get_status_cached", side_effect=Exception("boom")), + patch("app.worker_api.orchestrator.docker_available", return_value=False), + ): + resp = _client().get("/api/status", headers=_auth()) + assert resp.status_code == 200 + assert resp.json()["container_count"] == 0 + + def test_list_containers_success(self): + containers = [{"slug": "honeygain"}] + with patch("app.worker_api.orchestrator.get_status_cached", return_value=containers): + resp = _client().get("/api/containers", headers=_auth()) + assert resp.status_code == 200 + assert resp.json() == containers + + def test_list_containers_docker_unavailable_returns_503(self): + with patch("app.worker_api.orchestrator.get_status_cached", side_effect=RuntimeError("no docker")): + resp = _client().get("/api/containers", headers=_auth()) + assert resp.status_code == 503 + + +class TestDeployEndpointErrors: + def test_generic_exception_returns_500(self): + with patch("app.worker_api.orchestrator.deploy_raw", side_effect=Exception("boom")): + resp = _client().post( + "/api/containers/honeygain/deploy", + json={"image": "img"}, + headers=_auth(), + ) + assert resp.status_code == 500 + + +class TestStopRestartStartEndpoints: + def test_stop_success(self): + with patch("app.worker_api.orchestrator.stop_service") as mock_stop: + resp = _client().post("/api/containers/honeygain/stop", headers=_auth()) + assert resp.status_code == 200 + assert resp.json() == {"status": "stopped"} + mock_stop.assert_called_once_with("honeygain") + + def test_stop_not_found_returns_404(self): + with patch("app.worker_api.orchestrator.stop_service", side_effect=ValueError("not found")): + resp = _client().post("/api/containers/missing/stop", headers=_auth()) + assert resp.status_code == 404 + + def test_stop_docker_unavailable_returns_503(self): + with patch("app.worker_api.orchestrator.stop_service", side_effect=RuntimeError("no docker")): + resp = _client().post("/api/containers/honeygain/stop", headers=_auth()) + assert resp.status_code == 503 + + def test_restart_success(self): + with patch("app.worker_api.orchestrator.restart_service") as mock_restart: + resp = _client().post("/api/containers/honeygain/restart", headers=_auth()) + assert resp.status_code == 200 + assert resp.json() == {"status": "restarted"} + mock_restart.assert_called_once_with("honeygain") + + def test_restart_not_found_returns_404(self): + with patch("app.worker_api.orchestrator.restart_service", side_effect=ValueError("gone")): + resp = _client().post("/api/containers/x/restart", headers=_auth()) + assert resp.status_code == 404 + + def test_restart_docker_unavailable_returns_503(self): + with patch("app.worker_api.orchestrator.restart_service", side_effect=RuntimeError("no docker")): + resp = _client().post("/api/containers/honeygain/restart", headers=_auth()) + assert resp.status_code == 503 + + def test_start_success(self): + with patch("app.worker_api.orchestrator.start_service") as mock_start: + resp = _client().post("/api/containers/honeygain/start", headers=_auth()) + assert resp.status_code == 200 + assert resp.json() == {"status": "started"} + mock_start.assert_called_once_with("honeygain") + + def test_start_not_found_returns_404(self): + with patch("app.worker_api.orchestrator.start_service", side_effect=ValueError("gone")): + resp = _client().post("/api/containers/x/start", headers=_auth()) + assert resp.status_code == 404 + + def test_start_docker_unavailable_returns_503(self): + with patch("app.worker_api.orchestrator.start_service", side_effect=RuntimeError("no docker")): + resp = _client().post("/api/containers/honeygain/start", headers=_auth()) + assert resp.status_code == 503 + + +class TestRemoveEndpoint: + def test_remove_success(self): + removal = {"container": "cashpilot-honeygain", "deleted_volumes": [], "failed_volumes": []} + with patch("app.worker_api.orchestrator.remove_service", return_value=removal) as mock_remove: + resp = _client().delete("/api/containers/honeygain", headers=_auth()) + assert resp.status_code == 200 + body = resp.json() + assert body["status"] == "removed" + assert body["container"] == "cashpilot-honeygain" + mock_remove.assert_called_once_with("honeygain", delete_volumes=False) + + def test_remove_with_delete_volumes_query_param(self): + removal = {"container": "cashpilot-honeygain", "deleted_volumes": ["v1"], "failed_volumes": []} + with patch("app.worker_api.orchestrator.remove_service", return_value=removal) as mock_remove: + resp = _client().delete("/api/containers/honeygain?delete_volumes=true", headers=_auth()) + assert resp.status_code == 200 + mock_remove.assert_called_once_with("honeygain", delete_volumes=True) + + def test_remove_not_found_returns_404(self): + with patch("app.worker_api.orchestrator.remove_service", side_effect=ValueError("gone")): + resp = _client().delete("/api/containers/x", headers=_auth()) + assert resp.status_code == 404 + + def test_remove_docker_unavailable_returns_503(self): + with patch("app.worker_api.orchestrator.remove_service", side_effect=RuntimeError("no docker")): + resp = _client().delete("/api/containers/honeygain", headers=_auth()) + assert resp.status_code == 503 + + +class TestLogsEndpoint: + def test_logs_success(self): + with patch("app.worker_api.orchestrator.get_service_logs", return_value="line1\nline2") as mock_logs: + resp = _client().get("/api/containers/honeygain/logs", headers=_auth()) + assert resp.status_code == 200 + assert resp.json() == {"logs": "line1\nline2"} + mock_logs.assert_called_once_with("honeygain", lines=50) + + def test_logs_lines_capped_at_1000(self): + with patch("app.worker_api.orchestrator.get_service_logs", return_value="") as mock_logs: + resp = _client().get("/api/containers/honeygain/logs?lines=5000", headers=_auth()) + assert resp.status_code == 200 + mock_logs.assert_called_once_with("honeygain", lines=1000) + + def test_logs_not_found_returns_404(self): + with patch("app.worker_api.orchestrator.get_service_logs", side_effect=ValueError("gone")): + resp = _client().get("/api/containers/x/logs", headers=_auth()) + assert resp.status_code == 404 + + def test_logs_docker_unavailable_returns_503(self): + with patch("app.worker_api.orchestrator.get_service_logs", side_effect=RuntimeError("no docker")): + resp = _client().get("/api/containers/honeygain/logs", headers=_auth()) + assert resp.status_code == 503 From 18122601890cde5e564ec11512aa046b8bc1c874 Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Fri, 10 Jul 2026 17:35:09 +0200 Subject: [PATCH 3/5] feat(security): setup-token gate for first-run + trusted-proxy client IP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "private network only" guard on first-run /register trusts request.client.host, which behind a reverse proxy is the proxy (a loopback/ private address) — so the check always passes and the first public visitor could seize the owner account. Add a proxy-independent gate: while no users exist, startup generates a one-time setup token, persists it (encrypted config), and logs it; /register then requires it (query param, X-Setup-Token header, or form field) and it is retired the moment the owner account is created. Also resolve the real client IP behind a trusted proxy (opt-in CASHPILOT_TRUSTED_PROXY, right-most X-Forwarded-For), used by both the first-run network check and the login rate limiter — which previously collapsed every proxied client into one bucket keyed by the proxy IP. Tests: token module + client_ip (trusted/untrusted) + the first-run guard (missing/wrong/query/header/form token, public-IP-still-blocked, network-only fallback once retired) + register integration (403 without token, 303 + token cleared with it); conftest resets the token global between tests. --- app/deps.py | 53 +++++++++++++++++-- app/main.py | 49 +++++++++++++++--- app/routers/auth.py | 18 +++++-- app/setup_token.py | 62 ++++++++++++++++++++++ app/templates/auth.html | 3 ++ tests/conftest.py | 15 ++++++ tests/test_main_routes.py | 52 +++++++++++++++++++ tests/test_setup_token.py | 105 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 342 insertions(+), 15 deletions(-) create mode 100644 app/setup_token.py create mode 100644 tests/test_setup_token.py diff --git a/app/deps.py b/app/deps.py index d6bdc64..7fb4558 100644 --- a/app/deps.py +++ b/app/deps.py @@ -11,13 +11,14 @@ from __future__ import annotations import ipaddress +import os from typing import Any from fastapi import HTTPException, Request from fastapi.responses import RedirectResponse from fastapi.templating import Jinja2Templates -from app import auth +from app import auth, setup_token __all__ = [ "templates", @@ -26,8 +27,15 @@ "_require_writer", "_require_owner", "_require_private_network", + "_require_first_run_access", + "client_ip", ] +# Opt-in: set CASHPILOT_TRUSTED_PROXY=1 only when the app sits behind exactly one +# reverse proxy you control. X-Forwarded-For is attacker-controlled, so we ignore +# it unless the operator asserts a trusted proxy is stripping/appending it. +_TRUST_PROXY = os.getenv("CASHPILOT_TRUSTED_PROXY", "").strip().lower() in ("1", "true", "yes", "on") + templates = Jinja2Templates(directory="app/templates") @@ -57,13 +65,48 @@ def _require_owner(request: Request) -> dict[str, Any]: return user +def client_ip(request: Request) -> str | None: + """Best-effort real client IP. + + Behind a trusted reverse proxy (opt-in via ``CASHPILOT_TRUSTED_PROXY``) the + real peer is the right-most ``X-Forwarded-For`` entry — the value appended by + the trusted proxy, which a client cannot forge by prepending its own. Without + that opt-in we never trust the header and use the direct peer. + """ + if _TRUST_PROXY: + xff = request.headers.get("x-forwarded-for", "") + parts = [p.strip() for p in xff.split(",") if p.strip()] + if parts: + return parts[-1] + return request.client.host if request.client else None + + def _require_private_network(request: Request) -> None: - """Block requests from public IPs (for first-run setup).""" - if not request.client or not request.client.host: + """Block requests whose real client IP is public (first-run defense in depth).""" + ip_str = client_ip(request) + if not ip_str: return try: - client_ip = ipaddress.ip_address(request.client.host) + ip = ipaddress.ip_address(ip_str) except ValueError: return - if not (client_ip.is_loopback or client_ip.is_private): + if not (ip.is_loopback or ip.is_private): raise HTTPException(status_code=403, detail="First-run setup only allowed from private networks") + + +def _require_first_run_access(request: Request, setup_token_value: str | None = None) -> None: + """Gate first-run owner creation: private network AND the one-time setup token. + + The network check alone is spoofable behind a reverse proxy (the peer is then + the proxy), so the setup token — printed to the server logs, readable only + with host access — is the real gate. The token is accepted from the explicit + argument (a form field), the ``setup_token`` query param, or the + ``X-Setup-Token`` header. + """ + _require_private_network(request) + token = setup_token_value or request.query_params.get("setup_token") or request.headers.get("x-setup-token") + if not setup_token.verify(token): + raise HTTPException( + status_code=403, + detail="First-run setup requires the setup token printed in the server logs", + ) diff --git a/app/main.py b/app/main.py index e948b3a..cdeb292 100644 --- a/app/main.py +++ b/app/main.py @@ -31,7 +31,7 @@ from pydantic import BaseModel from starlette.middleware.base import BaseHTTPMiddleware -from app import auth, catalog, compose_generator, database, exchange_rates, fleet_key, metrics +from app import auth, catalog, compose_generator, database, exchange_rates, fleet_key, metrics, setup_token logging.basicConfig( level=logging.INFO, @@ -75,16 +75,16 @@ def _on_done(t: asyncio.Task) -> None: _LOGIN_WINDOW_SECONDS = 300 -def _check_login_rate(client_ip: str) -> None: +def _check_login_rate(ip: str) -> None: now = monotonic() - attempts = _login_attempts[client_ip] - _login_attempts[client_ip] = [t for t in attempts if now - t < _LOGIN_WINDOW_SECONDS] - if len(_login_attempts[client_ip]) >= _LOGIN_MAX_ATTEMPTS: + attempts = _login_attempts[ip] + _login_attempts[ip] = [t for t in attempts if now - t < _LOGIN_WINDOW_SECONDS] + if len(_login_attempts[ip]) >= _LOGIN_MAX_ATTEMPTS: raise HTTPException(status_code=429, detail="Too many login attempts. Try again in a few minutes.") -def _record_failed_login(client_ip: str) -> None: - _login_attempts[client_ip].append(monotonic()) +def _record_failed_login(ip: str) -> None: + _login_attempts[ip].append(monotonic()) def _safe_json(raw: str, fallback: Any = None) -> Any: @@ -273,6 +273,15 @@ async def _run_data_retention() -> None: logger.warning("Data retention error: %s", exc) +async def _run_vacuum() -> None: + """Reclaim disk left by retention deletes (SQLite does not auto-shrink).""" + try: + await database.vacuum_database() + logger.info("Database VACUUM complete") + except Exception as exc: + logger.warning("Database VACUUM error: %s", exc) + + async def _check_stale_workers() -> None: """Mark workers as offline if stale, and purge workers offline > 1 hour.""" try: @@ -312,6 +321,21 @@ async def lifespan(app: FastAPI): _changed = _u.get("password_changed_at") or 0.0 if _changed: auth.set_user_pwd_epoch(_u["id"], _changed) + # First-run setup token: while no users exist, require a one-time token + # (printed below) for /register so a proxy-exposed instance cannot be seized + # by the first public visitor. Persisted in config so it survives restarts; + # cleared once the owner account is created. + if not await database.has_any_users(): + _tok = await database.get_config("_setup_token") + if not _tok: + _tok = setup_token.generate() + await database.set_config("_setup_token", _tok) + setup_token.set_active(_tok) + logger.warning( + "FIRST-RUN SETUP: no account exists yet. Create the owner account at " + "/register?setup_token=%s — this token is required and is shown only here.", + _tok, + ) catalog.load_services() catalog.register_sighup() @@ -355,6 +379,15 @@ def _on_job_event(event): coalesce=True, misfire_grace_time=300, ) + scheduler.add_job( + _run_vacuum, + "interval", + weeks=1, + id="db_vacuum", + max_instances=1, + coalesce=True, + misfire_grace_time=300, + ) scheduler.add_job( exchange_rates.refresh, "interval", @@ -424,9 +457,11 @@ async def dispatch(self, request, call_next): from app.deps import ( # noqa: E402 _login_redirect, # noqa: F401 (re-exported for app.main.* test/router surface) _require_auth_api, + _require_first_run_access, # noqa: F401 (re-exported for app.main.* router surface) _require_owner, _require_private_network, # noqa: F401 (re-exported for app.main.* router surface) _require_writer, + client_ip, # noqa: F401 (re-exported for app.main.* router surface) templates, # noqa: F401 (re-exported for app.main.* router/test surface) ) diff --git a/app/routers/auth.py b/app/routers/auth.py index aa8ff62..fe53613 100644 --- a/app/routers/auth.py +++ b/app/routers/auth.py @@ -45,7 +45,7 @@ async def do_login( username: str = Form(...), password: str = Form(...), ): - client_ip = request.client.host if request.client else "unknown" + client_ip = main.client_ip(request) or "unknown" try: main._check_login_rate(client_ip) except HTTPException: @@ -86,7 +86,7 @@ async def page_register(request: Request, error: str = ""): if not user or user.get("r") != "owner": return RedirectResponse("/login", status_code=303) if is_first: - main._require_private_network(request) + main._require_first_run_access(request) return main.templates.TemplateResponse( request, @@ -99,6 +99,8 @@ async def page_register(request: Request, error: str = ""): "button_text": "Create Account", "error": error, "is_first": is_first, + # Carry the setup token through the form so the POST passes the same gate. + "setup_token": request.query_params.get("setup_token", "") if is_first else "", }, ) @@ -109,6 +111,7 @@ async def do_register( username: str = Form(...), password: str = Form(...), password_confirm: str = Form(...), + setup_token: str = Form(""), ): is_first = not await main.database.has_any_users() @@ -119,7 +122,7 @@ async def do_register( raise HTTPException(status_code=403, detail="Only owners can add users") if is_first: - main._require_private_network(request) + main._require_first_run_access(request, setup_token) if not re.match(r"^[a-zA-Z0-9_-]{3,32}$", username): return main.templates.TemplateResponse( @@ -133,6 +136,7 @@ async def do_register( "button_text": "Create Account", "error": "Username must be 3-32 alphanumeric characters (a-z, 0-9, _ -)", "is_first": is_first, + "setup_token": setup_token if is_first else "", }, status_code=400, ) @@ -149,6 +153,7 @@ async def do_register( "button_text": "Create Account", "error": "Passwords do not match", "is_first": is_first, + "setup_token": setup_token if is_first else "", }, status_code=400, ) @@ -165,6 +170,7 @@ async def do_register( "button_text": "Create Account", "error": "Password must be at least 10 characters", "is_first": is_first, + "setup_token": setup_token if is_first else "", }, status_code=400, ) @@ -182,6 +188,7 @@ async def do_register( "button_text": "Create Account", "error": "Username already taken", "is_first": is_first, + "setup_token": setup_token if is_first else "", }, status_code=400, ) @@ -191,6 +198,11 @@ async def do_register( hashed = main.auth.hash_password(password) user_id = await main.database.create_user(username, hashed, role) + if is_first: + # Owner now exists — retire the one-time setup token permanently. + await main.database.delete_config_keys(["_setup_token"]) + main.setup_token.clear() + token = main.auth.create_session_token(user_id, username, role) dest = "/setup" if is_first else "/" response = RedirectResponse(dest, status_code=303) diff --git a/app/setup_token.py b/app/setup_token.py new file mode 100644 index 0000000..cf95235 --- /dev/null +++ b/app/setup_token.py @@ -0,0 +1,62 @@ +"""First-run setup-token gate. + +The very first account created on a fresh install becomes the ``owner``. The +private-network check in :mod:`app.deps` is not enough on its own: behind a +reverse proxy ``request.client`` is the *proxy* (a loopback/private address), so +the network check always passes and the first person to reach ``/register`` from +the public internet could seize the owner account. + +This module adds a proxy-independent second factor: a one-time token generated at +startup while no users exist, printed to the container logs. Only someone who can +read the server logs (i.e. already has host access) can complete first-run setup. +Once the owner account is created the token is cleared and never required again +(further users are added by an authenticated owner). + +The active token is held in a module global so the synchronous request guard can +check it without a DB round-trip; it is also persisted in the ``config`` table so +it survives restarts until consumed. +""" + +from __future__ import annotations + +import hmac +import secrets + +# The token currently required for first-run registration, or ``None`` when no +# first-run gate is active (owner already exists, or not yet initialised). +_active: str | None = None + + +def generate() -> str: + """Return a fresh, URL-safe setup token.""" + return secrets.token_urlsafe(24) + + +def set_active(token: str | None) -> None: + """Install (or clear, with ``None``) the token required for first-run setup.""" + global _active + _active = token or None + + +def clear() -> None: + """Drop the first-run gate — called once the owner account exists.""" + set_active(None) + + +def active() -> str | None: + """Return the currently required setup token, or ``None`` if none is active.""" + return _active + + +def verify(provided: str | None) -> bool: + """Check a caller-supplied token against the active one. + + Returns ``True`` when no token is active (nothing to enforce) or when the + supplied value matches in constant time; ``False`` otherwise. + """ + current = _active + if current is None: + return True + if not provided: + return False + return hmac.compare_digest(current, provided) diff --git a/app/templates/auth.html b/app/templates/auth.html index c3b5da8..e457129 100644 --- a/app/templates/auth.html +++ b/app/templates/auth.html @@ -151,6 +151,9 @@ {% endif %}
+ {% if mode == "register" and is_first %} + + {% endif %}
diff --git a/tests/conftest.py b/tests/conftest.py index 1c86c0f..1ee50b4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -62,3 +62,18 @@ def _reset_login_attempts(): main._login_attempts.clear() yield + + +@pytest.fixture(autouse=True) +def _reset_setup_token(): + """Clear the first-run setup-token module global before every test. + + ``app.setup_token._active`` persists for the whole process; a test that runs + lifespan on a fresh DB (or exercises the token directly) would otherwise leak + an active token into later tests, making unrelated /register tests 403. + """ + with contextlib.suppress(Exception): + from app import setup_token + + setup_token.clear() + yield diff --git a/tests/test_main_routes.py b/tests/test_main_routes.py index c66de19..4e082bb 100644 --- a/tests/test_main_routes.py +++ b/tests/test_main_routes.py @@ -264,6 +264,53 @@ def test_register_first_user_success(self, client): patch("app.main.database.get_user_by_username", new_callable=AsyncMock, return_value=None), patch("app.main.auth.hash_password", return_value="hashed"), patch("app.main.database.create_user", new_callable=AsyncMock, return_value=1), + patch("app.main.database.delete_config_keys", new_callable=AsyncMock), + patch("app.main.auth.create_session_token", return_value="tok"), + patch("app.main.auth.set_session_cookie", side_effect=lambda r, t: r), + ): + resp = client.post( + "/register", + data={ + "username": "admin", + "password": "password123", + "password_confirm": "password123", + }, + follow_redirects=False, + ) + assert resp.status_code == 303 + + def test_register_first_user_requires_setup_token(self, client): + # When a first-run setup token is active, registering without it is refused + # (the proxy-independent gate against a public visitor seizing the owner). + from app import setup_token + + setup_token.set_active("the-token") + with ( + _no_auth(), + patch("app.main.database.has_any_users", new_callable=AsyncMock, return_value=False), + ): + resp = client.post( + "/register", + data={ + "username": "admin", + "password": "password123", + "password_confirm": "password123", + }, + follow_redirects=False, + ) + assert resp.status_code == 403 + + def test_register_first_user_with_setup_token_clears_it(self, client): + from app import setup_token + + setup_token.set_active("the-token") + with ( + _no_auth(), + patch("app.main.database.has_any_users", new_callable=AsyncMock, return_value=False), + patch("app.main.database.get_user_by_username", new_callable=AsyncMock, return_value=None), + patch("app.main.auth.hash_password", return_value="hashed"), + patch("app.main.database.create_user", new_callable=AsyncMock, return_value=1), + patch("app.main.database.delete_config_keys", new_callable=AsyncMock) as del_keys, patch("app.main.auth.create_session_token", return_value="tok"), patch("app.main.auth.set_session_cookie", side_effect=lambda r, t: r), ): @@ -273,10 +320,13 @@ def test_register_first_user_success(self, client): "username": "admin", "password": "password123", "password_confirm": "password123", + "setup_token": "the-token", }, follow_redirects=False, ) assert resp.status_code == 303 + del_keys.assert_awaited_once_with(["_setup_token"]) + assert setup_token.active() is None def test_register_password_mismatch(self, client): with ( @@ -1791,6 +1841,7 @@ async def _run(): patch("app.main.database.connect_shared", new_callable=AsyncMock), patch("app.main.database.close_shared", new_callable=AsyncMock), patch("app.main.database.list_users_with_pwd_epoch", new_callable=AsyncMock, return_value=[]), + patch("app.main.database.has_any_users", new_callable=AsyncMock, return_value=True), patch("app.main.catalog.load_services"), patch("app.main.catalog.register_sighup"), patch("app.main.exchange_rates.refresh", new_callable=AsyncMock), @@ -1831,6 +1882,7 @@ async def _run(): patch("app.main.database.connect_shared", new_callable=AsyncMock), patch("app.main.database.close_shared", new_callable=AsyncMock), patch("app.main.database.list_users_with_pwd_epoch", new_callable=AsyncMock, return_value=[]), + patch("app.main.database.has_any_users", new_callable=AsyncMock, return_value=True), patch("app.main.catalog.load_services"), patch("app.main.catalog.register_sighup"), patch("app.main.exchange_rates.refresh", new_callable=AsyncMock), diff --git a/tests/test_setup_token.py b/tests/test_setup_token.py new file mode 100644 index 0000000..59621e5 --- /dev/null +++ b/tests/test_setup_token.py @@ -0,0 +1,105 @@ +"""Tests for the first-run setup-token gate (app/setup_token.py + app/deps).""" + +from unittest.mock import MagicMock, patch + +import pytest +from fastapi import HTTPException + +from app import deps, setup_token + + +def _req(host="127.0.0.1", xff=None, query=None, headers=None): + """A minimal fake Request: real dicts for headers/query_params so .get works.""" + req = MagicMock() + if host is None: + req.client = None + else: + req.client.host = host + h = {} + if xff is not None: + h["x-forwarded-for"] = xff + if headers: + h.update(headers) + req.headers = h + req.query_params = query or {} + return req + + +class TestSetupTokenModule: + def test_verify_no_active_allows_anything(self): + setup_token.clear() + assert setup_token.verify(None) is True + assert setup_token.verify("whatever") is True + + def test_verify_active_requires_exact_match(self): + setup_token.set_active("s3cret") + assert setup_token.verify("s3cret") is True + assert setup_token.verify("wrong") is False + assert setup_token.verify(None) is False + assert setup_token.verify("") is False + + def test_generate_is_unique_and_long(self): + a, b = setup_token.generate(), setup_token.generate() + assert a != b + assert len(a) >= 20 + + def test_set_active_empty_string_deactivates(self): + setup_token.set_active("") + assert setup_token.active() is None + + +class TestClientIp: + def test_ignores_xff_without_trusted_proxy(self): + with patch.object(deps, "_TRUST_PROXY", False): + assert deps.client_ip(_req("10.0.0.1", xff="1.2.3.4")) == "10.0.0.1" + + def test_uses_rightmost_xff_with_trusted_proxy(self): + # The trusted proxy appends the real peer on the right; a client can only + # forge entries on the left, so the right-most value is authoritative. + with patch.object(deps, "_TRUST_PROXY", True): + req = _req("10.0.0.1", xff="9.9.9.9, 8.8.8.8, 203.0.113.5") + assert deps.client_ip(req) == "203.0.113.5" + + def test_trusted_proxy_falls_back_to_peer_without_xff(self): + with patch.object(deps, "_TRUST_PROXY", True): + assert deps.client_ip(_req("10.0.0.1")) == "10.0.0.1" + + def test_no_client_returns_none(self): + with patch.object(deps, "_TRUST_PROXY", False): + assert deps.client_ip(_req(host=None)) is None + + +class TestRequireFirstRunAccess: + def test_blocks_when_token_active_and_missing(self): + setup_token.set_active("tok") + with patch.object(deps, "_TRUST_PROXY", False), pytest.raises(HTTPException) as ei: + deps._require_first_run_access(_req("127.0.0.1")) + assert ei.value.status_code == 403 + + def test_allows_with_correct_token_arg(self): + setup_token.set_active("tok") + with patch.object(deps, "_TRUST_PROXY", False): + assert deps._require_first_run_access(_req("127.0.0.1"), "tok") is None + + def test_token_from_query_param(self): + setup_token.set_active("tok") + with patch.object(deps, "_TRUST_PROXY", False): + assert deps._require_first_run_access(_req("127.0.0.1", query={"setup_token": "tok"})) is None + + def test_token_from_header(self): + setup_token.set_active("tok") + with patch.object(deps, "_TRUST_PROXY", False): + assert deps._require_first_run_access(_req("127.0.0.1", headers={"x-setup-token": "tok"})) is None + + def test_public_ip_blocked_even_with_correct_token(self): + # The network check runs first: a public client is refused regardless of token. + setup_token.set_active("tok") + with patch.object(deps, "_TRUST_PROXY", False), pytest.raises(HTTPException) as ei: + deps._require_first_run_access(_req("8.8.8.8"), "tok") + assert ei.value.status_code == 403 + + def test_no_active_token_falls_back_to_network_only(self): + # Once the token is retired, first-run access is network-gated only. + setup_token.clear() + with patch.object(deps, "_TRUST_PROXY", False): + assert deps._require_first_run_access(_req("192.168.1.5")) is None From efe0ada1d1abd83e9481b6099df6581f15c26dda Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:56:49 +0200 Subject: [PATCH 4/5] fix(db): restore health-retention split + VACUUM lost in rebase; cover _run_vacuum + first-run token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A messy multi-rebase during PR stacking silently dropped the database.py half of the health_events change (HEALTH_CHECK_RETENTION_DAYS, the check-sample purge, and vacuum_database) while keeping main.py's _run_vacuum + scheduler job — leaving main calling a database function that no longer existed. Restore the database.py logic and its tests, and add coverage for main._run_vacuum (success + error) and the lifespan first-run setup-token generation (the codecov/patch gaps). Full pytest 1081 passed; ruff check + format clean. --- app/database.py | 40 +++++++++++++++++++++++++++++++-- tests/test_database.py | 44 ++++++++++++++++++++++++++++++++++++ tests/test_main_routes.py | 47 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 2 deletions(-) diff --git a/app/database.py b/app/database.py index a73ef06..efa96b0 100644 --- a/app/database.py +++ b/app/database.py @@ -1073,13 +1073,27 @@ async def get_health_scores(days: int = 7) -> list[dict[str, Any]]: # --- Data Retention --- RETENTION_DAYS = 400 +# High-frequency uptime samples (check_ok / check_down, one per service every 5 +# minutes) are the dominant source of health_events growth, yet get_health_scores +# only ever reads a bounded window. /api/health/scores caps that window at 90 days, +# so we keep samples just past that (95d) — enough that no allowed query can +# out-range its own samples, while still cutting the bulk sample rows ~76% versus +# the 400-day lifecycle-event history (start/stop/restart/crash), which we keep in +# full because those rows are rare and worth the long tail. +HEALTH_CHECK_RETENTION_DAYS = 95 +_HEALTH_CHECK_EVENTS = ("check_ok", "check_down") async def purge_old_data() -> int: - """Delete earnings and health_events older than RETENTION_DAYS. Returns rows deleted.""" + """Delete data past retention. Returns rows deleted. + + Earnings and lifecycle health events are kept RETENTION_DAYS; the far more + numerous uptime-sample events are trimmed to HEALTH_CHECK_RETENTION_DAYS. + """ db = await _get_db() try: cutoff = f"-{RETENTION_DAYS} days" + check_cutoff = f"-{HEALTH_CHECK_RETENTION_DAYS} days" c1 = await db.execute( "DELETE FROM earnings WHERE created_at < datetime('now', ?)", (cutoff,), @@ -1088,7 +1102,29 @@ async def purge_old_data() -> int: "DELETE FROM health_events WHERE created_at < datetime('now', ?)", (cutoff,), ) + c3 = await db.execute( + "DELETE FROM health_events WHERE event IN ('check_ok', 'check_down') AND created_at < datetime('now', ?)", + (check_cutoff,), + ) + await db.commit() + return (c1.rowcount or 0) + (c2.rowcount or 0) + (c3.rowcount or 0) + finally: + await db.close() + + +async def vacuum_database() -> None: + """Reclaim free pages left by retention deletes. + + SQLite never shrinks the file on DELETE alone, so without a periodic VACUUM the + database keeps its high-water-mark size forever even as old rows are purged. + Run off-peak (weekly) — VACUUM rewrites the whole file and briefly locks it. We + commit first because VACUUM cannot run inside an open transaction, and checkpoint + the WAL afterwards so the freed space is actually returned to the filesystem. + """ + db = await _get_db() + try: await db.commit() - return (c1.rowcount or 0) + (c2.rowcount or 0) + await db.execute("VACUUM") + await db.execute("PRAGMA wal_checkpoint(TRUNCATE)") finally: await db.close() diff --git a/tests/test_database.py b/tests/test_database.py index 62a56f9..230a496 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -424,6 +424,50 @@ async def run(): asyncio.run(run()) + def test_purge_trims_old_check_samples_but_keeps_lifecycle(self, db): + """check_ok/check_down samples are trimmed at HEALTH_CHECK_RETENTION_DAYS, + while lifecycle events (crash/restart/…) survive until RETENTION_DAYS and + recent samples are untouched.""" + + async def _insert(slug, event, days_ago): + conn = await database._get_db() + try: + await conn.execute( + "INSERT INTO health_events (slug, event, created_at) VALUES (?, ?, datetime('now', ?))", + (slug, event, f"-{days_ago} days"), + ) + await conn.commit() + finally: + await conn.close() + + async def run(): + await _insert("hg", "check_ok", 1) # recent sample -> kept + await _insert("hg", "check_ok", database.HEALTH_CHECK_RETENTION_DAYS + 10) # old sample -> purged + await _insert("hg", "crash", 300) # lifecycle < 400d -> kept + await _insert("hg", "crash", database.RETENTION_DAYS + 10) # lifecycle > 400d -> purged + + deleted = await database.purge_old_data() + assert deleted == 2 # the old sample + the ancient crash + + conn = await database._get_db() + try: + cur = await conn.execute("SELECT event, COUNT(*) c FROM health_events GROUP BY event ORDER BY event") + rows = {r["event"]: r["c"] for r in await cur.fetchall()} + finally: + await conn.close() + assert rows == {"check_ok": 1, "crash": 1} # one recent sample + the 300d crash + + asyncio.run(run()) + + def test_vacuum_runs_clean(self, db): + async def run(): + await database.record_health_event("hg", "check_ok") + # Must not raise (VACUUM cannot run inside a transaction — the impl + # commits first); a no-op-ish call on a small DB simply succeeds. + await database.vacuum_database() + + asyncio.run(run()) + class TestEncryption: def test_encrypt_decrypt_round_trip(self): diff --git a/tests/test_main_routes.py b/tests/test_main_routes.py index 4e082bb..f69459a 100644 --- a/tests/test_main_routes.py +++ b/tests/test_main_routes.py @@ -1900,6 +1900,53 @@ async def _run(): asyncio.run(_run()) assert captured["called"] is True + def test_lifespan_first_run_generates_setup_token(self): + """With no users, startup generates + persists + activates a setup token.""" + import app.main as main_mod + from app import setup_token + + async def _run(): + with ( + patch("app.main.database.init_db", new_callable=AsyncMock), + patch("app.main.database.connect_shared", new_callable=AsyncMock), + patch("app.main.database.close_shared", new_callable=AsyncMock), + patch("app.main.database.list_users_with_pwd_epoch", new_callable=AsyncMock, return_value=[]), + patch("app.main.database.has_any_users", new_callable=AsyncMock, return_value=False), + patch("app.main.database.get_config", new_callable=AsyncMock, return_value=None), + patch("app.main.database.set_config", new_callable=AsyncMock) as set_cfg, + patch("app.main.catalog.load_services"), + patch("app.main.catalog.register_sighup"), + patch("app.main.exchange_rates.refresh", new_callable=AsyncMock), + patch("app.main._run_collection", new_callable=AsyncMock), + patch("app.main.close_all_collectors", new_callable=AsyncMock, create=True), + patch("app.main.scheduler"), # isolate from the real module scheduler + ): + async with main_mod.lifespan(main_mod.app): + assert setup_token.active() is not None + set_cfg.assert_awaited_once() # persisted the freshly generated token + + asyncio.run(_run()) + + +class TestRunVacuum: + def test_run_vacuum_success(self): + import app.main as main_mod + + with patch("app.main.database.vacuum_database", new_callable=AsyncMock) as vac: + asyncio.run(main_mod._run_vacuum()) + vac.assert_awaited_once() + + def test_run_vacuum_swallows_error(self): + import app.main as main_mod + + with patch( + "app.main.database.vacuum_database", + new_callable=AsyncMock, + side_effect=Exception("boom"), + ): + # Must not raise — a failed VACUUM is logged, not fatal. + asyncio.run(main_mod._run_vacuum()) + # --------------------------------------------------------------------------- # Shared auth deps (app/deps.py) — guard branches From f78c323b315c3076b4769d158b346d7ce2cd1aae Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:04:09 +0200 Subject: [PATCH 5/5] fix(security): don't accept first-run setup token via query string (CodeRabbit) A ?setup_token= URL leaks the secret into reverse-proxy access logs and browser history. Drop query-param support in _require_first_run_access (form field + X-Setup-Token header only); the GET /register page now renders behind just the network check so the operator can reach it, the token is a visible form field they type from the logs, and the startup log no longer embeds the token in a URL. --- app/deps.py | 9 ++++++--- app/main.py | 5 +++-- app/routers/auth.py | 7 ++++--- app/templates/auth.html | 5 ++++- tests/test_setup_token.py | 10 +++++++--- 5 files changed, 24 insertions(+), 12 deletions(-) diff --git a/app/deps.py b/app/deps.py index 7fb4558..3df3c78 100644 --- a/app/deps.py +++ b/app/deps.py @@ -100,11 +100,14 @@ def _require_first_run_access(request: Request, setup_token_value: str | None = The network check alone is spoofable behind a reverse proxy (the peer is then the proxy), so the setup token — printed to the server logs, readable only with host access — is the real gate. The token is accepted from the explicit - argument (a form field), the ``setup_token`` query param, or the - ``X-Setup-Token`` header. + argument (the registration form field) or the ``X-Setup-Token`` header. + + Deliberately NOT read from the query string: a ``?setup_token=`` URL leaks the + secret into reverse-proxy access logs and browser history. The form field + (typed into the setup page) and the header keep it out of URLs. """ _require_private_network(request) - token = setup_token_value or request.query_params.get("setup_token") or request.headers.get("x-setup-token") + token = setup_token_value or request.headers.get("x-setup-token") if not setup_token.verify(token): raise HTTPException( status_code=403, diff --git a/app/main.py b/app/main.py index cdeb292..57dd646 100644 --- a/app/main.py +++ b/app/main.py @@ -332,8 +332,9 @@ async def lifespan(app: FastAPI): await database.set_config("_setup_token", _tok) setup_token.set_active(_tok) logger.warning( - "FIRST-RUN SETUP: no account exists yet. Create the owner account at " - "/register?setup_token=%s — this token is required and is shown only here.", + "FIRST-RUN SETUP: no account exists yet. Open /register and enter this " + "one-time setup token to create the owner account: %s (shown only here; " + "not embedded in any URL so it stays out of proxy logs and browser history)", _tok, ) catalog.load_services() diff --git a/app/routers/auth.py b/app/routers/auth.py index fe53613..7611dcb 100644 --- a/app/routers/auth.py +++ b/app/routers/auth.py @@ -85,8 +85,11 @@ async def page_register(request: Request, error: str = ""): user = main.auth.get_current_user(request) if not user or user.get("r") != "owner": return RedirectResponse("/login", status_code=303) + # The GET page is gated only by the network check so the operator can reach the + # form; the setup token is entered into a form field and verified on POST (never + # via URL, which would leak it into access logs / browser history). if is_first: - main._require_first_run_access(request) + main._require_private_network(request) return main.templates.TemplateResponse( request, @@ -99,8 +102,6 @@ async def page_register(request: Request, error: str = ""): "button_text": "Create Account", "error": error, "is_first": is_first, - # Carry the setup token through the form so the POST passes the same gate. - "setup_token": request.query_params.get("setup_token", "") if is_first else "", }, ) diff --git a/app/templates/auth.html b/app/templates/auth.html index e457129..c5139a6 100644 --- a/app/templates/auth.html +++ b/app/templates/auth.html @@ -152,7 +152,10 @@ {% if mode == "register" and is_first %} - +
+ + +
{% endif %}
diff --git a/tests/test_setup_token.py b/tests/test_setup_token.py index 59621e5..aa94b4f 100644 --- a/tests/test_setup_token.py +++ b/tests/test_setup_token.py @@ -81,10 +81,14 @@ def test_allows_with_correct_token_arg(self): with patch.object(deps, "_TRUST_PROXY", False): assert deps._require_first_run_access(_req("127.0.0.1"), "tok") is None - def test_token_from_query_param(self): + def test_query_param_token_is_rejected(self): + # Query-param tokens are intentionally NOT accepted — a ?setup_token= URL + # would leak the secret into proxy access logs / browser history. Even a + # correct token in the query string must be refused. setup_token.set_active("tok") - with patch.object(deps, "_TRUST_PROXY", False): - assert deps._require_first_run_access(_req("127.0.0.1", query={"setup_token": "tok"})) is None + with patch.object(deps, "_TRUST_PROXY", False), pytest.raises(HTTPException) as ei: + deps._require_first_run_access(_req("127.0.0.1", query={"setup_token": "tok"})) + assert ei.value.status_code == 403 def test_token_from_header(self): setup_token.set_active("tok")