diff --git a/docs/content/docs/learn/personalization.mdx b/docs/content/docs/learn/personalization.mdx index 039689d5e..6b927e945 100644 --- a/docs/content/docs/learn/personalization.mdx +++ b/docs/content/docs/learn/personalization.mdx @@ -172,7 +172,11 @@ Warmbly guards against broken templates at three points: - A campaign won't start if any step's template is malformed. You get an error naming the step, so recipients never receive raw `{{if ...}}` text. - If a broken template ever reaches sending, it falls back to plain variable substitution rather than failing the send. -Templates also have limits, so one template cannot tie up the server. `{{range}}` only loops over a field of the data (`{{range .items}}`), never over a number or a function's result, and loops nest at most two deep. `{{template}}` and `{{block}}` are not supported. A template that breaks one of these rules is treated as malformed, and one whose output passes 1 MB falls back to plain substitution. +Templates also have limits, so one template cannot tie up the server. `{{range}}` only loops over a field of the data (`{{range .items}}`), never over a number or a function's result, and loops nest at most two deep. `{{template}}`, `{{block}}` and `call` are not supported, a template declares at most 64 variables, and a function takes at most 32 arguments. A template that breaks one of these rules is treated as malformed. A render that passes 1 MB of output or too many loop steps falls back to plain substitution, and that fallback stays within the same 1 MB. + +## Values in the HTML body + +In the HTML body every variable is escaped for the place it lands, so a contact whose name is `Tom & Jerry` reads exactly that way instead of turning bold. A variable inside a link (``) keeps web, `mailto:` and `tel:` addresses and is replaced with a harmless `#ZgotmplZ` for anything else, and a value placed in a link's query is URL-encoded. The subject and the plain-text part are sent as written. The mailbox signature (`{{.Sender.SignatureHTML}}`) is your own markup and is kept as is. Always check Preview and send yourself a test email before launching. diff --git a/internal/tasks/ai_variables.go b/internal/tasks/ai_variables.go index 12066bd2f..a9beb30ea 100644 --- a/internal/tasks/ai_variables.go +++ b/internal/tasks/ai_variables.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "html" "regexp" "strings" "time" @@ -451,7 +452,7 @@ func BuildAIVariablePrompt(vc generation.VoiceContext, contact models.Contact, i } // substituteAIVariables replaces resolved blocks in all three strings. In HTML -// the WHOLE span is swapped for the text (config never ships to the recipient); +// the WHOLE span is swapped for the escaped text (config never ships to the recipient); // in plain text and subject the bare [[ai:ID]] token is swapped. Any leftover // [[ai:ID]] with no resolved entry is replaced with "" (missingkey=zero). func substituteAIVariables(subject, bodyHTML, bodyPlain string, resolved map[string]string) (string, string, string) { @@ -460,7 +461,7 @@ func substituteAIVariables(subject, bodyHTML, bodyPlain string, resolved map[str if len(m) < 2 { return "" } - return resolved[m[1]] // "" when unresolved + return html.EscapeString(resolved[m[1]]) // "" when unresolved }) bodyPlain = replaceAIVarTokens(bodyPlain, resolved) subject = replaceAIVarTokens(subject, resolved) diff --git a/internal/tasks/campaign_task.go b/internal/tasks/campaign_task.go index b40fac690..defc5b1f8 100644 --- a/internal/tasks/campaign_task.go +++ b/internal/tasks/campaign_task.go @@ -630,7 +630,7 @@ func (s *tasksService) HandleCampaignTask(task *proto.ProcessTask) (result *errx // {a|b|c} spintax per-recipient (only real |-groups; literal braces/CSS are // left intact) so each send varies for deliverability. subject := expandSpintax(RenderTemplateWith(rawSubject, *contact, extra)) - bodyHTML := expandSpintax(RenderTemplateWith(rawBodyHTML, *contact, extra)) + bodyHTML := expandSpintax(RenderHTMLTemplateWith(rawBodyHTML, *contact, extra)) bodyPlain := expandSpintax(RenderTemplateWith(rawBodyPlain, *contact, extra)) // A recorded conversation subject is already rendered: reuse it verbatim. if threadParent != nil && threadParent.SubjectSent && threadSubject != "" { @@ -658,7 +658,7 @@ func (s *tasksService) HandleCampaignTask(task *proto.ProcessTask) (result *errx if threadSubject == "" { subject = expandSpintax(RenderTemplateWith(selection.Subject, *contact, extra)) } - bodyHTML = expandSpintax(RenderTemplateWith(selection.BodyHTML, *contact, extra)) + bodyHTML = expandSpintax(RenderHTMLTemplateWith(selection.BodyHTML, *contact, extra)) bodyPlain = expandSpintax(RenderTemplateWith(selection.BodyPlain, *contact, extra)) // A variant may carry HTML only; keep the plain-text alternative. if bodyPlain == "" && bodyHTML != "" { diff --git a/internal/tasks/placement_task.go b/internal/tasks/placement_task.go index a14948d38..d1e7b1a68 100644 --- a/internal/tasks/placement_task.go +++ b/internal/tasks/placement_task.go @@ -276,7 +276,7 @@ func (s *tasksService) renderPlacementBase(ctx context.Context, test *models.Pla } extra := templateContext(account, unsubscribeURL) subject := expandSpintax(RenderTemplateWith(rawSubject, contact, extra)) - bodyHTML := expandSpintax(RenderTemplateWith(rawHTML, contact, extra)) + bodyHTML := expandSpintax(RenderHTMLTemplateWith(rawHTML, contact, extra)) bodyPlain := expandSpintax(RenderTemplateWith(rawPlain, contact, extra)) if bodyPlain == "" && bodyHTML != "" { bodyPlain = ExtractPlainTextFromHTML(bodyHTML) diff --git a/internal/tasks/template.go b/internal/tasks/template.go index 9f5db9ab3..989b3458f 100644 --- a/internal/tasks/template.go +++ b/internal/tasks/template.go @@ -183,19 +183,71 @@ func RenderTemplateWith(tmpl string, contact models.Contact, context TemplateCon return strings.ReplaceAll(out, "", "") } -// Recover standalone actions with the same engine, leaving broken control syntax literal. +// RenderHTMLTemplateWith renders an HTML body: every value is escaped for the +// context it lands in (text, attribute, URL, CSS), so contact data can never +// add markup or a script: link. Template text, comments included, is kept. +func RenderHTMLTemplateWith(tmpl string, contact models.Contact, context TemplateContext) string { + if !strings.Contains(tmpl, "{{") { + return tmpl + } + data := buildTemplateData(contact, context) + if body := compiledHTMLTemplate(tmpl); body != nil { + if out, err := tmplfuncs.Execute(body.t, data); err == nil { + return body.restore(out) + } + } + return fallbackRenderHTML(tmpl, data) +} + +// Recover standalone actions with the same engine, leaving broken control +// syntax literal; all of them share one MaxOutput budget. func fallbackRenderTemplate(tmpl string, data map[string]any) string { - return templateAction.ReplaceAllStringFunc(tmpl, func(action string) string { + return fallbackRender(tmpl, data, nil) +} + +// fallbackRenderHTML is fallbackRenderTemplate with each value escaped for +// where it sits in the document. +func fallbackRenderHTML(tmpl string, data map[string]any) string { + return fallbackRender(tmpl, data, &htmlScanner{}) +} + +func fallbackRender(tmpl string, data map[string]any, scan *htmlScanner) string { + budget := tmplfuncs.MaxOutput + var b strings.Builder + last := 0 + for _, loc := range templateAction.FindAllStringIndex(tmpl, -1) { + literal, action := tmpl[last:loc[0]], tmpl[loc[0]:loc[1]] + b.WriteString(literal) + last = loc[1] + var at htmlPosition + if scan != nil { + scan.feed(literal) + at = scan.position() + scan.sawValue() + } t := compiledTemplate(action) if t == nil { - return action + b.WriteString(action) + continue } - out, err := tmplfuncs.Execute(t, data) + out, err := tmplfuncs.ExecuteLimit(t, data, budget) if err != nil { - return action + b.WriteString(action) + continue } - return strings.ReplaceAll(out, "", "") - }) + out = strings.ReplaceAll(out, "", "") + if scan != nil { + out = at.escape(out) + } + if len(out) > budget { + b.WriteString(action) + continue + } + budget -= len(out) + b.WriteString(out) + } + b.WriteString(tmpl[last:]) + return b.String() } // TemplatePreview is the result of rendering a campaign template against one @@ -233,7 +285,7 @@ func PreviewTemplates(subject, bodyHTML, bodyPlain string, contact models.Contac func previewTemplatesWith(subject, bodyHTML, bodyPlain string, contact models.Contact, context TemplateContext) TemplatePreview { p := TemplatePreview{ Subject: expandSpintax(RenderTemplateWith(subject, contact, context)), - BodyHTML: expandSpintax(RenderTemplateWith(bodyHTML, contact, context)), + BodyHTML: expandSpintax(RenderHTMLTemplateWith(bodyHTML, contact, context)), BodyPlain: expandSpintax(RenderTemplateWith(bodyPlain, contact, context)), } for _, f := range []struct{ name, raw string }{{"subject", subject}, {"body", bodyHTML}, {"plain text", bodyPlain}} { diff --git a/internal/tasks/template_html.go b/internal/tasks/template_html.go new file mode 100644 index 000000000..ac9da1a7d --- /dev/null +++ b/internal/tasks/template_html.go @@ -0,0 +1,269 @@ +package tasks + +import ( + "html" + htmltemplate "html/template" + "regexp" + "strconv" + "strings" + + "github.com/warmbly/warmbly/internal/pkg/tmplfuncs" +) + +// htmlBody is a compiled HTML body plus the comments shielded from the escaper, +// which would otherwise strip them (Outlook's conditional comments among them). +type htmlBody struct { + t *htmltemplate.Template + comments []string +} + +var htmlTmplCache tmplfuncs.TypedCache[*htmlBody] + +var htmlComment = regexp.MustCompile(`(?s)`) + +// Placeholders hold NUL, which the escaper never emits and the source is cleared of. +const ( + shieldOpen = "\x00wo\x00" + shieldClose = "\x00wc\x00" + shieldBang = "\x00wb\x00" +) + +var shieldToken = regexp.MustCompile("\x00w(o|c|b|[0-9]+)\x00") + +// shieldComments swaps each comment for a placeholder; a comment holding an +// action keeps its body in place, so that action is escaped like any other. +func shieldComments(src string) (string, []string) { + var comments []string + out := htmlComment.ReplaceAllStringFunc(src, func(c string) string { + if strings.Contains(c, "{{") { + return shieldOpen + strings.ReplaceAll(c[4:len(c)-3], "" + case 'b': + return "= len(h.comments) { + return "" + } + return h.comments[i] + }) +} + +// compiledHTMLTemplate returns the cached HTML body for tmpl, nil when it does not compile. +func compiledHTMLTemplate(tmpl string) *htmlBody { + if body, ok := htmlTmplCache.Load(tmpl); ok { + return body + } + src, comments := shieldComments(strings.ReplaceAll(tmpl, "\x00", "")) + t, err := tmplfuncs.CompileHTML("body", rewriteSpacedFieldRefs(src)) + if err != nil { + htmlTmplCache.Store(tmpl, nil) + return nil + } + body := &htmlBody{t: t, comments: comments} + htmlTmplCache.Store(tmpl, body) + return body +} + +type htmlState int + +const ( + inText htmlState = iota + inComment + inTag + inQuotedValue + inUnquotedValue + inRawText +) + +// htmlScanner tracks where a fallback render is in the document, so each value +// it inserts is escaped for that place. +type htmlScanner struct { + state htmlState + quote byte + name strings.Builder + attr string + tag string + started bool +} + +// htmlPosition is where one value lands. +type htmlPosition struct { + state htmlState + attr string + tag string + started bool +} + +func (s *htmlScanner) position() htmlPosition { + return htmlPosition{state: s.state, attr: s.attr, tag: s.tag, started: s.started} +} + +// sawValue records that a value was written into the current attribute. +func (s *htmlScanner) sawValue() { + if s.state == inQuotedValue { + s.started = true + } +} + +func isLetter(c byte) bool { return c|0x20 >= 'a' && c|0x20 <= 'z' } + +func (s *htmlScanner) feed(text string) { + for i := 0; i < len(text); i++ { + c := text[i] + switch s.state { + case inText: + switch { + case strings.HasPrefix(text[i:], "") { + s.state, i = inText, i+2 + } + case inTag: + switch { + case c == '>': + s.state = inText + if s.tag == "script" || s.tag == "style" || s.tag == "textarea" || s.tag == "title" { + s.state = inRawText + } + case c == '=': + s.attr = strings.ToLower(strings.TrimSpace(s.name.String())) + s.name.Reset() + j := i + 1 + for j < len(text) && (text[j] == ' ' || text[j] == '\t' || text[j] == '\n' || text[j] == '\r') { + j++ + } + if j < len(text) && (text[j] == '"' || text[j] == '\'') { + s.state, s.quote, s.started, i = inQuotedValue, text[j], false, j + } else { + s.state, i = inUnquotedValue, j-1 + } + case c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '/': + s.name.Reset() + default: + s.name.WriteByte(c) + } + case inQuotedValue: + if c == s.quote { + s.state, s.attr = inTag, "" + } else if c != ' ' && c != '\t' && c != '\n' && c != '\r' { + s.started = true + } + case inUnquotedValue: + switch c { + case ' ', '\t', '\n', '\r': + s.state, s.attr = inTag, "" + case '>': + s.state, s.attr = inTag, "" + i-- + } + case inRawText: + if c == '<' && strings.HasPrefix(strings.ToLower(text[i:]), "= '0' && c <= '9' || c == '-' || c == '.' || c == '_' || c == '~' { + b.WriteByte(c) + continue + } + b.WriteByte('%') + b.WriteByte(hex[c>>4]) + b.WriteByte(hex[c&15]) + } + return b.String() +} diff --git a/internal/tasks/template_html_test.go b/internal/tasks/template_html_test.go new file mode 100644 index 000000000..d3b71d998 --- /dev/null +++ b/internal/tasks/template_html_test.go @@ -0,0 +1,140 @@ +package tasks + +import ( + "strings" + "testing" + + "github.com/google/uuid" + "github.com/warmbly/warmbly/internal/models" + "github.com/warmbly/warmbly/internal/pkg/tmplfuncs" +) + +const markupName = `Tom & "Jerry" 's` + +func htmlContact(name string) models.Contact { + return models.Contact{FirstName: name, Company: "Acme", CustomFields: map[string]string{ + "website": "https://acme.com/?a=1&b=2", + "evil": "javascript:alert(1)", + "Evil2": " JaVaScRiPt:alert(1)", + "job": "Head Sales", + "job title": "VP & CTO", + }} +} + +func TestRenderHTMLTemplateEscapesValuesInContext(t *testing.T) { + sender := TemplateContext{ + Sender: templateContext(&models.Email{SignatureHTML: "Ana"}, "").Sender, + UnsubscribeLink: "https://example.com/unsubscribe/tok_123", + } + cases := []struct { + name, tmpl, first, want string + }{ + {"plain name", `

