diff --git a/agents.md b/agents.md index f867d737..755a1bff 100644 --- a/agents.md +++ b/agents.md @@ -71,7 +71,7 @@ async with db_conn() as conn: await db_installed_apps.update_status(conn, "myapp", Status.RUNNING) ``` -Tables: `identities`, `terminals`, `installed_apps`, `peers`, `backups`, `tours`, `app_usage_tracks`, `kv_store`. +Tables: `identities`, `terminals`, `installed_apps`, `peers`, `backups`, `tours`, `app_usage_tracks`, `kv_store`, `app_secrets`. Postgres data is not part of the rclone backup set (which only syncs `core/`/`user_data/`). To keep it, `database/db_snapshot.py` dumps all application tables to `core/db_snapshot.json` before each backup, and `init_database()` restores it on a fresh shard (before the default identity is generated, so the restored identity survives). Pre-0.38 backups are restored from TinyDB by `tinydb_migration.py` instead. @@ -101,7 +101,7 @@ Blinker-based async signals defined in `util/signals.py`. DB-writing handlers ar ### App Installation Flow 0. Custom-app uploads (`POST /protected/apps`) are validated by `app_installation/app_zip.py` **before** any state is created: the zip is buffered to a temp dir, must carry `app_meta.json` + `docker-compose.yml.template` at its root (a single top-level directory is stripped), and the app name comes from the manifest, not the filename. Invalid uploads get a 400 and leave no row, no dir, no task. Extraction goes through `extract_app_zip`, which refuses members resolving outside the app dir. 1. Request queued → `InstallationWorker` picks it up -2. Docker Compose template rendered with Jinja2 (shard domain, data paths, etc.) +2. Docker Compose template rendered with Jinja2 (shard domain, data paths, per-app secrets, etc.) 3. Traefik dynamic config generated for the app's subdomain routing 4. `docker-compose up -d` via subprocess 5. Status updated in PostgreSQL @@ -120,6 +120,23 @@ are either not there yet or already being removed, and `write_traefik_dyn_config runs inside the lifespan — an unfiltered status whose metadata is missing raises `MetadataNotFound` and takes down the boot. +### App compose template context +`render_docker_compose_template` (`app_installation/util.py`) exposes three things to an +app's `docker-compose.yml.template`: `fs.*` (host data paths), `portal` (the shard +identity), and `secret('')`. The secret helper returns an app-scoped secret, +generating one (32 chars, `[A-Za-z0-9]`) on first reference and persisting it in the +`app_secrets` table (`database/app_secrets.py`); the name is arbitrary and declared +purely by referencing it. The resolver mints missing secrets synchronously during the +single jinja pass (jinja is sync, the DB is not) and they are persisted afterwards. A +secret is stable across +re-renders, upgrades and restarts (it is reused, never rotated) and rides along in the DB +snapshot backup. Secrets are kept on uninstall (so a reinstall of the same app reuses +them, matching the app's retained `user_data`), never rotated, and never encrypted at +rest. App-scoping is NOT a boundary against a hostile app author: the compose template is +rendered with a non-sandboxed jinja environment (as `portal`/`fs` already are), so a +malicious template can escape and read any app's secrets. Closing that gap would need a +`SandboxedEnvironment`; it is out of scope here. + ### Async Fire-and-Forget Long operations use `asyncio.create_task()` with done callbacks. No thread pools. diff --git a/migrations/shard-core-0002-app-secrets.sql b/migrations/shard-core-0002-app-secrets.sql new file mode 100644 index 00000000..14d39dc2 --- /dev/null +++ b/migrations/shard-core-0002-app-secrets.sql @@ -0,0 +1,9 @@ +-- shard-core-0002-app-secrets +-- depends: shard-core-0001-init + +CREATE TABLE IF NOT EXISTS app_secrets ( + app_name TEXT NOT NULL, + name TEXT NOT NULL, + value TEXT NOT NULL, + PRIMARY KEY (app_name, name) +); diff --git a/shard_core/database/app_secrets.py b/shard_core/database/app_secrets.py new file mode 100644 index 00000000..1747e1fe --- /dev/null +++ b/shard_core/database/app_secrets.py @@ -0,0 +1,17 @@ +from typing import LiteralString + +from psycopg import AsyncConnection + + +async def get_all_for_app(conn: AsyncConnection, app_name: str) -> dict[str, str]: + sql: LiteralString = "SELECT name, value FROM app_secrets WHERE app_name = %s" + async with conn.cursor() as cur: + await cur.execute(sql, (app_name,)) + return {name: value for name, value in await cur.fetchall()} + + +async def insert(conn: AsyncConnection, app_name: str, name: str, value: str): + sql: LiteralString = """INSERT INTO app_secrets (app_name, name, value) + VALUES (%(app_name)s, %(name)s, %(value)s) + ON CONFLICT (app_name, name) DO NOTHING""" + await conn.execute(sql, {"app_name": app_name, "name": name, "value": value}) diff --git a/shard_core/service/app_installation/app_secrets.py b/shard_core/service/app_installation/app_secrets.py new file mode 100644 index 00000000..3649a1bc --- /dev/null +++ b/shard_core/service/app_installation/app_secrets.py @@ -0,0 +1,48 @@ +import secrets +import string + +from shard_core.database import app_secrets as db_app_secrets +from shard_core.database.connection import db_conn + +_SECRET_ALPHABET = string.ascii_letters + string.digits +_SECRET_LENGTH = 32 + + +def generate_secret() -> str: + return "".join(secrets.choice(_SECRET_ALPHABET) for _ in range(_SECRET_LENGTH)) + + +class SecretResolver: + """Jinja ``secret('name')`` helper for an app's compose template. + + Returns an app-scoped secret, reusing an existing one or minting a new one + (32 chars from ``[A-Za-z0-9]``) on first reference. New secrets are only + remembered here; call :func:`persist_new_secrets` after rendering to store + them. Resolving synchronously keeps rendering to a single jinja pass even + though the DB is async. + """ + + def __init__(self, existing: dict[str, str]): + self._existing = existing + self.new: dict[str, str] = {} + + def __call__(self, name: str) -> str: + if name in self._existing: + return self._existing[name] + if name not in self.new: + self.new[name] = generate_secret() + return self.new[name] + + +async def load_secret_resolver(app_name: str) -> SecretResolver: + async with db_conn() as conn: + existing = await db_app_secrets.get_all_for_app(conn, app_name) + return SecretResolver(existing) + + +async def persist_new_secrets(app_name: str, resolver: SecretResolver): + if not resolver.new: + return + async with db_conn() as conn: + for name, value in resolver.new.items(): + await db_app_secrets.insert(conn, app_name, name, value) diff --git a/shard_core/service/app_installation/util.py b/shard_core/service/app_installation/util.py index a616746d..d3f4536f 100644 --- a/shard_core/service/app_installation/util.py +++ b/shard_core/service/app_installation/util.py @@ -13,6 +13,10 @@ from shard_core.data_model.app_meta import Status, InstalledApp from shard_core.data_model.identity import Identity, SafeIdentity from shard_core.service.app_installation.exceptions import AppInIllegalStatus +from shard_core.service.app_installation.app_secrets import ( + load_secret_resolver, + persist_new_secrets, +) from shard_core.service.app_tools import get_installed_apps_path, get_app_metadata from shard_core.settings import settings from shard_core.service.traefik_dynamic_config import AppInfo, compile_config @@ -102,12 +106,13 @@ async def render_docker_compose_template(app: InstalledApp): app_dir = get_installed_apps_path() / app.name template = jinja2.Template((app_dir / "docker-compose.yml.template").read_text()) - (app_dir / "docker-compose.yml").write_text( - template.render( - fs=fs, - portal=portal, - ) - ) + + secret_resolver = await load_secret_resolver(app.name) + rendered = template.render(fs=fs, portal=portal, secret=secret_resolver) + # Persist before writing the file so a persistence failure aborts the render + # instead of leaving a compose file with secrets that were never stored. + await persist_new_secrets(app.name, secret_resolver) + (app_dir / "docker-compose.yml").write_text(rendered) async def write_traefik_dyn_config(): diff --git a/tests/test_app_secrets.py b/tests/test_app_secrets.py new file mode 100644 index 00000000..f7ba4964 --- /dev/null +++ b/tests/test_app_secrets.py @@ -0,0 +1,218 @@ +import shutil +import string + +import pytest +import yaml + +from shard_core.data_model.app_meta import InstallationReason, InstalledApp, Status +from shard_core.database import app_secrets as db_app_secrets +from shard_core.database import installed_apps as db_installed_apps +from shard_core.database.connection import db_conn +from shard_core.service import identity +from shard_core.service.app_installation.app_secrets import generate_secret +from shard_core.service.app_installation.util import render_docker_compose_template +from shard_core.service.app_installation.worker import _uninstall_app +from shard_core.service.app_tools import get_installed_apps_path + +pytestmark = pytest.mark.asyncio + +_SECRET_CHARS = set(string.ascii_letters + string.digits) + + +async def _install_app_with_template(app_name: str, template: str) -> InstalledApp: + app = InstalledApp( + name=app_name, + installation_reason=InstallationReason.CUSTOM, + status=Status.STOPPED, + ) + async with db_conn() as conn: + await db_installed_apps.insert(conn, app.model_dump()) + app_dir = get_installed_apps_path() / app_name + app_dir.mkdir(parents=True, exist_ok=True) + (app_dir / "docker-compose.yml.template").write_text(template) + return app + + +def _rendered_env(app_name: str) -> dict: + rendered = (get_installed_apps_path() / app_name / "docker-compose.yml").read_text() + return yaml.safe_load(rendered)["services"]["app"]["environment"] + + +@pytest.fixture(autouse=True) +async def default_identity(db): + await identity.init_default_identity() + + +_TEMPLATE = """ +services: + app: + image: nginx:alpine + environment: + PASSWORD: "{{ secret('db_password') }}" +""" + + +async def test_secret_generated_persisted_and_injected(): + app = await _install_app_with_template("secret_app", _TEMPLATE) + + await render_docker_compose_template(app) + + injected = _rendered_env("secret_app")["PASSWORD"] + assert len(injected) == 32 + assert set(injected) <= _SECRET_CHARS + + async with db_conn() as conn: + stored = await db_app_secrets.get_all_for_app(conn, "secret_app") + assert stored == {"db_password": injected} + + +async def test_secret_reused_across_renders(): + app = await _install_app_with_template("reuse_app", _TEMPLATE) + + await render_docker_compose_template(app) + first = _rendered_env("reuse_app")["PASSWORD"] + + await render_docker_compose_template(app) + second = _rendered_env("reuse_app")["PASSWORD"] + + assert first == second + + +async def test_secret_reused_after_reinstall(): + # Intent (issue #138): reinstall reuses the kept secret. A reinstall wipes + # the app dir (worker._reinstall_app) but leaves the DB row and app_secrets. + app = await _install_app_with_template("reinstall_app", _TEMPLATE) + await render_docker_compose_template(app) + first = _rendered_env("reinstall_app")["PASSWORD"] + + app_dir = get_installed_apps_path() / "reinstall_app" + shutil.rmtree(app_dir) + app_dir.mkdir(parents=True) + (app_dir / "docker-compose.yml.template").write_text(_TEMPLATE) + + await render_docker_compose_template(app) + assert _rendered_env("reinstall_app")["PASSWORD"] == first + + +async def test_distinct_names_get_distinct_secrets(): + template = """ +services: + app: + image: nginx:alpine + environment: + A: "{{ secret('one') }}" + B: "{{ secret('two') }}" + A_AGAIN: "{{ secret('one') }}" +""" + app = await _install_app_with_template("multi_app", template) + + await render_docker_compose_template(app) + env = _rendered_env("multi_app") + + assert env["A"] == env["A_AGAIN"] + assert env["A"] != env["B"] + + async with db_conn() as conn: + stored = await db_app_secrets.get_all_for_app(conn, "multi_app") + assert stored == {"one": env["A"], "two": env["B"]} + + +async def test_secrets_kept_on_uninstall(mocker): + # Intent (issue #138): keep secrets on uninstall so reinstalling the same app + # reuses them, matching its retained user_data. + mocker.patch( + "shard_core.service.app_installation.worker.docker_stop_app", + mocker.AsyncMock(), + ) + mocker.patch( + "shard_core.service.app_installation.worker.docker_shutdown_app", + mocker.AsyncMock(), + ) + + app = await _install_app_with_template("kept_app", _TEMPLATE) + await render_docker_compose_template(app) + async with db_conn() as conn: + before = await db_app_secrets.get_all_for_app(conn, "kept_app") + assert before + + await _uninstall_app("kept_app") + + async with db_conn() as conn: + after = await db_app_secrets.get_all_for_app(conn, "kept_app") + assert after == before + + +async def test_secrets_are_app_scoped(): + app_a = await _install_app_with_template("app_a", _TEMPLATE) + app_b = await _install_app_with_template("app_b", _TEMPLATE) + + await render_docker_compose_template(app_a) + await render_docker_compose_template(app_b) + + secret_a = _rendered_env("app_a")["PASSWORD"] + secret_b = _rendered_env("app_b")["PASSWORD"] + assert secret_a != secret_b + + async with db_conn() as conn: + assert await db_app_secrets.get_all_for_app(conn, "app_a") == { + "db_password": secret_a + } + assert await db_app_secrets.get_all_for_app(conn, "app_b") == { + "db_password": secret_b + } + + +async def test_no_secrets_persisted_for_secret_free_template(): + template = """ +services: + app: + image: nginx:alpine + volumes: + - "{{ fs.app_data }}:/data" +""" + app = await _install_app_with_template("plain_app", template) + + await render_docker_compose_template(app) + + assert (get_installed_apps_path() / "plain_app" / "docker-compose.yml").exists() + async with db_conn() as conn: + assert await db_app_secrets.get_all_for_app(conn, "plain_app") == {} + + +async def test_render_aborts_without_writing_file_when_persist_fails(mocker): + mocker.patch( + "shard_core.service.app_installation.util.persist_new_secrets", + mocker.AsyncMock(side_effect=RuntimeError("db down")), + ) + app = await _install_app_with_template("fail_app", _TEMPLATE) + + with pytest.raises(RuntimeError): + await render_docker_compose_template(app) + + assert not (get_installed_apps_path() / "fail_app" / "docker-compose.yml").exists() + + +async def test_insert_is_idempotent(): + async with db_conn() as conn: + await db_installed_apps.insert( + conn, + InstalledApp( + name="idem_app", + installation_reason=InstallationReason.CUSTOM, + status=Status.STOPPED, + ).model_dump(), + ) + await db_app_secrets.insert(conn, "idem_app", "k", "first") + await db_app_secrets.insert(conn, "idem_app", "k", "second") + stored = await db_app_secrets.get_all_for_app(conn, "idem_app") + assert stored == {"k": "first"} + + +async def test_generate_secret_length_charset_and_uniqueness(): + values = [generate_secret() for _ in range(100)] + for v in values: + assert len(v) == 32 + assert set(v) <= _SECRET_CHARS + # distinct values across 100 draws → the generator is actually random, + # not returning a constant. + assert len(set(values)) == 100 diff --git a/tests/test_db_snapshot.py b/tests/test_db_snapshot.py index b78e21a0..3ae47b66 100644 --- a/tests/test_db_snapshot.py +++ b/tests/test_db_snapshot.py @@ -13,7 +13,7 @@ _APP_TABLES = ( "identities, terminals, installed_apps, peers, backups, tours, " - "app_usage_tracks, kv_store" + "app_usage_tracks, kv_store, app_secrets" ) @@ -42,6 +42,8 @@ async def _seed_shard_state(): "INSERT INTO backups (directories, start_time, end_time) VALUES (%s, %s, %s)", (Jsonb([]), now, now), ) + await conn.execute("""INSERT INTO app_secrets (app_name, name, value) + VALUES ('filebrowser', 'db_password', 'super-secret-value')""") @pytest.mark.asyncio @@ -76,6 +78,13 @@ async def test_snapshot_roundtrip_restores_core_state(db, tmp_path): ).fetchone() assert passphrase[0] == "correct horse battery staple" + secret = await ( + await conn.execute( + "SELECT value FROM app_secrets WHERE app_name = 'filebrowser'" + ) + ).fetchone() + assert secret[0] == "super-secret-value" + # SERIAL sequence advanced past the restored row → new insert must not collide. new_id = await ( await conn.execute(