From 3672107dd379aaaf47a1e584da1dc7ba5968a6aa Mon Sep 17 00:00:00 2001 From: piersolh <145268163+piersolh@users.noreply.github.com> Date: Sun, 14 Jun 2026 20:03:54 -0400 Subject: [PATCH 1/2] #95 - harden SES email feature: auth, plain-text, logging - Re-enable AuthGuard('jwt') on all bulk-send and template management endpoints - Attach plain-text alternative to every email (multipart/alternative) for deliverability - Add per-recipient send + failure logging for bulk campaigns; surface failed count - Wrap donation-response template variable replacement in try/catch with raw-template fallback - Only render https-hosted signature images (drop local PNG import) so images aren't referenced locally --- apps/backend/src/emails/amazon-ses.wrapper.ts | 4 ++ apps/backend/src/emails/emails.controller.ts | 14 ++--- apps/backend/src/emails/emails.service.ts | 60 ++++++++++--------- apps/backend/src/emails/html-to-text.util.ts | 45 ++++++++++++++ .../src/components/EmailComms/types.ts | 16 +++-- 5 files changed, 98 insertions(+), 41 deletions(-) create mode 100644 apps/backend/src/emails/html-to-text.util.ts diff --git a/apps/backend/src/emails/amazon-ses.wrapper.ts b/apps/backend/src/emails/amazon-ses.wrapper.ts index 1df7cde..751da2f 100644 --- a/apps/backend/src/emails/amazon-ses.wrapper.ts +++ b/apps/backend/src/emails/amazon-ses.wrapper.ts @@ -8,6 +8,7 @@ import { AMAZON_SES_CLIENT } from './amazon-ses-client.factory'; import MailComposer = require('nodemailer/lib/mail-composer'); import * as dotenv from 'dotenv'; import Mail from 'nodemailer/lib/mailer'; +import { htmlToPlainText } from './html-to-text.util'; dotenv.config(); export const AMAZON_SES_WRAPPER = 'AMAZON_SES_WRAPPER'; @@ -41,6 +42,9 @@ export class AmazonSESWrapper { to: recipientEmails, subject: subject, html: emailContent, + // Attach a plain-text alternative so the message is multipart/alternative, + // which renders for non-HTML clients and improves spam-filter scoring. + text: htmlToPlainText(emailContent), }; const messageData = await new MailComposer(mailOptions).compile().build(); diff --git a/apps/backend/src/emails/emails.controller.ts b/apps/backend/src/emails/emails.controller.ts index 67fef08..f5b31dc 100644 --- a/apps/backend/src/emails/emails.controller.ts +++ b/apps/backend/src/emails/emails.controller.ts @@ -21,8 +21,7 @@ export class EmailsController { ) {} @Post('send-email') - // TODO: re-enable auth guard temp disabled for local debugging - // @UseGuards(AuthGuard('jwt')) + @UseGuards(AuthGuard('jwt')) async sendVerificationEmail(@Body() body: CreateEmailDto) { await this.emailService.sendEmail( body.email, @@ -33,21 +32,20 @@ export class EmailsController { } @Get('template') + @UseGuards(AuthGuard('jwt')) async getTemplates() { return this.emailService.getAllTemplates(); } @Get('subscribers') - // TODO: re-enable auth guard temp disabled for local debugging - // @UseGuards(AuthGuard('jwt')) + @UseGuards(AuthGuard('jwt')) async getSubscribers() { const emails = await this.emailService.getSubscribers(); return { emails, count: emails.length }; } @Post('template') - // TODO: re-enable auth guard temp disabled for local debugging - // @UseGuards(AuthGuard('jwt')) + @UseGuards(AuthGuard('jwt')) async saveTemplate(@Body() body: SaveTemplateDto) { const template = await this.emailService.saveTemplate( body.type, @@ -66,8 +64,7 @@ export class EmailsController { } @Post('bulk-send') - // TODO: re-enable auth guard temp disabled for local debugging - // @UseGuards(AuthGuard('jwt')) + @UseGuards(AuthGuard('jwt')) async bulkSend(@Body() body: BulkSendDto) { let recipientEmails: string[] = []; @@ -98,6 +95,7 @@ export class EmailsController { return { message: 'Bulk email campaign sent successfully', sent: result.sent, + failed: result.failed, targetGroup: body.targetGroup, }; } diff --git a/apps/backend/src/emails/emails.service.ts b/apps/backend/src/emails/emails.service.ts index 325c8ab..56fd561 100644 --- a/apps/backend/src/emails/emails.service.ts +++ b/apps/backend/src/emails/emails.service.ts @@ -49,38 +49,33 @@ export class EmailsService { * @param recipientEmails array of recipient email addresses * @param subject the subject of the email * @param bodyHTML the HTML body of the email - * @resolves with the number of emails sent - * @rejects if sending fails + * @resolves with the number of emails sent and failed */ public async sendBulkEmail( recipientEmails: string[], subject: string, bodyHTML: string, - ): Promise<{ sent: number }> { - try { - // Send emails in batches to avoid rate limiting - const batchSize = 50; // AWS SES recommends batch sizes - const batches: string[][] = []; - - for (let i = 0; i < recipientEmails.length; i += batchSize) { - batches.push(recipientEmails.slice(i, i + batchSize)); - } - - let sentCount = 0; - for (const batch of batches) { - await this.amazonSESWrapper.sendEmail(batch, subject, bodyHTML); - sentCount += batch.length; - this.logger.log(`Sent batch of ${batch.length} emails`); + ): Promise<{ sent: number; failed: number }> { + // Send one email per recipient so a single bad address doesn't abort the + // whole campaign, each failure is logged individually, and recipients + // aren't exposed to each other via a shared To header. + let sent = 0; + let failed = 0; + + for (const email of recipientEmails) { + try { + await this.amazonSESWrapper.sendEmail([email], subject, bodyHTML); + sent += 1; + } catch (error) { + failed += 1; + this.logger.error(`Failed to send bulk email to ${email}`, error); } - - this.logger.log( - `Successfully sent ${sentCount} emails with subject: ${subject}`, - ); - return { sent: sentCount }; - } catch (error) { - this.logger.error('Error sending bulk email', error); - throw error; } + + this.logger.log( + `Bulk send complete: ${sent} sent, ${failed} failed (subject: ${subject})`, + ); + return { sent, failed }; } /** @@ -175,9 +170,18 @@ export class EmailsService { return; } - const bodyHTML = template.bodyHtml - .replace(/\{\{donorName\}\}/g, donorName) - .replace(/\{\{amount\}\}/g, amount.toString()); + let bodyHTML = template.bodyHtml; + try { + bodyHTML = template.bodyHtml + .replace(/\{\{donorName\}\}/g, donorName) + .replace(/\{\{amount\}\}/g, amount.toString()); + } catch (error) { + // Fall back to the raw template so a bad value doesn't drop the email. + this.logger.error( + 'Error replacing template variables, sending raw template', + error, + ); + } await this.sendEmail(recipientEmail, template.subject, bodyHTML); diff --git a/apps/backend/src/emails/html-to-text.util.ts b/apps/backend/src/emails/html-to-text.util.ts new file mode 100644 index 0000000..03bb3bd --- /dev/null +++ b/apps/backend/src/emails/html-to-text.util.ts @@ -0,0 +1,45 @@ +/** + * Converts an HTML email body into a readable plain-text approximation. + * + * Used to attach a `text/plain` alternative alongside the HTML part of every + * outgoing email. A multipart/alternative message improves deliverability + * (clients/spam filters penalize HTML-only mail) and degrades gracefully for + * recipients that don't render HTML. + * + * This is intentionally lightweight (no external dependency): it strips + * scripts/styles/comments and tags, decodes a handful of common entities, and + * normalizes whitespace. It does not aim for pixel-perfect rendering. + * + * @param html the HTML body of the email + * @returns a plain-text version of the body + */ +export function htmlToPlainText(html: string): string { + if (!html) { + return ''; + } + + return ( + html + // Drop content of non-visible elements entirely. + .replace(//gi, '') + .replace(//gi, '') + // Remove HTML comments, including FCC_META / FCC_BODY_* markers. + .replace(//g, '') + // Turn block-level boundaries into line breaks before stripping tags. + .replace(/<\/(p|div|h[1-6]|li|tr|table)>/gi, '\n') + .replace(//gi, '\n') + // Strip all remaining tags. + .replace(/<[^>]+>/g, '') + // Decode common HTML entities. + .replace(/ /gi, ' ') + .replace(/&/gi, '&') + .replace(/</gi, '<') + .replace(/>/gi, '>') + .replace(/"/gi, '"') + .replace(/'/gi, "'") + // Collapse runs of whitespace and blank lines. + .replace(/[ \t]+/g, ' ') + .replace(/\n\s*\n\s*\n+/g, '\n\n') + .replace(/^\s+|\s+$/g, '') + ); +} diff --git a/apps/frontend/src/components/EmailComms/types.ts b/apps/frontend/src/components/EmailComms/types.ts index b1c7c3c..e8053c1 100644 --- a/apps/frontend/src/components/EmailComms/types.ts +++ b/apps/frontend/src/components/EmailComms/types.ts @@ -1,4 +1,3 @@ -import FCCEmailMallory from './FCCEmailMallory.png'; export type EmailTabId = 'donation' | 'relapsed' | 'mass'; export type TabId = EmailTabId; @@ -96,16 +95,23 @@ export function buildSignatureHTML(sig: Signature): string { : '', ].join(''); - const imageSrc = sig.imageUrl ? sig.imageUrl : FCCEmailMallory; + // Only render the photo when an https-hosted image is provided. Local/bundled + // assets won't load in a recipient's inbox, so the signature must reference a + // remote URL (CDN/S3); otherwise the photo cell is omitted entirely. + const hasImage = /^https:\/\//i.test(sig.imageUrl); return ` - + ${ + hasImage + ? ` + ` + : '' + }
- ${sig.name} - From 505ef790b31dfcd4a38d9669f8ee3bd37c3ed522 Mon Sep 17 00:00:00 2001 From: piersolh <145268163+piersolh@users.noreply.github.com> Date: Sun, 14 Jun 2026 20:18:37 -0400 Subject: [PATCH 2/2] remove excessive comments --- apps/backend/src/emails/emails.service.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/apps/backend/src/emails/emails.service.ts b/apps/backend/src/emails/emails.service.ts index 56fd561..2d02c87 100644 --- a/apps/backend/src/emails/emails.service.ts +++ b/apps/backend/src/emails/emails.service.ts @@ -56,9 +56,6 @@ export class EmailsService { subject: string, bodyHTML: string, ): Promise<{ sent: number; failed: number }> { - // Send one email per recipient so a single bad address doesn't abort the - // whole campaign, each failure is logged individually, and recipients - // aren't exposed to each other via a shared To header. let sent = 0; let failed = 0;