fix(gallery): fail closed for null-user owner scope (#3613)

routes/gallery_helpers.py

@@ -11,6 +11,7 @@ from typing import Dict, Any, Optional
 from pydantic import BaseModel
 
 from core.database import GalleryImage
+from src.auth_helpers import _auth_disabled
 
 logger = logging.getLogger(__name__)
 
@@ -120,19 +121,18 @@ def _image_to_dict(img: GalleryImage, session_name: str = None) -> Dict[str, Any
     }
 
 
-def _owner_filter(q, user):
+def _owner_filter(q, user, model_cls=GalleryImage):
     """Apply owner filtering to a gallery query.
 
-    When auth is disabled (single-user mode) get_current_user returns None
-    and there is no per-user scoping. The main library list and stats already
-    treat None as "show everything" (`if user is not None`), so this helper
-    must too — otherwise the tag/model filter sidebars come back empty and the
-    tag-cleanup endpoints (clear-user-tags, clear-ai-tags, dedupe-tags)
-    silently affect zero rows in the most common self-hosted deployment.
+    ``get_current_user`` returns None both in auth-disabled single-user mode
+    and when auth is enabled but no current user was resolved. Preserve the
+    single-user behavior, but fail closed for auth-enabled null-user states.
     """
-    if user is None:
+    if user is not None:
+        return q.filter(model_cls.owner == user)
+    if _auth_disabled():
         return q
-    return q.filter(GalleryImage.owner == user)
+    return q.filter(False)
 
 
 

routes/gallery_routes.py

@@ -476,8 +476,7 @@ def setup_gallery_routes() -> APIRouter:
                 .outerjoin(DbSession, GalleryImage.session_id == DbSession.id)
                 .filter(GalleryImage.is_active == True)
             )
-            if user is not None:
-                q = q.filter(GalleryImage.owner == user)
+            q = _owner_filter(q, user)
 
             # Search filter (prompt + tags + ai_tags)
             if search:
@@ -579,28 +578,26 @@ def setup_gallery_routes() -> APIRouter:
         db = SessionLocal()
         try:
             q = db.query(GalleryAlbum)
-            if user:
-                q = q.filter(GalleryAlbum.owner == user)
+            q = _owner_filter(q, user, GalleryAlbum)
             albums = q.order_by(GalleryAlbum.created_at.desc()).all()
             result = []
             for a in albums:
                 _count_q = db.query(GalleryImage).filter(
                     GalleryImage.album_id == a.id, GalleryImage.is_active == True
                 )
-                if user:
-                    _count_q = _count_q.filter(GalleryImage.owner == user)
+                _count_q = _owner_filter(_count_q, user)
                 count = _count_q.count()
                 cover_url = None
                 if a.cover_id:
-                    cover = db.query(GalleryImage).filter(GalleryImage.id == a.cover_id).first()
+                    cover_q = db.query(GalleryImage).filter(GalleryImage.id == a.cover_id)
+                    cover = _owner_filter(cover_q, user).first()
                     if cover:
                         cover_url = f"/api/generated-image/{cover.filename}"
                 elif count > 0:
                     _cover_q = db.query(GalleryImage).filter(
                         GalleryImage.album_id == a.id, GalleryImage.is_active == True
                     )
-                    if user:
-                        _cover_q = _cover_q.filter(GalleryImage.owner == user)
+                    _cover_q = _owner_filter(_cover_q, user)
                     first = _cover_q.order_by(GalleryImage.created_at.desc()).first()
                     if first:
                         cover_url = f"/api/generated-image/{first.filename}"
@@ -643,10 +640,9 @@ def setup_gallery_routes() -> APIRouter:
             base = db.query(GalleryImage).filter(GalleryImage.is_active == True)
             size_q = db.query(func.sum(GalleryImage.file_size)).filter(GalleryImage.is_active == True)
             album_q = db.query(GalleryAlbum)
-            if user:
-                base = base.filter(GalleryImage.owner == user)
-                size_q = size_q.filter(GalleryImage.owner == user)
-                album_q = album_q.filter(GalleryAlbum.owner == user)
+            base = _owner_filter(base, user)
+            size_q = _owner_filter(size_q, user)
+            album_q = _owner_filter(album_q, user, GalleryAlbum)
             total = base.count()
             total_size = size_q.scalar() or 0
             fav_count = base.filter(GalleryImage.favorite == True).count()
@@ -674,8 +670,7 @@ def setup_gallery_routes() -> APIRouter:
                 GalleryImage.is_active == True,
                 (GalleryImage.ai_tags == None) | (GalleryImage.ai_tags == ""),
             )
