diff --git a/backend/src/mailer.ts b/backend/src/mailer.ts index d40bd29..bfa1237 100755 --- a/backend/src/mailer.ts +++ b/backend/src/mailer.ts @@ -85,10 +85,21 @@ export function isAllowedRecipient(to: string, allowlist: string | undefined): b }); } -export async function sendMail(to: string, subject: string, html: string): Promise { +/** + * What a send attempt actually did. + * + * `sendMail` returns early in two cases that used to be indistinguishable from + * success — no SMTP credentials, and a recipient outside MAIL_ALLOWLIST — which + * meant a caller could report "emailed" for a message nobody would ever + * receive. QA restricts delivery by design, so that was not a hypothetical: it + * is the normal case there. See #260. + */ +export type MailOutcome = 'sent' | 'skipped-unconfigured' | 'skipped-blocked'; + +export async function sendMail(to: string, subject: string, html: string): Promise { if (!process.env.SMTP_USER || !process.env.SMTP_PASSWORD) { console.warn(`SMTP not configured — skipping email to ${to}: "${subject}"`); - return; + return 'skipped-unconfigured'; } // Guarded here rather than at the four call sites, so every sender is covered @@ -101,7 +112,7 @@ export async function sendMail(to: string, subject: string, html: string): Promi // is the part that was missing when QA was simply muted. if (!isAllowedRecipient(to, process.env.MAIL_ALLOWLIST)) { console.warn(`[mail-blocked] ${to} is not on MAIL_ALLOWLIST — skipping "${subject}"`); - return; + return 'skipped-blocked'; } await transporter.sendMail({ @@ -110,4 +121,5 @@ export async function sendMail(to: string, subject: string, html: string): Promi subject, html }); + return 'sent'; } diff --git a/backend/tests/unit/mailOutcome.test.ts b/backend/tests/unit/mailOutcome.test.ts new file mode 100644 index 0000000..920a882 --- /dev/null +++ b/backend/tests/unit/mailOutcome.test.ts @@ -0,0 +1,54 @@ +import { sendMail } from '../../src/mailer'; + +/** + * What sendMail says it did. + * + * It returns early in two cases that are indistinguishable from success at the + * call site — SMTP unconfigured, and the recipient not on MAIL_ALLOWLIST — and + * #260 needs to tell them apart so the admin is not told a link was emailed + * when it was not. + * + * Only the two skip paths are covered. A real send needs an SMTP server, which + * a unit test has no business starting; the integration test in Task 4 covers + * the route's behaviour instead. + */ +describe('what sendMail reports', () => { + const original = { ...process.env }; + + afterEach(() => { + process.env = { ...original }; + }); + + it('says so when SMTP is not configured', async () => { + delete process.env.SMTP_USER; + delete process.env.SMTP_PASSWORD; + + await expect(sendMail('someone@example.com', 'subject', '

body

')).resolves.toBe( + 'skipped-unconfigured' + ); + }); + + // The case that matters most: QA restricts delivery, and a blocked address + // previously returned exactly as though it had sent. + it('says so when the recipient is not on the allowlist', async () => { + process.env.SMTP_USER = 'user'; + process.env.SMTP_PASSWORD = 'password'; + process.env.MAIL_ALLOWLIST = 'allowed@example.com'; + + await expect(sendMail('someone-else@example.com', 'subject', '

body

')).resolves.toBe( + 'skipped-blocked' + ); + }); + + it('does not report blocked for an address that is on the allowlist', async () => { + process.env.SMTP_USER = 'user'; + process.env.SMTP_PASSWORD = 'password'; + process.env.MAIL_ALLOWLIST = 'allowed@example.com'; + + // Not asserting 'sent': that would need a live SMTP server. Asserting only + // that the allowlist did not refuse it, which is this test's subject. + await expect( + sendMail('allowed@example.com', 'subject', '

body

').catch(() => 'threw') + ).resolves.not.toBe('skipped-blocked'); + }); +});