Hi {{.FirstName}},

`, "Ann", `

Hi Ann,

`}, + {"markup in a name", `

Hi {{.FirstName}},

`, markupName, `

Hi Tom & "Jerry" <b>'s,

`}, + {"markup in an attribute", `{{.FirstName}}`, markupName, `Tom & "Jerry" <b>'s`}, + {"custom field", `

{{.job}} at {{.Company}}

`, "Ann", `

Head <of> Sales at Acme

`}, + {"spaced custom field", `

{{.job title}}

`, "Ann", `

VP & CTO

`}, + {"href keeps a web link", `
site`, "Ann", `site`}, + {"href refuses a script link", `x`, "Ann", `x`}, + {"href refuses a padded script link", `x`, "Ann", `x`}, + {"value in a query", `x`, "Ann Lee", `x`}, + {"unsubscribe link", `Unsubscribe {{.UnsubscribeLink}}`, "Ann", `Unsubscribe https://example.com/unsubscribe/tok_123`}, + {"signature stays markup", `

{{.FirstName}}

{{.Sender.SignatureHTML}}`, "Ann", `

Ann

Ana`}, + {"missing key", `

[{{.nothing}}]

`, "Ann", `

[]

`}, + {"conditional", `{{if eq .FirstName "Tom & \"Jerry\" 's"}}