-            if user:
-                q = q.filter(GalleryImage.owner == user)
+            q = _owner_filter(q, user)
             if album_id:
                 q = q.filter(GalleryImage.album_id == album_id)
             untagged = q.count()

tests/test_gallery_album_owner_scope.py

@@ -40,9 +40,12 @@ def test_upload_validates_target_album_ownership():
 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
+    # The album list, per-album image count, explicit cover, and cover-fallback
+    # queries should all share the same gallery owner policy.
+    assert "q = _owner_filter(q, user, GalleryAlbum)" in body
+    assert "_count_q = _owner_filter(_count_q, user)" in body
+    assert "cover = _owner_filter(cover_q, user).first()" in body
+    assert "_cover_q = _owner_filter(_cover_q, user)" in body
 
 
 def test_delete_album_cleanup_is_owner_scoped():

tests/test_gallery_null_user_routes.py

@@ -0,0 +1,149 @@
+import uuid
+
+from fastapi import FastAPI
+from fastapi.testclient import TestClient
+from sqlalchemy import create_engine
+from sqlalchemy.orm import sessionmaker
+from sqlalchemy.pool import NullPool
+
+import core.database as cdb
+from core.database import GalleryAlbum, GalleryImage
+import routes.gallery_routes as gallery_routes
+
+
+def _client_with_gallery(monkeypatch, tmp_path):
+    engine = create_engine(
+        f"sqlite:///{tmp_path / 'gallery.db'}",
+        connect_args={"check_same_thread": False},
+        poolclass=NullPool,
+    )
+    cdb.Base.metadata.create_all(engine)
+    session_factory = sessionmaker(bind=engine, autoflush=False, autocommit=False)
+    monkeypatch.setattr(gallery_routes, "SessionLocal", session_factory)
+
+    db = session_factory()
+    try:
+        db.add_all(
+            [
+                GalleryAlbum(id="album-alice", name="Alice album", owner="alice"),
+                GalleryAlbum(id="album-bob", name="Bob album", owner="bob"),
+                GalleryImage(
+                    id="img-alice",
+                    filename=f"{uuid.uuid4().hex}.png",
+                    prompt="alice prompt",
+                    model="model-a",
+                    tags="alice-tag",
+                    ai_tags="",
+                    owner="alice",
+                    album_id="album-alice",
+                    is_active=True,
+                    file_size=10,
+                ),
+                GalleryImage(
+                    id="img-bob",
+                    filename=f"{uuid.uuid4().hex}.png",
+                    prompt="bob prompt",
+                    model="model-b",
+                    tags="bob-tag",
+                    ai_tags="",
+                    owner="bob",
+                    album_id="album-bob",
+                    is_active=True,
+                    file_size=20,
+                ),
+            ]
+        )
+        db.commit()
+    finally:
+        db.close()
+
+    app = FastAPI()
+    app.include_router(gallery_routes.setup_gallery_routes())
+    return TestClient(app)
+
+
+def test_auth_enabled_null_user_gallery_routes_fail_closed(monkeypatch, tmp_path):
+    monkeypatch.setenv("AUTH_ENABLED", "true")
+    client = _client_with_gallery(monkeypatch, tmp_path)
+
+    library = client.get("/api/gallery/library").json()
+    assert library["items"] == []
+    assert library["total"] == 0
+    assert library["total_tagged"] == 0
+    assert library["tags"] == []
+    assert library["models"] == []
+
+    shuffled = client.get("/api/gallery/library", params={"sort": "shuffle"}).json()
+    assert shuffled["items"] == []
+    assert shuffled["total"] == 0
+
+    assert client.get("/api/gallery/tags").json() == {"tags": []}
+    assert client.get("/api/gallery/albums").json() == {"albums": []}
+    assert client.get("/api/gallery/stats").json() == {
+        "total_photos": 0,
+        "total_size": 0,
+        "total_size_human": "0.0 B",
+        "favorites": 0,
+        "albums": 0,
+    }
+    assert client.post("/api/gallery/ai-tag-batch").json() == {
+        "ok": True,
+        "queued": 0,
+        "total_untagged": 0,
+        "image_ids": [],
+    }
+
+
+def test_auth_disabled_null_user_gallery_routes_keep_single_user_mode(monkeypatch, tmp_path):
+    monkeypatch.setenv("AUTH_ENABLED", "false")
+    client = _client_with_gallery(monkeypatch, tmp_path)
+
+    library = client.get("/api/gallery/library").json()
+    assert {item["id"] for item in library["items"]} == {"img-alice", "img-bob"}
+    assert library["total"] == 2
+    assert library["tags"] == ["alice-tag", "bob-tag"]
+    assert library["models"] == ["model-a", "model-b"]
+
+    assert client.get("/api/gallery/tags").json() == {"tags": ["alice-tag", "bob-tag"]}
+    assert len(client.get("/api/gallery/albums").json()["albums"]) == 2
+    assert client.get("/api/gallery/stats").json() == {
+        "total_photos": 2,
+        "total_size": 30,
+        "total_size_human": "30.0 B",
+        "favorites": 0,
+        "albums": 2,
+    }
+    batch = client.post("/api/gallery/ai-tag-batch").json()
+    assert batch["ok"] is True
+    assert batch["queued"] == 2
+    assert batch["total_untagged"] == 2
+    assert set(batch["image_ids"]) == {"img-alice", "img-bob"}
+
+
+def test_authenticated_gallery_routes_remain_owner_scoped(monkeypatch, tmp_path):
+    monkeypatch.setenv("AUTH_ENABLED", "true")
+    monkeypatch.setattr(gallery_routes, "get_current_user", lambda request: "alice")
+    client = _client_with_gallery(monkeypatch, tmp_path)
+
+    library = client.get("/api/gallery/library").json()
+    assert [item["id"] for item in library["items"]] == ["img-alice"]
+    assert library["total"] == 1
+    assert library["tags"] == ["alice-tag"]
+    assert library["models"] == ["model-a"]
+
+    assert client.get("/api/gallery/tags").json() == {"tags": ["alice-tag"]}
+    albums = client.get("/api/gallery/albums").json()["albums"]
+    assert [album["id"] for album in albums] == ["album-alice"]
+    assert client.get("/api/gallery/stats").json() == {
+        "total_photos": 1,
+        "total_size": 10,
+        "total_size_human": "10.0 B",
+        "favorites": 0,
+        "albums": 1,
+    }
+    assert client.post("/api/gallery/ai-tag-batch").json() == {
+        "ok": True,
+        "queued": 1,
+        "total_untagged": 1,
+        "image_ids": ["img-alice"],
+    }

