fix(plugin-email, service-sms): Send test goes to the signed-in admin when the request names no recipient

.changeset/24767-settings-test-send-recipient.md

@@ -0,0 +1,20 @@
+---
+'@objectstack/plugin-email': patch
+'@objectstack/service-sms': patch
+---
+
+fix(plugin-email, service-sms): Setup's "Send test email" and "Send test SMS" send to the signed-in admin when the request names no recipient (#24767)
+
+Clause-②: no
+
+The Mail and SMS settings pages offer a Send test button and no recipient input, so the console's request carries no `to`. "Send test email" fell back to the From address — often a no-reply mailbox nobody reads — and with From empty it refused, asking for a "to" address the page has no field for. "Send test SMS" refused every press the same way.
+
+With no `to` in the request, the test now goes to the person who pressed the button: `mail/test` sends to the caller's own account email and `sms/test` to the caller's own phone number, each read from the caller's own `sys_user` row. A `to` in the request body still wins, as before. When the caller's account has no address or number, or the account cannot be read, the action refuses and says that nothing was sent. A phone number is on an account only where phone sign-in is enabled, so on other deployments "Send test SMS" still needs a `to` in the request body.
+
+`sms/test` no longer reports a send through the log-only transport (the namespace's default `None (log only)` provider) as a success. It answers `ok: false` with severity `warning`, as `mail/test` already does for its log transport. When the form names no delivering provider and the transport the deployment configured at boot carries the test, the success message names that transport instead of saying the message went "via log".
+
+**Unchanged.**
+
+- The request and result shapes of `POST /api/settings/mail/test` and `POST /api/settings/sms/test`: no key, route or setting is added.
+- Which provider and credentials a test uses: the saved values overlaid with the form's unsaved edits, as before.
+- Saving the Mail form still judges a required field only when the save writes it, or writes a key its visibility depends on.

packages/plugins/plugin-email/src/email-plugin.queue-delivery.test.ts

@@ -517,8 +517,7 @@ describe('mail/test is never queued', () => {
 
     const result = await h.settings.action('test')!({
       values: { provider: 'log', from_email: 'no-reply@example.test' },
-      payload: {},
-      ctx: { body: { to: 'ops@example.test' } },
+      payload: { to: 'ops@example.test' },
     });
 
     // The answer came from the TRANSPORT — it either sent or it did not —
@@ -542,8 +541,7 @@ describe('mail/test is never queued', () => {
 
     const result = await h.settings.action('test')!({
       values: { provider: 'log', from_email: 'no-reply@example.test' },
-      payload: {},
-      ctx: { body: { to: 'ops@example.test' } },
+      payload: { to: 'ops@example.test' },
     });
 
     expect(result.ok).toBe(false);

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

@@ -148,12 +148,77 @@ interface MailSettingsSurface {
     action: string,
     handler: (input: {
       values?: Record<string, unknown>;
+      /** The request body the action was invoked with — the console posts its unsaved form edits here. */
       payload?: Record<string, unknown>;
-      ctx?: { body?: Record<string, unknown> };
+      /** The caller's settings context; `userId` is the signed-in admin who pressed the button. */
+      ctx?: { userId?: string };
     }) => Promise<unknown>,
   ): void;
 }
 
+/** The identity object a `mail/test` press reads the caller's own address from. */
+const ACCOUNT_OBJECT = 'sys_user';
+
+/**
+ * Who a `mail/test` press sends to: the `to` the request names, else the
+ * signed-in admin's own account address.
+ *
+ * The served form has no recipient input — a settings manifest cannot declare
+ * a value an action posts without storing it — so the console's request
+ * carries no `to`. This used to fall back to `from_email`, the SENDER address
+ * (usually a no-reply mailbox nobody reads), so "Sent test email to no-reply@…"
+ * reported a delivery the operator could not observe, and with From empty the
+ * button refused by asking for a "to" field the page does not have. The person
+ * who pressed the button is the one recipient who can confirm the delivery,
+ * and a signed-in admin has an account address.
+ *
+ * The read is pinned to the CALLER's own row — `where.id` is the request's
+ * own `userId`, never a value the body names — and projects `email` alone. It
+ * runs as the system because resolving who the caller is must not depend on
+ * the caller holding read permission on the identity table (the reason the
+ * shared authz resolver's own `sys_user` read gives); the one value it yields
+ * reaches the caller in the result message, and it is their own address, so
+ * nothing the caller could not already read is disclosed.
+ *
+ * Every refusal says that nothing was sent and what to supply. A read that
+ * fails is reported as a failed read, never as "you have no address".
+ */
+async function resolveMailTestRecipient(
+  payload: unknown,
+  userId: unknown,
+  engine: IDataEngine,
+): Promise<{ to: string } | { refusal: string }> {
+  const named = payload && typeof payload === 'object' ? (payload as { to?: unknown }).to : undefined;
+  if (typeof named === 'string' && named.trim() !== '') return { to: named.trim() };
+  if (typeof userId !== 'string' || userId === '') {
+    return {
+      refusal: 'No recipient — nothing was sent. This call names no "to" address and carries no signed-in '
+        + 'user whose own address the test could go to; pass "to" in the request body.',
+    };
+  }
+  let row: Record<string, unknown> | null;
+  try {
+    row = await engine.findOne(
+      ACCOUNT_OBJECT,
+      { where: { id: userId }, fields: ['email'] },
+      { context: { isSystem: true } },
+    );
+  } catch (err: any) {
+    return {
+      refusal: `Could not read your account's email address to send the test to (${err?.message ?? String(err)}) `
+        + '— nothing was sent. Pass "to" in the request body to name a recipient.',
+    };
+  }
+  const email = row?.email;
+  if (typeof email !== 'string' || email.trim() === '') {
+    return {
+      refusal: 'Your account has no email address, and the test is sent to the signed-in admin\'s own address '
+        + '— nothing was sent. Add an email address to your account, or pass "to" in the request body.',
+    };
+  }
+  return { to: email.trim() };
+}
+
 /**
  * Resolve a queue service that can actually carry a durable email job, or
  * `undefined`.
@@ -562,17 +627,20 @@ export class EmailServicePlugin implements Plugin {
           // provider/credentials differ from what the live `svc` is bound
           // to, a one-shot temporary `EmailService` is built so the
           // operator can validate edits before hitting "Save".
+          //
+          // The recipient is the request's `to`, else the signed-in admin's
+          // own address — see `resolveMailTestRecipient` for why it is not
+          // `from_email`.
           if (typeof settings.registerAction === 'function') {
             const svc = this.service;
             settings.registerAction('mail', 'test', async ({ values, payload, ctx: actionCtx }: any) => {
               const overrides = extractOverrides(payload);
               const merged: Record<string, unknown> = { ...(values ?? {}), ...overrides };
-              const to = (actionCtx?.body?.to as string | undefined)
-                ?? (payload?.to as string | undefined)
-                ?? (merged.from_email as string | undefined);
-              if (!to) {
-                return { ok: false, severity: 'error', message: 'Provide a "to" address (or set from_email).' };
+              const recipient = await resolveMailTestRecipient(payload, actionCtx?.userId, engine);
+              if ('refusal' in recipient) {
+                return { ok: false, severity: 'error', message: recipient.refusal };
               }
+              const { to } = recipient;
 
               // Build a temporary service from the merged values when
               // the form differs from the live svc — covers the

packages/plugins/plugin-email/src/mail-test-recipient.pin.test.ts

@@ -0,0 +1,196 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// Who "Send test email" sends to, driven through the SERVED door: a real
+// `SettingsService` holding the real `mail` manifest, its real REST routes, and
+// the real `mail/test` handler that `EmailServicePlugin` registers at
+// `kernel:ready`.
+//
+// The served form has no recipient input, so the console's request body is its
+// unsaved edits — `{}` on a page nobody has edited — and carries no `to`. The
+// handler used to fall back to `from_email`, the sender address: a "success"
+// mailed to a no-reply box, or, with From empty, a refusal asking for a field
+// the page does not have. The pins: with no `to`, the test goes to the signed-in
+// admin's own address, read from their own `sys_user` row; an explicit `to`
+// still wins; and every refusal that remains carries the ADR-0112 envelope.
+
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import type { IHttpServer, IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
+import { assertEngineFindOnePredicate, assertEngineUpdateDispatch } from '@objectstack/objectql';
+import { SettingsService, mailSettingsManifest, registerSettingsRoutes } from '@objectstack/service-settings';
+import { EmailServicePlugin } from './email-plugin.js';
+
+const nm = vi.hoisted(() => ({ createTransport: vi.fn(), sendMail: vi.fn() }));
+vi.mock('nodemailer', () => ({
+  default: { createTransport: nm.createTransport },
+  createTransport: nm.createTransport,
+}));
+
+class MockHttp implements IHttpServer {
+  routes = new Map<string, RouteHandler>();
+  private add(method: string, path: string, handler: RouteHandler) {
+    this.routes.set(`${method} ${path}`, handler);
+  }
+  get(path: string, h: RouteHandler) { this.add('GET', path, h); return this as any; }
+  post(path: string, h: RouteHandler) { this.add('POST', path, h); return this as any; }
+  put(path: string, h: RouteHandler) { this.add('PUT', path, h); return this as any; }
+  delete(path: string, h: RouteHandler) { this.add('DELETE', path, h); return this as any; }
+  patch(path: string, h: RouteHandler) { this.add('PATCH', path, h); return this as any; }
+  use() { return this as any; }
+  listen() { return Promise.resolve(); }
+  close() { return Promise.resolve(); }
+  getInstance() { return null; }
+}
+
+const ADMIN_ID = 'usr_admin';
+const ADMIN_EMAIL = 'admin@acme.test';
+const FROM_EMAIL = 'no-reply@acme.test';
+
+type UserRow = { id: string; email?: string | null };
+
+/** The engine double: `update` and `findOne` held to ObjectQL's own dispatch contract. */
+function fakeEngine(users: UserRow[], opts: { userReadFails?: boolean } = {}) {
+  return {
+    insert: vi.fn(async (_object: string, row: any) => ({ id: row?.id })),
+    update: vi.fn(async (_object: string, data: any, options?: any) => {
+      assertEngineUpdateDispatch(data, options);
+      return undefined;
+    }),
+    find: vi.fn(async () => []),
+    findOne: vi.fn(async (object: string, query: any) => {
+      assertEngineFindOnePredicate(object, query);
+      if (object !== 'sys_user') return null;
+      if (opts.userReadFails) throw new Error('sys_user is unavailable');
+      return users.find((u) => u.id === query?.where?.id) ?? null;
+    }),
+  };
+}
+
+function fakeCtx(services: Record<string, unknown>) {
+  const hooks: Record<string, Array<() => Promise<void> | void>> = {};
+  return {
+    logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
+    getService: <T,>(name: string): T => {
+      if (!(name in services)) throw new Error(`service '${name}' not registered`);
+      return services[name] as T;
+    },
+    registerService: (name: string, svc: unknown) => { services[name] = svc; },
+    hook: (name: string, fn: () => Promise<void> | void) => { (hooks[name] ??= []).push(fn); },
+    fire: async (name: string) => { for (const fn of hooks[name] ?? []) await fn(); },
+  };
+}
+
+/** Boot the served `mail/test` door: real settings service + routes, the plugin's real action. */
+async function serve(users: UserRow[], opts: { userReadFails?: boolean } = {}) {
+  const settings = new SettingsService({ env: {} });
+  settings.registerManifest(mailSettingsManifest);
+  await settings.setMany('mail', { provider: 'smtp', smtp_host: 'smtp.acme.test', from_email: FROM_EMAIL });
+  const engine = fakeEngine(users, opts);
+  const services: Record<string, unknown> = { manifest: { register: () => {} }, objectql: engine, settings };
+  const ctx = fakeCtx(services);
+  const plugin = new EmailServicePlugin();
+  await plugin.init(ctx as never);
+  await plugin.start(ctx as never);
+  await ctx.fire('kernel:ready');
+
+  const http = new MockHttp();
+  registerSettingsRoutes(http, settings, {
+    contextFromRequest: () => ({
+      enforced: true,
+      permissions: ['manage_platform_settings'],
+      userId: ADMIN_ID,
+    }),
+  });
+
+  /** POST /api/settings/mail/test with the body the console sends. */
+  const pressSendTest = async (body: unknown) => {
+    const handler = http.routes.get('POST /api/settings/:namespace/:actionId');
+    if (!handler) throw new Error('the settings action route is not mounted');
+    const captured: { status: number; body: any } = { status: 200, body: undefined };
+    const res: IHttpResponse = {
+      json: vi.fn((data: any) => { captured.body = data; }) as any,
+      send: vi.fn() as any,
+      status: vi.fn((code: number) => { captured.status = code; return res; }) as any,
+      header: vi.fn(() => res) as any,
+    };
+    const req: IHttpRequest = {
+      params: { namespace: 'mail', actionId: 'test' },
+      query: {},
+      body,
+      headers: {},
+      method: 'POST',
+      path: '/api/settings/mail/test',
+    };
+    await handler(req, res);
+    return captured;
+  };
+  return { engine, settings, pressSendTest };
+}
+
+/** The `to` of the one message nodemailer was handed, as text. */
+const sentTo = () => JSON.stringify(nm.sendMail.mock.calls[0]?.[0]?.to);
+
+beforeEach(() => {
+  nm.createTransport.mockReset();
+  nm.sendMail.mockReset();
+  nm.createTransport.mockImplementation(() => ({ sendMail: nm.sendMail, close: vi.fn() }));
+  nm.sendMail.mockResolvedValue({ messageId: '<wire-1@smtp>', response: '250 2.0.0 Ok' });
+});
+
+describe('served mail/test — the recipient', () => {
+  it('with no "to", sends to the signed-in admin\'s own address, not to From', async () => {
+    const { engine, pressSendTest } = await serve([{ id: ADMIN_ID, email: ADMIN_EMAIL }]);
+
+    const answer = await pressSendTest({});
+
+    expect(answer.status).toBe(200);
+    expect(answer.body?.data).toMatchObject({ ok: true });
+    expect(nm.sendMail).toHaveBeenCalledTimes(1);
+    expect(sentTo()).toContain(ADMIN_EMAIL);
+    expect(sentTo()).not.toContain(FROM_EMAIL);
+    // The read is the caller's OWN row, projected to the address alone.
+    expect(engine.findOne).toHaveBeenCalledWith(
+      'sys_user',
+      { where: { id: ADMIN_ID }, fields: ['email'] },
+      { context: expect.objectContaining({ isSystem: true }) },
+    );
+  });
+
+  it('control: an explicit "to" still wins, and no account is read', async () => {
+    const { engine, pressSendTest } = await serve([{ id: ADMIN_ID, email: ADMIN_EMAIL }]);
+
+    const answer = await pressSendTest({ to: 'ops@acme.test' });
+
+    expect(answer.status).toBe(200);
+    expect(nm.sendMail).toHaveBeenCalledTimes(1);
+    expect(sentTo()).toContain('ops@acme.test');
+    expect(sentTo()).not.toContain(ADMIN_EMAIL);
+    expect(engine.findOne).not.toHaveBeenCalledWith('sys_user', expect.anything(), expect.anything());
+  });
+
+  it.each([
+    ['an account with no email address', [{ id: ADMIN_ID, email: null }], {}],
+    ['an account row that is missing', [], {}],
+    ['a failed account read', [{ id: ADMIN_ID, email: ADMIN_EMAIL }], { userReadFails: true }],
+  ] as const)('refuses, sending nothing, for %s — ADR-0112 envelope', async (_name, users, opts) => {
+    const { pressSendTest } = await serve([...users], opts);
+
+    const answer = await pressSendTest({});
+
+    expect(answer.status).toBe(400);
+    expect(answer.body?.success).toBe(false);
+    expect(answer.body?.error?.code).toBe('SETTINGS_ACTION_FAILED');
+    expect(answer.body?.error?.details).toMatchObject({ ok: false, severity: 'error' });
+    expect(nm.sendMail).not.toHaveBeenCalled();
+  });
+
+  it('an in-process call with no "to" and no user refuses rather than mailing From', async () => {
+    // Not a public door — the routes refuse an anonymous caller before any
+    // action runs — but the trusted in-process path reaches the same handler.
+    const { settings } = await serve([{ id: ADMIN_ID, email: ADMIN_EMAIL }]);
+
+    const result = await settings.runAction('mail', 'test', {}, {});
+
+    expect(result).toMatchObject({ ok: false, severity: 'error' });
+    expect(nm.sendMail).not.toHaveBeenCalled();
+  });
+});