feat(mail): have sendMail say what it actually did (#260)
It returned Promise<void> and returned early in two cases that were indistinguishable from success at the call site: no SMTP credentials, and a recipient outside MAIL_ALLOWLIST. A caller could therefore report that it had emailed somebody a message nobody would ever receive, and in QA — which restricts delivery deliberately, as its entire safety property — that is the normal case rather than an edge one. It now returns a MailOutcome saying which of the three happened. No existing caller changes: there are seven and every one ignores the result, so this is additive. Re-deriving the answer at a second site would have duplicated isAllowedRecipient and the SMTP check, which is exactly the drift the guard-in-one-place comment above them exists to prevent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+15
-3
@@ -85,10 +85,21 @@ export function isAllowedRecipient(to: string, allowlist: string | undefined): b
|
||||
});
|
||||
}
|
||||
|
||||
export async function sendMail(to: string, subject: string, html: string): Promise<void> {
|
||||
/**
|
||||
* 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<MailOutcome> {
|
||||
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';
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user