Commit `3dd60ce9a5`

3dd60ce9a5b2f84cb7a0bfed8fdb8f637740acaf

parent: 3eb692a4a5

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-05 19:36 UTC

control: queue login link mail instead of a fire-and-forget goroutine

RequestLoginLink handed mail.Send to a goroutine so a hit and a miss
returned on the same path. Nothing drained those goroutines, so a
restart mid-delivery dropped the mail silently; http.Server.Shutdown
waits for requests, not for goroutines a handler spawned.

Replace it with store.EnqueueMail: the INSERT is the same order of
cost as the miss path's SELECT, so the timing property holds, and the
mail survives a crash rather than only a graceful shutdown.

notify.Mailer's retry schedule (30s, 60s, 120s, 240s, then
dead-lettered) tops out at 450s, well inside the 15-minute link TTL,
so a retried delivery cannot outlive the link it carries.

That bound is only as good as nobody moving either side of it, and
nothing enforced the coupling: notify's retry parameters were a bare
5 and a local 30-second literal in cmd/gitbayd/main.go, unreachable by
name from anywhere that would need to check them against a login
link's TTL. Named them (notify.DefaultMaxAttempts,
notify.DefaultRetryBase) and added
TestLoginLinkOutlivesMailerRetries in internal/control, which computes
the mailer's worst case from those constants and fails if it ever
reaches loginLinkTTL, so retuning mail retries for an unrelated reason
cannot silently let a link arrive after it has expired.

e2e's manual poll loops waited long enough for a synchronous send;
widen them to fit the mailer's 2-second poll tick.

Closes #159

Layout: unified · split

