mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 02:17:25 +02:00
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
This commit is contained in:
parent
c22ce9ebb2
commit
46c432719f
2 changed files with 70 additions and 3 deletions
|
|
@ -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)",
|
||||
|
|
|
|||
|
|
@ -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"`))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue