diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index 1375fde..9776916 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -1,5 +1,10 @@ # Continuous integration on git.menzel.center (Gitea Actions). # +# Two jobs: `check` runs the test suite, the linter and the frontend +# typecheck; `build-and-push` only starts once `check` is green, so a red +# suite never reaches the registry (and never reaches the self-update +# checker, which would happily offer a broken release). +# # Builds and pushes the three images to this instance's container registry # on every push to main: backend, frontend, and agent (which is built FROM # the backend image - see agent/Dockerfile - so it has to come after). Each @@ -24,7 +29,48 @@ env: REGISTRY: git.menzel.center/menzeljonas jobs: + check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + token: ${{ secrets.CI_TOKEN }} + + - uses: actions/setup-python@v5 + with: + python-version: "3.12" # matches backend/Dockerfile + + - name: Install backend + test dependencies + working-directory: backend + run: pip install -r requirements-dev.txt + + - name: Lint (ruff) + working-directory: backend + run: ruff check . + + # The suite runs without a Docker daemon on purpose: it drives the app + # through TestClient without the lifespan, so no background loops and no + # socket. See backend/tests/conftest.py. + - name: Test (pytest) + working-directory: backend + run: pytest + + - uses: actions/setup-node@v4 + with: + node-version: "20" + cache: npm + cache-dependency-path: frontend/package-lock.json + + - name: Install frontend dependencies + working-directory: frontend + run: npm ci + + - name: Typecheck (tsc) + working-directory: frontend + run: npx tsc --noEmit -p tsconfig.json + build-and-push: + needs: check runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 diff --git a/README.md b/README.md index cc050a2..7ba2aaf 100644 --- a/README.md +++ b/README.md @@ -15,14 +15,16 @@ as intuitive as Dockge, as capable as Portainer for Compose workflows. > (Auto-update) + Phase 23 (Secrets & configs) + Phase 24 (Design System v2) > complete. -## Upgrading to 0.44.0 — two defaults changed +## Upgrading to 0.44.0 / 0.45.0 — two defaults changed 0.44.0 closes a privilege-escalation hole and tightens two defaults. Both changes can affect an existing install: 1. **The `user` role loses read access to secrets.** The file browser (page and - `/api/files/*`), the host-path picker, the audit log, `GET /api/stacks/{id}/export` - and a stack's `.env` are now admin-only. Previously any logged-in account + `/api/files/*`), the host-path picker, the audit log, `GET /api/stacks/{id}/export`, + a stack's `.env` and the template *detail* route (0.45.0 — "save stack as + template" snapshots the stack's real `.env`) are now admin-only. The template + *listing* stays open. Previously any logged-in account could download `stackpilot.db`, every `.env` and every `.secrets/*` file — and none of it was audit-logged. If you gave someone a `user` account so they could look at stacks, they still can; they just no longer get the @@ -457,11 +459,17 @@ most important ones: | Variable | Default | Purpose | |-----------------|--------------------|-------------------------------------------| -| `SECRET_KEY` | _(auto, dev only)_ | JWT signing key — **set this in prod** | +| `SECRET_KEY` | _(auto, persisted)_| JWT + at-rest encryption key (see below) | | `STACKS_DIR` | `/opt/stacks` | Where stack folders live (in-container) | | `DATA_DIR` | `/data` | SQLite DB + app data | | `CORS_ORIGINS` | localhost | Allowed API origins (comma separated) | +`SECRET_KEY` signs JWTs **and** derives the key that encrypts backup-destination +credentials in the database. Leave it unset and one is generated and written to +`${DATA_DIR}/secret_key` (mode 0600) on first start, so sessions and stored +credentials survive restarts — that file is then part of your backup. Setting it +explicitly always wins and nothing is written. + The host path for stacks is set via `STACKS_HOST_DIR` in `.env`, and it should be **the same path as `STACKS_DIR`** (`/opt/stacks` by default). Compose runs inside the backend container, so a stack's relative bind mounts (`./config`) are @@ -491,6 +499,37 @@ npm install npm run dev # http://localhost:5173 ``` +### Tests & linting + +Same three commands the CI runs — `build-and-push` only starts once they pass. + +```bash +cd backend +pip install -r requirements-dev.txt +pytest # 670 tests, no Docker daemon needed +ruff check . +cd ../frontend && npx tsc --noEmit -p tsconfig.json +``` + +The suite drives the app through `TestClient` **without** the lifespan, so it +never opens a Docker socket and never starts the background loops; `conftest.py` +points `DATA_DIR`/`STACKS_DIR` at a temp directory before anything is imported. + +The load-bearing one is `tests/test_route_authorization.py`. Authorization lives +in the routers — each of 171 routes independently picks `require_admin` or +`get_current_user`, and nothing checked that the choice was right, which is how +0.43.0 shipped a read-only role that could download the auth database. That file +states the policy once — *every route requires admin unless it is listed* — and +fails on any route that disagrees. Adding a route the `user` role may reach means +adding it to `USER_READABLE` with a note on why it cannot return a credential. +`tests/test_agent_authorization.py` does the same for the agent, where a single +forgotten `Depends(verify_token)` would expose a whole host. + +`tests/test_bundled_templates.py` covers the 83 shipped templates: each must +parse, name an image per service, keep `.env.example` in sync with the variables +compose actually reads, ship every file it bind-mounts, and never come with a +working default password. + ## API surface (Phase 1) ``` diff --git a/backend/.dockerignore b/backend/.dockerignore index 836f13d..5b9594d 100644 --- a/backend/.dockerignore +++ b/backend/.dockerignore @@ -4,3 +4,10 @@ __pycache__ data *.db .env + +# Test + lint tooling: run in CI, never needed in the runtime image. +tests +pyproject.toml +requirements-dev.txt +.pytest_cache +.ruff_cache diff --git a/backend/pyproject.toml b/backend/pyproject.toml new file mode 100644 index 0000000..29e516a --- /dev/null +++ b/backend/pyproject.toml @@ -0,0 +1,38 @@ +# Tooling config only — StackPilot's backend is not packaged, it runs from +# source in the image (see backend/Dockerfile). Runtime deps stay in +# requirements.txt; test/lint deps in requirements-dev.txt. + +[tool.pytest.ini_options] +testpaths = ["tests"] +# Import test modules without needing tests/ to be a package, and make the +# backend root importable so `from services import ...` works the same way it +# does at runtime. +pythonpath = ["."] +addopts = "-q --strict-markers" +filterwarnings = [ + # passlib 1.7.4 reads bcrypt.__about__, which bcrypt 4.x removed. Cosmetic, + # and tracked as F8 (migrate off passlib). + "ignore:.*error reading bcrypt version.*:UserWarning", +] + +[tool.ruff] +target-version = "py312" +line-length = 100 +exclude = ["templates"] + +[tool.ruff.lint] +# Deliberately a floor, not a style bar: these are the rules that catch real +# defects (undefined names, unused imports, shadowed builtins, mutable +# defaults, swallowed exception context) without demanding a reformat of the +# existing 12k lines. Import sorting ("I") is left out on purpose — it is +# style, and turning it on would rewrite the import block of nine files that +# have nothing else wrong with them. Tighten over time rather than landing a +# big-bang cleanup. +select = ["F", "E9", "B"] +ignore = [ + "B008", # Depends()/Query() in defaults is how FastAPI is written. +] + +[tool.ruff.lint.per-file-ignores] +# Routers re-export request models for the agent proxy to reuse. +"routers/agents.py" = ["F401"] diff --git a/backend/requirements-dev.txt b/backend/requirements-dev.txt new file mode 100644 index 0000000..4f140fa --- /dev/null +++ b/backend/requirements-dev.txt @@ -0,0 +1,5 @@ +# Test + lint tooling. Not installed into the runtime image (see Dockerfile); +# used by `pytest` locally and by the CI's test job. +-r requirements.txt +pytest==8.3.4 +ruff==0.9.2 diff --git a/backend/routers/secrets.py b/backend/routers/secrets.py index f790292..151c8a0 100644 --- a/backend/routers/secrets.py +++ b/backend/routers/secrets.py @@ -7,7 +7,7 @@ to take effect on running containers. """ from __future__ import annotations -from fastapi import APIRouter, Depends, HTTPException, Query, Request +from fastapi import APIRouter, Depends, HTTPException, Request from pydantic import BaseModel from sqlmodel import Session diff --git a/backend/routers/stacks.py b/backend/routers/stacks.py index 384a6c5..f9a5d5d 100644 --- a/backend/routers/stacks.py +++ b/backend/routers/stacks.py @@ -387,11 +387,10 @@ def export_stack( ): """Download the whole stack folder as a tarball. Admin only: the archive contains the ``.env`` and every ``.secrets/*`` file verbatim.""" - import io import tarfile import tempfile - stack = _get_stack_or_404(session, stack_id) + _get_stack_or_404(session, stack_id) # 404s if unknown directory = compose_service.stack_dir(stack_id) if not os.path.isdir(directory): raise HTTPException(status_code=404, detail="Stack directory missing") diff --git a/backend/routers/system.py b/backend/routers/system.py index 13f0c61..4bf338f 100644 --- a/backend/routers/system.py +++ b/backend/routers/system.py @@ -126,7 +126,7 @@ def self_update_apply( try: result = self_update_service.apply_update() except self_update_service.SelfUpdateError as exc: - raise HTTPException(status_code=400, detail=str(exc)) + raise HTTPException(status_code=400, detail=str(exc)) from exc audit_service.record( session, user=user.username, action="system.update", target=result.get("helper", ""), detail=result.get("command"), diff --git a/backend/routers/templates.py b/backend/routers/templates.py index 2333a9e..243e9f9 100644 --- a/backend/routers/templates.py +++ b/backend/routers/templates.py @@ -40,8 +40,12 @@ def list_templates( @router.get("/{template_id}") def get_template( template_id: str, - _user: User = Depends(get_current_user), + _admin: User = Depends(require_admin), ) -> dict: + """Full template incl. compose and env. Admin only: "save stack as + template" snapshots the stack's real ``.env`` into the template, so this + can carry live credentials. Only admins can instantiate a template anyway; + the listing above stays open to everyone.""" tpl = template_service.get_template(template_id) if not tpl: raise HTTPException(status_code=404, detail="Template not found") @@ -124,7 +128,7 @@ async def instantiate( raise HTTPException( status_code=exc.status if exc.status >= 400 else 502, detail={"error": exc.error, "detail": exc.detail}, - ) + ) from exc audit_service.record( session, user=user.username, action="template.instantiate", target=f"{agent.name}/{result.get('id')}", detail=template_id, ip=_ip(request), @@ -138,10 +142,12 @@ async def instantiate( try: template_service.copy_into_stack(template_id, stack_id) - except FileExistsError: - raise HTTPException(status_code=409, detail=f"Stack '{stack_id}' already exists") - except FileNotFoundError: - raise HTTPException(status_code=404, detail="Template not found") + except FileExistsError as exc: + raise HTTPException( + status_code=409, detail=f"Stack '{stack_id}' already exists" + ) from exc + except FileNotFoundError as exc: + raise HTTPException(status_code=404, detail="Template not found") from exc stack = Stack(id=stack_id, name=body.name, description=tpl.get("description")) session.add(stack) diff --git a/backend/services/backup_destination_service.py b/backend/services/backup_destination_service.py index 52018e1..9b23c8c 100644 --- a/backend/services/backup_destination_service.py +++ b/backend/services/backup_destination_service.py @@ -12,7 +12,6 @@ from __future__ import annotations import io import json import logging -import os import posixpath import stat import tarfile diff --git a/backend/services/device_service.py b/backend/services/device_service.py index ce0d941..ad28358 100644 --- a/backend/services/device_service.py +++ b/backend/services/device_service.py @@ -4,7 +4,6 @@ from __future__ import annotations import glob import os from dataclasses import asdict, dataclass -from typing import Optional from config import settings diff --git a/backend/services/exec_service.py b/backend/services/exec_service.py index ec47762..417c773 100644 --- a/backend/services/exec_service.py +++ b/backend/services/exec_service.py @@ -14,7 +14,7 @@ import socket as _socket from starlette.websockets import WebSocketDisconnect -from docker_client import DockerError, get_client, safe_call +from docker_client import get_client, safe_call from services.container_service import _get_managed DEFAULT_SHELL = "/bin/sh" diff --git a/backend/services/settings_service.py b/backend/services/settings_service.py index d52ded4..eb0f57e 100644 --- a/backend/services/settings_service.py +++ b/backend/services/settings_service.py @@ -4,7 +4,7 @@ from __future__ import annotations import json from typing import Any, Optional -from sqlmodel import Session, select +from sqlmodel import Session from config import settings as env_settings from database import engine diff --git a/backend/services/update_service.py b/backend/services/update_service.py index a4fc952..3eda132 100644 --- a/backend/services/update_service.py +++ b/backend/services/update_service.py @@ -14,7 +14,6 @@ from typing import Optional import httpx -from config import settings from docker_client import DockerError, get_client, safe_call from models.setting import EVENT_UPDATE_AVAILABLE from services import notify_service, settings_service diff --git a/backend/templates/authentik/.env.example b/backend/templates/authentik/.env.example index 72c4742..67934d2 100644 --- a/backend/templates/authentik/.env.example +++ b/backend/templates/authentik/.env.example @@ -2,6 +2,7 @@ DATA_PATH=/srv/authentik AUTHENTIK_TAG=2026.8.0 HTTP_PORT=9200 HTTPS_PORT=9243 -# Both required. Generate with: openssl rand -base64 36 -PG_PASS=change-me -AUTHENTIK_SECRET_KEY=change-me +# Both required — the stack refuses to start until they are set. +# Generate each with: openssl rand -base64 36 +PG_PASS= +AUTHENTIK_SECRET_KEY= diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py new file mode 100644 index 0000000..9c178a2 --- /dev/null +++ b/backend/tests/conftest.py @@ -0,0 +1,142 @@ +"""Shared fixtures. + +Two things have to happen before anything from the backend is imported, which +is why they sit at module level rather than in a fixture: + +* ``DATA_DIR`` / ``STACKS_DIR`` must point at a throwaway directory — ``config`` + and ``database`` read them at import time (the SQLite path is computed then), + so a fixture would be too late and the suite would scribble on a real install. +* ``SECRET_KEY`` must be set, or ``config._ensure_secret`` would generate one + and persist it into the temp data dir. Harmless, but a fixed key keeps token + fixtures reproducible. + +The app is driven through ``TestClient`` **without** entering it as a context +manager, which deliberately skips the lifespan: no background update/schedule +loops, and no Docker connection. Tests that need database tables depend on the +``db`` fixture, which runs ``init_db()`` once per session. +""" +from __future__ import annotations + +import os +import sys +import tempfile +from pathlib import Path + +import pytest + +_TMP = tempfile.mkdtemp(prefix="stackpilot-tests-") +os.environ["DATA_DIR"] = os.path.join(_TMP, "data") +os.environ["STACKS_DIR"] = os.path.join(_TMP, "stacks") +os.environ["SECRET_KEY"] = "test-secret-key-not-used-anywhere-real" +os.environ["ALLOWED_BROWSE_ROOTS"] = "/mnt,/media,/srv,/opt,/home" +os.environ["HOST_ROOT_PREFIX"] = "" + +# The backend runs from its own root at runtime (`uvicorn main:app` with +# /app as the workdir), so make the same layout importable here. +BACKEND_ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(BACKEND_ROOT)) + +os.makedirs(os.environ["DATA_DIR"], exist_ok=True) +os.makedirs(os.environ["STACKS_DIR"], exist_ok=True) + + +@pytest.fixture(scope="session") +def db(): + """Create the schema in the throwaway SQLite file. Idempotent.""" + from database import engine, init_db + + init_db() + return engine + + +@pytest.fixture(scope="session") +def app(db): + from main import app as fastapi_app + + return fastapi_app + + +@pytest.fixture(scope="session") +def users(app): + """One admin and one plain user, created directly in the DB. + + Returns ``(admin, user)`` as detached copies — the ORM objects themselves + would be bound to a closed session. + """ + from sqlmodel import Session, select + + import auth as auth_mod + from database import engine + from models.user import User + + with Session(engine) as session: + for username, role in (("test-admin", "admin"), ("test-user", "user")): + if not session.exec(select(User).where(User.username == username)).first(): + session.add( + User( + username=username, + hashed_password=auth_mod.hash_password("pw-" + username), + role=role, + ) + ) + session.commit() + admin = session.exec(select(User).where(User.username == "test-admin")).one() + user = session.exec(select(User).where(User.username == "test-user")).one() + session.expunge_all() + return admin, user + + +@pytest.fixture(scope="session") +def admin_token(users): + import auth as auth_mod + + return auth_mod.create_access_token(users[0]) + + +@pytest.fixture(scope="session") +def user_token(users): + import auth as auth_mod + + return auth_mod.create_access_token(users[1]) + + +@pytest.fixture(scope="session") +def client(app): + from starlette.testclient import TestClient + + # No `with`: skips lifespan, so no background loops and no Docker. + return TestClient(app, raise_server_exceptions=False) + + +@pytest.fixture +def as_admin(client, admin_token): + return _AuthedClient(client, admin_token) + + +@pytest.fixture +def as_user(client, user_token): + return _AuthedClient(client, user_token) + + +class _AuthedClient: + """Thin wrapper that attaches a bearer token to every request.""" + + def __init__(self, client, token: str): + self._client = client + self._headers = {"Authorization": f"Bearer {token}"} + + def request(self, method: str, url: str, **kwargs): + headers = {**self._headers, **kwargs.pop("headers", {})} + return self._client.request(method, url, headers=headers, **kwargs) + + def get(self, url, **kw): + return self.request("GET", url, **kw) + + def post(self, url, **kw): + return self.request("POST", url, **kw) + + def put(self, url, **kw): + return self.request("PUT", url, **kw) + + def delete(self, url, **kw): + return self.request("DELETE", url, **kw) diff --git a/backend/tests/test_agent_authorization.py b/backend/tests/test_agent_authorization.py new file mode 100644 index 0000000..22be6f2 --- /dev/null +++ b/backend/tests/test_agent_authorization.py @@ -0,0 +1,97 @@ +"""The agent's token guard. + +The agent has no users and no roles: one shared ``AGENT_TOKEN`` is the whole +access-control model, declared per route as +``dependencies=[Depends(verify_token)]``. That makes a forgotten decorator +argument the entire failure mode — one route without it hands anonymous full +Docker control of that host, and nothing in review would show it. + +So the invariant is asserted here rather than assumed across 50 decorators. +""" +from __future__ import annotations + +import pytest + +#: The only agent routes that may answer without a token — the liveness probe +#: the container's HEALTHCHECK calls, which returns nothing but a version. +UNAUTHENTICATED = {"GET /agent/health"} + + +@pytest.fixture(scope="module") +def agent_app(): + import agent_app as module + + return module.app + + +def _routes(app): + from fastapi.routing import APIRoute + + out = [] + for route in app.routes: + if not isinstance(route, APIRoute): + continue + for method in sorted(route.methods - {"HEAD", "OPTIONS"}): + out.append((f"{method} {route.path}", route)) + return sorted(out, key=lambda r: r[0]) + + +def _has_token_guard(route) -> bool: + from agent_app import verify_token + + found = [False] + + def walk(dependant): + for sub in dependant.dependencies: + if sub.call is verify_token: + found[0] = True + walk(sub) + + walk(route.dependant) + return found[0] + + +def test_every_agent_route_requires_the_token(agent_app): + unguarded = { + key + for key, route in _routes(agent_app) + if not _has_token_guard(route) and not key.startswith("GET /agent/health") + } + assert not unguarded, ( + "These agent routes answer without AGENT_TOKEN, which is full Docker " + f"access to the host: {sorted(unguarded)}" + ) + + +def test_only_the_health_probe_is_unauthenticated(agent_app): + open_routes = {key for key, route in _routes(agent_app) if not _has_token_guard(route)} + assert open_routes == UNAUTHENTICATED + + +def test_a_wrong_token_is_rejected(agent_app, monkeypatch): + from starlette.testclient import TestClient + + from config import settings + + monkeypatch.setattr(settings, "AGENT_TOKEN", "the-real-token", raising=False) + client = TestClient(agent_app, raise_server_exceptions=False) + + assert client.get("/agent/ping").status_code == 401 + assert client.get( + "/agent/ping", headers={"Authorization": "Bearer wrong"} + ).status_code == 401 + # The health probe stays open so the container's HEALTHCHECK works. + assert client.get("/agent/health").status_code == 200 + + +def test_an_unset_token_refuses_everything(agent_app, monkeypatch): + """An agent started without AGENT_TOKEN must not be wide open.""" + from starlette.testclient import TestClient + + from config import settings + + monkeypatch.setattr(settings, "AGENT_TOKEN", "", raising=False) + client = TestClient(agent_app, raise_server_exceptions=False) + + response = client.get("/agent/ping", headers={"Authorization": "Bearer anything"}) + assert response.status_code == 503 diff --git a/backend/tests/test_browse_sandbox.py b/backend/tests/test_browse_sandbox.py new file mode 100644 index 0000000..25ee48e --- /dev/null +++ b/backend/tests/test_browse_sandbox.py @@ -0,0 +1,116 @@ +"""The host-browser sandbox. + +Two independent gates, both in :mod:`services.device_service`: + +* ``_is_allowed`` — is the logical path under one of ``ALLOWED_BROWSE_ROOTS``? +* ``_real_root`` — maps the logical path into the container's view, and refuses + anything landing inside StackPilot's own ``DATA_DIR``. + +The second gate exists because the API deliberately never hands out what lives +there (agent tokens come back as a bool, destination secrets come back masked), +so the file browser must not be the way around that — for admins either. +""" +from __future__ import annotations + +import pytest + + +@pytest.fixture +def sandbox(monkeypatch): + """Pin the sandbox settings so the tests don't depend on deployment config.""" + from config import settings + from services import device_service + + monkeypatch.setattr(settings, "DATA_DIR", "/data", raising=False) + monkeypatch.setattr(settings, "HOST_ROOT_PREFIX", "", raising=False) + monkeypatch.setattr( + settings, "ALLOWED_BROWSE_ROOTS", ["/mnt", "/media", "/srv", "/opt", "/home"], + raising=False, + ) + return device_service + + +def _refused(mod, path: str) -> bool: + """Whether the sandbox rejects a path, by either gate.""" + if not mod._is_allowed(path): + return True + try: + mod._real_root(path) + return False + except mod.BrowseError: + return True + + +@pytest.mark.parametrize( + "path", + [ + "/data", + "/data/stackpilot.db", + "/data/secret_key", + "/opt/../data/stackpilot.db", # traversal into it + ], +) +def test_own_data_dir_is_refused(sandbox, path): + assert _refused(sandbox, path), f"{path} would expose StackPilot's own database" + + +@pytest.mark.parametrize( + "path", + ["/etc/shadow", "/root/.ssh/id_rsa", "/var/run/docker.sock", "/proc/self/environ"], +) +def test_paths_outside_the_roots_are_refused(sandbox, path): + assert _refused(sandbox, path) + + +@pytest.mark.parametrize( + "path", + ["/opt", "/opt/stacks/jellyfin/.env", "/srv/media", "/mnt", "/home/someone"], +) +def test_allowed_roots_stay_reachable(sandbox, path): + assert not _refused(sandbox, path), f"{path} should still be browsable" + + +def test_slash_in_the_roots_opens_everything_except_the_data_dir(sandbox, monkeypatch): + """A "/" entry switches the sandbox off — that is why it is not a default. + + It still must not open StackPilot's own data directory, since that gate is + independent of the root list. + """ + from config import settings + + monkeypatch.setattr(settings, "ALLOWED_BROWSE_ROOTS", ["/"], raising=False) + assert not _refused(sandbox, "/etc/shadow") + assert _refused(sandbox, "/data/stackpilot.db") + + +def test_host_root_prefix_maps_paths_into_the_container(sandbox, monkeypatch): + """With the host mounted at a prefix, /data is a host path, not our own. + + The container's own ``/data`` becomes unreachable by any logical path in + this mode, so the refusal correctly does not apply. + """ + from config import settings + + monkeypatch.setattr(settings, "HOST_ROOT_PREFIX", "/host_root", raising=False) + monkeypatch.setattr(settings, "ALLOWED_BROWSE_ROOTS", ["/data", "/opt"], raising=False) + assert sandbox._real_root("/data/foo") == "/host_root/data/foo" + + +def test_file_service_shares_the_same_gate(sandbox): + """``file_service`` must not have its own, weaker path check.""" + from services import file_service + + with pytest.raises(file_service.BrowseError): + file_service._safe_real("/data/stackpilot.db") + with pytest.raises(file_service.BrowseError): + file_service._safe_real("/etc/shadow") + assert file_service._safe_real("/opt/stacks") == "/opt/stacks" + + +@pytest.mark.parametrize("name", ["..", ".", "a/b", "a\\b", ""]) +def test_child_rejects_anything_but_a_single_component(sandbox, name): + """Upload and rename build paths through ``_child``; traversal dies here.""" + from services import file_service + + with pytest.raises(file_service.BrowseError): + file_service._child("/opt", name) diff --git a/backend/tests/test_bundled_templates.py b/backend/tests/test_bundled_templates.py new file mode 100644 index 0000000..d3c763d --- /dev/null +++ b/backend/tests/test_bundled_templates.py @@ -0,0 +1,172 @@ +"""The bundled template library. + +83 templates ship in the image, and a broken one only shows up when somebody +pulls it and the deploy fails. These checks are what was run by hand when the +library was written, made permanent: every template must be discoverable by +the service, parse as YAML, name an image for every service, and keep its +``.env.example`` in sync with the variables its compose file actually uses. + +The last one is the rule that keeps the library trustworthy: a ``${VAR}`` +without a default and without an ``.env.example`` entry deploys as an empty +string, which is how you get a container listening on ``:`` or a database with +a blank password. +""" +from __future__ import annotations + +import json +import re +from pathlib import Path + +import pytest +import yaml + +TEMPLATES_DIR = Path(__file__).resolve().parent.parent / "templates" + +#: ``${NAME}``, ``${NAME:-default}``, ``${NAME:?required}`` … +VAR = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)(:?[-?][^}]*)?\}") + +COMPOSE_NAMES = ("compose.yaml", "compose.yml", "docker-compose.yml", "docker-compose.yaml") + + +def _template_dirs(): + return sorted(p for p in TEMPLATES_DIR.iterdir() if p.is_dir()) + + +def _compose_file(folder: Path) -> Path | None: + for name in COMPOSE_NAMES: + if (folder / name).is_file(): + return folder / name + return None + + +def _env_example(folder: Path) -> dict[str, str]: + path = folder / ".env.example" + if not path.is_file(): + return {} + out = {} + for line in path.read_text().splitlines(): + line = line.strip() + if line and not line.startswith("#") and "=" in line: + key, value = line.split("=", 1) + out[key.strip()] = value.strip() + return out + + +def _compose_vars(text: str) -> dict[str, str]: + """Variable name -> its modifier (``:-``, ``:?`` or empty).""" + found: dict[str, str] = {} + for match in VAR.finditer(text): + name, modifier = match.group(1), match.group(2) or "" + # A required marker anywhere wins over a defaulted use elsewhere. + if name not in found or modifier.startswith((":?", "?")): + found[name] = modifier + return found + + +SLUGS = [p.name for p in _template_dirs()] + + +def test_the_library_is_not_empty(): + assert len(SLUGS) > 50, f"only {len(SLUGS)} templates found — is the directory intact?" + + +@pytest.mark.parametrize("slug", SLUGS) +def test_template_is_discoverable_by_the_service(slug): + """What the service lists is what ships — a folder without a compose file + is silently skipped by ``_is_template`` and would never appear in the UI.""" + from services import template_service + + detail = template_service.get_template(slug) + assert detail is not None, f"{slug} is on disk but the service does not see it" + assert detail["name"] + assert detail["compose"].strip() + + +@pytest.mark.parametrize("slug", SLUGS) +def test_metadata_is_complete(slug): + meta = json.loads((TEMPLATES_DIR / slug / "template.json").read_text()) + for key in ("name", "description", "tags", "gpu"): + assert key in meta, f"{slug}: template.json is missing '{key}'" + assert meta["name"].strip() + assert len(meta["description"]) > 20, f"{slug}: description is too thin to be useful" + assert isinstance(meta["tags"], list) and meta["tags"], f"{slug}: needs at least one tag" + + +@pytest.mark.parametrize("slug", SLUGS) +def test_compose_parses_and_every_service_names_an_image(slug): + folder = TEMPLATES_DIR / slug + compose_file = _compose_file(folder) + assert compose_file is not None, f"{slug}: no compose file" + parsed = yaml.safe_load(compose_file.read_text()) + assert isinstance(parsed, dict) and parsed.get("services"), f"{slug}: no services block" + for name, service in parsed["services"].items(): + assert "image" in service, f"{slug}: service '{name}' has no image" + + +@pytest.mark.parametrize("slug", SLUGS) +def test_every_variable_has_a_default_or_an_env_entry(slug): + """No ``${VAR}`` may silently interpolate to an empty string. + + Either compose carries a default (``${PORT:-8080}``) or ``.env.example`` + lists the variable so the user is prompted for it. + """ + folder = TEMPLATES_DIR / slug + env = _env_example(folder) + used = _compose_vars(_compose_file(folder).read_text()) + missing = [ + name + for name, modifier in used.items() + if name not in env and not modifier.startswith((":-", "-")) + ] + assert not missing, f"{slug}: {missing} have no default and no .env.example entry" + + +@pytest.mark.parametrize("slug", SLUGS) +def test_env_example_has_no_dead_entries(slug): + """A variable in .env.example that compose never reads is a trap — it looks + like a knob and does nothing.""" + folder = TEMPLATES_DIR / slug + used = _compose_vars(_compose_file(folder).read_text()) + dead = [name for name in _env_example(folder) if name not in used] + assert not dead, f"{slug}: .env.example defines {dead}, which compose never uses" + + +@pytest.mark.parametrize("slug", SLUGS) +def test_required_secrets_ship_empty(slug): + """A template must never come with a working default password. + + Anything marked required (``${VAR:?…}``) has to be blank in .env.example so + the deploy fails loudly instead of starting with a known credential. + """ + folder = TEMPLATES_DIR / slug + env = _env_example(folder) + required = [n for n, m in _compose_vars(_compose_file(folder).read_text()).items() + if m.startswith((":?", "?"))] + prefilled = [n for n in required if env.get(n)] + assert not prefilled, f"{slug}: required secrets {prefilled} ship with a value" + + +@pytest.mark.parametrize("slug", SLUGS) +def test_extra_files_are_shipped_not_just_referenced(slug): + """A ``./file`` bind mount must point at a file the template actually ships, + or the deploy creates a *directory* there and the app misreads its config.""" + folder = TEMPLATES_DIR / slug + compose = _compose_file(folder).read_text() + for match in re.finditer(r"^\s*-\s*\./([^:\s]+):", compose, re.M): + referenced = folder / match.group(1) + assert referenced.is_file(), ( + f"{slug}: compose mounts ./{match.group(1)} but the template does not ship it" + ) + + +def test_pulling_a_template_promotes_env_example_to_env(tmp_path): + """The pull path itself: the folder is copied, template.json is left behind + and .env.example becomes a real .env.""" + from services import template_service + + target = tmp_path / "pulled" + template_service.copy_into_stack("uptime-kuma", "pulled", override=str(tmp_path)) + assert (target / "compose.yaml").is_file() + assert (target / ".env").is_file() + assert not (target / ".env.example").exists() + assert not (target / "template.json").exists() diff --git a/backend/tests/test_compose_service.py b/backend/tests/test_compose_service.py new file mode 100644 index 0000000..fe1c51b --- /dev/null +++ b/backend/tests/test_compose_service.py @@ -0,0 +1,161 @@ +"""Stack storage and status derivation. + +``compose_service`` is where the file-is-the-truth model lives: slugs become +directory names, directory names become compose project names, and container +states become the one status the UI shows. All three are pure enough to test +without a Docker daemon. +""" +from __future__ import annotations + +import os + +import pytest + + +@pytest.fixture +def svc(): + from services import compose_service + + return compose_service + + +# --------------------------------------------------------------------------- # +# Slugs — these become directory names and compose project names +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + "name,expected", + [ + ("Jellyfin", "jellyfin"), + ("My Media Server", "my-media-server"), + ("Paperless-NGX", "paperless-ngx"), + (" spaces ", "spaces"), + ("Wiki.js", "wiki-js"), + ("a---b", "a-b"), + ("--leading-and-trailing--", "leading-and-trailing"), + ("Ümlaut Stack", "mlaut-stack"), + ], +) +def test_slugify(svc, name, expected): + assert svc.slugify(name) == expected + + +@pytest.mark.parametrize("name", ["", " ", "///", "..."]) +def test_slugify_never_returns_an_empty_or_traversing_slug(svc, name): + """The slug is joined onto STACKS_DIR, so an empty or dotted result would + point the stack directory at the root itself.""" + slug = svc.slugify(name) + assert slug + assert slug not in (".", "..") + assert "/" not in slug + + +def test_stack_dir_stays_under_the_root(svc): + root = svc.stacks_root() + assert svc.stack_dir("jellyfin").startswith(root + os.sep) + + +# --------------------------------------------------------------------------- # +# Status derivation +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + "states,expected", + [ + ([], "stopped"), + (["running"], "running"), + (["running", "running"], "running"), + (["running", "exited"], "partial"), + (["exited", "exited"], "stopped"), + (["created"], "stopped"), + (["running", "dead"], "error"), + (["dead"], "error"), + ], +) +def test_status_from_states(svc, states, expected): + assert svc._status_from_states(states) == expected + + +def test_busy_stacks_report_as_updating(svc): + """The busy flag has to win over the container states, or a stack shows + 'stopped' for the moment between `down` and `up` during an update.""" + svc.mark_busy("busy-stack") + try: + assert svc.compute_status("busy-stack", containers=[]) == "updating" + finally: + svc.clear_busy("busy-stack") + assert svc.compute_status("busy-stack", containers=[]) == "stopped" + + +# --------------------------------------------------------------------------- # +# Reading and writing stack files +# --------------------------------------------------------------------------- # + + +def test_write_compose_keeps_a_backup_of_the_previous_version(svc, tmp_path): + """Every save writes a .bak — the raw material for a rollback feature.""" + stack_id = "backup-check" + svc.write_compose(stack_id, "services:\n a:\n image: alpine\n", override=str(tmp_path)) + svc.write_compose(stack_id, "services:\n b:\n image: nginx\n", override=str(tmp_path)) + + directory = tmp_path / stack_id + assert "image: nginx" in (directory / "compose.yaml").read_text() + assert "image: alpine" in (directory / "compose.yaml.bak").read_text() + + +def test_reading_a_missing_stack_returns_empty_not_an_error(svc, tmp_path): + assert svc.read_compose("does-not-exist", override=str(tmp_path)) == "" + assert svc.read_env("does-not-exist", override=str(tmp_path)) == "" + + +def test_discover_stacks_finds_only_directories_with_a_compose_file(svc, tmp_path): + (tmp_path / "real").mkdir() + (tmp_path / "real" / "compose.yaml").write_text("services: {}\n") + (tmp_path / "legacy").mkdir() + (tmp_path / "legacy" / "docker-compose.yml").write_text("services: {}\n") + (tmp_path / "not-a-stack").mkdir() + (tmp_path / "not-a-stack" / "readme.txt").write_text("hi\n") + + assert svc.discover_stacks(override=str(tmp_path)) == ["legacy", "real"] + + +def test_clone_refuses_to_overwrite_an_existing_stack(svc, tmp_path): + svc.write_compose("source", "services: {}\n", override=str(tmp_path)) + svc.write_compose("target", "services: {}\n", override=str(tmp_path)) + with pytest.raises(svc.StackFileError): + svc.clone_stack_files("source", "target", override=str(tmp_path)) + + +# --------------------------------------------------------------------------- # +# Per-stack secrets +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize("name", ["../escape", "a/b", ".hidden", "", "na me"]) +def test_secret_names_reject_traversal_and_hidden_files(name): + """Secret names become filenames inside the stack's .secrets directory.""" + from services import secret_service + + with pytest.raises(secret_service.SecretError): + secret_service._check_name(name) + + +def test_secret_files_are_written_owner_only(tmp_path): + from services import secret_service + + secret_service.write_secret("s", "secret", "db_password", "hunter2", override=str(tmp_path)) + path = tmp_path / "s" / ".secrets" / "db_password" + assert path.read_text() == "hunter2" + assert oct(path.stat().st_mode)[-3:] == "600" + assert oct(path.parent.stat().st_mode)[-3:] == "700" + + +def test_listing_secrets_never_returns_their_content(tmp_path): + from services import secret_service + + secret_service.write_secret("s2", "secret", "token", "top-secret", override=str(tmp_path)) + listed = secret_service.list_secrets("s2", "secret", override=str(tmp_path)) + assert [item["name"] for item in listed] == ["token"] + assert "top-secret" not in str(listed) diff --git a/backend/tests/test_crypto_service.py b/backend/tests/test_crypto_service.py new file mode 100644 index 0000000..3689373 --- /dev/null +++ b/backend/tests/test_crypto_service.py @@ -0,0 +1,113 @@ +"""Encryption of the secrets that have to live in the database. + +Backup-destination credentials can't be files on disk — background jobs need +them — so they are encrypted at rest with a key derived from ``SECRET_KEY``. +The behaviour that matters beyond the round-trip: rows written before this +existed are plaintext and must keep working, and a changed ``SECRET_KEY`` must +produce a clear error rather than silent garbage. +""" +from __future__ import annotations + +import json + +import pytest + + +@pytest.fixture +def crypto(): + from services import crypto_service + + return crypto_service + + +def test_round_trip(crypto): + payload = json.dumps({"host": "nas.lan", "password": "hunter2"}) + encrypted = crypto.encrypt(payload) + assert encrypted.startswith(crypto.PREFIX) + assert "hunter2" not in encrypted + assert crypto.decrypt(encrypted) == payload + + +def test_encrypting_twice_is_a_no_op(crypto): + """Guards the migration, which may run over a mix of both forms.""" + once = crypto.encrypt("secret") + assert crypto.encrypt(once) == once + + +def test_plaintext_rows_are_passed_through(crypto): + """Rows written before encryption existed must not break on read.""" + legacy = '{"host": "old.example"}' + assert crypto.is_encrypted(legacy) is False + assert crypto.decrypt(legacy) == legacy + + +def test_empty_values(crypto): + assert crypto.decrypt(crypto.encrypt("")) == "" + assert crypto.decrypt("") == "" + + +def test_a_changed_secret_key_raises_a_useful_error(crypto, monkeypatch): + from config import settings + + encrypted = crypto.encrypt("secret") + monkeypatch.setattr(settings, "SECRET_KEY", "a-completely-different-key", raising=False) + with pytest.raises(crypto.DecryptError) as excinfo: + crypto.decrypt(encrypted) + assert "SECRET_KEY" in str(excinfo.value) + + +def test_destination_config_round_trips_through_the_service(monkeypatch): + """The service-level pair the routers actually use.""" + from models.backup_destination import BackupDestination + from services import backup_destination_service as svc + + config = {"host": "nas.lan", "username": "backup", "password": "hunter2"} + stored = svc.dump_config(config) + assert "hunter2" not in stored + dest = BackupDestination(name="nas", type="sftp", config=stored) + assert svc.parse_config(dest) == config + + +def test_undecryptable_destination_reads_as_unconfigured(monkeypatch): + """A lost key must not take the destinations list down with a 500. + + The destination shows up empty (and the reason is logged) so the rest of the + UI keeps working and the user can re-enter the credentials. + """ + from config import settings + from models.backup_destination import BackupDestination + from services import backup_destination_service as svc + + dest = BackupDestination(name="nas", type="sftp", config=svc.dump_config({"host": "x"})) + monkeypatch.setattr(settings, "SECRET_KEY", "yet-another-key", raising=False) + assert svc.parse_config(dest) == {} + + +def test_migration_encrypts_plaintext_rows_and_is_idempotent(db): + from sqlmodel import Session, select + + from database import engine + from models.backup_destination import BackupDestination + from services import backup_destination_service as svc + from services import crypto_service + + with Session(engine) as session: + session.add( + BackupDestination( + name="legacy-plaintext", + type="sftp", + config='{"host": "legacy.example", "password": "in-the-clear"}', + ) + ) + session.commit() + + assert svc.migrate_plaintext_configs(session) >= 1 + row = session.exec( + select(BackupDestination).where(BackupDestination.name == "legacy-plaintext") + ).one() + assert crypto_service.is_encrypted(row.config) + assert "in-the-clear" not in row.config + assert svc.parse_config(row)["password"] == "in-the-clear" + + # Second pass finds nothing left to do. + assert svc.migrate_plaintext_configs(session) == 0 diff --git a/backend/tests/test_route_authorization.py b/backend/tests/test_route_authorization.py new file mode 100644 index 0000000..ab45915 --- /dev/null +++ b/backend/tests/test_route_authorization.py @@ -0,0 +1,299 @@ +"""The authorization matrix — StackPilot's most load-bearing test. + +Authorization lives in the routers: each one picks ``require_admin`` or +``get_current_user`` per route, and nothing checks that the choice was right. +That is how 0.43.0 shipped a ``user`` role that could download the auth +database, every stack's ``.env`` and every ``.secrets/*`` file — a wrong default +on four routes, invisible in review. + +So the policy is written down here instead of being implied by 171 individual +decisions: + + every route requires admin, unless it is listed in USER_READABLE or PUBLIC. + +Adding a route that the read-only role can reach means adding it to the list, +which is the review moment this test exists to force. The check is static — it +reads FastAPI's dependency graph rather than calling the routes — because +calling all 171 would mean constructing valid bodies for each and would happily +fire ``POST /stacks/{id}/down`` at whatever Docker is around. + +The dynamic tests at the bottom are the direct regression net for the specific +leaks that were found: they use a real non-admin token and assert 403. +""" +from __future__ import annotations + +import pytest + +# --------------------------------------------------------------------------- # +# The policy +# --------------------------------------------------------------------------- # + +#: Reachable without any token. Everything needed to log in, plus the health +#: probe the container's HEALTHCHECK hits. +PUBLIC = { + "GET /api/health", + "GET /api/auth/needs-setup", + "POST /api/auth/setup", + "POST /api/auth/login", + "POST /api/auth/refresh", +} + +#: Reachable by the read-only ``user`` role. Everything here has been checked +#: for whether it can return a credential: +#: +#: * ``GET /api/agents`` returns ``AgentRead``, whose ``token_set`` is a bool. +#: * ``GET /api/settings`` returns the interval and counts — webhook URLs (which +#: carry tokens) come from the admin-only ``/api/settings/webhooks``. +#: * ``GET /api/stacks/{id}`` and its agent twin blank out ``env`` for non-admins. +#: * ``GET /api/templates`` is metadata only; the detail route, which returns a +#: template's env, is admin-only. +#: * The ``/api/editor/*`` routes are pure YAML transformations — they take YAML +#: in and hand YAML back, touching nothing on disk. +USER_READABLE = { + "GET /api/auth/me", + "GET /api/dashboard/fleet", + # Stacks: status, logs and the compose file. Not the .env, not the export. + "GET /api/stacks", + "GET /api/stacks/stats", + "GET /api/stacks/updates", + "GET /api/stacks/{stack_id}", + "GET /api/stacks/{stack_id}/auto-update", + "GET /api/stacks/{stack_id}/logs", + "GET /api/stacks/{stack_id}/services/{service}/logs", + "POST /api/stacks/convert", + # Editor helpers: stateless YAML rewriting. + "POST /api/editor/validate", + "POST /api/editor/services", + "POST /api/editor/add-volume", + "POST /api/editor/add-device", + "POST /api/editor/remove-device", + "POST /api/editor/set-gpu", + "POST /api/editor/set-privileged", + "POST /api/editor/set-resources", + # Read-only inventory. + "GET /api/containers/{container_id}", + "GET /api/images", + "GET /api/images/updates", + "GET /api/networks", + "GET /api/networks/{network_id}", + "GET /api/networks/{network_id}/containers", + "GET /api/volumes", + "GET /api/volumes/sizes", + "GET /api/volumes/orphaned", + "POST /api/volumes/generate-yaml", + "POST /api/ports/conflicts", + "GET /api/settings", + "GET /api/system/info", + "GET /api/system/gpus", + "GET /api/system/devices", + "GET /api/system/update", + "GET /api/templates", + # Remote hosts: the same read-only surface, proxied. + "GET /api/agents", + "POST /api/agents/{agent_id}/ping", + "GET /api/agents/{agent_id}/system", + "GET /api/agents/{agent_id}/stacks", + "GET /api/agents/{agent_id}/stacks/stats", + "GET /api/agents/{agent_id}/stacks/updates", + "GET /api/agents/{agent_id}/stacks/{stack_id}", + "GET /api/agents/{agent_id}/stacks/{stack_id}/auto-update", + "GET /api/agents/{agent_id}/stacks/{stack_id}/logs", + "GET /api/agents/{agent_id}/containers/{container_id}", + "GET /api/agents/{agent_id}/images", + "GET /api/agents/{agent_id}/images/updates", + "GET /api/agents/{agent_id}/networks", + "GET /api/agents/{agent_id}/networks/{network_id}", + "GET /api/agents/{agent_id}/networks/{network_id}/containers", + "GET /api/agents/{agent_id}/volumes", + "GET /api/agents/{agent_id}/volumes/sizes", +} + + +# --------------------------------------------------------------------------- # +# Reading the declared authorization off the routes +# --------------------------------------------------------------------------- # + + +def _declared_auth(route) -> str: + """What a route actually requires, per its dependency graph.""" + from auth import get_current_user, require_admin + + found = set() + + def walk(dependant): + for sub in dependant.dependencies: + if sub.call is require_admin: + found.add("admin") + elif sub.call is get_current_user: + found.add("user") + walk(sub) + + walk(route.dependant) + if "admin" in found: + return "admin" + if "user" in found: + return "user" + return "public" + + +def _expected_auth(key: str) -> str: + if key in PUBLIC: + return "public" + if key in USER_READABLE: + return "user" + return "admin" + + +def _all_routes(app): + from fastapi.routing import APIRoute + + out = [] + for route in app.routes: + if not isinstance(route, APIRoute): + continue + for method in sorted(route.methods - {"HEAD", "OPTIONS"}): + out.append((f"{method} {route.path}", route)) + return sorted(out, key=lambda r: r[0]) + + +@pytest.fixture(scope="session") +def routes(app): + return _all_routes(app) + + +# --------------------------------------------------------------------------- # +# The matrix +# --------------------------------------------------------------------------- # + + +def test_every_route_matches_the_declared_policy(routes): + """Each route requires exactly what PUBLIC / USER_READABLE say it should. + + A new route that nobody classified defaults to "admin" — which is the safe + direction. What this catches is the dangerous one: a route written with + ``get_current_user`` that was never weighed against "can this return a + credential". + """ + wrong = [] + for key, route in routes: + actual, expected = _declared_auth(route), _expected_auth(key) + if actual != expected: + wrong.append(f" {key}\n declared={actual} expected={expected}") + assert not wrong, ( + "Route authorization does not match the policy in this file.\n\n" + + "\n".join(wrong) + + "\n\nIf the route is genuinely safe for the read-only role, add it to " + "USER_READABLE with a note on why it cannot return a credential. " + "Otherwise give it require_admin." + ) + + +def test_no_unlisted_route_is_reachable_without_a_token(routes): + """Only the login/health surface may skip authentication entirely.""" + unauthenticated = {key for key, route in routes if _declared_auth(route) == "public"} + assert unauthenticated == PUBLIC + + +def test_policy_lists_have_no_stale_entries(routes): + """Keep the lists honest when routes get renamed or removed.""" + known = {key for key, _ in routes} + assert not (PUBLIC - known), f"PUBLIC lists routes that no longer exist: {PUBLIC - known}" + assert not (USER_READABLE - known), ( + f"USER_READABLE lists routes that no longer exist: {USER_READABLE - known}" + ) + + +def test_everything_touching_the_filesystem_requires_admin(routes): + """The file browser, host-path picker and stack export, as one rule. + + These reach whatever the backend container can see — which includes every + ``.env`` and ``.secrets/*``. Reads are no less sensitive than writes here, + which is the mistake this test exists to prevent a repeat of. + """ + offenders = [ + key + for key, route in routes + if ("/files" in key or "/host/paths" in key or key.endswith("/export")) + and _declared_auth(route) != "admin" + ] + assert not offenders, f"Filesystem routes must be admin-only: {offenders}" + + +# --------------------------------------------------------------------------- # +# Regression tests for the leaks that were actually found (F1, F3) +# --------------------------------------------------------------------------- # + +#: Routes a ``user`` token could reach before 0.44.0, each of which handed out +#: credentials. A 403 here is the whole point. +LEAKED_BEFORE_0_44 = [ + ("GET", "/api/files/list?path=/opt"), + ("GET", "/api/files/read?path=/opt/stacks/x/.env"), + ("GET", "/api/files/download?path=/opt/stacks/x/.env"), + ("GET", "/api/host/paths?path=/opt"), + ("GET", "/api/audit"), + ("GET", "/api/stacks/anything/export"), + ("GET", "/api/templates/jellyfin"), +] + + +@pytest.mark.parametrize("method,url", LEAKED_BEFORE_0_44) +def test_read_only_role_is_refused(as_user, method, url): + assert as_user.request(method, url).status_code == 403, ( + f"{method} {url} is reachable by the read-only role again" + ) + + +@pytest.mark.parametrize("method,url", LEAKED_BEFORE_0_44) +def test_admin_is_not_refused(as_admin, method, url): + """The same routes must still work for admins. + + Anything but 403 passes: without a Docker daemon or the referenced paths + these legitimately answer 400/404/502, and this test is about the + authorization layer, not the handler. + """ + assert as_admin.request(method, url).status_code != 403 + + +def test_missing_token_is_401_not_403(client): + assert client.get("/api/files/read?path=/etc/hostname").status_code == 401 + + +# --------------------------------------------------------------------------- # +# The .env is withheld from the read-only role (F1, via the stacks API) +# --------------------------------------------------------------------------- # + + +@pytest.fixture +def stack_with_env(app): + """A real stack folder plus its DB row, so GET /api/stacks/{id} resolves.""" + import os + + from sqlmodel import Session + + from database import engine + from models.stack import Stack + from services import compose_service + + stack_id = "authz-fixture" + directory = compose_service.stack_dir(stack_id) + os.makedirs(directory, exist_ok=True) + compose_service.write_compose(stack_id, "services:\n app:\n image: alpine\n") + compose_service.write_env(stack_id, "DB_PASSWORD=super-secret-value\n") + with Session(engine) as session: + if not session.get(Stack, stack_id): + session.add(Stack(id=stack_id, name="authz fixture")) + session.commit() + return stack_id + + +def test_stack_detail_withholds_env_from_the_read_only_role(as_user, stack_with_env): + body = as_user.get(f"/api/stacks/{stack_with_env}").json() + assert body["env"] == "" + assert "super-secret-value" not in str(body) + # The compose file is still there — the read-only role keeps a useful view. + assert "image: alpine" in body["yaml"] + + +def test_stack_detail_gives_admins_the_env(as_admin, stack_with_env): + body = as_admin.get(f"/api/stacks/{stack_with_env}").json() + assert "super-secret-value" in body["env"] diff --git a/backend/version.py b/backend/version.py index c3fc0f9..9b6d831 100644 --- a/backend/version.py +++ b/backend/version.py @@ -1,3 +1,3 @@ """Single source of truth for the StackPilot release version.""" -APP_VERSION = "0.44.0" +APP_VERSION = "0.45.0" diff --git a/frontend/package.json b/frontend/package.json index 0e97b36..bcb7b0e 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "stackpilot-frontend", "private": true, - "version": "0.44.0", + "version": "0.45.0", "type": "module", "scripts": { "dev": "vite",