chore: harden the outbound mail pipeline#123
Merged
Merged
Conversation
Caller-supplied content (events htmlBody, contact-form text, …) flowed
through microTemplate → mjml2html with no escaping or sanitization, so
arbitrary HTML/JS rendered in recipients' mail clients. Tighten the
trust boundary and the /api/mails origin/auth check.
- /api/mails: switch to assertReqInternalSecret (internal-origin +
secret in one helper). Accepts the modern x-secret-key header while
keeping the legacy ?key= query-param fallback for graceful migration.
- /api/mails: caller-supplied html runs through sanitize-html with a
conservative allow-list, http/https/mailto hrefs only; text fallback
is HTML-escaped (textToSafeHtml).
- /api/mails/contact: user-typed text reaches htmlMsg only via
textToSafeHtml (escape + \n→<br>); the contact-form schema admits
text only.
- sendMailI18n: reject non-http(s) params.link before deriving the
{host}/{origin}/{link} substitutions used as button hrefs.
- New api/src/mails/escape.ts holds both helpers as thin wrappers over
sanitize-html, so the entity-escape and sanitize policies live in one
library.
- New docs/architecture/emails.md captures the send-pipeline trust
model and invariants; AGENTS.md references it alongside the existing
email-trust-and-site-isolation doc.
- IGNORE_ASSERT_REQ_INTERNAL=1 added to the api dev script and the
in-process test harness so existing tests through nginx still
exercise the route; production has no env-var bypass.
- test-env DELETE also wipes sd-rate-limiter-contact, matching the
existing sd-rate-limiter-auth cleanup.
- mails.api.spec.ts: 4 new cases — plain-text escape, html
sanitization, javascript: href stripping, contact-form escape.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hardens the
/api/mails*trust boundary so a caller cannot inject arbitrary HTML/JS or non-web URLs into outbound emails, and tightens the auth onPOST /api/mails.POST /api/mailsnow goes throughassertReqInternalSecret(shared helper) instead of a hand-rolledkey=query check plus a softreqIsInternalwarning. Missing/bad secret now returns 401, and the check is blocking rather than logged.
api/src/mails/escape.tsexposes two policies built onsanitize-html:textToSafeHtml— HTML-escapes caller-supplied plain text and turns newlines into<br>, used for thetext-only path of/api/mailsand for the/api/mails/contactbody.sanitizeMailHtml— conservative allow-list (basic formatting tags,a[href]restricted tohttp/https/mailto), applied to caller-suppliedhtmlon/api/mails.sendMailI18nrefuses to send whenparams.linkis nothttp(s):— defends the templated-link path againstjavascript:/data:URLs sneaking in via callers.
tests/features/mails.api.spec.tscover: text escape, html sanitization (stripping<script>, event handlers, attacker<img>),javascript:href stripping, and the contact-form escape. The existing "no secret" test now asserts 401.test-envDELETE /also clears thesd-rate-limiter-contactcollection so the contact tests don't interact with each other.docs/architecture/emails.mddescribing the pipeline (two endpoints, sanitization boundary, MJML substitution,sendMailI18n) and referencesit from
AGENTS.mdas required reading for changes underapi/src/mails/.npm run devand the in-process test server setIGNORE_ASSERT_REQ_INTERNAL=1so local/dev callers aren't blocked by the newinternal-secret assertion.
sanitize-html+@types/sanitize-htmldeps.