feat(core): built-in SMTP email transport for generic SMTP credentials#2156
feat(core): built-in SMTP email transport for generic SMTP credentials#2156swissky wants to merge 34 commits into
Conversation
🦋 Changeset detectedLatest commit: 2826cc7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
review/needs-review
Scope checkThis PR changes 842 lines across 5 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
7059d92 to
b21e3c5
Compare
Adds an email:deliver provider that works with any standard SMTP server (Brevo relay, Office365, Fastmail, Amazon SES, self-hosted Postfix) via raw TCP — the one network primitive sandboxed plugins cannot use. Configuration is env-only (EMAIL_SMTP_HOST/PORT/USER/PASS/FROM). Supports STARTTLS (587) and implicit TLS (465); port 25 is refused because Cloudflare blocks it. TLS is always required. Works on Cloudflare Workers (cloudflare:sockets) and Node (node:net/node:tls). Registered as a built-in plugin so it participates in exclusive hook resolution like any other provider — explicitly selected plugin transports still take precedence. Closes emdash-cms#1541
b21e3c5 to
e142700
Compare
Extends the built-in SMTP email provider to be configurable via Settings → Email in the admin UI, not just environment variables. - DB-backed SMTP config (encrypted with EMDASH_ENCRYPTION_KEY) - Provider selection dropdown (None / SMTP / Cloudflare Email) - PUT /_emdash/api/settings/email endpoint for saving config - SMTP form with host, port, security, user, password, sender - Cloudflare Email option (requires astro.config.mjs + wrangler binding) - Test email button for verifying the full pipeline - Unit tests for DB config save/load/clear/encrypt The password is AES-GCM encrypted with the same key used for plugin secrets. Environment variables remain as a fallback when no DB config is stored.
github-actions
Bot
added
area/admin
and removed
review/needs-review
- Admin Email Settings: provider selection (none/SMTP/Cloudflare), structured sender fields (fromName/fromEmail/replyTo) for both - Built-in Cloudflare Email provider via send_email binding, always registered so it appears in Settings; config from DB or env vars - Explicit "none" selection persists (__none__ sentinel) so exclusive-hook resolution never auto-selects a provider again after restart - Test Binding button checks the send_email binding in the Worker runtime - Fix: strip emdash_enc_v1_ prefix before encrypt()/decrypt() for SMTP password storage (auth helpers expect raw base64url key material)
…ry (emdash-cms#2102) * i18n(fr): translate 138 missing keys + consistency around media library * consistency around media library Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * reword download backup heading --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…-cms#1518) (emdash-cms#1957) Pages with computed titles/descriptions had to drop down to getContentSeo and merge panel-vs-default by hand, because getSeoMeta's only fallbacks were data.title/data.excerpt. The new options rank between the SEO panel value and the data fallback, so editor-set panel values still win.
…mdash-cms#1954) * feat(mcp): add media_upload tool for programmatic media management Adds a media_upload MCP tool that accepts base64-encoded file data or a public URL, runs the same pipeline as the REST upload route (global MIME allowlist, size limit, content-hash dedupe, storage upload, image metadata enrichment), and returns the media item ready to reference from content fields. URL fetches go through ssrfSafeFetch so redirects and private hosts are rejected. Closes emdash-cms#620, closes emdash-cms#1825. * fix(mcp): validate MIME string before allowlist and storage upload The prefix-based allowlist check let a crafted contentType like 'image/png\r\nX-Evil: 1' through to the storage backend's ContentType header (echoed by the media file route). Validate against the existing CONTENT_TYPE_RE first, and tighten the MCP input schema (regex on contentType, .url() on url) so malformed input is rejected before any bytes are buffered or fetched. --------- Co-authored-by: Matt Kane <mkane@cloudflare.com>
…mdash-cms#1079) handleContentList only special-cased a missing table, so a backing ec_{collection} table without the deleted_at column threw "no such column" and fell through to a generic CONTENT_LIST_ERROR, hiding published content from editors with no diagnostic. Detect the missing deleted_at column and return COLLECTION_SCHEMA_MISMATCH naming the collection. isMissingColumnError mirrors isMissingTableError and takes an optional column scope so an unrelated unknown-column error still returns the generic error. Closes emdash-cms#709 Co-authored-by: Matt Kane <mkane@cloudflare.com>
…tries (emdash-cms#2153) On revision-supporting collections, staging a draft (Save/Autosave) on a published entry previously stamped updated_at on the live content row even though no public-facing column changed. Public SEO surfaces that read updated_at as "content last modified" — sitemap <lastmod>, JSON-LD dateModified — therefore registered a phantom modification every time an editor staged or discarded a draft. Skip the updated_at stamp on column-no-op writes. version still increments on every update so the _rev optimistic-concurrency token (version:updated_at) continues to catch concurrent editors with a 409. Fixes emdash-cms#2143
… and test quality (emdash-cms#2155)
* Update admin icon vocabulary * Refine admin icon overrides * Optimize admin icon lookup
* feat(plugins): add explicit MCP tool declarations * fix(plugins): harden MCP request and consent handling * fix(admin): send marketplace capability consent flag
…ms#2122) * Expose Astro's trailingSlash config to plugins via ctx.site Plugins that build absolute URLs (sitemap, canonical, hreflang) need to match the site's URL convention, but ctx.site only carried name/url/locale, so plugins had to hardcode a trailing slash. That's wrong for a site configured with trailingSlash: 'never' -- notably a headless front-end that serves bare URLs, where the plugin's sitemap/canonical then advertise URLs the site doesn't serve. Capture astroConfig.trailingSlash at astro:config:setup, carry it on virtual:emdash/config, and surface it as ctx.site.trailingSlash -- the same path the i18n config already rides. Optional on SiteInfo; createSiteInfo defaults it to 'ignore' (Astro's default) so existing construction is unaffected. * Type trailingSlash on the sandbox site payload + add changeset Review follow-up (emdash-cms#2122): createSiteInfo already flows the full SiteInfo — including trailingSlash — to sandboxed plugins via the runner options, matching the documented 'same normalized site context used by trusted plugin hooks and routes' parity. Make that explicit by adding trailingSlash to the sandbox site-shape types (core + the workerd/cloudflare wrappers) so the wire format is typed, not silently widened. Add the changeset for the published emdash change. * changeset: declare @emdash-cms/cloudflare + @emdash-cms/sandbox-workerd The sandbox runners/wrappers forward trailingSlash into generated worker code, so both published packages change behavior and must be released alongside emdash core. * test: expect trailingSlash in normalized siteInfo createSiteInfo now always populates trailingSlash (default "ignore"), so the strict siteInfo expectations must include it: sandbox-runner-options (toEqual x2), create.test.ts (objectContaining on the sandbox-runner siteInfo), and the plugin-route-site-info integration test (end-to-end). Verified by running the full core suite locally (4753 passing). * style: format --------- Co-authored-by: Matt Kane <mkane@cloudflare.com> Co-authored-by: emdashbot[bot] <emdashbot[bot]@users.noreply.github.com>
* test: add initial visual regression baselines * docs: update visual test gating comment
…h-cms#2158) * fix(admin): improve editor toolbar visual feedback * feat(admin): add editor toolbar tooltips * fix(admin): reflect selected text alignment * refactor(admin): clarify toolbar tooltip triggers * test(admin): harden bubble menu style assertions
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…#2171) * ci: accept visual baselines via /accept-baselines comment Replace the reaction-poll apply job with an issue_comment trigger. A maintainer comments /accept-baselines; the job checks write access via author association, finds the measure run for the PR head, and commits the candidate Linux baselines. Drops the schedule poller, the all-PR scan, the reactions API, and the collaborator-permission lookup, and posts explicit result comments. Update the sticky comment and docs copy. * ci: address review on /accept-baselines apply - Require an exact `/accept-baselines` comment (matches format-command) so quoted or suffixed text can't trigger a privileged write - Grant issues: write for the reaction API, and make the 👀 ack best-effort so it can't block the apply - Select the newest completed measure run and tell the maintainer to wait when it's still in progress, instead of a misleading "expired" - Broaden the failure comment to any post-gate failure
… admin - deliverSmtp threw inside a setTimeout callback, which never reaches the awaiting promise — the delivery kept hanging until the 30s hook timeout killed it with no SMTP trace. Race a rejecting timeout promise instead. - Return smtp.user in the settings API and repopulate the username field on reload; add user to SmtpConfigStatus type. - Surface the underlying delivery error in the test-email endpoint instead of a bare 500, and auto-suggest the security mode from the port. - Add regression tests for the timeout race and the STARTTLS upgrade flow.
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
The approach is sound: a built-in SMTP transport is the right way to give operators generic SMTP credentials on Cloudflare Workers, and it fits the existing exclusive email:deliver hook model alongside the dev console and Cloudflare Email providers. The PR references the approved discussion and adds a proper changeset.
Static review found several real issues, though:
- Admin UI labels are not localized. Hard-coded English provider names and security-mode labels in
EmailSettings.tsxviolate the Lingui/i18n convention. - SMTP configured via the admin UI does not become active until the runtime restarts. The SMTP built-in is registered only at cold-start when a complete config is already present. The settings route saves the DB record and sets the exclusive selection, but the current pipeline has no SMTP handler, so
isAvailable()returnstruewhilesend()throwsEmailNotConfiguredError. - The admin API allows saving port 25. The PR claims port 25 is refused with a clear error, but that refusal only lives in
loadSmtpConfigFromEnv(). The Zod schema used by the settings route accepts any port 1–65535. - SMTP MIME is not safe for non-ASCII content. Headers (
Subject,From,Reply-To) are emitted as raw UTF-8 and the body is declaredContent-Transfer-Encoding: 7bit, which will mojibake or be rejected for localized/RTL content. - Stale/narrative comments. The SMTP plugin header still says "env-only … no admin UI yet", and tests reference issue numbers in comments.
I did not run the test suite, linter, or builds; the findings below are based on reading the diff, tracing call sites, and checking the repo conventions in AGENTS.md.
| const PROVIDER_OPTIONS: { value: EmailProviderChoice; label: string }[] = [ | ||
| { value: "none", label: "None" }, | ||
| { value: "smtp", label: "SMTP" }, | ||
| { value: "cloudflare", label: "Cloudflare Email" }, |
There was a problem hiding this comment.
[needs fixing] These select labels are user-facing English strings rendered in the DOM, but they bypass Lingui. Per AGENTS.md every admin UI string must go through @lingui/react/macro / @lingui/core/macro.
Use msg descriptors at module scope and resolve them with t() when rendering the Select items:
| const PROVIDER_OPTIONS: { value: EmailProviderChoice; label: string }[] = [ | |
| { value: "none", label: "None" }, | |
| { value: "smtp", label: "SMTP" }, | |
| { value: "cloudflare", label: "Cloudflare Email" }, | |
| import { msg } from "@lingui/core/macro"; | |
| import type { MessageDescriptor } from "@lingui/core"; | |
| const PROVIDER_OPTIONS: { value: EmailProviderChoice; label: MessageDescriptor }[] = [ | |
| { value: "none", label: msg`None` }, | |
| { value: "smtp", label: msg`SMTP` }, | |
| { value: "cloudflare", label: msg`Cloudflare Email` }, | |
| ]; |
Then map with label: t(opt.label) in the <Select items={...}> call on line 263.
| { value: "starttls", label: "STARTTLS (port 587)" }, | ||
| { value: "tls", label: "Implicit TLS (port 465)" }, |
There was a problem hiding this comment.
[needs fixing] The STARTTLS / implicit TLS option labels are hard-coded English. They are user-visible strings in the security <Select> and must be localized.
| { value: "starttls", label: "STARTTLS (port 587)" }, | |
| { value: "tls", label: "Implicit TLS (port 465)" }, | |
| items={[ | |
| { value: "starttls", label: t`STARTTLS (port 587)` }, | |
| { value: "tls", label: t`Implicit TLS (port 465)` }, | |
| ]} |
| // Persist and activate SMTP as the selected provider | ||
| await optionsRepo.set(optionKey, SMTP_EMAIL_PLUGIN_ID); | ||
| emdash.hooks.setExclusiveSelection(EMAIL_DELIVER_HOOK, SMTP_EMAIL_PLUGIN_ID); | ||
|
|
There was a problem hiding this comment.
[needs fixing] This saves the SMTP config to the DB and immediately sets it as the selected exclusive provider, but nothing rebuilds the hook pipeline. EmDashRuntime only registers the built-in SMTP plugin at cold-start when loadSmtpConfig() already returns a complete config (emdash-runtime.ts:1359). If the worker started without SMTP env vars or an existing DB config, the SMTP handler is not in the current pipeline, so emdash.email.isAvailable() will report true while send() later throws EmailNotConfiguredError.
Fix by making the SMTP provider available immediately after admin configuration. The cleanest path is to register the built-in SMTP plugin unconditionally (like Cloudflare Email) and have its handler reject an incomplete config with a clear message, or expose a hook-pipeline rebuild that this route can invoke after persisting a new provider.
| const smtpConfigSchema = z.object({ | ||
| host: z.string().min(1), | ||
| port: z.number().int().min(1).max(65535), | ||
| secure: z.enum(["starttls", "tls"]), |
There was a problem hiding this comment.
[needs fixing] The PR description says port 25 is refused with a clear error because Cloudflare blocks it, but this Zod schema accepts any port 1–65535. A user can save port 25 through the admin UI, after which delivery will hang/fail at runtime.
Add a refine so the API rejects port 25 at the same place it rejects other invalid values:
| secure: z.enum(["starttls", "tls"]), | |
| const smtpConfigSchema = z.object({ | |
| host: z.string().min(1), | |
| port: z.number().int().min(1).max(65535), | |
| secure: z.enum(["starttls", "tls"]), | |
| user: z.string().min(1), | |
| pass: z.string().min(1).optional(), // undefined = keep existing password | |
| fromName: z.string().optional(), | |
| fromEmail: z.string().email().optional(), | |
| replyTo: z.string().email().optional(), | |
| }).refine((v) => v.port !== 25, { | |
| message: "Port 25 is not supported on Cloudflare Workers; use 587 (STARTTLS) or 465 (implicit TLS).", | |
| }); |
| ].join("\r\n"); | ||
| } else { | ||
| headers.push(`Content-Type: text/plain; charset=utf-8`); | ||
| headers.push(`Content-Transfer-Encoding: 7bit`); |
There was a problem hiding this comment.
[needs fixing] The MIME body is declared Content-Transfer-Encoding: 7bit and Subject / From / Reply-To headers are emitted as raw UTF-8. That violates RFC 5322/2047 and will mojibake or be rejected for any non-ASCII content, including localized site names, RTL text, or sender names.
At minimum, encode non-ASCII headers per RFC 2047 and switch the body transfer encoding to quoted-printable or base64 (or negotiate BODY=8BITMIME and use 8bit). As written, a user-visible <strong>${siteName}</strong> from the test email or any localized subject will be corrupted.
| * uses `node:net` / `node:tls`. Configuration is env-only for the first | ||
| * iteration — no admin UI yet. |
There was a problem hiding this comment.
[suggestion] This header comment is stale: it says the SMTP config is "env-only" and that "no admin UI yet" exists, but this PR adds exactly that admin UI. Update or remove the outdated line so future readers don't believe the feature is CLI/env-only.
| * uses `node:net` / `node:tls`. Configuration is env-only for the first | |
| * iteration — no admin UI yet. | |
| * via raw TCP — the one network primitive sandboxed plugins cannot use. | |
| * | |
| * Registered as a built-in `email:deliver` provider when SMTP is configured, | |
| * either via env vars or via Settings → Email in the admin UI. |
| }); | ||
| }); | ||
|
|
||
| describe("Cloudflare socket edge cases (regression #1541)", () => { |
There was a problem hiding this comment.
[suggestion] AGENTS.md says comments should never reference issues/PRs; that context belongs in commit messages. The describe block name and the inline // Regression #1541 comment both violate this. Rename the suite to something descriptive (e.g., Cloudflare socket edge cases) and delete the issue-number comment on line 462.
| } | ||
| }); | ||
|
|
||
| it("uses 25s timeout so hook timeout (30s) does not swallow the error", async () => { |
There was a problem hiding this comment.
[suggestion] The test name claims to verify that the default 25s timeout is shorter than the 30s hook timeout, but it actually passes timeoutMs: 50 and only asserts the configurable timeout is used. It cannot fail on the real regression it claims to guard against (someone changing the default 25s value). Either drop the test or mock timers and assert the actual default.
Resolve conflicts: revert messages.po to upstream (workflow regenerates on merge), take upstream AGENTS.md and SKILL.md (both upstream-only changes).
github-actions
Bot
added
review/awaiting-author
There was a problem hiding this comment.
Re-review summary
The approach is still sound: a built-in SMTP transport is the right way to unblock operators with generic SMTP credentials on Cloudflare Workers, and adding Settings → Email UI support fits EmDash’s exclusive-hook email model. However, this revision does not resolve the main blockers from the first review. I checked the diff, the full changed files, cross-references to EmailPipeline/HookPipeline, and the repo conventions in AGENTS.md.
What changed since first review
- The admin settings route now persists SMTP config to the DB and sets the exclusive provider selection.
- Cloudflare Email also gained a DB-backed config path.
- New tests cover DB save/load and Cloudflare Email.
What is still broken / unaddressed
- Admin UI labels are still hard-coded English (
PROVIDER_OPTIONS, security-mode labels, placeholders) — a directAGENTS.md/Localize everything user-facingviolation. - Success toasts display raw server English messages (
"SMTP configured and activated", etc.) instead of localized text. - SMTP configured via the admin UI does not actually deliver email until the runtime restarts. The plugin is only registered at cold-start when a complete config already exists; the settings route can set the exclusive selection, but the hook pipeline has no
email:deliverhandler foremdash-smtp. - Port 25 is still accepted by the admin API. The only refusal is in
loadSmtpConfigFromEnv(); the Zod schema andloadSmtpConfigFromDballow port 25, defeating the documented guarantee. - SMTP MIME is not safe for non-ASCII / RTL content. Headers are emitted raw and the body is declared
Content-Transfer-Encoding: 7bit. - Stale/narrative comments remain. The SMTP module header still says “env-only … no admin UI yet”, and tests reference issue numbers.
I did not run the test suite, linter, or build; the findings below are based on static reading and tracing call sites.
| const PROVIDER_OPTIONS: { value: EmailProviderChoice; label: string }[] = [ | ||
| { value: "none", label: "None" }, | ||
| { value: "smtp", label: "SMTP" }, | ||
| { value: "cloudflare", label: "Cloudflare Email" }, |
There was a problem hiding this comment.
[needs fixing] Provider and security-mode labels are still hard-coded English: "None", "SMTP", "Cloudflare Email", "STARTTLS (port 587)", "Implicit TLS (port 465)", and all the SMTP form placeholders. AGENTS.md requires every admin UI string to go through Lingui.
| const PROVIDER_OPTIONS: { value: EmailProviderChoice; label: string }[] = [ | |
| { value: "none", label: "None" }, | |
| { value: "smtp", label: "SMTP" }, | |
| { value: "cloudflare", label: "Cloudflare Email" }, | |
| import { msg } from "@lingui/core/macro"; | |
| import type { MessageDescriptor } from "@lingui/core"; | |
| const PROVIDER_OPTIONS: { value: EmailProviderChoice; label: MessageDescriptor }[] = [ | |
| { value: "none", label: msg`None` }, | |
| { value: "smtp", label: msg`SMTP` }, | |
| { value: "cloudflare", label: msg`Cloudflare Email` }, | |
| ]; | |
| // inside the component, render with t(opt.label) |
| }, [settings]); | ||
|
|
||
| const saveMutation = useMutation({ | ||
| mutationFn: saveEmailSettings, |
There was a problem hiding this comment.
[needs fixing] The save-success toast shows result.message directly. The API returns English strings like "SMTP configured and activated", so non-English admins will see untranslated text. The same pattern is used for testMutation and bindingMutation success toasts.
| mutationFn: saveEmailSettings, | |
| onSuccess: () => { | |
| toastManager.add({ title: t`Email settings saved`, variant: "success", timeout: 5000 }); | |
| setSmtpPass(""); // clear password field after save | |
| void queryClient.invalidateQueries({ queryKey: ["email-settings"] }); | |
| }, |
If the message needs to vary by provider, return a stable code from the API and map it to a localized string on the client.
| // WP Mail SMTP's is_mailer_complete(): a half-saved config (host but no | ||
| // password) must not register a provider — the resulting 535 on send is | ||
| // more confusing than a clean "not configured" state. | ||
| if (isSmtpConfigComplete(smtpConfig)) { |
There was a problem hiding this comment.
[needs fixing] The SMTP built-in is registered only when isSmtpConfigComplete(loadSmtpConfig(...)) is true at cold start. The settings route (packages/core/src/astro/routes/api/settings/email.ts:337) can save DB config and call emdash.hooks.setExclusiveSelection(EMAIL_DELIVER_HOOK, SMTP_EMAIL_PLUGIN_ID), but if the plugin was never registered (because no SMTP config existed at startup), the pipeline has no email:deliver handler for it. EmailPipeline.isAvailable() will return true while send() throws EmailNotConfiguredError.
The Cloudflare Email built-in is always registered (emdash-runtime.ts, just below this block); SMTP should be registered the same way, with its handler loading the latest DB config on demand and failing cleanly when the config is incomplete.
|
|
||
| const smtpConfigSchema = z.object({ | ||
| host: z.string().min(1), | ||
| port: z.number().int().min(1).max(65535), |
There was a problem hiding this comment.
[needs fixing] The PR states that port 25 is refused, but the admin API schema accepts any port from 1–65535. The only port-25 refusal is in loadSmtpConfigFromEnv(); a config saved through the admin UI will bypass that check both at validation time and in loadSmtpConfigFromDb().
| port: z.number().int().min(1).max(65535), | |
| const smtpConfigSchema = z.object({ | |
| host: z.string().min(1), | |
| port: z.number() | |
| .int() | |
| .min(1) | |
| .max(65535) | |
| .refine((p) => p !== 25, { | |
| message: "Port 25 is not supported; use 587 (STARTTLS) or 465 (implicit TLS).", | |
| }), | |
| secure: z.enum(["starttls", "tls"]), | |
| user: z.string().min(1), | |
| pass: z.string().min(1).optional(), | |
| fromName: z.string().optional(), | |
| fromEmail: z.string().email().optional(), | |
| replyTo: z.string().email().optional(), | |
| }); |
The same refusal should also be added to loadSmtpConfigFromDb() and to the client-side handleSave port check.
| `Subject: ${sanitizeHeader(subject)}`, | ||
| `Date: ${new Date().toUTCString()}`, | ||
| `Message-ID: <${crypto.randomUUID()}@emdash>`, | ||
| `MIME-Version: 1.0`, | ||
| ]; | ||
| if (replyTo) { | ||
| headers.push(`Reply-To: ${sanitizeHeader(replyTo)}`); | ||
| } | ||
|
|
||
| let body: string; | ||
| if (html) { | ||
| const boundary = `----=_emdash_${crypto.randomUUID()}`; | ||
| headers.push(`Content-Type: multipart/alternative; boundary="${boundary}"`); | ||
| body = [ | ||
| `--${boundary}`, | ||
| `Content-Type: text/plain; charset=utf-8`, | ||
| `Content-Transfer-Encoding: 7bit`, |
There was a problem hiding this comment.
[needs fixing] buildMime emits raw UTF-8 in Subject, From, and Reply-To headers and declares Content-Transfer-Encoding: 7bit for both plain-text and HTML parts. For localized or RTL content this will mojibake or be rejected by strict SMTP servers. Headers should be RFC 2047 encoded and the transfer encoding should be base64/quoted-printable when non-ASCII bytes are present.
| * Registered as a built-in `email:deliver` provider when SMTP env vars are | ||
| * present. On Cloudflare Workers it uses `cloudflare:sockets`; on Node it | ||
| * uses `node:net` / `node:tls`. Configuration is env-only for the first | ||
| * iteration — no admin UI yet. |
There was a problem hiding this comment.
[needs fixing] The module header is stale: it says the config is "env-only … no admin UI yet", but this PR adds exactly that. There are also issue-number references in the new test comments (packages/core/tests/unit/plugins/email-smtp.test.ts:453, :462). AGENTS.md prohibits issue/PR references in comments and narrative/stale documentation.
| * iteration — no admin UI yet. | |
| * Registered as a built-in `email:deliver` provider when SMTP is configured | |
| * via environment variables or the Settings → Email admin UI. |
- Localize provider names and security-mode labels in EmailSettings - Register SMTP provider unconditionally; load config from DB on send so admin-saved settings take effect without restart - Refuse port 25 in the admin API schema, matching the env loader - Encode MIME headers per RFC 2047 and bodies as base64 for UTF-8 safety - Apply dot-stuffing to raw text before base64 encoding - Remove stale comments and issue references from tests
github-actions
Bot
added
review/needs-rereview
CI (Node 22) flags the inline /[^\x20-\x7E]/ regex in encodeHeader as a performance issue — it recompiles on every call. Local lint (Node 24) misses it because the type-aware analysis behaves differently across Node versions. Also add scripts/pre-push-check.sh that documents the Node 22 vs 24 limitation and runs the checks that work locally.
10 participants
What does this PR do?
Adds a built-in
email:delivertransport that works with any standard SMTP server (Brevo relay, Office365, Google Workspace, Fastmail, Amazon SES, self-hosted Postfix) via raw TCP — the one network primitive sandboxed plugins cannot use. This unblocks operators who only have generic SMTP credentials and cannot use HTTP transactional APIs (Brevo HTTP API, SendGrid, etc.).How it works:
EMAIL_SMTP_HOST,EMAIL_SMTP_PORT,EMAIL_SMTP_USER,EMAIL_SMTP_PASS, optionalEMAIL_SMTP_FROM) or via the new Settings → Email admin UI (password stored encrypted).cloudflare:socketsand on Node vianode:net/node:tls, with automatic fallback.Cloudflare Workers specifics fixed:
WritableStreamlock: the plaintext socket's writer must be closed beforestartTls()upgrades the socket, otherwise the first write to the TLS socket throws "WritableStream is currently locked".sock.openedmust be awaited before writing — unlike Node, where the connect callback signals readiness.secureTransport: "starttls"is required during the initial connection to later allowsock.startTls().throwinsidesetTimeoutnever reaches the awaiting promise and becomes an unhandled exception.Admin UI:
__none__sentinel for exclusive hooks.send_emailbinding.Closes #1541
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output
Unit tests:
packages/core/tests/unit/plugins/email-smtp.test.ts— 21 tests covering env config parsing, port-25 refusal, full STARTTLS session flow, AUTH failure, dot-stuffing, the plugin handler contract, and Cloudflare-specific regressions (timeout race, STARTTLS upgrade flow).Live verification on Cloudflare Workers (Brevo SMTP, port 465 implicit TLS): test email delivered successfully.