Skip to content

feat(web): SMTP relay in the setup wizard with provider presets - #233

Merged
marcelxpfeifer merged 12 commits into
mainfrom
feat/exp-a2-smtp-wizard
Jul 10, 2026
Merged

feat(web): SMTP relay in the setup wizard with provider presets#233
marcelxpfeifer merged 12 commits into
mainfrom
feat/exp-a2-smtp-wizard

Conversation

@marcelxpfeifer

Copy link
Copy Markdown
Contributor

Part of the 2026-07-10 experience plan (Sending Transports epic). Implements locked decisions #1 & #2: transport is instance-level (own MTA / SES / SMTP relay), and the long tail is a single generic SMTP relay with provider presets — no per-provider API adapters.

The backend smtp send adapter, the SMTP_RELAY_* env keys, and their inclusion in CONVEX_RUNTIME_ENV_KEYS already shipped; this PR makes the setup wizard actually offer and validate an SMTP relay, end to end.

What changed

  • Wizard composable (useSetupWizard.ts) — ProviderChoice gains smtp; EmailStepDraft carries an SmtpRelayDraft; validateEmailStep / buildProviderEnv / PROVIDER_ENV_KEYS / PROVIDER_LABELS / buildSetupSummary all carry the relay fields into an EMAIL_PROVIDER=smtp + SMTP_RELAY_* env patch. New exported SMTP_RELAY_PRESETS (Mailgun / Postmark / SendGrid / Brevo / Custom) prefill host/port/TLS.
  • Email step page (setup/email.vue) — fourth "SMTP relay" option with a preset select + host/port/TLS/username/password form; a named preset locks the host to its documented submission endpoint, Custom leaves it editable. Step copy reframed as an honest choice (own MTA / Amazon SES / SMTP relay). Uses only FF design tokens + UiSelect/UiInput; error state wired.
  • Live validation/api/setup/validate-provider gains an smtp branch backed by a new shared validateSmtpRelay, which opens a real SMTP submission connection (implicit TLS or STARTTLS), runs EHLO + an AUTH exchange, and sends no message. SSRF-guarded (reuses isBlockedSsrfHost) and bounded by a per-step timeout, mirroring the existing PostHog-host check. Clear operator-facing error strings.
  • setup-cli paritySendingConfig + parseSending + buildEnvPatchFromConfig accept provider: 'smtp'; validators.ts re-exports validateSmtpRelay; the terminal wizard gains an SMTP-relay branch with the same presets and live handshake. No compose profile change — a relay is external, so getActiveProfiles correctly adds no mta service.
  • Docs — supported send-provider kinds + factory layout updated; setup-cli guide describes the three-way choice with live validation.

Acceptance criteria

  • Fourth "SMTP relay" provider option with Mailgun/Postmark/SendGrid/Brevo/Custom presets prefilling host/port/TLS
  • Credential form (username, password) beneath the preset
  • smtp fields flow through ProviderChoice / EmailStepDraft / validateEmailStep / buildProviderEnv / PROVIDER_ENV_KEYSEMAIL_PROVIDER=smtp + SMTP_RELAY_*
  • Real validation before apply: SMTP handshake + AUTH check with bounded timeout and clear errors
  • Step copy reframed as an honest own-MTA / SES / SMTP-relay choice
  • setup-cli validators.ts + config accept EMAIL_PROVIDER=smtp (no compose profile change)
  • Provider-choice docs updated
  • Vitest: presets fill correctly, missing creds block, env patch shape, SSRF/port guards

Preserved behavior

  • Existing provider choices (own MTA, Resend, SES, receive-only "none") and their live-validation and env paths are untouched; Resend remains a listed option.
  • apply.post.ts's delivery-provider floor already accepts smtp via isDeliveryProviderKind — no change needed.
  • Wizard navigation, Back/return-with-values seeding, and the required-provider gate all survive; the smtp draft is seeded back from env on return.
  • No new env reads outside lib/env.ts; SMTP_RELAY_* were already runtime-pushed.

Test notes

Vitest only (CI gate — not run locally per pipeline rules). Added: composable smtp validation + env-patch cases, setup-cli buildEnvPatchFromConfig/parseSending smtp cases, and validateSmtpRelay input-guard tests that return before any socket opens (no network in CI).

Auto-merge pipeline: squash-merges on reviewer approval + green CI.

