diff --git a/.changeset/20374-plain-text-faces-unescaped.md b/.changeset/20374-plain-text-faces-unescaped.md new file mode 100644 index 00000000000..fe962590fe2 --- /dev/null +++ b/.changeset/20374-plain-text-faces-unescaped.md @@ -0,0 +1,12 @@ +--- +"@objectstack/plugin-email": patch +--- + +A template's plain-text faces — the subject and `body_text` — now render their `{{x}}` values verbatim instead of HTML-escaping them, so a link in the text part keeps its literal `&` (#20374). + +Clause-②: no + +- **What was wrong.** `EmailService` rendered every face of a `sys_email_template` row through the HTML escaper. In the text/plain part of the built-in verification, password-reset, invitation and magic-link mails the link read `…?token=…&callbackURL=%2F`: a plain-text client, or a user copying the link, got a parameter named `amp;callbackURL`, and the post-verification redirect fell back to `/`. The same escaping put `&` / `'` into subjects built from names such as `R&D` or `O'Brien`. +- **What changes.** Escaping now follows the face. `body_html` is markup and is rendered exactly as before: `{{x}}` HTML-escaped, `{{{x}}}` not. The subject and `body_text` are plain text: every hole renders its value as-is, and triple braces mean the same as double there. The switch is in the renderer, so it covers every row that reaches `sendTemplate` / `renderTemplate` — the built-in auth templates, declared `emailTemplates` and rows authored in Studio — with no template edit. +- **Who sees it.** The persisted `sys_email.body_text` and `subject`, the delivered text part and Subject header, and `IEmailService.renderTemplate()`'s `text` / `subject` (which the messaging inbox channel stores as a notification's body and title). A row with no `body_text` is unchanged: its text part was already derived from the HTML with the entities decoded. +- **Unchanged.** The exported `renderTemplate()` helper is still the HTML renderer. Nothing an author writes needs to change. diff --git a/content/docs/automation/email-templates.mdx b/content/docs/automation/email-templates.mdx index 5f0b3c6dc68..47760d55b13 100644 --- a/content/docs/automation/email-templates.mdx +++ b/content/docs/automation/email-templates.mdx @@ -64,12 +64,16 @@ rather than silently sending nothing. ## Placeholders Subject and both bodies are rendered by a deliberately tiny mustache-style -renderer: +renderer. Escaping follows the face: only `bodyHtml` is markup, so only +`bodyHtml` HTML-escapes its values. `subject` and `bodyText` are plain text and +render every value verbatim — an `&` in a link stays `&`, so the plain-text part +never needs triple braces. - `{{path.to.value}}` — dotted-path lookup against the send's `data` object, - **HTML-escaped**. -- `{{{path.to.value}}}` — the same value, *not* escaped. Use it only for - pre-rendered HTML fragments such as a URL you are dropping into `href`. + **HTML-escaped in `bodyHtml`**, verbatim in `subject` and `bodyText`. +- `{{{path.to.value}}}` — the same value, *not* escaped even in `bodyHtml`. Use + it only for pre-rendered HTML fragments such as a URL you are dropping into + `href`. - `{{ order.total | currency:EUR }}` / `{{ ts | datetime }}` — an optional formatter from the shared formula whitelist, so money and dates render the same way they do in-app. `datetime` honours the reference timezone the caller diff --git a/packages/plugins/plugin-email/src/email-service.ts b/packages/plugins/plugin-email/src/email-service.ts index 8b4993dd582..e5a222f83fb 100644 --- a/packages/plugins/plugin-email/src/email-service.ts +++ b/packages/plugins/plugin-email/src/email-service.ts @@ -15,7 +15,7 @@ import type { IQueueService, QueueBackoffPolicy, } from '@objectstack/spec/contracts'; -import { renderTemplate, requireVars, htmlToText } from './template-engine.js'; +import { renderTemplate, renderPlainTextTemplate, requireVars, htmlToText } from './template-engine.js'; import { SYS_EMAIL_ATTACHMENT_LIMIT_BYTES, encodeAttachmentsForRow, @@ -1375,10 +1375,18 @@ export class EmailService implements IEmailService { ...(locale ? { locale } : {}), ...(input.timezone ? { timeZone: input.timezone } : {}), }; - const subject = renderTemplate(row.subject, data, renderOpts); + // Each face is rendered in ITS OWN encoding. Only `body_html` is markup, + // so only it HTML-escapes its `{{x}}` holes. The subject (a mail header, + // an inbox title) and `body_text` (the plain-text part, an inbox body) are + // plain text: escaping there put `&` into every link carrying a + // query string — a plain-text reader, or anyone copying the link, got a + // parameter named `amp;callbackURL` and lost the post-verification + // redirect. The derived fallback was already right: `htmlToText` decodes + // the entities the HTML render introduced. + const subject = renderPlainTextTemplate(row.subject, data, renderOpts); const html = renderTemplate(row.body_html, data, renderOpts); const text = row.body_text - ? renderTemplate(row.body_text, data, renderOpts) + ? renderPlainTextTemplate(row.body_text, data, renderOpts) : htmlToText(html); return { row, rendered: { subject, html, text } }; diff --git a/packages/plugins/plugin-email/src/plain-text-faces.test.ts b/packages/plugins/plugin-email/src/plain-text-faces.test.ts new file mode 100644 index 00000000000..f73a800bfd5 --- /dev/null +++ b/packages/plugins/plugin-email/src/plain-text-faces.test.ts @@ -0,0 +1,207 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The plain-text faces of a template render WITHOUT HTML escaping (#20374). + * + * A template has three faces and only one of them is markup: `body_html`. + * The subject (a mail header, an inbox title) and `body_text` (the text/plain + * part, an inbox body) are plain text. Rendering them through the HTML + * escaper put `&` into every link carrying a query string, so a + * plain-text client — or a user copying the text link — received + * `…?token=…&callbackURL=%2F`: the token still parsed, the redirect + * parameter arrived as `amp;callbackURL`, and a verified invitee landed on + * `/` instead of the accept page. + * + * The switch lives in the engine (`renderPlainTextTemplate`), not in the + * templates, so these pins run the REAL `EmailService` over the REAL seeded + * built-in rows AND over an authored row nobody hand-braced — a per-template + * `{{{…}}}` patch would pass the first half and fail the second. + * + * ⚠️ Every expectation is an independent literal: nothing is derived from the + * template constants or from the renderer under test, so an edit that + * reintroduces escaping cannot quietly agree with itself. + */ + +import { describe, it, expect } from 'vitest'; +import { EmailService, type EmailTemplateRow, type TemplateLoader } from './email-service.js'; +import { BUILTIN_AUTH_TEMPLATES } from './templates/auth-templates.js'; +import type { + IEmailTransport, + NormalizedEmailMessage, + TransportSendResult, +} from '@objectstack/spec/contracts'; + +const LOCALES = ['en-US', 'zh-CN', 'ja-JP', 'es-ES'] as const; + +/** A link whose second query parameter is the one the defect dropped. */ +const LINK = 'https://acme.test/api/v1/auth/verify-email?token=TOK123&callbackURL=%2Faccept-invitation%2Finv_1'; +/** The same link as HTML markup spells it — correct inside `body_html` only. */ +const LINK_AS_MARKUP = 'https://acme.test/api/v1/auth/verify-email?token=TOK123&callbackURL=%2Faccept-invitation%2Finv_1'; + +class CaptureTransport implements IEmailTransport { + public sent: NormalizedEmailMessage[] = []; + async send(message: NormalizedEmailMessage): Promise { + this.sent.push(message); + return { messageId: `msg-${this.sent.length}` }; + } +} + +/** Exactly what `EmailServicePlugin` seeds, matched on `(name, locale)` with no fallback of its own. */ +function seededRow(name: string, locale: string): EmailTemplateRow | null { + const hit = BUILTIN_AUTH_TEMPLATES.find((t) => t.name === name && t.locale === locale); + if (!hit) return null; + return { + name: hit.name, + locale: hit.locale ?? 'en-US', + subject: hit.subject, + body_html: hit.bodyHtml ?? '', + body_text: hit.bodyText ?? null, + active: hit.active !== false, + variables_json: JSON.stringify(hit.variables ?? []), + }; +} + +function harness(extraRows: EmailTemplateRow[] = []) { + const transport = new CaptureTransport(); + const rows: Array> = []; + const loader: TemplateLoader = { + async load(name, locale) { + const authored = extraRows.find((r) => r.name === name && (locale === undefined || r.locale === locale)); + if (authored) return authored; + return locale === undefined ? null : seededRow(name, locale); + }, + }; + const svc = new EmailService({ + transport, + defaultFrom: { address: 'no-reply@acme.test' }, + templateLoader: loader, + persistence: { + async insert(row) { rows.push(row); return { id: row.id }; }, + async update() { /* noop */ }, + }, + }); + return { svc, transport, rows }; +} + +const USER = { name: 'Alice', email: 'alice@acme.test', id: 'usr_1' }; + +/** The three link-carrying sends the card names, plus the magic link that shares the shape. */ +const LINK_SENDS = [ + { template: 'auth.verify_email', data: { user: USER, verificationUrl: LINK, appName: 'Acme' } }, + { template: 'auth.password_reset', data: { user: USER, resetUrl: LINK, expiresInMinutes: 30, appName: 'Acme' } }, + { + template: 'auth.invitation', + data: { + inviter: { name: 'Bob', email: 'bob@acme.test' }, + organization: { name: 'Acme' }, + role: 'member', + acceptUrl: LINK, + appName: 'Acme', + }, + }, + { template: 'auth.magic_link', data: { magicLinkUrl: LINK, expiresInMinutes: 10, appName: 'Acme' } }, +] as const; + +describe('auth mail: the plain-text part carries the link verbatim', () => { + for (const { template, data } of LINK_SENDS) { + for (const locale of LOCALES) { + it(`${template} [${locale}]: sys_email.body_text and the text part keep a literal &`, async () => { + const { svc, transport, rows } = harness(); + + await svc.sendTemplate({ template, to: 'alice@acme.test', locale, data: data as Record }); + + expect(rows).toHaveLength(1); + const bodyText = String(rows[0].body_text); + // The persisted audit row and the delivered text/plain part are the + // same string, and both carry the link a user can actually follow. + expect(bodyText).toContain(LINK); + expect(bodyText).not.toContain('&'); + expect(transport.sent[0].text).toBe(bodyText); + }); + + it(`${template} [${locale}]: the HTML part is unchanged — href verbatim, visible copy still escaped markup`, async () => { + const { svc, transport } = harness(); + + await svc.sendTemplate({ template, to: 'alice@acme.test', locale, data: data as Record }); + + const html = String(transport.sent[0].html); + // `{{{url}}}` in the href was never escaped … + expect(html).toContain(`href="${LINK}"`); + // … and the copy-paste `{{url}}` span is still HTML-escaped, which is + // correct in markup: a mail client decodes it back to `&` on display. + expect(html).toContain(LINK_AS_MARKUP); + }); + } + } +}); + +describe('control: a value carrying markup characters, per face', () => { + it('a built-in template: < and & are escaped in body_html and literal in body_text and the subject', async () => { + const { svc, transport, rows } = harness(); + + await svc.sendTemplate({ + template: 'auth.invitation', + to: 'alice@acme.test', + locale: 'en-US', + data: { + inviter: { name: "O'Brien", email: 'ob@acme.test' }, + organization: { name: 'R&D ' }, + role: 'member', + acceptUrl: LINK, + appName: 'Acme', + }, + }); + + const sent = transport.sent[0]; + expect(sent.html).toContain('R&D <Lab>'); + expect(sent.html).toContain('O'Brien'); + expect(sent.text).toBe( + "O'Brien (ob@acme.test) invited you to join R&D on Acme.\n\n" + `Accept: ${LINK}`, + ); + expect(sent.subject).toBe("O'Brien invited you to R&D "); + expect(rows[0].subject).toBe("O'Brien invited you to R&D "); + }); + + const AUTHORED: EmailTemplateRow = { + name: 'crm.deal_won', + locale: 'en-US', + subject: 'Won: {{deal.name}}', + body_html: '

Deal {{deal.name}} closed. Open {{deal.url}}

', + body_text: 'Deal {{deal.name}} closed. Open: {{deal.url}}', + active: true, + }; + const DEAL = { deal: { name: 'Q3 & more', url: 'https://acme.test/deal?id=7&tab=notes' } }; + + it('an authored template (no per-template braces): escaped in body_html, literal in body_text and the subject', async () => { + const { svc, transport, rows } = harness([AUTHORED]); + + await svc.sendTemplate({ template: 'crm.deal_won', to: 'alice@acme.test', data: DEAL }); + + const sent = transport.sent[0]; + expect(sent.html).toBe( + '

Deal Q3 <Renewal> & more closed. ' + + 'Open https://acme.test/deal?id=7&tab=notes

', + ); + expect(sent.text).toBe('Deal Q3 & more closed. Open: https://acme.test/deal?id=7&tab=notes'); + expect(sent.subject).toBe('Won: Q3 & more'); + expect(rows[0].body_text).toBe(sent.text); + }); + + it('renderTemplate (the render-only face the inbox channel reads) answers the same text and subject', async () => { + const { svc } = harness([AUTHORED]); + + const out = await svc.renderTemplate({ template: 'crm.deal_won', data: DEAL }); + + expect(out.text).toBe('Deal Q3 & more closed. Open: https://acme.test/deal?id=7&tab=notes'); + expect(out.subject).toBe('Won: Q3 & more'); + expect(out.html).toContain('Q3 <Renewal> & more'); + }); + + it('a row with NO body_text still derives its text part with the entities decoded', async () => { + const { svc, transport } = harness([{ ...AUTHORED, body_text: null }]); + + await svc.sendTemplate({ template: 'crm.deal_won', to: 'alice@acme.test', data: DEAL }); + + expect(transport.sent[0].text).toBe('Deal Q3 & more closed. Open https://acme.test/deal?id=7&tab=notes'); + }); +}); diff --git a/packages/plugins/plugin-email/src/template-engine.test.ts b/packages/plugins/plugin-email/src/template-engine.test.ts index 377a198f2a9..30b1f458e33 100644 --- a/packages/plugins/plugin-email/src/template-engine.test.ts +++ b/packages/plugins/plugin-email/src/template-engine.test.ts @@ -1,9 +1,44 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { renderTemplate, requireVars, htmlToText } from './template-engine.js'; +import { renderTemplate, renderPlainTextTemplate, requireVars, htmlToText } from './template-engine.js'; describe('template-engine', () => { + // The plain-text face (`subject`, `body_text`): no HTML escaping at all. + // Expectations are literals; the last case runs the same inputs through the + // HTML face, which still escapes — so the pair pins the SWITCH, not a + // global change to escaping. + describe('renderPlainTextTemplate', () => { + it('renders a double-braced link with a literal &', () => { + expect(renderPlainTextTemplate('Open: {{url}}', { url: 'https://x.test/v?token=t&callbackURL=%2F' })) + .toBe('Open: https://x.test/v?token=t&callbackURL=%2F'); + }); + + it('renders every character the HTML face escapes verbatim', () => { + expect(renderPlainTextTemplate('{{s}}', { s: `&<>"'` })).toBe(`&<>"'`); + }); + + it('renders triple braces verbatim too', () => { + expect(renderPlainTextTemplate('{{{s}}}', { s: 'a&b' })).toBe('a&b'); + }); + + it('renders formatted holes verbatim', () => { + expect(renderPlainTextTemplate('{{ s | upper }}', { s: 'a&b' })).toBe('A&B'); + }); + + it('keeps the shared lookup rules: missing → empty, non-hole left verbatim, scalars stringified', () => { + expect(renderPlainTextTemplate('a={{a}} b={{b}} n={{n}}', { a: 'A&', n: 3 })).toBe('a=A& b= n=3'); + expect(renderPlainTextTemplate('{{ not a hole }}', {})).toBe('{{ not a hole }}'); + expect(renderPlainTextTemplate('', { a: 1 })).toBe(''); + }); + + it('control: the HTML face still escapes the same inputs', () => { + expect(renderTemplate('Open: {{url}}', { url: 'https://x.test/v?token=t&callbackURL=%2F' })) + .toBe('Open: https://x.test/v?token=t&callbackURL=%2F'); + expect(renderTemplate('{{ s | upper }}', { s: 'a&b' })).toBe('A&B'); + }); + }); + describe('renderTemplate', () => { it('substitutes dotted paths', () => { expect(renderTemplate('Hi {{user.name}}', { user: { name: 'Alice' } })) diff --git a/packages/plugins/plugin-email/src/template-engine.ts b/packages/plugins/plugin-email/src/template-engine.ts index 40b7cd1f93a..86d358762d5 100644 --- a/packages/plugins/plugin-email/src/template-engine.ts +++ b/packages/plugins/plugin-email/src/template-engine.ts @@ -4,10 +4,21 @@ * Minimal mustache-style template renderer. * * Supports `{{path.to.value}}` placeholders resolved against a plain - * JS object via dotted-path lookup. Values are HTML-escaped by - * default; use `{{{path}}}` (triple braces) to opt out of escaping - * (e.g. when injecting pre-rendered HTML fragments such as URLs in - * ``). + * JS object via dotted-path lookup. + * + * Escaping follows the FACE being rendered, so there are two entry points: + * + * - {@link renderTemplate} renders an HTML face (`body_html`). Values are + * HTML-escaped by default; use `{{{path}}}` (triple braces) to opt out + * of escaping (e.g. when injecting pre-rendered HTML fragments such as + * URLs in ``). + * - {@link renderPlainTextTemplate} renders a plain-text face (`subject`, + * `body_text`). Every hole, double- or triple-braced, renders its value + * verbatim: HTML entities in plain text are not markup, they are + * corruption — a `&` in a link turned into `&` hands a plain-text + * reader a query parameter named `amp;callbackURL`. The switch lives + * here, once, so every template that reaches the renderer — built-in, + * declared or authored — gets it without a per-template brace change. * * A hole may carry an optional formatter from the shared formula * whitelist — `{{ order.total | currency:EUR }}`, `{{ ts | datetime }}` — @@ -84,8 +95,14 @@ function escapeHtml(s: string): string { .replace(/'/g, '''); } +/** How a double-braced hole's value is encoded for the face being rendered. */ +type HoleEncoder = (value: string) => string; + +const verbatim: HoleEncoder = (value) => value; + /** - * Render `template` with values from `data`. Missing placeholders + * Render `template` as an HTML face with values from `data`: `{{x}}` + * holes are HTML-escaped, `{{{x}}}` holes are not. Missing placeholders * render as empty strings (no throw); call `requireVars()` first if * you need strict validation. */ @@ -93,6 +110,30 @@ export function renderTemplate( template: string, data: Record, opts: RenderOptions = {}, +): string { + return render(template, data, opts, escapeHtml); +} + +/** + * Render `template` as a PLAIN-TEXT face (`subject`, `body_text`) with + * values from `data`: every hole renders its value verbatim, with no HTML + * escaping — plain text is not markup, so an entity there is a corrupted + * character, never a safe one. Same lookup, formatter and missing-value + * rules as {@link renderTemplate}. + */ +export function renderPlainTextTemplate( + template: string, + data: Record, + opts: RenderOptions = {}, +): string { + return render(template, data, opts, verbatim); +} + +function render( + template: string, + data: Record, + opts: RenderOptions, + encode: HoleEncoder, ): string { if (!template) return ''; return template.replace( @@ -117,7 +158,7 @@ export function renderTemplate( if (raw == null) return ''; str = typeof raw === 'string' ? raw : String(raw); } - return isUnescaped ? str : escapeHtml(str); + return isUnescaped ? str : encode(str); }, ); } diff --git a/packages/plugins/plugin-email/src/template-locale-resolution.test.ts b/packages/plugins/plugin-email/src/template-locale-resolution.test.ts index 2727451e5c6..d0e5dec8814 100644 --- a/packages/plugins/plugin-email/src/template-locale-resolution.test.ts +++ b/packages/plugins/plugin-email/src/template-locale-resolution.test.ts @@ -300,7 +300,11 @@ function expected(locale: string) { }; } -/** Mirror of the template engine's escaper — the rendered output is escaped. */ +/** + * Mirror of the template engine's HTML escaper — applied to the `html` face + * only. The subject is a plain-text face and renders unescaped, so subject + * expectations below compare the formatter output as-is. + */ const esc = (s: string) => s .replace(/&/g, '&').replace(//g, '>') .replace(/"/g, '"').replace(/'/g, '''); @@ -316,7 +320,7 @@ describe('sendTemplate — format filters follow the RESOLVED ROW\'s locale (#78 await svc.sendTemplate({ template: 'receipt', to: 'a@x.test', timezone: 'UTC', data: FMT_DATA }); const en = expected('en-US'); - expect(transport.sent[0].subject).toBe(`[en-US] ${esc(en.when)}`); + expect(transport.sent[0].subject).toBe(`[en-US] ${en.when}`); expect(transport.sent[0].html).toContain(esc(en.amount)); }); @@ -336,8 +340,8 @@ describe('sendTemplate — format filters follow the RESOLVED ROW\'s locale (#78 await svc.sendTemplate({ template: 'receipt', to: 'a@x.test', timezone: 'UTC', data: FMT_DATA }); const zh = expected('zh-CN'); - expect(transport.sent[0].subject).toBe(`[zh-CN] ${esc(zh.when)}`); - expect(transport.sent[0].subject).not.toContain(esc(expected('en-US').when)); + expect(transport.sent[0].subject).toBe(`[zh-CN] ${zh.when}`); + expect(transport.sent[0].subject).not.toContain(expected('en-US').when); }); // Pin (b): the row is the authority only when the caller named NOBODY. @@ -349,7 +353,7 @@ describe('sendTemplate — format filters follow the RESOLVED ROW\'s locale (#78 }); const de = expected('de-DE'); - expect(transport.sent[0].subject).toBe(`[de-DE] ${esc(de.when)}`); + expect(transport.sent[0].subject).toBe(`[de-DE] ${de.when}`); expect(transport.sent[0].html).toContain(esc(de.amount)); }); @@ -365,7 +369,7 @@ describe('sendTemplate — format filters follow the RESOLVED ROW\'s locale (#78 }); const de = expected('de-DE'); - expect(transport.sent[0].subject).toBe(`[en-US] ${esc(de.when)}`); + expect(transport.sent[0].subject).toBe(`[en-US] ${de.when}`); expect(transport.sent[0].html).toContain(esc(de.amount)); }); @@ -379,7 +383,7 @@ describe('sendTemplate — format filters follow the RESOLVED ROW\'s locale (#78 template: 'receipt', to: 'a@x.test', locale: ' de-DE ', timezone: 'UTC', data: FMT_DATA, }); - expect(transport.sent[0].subject).toBe(`[de-DE] ${esc(expected('de-DE').when)}`); + expect(transport.sent[0].subject).toBe(`[de-DE] ${expected('de-DE').when}`); }); });