fix(permissions): address gantoine review + Postgres-safe single migration

Visibility-coverage gaps (404-mask hidden entities, mirroring the existing
delete/read paths):
- update_rom (PUT /roms/{id}) and update_rom_user (PUT /roms/{id}/props)
- add_firmware: platform-hide now cascades to firmware uploads
- patch_rom: resolve the parent rom of both the base and patch files and
  404 when hidden, so a hidden rom's bytes can no longer be streamed back
- activity feeds (get_all_activity / get_rom_activity): drop sessions whose
  rom is hidden from the caller

Migration: make the role enum -> varchar narrowing Postgres-safe. The cast
now uses postgresql_using, the orphaned native role type is dropped on
upgrade, and downgrade recreates it explicitly (create_type=False) before
re-typing the column. Verified up/down/up on Postgres 16 and MariaDB.

Also collapses the two permission migrations into a single 0092 and notes
the override own_only replacement granularity limit in the resolver.

AI assistance: implemented with Claude Code (review-fix pass).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

backend/alembic/versions/0092_narrow_role_to_admin_user.py

@@ -1,56 +0,0 @@
-"""Narrow the user role vocabulary from viewer/editor/admin to user/admin.
-
-The viewer/editor distinction now lives entirely in permission groups, so the
-`role` column collapses to two kinds: `admin` (bypasses everything) and `user`.
-
-The column is converted from a native enum to a portable VARCHAR (matching the
-rest of the permission vocabulary, `native_enum=False`) and existing values are
-normalized: ADMIN -> admin, VIEWER/EDITOR -> user. Stored values are lowercase.
-
-Revision ID: 0092_narrow_role_to_admin_user
-Revises: 0091_permission_system
-Create Date: 2026-06-25 18:00:00.000000
-
-"""
-
-import sqlalchemy as sa
-from alembic import op
-
-# revision identifiers, used by Alembic.
-revision = "0092_narrow_role_to_admin_user"
-down_revision = "0091_permission_system"
-branch_labels = None
-depends_on = None
-
-
-def upgrade() -> None:
-    # Drop the native 3-value enum in favour of a plain VARCHAR so the
-    # vocabulary can change without a destructive cross-dialect ALTER TYPE.
-    with op.batch_alter_table("users") as batch_op:
-        batch_op.alter_column(
-            "role",
-            existing_type=sa.Enum("VIEWER", "EDITOR", "ADMIN", name="role"),
-            type_=sa.String(length=20),
-            existing_nullable=False,
-        )
-
-    # Collapse to the two-value lowercase vocabulary. UPPER() guards against
-    # either casing being present (native enums stored the uppercase name).
-    op.execute("UPDATE users SET role = 'admin' WHERE UPPER(role) = 'ADMIN'")
-    op.execute(
-        "UPDATE users SET role = 'user' "
-        "WHERE UPPER(role) IN ('VIEWER', 'EDITOR', 'USER') OR role IS NULL"
-    )
-
-
-def downgrade() -> None:
-    # Best-effort reverse: map user -> viewer, keep admin, restore the enum.
-    op.execute("UPDATE users SET role = 'VIEWER' WHERE LOWER(role) = 'user'")
-    op.execute("UPDATE users SET role = 'ADMIN' WHERE LOWER(role) = 'admin'")
-    with op.batch_alter_table("users") as batch_op:
-        batch_op.alter_column(
-            "role",
-            existing_type=sa.String(length=20),
-            type_=sa.Enum("VIEWER", "EDITOR", "ADMIN", name="role"),
-            existing_nullable=False,
-        )

backend/alembic/versions/0092_permission_system.py

@@ -1,8 +1,8 @@
-"""Granular permission system: groups, grants, per-user overrides, hidden entities.
+"""Granular permission system: groups, grants, overrides, hidden entities, and
+the narrowing of the user role vocabulary to admin/user.
 
-Single migration for the whole permissions feature (tables, group colour, and
-the legacy-group backfill). Backfills so that NO user gains or loses access on
-upgrade:
+Single migration for the whole permissions feature. Backfills so that NO user
+gains or loses access on upgrade:
 
   * "Viewer (legacy)" group reproduces the old non-kiosk default user
     (WRITE_SCOPES): read the library, manage only own collections/assets/devices.