Marcel Pfeifer added 5 commits July 10, 2026 12:24
Add validateSmtpRelay to the shared setup validators: it opens a real
SMTP submission connection (implicit TLS or STARTTLS), runs EHLO and an
AUTH exchange, and never sends a message — so a wrong host/port/TLS/
credential is caught at setup, not at first send. SSRF-guarded and
bounded by a per-step timeout, mirroring the existing PostHog-host check.

Wire it into /api/setup/validate-provider under provider='smtp' alongside
the existing API-key path.
Add a fourth transport to the wizard: an SMTP relay with Mailgun/Postmark/
SendGrid/Brevo/custom presets that prefill host/port/TLS, plus a username/
password form. Carry the smtp fields through ProviderChoice / EmailStepDraft
/ validateEmailStep / buildProviderEnv (EMAIL_PROVIDER=smtp + SMTP_RELAY_*),
and validate the relay with a live handshake before advancing. Reframe the
step copy as an honest choice: run your own MTA, Amazon SES, or an SMTP relay.
…izard

Add the smtp variant to SendingConfig + parseSending + buildEnvPatchFromConfig
(host/username/password required; port/TLS default to 587/STARTTLS), re-export
validateSmtpRelay, and add an SMTP-relay branch to the terminal wizard with the
same presets and a live handshake check. No compose profile change — a relay is
external, so getActiveProfiles adds no mta service for it.
validateEmailStep/buildProviderEnv for the smtp choice (presets fill, missing
creds block, default port omitted), buildEnvPatchFromConfig + parseSending
rejections in setup-cli, and validateSmtpRelay input guards (SSRF, port range,
missing credentials) that return before any socket opens.
Add smtp to the supported send-provider kinds and the factory layout, and
describe the honest own-MTA / SES / SMTP-relay wizard choice (with live
credential validation) in the setup-cli guide.
@marcelxpfeifer

Copy link
Copy Markdown
Contributor Author

Review — round 1

Verdict: REQUEST_CHANGES

