fix(apps): fail fast on invalid custom-app zip + zip-slip hardening

agents.md

@@ -96,6 +96,7 @@ Blinker-based async signals defined in `util/signals.py`. DB-writing handlers ar
 - `async_on_peer_write` — peer added/updated
 
 ### 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.)
 3. Traefik dynamic config generated for the app's subdomain routing

shard_core/service/app_installation/__init__.py

@@ -1,11 +1,14 @@
 import logging
+import shutil
+from pathlib import Path
 
 from shard_core.database import database
 from shard_core.database.connection import db_conn
 from shard_core.database import installed_apps as db_installed_apps
 from shard_core.data_model.app_meta import InstallationReason, InstalledApp, Status
 from shard_core.util import signals
 from shard_core.settings import settings
+from shard_core.service.app_tools import get_installed_apps_path
 from shard_core.util.subprocess import subprocess, SubprocessError
 from . import util, worker
 from .exceptions import AppAlreadyInstalled, AppDoesNotExist, AppNotInstalled
@@ -42,12 +45,18 @@ async def install_app_from_store(
     log.info(f"created {installation_task}")
 
 
-async def install_app_from_existing_zip(
-    name: str, installation_reason: InstallationReason = InstallationReason.CUSTOM
+async def install_app_from_uploaded_zip(
+    name: str,
+    zip_file: Path,
+    installation_reason: InstallationReason = InstallationReason.CUSTOM,
 ):
     if await util.app_exists_in_db(name):
         raise AppAlreadyInstalled(name)
 
+    target_zip = get_installed_apps_path() / name / f"{name}.zip"
+    target_zip.parent.mkdir(parents=True, exist_ok=True)
+    shutil.move(zip_file, target_zip)
+
     async with db_conn() as conn:
         installed_app = InstalledApp(
             name=name,

shard_core/service/app_installation/app_zip.py

@@ -0,0 +1,93 @@
+import json
+import logging
+import re
+import shutil
+import zipfile
+from pathlib import Path
+
+from shard_core.data_model.app_meta import AppMeta
+from .exceptions import InvalidAppZip
+
+log = logging.getLogger(__name__)
+
+REQUIRED_FILES = ("app_meta.json", "docker-compose.yml.template")
+APP_NAME_PATTERN = re.compile(r"[a-zA-Z0-9][a-zA-Z0-9_-]{0,63}")
+
+
+def validate_app_zip(zip_file: Path) -> AppMeta:
+    """Read the app metadata from an archive, rejecting anything unfit to install."""
+    try:
+        with zipfile.ZipFile(zip_file, "r") as zip_ref:
+            names = _member_names(zip_ref)
+            root = _find_root(names)
+            for required in REQUIRED_FILES:
+                if root + required not in names:
+                    raise InvalidAppZip(f"{required} is missing from the archive")
+            app_meta = _parse_app_meta(zip_ref.read(root + "app_meta.json"))
+    except zipfile.BadZipFile as e:
+        raise InvalidAppZip(f"not a valid zip archive: {e}")
+
+    if not APP_NAME_PATTERN.fullmatch(app_meta.name):
+        raise InvalidAppZip(
+            f"app name {app_meta.name!r} in app_meta.json is not a valid app name"
+        )
+    return app_meta
+
+
+def extract_app_zip(zip_file: Path, target_dir: Path):
+    """Extract an app archive, stripping a single top-level directory if present."""
+    target_root = target_dir.resolve()
+    with zipfile.ZipFile(zip_file, "r") as zip_ref:
+        names = _member_names(zip_ref)
+        root = _find_root(names)
+        for name in names:
+            relative = name[len(root) :]
+            target = (target_dir / relative).resolve()
+            if not relative or target_root not in target.parents:
+                raise InvalidAppZip(f"archive member {name!r} has an illegal path")
+            target.parent.mkdir(parents=True, exist_ok=True)
+            with zip_ref.open(name) as source, open(target, "wb") as f:
+                shutil.copyfileobj(source, f)
+
+
+def _member_names(zip_ref: zipfile.ZipFile) -> list[str]:
+    names = [
+        i.filename
+        for i in zip_ref.infolist()
+        if not i.is_dir() and not _is_archiver_cruft(i.filename)
+    ]
+    if not names:
+        raise InvalidAppZip("the archive is empty")
+    for name in names:
+        parts = Path(name).parts
+        if name.startswith("/") or ".." in parts or Path(name).is_absolute():
+            raise InvalidAppZip(f"archive member {name!r} has an illegal path")
+    return names
+
+
+def _is_archiver_cruft(name: str) -> bool:
+    # macOS "Compress" adds these next to the compressed folder — without ignoring
+    # them, such an archive looks like it has two top-level directories
+    return name.startswith("__MACOSX/") or Path(name).name == ".DS_Store"
+
+
+def _find_root(names: list[str]) -> str:
+    """Return the prefix the app files live under: "" or "<single top-level dir>/"."""
+    if "app_meta.json" in names:
+        return ""
+    top_level = {name.split("/", 1)[0] for name in names}
+    if len(top_level) == 1:
+        root = top_level.pop() + "/"
+        if root + "app_meta.json" in names:
+            return root
+    raise InvalidAppZip(
+        "app_meta.json is missing from the archive root; the app files must be at "
+        "the top level of the zip, or inside a single directory"
+    )
+
+
+def _parse_app_meta(raw: bytes) -> AppMeta:
+    try:
+        return AppMeta.model_validate(json.loads(raw))
+    except Exception as e:
+        raise InvalidAppZip(f"app_meta.json is invalid: {e}")

shard_core/service/app_installation/exceptions.py

@@ -12,3 +12,7 @@ class AppNotInstalled(Exception):
 
 class AppInIllegalStatus(Exception):
     pass
+
+
+class InvalidAppZip(Exception):
+    pass

shard_core/service/app_installation/worker.py

@@ -1,7 +1,6 @@
 import asyncio
 import logging
 import shutil
-import zipfile
 from contextlib import suppress
 from pathlib import Path
 from typing import Literal
@@ -21,6 +20,7 @@
 )
 from shard_core.settings import settings
 from shard_core.util import signals
+from .app_zip import extract_app_zip
 from .exceptions import AppDoesNotExist
 from .util import (
     update_app_status,
@@ -174,8 +174,7 @@ async def _reinstall_app(app_name: str):
 
 
 async def _install_app_from_zip(installed_app, zip_file):
-    with zipfile.ZipFile(zip_file, "r") as zip_ref:
-        zip_ref.extractall(zip_file.parent)
+    extract_app_zip(zip_file, zip_file.parent)
     await signals.on_apps_update.send_async()
     zip_file.unlink()
 

shard_core/web/protected/apps.py

@@ -1,6 +1,8 @@
 import io
 import logging
 import mimetypes
+import tempfile
+from pathlib import Path
 from typing import List
 
 import aiofiles
@@ -11,9 +13,11 @@
 from shard_core.database import installed_apps as db_installed_apps
 from shard_core.data_model.app_meta import InstalledAppWithMeta, InstalledApp
 from shard_core.service import app_installation
+from shard_core.service.app_installation.app_zip import validate_app_zip
 from shard_core.service.app_installation.exceptions import (
     AppAlreadyInstalled,
     AppNotInstalled,
+    InvalidAppZip,
 )
 from shard_core.service.app_tools import (
     get_installed_apps_path,
@@ -112,17 +116,26 @@ async def install_custom_app(file: UploadFile):
             detail="Only zip files are supported",
         )
 
-    file_path = get_installed_apps_path() / file.filename[:-4] / file.filename
-    file_path.parent.mkdir(parents=True, exist_ok=True)
-
-    async with aiofiles.open(file_path, "wb") as f:
-        while chunk := await file.read(1024):
-            await f.write(chunk)
-
-    try:
-        await app_installation.install_app_from_existing_zip(file.filename[:-4])
-    except app_installation.exceptions.AppAlreadyInstalled:
-        raise HTTPException(
-            status_code=status.HTTP_409_CONFLICT,
-            detail=f"App {file.filename[:-4]} is already installed",
-        )
+    with tempfile.TemporaryDirectory() as tmp_dir:
+        upload_path = Path(tmp_dir) / "upload.zip"
+        async with aiofiles.open(upload_path, "wb") as f:
+            while chunk := await file.read(64 * 1024):
+                await f.write(chunk)
+
+        try:
+            app_meta = validate_app_zip(upload_path)
+        except InvalidAppZip as e:
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail=f"The uploaded app is invalid: {e}",
+            )
+
+        try:
+            await app_installation.install_app_from_uploaded_zip(
+                app_meta.name, upload_path
+            )
+        except AppAlreadyInstalled:
+            raise HTTPException(
+                status_code=status.HTTP_409_CONFLICT,
+                detail=f"App {app_meta.name} is already installed",
+            )

tests/test_app_installation.py

@@ -1,9 +1,13 @@
+import io
+import zipfile
+
 import docker
 import pytest
 from docker.errors import NotFound
 from fastapi import status
 from httpx import AsyncClient
 
+from shard_core.service.app_tools import get_installed_apps_path
 from tests.util import (
     wait_until_app_installed,
     mock_app_store_path,
@@ -112,16 +116,33 @@ async def test_uninstall_running_app(api_client: AsyncClient):
         docker_client.containers.get(app_name)
 
 
+async def _upload_custom_app(api_client: AsyncClient, filename: str, content: bytes):
+    return await api_client.post(
+        "protected/apps",
+        files={"file": (filename, content, "application/zip")},
+    )
+
+
+def _mock_app_zip_bytes() -> bytes:
+    return (mock_app_store_path() / "mock_app" / "mock_app.zip").read_bytes()
+
+
+def _nested_mock_app_zip_bytes() -> bytes:
+    """The mock_app zip, but with all files inside a single top-level directory."""
+    buffer = io.BytesIO()
+    with zipfile.ZipFile(io.BytesIO(_mock_app_zip_bytes())) as source:
+        with zipfile.ZipFile(buffer, "w") as target:
+            for name in source.namelist():
+                target.writestr(f"mock_app/{name}", source.read(name))
+    return buffer.getvalue()
+
+
 async def test_install_custom_app(api_client: AsyncClient):
     app_name = "mock_app"
     docker_client = docker.from_env()
 
-    app_zip = mock_app_store_path() / app_name / f"{app_name}.zip"
-    with open(app_zip, "rb") as f:
-        content = f.read()
-    response = await api_client.post(
-        "protected/apps",
-        files={"file": (f"{app_name}.zip", content, "application/zip")},
+    response = await _upload_custom_app(
+        api_client, f"{app_name}.zip", _mock_app_zip_bytes()
     )
     assert response.status_code == status.HTTP_201_CREATED
 
@@ -131,3 +152,52 @@ async def test_install_custom_app(api_client: AsyncClient):
 
     response = (await api_client.get("protected/apps")).json()
     assert len(response) == 4
+
+
+async def test_install_custom_app_uses_name_from_app_meta(api_client: AsyncClient):
+    response = await _upload_custom_app(
+        api_client, "some-unrelated-filename.zip", _mock_app_zip_bytes()
+    )
+    assert response.status_code == status.HTTP_201_CREATED
+
+    await wait_until_app_installed(api_client, "mock_app")
+
+    response = await api_client.get("protected/apps/some-unrelated-filename")
+    assert response.status_code == status.HTTP_404_NOT_FOUND
+
+
+async def test_install_custom_app_with_single_top_level_dir(api_client: AsyncClient):
+    app_name = "mock_app"
+    docker_client = docker.from_env()
+
+    response = await _upload_custom_app(
+        api_client, f"{app_name}.zip", _nested_mock_app_zip_bytes()
+    )
+    assert response.status_code == status.HTTP_201_CREATED
+
+    await wait_until_app_installed(api_client, app_name)
+
+    docker_client.containers.get(app_name)
+    app = (await api_client.get(f"protected/apps/{app_name}")).json()
+    assert app["meta"]["name"] == app_name
+
+
+async def test_install_custom_app_without_app_meta(api_client: AsyncClient):
+    buffer = io.BytesIO()
+    with zipfile.ZipFile(buffer, "w") as zip_ref:
+        zip_ref.writestr("docker-compose.yml.template", "services: {}")
+
+    response = await _upload_custom_app(api_client, "mock_app.zip", buffer.getvalue())
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+    assert "app_meta.json is missing" in response.json()["detail"]
+
+    assert len((await api_client.get("protected/apps")).json()) == 3
+    assert not (get_installed_apps_path() / "mock_app").exists()
+
+
+async def test_install_custom_app_that_is_not_a_zip(api_client: AsyncClient):
+    response = await _upload_custom_app(api_client, "mock_app.zip", b"not a zip file")
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+
+    assert len((await api_client.get("protected/apps")).json()) == 3
+    assert not (get_installed_apps_path() / "mock_app").exists()