Security hardening (maziggy/bambuddy-security #25)

backend/app/api/routes/library.py

@@ -3196,7 +3196,7 @@ async def add_files_to_queue(
     # about which ids exist. Ownerless rows need LIBRARY_READ_ALL, matching
     # _ensure_library_file_visible.
     if actor is not None and not actor.has_permission(Permission.LIBRARY_READ_ALL.value):
-        files = {fid: f for fid, f in files.items() if f.created_by_id == actor.id}
+        files = {fid: f for fid, f in files.items() if f.created_by_id is not None and f.created_by_id == actor.id}
 
     # Project attribution (#1897): a file queued from a project-linked folder
     # inherits that project, so the resulting archive counts toward the

backend/app/api/routes/print_queue.py

@@ -169,15 +169,19 @@ def _extract_filament_types_from_3mf(file_path: Path, plate_id: int | None = Non
 _extract_print_time_from_3mf = extract_print_time_from_3mf
 
 
-def _assert_can_queue_archive(archive: PrintArchive, current_user: User | ApiKeyActor | None) -> None:
+def _assert_can_queue_archive(
+    archive: PrintArchive, current_user: User | ApiKeyActor | None, printer_scope: PrinterScope
+) -> None:
     """Gate turning *archive* into a print. Raises rather than returning a verdict.
 
     Shared by every route that creates queue items from an archive, so a new
     one can't quietly become a weaker door to the same action than
     ``POST /queue/`` is.
 
-    Two separate checks:
+    Three checks:
 
+    * The archive's printer must be one the caller may use; otherwise the
+      archive reads as missing, as the archive routes answer for it.
     * IDOR fix (maziggy/bambuddy-security #2): without this, a caller with
       QUEUE_CREATE could queue any user's archive even without ARCHIVES_READ on
       it — Landon's PoC enumerated this on admin's archives as operator1. Gate
@@ -191,12 +195,17 @@ def _assert_can_queue_archive(archive: PrintArchive, current_user: User | ApiKey
       allows any archive, REPRINT_OWN allows own only, ownerless archives
       require REPRINT_ALL (fail-closed).
     """
-    # None is auth off. An API key arrives as its RequestActor, never None.
+    # An archive from a printer the caller can't use reads as missing, like the
+    # archive routes answer for it.
+    if not printer_scope.allows(archive.printer_id):
+        raise HTTPException(404, "Archive not found")
+    # None is auth off (for an API key too).
     if current_user is None:
         return
-    if not current_user.has_permission(Permission.ARCHIVES_READ_ALL.value) and archive.created_by_id != current_user.id:
-        raise HTTPException(404, "Archive not found")
     owns_archive = archive.created_by_id is not None and archive.created_by_id == current_user.id
+    # An ownerless archive is no one's own, also not a caller without an id.
+    if not current_user.has_permission(Permission.ARCHIVES_READ_ALL.value) and not owns_archive:
+        raise HTTPException(404, "Archive not found")
     has_reprint = current_user.has_permission(Permission.ARCHIVES_REPRINT_ALL.value) or (
         owns_archive and current_user.has_permission(Permission.ARCHIVES_REPRINT_OWN.value)
     )
@@ -212,10 +221,9 @@ def _assert_can_queue_library_file(library_file: LibraryFile, current_user: User
     # None is auth off. An API key arrives as its RequestActor, never None.
     if current_user is None:
         return
-    if (
-        not current_user.has_permission(Permission.LIBRARY_READ_ALL.value)
-        and library_file.created_by_id != current_user.id
-    ):
+    # An ownerless file is no one's own, also not a caller without an id.
+    owns_file = library_file.created_by_id is not None and library_file.created_by_id == current_user.id
+    if not current_user.has_permission(Permission.LIBRARY_READ_ALL.value) and not owns_file:
         raise HTTPException(404, "Library file not found")
 
 
@@ -279,7 +287,7 @@ async def _is_orders_last_source(db: AsyncSession, item: PrintQueueItem) -> bool
 
 
 async def _assert_can_dispatch_batch_sources(
-    db: AsyncSession, batch_id: int, current_user: User | ApiKeyActor | None
+    db: AsyncSession, batch_id: int, current_user: User | ApiKeyActor | None, printer_scope: PrinterScope
 ) -> None:
     """Apply the ``POST /queue/`` source-file gates to everything a dispatch would print.
 
@@ -333,7 +341,7 @@ async def _assert_can_dispatch_batch_sources(
         archive = (await db.execute(select(PrintArchive).where(PrintArchive.id == archive_id))).scalar_one_or_none()
         # A deleted source can't be printed; dispatch will fail on it anyway.
         if archive is not None:
-            _assert_can_queue_archive(archive, current_user)
+            _assert_can_queue_archive(archive, current_user, printer_scope)
 
     for library_file_id in library_file_ids:
         library_file = (
@@ -719,7 +727,7 @@ async def _resolve_queue_variants(
         if not library_file or (
             current_user
             and not current_user.has_permission(Permission.LIBRARY_READ_ALL.value)
-            and library_file.created_by_id != current_user.id
+            and (library_file.created_by_id is None or library_file.created_by_id != current_user.id)
         ):
             raise HTTPException(404, f"Library file not found: {spec.library_file_id}")
 
@@ -905,17 +913,19 @@ async def add_to_queue(
     if data.archive_id:
         result = await db.execute(select(PrintArchive).where(PrintArchive.id == data.archive_id))
         archive = result.scalar_one_or_none()
+        # Missing answers like hidden (404 from the gate below).
         if not archive:
-            raise HTTPException(400, "Archive not found")
-        _assert_can_queue_archive(archive, actor)
+            raise HTTPException(404, "Archive not found")
+        _assert_can_queue_archive(archive, actor, printer_scope)
 
     # Validate library file exists (if provided) and get it for filament extraction
     library_file = None
     if data.library_file_id:
         result = await db.execute(LibraryFile.active().where(LibraryFile.id == data.library_file_id))
         library_file = result.scalar_one_or_none()
+        # Missing answers like hidden (404 from the gate below).
         if not library_file:
-            raise HTTPException(400, "Library file not found")
+            raise HTTPException(404, "Library file not found")
         _assert_can_queue_library_file(library_file, actor)
         # Bambu SD card is FAT32/exFAT — illegal filename chars would 553 at
         # FTP upload time (#1540). Reject at queue time so the user gets the
@@ -1674,7 +1684,7 @@ async def dispatch_batch(
         raise HTTPException(400, "Cannot dispatch a cancelled batch")
 
     # Dispatch starts prints, so it must not be a weaker door than POST /queue/.
-    await _assert_can_dispatch_batch_sources(db, batch.id, actor)
+    await _assert_can_dispatch_batch_sources(db, batch.id, actor, printer_scope)
 
     try:
         created = await dispatch_remaining(

backend/app/api/routes/webhook.py

@@ -5,7 +5,9 @@ from pydantic import BaseModel
 from sqlalchemy import select
 from sqlalchemy.ext.asyncio import AsyncSession
 
+from backend.app.api.routes.print_queue import _assert_can_queue_archive
 from backend.app.core.auth import (
+    ApiKeyActor,
     api_key_printer_scope,
     check_webhook_permission,
     ensure_api_key_printer_access,
@@ -112,11 +114,15 @@ async def webhook_add_to_queue(
     await check_webhook_permission(db, api_key, "queue")
     await ensure_api_key_printer_access(db, api_key, data.printer_id)
 
-    # Verify archive exists
+    # Verify archive exists and passes the same source gate as POST /queue/,
+    # with the key acting for its owner within its printers.
     result = await db.execute(select(PrintArchive).where(PrintArchive.id == data.archive_id))
     archive = result.scalar_one_or_none()
     if not archive:
         raise HTTPException(status_code=404, detail="Archive not found")
+    # Auth off: no owner to act for, as on POST /queue/; the key's printers still apply.
+    actor = ApiKeyActor(api_key, await resolve_apikey_owner(db, api_key)) if await is_auth_enabled(db) else None
+    _assert_can_queue_archive(archive, actor, await api_key_printer_scope(db, api_key))
 
     # Verify printer exists
     result = await db.execute(select(Printer).where(Printer.id == data.printer_id))

backend/tests/integration/test_queue_creation_attribution.py

@@ -164,6 +164,8 @@ class TestWebhookQueueAddAttribution:
                 key_prefix=key_prefix,
                 user_id=owner_id,
                 can_queue=True,
+                # Queueing an archive someone else made needs read access to it.
+                can_read_status=True,
             )
         )
         await db_session.commit()
@@ -174,11 +176,16 @@ class TestWebhookQueueAddAttribution:
     async def test_credits_the_key_owner(self, async_client: AsyncClient, db_session, test_engine, printer_and_archive):
         printer, archive = printer_and_archive
 
-        # The owner needs queue:create in their own right: a key is capped by
-        # its owner's permissions (#1894), so a bare account with no groups
-        # cannot queue through a key however its scope flags are set. This test
-        # is about who the row is credited to, not about the gate.
-        group = Group(name="queue-writers", description="t", permissions=["queue:create"], is_system=False)
+        # The owner needs the queue and archive permissions in their own right:
+        # a key is capped by its owner's permissions (#1894), so a bare account
+        # with no groups cannot queue through a key however its scope flags are
+        # set. This test is about who the row is credited to, not about the gate.
+        group = Group(
+            name="queue-writers",
+            description="t",
+            permissions=["queue:create", "archives:read_all", "archives:reprint_all"],
+            is_system=False,
+        )
         db_session.add(group)
         await db_session.flush()
         owner = User(username="keyowner", password_hash="x", is_active=True, groups=[group])

backend/tests/integration/test_queue_source_gate.py

@@ -0,0 +1,218 @@
+"""Every way of queueing an existing archive applies the same source checks.
+
+The queue, batch dispatch and webhook routes decide whether a caller may turn an
+archive into a print. They now agree: an archive the caller may not read, or
+one from a printer outside the caller's printers, reads as missing, and a
+missing archive or library file answers the same way as a hidden one.
+"""
+
+from __future__ import annotations
+
+import pytest
+from httpx import AsyncClient
+
+from backend.tests.integration.test_printer_scope_1727 import TEAM_PERMISSIONS, _admin_token, _auth, _group, _user
+
+pytestmark = [pytest.mark.asyncio, pytest.mark.integration]
+
+OWN_ONLY = [
+    "printers:read",
+    "queue:create",
+    "queue:read_own",
+    "archives:read_own",
+    "archives:reprint_own",
+    "api_keys:create",
+]
+
+
+async def _key(client: AsyncClient, headers: dict[str, str], **fields) -> dict[str, str]:
+    created = await client.post("/api/v1/api-keys/", headers=headers, json={"name": "k", "can_queue": True, **fields})
+    assert created.status_code in (200, 201), created.text
+    return {"X-API-Key": created.json()["key"]}
+
+
+async def _webhook(client: AsyncClient, key: dict[str, str], archive_id: int, printer_id: int):
+    return await client.post(
+        "/api/v1/webhook/queue/add", headers=key, json={"archive_id": archive_id, "printer_id": printer_id}
+    )
+
+
+async def test_the_webhook_refuses_an_archive_its_owner_cannot_read(
+    async_client: AsyncClient, printer_factory, archive_factory
+):
+    printer = await printer_factory(name="A")
+    admin = await _admin_token(async_client)
+    jwt, user_id = await _user(
+        async_client, admin, "user", [await _group(async_client, admin, "g", permissions=OWN_ONLY)]
+    )
+    key = await _key(async_client, _auth(jwt))
+    someone_elses = await archive_factory(printer.id)
+    own = await archive_factory(printer.id, created_by_id=user_id)
+
+    refused = await _webhook(async_client, key, someone_elses.id, printer.id)
+    allowed = await _webhook(async_client, key, own.id, printer.id)
+
+    assert (refused.status_code, refused.json()["detail"]) == (404, "Archive not found")
+    assert allowed.status_code == 200, allowed.text
+
+
+async def test_the_webhook_refuses_an_archive_from_another_printer(
+    async_client: AsyncClient, printer_factory, archive_factory
+):
+    a, b = await printer_factory(name="A"), await printer_factory(name="B")
+    admin = await _admin_token(async_client)
+    key = await _key(async_client, _auth(admin), can_read_status=True, printer_ids=[a.id])
+    on_a, on_b = await archive_factory(a.id), await archive_factory(b.id)
+
+    refused = await _webhook(async_client, key, on_b.id, a.id)
+    allowed = await _webhook(async_client, key, on_a.id, a.id)
+
+    assert (refused.status_code, refused.json()["detail"]) == (404, "Archive not found")
+    assert allowed.status_code == 200, allowed.text
+
+
+async def test_queueing_an_archive_from_a_printer_outside_the_team_is_refused(
+    async_client: AsyncClient, printer_factory, archive_factory
+):
+    a, b = await printer_factory(name="A"), await printer_factory(name="B")
+    admin = await _admin_token(async_client)
+    perms = await _group(async_client, admin, "perms", permissions=[*TEAM_PERMISSIONS, "archives:reprint_all"])
+    team = await _group(async_client, admin, "team", printer_ids=[a.id])
+    member = _auth((await _user(async_client, admin, "member", [perms, team]))[0])
+    on_a, on_b = await archive_factory(a.id), await archive_factory(b.id)
+
+    refused = await async_client.post(
+        "/api/v1/queue/", headers=member, json={"archive_id": on_b.id, "printer_id": a.id}
+    )
+    allowed = await async_client.post(
+        "/api/v1/queue/", headers=member, json={"archive_id": on_a.id, "printer_id": a.id}
+    )
+
+    assert (refused.status_code, refused.json()["detail"]) == (404, "Archive not found")
+    assert allowed.status_code == 200, allowed.text
+
+
+async def test_dispatching_a_batch_checks_the_archive_printer(
+    async_client: AsyncClient, printer_factory, archive_factory
+):
+    a, b = await printer_factory(name="A"), await printer_factory(name="B")
+    admin = await _admin_token(async_client)
+    on_b = await archive_factory(b.id)
+    # An order for archive B, printed on A, with one more run owed.
+    batch = await async_client.post(
+        "/api/v1/queue/batches",
+        headers=_auth(admin),
+        json={"name": "order", "archive_id": on_b.id, "plates": [{"plate_id": 1, "quantity_target": 2}]},
+    )
+    assert batch.status_code == 200, batch.text
+    batch_id = batch.json()["id"]
+    queued = await async_client.post(
+        "/api/v1/queue/",
+        headers=_auth(admin),
+        json={"archive_id": on_b.id, "printer_id": a.id, "batch_id": batch_id, "plate_id": 1},
+    )
+    assert queued.status_code == 200, queued.text
+    perms = await _group(
+        async_client, admin, "perms", permissions=[*TEAM_PERMISSIONS, "archives:reprint_all", "queue:update_all"]
+    )
+    team = await _group(async_client, admin, "team", printer_ids=[a.id])
+    member = _auth((await _user(async_client, admin, "member", [perms, team]))[0])
+
+    response = await async_client.post(f"/api/v1/queue/batches/{batch_id}/dispatch", headers=member, json={})
+
+    assert response.status_code == 404, response.text
+    assert response.json()["detail"] == "Archive not found"
+
+
+@pytest.mark.parametrize(
+    ("field", "detail"), [("archive_id", "Archive not found"), ("library_file_id", "Library file not found")]
+)
+async def test_a_missing_source_answers_like_a_hidden_one(async_client: AsyncClient, printer_factory, field, detail):
+    printer = await printer_factory()
+
+    response = await async_client.post("/api/v1/queue/", json={field: 9999, "printer_id": printer.id})
+
+    assert (response.status_code, response.json()["detail"]) == (404, detail)
+
+
+async def test_a_key_without_an_owner_needs_read_access_for_an_ownerless_archive(
+    async_client: AsyncClient, db_session, printer_factory, archive_factory
+):
+    from backend.app.core.auth import generate_api_key
+    from backend.app.models.api_key import APIKey
+
+    printer = await printer_factory()
+    await _admin_token(async_client)
+    ownerless = await archive_factory(printer.id)
+    keys = {}
+    for name, read in (("queue-only", False), ("queue-and-read", True)):
+        full_key, key_hash, key_prefix = generate_api_key()
+        db_session.add(
+            APIKey(name=name, key_hash=key_hash, key_prefix=key_prefix, can_queue=True, can_read_status=read)
+        )
+        keys[name] = {"X-API-Key": full_key}
+    await db_session.commit()
+
+    refused = await _webhook(async_client, keys["queue-only"], ownerless.id, printer.id)
+    allowed = await _webhook(async_client, keys["queue-and-read"], ownerless.id, printer.id)
+
+    assert (refused.status_code, refused.json()["detail"]) == (404, "Archive not found")
+    assert allowed.status_code == 200, allowed.text
+
+
+async def test_with_auth_off_the_webhook_queues_like_the_queue_route(
+    async_client: AsyncClient, db_session, printer_factory, archive_factory
+):
+    # No owner to act for: only the key's printers apply, as on POST /queue/.
+    from backend.app.core.auth import generate_api_key
+    from backend.app.models.api_key import APIKey
+
+    a, b = await printer_factory(name="A"), await printer_factory(name="B")
+    on_a, on_b = await archive_factory(a.id), await archive_factory(b.id)
+    full_key, key_hash, key_prefix = generate_api_key()
+    db_session.add(
+        APIKey(
+            name="k",
+            key_hash=key_hash,
+            key_prefix=key_prefix,
+            can_queue=True,
+            can_read_status=False,
+            printer_ids=[a.id],
+        )
+    )
+    await db_session.commit()
+    key = {"X-API-Key": full_key}
+
+    allowed = await _webhook(async_client, key, on_a.id, a.id)
+    refused = await _webhook(async_client, key, on_b.id, a.id)
+
+    assert allowed.status_code == 200, allowed.text
+    assert (refused.status_code, refused.json()["detail"]) == (404, "Archive not found")
+
+
+async def test_a_key_without_an_owner_needs_read_access_for_an_ownerless_library_file(
+    async_client: AsyncClient, db_session, printer_factory
+):
+    from backend.app.core.auth import generate_api_key
+    from backend.app.models.api_key import APIKey
+    from backend.app.models.library import LibraryFile
+
+    printer = await printer_factory()
+    await _admin_token(async_client)
+    row = LibraryFile(filename="part.gcode.3mf", file_path="library/part.gcode.3mf", file_size=5, file_type="3mf")
+    db_session.add(row)
+    keys = {}
+    for name, read in (("queue-only", False), ("queue-and-read", True)):
+        full_key, key_hash, key_prefix = generate_api_key()
+        db_session.add(
+            APIKey(name=name, key_hash=key_hash, key_prefix=key_prefix, can_queue=True, can_read_status=read)
+        )
+        keys[name] = {"X-API-Key": full_key}
+    await db_session.commit()
+    body = {"library_file_id": row.id, "printer_id": printer.id}
+
+    refused = await async_client.post("/api/v1/queue/", headers=keys["queue-only"], json=body)
+    allowed = await async_client.post("/api/v1/queue/", headers=keys["queue-and-read"], json=body)
+
+    assert (refused.status_code, refused.json()["detail"]) == (404, "Library file not found")
+    assert allowed.status_code == 200, allowed.text