Blocking

  • CI is failingLint & Typecheck fails on lint:filesize: apps/setup-cli/src/commands/setup.ts grew from 469 to 601 LOC, over the ~500 ratchet and not in the frozen baseline (adding it is forbidden). A large share of the growth is formatting-only rewraps of functions this piece never touches (pickAIProvider, collectPostHog, collectAdmin, launchWebWizard, …), which also bloats the diff against the "focused diff" rule. Fix both at once: extract the sending-provider picker (the option list, SMTP_RELAY_CLI_PRESETS, and the new smtp branch) into its own module (e.g. apps/setup-cli/src/commands/setupSendingProvider.ts), and revert the drive-by reformatting of unrelated functions.
  • apps/setup-cli/src/commands/setup.ts:376 + :396the port that gets validated is not the port that gets persisted. Number.parseInt(result.port, 10) feeds the live handshake (falling back to 587 when not finite), but result.port.trim() is written verbatim to SMTP_RELAY_PORT. Type 587x and the handshake validates 587 (parseInt stops at the first non-digit) while the env carries SMTP_RELAY_PORT=587x, which makes the backend adapter throw Invalid SMTP_RELAY_PORT at first send — exactly the failure live validation exists to prevent; abc validates 587 and persists abc. Validate the port string at the prompt (digits only, 1–65535, mirroring the web's isValidSmtpPort) and persist the same numeric value the handshake used.
  • packages/shared/src/setupValidators.ts:346startTls() has no timeout. SmtpProbe.open and read() are both bounded by SMTP_PROBE_TIMEOUT_MS, but a relay that answers 220 to STARTTLS and then never completes the TLS handshake leaves the tlsConnect promise pending forever, hanging the Nitro request / CLI step indefinitely. The spec requires a bounded timeout on the whole probe. Wrap the upgrade in the same SMTP_PROBE_TIMEOUT_MS timer (destroy the socket and reject on expiry), like open() does.

Improvements

  • apps/setup-cli/src/commands/setup.ts:290 / apps/web/app/composables/useSetupWizard.ts:81Duplicated Code: SMTP_RELAY_CLI_PRESETS re-declares the same Mailgun/Postmark/SendGrid/Brevo host/port table as the web's SMTP_RELAY_PRESETS, held in sync only by a comment. Both sides already import from @owlat/shared — move one preset table there (e.g. next to setupValidators) and consume it from both, so the endpoints can never drift.
  • packages/shared/src/setupValidators.ts:327 (via expect()describeSmtpError) — error strings embed the raw, unbounded remote reply (${reply.code} ${reply.text}, all lines joined) and describeSmtpError passes anything starting with "SMTP relay" through verbatim to the setup-mode caller. A non-SMTP or hostile server (including an internal service reached through a DNS name the literal-IP SSRF guard cannot catch) gets its banner/response echoed back wholesale. The neighboring validatePostHogHost deliberately suppresses raw probe output for this reason. Truncate the echoed reply text (~120 chars) and strip control characters before including it in the message.
  • apps/web/server/api/setup/validate-provider.post.ts:54typeof smtp.port === 'number' ? smtp.port : 587 silently substitutes 587 when a client sends a malformed port (e.g. a string), so the endpoint can report "credentials accepted" for a different port than the caller asked about. Reject a present-but-non-numeric smtp.port with a 400 instead of coercing.
  • apps/setup-cli/src/commands/setup.ts:359-373 — the CLI smtp prompts accept empty host/username/password and defer everything to the network probe, while the web step blocks empty fields before any socket opens. Add validate: callbacks to the host/username/password prompts (the same pattern collectAdmin uses for email) so an empty field is caught at the prompt.

Notes

  • Web wizard flow, presets, live-validation branch, honest three-way copy, FF tokens, docs, and the vitest coverage (composable gate + env patch, setup-cli config, validateSmtpRelay input guards with no sockets) all check out; commits are atomic and clean. Only the items above stand between this and approval.

Marcel Pfeifer added 4 commits July 10, 2026 12:49
The STARTTLS upgrade had no timeout, so a relay that answers 220 and then
never completes the TLS handshake would leave the promise pending forever and
hang the setup endpoint / CLI step. Wrap it in the same SMTP_PROBE_TIMEOUT_MS
timer as open()/read() (destroy the socket and reject on expiry).

Raw remote replies were also embedded verbatim (and unbounded) in operator-
facing error strings, so a non-SMTP or hostile server could echo its banner or
terminal control codes into the setup response. Truncate to 120 chars and strip
control characters before echoing.
The web wizard and setup CLI each carried their own copy of the Mailgun /
Postmark / SendGrid / Brevo host/port table, kept in sync only by a comment.
Move the single source of truth to @owlat/shared/setupSendingPresets and consume
it from both so the two can never drift.
… module

setup.ts had grown past the file-size ratchet, largely from the inline SMTP
relay branch. Move pickSendingProvider into setupSendingProvider.ts and lift the
shared validateWithSpinner helper into lib/progress, dropping setup.ts back under
the cap and reverting the drive-by reformatting of untouched functions.

While extracting, close two gaps the reviewer flagged: the SMTP port is now
validated at the prompt (digits, 1-65535) and the same numeric value that fed the
handshake is what gets persisted to SMTP_RELAY_PORT (so '587x' can no longer
validate 587 but write 587x), and host/username/password prompts reject empty
input up front instead of deferring to the network probe.
… 587

A present-but-non-numeric smtp.port was silently substituted with 587, so the
validate-provider endpoint could report success for a different port than the
caller asked about. Reject it with a 400; an absent port still defaults to 587.
@marcelxpfeifer

Copy link
Copy Markdown
Contributor Author

Author response — round 2

Thanks for the thorough pass. All three blocking findings and every improvement are addressed.

Blocking

  • CI lint:filesize / drive-by reformatting — extracted the whole sending-provider picker into a new apps/setup-cli/src/commands/setupSendingProvider.ts and lifted the shared validateWithSpinner helper into lib/progress. setup.ts is back down to 470 LOC (from 601) and its diff is now just the two import lines plus the removed functions — the oxfmt rewraps of pickAIProvider/collectPostHog/etc. are gone because those functions are no longer in the touched region. (Note: the format gate ratchets every touched file, so any edit to setup.ts forces oxfmt to reflow the whole file — the extraction is what removes the unrelated churn, not a manual un-format.)
  • Validated port ≠ persisted port — the port is now validated at the prompt (validatePortInput: digits only, 1–65535, blank ⇒ 587, mirroring the web's isValidSmtpPort), and the same number that fed the handshake is what gets written to SMTP_RELAY_PORT. 587x/abc are rejected at the prompt, so they can never reach the backend adapter.
  • startTls() had no timeout — the TLS upgrade is now wrapped in the same SMTP_PROBE_TIMEOUT_MS timer as open()/read(): on expiry it destroys the socket and rejects. A relay that answers 220 to STARTTLS and then stalls can no longer hang the endpoint/CLI.

Improvements

  • Duplicated preset table — moved to @owlat/shared/setupSendingPresets (SMTP_RELAY_PRESETS + SmtpRelayPreset), consumed by both the web composable and the CLI picker; the web composable now re-exports it so email.vue and its tests are unchanged.
  • Unbounded/raw echoed reply — added sanitizeReplyText: strips control characters and truncates to 120 chars, applied at both expect() and the AUTH-failure message before anything is echoed, so a hostile/non-SMTP banner can't be reflected wholesale.
  • Endpoint coercing a bad portvalidate-provider.post.ts now rejects a present-but-non-numeric smtp.port with a 400 instead of substituting 587 (absent still defaults to 587).
  • CLI accepted empty fields — host/username/password prompts now carry validate: callbacks (requireNonEmpty) that block empties up front, matching the web step.

To keep the newly-touched setupValidators.ts under the 500-LOC cap after the security additions, the preset table extraction pulls its data out into the shared module; the file now holds only the probe + validators.

@marcelxpfeifer

Copy link
Copy Markdown
Contributor Author

Review — round 2

Verdict: REQUEST_CHANGES

All three round-1 blockers and all four improvements are verified fixed: setup.ts is back to 470 LOC with the picker cleanly extracted, the CLI prompt now validates the port it persists, startTls() is bounded by SMTP_PROBE_TIMEOUT_MS, the preset table lives once in @owlat/shared/setupSendingPresets, echoed SMTP replies are control-stripped and truncated at both expect() and the AUTH-failure path, the endpoint 400s on a non-numeric port, and the CLI prompts block empty fields. Lint & Typecheck and every test job are green. What remains is one correctness hole the port rework left open, an infra-flaked CI run, and two small items in the new code.

Blocking

  • apps/setup-cli/src/commands/setupSendingProvider.ts:148the validated port still isn't always the persisted port, via the env merge. The env patch only carries SMTP_RELAY_PORT when port !== 587, but setup.ts applies it with mergeEnv(existingEnv, envPatch) = { ...existing, ...patch }, and re-running setup over an existing install is an explicitly supported flow ("Re-running setup will update .env"). A prior install with SMTP_RELAY_PORT=465 (or 2525) that is re-configured to the default 587 validates the handshake on 587 yet keeps sending on the stale port — worst case SMTP_RELAY_SECURE flips to false while the port stays 465, so the first real send does cleartext-STARTTLS against an implicit-TLS port and fails, exactly what live validation exists to prevent. The comment above this block ("The port that gets validated is the port that gets persisted") asserts the guarantee the merge breaks. Fix: always write SMTP_RELAY_PORT: String(port) (drop the !== 587 conditional). Apply the same rule at apps/web/app/composables/useSetupWizard.ts:218apply.post.ts also merges { ...existing, ...body.env } over the on-disk .env, so an omitted key can't displace a stale one there either; emit '587' when the field is blank.
  • CI run has 3 failed checksDocker Build (code-worker / convex-deploy / mta) all died in ~15s with ERROR: Error response from daemon: received unexpected HTTP status: 500 before any build step ran. This PR touches none of those images, so it's a registry/runner flake, not a code defect — but the merge gate needs green, so re-run the failed jobs (a re-push for the fix above will re-trigger anyway).

Improvements

  • apps/web/app/composables/useSetupWizard.ts:26-27 — the re-export comment says the single source of truth "lives in @owlat/shared/setupValidators", but the dedupe put it in @owlat/shared/setupSendingPresets (which line 23 imports). Fix the module name. While in comment territory: packages/shared/src/setupSendingPresets.ts:21 hangs "blank host ⇒ operator fills it in" on the port field's doc comment — move that clause to host (or drop it).
  • apps/setup-cli/src/commands/setupSendingProvider.ts:98 — the new smtp group() never handles cancellation: @clack/prompts without an onCancel option keeps running the remaining prompts and stores the cancel symbol in the results, so Ctrl-C at any of the five prompts ends in result.port.trim() / validateSmtpRelay({ host: Symbol… }) throwing a TypeError instead of the clean return null every single-prompt path has. Pass group(prompts, { onCancel }) (or isCancel-check the results) and return null; the moved SES group at line 74 shares the flaw and can take the same one-liner while you're in the file.

Notes

  • Round-1 fixes are all genuine and well done — the setupSendingProvider.ts extraction reads better than the original, validateWithSpinner in lib/progress is a real shared home (three callers), and the new tests (preset shape, env-patch cases, config rejections, input guards with no sockets) cover the added logic properly. Commits stay atomic and attribution-free; no bun.lock churn; the packages/shared export addition is sanctioned by the dedupe.

Marcel Pfeifer added 3 commits July 10, 2026 13:02
The env patch omitted SMTP_RELAY_PORT when the port was the default 587.
Both apply.post.ts (web) and mergeEnv (CLI) apply the patch over the
on-disk .env, so re-running setup over an install that previously used
465/2525 kept the stale port while the handshake validated 587 — worst
case SMTP_RELAY_SECURE flips to false against an implicit-TLS port and
the first send fails. Always write the port (default '587' when blank)
so it can't diverge from the validated value.
group() without an onCancel keeps prompting after Ctrl-C and stores the
cancel symbol in the results, so a later result.port.trim() on a symbol
threw a TypeError instead of the clean return-null every single-prompt
path has. Detect a cancel in any field of the SES and SMTP groups and
bail out cleanly.
The re-export comment named @owlat/shared/setupValidators; the deduped
preset table lives in @owlat/shared/setupSendingPresets. Also move the
'blank host ⇒ operator fills it in' note off the port field's doc onto
host, where it belongs.
@marcelxpfeifer

Copy link
Copy Markdown
Contributor Author

Author response — round 3

Both round-2 blockers and both improvements are addressed.

Blocking

  • Validated port ≠ persisted port via the env merge — the env patch now always writes SMTP_RELAY_PORT (dropped the !== 587 conditional) in setupSendingProvider.ts, and useSetupWizard.ts emits '587' on a blank field instead of omitting the key. Since both apply.post.ts and mergeEnv apply the patch over the on-disk .env, an omitted key could no longer let a stale SMTP_RELAY_PORT=465/2525 survive and diverge from the handshake-validated value. Updated the composable test that asserted the old omit-on-default behavior to expect '587'.
  • CI infra flake (Docker Build code-worker / convex-deploy / mta) — this fix push re-triggers those jobs; the PR touches none of those images.

Improvements

  • Stale re-export comment — corrected useSetupWizard.ts to name @owlat/shared/setupSendingPresets (the actual dedupe home), and moved the 'blank host ⇒ operator fills it in' clause off the port field's doc onto host in setupSendingPresets.ts.
  • group() cancellation — added a groupCancelled() helper that isCancel-checks every field; both the SES and SMTP group() call sites now return null on Ctrl-C instead of falling through to result.port.trim() / validateSmtpRelay({ host: Symbol… }) and throwing a TypeError.

Intentionally unchanged

  • apps/setup-cli/src/lib/setupConfig/override.ts (the declarative config-file → env patch) still omits the default port. The finding scoped the fix to the two interactive/wizard merge paths; the config-file path is a full declarative env spec authored by the operator, not a validated-then-merged handshake, and its tests assert the omit-on-default mapping. Left in scope as named.

@marcelxpfeifer

Copy link
Copy Markdown
Contributor Author

Review — round 3

Verdict: APPROVE

Blocking

none

Improvements

none

Both round-2 blockers and both improvements are verified fixed at head:

  • apps/setup-cli/src/commands/setupSendingProvider.ts — the env patch now always writes SMTP_RELAY_PORT: String(port) (the same number the handshake used), and apps/web/app/composables/useSetupWizard.ts emits '587' on a blank field, so neither merge path (mergeEnv / apply.post.ts) can let a stale on-disk port diverge from the validated value. The composable test asserts the new behavior.
  • CI is fully green on the head SHA — all 34 checks pass, including the three previously registry-flaked Docker builds (code-worker / convex-deploy / mta).
  • The re-export comment now correctly names @owlat/shared/setupSendingPresets, and the "blank host" clause sits on the host field's doc in setupSendingPresets.ts.
  • groupCancelled() isCancel-checks every field of both the SES and SMTP group() results and bails with return null, matching the single-prompt paths.

On the intentionally-unchanged setupConfig.ts omit-on-default port mapping: accepted as scoped. That path is a declarative config with no live handshake (so the validated-vs-persisted invariant doesn't apply), it primarily serves fresh headless installs, and its tests pin the mapping.

Regression sweep of the three fix commits found nothing new; the earlier security properties (bounded startTls, sanitizeReplyText on every echoed reply, SSRF guard, endpoint 400 on non-numeric port, prompt-level input validation) are all intact at head. Commits stay atomic and attribution-free; no bun.lock churn. The piece delivers its spec end to end — web wizard, live SMTP handshake validation, CLI parity, shared presets, tests, and docs.

@marcelxpfeifer
marcelxpfeifer merged commit d72f2b0 into main Jul 10, 2026
33 checks passed
@marcelxpfeifer
marcelxpfeifer deleted the feat/exp-a2-smtp-wizard branch July 10, 2026 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant