From ed0e0c97374f3db91fc3d97cdd83f5860ccebf00 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 06:30:51 +0000 Subject: [PATCH] fix(cli,plugin-email)!: OS_EMAIL_PROVIDER=resend/postmark with no apiKey fails the boot instead of becoming a LogTransport (#5132) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resolveEmailCapabilityArg` answered a missing API key by rewriting the provider to `log`, printing a warning, and booting: a server that accepted every send, recorded each in `sys_email` as sent, and delivered nothing. #5087 closed that inside plugin-email (`makeTransport` throws rather than substituting a transport); the CLI kept doing it one layer up, which the #5087 PR itself flagged in this function's docstring. It now refuses every mail configuration it cannot deliver through, the way its neighbouring `smtp` arm already did — resend/postmark with no key, and a provider tag outside the supported set (retired `sendgrid`/`ses` get the SMTP migration). Each message names the consequence and both fixes, per AGENTS.md degradation-log-level. Refusing is only fair because `OS_EMAIL_PROVIDER=log` is how a deployment says "no mail from here" — a test pins that it still boots. The provider vocabulary is read from `@objectstack/plugin-email` (`isEmailTransportProvider` / `unsupportedProviderFix` from #5094, plus the new `API_KEY_EMAIL_PROVIDERS` / `emailProviderRequiresApiKey`) rather than restated in the CLI, and the new constant is tied to `makeTransport` by a compile error in one direction and a contract test in the other. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017MCKJaEomEqg4tvz4SzdNd --- ...i-email-provider-missing-api-key-throws.md | 46 ++++++++++ .../docs/deployment/environment-variables.mdx | 4 +- .../commands/serve-email-capability.test.ts | 84 ++++++++++++++----- packages/cli/src/commands/serve.ts | 74 +++++++++++----- packages/plugins/plugin-email/src/index.ts | 3 + .../api-key-providers.contract.test.ts | 71 ++++++++++++++++ .../plugin-email/src/transports/index.ts | 50 ++++++++++- 7 files changed, 285 insertions(+), 47 deletions(-) create mode 100644 .changeset/cli-email-provider-missing-api-key-throws.md create mode 100644 packages/plugins/plugin-email/src/transports/api-key-providers.contract.test.ts diff --git a/.changeset/cli-email-provider-missing-api-key-throws.md b/.changeset/cli-email-provider-missing-api-key-throws.md new file mode 100644 index 0000000000..63b4c07067 --- /dev/null +++ b/.changeset/cli-email-provider-missing-api-key-throws.md @@ -0,0 +1,46 @@ +--- +"@objectstack/plugin-email": minor +"@objectstack/cli": major +--- + +fix(cli,plugin-email)!: `OS_EMAIL_PROVIDER=resend/postmark` without an API key now fails the boot instead of silently becoming the log transport (#5132) + +**BREAKING for one configuration: a delivery provider selected without the +credential it needs.** `os serve` used to answer that by rewriting `provider` to +`log`, printing a warning, and booting normally. The result was a server that +accepted every send, recorded each one in `sys_email` as sent, and delivered +nothing — the warning scrolled past in CI logs and the truth surfaced when a +user reported never receiving a verification code. #5087 closed exactly this gap +inside `@objectstack/plugin-email` (`makeTransport` throws rather than +substituting a transport); the CLI's own capability assembly kept doing it one +layer up, for `resend` / `postmark`. + +`resolveEmailCapabilityArg` now refuses every mail configuration it cannot +deliver through, the way its neighbouring `smtp` arm already did: + +- `resend` / `postmark` with no `OS_EMAIL_API_KEY` (or `config.email.apiKey`); +- a `provider` tag outside `log` / `smtp` / `resend` / `postmark` — including + the retired `sendgrid` / `ses`, which get their SMTP migration in the message. + +**Who is affected:** deployments (typically CI or preview environments) that set +`OS_EMAIL_PROVIDER=resend` or `=postmark` without a key and relied on the +fallback to boot. Nothing else changes — a complete configuration is passed +through untouched, and an unset `OS_EMAIL_PROVIDER` still defaults to `log`. + +**Migration — one line, either direction:** + +- the environment is *not* meant to send mail → `OS_EMAIL_PROVIDER=log` + (that explicit value is the supported way to say so, and why refusing the + others is fair); +- the environment *is* meant to send mail → set `OS_EMAIL_API_KEY` (or + `config.email.apiKey`). + +Both errors name the consequence and both fixes, per AGENTS.md's +degradation-log-level rule. + +`@objectstack/plugin-email` gains the vocabulary the CLI reads instead of +restating: `API_KEY_EMAIL_PROVIDERS`, `emailProviderRequiresApiKey()` and the +`ApiKeyEmailProvider` type, alongside `EMAIL_TRANSPORT_PROVIDERS` / +`isEmailTransportProvider` / `unsupportedProviderFix` from #5094. One vocabulary, +two consumers, pinned by a contract test — a second literal list in the CLI is +how the settings dropdown and the transports drifted apart in the first place. diff --git a/content/docs/deployment/environment-variables.mdx b/content/docs/deployment/environment-variables.mdx index 26ef83dc33..dafc8c08a4 100644 --- a/content/docs/deployment/environment-variables.mdx +++ b/content/docs/deployment/environment-variables.mdx @@ -124,8 +124,8 @@ Auth settings precedence: | Variable | Type | Default | Description | |:---|:---|:---|:---| -| `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_PROVIDER` | enum | `log` | Transport. `log` \| `smtp` \| `resend` \| `postmark`. `log` (default) prints to stdout without sending — it is also how a deployment *declares* that it does not send mail. Any other value is a delivery intent, and a boot that cannot honour it fails loudly instead of substituting the log transport. | +| `OS_EMAIL_API_KEY` | string | — | API key for `resend` / `postmark`. **Required** when either is selected — a boot without it fails rather than starting with a transport that records every message in `sys_email` as sent and delivers nothing. Set `OS_EMAIL_PROVIDER=log` for environments that should not send mail. | | `OS_EMAIL_FROM` | email | — | Default `From:` address. | | `OS_EMAIL_RETRIES` | number | `0` | Retry count for transient send failures (`0` = no retry). | | `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. | diff --git a/packages/cli/src/commands/serve-email-capability.test.ts b/packages/cli/src/commands/serve-email-capability.test.ts index 2b7fb4c948..caf45134d7 100644 --- a/packages/cli/src/commands/serve-email-capability.test.ts +++ b/packages/cli/src/commands/serve-email-capability.test.ts @@ -1,16 +1,21 @@ // 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. + * framework#5087 / #5132 — what `EmailServicePlugin` is constructed with on the + * `os serve` path, and what happens when the configuration cannot deliver. * * `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. + * #5087 closed that for `smtp`; the same arm went on doing it to `resend` / + * `postmark` until #5132. + * + * These pin the invariant in one piece: a complete configuration reaches the + * plugin unchanged, and an incomplete one throws — for every provider, not + * just SMTP. The counterpart the throw depends on is pinned too: an operator + * who does not want mail sent says so with `OS_EMAIL_PROVIDER=log`, and that + * still boots. */ import { describe, it, expect } from 'vitest'; @@ -18,14 +23,24 @@ import { resolveEmailCapabilityArg } from './serve.js'; describe('resolveEmailCapabilityArg', () => { it('defaults to the log provider when nothing is configured', () => { - const { options, warning } = resolveEmailCapabilityArg({}, {}); + const { options } = resolveEmailCapabilityArg({}, {}); expect(options).toMatchObject({ provider: 'log' }); expect(options).not.toHaveProperty('providerOptions'); - expect(warning).toBeUndefined(); + }); + + it('boots on an EXPLICIT provider=log — the way to say "this environment does not send mail"', () => { + // The premise of every throw below: refusing an undeliverable provider is + // only fair because "no mail from here" has its own spelling. If this ever + // stops booting, the errors elsewhere in this file stop being actionable. + expect(() => resolveEmailCapabilityArg({}, { OS_EMAIL_PROVIDER: 'log' })).not.toThrow(); + expect(resolveEmailCapabilityArg({}, { OS_EMAIL_PROVIDER: 'log' }).options) + .toMatchObject({ provider: 'log' }); + // …including from objectstack.config.ts, and with no API key anywhere. + expect(resolveEmailCapabilityArg({ provider: 'log' }, {}).options).toMatchObject({ provider: 'log' }); }); it('assembles the SMTP connection from OS_EMAIL_SMTP_*', () => { - const { options, warning } = resolveEmailCapabilityArg({}, { + const { options } = resolveEmailCapabilityArg({}, { OS_EMAIL_PROVIDER: 'smtp', OS_EMAIL_SMTP_HOST: ' smtp.exmail.qq.com ', OS_EMAIL_SMTP_PORT: '465', @@ -34,7 +49,6 @@ describe('resolveEmailCapabilityArg', () => { OS_EMAIL_SMTP_PASSWORD: 'sekrit', OS_EMAIL_FROM: 'Acme ', }); - expect(warning).toBeUndefined(); expect(options).toMatchObject({ provider: 'smtp', providerOptions: { @@ -80,25 +94,53 @@ describe('resolveEmailCapabilityArg', () => { .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({}, { + it('never demands an apiKey from smtp — it has no API to key', () => { + const { options } = resolveEmailCapabilityArg({}, { OS_EMAIL_PROVIDER: 'smtp', OS_EMAIL_SMTP_HOST: 'smtp.x', }); expect(options.provider).toBe('smtp'); - expect(warning).toBeUndefined(); + expect(options).not.toHaveProperty('apiKey'); }); - 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(); + it('passes a complete resend/postmark configuration through untouched', () => { + const resend = resolveEmailCapabilityArg({}, { OS_EMAIL_PROVIDER: 'resend', OS_EMAIL_API_KEY: 're_x' }); + expect(resend.options).toMatchObject({ provider: 'resend', apiKey: 're_x' }); + + // The key may equally come from objectstack.config.ts. + const postmark = resolveEmailCapabilityArg({ provider: 'postmark', apiKey: 'pm_x' }, {}); + expect(postmark.options).toMatchObject({ provider: 'postmark', apiKey: 'pm_x' }); + }); + + it('THROWS on resend/postmark without an apiKey — no silent LogTransport (#5132)', () => { + // This case used to return `{ provider: 'log' }` plus a warning: the server + // booted, `sys_email` filled with rows marked sent, and no mail was ever + // delivered. It is now the same refusal the neighbouring `smtp` arm makes. + for (const provider of ['resend', 'postmark']) { + const boot = () => resolveEmailCapabilityArg({}, { OS_EMAIL_PROVIDER: provider }); + expect(boot, provider).toThrow(new RegExp(`provider='${provider}'`)); + // Consequence AND fix in the one message (AGENTS.md degradation-log-level). + expect(boot, provider).toThrow(/sys_email as sent and nothing would leave the box/); + expect(boot, provider).toThrow(/OS_EMAIL_API_KEY/); + expect(boot, provider).toThrow(/OS_EMAIL_PROVIDER=log/); + // …and never the old silent rewrite. + expect(boot, provider).not.toThrow(/Falling back to LogTransport/); + } + // config.email.provider is the same declaration by another channel. + expect(() => resolveEmailCapabilityArg({ provider: 'resend' }, {})).toThrow(/OS_EMAIL_API_KEY/); + }); - const noKey = resolveEmailCapabilityArg({}, { OS_EMAIL_PROVIDER: 'postmark' }); - expect(noKey.options.provider).toBe('log'); - expect(noKey.warning).toMatch(/no apiKey found/); + it('THROWS on a provider tag no transport can deliver, carrying the migration', () => { + // A stored/typo'd tag used to take the same silent `log` path when no key + // was set. `sendgrid` / `ses` get the SMTP migration from plugin-email + // (#5094), anything else gets the supported list — one vocabulary, not a + // second literal maintained here. + expect(() => resolveEmailCapabilityArg({}, { OS_EMAIL_PROVIDER: 'sendgrid' })) + .toThrow(/smtp\.sendgrid\.net/); + expect(() => resolveEmailCapabilityArg({}, { OS_EMAIL_PROVIDER: 'ses', OS_EMAIL_API_KEY: 'k' })) + .toThrow(/email-smtp\.\.amazonaws\.com/); + expect(() => resolveEmailCapabilityArg({}, { OS_EMAIL_PROVIDER: 'mailgun' })) + .toThrow(/log \/ resend \/ postmark \/ smtp/); }); it('still derives the fallback from-address and template context', () => { diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 13894505b4..2c99940b47 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -17,6 +17,13 @@ import { resolveDriverType, resolveStorageDefinition, UnsupportedDriverError } f import { readEnvWithDeprecation, resolveTenancyPosture, resolveAllowDegradedTenancy, isMcpServerEnabled, stampSearchPinyinEnabled, isModuleNotFoundError } from '@objectstack/types'; import { PLATFORM_CAPABILITY_TOKENS, PLATFORM_ALWAYS_ON_CAPABILITIES } from '@objectstack/spec/kernel'; import { missingProviderMessage } from '../utils/capability-preflight.js'; +// The mail provider vocabulary, read from the package that materialises the +// transports rather than restated here (#5132) — `resolveEmailCapabilityArg` +// has to refuse exactly the configurations `makeTransport` cannot build, and +// two literal lists for one vocabulary is the drift #5094 was filed for. Values +// only (no plugin class): `os serve` loads `EmailServicePlugin` itself through +// the capability loop's dynamic import, host copy first. +import { isEmailTransportProvider, emailProviderRequiresApiKey, unsupportedProviderFix } from '@objectstack/plugin-email'; import { resolveObjectStackHome } from '@objectstack/runtime'; import { LOG_LEVELS, resolveLogLevel, readLogLevelEnv } from '../utils/log-level.js'; import { BootLogCapture, isVerboseBootLevel } from '../utils/boot-log-capture.js'; @@ -2269,13 +2276,15 @@ export default class Serve extends Command { const cubes = (config as any).analyticsCubes ?? (config as any).cubes ?? []; arg = { cubes }; } else if (cap === 'email') { - const emailArg = resolveEmailCapabilityArg( + // Throws on a mail configuration that cannot deliver (#5087, + // #5132) — the catch below turns that into the boot failure / + // loud error it should be, never a LogTransport substituted + // behind the operator's back. + arg = 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}`)); + ).options; } else if (cap === 'sms') { // Compose SmsServicePlugin options from config.sms + OS_SMS_* env // (#2780). Same precedence as email: env beats config. Provider @@ -2812,12 +2821,16 @@ export function resolveStorageCapabilityArg(envRoot?: string): StorageCapability } /** - * Constructor options for `EmailServicePlugin`, plus an optional warning for - * the caller to print (degraded, but still bootable, configurations). + * Constructor options for `EmailServicePlugin`. + * + * There is no `warning` channel here any more (#5132). It carried exactly one + * message — "provider=resend but no apiKey, falling back to LogTransport" — + * and that fallback is now a throw, because a mail configuration that cannot + * deliver has no "degraded but still fine" reading: it is a server that + * accepts every send and delivers nothing. */ export interface EmailCapabilityArg { options: Record; - warning?: string; } /** @@ -2829,12 +2842,26 @@ export interface EmailCapabilityArg { * 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. + * **Every provider that cannot deliver throws** — `smtp` with no host, and + * (since #5132) `resend`/`postmark` with no API key, or a provider tag outside + * `EMAIL_TRANSPORT_PROVIDERS` altogether. The capability loop turns that into a + * loud failure — a hard boot error when the app declared `requires: ['email']`, + * otherwise a `console.error` and no email service — which is the point: the + * alternative (quietly substituting the LogTransport, as this function's + * `resend`/`postmark` arm used to do for a missing API key) hands the operator a + * server that accepts every send, records it in `sys_email` as sent, and + * delivers nothing — the exact declared-but-not-delivered gap #5087 closed + * inside the plugin, left behind one layer up. + * + * Refusing is only defensible because "this environment does not send mail" has + * a way to say itself: `OS_EMAIL_PROVIDER=log` (the default). An operator who + * names a delivery provider has declared an intent, and the honest answer to an + * intent we cannot honour is a failure, not a substitute transport. + * + * The provider vocabulary and the "needs an API key" question are both read + * from `@objectstack/plugin-email` — the package that has to materialise the + * transport — rather than restated here. Two literals describing one vocabulary + * is how the settings dropdown and the transports drifted apart (#5094). */ export function resolveEmailCapabilityArg( cfgEmail: Record = {}, @@ -2891,6 +2918,14 @@ export function resolveEmailCapabilityArg( defaultTemplateContext, }; + if (!isEmailTransportProvider(provider)) { + throw new Error( + `provider='${provider}' is not a transport this server can deliver through, so no mail would go out — ` + + `${unsupportedProviderFix(provider)} ` + + 'On this boot path the provider is OS_EMAIL_PROVIDER or config.email.provider; set ' + + 'OS_EMAIL_PROVIDER=log if this environment is not meant to send mail.', + ); + } if (provider === 'smtp' && !providerOptions.host) { throw new Error( "provider='smtp' selects SMTP delivery but no SMTP host is configured — set OS_EMAIL_SMTP_HOST " @@ -2898,13 +2933,12 @@ export function resolveEmailCapabilityArg( + '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.', - }; + if (emailProviderRequiresApiKey(provider) && !apiKey) { + throw new Error( + `provider='${provider}' selects ${provider} delivery but no API key is configured, so every send would ` + + 'be recorded in sys_email as sent and nothing would leave the box — set OS_EMAIL_API_KEY ' + + '(or config.email.apiKey), or set OS_EMAIL_PROVIDER=log if this environment is not meant to send mail.', + ); } return { options }; } diff --git a/packages/plugins/plugin-email/src/index.ts b/packages/plugins/plugin-email/src/index.ts index 8355f60bbc..4cb7c00fc0 100644 --- a/packages/plugins/plugin-email/src/index.ts +++ b/packages/plugins/plugin-email/src/index.ts @@ -26,8 +26,10 @@ export { makeTransport, smtpOptionsFromMailSettings, EMAIL_TRANSPORT_PROVIDERS, + API_KEY_EMAIL_PROVIDERS, RETIRED_EMAIL_PROVIDERS, isEmailTransportProvider, + emailProviderRequiresApiKey, retiredProviderGuidance, unsupportedProviderFix, type ResendTransportOptions, @@ -35,6 +37,7 @@ export { type SmtpTransportOptions, type MakeTransportOptions, type EmailTransportProvider, + type ApiKeyEmailProvider, } from './transports/index.js'; export { bootstrapDeclaredEmailTemplates, diff --git a/packages/plugins/plugin-email/src/transports/api-key-providers.contract.test.ts b/packages/plugins/plugin-email/src/transports/api-key-providers.contract.test.ts new file mode 100644 index 0000000000..6e7cd4ee9f --- /dev/null +++ b/packages/plugins/plugin-email/src/transports/api-key-providers.contract.test.ts @@ -0,0 +1,71 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// `API_KEY_EMAIL_PROVIDERS` ↔ what `makeTransport` actually refuses (#5132). +// +// The constant exists so the sites that must reject an incomplete mail +// configuration — `makeTransport` here, `resolveEmailCapabilityArg` in +// `@objectstack/cli` — read one vocabulary instead of each restating +// `provider !== 'log' && provider !== 'smtp'`. A constant that says something +// the `switch` does not do would be worse than no constant at all: the CLI +// would refuse a boot this package would have served, or (the #5132 defect) +// wave through one it cannot deliver. +// +// One direction is already a compile error — the `resend`/`postmark` arms pass +// their tag to `requireApiKey(provider: ApiKeyEmailProvider, …)`, so removing a +// tag from the constant breaks the build. This pins the other direction: every +// provider the constant claims needs a key is one `makeTransport` genuinely +// will not build without one, and every provider it does not claim builds fine +// with no key at all. + +import { describe, it, expect } from 'vitest'; +import { + makeTransport, + EMAIL_TRANSPORT_PROVIDERS, + API_KEY_EMAIL_PROVIDERS, + emailProviderRequiresApiKey, +} from './index.js'; + +/** Everything a provider needs EXCEPT an API key. */ +const WITHOUT_KEY: Record[0]> = { + log: { provider: 'log' }, + resend: { provider: 'resend' }, + postmark: { provider: 'postmark' }, + smtp: { provider: 'smtp', options: { host: 'smtp.example.test' } }, +}; + +describe('API_KEY_EMAIL_PROVIDERS ↔ makeTransport', () => { + it('is a subset of the providers this package can materialise', () => { + for (const provider of API_KEY_EMAIL_PROVIDERS) { + expect(EMAIL_TRANSPORT_PROVIDERS).toContain(provider); + } + }); + + it.each([...EMAIL_TRANSPORT_PROVIDERS])( + 'agrees with what makeTransport does for provider=%s without a key', + (provider) => { + const args = WITHOUT_KEY[provider]; + expect(args, `no key-less recipe for provider '${provider}'`).toBeDefined(); + const build = () => makeTransport(args); + + if (emailProviderRequiresApiKey(provider)) { + // Claimed to need a key ⇒ must actually refuse, and say which key. + expect(build).toThrow(/requires apiKey/); + expect(build).toThrow(/OS_EMAIL_API_KEY/); + } else { + // Not claimed ⇒ must build with no key at all, or the CLI would be + // demanding a credential this package never reads. + expect(build).not.toThrow(); + } + }, + ); + + it('narrows only the SaaS-API tags', () => { + expect(emailProviderRequiresApiKey('resend')).toBe(true); + expect(emailProviderRequiresApiKey('postmark')).toBe(true); + expect(emailProviderRequiresApiKey('smtp')).toBe(false); + expect(emailProviderRequiresApiKey('log')).toBe(false); + // Not a provider at all — the caller's next stop is `unsupportedProviderFix`. + expect(emailProviderRequiresApiKey('sendgrid')).toBe(false); + expect(emailProviderRequiresApiKey(undefined)).toBe(false); + }); +}); diff --git a/packages/plugins/plugin-email/src/transports/index.ts b/packages/plugins/plugin-email/src/transports/index.ts index 6e6ea4820c..a18fcf6ccd 100644 --- a/packages/plugins/plugin-email/src/transports/index.ts +++ b/packages/plugins/plugin-email/src/transports/index.ts @@ -37,6 +37,36 @@ export function isEmailTransportProvider(value: unknown): value is EmailTranspor && (EMAIL_TRANSPORT_PROVIDERS as readonly string[]).includes(value); } +/** + * Providers whose transport is an HTTPS SaaS API and therefore cannot be built + * at all without a key — the single source of truth for "this provider needs + * `apiKey`". + * + * It exists as a value because more than one assembly site has to answer that + * question and refuse an incomplete configuration: {@link makeTransport} here, + * and `resolveEmailCapabilityArg` in `@objectstack/cli` (#5132), which used to + * answer it with its own `provider !== 'log' && provider !== 'smtp'` literal + * and silently rewrote the provider to `log` when the key was missing. Two + * literals describing one vocabulary is exactly the drift #5094 was filed for, + * so the CLI imports this instead of restating it. + * + * The tie to {@link makeTransport} is a compile error rather than a convention: + * its `resend` / `postmark` arms pass their tag to `requireApiKey`, whose + * parameter is {@link ApiKeyEmailProvider}, so dropping a tag here stops that + * switch from compiling. The other direction — a tag added here whose arm + * forgets the check — is pinned by `api-key-providers.contract.test.ts`. + */ +export const API_KEY_EMAIL_PROVIDERS = ['resend', 'postmark'] as const satisfies readonly EmailTransportProvider[]; + +/** A provider that cannot be built without an API key. */ +export type ApiKeyEmailProvider = (typeof API_KEY_EMAIL_PROVIDERS)[number]; + +/** Does this provider tag need an `apiKey` before a transport can be built? */ +export function emailProviderRequiresApiKey(value: unknown): value is ApiKeyEmailProvider { + return typeof value === 'string' + && (API_KEY_EMAIL_PROVIDERS as readonly string[]).includes(value); +} + /** * Provider tags the settings page used to offer and this package never * implemented, mapped to the migration that replaces them (#5094). @@ -76,6 +106,20 @@ export function unsupportedProviderFix(provider: string): string { ?? `pick one of ${EMAIL_TRANSPORT_PROVIDERS.join(' / ')} (Settings → Mail → Provider).`; } +/** + * The API key for a provider that has no transport without one, or a throw. + * + * The parameter type is what keeps {@link API_KEY_EMAIL_PROVIDERS} honest: only + * a tag listed there can be passed here, so the constant cannot shrink away + * from the `switch` arms that rely on it without breaking the build. + */ +function requireApiKey(provider: ApiKeyEmailProvider, apiKey: string | undefined): string { + if (!apiKey) { + throw new Error(`makeTransport: provider='${provider}' requires apiKey (OS_EMAIL_API_KEY)`); + } + return apiKey; +} + export interface MakeTransportOptions { provider: EmailTransportProvider; apiKey?: string; @@ -110,11 +154,9 @@ export function makeTransport(opts: MakeTransportOptions): IEmailTransport { case 'log': return new LogTransport(logger); case 'resend': - if (!apiKey) throw new Error("makeTransport: provider='resend' requires apiKey (OS_EMAIL_API_KEY)"); - return new ResendTransport({ apiKey, ...(options as any) }); + return new ResendTransport({ apiKey: requireApiKey(provider, apiKey), ...(options as any) }); case 'postmark': - if (!apiKey) throw new Error("makeTransport: provider='postmark' requires apiKey (OS_EMAIL_API_KEY)"); - return new PostmarkTransport({ apiKey, ...(options as any) }); + return new PostmarkTransport({ apiKey: requireApiKey(provider, apiKey), ...(options as any) }); case 'smtp': { const smtp = options as Partial; if (!smtp?.host) {