cmd/gitbayd/main.go +5 −3

		@@ -160,9 +160,11 @@ func serveCmd() *cobra.Command {
160	160	ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
161	161	defer stop()
162	162
163		// Outbound webhook deliveries. The retry base is overridable
164		// for tests via GITBAY_WEBHOOK_RETRY_BASE.
165		retryBase := 30 * time.Second
	163	// Outbound webhook deliveries, and the mail queue when SMTP is
	164	// configured, share one retry base. It is overridable for
	165	// tests via GITBAY_WEBHOOK_RETRY_BASE; notify.DefaultRetryBase
	166	// names the production default so nothing else has to guess it.
	167	retryBase := notify.DefaultRetryBase
166	168	if v := os.Getenv("GITBAY_WEBHOOK_RETRY_BASE"); v != "" {
167	169	if d, err := time.ParseDuration(v); err == nil {
168	170	retryBase = d

e2e/emaillogin_test.go +6 −4

		@@ -57,7 +57,9 @@ func TestEmailLogin(t *testing.T) {
57	57	if status != 200 \|\| !strings.Contains(body, "on its way") {
58	58	t.Fatalf("POST /login by username: %d", status)
59	59	}
60		deadline := time.Now().Add(2 * time.Second)
	60	// Mail goes out through the notification queue, not on the request
	61	// path, so give the mailer's poll tick time to pick it up.
	62	deadline := time.Now().Add(5 * time.Second)
61	63	for time.Now().Before(deadline) && len(smtp.mailTo("dana@example.test")) < 2 {
62	64	time.Sleep(25 * time.Millisecond)
63	65	}
		@@ -146,10 +148,10 @@ func TestEmailLoginThrottled(t *testing.T) {
146	148	t.Error("the throttled response differs from the first")
147	149	}
148	150
149		// Mail goes out from a goroutine, not on the request path, so give the
150		// last permitted one time to land before counting.
	151	// Mail goes out through the notification queue, not on the request
	152	// path, so give the last permitted one time to land before counting.
151	153	var n int
152		deadline := time.Now().Add(2 * time.Second)
	154	deadline := time.Now().Add(5 * time.Second)
153	155	for time.Now().Before(deadline) {
154	156	n = len(smtp.mailTo("dana@example.test"))
155	157	if n >= 5 {

internal/control/loginlink.go +9 −13

		@@ -2,12 +2,10 @@ package control
2	2
3	3	import (
4	4	"fmt"
5		"log/slog"
6	5	"strings"
7	6	"time"
8	7
9	8	"gitbay.org/gitbay/internal/config"
10		"gitbay.org/gitbay/internal/mail"
11	9	"gitbay.org/gitbay/internal/store"
12	10	)
13	11
		@@ -100,15 +98,13 @@ func RequestLoginLink(cfg config.Config, st *store.Store, identifier string) err
100	98	host, strings.TrimSuffix(cfg.Server.SiteURL, "/"), token)
101	99	subject := "log in to " + host
102	100
103		// Sent in the background: mail.Send is a synchronous SMTP round trip to
104		// the relay, tens to hundreds of milliseconds against the sub-millisecond
105		// a miss takes to answer. Returning before it completes keeps every case
106		// — hit, miss, unverified, throttled — on the same DB-bound path, so
107		// response time cannot answer what the response body is built not to.
108		go func() {
109		if err := mail.Send(cfg, address, subject, body); err != nil {
110		slog.Error("login link mail", "user", user.ID, "err", err)
111		}
112		}()
113		return nil
	101	// Queued rather than sent inline: the INSERT is sub-millisecond, the
	102	// same order of cost as the miss path's SELECT, so every case — hit,
	103	// miss, unverified, throttled — still resolves on the same DB-bound
	104	// path. notify.Mailer drains the queue with retries (30s, 60s, 120s,
	105	// 240s, then dead-lettered) that top out at 450s, comfortably inside
	106	// the 15-minute link TTL, so a retried delivery cannot outlive the
	107	// link it carries. Unlike the goroutine this replaces, a crash mid
	108	// delivery does not lose the mail.
	109	return st.EnqueueMail(address, subject, body)
114	110	}

internal/control/loginlink_test.go added +31

		@@ -0,0 +1,31 @@
	1	package control
	2
	3	import (
	4	"testing"
	5	"time"
	6
	7	// Aliased: this package already has a top-level function named notify
	8	// (notifications.go), which the default import name would collide with.
	9	mailqueue "gitbay.org/gitbay/internal/notify"
	10	)
	11
	12	// Nothing ties notify's retry parameters to loginLinkTTL: someone tuning
	13	// mail retries for a slow relay has no reason to think about login links,
	14	// and the failure if they drift apart is silent — a link delivered after
	15	// it has already expired, refused with no indication why. This computes
	16	// the mailer's worst-case delivery time from notify's own named constants
	17	// (attempts 1..MaxAttempts-1 each wait RetryBase<<(attempt-1); the
	18	// MaxAttempts'th failure is dead-lettered immediately, per notify.Mailer.Run)
	19	// rather than a copy of the numbers, so either side moving breaks this test.
	20	func TestLoginLinkOutlivesMailerRetries(t *testing.T) {
	21	var worst time.Duration
	22	for attempt := 1; attempt < mailqueue.DefaultMaxAttempts; attempt++ {
	23	worst += mailqueue.DefaultRetryBase << (attempt - 1)
	24	}
	25	if worst >= loginLinkTTL {
	26	t.Fatalf("mailqueue.DefaultRetryBase=%s, mailqueue.DefaultMaxAttempts=%d: worst-case "+
	27	"delivery is %s, not less than loginLinkTTL=%s — a retried login link mail can "+
	28	"arrive after the link it carries has expired",
	29	mailqueue.DefaultRetryBase, mailqueue.DefaultMaxAttempts, worst, loginLinkTTL)
	30	}
	31	}

internal/notify/notify.go +11 −1

		@@ -13,6 +13,16 @@ import (
13	13	"gitbay.org/gitbay/internal/store"
14	14	)
15	15
	16	// DefaultMaxAttempts and DefaultRetryBase are the retry parameters gitbayd
	17	// wires up in production (cmd/gitbayd/main.go), named so anything that
	18	// needs to reason about the mailer's worst-case delivery time — such as
	19	// checking it against a login link's TTL — computes it from the numbers
	20	// actually in force rather than a copy of them.
	21	const (
	22	DefaultMaxAttempts = 5
	23	DefaultRetryBase = 30 * time.Second
	24	)
	25
16	26	type Mailer struct {
17	27	St *store.Store
18	28	Cfg config.Config
		@@ -21,7 +31,7 @@ type Mailer struct {
21	31	}
22	32
23	33	func New(st store.Store, cfg config.Config, retryBase time.Duration) Mailer {
24		return &Mailer{St: st, Cfg: cfg, RetryBase: retryBase, MaxAttempts: 5}
	34	return &Mailer{St: st, Cfg: cfg, RetryBase: retryBase, MaxAttempts: DefaultMaxAttempts}
25	35	}
26	36
27	37	// Run polls for due mail until ctx is done.

Commit 3dd60ce9a5

Commit `3dd60ce9a5`