diff --git a/.changeset/smtp-transport-plugin-email.md b/.changeset/smtp-transport-plugin-email.md new file mode 100644 index 0000000000..630ae5532e --- /dev/null +++ b/.changeset/smtp-transport-plugin-email.md @@ -0,0 +1,55 @@ +--- +"@objectstack/plugin-email": minor +"@objectstack/service-settings": patch +"@objectstack/cli": patch +--- + +feat(plugin-email): real SMTP delivery — `SmtpTransport`, settings hot-swap, and a `mail/test` that actually sends (#5087) + +The **Mail Delivery** settings page has always defaulted to SMTP and offered a +full host / port / TLS / username / password form. Nothing behind it delivered: +`applyMailSettings` treated `provider: 'smtp'` as a no-op ("transport +unchanged"), `mail/test` answered `ok: true, "Configuration looks valid … Wire +@objectstack/plugin-mail for actual delivery"` — a success toast for a message +nobody sent, naming a package that has never existed — and the code pointed +operators at `@objectstack/plugin-mail-smtp`, which is not in this repo or on +npm. A workspace that selected SMTP got a green form, a green test button, and +mail that only ever reached the log and the `sys_email` table. For deployments +in China this left **no** working channel at all: Resend and Postmark are +overseas HTTPS SaaS with unreliable reach and deliverability to QQ / 163 / +enterprise mailboxes, where SMTP is the normal path (Aliyun DirectMail, Tencent +SES, corporate mail servers). + +**`SmtpTransport` now ships in `@objectstack/plugin-email`** (ADR-0012: SMTP in +core, implemented with `nodemailer`). `nodemailer` is a real dependency but is +imported **lazily on the first send**, so deployments that never select SMTP — +and non-Node runtimes — never load `node:net` / `node:tls`. + +Three doors reach it, all sharing one options reader so they cannot drift: + +- **Settings → Mail** (`smtp_host` / `smtp_port` / `smtp_secure` / `smtp_user` / + `smtp_password`) hot-swaps the live transport on save, no restart. +- **`os serve`** via `OS_EMAIL_PROVIDER=smtp` plus the new `OS_EMAIL_SMTP_HOST` / + `_PORT` / `_SECURE` / `_USER` / `_PASSWORD` (or `config.email.options`). +- **Constructor**: `new EmailServicePlugin({ provider: 'smtp', providerOptions: + { host, port, secure, user, password } })`. + +TLS is one toggle with the wire behaviour derived from the port, as providers +document it: on `465` implicit TLS (SMTPS); on any other port a **required** +STARTTLS upgrade, so a server that refuses to upgrade fails the send instead of +leaking credentials over a cleartext socket; `secure: false` connects in the +clear and upgrades only when STARTTLS is offered. + +**Failure is loud everywhere, because a silent fallback is the bug this fixes.** +On the construction path (CLI / plugin options) a `smtp` provider with no host +**throws** and the boot fails — it no longer degrades into a LogTransport that +reports every send as successful. On the settings hot-swap path a save can never +kill a running server, so the previous transport is kept — but the failure is +logged at `error` naming the consequence and the fix, and **`mail/test` now +performs a real delivery** through the settings on screen and reports the SMTP +server's own words (`535 … authentication failed`) instead of a green toast. The +built-in fallback `mail/test` handler (used only when no email plugin is +mounted) answers `ok: false` and says plainly that nothing was sent. + +Nothing to migrate: `log`, `resend` and `postmark` behave exactly as before, and +a deployment that never selects `smtp` is unaffected. diff --git a/content/docs/deployment/environment-variables.mdx b/content/docs/deployment/environment-variables.mdx index cc07ce0b4d..26ef83dc33 100644 --- a/content/docs/deployment/environment-variables.mdx +++ b/content/docs/deployment/environment-variables.mdx @@ -124,16 +124,29 @@ Auth settings precedence: | Variable | Type | Default | Description | |:---|:---|:---|:---| -| `OS_EMAIL_PROVIDER` | enum | `log` | Transport. `log` \| `resend` \| `postmark`. `log` (default) prints to stdout without sending. Real SMTP delivery requires the separate `@objectstack/plugin-mail-smtp` package. | +| `OS_EMAIL_PROVIDER` | enum | `log` | Transport. `log` \| `smtp` \| `resend` \| `postmark`. `log` (default) prints to stdout without sending. | | `OS_EMAIL_API_KEY` | string | — | API key for `resend` / `postmark`. | | `OS_EMAIL_FROM` | email | — | Default `From:` address. | | `OS_EMAIL_RETRIES` | number | `0` | Retry count for transient send failures (`0` = no retry). | - -> No SMTP transport ships in the open-core runtime — `OS_EMAIL_PROVIDER` -> only materialises `log`, `resend`, or `postmark`. Real SMTP delivery -> requires the separate `@objectstack/plugin-mail-smtp` package, which reads -> its own SMTP settings (configured in **Settings → Mail** or via the -> plugin's constructor options). +| `OS_EMAIL_SMTP_HOST` | string | — | SMTP server hostname. **Required** when `OS_EMAIL_PROVIDER=smtp` — a boot without it fails loudly rather than starting with a transport that logs mail instead of sending it. | +| `OS_EMAIL_SMTP_PORT` | number | `587` | SMTP port. `465` selects implicit TLS (SMTPS). | +| `OS_EMAIL_SMTP_SECURE` | boolean | `true` | Require TLS. On port `465` that means implicit TLS; on any other port a **required** STARTTLS upgrade (a server that will not upgrade fails the send instead of leaking credentials in the clear). `false` connects in the clear and upgrades only when STARTTLS is offered. | +| `OS_EMAIL_SMTP_USER` | string | — | SMTP AUTH username. Omit for servers that accept unauthenticated relay. | +| `OS_EMAIL_SMTP_PASSWORD` | string | — | SMTP AUTH password. | + +> SMTP delivery ships in `@objectstack/plugin-email` (ADR-0012) and is +> implemented with `nodemailer`, imported lazily so deployments that never +> select SMTP — and non-Node runtimes — never load `node:net` / `node:tls`. +> Any provider that speaks SMTP works through it: a corporate mail server, +> Aliyun DirectMail, Tencent SES, QQ / 163 enterprise mailboxes. +> +> These variables configure the transport at **boot**. The same connection +> can be configured at runtime in **Settings → Mail** (namespace `mail`, +> keys `smtp_host` / `smtp_port` / `smtp_secure` / `smtp_user` / +> `smtp_password`), which hot-swaps the live transport without a restart and +> whose **Send test email** button performs a real delivery. Settings-page +> values are overridden by the namespace's own `OS_MAIL_*` env door +> (e.g. `OS_MAIL_SMTP_HOST`), which locks the field in the UI. --- diff --git a/packages/cli/src/commands/serve-email-capability.test.ts b/packages/cli/src/commands/serve-email-capability.test.ts new file mode 100644 index 0000000000..2b7fb4c948 --- /dev/null +++ b/packages/cli/src/commands/serve-email-capability.test.ts @@ -0,0 +1,109 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * framework#5087 — what `EmailServicePlugin` is constructed with on the + * `os serve` path, and specifically what happens when SMTP is selected. + * + * `OS_EMAIL_PROVIDER=smtp` used to be unreachable: the plugin knew three + * providers (`log`/`resend`/`postmark`), so `smtp` fell into the "no apiKey" + * arm and was silently rewritten to `log`. The server then booted "fine", + * every send was recorded in `sys_email` as sent, and nothing left the box. + * These pin the opposite: a complete SMTP configuration reaches the plugin, + * and an incomplete one fails the boot instead of degrading into a transport + * that reports success. + */ + +import { describe, it, expect } from 'vitest'; +import { resolveEmailCapabilityArg } from './serve.js'; + +describe('resolveEmailCapabilityArg', () => { + it('defaults to the log provider when nothing is configured', () => { + const { options, warning } = resolveEmailCapabilityArg({}, {}); + expect(options).toMatchObject({ provider: 'log' }); + expect(options).not.toHaveProperty('providerOptions'); + expect(warning).toBeUndefined(); + }); + + it('assembles the SMTP connection from OS_EMAIL_SMTP_*', () => { + const { options, warning } = resolveEmailCapabilityArg({}, { + OS_EMAIL_PROVIDER: 'smtp', + OS_EMAIL_SMTP_HOST: ' smtp.exmail.qq.com ', + OS_EMAIL_SMTP_PORT: '465', + OS_EMAIL_SMTP_SECURE: 'true', + OS_EMAIL_SMTP_USER: 'ops@example.cn', + OS_EMAIL_SMTP_PASSWORD: 'sekrit', + OS_EMAIL_FROM: 'Acme ', + }); + expect(warning).toBeUndefined(); + expect(options).toMatchObject({ + provider: 'smtp', + providerOptions: { + host: 'smtp.exmail.qq.com', + port: 465, + secure: true, + user: 'ops@example.cn', + password: 'sekrit', + }, + defaultFrom: { name: 'Acme', address: 'no-reply@example.cn' }, + }); + }); + + it('reads OS_EMAIL_SMTP_SECURE=false as plain-connect', () => { + const { options } = resolveEmailCapabilityArg({}, { + OS_EMAIL_PROVIDER: 'smtp', + OS_EMAIL_SMTP_HOST: 'smtp.x', + OS_EMAIL_SMTP_SECURE: 'false', + }); + expect((options.providerOptions as any).secure).toBe(false); + expect(resolveEmailCapabilityArg({}, { + OS_EMAIL_PROVIDER: 'smtp', OS_EMAIL_SMTP_HOST: 'smtp.x', OS_EMAIL_SMTP_SECURE: '0', + }).options.providerOptions).toMatchObject({ secure: false }); + }); + + it('layers env over config.email.options', () => { + const { options } = resolveEmailCapabilityArg( + { provider: 'smtp', options: { host: 'smtp.config', port: 25 } }, + { OS_EMAIL_SMTP_HOST: 'smtp.env' }, + ); + expect(options.providerOptions).toEqual({ host: 'smtp.env', port: 25 }); + }); + + it('accepts an SMTP host declared only in config.email.options', () => { + const { options } = resolveEmailCapabilityArg({ provider: 'smtp', options: { host: 'smtp.config' } }, {}); + expect(options).toMatchObject({ provider: 'smtp', providerOptions: { host: 'smtp.config' } }); + }); + + it('THROWS on provider=smtp without a host — never a silent LogTransport', () => { + expect(() => resolveEmailCapabilityArg({}, { OS_EMAIL_PROVIDER: 'smtp' })) + .toThrow(/no SMTP host is configured/); + expect(() => resolveEmailCapabilityArg({}, { OS_EMAIL_PROVIDER: 'smtp', OS_EMAIL_SMTP_PORT: '587' })) + .toThrow(/OS_EMAIL_SMTP_HOST/); + }); + + it('does not apply the apiKey fallback to smtp', () => { + // The `resend`/`postmark` arm degrades to `log` when the key is missing; + // smtp must never reach it (it needs no apiKey at all). + const { options, warning } = resolveEmailCapabilityArg({}, { + OS_EMAIL_PROVIDER: 'smtp', + OS_EMAIL_SMTP_HOST: 'smtp.x', + }); + expect(options.provider).toBe('smtp'); + expect(warning).toBeUndefined(); + }); + + it('keeps the pre-existing resend/postmark behaviour', () => { + const withKey = resolveEmailCapabilityArg({}, { OS_EMAIL_PROVIDER: 'resend', OS_EMAIL_API_KEY: 're_x' }); + expect(withKey.options).toMatchObject({ provider: 'resend', apiKey: 're_x' }); + expect(withKey.warning).toBeUndefined(); + + const noKey = resolveEmailCapabilityArg({}, { OS_EMAIL_PROVIDER: 'postmark' }); + expect(noKey.options.provider).toBe('log'); + expect(noKey.warning).toMatch(/no apiKey found/); + }); + + it('still derives the fallback from-address and template context', () => { + const { options } = resolveEmailCapabilityArg({}, { OS_APP_NAME: 'Acme CRM' }, 'ignored'); + expect(options.defaultTemplateContext).toMatchObject({ appName: 'Acme CRM' }); + expect(options.defaultFrom).toEqual({ name: 'Acme CRM', address: 'no-reply@acme-crm.local' }); + }); +}); diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index c2c00ec86b..13894505b4 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -2269,53 +2269,13 @@ export default class Serve extends Command { const cubes = (config as any).analyticsCubes ?? (config as any).cubes ?? []; arg = { cubes }; } else if (cap === 'email') { - // Compose EmailServicePlugin options from config.email + OS_EMAIL_* env. - // Env precedence: env beats config so operators can override per-environment. - const cfgEmail = (config as any).email ?? {}; - const envProvider = process.env.OS_EMAIL_PROVIDER; - const provider = (envProvider || cfgEmail.provider || 'log').toLowerCase(); - const apiKey = process.env.OS_EMAIL_API_KEY || cfgEmail.apiKey; - const envFrom = process.env.OS_EMAIL_FROM; - // OS_EMAIL_FROM supports either "addr@x" or "Name ". - let defaultFrom = cfgEmail.defaultFrom; - if (envFrom) { - const m = envFrom.match(/^\s*(?:"?([^"<]*?)"?\s*<\s*([^>]+)\s*>|(\S+))\s*$/); - if (m) { - const name = (m[1] ?? '').trim(); - const address = (m[2] ?? m[3] ?? '').trim(); - if (address) defaultFrom = name ? { name, address } : { address }; - } - } - const retries = process.env.OS_EMAIL_RETRIES - ? Number(process.env.OS_EMAIL_RETRIES) - : cfgEmail.retries; - const defaultTemplateContext = { - appName: process.env.OS_APP_NAME || cfgEmail.appName || (config as any).appName || 'ObjectStack', - ...(cfgEmail.defaultTemplateContext || {}), - }; - // Provide a sensible fallback `from` so templates can render - // even before operators configure SMTP/SaaS. The log transport - // simply prints to stdout; the address never leaves the box. - if (!defaultFrom) { - const slug = String(defaultTemplateContext.appName || 'objectstack') - .toLowerCase() - .replace(/[^a-z0-9]+/g, '-') - .replace(/^-+|-+$/g, '') || 'objectstack'; - defaultFrom = { name: defaultTemplateContext.appName, address: `no-reply@${slug}.local` }; - } - arg = { - provider, - ...(apiKey ? { apiKey } : {}), - defaultFrom, - ...(retries != null && !Number.isNaN(retries) ? { retries } : {}), - defaultTemplateContext, - }; - if (provider !== 'log' && !apiKey) { - console.warn(chalk.yellow( - ` ⚠ Capability "email": provider='${provider}' but no apiKey found (set OS_EMAIL_API_KEY or config.email.apiKey). Falling back to LogTransport.`, - )); - arg.provider = 'log'; - } + const emailArg = resolveEmailCapabilityArg( + (config as any).email ?? {}, + process.env, + (config as any).appName, + ); + arg = emailArg.options; + if (emailArg.warning) console.warn(chalk.yellow(` ⚠ Capability "email": ${emailArg.warning}`)); } else if (cap === 'sms') { // Compose SmsServicePlugin options from config.sms + OS_SMS_* env // (#2780). Same precedence as email: env beats config. Provider @@ -2851,6 +2811,104 @@ export function resolveStorageCapabilityArg(envRoot?: string): StorageCapability return { options: { adapter: 'local', local: { rootDir } }, localRoot: rootDir }; } +/** + * Constructor options for `EmailServicePlugin`, plus an optional warning for + * the caller to print (degraded, but still bootable, configurations). + */ +export interface EmailCapabilityArg { + options: Record; + warning?: string; +} + +/** + * Resolve what `EmailServicePlugin` is constructed with, from `config.email` + * plus `OS_EMAIL_*` env (env wins, so an operator can override per environment). + * + * SMTP (#5087, ADR-0012) is configured through `OS_EMAIL_SMTP_HOST` / `_PORT` / + * `_SECURE` / `_USER` / `_PASSWORD` — the `OS_{DOMAIN}_{FEATURE}_{QUALIFIER}` + * shape of Prime Directive #9, grouped with the email vars rather than the bare + * third-party `SMTP_*` names — layered over `config.email.options`. + * + * `provider='smtp'` with no host **throws**. The capability loop turns that into + * a boot failure, which is the point: the alternative (quietly substituting the + * LogTransport, as this function's `resend`/`postmark` arm still does for a + * missing API key) hands the operator a server that accepts every send, records + * it in `sys_email`, and delivers nothing — the exact declared-but-not-delivered + * gap #5087 closed inside the plugin. + */ +export function resolveEmailCapabilityArg( + cfgEmail: Record = {}, + env: NodeJS.ProcessEnv = process.env, + configAppName?: string, +): EmailCapabilityArg { + const provider = String(env.OS_EMAIL_PROVIDER || cfgEmail.provider || 'log').toLowerCase(); + const apiKey = env.OS_EMAIL_API_KEY || cfgEmail.apiKey; + + // OS_EMAIL_FROM supports either "addr@x" or "Name ". + let defaultFrom = cfgEmail.defaultFrom; + if (env.OS_EMAIL_FROM) { + const m = env.OS_EMAIL_FROM.match(/^\s*(?:"?([^"<]*?)"?\s*<\s*([^>]+)\s*>|(\S+))\s*$/); + if (m) { + const name = (m[1] ?? '').trim(); + const address = (m[2] ?? m[3] ?? '').trim(); + if (address) defaultFrom = name ? { name, address } : { address }; + } + } + const retries = env.OS_EMAIL_RETRIES ? Number(env.OS_EMAIL_RETRIES) : cfgEmail.retries; + const defaultTemplateContext = { + appName: env.OS_APP_NAME || cfgEmail.appName || configAppName || 'ObjectStack', + ...(cfgEmail.defaultTemplateContext || {}), + }; + // Provide a sensible fallback `from` so templates can render even before + // operators configure SMTP/SaaS. The log transport simply prints to stdout; + // the address never leaves the box. + if (!defaultFrom) { + const slug = String(defaultTemplateContext.appName || 'objectstack') + .toLowerCase() + .replace(/[^a-z0-9]+/g, '-') + .replace(/^-+|-+$/g, '') || 'objectstack'; + defaultFrom = { name: defaultTemplateContext.appName, address: `no-reply@${slug}.local` }; + } + + const smtpEnv: Record = {}; + const smtpHost = env.OS_EMAIL_SMTP_HOST?.trim(); + if (smtpHost) smtpEnv.host = smtpHost; + if (env.OS_EMAIL_SMTP_PORT) smtpEnv.port = Number(env.OS_EMAIL_SMTP_PORT); + if (env.OS_EMAIL_SMTP_SECURE != null) { + const raw = String(env.OS_EMAIL_SMTP_SECURE).trim().toLowerCase(); + smtpEnv.secure = raw !== 'false' && raw !== '0'; + } + if (env.OS_EMAIL_SMTP_USER) smtpEnv.user = env.OS_EMAIL_SMTP_USER; + if (env.OS_EMAIL_SMTP_PASSWORD) smtpEnv.password = env.OS_EMAIL_SMTP_PASSWORD; + const providerOptions = { ...(cfgEmail.options ?? {}), ...smtpEnv }; + + const options: Record = { + provider, + ...(apiKey ? { apiKey } : {}), + ...(Object.keys(providerOptions).length > 0 ? { providerOptions } : {}), + defaultFrom, + ...(retries != null && !Number.isNaN(retries) ? { retries } : {}), + defaultTemplateContext, + }; + + if (provider === 'smtp' && !providerOptions.host) { + throw new Error( + "provider='smtp' selects SMTP delivery but no SMTP host is configured — set OS_EMAIL_SMTP_HOST " + + '(plus OS_EMAIL_SMTP_PORT / _SECURE / _USER / _PASSWORD) or config.email.options.host, ' + + 'or choose another provider.', + ); + } + if (provider !== 'log' && provider !== 'smtp' && !apiKey) { + options.provider = 'log'; + return { + options, + warning: `provider='${provider}' but no apiKey found (set OS_EMAIL_API_KEY or config.email.apiKey). ` + + 'Falling back to LogTransport.', + }; + } + return { options }; +} + /** * Best-effort driver introspection. * diff --git a/packages/plugins/plugin-email/package.json b/packages/plugins/plugin-email/package.json index 706fdfb052..15bfe4a92e 100644 --- a/packages/plugins/plugin-email/package.json +++ b/packages/plugins/plugin-email/package.json @@ -21,10 +21,12 @@ "@objectstack/core": "workspace:*", "@objectstack/formula": "workspace:*", "@objectstack/platform-objects": "workspace:*", - "@objectstack/spec": "workspace:*" + "@objectstack/spec": "workspace:*", + "nodemailer": "^9.0.3" }, "devDependencies": { "@types/node": "^26.1.2", + "@types/nodemailer": "^8.0.1", "typescript": "^6.0.3", "vitest": "^4.1.10" }, diff --git a/packages/plugins/plugin-email/src/email-plugin.mail-settings.test.ts b/packages/plugins/plugin-email/src/email-plugin.mail-settings.test.ts new file mode 100644 index 0000000000..43e992182a --- /dev/null +++ b/packages/plugins/plugin-email/src/email-plugin.mail-settings.test.ts @@ -0,0 +1,305 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +// +// EmailServicePlugin ↔ `mail` settings namespace (#5087). +// +// The gap this pins: the settings page offers a full SMTP form and SMTP is its +// DEFAULT provider, while `applyMailSettings` used to answer "transport +// unchanged" and `mail/test` answered "Configuration looks valid … wire +// @objectstack/plugin-mail for actual delivery" — a success toast for a mail +// nobody sent. Every assertion below is about that pair: a saved SMTP config +// must actually become the live transport, and every way it can fail to must +// be LOUD (error log + a failing test action), never a LogTransport reporting +// success. + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { EmailServicePlugin } from './email-plugin.js'; +import { EmailService, LogTransport } from './email-service.js'; +import { SmtpTransport } from './transports/smtp.js'; + +const nm = vi.hoisted(() => ({ createTransport: vi.fn(), sendMail: vi.fn() })); +vi.mock('nodemailer', () => ({ + default: { createTransport: nm.createTransport }, + createTransport: nm.createTransport, +})); + +// ── harness ──────────────────────────────────────────────────────────────── + +interface Resolved { value: unknown; source?: string } + +function fakeSettings(values: Record) { + const listeners: Array<() => void> = []; + const actions = new Map Promise>(); + return { + createClient: () => ({}), + getNamespace: async () => ({ values }), + subscribe: (_ns: string, cb: () => void) => { listeners.push(cb); }, + registerAction: (ns: string, id: string, fn: (a: any) => Promise) => { + actions.set(`${ns}/${id}`, fn); + }, + /** Simulate a save: patch the snapshot and emit settings:changed. */ + async save(patch: Record) { + Object.assign(values, patch); + for (const l of listeners) l(); + // let the async re-apply settle + await new Promise((r) => setTimeout(r, 0)); + }, + action: (id: string) => actions.get(`mail/${id}`), + }; +} + +function fakeEngine() { + const inserted: Array<{ object: string; row: any }> = []; + const updated: Array<{ object: string; patch: any }> = []; + return { + inserted, + updated, + async insert(object: string, row: any) { inserted.push({ object, row }); return { id: row.id }; }, + async update(object: string, patch: any) { updated.push({ object, patch }); }, + async find() { return []; }, + }; +} + +function fakeCtx(services: Record) { + const hooks: Record Promise | void>> = {}; + const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + return { + logger, + getService: (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) => { (hooks[name] ??= []).push(fn); }, + fire: async (name: string) => { for (const fn of hooks[name] ?? []) await fn(); }, + }; +} + +const MAIL_DEFAULTS: Record = { + provider: { value: 'smtp', source: 'default' }, + smtp_port: { value: 587, source: 'default' }, + smtp_secure: { value: true, source: 'default' }, + from_email: { value: 'no-reply@example.test', source: 'global' }, + from_name: { value: 'ObjectStack', source: 'default' }, +}; + +/** Boot the plugin against fake services and run its kernel:ready hook. */ +async function boot(mailValues: Record, opts: any = {}) { + const engine = fakeEngine(); + const settings = fakeSettings({ ...MAIL_DEFAULTS, ...mailValues }); + const services: Record = { + manifest: { register: () => {} }, + objectql: engine, + settings, + }; + const ctx = fakeCtx(services); + const plugin = new EmailServicePlugin({ seedTemplates: false, ...opts }); + await plugin.init(ctx as never); + await plugin.start(ctx as never); + await ctx.fire('kernel:ready'); + const service = services.email as EmailService; + return { plugin, ctx, engine, settings, service }; +} + +const transportOf = (service: EmailService) => service.options.transport; + +beforeEach(() => { + nm.createTransport.mockReset(); + nm.sendMail.mockReset(); + nm.createTransport.mockImplementation(() => ({ sendMail: nm.sendMail, close: vi.fn() })); + nm.sendMail.mockResolvedValue({ messageId: '', response: '250 2.0.0 Ok' }); +}); + +// ── hot swap ─────────────────────────────────────────────────────────────── + +describe('applyMailSettings — provider=smtp', () => { + it('replaces the LogTransport with a real SmtpTransport built from the saved settings', async () => { + const { service, ctx } = await boot({ + provider: { value: 'smtp', source: 'global' }, + smtp_host: { value: 'smtp.exmail.qq.com', source: 'global' }, + smtp_port: { value: 465, source: 'global' }, + smtp_secure: { value: true, source: 'global' }, + smtp_user: { value: 'ops@example.cn', source: 'global' }, + smtp_password: { value: 'sekrit', source: 'global' }, + }); + + const transport = transportOf(service); + expect(transport).toBeInstanceOf(SmtpTransport); + expect((transport as SmtpTransport).describe()).toMatchObject({ + host: 'smtp.exmail.qq.com', + port: 465, + secure: true, + auth: { user: 'ops@example.cn' }, + }); + expect(ctx.logger.error).not.toHaveBeenCalled(); + }); + + it('re-swaps the transport when the settings are saved again (no restart)', async () => { + const { service, settings } = await boot({ + provider: { value: 'smtp', source: 'global' }, + smtp_host: { value: 'smtp.first.test', source: 'global' }, + }); + expect((transportOf(service) as SmtpTransport).describe()).toMatchObject({ host: 'smtp.first.test' }); + + await settings.save({ smtp_host: { value: 'smtp.second.test', source: 'global' } }); + + expect(transportOf(service)).toBeInstanceOf(SmtpTransport); + expect((transportOf(service) as SmtpTransport).describe()).toMatchObject({ host: 'smtp.second.test' }); + }); + + it('delivers through the swapped-in transport and still records sys_email', async () => { + const { service, engine } = await boot({ + provider: { value: 'smtp', source: 'global' }, + smtp_host: { value: 'smtp.163.com', source: 'global' }, + }); + + const res = await service.send({ + to: 'user@example.test', + subject: '【ObjectStack】您的验证码', + html: '

你好,世界

', + }); + + expect(res.status).toBe('sent'); + expect(nm.sendMail).toHaveBeenCalledTimes(1); + // The message handed to nodemailer keeps the authored UTF-8 text intact — + // MIME/RFC 2047 encoding is nodemailer's job (proved on the wire in + // smtp.wire.test.ts), never a lossy transform of ours. + expect(nm.sendMail.mock.calls[0][0]).toMatchObject({ + subject: '【ObjectStack】您的验证码', + html: '

你好,世界

', + to: ['user@example.test'], + from: 'ObjectStack ', + }); + + // sys_email persistence survives the transport swap. + const row = engine.inserted.find((r) => r.object === 'sys_email'); + expect(row?.row).toMatchObject({ + subject: '【ObjectStack】您的验证码', + body_html: '

你好,世界

', + status: 'queued', + }); + expect(engine.updated[engine.updated.length - 1]?.patch).toMatchObject({ + status: 'sent', + message_id: '', + }); + }); +}); + +describe('applyMailSettings — provider=smtp that cannot be built', () => { + it('keeps the previous transport and logs at ERROR when SMTP was actually selected', async () => { + const { service, ctx } = await boot({ + provider: { value: 'smtp', source: 'global' }, + // no smtp_host + }); + + expect(transportOf(service)).toBeInstanceOf(LogTransport); + expect(ctx.logger.error).toHaveBeenCalledTimes(1); + const line = ctx.logger.error.mock.calls[0][0] as string; + // An error owes the consequence and the fix. + expect(line).toMatch(/NO mail is delivered over SMTP/); + expect(line).toMatch(/Settings → Mail → Host|OS_MAIL_SMTP_HOST/); + }); + + it('keeps a boot-configured SMTP transport when the settings page carries no host', async () => { + // The deployment shape this feature enables: SMTP set through + // OS_EMAIL_SMTP_* at boot, settings page never touched. Mail IS being + // delivered, so there is nothing to report — an error here would be a + // false alarm on every start. + const { service, ctx } = await boot( + { provider: { value: 'smtp', source: 'global' } }, + { provider: 'smtp', providerOptions: { host: 'smtp.boot.test' } }, + ); + expect((transportOf(service) as SmtpTransport).describe()).toMatchObject({ host: 'smtp.boot.test' }); + expect(ctx.logger.error).not.toHaveBeenCalled(); + }); + + it('does not shout on the out-of-the-box default (provider never configured)', async () => { + const { service, ctx } = await boot({}); + expect(transportOf(service)).toBeInstanceOf(LogTransport); + expect(ctx.logger.error).not.toHaveBeenCalled(); + expect(ctx.logger.info.mock.calls.some((args: unknown[]) => /out-of-the-box state/.test(String(args[0])))) + .toBe(true); + }); +}); + +describe('EmailServicePlugin constructor path (CLI / os serve)', () => { + it('THROWS when provider=smtp has no host — a boot that cannot deliver fails loudly', async () => { + const ctx = fakeCtx({ manifest: { register: () => {} } }); + const plugin = new EmailServicePlugin({ provider: 'smtp', seedTemplates: false }); + await expect(plugin.init(ctx as never)).rejects.toThrow(/requires a host/); + }); + + it('builds the SMTP transport from providerOptions', async () => { + const services: Record = { manifest: { register: () => {} } }; + const ctx = fakeCtx(services); + const plugin = new EmailServicePlugin({ + provider: 'smtp', + providerOptions: { host: 'smtp.aliyun.test', port: 465 }, + seedTemplates: false, + }); + await plugin.init(ctx as never); + expect(transportOf(services.email as EmailService)).toBeInstanceOf(SmtpTransport); + }); +}); + +// ── mail/test ────────────────────────────────────────────────────────────── + +describe('mail/test action', () => { + it('performs a REAL send through the form\'s SMTP settings', async () => { + const { settings, engine } = await boot({ + provider: { value: 'smtp', source: 'global' }, + smtp_host: { value: 'smtp.example.cn', source: 'global' }, + }); + + const result = await settings.action('test')!({ + values: { provider: 'smtp', smtp_host: 'smtp.example.cn', from_email: 'no-reply@example.test' }, + payload: { to: 'admin@example.test' }, + }); + + expect(nm.sendMail).toHaveBeenCalledTimes(1); + expect(result).toMatchObject({ ok: true, severity: 'info' }); + expect(result.message).toContain('smtp.example.cn'); + // A test send is a send: it lands in sys_email like any other. + expect(engine.inserted.some((r) => r.object === 'sys_email')).toBe(true); + }); + + it('reports the SMTP server\'s own error when authentication fails', async () => { + nm.sendMail.mockRejectedValue(new Error('Invalid login: 535 5.7.8 Error: authentication failed')); + const { settings } = await boot({ + provider: { value: 'smtp', source: 'global' }, + smtp_host: { value: 'smtp.example.cn', source: 'global' }, + }); + + const result = await settings.action('test')!({ + values: { provider: 'smtp', smtp_host: 'smtp.example.cn', smtp_user: 'u', smtp_password: 'bad', from_email: 'no-reply@example.test' }, + payload: { to: 'admin@example.test' }, + }); + + expect(result.ok).toBe(false); + expect(result.severity).toBe('error'); + expect(result.message).toMatch(/535 5\.7\.8 Error: authentication failed/); + }); + + it('refuses to "test" an SMTP config with no host', async () => { + const { settings } = await boot({ provider: { value: 'smtp', source: 'global' } }); + const result = await settings.action('test')!({ + values: { provider: 'smtp', from_email: 'no-reply@example.test' }, + payload: { to: 'admin@example.test' }, + }); + expect(result).toMatchObject({ ok: false, severity: 'error' }); + expect(result.message).toMatch(/SMTP host is required/); + expect(nm.sendMail).not.toHaveBeenCalled(); + }); + + it('never reports success while only the LogTransport is active', async () => { + const { settings } = await boot({ provider: { value: 'log', source: 'global' } }); + const result = await settings.action('test')!({ + values: { provider: 'log', from_email: 'no-reply@example.test' }, + payload: { to: 'admin@example.test' }, + }); + expect(result.ok).toBe(false); + expect(result.severity).toBe('warning'); + expect(result.message).toMatch(/only logged/); + // ...and the old lie is gone for good. + expect(result.message).not.toMatch(/Configuration looks valid|plugin-mail/); + }); +}); diff --git a/packages/plugins/plugin-email/src/email-plugin.ts b/packages/plugins/plugin-email/src/email-plugin.ts index 1bacc01716..b3466fa9e3 100644 --- a/packages/plugins/plugin-email/src/email-plugin.ts +++ b/packages/plugins/plugin-email/src/email-plugin.ts @@ -9,7 +9,12 @@ import type { } from '@objectstack/spec/contracts'; import { SysEmail, SysEmailTemplate } from '@objectstack/platform-objects/audit'; import { EmailService, LogTransport, type EmailPersistence, type TemplateLoader, type EmailTemplateRow } from './email-service.js'; -import { makeTransport } from './transports/index.js'; +import { + makeTransport, + SmtpTransport, + smtpOptionsFromMailSettings, + type EmailTransportProvider, +} from './transports/index.js'; import { BUILTIN_AUTH_TEMPLATES } from './templates/auth-templates.js'; import type { EmailTemplateDefinition as EmailTemplate } from '@objectstack/spec/system'; import { @@ -35,11 +40,17 @@ export interface EmailServicePluginOptions { * `LogTransport` (no real send). */ transport?: IEmailTransport; - /** Provider tag — `'log' | 'resend' | 'postmark'`. Default `'log'`. */ - provider?: 'log' | 'resend' | 'postmark'; + /** Provider tag — `'log' | 'resend' | 'postmark' | 'smtp'`. Default `'log'`. */ + provider?: EmailTransportProvider; /** API key for resend/postmark. */ apiKey?: string; - /** Provider-specific extra options (e.g. Postmark messageStream). */ + /** + * Provider-specific extra options — Postmark `messageStream`, or the + * SMTP connection for `provider: 'smtp'` (`host` / `port` / `secure` / + * `user` / `password`, see `SmtpTransportOptions`). A `smtp` provider + * with no `host` THROWS at init: a boot that cannot deliver must fail + * loudly rather than degrade to a LogTransport that reports success. + */ providerOptions?: Record; /** Default `From` address applied when `input.from` is omitted. */ defaultFrom?: EmailAddress; @@ -83,21 +94,35 @@ export class EmailServicePlugin implements Plugin { private boundEngine?: IDataEngine; /** Live `email_template` metadata subscription — detached in dispose(). */ private unsubscribeTemplates?: () => void; + /** SMTP transport currently in use, if any — closed in dispose(). */ + private liveSmtp?: SmtpTransport; constructor(options: EmailServicePluginOptions = {}) { this.options = options; } + /** + * Materialise the constructor-configured transport. + * + * Deliberately propagates `makeTransport`'s throw (missing SMTP host, + * missing API key): on the construction path — `os serve`, an explicit + * `new EmailServicePlugin({ provider: 'smtp' })` — a provider that cannot + * be built must fail the boot. Falling back to a LogTransport here would + * hand the operator a server that reports every send as successful and + * delivers nothing (#5087). + */ private resolveTransport(ctx: PluginContext): IEmailTransport { if (this.options.transport) return this.options.transport; const provider = this.options.provider ?? 'log'; if (provider === 'log') return new LogTransport(ctx.logger); - return makeTransport({ + const transport = makeTransport({ provider, apiKey: this.options.apiKey, options: this.options.providerOptions, logger: ctx.logger, }); + if (transport instanceof SmtpTransport) this.liveSmtp = transport; + return transport; } async init(ctx: PluginContext): Promise { @@ -156,10 +181,12 @@ export class EmailServicePlugin implements Plugin { try { const payload = await settings.getNamespace('mail'); const values: Record = {}; + const sources: Record = {}; for (const [k, v] of Object.entries(payload.values as Record)) { values[k] = v?.value; + if (v?.source) sources[k] = String(v.source); } - this.applyMailSettings(values, ctx); + this.applyMailSettings(values, sources, ctx); } catch (err: any) { ctx.logger.warn('EmailServicePlugin: failed to apply mail settings: ' + (err?.message ?? err)); } @@ -173,13 +200,15 @@ export class EmailServicePlugin implements Plugin { ctx.logger.info('EmailServicePlugin: bound to settings:changed for namespace=mail'); } - // Register the `mail/test` action handler so saving + sending - // a test email actually exercises the live transport. + // Register the `mail/test` action handler so pressing "Send test + // email" actually delivers one. This OVERRIDES the built-in + // fallback in service-settings, which can only validate the form + // (and says so) — the same pattern `storage/test` uses. // // The handler accepts both the persisted snapshot (`values`) // and the (possibly unsaved) form state posted as // `payload.values`, with overrides winning. When the merged - // provider/api_key differ from what the live `svc` is bound + // 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". if (typeof settings.registerAction === 'function') { @@ -196,12 +225,43 @@ export class EmailServicePlugin implements Plugin { // Build a temporary service from the merged values when // the form differs from the live svc — covers the - // "edited but not saved" path. + // "edited but not saved" path. For `smtp` this ALWAYS + // happens: the button must exercise the host/port/TLS/ + // credentials on screen, and a real connection is the only + // thing that can report an authentication failure honestly + // (#5087 — this action used to report success for SMTP + // while the live transport was still the LogTransport). let target: EmailService = svc; let tempDescription = ''; + /** One-shot SMTP transport built for this test — closed below. */ + let tempSmtp: SmtpTransport | undefined; const provider = String(merged.provider ?? 'smtp'); const apiKey = typeof merged.api_key === 'string' ? merged.api_key : undefined; - if (provider !== 'smtp' && provider !== 'log') { + if (provider === 'smtp') { + const smtp = smtpOptionsFromMailSettings(merged); + if (!smtp.host) { + return { ok: false, severity: 'error', message: 'SMTP host is required — nothing was sent.' }; + } + try { + tempSmtp = new SmtpTransport({ ...smtp, logger: ctx.logger }); + target = new EmailService({ + transport: tempSmtp, + defaultFrom: merged.from_email + ? { + address: String(merged.from_email), + name: merged.from_name ? String(merged.from_name) : undefined, + } + : undefined, + // Same sys_email audit trail as any other delivery — + // a test send is a send. + ...(svc.options.persistence ? { persistence: svc.options.persistence } : {}), + logger: ctx.logger, + }); + tempDescription = ` via smtp (${smtp.host}:${smtp.port ?? 587})`; + } catch (err: any) { + return { ok: false, severity: 'error', message: `Failed to build SMTP transport: ${err?.message ?? String(err)}` }; + } + } else if (provider !== 'log') { if (!apiKey) { return { ok: false, severity: 'error', message: `${provider}: api_key is required.` }; } @@ -219,6 +279,7 @@ export class EmailServicePlugin implements Plugin { name: merged.from_name ? String(merged.from_name) : undefined, } : undefined, + ...(svc.options.persistence ? { persistence: svc.options.persistence } : {}), logger: ctx.logger, }); tempDescription = ` via ${provider}`; @@ -238,7 +299,24 @@ export class EmailServicePlugin implements Plugin { text: 'This is a test email from the ObjectStack settings page.', }); if (result.status === 'failed') { - return { ok: false, severity: 'error', message: result.error ?? 'Send failed.' }; + // Carry the transport's own words (SMTP reply codes, + // provider error bodies) — the operator needs to read + // "535 authentication failed", not "Send failed". + return { + ok: false, + severity: 'error', + message: `Test send failed${tempDescription}: ${result.error ?? 'unknown transport error'}`, + }; + } + // A LogTransport "send" is not a delivery. Say so instead of + // reporting the success it never had (#5087). + if (target === svc && svc.options.transport instanceof LogTransport) { + return { + ok: false, + severity: 'warning', + message: 'No delivery transport is active — the message was only logged and recorded in sys_email. ' + + 'Configure an SMTP host (or an API provider) and save before testing.', + }; } return { ok: true, @@ -247,6 +325,10 @@ export class EmailServicePlugin implements Plugin { }; } catch (err: any) { return { ok: false, severity: 'error', message: err?.message ?? String(err) }; + } finally { + // The test transport is this call's own — release it rather + // than leaving a connection behind on every button press. + await tempSmtp?.close(); } }); } @@ -463,6 +545,10 @@ export class EmailServicePlugin implements Plugin { async dispose(): Promise { try { this.unsubscribeTemplates?.(); } catch { /* best effort */ } this.unsubscribeTemplates = undefined; + if (this.liveSmtp) { + try { await this.liveSmtp.close(); } catch { /* best effort */ } + this.liveSmtp = undefined; + } if (this.boundEngine) { try { unbindEmailTemplateProvenanceStamp(this.boundEngine as any); } catch { /* best effort */ } this.boundEngine = undefined; @@ -474,18 +560,40 @@ export class EmailServicePlugin implements Plugin { * and `defaultFrom`, then hot-swap them on the running EmailService. * * Behaviour: - * - `provider = 'log' | 'smtp'` keeps the LogTransport (real SMTP - * delivery requires `@objectstack/plugin-mail-smtp`, which is not - * a dependency of this package). The from-address is still applied. + * - `provider = 'smtp'` builds a real {@link SmtpTransport} from + * `smtp_host` / `smtp_port` / `smtp_secure` / `smtp_user` / + * `smtp_password` and swaps it in (ADR-0012 — SMTP ships in core). + * - `provider = 'log'` keeps the LogTransport. The from-address is + * still applied. * - `provider = 'resend' | 'postmark'` rebuilds the transport using - * `api_key` from settings. If `api_key` is missing the swap is - * skipped and a warning is logged — the previous transport stays. + * `api_key` from settings. + * + * **This path never throws.** A settings save must not be able to kill a + * running server, so a transport that cannot be built leaves the previous + * one in place — but it says so at `error` level, naming the consequence + * (mail is NOT being delivered) and the fix, and `mail/test` surfaces the + * same failure to whoever pressed the button. What it must never do is + * keep a LogTransport and report success: that silent gap IS #5087. + * (The constructor / CLI path is the opposite — it throws, so a boot that + * cannot deliver fails loudly instead of starting half-configured.) + * + * `sources` carries each key's provenance from the resolver so the + * unconfigured out-of-the-box state (`provider` still at its manifest + * default of `smtp`, no host anywhere) is reported as the information it + * is, while an OPERATOR-selected SMTP with no host is an error. Escalating + * both would print an error on every fresh dev boot and train everyone to + * skim errors — the failure mode AGENTS.md's degradation-log-level section + * warns about. * * Env-locked fields (handled in SettingsService.get) still resolve * before this method ever sees them, so an env override transparently * wins. */ - private applyMailSettings(values: Record, ctx: PluginContext): void { + private applyMailSettings( + values: Record, + sources: Record, + ctx: PluginContext, + ): void { if (!this.service) return; const fromEmail = typeof values.from_email === 'string' ? values.from_email : undefined; @@ -493,21 +601,59 @@ export class EmailServicePlugin implements Plugin { if (fromEmail) this.service.setDefaultFrom({ address: fromEmail, name: fromName }); const provider = String(values.provider ?? 'smtp'); - if (provider === 'smtp' || provider === 'log') { - // No SMTP transport ships in core; settings-only edits become - // a no-op for transport but still apply `defaultFrom`. Users - // wanting real SMTP install `@objectstack/plugin-mail-smtp` - // and configure it via constructor opts. + + if (provider === 'smtp') { + const smtp = smtpOptionsFromMailSettings(values); + if (!smtp.host) { + // The settings page carries no host — but the boot may already have + // built one from OS_EMAIL_SMTP_* / providerOptions, in which case + // SMTP mail IS being delivered and there is nothing to report. + if (this.service.options.transport instanceof SmtpTransport) { + ctx.logger.info( + 'EmailServicePlugin: mail settings carry no SMTP host — keeping the SMTP transport configured ' + + 'at boot (OS_EMAIL_SMTP_HOST / providerOptions).', + ); + return; + } + const selected = (sources.provider ?? 'default') !== 'default'; + const line = "EmailServicePlugin: provider='smtp' but no SMTP host is configured — the previous " + + 'transport is kept and NO mail is delivered over SMTP. Fix: set Settings → Mail → Host ' + + '(or OS_MAIL_SMTP_HOST), or select another provider.'; + if (selected) ctx.logger.error(line); + else ctx.logger.info(`${line} (Mail has never been configured — this is the out-of-the-box state.)`); + return; + } + try { + const transport = new SmtpTransport({ ...smtp, logger: ctx.logger }); + this.service.setTransport(transport); + this.liveSmtp = transport; + ctx.logger.info( + `EmailServicePlugin: SMTP transport built from settings (host=${smtp.host}:${smtp.port ?? 587}, ` + + `tls=${smtp.secure !== false}, auth=${smtp.user ? 'yes' : 'no'}).`, + ); + } catch (err: any) { + ctx.logger.error( + "EmailServicePlugin: provider='smtp' selected but the SMTP transport could NOT be built — the " + + 'previous transport is kept and NO mail is delivered over SMTP. Fix the SMTP settings and save ' + + 'again. Cause: ' + (err?.message ?? err), + ); + } + return; + } + + if (provider === 'log') { ctx.logger.info( - `EmailServicePlugin: mail settings applied (provider=${provider}, from=${fromEmail ?? '∅'}); transport unchanged.`, + `EmailServicePlugin: mail settings applied (provider=log, from=${fromEmail ?? '∅'}); ` + + 'transport unchanged — messages are logged and recorded in sys_email, never delivered.', ); return; } const apiKey = typeof values.api_key === 'string' ? values.api_key : undefined; if (!apiKey) { - ctx.logger.warn( - `EmailServicePlugin: provider='${provider}' selected but api_key is empty — transport NOT rebuilt.`, + ctx.logger.error( + `EmailServicePlugin: provider='${provider}' selected but api_key is empty — the previous transport ` + + 'is kept and NO mail is delivered through it. Fix: set Settings → Mail → API key.', ); return; } @@ -519,9 +665,13 @@ export class EmailServicePlugin implements Plugin { logger: ctx.logger, }); this.service.setTransport(transport); + this.liveSmtp = undefined; ctx.logger.info(`EmailServicePlugin: transport rebuilt from settings (provider=${provider}).`); } catch (err: any) { - ctx.logger.warn('EmailServicePlugin: failed to rebuild transport: ' + (err?.message ?? err)); + ctx.logger.error( + `EmailServicePlugin: provider='${provider}' selected but the transport could NOT be built — the ` + + 'previous transport is kept and NO mail is delivered through it. Cause: ' + (err?.message ?? err), + ); } } diff --git a/packages/plugins/plugin-email/src/index.ts b/packages/plugins/plugin-email/src/index.ts index a5044e55ec..85d7491445 100644 --- a/packages/plugins/plugin-email/src/index.ts +++ b/packages/plugins/plugin-email/src/index.ts @@ -17,10 +17,14 @@ export { renderTemplate, requireVars, htmlToText } from './template-engine.js'; export { ResendTransport, PostmarkTransport, + SmtpTransport, makeTransport, + smtpOptionsFromMailSettings, type ResendTransportOptions, type PostmarkTransportOptions, + type SmtpTransportOptions, type MakeTransportOptions, + type EmailTransportProvider, } from './transports/index.js'; export { bootstrapDeclaredEmailTemplates, diff --git a/packages/plugins/plugin-email/src/transports/index.ts b/packages/plugins/plugin-email/src/transports/index.ts index d8470c2a2b..3706619619 100644 --- a/packages/plugins/plugin-email/src/transports/index.ts +++ b/packages/plugins/plugin-email/src/transports/index.ts @@ -4,13 +4,23 @@ import type { IEmailTransport } from '@objectstack/spec/contracts'; import { LogTransport } from '../email-service.js'; import { ResendTransport } from './resend.js'; import { PostmarkTransport } from './postmark.js'; +import { SmtpTransport, type SmtpTransportOptions } from './smtp.js'; export { ResendTransport, type ResendTransportOptions } from './resend.js'; export { PostmarkTransport, type PostmarkTransportOptions } from './postmark.js'; +export { SmtpTransport, smtpOptionsFromMailSettings, type SmtpTransportOptions } from './smtp.js'; + +/** Transport tags this package can materialise. */ +export type EmailTransportProvider = 'log' | 'resend' | 'postmark' | 'smtp'; export interface MakeTransportOptions { - provider: 'log' | 'resend' | 'postmark'; + provider: EmailTransportProvider; apiKey?: string; + /** + * Provider-specific options. For `smtp` this is {@link SmtpTransportOptions} + * (`host` / `port` / `secure` / `user` / `password`); for `postmark`, + * `messageStream`; etc. + */ options?: Record; logger?: { info: (msg: string, meta?: any) => void }; } @@ -20,7 +30,10 @@ export interface MakeTransportOptions { * EmailServicePlugin to materialise the transport selected by * `EmailServiceConfig.provider`. * - * Throws when a non-`log` provider is requested without an `apiKey`. + * Throws — never degrades to `LogTransport` — when the selected provider + * cannot be built: `resend`/`postmark` without an `apiKey`, `smtp` without + * a `host`. A transport that silently becomes a no-op while the caller + * believes mail is configured is the defect #5087 exists to close. */ export function makeTransport(opts: MakeTransportOptions): IEmailTransport { const { provider, apiKey, options = {}, logger } = opts; @@ -33,6 +46,16 @@ export function makeTransport(opts: MakeTransportOptions): IEmailTransport { case 'postmark': if (!apiKey) throw new Error("makeTransport: provider='postmark' requires apiKey (OS_EMAIL_API_KEY)"); return new PostmarkTransport({ apiKey, ...(options as any) }); + case 'smtp': { + const smtp = options as Partial; + if (!smtp?.host) { + throw new Error( + "makeTransport: provider='smtp' requires a host " + + '(OS_EMAIL_SMTP_HOST, config.email.options.host, or Settings → Mail → Host)', + ); + } + return new SmtpTransport({ ...smtp, host: smtp.host, logger }); + } default: throw new Error(`makeTransport: unknown provider '${provider}'`); } diff --git a/packages/plugins/plugin-email/src/transports/smtp.test.ts b/packages/plugins/plugin-email/src/transports/smtp.test.ts new file mode 100644 index 0000000000..69bc6005c4 --- /dev/null +++ b/packages/plugins/plugin-email/src/transports/smtp.test.ts @@ -0,0 +1,219 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +// +// SmtpTransport — option mapping, lazy loading and loud failure (#5087). +// nodemailer is mocked here so the assertions are about what WE hand it; the +// real library (and the real wire bytes) are exercised in smtp.wire.test.ts. + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { SmtpTransport, smtpOptionsFromMailSettings } from './smtp.js'; +import { makeTransport } from './index.js'; + +const nm = vi.hoisted(() => ({ + createTransport: vi.fn(), + sendMail: vi.fn(), + close: vi.fn(), +})); + +vi.mock('nodemailer', () => ({ + default: { createTransport: nm.createTransport }, + createTransport: nm.createTransport, +})); + +const MSG = { + to: ['rcpt@example.test'], + from: 'ObjectStack ', + subject: 'Hi', + text: 'hello', +}; + +beforeEach(() => { + nm.createTransport.mockReset(); + nm.sendMail.mockReset(); + nm.createTransport.mockImplementation(() => ({ sendMail: nm.sendMail, close: nm.close })); + nm.sendMail.mockResolvedValue({ messageId: '', response: '250 2.0.0 Ok: queued' }); +}); + +describe('SmtpTransport — construction', () => { + it('refuses to exist without a host (never a silent no-op transport)', () => { + expect(() => new SmtpTransport({ host: '' })).toThrow(/host is required/); + expect(() => new SmtpTransport({ host: ' ' })).toThrow(/host is required/); + expect(() => new SmtpTransport({} as never)).toThrow(/host is required/); + }); + + it('rejects an out-of-range port', () => { + expect(() => new SmtpTransport({ host: 'smtp.x', port: 0 })).toThrow(/invalid port/); + expect(() => new SmtpTransport({ host: 'smtp.x', port: 99999 })).toThrow(/invalid port/); + }); + + it('does NOT load nodemailer until the first send', async () => { + const t = new SmtpTransport({ host: 'smtp.x' }); + expect(nm.createTransport).not.toHaveBeenCalled(); + await t.send(MSG); + expect(nm.createTransport).toHaveBeenCalledTimes(1); + // ...and reuses the transporter afterwards. + await t.send(MSG); + expect(nm.createTransport).toHaveBeenCalledTimes(1); + }); +}); + +describe('SmtpTransport — TLS / auth option mapping', () => { + it('defaults to :587 with a REQUIRED STARTTLS upgrade', async () => { + await new SmtpTransport({ host: 'smtp.example.com' }).send(MSG); + expect(nm.createTransport).toHaveBeenCalledWith(expect.objectContaining({ + host: 'smtp.example.com', + port: 587, + secure: false, + requireTLS: true, + })); + }); + + it('uses implicit TLS (SMTPS) on :465', async () => { + await new SmtpTransport({ host: 'smtp.exmail.qq.com', port: 465 }).send(MSG); + expect(nm.createTransport).toHaveBeenCalledWith(expect.objectContaining({ + port: 465, + secure: true, + requireTLS: false, + })); + }); + + it('secure=false connects in the clear (opportunistic STARTTLS only)', async () => { + await new SmtpTransport({ host: 'smtp.x', port: 25, secure: false }).send(MSG); + expect(nm.createTransport).toHaveBeenCalledWith(expect.objectContaining({ + port: 25, + secure: false, + requireTLS: false, + })); + }); + + it('sends auth only when a user is configured', async () => { + await new SmtpTransport({ host: 'smtp.x' }).send(MSG); + expect(nm.createTransport.mock.calls[0][0]).not.toHaveProperty('auth'); + + nm.createTransport.mockClear(); + await new SmtpTransport({ host: 'smtp.x', user: 'u@x', password: 'p' }).send(MSG); + expect(nm.createTransport).toHaveBeenCalledWith(expect.objectContaining({ + auth: { user: 'u@x', pass: 'p' }, + })); + }); + + it('lets transportOptions override the derived options (escape hatch)', async () => { + await new SmtpTransport({ + host: 'smtp.x', + port: 2525, + transportOptions: { secure: true, pool: true }, + }).send(MSG); + expect(nm.createTransport).toHaveBeenCalledWith(expect.objectContaining({ + port: 2525, + secure: true, + pool: true, + })); + }); + + it('describe() reports the connection without the password', () => { + const d = new SmtpTransport({ host: 'smtp.x', user: 'u@x', password: 'sekrit' }).describe(); + expect(d).toMatchObject({ host: 'smtp.x', port: 587, auth: { user: 'u@x' } }); + expect(JSON.stringify(d)).not.toContain('sekrit'); + }); +}); + +describe('SmtpTransport — send', () => { + it('maps every NormalizedEmailMessage field onto the nodemailer envelope', async () => { + await new SmtpTransport({ host: 'smtp.x' }).send({ + to: ['a@x.test', 'b@x.test'], + from: 'From ', + cc: ['c@x.test'], + bcc: ['d@x.test'], + replyTo: 'r@x.test', + subject: 'S', + text: 'T', + html: '

H

', + headers: { 'X-Trace': '42' }, + attachments: [{ filename: 'a.txt', content: 'body', contentType: 'text/plain', cid: 'cid1' }], + }); + expect(nm.sendMail).toHaveBeenCalledWith({ + from: 'From ', + to: ['a@x.test', 'b@x.test'], + cc: ['c@x.test'], + bcc: ['d@x.test'], + replyTo: 'r@x.test', + subject: 'S', + text: 'T', + html: '

H

', + headers: { 'X-Trace': '42' }, + attachments: [{ filename: 'a.txt', content: 'body', contentType: 'text/plain', cid: 'cid1' }], + }); + }); + + it('surfaces the SMTP server error verbatim (authentication failure is LOUD)', async () => { + nm.sendMail.mockRejectedValueOnce( + new Error('Invalid login: 535 Error: authentication failed'), + ); + await expect(new SmtpTransport({ host: 'smtp.x' }).send(MSG)) + .rejects.toThrow(/535 Error: authentication failed/); + }); + + it('does not cache a failed transporter construction', async () => { + nm.createTransport.mockImplementationOnce(() => { throw new Error('bad config'); }); + const t = new SmtpTransport({ host: 'smtp.x' }); + await expect(t.send(MSG)).rejects.toThrow(/bad config/); + // A settings fix must take effect on the next send, not stick to the error. + await expect(t.send(MSG)).resolves.toMatchObject({ messageId: '' }); + }); + + it('rejects a send the server accepted without a Message-ID', async () => { + nm.sendMail.mockResolvedValueOnce({ response: '250 ok' }); + await expect(new SmtpTransport({ host: 'smtp.x' }).send(MSG)) + .rejects.toThrow(/no Message-ID/); + }); + + it('returns the transport result', async () => { + const res = await new SmtpTransport({ host: 'smtp.x' }).send(MSG); + expect(res).toEqual({ messageId: '', response: '250 2.0.0 Ok: queued' }); + }); +}); + +describe('makeTransport(provider="smtp")', () => { + it('builds an SmtpTransport from providerOptions', () => { + const t = makeTransport({ provider: 'smtp', options: { host: 'smtp.x', port: 465 } }); + expect(t).toBeInstanceOf(SmtpTransport); + }); + + it('throws instead of degrading to LogTransport when the host is missing', () => { + expect(() => makeTransport({ provider: 'smtp', options: {} })) + .toThrow(/requires a host/); + expect(() => makeTransport({ provider: 'smtp' })).toThrow(/requires a host/); + }); +}); + +describe('smtpOptionsFromMailSettings', () => { + it('maps the mail namespace keys onto the transport options', () => { + expect(smtpOptionsFromMailSettings({ + smtp_host: 'smtp.163.com', + smtp_port: 465, + smtp_secure: true, + smtp_user: 'ops@163.com', + smtp_password: 'pw', + provider: 'smtp', + from_email: 'ops@163.com', + })).toEqual({ + host: 'smtp.163.com', + port: 465, + secure: true, + user: 'ops@163.com', + password: 'pw', + }); + }); + + it('coerces the string forms that arrive through the OS_MAIL_* env door', () => { + expect(smtpOptionsFromMailSettings({ smtp_host: ' smtp.x ', smtp_port: '2525', smtp_secure: 'false' })) + .toEqual({ host: 'smtp.x', port: 2525, secure: false }); + expect(smtpOptionsFromMailSettings({ smtp_host: 'smtp.x', smtp_secure: '0' }).secure).toBe(false); + expect(smtpOptionsFromMailSettings({ smtp_host: 'smtp.x', smtp_secure: 'true' }).secure).toBe(true); + }); + + it('reports an unset host as empty rather than inventing one', () => { + expect(smtpOptionsFromMailSettings({}).host).toBe(''); + expect(smtpOptionsFromMailSettings({ smtp_host: ' ' }).host).toBe(''); + expect(smtpOptionsFromMailSettings({ smtp_host: 'smtp.x' })).toEqual({ host: 'smtp.x' }); + }); +}); diff --git a/packages/plugins/plugin-email/src/transports/smtp.ts b/packages/plugins/plugin-email/src/transports/smtp.ts new file mode 100644 index 0000000000..2e56f6490f --- /dev/null +++ b/packages/plugins/plugin-email/src/transports/smtp.ts @@ -0,0 +1,282 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import type { + IEmailTransport, + NormalizedEmailMessage, + TransportSendResult, +} from '@objectstack/spec/contracts'; + +/** + * SmtpTransport — self-hosted / provider SMTP delivery via nodemailer. + * + * ADR-0012 ("Email transport sub-system"): SMTP ships **in core** and is + * non-negotiable for on-prem, air-gapped and China-region deployments — + * Resend / Postmark are HTTPS SaaS and are neither reachable nor reliable + * for QQ / 163 / enterprise mailboxes. That decision also names the + * implementation: `nodemailer`, not a hand-rolled client. A partial SMTP + * client works against permissive servers and fails *silently* against the + * strict ones (STARTTLS negotiation, AUTH LOGIN/PLAIN/CRAM-MD5, RFC 2047 + * header encoding, dot-stuffing), which would push this transport's own + * bug one layer down instead of fixing it. Do not replace it with a + * bespoke implementation without a superseding ADR. + * + * `nodemailer` is a hard dependency of this package but is imported + * **lazily**, on the first `send()`. A deployment that never selects SMTP + * (and any non-Node runtime — Workers / edge) therefore never loads + * `node:net` / `node:tls`. + * + * TLS mapping — one toggle (`secure`, the settings page's "Use TLS"), the + * wire behaviour derived from the port, the way every mail provider + * documents it: + * + * | `secure` | port | connection | + * |:---------|:--------|:--------------------------------------------------| + * | `true` | `465` | implicit TLS (SMTPS) from the first byte | + * | `true` | other | plain connect + **required** STARTTLS upgrade | + * | `false` | any | plain connect, opportunistic STARTTLS when offered | + * + * "Required" means loud: a server that will not upgrade makes the send + * fail rather than leaking credentials over a cleartext socket. + * + * @example + * ```ts + * new EmailServicePlugin({ + * provider: 'smtp', + * providerOptions: { + * host: 'smtp.exmail.qq.com', + * port: 465, + * secure: true, + * user: 'no-reply@acme.cn', + * password: process.env.SMTP_PASSWORD, + * }, + * defaultFrom: { name: 'Acme', address: 'no-reply@acme.cn' }, + * }); + * ``` + */ +export interface SmtpTransportOptions { + /** SMTP server hostname, e.g. `smtp.exmail.qq.com`. Required. */ + host: string; + /** SMTP port. Default 587 (submission). 465 selects implicit TLS. */ + port?: number; + /** + * Require TLS. Default `true`. Implicit TLS on port 465, otherwise a + * REQUIRED STARTTLS upgrade. `false` connects in the clear and upgrades + * only when the server offers STARTTLS. + */ + secure?: boolean; + /** SMTP AUTH username. Omit for servers that accept unauthenticated relay. */ + user?: string; + /** SMTP AUTH password. Ignored when `user` is empty. */ + password?: string; + /** Socket / greeting / connection timeout in ms. Default 20000. */ + timeout?: number; + /** + * Escape hatch: raw nodemailer SMTP options merged **last** (e.g. + * `{ pool: true }`, `{ tls: { rejectUnauthorized: false } }`, or forcing + * `{ secure: true }` on a non-standard port). Use sparingly — anything + * routinely needed belongs on this interface. + */ + transportOptions?: Record; + /** Diagnostic logger. */ + logger?: { info: (msg: string, meta?: any) => void; warn?: (msg: string, meta?: any) => void }; +} + +/** Minimal structural view of the nodemailer surface this transport uses. */ +interface NodemailerLike { + createTransport(options: Record): NodemailerTransporterLike; +} +interface NodemailerTransporterLike { + sendMail(mail: Record): Promise<{ messageId?: string; response?: string; rejected?: unknown[] }>; + close?(): void; +} + +const DEFAULT_PORT = 587; +const IMPLICIT_TLS_PORT = 465; +const DEFAULT_TIMEOUT_MS = 20_000; + +export class SmtpTransport implements IEmailTransport { + private readonly opts: SmtpTransportOptions; + private readonly port: number; + private transporter?: Promise; + + constructor(opts: SmtpTransportOptions) { + if (!opts?.host || !String(opts.host).trim()) { + // Loud at construction: a "configured" SMTP provider with no host is + // exactly the declared-but-not-delivered state this transport exists + // to end (#5087). Never degrade to a transport that pretends to send. + throw new Error( + 'SmtpTransport: host is required (Settings → Mail → Host, or OS_EMAIL_SMTP_HOST)', + ); + } + this.opts = opts; + const port = Number(opts.port ?? DEFAULT_PORT); + if (!Number.isFinite(port) || port < 1 || port > 65535) { + throw new Error(`SmtpTransport: invalid port '${String(opts.port)}' (expected 1-65535)`); + } + this.port = port; + } + + /** + * The nodemailer SMTP options this transport connects with. Exposed for + * diagnostics and tests — the password is never included. + */ + describe(): Record { + const { auth, ...rest } = this.buildSmtpOptions(); + return { ...rest, auth: auth ? { user: (auth as any).user } : undefined }; + } + + private buildSmtpOptions(): Record { + const secure = this.opts.secure !== false; + const implicitTls = secure && this.port === IMPLICIT_TLS_PORT; + const timeout = this.opts.timeout ?? DEFAULT_TIMEOUT_MS; + const user = this.opts.user?.trim(); + return { + host: String(this.opts.host).trim(), + port: this.port, + secure: implicitTls, + // STARTTLS is REQUIRED (not merely offered) whenever TLS is on and the + // port is not the implicit-TLS one — a server that refuses to upgrade + // must fail the send, not silently downgrade to cleartext AUTH. + requireTLS: secure && !implicitTls, + connectionTimeout: timeout, + greetingTimeout: timeout, + socketTimeout: timeout, + ...(user ? { auth: { user, pass: this.opts.password ?? '' } } : {}), + ...(this.opts.transportOptions ?? {}), + }; + } + + private async loadNodemailer(): Promise { + let mod: any; + try { + mod = await import('nodemailer'); + } catch (err: any) { + throw new Error( + 'SmtpTransport: failed to load `nodemailer` — SMTP delivery is unavailable. ' + + 'It is a dependency of @objectstack/plugin-email; reinstall it (`pnpm add nodemailer`) ' + + 'or select a non-SMTP provider. Cause: ' + + (err?.message ?? String(err)), + ); + } + // nodemailer is CommonJS: under ESM the named export may arrive on the + // namespace or only on `default` depending on the loader. This is a + // module-format interop boundary with a third-party package, not a + // tolerated dialect of our own contract. + const createTransport = mod?.createTransport ?? mod?.default?.createTransport; + if (typeof createTransport !== 'function') { + throw new Error('SmtpTransport: `nodemailer` loaded but exposes no createTransport()'); + } + return { createTransport } as NodemailerLike; + } + + private async getTransporter(): Promise { + if (!this.transporter) { + this.transporter = (async () => { + const nodemailer = await this.loadNodemailer(); + const smtpOptions = this.buildSmtpOptions(); + this.opts.logger?.info?.( + `SmtpTransport: connecting to ${smtpOptions.host}:${smtpOptions.port} ` + + `(implicitTls=${smtpOptions.secure}, requireStartTls=${smtpOptions.requireTLS}, ` + + `auth=${smtpOptions.auth ? 'yes' : 'no'})`, + ); + return nodemailer.createTransport(smtpOptions); + })().catch((err) => { + // Do not cache a failed construction — a settings fix should be + // retried on the next send rather than sticking to the first error. + this.transporter = undefined; + throw err; + }); + } + return this.transporter; + } + + async send(message: NormalizedEmailMessage): Promise { + const transporter = await this.getTransporter(); + // nodemailer owns MIME construction: RFC 2047 header encoding (so a + // Chinese subject survives), quoted-printable / base64 bodies, multipart + // alternative for text+html, and dot-stuffing. + const mail: Record = { + from: message.from, + to: message.to, + subject: message.subject, + }; + if (message.text !== undefined) mail.text = message.text; + if (message.html !== undefined) mail.html = message.html; + if (message.cc?.length) mail.cc = message.cc; + if (message.bcc?.length) mail.bcc = message.bcc; + if (message.replyTo) mail.replyTo = message.replyTo; + if (message.headers && Object.keys(message.headers).length > 0) mail.headers = message.headers; + if (message.attachments?.length) { + mail.attachments = message.attachments.map((a) => ({ + filename: a.filename, + content: a.content, + ...(a.contentType ? { contentType: a.contentType } : {}), + ...(a.cid ? { cid: a.cid } : {}), + })); + } + + const info = await transporter.sendMail(mail); + const messageId = String(info?.messageId ?? ''); + if (!messageId) { + throw new Error('SMTP: server accepted the message but returned no Message-ID'); + } + return { + messageId, + response: info?.response ? String(info.response) : 'smtp:ok', + }; + } + + /** Release the underlying transporter (pooled connections). Best effort. */ + async close(): Promise { + const pending = this.transporter; + this.transporter = undefined; + if (!pending) return; + try { + const transporter = await pending; + transporter.close?.(); + } catch { + /* already failed to build — nothing to close */ + } + } +} + +/** + * Translate a `mail` settings-namespace snapshot into {@link SmtpTransportOptions}. + * + * The settings manifest owns its key names (`smtp_host` / `smtp_port` / + * `smtp_secure` / `smtp_user` / `smtp_password`); this transport owns its + * own (`host` / `port` / `secure` / …). Exactly ONE conversion between the + * two lives here, and every settings door goes through it — the hot-swap on + * `settings:changed` and the `mail/test` action — so the two cannot drift + * into different readings of "Use TLS". The transport itself never learns + * the `smtp_*` vocabulary. + * + * Values arrive typed from the resolver but may be strings when supplied via + * `OS_MAIL_SMTP_*` env — coerced here, at that boundary, and nowhere else. + * `host` comes back `''` when unset; the caller decides how loud that is. + */ +export function smtpOptionsFromMailSettings(values: Record): SmtpTransportOptions { + const str = (v: unknown): string | undefined => { + if (v == null) return undefined; + const t = String(v).trim(); + return t === '' ? undefined : t; + }; + const port = values.smtp_port == null || values.smtp_port === '' + ? undefined + : Number(values.smtp_port); + const rawSecure = values.smtp_secure; + const secure = rawSecure == null + ? undefined + : typeof rawSecure === 'string' + ? !(rawSecure.trim().toLowerCase() === 'false' || rawSecure.trim() === '0') + : Boolean(rawSecure); + const user = str(values.smtp_user); + const password = str(values.smtp_password); + return { + host: str(values.smtp_host) ?? '', + ...(port != null && Number.isFinite(port) ? { port } : {}), + ...(secure != null ? { secure } : {}), + ...(user ? { user } : {}), + ...(password ? { password } : {}), + }; +} diff --git a/packages/plugins/plugin-email/src/transports/smtp.wire.test.ts b/packages/plugins/plugin-email/src/transports/smtp.wire.test.ts new file mode 100644 index 0000000000..2b0551ee64 --- /dev/null +++ b/packages/plugins/plugin-email/src/transports/smtp.wire.test.ts @@ -0,0 +1,221 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +// +// SmtpTransport against a REAL nodemailer talking to an in-process fake SMTP +// server (#5087). No network leaves the box and no mock stands between the +// transport and the wire, so this is the only place that can prove: +// +// * the SMTP conversation actually happens (EHLO / AUTH / MAIL / RCPT / DATA); +// * a Chinese subject is RFC 2047 encoded and a Chinese HTML body survives +// the transfer encoding — the encoding a hand-rolled SMTP client gets +// wrong quietly, which is why ADR-0012 says nodemailer; +// * an AUTH rejection (535) reaches the caller instead of being swallowed. + +import { describe, it, expect, afterEach } from 'vitest'; +import net from 'node:net'; +import { SmtpTransport } from './smtp.js'; + +interface FakeSmtp { + port: number; + /** Commands the client sent, uppercased verb + raw line. */ + commands: string[]; + /** Raw DATA payloads (headers + body), one per delivered message. */ + messages: string[]; + close(): Promise; +} + +/** + * Minimal ESMTP server: greeting, EHLO, AUTH PLAIN/LOGIN, MAIL/RCPT/DATA/QUIT. + * Deliberately does NOT advertise STARTTLS — the tests connect with + * `secure: false` so nodemailer stays in the clear against localhost. + */ +async function startFakeSmtp(opts: { authOk?: boolean } = {}): Promise { + const authOk = opts.authOk !== false; + const commands: string[] = []; + const messages: string[] = []; + + const server = net.createServer((socket) => { + let buffer = ''; + let inData = false; + let dataBuf = ''; + socket.setEncoding('utf8'); + socket.write('220 fake.smtp.test ESMTP ready\r\n'); + + socket.on('data', (chunk: string) => { + if (inData) { + dataBuf += chunk; + const end = dataBuf.indexOf('\r\n.\r\n'); + if (end === -1) return; + messages.push(dataBuf.slice(0, end)); + dataBuf = ''; + inData = false; + socket.write('250 2.0.0 Ok: queued as FAKE123\r\n'); + return; + } + buffer += chunk; + let idx: number; + while ((idx = buffer.indexOf('\r\n')) !== -1) { + const line = buffer.slice(0, idx); + buffer = buffer.slice(idx + 2); + commands.push(line); + const verb = line.split(/[ :]/)[0].toUpperCase(); + if (verb === 'EHLO' || verb === 'HELO') { + socket.write('250-fake.smtp.test\r\n250-AUTH PLAIN LOGIN\r\n250 SMTPUTF8\r\n'); + } else if (verb === 'AUTH') { + if (/LOGIN/i.test(line)) { + // LOGIN is a 3-step challenge; accept/deny at the end. + socket.write('334 VXNlcm5hbWU6\r\n'); + } else { + socket.write(authOk ? '235 2.7.0 Accepted\r\n' : '535 5.7.8 Error: authentication failed\r\n'); + } + } else if (/^[A-Za-z0-9+/=]+$/.test(line) && commands.some((c) => /^AUTH LOGIN/i.test(c))) { + // base64 continuation of AUTH LOGIN (username, then password) + const step = commands.filter((c) => /^[A-Za-z0-9+/=]+$/.test(c)).length; + if (step === 1) socket.write('334 UGFzc3dvcmQ6\r\n'); + else socket.write(authOk ? '235 2.7.0 Accepted\r\n' : '535 5.7.8 Error: authentication failed\r\n'); + } else if (verb === 'MAIL' || verb === 'RCPT') { + socket.write('250 2.1.0 Ok\r\n'); + } else if (verb === 'DATA') { + inData = true; + socket.write('354 End data with .\r\n'); + } else if (verb === 'QUIT') { + socket.write('221 2.0.0 Bye\r\n'); + socket.end(); + } else { + socket.write('250 2.0.0 Ok\r\n'); + } + } + }); + socket.on('error', () => { /* client hung up */ }); + }); + + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const port = (server.address() as net.AddressInfo).port; + return { + port, + commands, + messages, + close: () => new Promise((resolve) => server.close(() => resolve())), + }; +} + +/** Decode an RFC 2047 encoded-word run (`=?UTF-8?B?..?=` / `?Q?..?=`). */ +function decodeEncodedWords(input: string): string { + return input.replace(/=\?utf-8\?(b|q)\?([^?]+)\?=/gi, (_m, enc: string, payload: string) => { + if (enc.toLowerCase() === 'b') return Buffer.from(payload, 'base64').toString('utf8'); + const bytes = payload + .replace(/_/g, ' ') + .replace(/=([0-9A-Fa-f]{2})/g, (_x, hex: string) => String.fromCharCode(parseInt(hex, 16))); + return Buffer.from(bytes, 'binary').toString('utf8'); + }); +} + +/** Decode every transfer-encoded body part so the text can be asserted. */ +function decodeBodies(raw: string): string { + const out: string[] = [raw]; + // quoted-printable + out.push(Buffer.from( + raw.replace(/=\r?\n/g, '').replace(/=([0-9A-Fa-f]{2})/g, (_m, hex: string) => String.fromCharCode(parseInt(hex, 16))), + 'binary', + ).toString('utf8')); + // base64 blocks (4+ full base64 lines in a row) + for (const block of raw.match(/(?:^[A-Za-z0-9+/=]{20,}\r?\n?){1,}/gm) ?? []) { + out.push(Buffer.from(block.replace(/\s+/g, ''), 'base64').toString('utf8')); + } + return out.join('\n'); +} + +let server: FakeSmtp | undefined; +afterEach(async () => { + await server?.close(); + server = undefined; +}); + +describe('SmtpTransport over a real SMTP conversation', () => { + it('delivers through EHLO/AUTH/MAIL/RCPT/DATA and returns the server response', async () => { + server = await startFakeSmtp(); + const transport = new SmtpTransport({ + host: '127.0.0.1', + port: server.port, + secure: false, + user: 'ops@example.test', + password: 'sekrit', + timeout: 5_000, + }); + + const res = await transport.send({ + to: ['rcpt@example.test'], + from: 'ObjectStack ', + subject: 'Hello', + text: 'plain', + }); + await transport.close(); + + expect(res.messageId).toMatch(/@/); + expect(res.response).toContain('250'); + expect(server.commands.some((c) => /^EHLO /i.test(c))).toBe(true); + expect(server.commands.some((c) => /^AUTH /i.test(c))).toBe(true); + expect(server.commands.some((c) => /^MAIL FROM:/i.test(c))).toBe(true); + expect(server.commands.some((c) => /^RCPT TO:/i.test(c))).toBe(true); + expect(server.messages).toHaveLength(1); + }, 20_000); + + it('encodes a Chinese subject (RFC 2047) and a Chinese HTML body', async () => { + server = await startFakeSmtp(); + const transport = new SmtpTransport({ + host: '127.0.0.1', + port: server.port, + secure: false, + timeout: 5_000, + }); + + await transport.send({ + to: ['收件人 '], + from: 'ObjectStack 通知 ', + subject: '【ObjectStack】您的验证码', + html: '

你好,世界 — 这是一封测试邮件。

', + text: '你好,世界 — 这是一封测试邮件。', + }); + await transport.close(); + + const raw = server.messages[0]; + expect(raw).toBeTruthy(); + // Unfold first: a long encoded subject is split across continuation + // lines, and adjacent encoded-words rejoin without whitespace. + const headerLines = raw.replace(/\r\n[ \t]+/g, '').split('\r\n'); + + // The header must not carry raw non-ASCII bytes... + const subjectLine = headerLines.find((l) => l.startsWith('Subject:'))!; + expect(subjectLine).not.toContain('您的验证码'); + // ...and must decode back to exactly what was sent. + expect(decodeEncodedWords(subjectLine.replace(/^Subject:\s*/, ''))) + .toContain('【ObjectStack】您的验证码'); + // Display names travel as encoded words too. + expect(decodeEncodedWords(headerLines.find((l) => l.startsWith('To:'))!)).toContain('收件人'); + + const decoded = decodeBodies(raw); + expect(decoded).toContain('你好,世界'); + expect(decoded).toContain('

你好,世界 — 这是一封测试邮件。

'); + expect(raw.toLowerCase()).toContain('charset=utf-8'); + }, 20_000); + + it('fails loudly when the server rejects the credentials', async () => { + server = await startFakeSmtp({ authOk: false }); + const transport = new SmtpTransport({ + host: '127.0.0.1', + port: server.port, + secure: false, + user: 'ops@example.test', + password: 'wrong', + timeout: 5_000, + }); + + await expect(transport.send({ + to: ['rcpt@example.test'], + from: 'no-reply@example.test', + subject: 'Hello', + text: 'plain', + })).rejects.toThrow(/535|[Ii]nvalid login|authentication/); + await transport.close(); + expect(server.messages).toHaveLength(0); + }, 20_000); +}); diff --git a/packages/services/service-settings/src/manifests/mail.manifest.ts b/packages/services/service-settings/src/manifests/mail.manifest.ts index f772e120e8..3fe651f384 100644 --- a/packages/services/service-settings/src/manifests/mail.manifest.ts +++ b/packages/services/service-settings/src/manifests/mail.manifest.ts @@ -61,7 +61,21 @@ const manifest = { /** Mail Delivery — SMTP / API provider configuration. */ export const mailSettingsManifest = manifest as unknown as SettingsManifest; -/** Built-in action handler stub for `mail/test`. */ +/** + * Built-in FALLBACK handler for `mail/test`. + * + * The real one lives in `@objectstack/plugin-email`, which overrides this + * via `registerAction` on `kernel:ready` and actually delivers a message + * through the configured transport (same pattern as `storage/test`). This + * fallback therefore runs only where no email plugin is mounted — it can + * check the form, and it cannot send anything. + * + * So it reports `ok: false`. It previously answered `ok: true` with + * "Configuration looks valid … Wire @objectstack/plugin-mail for actual + * delivery": a success toast for a mail nobody sent, naming a package that + * has never existed. An action button that says "Send test email" must + * never report success for a send that did not happen (framework#5087). + */ export const mailTestActionHandler: SettingsActionHandler = async ({ values }) => { const provider = String(values.provider ?? 'smtp'); const fromEmail = values.from_email as string | undefined; @@ -75,8 +89,9 @@ export const mailTestActionHandler: SettingsActionHandler = async ({ values }) = return { ok: false, severity: 'error', message: 'API key is required.' }; } return { - ok: true, - severity: 'info', - message: `Configuration looks valid (provider=${provider}). Wire @objectstack/plugin-mail for actual delivery.`, + ok: false, + severity: 'warning', + message: `No email service is mounted, so NO test message was sent (the form itself is well-formed, provider=${provider}). ` + + 'Add the "email" capability (@objectstack/plugin-email) to deliver mail and to make this button send a real test.', }; }; diff --git a/packages/services/service-settings/src/settings-service.test.ts b/packages/services/service-settings/src/settings-service.test.ts index a054fe2b12..8a5c72e1ee 100644 --- a/packages/services/service-settings/src/settings-service.test.ts +++ b/packages/services/service-settings/src/settings-service.test.ts @@ -235,7 +235,27 @@ describe('SettingsService — runAction', () => { svc.registerAction('mail', 'test', mailTestActionHandler); await svc.setMany('mail', { provider: 'smtp', smtp_host: 'smtp.x', from_email: 'a@b.com' }); const r = await svc.runAction('mail', 'test', null); - expect(r.ok).toBe(true); + // The handler ran and read the saved values (it echoes the provider) — + // but this built-in is the FALLBACK, mounted only when no email plugin + // is present, so it cannot send and must not claim it did (#5087). The + // real sending handler is registered by @objectstack/plugin-email. + expect(r.ok).toBe(false); + expect(r.severity).toBe('warning'); + expect(r.message).toContain('provider=smtp'); + expect(r.message).toMatch(/NO test message was sent/); + }); + + it('the fallback mail/test handler still rejects an incomplete config', async () => { + // `setMany` already refuses to SAVE provider=smtp without a host, so the + // incomplete state can only arrive through the env door — which is exactly + // where it must still be caught. + const svc = new SettingsService({ env: { OS_MAIL_PROVIDER: 'smtp' } }); + svc.registerManifest(mailSettingsManifest); + svc.registerAction('mail', 'test', mailTestActionHandler); + await svc.setMany('mail', { from_email: 'a@b.com' }); + const r = await svc.runAction('mail', 'test', null); + expect(r).toMatchObject({ ok: false, severity: 'error' }); + expect(r.message).toContain('SMTP host is required'); }); it('catches handler exceptions', async () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 15c90e3978..d69e1aaec7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1468,10 +1468,16 @@ importers: '@objectstack/spec': specifier: workspace:* version: link:../../spec + nodemailer: + specifier: ^9.0.3 + version: 9.0.3 devDependencies: '@types/node': specifier: ^26.1.2 version: 26.1.2 + '@types/nodemailer': + specifier: ^8.0.1 + version: 8.0.1 typescript: specifier: ^6.0.3 version: 6.0.3 @@ -4635,6 +4641,9 @@ packages: '@types/node@26.1.2': resolution: {integrity: sha512-Vu4a5UFA9rIIFJ7rB/Vaafh9lrCQszopTCx6KjFboXTGQbPNasehVR5TEiithSDGyd1DEiUByggTZsg8jukeIg==} + '@types/nodemailer@8.0.1': + resolution: {integrity: sha512-PxpaInm8V1JQDd4j0ds5HfvWQk8JupS1C0Picb96QJsrrRDjBH+DlK7L4ZdNSqNULhiZRQHc40nLVShaGxXAMw==} + '@types/normalize-package-data@2.4.4': resolution: {integrity: sha512-37i+OaWTh9qeK4LSHPsyRC7NahnGotNuZvjLSgcPzblpHB3rrCJxAOgI5gCdKm7coonsaX1Of0ILiTcnZjbfxA==} @@ -7607,6 +7616,10 @@ packages: resolution: {integrity: sha512-tGnJW6OKRii9u/b2WiUViTJS+h7Apxx17qsMUjsUeNDiMMX5ZFf8F8Fcz7PAQ6omvOxHZtvDTmOYKJQwmfpjeg==} engines: {node: '>=20'} + nodemailer@9.0.3: + resolution: {integrity: sha512-n+YP+NKwR5zRWa60k3GiQ6Q3B4KXCoAw40dAKeCtYn020iNN74aWK2liXIC3ZEATeGql7we3tE3t8QwhY0eskw==} + engines: {node: '>=6.0.0'} + normalize-package-data@6.0.2: resolution: {integrity: sha512-V6gygoYb/5EmNI+MEGrWkC+e6+Rr7mTmfHrxDbLzxQogBkgzo76rkok0Am6thgSF7Mv2nLOajAJj5vDJZEFn7g==} engines: {node: ^16.14.0 || >=18.0.0} @@ -11470,6 +11483,10 @@ snapshots: dependencies: undici-types: 8.3.0 + '@types/nodemailer@8.0.1': + dependencies: + '@types/node': 26.1.2 + '@types/normalize-package-data@2.4.4': {} '@types/react-dom@19.2.4(@types/react@19.2.18)': @@ -14796,6 +14813,8 @@ snapshots: '@types/sarif': 2.1.7 fs-extra: 11.3.6 + nodemailer@9.0.3: {} + normalize-package-data@6.0.2: dependencies: hosted-git-info: 7.0.2