fix: split html sanitization from plaintext handling (#5403)

* fix: split html sanitization from plaintext handling

Add a dedicated SanitizeHTML helper for HTML-rendered values so entity-encoded markup is decoded before bluemonday sanitization. Use the new helper for the login welcome message and artist biographies while preserving SanitizeText semantics for lyrics and other plaintext callers. Add regression coverage for both helpers and the serveIndex welcomeMessage path.

* docs: add SanitizeText and SanitizeHTML godoc

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: preserve plain text in artist biographies

Revert artist biography storage to SanitizeText so entity-encoded plain text remains decoded for Subsonic consumers. This avoids double-escaping values like R&B in XML responses while keeping the new welcomeMessage HTML sanitization in place, and adds a regression test covering the biography storage behavior.

---------

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan Quintão 2026-04-23 17:53:28 -04:00 committed by Rob Emery
parent 987b16079a
commit f784224aef
5 changed files with 74 additions and 1 deletions

View File

@ -105,6 +105,29 @@ var _ = Describe("Provider - UpdateArtistInfo", func() {
ag.AssertExpectations(GinkgoT())
})
It("preserves decoded plain text in biography storage", func() {
originalArtist := &model.Artist{
ID: "ar-encoded-bio",
Name: "Encoded Bio Artist",
}
mockArtistRepo.SetData(model.Artists{*originalArtist})
expectedMBID := "mbid-encoded-bio"
expectedBio := "R&amp;B"
ag.On("GetArtistMBID", ctx, "ar-encoded-bio", "Encoded Bio Artist").Return(expectedMBID, nil).Once()
ag.On("GetArtistImages", ctx, "ar-encoded-bio", "Encoded Bio Artist", expectedMBID).Return(nil, nil).Maybe()
ag.On("GetArtistBiography", ctx, "ar-encoded-bio", "Encoded Bio Artist", expectedMBID).Return(expectedBio, nil).Once()
ag.On("GetArtistURL", ctx, "ar-encoded-bio", "Encoded Bio Artist", expectedMBID).Return("", nil).Maybe()
ag.On("GetSimilarArtists", ctx, "ar-encoded-bio", "Encoded Bio Artist", expectedMBID, 100).Return(nil, nil).Maybe()
updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-encoded-bio", 10, false)
Expect(err).NotTo(HaveOccurred())
Expect(updatedArtist).NotTo(BeNil())
Expect(updatedArtist.Biography).To(Equal("R&B"))
})
It("returns cached info when artist exists and info is not expired", func() {
now := time.Now()
originalArtist := &model.Artist{

View File

@ -45,7 +45,7 @@ func serveIndex(ds model.DataStore, fs fs.FS, shareInfo *model.Share) http.Handl
"variousArtistsId": consts.VariousArtistsID,
"baseURL": str.SanitizeText(strings.TrimSuffix(conf.Server.BasePath, "/")),
"loginBackgroundURL": str.SanitizeText(conf.Server.UILoginBackgroundURL),
"welcomeMessage": str.SanitizeText(conf.Server.UIWelcomeMessage),
"welcomeMessage": str.SanitizeHTML(conf.Server.UIWelcomeMessage),
"maxSidebarPlaylists": conf.Server.MaxSidebarPlaylists,
"enableTranscodingConfig": conf.Server.EnableTranscodingConfig,
"enableDownloads": conf.Server.EnableDownloads,

View File

@ -108,6 +108,18 @@ var _ = Describe("serveIndex", func() {
Entry("extAuthLogoutURL", func() { conf.Server.ExtAuth.LogoutURL = "https://auth.example.com/logout" }, "extAuthLogoutURL", "https://auth.example.com/logout"),
)
It("sanitizes entity-encoded welcomeMessage as html", func() {
conf.Server.UIWelcomeMessage = `&lt;img src=x onerror=alert(1)&gt;&lt;b&gt;Hello&lt;/b&gt;`
r := httptest.NewRequest("GET", "/index.html", nil)
w := httptest.NewRecorder()
serveIndex(ds, fs, nil)(w, r)
config := extractAppConfig(w.Body.String())
Expect(config).To(HaveKey("welcomeMessage"))
Expect(config["welcomeMessage"]).To(Equal(`<img src="x"><b>Hello</b>`))
})
DescribeTable("sets other UI configuration values",
func(configKey string, expectedValueFunc func() any) {
r := httptest.NewRequest("GET", "/index.html", nil)

View File

@ -38,11 +38,20 @@ func SanitizeStrings(text ...string) string {
var policy = bluemonday.UGCPolicy()
// SanitizeText unescapes the input string before sanitizing it as text.
// This should be used for fields rendered as plain text in the UI (e.g. lyrics, song titles, artist names)
func SanitizeText(text string) string {
s := policy.Sanitize(text)
return html.UnescapeString(s)
}
// SanitizeHTML unescapes the input string before sanitizing it as HTML.
// This should be used for fields rendered as HTML by clients (e.g. biographies, welcome messages)
// to prevent XSS bypasses via entity-encoded tags.
func SanitizeHTML(text string) string {
return policy.Sanitize(html.UnescapeString(text))
}
func SanitizeFieldForSorting(originalValue string) string {
v := strings.TrimSpace(sanitize.Accents(originalValue))
return Clear(strings.ToLower(v))

View File

@ -64,6 +64,35 @@ var _ = Describe("Sanitize Strings", func() {
})
})
Describe("SanitizeText", func() {
It("preserves decoded plaintext", func() {
Expect(str.SanitizeText("Tom &amp; Jerry")).To(Equal("Tom & Jerry"))
Expect(str.SanitizeText("Tom & Jerry")).To(Equal("Tom & Jerry"))
})
It("keeps entity-encoded html readable", func() {
Expect(str.SanitizeText(`&lt;b&gt;ok&lt;/b&gt;`)).To(Equal("<b>ok</b>"))
})
})
Describe("SanitizeHTML", func() {
It("removes dangerous content from raw html", func() {
sanitized := str.SanitizeHTML(`<img src=x onerror=alert(1)><script>alert(2)</script><b>ok</b>`)
Expect(sanitized).To(ContainSubstring("<b>ok</b>"))
Expect(sanitized).ToNot(ContainSubstring("onerror"))
Expect(sanitized).ToNot(ContainSubstring("<script"))
})
It("removes dangerous content from entity-encoded html", func() {
sanitized := str.SanitizeHTML(`&lt;img src=x onerror=alert(1)&gt;&lt;script&gt;alert(2)&lt;/script&gt;&lt;b&gt;ok&lt;/b&gt;`)
Expect(sanitized).To(ContainSubstring("<b>ok</b>"))
Expect(sanitized).ToNot(ContainSubstring("onerror"))
Expect(sanitized).ToNot(ContainSubstring("<script"))
})
})
Describe("SanitizeFieldForSortingNoArticle", func() {
BeforeEach(func() {
conf.Server.IgnoredArticles = "The O"