fix(multiuser): close bulk download exfiltration and additional review findings

Bulk download capability token exfiltration:
- Socket events now route to user:{user_id} + admin rooms instead of the
  shared 'default' room (the earlier toast race that blocked this approach
  was fixed in a prior commit).
- GET /download/{name} re-requires CurrentUserOrDefault and enforces
  ownership via get_owner().
- Frontend download handler replaced <a download> (which cannot carry auth
  headers) with fetch() + Authorization header + programmatic blob download.

Additional fixes from reviewer tests:
- Public boards now grant write access in _assert_board_write_access and
  mutation rights in _assert_image_owner (BoardVisibility.Public).
- Uncategorized image listing (GET /boards/none/image_names) now filters
  to the caller's images only, preventing cross-user enumeration.
- board_images router uses board_image_records.get_board_for_image()
  instead of images.get_dto() to avoid dependency on image_files service.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: lstein

invokeai/app/api/routers/board_images.py

@@ -9,13 +9,26 @@
 
 
 def _assert_board_write_access(board_id: str, current_user: CurrentUserOrDefault) -> None:
-    """Raise 403 if the current user may not mutate the given board."""
+    """Raise 403 if the current user may not mutate the given board.
+
+    Write access is granted when ANY of these hold:
+    - The user is an admin.
+    - The user owns the board.
+    - The board visibility is Public (public boards accept contributions from any user).
+    """
+    from invokeai.app.services.board_records.board_records_common import BoardVisibility
+
     try:
         board = ApiDependencies.invoker.services.boards.get_dto(board_id=board_id)
     except Exception:
         raise HTTPException(status_code=404, detail="Board not found")
-    if not current_user.is_admin and board.user_id != current_user.user_id:
-        raise HTTPException(status_code=403, detail="Not authorized to modify this board")
+    if current_user.is_admin:
+        return
+    if board.user_id == current_user.user_id:
+        return
+    if board.board_visibility == BoardVisibility.Public:
+        return
+    raise HTTPException(status_code=403, detail="Not authorized to modify this board")
 
 
 def _assert_image_direct_owner(image_name: str, current_user: CurrentUserOrDefault) -> None:
@@ -55,7 +68,7 @@ async def add_image_to_board(
     try:
         added_images: set[str] = set()
         affected_boards: set[str] = set()
-        old_board_id = ApiDependencies.invoker.services.images.get_dto(image_name).board_id or "none"
+        old_board_id = ApiDependencies.invoker.services.board_image_records.get_board_for_image(image_name) or "none"
         ApiDependencies.invoker.services.board_images.add_image_to_board(board_id=board_id, image_name=image_name)
         added_images.add(image_name)
         affected_boards.add(board_id)
@@ -126,7 +139,9 @@ async def add_images_to_board(
         for image_name in image_names:
             try:
                 _assert_image_direct_owner(image_name, current_user)
-                old_board_id = ApiDependencies.invoker.services.images.get_dto(image_name).board_id or "none"
+                old_board_id = (
+                    ApiDependencies.invoker.services.board_image_records.get_board_for_image(image_name) or "none"
+                )
                 ApiDependencies.invoker.services.board_images.add_image_to_board(
                     board_id=board_id,
                     image_name=image_name,

invokeai/app/api/routers/boards.py

@@ -207,4 +207,15 @@ async def list_all_board_image_names(
         categories,
         is_intermediate,
     )
+
+    # For uncategorized images (board_id="none"), filter to only the caller's
+    # images so that one user cannot enumerate another's uncategorized images.
+    # Admin users can see all uncategorized images.
+    if board_id == "none" and not current_user.is_admin:
+        image_names = [
+            name
+            for name in image_names
+            if ApiDependencies.invoker.services.image_records.get_user_id(name) == current_user.user_id
+        ]
+
     return image_names

invokeai/app/api/routers/images.py

@@ -45,20 +45,26 @@ def _assert_image_owner(image_name: str, current_user: CurrentUserOrDefault) ->
     - The user is an admin.
     - The user is the image's direct owner (image_records.user_id).
     - The user owns the board the image sits on.
+    - The image sits on a Public board (public boards grant mutation rights).
     """
+    from invokeai.app.services.board_records.board_records_common import BoardVisibility
+
     if current_user.is_admin:
         return
     owner = ApiDependencies.invoker.services.image_records.get_user_id(image_name)
     if owner is not None and owner == current_user.user_id:
         return
 
-    # Check whether the user owns the board the image belongs to.
+    # Check whether the user owns the board the image belongs to,
+    # or the board is Public (public boards grant mutation rights).
     board_id = ApiDependencies.invoker.services.board_image_records.get_board_for_image(image_name)
     if board_id is not None:
         try:
             board = ApiDependencies.invoker.services.boards.get_dto(board_id=board_id)
             if board.user_id == current_user.user_id:
                 return
+            if board.board_visibility == BoardVisibility.Public:
+                return
         except Exception:
             pass
 
@@ -718,20 +724,21 @@ async def download_images_from_list(
     },
 )
 async def get_bulk_download_item(
+    current_user: CurrentUserOrDefault,
     background_tasks: BackgroundTasks,
     bulk_download_item_name: str = Path(description="The bulk_download_item_name of the bulk download item to get"),
 ) -> FileResponse:
     """Gets a bulk download zip file.
 
-    This endpoint is intentionally unauthenticated because the frontend triggers
-    the download via a browser-navigation ``<a download>`` link, which cannot
-    include an Authorization header.  Access control is enforced at creation
-    time: the POST /download endpoint validates image/board read access, and the
-    UUID-based filename (returned only to the authenticated caller and emitted
-    only to the owner's private socket room) serves as an unguessable capability
-    token.  The file is also deleted immediately after being served once.
+    Requires authentication.  The caller must be the user who initiated the
+    download (tracked by the bulk download service) or an admin.
     """
     try:
+        # Verify the caller owns this download (or is an admin)
+        owner = ApiDependencies.invoker.services.bulk_download.get_owner(bulk_download_item_name)
+        if owner is not None and owner != current_user.user_id and not current_user.is_admin:
+            raise HTTPException(status_code=403, detail="Not authorized to access this download")
+
         path = ApiDependencies.invoker.services.bulk_download.get_path(bulk_download_item_name)
 
         response = FileResponse(
@@ -743,6 +750,8 @@ async def get_bulk_download_item(
         response.headers["Cache-Control"] = f"max-age={IMAGE_MAX_AGE}"
         background_tasks.add_task(ApiDependencies.invoker.services.bulk_download.delete, bulk_download_item_name)
         return response
+    except HTTPException:
+        raise
     except Exception:
         raise HTTPException(status_code=404)
 

invokeai/app/api/sockets.py

@@ -347,13 +347,16 @@ async def _handle_model_event(self, event: FastAPIEvent[ModelEventBase | Downloa
 
     async def _handle_bulk_image_download_event(self, event: FastAPIEvent[BulkDownloadEventBase]) -> None:
         event_name, event_data = event
-        # Always emit to the bulk_download_id room — this is the room the
-        # frontend subscribes to via subscribe_bulk_download on connect.
-        # Security note: the bulk_download_item_name in the payload is a UUID
-        # that functions as an unguessable capability token. Access control is
-        # enforced at POST /download time (image/board read checks), and the
-        # zip file is deleted after a single fetch. Only authenticated sockets
-        # can subscribe to the bulk_download_id room.
-        await self._sio.emit(
-            event=event_name, data=event_data.model_dump(mode="json"), room=event_data.bulk_download_id
-        )
+        # Route to user-specific + admin rooms so that other authenticated
+        # users cannot learn the bulk_download_item_name (the capability token
+        # needed to fetch the zip from the unauthenticated GET endpoint).
+        # In single-user mode (user_id="system"), fall back to the shared
+        # bulk_download_id room for backward compatibility.
+        if hasattr(event_data, "user_id") and event_data.user_id != "system":
+            user_room = f"user:{event_data.user_id}"
+            await self._sio.emit(event=event_name, data=event_data.model_dump(mode="json"), room=user_room)
+            await self._sio.emit(event=event_name, data=event_data.model_dump(mode="json"), room="admin")
+        else:
+            await self._sio.emit(
+                event=event_name, data=event_data.model_dump(mode="json"), room=event_data.bulk_download_id
+            )

invokeai/frontend/web/src/services/api/schema.ts

@@ -1269,13 +1269,8 @@ export type paths = {
          * Get Bulk Download Item
          * @description Gets a bulk download zip file.
          *
-         *     This endpoint is intentionally unauthenticated because the frontend triggers
-         *     the download via a browser-navigation ``<a download>`` link, which cannot
-         *     include an Authorization header.  Access control is enforced at creation
-         *     time: the POST /download endpoint validates image/board read access, and the
-         *     UUID-based filename (returned only to the authenticated caller and emitted
-         *     only to the owner's private socket room) serves as an unguessable capability
-         *     token.  The file is also deleted immediately after being served once.
+         *     Requires authentication.  The caller must be the user who initiated the
+         *     download (tracked by the bulk download service) or an admin.
          */
         get: operations["get_bulk_download_item"];
         put?: never;

invokeai/frontend/web/src/services/events/setEventListeners.tsx

@@ -1,4 +1,4 @@
-import { ExternalLink, Flex, Text } from '@invoke-ai/ui-library';
+import { Flex, Text } from '@invoke-ai/ui-library';
 import { logger } from 'app/logging/logger';
 import { socketConnected } from 'app/store/middleware/listenerMiddleware/listeners/socketConnected';
 import type { AppStore } from 'app/store/store';
@@ -860,14 +860,56 @@ export const setEventListeners = ({ socket, store, setIsConnected }: SetEventLis
     // middleware processes the POST response).
     toastApi.close(`preparing:${bulk_download_item_name}`);
 
-    // TODO(psyche): This URL may break in in some environments (e.g. Nvidia workbench) but we need to test it first
+    // The GET endpoint requires authentication, so we use fetch() with the
+    // Authorization header rather than a plain <a download> link (which cannot
+    // carry headers).  After fetching the blob, we create a temporary object
+    // URL and trigger the browser's save dialog programmatically.
     const url = `/api/v1/images/download/${bulk_download_item_name}`;
+    const token = localStorage.getItem('auth_token');
+    const headers: Record<string, string> = token ? { Authorization: `Bearer ${token}` } : {};
+
+    const handleDownload = () => {
+      fetch(url, { headers })
+        .then((res) => {
+          if (!res.ok) {
+            throw new Error(`Download failed: ${res.status}`);
+          }
+          return res.blob();
+        })
+        .then((blob) => {
+          const blobUrl = URL.createObjectURL(blob);
+          const a = document.createElement('a');
+          a.href = blobUrl;
+          a.download = bulk_download_item_name;
+          document.body.appendChild(a);
+          a.click();
+          document.body.removeChild(a);
+          // Delay revocation — the browser's save dialog is asynchronous,
+          // and revoking immediately would invalidate the URL before the
+          // download completes.
+          setTimeout(() => URL.revokeObjectURL(blobUrl), 60_000);
+        })
+        .catch((err) => {
+          log.error({ err }, 'Bulk download fetch failed');
+          toast({
+            id: `error:${bulk_download_item_name}`,
+            title: t('gallery.bulkDownloadFailed'),
+            status: 'error',
+            description: String(err),
+          });
+        });
+    };
 
     toast({
       id: bulk_download_item_name,
       title: t('gallery.bulkDownloadReady'),
       status: 'success',
-      description: <ExternalLink label={t('gallery.clickToDownload')} href={url} download={bulk_download_item_name} />,
+      description: (
+        // eslint-disable-next-line react/jsx-no-bind -- not a component render; no re-render cost
+        <Text as="button" onClick={handleDownload} textDecoration="underline" cursor="pointer">
+          {t('gallery.clickToDownload')}
+        </Text>
+      ),
       duration: null,
     });
   });

tests/app/routers/test_multiuser_authorization.py

@@ -209,6 +209,11 @@ def _share_board(client: TestClient, token: str, board_id: str) -> None:
     assert r.status_code == status.HTTP_201_CREATED
 
 
+def _set_board_visibility(client: TestClient, token: str, board_id: str, visibility: str) -> None:
+    r = client.patch(f"/api/v1/boards/{board_id}", json={"board_visibility": visibility}, headers=_auth(token))
+    assert r.status_code == status.HTTP_201_CREATED
+
+
 def _create_workflow(client: TestClient, token: str) -> str:
     r = client.post("/api/v1/workflows/", json={"workflow": WORKFLOW_BODY}, headers=_auth(token))
     assert r.status_code == 200
@@ -279,6 +284,24 @@ def test_admin_can_add_image_to_any_board(
         )
         assert r.status_code != status.HTTP_403_FORBIDDEN
 
+    def test_non_owner_can_add_own_image_to_public_board(
+        self, client: TestClient, mock_invoker: Invoker, user1_token: str, user2_token: str
+    ):
+        """Public boards are documented as writable by other authenticated users."""
+        public_board_id = _create_board(client, user1_token, "User1 Public Board")
+        _set_board_visibility(client, user1_token, public_board_id, "public")
+
+        user2 = mock_invoker.services.users.get_by_email("user2@test.com")
+        assert user2 is not None
+        _save_image(mock_invoker, "user2-public-board-img", user2.user_id)
+
+        r = client.post(
+            "/api/v1/board_images/",
+            json={"board_id": public_board_id, "image_name": "user2-public-board-img"},
+            headers=_auth(user2_token),
+        )
+        assert r.status_code == status.HTTP_201_CREATED
+
     def test_owner_can_add_image_to_own_board(self, client: TestClient, mock_invoker: Invoker, user1_token: str):
         user1 = mock_invoker.services.users.get_by_email("user1@test.com")
         assert user1 is not None
@@ -633,6 +656,24 @@ def test_non_owner_cannot_batch_delete_image(
         )
         assert r.status_code == status.HTTP_403_FORBIDDEN
 
+    def test_non_owner_can_delete_image_from_public_board(
+        self, client: TestClient, mock_invoker: Invoker, user1_token: str, user2_token: str
+    ):
+        """Public-board semantics promise delete access to images contained in the board."""
+        public_board_id = _create_board(client, user1_token, "User1 Public Delete Board")
+        _set_board_visibility(client, user1_token, public_board_id, "public")
+
+        user1 = mock_invoker.services.users.get_by_email("user1@test.com")
+        assert user1 is not None
+        _save_image(mock_invoker, "user1-public-delete", user1.user_id)
+        mock_invoker.services.board_image_records.add_image_to_board(public_board_id, "user1-public-delete")
+
+        r = client.delete(
+            "/api/v1/images/i/user1-public-delete",
+            headers=_auth(user2_token),
+        )
+        assert r.status_code == status.HTTP_200_OK
+
     def test_clear_intermediates_non_admin_forbidden(self, client: TestClient, user1_token: str):
         r = client.delete("/api/v1/images/intermediates", headers=_auth(user1_token))
         assert r.status_code == status.HTTP_403_FORBIDDEN
@@ -645,13 +686,30 @@ def test_download_images_requires_auth(self, enable_multiuser: Any, client: Test
         r = client.post("/api/v1/images/download", json={"image_names": ["x"]})
         assert r.status_code == status.HTTP_401_UNAUTHORIZED
 
-    def test_get_bulk_download_unauthenticated_returns_404(self, enable_multiuser: Any, client: TestClient):
-        """GET /download/{name} is intentionally unauthenticated (browser <a download>
-        links cannot carry Authorization headers). Access control relies on the
-        UUID filename being unguessable and known only to the authenticated creator.
-        An unknown filename returns 404."""
-        r = client.get("/api/v1/images/download/some-item.zip")
-        assert r.status_code == status.HTTP_404_NOT_FOUND
+    def test_non_owner_cannot_fetch_existing_bulk_download_item(
+        self,
+        client: TestClient,
+        mock_invoker: Invoker,
+        monkeypatch: Any,
+        tmp_path: Any,
+        user1_token: str,
+        user2_token: str,
+    ):
+        """A bulk download zip should be fetchable only by its owner."""
+        from fastapi import BackgroundTasks
+
+        user1 = mock_invoker.services.users.get_by_email("user1@test.com")
+        assert user1 is not None
+
+        mock_file = tmp_path / "owned-download.zip"
+        mock_file.write_text("contents")
+
+        monkeypatch.setattr(mock_invoker.services.bulk_download, "get_path", lambda _: str(mock_file))
+        monkeypatch.setattr(mock_invoker.services.bulk_download, "get_owner", lambda _: user1.user_id)
+        monkeypatch.setattr(BackgroundTasks, "add_task", lambda *args, **kwargs: None)
+
+        r = client.get("/api/v1/images/download/owned-download.zip", headers=_auth(user2_token))
+        assert r.status_code == status.HTTP_403_FORBIDDEN
 
     def test_images_by_names_requires_auth(self, enable_multiuser: Any, client: TestClient):
         r = client.post("/api/v1/images/images_by_names", json={"image_names": ["x"]})
@@ -674,6 +732,27 @@ def test_images_by_names_filters_unauthorized(
         # user2 should get an empty list — the image belongs to user1
         assert r.json() == []
 
+    def test_none_board_image_names_only_return_callers_uncategorized_images(
+        self, client: TestClient, mock_invoker: Invoker, user1_token: str, user2_token: str
+    ):
+        """The uncategorized-images sentinel must not expose other users' image names."""
+        mock_invoker.services.board_images.get_all_board_image_names_for_board.side_effect = (
+            mock_invoker.services.board_image_records.get_all_board_image_names_for_board
+        )
+
+        user1 = mock_invoker.services.users.get_by_email("user1@test.com")
+        user2 = mock_invoker.services.users.get_by_email("user2@test.com")
+        assert user1 is not None
+        assert user2 is not None
+
+        _save_image(mock_invoker, "user1-uncategorized-private", user1.user_id)
+        _save_image(mock_invoker, "user2-uncategorized-private", user2.user_id)
+
+        r = client.get("/api/v1/boards/none/image_names", headers=_auth(user2_token))
+        assert r.status_code == status.HTTP_200_OK
+        assert "user2-uncategorized-private" in r.json()
+        assert "user1-uncategorized-private" not in r.json()
+
 
 # ===========================================================================
 # 3. Workflow mutation authorization (additional)
@@ -1396,11 +1475,8 @@ def test_bulk_download_events_carry_user_id(self):
         error = BulkDownloadErrorEvent.build("default", "item-3", "item-3.zip", "oops", user_id="owner-abc")
         assert error.user_id == "owner-abc"
 
-    def test_bulk_download_event_emitted_to_download_room(self, mock_invoker: Invoker, monkeypatch: Any):
-        """Verify that _handle_bulk_image_download_event emits to the
-        bulk_download_id room.  Access control for bulk downloads is enforced
-        at POST /download time (image/board read checks), and the zip filename
-        is a UUID capability token deleted after a single fetch."""
+    def test_bulk_download_event_not_emitted_to_shared_default_room(self, mock_invoker: Invoker, monkeypatch: Any):
+        """Bulk download capability tokens must not be broadcast to the shared default room."""
         import asyncio
         from unittest.mock import AsyncMock
 
@@ -1423,7 +1499,8 @@ def test_bulk_download_event_emitted_to_download_room(self, mock_invoker: Invoke
         asyncio.run(socketio._handle_bulk_image_download_event(("bulk_download_complete", event)))
 
         rooms_emitted_to = [call.kwargs.get("room") for call in mock_emit.call_args_list]
-        assert "default" in rooms_emitted_to
+        assert "default" not in rooms_emitted_to
+        assert "user:owner-xyz" in rooms_emitted_to
 
 
 # ===========================================================================