@@ -11,11 +11,15 @@ upgrade:
     editors can already delete -- preserved here as explicit delete grants).
   * Existing users are assigned to the matching group by their current role.
     Admins are left unassigned (they bypass groups entirely).
+  * The `role` column is then narrowed from the viewer/editor/admin enum to a
+    portable VARCHAR holding just `admin`/`user` (the viewer/editor distinction
+    now lives entirely in permission groups). The group assignment above MUST
+    happen before this, since it reads the old VIEWER/EDITOR values.
 
 The seeded grant matrix is the frozen twin of ``handler/auth/permissions_map.py``;
 ``tests/handler/auth/test_permissions_parity.py`` keeps the two in lockstep.
 
-Revision ID: 0091_permission_system
+Revision ID: 0092_permission_system
 Revises: 0091_unique_platform_fs_name
 Create Date: 2026-06-22 00:00:00.000000
 
@@ -23,9 +27,12 @@ Create Date: 2026-06-22 00:00:00.000000
 
 import sqlalchemy as sa
 from alembic import op
+from sqlalchemy.dialects.postgresql import ENUM as PostgresEnum
+
+from utils.database import is_postgresql
 
 # revision identifiers, used by Alembic.
-revision = "0091_permission_system"
+revision = "0092_permission_system"
 down_revision = "0091_unique_platform_fs_name"
 branch_labels = None
 depends_on = None
@@ -61,6 +68,9 @@ EDITOR_GRANTS = VIEWER_GRANTS + EDITOR_EXTRA
 VIEWER_COLOR = "#3b82f6"
 EDITOR_COLOR = "#f59e0b"
 
+# The pre-narrowing role enum, used by the role alter_column in both directions.
+_ROLE_ENUM = sa.Enum("VIEWER", "EDITOR", "ADMIN", name="role")
+
 
 def _timestamp_cols() -> tuple[sa.Column, sa.Column]:
     # Fresh Column objects per call: a Column instance can only belong to one
@@ -217,8 +227,30 @@ def upgrade() -> None:
             if_not_exists=True,
         )
 
+    # Seed + assign by current role while `role` still holds VIEWER/EDITOR/ADMIN.
     _seed_legacy_groups()
 
+    # Now collapse the role vocabulary to the two-value lowercase set. Convert
+    # the native enum to a portable VARCHAR first, then normalize the values.
+    # On Postgres the cast needs an explicit USING and the orphaned enum type
+    # must be dropped so a later downgrade can recreate it cleanly.
+    connection = op.get_bind()
+    with op.batch_alter_table("users") as batch_op:
+        batch_op.alter_column(
+            "role",
+            existing_type=_ROLE_ENUM,
+            type_=sa.String(length=20),
+            existing_nullable=False,
+            postgresql_using="role::text",
+        )
+    op.execute("UPDATE users SET role = 'admin' WHERE UPPER(role) = 'ADMIN'")
+    op.execute(
+        "UPDATE users SET role = 'user' "
+        "WHERE UPPER(role) IN ('VIEWER', 'EDITOR', 'USER') OR role IS NULL"
+    )
+    if is_postgresql(connection):
+        PostgresEnum(name="role").drop(connection, checkfirst=True)
+
 
 def _seed_legacy_groups() -> None:
     conn = op.get_bind()
@@ -300,6 +332,29 @@ def _seed_legacy_groups() -> None:
 
 
 def downgrade() -> None:
+    # Restore the role enum (reverse of the narrowing) before tearing down the
+    # permission infrastructure. Values are restored while still VARCHAR.
+    connection = op.get_bind()
+    op.execute("UPDATE users SET role = 'VIEWER' WHERE LOWER(role) = 'user'")
+    op.execute("UPDATE users SET role = 'ADMIN' WHERE LOWER(role) = 'admin'")
+    # On Postgres the native enum type was dropped on upgrade; recreate it
+    # explicitly (create_type=False keeps the alter from re-issuing CREATE TYPE)
+    # and cast the column back with an explicit USING.
+    role_type: sa.Enum = _ROLE_ENUM
+    if is_postgresql(connection):
+        role_type = PostgresEnum(
+            "VIEWER", "EDITOR", "ADMIN", name="role", create_type=False
+        )
+        role_type.create(connection, checkfirst=True)
+    with op.batch_alter_table("users") as batch_op:
+        batch_op.alter_column(
+            "role",
+            existing_type=sa.String(length=20),
+            type_=role_type,
+            existing_nullable=False,
+            postgresql_using="role::role",
+        )
+
     with op.batch_alter_table("users") as batch_op:
         batch_op.drop_constraint("fk_users_permission_group_id", type_="foreignkey")
         batch_op.drop_index("ix_users_permission_group_id", if_exists=True)

