Use Cover Art Archive for albums with MusicBrainz IDs #12

Closed
sproutsberry wants to merge 11 commits from cover-art-archive into main
2 changed files with 168 additions and 20 deletions
Showing only changes of commit b9214fcc82 - Show all commits
+157 -19
View File
@@ -9,8 +9,11 @@ import (
"github.com/navidrome/navidrome/plugins/pdk/go/pdk" "github.com/navidrome/navidrome/plugins/pdk/go/pdk"
) )
// Configuration key for uguu.se image hosting const cacheKeyFormat = "artwork.%s"
const uguuEnabledKey = "uguuenabled"
// ============================================================================
// uguu.se
// ============================================================================
// uguu.se API response // uguu.se API response
type uguuResponse struct { type uguuResponse struct {
@@ -20,15 +23,6 @@ type uguuResponse struct {
} `json:"files"` } `json:"files"`
} }
// getImageURL retrieves the track artwork URL, optionally uploading to uguu.se.
func getImageURL(username, trackID string) string {
uguuEnabled, _ := pdk.GetConfig(uguuEnabledKey)
if uguuEnabled == "true" {
return getImageViaUguu(username, trackID)
}
return getImageDirect(trackID)
}
// getImageDirect returns the artwork URL directly from Navidrome (current behavior). // getImageDirect returns the artwork URL directly from Navidrome (current behavior).
func getImageDirect(trackID string) string { func getImageDirect(trackID string) string {
artworkURL, err := host.ArtworkGetTrackUrl(trackID, 300) artworkURL, err := host.ArtworkGetTrackUrl(trackID, 300)
@@ -46,14 +40,6 @@ func getImageDirect(trackID string) string {
// getImageViaUguu fetches artwork and uploads it to uguu.se. // getImageViaUguu fetches artwork and uploads it to uguu.se.
func getImageViaUguu(username, trackID string) string { func getImageViaUguu(username, trackID string) string {
// Check cache first
cacheKey := fmt.Sprintf("uguu.artwork.%s", trackID)
cachedURL, exists, err := host.CacheGetString(cacheKey)
if err == nil && exists {
pdk.Log(pdk.LogDebug, fmt.Sprintf("Cache hit for uguu artwork: %s", trackID))
return cachedURL
}
// Fetch artwork data from Navidrome // Fetch artwork data from Navidrome
contentType, data, err := host.SubsonicAPICallRaw(fmt.Sprintf("/getCoverArt?u=%s&id=%s&size=300", username, trackID)) contentType, data, err := host.SubsonicAPICallRaw(fmt.Sprintf("/getCoverArt?u=%s&id=%s&size=300", username, trackID))
if err != nil { if err != nil {
@@ -68,7 +54,9 @@ func getImageViaUguu(username, trackID string) string {
return "" return ""
gemini-code-assist[bot] commented 2026-02-08 16:36:51 -07:00 (Migrated from github.com)
Review

high

Caching an empty URL string prevents the fallback logic in getImageURL from working on subsequent requests for the same track. The cache should only be set if a valid URL is obtained.

    if url != "" {
        cacheKey := fmt.Sprintf(cacheKeyFormat, trackID)
        _ = host.CacheSetString(cacheKey, url, 9000)
    }

    return url

![high](https://www.gstatic.com/codereviewagent/high-priority.svg) Caching an empty URL string prevents the fallback logic in `getImageURL` from working on subsequent requests for the same track. The cache should only be set if a valid URL is obtained. ```go if url != "" { cacheKey := fmt.Sprintf(cacheKeyFormat, trackID) _ = host.CacheSetString(cacheKey, url, 9000) } return url ```
sproutsberry commented 2026-02-08 16:44:26 -07:00 (Migrated from github.com)
Review

Not relevant

Not relevant
} }
cacheKey := fmt.Sprintf(cacheKeyFormat, trackID)
_ = host.CacheSetString(cacheKey, url, 9000) _ = host.CacheSetString(cacheKey, url, 9000)
return url return url
} }
@@ -108,3 +96,153 @@ func uploadToUguu(imageData []byte, contentType string) (string, error) {
gemini-code-assist[bot] commented 2026-02-08 16:36:51 -07:00 (Migrated from github.com)
Review

high

Caching an empty URL string will prevent the fallback logic in getImageURL (to uguu.se or direct) from triggering on subsequent requests for the same track. The cache should only be set when a valid image URL is found.

    if url != "" {
        cacheKey := fmt.Sprintf(cacheKeyFormat, trackID)
        _ = host.CacheSetString(cacheKey, url, 86400)
    }

    return url

![high](https://www.gstatic.com/codereviewagent/high-priority.svg) Caching an empty URL string will prevent the fallback logic in `getImageURL` (to `uguu.se` or direct) from triggering on subsequent requests for the same track. The cache should only be set when a valid image URL is found. ```go if url != "" { cacheKey := fmt.Sprintf(cacheKeyFormat, trackID) _ = host.CacheSetString(cacheKey, url, 86400) } return url ```
gemini-code-assist[bot] commented 2026-02-08 16:36:51 -07:00 (Migrated from github.com)
Review

security-medium medium

The username and trackID variables are concatenated directly into the query string of the Subsonic API call without URL encoding. This makes the application vulnerable to HTTP Parameter Injection (HPP). An attacker could provide a malicious trackID (e.g., id1&u=admin) to inject additional parameters into the internal API request, potentially bypassing access controls or manipulating the API's behavior.

Note: You will need to import the net/url package to use url.QueryEscape.

data, err := host.SubsonicAPICall(fmt.Sprintf("getSong?u=%s&id=%s", url.QueryEscape(username), url.QueryEscape(trackID)))
![security-medium](https://www.gstatic.com/codereviewagent/security-medium-priority.svg) ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) The `username` and `trackID` variables are concatenated directly into the query string of the Subsonic API call without URL encoding. This makes the application vulnerable to HTTP Parameter Injection (HPP). An attacker could provide a malicious `trackID` (e.g., `id1&u=admin`) to inject additional parameters into the internal API request, potentially bypassing access controls or manipulating the API's behavior. Note: You will need to import the `net/url` package to use `url.QueryEscape`. ```suggestion data, err := host.SubsonicAPICall(fmt.Sprintf("getSong?u=%s&id=%s", url.QueryEscape(username), url.QueryEscape(trackID))) ```
gemini-code-assist[bot] commented 2026-02-08 16:36:51 -07:00 (Migrated from github.com)
Review

security-medium medium

Similar to the getSong call, the getAlbum API call is vulnerable to HTTP Parameter Injection because the username and albumID are not URL-encoded before being concatenated into the query string.

Note: You will need to import the net/url package to use url.QueryEscape.

data, err := host.SubsonicAPICall(fmt.Sprintf("getAlbum?u=%s&id=%s", url.QueryEscape(username), url.QueryEscape(albumID)))
![security-medium](https://www.gstatic.com/codereviewagent/security-medium-priority.svg) ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) Similar to the `getSong` call, the `getAlbum` API call is vulnerable to HTTP Parameter Injection because the `username` and `albumID` are not URL-encoded before being concatenated into the query string. Note: You will need to import the `net/url` package to use `url.QueryEscape`. ```suggestion data, err := host.SubsonicAPICall(fmt.Sprintf("getAlbum?u=%s&id=%s", url.QueryEscape(username), url.QueryEscape(albumID))) ```
gemini-code-assist[bot] commented 2026-02-08 16:36:51 -07:00 (Migrated from github.com)
Review

security-medium medium

The musicBrainzID is concatenated directly into the URL path for an external request to coverartarchive.org without any validation or escaping. If a malicious MusicBrainz ID containing path traversal sequences (e.g., ../../) is retrieved from the database (which can be populated from untrusted file tags), it could lead to a request being sent to an unintended endpoint on the target host, a form of Server-Side Request Forgery (SSRF).

Note: You will need to import the net/url package to use url.PathEscape.

req := pdk.NewHTTPRequest(pdk.MethodGet, fmt.Sprintf("https://coverartarchive.org/release/%s", url.PathEscape(musicBrainzID)))
![security-medium](https://www.gstatic.com/codereviewagent/security-medium-priority.svg) ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) The `musicBrainzID` is concatenated directly into the URL path for an external request to `coverartarchive.org` without any validation or escaping. If a malicious MusicBrainz ID containing path traversal sequences (e.g., `../../`) is retrieved from the database (which can be populated from untrusted file tags), it could lead to a request being sent to an unintended endpoint on the target host, a form of Server-Side Request Forgery (SSRF). Note: You will need to import the `net/url` package to use `url.PathEscape`. ```suggestion req := pdk.NewHTTPRequest(pdk.MethodGet, fmt.Sprintf("https://coverartarchive.org/release/%s", url.PathEscape(musicBrainzID))) ```
gemini-code-assist[bot] commented 2026-02-08 16:36:51 -07:00 (Migrated from github.com)
Review

medium

The field name Song is confusing here, as the JSON key is "album". For better code clarity and maintainability, it's recommended to rename this field to Album. You will also need to update the usage of this field in getMusicBrainzIDFromAlbumID. I've added a separate comment for that.

        Album struct {

![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) The field name `Song` is confusing here, as the JSON key is `"album"`. For better code clarity and maintainability, it's recommended to rename this field to `Album`. You will also need to update the usage of this field in `getMusicBrainzIDFromAlbumID`. I've added a separate comment for that. ```go Album struct { ```
gemini-code-assist[bot] commented 2026-02-08 16:36:51 -07:00 (Migrated from github.com)
Review

medium

To correspond with the suggested change of renaming the Song field to Album in subsonicGetAlbumResponse, this line should be updated to access the field via response.Data.Album.

    return response.Data.Album.MusicBrainzId, nil

![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) To correspond with the suggested change of renaming the `Song` field to `Album` in `subsonicGetAlbumResponse`, this line should be updated to access the field via `response.Data.Album`. ```go return response.Data.Album.MusicBrainzId, nil ```
gemini-code-assist[bot] commented 2026-02-08 16:36:51 -07:00 (Migrated from github.com)
Review

medium

The TODO comment here is misleading. In Go, a string variable cannot be nil, so checking for an empty string "" is the correct and sufficient way to see if it has a value. The TODO can be removed to avoid confusion.

    if musicBrainzID == "" {

![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) The `TODO` comment here is misleading. In Go, a string variable cannot be `nil`, so checking for an empty string `""` is the correct and sufficient way to see if it has a value. The TODO can be removed to avoid confusion. ```go if musicBrainzID == "" { ```
gemini-code-assist[bot] commented 2026-02-08 16:36:51 -07:00 (Migrated from github.com)
Review

medium

There's a TODO here to name this section. It's good practice to resolve these before merging. A descriptive name would improve code readability.

// Image URL Resolution

![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) There's a `TODO` here to name this section. It's good practice to resolve these before merging. A descriptive name would improve code readability. ```go // Image URL Resolution ```
sproutsberry commented 2026-02-08 16:44:34 -07:00 (Migrated from github.com)
Review

Intentional

Intentional
return result.Files[0].URL, nil return result.Files[0].URL, nil
} }
// ============================================================================
// Cover Art Archive
// ============================================================================
type subsonicGetSongResponse struct {
Data struct {
Song struct {
AlbumID string `json:"albumId"`
} `json:"song"`
} `json:"subsonic-response"`
}
type subsonicGetAlbumResponse struct {
Data struct {
Song struct {
MusicBrainzId string `json:"musicBrainzId"`
} `json:"album"`
} `json:"subsonic-response"`
}
// https://musicbrainz.org/doc/Cover_Art_Archive/API
type caaResponse struct {
Images []struct {
Front bool `json:"front"`
Back bool `json:"back"`
ImageURL string `json:"image"`
ThumbnailImageURLs struct {
Size250 string `json:"250"`
Size500 string `json:"500"`
Size1200 string `json:"1200"`
Small string `json:"small"` // deprecated; use 250
Large string `json:"large"` // deprecated; use 500
} `json:"thumbnails"`
} `json:"images"`
ReleaseURL string `json:"release"`
}
func getAlbumIDFromTrackID(username, trackID string) (string, error) {
data, err := host.SubsonicAPICall(fmt.Sprintf("getSong?u=%s&id=%s", username, trackID))
if err != nil {
return "", fmt.Errorf("getSong failed: %w", err)
}
var response subsonicGetSongResponse
if err := json.Unmarshal([]byte(data), &response); err != nil {
return "", fmt.Errorf("failed to parse getSong response: %w", err)
}
return response.Data.Song.AlbumID, nil
}
func getMusicBrainzIDFromAlbumID(username, albumID string) (string, error) {
data, err := host.SubsonicAPICall(fmt.Sprintf("getAlbum?u=%s&id=%s", username, albumID))
if err != nil {
return "", fmt.Errorf("getAlbum failed: %w", err)
}
var response subsonicGetAlbumResponse
if err := json.Unmarshal([]byte(data), &response); err != nil {
return "", fmt.Errorf("failed to parse getAlbum response: %w", err)
}
return response.Data.Song.MusicBrainzId, nil
}
func fetchImageFromCAA(username, trackID string) (string, error) {
albumID, err := getAlbumIDFromTrackID(username, trackID)
if err != nil {
return "", fmt.Errorf("failed to get album ID from track ID %s: %w", trackID, err)
}
musicBrainzID, err := getMusicBrainzIDFromAlbumID(username, albumID)
if err != nil {
return "", fmt.Errorf("failed to get MusicBrainz ID from album ID %s: %w", trackID, err)
}
if musicBrainzID == "" { // TODO: Check for nil as well
pdk.Log(pdk.LogDebug, fmt.Sprintf("No MusicBrainz ID for album %s", albumID))
return "", nil
}
req := pdk.NewHTTPRequest(pdk.MethodGet, fmt.Sprintf("https://coverartarchive.org/release/%s", musicBrainzID))
resp := req.Send()
status := resp.Status()
if status == 404 {
pdk.Log(pdk.LogDebug, fmt.Sprintf("No cover art for MusicBrainz ID %s", musicBrainzID))
return "", nil
}
if status >= 400 {
return "", fmt.Errorf("Cover Art Archive request failed: HTTP %d", resp.Status())
}
var result caaResponse
if err := json.Unmarshal(resp.Body(), &result); err != nil {
return "", fmt.Errorf("failed to parse Cover Art Archive response: %w", err)
}
for _, image := range result.Images {
if image.Front {
return image.ThumbnailImageURLs.Size250, nil
}
}
pdk.Log(pdk.LogDebug, fmt.Sprintf("No viable cover art for MusicBrainz ID %s (%d images)", musicBrainzID, len(result.Images)))
return "", nil
}
func getImageViaCAA(username, trackID string) string {
url, err := fetchImageFromCAA(username, trackID)
if err != nil {
pdk.Log(pdk.LogWarn, fmt.Sprintf("Failed to get image from Cover Art archive: %v", err))
return ""
}
cacheKey := fmt.Sprintf(cacheKeyFormat, trackID)
_ = host.CacheSetString(cacheKey, url, 86400)
return url
}
// ============================================================================
// TODO: name this section
// ============================================================================
const uguuEnabledKey = "uguuenabled"
const caaEnabledKey = "caaenabled"
func getImageURL(username, trackID string) string {
cacheKey := fmt.Sprintf(cacheKeyFormat, trackID)
cachedURL, exists, err := host.CacheGetString(cacheKey)
if err == nil && exists {
pdk.Log(pdk.LogDebug, fmt.Sprintf("Cache hit for artwork: %s", trackID))
return cachedURL
}
caaEnabled, _ := pdk.GetConfig(caaEnabledKey)
if caaEnabled == "true" {
if url := getImageViaCAA(username, trackID); url != "" {
return url
}
}
uguuEnabled, _ := pdk.GetConfig(uguuEnabledKey)
if uguuEnabled == "true" {
return getImageViaUguu(username, trackID)
}
return getImageDirect(trackID)
}
+11 -1
View File
@@ -13,7 +13,8 @@
"reason": "To communicate with Discord API for gateway discovery and image uploads", "reason": "To communicate with Discord API for gateway discovery and image uploads",
"requiredHosts": [ "requiredHosts": [
"discord.com", "discord.com",
"uguu.se" "uguu.se",
"coverartarchive.org"
] ]
}, },
"websocket": { "websocket": {
@@ -64,6 +65,11 @@
"title": "Upload artwork to uguu.se (enable if Navidrome is not publicly accessible)", "title": "Upload artwork to uguu.se (enable if Navidrome is not publicly accessible)",
deluan commented 2026-02-09 12:49:21 -07:00 (Migrated from github.com)
Review

The default should be false.

The default should be false.
"default": false "default": false
}, },
"caaenabled": {
"type": "boolean",
"title": "Use artwork from Cover Art Archive when available",
"default": true
},
"users": { "users": {
"type": "array", "type": "array",
"title": "User Tokens", "title": "User Tokens",
@@ -111,6 +117,10 @@
"format": "radio" "format": "radio"
} }
}, },
{
"type": "Control",
"scope": "#/properties/caaenabled"
},
{ {
"type": "Control", "type": "Control",
"scope": "#/properties/uguuenabled" "scope": "#/properties/uguuenabled"