Skip to content

Commit 8e9e630

Browse files
hotlongclaude
andauthored
fix(plugin-email): validate the default sender at configuration time and record pre-delivery rejections in sys_email (#14371)
* fix(plugin-email): validate the default sender at configuration time and record pre-delivery rejections in sys_email An unsendable OS_EMAIL_FROM was first judged inside normalizeMessage on the first send, which for a fresh deployment is the first user's sign-up -- and better-auth's runInBackgroundOrAwait logs and swallows that throw, so the UI reported a verification email nobody had sent. EmailServicePlugin.init() now refuses a declared defaultFrom no message could be sent from; the mail settings channel refuses the value, keeps the previous sender and says so at error. EmailService also writes a sys_email row at status='failed' for a message rejected before delivery -- the one window that previously produced no record. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(plugin-email): route the new fake engine's update() through assertEngineUpdateDispatch check:engine-double-contract flagged the double in sender-address-validation.test.ts: a fake looser than ObjectQL.update is how a dead route once shipped with a green suite. Routed through the predicate (the shape email-plugin.template-runtime-write.test.ts already pins) and recorded the new pinned coverage in the ledger with --write. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 159e05e commit 8e9e630

6 files changed

Lines changed: 664 additions & 4 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
"@objectstack/plugin-email": patch
3+
---
4+
5+
fix(plugin-email): judge the default sender where it is configured, and record a send rejected before delivery (#14318)
6+
7+
Measured on a local rig with `OS_EMAIL_FROM="ObjectOS Local <noreply@localhost>"`,
8+
`OS_EMAIL_PROVIDER=log` and `OS_AUTH_REQUIRE_EMAIL_VERIFICATION=true`: the server
9+
booted clean, the first sign-up answered `200`, the UI said a verification email
10+
had been sent, and nothing had been. `EMAIL_REGEX` requires a dotted domain and
11+
correctly refuses `noreply@localhost` — but it refused it inside
12+
`normalizeMessage`, on the **first send**, which for a fresh deployment is the
13+
first user's sign-up. better-auth runs `sendVerificationEmail` through
14+
`runInBackgroundOrAwait`, which logs `Failed to run background task` and returns
15+
normally, so the account was created and the user was parked on a verify screen
16+
whose Resend repeated the whole sequence. `sys_email` held no row for any of it:
17+
the throw happened *before* the row insert, and every other failure shape in the
18+
service is a row at `status:'failed'`.
19+
20+
Two changes, one per half.
21+
22+
**The address is judged where it is configured.** `EmailServicePlugin.init()`
23+
now refuses a declared `defaultFrom` that no message could ever be sent from, so
24+
a deployment that names an unsendable sender fails its boot instead of failing
25+
every send — the same trade `resolveTransport` already makes for an SMTP
26+
provider with no host, and the error names the consequence and the fix
27+
(`OS_EMAIL_FROM` / `config.email.defaultFrom`). An **absent** sender is still
28+
accepted: callers that always pass `input.from` are a complete configuration,
29+
and `normalizeMessage` already refuses a send that has neither.
30+
31+
The `mail` settings channel takes that method's opposite, stated trade — a save
32+
must not kill a running server — so an unsendable saved From address is
33+
**refused, the previous sender kept**, and the consequence stated at `error`.
34+
`error` and not `warn` because nothing looks broken afterwards: the save
35+
succeeds and the settings page shows the address the operator typed.
36+
37+
**A send rejected before delivery now leaves a `sys_email` row.** The
38+
`normalizeMessage` window was the one path on which a send produced no record at
39+
all. It now writes `status:'failed'` with the reason, prefixed
40+
`rejected before delivery:` so the column distinguishes a message that never
41+
reached a transport from one an SMTP host refused. The envelope columns carry
42+
what the caller actually passed (never re-canonicalised — canonicalisation is
43+
what threw), `(none)` where the input named nothing, since `from_address` /
44+
`to_addresses` / `subject` are required. The row is safe by construction: both
45+
re-delivery paths — the `afterInsert` outbox drain hook and the boot outbox
46+
sweep — gate on `status === 'queued'`, so a rejection record can never be
47+
mistaken for an outbox entry. Persisting it is best-effort and never replaces
48+
the caller's error.
49+
50+
Unchanged: `formatAddress`, `EMAIL_REGEX` and `normalizeMessage` keep their
51+
exact verdicts (the new `isSendableAddress` predicate shares the one regex, so a
52+
boot cannot pass a check the send path then fails), `send()` still throws on
53+
validation failure rather than answering `failed`, and the auth layer's
54+
propagation is as it was — `sendVerificationEmail` already rejects on both a
55+
throw and a returned `status:'failed'`, which is now pinned by a test.
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #14318 — the `sendVerificationEmail` callback must hand a send failure back
5+
* to whoever called it.
6+
*
7+
* Why this is pinned rather than assumed. The reported symptom was a sign-up
8+
* that answered 200 while the verification mail was never sent, and the
9+
* obvious reading is "the auth layer swallows the failure". It does not: both
10+
* failure shapes `IEmailService` can produce — a THROW (template resolution,
11+
* or `normalizeMessage` refusing an unsendable `from`) and a returned
12+
* `status:'failed'` (transport error) — leave this callback as a rejection.
13+
*
14+
* What swallows it is one layer further out, and it is not ours:
15+
* better-auth's sign-up route invokes the callback through
16+
* `runInBackgroundOrAwait`, which awaits the promise inside a `try/catch` that
17+
* logs `Failed to run background task` and returns normally
18+
* (`better-auth/dist/context/create-context.mjs`). The `/send-verification-email`
19+
* route does NOT — it awaits `sendVerificationEmailFn` directly and rethrows —
20+
* so the resend path is honest today and must stay that way.
21+
*
22+
* Hence these assertions: they are the contract the cloud verify-email screen
23+
* reads through the resend endpoint, and the reason the *first* half of #14318
24+
* is a configuration-time refusal in plugin-email rather than another layer of
25+
* error plumbing here.
26+
*/
27+
28+
import { describe, it, expect, vi } from 'vitest';
29+
import { AuthManager } from './auth-manager';
30+
31+
vi.mock('better-auth', () => ({
32+
betterAuth: vi.fn(() => ({ handler: vi.fn(), api: {} })),
33+
}));
34+
vi.mock('better-auth/plugins/organization', () => ({
35+
organization: vi.fn((opts: any) => ({ id: 'organization', _opts: opts })),
36+
}));
37+
vi.mock('better-auth/plugins/magic-link', () => ({
38+
magicLink: vi.fn((opts: any) => ({ id: 'magic-link', _opts: opts })),
39+
}));
40+
vi.mock('better-auth/plugins/two-factor', () => ({
41+
twoFactor: vi.fn((opts: any) => ({ id: 'two-factor', _opts: opts })),
42+
}));
43+
vi.mock('better-auth/plugins/custom-session', () => ({
44+
customSession: vi.fn((fn: any) => ({ id: 'custom-session', _fn: fn })),
45+
}));
46+
vi.mock('better-auth/plugins/haveibeenpwned', () => ({
47+
haveIBeenPwned: vi.fn((opts: any) => ({ id: 'have-i-been-pwned', _opts: opts })),
48+
}));
49+
50+
const USER = { id: 'u1', email: 'ada@example.com', name: 'Ada' };
51+
52+
/** Boot an AuthManager whose `sendTemplate` behaves as `sendTemplate` says. */
53+
async function boot(sendTemplate: (input: any) => Promise<any>) {
54+
const { betterAuth } = await import('better-auth');
55+
let capturedConfig: any;
56+
(betterAuth as any).mockImplementation((config: any) => {
57+
capturedConfig = config;
58+
return { handler: vi.fn(), api: {} };
59+
});
60+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
61+
const manager = new AuthManager({
62+
secret: 'test-secret-at-least-32-chars-long',
63+
baseUrl: 'http://localhost:3000',
64+
emailAndPassword: { enabled: true },
65+
emailVerification: { sendOnSignUp: true },
66+
} as never);
67+
manager.setEmailService({
68+
async send() { return { id: 'e', status: 'sent' }; },
69+
sendTemplate,
70+
} as never);
71+
await manager.getAuthInstance();
72+
warnSpy.mockRestore();
73+
return capturedConfig;
74+
}
75+
76+
const drive = (config: any) => config.emailVerification.sendVerificationEmail({
77+
user: USER,
78+
url: 'http://x/verify',
79+
token: 't',
80+
});
81+
82+
describe('sendVerificationEmail — failure reaches the caller', () => {
83+
it('rejects when the send THROWS (the unsendable-from shape)', async () => {
84+
// Exactly what `EmailService.send` does with
85+
// OS_EMAIL_FROM="ObjectOS Local <noreply@localhost>": `normalizeMessage`
86+
// refuses the sender and the throw travels out of `sendTemplate`.
87+
const config = await boot(async () => {
88+
throw new Error('Invalid email address: noreply@localhost');
89+
});
90+
await expect(drive(config)).rejects.toThrow(/Invalid email address: noreply@localhost/);
91+
});
92+
93+
it('rejects when the send RETURNS status:failed, naming the recipient and the cause', async () => {
94+
const config = await boot(async () => ({ id: 'e1', status: 'failed', error: 'smtp 421' }));
95+
// Both facts matter to whoever reads the resend response: which address
96+
// was not reached, and why.
97+
await expect(drive(config)).rejects.toThrow(/ada@example\.com/);
98+
await expect(drive(config)).rejects.toThrow(/smtp 421/);
99+
});
100+
101+
it('resolves on a successful send — the control', async () => {
102+
const sent: any[] = [];
103+
const config = await boot(async (input: any) => {
104+
sent.push(input);
105+
return { id: 'e1', status: 'sent' };
106+
});
107+
await expect(drive(config)).resolves.toBeUndefined();
108+
expect(sent).toHaveLength(1);
109+
expect(sent[0]).toMatchObject({ template: 'auth.verify_email' });
110+
});
111+
});

packages/plugins/plugin-email/src/email-plugin.ts

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
EmailService,
1515
LogTransport,
1616
EMAIL_SEND_QUEUE,
17+
isSendableAddress,
1718
type EmailPersistence,
1819
type EmailQueueDelivery,
1920
type TemplateLoader,
@@ -245,6 +246,48 @@ export function resolveDurableQueue(getService: (name: string) => unknown): IQue
245246
return queue as IQueueService;
246247
}
247248

249+
/** How the settings page and the deployment channel name the same address. */
250+
const DEFAULT_FROM_FIX =
251+
'Fix: set a sender with a dotted domain — OS_EMAIL_FROM="Name <no-reply@example.com>" or '
252+
+ 'config.email.defaultFrom (Settings → Mail → From address on the settings channel). '
253+
+ 'Bare hostnames such as "noreply@localhost" are not deliverable addresses.';
254+
255+
/**
256+
* Refuse a **declared** default sender no message could ever be sent from
257+
* (#14318).
258+
*
259+
* This is the constructor / CLI channel, so it THROWS, exactly as an
260+
* unbuildable transport does in {@link EmailServicePlugin.resolveTransport}: a
261+
* deployment that names a sender is declaring one, and a boot that cannot
262+
* honour the declaration must fail loudly rather than start half-configured.
263+
* The alternative was measured — `OS_EMAIL_FROM="ObjectOS Local
264+
* <noreply@localhost>"` booted clean, and the address was first judged inside
265+
* `normalizeMessage` on the first real send, which for a fresh deployment is
266+
* the first user's sign-up: better-auth ran that send through
267+
* `runInBackgroundOrAwait`, logged the throw and swallowed it, so the account
268+
* was created, the UI reported "verification email sent", and nothing had
269+
* been.
270+
*
271+
* `undefined` is NOT rejected: a service with no default sender is a
272+
* complete, working configuration for callers that always pass `input.from`,
273+
* and `normalizeMessage` already refuses a send that has neither.
274+
*
275+
* The settings channel takes the opposite trade — see
276+
* {@link EmailServicePlugin.applyMailSettings}: one bad save must not stop a
277+
* running server, so there the value is refused, the previous sender kept,
278+
* and the consequence stated at `error`.
279+
*/
280+
function assertSendableDefaultFrom(from: EmailAddress | undefined): void {
281+
if (from === undefined || isSendableAddress(from)) return;
282+
const shown = typeof from === 'string' ? from : (from?.address ?? '');
283+
throw new Error(
284+
`EmailServicePlugin: the configured default sender '${String(shown) || '(empty)'}' is not a valid email `
285+
+ 'address, so EVERY message this deployment sends would be rejected before it reached the transport — '
286+
+ 'including the sign-up verification mail, which is discarded in a background task and would leave users '
287+
+ `told their mail was sent. ${DEFAULT_FROM_FIX}`,
288+
);
289+
}
290+
248291
/**
249292
* EmailServicePlugin — registers the `email` service.
250293
*
@@ -363,6 +406,10 @@ export class EmailServicePlugin implements Plugin {
363406
}
364407

365408
async init(ctx: PluginContext): Promise<void> {
409+
// The declared sender has to be sendable, and this is where that is
410+
// knowable — before a single message depends on it (#14318).
411+
assertSendableDefaultFrom(this.options.defaultFrom);
412+
366413
// Register sys_email + sys_email_template via manifest service.
367414
ctx.getService<{ register(m: any): void }>('manifest').register({
368415
id: 'com.objectstack.service.email',
@@ -1404,7 +1451,23 @@ export class EmailServicePlugin implements Plugin {
14041451

14051452
const fromEmail = typeof values.from_email === 'string' ? values.from_email : undefined;
14061453
const fromName = typeof values.from_name === 'string' ? values.from_name : undefined;
1407-
if (fromEmail) this.service.setDefaultFrom({ address: fromEmail, name: fromName });
1454+
let appliedFrom: string | undefined;
1455+
if (fromEmail && !isSendableAddress({ address: fromEmail, name: fromName })) {
1456+
// #14318 — the settings twin of the constructor assertion, taking this
1457+
// method's stated trade: refuse the value, KEEP the previous sender,
1458+
// never throw. `error` and not `warn` because nothing looks broken
1459+
// afterwards — the save succeeds, the page shows the address the
1460+
// operator typed — while every send made with it would be rejected
1461+
// before it reached the transport.
1462+
ctx.logger.error(
1463+
`EmailServicePlugin: the saved From address '${fromEmail}' is not a valid email address — it is `
1464+
+ 'NOT applied (the previous sender is kept) because every message sent from it would be rejected '
1465+
+ `before reaching the transport, silently on the sign-up path. ${DEFAULT_FROM_FIX}`,
1466+
);
1467+
} else if (fromEmail) {
1468+
this.service.setDefaultFrom({ address: fromEmail, name: fromName });
1469+
appliedFrom = fromEmail;
1470+
}
14081471

14091472
const provider = String(values.provider ?? 'smtp');
14101473

@@ -1449,7 +1512,7 @@ export class EmailServicePlugin implements Plugin {
14491512

14501513
if (provider === 'log') {
14511514
ctx.logger.info(
1452-
`EmailServicePlugin: mail settings applied (provider=log, from=${fromEmail ?? '∅'}); `
1515+
`EmailServicePlugin: mail settings applied (provider=log, from=${appliedFrom ?? '∅'}); `
14531516
+ 'transport unchanged — messages are logged and recorded in sys_email, never delivered.',
14541517
);
14551518
return;

0 commit comments

Comments
 (0)