Fix #766: Navidrome album covers blank in the sync editor (+ other modals)

The sync editor renders server covers as <img src="/api/navidrome/cover/{id}">,
but no Flask route ever served that path — so every Navidrome cover 404'd, on
every album, art or not. The source (left) side then went blank too: a source
row with no native art (e.g. YouTube, which provides none at mirror time) falls
back to borrowing the matched server track's cover — i.e. that same dead route.
So both sides collapsed to nothing.

Fix:
- New NavidromeClient.build_cover_art_url(cover_id) — builds the absolute,
  Subsonic-authenticated getCoverArt URL (base_url + token/salt), keeping
  credentials server-side. Uses a FIXED cover-art salt so the URL is
  deterministic for a given (server, password, cover_id): a rotating salt (as
  in _generate_auth_params) would make every request a unique URL → image-cache
  miss every time + a dead, never-reused cache row per fetch. Token auth doesn't
  require a unique salt, and the password is never exposed (only its salted md5).
- New route /api/navidrome/cover/<cover_id> — resolves that URL and streams the
  image through the shared image cache (same pattern as /api/image-proxy), with
  a private max-age so the browser caches by the stable route URL.

Effect: server side works for any album that has art in Navidrome; matched
source rows with no native art now borrow the (now-working) server cover.
Unmatched YouTube rows stay blank — no image exists anywhere to show.

Tests: tests/test_navidrome_cover_url.py (8) — URL structure + salted-token auth
(never the raw password), determinism (same id -> same URL so the cache hits;
different id/password -> different URL), optional size, and the not-connected /
no-id / no-credentials guards.

Caveats: not executed against a live Navidrome (no server in CI) — the URL
builder is unit-tested; the route's cache→HTTP→bytes round-trip is read-verified
only. Scope is the sync editor's Navidrome route; Plex/Jellyfin server-cover
branches and any other modals using a different mechanism are untouched.

core/navidrome_client.py

@@ -3,6 +3,7 @@ import hashlib
 import secrets
 from typing import List, Optional, Dict, Any
 from datetime import datetime
+from urllib.parse import urlencode
 import json
 from utils.logging_config import get_logger
 from config.settings import config_manager
@@ -316,6 +317,42 @@ class NavidromeClient(MediaServerClient):
             'f': 'json'  # Response format
         }
 
