From c12472bd19449db025dc3f633812069dbc4ff266 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Fri, 30 May 2025 08:29:36 -0400 Subject: [PATCH 01/19] fix(ui): update song fetching logic to disable for radio (#4149) Signed-off-by: Deluan --- ui/src/audioplayer/PlayerToolbar.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/audioplayer/PlayerToolbar.jsx b/ui/src/audioplayer/PlayerToolbar.jsx index 5230b30f2..4812141ab 100644 --- a/ui/src/audioplayer/PlayerToolbar.jsx +++ b/ui/src/audioplayer/PlayerToolbar.jsx @@ -57,7 +57,7 @@ const useStyles = makeStyles((theme) => ({ const PlayerToolbar = ({ id, isRadio }) => { const dispatch = useDispatch() - const { data, loading } = useGetOne('song', id, { enabled: !!id }) + const { data, loading } = useGetOne('song', id, { enabled: !!id && !isRadio }) const [toggleLove, toggling] = useToggleLove('song', data) const isDesktop = useMediaQuery('(min-width:810px)') const classes = useStyles() From 920800e909898ddfbc67a69465185bf5ab2ebde1 Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 30 May 2025 16:18:07 -0400 Subject: [PATCH 02/19] fix(ui): restructure AboutDialog's version notification layout Signed-off-by: Deluan --- ui/src/dialogs/AboutDialog.jsx | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/ui/src/dialogs/AboutDialog.jsx b/ui/src/dialogs/AboutDialog.jsx index 4f074002b..c220784a8 100644 --- a/ui/src/dialogs/AboutDialog.jsx +++ b/ui/src/dialogs/AboutDialog.jsx @@ -73,12 +73,16 @@ const ShowVersion = ({ uiVersion, serverVersion }) => { UI {translate('menu.version')}: - - window.location.reload()}> - - {' ' + translate('ra.notification.new_version')} - - +
+ +
+
+ window.location.reload()}> + + {translate('ra.notification.new_version')} + + +
)} From 623919f53e753ed3ba1dedc02c47ba1ef4aa3588 Mon Sep 17 00:00:00 2001 From: Kevian <149390935+Keviannn@users.noreply.github.com> Date: Fri, 30 May 2025 23:19:04 +0200 Subject: [PATCH 03/19] fix(ui): update Spanish translation (#4146) Changed translation of "Top Rated" from "Los Mejores Calificados" to "Mejor Calificados" for consistency purposes with other list entries. While the previous version was correct, this version is shorter and aligns better with the rest of the terms. --- resources/i18n/es.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/i18n/es.json b/resources/i18n/es.json index 2fdbb8fda..b640ec115 100644 --- a/resources/i18n/es.json +++ b/resources/i18n/es.json @@ -94,7 +94,7 @@ "recentlyPlayed": "Recientes", "mostPlayed": "Más reproducidos", "starred": "Favoritos", - "topRated": "Los mejores calificados" + "topRated": "Mejor calificados" } }, "artist": { @@ -523,4 +523,4 @@ "current_song": "Canción actual" } } -} \ No newline at end of file +} From 11c9dd4bd9a60a59638046f144b87747ef413caf Mon Sep 17 00:00:00 2001 From: Michael Tighe Date: Fri, 30 May 2025 14:28:39 -0700 Subject: [PATCH 04/19] fix(ui): reset page to 1 on playlist change - #1676 (#4154) Signed-off-by: Michael Tighe --- ui/src/playlist/PlaylistSongs.jsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ui/src/playlist/PlaylistSongs.jsx b/ui/src/playlist/PlaylistSongs.jsx index cc3e0fb1c..d9cbbbfd6 100644 --- a/ui/src/playlist/PlaylistSongs.jsx +++ b/ui/src/playlist/PlaylistSongs.jsx @@ -1,4 +1,4 @@ -import React, { useCallback, useMemo } from 'react' +import React, { useCallback, useEffect, useMemo } from 'react' import { BulkActionsToolbar, ListToolbar, @@ -84,7 +84,8 @@ const ReorderableList = ({ readOnly, children, ...rest }) => { const PlaylistSongs = ({ playlistId, readOnly, actions, ...props }) => { const listContext = useListContext() - const { data, ids, selectedIds, onUnselectItems, refetch } = listContext + const { data, ids, selectedIds, onUnselectItems, refetch, setPage } = + listContext const isDesktop = useMediaQuery((theme) => theme.breakpoints.up('md')) const classes = useStyles({ isDesktop }) const dispatch = useDispatch() @@ -93,6 +94,11 @@ const PlaylistSongs = ({ playlistId, readOnly, actions, ...props }) => { const version = useVersion() useResourceRefresh('song', 'playlist') + useEffect(() => { + setPage(1) + window.scrollTo({ top: 0, behavior: 'smooth' }) + }, [playlistId, setPage]) + const onAddToPlaylist = useCallback( (pls) => { if (pls.id === playlistId) { From 22c3486e3836e178beefa323f2799a77e9184f92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Fri, 30 May 2025 18:06:14 -0400 Subject: [PATCH 05/19] fix(server): enhance artist folder detection with directory traversal (#4151) * fix: enhance artist folder detection with directory traversal Enhanced fromArtistFolder function to implement directory traversal fallback for finding artist images. The original implementation only searched in the calculated artist folder, which failed for single album artists where artist.jpg files were not detected. Changes: Modified fromArtistFolder to search up to 3 directory levels (artist folder + 2 parent levels), extracted findImageInFolder helper function for cleaner code organization, added proper boundary checks to prevent infinite traversal, maintained backward compatibility with existing functionality. This fix ensures artist.jpg files are properly detected for single album artists while preserving all existing behavior for multi-album artists. * refactor: address PR review suggestions Applied review suggestions from gemini-code-assist bot: - Added maxArtistFolderTraversalDepth constant instead of hardcoded value 3 - Updated error message to mention that parent directories were also searched - Enhanced test assertion to verify the improved error message * fix: improve artist folder traversal logic and enhance error logging Signed-off-by: Deluan * fix: remove test for special glob characters in artist folder detection Signed-off-by: Deluan * fix: add logging for artist image search in folder Signed-off-by: Deluan --------- Signed-off-by: Deluan --- core/artwork/reader_artist.go | 62 ++++--- core/artwork/reader_artist_test.go | 250 +++++++++++++++++++++++++++++ 2 files changed, 292 insertions(+), 20 deletions(-) diff --git a/core/artwork/reader_artist.go b/core/artwork/reader_artist.go index 487346b4d..cb029a16e 100644 --- a/core/artwork/reader_artist.go +++ b/core/artwork/reader_artist.go @@ -20,6 +20,12 @@ import ( "github.com/navidrome/navidrome/utils/str" ) +const ( + // maxArtistFolderTraversalDepth defines how many directory levels to search + // when looking for artist images (artist folder + parent directories) + maxArtistFolderTraversalDepth = 3 +) + type artistReader struct { cacheKey a *artwork @@ -108,36 +114,52 @@ func (a *artistReader) fromArtistArtPriority(ctx context.Context, priority strin func fromArtistFolder(ctx context.Context, artistFolder string, pattern string) sourceFunc { return func() (io.ReadCloser, string, error) { - fsys := os.DirFS(artistFolder) - matches, err := fs.Glob(fsys, pattern) - if err != nil { - log.Warn(ctx, "Error matching artist image pattern", "pattern", pattern, "folder", artistFolder) - return nil, "", err - } - if len(matches) == 0 { - return nil, "", fmt.Errorf(`no matches for '%s' in '%s'`, pattern, artistFolder) - } - for _, m := range matches { - filePath := filepath.Join(artistFolder, m) - if !model.IsImageFile(m) { - continue + current := artistFolder + for i := 0; i < maxArtistFolderTraversalDepth; i++ { + if reader, path, err := findImageInFolder(ctx, current, pattern); err == nil { + return reader, path, nil } - f, err := os.Open(filePath) - if err != nil { - log.Warn(ctx, "Could not open cover art file", "file", filePath, err) - return nil, "", err + + parent := filepath.Dir(current) + if parent == current { + break } - return f, filePath, nil + current = parent } - return nil, "", nil + return nil, "", fmt.Errorf(`no matches for '%s' in '%s' or its parent directories`, pattern, artistFolder) } } +func findImageInFolder(ctx context.Context, folder, pattern string) (io.ReadCloser, string, error) { + log.Trace(ctx, "looking for artist image", "pattern", pattern, "folder", folder) + fsys := os.DirFS(folder) + matches, err := fs.Glob(fsys, pattern) + if err != nil { + log.Warn(ctx, "Error matching artist image pattern", "pattern", pattern, "folder", folder, err) + return nil, "", err + } + + for _, m := range matches { + if !model.IsImageFile(m) { + continue + } + filePath := filepath.Join(folder, m) + f, err := os.Open(filePath) + if err != nil { + log.Warn(ctx, "Could not open cover art file", "file", filePath, err) + continue + } + return f, filePath, nil + } + + return nil, "", fmt.Errorf(`no matches for '%s' in '%s'`, pattern, folder) +} + func loadArtistFolder(ctx context.Context, ds model.DataStore, albums model.Albums, paths []string) (string, time.Time, error) { if len(albums) == 0 { return "", time.Time{}, nil } - libID := albums[0].LibraryID // Just need one of the albums, as they should all be in the same Library + libID := albums[0].LibraryID // Just need one of the albums, as they should all be in the same Library - for now! TODO: Support multiple libraries folderPath := str.LongestCommonPrefix(paths) if !strings.HasSuffix(folderPath, string(filepath.Separator)) { diff --git a/core/artwork/reader_artist_test.go b/core/artwork/reader_artist_test.go index 294a5db0b..527b0849f 100644 --- a/core/artwork/reader_artist_test.go +++ b/core/artwork/reader_artist_test.go @@ -3,6 +3,8 @@ package artwork import ( "context" "errors" + "io" + "os" "path/filepath" "time" @@ -108,6 +110,254 @@ var _ = Describe("artistArtworkReader", func() { }) }) }) + + var _ = Describe("fromArtistFolder", func() { + var ( + ctx context.Context + tempDir string + testFunc sourceFunc + ) + + BeforeEach(func() { + ctx = context.Background() + tempDir = GinkgoT().TempDir() + }) + + When("artist folder contains matching image", func() { + BeforeEach(func() { + // Create test structure: /temp/artist/artist.jpg + artistDir := filepath.Join(tempDir, "artist") + Expect(os.MkdirAll(artistDir, 0755)).To(Succeed()) + + artistImagePath := filepath.Join(artistDir, "artist.jpg") + Expect(os.WriteFile(artistImagePath, []byte("fake image data"), 0600)).To(Succeed()) + + testFunc = fromArtistFolder(ctx, artistDir, "artist.*") + }) + + It("finds and returns the image", func() { + reader, path, err := testFunc() + Expect(err).ToNot(HaveOccurred()) + Expect(reader).ToNot(BeNil()) + Expect(path).To(ContainSubstring("artist.jpg")) + + // Verify we can read the content + data, err := io.ReadAll(reader) + Expect(err).ToNot(HaveOccurred()) + Expect(string(data)).To(Equal("fake image data")) + reader.Close() + }) + }) + + When("artist folder is empty but parent contains image", func() { + BeforeEach(func() { + // Create test structure: /temp/parent/artist.jpg and /temp/parent/artist/album/ + parentDir := filepath.Join(tempDir, "parent") + artistDir := filepath.Join(parentDir, "artist") + albumDir := filepath.Join(artistDir, "album") + Expect(os.MkdirAll(albumDir, 0755)).To(Succeed()) + + // Put artist image in parent directory + artistImagePath := filepath.Join(parentDir, "artist.jpg") + Expect(os.WriteFile(artistImagePath, []byte("parent image"), 0600)).To(Succeed()) + + testFunc = fromArtistFolder(ctx, artistDir, "artist.*") + }) + + It("finds image in parent directory", func() { + reader, path, err := testFunc() + Expect(err).ToNot(HaveOccurred()) + Expect(reader).ToNot(BeNil()) + Expect(path).To(ContainSubstring("parent" + string(filepath.Separator) + "artist.jpg")) + + data, err := io.ReadAll(reader) + Expect(err).ToNot(HaveOccurred()) + Expect(string(data)).To(Equal("parent image")) + reader.Close() + }) + }) + + When("image is two levels up", func() { + BeforeEach(func() { + // Create test structure: /temp/grandparent/artist.jpg and /temp/grandparent/parent/artist/ + grandparentDir := filepath.Join(tempDir, "grandparent") + parentDir := filepath.Join(grandparentDir, "parent") + artistDir := filepath.Join(parentDir, "artist") + Expect(os.MkdirAll(artistDir, 0755)).To(Succeed()) + + // Put artist image in grandparent directory + artistImagePath := filepath.Join(grandparentDir, "artist.jpg") + Expect(os.WriteFile(artistImagePath, []byte("grandparent image"), 0600)).To(Succeed()) + + testFunc = fromArtistFolder(ctx, artistDir, "artist.*") + }) + + It("finds image in grandparent directory", func() { + reader, path, err := testFunc() + Expect(err).ToNot(HaveOccurred()) + Expect(reader).ToNot(BeNil()) + Expect(path).To(ContainSubstring("grandparent" + string(filepath.Separator) + "artist.jpg")) + + data, err := io.ReadAll(reader) + Expect(err).ToNot(HaveOccurred()) + Expect(string(data)).To(Equal("grandparent image")) + reader.Close() + }) + }) + + When("images exist at multiple levels", func() { + BeforeEach(func() { + // Create test structure with images at multiple levels + grandparentDir := filepath.Join(tempDir, "grandparent") + parentDir := filepath.Join(grandparentDir, "parent") + artistDir := filepath.Join(parentDir, "artist") + Expect(os.MkdirAll(artistDir, 0755)).To(Succeed()) + + // Put artist images at all levels + Expect(os.WriteFile(filepath.Join(artistDir, "artist.jpg"), []byte("artist level"), 0600)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(parentDir, "artist.jpg"), []byte("parent level"), 0600)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(grandparentDir, "artist.jpg"), []byte("grandparent level"), 0600)).To(Succeed()) + + testFunc = fromArtistFolder(ctx, artistDir, "artist.*") + }) + + It("prioritizes the closest (artist folder) image", func() { + reader, path, err := testFunc() + Expect(err).ToNot(HaveOccurred()) + Expect(reader).ToNot(BeNil()) + Expect(path).To(ContainSubstring("artist" + string(filepath.Separator) + "artist.jpg")) + + data, err := io.ReadAll(reader) + Expect(err).ToNot(HaveOccurred()) + Expect(string(data)).To(Equal("artist level")) + reader.Close() + }) + }) + + When("pattern matches multiple files", func() { + BeforeEach(func() { + artistDir := filepath.Join(tempDir, "artist") + Expect(os.MkdirAll(artistDir, 0755)).To(Succeed()) + + // Create multiple matching files + Expect(os.WriteFile(filepath.Join(artistDir, "artist.jpg"), []byte("jpg image"), 0600)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(artistDir, "artist.png"), []byte("png image"), 0600)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(artistDir, "artist.txt"), []byte("text file"), 0600)).To(Succeed()) + + testFunc = fromArtistFolder(ctx, artistDir, "artist.*") + }) + + It("returns the first valid image file", func() { + reader, path, err := testFunc() + Expect(err).ToNot(HaveOccurred()) + Expect(reader).ToNot(BeNil()) + + // Should return an image file, not the text file + Expect(path).To(SatisfyAny( + ContainSubstring("artist.jpg"), + ContainSubstring("artist.png"), + )) + Expect(path).ToNot(ContainSubstring("artist.txt")) + reader.Close() + }) + }) + + When("no matching files exist anywhere", func() { + BeforeEach(func() { + artistDir := filepath.Join(tempDir, "artist") + Expect(os.MkdirAll(artistDir, 0755)).To(Succeed()) + + // Create non-matching files + Expect(os.WriteFile(filepath.Join(artistDir, "cover.jpg"), []byte("cover image"), 0600)).To(Succeed()) + + testFunc = fromArtistFolder(ctx, artistDir, "artist.*") + }) + + It("returns an error", func() { + reader, path, err := testFunc() + Expect(err).To(HaveOccurred()) + Expect(reader).To(BeNil()) + Expect(path).To(BeEmpty()) + Expect(err.Error()).To(ContainSubstring("no matches for 'artist.*'")) + Expect(err.Error()).To(ContainSubstring("parent directories")) + }) + }) + + When("directory traversal reaches filesystem root", func() { + BeforeEach(func() { + // Start from a shallow directory to test root boundary + artistDir := filepath.Join(tempDir, "artist") + Expect(os.MkdirAll(artistDir, 0755)).To(Succeed()) + + testFunc = fromArtistFolder(ctx, artistDir, "artist.*") + }) + + It("handles root boundary gracefully", func() { + reader, path, err := testFunc() + Expect(err).To(HaveOccurred()) + Expect(reader).To(BeNil()) + Expect(path).To(BeEmpty()) + // Should not panic or cause infinite loop + }) + }) + + When("file exists but cannot be opened", func() { + BeforeEach(func() { + artistDir := filepath.Join(tempDir, "artist") + Expect(os.MkdirAll(artistDir, 0755)).To(Succeed()) + + // Create a file that cannot be opened (permission denied) + restrictedFile := filepath.Join(artistDir, "artist.jpg") + Expect(os.WriteFile(restrictedFile, []byte("restricted"), 0600)).To(Succeed()) + + testFunc = fromArtistFolder(ctx, artistDir, "artist.*") + }) + + It("logs warning and continues searching", func() { + // This test depends on the ability to restrict file permissions + // For now, we'll just ensure it doesn't panic and returns appropriate error + reader, _, err := testFunc() + // The file should be readable in test environment, so this will succeed + // In a real scenario with permission issues, it would continue searching + if err == nil { + Expect(reader).ToNot(BeNil()) + reader.Close() + } + }) + }) + + When("single album artist scenario (original issue)", func() { + BeforeEach(func() { + // Simulate the exact folder structure from the issue: + // /music/artist/album1/ (single album) + // /music/artist/artist.jpg (artist image that should be found) + artistDir := filepath.Join(tempDir, "music", "artist") + albumDir := filepath.Join(artistDir, "album1") + Expect(os.MkdirAll(albumDir, 0755)).To(Succeed()) + + // Create artist.jpg in the artist folder (this was not being found before) + artistImagePath := filepath.Join(artistDir, "artist.jpg") + Expect(os.WriteFile(artistImagePath, []byte("single album artist image"), 0600)).To(Succeed()) + + // The fromArtistFolder is called with the artist folder path + testFunc = fromArtistFolder(ctx, artistDir, "artist.*") + }) + + It("finds artist.jpg in artist folder for single album artist", func() { + reader, path, err := testFunc() + Expect(err).ToNot(HaveOccurred()) + Expect(reader).ToNot(BeNil()) + Expect(path).To(ContainSubstring("artist.jpg")) + Expect(path).To(ContainSubstring("artist")) + + // Verify the content + data, err := io.ReadAll(reader) + Expect(err).ToNot(HaveOccurred()) + Expect(string(data)).To(Equal("single album artist image")) + reader.Close() + }) + }) + }) }) type fakeFolderRepo struct { From 6dd98e0bede6ef258d59f7336fcab870daf9166e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Fri, 30 May 2025 21:07:08 -0400 Subject: [PATCH 06/19] feat(ui): add configuration tab in About dialog (#4142) * Flatten config endpoint and improve About dialog * add config resource Signed-off-by: Deluan * fix(ui): replace `==` with `===` Signed-off-by: Deluan * feat(ui): add environment variables Signed-off-by: Deluan * feat(ui): add sensitive value redaction Signed-off-by: Deluan * feat(ui): more translations Signed-off-by: Deluan * address PR comments Signed-off-by: Deluan * feat(ui): add configuration export feature in About dialog Signed-off-by: Deluan * feat(ui): translate development flags section header Signed-off-by: Deluan * refactor Signed-off-by: Deluan * feat(api): refactor routes for keepalive and insights endpoints Signed-off-by: Deluan * lint Signed-off-by: Deluan * fix(ui): enhance string escaping in formatTomlValue function Updated the formatTomlValue function to properly escape backslashes in addition to quotes. Added new test cases to ensure correct handling of strings containing both backslashes and quotes. Signed-off-by: Deluan * feat(ui): adjust dialog size Signed-off-by: Deluan --------- Signed-off-by: Deluan --- conf/configuration.go | 2 + resources/i18n/pt-br.json | 17 +- server/nativeapi/config.go | 133 ++++++++++ server/nativeapi/config_test.go | 268 ++++++++++++++++++++ server/nativeapi/native_api.go | 41 +-- server/serve_index.go | 1 + server/serve_index_test.go | 11 + ui/src/App.jsx | 3 + ui/src/common/SongInfo.jsx | 2 +- ui/src/config.js | 1 + ui/src/dialogs/AboutDialog.jsx | 424 ++++++++++++++++++++++++++------ ui/src/i18n/en.json | 15 ++ ui/src/utils/toml.js | 170 +++++++++++++ ui/src/utils/toml.test.js | 363 +++++++++++++++++++++++++++ 14 files changed, 1356 insertions(+), 95 deletions(-) create mode 100644 server/nativeapi/config.go create mode 100644 server/nativeapi/config_test.go create mode 100644 ui/src/utils/toml.js create mode 100644 ui/src/utils/toml.test.js diff --git a/conf/configuration.go b/conf/configuration.go index 67f43294d..64a4e6a75 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -112,6 +112,7 @@ type configOptions struct { DevActivityPanelUpdateRate time.Duration DevSidebarPlaylists bool DevShowArtistPage bool + DevUIShowConfig bool DevOffsetOptimize int DevArtworkMaxRequests int DevArtworkThrottleBacklogLimit int @@ -553,6 +554,7 @@ func setViperDefaults() { viper.SetDefault("devactivitypanelupdaterate", 300*time.Millisecond) viper.SetDefault("devsidebarplaylists", true) viper.SetDefault("devshowartistpage", true) + viper.SetDefault("devuishowconfig", true) viper.SetDefault("devoffsetoptimize", 50000) viper.SetDefault("devartworkmaxrequests", max(2, runtime.NumCPU()/3)) viper.SetDefault("devartworkthrottlebackloglimit", consts.RequestThrottleBacklogLimit) diff --git a/resources/i18n/pt-br.json b/resources/i18n/pt-br.json index febdcf769..cc771e8fa 100644 --- a/resources/i18n/pt-br.json +++ b/resources/i18n/pt-br.json @@ -496,6 +496,21 @@ "disabled": "Desligado", "waiting": "Aguardando" } + }, + "tabs": { + "about": "Sobre", + "config": "Configuração" + }, + "config": { + "configName": "Nome da Configuração", + "environmentVariable": "Variável de Ambiente", + "currentValue": "Valor Atual", + "configurationFile": "Arquivo de Configuração", + "exportToml": "Exportar Configuração (TOML)", + "exportSuccess": "Configuração exportada para o clipboard em formato TOML", + "exportFailed": "Falha ao copiar configuração", + "devFlagsHeader": "Flags de Desenvolvimento (sujeitas a mudança/remoção)", + "devFlagsComment": "Estas são configurações experimentais e podem ser removidas em versões futuras" } }, "activity": { @@ -523,4 +538,4 @@ "current_song": "Vai para música atual" } } -} \ No newline at end of file +} diff --git a/server/nativeapi/config.go b/server/nativeapi/config.go new file mode 100644 index 000000000..500e9098f --- /dev/null +++ b/server/nativeapi/config.go @@ -0,0 +1,133 @@ +package nativeapi + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "reflect" + "strings" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model/request" +) + +// sensitiveFieldsPartialMask contains configuration field names that should be redacted +// using partial masking (first and last character visible, middle replaced with *). +// For values with 7+ characters: "secretvalue123" becomes "s***********3" +// For values with <7 characters: "short" becomes "****" +// Add field paths using dot notation (e.g., "LastFM.ApiKey", "Spotify.Secret") +var sensitiveFieldsPartialMask = []string{ + "LastFM.ApiKey", + "LastFM.Secret", + "Prometheus.MetricsPath", + "Spotify.ID", + "Spotify.Secret", + "DevAutoLoginUsername", +} + +// sensitiveFieldsFullMask contains configuration field names that should always be +// completely masked with "****" regardless of their length. +// Add field paths using dot notation for any fields that should never show any content. +var sensitiveFieldsFullMask = []string{ + "DevAutoCreateAdminPassword", + "PasswordEncryptionKey", + "Prometheus.Password", +} + +type configEntry struct { + Key string `json:"key"` + EnvVar string `json:"envVar"` + Value interface{} `json:"value"` +} + +type configResponse struct { + ID string `json:"id"` + ConfigFile string `json:"configFile"` + Config []configEntry `json:"config"` +} + +func redactValue(key string, value string) string { + // Return empty values as-is + if len(value) == 0 { + return value + } + + // Check if this field should be fully masked + for _, field := range sensitiveFieldsFullMask { + if field == key { + return "****" + } + } + + // Check if this field should be partially masked + for _, field := range sensitiveFieldsPartialMask { + if field == key { + if len(value) < 7 { + return "****" + } + // Show first and last character with * in between + return string(value[0]) + strings.Repeat("*", len(value)-2) + string(value[len(value)-1]) + } + } + + // Return original value if not sensitive + return value +} + +func flatten(ctx context.Context, entries *[]configEntry, prefix string, v reflect.Value) { + if v.Kind() == reflect.Struct && v.Type().PkgPath() != "time" { + t := v.Type() + for i := 0; i < v.NumField(); i++ { + if !t.Field(i).IsExported() { + continue + } + flatten(ctx, entries, prefix+"."+t.Field(i).Name, v.Field(i)) + } + return + } + + key := strings.TrimPrefix(prefix, ".") + envVar := "ND_" + strings.ToUpper(strings.ReplaceAll(key, ".", "_")) + var val interface{} + switch v.Kind() { + case reflect.Map, reflect.Slice, reflect.Array: + b, err := json.Marshal(v.Interface()) + if err != nil { + log.Error(ctx, "Error marshalling config value", "key", key, err) + val = "error marshalling value" + } else { + val = string(b) + } + default: + originalValue := fmt.Sprint(v.Interface()) + val = redactValue(key, originalValue) + } + + *entries = append(*entries, configEntry{Key: key, EnvVar: envVar, Value: val}) +} + +func getConfig(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + user, _ := request.UserFrom(ctx) + if !user.IsAdmin { + http.Error(w, "Config endpoint is only available to admin users", http.StatusUnauthorized) + return + } + + entries := make([]configEntry, 0) + v := reflect.ValueOf(*conf.Server) + t := reflect.TypeOf(*conf.Server) + for i := 0; i < v.NumField(); i++ { + fieldVal := v.Field(i) + fieldType := t.Field(i) + flatten(ctx, &entries, fieldType.Name, fieldVal) + } + + resp := configResponse{ID: "config", ConfigFile: conf.Server.ConfigFile, Config: entries} + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(resp); err != nil { + log.Error(ctx, "Error encoding config response", err) + } +} diff --git a/server/nativeapi/config_test.go b/server/nativeapi/config_test.go new file mode 100644 index 000000000..eef8a81a2 --- /dev/null +++ b/server/nativeapi/config_test.go @@ -0,0 +1,268 @@ +package nativeapi + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("config endpoint", func() { + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + }) + + It("rejects non admin users", func() { + req := httptest.NewRequest("GET", "/config", nil) + w := httptest.NewRecorder() + ctx := request.WithUser(req.Context(), model.User{IsAdmin: false}) + getConfig(w, req.WithContext(ctx)) + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + }) + + It("returns configuration entries", func() { + req := httptest.NewRequest("GET", "/config", nil) + w := httptest.NewRecorder() + ctx := request.WithUser(req.Context(), model.User{IsAdmin: true}) + getConfig(w, req.WithContext(ctx)) + Expect(w.Code).To(Equal(http.StatusOK)) + var resp configResponse + Expect(json.Unmarshal(w.Body.Bytes(), &resp)).To(Succeed()) + Expect(resp.ID).To(Equal("config")) + + // Verify that we have both Dev and non-Dev fields + var hasDevFields = false + var hasNonDevFields = false + for _, e := range resp.Config { + if strings.HasPrefix(e.Key, "Dev") { + hasDevFields = true + } else { + hasNonDevFields = true + } + } + + Expect(hasDevFields).To(BeTrue(), "Should have Dev* configuration fields") + Expect(hasNonDevFields).To(BeTrue(), "Should have non-Dev configuration fields") + Expect(len(resp.Config)).To(BeNumerically(">", 0), "Should return configuration entries") + }) + + It("includes flattened struct fields", func() { + req := httptest.NewRequest("GET", "/config", nil) + w := httptest.NewRecorder() + ctx := request.WithUser(req.Context(), model.User{IsAdmin: true}) + getConfig(w, req.WithContext(ctx)) + Expect(w.Code).To(Equal(http.StatusOK)) + var resp configResponse + Expect(json.Unmarshal(w.Body.Bytes(), &resp)).To(Succeed()) + values := map[string]string{} + for _, e := range resp.Config { + if s, ok := e.Value.(string); ok { + values[e.Key] = s + } + } + Expect(values).To(HaveKeyWithValue("Inspect.MaxRequests", "1")) + Expect(values).To(HaveKeyWithValue("HTTPSecurityHeaders.CustomFrameOptionsValue", "DENY")) + }) + + It("includes the config file path", func() { + req := httptest.NewRequest("GET", "/config", nil) + w := httptest.NewRecorder() + ctx := request.WithUser(req.Context(), model.User{IsAdmin: true}) + getConfig(w, req.WithContext(ctx)) + Expect(w.Code).To(Equal(http.StatusOK)) + var resp configResponse + Expect(json.Unmarshal(w.Body.Bytes(), &resp)).To(Succeed()) + Expect(resp.ConfigFile).To(Not(BeEmpty())) + }) + + It("includes environment variable names", func() { + req := httptest.NewRequest("GET", "/config", nil) + w := httptest.NewRecorder() + ctx := request.WithUser(req.Context(), model.User{IsAdmin: true}) + getConfig(w, req.WithContext(ctx)) + Expect(w.Code).To(Equal(http.StatusOK)) + var resp configResponse + Expect(json.Unmarshal(w.Body.Bytes(), &resp)).To(Succeed()) + + // Create a map to check specific env var mappings + envVars := map[string]string{} + for _, e := range resp.Config { + envVars[e.Key] = e.EnvVar + } + + Expect(envVars).To(HaveKeyWithValue("MusicFolder", "ND_MUSICFOLDER")) + Expect(envVars).To(HaveKeyWithValue("Scanner.Enabled", "ND_SCANNER_ENABLED")) + Expect(envVars).To(HaveKeyWithValue("HTTPSecurityHeaders.CustomFrameOptionsValue", "ND_HTTPSECURITYHEADERS_CUSTOMFRAMEOPTIONSVALUE")) + }) + + Context("redaction functionality", func() { + It("redacts sensitive values with partial masking for long values", func() { + // Set up test values + conf.Server.LastFM.ApiKey = "ba46f0e84a123456" + conf.Server.Spotify.Secret = "verylongsecret123" + + req := httptest.NewRequest("GET", "/config", nil) + w := httptest.NewRecorder() + ctx := request.WithUser(req.Context(), model.User{IsAdmin: true}) + getConfig(w, req.WithContext(ctx)) + + Expect(w.Code).To(Equal(http.StatusOK)) + var resp configResponse + Expect(json.Unmarshal(w.Body.Bytes(), &resp)).To(Succeed()) + + values := map[string]string{} + for _, e := range resp.Config { + if s, ok := e.Value.(string); ok { + values[e.Key] = s + } + } + + Expect(values).To(HaveKeyWithValue("LastFM.ApiKey", "b**************6")) + Expect(values).To(HaveKeyWithValue("Spotify.Secret", "v***************3")) + }) + + It("redacts sensitive values with full masking for short values", func() { + // Set up test values with short secrets + conf.Server.LastFM.Secret = "short" + conf.Server.Spotify.ID = "abc123" + + req := httptest.NewRequest("GET", "/config", nil) + w := httptest.NewRecorder() + ctx := request.WithUser(req.Context(), model.User{IsAdmin: true}) + getConfig(w, req.WithContext(ctx)) + + Expect(w.Code).To(Equal(http.StatusOK)) + var resp configResponse + Expect(json.Unmarshal(w.Body.Bytes(), &resp)).To(Succeed()) + + values := map[string]string{} + for _, e := range resp.Config { + if s, ok := e.Value.(string); ok { + values[e.Key] = s + } + } + + Expect(values).To(HaveKeyWithValue("LastFM.Secret", "****")) + Expect(values).To(HaveKeyWithValue("Spotify.ID", "****")) + }) + + It("fully masks password fields", func() { + // Set up test values for password fields + conf.Server.DevAutoCreateAdminPassword = "adminpass123" + conf.Server.Prometheus.Password = "prometheuspass" + + req := httptest.NewRequest("GET", "/config", nil) + w := httptest.NewRecorder() + ctx := request.WithUser(req.Context(), model.User{IsAdmin: true}) + getConfig(w, req.WithContext(ctx)) + + Expect(w.Code).To(Equal(http.StatusOK)) + var resp configResponse + Expect(json.Unmarshal(w.Body.Bytes(), &resp)).To(Succeed()) + + values := map[string]string{} + for _, e := range resp.Config { + if s, ok := e.Value.(string); ok { + values[e.Key] = s + } + } + + Expect(values).To(HaveKeyWithValue("DevAutoCreateAdminPassword", "****")) + Expect(values).To(HaveKeyWithValue("Prometheus.Password", "****")) + }) + + It("does not redact non-sensitive values", func() { + conf.Server.MusicFolder = "/path/to/music" + conf.Server.Port = 4533 + + req := httptest.NewRequest("GET", "/config", nil) + w := httptest.NewRecorder() + ctx := request.WithUser(req.Context(), model.User{IsAdmin: true}) + getConfig(w, req.WithContext(ctx)) + + Expect(w.Code).To(Equal(http.StatusOK)) + var resp configResponse + Expect(json.Unmarshal(w.Body.Bytes(), &resp)).To(Succeed()) + + values := map[string]string{} + for _, e := range resp.Config { + if s, ok := e.Value.(string); ok { + values[e.Key] = s + } + } + + Expect(values).To(HaveKeyWithValue("MusicFolder", "/path/to/music")) + Expect(values).To(HaveKeyWithValue("Port", "4533")) + }) + + It("handles empty sensitive values", func() { + conf.Server.LastFM.ApiKey = "" + conf.Server.PasswordEncryptionKey = "" + + req := httptest.NewRequest("GET", "/config", nil) + w := httptest.NewRecorder() + ctx := request.WithUser(req.Context(), model.User{IsAdmin: true}) + getConfig(w, req.WithContext(ctx)) + + Expect(w.Code).To(Equal(http.StatusOK)) + var resp configResponse + Expect(json.Unmarshal(w.Body.Bytes(), &resp)).To(Succeed()) + + values := map[string]string{} + for _, e := range resp.Config { + if s, ok := e.Value.(string); ok { + values[e.Key] = s + } + } + + // Empty sensitive values should remain empty + Expect(values["LastFM.ApiKey"]).To(Equal("")) + Expect(values["PasswordEncryptionKey"]).To(Equal("")) + }) + }) +}) + +var _ = Describe("redactValue function", func() { + It("partially masks long sensitive values", func() { + Expect(redactValue("LastFM.ApiKey", "ba46f0e84a")).To(Equal("b********a")) + Expect(redactValue("Spotify.Secret", "verylongsecret123")).To(Equal("v***************3")) + }) + + It("fully masks long sensitive values that should be completely hidden", func() { + Expect(redactValue("PasswordEncryptionKey", "1234567890")).To(Equal("****")) + Expect(redactValue("DevAutoCreateAdminPassword", "1234567890")).To(Equal("****")) + Expect(redactValue("Prometheus.Password", "1234567890")).To(Equal("****")) + }) + + It("fully masks short sensitive values", func() { + Expect(redactValue("LastFM.Secret", "short")).To(Equal("****")) + Expect(redactValue("Spotify.ID", "abc")).To(Equal("****")) + Expect(redactValue("PasswordEncryptionKey", "12345")).To(Equal("****")) + Expect(redactValue("DevAutoCreateAdminPassword", "short")).To(Equal("****")) + Expect(redactValue("Prometheus.Password", "short")).To(Equal("****")) + }) + + It("does not mask non-sensitive values", func() { + Expect(redactValue("MusicFolder", "/path/to/music")).To(Equal("/path/to/music")) + Expect(redactValue("Port", "4533")).To(Equal("4533")) + Expect(redactValue("SomeOtherField", "secretvalue")).To(Equal("secretvalue")) + }) + + It("handles empty values", func() { + Expect(redactValue("LastFM.ApiKey", "")).To(Equal("")) + Expect(redactValue("NonSensitive", "")).To(Equal("")) + }) + + It("handles edge case values", func() { + Expect(redactValue("LastFM.ApiKey", "a")).To(Equal("****")) + Expect(redactValue("LastFM.ApiKey", "ab")).To(Equal("****")) + Expect(redactValue("LastFM.ApiKey", "abcdefg")).To(Equal("a*****g")) + }) +}) diff --git a/server/nativeapi/native_api.go b/server/nativeapi/native_api.go index ddf5df1c3..f2c13fa3a 100644 --- a/server/nativeapi/native_api.go +++ b/server/nativeapi/native_api.go @@ -61,21 +61,9 @@ func (n *Router) routes() http.Handler { n.addPlaylistTrackRoute(r) n.addMissingFilesRoute(r) n.addInspectRoute(r) - - // Keepalive endpoint to be used to keep the session valid (ex: while playing songs) - r.Get("/keepalive/*", func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write([]byte(`{"response":"ok", "id":"keepalive"}`)) - }) - - // Insights status endpoint - r.Get("/insights/*", func(w http.ResponseWriter, r *http.Request) { - last, success := n.insights.LastRun(r.Context()) - if conf.Server.EnableInsightsCollector { - _, _ = w.Write([]byte(`{"id":"insights_status", "lastRun":"` + last.Format("2006-01-02 15:04:05") + `", "success":` + strconv.FormatBool(success) + `}`)) - } else { - _, _ = w.Write([]byte(`{"id":"insights_status", "lastRun":"disabled", "success":false}`)) - } - }) + n.addConfigRoute(r) + n.addKeepAliveRoute(r) + n.addInsightsRoute(r) }) return r @@ -196,3 +184,26 @@ func (n *Router) addInspectRoute(r chi.Router) { }) } } + +func (n *Router) addConfigRoute(r chi.Router) { + if conf.Server.DevUIShowConfig { + r.Get("/config/*", getConfig) + } +} + +func (n *Router) addKeepAliveRoute(r chi.Router) { + r.Get("/keepalive/*", func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"response":"ok", "id":"keepalive"}`)) + }) +} + +func (n *Router) addInsightsRoute(r chi.Router) { + r.Get("/insights/*", func(w http.ResponseWriter, r *http.Request) { + last, success := n.insights.LastRun(r.Context()) + if conf.Server.EnableInsightsCollector { + _, _ = w.Write([]byte(`{"id":"insights_status", "lastRun":"` + last.Format("2006-01-02 15:04:05") + `", "success":` + strconv.FormatBool(success) + `}`)) + } else { + _, _ = w.Write([]byte(`{"id":"insights_status", "lastRun":"disabled", "success":false}`)) + } + }) +} diff --git a/server/serve_index.go b/server/serve_index.go index 9a457ac20..1e55743f0 100644 --- a/server/serve_index.go +++ b/server/serve_index.go @@ -65,6 +65,7 @@ func serveIndex(ds model.DataStore, fs fs.FS, shareInfo *model.Share) http.Handl "devSidebarPlaylists": conf.Server.DevSidebarPlaylists, "lastFMEnabled": conf.Server.LastFM.Enabled, "devShowArtistPage": conf.Server.DevShowArtistPage, + "devUIShowConfig": conf.Server.DevUIShowConfig, "listenBrainzEnabled": conf.Server.ListenBrainz.Enabled, "enableExternalServices": conf.Server.EnableExternalServices, "enableReplayGain": conf.Server.EnableReplayGain, diff --git a/server/serve_index_test.go b/server/serve_index_test.go index 0f02153fd..fd0d42193 100644 --- a/server/serve_index_test.go +++ b/server/serve_index_test.go @@ -304,6 +304,17 @@ var _ = Describe("serveIndex", func() { Expect(config).To(HaveKeyWithValue("devShowArtistPage", true)) }) + It("sets the devUIShowConfig", func() { + conf.Server.DevUIShowConfig = true + r := httptest.NewRequest("GET", "/index.html", nil) + w := httptest.NewRecorder() + + serveIndex(ds, fs, nil)(w, r) + + config := extractAppConfig(w.Body.String()) + Expect(config).To(HaveKeyWithValue("devUIShowConfig", true)) + }) + It("sets the listenBrainzEnabled", func() { conf.Server.ListenBrainz.Enabled = true r := httptest.NewRequest("GET", "/index.html", nil) diff --git a/ui/src/App.jsx b/ui/src/App.jsx index 1b89f7b8c..4a38051b4 100644 --- a/ui/src/App.jsx +++ b/ui/src/App.jsx @@ -137,6 +137,9 @@ const Admin = (props) => { , , , + permissions === 'admin' && config.devUIShowConfig ? ( + + ) : null, , ]} diff --git a/ui/src/common/SongInfo.jsx b/ui/src/common/SongInfo.jsx index 77e91b653..9b9ca18cd 100644 --- a/ui/src/common/SongInfo.jsx +++ b/ui/src/common/SongInfo.jsx @@ -138,7 +138,7 @@ export const SongInfo = (props) => { )}