# SMTP mail sending alongside Postmark

## Context

Project sends mail only through Postmark (API token). Self-hosters with their own SMTP server cannot use the bot's email features (confirmation emails). The ask: introduce a common `Sender` interface in `internal/mail/`, split the Postmark transport into its own file, add an SMTP transport via stdlib `net/smtp`, wire config + `main.go` + README, and test everything.

`postmark.Email` stays the shared email payload for both transports. Downstream `EmailSender` interfaces in `sub`, `ext` are structurally satisfied by `*mail.Client` and are NOT touched. Their callsites today set: To, Tag, Subject, TextBody (confirmation), MessageStream + TextBody + HTMLBody. `mail.Client.Send` always stamps From + ReplyTo.

User decisions (locked): SMTP wins when both SMTP host and Postmark token are configured; port 465 uses implicit TLS.

## Approach

### 1. `internal/mail/mail.go` — Sender interface + refactored client

Replace whole file. Interface first, then Client, then funcs (decorder: type before func).

- `type Sender interface { Send(ctx context.Context, req *postmark.Email) error }`
- `type Client struct { from, replyto string; sender Sender }`
- `func New(from, replyto string, sender Sender) *Client` — returns `nil` when `sender == nil` (preserves today's nil-on-unconfigured behavior; downstream `kit.IsNil` nil-checks in `confirmation.go` already handle it).
- `func (c *Client) Send(ctx context.Context, req *postmark.Email) error` — identical behavior to today: set `req.From = c.from`, `req.ReplyTo = c.replyto`, then `err := c.sender.Send(ctx, req)`; on error `log.Error().Err(err).Msg("cannot send email")` and return err; on success `log.Debug().Str("subject", req.Subject).Str("to", req.To).Msg("email has been sent")` and return nil. Log strings unchanged; `log := zerolog.Ctx(ctx)`.
- Package comment: `package mail //nolint:var-naming // Package mail provides a client to send mail via configured sender`.

### 2. `internal/mail/postmark.go` — Postmark transport

- `type PostmarkSender struct { client *postmark.Client }`
- `func NewPostmark(token string, opts ...postmark.Option) Sender` — variadic opts exist so tests inject `postmark.WithBackendURL(...)` (no mutable globals). Build: `postmark.NewClient(append([]postmark.Option{postmark.WithClient(&http.Client{Transport: &postmark.AuthTransport{Token: token, Transport: httpclient.NewTransport()}})}, opts...)...)`.
  - `httpclient` is `github.com/etkecc/go-kit/httpclient` (go-kit already a direct dep, vendored). `NewTransport()` = tuned base transport: 30s dial timeout, TLS handshake timeout, response header timeout, TLS 1.2 floor, no retry layer. Satisfies the no-timeoutless-client rule; retry layer deliberately not used (POST is non-idempotent). Wrap order is load-bearing: AuthTransport outer (adds `X-Postmark-Server-Token`), go-kit transport inner.
- `func (p *PostmarkSender) Send(_ context.Context, req *postmark.Email) error` — `_` for ctx is mandatory: postmark-go v1.0.0 has no ctx API and `unparam` with `check-exported: true` fails otherwise; the go-kit transport timeouts bound the call.
  - Body: `_, resp, err := p.client.Email.Send(req)`; on error: if `resp == nil` → `return fmt.Errorf("postmark: %w", err)`; else `return fmt.Errorf("postmark: %w (status: %d, message: %s)", err, resp.StatusCode, resp.Message)`. The `resp == nil` branch is load-bearing: postmark-go `Do()` returns a nil response on transport error (prior abandoned implementation panicked here).

### 3. `internal/mail/smtp.go` — SMTP transport (dial/auth/deliver)

- Constants (file top, single var/const block per decorder): `const smtpImplicitTLSPort = "465"`, `const smtpDialTimeout = 10 * time.Second`, `const smtpSessionTimeout = 30 * time.Second`.
- `type SMTPSender struct { host, port, login, password string; tlsCfg *tls.Config }`
- `func NewSMTP(host, port, login, password string) Sender` — returns `&SMTPSender{host: host, port: port, login: login, password: password, tlsCfg: &tls.Config{ServerName: host}}`.
- `func (s *SMTPSender) Send(ctx context.Context, req *postmark.Email) error`:
  1. `recipients, err := parseRecipients(req)` — helper below; on error return it.
  2. `msg, err := buildMessage(req)` (in smtpmsg.go, step 4); on error return it.
  3. `return s.deliver(ctx, req.From, recipients, msg)`.
- `func parseRecipients(req *postmark.Email) ([]string, error)` — build list `req.To, req.Cc, req.Bcc` joined with `", "` skipping empties (To always included even if empty so parse error surfaces), `net/mail.ParseAddressList`; on error `return nil, fmt.Errorf("mail: cannot parse recipients: %w", err)`; empty result → `errors.New("mail: no recipients")`; return each `addr.Address` (bare form, display names stripped).
- `func (s *SMTPSender) dial(ctx context.Context) (*smtp.Client, error)`:
  - `addr := net.JoinHostPort(s.host, s.port)` (IPv6-safe).
  - If `s.port == smtpImplicitTLSPort`: `d := tls.Dialer{NetDialer: &net.Dialer{Timeout: smtpDialTimeout}, Config: s.tlsCfg}`; `conn, err := d.DialContext(ctx, "tcp", addr)`. Else: `d := net.Dialer{Timeout: smtpDialTimeout}`; `conn, err := d.DialContext(ctx, "tcp", addr)`.
  - On dial error return it. `if err := conn.SetDeadline(time.Now().Add(smtpSessionTimeout)); err != nil { conn.Close(); return nil, err }` — one deadline bounds the whole sequential session (SMTP is request/response; no per-command bookkeeping).
  - `c, err := smtp.NewClient(conn, s.host)` — on error `conn.Close()` then return (stdlib does not own the conn on greeting failure); on success `return c, nil`.
- `func (s *SMTPSender) deliver(ctx context.Context, from string, recipients []string, msg []byte) error`:
  1. `c, err := s.dial(ctx)`; on error return it.
  2. `defer c.Close()` — immediately after successful dial, covers EVERY error path below (failed STARTTLS, failed AUTH, rejected MAIL/RCPT, DATA failure). After a successful `c.Quit()` the deferred `Close` returns an already-closed error which is discarded (deferred close error ignored, conn is gone either way). No fd leak.
  3. STARTTLS: `if ok, _ := c.Extension("STARTTLS"); ok && s.port != smtpImplicitTLSPort` → `if err := c.StartTLS(s.tlsCfg); err != nil { return err }`. (On 465 the connection is already TLS; Extension("STARTTLS") must not trigger a second handshake. stdlib `StartTLS` on an already-TLS client errors, so the port guard is required.)
  4. Auth: only when `s.login != ""` — `if ok, _ := c.Extension("AUTH"); !ok { return errors.New("mail: smtp server does not advertise AUTH") }`; `if err := c.Auth(smtp.PlainAuth("", s.login, s.password, s.host)); err != nil { return err }`. Arg order load-bearing: identity, username=login, password, host. stdlib `plainAuth` refuses cleartext credentials unless TLS or localhost (verified stdlib source) and checks `server.Name == host` — cleartext credential leak is impossible by construction; fail closed with "unencrypted connection" wrapped error.
  5. Envelope: `if err := c.Mail(from); err != nil { return err }`; `for _, rcpt := range recipients { if err := c.Rcpt(rcpt); err != nil { return err } }`.
  6. Data: `w, err := c.Data()`; on error return; `if _, err := w.Write(msg); err != nil { w.Close(); return err }`; `if err := w.Close(); err != nil { return err }`; `return c.Quit()`. stdlib `Data()` uses `DotWriter` (dot-stuffing handled by stdlib).
- Postmark-only fields (MessageStream, TemplateID, TemplateModel, TrackOpens, Metadata, Attachments) are deliberately ignored by the SMTP transport — they are Postmark API semantics with no SMTP meaning; scope is SMTP delivery of the same submissions, no mapping invented.

### 4. `internal/mail/smtpmsg.go` — message building

- `func buildMessage(req *postmark.Email) ([]byte, error)` — orchestrates, complexity stays under gocognit 15 by delegating:
  1. `if err := guardHeaders(req); err != nil { return nil, err }`.
  2. `if req.From == "" { return nil, errors.New("mail: empty from address") }`.
  3. `if req.TextBody == "" && req.HTMLBody == "" { return nil, errors.New("mail: empty message body") }`.
  4. `var b bytes.Buffer`; `writeHeaders(&b, req)`; `writeBody(&b, req)`.
  5. `return b.Bytes(), nil`.
- `func guardHeaders(req *postmark.Email) error` — for every value in `req.From, req.To, req.Cc, req.Bcc, req.ReplyTo, req.Tag, req.Subject` and for every `req.Headers[i].Name` AND `req.Headers[i].Value`: `if strings.ContainsAny(v, "\r\n") { return errors.New("mail: header value must not contain line breaks") }`. Additionally each Header Name: `if strings.Contains(name, ":") { return errors.New("mail: header name must not contain colon") }`. Empty strings pass (ContainsAny on "" is false).
- `func writeHeaders(b *bytes.Buffer, req *postmark.Email)` — in order, all CRLF:
  - `From: <req.From>`, `Date: <time.Now().UTC().Format(time.RFC1123Z)>`, `To: <req.To>`, `Cc: <req.Cc>` (only if non-empty), `Reply-To: <req.ReplyTo>` (only if non-empty), `X-Tag: <req.Tag>` (only if non-empty), `Subject: <mime.BEncoding.Encode("utf-8", req.Subject)>` (stdlib passes pure ASCII through unchanged, encodes only when needed), then each `req.Headers[i]` as `Name: Value`, then `Message-ID: <<crypto/rand.Text()@<domain>>` where domain = part after last `@` of `mail.ParseAddress(req.From).Address` (if parse fails, skip Message-ID silently — it is recommended, not mandatory), then `MIME-Version: 1.0`.
  - **Bcc is never written to the body** — envelope only (secrecy).
- `func writeBody(b *bytes.Buffer, req *postmark.Email)` — normalize both bodies: `\r\n` → `\n`, then `\n` → `\r\n` (via two `strings.ReplaceAll`); then:
  - both TextBody+HTMLBody non-empty: `Content-Type: multipart/alternative; boundary="<crypto/rand.Text()>"`, blank line, then `--<boundary>` + `Content-Type: text/plain; charset=utf-8` + blank + text, `--<boundary>` + `Content-Type: text/html; charset=utf-8` + blank + html, `--<boundary>--` terminator.
  - TextBody only: `Content-Type: text/plain; charset=utf-8`, blank line, text.
  - HTMLBody only: `Content-Type: text/html; charset=utf-8`, blank line, html.
- No equivalent repo utility exists for RFC822 message building (searched: none).

### 5. Config

- `internal/config/types.go` — SMTP struct becomes: `Host, Port, Login, Password, ReplyTo, From string` + `EnforceValidation bool`, doc comments single-line. Keep `From` where it is plus new fields; field order: Host, Port, Login, Password, ReplyTo, From, EnforceValidation.
- `internal/config/config.go` — SMTP block: `Host: env.String("smtp.host")`, `Port: env.String("smtp.port", defaultConfig.SMTP.Port)`, `Login: env.String("smtp.login")`, `Password: env.String("smtp.password")`, `ReplyTo: env.String("smtp.replyto")`, `From: env.String("smtp.from")` (unchanged, no fallback), `EnforceValidation: env.Bool("smtp.validation")` (unchanged).
- `internal/config/defaults.go` — SMTP block: remove dead `From: "test@ilydeen.org"` line (verified dead: `config.go` never references `defaultConfig.SMTP.From`), add `Port: "587"`. No comment needed beyond what fits.
- `internal/config/config_test.go` — add to shared `values` map: `"SMTP_HOST": "smtp.example.com"`, `"SMTP_PORT": "2525"`, `"SMTP_LOGIN": "login"`, `"SMTP_PASSWORD": "password"`, `"SMTP_REPLYTO": "reply@example.com"`. Extend `TestNew` assertions: those five values match. Add one focused test `TestNew_SMTPDefaults`: unset SMTP_PORT (save/restore via `os.Unsetenv` in the test, `t.Setenv` not used because suite pattern uses os.Setenv; use `s.T().Unsetenv`? — decision: use `t.Setenv("SMTP_PORT", "")`... exact: within the test call `os.Unsetenv("SMTP_PORT")` then `s.Equal("587", New().SMTP.Port)`. No `t.Setenv` (parallel not used; suite pattern is os.Setenv/Unsetenv).

### 6. `cmd/project/main.go` — wiring

In `initControllers` replace `pm := mail.New(cfg.Postmark.Token, cfg.Postmark.From, cfg.Postmark.ReplyTo)` with:

var sender mail.Sender
var from, replyto string
switch {
case cfg.SMTP.Host != "":
	sender = mail.NewSMTP(cfg.SMTP.Host, cfg.SMTP.Port, cfg.SMTP.Login, cfg.SMTP.Password)
	from, replyto = cfg.SMTP.From, cfg.SMTP.ReplyTo
case cfg.Postmark.Token != "":
	sender = mail.NewPostmark(cfg.Postmark.Token)
	from, replyto = cfg.Postmark.From, cfg.Postmark.ReplyTo
}
pm := mail.New(from, replyto, sender)

- SMTP wins (user decision). Neither set → `sender == nil` → `pm == nil` → downstream nil handling, same as today's empty-token behavior.
- Validator config above still reads `cfg.SMTP.From` unchanged — SMTP-validation behavior is untouched regardless of transport.
- `SMTP_HOST` set with `SMTP_FROM` unset: `from == ""` → every send fails loudly with `mail: empty from address` in the error log. Fail-closed by design; README documents that SMTP_FROM is required when SMTP_HOST is set.

### 7. README.md

In the "optional project configs" section after `PM_REPLYTO`:
- `SMTP_HOST` - SMTP server host for sending mail; when set, mail is sent via SMTP instead of Postmark; requires `SMTP_FROM`
- `SMTP_PORT` - SMTP server port, default: `587`; port `465` uses implicit TLS, other ports use STARTTLS when advertised
- `SMTP_LOGIN` - SMTP login; empty means no auth
- `SMTP_PASSWORD` - SMTP password
- `SMTP_REPLYTO` - reply-to email header for SMTP-sent mail
- Amend `SMTP_FROM` bullet: From address for SMTP-sent mail (required when `SMTP_HOST` is set) and for SMTP validation
- Amend the two `NAME_CONFIRMATION_SUBJECT/BODY` bullets: "postmark (or SMTP) configuration"

### 8. Tests (all testify suite, mirror-source-name, `TestX` entry points)

**`internal/mail/mail_test.go`** — hand-rolled fake (no mockery; the `mocks/` dir is for downstream interfaces and stays untouched):
- `fakeSender struct{ req *postmark.Email; err error }` with `func (f *fakeSender) Send(_ context.Context, req *postmark.Email) error { f.req = req; return f.err }`.
- `TestNew`: `New("from@example.com", "reply@example.com", NewPostmark("token"))` is `*Client`.
- `TestNew_Empty`: `New("from@example.com", "reply@example.com", nil)` is nil.
- `TestClient_Send`: success → `NoError`, `fs.req == req`, `req.From`/`req.ReplyTo` stamped with client values; then `fs.err = errors.New("boom")` → `EqualError(..., "boom")`.

**`internal/mail/postmark_test.go`**:
- success: `httptest.NewServer` returning 200 `{"To":"to@example.com","MessageID":"msg-1"}`; `NewPostmark("token", postmark.WithBackendURL(srv.URL))`; `Send` → NoError; assert request path `/email`, header `X-Postmark-Server-Token == "token"`, body JSON contains From/To/Subject/TextBody.
- API error: 422 `{"ErrorCode":10,"Message":"Unprocessable"}` → error contains "postmark:" and "Unprocessable".
- transport error: `srv.Close()` then `Send` → error contains "postmark:", and NO panic (exercises the `resp == nil` branch).

**`internal/mail/smtpmsg_test.go`** — pure table tests on `buildMessage` (+ `parseRecipients`):
- text-only: From/To/Subject/Date/MIME-Version/Content-Type text/plain, body after `\r\n\r\n`.
- html-only: Content-Type text/html.
- both: multipart/alternative, boundary present, two parts, closing `--<boundary>--`.
- non-ASCII subject (`Update 🚀`): encoded `=?utf-8?b?` form present, raw emoji absent.
- CRLF injection: Subject/Tag/Header Value/Header Name each with `\r\n` → error "mail: header value must not contain line breaks"; Header Name containing `:` → "mail: header name must not contain colon".
- Bcc present in req → absent from message body.
- empty From → "mail: empty from address"; empty both bodies → "mail: empty message body"; empty To → "mail: no recipients"; To with display name `Name <a@b.c>` → envelope parse yields bare `a@b.c`.
- Message-ID present with From domain; LF-only body normalized to CRLF.

**`internal/mail/smtp_test.go`** — in-package fake SMTP server:
- `fakeSMTP` struct: `net.Listen("tcp", "127.0.0.1:0")`, per-conn goroutine, `bufio` line reader, scripted switch on command (`EHLO` → `250-...\r\n250-AUTH PLAIN\r\n250-STARTTLS\r\n250 OK` variants per test config; `AUTH` → record base64 payload, `235`; `MAIL`/`RCPT` → record, `250`; `DATA` → `354` then read until `\r\n.\r\n`, record payload, `250`; `QUIT` → `221`); fields `authPayload, mailFrom, rcptTo []string, data string`, `starttls bool`, `implicitTLS bool` + `tls.Config` for TLS modes.
- cert helper `genCert()` (~12 lines): `ecdsa.GenerateKey(elliptic.P256(), nil)` (Go 1.26: reader arg ignored, pass nil), `x509.CreateCertificate` self-signed, returns `tls.Certificate`.
- TLS override helper (same package): `withInsecureTLS(s Sender) *SMTPSender` → assert `*SMTPSender`, set `tlsCfg.InsecureSkipVerify = true`, return it. gosec G402 excluded for `_test.go` (verified `.golangci.yml` exclusions).
- e2e plaintext (587-style, no STARTTLS advertised): `NewSMTP("127.0.0.1", port, "login", "password")` → Send succeeds (stdlib `isLocalhost` exemption lets AUTH PLAIN proceed on cleartext loopback, verified stdlib source); assert `authPayload` decodes to `\x00login\x00password` (base64 decode), `mailFrom == [from]`, `rcptTo` contains To, Cc, and Bcc addresses (Bcc in envelope), DATA payload has headers + CRLF + ends `\r\n.\r\n`, NO `Bcc:` header in DATA.
- no-auth: login empty → no AUTH command recorded, send succeeds.
- 465: listener wrapped `tls.NewListener(l, cfg)` with genCert; client `NewSMTP("127.0.0.1", "465", "", "")` + insecure override → send succeeds; server never saw STARTTLS command.
- STARTTLS upgrade: plain listener advertising STARTTLS; on STARTTLS cmd → `220`, wrap `tls.Server(rawConn, cfg)`, handshake, continue reading TLS frames; client (insecure override) upgrades and completes MAIL/RCPT/DATA/QUIT.
- ctx canceled: `context.WithCancel` → cancel before Send → error non-nil, fast (dial fails), no panic.
- RCPT rejected: server scripted `550` for RCPT → `Send` error non-nil (connection closed via defer, no leak; assert server observed connection close by server conn read returning EOF — keep simple: error non-nil suffices).

### 9. Not touched (verified, no changes needed)

- `internal/sub/handler.go`, `internal/sub/ext/ext.go`: their local `EmailSender` interfaces are satisfied structurally by `*mail.Client` (Send signature unchanged).
- `mocks/`: `EmailSender` mock still matches; no new interface introduced downstream. `just mocks` recipe is stale (mockery v3 installed, v2 `--all` flag) — not this change's scope.
- `go.mod`/`vendor/`: all imports are stdlib or already-vendored direct deps (`go-kit`, `postmark-go`). No tidy/vendor run.
- `docs/`: regenerated by `just lint`, never hand-edited.

## Critical files & anchors

- `internal/mail/mail.go` — full rewrite: Sender interface + New(from, replyto, sender) + Client.Send delegation.
- `internal/mail/smtp.go` (new) — dial/deliver: `defer c.Close()` fd-leak fix, 465 guard on STARTTLS, PlainAuth arg order, session deadline.
- `internal/mail/smtpmsg.go` (new) — buildMessage split (gocognit guard): guardHeaders/writeHeaders/writeBody, Bcc secrecy, BEncoding subject.
- `cmd/project/main.go:190` — the single mail.New callsite; sender switch, SMTP-wins precedence.
- `internal/config/{types,config,defaults,config_test}.go` — SMTP struct/env/default "587"/tests; dead `From` default removal.

## Verification

Prerequisites: Go 1.26, cwd repo root, vendor mode default (`vendor/` present).

1. `just build` — whole tree compiles with the new wiring.
2. `go test ./internal/mail/... -v -count=1` — new suites green: mail delegation, postmark httptest (token header, `/email`, nil-resp no-panic), smtp e2e (AUTH PLAIN payload, envelope incl. Bcc, dot-termination, 465, STARTTLS, ctx-cancel, RCPT-reject), buildMessage tables (injection, encoding, multipart, Bcc secrecy, empty-from/body/recipients).
3. `go test ./internal/config/... -v -count=1` — SMTP env parsing + Port default 587.
4. `go test ./... -count=1` — full suite green.
5. `just lint` — comment-guard + golangci clean; docs/ regenerated diff belongs to the change.
6. Behavior proof (beyond suites): run a live-ish check with a scratch listener — `go test ./internal/mail -run TestSMTP -v` output shows the e2e dialogue assertions passing (AUTH payload, MAIL FROM, RCPT TO incl. Bcc, DATA dot-terminated); that IS the transport proof.
7. Repo rule: NEVER `git commit`.

## Assumptions & contingencies

- SMTP wins when both configured; 465 implicit TLS; no redteam — user-confirmed.
- `SMTP_FROM` unset with HOST set → per-send `mail: empty from address` error (fail-closed, documented in README). If the user prefers startup validation instead, that is a config-layer feature outside this ask.
- If smtp_test.go exceeds the 500 hard limit despite the split, move the fake server into its own `fake_smtp_test.go` (test-only file, still package mail) — pre-decided, mirror naming maintained for the suites.
- STARTTLS fake server handshake flakiness (timing): the fake is single-conn sequential with a 30s session deadline on the client side; if the STARTTLS test is flaky in CI, read the client error and extend the fake's EHLO response to include `250 HELP` — do not add sleeps (repo test rule).