match

{{end}}`, markupName, `

match

`}, + {"outlook comments", `

{{.FirstName}}

`, "Ann", + `

Ann

`}, + {"action inside a comment", `

{{.FirstName}}

`, "Ann", + `

Ann

`}, + {"no actions is untouched", `

1 < 2 & a+b

`, "Ann", `

1 < 2 & a+b

`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := RenderHTMLTemplateWith(tc.tmpl, htmlContact(tc.first), sender); got != tc.want { + t.Fatalf("got %q\nwant %q", got, tc.want) + } + }) + } +} + +func TestRenderHTMLTemplateFallbackEscapes(t *testing.T) { + cases := []struct{ name, tmpl, want string }{ + {"text", `{{if .Company}}

{{.FirstName}}

`, `{{if .Company}}

Tom & "Jerry" <b>'s

`}, + {"attribute", `{{if .Company}}

`, `{{if .Company}}

`}, + {"script link", `{{if .Company}}x`, `{{if .Company}}x`}, + {"web link", `{{if .Company}}x`, `{{if .Company}}x`}, + {"split link", `{{if .Company}}x`, `{{if .Company}}x`}, + {"event handler", `{{if .Company}}

`, `{{if .Company}}

`}, + {"inside a tag", `{{if .Company}}

`, `{{if .Company}}

