Просмотр исходного кода

fix(pending-uploads): check the project before archiving an upload into it

Archiving a pending upload stored the requested project id on the archive
and the pending row without checking it, and only after the file had been
archived. An unknown project failed with a foreign key error (500) on
PostgreSQL with the archive files already written, and was stored as a
broken link on SQLite. 0 skipped the archive but was stored on the pending
row.

The project is now checked first: an unknown one is refused with 400
"Project not found" and the upload stays pending. 0 means no project, and
the archive and the pending row get the same id.
maziggy 1 неделя назад
Родитель
Сommit
a399ef0361

+ 1 - 0
CHANGELOG.md

@@ -162,6 +162,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **The frontend build no longer warns about `path` and `crypto` being externalized for the STEP previewer (#2976)** — `occt-import-js`, the Emscripten build behind STEP previews, requires both modules, but only inside its `ENVIRONMENT_IS_NODE` branches; in the browser it loads its `.wasm` from the URL the preview worker passes and draws randomness from `crypto.getRandomValues`. Vite still externalized both and printed two warnings on every build. `vite.config.ts` now drops exactly those two warnings for that one package through `build.rolldownOptions.onLog`, so an externalization anywhere else, or of any other module, still shows.
 
 ### Fixed
+- **Archiving a virtual-printer upload into a project that does not exist is refused up front** — The project chosen when archiving a pending upload was saved without checking it. On PostgreSQL an unknown project failed with a server error after the file had already been archived; on SQLite the archive was saved with a broken project link. The project is now checked first and an unknown one refused with "Project not found", before anything is archived.
 - **A project's bill of materials no longer shows archives from printers outside the user's groups** — A BOM line can be linked to an archive through the API, and the BOM answered with that archive's name whichever printer it came from. A member of a group limited to some printers could read the name of any archive by linking a line to it, and everyone reading the project saw the names of archives from printers they have no access to. A link to an archive that does not exist was saved too (a server error on PostgreSQL). A BOM line now links only to an archive the user can see, and shows a linked archive only to users who can see it.
 - **Adding archives or queue jobs to a project keeps to the user's own printers and prints** — Adding archives to a project (**Assign to project**), removing them, and adding queue jobs took any archive or job by its id. A member of a group limited to some printers could link another team's archives and jobs to a project or unlink them, and the reply's count told them which hidden archives exist; any project editor could also relink other users' prints. These now change only the archives and jobs the user could edit directly: on their printers, and only their own unless they may edit everyone's (Operators: their own). Others are skipped like missing ones, and **Assign to project** now reports how many were actually added. An API key with project access still needs no archive or queue access for this, but changes only the archives and jobs of its printers that its owner could change.
 - **Editing an archive can no longer move it to a printer outside the user's groups** — A member of a group limited to some printers could move an archive of one of their printers to any other printer through the archive edit, into the history of a team they have no access to, although uploading an archive to such a printer was already refused. Any edit could also link an archive to a printer or project that does not exist (a server error on PostgreSQL, a broken link on SQLite). Such an edit is now refused with "Printer not found" or "Project not found". Moving an archive between the user's own printers, and removing its printer or project, work as before.

+ 9 - 3
backend/app/api/routes/pending_uploads.py

@@ -12,6 +12,7 @@ from backend.app.core.auth import RequirePermissionIfAuthEnabled, require_owners
 from backend.app.core.database import get_db
 from backend.app.core.permissions import Permission
 from backend.app.models.pending_upload import PendingUpload
+from backend.app.models.project import Project
 from backend.app.models.user import User
 from backend.app.services.archive import ArchiveService, resolve_display_stem
 
@@ -256,6 +257,11 @@ async def archive_pending_upload(
     if not file_path.exists():
         raise HTTPException(status_code=404, detail="Upload file not found on disk")
 
+    # The project must exist before anything is archived; 0 means none.
+    project_id = (request.project_id or None) if request else None
+    if project_id is not None and await db.get(Project, project_id) is None:
+        raise HTTPException(status_code=400, detail="Project not found")
+
     # Archive the file
     from backend.app.api.routes.settings import get_setting
 
@@ -281,8 +287,8 @@ async def archive_pending_upload(
             archive.tags = request.tags
         if request.notes:
             archive.notes = request.notes
-        if request.project_id:
-            archive.project_id = request.project_id
+        if project_id is not None:
+            archive.project_id = project_id
 
     # Update pending record
     pending.status = "archived"
@@ -291,7 +297,7 @@ async def archive_pending_upload(
     if request:
         pending.tags = request.tags
         pending.notes = request.notes
-        pending.project_id = request.project_id
+        pending.project_id = project_id
 
     await db.commit()
 

+ 85 - 0
backend/tests/integration/test_pending_upload_archive_project.py

@@ -0,0 +1,85 @@
+"""Archiving a pending upload into a project checks the project first.
+
+The project id from the request was stored on the archive and the pending
+upload unchecked, after the file had already been archived: an unknown project
+failed with a server error on PostgreSQL (foreign key) with the archive files
+already written, and was stored as a broken link on SQLite. 0 skipped the
+archive but was stored on the pending upload.
+"""
+
+from __future__ import annotations
+
+from unittest.mock import AsyncMock, patch
+
+import pytest
+from httpx import AsyncClient
+from sqlalchemy import select
+
+from backend.app.models.archive import PrintArchive
+from backend.app.models.pending_upload import PendingUpload
+
+pytestmark = [pytest.mark.asyncio, pytest.mark.integration]
+
+
+async def _pending(db_session, tmp_path) -> PendingUpload:
+    file = tmp_path / "part.3mf"
+    file.write_bytes(b"3mf")
+    upload = PendingUpload(
+        filename="part.3mf", file_path=str(file), file_size=3, source_ip="192.168.1.100", status="pending"
+    )
+    db_session.add(upload)
+    await db_session.commit()
+    await db_session.refresh(upload)
+    return upload
+
+
+async def test_an_unknown_project_is_refused_before_anything_is_archived(
+    async_client: AsyncClient, db_session, tmp_path
+):
+    upload_id = (await _pending(db_session, tmp_path)).id
+
+    with patch("backend.app.api.routes.pending_uploads.ArchiveService.archive_print", new=AsyncMock()) as archive:
+        response = await async_client.post(f"/api/v1/pending-uploads/{upload_id}/archive", json={"project_id": 9999})
+
+    assert response.status_code == 400, response.text
+    assert response.json()["detail"] == "Project not found"
+    archive.assert_not_awaited()
+    db_session.expire_all()
+    row = (
+        await db_session.execute(
+            select(PendingUpload.status, PendingUpload.project_id).where(PendingUpload.id == upload_id)
+        )
+    ).one()
+    assert tuple(row) == ("pending", None)
+    assert (tmp_path / "part.3mf").exists()
+
+
+async def _fake_archive_print(self, **_kwargs) -> PrintArchive:
+    """Stands in for archiving a real 3MF: one archive row in the route's session."""
+    archive = PrintArchive(filename="part.3mf", file_path="archive/part.3mf", file_size=3, status="archived")
+    self.db.add(archive)
+    await self.db.flush()
+    return archive
+
+
+@pytest.mark.parametrize("given", ["known", 0, None])
+async def test_a_known_project_or_none_is_stored(async_client: AsyncClient, db_session, tmp_path, given):
+    upload_id = (await _pending(db_session, tmp_path)).id
+    project_id = None
+    if given == "known":
+        project_id = (await async_client.post("/api/v1/projects/", json={"name": "P"})).json()["id"]
+
+    with patch("backend.app.api.routes.pending_uploads.ArchiveService.archive_print", new=_fake_archive_print):
+        response = await async_client.post(
+            f"/api/v1/pending-uploads/{upload_id}/archive",
+            json={"project_id": project_id if given == "known" else given},
+        )
+
+    assert response.status_code == 200, response.text
+    db_session.expire_all()
+    stored = (
+        await db_session.execute(select(PrintArchive.project_id).where(PrintArchive.id == response.json()["id"]))
+    ).scalar()
+    pending = (await db_session.execute(select(PendingUpload.project_id).where(PendingUpload.id == upload_id))).scalar()
+    assert stored == project_id
+    assert pending == project_id