Skip to content

fix(mailing): send real HTML with a text alternative, not bare text - #1380

Open
marcelo-maciel wants to merge 4 commits into
fullstackhero:mainfrom
marcelo-maciel:fix/mailing-html-bodies
Open

marcelo-maciel wants to merge 4 commits into
fullstackhero:mainfrom
marcelo-maciel:fix/mailing-html-bodies

Conversation

@marcelo-maciel

Copy link
Copy Markdown
Contributor

Reopened from #1351. That PR was closed automatically on 2026-09-14, when the head fork
was deleted. The branch and the commits are unchanged — the head commit is still
bada7ceeea0a11097ab9deab93374e92d5317650. The earlier review history stays on #1351.


Problem

Every mail provider in the kit puts MailRequest.Body in the HTML slot:

// SmtpMailService
var builder = new BodyBuilder { HtmlBody = request.Body };

// SendGridMailService
MailHelper.CreateSingleEmail(from, to, request.Subject, request.Body, request.Body);
//                                                      ^ plainText   ^ html — the same string

But two callers pass plain text into it:

  • UserPasswordService.ForgotPasswordAsync$"Please reset your password using the following link: {resetPasswordUri}"
  • UserRegisteredEmailHandler$"Hi {@event.FirstName}, thanks for registering."

A bare URL inside a text/html part is not auto-linked by most clients (auto-linking is text/plain behaviour), so the password-reset link arrives as dead text and the user cannot complete the flow. I hit this on a real deployment: the reset mail landed with the URL unclickable.

Two more consequences of the same root cause:

  • The welcome mail interpolates FirstName — user-supplied — straight into markup. A name containing < breaks the message; it is an HTML injection into the rendered mail.
  • SendGrid received Body as both parts, so the confirmation and billing templates were shipped as the text/plain alternative too: a text-only client rendered raw markup.

The confirmation mail (BuildConfirmationEmailHtml) and the billing bodies were already correct HTML with escaping — the defect is the inconsistency, not the templates.

Solution

MailRequest gains an optional TextBody (appended last, so existing positional calls keep compiling):

  • SmtpMailService sets both HtmlBody and TextBody → MailKit emits multipart/alternative.
  • SendGridMailService maps TextBodyplainTextContent and BodyhtmlContent, instead of sending the HTML as both.
  • New EmailBodies helper in Identity (LinkActionHtml, NoticeHtml) renders the action link as a real <a href> and HTML-encodes every interpolated value.
  • Reset, confirmation, welcome and the four billing mails now all carry a text/plain twin, so nothing goes out HTML-only.

Note for reviewers: inside HTML the query separator is &amp;, so the reset URL in the HTML part reads ...?token=…&amp;email=…&amp;tenant=…. That is correct per the HTML spec — the browser hands & to the server. The verbatim URL lives in TextBody, which is where the existing link-shape test now asserts.

Changes to src/BuildingBlocks (Golden Rule #4, requesting sign-off)

  • Mailing/MailRequest.cs — new optional TextBody property + XML docs stating that Body is HTML.
  • Mailing/Services/SmtpMailService.cs — one line: TextBody on the BodyBuilder.
  • Mailing/Services/SendGridMailService.csplainTextContent now comes from TextBody.

No behaviour change for a caller that does not set TextBody, except on SendGrid, where the text part becomes absent instead of being a copy of the markup.

Tests

  • UserPasswordServiceTests — the reset link is a real anchor; &amp; in the HTML part; a text alternative exists, carries the verbatim URL and no markup. The pre-existing link-shape test (single slash, tenant, %2B encoding) now asserts on TextBody.
  • UserRegisteredEmailHandlerTests (new) — a first name of <script>alert(1)</script> comes out encoded; the text alternative is present; nothing is sent when the event carries no e-mail.
  • SendGridMailServiceTests — the two bodies land in their own MIME parts (text/html / text/plain).
  • MailRequestTestsTextBody round-trips and defaults to null.

Verified locally on the pushed tree, with the NuGet audit on rather than disabled:

  • dotnet restore src/FSH.Starter.slnx: exit 0, zero NU1903.
  • dotnet build -warnaserror: exit 0.
  • Full suite: 14 assemblies, 1806 passed / 0 failed / 1 skipped, including Integration at 746 passed / 1 skipped against a real Postgres (Testcontainers).

That closes the gap left in the earlier description, which said the integration suite would not run here and leaned on CI for it. The fault was local and is cleared; the suite ran end to end this time. It remains true that no integration test exercises the mail path itself — the harness does not run enqueued mail jobs — so the mail assertions are the unit tests listed above.

Rebased on main, and the SSH.NET pin

This branch was CONFLICTING. main has since added the System.Security.Cryptography.Xml 10.0.10 pin that this PR was carrying, which was the only conflict. Resolved by keeping main's version, so this PR no longer touches that pin at all — one less shared-config edit to review.

The remaining red was a different advisory: NU1903 / GHSA-q939-rpr3-3284 on SSH.NET 2025.1.0, pulled transitively by Testcontainers, which fails restore for the whole solution under TreatWarningsAsErrors — on main too, re-verified today at 3f2959e6. The fix belongs to #1333, still open. Rather than leave an approved PR red on someone else's advisory, the pin is carried here byte-identical to #1333's version of the file, comment included (same blob), so both stay mergeable in either order and this copy can be dropped once #1333 lands.

Every provider puts MailRequest.Body in the HTML slot — MailKit's
BodyBuilder.HtmlBody, SendGrid's htmlContent — but the password-reset and
welcome mails passed plain text. A bare URL inside an HTML part is not
auto-linked by most clients, so the reset link arrived as dead text and the
user had no way to complete the flow. The welcome mail additionally
interpolated the user-supplied first name straight into that HTML.

MailRequest gains an optional TextBody carrying the text/plain alternative.
SmtpMailService emits both parts as multipart/alternative; SendGridMailService
stops passing Body as plainTextContent, which had been shipping raw markup to
text-only clients. Identity builds its bodies through EmailBodies, which
HTML-encodes every interpolated value, and billing bodies gained their plain
twin so no message goes out HTML-only.

Verified: build -warnaserror 0/0; unit suites green (Identity 317,
Framework 122, Billing 123, and the rest).
The test hosts pull 10.0.8 transitively, which carries HIGH-severity
advisories (GHSA-23rf-6693-g89p, GHSA-8q5v-6pqq-x66h, GHSA-cvvh-rhrc-wg4q,
GHSA-g8r8-53c2-pm3f) and trips NuGetAudit under TreatWarningsAsErrors,
breaking the build of every test project. 10.0.10 is the patched
servicing release. Mirrors the existing Microsoft.OpenApi transitive pin.
…1333 is open

`NU1903` / `GHSA-q939-rpr3-3284` on `SSH.NET` 2025.1.0, pulled transitively by
Testcontainers, fails `restore` for the whole solution under
`TreatWarningsAsErrors` — on `main` too. It is not introduced here and the fix
belongs to fullstackhero#1333, which is still open.

Carried byte-identical to fullstackhero#1333's version of the file, comment included, so both
stay mergeable in either order and this copy can simply be dropped once fullstackhero#1333
lands.
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.

1 participant