`}, + {"script", `{{if .Company}}`, `{{if .Company}}`}, + {"after a comment", `{{if .Company}}

{{.FirstName}}

`, `{{if .Company}}

Tom & "Jerry" <b>'s

`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := RenderHTMLTemplateWith(tc.tmpl, htmlContact(markupName), TemplateContext{}); got != tc.want { + t.Fatalf("got %q\nwant %q", got, tc.want) + } + }) + } +} + +func TestRenderTemplatePlainPartIsNotEscaped(t *testing.T) { + if got := RenderTemplateWith(`Hi {{.FirstName}}, {{.website}}`, htmlContact(markupName), TemplateContext{}); got != `Hi `+markupName+`, https://acme.com/?a=1&b=2` { + t.Fatalf("plain text was altered: %q", got) + } +} + +func TestRenderHTMLTemplateWithSpintax(t *testing.T) { + got := expandSpintax(RenderHTMLTemplateWith(`

{Hi|Hi} {{.FirstName}}

`, htmlContact("{a|a}"), TemplateContext{})) + if got != `

Hi <i>a</i>

` { + t.Fatalf("got %q", got) + } +} + +func TestRenderHTMLTemplateThenTrackLinks(t *testing.T) { + body := RenderHTMLTemplateWith(`site x`, htmlContact("Ann"), TemplateContext{}) + _, links := WrapLinksForTracking(body, uuid.New(), uuid.New(), "track.example.com") + if len(links) != 1 || links[0].Destination != "https://acme.com/?a=1&b=2" { + t.Fatalf("tracked %+v from %q", links, body) + } +} + +func TestRenderHTMLTemplateThenLinkifyUnsubscribe(t *testing.T) { + link := "https://example.com/unsubscribe/tok_123" + body := RenderHTMLTemplateWith(`

