`cc90ea1`

fix(email): strip display name from SMTP envelope so MAIL FROM is RFC-5321 valid

Authored by mfwolffe <wolffemf@dukes.jmu.edu> 5 days ago

Status	File	+	-
M	`internal/auth/email/sender.go`	19	1
M	`internal/auth/email/sender_test.go`	16	0
M	`internal/issues/issues.go`	7	16

internal/auth/email/sender.gomodified

  // Send implements Sender. Builds a multipart/alternative body so MUAs can
  // pick the variant they prefer.
 +//
 +// The SMTP envelope sender is the bare address — `MAIL FROM:<...>` per
 +// RFC 5321 forbids nested angle brackets, so a configured `From` like
 +// `shithub <noreply@shithub.local>` only goes in the message header.
 +// The displayed-name form lives unchanged in the `From:` header inside
 +// the body.
  func (s *SMTPSender) Send(_ context.Context, m Message) error {
  	if m.From == "" {
  		m.From = s.From
+ 		}
  		auth = smtp.PlainAuth("", s.Username, s.Password, host)
+ 	}
 -	return smtp.SendMail(s.Addr, auth, m.From, []string{m.To}, body)
 +	return smtp.SendMail(s.Addr, auth, envelopeAddress(m.From), []string{m.To}, body)
 +}
++
 +// envelopeAddress extracts the bare email address from a possibly-
 +// displayed-name form like `Name <addr@example>`. Returns the input
 +// unchanged when no angle brackets are present (already bare).
 +func envelopeAddress(from string) string {
 +	if i := strings.IndexByte(from, '<'); i >= 0 {
 +		if j := strings.IndexByte(from[i+1:], '>'); j >= 0 {
 +			return from[i+1 : i+1+j]
 +		}
 +	}
 +	return from
+ }
  // boundary is a fixed multipart boundary. Per RFC 2046 the boundary need

internal/auth/email/sender_test.gomodified

  	"testing"
+ )
 +func TestEnvelopeAddress_StripsDisplayName(t *testing.T) {
 +	t.Parallel()
 +	cases := []struct{ in, want string }{
 +		{"shithub <noreply@shithub.local>", "noreply@shithub.local"},
 +		{"<bare@example.com>", "bare@example.com"},
 +		{"plain@example.com", "plain@example.com"},
 +		{"Name With Spaces <a@b>", "a@b"},
 +		{"malformed <still works", "malformed <still works"},
 +	}
 +	for _, c := range cases {
 +		if got := envelopeAddress(c.in); got != c.want {
 +			t.Errorf("envelopeAddress(%q): want %q, got %q", c.in, c.want, got)
 +		}
 +	}
 +}
++
  func TestStdoutSender_WritesBothBodies(t *testing.T) {
  	t.Parallel()
  	var buf bytes.Buffer

internal/issues/issues.gomodified

  	return nil
+ }
 -// renderBodyHTML wraps markdown.RenderHTML with a logger-aware error
 -// path. Body length is bounded upstream (orchestrator validation +
 -// DB CHECK at 65535), so ErrInputTooLarge is structurally impossible
 -// here — but if it ever fires, log loudly: it means a precondition
 -// somewhere upstream regressed. The audit (S00-S25, M) flagged the
 -// `_`-discard pattern as the kind of slop where a real bug could hide.
 -func renderBodyHTML(ctx context.Context, deps Deps, body string) string {
 -	html, _ := renderBody(ctx, deps, body)
 -	return html
 -}
+-
 -// renderBody is the mention-aware variant. Returns the cleaned HTML
 -// plus the resolved mentions list — callers that emit notification
 -// events use the mentions to fan out @-pings. The `_` shimming under
 -// renderBodyHTML keeps existing call sites that don't care about
 -// mentions untouched.
 +// renderBody renders markdown to sanitized HTML and returns the
 +// resolved mention list. Body length is bounded upstream
 +// (orchestrator validation + DB CHECK at 65535), so
 +// ErrInputTooLarge is structurally impossible here — but if it ever
 +// fires, log loudly: it means a precondition somewhere upstream
 +// regressed. Mentions feed the S29 fan-out worker via the event
 +// payload's `mentions` array.
  func renderBody(ctx context.Context, deps Deps, body string) (string, []mdrender.Mention) {
  	if body == "" {
  		return "", nil