Commit `f6981be6cb`

f6981be6cb0fc43f165faaa491c1bc234b744390

parent: 7f2fd45218

Verified · cmc ci/build: success ci/sonar: success ci/test: success ci/vuln: success

cmc <hello@cleberg.net> · 2026-09-06 18:13 UTC

notify, web, wiki: log dead-lettered mail by queue row, not recipient

The dead-letter path logged the plaintext address, and the relay's error
usually quotes it too, so dropping the field alone would have moved the
leak rather than closed it. Both are now redacted; the row id identifies
the mail and /admin maps it back.

Closes #173

Layout: unified · split

.gitbay/wiki/Admin.org +6

 In accounts mode the same read renders at =/admin=, linked from the rail
 for admins. Anyone else gets a 404 there.
+A dead-lettered mail is logged as =notification dead-lettered mail=<id>=,
+with the address redacted out of the relay's error. The id is the queue
+row: find it in the Mail table on =/admin=, or in =dashboard --json=,
+where the recipient and the unredacted error are. That is deliberate —
+see the Threat-Model page.
 * Maintenance
 #+begin_src sh

.gitbay/wiki/Threat-Model.org +7

	@@ -20,6 +20,13 @@ log and hardening notes in [[Admin]].
20	registration invites, and API tokens travel on stdin or in request	20	registration invites, and API tokens travel on stdin or in request
21	bodies, never as command arguments (visible in =/proc=) or query	21	bodies, never as command arguments (visible in =/proc=) or query
22	strings. Tokens are stored only as SHA-256 hashes.	22	strings. Tokens are stored only as SHA-256 hashes.
		23	- Name a mail recipient in the log. A queued mail is logged by its
		24	queue row id, never by address, and the relay's own error is redacted
		25	before it is logged because a rejection usually quotes the address it
		26	rejected. The unredacted error and the address stay on the row, which
		27	an instance admin reads on =/admin=: the database holds who, the log
		28	holds which. One rule for every mail type — notifications, dependency
		29	reports and login links all drain the same queue (krz/gitbay#173).
23	- Confirm the existence of private repositories. Every surface answers	30	- Confirm the existence of private repositories. Every surface answers
24	"not found" identically for a private repo and a nonexistent one — web	31	"not found" identically for a private repo and a nonexistent one — web
25	pages, git transport, control commands, release asset downloads.	32	pages, git transport, control commands, release asset downloads.

internal/notify/notify.go +16 −1

 import (
 	"context"
 	"log/slog"
+	"regexp"
 	"time"
 	"gitbay.org/gitbay/internal/config"
 					attempt := q.Attempts + 1
 					if attempt >= m.MaxAttempts {
 						m.St.MarkMailFailed(q.ID, err.Error(), nil)
-						slog.Warn("notification dead-lettered", "recipient", q.Recipient, "err", err)
+						slog.Warn("notification dead-lettered",
+							"mail", q.ID, "attempts", attempt, "err", redactAddresses(err.Error()))
 					} else {
 						next := time.Now().Add(m.RetryBase << (attempt - 1))
 						m.St.MarkMailFailed(q.ID, err.Error(), &next)
 		}
 	}
 }
+// A relay's rejection usually quotes the address it rejected — "550 5.1.1
+// <x@y>: Recipient address rejected" — so dropping the recipient field
+// alone would not keep an address out of the log.
+var addressPat = regexp.MustCompile(`[^\s<>@,;:"]+@[^\s<>@,;:"]+`)
+// redactAddresses removes mail addresses from text bound for the log. The
+// unredacted error is still recorded on the queue row, where an instance
+// admin reads it on /admin: the database holds the address, the log does
+// not (#173).
+func redactAddresses(s string) string {
+	return addressPat.ReplaceAllString(s, "<address>")
+}

internal/notify/redact_test.go added +39

