Merge pull request #3703 from rommapp/posthog-code/fix-romfile-visibility-bypass

fix(roms): enforce parent ROM visibility on direct file endpoints

Author: gantoine

backend/endpoints/roms/files.py

@@ -10,6 +10,7 @@
 from decorators.auth import protected_route
 from endpoints.responses.rom import RomFileSchema
 from handler.auth.constants import Scope
+from handler.auth.dependencies import assert_rom_visible
 from handler.database import db_rom_handler
 from handler.filesystem import fs_rom_handler
 from logger.formatter import BLUE
@@ -42,6 +43,16 @@ async def get_romfile(
             detail="File not found",
         )
 
+    # Resolve back to the parent rom and enforce its visibility, so a file
+    # belonging to a hidden rom can't be read by direct RomFile.id.
+    rom = db_rom_handler.get_rom(file.rom_id)
+    if not rom:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND,
+            detail="File not found",
+        )
+    assert_rom_visible(request, rom, not_found_detail="File not found")
+
     return RomFileSchema.model_validate(file)
 
 
@@ -69,6 +80,16 @@ async def get_romfile_content(
             detail="File not found",
         )
 
+    # 404-mask file bytes of roms hidden from the caller: resolve the parent
+    # rom and apply its visibility before serving any content.
+    rom = db_rom_handler.get_rom(file.rom_id)
+    if not rom:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND,
+            detail="File not found",
+        )
+    assert_rom_visible(request, rom, not_found_detail="File not found")
+
     log.info(
         f"User {hl(current_username, color=BLUE)} is downloading {hl(file.file_name)}"
     )

backend/tests/endpoints/roms/test_rom.py

@@ -6,6 +6,7 @@
 
 from config.config_manager import MetadataMediaType
 from handler.database import db_collection_handler, db_rom_handler
+from handler.database.base_handler import sync_session
 from handler.filesystem.resources_handler import FSResourcesHandler
 from handler.filesystem.roms_handler import FSRomsHandler
 from handler.metadata.flashpoint_handler import FlashpointHandler, FlashpointRom
@@ -16,6 +17,7 @@
 from handler.metadata.ra_handler import RAGameRom, RAHandler
 from handler.metadata.ss_handler import SSHandler, SSRom
 from models.collection import Collection
+from models.permission import HiddenEntity, PermEntity
 from models.platform import Platform
 from models.rom import Rom, RomFile, compute_name_sort_key
 from models.user import User
@@ -318,6 +320,50 @@ def test_get_rom_content_missing_rom_returns_404(client: TestClient, access_toke
     assert response.status_code == status.HTTP_404_NOT_FOUND
 
 
+def _hide_rom_for_user(rom_id: int, user_id: int) -> None:
+    with sync_session.begin() as s:
+        s.add(HiddenEntity(entity=PermEntity.ROMS, entity_id=rom_id, user_id=user_id))
+
+
+def test_get_romfile_content_visible_rom(
+    client: TestClient, viewer_access_token: str, rom: Rom, rom_file
+):
+    # Baseline: a rom the viewer can see is downloadable by direct RomFile.id.
+    response = client.get(
+        f"/api/roms/{rom_file.id}/files/content/whatever.bin",
+        headers={"Authorization": f"Bearer {viewer_access_token}"},
+        follow_redirects=False,
+    )
+    assert response.status_code == status.HTTP_200_OK
+    assert "X-Accel-Redirect" in response.headers
+
+
+def test_get_romfile_content_hidden_rom_returns_404(
+    client: TestClient, viewer_access_token: str, viewer_user, rom: Rom, rom_file
+):
+    # A file belonging to a hidden rom must 404 even by direct RomFile.id,
+    # matching the ROM-level content endpoint (visibility-bypass regression).
+    _hide_rom_for_user(rom.id, viewer_user.id)
+    response = client.get(
+        f"/api/roms/{rom_file.id}/files/content/whatever.bin",
+        headers={"Authorization": f"Bearer {viewer_access_token}"},
+        follow_redirects=False,
+    )
+    assert response.status_code == status.HTTP_404_NOT_FOUND
+
+
+def test_get_romfile_hidden_rom_returns_404(
+    client: TestClient, viewer_access_token: str, viewer_user, rom: Rom, rom_file
+):
+    # The file metadata endpoint must not leak files of a hidden rom either.
+    _hide_rom_for_user(rom.id, viewer_user.id)
+    response = client.get(
+        f"/api/roms/{rom_file.id}/files",
+        headers={"Authorization": f"Bearer {viewer_access_token}"},
+    )
+    assert response.status_code == status.HTTP_404_NOT_FOUND
+
+
 @patch.object(FSRomsHandler, "rename_fs_rom")
 @patch.object(IGDBHandler, "get_rom_by_id", return_value=IGDBRom(igdb_id=None))
 def test_update_rom(