Harden gallery album assignment scope (#3004)

routes/gallery_routes.py

@@ -76,6 +76,9 @@ def setup_gallery_routes() -> APIRouter:
         file_hash = hashlib.sha256(content).hexdigest()
         db = SessionLocal()
         try:
+            if album_id and user is not None:
+                _get_or_404_album(db, album_id, user)
+
             # SECURITY: scope the dup-detect to THIS user — otherwise a
             # caller can probe whether someone else uploaded the same
             # file (the response leaks the existing row's id+filename).
@@ -1669,9 +1672,10 @@ def setup_gallery_routes() -> APIRouter:
         db = SessionLocal()
         try:
             album = _get_or_404_album(db, album_id, user)
-            db.query(GalleryImage).filter(GalleryImage.album_id == album_id).update(
-                {"album_id": None}, synchronize_session=False
-            )
+            q = db.query(GalleryImage).filter(GalleryImage.album_id == album_id)
+            if user is not None:
+                q = q.filter(GalleryImage.owner == user)
+            q.update({"album_id": None}, synchronize_session=False)
             db.delete(album)
             db.commit()
             return {"ok": True}

tests/test_gallery_album_owner_scope.py

@@ -30,6 +30,13 @@ def test_patch_validates_target_album_ownership():
     assert "_get_or_404_album(db, req.album_id, user)" in body
 
 
+def test_upload_validates_target_album_ownership():
+    fns = _function_sources()
+    body = fns["gallery_upload"]
+    assert "album_id" in body
+    assert "_get_or_404_album(db, album_id, user)" in body
+
+
 def test_list_albums_count_and_cover_are_owner_scoped():
     fns = _function_sources()
     body = fns["list_albums"]
@@ -38,6 +45,14 @@ def test_list_albums_count_and_cover_are_owner_scoped():
     assert body.count("GalleryImage.owner == user") >= 2
 
 
+def test_delete_album_cleanup_is_owner_scoped():
+    fns = _function_sources()
+    body = fns["delete_album"]
+    assert "GalleryImage.album_id == album_id" in body
+    assert "GalleryImage.owner == user" in body
+    assert 'q.update({"album_id": None}' in body
+
+
 def test_get_or_404_album_enforces_owner():
     # Guard the precedent we rely on: the helper rejects another user's album.
     fns = _function_sources()