From ef7ba37e2656a8fdb2a50b563c45ccfd959aeb60 Mon Sep 17 00:00:00 2001 From: "hoffman.peter" Date: Mon, 14 Sep 2026 08:15:30 +0200 Subject: [PATCH 01/10] fix(instance-admin): add SMTP test email endpoint Co-Authored-By: Codex --- apps/api/internal/handler/instance.go | 82 +++++++++++++++++++++++++++ apps/api/internal/mail/mail.go | 40 ++++++++----- apps/api/internal/router/router.go | 3 +- 3 files changed, 109 insertions(+), 16 deletions(-) diff --git a/apps/api/internal/handler/instance.go b/apps/api/internal/handler/instance.go index e4c1bd86..6a6428e9 100644 --- a/apps/api/internal/handler/instance.go +++ b/apps/api/internal/handler/instance.go @@ -7,12 +7,15 @@ import ( "encoding/json" "errors" "io" + "log/slog" "net/http" "net/url" + "strconv" "strings" "github.com/Devlaner/devlane/api/internal/auth" "github.com/Devlaner/devlane/api/internal/crypto" + "github.com/Devlaner/devlane/api/internal/mail" "github.com/Devlaner/devlane/api/internal/middleware" "github.com/Devlaner/devlane/api/internal/model" "github.com/Devlaner/devlane/api/internal/store" @@ -40,6 +43,7 @@ type InstanceSettingsHandler struct { Settings *store.InstanceSettingStore Admins *store.InstanceAdminStore Users *store.UserStore + Log *slog.Logger // OnSectionUpdated, if set, is invoked after a successful update with the // section key. Used for hot-reload of integration clients (e.g. github_app) // so the new credentials take effect without an API restart. @@ -617,3 +621,81 @@ func (h *InstanceSettingsHandler) UnsplashSearch(c *gin.Context) { } c.JSON(http.StatusOK, gin.H{"results": results}) } + +type sendTestEmailRequest struct { + Host string `json:"host" binding:"required"` + Port string `json:"port" binding:"required"` + SenderEmail string `json:"sender_email" binding:"required,email"` + Security string `json:"security" binding:"required"` + Username string `json:"username"` + Password string `json:"password"` +} + +// SendTestEmail sends a test email using the SMTP values supplied by the +// instance-admin form. The values are not persisted by this endpoint. +// POST /api/instance/settings/email/test +func (h *InstanceSettingsHandler) SendTestEmail(c *gin.Context) { + if !h.requireInstanceAdmin(c) { + return + } + + var req sendTestEmailRequest + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid email settings", "detail": err.Error()}) + return + } + + host := strings.TrimSpace(req.Host) + if host == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "SMTP host is required"}) + return + } + + port, err := strconv.Atoi(strings.TrimSpace(req.Port)) + if err != nil || port < 1 || port > 65535 { + c.JSON(http.StatusBadRequest, gin.H{"error": "SMTP port must be between 1 and 65535"}) + return + } + + security := strings.TrimSpace(req.Security) + switch security { + case "TLS", "SSL", "None": + default: + c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid email security setting"}) + return + } + + user := middleware.GetUser(c) + if user == nil || user.Email == nil || strings.TrimSpace(*user.Email) == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "Your account does not have an email address"}) + return + } + + recipient := strings.TrimSpace(*user.Email) + cfg := &mail.SMTPSettings{ + Host: host, + Port: port, + SenderEmail: strings.TrimSpace(req.SenderEmail), + Security: security, + Username: strings.TrimSpace(req.Username), + Password: req.Password, + } + + if err := mail.SendWithSMTPSettings( + cfg, + recipient, + "Devlane SMTP test email", + "This is a test email from Devlane. Your SMTP settings are working.", + h.Log, + ); err != nil { + if h.Log != nil { + h.Log.Error("send SMTP test email", "error", err, "recipient", recipient) + } + c.JSON(http.StatusBadGateway, gin.H{ + "error": "Failed to send test email. Check the SMTP settings and server logs.", + }) + return + } + + c.JSON(http.StatusOK, gin.H{"message": "Test email sent"}) +} diff --git a/apps/api/internal/mail/mail.go b/apps/api/internal/mail/mail.go index 7d2bfbb4..84fc0cbd 100644 --- a/apps/api/internal/mail/mail.go +++ b/apps/api/internal/mail/mail.go @@ -13,7 +13,7 @@ import ( "github.com/Devlaner/devlane/api/internal/store" ) -type smtpSettings struct { +type SMTPSettings struct { Host string Port int SenderEmail string @@ -22,7 +22,7 @@ type smtpSettings struct { Password string } -func getEmailSettings(ctx context.Context, s *store.InstanceSettingStore) (*smtpSettings, error) { +func getEmailSettings(ctx context.Context, s *store.InstanceSettingStore) (*SMTPSettings, error) { row, err := s.Get(ctx, "email") if err != nil || row == nil { return nil, fmt.Errorf("email settings not found") @@ -55,7 +55,7 @@ func getEmailSettings(ctx context.Context, s *store.InstanceSettingStore) (*smtp if host == "" { return nil, fmt.Errorf("email host not configured") } - return &smtpSettings{ + return &SMTPSettings{ Host: host, Port: port, SenderEmail: strings.TrimSpace(sender), @@ -78,24 +78,34 @@ func NewSMTPEmailSender(instanceSettings *store.InstanceSettingStore, log *slog. LogSkip(log, "instance email not configured", to, err) return err } - from := cfg.SenderEmail - if from == "" { - from = cfg.Username - } - if from == "" { - LogSkip(log, "sender_email and username empty", to, fmt.Errorf("sender not set")) - return fmt.Errorf("sender email not configured") - } - addr := fmt.Sprintf("%s:%d", cfg.Host, cfg.Port) - auth := smtp.PlainAuth("", cfg.Username, cfg.Password, cfg.Host) - msg := buildMessage(to, from, subject, body) - if err := sendMailWithConfig(addr, cfg.Host, cfg.Port, cfg.Security, auth, from, to, msg); err != nil { + if err := SendWithSMTPSettings(cfg, to, subject, body, log); err != nil { return err } return nil } } +func SendWithSMTPSettings(cfg *SMTPSettings, to, subject, body string, log *slog.Logger) error { + if cfg == nil { + return fmt.Errorf("SMTP settings not configured") + } + from := cfg.SenderEmail + if from == "" { + from = cfg.Username + } + if from == "" { + LogSkip(log, "sender_email and username empty", to, fmt.Errorf("sender not set")) + return fmt.Errorf("sender email not configured") + } + addr := fmt.Sprintf("%s:%d", cfg.Host, cfg.Port) + auth := smtp.PlainAuth("", cfg.Username, cfg.Password, cfg.Host) + msg := buildMessage(to, from, subject, body) + if err := sendMailWithConfig(addr, cfg.Host, cfg.Port, cfg.Security, auth, from, to, msg); err != nil { + return err + } + return nil +} + // sendMailWithConfig sends email using smtp.SendMail or, for port 465 with SSL, // an explicit TLS connection (smtp.SendMail only supports STARTTLS). func sendMailWithConfig(addr, host string, port int, security string, auth smtp.Auth, from, to string, msg []byte) error { diff --git a/apps/api/internal/router/router.go b/apps/api/internal/router/router.go index ad7620a7..39bd66de 100644 --- a/apps/api/internal/router/router.go +++ b/apps/api/internal/router/router.go @@ -140,7 +140,7 @@ func New(cfg Config) (*gin.Engine, *service.ImporterService) { r.GET("/api/invitations/by-token/", invitationHandler.GetInviteByToken) r.POST("/api/invitations/decline/", invitationHandler.DeclineInviteByToken) - instanceSettingsHandler := &handler.InstanceSettingsHandler{Settings: instanceSettingStore, Admins: instanceAdminStore, Users: userStore} + instanceSettingsHandler := &handler.InstanceSettingsHandler{Settings: instanceSettingStore, Admins: instanceAdminStore, Users: userStore, Log: cfg.Log} // Services workspaceSvc := service.NewWorkspaceService(workspaceStore, workspaceInviteStore, userStore) @@ -313,6 +313,7 @@ func New(cfg Config) (*gin.Engine, *service.ImporterService) { api.DELETE("/workspaces/:slug/favorites/:favId/", favoriteHandler.DeleteFavorite) api.GET("/instance/settings/", instanceSettingsHandler.GetSettings) api.PATCH("/instance/settings/:key", instanceSettingsHandler.UpdateSetting) + api.POST("/instance/settings/email/test", instanceSettingsHandler.SendTestEmail) api.GET("/instance/unsplash/search", instanceSettingsHandler.UnsplashSearch) // Instance-admin management (admin-gated inside the handler). api.GET("/instance/admins/", instanceSettingsHandler.ListAdmins) From fbff5e3403c01d28302e959330a41ff552ed21bc Mon Sep 17 00:00:00 2001 From: "hoffman.peter" Date: Tue, 15 Sep 2026 05:48:10 +0200 Subject: [PATCH 02/10] fix(instance-admin): enable SMTP test email UI Co-Authored-By: Codex --- apps/web/src/api/types.ts | 10 +++ apps/web/src/i18n/locales/en/translation.json | 2 + .../instance-admin/InstanceAdminEmailPage.tsx | 85 ++++++++++++++++--- apps/web/src/services/instanceService.ts | 11 ++- 4 files changed, 95 insertions(+), 13 deletions(-) diff --git a/apps/web/src/api/types.ts b/apps/web/src/api/types.ts index 72ee42ca..54062f3a 100644 --- a/apps/web/src/api/types.ts +++ b/apps/web/src/api/types.ts @@ -556,6 +556,16 @@ export interface InstanceEmailSection { password?: string; } +/** SMTP settings used only to send a test email; they are not persisted. */ +export interface InstanceEmailTestRequest { + host: string; + port: string; + sender_email: string; + security: string; + username: string; + password: string; +} + /** Auth section shape */ export interface InstanceAuthSection { allow_public_signup?: boolean; diff --git a/apps/web/src/i18n/locales/en/translation.json b/apps/web/src/i18n/locales/en/translation.json index 2f089869..8ac09139 100644 --- a/apps/web/src/i18n/locales/en/translation.json +++ b/apps/web/src/i18n/locales/en/translation.json @@ -877,10 +877,12 @@ "instanceAdmin.email.securityNone": "None", "instanceAdmin.email.senderEmail": "Sender's email address", "instanceAdmin.email.senderEmailHint": "This is the email address your users will see when getting emails from this instance. You will need to verify this address.", + "instanceAdmin.email.sendingTest": "Sending test email…", "instanceAdmin.email.sendTest": "Send test email", "instanceAdmin.email.setPassword": "Set password", "instanceAdmin.email.showPassword": "Show password", "instanceAdmin.email.subtitle": "Devlane can send useful emails to you and your users from your own instance without talking to the Internet. Set it up below and please test your settings before you save them.", + "instanceAdmin.email.testSuccess": "Test email sent to your account email.", "instanceAdmin.email.title": "Secure emails from your own instance", "instanceAdmin.email.username": "Username", "instanceAdmin.general.documentTitle": "General settings", diff --git a/apps/web/src/pages/instance-admin/InstanceAdminEmailPage.tsx b/apps/web/src/pages/instance-admin/InstanceAdminEmailPage.tsx index 38c9f900..f5fdffe5 100644 --- a/apps/web/src/pages/instance-admin/InstanceAdminEmailPage.tsx +++ b/apps/web/src/pages/instance-admin/InstanceAdminEmailPage.tsx @@ -1,8 +1,8 @@ import { useEffect, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { Button, IconEye, IconEyeOff, Skeleton } from '../../components/ui'; -import { instanceSettingsService } from '../../services/instanceService'; -import { getApiErrorMessage } from '../../api/client'; +import { instanceSettingsService } from '../../services'; +import { getApiErrorMessage } from '../../api'; import { useDocumentTitle } from '../../hooks/useDocumentTitle'; import type { InstanceEmailSection } from '../../api/types'; @@ -21,11 +21,25 @@ export function InstanceAdminEmailPage() { const [showSmtpPassword, setShowSmtpPassword] = useState(false); const [smtpPasswordLocal, setSmtpPasswordLocal] = useState(undefined); // undefined = show stored mask, '' = user cleared, string = user typed const [error, setError] = useState(''); + const [testing, setTesting] = useState(false); + const [testSuccess, setTestSuccess] = useState(''); useDocumentTitle(t('instanceAdmin.email.documentTitle', 'Email')); const smtpPasswordDisplay = smtpPasswordLocal !== undefined ? smtpPasswordLocal : (email.password ?? ''); + const updateEmail = (changes: Partial) => { + setEmail((previous) => ({ ...previous, ...changes })); + setTestSuccess(''); + }; + + const portText = (email.port ?? '').trim(); + const smtpPort = Number(portText); + const hasValidPort = + /^\d+$/.test(portText) && Number.isInteger(smtpPort) && smtpPort >= 1 && smtpPort <= 65535; + const hasValidSender = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test((email.sender_email ?? '').trim()); + const canSendTest = + !saving && !testing && (email.host ?? '').trim() !== '' && hasValidPort && hasValidSender; useEffect(() => { let cancelled = false; instanceSettingsService @@ -72,6 +86,31 @@ export function InstanceAdminEmailPage() { .finally(() => setSaving(false)); }; + const handleSendTest = () => { + setError(''); + setTestSuccess(''); + setTesting(true); + + const passwordToSend = smtpPasswordLocal !== undefined ? smtpPasswordLocal : email.password; + + instanceSettingsService + .sendTestEmail({ + host: (email.host ?? '').trim(), + port: (email.port ?? '').trim(), + sender_email: (email.sender_email ?? '').trim(), + security: email.security ?? 'TLS', + username: (email.username ?? '').trim(), + password: passwordToSend ?? '', + }) + .then(() => { + setTestSuccess( + t('instanceAdmin.email.testSuccess', 'Test email sent to your account email.'), + ); + }) + .catch((err) => setError(getApiErrorMessage(err))) + .finally(() => setTesting(false)); + }; + if (loading) { return (
@@ -139,7 +178,17 @@ export function InstanceAdminEmailPage() {

- {error &&

{error}

} + {error && ( +

+ {error} +

+ )} + + {testSuccess && ( +

+ {testSuccess} +

+ )}
@@ -148,7 +197,7 @@ export function InstanceAdminEmailPage() { setEmail((p) => ({ ...p, host: e.target.value }))} + onChange={(e) => updateEmail({ host: e.target.value })} className="mt-0.5 block w-full rounded border border-(--border-subtle) bg-(--bg-surface-1) px-2.5 py-1.5 text-xs text-(--txt-primary) focus:outline-none" /> @@ -157,7 +206,7 @@ export function InstanceAdminEmailPage() { setEmail((p) => ({ ...p, port: e.target.value }))} + onChange={(e) => updateEmail({ port: e.target.value })} className="mt-0.5 block w-full rounded border border-(--border-subtle) bg-(--bg-surface-1) px-2.5 py-1.5 text-xs text-(--txt-primary) focus:outline-none" /> @@ -167,7 +216,7 @@ export function InstanceAdminEmailPage() { setEmail((p) => ({ ...p, sender_email: e.target.value }))} + onChange={(e) => updateEmail({ sender_email: e.target.value })} className="mt-0.5 block w-full rounded border border-(--border-subtle) bg-(--bg-surface-1) px-2.5 py-1.5 text-xs text-(--txt-primary) focus:outline-none" />

@@ -181,7 +230,7 @@ export function InstanceAdminEmailPage() { {t('instanceAdmin.email.security', 'Email security')} setEmail((p) => ({ ...p, username: e.target.value }))} + onChange={(e) => updateEmail({ username: e.target.value })} className="mt-0.5 block w-full rounded border border-(--border-subtle) bg-(--bg-surface-1) px-2.5 py-1.5 text-xs text-(--txt-primary) focus:outline-none" /> @@ -218,7 +267,10 @@ export function InstanceAdminEmailPage() { setSmtpPasswordLocal(e.target.value)} + onChange={(e) => { + setSmtpPasswordLocal(e.target.value); + setTestSuccess(''); + }} onFocus={() => { // Only copy the loaded password into local edit state when it is non-empty. // Otherwise we would set local state to "" and the next save would send an empty @@ -252,11 +304,20 @@ export function InstanceAdminEmailPage() {

- -
diff --git a/apps/web/src/services/instanceService.ts b/apps/web/src/services/instanceService.ts index 9fe1e97c..b522c00c 100644 --- a/apps/web/src/services/instanceService.ts +++ b/apps/web/src/services/instanceService.ts @@ -1,4 +1,4 @@ -import { apiClient } from '../api/client'; +import { apiClient } from '../api'; import type { InstanceSetupStatusResponse, InstanceSetupRequest, @@ -6,6 +6,7 @@ import type { InstanceSettingsResponse, InstanceSettingSectionValue, InstanceAdminApiResponse, + InstanceEmailTestRequest, } from '../api/types'; /** @@ -50,6 +51,14 @@ export const instanceSettingsService = { return data; }, + async sendTestEmail(payload: InstanceEmailTestRequest): Promise<{ message: string }> { + const { data } = await apiClient.post<{ message: string }>( + '/api/instance/settings/email/test', + payload, + ); + return data; + }, + /** Search Unsplash (uses instance image API key). GET /api/instance/unsplash/search?q= */ async unsplashSearch(q: string): Promise<{ results: UnsplashSearchResult[] }> { const { data } = await apiClient.get<{ results: UnsplashSearchResult[] }>( From e849ef69dd0354c23ff383aa6d2d84e1e674aa72 Mon Sep 17 00:00:00 2001 From: "hoffman.peter" Date: Tue, 15 Sep 2026 05:48:10 +0200 Subject: [PATCH 03/10] fix(instance-admin): enable SMTP test email UI Co-Authored-By: Codex --- apps/api/internal/mail/mail.go | 11 ++- apps/web/src/api/types.ts | 10 +++ apps/web/src/i18n/locales/en/translation.json | 2 + .../instance-admin/InstanceAdminEmailPage.tsx | 85 ++++++++++++++++--- apps/web/src/services/instanceService.ts | 11 ++- 5 files changed, 103 insertions(+), 16 deletions(-) diff --git a/apps/api/internal/mail/mail.go b/apps/api/internal/mail/mail.go index 84fc0cbd..eb819684 100644 --- a/apps/api/internal/mail/mail.go +++ b/apps/api/internal/mail/mail.go @@ -98,7 +98,10 @@ func SendWithSMTPSettings(cfg *SMTPSettings, to, subject, body string, log *slog return fmt.Errorf("sender email not configured") } addr := fmt.Sprintf("%s:%d", cfg.Host, cfg.Port) - auth := smtp.PlainAuth("", cfg.Username, cfg.Password, cfg.Host) + var auth smtp.Auth + if cfg.Username != "" || cfg.Password != "" { + auth = smtp.PlainAuth("", cfg.Username, cfg.Password, cfg.Host) + } msg := buildMessage(to, from, subject, body) if err := sendMailWithConfig(addr, cfg.Host, cfg.Port, cfg.Security, auth, from, to, msg); err != nil { return err @@ -121,8 +124,10 @@ func sendMailWithConfig(addr, host string, port int, security string, auth smtp. return err } defer client.Close() - if err := client.Auth(auth); err != nil { - return err + if auth != nil { + if err := client.Auth(auth); err != nil { + return err + } } if err := client.Mail(from); err != nil { return err diff --git a/apps/web/src/api/types.ts b/apps/web/src/api/types.ts index 72ee42ca..54062f3a 100644 --- a/apps/web/src/api/types.ts +++ b/apps/web/src/api/types.ts @@ -556,6 +556,16 @@ export interface InstanceEmailSection { password?: string; } +/** SMTP settings used only to send a test email; they are not persisted. */ +export interface InstanceEmailTestRequest { + host: string; + port: string; + sender_email: string; + security: string; + username: string; + password: string; +} + /** Auth section shape */ export interface InstanceAuthSection { allow_public_signup?: boolean; diff --git a/apps/web/src/i18n/locales/en/translation.json b/apps/web/src/i18n/locales/en/translation.json index 2f089869..8ac09139 100644 --- a/apps/web/src/i18n/locales/en/translation.json +++ b/apps/web/src/i18n/locales/en/translation.json @@ -877,10 +877,12 @@ "instanceAdmin.email.securityNone": "None", "instanceAdmin.email.senderEmail": "Sender's email address", "instanceAdmin.email.senderEmailHint": "This is the email address your users will see when getting emails from this instance. You will need to verify this address.", + "instanceAdmin.email.sendingTest": "Sending test email…", "instanceAdmin.email.sendTest": "Send test email", "instanceAdmin.email.setPassword": "Set password", "instanceAdmin.email.showPassword": "Show password", "instanceAdmin.email.subtitle": "Devlane can send useful emails to you and your users from your own instance without talking to the Internet. Set it up below and please test your settings before you save them.", + "instanceAdmin.email.testSuccess": "Test email sent to your account email.", "instanceAdmin.email.title": "Secure emails from your own instance", "instanceAdmin.email.username": "Username", "instanceAdmin.general.documentTitle": "General settings", diff --git a/apps/web/src/pages/instance-admin/InstanceAdminEmailPage.tsx b/apps/web/src/pages/instance-admin/InstanceAdminEmailPage.tsx index 38c9f900..f5fdffe5 100644 --- a/apps/web/src/pages/instance-admin/InstanceAdminEmailPage.tsx +++ b/apps/web/src/pages/instance-admin/InstanceAdminEmailPage.tsx @@ -1,8 +1,8 @@ import { useEffect, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { Button, IconEye, IconEyeOff, Skeleton } from '../../components/ui'; -import { instanceSettingsService } from '../../services/instanceService'; -import { getApiErrorMessage } from '../../api/client'; +import { instanceSettingsService } from '../../services'; +import { getApiErrorMessage } from '../../api'; import { useDocumentTitle } from '../../hooks/useDocumentTitle'; import type { InstanceEmailSection } from '../../api/types'; @@ -21,11 +21,25 @@ export function InstanceAdminEmailPage() { const [showSmtpPassword, setShowSmtpPassword] = useState(false); const [smtpPasswordLocal, setSmtpPasswordLocal] = useState(undefined); // undefined = show stored mask, '' = user cleared, string = user typed const [error, setError] = useState(''); + const [testing, setTesting] = useState(false); + const [testSuccess, setTestSuccess] = useState(''); useDocumentTitle(t('instanceAdmin.email.documentTitle', 'Email')); const smtpPasswordDisplay = smtpPasswordLocal !== undefined ? smtpPasswordLocal : (email.password ?? ''); + const updateEmail = (changes: Partial) => { + setEmail((previous) => ({ ...previous, ...changes })); + setTestSuccess(''); + }; + + const portText = (email.port ?? '').trim(); + const smtpPort = Number(portText); + const hasValidPort = + /^\d+$/.test(portText) && Number.isInteger(smtpPort) && smtpPort >= 1 && smtpPort <= 65535; + const hasValidSender = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test((email.sender_email ?? '').trim()); + const canSendTest = + !saving && !testing && (email.host ?? '').trim() !== '' && hasValidPort && hasValidSender; useEffect(() => { let cancelled = false; instanceSettingsService @@ -72,6 +86,31 @@ export function InstanceAdminEmailPage() { .finally(() => setSaving(false)); }; + const handleSendTest = () => { + setError(''); + setTestSuccess(''); + setTesting(true); + + const passwordToSend = smtpPasswordLocal !== undefined ? smtpPasswordLocal : email.password; + + instanceSettingsService + .sendTestEmail({ + host: (email.host ?? '').trim(), + port: (email.port ?? '').trim(), + sender_email: (email.sender_email ?? '').trim(), + security: email.security ?? 'TLS', + username: (email.username ?? '').trim(), + password: passwordToSend ?? '', + }) + .then(() => { + setTestSuccess( + t('instanceAdmin.email.testSuccess', 'Test email sent to your account email.'), + ); + }) + .catch((err) => setError(getApiErrorMessage(err))) + .finally(() => setTesting(false)); + }; + if (loading) { return (
@@ -139,7 +178,17 @@ export function InstanceAdminEmailPage() {

- {error &&

{error}

} + {error && ( +

+ {error} +

+ )} + + {testSuccess && ( +

+ {testSuccess} +

+ )}
@@ -148,7 +197,7 @@ export function InstanceAdminEmailPage() { setEmail((p) => ({ ...p, host: e.target.value }))} + onChange={(e) => updateEmail({ host: e.target.value })} className="mt-0.5 block w-full rounded border border-(--border-subtle) bg-(--bg-surface-1) px-2.5 py-1.5 text-xs text-(--txt-primary) focus:outline-none" /> @@ -157,7 +206,7 @@ export function InstanceAdminEmailPage() { setEmail((p) => ({ ...p, port: e.target.value }))} + onChange={(e) => updateEmail({ port: e.target.value })} className="mt-0.5 block w-full rounded border border-(--border-subtle) bg-(--bg-surface-1) px-2.5 py-1.5 text-xs text-(--txt-primary) focus:outline-none" /> @@ -167,7 +216,7 @@ export function InstanceAdminEmailPage() { setEmail((p) => ({ ...p, sender_email: e.target.value }))} + onChange={(e) => updateEmail({ sender_email: e.target.value })} className="mt-0.5 block w-full rounded border border-(--border-subtle) bg-(--bg-surface-1) px-2.5 py-1.5 text-xs text-(--txt-primary) focus:outline-none" />

@@ -181,7 +230,7 @@ export function InstanceAdminEmailPage() { {t('instanceAdmin.email.security', 'Email security')} setEmail((p) => ({ ...p, username: e.target.value }))} + onChange={(e) => updateEmail({ username: e.target.value })} className="mt-0.5 block w-full rounded border border-(--border-subtle) bg-(--bg-surface-1) px-2.5 py-1.5 text-xs text-(--txt-primary) focus:outline-none" /> @@ -218,7 +267,10 @@ export function InstanceAdminEmailPage() { setSmtpPasswordLocal(e.target.value)} + onChange={(e) => { + setSmtpPasswordLocal(e.target.value); + setTestSuccess(''); + }} onFocus={() => { // Only copy the loaded password into local edit state when it is non-empty. // Otherwise we would set local state to "" and the next save would send an empty @@ -252,11 +304,20 @@ export function InstanceAdminEmailPage() {

- -
diff --git a/apps/web/src/services/instanceService.ts b/apps/web/src/services/instanceService.ts index 9fe1e97c..b522c00c 100644 --- a/apps/web/src/services/instanceService.ts +++ b/apps/web/src/services/instanceService.ts @@ -1,4 +1,4 @@ -import { apiClient } from '../api/client'; +import { apiClient } from '../api'; import type { InstanceSetupStatusResponse, InstanceSetupRequest, @@ -6,6 +6,7 @@ import type { InstanceSettingsResponse, InstanceSettingSectionValue, InstanceAdminApiResponse, + InstanceEmailTestRequest, } from '../api/types'; /** @@ -50,6 +51,14 @@ export const instanceSettingsService = { return data; }, + async sendTestEmail(payload: InstanceEmailTestRequest): Promise<{ message: string }> { + const { data } = await apiClient.post<{ message: string }>( + '/api/instance/settings/email/test', + payload, + ); + return data; + }, + /** Search Unsplash (uses instance image API key). GET /api/instance/unsplash/search?q= */ async unsplashSearch(q: string): Promise<{ results: UnsplashSearchResult[] }> { const { data } = await apiClient.get<{ results: UnsplashSearchResult[] }>( From 6a4e837af19332fa29fb69d9ba1fdb854bce6203 Mon Sep 17 00:00:00 2001 From: "hoffman.peter" Date: Tue, 15 Sep 2026 10:44:28 +0200 Subject: [PATCH 04/10] chore(migrations): renumber Slack integration migrations --- ...ack_integration.down.sql => 000014_slack_integration.down.sql} | 0 ...3_slack_integration.up.sql => 000014_slack_integration.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename apps/api/migrations/{000013_slack_integration.down.sql => 000014_slack_integration.down.sql} (100%) rename apps/api/migrations/{000013_slack_integration.up.sql => 000014_slack_integration.up.sql} (100%) diff --git a/apps/api/migrations/000013_slack_integration.down.sql b/apps/api/migrations/000014_slack_integration.down.sql similarity index 100% rename from apps/api/migrations/000013_slack_integration.down.sql rename to apps/api/migrations/000014_slack_integration.down.sql diff --git a/apps/api/migrations/000013_slack_integration.up.sql b/apps/api/migrations/000014_slack_integration.up.sql similarity index 100% rename from apps/api/migrations/000013_slack_integration.up.sql rename to apps/api/migrations/000014_slack_integration.up.sql From 5cd2d56cb2f9fff4e05c94818e43cd46b3c487fb Mon Sep 17 00:00:00 2001 From: "hoffman.peter" Date: Tue, 15 Sep 2026 10:49:46 +0200 Subject: [PATCH 05/10] chore(mail): add SMTP sender docstring --- apps/api/internal/mail/mail.go | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/api/internal/mail/mail.go b/apps/api/internal/mail/mail.go index eb819684..d4452c35 100644 --- a/apps/api/internal/mail/mail.go +++ b/apps/api/internal/mail/mail.go @@ -85,6 +85,7 @@ func NewSMTPEmailSender(instanceSettings *store.InstanceSettingStore, log *slog. } } +// SendWithSMTPSettings sends an email using the supplied SMTP settings without persisting them. func SendWithSMTPSettings(cfg *SMTPSettings, to, subject, body string, log *slog.Logger) error { if cfg == nil { return fmt.Errorf("SMTP settings not configured") From c945ef2504abafc30a575deec738cb1cf0e1375d Mon Sep 17 00:00:00 2001 From: "hoffman.peter" Date: Wed, 16 Sep 2026 17:48:07 +0200 Subject: [PATCH 06/10] fix(mail): add SMTP operation timeouts --- apps/api/internal/handler/instance.go | 1 + apps/api/internal/mail/mail.go | 112 +++++++++++++++++++------- 2 files changed, 83 insertions(+), 30 deletions(-) diff --git a/apps/api/internal/handler/instance.go b/apps/api/internal/handler/instance.go index 6a6428e9..7ef7ef1b 100644 --- a/apps/api/internal/handler/instance.go +++ b/apps/api/internal/handler/instance.go @@ -682,6 +682,7 @@ func (h *InstanceSettingsHandler) SendTestEmail(c *gin.Context) { } if err := mail.SendWithSMTPSettings( + c.Request.Context(), cfg, recipient, "Devlane SMTP test email", diff --git a/apps/api/internal/mail/mail.go b/apps/api/internal/mail/mail.go index d4452c35..baa66782 100644 --- a/apps/api/internal/mail/mail.go +++ b/apps/api/internal/mail/mail.go @@ -5,9 +5,11 @@ import ( "crypto/tls" "fmt" "log/slog" + "net" "net/smtp" "strconv" "strings" + "time" "github.com/Devlaner/devlane/api/internal/crypto" "github.com/Devlaner/devlane/api/internal/store" @@ -78,19 +80,28 @@ func NewSMTPEmailSender(instanceSettings *store.InstanceSettingStore, log *slog. LogSkip(log, "instance email not configured", to, err) return err } - if err := SendWithSMTPSettings(cfg, to, subject, body, log); err != nil { + if err := SendWithSMTPSettings(ctx, cfg, to, subject, body, log); err != nil { return err } return nil } } +const smtpSendTimeout = 15 * time.Second + // SendWithSMTPSettings sends an email using the supplied SMTP settings without persisting them. -func SendWithSMTPSettings(cfg *SMTPSettings, to, subject, body string, log *slog.Logger) error { +func SendWithSMTPSettings(ctx context.Context, cfg *SMTPSettings, to, subject, body string, log *slog.Logger) error { if cfg == nil { return fmt.Errorf("SMTP settings not configured") } from := cfg.SenderEmail + + if ctx == nil { + ctx = context.Background() + } + ctx, cancel := context.WithTimeout(ctx, smtpSendTimeout) + defer cancel() + if from == "" { from = cfg.Username } @@ -104,53 +115,94 @@ func SendWithSMTPSettings(cfg *SMTPSettings, to, subject, body string, log *slog auth = smtp.PlainAuth("", cfg.Username, cfg.Password, cfg.Host) } msg := buildMessage(to, from, subject, body) - if err := sendMailWithConfig(addr, cfg.Host, cfg.Port, cfg.Security, auth, from, to, msg); err != nil { + if err := sendMailWithConfig(ctx, addr, cfg.Host, cfg.Port, cfg.Security, auth, from, to, msg); err != nil { return err } return nil } -// sendMailWithConfig sends email using smtp.SendMail or, for port 465 with SSL, -// an explicit TLS connection (smtp.SendMail only supports STARTTLS). -func sendMailWithConfig(addr, host string, port int, security string, auth smtp.Auth, from, to string, msg []byte) error { +// sendMailWithConfig delivers an email over SMTP using context-aware dialing +// and connection deadlines to bound SMTP read and write operations. +func sendMailWithConfig( + ctx context.Context, + addr, host string, + port int, + security string, + auth smtp.Auth, + from, to string, + msg []byte, +) error { + conn, err := (&net.Dialer{}).DialContext(ctx, "tcp", addr) + if err != nil { + return err + } + defer conn.Close() + + if deadline, ok := ctx.Deadline(); ok { + if err := conn.SetDeadline(deadline); err != nil { + return err + } + } + + stopCancel := context.AfterFunc(ctx, func() { + _ = conn.SetDeadline(time.Now()) + }) + defer stopCancel() + + var client *smtp.Client useImplicitTLS := port == 465 && strings.EqualFold(strings.TrimSpace(security), "SSL") + if useImplicitTLS { - conn, err := tls.Dial("tcp", addr, &tls.Config{ServerName: host}) - if err != nil { + tlsConn := tls.Client(conn, &tls.Config{ServerName: host}) + if err := tlsConn.HandshakeContext(ctx); err != nil { return err } - defer conn.Close() - client, err := smtp.NewClient(conn, host) + + client, err = smtp.NewClient(tlsConn, host) if err != nil { return err } - defer client.Close() - if auth != nil { - if err := client.Auth(auth); err != nil { - return err - } - } - if err := client.Mail(from); err != nil { - return err - } - if err := client.Rcpt(to); err != nil { - return err - } - w, err := client.Data() + } else { + client, err = smtp.NewClient(conn, host) if err != nil { return err } - if _, err := w.Write(msg); err != nil { - _ = w.Close() - return err + + // Preserve smtp.SendMail's existing behavior: use STARTTLS when the + // server advertises it. + if ok, _ := client.Extension("STARTTLS"); ok { + if err := client.StartTLS(&tls.Config{ServerName: host}); err != nil { + return err + } } - if err := w.Close(); err != nil { + } + defer client.Close() + + if auth != nil { + if err := client.Auth(auth); err != nil { return err } - return client.Quit() } - // STARTTLS (port 587) or no security: standard SendMail - return smtp.SendMail(addr, auth, from, []string{to}, msg) + if err := client.Mail(from); err != nil { + return err + } + if err := client.Rcpt(to); err != nil { + return err + } + + writer, err := client.Data() + if err != nil { + return err + } + if _, err := writer.Write(msg); err != nil { + _ = writer.Close() + return err + } + if err := writer.Close(); err != nil { + return err + } + + return client.Quit() } // sanitizeHeader removes CR/LF to prevent header injection. From 364774c1993b6b7fb7ca5e07cdce26d5d0b596f6 Mon Sep 17 00:00:00 2001 From: "hoffman.peter" Date: Wed, 16 Sep 2026 17:55:12 +0200 Subject: [PATCH 07/10] test(mail): cover stalled SMTP server timeout Co-Authored-By: Codex --- apps/api/internal/mail/mail.go | 2 +- apps/api/internal/mail/mail_timeout_test.go | 64 +++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 apps/api/internal/mail/mail_timeout_test.go diff --git a/apps/api/internal/mail/mail.go b/apps/api/internal/mail/mail.go index baa66782..8a44c72b 100644 --- a/apps/api/internal/mail/mail.go +++ b/apps/api/internal/mail/mail.go @@ -87,7 +87,7 @@ func NewSMTPEmailSender(instanceSettings *store.InstanceSettingStore, log *slog. } } -const smtpSendTimeout = 15 * time.Second +var smtpSendTimeout = 15 * time.Second // SendWithSMTPSettings sends an email using the supplied SMTP settings without persisting them. func SendWithSMTPSettings(ctx context.Context, cfg *SMTPSettings, to, subject, body string, log *slog.Logger) error { diff --git a/apps/api/internal/mail/mail_timeout_test.go b/apps/api/internal/mail/mail_timeout_test.go new file mode 100644 index 00000000..53b9a314 --- /dev/null +++ b/apps/api/internal/mail/mail_timeout_test.go @@ -0,0 +1,64 @@ +package mail + +import ( + "context" + "net" + "testing" + "time" +) + +func TestSendWithSMTPSettings_TimesOutWhenSMTPServerStalls(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + t.Cleanup(func() { + _ = listener.Close() + }) + + releaseConnection := make(chan struct{}) + t.Cleanup(func() { + close(releaseConnection) + }) + + // Accept the TCP connection but deliberately never send the SMTP greeting. + go func() { + conn, err := listener.Accept() + if err != nil { + return + } + defer conn.Close() + <-releaseConnection + }() + + previousTimeout := smtpSendTimeout + smtpSendTimeout = 100 * time.Millisecond + t.Cleanup(func() { + smtpSendTimeout = previousTimeout + }) + + port := listener.Addr().(*net.TCPAddr).Port + started := time.Now() + + err = SendWithSMTPSettings( + context.Background(), + &SMTPSettings{ + Host: "127.0.0.1", + Port: port, + SenderEmail: "sender@example.test", + Security: "None", + }, + "admin@example.test", + "Test subject", + "Test body", + nil, + ) + + if err == nil { + t.Fatal("expected SMTP send to time out") + } + + if elapsed := time.Since(started); elapsed > time.Second { + t.Fatalf("SMTP timeout took too long: %s", elapsed) + } +} From 11a4552e581f2c7dcee3796a0fc2088755de26cc Mon Sep 17 00:00:00 2001 From: Danny Ward Date: Sat, 24 Oct 2026 00:39:06 -0400 Subject: [PATCH 08/10] fix(db): assign Slack integration migration a unique version Migration 000013 is already used by the cancelled-state migration. Assign Slack integration version 000014 to allow migration loading and database-backed tests. Co-Authored-By: OpenAI Codex --- ...ack_integration.down.sql => 000014_slack_integration.down.sql} | 0 ...3_slack_integration.up.sql => 000014_slack_integration.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename apps/api/migrations/{000013_slack_integration.down.sql => 000014_slack_integration.down.sql} (100%) rename apps/api/migrations/{000013_slack_integration.up.sql => 000014_slack_integration.up.sql} (100%) diff --git a/apps/api/migrations/000013_slack_integration.down.sql b/apps/api/migrations/000014_slack_integration.down.sql similarity index 100% rename from apps/api/migrations/000013_slack_integration.down.sql rename to apps/api/migrations/000014_slack_integration.down.sql diff --git a/apps/api/migrations/000013_slack_integration.up.sql b/apps/api/migrations/000014_slack_integration.up.sql similarity index 100% rename from apps/api/migrations/000013_slack_integration.up.sql rename to apps/api/migrations/000014_slack_integration.up.sql From 04474b8712e4504ab7a967d744f08ca59646ff6d Mon Sep 17 00:00:00 2001 From: Danny Ward Date: Sat, 24 Oct 2026 00:41:00 -0400 Subject: [PATCH 09/10] fix(mail): honor configured SMTP security mode Honor None by skipping STARTTLS and require a successful STARTTLS upgrade for TLS. Keep SSL on implicit TLS and cover the wire behavior with local SMTP probes. Closes #404 Co-Authored-By: OpenAI Codex --- apps/api/internal/mail/mail.go | 88 +++++-- apps/api/internal/mail/mail_test.go | 346 ++++++++++++++++++++++++++++ 2 files changed, 413 insertions(+), 21 deletions(-) create mode 100644 apps/api/internal/mail/mail_test.go diff --git a/apps/api/internal/mail/mail.go b/apps/api/internal/mail/mail.go index 7d2bfbb4..8380b6dd 100644 --- a/apps/api/internal/mail/mail.go +++ b/apps/api/internal/mail/mail.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "fmt" "log/slog" + "net" "net/smtp" "strconv" "strings" @@ -87,7 +88,7 @@ func NewSMTPEmailSender(instanceSettings *store.InstanceSettingStore, log *slog. return fmt.Errorf("sender email not configured") } addr := fmt.Sprintf("%s:%d", cfg.Host, cfg.Port) - auth := smtp.PlainAuth("", cfg.Username, cfg.Password, cfg.Host) + auth := newSMTPAuth(cfg.Host, cfg.Username, cfg.Password) msg := buildMessage(to, from, subject, body) if err := sendMailWithConfig(addr, cfg.Host, cfg.Port, cfg.Security, auth, from, to, msg); err != nil { return err @@ -96,11 +97,22 @@ func NewSMTPEmailSender(instanceSettings *store.InstanceSettingStore, log *slog. } } -// sendMailWithConfig sends email using smtp.SendMail or, for port 465 with SSL, -// an explicit TLS connection (smtp.SendMail only supports STARTTLS). +func newSMTPAuth(host, username, password string) smtp.Auth { + if username == "" { + return nil + } + return smtp.PlainAuth("", username, password, host) +} + +// sendMailWithConfig sends email using the configured SMTP security mode: +// TLS requires STARTTLS, SSL uses implicit TLS, and None sends plaintext. func sendMailWithConfig(addr, host string, port int, security string, auth smtp.Auth, from, to string, msg []byte) error { - useImplicitTLS := port == 465 && strings.EqualFold(strings.TrimSpace(security), "SSL") - if useImplicitTLS { + security = strings.ToUpper(strings.TrimSpace(security)) + if security == "" { + security = "TLS" + } + + if security == "SSL" { conn, err := tls.Dial("tcp", addr, &tls.Config{ServerName: host}) if err != nil { return err @@ -111,30 +123,64 @@ func sendMailWithConfig(addr, host string, port int, security string, auth smtp. return err } defer client.Close() - if err := client.Auth(auth); err != nil { + if err := sendMailWithClient(client, auth, from, to, msg); err != nil { return err } - if err := client.Mail(from); err != nil { - return err - } - if err := client.Rcpt(to); err != nil { - return err - } - w, err := client.Data() - if err != nil { - return err + return client.Quit() + } + + conn, err := net.Dial("tcp", addr) + if err != nil { + return err + } + client, err := smtp.NewClient(conn, host) + if err != nil { + _ = conn.Close() + return err + } + defer client.Close() + + switch security { + case "TLS": + if ok, _ := client.Extension("STARTTLS"); !ok { + return fmt.Errorf("SMTP server does not support STARTTLS") } - if _, err := w.Write(msg); err != nil { - _ = w.Close() + if err := client.StartTLS(&tls.Config{ServerName: host}); err != nil { return err } - if err := w.Close(); err != nil { + case "NONE": + // The explicit None setting opts out of Go's opportunistic STARTTLS. + default: + return fmt.Errorf("unsupported SMTP security setting %q", security) + } + + if err := sendMailWithClient(client, auth, from, to, msg); err != nil { + return err + } + return client.Quit() +} + +func sendMailWithClient(client *smtp.Client, auth smtp.Auth, from, to string, msg []byte) error { + if auth != nil { + if err := client.Auth(auth); err != nil { return err } - return client.Quit() } - // STARTTLS (port 587) or no security: standard SendMail - return smtp.SendMail(addr, auth, from, []string{to}, msg) + if err := client.Mail(from); err != nil { + return err + } + if err := client.Rcpt(to); err != nil { + return err + } + w, err := client.Data() + if err != nil { + return err + } + if _, err := w.Write(msg); err != nil { + _ = w.Close() + return err + } + return w.Close() } // sanitizeHeader removes CR/LF to prevent header injection. diff --git a/apps/api/internal/mail/mail_test.go b/apps/api/internal/mail/mail_test.go new file mode 100644 index 00000000..4c0eefe3 --- /dev/null +++ b/apps/api/internal/mail/mail_test.go @@ -0,0 +1,346 @@ +package mail + +import ( + "bufio" + "crypto/tls" + "fmt" + "net" + "net/http" + "net/http/httptest" + "net/smtp" + "strconv" + "strings" + "testing" + "time" +) + +func TestNewSMTPAuthWithoutUsernameReturnsNil(t *testing.T) { + if auth := newSMTPAuth("smtp.example.test", "", "password"); auth != nil { + t.Fatal("expected SMTP authentication to be disabled without a username") + } +} + +func TestNewSMTPAuthWithUsernameReturnsAuth(t *testing.T) { + if auth := newSMTPAuth("smtp.example.test", "sender", "password"); auth == nil { + t.Fatal("expected SMTP authentication when a username is configured") + } +} + +func TestSendMailWithConfigNoneDoesNotAttemptSTARTTLS(t *testing.T) { + server := startSMTPProbe(t, []string{"STARTTLS"}) + port := smtpTestPort(t, server.addr) + if err := sendMailWithConfig(server.addr, "smtp.example.test", port, " None ", nil, + "sender@example.com", "recipient@example.com", []byte("Subject: test\r\n\r\nbody")); err != nil { + t.Fatalf("sendMailWithConfig returned an error: %v", err) + } + result := server.wait(t) + if containsSMTPCommand(result.commands, "STARTTLS") { + t.Fatalf("client attempted STARTTLS when security was None: %v", result.commands) + } +} + +func TestSendMailWithConfigTLSRequiresSTARTTLS(t *testing.T) { + for _, security := range []string{"TLS", ""} { + t.Run("security="+security, func(t *testing.T) { + server := startSMTPProbe(t, nil) + port := smtpTestPort(t, server.addr) + err := sendMailWithConfig(server.addr, "smtp.example.test", port, security, nil, + "sender@example.com", "recipient@example.com", []byte("Subject: test\r\n\r\nbody")) + result := server.wait(t) + if err == nil || !strings.Contains(err.Error(), "does not support STARTTLS") { + t.Fatalf("expected TLS mode to fail when STARTTLS is unavailable, got %v", err) + } + if containsSMTPCommand(result.commands, "STARTTLS") { + t.Fatalf("client attempted an unadvertised STARTTLS command: %v", result.commands) + } + assertNoSensitiveSMTPCommands(t, result.commands) + }) + } +} + +func TestSendMailWithConfigTLSRejectsUntrustedSTARTTLSCertificate(t *testing.T) { + certificate := smtpTestCertificate(t) + server := startSMTPProbe(t, []string{"STARTTLS", "AUTH PLAIN"}, &certificate) + port := smtpTestPort(t, server.addr) + + err := sendMailWithConfig(server.addr, "example.com", port, "TLS", nil, + "sender@example.com", "recipient@example.com", []byte("Subject: test\r\n\r\nbody")) + if err == nil || !strings.Contains(err.Error(), "certificate signed by unknown authority") { + t.Fatalf("expected TLS mode to reject the untrusted certificate, got %v", err) + } + + select { + case result := <-server.result: + if result.err == nil { + t.Fatal("expected the server to observe the client reject the TLS handshake") + } + if !containsSMTPCommand(result.commands, "STARTTLS") { + t.Fatalf("expected the client to request STARTTLS, got %v", result.commands) + } + assertNoSensitiveSMTPCommands(t, result.commands) + case <-time.After(6 * time.Second): + t.Fatal("timed out waiting for fake SMTP server") + } +} + +func TestSendMailWithConfigTLSDoesNotDowngradeWhenSTARTTLSFails(t *testing.T) { + server := startSMTPProbe(t, []string{"STARTTLS", "AUTH PLAIN"}) + port := smtpTestPort(t, server.addr) + err := sendMailWithConfig(server.addr, "smtp.example.test", port, "TLS", nil, + "sender@example.com", "recipient@example.com", []byte("Subject: test\r\n\r\nbody")) + if err == nil || !strings.Contains(err.Error(), "STARTTLS unavailable") { + t.Fatalf("expected STARTTLS refusal error, got %v", err) + } + result := server.wait(t) + if !containsSMTPCommand(result.commands, "STARTTLS") { + t.Fatalf("expected a STARTTLS command, got %v", result.commands) + } + assertNoSensitiveSMTPCommands(t, result.commands) +} + +func TestSendMailWithConfigNoneDoesNotSendPlainAuthCredentialsRemotely(t *testing.T) { + server := startSMTPProbe(t, []string{"STARTTLS", "AUTH PLAIN"}) + port := smtpTestPort(t, server.addr) + auth := smtp.PlainAuth("", "sender", "password", "smtp.example.test") + + err := sendMailWithConfig(server.addr, "smtp.example.test", port, "None", auth, + "sender@example.com", "recipient@example.com", []byte("Subject: test\r\n\r\nbody")) + if err == nil || !strings.Contains(err.Error(), "unencrypted connection") { + t.Fatalf("expected Go to refuse PLAIN authentication over plaintext, got %v", err) + } + + result := server.wait(t) + if containsSMTPCommand(result.commands, "STARTTLS") { + t.Fatalf("client attempted STARTTLS when security was None: %v", result.commands) + } + assertNoSensitiveSMTPCommands(t, result.commands) +} + +func TestSendMailWithConfigSSLUsesImplicitTLS(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = listener.Close() }) + firstByte := make(chan byte, 1) + go func() { + conn, err := listener.Accept() + if err != nil { + return + } + defer conn.Close() + _ = conn.SetReadDeadline(time.Now().Add(5 * time.Second)) + var header [1]byte + if _, err := conn.Read(header[:]); err == nil { + firstByte <- header[0] + } + }() + + // Use a local ephemeral listener while passing the standard implicit-TLS port. + // The probe closes during the handshake; the first byte still proves the + // client started TLS immediately instead of sending an SMTP greeting. + if err := sendMailWithConfig(listener.Addr().String(), "smtp.example.test", 465, "SSL", nil, + "sender@example.com", "recipient@example.com", nil); err == nil { + t.Fatal("expected the SMTP probe to close the incomplete TLS handshake") + } + select { + case got := <-firstByte: + if got != 0x16 { + t.Fatalf("expected a TLS handshake byte, got 0x%x", got) + } + case <-time.After(6 * time.Second): + t.Fatal("timed out waiting for the implicit TLS handshake") + } +} + +type smtpProbe struct { + addr string + result <-chan smtpProbeResult +} + +type smtpProbeResult struct { + commands []string + err error +} + +func startSMTPProbe(t *testing.T, extensions []string, tlsCertificates ...*tls.Certificate) smtpProbe { + t.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = listener.Close() }) + result := make(chan smtpProbeResult, 1) + var tlsCertificate *tls.Certificate + if len(tlsCertificates) > 0 { + tlsCertificate = tlsCertificates[0] + } + go func() { + probeResult := smtpProbeResult{} + defer func() { result <- probeResult }() + conn, err := listener.Accept() + if err != nil { + probeResult.err = err + return + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(5 * time.Second)) + reader := bufio.NewReader(conn) + write := func(line string) error { + _, err := fmt.Fprint(conn, line+"\r\n") + return err + } + if err := write("220 smtp.example.test ESMTP"); err != nil { + probeResult.err = err + return + } + for { + line, err := reader.ReadString('\n') + if err != nil { + if netErr, ok := err.(net.Error); ok && netErr.Timeout() { + probeResult.err = err + } + return + } + command := strings.TrimSpace(line) + probeResult.commands = append(probeResult.commands, command) + switch { + case strings.HasPrefix(strings.ToUpper(command), "EHLO "): + if len(extensions) == 0 { + if err := write("250 smtp.example.test"); err != nil { + probeResult.err = err + return + } + continue + } + if err := write("250-smtp.example.test"); err != nil { + probeResult.err = err + return + } + for i, extension := range extensions { + separator := "-" + if i == len(extensions)-1 { + separator = " " + } + if err := write("250" + separator + extension); err != nil { + probeResult.err = err + return + } + } + case strings.EqualFold(command, "STARTTLS"): + if tlsCertificate == nil { + if err := write("454 4.7.0 STARTTLS unavailable in SMTP probe"); err != nil { + probeResult.err = err + return + } + continue + } + if err := write("220 2.0.0 Ready to start TLS"); err != nil { + probeResult.err = err + return + } + tlsConn := tls.Server(conn, &tls.Config{Certificates: []tls.Certificate{*tlsCertificate}}) + if err := tlsConn.Handshake(); err != nil { + probeResult.err = err + return + } + conn = tlsConn + reader = bufio.NewReader(conn) + case strings.HasPrefix(strings.ToUpper(command), "AUTH "): + if err := write("235 2.7.0 authenticated"); err != nil { + probeResult.err = err + return + } + case strings.HasPrefix(strings.ToUpper(command), "MAIL FROM:"): + if err := write("250 sender accepted"); err != nil { + probeResult.err = err + return + } + case strings.HasPrefix(strings.ToUpper(command), "RCPT TO:"): + if err := write("250 recipient accepted"); err != nil { + probeResult.err = err + return + } + case strings.EqualFold(command, "DATA"): + if err := write("354 send message"); err != nil { + probeResult.err = err + return + } + for { + dataLine, err := reader.ReadString('\n') + if err != nil { + probeResult.err = err + return + } + if strings.TrimRight(dataLine, "\r\n") == "." { + break + } + } + if err := write("250 message accepted"); err != nil { + probeResult.err = err + return + } + case strings.EqualFold(command, "QUIT"): + probeResult.err = write("221 goodbye") + return + default: + probeResult.err = fmt.Errorf("unexpected SMTP command %q", command) + return + } + } + }() + return smtpProbe{addr: listener.Addr().String(), result: result} +} + +func (server smtpProbe) wait(t *testing.T) smtpProbeResult { + t.Helper() + select { + case result := <-server.result: + if result.err != nil { + t.Fatalf("fake SMTP server: %v", result.err) + } + return result + case <-time.After(6 * time.Second): + t.Fatal("timed out waiting for fake SMTP server") + return smtpProbeResult{} + } +} + +func smtpTestPort(t *testing.T, addr string) int { + t.Helper() + _, portText, err := net.SplitHostPort(addr) + if err != nil { + t.Fatal(err) + } + port, err := strconv.Atoi(portText) + if err != nil { + t.Fatal(err) + } + return port +} + +func smtpTestCertificate(t *testing.T) tls.Certificate { + t.Helper() + server := httptest.NewTLSServer(http.NotFoundHandler()) + defer server.Close() + return server.TLS.Certificates[0] +} + +func assertNoSensitiveSMTPCommands(t *testing.T, commands []string) { + t.Helper() + for _, command := range commands { + upper := strings.ToUpper(command) + if strings.HasPrefix(upper, "AUTH ") || strings.HasPrefix(upper, "MAIL FROM:") { + t.Fatalf("credentials or message were sent before secure SMTP was established: %v", commands) + } + } +} + +func containsSMTPCommand(commands []string, want string) bool { + for _, command := range commands { + if strings.EqualFold(command, want) { + return true + } + } + return false +} From 536c0a2d55387c25c31c72e845a063bc0acc46ef Mon Sep 17 00:00:00 2001 From: martian56 Date: Sun, 25 Oct 2026 11:51:16 +0400 Subject: [PATCH 10/10] fix(db): make renumbered Slack migration safe for upgraded installs Installs that applied the Slack migration as 000013 would re-run it as 000014 and fail on CREATE TABLE; use IF NOT EXISTS (the integrations insert already uses ON CONFLICT DO NOTHING). Those installs also never ran 000013_fix_cancelled_state_group, so re-apply that idempotent correction as 000015. Co-Authored-By: Claude Opus 5.5 --- apps/api/migrations/000014_slack_integration.up.sql | 2 +- .../migrations/000015_refix_cancelled_state_group.down.sql | 2 ++ .../api/migrations/000015_refix_cancelled_state_group.up.sql | 5 +++++ 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 apps/api/migrations/000015_refix_cancelled_state_group.down.sql create mode 100644 apps/api/migrations/000015_refix_cancelled_state_group.up.sql diff --git a/apps/api/migrations/000014_slack_integration.up.sql b/apps/api/migrations/000014_slack_integration.up.sql index 80ec48f7..513341f9 100644 --- a/apps/api/migrations/000014_slack_integration.up.sql +++ b/apps/api/migrations/000014_slack_integration.up.sql @@ -11,7 +11,7 @@ VALUES ( ON CONFLICT (provider) DO NOTHING; -- Create slack_channel_links table -- -CREATE TABLE slack_channel_links ( +CREATE TABLE IF NOT EXISTS slack_channel_links ( id UUID PRIMARY KEY DEFAULT gen_random_uuid(), workspace_integration_id UUID NOT NULL REFERENCES workspace_integrations (id) ON DELETE CASCADE, project_id UUID NOT NULL REFERENCES projects (id) ON DELETE CASCADE, diff --git a/apps/api/migrations/000015_refix_cancelled_state_group.down.sql b/apps/api/migrations/000015_refix_cancelled_state_group.down.sql new file mode 100644 index 00000000..f6bc7c19 --- /dev/null +++ b/apps/api/migrations/000015_refix_cancelled_state_group.down.sql @@ -0,0 +1,2 @@ +-- Intentionally a no-op: see 000013_fix_cancelled_state_group.down.sql. +SELECT 1; diff --git a/apps/api/migrations/000015_refix_cancelled_state_group.up.sql b/apps/api/migrations/000015_refix_cancelled_state_group.up.sql new file mode 100644 index 00000000..559f08c6 --- /dev/null +++ b/apps/api/migrations/000015_refix_cancelled_state_group.up.sql @@ -0,0 +1,5 @@ +-- Installations that applied the Slack integration migration while it was +-- (mis)numbered 000013 never ran 000013_fix_cancelled_state_group, so their +-- default Cancelled states may still be in the misspelled "canceled" group. +-- Re-apply the correction; it is a no-op where it already ran. +UPDATE states SET "group" = 'cancelled' WHERE "group" = 'canceled';