tests/test_gallery_owner_filter_single_user.py

@@ -1,11 +1,8 @@
-"""_owner_filter must not blank out the gallery in single-user mode.
+"""_owner_filter must separate single-user mode from anonymous callers.
 
-When AUTH_ENABLED=false, get_current_user returns None. The gallery main
-list and stats treat None as "show all images" (`if user is not None`), but
-_owner_filter returned q.filter(False) (zero rows) for None. So the tag and
-model filter chips were always empty and clear-user-tags / clear-ai-tags /
-dedupe-tags silently no-oped. _owner_filter must match the main list: no
-filter when user is None, owner-scoped otherwise.
+When AUTH_ENABLED=false, get_current_user returns None and gallery routes should
+stay all-visible. When AUTH_ENABLED=true and no current user resolves, the same
+None means an anonymous caller and gallery queries must fail closed.
 """
 import tempfile
 import uuid
@@ -36,7 +33,8 @@ def _seed(*owners):
         db.close()
 
 
-def test_none_user_returns_all_rows():
+def test_none_user_returns_all_rows(monkeypatch):
+    monkeypatch.setenv("AUTH_ENABLED", "false")
     _seed(None, None, "alice")
     db = _TS()
     try:
@@ -54,3 +52,13 @@ def test_named_user_is_still_scoped():
         assert _owner_filter(db.query(GalleryImage), "bob").count() == 1
     finally:
         db.close()
+
+
+def test_none_user_blocks_when_auth_is_enabled(monkeypatch):
+    monkeypatch.setenv("AUTH_ENABLED", "true")
+    _seed(None, "alice", "bob")
+    db = _TS()
+    try:
+        assert _owner_filter(db.query(GalleryImage), None).count() == 0
+    finally:
+        db.close()

tests/test_null_owner_gates.py

@@ -153,11 +153,20 @@ def test_document_owner_filter_applies_owner_clause():
 # gallery._owner_filter
 # ---------------------------------------------------------------------------
 
-def test_gallery_owner_filter_allows_single_user_mode():
+def test_gallery_owner_filter_blocks_anonymous(monkeypatch):
+    monkeypatch.setenv("AUTH_ENABLED", "true")
+    from routes.gallery_routes import _owner_filter
+    fake_q = MagicMock()
+    out = _owner_filter(fake_q, user=None)
+    fake_q.filter.assert_called_once_with(False)
+    assert out is fake_q.filter.return_value
+
+
+def test_gallery_owner_filter_allows_single_user_mode(monkeypatch):
+    monkeypatch.setenv("AUTH_ENABLED", "false")
     from routes.gallery_routes import _owner_filter
     fake_q = MagicMock()
     out = _owner_filter(fake_q, user=None)
-    # user=None means single-user/auth-disabled mode: return q unchanged, no filter.
     fake_q.filter.assert_not_called()
     assert out is fake_q