backend/endpoints/activity.py

@@ -7,6 +7,7 @@ from decorators.auth import protected_route
 from endpoints.responses.activity import ActivityClearSchema, ActivityEntrySchema
 from handler.activity_handler import ActivityEntry, activity_handler
 from handler.auth.constants import Scope
+from handler.auth.dependencies import get_permissions
 from handler.database import db_device_handler, db_rom_handler, db_save_handler
 from handler.socket_handler import socket_handler
 from logger.logger import log
@@ -19,6 +20,22 @@ router = APIRouter(
 )
 
 
+def _visible_activity(
+    request: Request, entries: list[ActivityEntry]
+) -> list[ActivityEntrySchema]:
+    """Drop sessions whose ROM is hidden from the caller (platform or rom hide)."""
+    perms = get_permissions(request)
+    if not perms.is_admin and (perms.hidden_platform_ids or perms.hidden_rom_ids):
+        rom_ids = [e["rom_id"] for e in entries]
+        hidden = db_rom_handler.get_hidden_rom_ids_among(
+            rom_ids,
+            list(perms.hidden_platform_ids),
+            list(perms.hidden_rom_ids),
+        )
+        entries = [e for e in entries if e["rom_id"] not in hidden]
+    return [ActivityEntrySchema(**e) for e in entries]
+
+
 class DeviceHeartbeatPayload(BaseModel):
     rom_id: int = Field(ge=1)
     device_id: str = Field(min_length=1, max_length=255)
@@ -28,14 +45,14 @@ class DeviceHeartbeatPayload(BaseModel):
 async def get_all_activity(request: Request) -> list[ActivityEntrySchema]:
     """Return every currently active play session across all users."""
     entries = await activity_handler.get_all_active()
-    return [ActivityEntrySchema(**e) for e in entries]
+    return _visible_activity(request, entries)
 
 
 @protected_route(router.get, "/rom/{rom_id}", [Scope.ROMS_USER_READ])
 async def get_rom_activity(request: Request, rom_id: int) -> list[ActivityEntrySchema]:
     """Return all active play sessions for a specific ROM."""
     entries = await activity_handler.get_active_for_rom(rom_id)
-    return [ActivityEntrySchema(**e) for e in entries]
+    return _visible_activity(request, entries)
 
 
 @protected_route(router.post, "/heartbeat", [Scope.ROMS_USER_WRITE])

backend/endpoints/firmware.py

@@ -51,6 +51,11 @@ async def add_firmware(
         log.error(error)
         raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=error)
 
+    # Platform-hide cascades to firmware; 404-mask uploads to hidden platforms.
+    if not get_permissions(request).can_see_platform(db_platform.id):
+        error = f"Platform with ID {platform_id} not found"
+        raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=error)
+
     uploaded_firmware = []
     firmware_path = fs_firmware_handler.get_firmware_fs_structure(db_platform.fs_slug)
 

backend/endpoints/roms/__init__.py

