From 46c432719fdd2fb34c2ec6c4f573a1488fb90215 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= <deluan@navidrome.org>
Date: Sat, 26 Sep 2026 23:30:50 -0400
Subject: [PATCH] fix(log): redact LastFM keys and Prometheus password in
 config dump (#6233)

The startup Configuration dump is rendered with pretty.Sprintf("%# v"), which
pads multi-line struct fields with spaces after the colon. The ApiKey and
Secret redaction patterns required the quote right after the colon, so
LastFM.ApiKey and LastFM.Secret were logged in clear text even with
EnableLogRedacting on. Allow optional whitespace after the colon, like the
other config patterns already do.

Prometheus.Password had no redaction pattern at all. Add one that also skips
escaped quotes, since the password can hold any character and pretty prints
it Go-quoted.

Add tests for the padded and unpadded forms, plus one that redacts a real
pretty.Sprintf dump of LastFM- and Prometheus-shaped structs so a padding
change in pretty can't bring the leak back.

Reported in https://github.com/navidrome/navidrome/discussions/6232
---
 log/log.go      |  6 +++--
 log/log_test.go | 67 ++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/log/log.go b/log/log.go
index de2f171b2..da1d7622e 100644
--- a/log/log.go
+++ b/log/log.go
@@ -27,14 +27,16 @@ var redacted = &Hook{
 	AcceptedLevels: logrus.AllLevels,
 	RedactionList: []string{
 		// Keys from the config
-		"(ApiKey:\")[\\w]*",
-		"(Secret:\")[\\w]*",
+		"(ApiKey:[\\s]*\")[\\w]*",
+		"(Secret:[\\s]*\")[\\w]*",
 		"(PasswordEncryptionKey:[\\s]*\")[^\"]*",
 		"(UserHeader:[\\s]*\")[^\"]*",
 		"(TrustedSources:[\\s]*\")[^\"]*",
 		"(MetricsPath:[\\s]*\")[^\"]*",
 		"(DevAutoCreateAdminPassword:[\\s]*\")[^\"]*",
 		"(DevAutoLoginUsername:[\\s]*\")[^\"]*",
+		// Prometheus.Password. Any character is allowed, so skip escaped quotes in the value
+		`(Password:[\s]*")(?:[^"\\]|\\.)*`,
 
 		// UI appConfig
 		"(subsonicToken:)[\\w]+(\\s)",
diff --git a/log/log_test.go b/log/log_test.go
index 82207c672..184ff57db 100644
--- a/log/log_test.go
+++ b/log/log_test.go
@@ -9,6 +9,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/kr/pretty"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/sirupsen/logrus"
@@ -94,7 +95,7 @@ var _ = Describe("Logger", func() {
 			SetLogSourceLine(true)
 			Error("A crash happened")
 			// NOTE: This assertion breaks if the line number above changes
-			Expect(hook.LastEntry().Data[" source"]).To(ContainSubstring("/log/log_test.go:95"))
+			Expect(hook.LastEntry().Data[" source"]).To(ContainSubstring("/log/log_test.go:96"))
 			Expect(hook.LastEntry().Message).To(Equal("A crash happened"))
 		})
 
@@ -291,5 +292,69 @@ var _ = Describe("Logger", func() {
 			Expect(got).ToNot(ContainSubstring("secret"))
 			Expect(got).To(ContainSubstring(`"User-Agent":["Finamp/1.0"]`))
 		})
+
+		// https://github.com/navidrome/navidrome/discussions/6232
+		DescribeTable("redacts config keys in the startup Configuration dump",
+			func(line, expected string) {
+				Expect(Redact(line)).To(Equal(expected))
+			},
+			Entry("unpadded ApiKey", `ApiKey:"0123456789abcdef0123456789abcdef"`, `ApiKey:"[REDACTED]"`),
+			Entry("unpadded Secret", `Secret:"fedcba9876543210fedcba9876543210"`, `Secret:"[REDACTED]"`),
+			Entry("padded ApiKey", `        ApiKey:                  "0123456789abcdef0123456789abcdef",`,
+				`        ApiKey:                  "[REDACTED]",`),
+			Entry("padded Secret", `        Secret:                  "fedcba9876543210fedcba9876543210",`,
+				`        Secret:                  "[REDACTED]",`),
+			Entry("unpadded Prometheus Password", `Password:"p@ss w0rd!"`, `Password:"[REDACTED]"`),
+			Entry("padded Prometheus Password", `        Password:    "p@ss w0rd!",`, `        Password:    "[REDACTED]",`),
+			Entry("Prometheus Password with escaped quotes", `        Password:    "a\"b\\\"c",`,
+				`        Password:    "[REDACTED]",`),
+		)
+
+		It("redacts secrets in a pretty-printed config struct", func() {
+			// Mirrors conf.lastfmOptions and conf.prometheusOptions (conf imports log, so it can't be
+			// used here). pretty only breaks a struct into padded lines when it is long enough, so
+			// keep all the fields.
+			type lastfmOptions struct {
+				Enabled                 bool
+				ApiKey                  string
+				Secret                  string
+				Language                string
+				ScrobbleFirstArtistOnly bool
+				Languages               []string
+			}
+			type prometheusOptions struct {
+				Enabled     bool
+				MetricsPath string
+				Password    string
+			}
+			type configOptions struct {
+				Address    string
+				LastFM     lastfmOptions
+				Prometheus prometheusOptions
+			}
+			cfg := configOptions{
+				Address: "0.0.0.0",
+				LastFM: lastfmOptions{ //nolint:gosec
+					Enabled:   true,
+					ApiKey:    "0123456789abcdef0123456789abcdef",
+					Secret:    "fedcba9876543210fedcba9876543210",
+					Language:  "en",
+					Languages: []string{"en"},
+				},
+				Prometheus: prometheusOptions{ //nolint:gosec
+					Enabled:     true,
+					MetricsPath: "/metrics",
+					Password:    `prom"pass-tail`,
+				},
+			}
+			dump := pretty.Sprintf("Configuration: %# v", cfg)
+			Expect(dump).To(MatchRegexp(`ApiKey:\s{2,}"`), "the dump must use the padded layout")
+
+			got := Redact(dump)
+			Expect(got).ToNot(ContainSubstring(cfg.LastFM.ApiKey))
+			Expect(got).ToNot(ContainSubstring(cfg.LastFM.Secret))
+			Expect(got).ToNot(ContainSubstring("pass-tail"))
+			Expect(got).To(ContainSubstring(`"en"`))
+		})
 	})
 })