fix(plugin-email): validate the default sender at configuration time and record pre-delivery rejections in sys_email

.changeset/email-sender-validated-at-configuration-time.md

@@ -0,0 +1,55 @@
+---
+"@objectstack/plugin-email": patch
+---
+
+fix(plugin-email): judge the default sender where it is configured, and record a send rejected before delivery (#14318)
+
+Measured on a local rig with `OS_EMAIL_FROM="ObjectOS Local <noreply@localhost>"`,
+`OS_EMAIL_PROVIDER=log` and `OS_AUTH_REQUIRE_EMAIL_VERIFICATION=true`: the server
+booted clean, the first sign-up answered `200`, the UI said a verification email
+had been sent, and nothing had been. `EMAIL_REGEX` requires a dotted domain and
+correctly refuses `noreply@localhost` — but it refused it inside
+`normalizeMessage`, on the **first send**, which for a fresh deployment is the
+first user's sign-up. better-auth runs `sendVerificationEmail` through
+`runInBackgroundOrAwait`, which logs `Failed to run background task` and returns
+normally, so the account was created and the user was parked on a verify screen
+whose Resend repeated the whole sequence. `sys_email` held no row for any of it:
+the throw happened *before* the row insert, and every other failure shape in the
+service is a row at `status:'failed'`.
+
+Two changes, one per half.
+
+**The address is judged where it is configured.** `EmailServicePlugin.init()`
+now refuses a declared `defaultFrom` that no message could ever be sent from, so
+a deployment that names an unsendable sender fails its boot instead of failing
+every send — the same trade `resolveTransport` already makes for an SMTP
+provider with no host, and the error names the consequence and the fix
+(`OS_EMAIL_FROM` / `config.email.defaultFrom`). An **absent** sender is still
+accepted: callers that always pass `input.from` are a complete configuration,
+and `normalizeMessage` already refuses a send that has neither.
+
+The `mail` settings channel takes that method's opposite, stated trade — a save
+must not kill a running server — so an unsendable saved From address is
+**refused, the previous sender kept**, and the consequence stated at `error`.
+`error` and not `warn` because nothing looks broken afterwards: the save
+succeeds and the settings page shows the address the operator typed.
+
+**A send rejected before delivery now leaves a `sys_email` row.** The
+`normalizeMessage` window was the one path on which a send produced no record at
+all. It now writes `status:'failed'` with the reason, prefixed
+`rejected before delivery:` so the column distinguishes a message that never
+reached a transport from one an SMTP host refused. The envelope columns carry
+what the caller actually passed (never re-canonicalised — canonicalisation is
+what threw), `(none)` where the input named nothing, since `from_address` /
+`to_addresses` / `subject` are required. The row is safe by construction: both
+re-delivery paths — the `afterInsert` outbox drain hook and the boot outbox
+sweep — gate on `status === 'queued'`, so a rejection record can never be
+mistaken for an outbox entry. Persisting it is best-effort and never replaces
+the caller's error.
+
+Unchanged: `formatAddress`, `EMAIL_REGEX` and `normalizeMessage` keep their
+exact verdicts (the new `isSendableAddress` predicate shares the one regex, so a
+boot cannot pass a check the send path then fails), `send()` still throws on
+validation failure rather than answering `failed`, and the auth layer's
+propagation is as it was — `sendVerificationEmail` already rejects on both a
+throw and a returned `status:'failed'`, which is now pinned by a test.

packages/plugins/plugin-auth/src/verification-email-failure-propagation.test.ts

@@ -0,0 +1,111 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #14318 — the `sendVerificationEmail` callback must hand a send failure back
+ * to whoever called it.
+ *
+ * Why this is pinned rather than assumed. The reported symptom was a sign-up
+ * that answered 200 while the verification mail was never sent, and the
+ * obvious reading is "the auth layer swallows the failure". It does not: both
+ * failure shapes `IEmailService` can produce — a THROW (template resolution,
+ * or `normalizeMessage` refusing an unsendable `from`) and a returned
+ * `status:'failed'` (transport error) — leave this callback as a rejection.
+ *
+ * What swallows it is one layer further out, and it is not ours:
+ * better-auth's sign-up route invokes the callback through
+ * `runInBackgroundOrAwait`, which awaits the promise inside a `try/catch` that
+ * logs `Failed to run background task` and returns normally
+ * (`better-auth/dist/context/create-context.mjs`). The `/send-verification-email`
+ * route does NOT — it awaits `sendVerificationEmailFn` directly and rethrows —
+ * so the resend path is honest today and must stay that way.
+ *
+ * Hence these assertions: they are the contract the cloud verify-email screen
+ * reads through the resend endpoint, and the reason the *first* half of #14318
+ * is a configuration-time refusal in plugin-email rather than another layer of
+ * error plumbing here.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { AuthManager } from './auth-manager';
+
+vi.mock('better-auth', () => ({
+  betterAuth: vi.fn(() => ({ handler: vi.fn(), api: {} })),
+}));
+vi.mock('better-auth/plugins/organization', () => ({
+  organization: vi.fn((opts: any) => ({ id: 'organization', _opts: opts })),
+}));
+vi.mock('better-auth/plugins/magic-link', () => ({
+  magicLink: vi.fn((opts: any) => ({ id: 'magic-link', _opts: opts })),
+}));
+vi.mock('better-auth/plugins/two-factor', () => ({
+  twoFactor: vi.fn((opts: any) => ({ id: 'two-factor', _opts: opts })),
+}));
+vi.mock('better-auth/plugins/custom-session', () => ({
+  customSession: vi.fn((fn: any) => ({ id: 'custom-session', _fn: fn })),
+}));
+vi.mock('better-auth/plugins/haveibeenpwned', () => ({
+  haveIBeenPwned: vi.fn((opts: any) => ({ id: 'have-i-been-pwned', _opts: opts })),
+}));
+
+const USER = { id: 'u1', email: 'ada@example.com', name: 'Ada' };
+
+/** Boot an AuthManager whose `sendTemplate` behaves as `sendTemplate` says. */
+async function boot(sendTemplate: (input: any) => Promise<any>) {
+  const { betterAuth } = await import('better-auth');
+  let capturedConfig: any;
+  (betterAuth as any).mockImplementation((config: any) => {
+    capturedConfig = config;
+    return { handler: vi.fn(), api: {} };
+  });
+  const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+  const manager = new AuthManager({
+    secret: 'test-secret-at-least-32-chars-long',
+    baseUrl: 'http://localhost:3000',
+    emailAndPassword: { enabled: true },
+    emailVerification: { sendOnSignUp: true },
+  } as never);
+  manager.setEmailService({
+    async send() { return { id: 'e', status: 'sent' }; },
+    sendTemplate,
+  } as never);
+  await manager.getAuthInstance();
+  warnSpy.mockRestore();
+  return capturedConfig;
+}
+
+const drive = (config: any) => config.emailVerification.sendVerificationEmail({
+  user: USER,
+  url: 'http://x/verify',
+  token: 't',
+});
+
+describe('sendVerificationEmail — failure reaches the caller', () => {
+  it('rejects when the send THROWS (the unsendable-from shape)', async () => {
+    // Exactly what `EmailService.send` does with
+    // OS_EMAIL_FROM="ObjectOS Local <noreply@localhost>": `normalizeMessage`
+    // refuses the sender and the throw travels out of `sendTemplate`.
+    const config = await boot(async () => {
+      throw new Error('Invalid email address: noreply@localhost');
+    });
+    await expect(drive(config)).rejects.toThrow(/Invalid email address: noreply@localhost/);
+  });
+
+  it('rejects when the send RETURNS status:failed, naming the recipient and the cause', async () => {
+    const config = await boot(async () => ({ id: 'e1', status: 'failed', error: 'smtp 421' }));
+    // Both facts matter to whoever reads the resend response: which address
+    // was not reached, and why.
+    await expect(drive(config)).rejects.toThrow(/ada@example\.com/);
+    await expect(drive(config)).rejects.toThrow(/smtp 421/);
+  });
+
+  it('resolves on a successful send — the control', async () => {
+    const sent: any[] = [];
+    const config = await boot(async (input: any) => {
+      sent.push(input);
+      return { id: 'e1', status: 'sent' };
+    });
+    await expect(drive(config)).resolves.toBeUndefined();
+    expect(sent).toHaveLength(1);
+    expect(sent[0]).toMatchObject({ template: 'auth.verify_email' });
+  });
+});

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

@@ -14,6 +14,7 @@ import {
   EmailService,
   LogTransport,
   EMAIL_SEND_QUEUE,
+  isSendableAddress,
   type EmailPersistence,
   type EmailQueueDelivery,
   type TemplateLoader,
@@ -245,6 +246,48 @@ export function resolveDurableQueue(getService: (name: string) => unknown): IQue
   return queue as IQueueService;
 }
 
+/** How the settings page and the deployment channel name the same address. */
+const DEFAULT_FROM_FIX =
+  'Fix: set a sender with a dotted domain — OS_EMAIL_FROM="Name <no-reply@example.com>" or '
+  + 'config.email.defaultFrom (Settings → Mail → From address on the settings channel). '
+  + 'Bare hostnames such as "noreply@localhost" are not deliverable addresses.';
+
+/**
+ * Refuse a **declared** default sender no message could ever be sent from
+ * (#14318).
+ *
+ * This is the constructor / CLI channel, so it THROWS, exactly as an
+ * unbuildable transport does in {@link EmailServicePlugin.resolveTransport}: a
+ * deployment that names a sender is declaring one, and a boot that cannot
+ * honour the declaration must fail loudly rather than start half-configured.
+ * The alternative was measured — `OS_EMAIL_FROM="ObjectOS Local
+ * <noreply@localhost>"` booted clean, and the address was first judged inside
+ * `normalizeMessage` on the first real send, which for a fresh deployment is
+ * the first user's sign-up: better-auth ran that send through
+ * `runInBackgroundOrAwait`, logged the throw and swallowed it, so the account
+ * was created, the UI reported "verification email sent", and nothing had
+ * been.
+ *
+ * `undefined` is NOT rejected: a service with no default sender is a
+ * complete, working configuration for callers that always pass `input.from`,
+ * and `normalizeMessage` already refuses a send that has neither.
+ *
+ * The settings channel takes the opposite trade — see
+ * {@link EmailServicePlugin.applyMailSettings}: one bad save must not stop a
+ * running server, so there the value is refused, the previous sender kept,
+ * and the consequence stated at `error`.
+ */
+function assertSendableDefaultFrom(from: EmailAddress | undefined): void {
+  if (from === undefined || isSendableAddress(from)) return;
+  const shown = typeof from === 'string' ? from : (from?.address ?? '');
+  throw new Error(
+    `EmailServicePlugin: the configured default sender '${String(shown) || '(empty)'}' is not a valid email `
+    + 'address, so EVERY message this deployment sends would be rejected before it reached the transport — '
+    + 'including the sign-up verification mail, which is discarded in a background task and would leave users '
+    + `told their mail was sent. ${DEFAULT_FROM_FIX}`,
+  );
+}
+
 /**
  * EmailServicePlugin — registers the `email` service.
  *
@@ -363,6 +406,10 @@ export class EmailServicePlugin implements Plugin {
   }
 
   async init(ctx: PluginContext): Promise<void> {
+    // The declared sender has to be sendable, and this is where that is
+    // knowable — before a single message depends on it (#14318).
+    assertSendableDefaultFrom(this.options.defaultFrom);
+
     // Register sys_email + sys_email_template via manifest service.
     ctx.getService<{ register(m: any): void }>('manifest').register({
       id: 'com.objectstack.service.email',
@@ -1404,7 +1451,23 @@ export class EmailServicePlugin implements Plugin {
 
     const fromEmail = typeof values.from_email === 'string' ? values.from_email : undefined;
     const fromName = typeof values.from_name === 'string' ? values.from_name : undefined;
-    if (fromEmail) this.service.setDefaultFrom({ address: fromEmail, name: fromName });
+    let appliedFrom: string | undefined;
+    if (fromEmail && !isSendableAddress({ address: fromEmail, name: fromName })) {
+      // #14318 — the settings twin of the constructor assertion, taking this
+      // method's stated trade: refuse the value, KEEP the previous sender,
+      // never throw. `error` and not `warn` because nothing looks broken
+      // afterwards — the save succeeds, the page shows the address the
+      // operator typed — while every send made with it would be rejected
+      // before it reached the transport.
+      ctx.logger.error(
+        `EmailServicePlugin: the saved From address '${fromEmail}' is not a valid email address — it is `
+        + 'NOT applied (the previous sender is kept) because every message sent from it would be rejected '
+        + `before reaching the transport, silently on the sign-up path. ${DEFAULT_FROM_FIX}`,
+      );
+    } else if (fromEmail) {
+      this.service.setDefaultFrom({ address: fromEmail, name: fromName });
+      appliedFrom = fromEmail;
+    }
 
     const provider = String(values.provider ?? 'smtp');
 
@@ -1449,7 +1512,7 @@ export class EmailServicePlugin implements Plugin {
 
     if (provider === 'log') {
       ctx.logger.info(
-        `EmailServicePlugin: mail settings applied (provider=log, from=${fromEmail ?? '∅'}); `
+        `EmailServicePlugin: mail settings applied (provider=log, from=${appliedFrom ?? '∅'}); `
         + 'transport unchanged — messages are logged and recorded in sys_email, never delivered.',
       );
       return;