Opt out: {{.UnsubscribeLink}}

`, htmlContact("Ann"), TemplateContext{UnsubscribeLink: link}) + if got := linkifyUnsubscribeURL(body, link, "Unsubscribe"); got != `

Opt out: Unsubscribe

` { + t.Fatalf("got %q", got) + } +} + +func TestRenderTemplateOutputCapHoldsOnFallback(t *testing.T) { + big := strings.Repeat("x", tmplfuncs.MaxOutput/4) + contact := models.Contact{CustomFields: map[string]string{"big": big}} + for _, tmpl := range []string{ + strings.Repeat("{{.big}}", 10), + "{{if .big}}" + strings.Repeat("{{.big}}", 10), + } { + for name, render := range map[string]func(string, models.Contact, TemplateContext) string{ + "text": RenderTemplateWith, "html": RenderHTMLTemplateWith, + } { + out := render(tmpl, contact, TemplateContext{}) + if len(out) > tmplfuncs.MaxOutput+len(tmpl) { + t.Fatalf("%s render wrote %d bytes", name, len(out)) + } + if !strings.Contains(out, big) { + t.Fatalf("%s render dropped every value", name) + } + } + } +} + +func TestSubstituteAIVariablesEscapesHTMLOnly(t *testing.T) { + span := `[[ai:a1]]` + subject, bodyHTML, bodyPlain := substituteAIVariables("Re [[ai:a1]]", "

"+span+"

", "Hi [[ai:a1]]", map[string]string{"a1": ` & co`}) + if bodyHTML != `

