fix(notifications): only sanitze html content in notifications, do not convert it to markdown

Resolves https://community.vikunja.io/t/trello-import-html-mails/2197

pkg/models/notifications.go

@@ -17,10 +17,8 @@
 package models
 
 import (
-	"bufio"
 	"sort"
 	"strconv"
-	"strings"
 	"time"
 
 	"code.vikunja.io/api/pkg/utils"
@@ -77,20 +75,16 @@ func (n *TaskCommentNotification) SubjectID() int64 {
 func (n *TaskCommentNotification) ToMail() *notifications.Mail {
 
 	mail := notifications.NewMail().
-		From(n.Doer.GetNameAndFromEmail())
+		From(n.Doer.GetNameAndFromEmail()).
+		Subject("Re: " + n.Task.Title)
 
-	subject := "Re: " + n.Task.Title
 	if n.Mentioned {
-		subject = n.Doer.GetName() + ` mentioned you in a comment in "` + n.Task.Title + `"`
-		mail.Line("**" + n.Doer.GetName() + "** mentioned you in a comment:")
+		mail.
+			Line("**" + n.Doer.GetName() + "** mentioned you in a comment:").
+			Subject(n.Doer.GetName() + ` mentioned you in a comment in "` + n.Task.Title + `"`)
 	}
 
-	mail.Subject(subject)
-
-	lines := bufio.NewScanner(strings.NewReader(n.Comment.Comment))
-	for lines.Scan() {
-		mail.Line(lines.Text())
-	}
+	mail.HTML(n.Comment.Comment)
 
 	return mail.
 		Action("View Task", n.Task.GetFrontendURL())
@@ -306,12 +300,8 @@ func (n *UserMentionedInTaskNotification) ToMail() *notifications.Mail {
 	mail := notifications.NewMail().
 		From(n.Doer.GetNameAndFromEmail()).
 		Subject(subject).
-		Line("**" + n.Doer.GetName() + "** mentioned you in a task:")
-
-	lines := bufio.NewScanner(strings.NewReader(n.Task.Description))
-	for lines.Scan() {
-		mail.Line(lines.Text())
-	}
+		Line("**" + n.Doer.GetName() + "** mentioned you in a task:").
+		HTML(n.Task.Description)
 
 	return mail.
 		Action("View Task", n.Task.GetFrontendURL())

pkg/notifications/mail.go

@@ -26,8 +26,13 @@ type Mail struct {
 	actionText string
 	actionURL  string
 	greeting   string
-	introLines []string
-	outroLines []string
+	introLines []*mailLine
+	outroLines []*mailLine
+}
+
+type mailLine struct {
+	text   string
+	isHTML bool
 }
 
 // NewMail creates a new mail object with a default greeting
@@ -68,12 +73,26 @@ func (m *Mail) Action(text, url string) *Mail {
 
 // Line adds a line of text to the mail
 func (m *Mail) Line(line string) *Mail {
+	return m.appendLine(line, false)
+}
+
+func (m *Mail) HTML(line string) *Mail {
+	return m.appendLine(line, true)
+}
+
+func (m *Mail) appendLine(line string, isHTML bool) *Mail {
 	if m.actionURL == "" {
-		m.introLines = append(m.introLines, line)
+		m.introLines = append(m.introLines, &mailLine{
+			text:   line,
+			isHTML: isHTML,
+		})
 		return m
 	}
 
-	m.outroLines = append(m.outroLines, line)
+	m.outroLines = append(m.outroLines, &mailLine{
+		text:   line,
+		isHTML: isHTML,
+	})
 
 	return m
 }

pkg/notifications/mail_render.go

@@ -23,6 +23,8 @@ import (
 	templatehtml "html/template"
 	templatetext "text/template"
 
+	"github.com/microcosm-cc/bluemonday"
+
 	"code.vikunja.io/api/pkg/config"
 	"code.vikunja.io/api/pkg/mail"
 	"code.vikunja.io/api/pkg/utils"
@@ -117,29 +119,43 @@ func RenderMail(m *Mail) (mailOpts *mail.Opts, err error) {
 	data["Boundary"] = boundary
 	data["FrontendURL"] = config.ServicePublicURL.GetString()
 
+	p := bluemonday.UGCPolicy()
+
 	var introLinesHTML []templatehtml.HTML
 	for _, line := range m.introLines {
-		md := []byte(templatehtml.HTMLEscapeString(line))
+		if line.isHTML {
+			// #nosec G203 -- the html is sanitized
+			introLinesHTML = append(introLinesHTML, templatehtml.HTML(p.Sanitize(line.text)))
+			continue
+		}
+
+		md := []byte(templatehtml.HTMLEscapeString(line.text))
 		var buf bytes.Buffer
 		err = goldmark.Convert(md, &buf)
 		if err != nil {
 			return nil, err
 		}
-		//#nosec - the html is escaped few lines before
-		introLinesHTML = append(introLinesHTML, templatehtml.HTML(buf.String()))
+		// #nosec G203 -- the html is sanitized
+		introLinesHTML = append(introLinesHTML, templatehtml.HTML(p.Sanitize(buf.String())))
 	}
 	data["IntroLinesHTML"] = introLinesHTML
 
 	var outroLinesHTML []templatehtml.HTML
 	for _, line := range m.outroLines {
-		md := []byte(templatehtml.HTMLEscapeString(line))
+		if line.isHTML {
+			// #nosec G203 -- the html is sanitized
+			outroLinesHTML = append(outroLinesHTML, templatehtml.HTML(p.Sanitize(line.text)))
+			continue
+		}
+
+		md := []byte(templatehtml.HTMLEscapeString(line.text))
 		var buf bytes.Buffer
 		err = goldmark.Convert(md, &buf)
 		if err != nil {
 			return nil, err
 		}
-		//#nosec - the html is escaped few lines before
-		outroLinesHTML = append(outroLinesHTML, templatehtml.HTML(buf.String()))
+		// #nosec G203 -- the html is sanitized
+		outroLinesHTML = append(outroLinesHTML, templatehtml.HTML(p.Sanitize(buf.String())))
 	}
 	data["OutroLinesHTML"] = outroLinesHTML