@@ -0,0 +1,39 @@
	1	package notify
	2
	3	import "testing"
	4
	5	// The log names the queue row, not the person. A relay's rejection quotes
	6	// the address it rejected, so the error text is redacted too — otherwise
	7	// dropping the recipient field would move the leak rather than close it
	8	// (#173).
	9	func TestRedactAddresses(t *testing.T) {
	10	cases := []struct{ in, want string }{
	11	{"550 5.1.1 <alice@example.test>: Recipient address rejected",
	12	"550 5.1.1 <<address>>: Recipient address rejected"},
	13	{"dial tcp 10.0.0.1:587: connect: connection refused",
	14	"dial tcp 10.0.0.1:587: connect: connection refused"},
	15	{"554 alice@a.test, bob@b.test both unknown",
	16	"554 <address>, <address> both unknown"},
	17	{"x509: certificate signed by unknown authority", "x509: certificate signed by unknown authority"},
	18	}
	19	for _, c := range cases {
	20	if got := redactAddresses(c.in); got != c.want {
	21	t.Errorf("redactAddresses(%q) = %q, want %q", c.in, got, c.want)
	22	}
	23	}
	24	}
	25
	26	// Whatever the relay says, no @ survives into the log line.
	27	func TestRedactLeavesNoAddress(t *testing.T) {
	28	for _, s := range []string{
	29	"550 <a.b+tag@sub.example.co.uk> over quota",
	30	`smtp: 553 "weird name"@host.test refused`,
	31	"relay said: alice@example.test; bob@example.test",
	32	} {
	33	if got := redactAddresses(s); containsAddress(got) {
	34	t.Errorf("address survived redaction: %q -> %q", s, got)
	35	}
	36	}
	37	}
	38
	39	func containsAddress(s string) bool { return addressPat.MatchString(s) }

internal/web/templates/admin.html +2 −2

	@@ -14,8 +14,8 @@
14	{{with .Queues.Mail}}	14	{{with .Queues.Mail}}
15	<h2>Mail <span class="count">{{.Pending}}</span></h2>	15	<h2>Mail <span class="count">{{.Pending}}</span></h2>
16	<p class="meta">{{.Pending}} pending · {{.Retrying}} retrying · {{.Failed}} failed{{if .OldestPending}} · oldest pending {{when .OldestPending}}{{end}}</p>	16	<p class="meta">{{.Pending}} pending · {{.Retrying}} retrying · {{.Failed}} failed{{if .OldestPending}} · oldest pending {{when .OldestPending}}{{end}}</p>
17	{{if .Items}}<div class="tablewrap"><table class="keys"><thead><tr class="cols"><th>Recipient</th><th>Subject</th><th>Attempts</th><th>State</th><th>Last error</th></tr></thead><tbody>	17	{{if .Items}}<div class="tablewrap"><table class="keys"><thead><tr class="cols"><th>Mail</th><th>Recipient</th><th>Subject</th><th>Attempts</th><th>State</th><th>Last error</th></tr></thead><tbody>
18	{{range .Items}}<tr><td>{{.Recipient}}</td><td>{{.Subject}}</td><td>{{.Attempts}}</td><td>{{if .FailedAt}}failed {{when .FailedAt}}{{else}}retrying{{end}}</td><td>{{.LastError}}</td></tr>	18	{{range .Items}}<tr><td class="mono">{{.ID}}</td><td>{{.Recipient}}</td><td>{{.Subject}}</td><td>{{.Attempts}}</td><td>{{if .FailedAt}}failed {{when .FailedAt}}{{else}}retrying{{end}}</td><td>{{.LastError}}</td></tr>
19	{{end}}</tbody></table></div>{{else}}<p class="none">Nothing retrying or failed</p>{{end}}	19	{{end}}</tbody></table></div>{{else}}<p class="none">Nothing retrying or failed</p>{{end}}
20	{{end}}	20	{{end}}
21		21

Commit f6981be6cb

Commit `f6981be6cb`