+    # Fixed salt for cover-art URLs ONLY. Subsonic token auth (t=md5(password
+    # +salt), s=salt) does not require a unique salt per request, and a stable
+    # one makes the cover URL deterministic — so the image cache and the
+    # browser cache actually HIT. The rotating salt from _generate_auth_params
+    # would make every request a unique URL → cache miss every time + a dead,
+    # never-reused cache row per fetch (#766 review). The password is never
+    # exposed either way (only its salted md5).
+    _COVER_ART_SALT = 'soulsync-cover'
+
+    def build_cover_art_url(self, cover_id, size=None) -> Optional[str]:
+        """Absolute, Subsonic-authenticated getCoverArt URL for ``cover_id``.
+
+        Deterministic for a given (server, password, cover_id) so it caches.
+        The web layer proxies this to the browser (sync editor + modals).
+        Returns ``None`` when not connected or no id was supplied. #766: the
+        ``/api/navidrome/cover/<id>`` route had no working URL behind it, so
+        every Navidrome cover came back blank."""
+        if not self.base_url or not cover_id:
+            return None
+        if not self.username or not self.password:
+            return None
+        salt = self._COVER_ART_SALT
+        token = hashlib.md5((self.password + salt).encode()).hexdigest()
+        params = {
+            'u': self.username,
+            't': token,
+            's': salt,
+            'v': '1.16.1',
+            'c': 'SoulSync',
+            'f': 'json',  # harmless for getCoverArt — it returns image binary
+            'id': str(cover_id),
+        }
+        if size:
+            params['size'] = str(size)
+        return f"{self.base_url}/rest/getCoverArt?{urlencode(params)}"
+
     # Subsonic endpoints that modify data — use POST to avoid URL length limits
     _WRITE_ENDPOINTS = frozenset({
         'createPlaylist', 'updatePlaylist', 'deletePlaylist',

tests/test_navidrome_cover_url.py

@@ -0,0 +1,83 @@
+"""Tests for Navidrome cover-art URL building (#766).
+
+The sync editor + modals referenced /api/navidrome/cover/<id> but no route
+served it, and the URL behind it had to be a fully-authenticated Subsonic
+getCoverArt URL. build_cover_art_url is that builder — these pin its shape and
+the not-connected guards (the token/salt are random per call, so we assert
+structure + required params rather than an exact string).
+"""
+
+from __future__ import annotations
+
+from urllib.parse import parse_qs, urlsplit
+
+from core.navidrome_client import NavidromeClient
+
+
+def _connected_client():
+    c = NavidromeClient()
+    c.base_url = "https://nav.example.com"
+    c.username = "boulder"
+    c.password = "hunter2"
+    return c
+
+
+def test_builds_authenticated_cover_url():
+    url = _connected_client().build_cover_art_url("al-123")
+    parts = urlsplit(url)
+    assert parts.scheme == "https"
+    assert parts.netloc == "nav.example.com"
+    assert parts.path == "/rest/getCoverArt"
+    q = parse_qs(parts.query)
+    assert q["id"] == ["al-123"]
+    assert q["u"] == ["boulder"]
+    # Subsonic token auth: salted md5, never the raw password.
+    assert q["t"] and q["t"][0] != "hunter2"
+    assert q["s"]            # salt present
+    assert "hunter2" not in url
+    for required in ("t", "s", "v", "c"):
+        assert required in q
+
+
+def test_cover_url_is_deterministic_so_the_cache_hits():
+    # #766 review: the URL must be stable for a given (server, password,
+    # cover_id) — otherwise the image cache keys on a rotating salt and misses
+    # every request, re-fetching Navidrome each time + leaking dead rows.
+    c = _connected_client()
+    assert c.build_cover_art_url("al-123") == c.build_cover_art_url("al-123")
+    # ...and different covers still produce different URLs.
+    assert c.build_cover_art_url("al-123") != c.build_cover_art_url("al-999")
+
+
+def test_cover_url_changes_with_password():
+    # A password change must invalidate the cached URL (new token).
+    c1 = _connected_client()
+    c2 = _connected_client()
+    c2.password = "different"
+    assert c1.build_cover_art_url("al-1") != c2.build_cover_art_url("al-1")
+
+
+def test_size_param_optional():
+    assert "size" not in (_connected_client().build_cover_art_url("x") or "")
+    assert "size=300" in _connected_client().build_cover_art_url("x", size=300)
+
+
+def test_cover_id_is_stringified():
+    url = _connected_client().build_cover_art_url(12345)
+    assert "id=12345" in url
+
+
+def test_returns_none_when_not_connected():
+    c = NavidromeClient()  # base_url is None
+    assert c.build_cover_art_url("al-1") is None
+
+
+def test_returns_none_for_empty_cover_id():
+    assert _connected_client().build_cover_art_url("") is None
+    assert _connected_client().build_cover_art_url(None) is None
+
+
+def test_returns_none_without_credentials():
+    c = NavidromeClient()
+    c.base_url = "https://nav.example.com"  # but no username/password
+    assert c.build_cover_art_url("al-1") is None

web_server.py

@@ -4133,6 +4133,33 @@ def select_jellyfin_music_library():
         logger.error(f"Error setting Jellyfin music library: {e}")
         return jsonify({"success": False, "error": str(e)}), 500
 
+@app.route('/api/navidrome/cover/<cover_id>', methods=['GET'])
+def navidrome_cover(cover_id):
+    """Proxy a Navidrome (Subsonic) cover-art image to the browser.
+
+    The sync editor and other modals reference /api/navidrome/cover/<id>,
+    but no route served it — so every Navidrome cover came back blank (#766).
+    We build the authenticated getCoverArt URL server-side (keeping Subsonic
+    credentials off the client) and stream it through the shared image cache.
+    """
+    try:
+        client = media_server_engine.client('navidrome')
+        if not client:
+            return '', 404
+        url = client.build_cover_art_url(cover_id)
+        if not url:
+            return '', 404
+        from core.image_cache import get_image_cache
+        cached = get_image_cache().get_url(url)
+        response = send_file(cached.path, mimetype=cached.mime_type, conditional=True)
+        max_age = int(config_manager.get("image_cache.ttl_seconds", 2592000))
+        response.headers['Cache-Control'] = f'private, max-age={max_age}'
+        return response
+    except Exception as exc:
+        logger.debug("navidrome cover proxy failed for %s: %s", cover_id, exc)
+        return '', 502
+
+
 @app.route('/api/navidrome/music-folders', methods=['GET'])
 def get_navidrome_music_folders():
     """Get list of available music folders from Navidrome"""