Security hardening (maziggy/bambuddy-security #35)

backend/app/api/routes/projects.py

@@ -15,7 +15,13 @@ from sqlalchemy import and_, case, func, or_, select, update
 from sqlalchemy.ext.asyncio import AsyncSession
 from sqlalchemy.orm import selectinload
 
-from backend.app.api.routes.library import get_library_dir
+from backend.app.api.routes.library import (
+    classify_file_type,
+    get_library_dir,
+    get_library_files_dir,
+    to_absolute_path,
+    to_relative_path,
+)
 from backend.app.core.auth import (
     ApiKeyActor,
     RequestActor,
@@ -51,8 +57,9 @@ from backend.app.schemas.project import (
     ProjectUpdate,
     TimelineEvent,
 )
+from backend.app.services.library_folder_access import load_folder_index, visible_folder_ids
 from backend.app.utils.http import build_content_disposition
-from backend.app.utils.safe_path import safe_join_under
+from backend.app.utils.safe_path import PathTraversalError, safe_join_under
 
 logger = logging.getLogger(__name__)
 
@@ -1983,12 +1990,33 @@ async def get_project_timeline(
 # ============ Phase 10: Import/Export Endpoints ============
 
 
+def _stored_library_path(stored: str | None) -> Path | None:
+    """Where a library file's stored path points.
+
+    Paths are relative to the data directory, except those of files imported
+    by older versions, which are relative to the library directory.
+    """
+    try:
+        path = to_absolute_path(stored)
+    except ValueError:
+        return None
+    if path is not None and not path.exists() and stored and not Path(stored).is_absolute():
+        try:
+            legacy = safe_join_under(get_library_dir(), *Path(stored).parts, http=False)
+        except PathTraversalError:
+            return path
+        if legacy.exists():
+            return legacy
+    return path
+
+
 @router.get("/{project_id}/export")
 async def export_project(
     project_id: int,
     format: str = "zip",  # "zip" (with files) or "json" (metadata only)
     db: AsyncSession = Depends(get_db),
     _: User | None = RequirePermissionIfAuthEnabled(Permission.PROJECTS_READ),
+    actor: User | ApiKeyActor | None = RequestActor,
 ):
     """Export a project. Use format=zip (default) for full export with files, or format=json for metadata only."""
     result = await db.execute(select(Project).where(Project.id == project_id))
@@ -2022,13 +2050,23 @@ async def export_project(
     )
     linked_folders = folders_result.scalars().all()
 
+    # The folders and files the caller could open in the library, and no more.
+    reads_all_files = actor is None or actor.has_permission(Permission.LIBRARY_READ_ALL.value)
+    if actor is not None and not reads_all_files:
+        if actor.has_permission(Permission.LIBRARY_READ_OWN.value):
+            visible = visible_folder_ids(await load_folder_index(db), actor)
+            linked_folders = [f for f in linked_folders if visible is None or f.id in visible]
+        else:
+            linked_folders = []
+
     folders_export = []
     files_to_include = []  # (archive_path, zip_path)
 
     for folder in linked_folders:
         # Get files in this folder
+        own_files = [] if actor is None or reads_all_files else [LibraryFile.created_by_id == actor.id]
         files_result = await db.execute(
-            LibraryFile.active().where(LibraryFile.folder_id == folder.id).order_by(LibraryFile.filename)
+            LibraryFile.active().where(LibraryFile.folder_id == folder.id, *own_files).order_by(LibraryFile.filename)
         )
         files = files_result.scalars().all()
 
@@ -2042,15 +2080,14 @@ async def export_project(
                 }
             )
             # Add file to include in ZIP
-            library_dir = get_library_dir()
-            file_path = library_dir / f.file_path
-            if file_path.exists():
+            file_path = _stored_library_path(f.file_path)
+            if file_path is not None and file_path.exists():
                 zip_path = f"files/{folder.name}/{f.filename}"
                 files_to_include.append((file_path, zip_path))
                 # Also include thumbnail if exists
                 if f.thumbnail_path:
-                    thumb_path = library_dir / f.thumbnail_path
-                    if thumb_path.exists():
+                    thumb_path = _stored_library_path(f.thumbnail_path)
+                    if thumb_path is not None and thumb_path.exists():
                         thumb_zip_path = f"files/{folder.name}/.thumbnails/{f.filename}.png"
                         files_to_include.append((thumb_path, thumb_zip_path))
 
@@ -2106,11 +2143,50 @@ async def export_project(
     )
 
 
+async def _import_folder(
+    db: AsyncSession, name: str, project_id: int, actor: User | ApiKeyActor | None
+) -> LibraryFolder:
+    """The top-level folder an imported project links, by name.
+
+    An existing one is taken only when the caller could link it themselves,
+    which takes library:update_all. Otherwise a new folder of that name is
+    made, shared like every project folder (#3201).
+    """
+    candidates = (
+        await db.execute(
+            select(LibraryFolder)
+            .where(
+                LibraryFolder.name == name,
+                LibraryFolder.parent_id.is_(None),
+                LibraryFolder.is_external.is_(False),
+            )
+            .order_by(LibraryFolder.id)
+        )
+    ).scalars()
+    for folder in candidates:
+        if actor is None or actor.has_permission(Permission.LIBRARY_UPDATE_ALL.value):
+            folder.project_id = project_id
+            return folder
+    folder = LibraryFolder(
+        name=name,
+        project_id=project_id,
+        is_external=False,
+        external_readonly=False,
+        external_show_hidden=False,
+        created_by_id=actor.id if actor else None,
+        shared=True,
+    )
+    db.add(folder)
+    await db.flush()
+    return folder
+
+
 @router.post("/import", response_model=ProjectResponse)
 async def import_project(
     data: ProjectImport,
     db: AsyncSession = Depends(get_db),
-    current_user: User | None = RequirePermissionIfAuthEnabled(Permission.PROJECTS_CREATE),
+    _: User | None = RequirePermissionIfAuthEnabled(Permission.PROJECTS_CREATE),
+    actor: User | ApiKeyActor | None = RequestActor,
 ):
     """Import a project with optional BOM items and linked folders."""
     # Create the project
@@ -2146,33 +2222,9 @@ async def import_project(
         )
         db.add(bom_item)
 
-    # Create linked folders in library
+    # Link a folder in the library for each
     for folder_data in data.linked_folders:
-        # Check if folder with this name already exists at root level
-        existing_result = await db.execute(
-            select(LibraryFolder).where(
-                LibraryFolder.name == folder_data.name,
-                LibraryFolder.parent_id.is_(None),
-            )
-        )
-        existing_folder = existing_result.scalar_one_or_none()
-
-        if existing_folder:
-            # Link existing folder to project
-            existing_folder.project_id = project.id
-        else:
-            # Create new folder linked to project
-            # A project's folder is shared, like every linked folder (#3201).
-            new_folder = LibraryFolder(
-                name=folder_data.name,
-                project_id=project.id,
-                is_external=False,
-                external_readonly=False,
-                external_show_hidden=False,
-                created_by_id=current_user.id if current_user else None,
-                shared=True,
-            )
-            db.add(new_folder)
+        await _import_folder(db, folder_data.name, project.id, actor)
 
     await db.flush()
     await db.refresh(project)
@@ -2211,7 +2263,8 @@ async def import_project(
 async def import_project_file(
     file: UploadFile = File(...),
     db: AsyncSession = Depends(get_db),
-    current_user: User | None = RequirePermissionIfAuthEnabled(Permission.PROJECTS_CREATE),
+    _: User | None = RequirePermissionIfAuthEnabled(Permission.PROJECTS_CREATE),
+    actor: User | ApiKeyActor | None = RequestActor,
 ):
     """Import a project from a ZIP or JSON file."""
     if not file.filename:
@@ -2288,38 +2341,9 @@ async def import_project_file(
         # previous code did ``library_dir / folder_name`` directly, which
         # collapses to ``Path(folder_name)`` when folder_name is absolute
         # and lets ``..`` escape after mkdir.
-        folder_path = safe_join_under(library_dir, folder_name)
-
-        # Check if folder exists
-        existing_result = await db.execute(
-            select(LibraryFolder).where(
-                LibraryFolder.name == folder_name,
-                LibraryFolder.parent_id.is_(None),
-            )
-        )
-        existing_folder = existing_result.scalar_one_or_none()
+        safe_join_under(library_dir, folder_name)
 
-        if existing_folder:
-            # Link existing folder to project
-            existing_folder.project_id = project.id
-            folder = existing_folder
-        else:
-            # Create new folder
-            # A project's folder is shared, like every linked folder (#3201).
-            folder = LibraryFolder(
-                name=folder_name,
-                project_id=project.id,
-                is_external=False,
-                external_readonly=False,
-                external_show_hidden=False,
-                created_by_id=current_user.id if current_user else None,
-                shared=True,
-            )
-            db.add(folder)
-            await db.flush()
-
-            # Create folder on disk
-            folder_path.mkdir(parents=True, exist_ok=True)
+        folder = await _import_folder(db, folder_name, project.id, actor)
 
         # Import files for this folder from ZIP
         folder_prefix = f"files/{folder_name}/"
@@ -2340,33 +2364,24 @@ async def import_project_file(
             # parts because ``safe_join_under`` rejects parts that start
             # with ``/``, and a single combined string would hide an
             # embedded ``..`` segment behind a forward slash.
-            file_disk_path = safe_join_under(
-                library_dir,
-                folder_name,
-                *Path(relative_path).parts,
+            safe_join_under(library_dir, folder_name, *Path(relative_path).parts)
+            # Stored under a name of its own, as an upload is: another folder
+            # of the same name must not share (and overwrite) its files.
+            suffix = (
+                ".gcode.3mf" if relative_path.lower().endswith(".gcode.3mf") else Path(relative_path).suffix.lower()
             )
-            file_disk_path.parent.mkdir(parents=True, exist_ok=True)
+            file_disk_path = get_library_files_dir() / f"{uuid.uuid4().hex}{suffix}"
             file_disk_path.write_bytes(file_content)
 
-            # Determine file type
-            ext = Path(relative_path).suffix.lower()
-            if ext in [".stl", ".3mf", ".obj"]:
-                file_type = "model"
-            elif ext in [".gcode"]:
-                file_type = "gcode"
-            elif ext in [".jpg", ".jpeg", ".png", ".gif", ".webp"]:
-                file_type = "image"
-            else:
-                file_type = "other"
-
             # Create library file record
             lib_file = LibraryFile(
                 folder_id=folder.id,
                 filename=relative_path,
-                file_path=f"{folder_name}/{relative_path}",
-                file_type=file_type,
+                file_path=to_relative_path(file_disk_path),
+                file_type=classify_file_type(relative_path, file_disk_path),
                 file_size=len(file_content),
                 is_external=False,
+                created_by_id=actor.id if actor else None,
             )
             db.add(lib_file)
 

backend/app/services/library_folder_access.py

@@ -23,6 +23,7 @@ owner or the shared flag from the upgrade backfill in ``core/database.py``.
 from __future__ import annotations
 
 from dataclasses import dataclass, field
+from typing import TYPE_CHECKING
 
 from fastapi import HTTPException
 from sqlalchemy import select
@@ -32,13 +33,16 @@ from backend.app.core.permissions import Permission
 from backend.app.models.library import LibraryFile, LibraryFolder
 from backend.app.models.user import User
 
+if TYPE_CHECKING:
+    from backend.app.core.auth import ApiKeyActor
 
-def sees_all_folders(user: User | None) -> bool:
+
+def sees_all_folders(user: User | ApiKeyActor | None) -> bool:
     """True for ``library:read_all``, and for ``None`` (auth off)."""
     return user is None or user.has_permission(Permission.LIBRARY_READ_ALL.value)
 
 
-def owns(folder: LibraryFolder, user: User | None) -> bool:
+def owns(folder: LibraryFolder, user: User | ApiKeyActor | None) -> bool:
     return user is not None and folder.created_by_id is not None and folder.created_by_id == user.id
 
 
@@ -105,7 +109,7 @@ async def load_folder_index(db: AsyncSession, *, with_files: bool = True) -> Fol
     return index
 
 
-def visible_folder_ids(index: FolderIndex, user: User | None) -> set[int] | None:
+def visible_folder_ids(index: FolderIndex, user: User | ApiKeyActor | None) -> set[int] | None:
     """The folders ``user`` may see, or ``None`` for all of them."""
     if sees_all_folders(user):
         return None

backend/tests/integration/test_project_import_folders.py

@@ -0,0 +1,214 @@
+"""Project import and export: linked folders and their files."""
+
+from __future__ import annotations
+
+import io
+import json
+import zipfile
+
+import pytest
+from httpx import AsyncClient
+
+pytestmark = [pytest.mark.asyncio, pytest.mark.integration]
+
+STL = b"solid probe\nendsolid probe\n"
+
+
+def _zip(folder: str, files: dict[str, bytes]) -> bytes:
+    buffer = io.BytesIO()
+    with zipfile.ZipFile(buffer, "w") as zf:
+        zf.writestr("project.json", json.dumps({"name": "P", "linked_folders": [{"name": folder}]}))
+        for name, content in files.items():
+            zf.writestr(f"files/{folder}/{name}", content)
+    return buffer.getvalue()
+
+
+async def _import_zip(client: AsyncClient, data: bytes, headers=None):
+    files = {"file": ("p.zip", io.BytesIO(data), "application/zip")}
+    return await client.post("/api/v1/projects/import/file", files=files, headers=headers or {})
+
+
+async def _folder_files(client: AsyncClient, folder_id: int, headers=None) -> list[dict]:
+    listed = await client.get("/api/v1/library/files", params={"folder_id": folder_id}, headers=headers or {})
+    return listed.json()
+
+
+async def test_an_imported_file_can_be_downloaded(async_client: AsyncClient):
+    imported = await _import_zip(async_client, _zip("Models", {"part.stl": STL}))
+    assert imported.status_code == 200, imported.text
+    folders = (await async_client.get("/api/v1/library/folders/by-project/" + str(imported.json()["id"]))).json()
+
+    files = await _folder_files(async_client, folders[0]["id"])
+    downloaded = await async_client.get(f"/api/v1/library/files/{files[0]['id']}/download")
+
+    assert [f["filename"] for f in files] == ["part.stl"]
+    assert (downloaded.status_code, downloaded.content) == (200, STL)
+
+
+async def test_an_export_carries_the_folder_files(async_client: AsyncClient):
+    project = (await async_client.post("/api/v1/projects/", json={"name": "P"})).json()
+    folder = (await async_client.post("/api/v1/library/folders/", json={"name": "Parts"})).json()
+    await async_client.put(f"/api/v1/library/folders/{folder['id']}", json={"project_id": project["id"]})
+    uploaded = await async_client.post(
+        "/api/v1/library/files",
+        params={"folder_id": folder["id"], "generate_stl_thumbnails": False},
+        files={"file": ("bracket.stl", io.BytesIO(STL), "model/stl")},
+    )
+    assert uploaded.status_code == 200, uploaded.text
+
+    exported = await async_client.get(f"/api/v1/projects/{project['id']}/export")
+
+    with zipfile.ZipFile(io.BytesIO(exported.content)) as zf:
+        assert zf.read("files/Parts/bracket.stl") == STL
+
+
+PERMS = ["projects:read", "projects:create", "library:read_own", "library:upload", "library:update_own"]
+
+
+async def _users(client: AsyncClient):
+    from backend.tests.integration.test_printer_scope_1727 import _admin_token, _auth, _group, _user
+
+    token = await _admin_token(client)
+    group = await _group(client, token, "makers", permissions=PERMS)
+    a = _auth((await _user(client, token, "anna", [group]))[0])
+    b = _auth((await _user(client, token, "ben", [group]))[0])
+    return _auth(token), a, b
+
+
+async def test_an_import_leaves_another_user_folder_of_the_same_name_alone(async_client: AsyncClient):
+    admin, a, b = await _users(async_client)
+    bens = (await async_client.post("/api/v1/library/folders/", headers=b, json={"name": "Models"})).json()
+
+    from_zip = await _import_zip(async_client, _zip("Models", {"part.stl": STL}), headers=a)
+    from_json = await async_client.post(
+        "/api/v1/projects/import", headers=a, json={"name": "J", "linked_folders": [{"name": "Models"}]}
+    )
+
+    assert (from_zip.status_code, from_json.status_code) == (200, 200)
+    kept = (await async_client.get(f"/api/v1/library/folders/{bens['id']}", headers=admin)).json()
+    assert kept["project_id"] is None
+    assert await _folder_files(async_client, bens["id"], headers=admin) == []
+    linked = (
+        await async_client.get(f"/api/v1/library/folders/by-project/{from_json.json()['id']}", headers=admin)
+    ).json()
+    assert [f["name"] for f in linked] == ["Models"]
+    assert linked[0]["id"] != bens["id"]
+
+
+async def test_an_import_links_an_existing_folder_only_with_library_update_all(async_client: AsyncClient):
+    admin, a, b = await _users(async_client)
+    annas = (await async_client.post("/api/v1/library/folders/", headers=a, json={"name": "Mine"})).json()
+    bens = (await async_client.post("/api/v1/library/folders/", headers=b, json={"name": "Shop"})).json()
+    await async_client.post("/api/v1/library/folders/", headers=a, json={"name": "Shop"})
+
+    own = await async_client.post(
+        "/api/v1/projects/import", headers=a, json={"name": "A", "linked_folders": [{"name": "Mine"}]}
+    )
+    as_admin = await async_client.post(
+        "/api/v1/projects/import", headers=admin, json={"name": "B", "linked_folders": [{"name": "Shop"}]}
+    )
+
+    assert (own.status_code, as_admin.status_code) == (200, 200)
+    mine = (await async_client.get(f"/api/v1/library/folders/{annas['id']}", headers=admin)).json()
+    shop = (await async_client.get(f"/api/v1/library/folders/{bens['id']}", headers=admin)).json()
+    # Linking a folder takes library:update_all, her own included.
+    assert mine["project_id"] is None
+    assert shop["project_id"] == as_admin.json()["id"]
+
+
+async def test_two_imports_into_same_named_folders_keep_their_own_files(async_client: AsyncClient):
+    admin, a, b = await _users(async_client)
+    first = await _import_zip(async_client, _zip("Models", {"part.stl": b"solid a\nendsolid a\n"}), headers=a)
+    second = await _import_zip(async_client, _zip("Models", {"part.stl": b"solid b\nendsolid b\n"}), headers=b)
+
+    contents = []
+    for imported, who in ((first, a), (second, b)):
+        folder = (
+            await async_client.get(f"/api/v1/library/folders/by-project/{imported.json()['id']}", headers=admin)
+        ).json()[0]
+        file = (await _folder_files(async_client, folder["id"], headers=who))[0]
+        contents.append((await async_client.get(f"/api/v1/library/files/{file['id']}/download", headers=who)).content)
+
+    assert contents == [b"solid a\nendsolid a\n", b"solid b\nendsolid b\n"]
+
+
+async def test_an_export_still_carries_files_imported_by_older_versions(async_client: AsyncClient, db_session):
+    from backend.app.api.routes.library import get_library_dir
+    from backend.app.models.library import LibraryFile
+
+    project = (await async_client.post("/api/v1/projects/", json={"name": "Old"})).json()
+    folder = (await async_client.post("/api/v1/library/folders/", json={"name": "OldImport"})).json()
+    await async_client.put(f"/api/v1/library/folders/{folder['id']}", json={"project_id": project["id"]})
+    disk = get_library_dir() / "OldImport" / "legacy.stl"
+    disk.parent.mkdir(parents=True, exist_ok=True)
+    disk.write_bytes(STL)
+    db_session.add(
+        LibraryFile(
+            folder_id=folder["id"],
+            filename="legacy.stl",
+            file_path="OldImport/legacy.stl",
+            file_type="model",
+            file_size=1,
+        )
+    )
+    await db_session.commit()
+    try:
+        exported = await async_client.get(f"/api/v1/projects/{project['id']}/export")
+        with zipfile.ZipFile(io.BytesIO(exported.content)) as zf:
+            assert zf.read("files/OldImport/legacy.stl") == STL
+    finally:
+        disk.unlink()
+        disk.parent.rmdir()
+
+
+async def test_an_export_carries_only_files_the_caller_may_open(async_client: AsyncClient):
+    from backend.tests.integration.test_printer_scope_1727 import _admin_token, _auth, _group, _user
+
+    token = await _admin_token(async_client)
+    admin = _auth(token)
+    own = await _group(async_client, token, "own", permissions=["projects:read", "library:read_own", "library:upload"])
+    none = await _group(async_client, token, "none", permissions=["projects:read"])
+    reader = _auth((await _user(async_client, token, "reader", [own]))[0])
+    outsider = _auth((await _user(async_client, token, "outsider", [none]))[0])
+    project = (await async_client.post("/api/v1/projects/", headers=admin, json={"name": "P"})).json()
+    folder = (await async_client.post("/api/v1/library/folders/", headers=admin, json={"name": "Private"})).json()
+    await async_client.put(f"/api/v1/library/folders/{folder['id']}", headers=admin, json={"project_id": project["id"]})
+    shared = (await async_client.post("/api/v1/library/folders/", headers=admin, json={"name": "Shared"})).json()
+    await async_client.put(
+        f"/api/v1/library/folders/{shared['id']}", headers=admin, json={"project_id": project["id"], "shared": True}
+    )
+    for who, folder_id, name in (
+        (admin, folder["id"], "secret.stl"),
+        (admin, shared["id"], "theirs.stl"),
+        (reader, shared["id"], "mine.stl"),
+    ):
+        uploaded = await async_client.post(
+            "/api/v1/library/files",
+            headers=who,
+            params={"folder_id": folder_id, "generate_stl_thumbnails": False},
+            files={"file": (name, io.BytesIO(STL), "model/stl")},
+        )
+        assert uploaded.status_code == 200, uploaded.text
+
+    def contents(response) -> tuple[list[str], list[str]]:
+        with zipfile.ZipFile(io.BytesIO(response.content)) as zf:
+            folders = [f["name"] for f in json.loads(zf.read("project.json"))["linked_folders"]]
+            return folders, [n for n in zf.namelist() if n.startswith("files/")]
+
+    exported = {
+        name: contents(await async_client.get(f"/api/v1/projects/{project['id']}/export", headers=who))
+        for name, who in (("admin", admin), ("reader", reader), ("outsider", outsider))
+    }
+
+    assert exported["admin"] == (
+        ["Private", "Shared"],
+        ["files/Private/secret.stl", "files/Shared/mine.stl", "files/Shared/theirs.stl"],
+    )
+    assert exported["reader"] == (["Shared"], ["files/Shared/mine.stl"])
+    as_json = await async_client.get(
+        f"/api/v1/projects/{project['id']}/export", headers=reader, params={"format": "json"}
+    )
+    assert [(f["name"], [x["filename"] for x in f["files"]]) for f in as_json.json()["linked_folders"]] == [
+        ("Shared", ["mine.stl"])
+    ]
+    assert exported["outsider"] == ([], [])