@@ -1318,6 +1318,10 @@ async def update_rom(
     if not rom:
         raise RomNotFoundInDatabaseException(id)
 
+    # 404-mask roms hidden from the caller rather than letting them edit.
+    if not get_permissions(request).can_see_rom(rom.id, rom.platform_id):
+        raise RomNotFoundInDatabaseException(id)
+
     if unmatch_metadata:
         db_rom_handler.update_rom(
             id,
@@ -1830,6 +1834,10 @@ async def update_rom_user(
     if not rom:
         raise RomNotFoundInDatabaseException(id)
 
+    # 404-mask roms hidden from the caller rather than confirming existence.
+    if not get_permissions(request).can_see_rom(rom.id, rom.platform_id):
+        raise RomNotFoundInDatabaseException(id)
+
     db_rom_user = db_rom_handler.get_rom_user(
         id, request.user.id
     ) or db_rom_handler.add_rom_user(id, request.user.id)

backend/endpoints/roms/patch.py

@@ -14,6 +14,7 @@ from starlette.responses import FileResponse
 from config import ROM_PATCHER_MAX_FILE_SIZE_BYTES
 from decorators.auth import protected_route
 from handler.auth.constants import Scope
+from handler.auth.dependencies import get_permissions
 from handler.database import db_rom_handler
 from handler.filesystem import fs_rom_handler
 from logger.formatter import BLUE
@@ -64,12 +65,21 @@ async def patch_rom(
         request.user.username if request.user.is_authenticated else "unknown"
     )
 
+    perms = get_permissions(request)
+
     rom_file = db_rom_handler.get_rom_file_by_id(id)
     if not rom_file:
         raise HTTPException(
             status_code=status.HTTP_404_NOT_FOUND,
             detail=f"ROM file with id {id} not found",
         )
+    # 404-mask file bytes of roms hidden from the caller.
+    base_rom = db_rom_handler.get_rom(rom_file.rom_id)
+    if not base_rom or not perms.can_see_rom(base_rom.id, base_rom.platform_id):
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND,
+            detail=f"ROM file with id {id} not found",
+        )
     if rom_file.missing_from_fs:
         raise HTTPException(
             status_code=status.HTTP_404_NOT_FOUND,
@@ -82,6 +92,15 @@ async def patch_rom(
             status_code=status.HTTP_404_NOT_FOUND,
             detail=f"Patch file with id {patch_request.patch_file_id} not found",
         )
+    # The patch file's bytes are read too; mask it if its rom is hidden.
+    patch_rom_parent = db_rom_handler.get_rom(patch_file.rom_id)
+    if not patch_rom_parent or not perms.can_see_rom(
+        patch_rom_parent.id, patch_rom_parent.platform_id
+    ):
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND,
+            detail=f"Patch file with id {patch_request.patch_file_id} not found",
+        )
     if patch_file.missing_from_fs:
         raise HTTPException(
             status_code=status.HTTP_404_NOT_FOUND,

backend/handler/auth/permissions.py

@@ -94,6 +94,10 @@ def _resolve_grant_map(
             base[(g.entity, g.action)] = g.own_only
 
     # Per-user overrides win over the group: grant adds, revoke removes.
+    # Override identity is (entity, action) only, so a grant override fully
+    # replaces the group's own_only for that key rather than merging. Both
+    # directions are admin-initiated and the narrowing case fails closed, so
+    # this is a granularity limit, not a privilege leak.
     if user.id is not None:
         for ov in db_permission_handler.get_user_overrides(user.id, session=session):
             if ov.granted:

backend/tests/endpoints/test_permissions_visibility.py

@@ -120,6 +120,37 @@ def test_hidden_rom_cannot_be_downloaded_by_id(client, viewer_user, rom):
     assert content_resp.status_code == status.HTTP_404_NOT_FOUND
 
 
+def test_hidden_rom_update_is_404_masked(client, editor_user, rom):
+    # Editor holds library-wide roms write (passes the coarse gate), so the
+    # hidden rom must be 404-masked instead of being editable.
+    _hide(PermEntity.ROMS, rom.id, editor_user.id)
+    resp = client.put(
+        f"/api/roms/{rom.id}", headers=_auth(editor_user), data={"name": "x"}
+    )
+    assert resp.status_code == status.HTTP_404_NOT_FOUND
+
+
+def test_hidden_rom_props_update_is_404_masked(client, viewer_user, rom):
+    # ROMS_USER_WRITE is a self-service scope every user holds, so the coarse
+    # gate passes; the hidden rom must still be masked, not confirmed.
+    _hide(PermEntity.ROMS, rom.id, viewer_user.id)
+    resp = client.put(
+        f"/api/roms/{rom.id}/props", headers=_auth(viewer_user), json={"rating": 5}
+    )
+    assert resp.status_code == status.HTTP_404_NOT_FOUND
+
+
+def test_hidden_rom_patch_is_404_masked(client, viewer_user, rom, rom_file):
+    # patch_rom streams file bytes back; a hidden rom's bytes must not leak.
+    _hide(PermEntity.ROMS, rom.id, viewer_user.id)
+    resp = client.post(
+        f"/api/roms/{rom_file.id}/patch",
+        headers=_auth(viewer_user),
+        json={"patch_file_id": rom_file.id},
+    )
+    assert resp.status_code == status.HTTP_404_NOT_FOUND
+
+
 def test_delete_requires_delete_grant_even_with_write_scope(
     client, viewer_user, platform
 ):