diff --git a/routes/gallery_routes.py b/routes/gallery_routes.py index d8d52b2ba..ce6f6271b 100644 --- a/routes/gallery_routes.py +++ b/routes/gallery_routes.py @@ -526,18 +526,24 @@ def setup_gallery_routes() -> APIRouter: albums = q.order_by(GalleryAlbum.created_at.desc()).all() result = [] for a in albums: - count = db.query(GalleryImage).filter( + _count_q = db.query(GalleryImage).filter( GalleryImage.album_id == a.id, GalleryImage.is_active == True - ).count() + ) + if user: + _count_q = _count_q.filter(GalleryImage.owner == user) + count = _count_q.count() cover_url = None if a.cover_id: cover = db.query(GalleryImage).filter(GalleryImage.id == a.cover_id).first() if cover: cover_url = f"/api/generated-image/{cover.filename}" elif count > 0: - first = db.query(GalleryImage).filter( + _cover_q = db.query(GalleryImage).filter( GalleryImage.album_id == a.id, GalleryImage.is_active == True - ).order_by(GalleryImage.created_at.desc()).first() + ) + if user: + _cover_q = _cover_q.filter(GalleryImage.owner == user) + first = _cover_q.order_by(GalleryImage.created_at.desc()).first() if first: cover_url = f"/api/generated-image/{first.filename}" result.append({ @@ -670,7 +676,14 @@ def setup_gallery_routes() -> APIRouter: if req.favorite is not None: img.favorite = req.favorite if req.album_id is not None: - img.album_id = req.album_id if req.album_id else None + if req.album_id: + # Validate the target album belongs to the caller before + # moving the image into it — mirrors add_to_album, so you + # cannot file your image into another user's album. + _get_or_404_album(db, req.album_id, user) + img.album_id = req.album_id + else: + img.album_id = None db.commit() db.refresh(img) return _image_to_dict(img) diff --git a/tests/test_gallery_album_owner_scope.py b/tests/test_gallery_album_owner_scope.py new file mode 100644 index 000000000..eafc0a182 --- /dev/null +++ b/tests/test_gallery_album_owner_scope.py @@ -0,0 +1,45 @@ +"""Issue #2754 — gallery owner-scoping. + +`patch_gallery_image` must validate that the *target album* belongs to the caller +before moving an image into it (otherwise user B can file B's image into user A's +album), and `list_albums` must owner-scope the per-album count + cover-fallback +queries. The gallery route handlers are closures, so — matching the AST-assertion +convention of test_gallery_image_privileges.py — we assert the guards are present +in the source. +""" +import ast +from pathlib import Path + + +def _function_sources(): + source = Path("routes/gallery_routes.py").read_text(encoding="utf-8") + tree = ast.parse(source) + return { + node.name: ast.get_source_segment(source, node) or "" + for node in ast.walk(tree) + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) + } + + +def test_patch_validates_target_album_ownership(): + fns = _function_sources() + body = fns["patch_gallery_image"] + assert "req.album_id" in body + # The target album must be ownership-validated (via the same helper the + # sibling mutators use) before the image is reassigned to it. + assert "_get_or_404_album(db, req.album_id, user)" in body + + +def test_list_albums_count_and_cover_are_owner_scoped(): + fns = _function_sources() + body = fns["list_albums"] + # Both the per-album image count and the cover-fallback query must owner-scope + # by GalleryImage.owner (the album list itself already filters by owner). + assert body.count("GalleryImage.owner == user") >= 2 + + +def test_get_or_404_album_enforces_owner(): + # Guard the precedent we rely on: the helper rejects another user's album. + fns = _function_sources() + helper = fns["_get_or_404_album"] + assert "album.owner != user" in helper