From e8052adf1d2f1717652ac5d6b8cd8bcc01787189 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Sun, 3 May 2026 22:06:06 +0200 Subject: [PATCH] fix(email): render conditionals, localise password placeholders, fix caller/template variable drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bundle of email-renderer and email-caller fixes triggered by a reproducer on picpeak.nothaft.cloud (gallery_created mail showing literal `{{#if welcome_message}}` markers and `Passwort: (set at creation)`). The audit that followed surfaced six more user-visible defects in the same surface; all are fixed here so customer-facing mail renders cleanly. Renderer (`backend/src/services/emailProcessor.js`) - `safeTemplateReplace` now resolves `{{#if VAR}}…{{/if}}` blocks before flat `{{var}}` substitution. The shipped templates have used Handlebars-style conditionals since migration 026; the renderer ignored them, so the markers leaked verbatim into every mail with an empty welcome_message. Lifted to module scope and exported so the conditional contract is unit-testable. Single-pass, non-nested (commented). - Added `passwordSetAtCreationI18n` next to the existing two i18n password sentinels so `(set at creation)` (sent by the publish- from-draft flow when only the bcrypt hash remains) is localised to "Das bei der Erstellung der Galerie gesetzte Passwort" / equivalent in EN/DE/NL/PT/RU instead of the raw English string. - Added an opt-in `{ escapeHtml: true }` mode to `safeTemplateReplace` so admin-supplied free text (`event_name`, `host_name`, …) is HTML-escaped on substitution into the HTML body. Allowlist of passthrough keys (`welcome_message` already-HTML, server-generated URLs `gallery_link` / `client_link`). Subject and text body keep the legacy unescaped behaviour. `formatWelcomeMessage` now escapes before nl2br so the welcome_message allowlist is safe. - New `htmlToText()` strips `Hello'; + const out = htmlToText(html); + expect(out).toBe('Hello'); + expect(out).not.toMatch(/margin/); + expect(out).not.toMatch(/color/); + }); + + it('strips Hi'; + expect(htmlToText(html)).toBe('Hi'); + }); + + it('converts
tags to newlines', () => { + expect(htmlToText('a
b
c
d')).toBe('a\nb\nc\nd'); + }); + + it('keeps a paragraph break between adjacent

tags', () => { + expect(htmlToText('

one

two

')).toBe('one\n\ntwo'); + }); + + it('decodes the common HTML entities', () => { + expect(htmlToText('Tom & Jerry <3 "hi"')) + .toBe('Tom & Jerry <3 "hi"'); + }); + + it('handles a fully-wrapped email body without leaking CSS rules', () => { + // Shape mirrors what wrapEmailHtml() produces: a + + +

Galerie erfolgreich erstellt

+

Liebe(r) Natalie,

+ +`; + const out = htmlToText(wrapped); + expect(out).toContain('Galerie erfolgreich erstellt'); + expect(out).toContain('Liebe(r) Natalie'); + expect(out).not.toMatch(/margin/); + expect(out).not.toMatch(/font-family/); + expect(out).not.toMatch(/background-color/); + expect(out).not.toMatch(/\.button/); + }); +}); diff --git a/backend/__tests__/services/emailProcessor.safeTemplateReplace.test.js b/backend/__tests__/services/emailProcessor.safeTemplateReplace.test.js new file mode 100644 index 00000000..7e3ef16c --- /dev/null +++ b/backend/__tests__/services/emailProcessor.safeTemplateReplace.test.js @@ -0,0 +1,138 @@ +/** + * Unit tests for emailProcessor.safeTemplateReplace. + * + * Covers the two regressions that hit picpeak.nothaft.cloud on the + * 3.32.x betas: + * - {{#if VAR}}…{{/if}} blocks rendered as literal text in the email + * because the renderer only handled {{var}} substitution and the + * shipped templates use Handlebars-style conditionals. + * - {{var}} substitution inside a kept conditional block. + * + * The publish-from-draft password localisation lives inside the wider + * processTemplate() pipeline (DB-backed), so it isn't covered here — the + * sentinel string '(set at creation)' is asserted only at the i18n-map + * level by integration in adminEvents.js. + */ + +jest.mock('../../src/database/db', () => ({ db: jest.fn() })); + +const { safeTemplateReplace } = require('../../src/services/emailProcessor'); + +describe('safeTemplateReplace', () => { + describe('flat variable substitution', () => { + it('replaces {{var}} with the variable value', () => { + expect(safeTemplateReplace('Hello {{name}}!', { name: 'Paul' })) + .toBe('Hello Paul!'); + }); + + it('leaves unknown variables untouched', () => { + expect(safeTemplateReplace('Hello {{name}}!', {})) + .toBe('Hello {{name}}!'); + }); + + it('coerces non-string values to string', () => { + expect(safeTemplateReplace('Count: {{n}}', { n: 42 })) + .toBe('Count: 42'); + }); + + it('handles empty templates and missing variables map', () => { + expect(safeTemplateReplace('', { x: 1 })).toBe(''); + expect(safeTemplateReplace('plain text', undefined)).toBe('plain text'); + expect(safeTemplateReplace(null, {})).toBe(null); + }); + }); + + describe('{{#if VAR}}…{{/if}} blocks', () => { + it('strips the block when the variable is missing', () => { + const tpl = 'before {{#if welcome}}HELLO {{welcome}}{{/if}} after'; + expect(safeTemplateReplace(tpl, {})).toBe('before after'); + }); + + it('strips the block when the variable is an empty string', () => { + const tpl = 'before {{#if welcome}}HELLO {{welcome}}{{/if}} after'; + expect(safeTemplateReplace(tpl, { welcome: '' })).toBe('before after'); + }); + + it('strips the block when the variable is null', () => { + const tpl = '{{#if x}}kept{{/if}}'; + expect(safeTemplateReplace(tpl, { x: null })).toBe(''); + }); + + it('keeps the block and substitutes inside it when truthy', () => { + const tpl = 'before {{#if welcome}}HELLO {{welcome}}{{/if}} after'; + expect(safeTemplateReplace(tpl, { welcome: 'world' })) + .toBe('before HELLO world after'); + }); + + it('handles multi-line conditional blocks', () => { + const tpl = [ + 'Liebe(r) {{host_name}},', + '', + '{{#if welcome_message}}', + 'Persönliche Nachricht:', + '{{welcome_message}}', + '{{/if}}', + 'Galerie-Details:', + ].join('\n'); + + const withMsg = safeTemplateReplace(tpl, { + host_name: 'Natalie', + welcome_message: 'Schön, dass ihr da seid!', + }); + expect(withMsg).toContain('Persönliche Nachricht:'); + expect(withMsg).toContain('Schön, dass ihr da seid!'); + expect(withMsg).not.toContain('{{#if'); + expect(withMsg).not.toContain('{{/if'); + + const withoutMsg = safeTemplateReplace(tpl, { + host_name: 'Natalie', + welcome_message: '', + }); + expect(withoutMsg).not.toContain('Persönliche Nachricht'); + expect(withoutMsg).not.toContain('{{#if'); + expect(withoutMsg).not.toContain('{{/if'); + expect(withoutMsg).toContain('Liebe(r) Natalie,'); + expect(withoutMsg).toContain('Galerie-Details:'); + }); + + it('handles multiple sibling conditionals independently', () => { + const tpl = '{{#if a}}A{{/if}}|{{#if b}}B{{/if}}|{{#if c}}C{{/if}}'; + expect(safeTemplateReplace(tpl, { a: 1, c: 'yes' })).toBe('A||C'); + }); + + it('treats numeric 0 as falsy', () => { + expect(safeTemplateReplace('{{#if n}}has-n{{/if}}', { n: 0 })).toBe(''); + }); + }); + + describe('HTML escaping (escapeHtml: true)', () => { + it('does not escape by default', () => { + const tpl = 'Welcome to {{event_name}}'; + expect(safeTemplateReplace(tpl, { event_name: 'Test ' }, { escapeHtml: true })) + .toBe('Welcome to Test <script>alert(1)</script>'); + }); + + it('escapes both the < > and & characters and quotes', () => { + expect(safeTemplateReplace('{{x}}', { x: 'A & B\'s' }, { escapeHtml: true })) + .toBe('<a href="evil">A & B's</a>'); + }); + + it('passes welcome_message through unescaped (already HTML from formatWelcomeMessage)', () => { + const tpl = '

{{welcome_message}}

'; + expect(safeTemplateReplace(tpl, { welcome_message: 'Hi
there' }, { escapeHtml: true })) + .toBe('

Hi
there

'); + }); + + it('passes server-generated URLs through unescaped', () => { + const tpl = 'link'; + expect(safeTemplateReplace(tpl, { gallery_link: 'https://example.com/g/abc?token=xyz&u=1' }, { escapeHtml: true })) + .toBe('link'); + }); + }); +}); diff --git a/backend/__tests__/utils/formatters.test.js b/backend/__tests__/utils/formatters.test.js new file mode 100644 index 00000000..c5f8f9ac --- /dev/null +++ b/backend/__tests__/utils/formatters.test.js @@ -0,0 +1,66 @@ +/** + * Unit tests for formatters.js — focused on the HTML-escape behaviour added + * so admin-supplied welcome messages can't inject markup into customer mail. + */ + +const { escapeHtml, nl2br, formatWelcomeMessage } = require('../../src/utils/formatters'); + +describe('escapeHtml', () => { + it('escapes the five HTML metacharacters', () => { + expect(escapeHtml('& < > " \'')).toBe('& < > " ''); + }); + + it('returns empty string for null/undefined', () => { + expect(escapeHtml(null)).toBe(''); + expect(escapeHtml(undefined)).toBe(''); + }); + + it('coerces non-string values', () => { + expect(escapeHtml(42)).toBe('42'); + }); + + it('escapes & before introducing new entities', () => { + expect(escapeHtml('<&>')).toBe('<&>'); + }); +}); + +describe('nl2br', () => { + it('joins non-empty lines with
', () => { + expect(nl2br('a\nb\nc')).toBe('a
b
c'); + }); + + it('normalises CRLF and CR', () => { + expect(nl2br('a\r\nb\rc')).toBe('a
b
c'); + }); + + it('drops empty lines', () => { + expect(nl2br('a\n\n\nb')).toBe('a
b'); + }); + + it('returns empty for empty input', () => { + expect(nl2br('')).toBe(''); + expect(nl2br(null)).toBe(''); + }); +}); + +describe('formatWelcomeMessage', () => { + it('returns empty string for empty input', () => { + expect(formatWelcomeMessage('')).toBe(''); + expect(formatWelcomeMessage(' ')).toBe(''); + }); + + it('escapes HTML metacharacters before nl2br', () => { + expect(formatWelcomeMessage('Hello world')) + .toBe('Hello <b>world</b>'); + }); + + it('renders newlines as
while keeping content escaped', () => { + expect(formatWelcomeMessage('line 1\n\nline 3')) + .toBe('line 1
<script>x</script>
line 3'); + }); + + it('escapes ampersands and quotes that would otherwise break HTML', () => { + expect(formatWelcomeMessage('Tom & Jerry\'s "show"')) + .toBe('Tom & Jerry's "show"'); + }); +}); diff --git a/backend/src/services/archiveService.js b/backend/src/services/archiveService.js index fae51523..217127bd 100644 --- a/backend/src/services/archiveService.js +++ b/backend/src/services/archiveService.js @@ -5,7 +5,7 @@ const path = require('path'); const os = require('os'); const crypto = require('crypto'); const { db } = require('../database/db'); -const { queueEmail } = require('./emailProcessor'); +const { queueEmail, getSupportEmail } = require('./emailProcessor'); const logger = require('../utils/logger'); const feedbackService = require('./feedbackService'); const { getStorage } = require('./storage'); @@ -146,10 +146,20 @@ async function archiveEvent(event) { // Queue completion email — admin_email is nullable on events (migration 073); // skip queueing rather than violating email_queue.recipient_email NOT NULL. + // + // The shipped EN/DE templates (legacy 028) and NL/PT/RU (core 075) reference + // {{host_name}}, {{photo_count}}, {{archive_date}} and {{support_email}}; + // without these the recipient saw literal {{...}} placeholders. if (event.admin_email) { + const supportEmail = await getSupportEmail(); await queueEmail(event.id, event.admin_email, 'archive_complete', { + host_name: event.customer_name || event.host_name || 'Admin', event_name: event.event_name, + event_date: event.event_date, + photo_count: photoEntries.length, archive_size: (totalBytes / 1024 / 1024).toFixed(2) + ' MB', + archive_date: new Date(), + support_email: supportEmail }); } else { logger.info(`Skipping archive_complete email for event ${event.slug}: no admin_email set`); diff --git a/backend/src/services/emailProcessor.js b/backend/src/services/emailProcessor.js index 0eaea247..ea4537d0 100644 --- a/backend/src/services/emailProcessor.js +++ b/backend/src/services/emailProcessor.js @@ -1,5 +1,4 @@ const nodemailer = require('nodemailer'); -const Handlebars = require('handlebars'); const { db } = require('../database/db'); const logger = require('../utils/logger'); const { getFrontendBaseUrl } = require('../utils/frontendUrl'); @@ -64,6 +63,39 @@ async function initializeTransporter(forceReinit = false) { } } +// Read the support contact email used in customer-facing notifications +// (gallery_expired, archive_complete, …). Looks up `branding_support_email` +// from app_settings (JSON-encoded), falling back to the SMTP from-address +// so templates that reference {{support_email}} never render the literal +// placeholder. Returns '' when neither is configured — templates should +// degrade by hiding the line via a {{#if support_email}} block. +async function getSupportEmail() { + try { + const row = await db('app_settings') + .where('setting_key', 'branding_support_email') + .first(); + if (row && row.setting_value) { + try { + const parsed = JSON.parse(row.setting_value); + if (typeof parsed === 'string' && parsed.trim()) return parsed.trim(); + } catch (e) { + if (typeof row.setting_value === 'string' && row.setting_value.trim()) { + return row.setting_value.trim(); + } + } + } + } catch (err) { + logger.debug('getSupportEmail: app_settings lookup failed', { error: err.message }); + } + try { + const config = await db('email_configs').first(); + if (config && config.from_email) return config.from_email; + } catch (err) { + logger.debug('getSupportEmail: email_configs lookup failed', { error: err.message }); + } + return ''; +} + // Get the appropriate language for a recipient async function getRecipientLanguage(email, eventId = null) { // First priority: Check event language setting if eventId is provided @@ -302,6 +334,83 @@ async function wrapEmailHtml(htmlBody, subject, language = 'en') { `; } +// Convert an HTML body to a plain-text fallback. The naive +// `html.replace(/<[^>]*>/g, '')` strips angle-bracket tags but leaves the +// *contents* of