diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go index 5dfcc43851..7f86857fe3 100644 --- a/coderd/notifications/dispatch/smtp.go +++ b/coderd/notifications/dispatch/smtp.go @@ -66,7 +66,7 @@ func (s *SMTPHandler) Dispatcher(payload types.MessagePayload, titleTmpl, bodyTm return nil, xerrors.Errorf("render subject: %w", err) } - htmlBody := markdown.HTMLFromMarkdown(bodyTmpl) + htmlBody := markdown.HTMLFromMarkdownSafe(bodyTmpl) plainBody, err := markdown.PlaintextFromMarkdown(bodyTmpl) if err != nil { return nil, xerrors.Errorf("render plaintext body: %w", err) diff --git a/coderd/notifications/dispatch/smtp/html.gotmpl b/coderd/notifications/dispatch/smtp/html.gotmpl index cecba560af..770696e870 100644 --- a/coderd/notifications/dispatch/smtp/html.gotmpl +++ b/coderd/notifications/dispatch/smtp/html.gotmpl @@ -3,7 +3,7 @@ - {{ .Labels._subject }} + {{ .Labels._subject | html }}
@@ -11,16 +11,16 @@ {{ app_name | html }} Logo

- {{ .Labels._subject }} + {{ .Labels._subject | html }}

-

Hi {{ .UserName }},

+

Hi {{ .UserName | html }},

{{ .Labels._body }}
{{ range $action := .Actions }} - - {{ $action.Label }} + + {{ $action.Label | html }} {{ end }}
diff --git a/coderd/notifications/dispatch/smtp_internal_test.go b/coderd/notifications/dispatch/smtp_internal_test.go index 2e7dff8cbe..5aee2385c2 100644 --- a/coderd/notifications/dispatch/smtp_internal_test.go +++ b/coderd/notifications/dispatch/smtp_internal_test.go @@ -7,6 +7,8 @@ import ( "github.com/stretchr/testify/require" + markdown "github.com/coder/coder/v2/coderd/render" + "github.com/coder/coder/v2/coderd/notifications/render" "github.com/coder/coder/v2/coderd/notifications/types" ) @@ -43,6 +45,354 @@ func TestSMTPHTMLTemplateEscapesAppearanceHelpers(t *testing.T) { require.False(t, strings.Contains(got, logoURL), "raw logo URL must not be rendered") } +// TestSMTPHTMLTemplateMarkdownInjection is an end-to-end regression test that +// exercises the full notification email rendering pipeline with malicious input. +// It simulates the complete flow: SanitizePayload -> GoTemplate (body) -> +// HTMLFromMarkdownSafe -> GoTemplate (outer HTML template) and asserts that the +// final HTML email output contains no attacker-controlled links, headings, images, +// scripts, or other injected content. +// +// If this test fails, it likely means a change to the rendering pipeline has +// introduced a Markdown or HTML injection vulnerability in notification emails. +func TestSMTPHTMLTemplateMarkdownInjection(t *testing.T) { + t.Parallel() + + helpers := map[string]any{ + "base_url": func() string { return "https://coder.example.com" }, + "current_year": func() string { return "2026" }, + "logo_url": func() string { return "https://coder.example.com/logo.png" }, + "app_name": func() string { return "Coder" }, + } + + tests := []struct { + name string + // payload is the unsanitized notification payload (as it would arrive + // from the database before SanitizePayload runs). + payload types.MessagePayload + // bodyTemplate is the notification body template (stored in the DB). + bodyTemplate string + // absentInHTML lists substrings that must NOT appear in the final HTML + // email output. + absentInHTML []string + // presentInHTML lists substrings that MUST appear in the final HTML. + presentInHTML []string + }{ + { + name: "markdown link injection via display name", + payload: types.MessagePayload{ + NotificationTemplateID: "00000000-0000-0000-0000-000000000000", + UserName: "Eve\n[Re-authenticate](https://evil.example/login)", + Labels: map[string]string{ + "created_account_user_name": "Eve\n[Re-authenticate](https://evil.example/login)", + "initiator": "admin", + }, + }, + bodyTemplate: `Account created for **{{.Labels.created_account_user_name}}** by **{{.Labels.initiator}}**.`, + absentInHTML: []string{ + `admin", + }, + }, + { + name: "heading injection via display name", + payload: types.MessagePayload{ + NotificationTemplateID: "00000000-0000-0000-0000-000000000000", + UserName: "Eve\n## URGENT SECURITY ALERT", + Labels: map[string]string{ + "suspended_account_user_name": "Eve\n## URGENT SECURITY ALERT", + "initiator": "admin", + }, + }, + bodyTemplate: `Account **{{.Labels.suspended_account_user_name}}** suspended by **{{.Labels.initiator}}**.`, + absentInHTML: []string{ + "

", + "

", + "

", + }, + presentInHTML: []string{ + "## URGENT SECURITY ALERT", // rendered as literal text + }, + }, + { + name: "image tracking pixel injection", + payload: types.MessagePayload{ + NotificationTemplateID: "00000000-0000-0000-0000-000000000000", + UserName: "![](https://evil.example/track.gif)", + Labels: map[string]string{ + "name": "![](https://evil.example/track.gif)", + }, + }, + bodyTemplate: `Workspace **{{.Labels.name}}** deleted.`, + absentInHTML: []string{ + `src="https://evil.example`, // no attacker-controlled image source + }, + presentInHTML: []string{ + "![]", // rendered as literal text + }, + }, + { + name: "javascript URI via markdown link", + payload: types.MessagePayload{ + NotificationTemplateID: "00000000-0000-0000-0000-000000000000", + UserName: "normal", + Labels: map[string]string{ + "name": "[xss](javascript:alert(document.cookie))", + }, + }, + bodyTemplate: `Workspace **{{.Labels.name}}** updated.`, + absentInHTML: []string{ + `href="javascript:`, // no javascript link + }, + }, + { + name: "raw HTML injection in display name", + payload: types.MessagePayload{ + NotificationTemplateID: "00000000-0000-0000-0000-000000000000", + UserName: ``, + Labels: map[string]string{}, + }, + bodyTemplate: `Hello.`, + absentInHTML: []string{ + ")", + absent: []string{"data:", "bold", "italic"}, + }, + { + name: "raw HTML stripped", + input: ``, + absent: []string{"