refactor(mailing): one HTML shell and one encoder for every module

[marcelo-maciel]

Takes you up on the non-blocking nit from your approval of #1351: EmailBodies (Identity) and BillingEmailBodies.Wrap (Notifications) as two independent shells with two escapers, the hand-rolled one being the weaker.

Read the six files below, not the diff. This branch is cut from #1351, whose files this refactors, and a fork PR cannot be based on another PR's branch — so until #1351 merges the diff here also shows its 17 files. Mine are:

• src/BuildingBlocks/Mailing/HtmlEmail.cs (new)
• src/Modules/Identity/Modules.Identity/Services/EmailBodies.cs (deleted)
• src/Modules/Identity/Modules.Identity/Services/UserPasswordService.cs
• src/Modules/Identity/Modules.Identity/Events/UserRegisteredEmailHandler.cs
• src/Modules/Notifications/Modules.Notifications/IntegrationEventHandlers/BillingEmailBodies.cs
• src/Tests/Framework.Tests/Mailing/HtmlEmailTests.cs (new)

Rebasing onto main after #1351 lands leaves exactly those six, with no content change.

⚠️ Touches protected src/BuildingBlocks (Golden Rule #4, requesting sign-off)

One file, and it is an addition: Mailing/HtmlEmail.cs. Nothing existing under BuildingBlocks is modified — no signature, no behaviour, no registration. It holds the document shell (doctype, charset, viewport, card) and Encode.

What changes

• Encode is WebUtility.HtmlEncode everywhere. The four-Replace chain covered only &, < and >. That is safe in element content and not in an attribute, which is exactly the drift you predicted.
• EmailBodies is deleted rather than left as a pass-through. Its two callers use HtmlEmail directly. Both modules already referenced the Mailing building block, so there is no new project reference.
• BillingEmailBodies keeps its own copy and its Text() twin, and loses only Wrap and Escape. The automated-message footer stays with the billing copy, since it is the one part genuinely specific to those e-mails.

Behaviour change, stated rather than buried

Billing mail now renders in the same document as identity mail, so it gains a doctype, a <meta charset> and the card. Previously it was a bare <div> fragment with no charset declared. Visible to the recipient.

Accented text in billing mail becomes numeric entities (ç becomes ç), because WebUtility.HtmlEncode entitises the Latin-1 supplement while the old escaper left it raw. Renders identically and is more robust to a mis-declared charset. Identity mail already behaved this way. Worth knowing that the encoder is asymmetric here: characters above 255 (CJK, for instance) stay verbatim and rely on the declared utf-8. Pinned by a test so it is documented rather than discovered.

The one contract that needs care

Shell(heading, innerHtml) inserts innerHtml verbatim. That is the point — callers compose markup from literals plus Encoded values — but it means a future contributor "hardening" it by encoding would render every e-mail as visible tags. The doc comment says so and a test pins it.

Tests

15 new cases in HtmlEmailTests: script tags, quotes and apostrophes, already-escaped & encoded exactly once, the Latin-1 vs higher-plane asymmetry, empty and null input, the complete document, verbatim innerHtml, a real anchor, & in a query string, and a quote inside the URL that would otherwise close the href.

Verified on the pushed tree with the NuGet audit on:

• dotnet restore: exit 0. dotnet build -warnaserror: exit 0.
• Full suite: 14 assemblies, 1821 passed / 0 failed / 1 skipped, including Integration at 746 passed / 1 skipped against a real Postgres. That is fix(mailing): send real HTML with a text alternative, not bare text #1351's 1806 plus these 15.
• No existing test needed editing. That was the tripwire: had UserPasswordServiceTests or UserRegisteredEmailHandlerTests required a change, the refactor would have altered identity behaviour and stopped being a refactor.
• Each new test is a real gate, not just green: reverting Encode to the four-Replace chain, making Shell encode innerHtml, dropping the doctype, and leaving the URL raw in the href each turn the matching test red, with the file restored byte-exact (sha256 checked).

Also folded in: the unescaped amount

The earlier version of this description offered to send this separately. It is one line in the same file this PR is already unifying, and the same defect class the nit is about, so it rides here instead.

BillingEmailBodies.InvoiceIssued interpolated amountText — which embeds currency, a data-driven value — into the markup unencoded. It was the only value in that file reaching markup raw; invoiceNumber, tenantName and plan were all escaped.

Not a vulnerability today, and I would rather say that than dress it up. I traced both paths that reach this e-mail: the top-up invoice passes a hardcoded "USD", and the subscription invoice takes the currency from the plan, where CreatePlanCommandValidator enforces NotEmpty().Length(3) behind BillingPermissions.Manage. Three characters, in element content, written by an operator, cannot form a working payload. What makes it worth fixing is that the only thing standing between that value and the markup is a length rule in a different module, which nobody would think to check before relaxing.

It is a no-op for every real currency code: encoding 100.00 USD returns it unchanged, so no delivered e-mail changes.

Stated rather than glossed: this line is not pinned by a test. BillingEmailBodies is internal to Notifications, which has no test project — the module's AssemblyInfo.cs already declares InternalsVisibleTo("Notifications.Tests") for one that was never created. Adding it here would mean a new project plus a src/FSH.Starter.slnx entry, turning a six-file refactor into a solution-structure change for a one-line fix. Happy to add the project if you would rather have the coverage than the smaller diff.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.