Skip to content

fix(security): sanitize user-controlled fields before email rendering - #210

Open
Emmanuelchukwunonso wants to merge 2 commits into
parkerwinner:mainfrom
Emmanuelchukwunonso:security/email-sanitization
Open

fix(security): sanitize user-controlled fields before email rendering#210
Emmanuelchukwunonso wants to merge 2 commits into
parkerwinner:mainfrom
Emmanuelchukwunonso:security/email-sanitization

Conversation

@Emmanuelchukwunonso

Copy link
Copy Markdown
Contributor

Summary

Investigated #194's premise before implementing: all 4 email templates already use html/template (not text/template), and none disable escaping via template.HTML(...). Verified empirically that html/template already contextually escapes malicious markup for every {{.Field}} substitution (e.g. <script>alert(1)</script> renders as &lt;script&gt;alert(1)&lt;/script&gt;). Subject lines carry no user-controlled data, so header injection isn't reachable there either.

Added sanitizeEmailField() as defense-in-depth per the issue's literal acceptance criteria ("sanitize all user input before email rendering"): strips HTML/script tags and control characters (including CR/LF) from UserName, RecipientAccount, and Reason fields before they enter template data, so this property doesn't silently depend on every future template staying on html/template.

Also fixed (blocking, unrelated to this issue, pre-existing on main):

  • handlers/auth.go: stray }) after ResetPassword's closing brace — a syntax error that blocks the whole module from building.
  • go.mod: testcontainers-go v0.17.1 doesn't exist on the module proxy; pinned to v0.17.0.

Test plan

  • go build ./services/...
  • go test ./services/... -v — existing suite passes unchanged
  • New email_sanitization_test.go: tag stripping, control-character stripping, whitespace trimming, and a pinning test for html/template's own auto-escaping

Closes #194

Investigation: all 4 email templates (payment completed, escrow
expiration warning, payment failed, password reset) already use
html/template (not text/template), and none use template.HTML(...) to
disable escaping — confirmed empirically that html/template already
contextually escapes malicious markup (e.g. "<script>...</script>"
renders as "&lt;script&gt;...&lt;/script&gt;") for every {{.Field}}
substitution. Subject lines carry no user-controlled data, so header
injection isn't reachable there either.

Added sanitizeEmailField() as defense-in-depth per the issue's
acceptance criteria: strips HTML/script tags and control characters
(including CR/LF) from user, recipient, and reason fields before they
enter template data, so the property doesn't silently depend on every
future template staying on html/template.

Also fixed (blocking, unrelated to this issue, pre-existing on main):
- handlers/auth.go: stray `})` after ResetPassword's closing brace,
  a syntax error blocking the whole module from building.
- go.mod: testcontainers-go v0.17.1 doesn't exist on the module
  proxy; pinned to v0.17.0.

Test plan:
- go build ./services/...
- go test ./services/... -v (existing suite + new
  email_sanitization_test.go covering tag stripping, control-character
  stripping, whitespace trimming, and html/template's own escaping)

Closes parkerwinner#194
@parkerwinner

Copy link
Copy Markdown
Owner

fix conflict

@drips-wave

drips-wave Bot commented Jul 30, 2026

Copy link
Copy Markdown

@Emmanuelchukwunonso Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No Input Sanitization in Email Service

2 participants