<img src=x onerror=alert(1)> & co

` { + t.Fatalf("html: %q", bodyHTML) + } + if bodyPlain != `Hi & co` || subject != `Re & co` { + t.Fatalf("plain %q subject %q", bodyPlain, subject) + } +} diff --git a/internal/tasks/template_sender.go b/internal/tasks/template_sender.go index 5f09063c8..53a928e5d 100644 --- a/internal/tasks/template_sender.go +++ b/internal/tasks/template_sender.go @@ -1,6 +1,7 @@ package tasks import ( + htmltemplate "html/template" "strings" "time" @@ -9,13 +10,14 @@ import ( // TemplateSender is an explicit allowlist: never embed the mailbox model or its methods. type TemplateSender struct { - Name string - Email string - MailboxEmail string - SendAsEmail string - ReplyTo string - SignaturePlain string - SignatureHTML string + Name string + Email string + MailboxEmail string + SendAsEmail string + ReplyTo string + SignaturePlain string + // SignatureHTML is the mailbox's own markup, so an HTML body emits it as written. + SignatureHTML htmltemplate.HTML SignatureSync bool SignatureCode bool Provider string @@ -73,7 +75,7 @@ func templateContext(account *models.Email, unsubscribeURL string) TemplateConte SendAsEmail: strings.TrimSpace(account.SendAsEmail), ReplyTo: account.ReplyToHeader(), SignaturePlain: account.SignaturePlain, - SignatureHTML: account.SignatureHTML, + SignatureHTML: htmltemplate.HTML(strings.ReplaceAll(account.SignatureHTML, "\x00", "")), SignatureSync: account.SignatureSync, SignatureCode: account.SignatureCode, Provider: account.Provider, diff --git a/internal/tasks/template_test.go b/internal/tasks/template_test.go index 37109e51a..d3dd1afd2 100644 --- a/internal/tasks/template_test.go +++ b/internal/tasks/template_test.go @@ -1,6 +1,7 @@ package tasks import ( + htmltemplate "html/template" "reflect" "strings" "testing" @@ -174,6 +175,9 @@ func TestTemplateSender_MapsEveryAllowedMailboxField(t *testing.T) { if name == "ReplyTo" { want = account.ReplyToHeader() } + if name == "SignatureHTML" { + want = htmltemplate.HTML(account.SignatureHTML) + } if !reflect.DeepEqual(sender.Field(i).Interface(), want) { t.Errorf("Sender.%s = %v, want %v", name, sender.Field(i).Interface(), want) }