From 53c345219112c445e171d081671c9d83189f5f87 Mon Sep 17 00:00:00 2001 From: Distronode <254259913+distronode-com@users.noreply.github.com> Date: Fri, 11 Sep 2026 02:19:49 -0400 Subject: [PATCH] mailer: a booker's address is validated at intake, and never written into a header unparsed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The To: header is the one header field assembled from caller-supplied input with nothing encoding it. buildRaw parsed each recipient with mail.ParseAddress and, when that failed, appended the raw string anyway, so a CR/LF inside an address would have closed the To: line and started a header of the sender's choosing. Every other field was already safe: Subject and the From display name go through mime.QEncoding, which renders CR and LF as =0D and =0A; attachment filenames are written with %q; bodies sit below the blank line inside random-boundary MIME parts. The value reaching it is public and unauthenticated. POST /v1/bookings takes `email` straight from the request body, checks only that it is non-empty, and carries it to mailer.Message.To through dispatchBookingConfirmation. The conversational booking assistant's `book` tool is the same hole with a looser source: the address is whatever the model extracted from booker chat. Neither path called mail.ParseAddress anywhere. It is not exploitable as it stands, and the reason is worth stating precisely rather than relying on. Send issues c.Rcpt(to) for every recipient before it opens DATA, and net/smtp runs validateLine inside Rcpt, which refuses any string containing CR or LF. A CRLF-bearing address therefore aborts the exchange at RCPT TO and never reaches the body. That is incidental protection: it lives one call away in the standard library, it is not a property of this file, and it does nothing for a future transport that reuses buildRaw to assemble a message. The guarantee should be local to where the header is built. Two layers, because the two answer different questions. Intake decides what is stored. normalizeBookerEmail trims, parses with mail.ParseAddress, and returns a.Address — the bare address, not the input. CreateBooking calls it immediately after the emptiness check, before anything reads req.Email, and answers 400 "email must be a valid email address" in the same {"error": …} shape as its neighbours. createBookingForSlug — the shared core behind the assistant's `book` tool and the MCP create_booking tool — calls it before it persists anything; the REST handler does not route through that core, which is why both are touched. mail.ParseAddress is the whole rule: no length or domain heuristics, since the property that matters is parseability, not plausibility. The mailer decides what can be written. buildRaw returns ([]byte, error) and refuses an address it cannot parse, and an empty recipient list, with ErrInvalidRecipient. Send already built the message before dialing, so the refusal costs no connection — a test drives Send at a listener that counts accepts and asserts zero. The rejected value is deliberately kept out of the error, which is logged and may carry CR/LF; the error names the index instead. Storing a.Address rather than the input is a small deliberate behaviour change: a pasted `Bob ` is now recorded as bob@example.com. That is what the rest of the system already assumes it holds — the hourly per-email throttle, the per-invitee active-booking cap and the To: header all compare or emit the stored value as a plain address. resend.go, the HTTPS transport, posts msg.To as JSON and so has no header to inject into. It does accept unparsed recipients and is left alone; its callers now come through the same validated intake. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 25 +++++ internal/handler/booking_assistant.go | 4 + internal/handler/booking_email_test.go | 122 +++++++++++++++++++++++++ internal/handler/booking_handler.go | 45 +++++++++ internal/mailer/ics_test.go | 2 +- internal/mailer/mailer_test.go | 77 +++++++++++++++- internal/mailer/smtp.go | 42 ++++++--- internal/mailer/smtp_test.go | 48 ++++++++++ 8 files changed, 347 insertions(+), 18 deletions(-) create mode 100644 internal/handler/booking_email_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f45a54c..375ea1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,31 @@ exact tag (`ghcr.io/calnode/calnode:0.1.0`) if you need stability between upgrad ## [Unreleased] +### Security +- **A booker's email address is validated where it enters, and is never written into an + email header unparsed.** The `To:` header was the one header field assembled from + caller-supplied input with no encoder in front of it: `buildRaw` parsed each recipient + with `net/mail` and, when that failed, appended the raw string anyway, so a CR/LF inside + an address would have ended the `To:` line and started a header of the sender's + choosing. Subject and the `From` display name already go through `mime.QEncoding` + (which renders CR/LF as `=0D`/`=0A`) and attachment filenames through `%q`. + + Not exploitable as shipped: `Send` issues `c.Rcpt(to)` before `DATA`, and `net/smtp` + runs `validateLine` inside `Rcpt`, refusing any CR or LF - so a CRLF-bearing address + aborted the exchange at `RCPT TO` and never reached the body. That protection is + incidental, lives one call away in the standard library, and covers only this + transport. `buildRaw` now refuses an unparsed address (and an empty recipient list) + outright, returning `mailer.ErrInvalidRecipient` before anything is dialed; the + rejected value is kept out of the error, which is logged. + + The public booking paths validate at intake rather than relying on the mailer: the REST + handler (`POST /v1/bookings`) answers 400 "email must be a valid email address", and + the shared core behind the conversational assistant's `book` tool and the MCP + `create_booking` tool checks the address before it persists anything. Both store the + parsed bare address, so a pasted `Bob ` is recorded as + `bob@example.com` - a small deliberate behaviour change, matching what the hourly + throttle, the per-invitee cap and the `To:` header already assume they hold. + ## [0.9.0] - 2026-09-10 ### Added diff --git a/internal/handler/booking_assistant.go b/internal/handler/booking_assistant.go index eaa2110..4cbe512 100644 --- a/internal/handler/booking_assistant.go +++ b/internal/handler/booking_assistant.go @@ -439,6 +439,10 @@ func assistantBookError(err error) string { return "that time was just taken — please choose another slot" case errors.Is(err, booking.ErrBookingLimitReached): return "you already have the maximum number of upcoming bookings for this event" + case errors.Is(err, errInvalidBookerEmail): + // Worth its own case: the model can fix this by asking again, which the generic + // fallback below does not tell it. + return "that email address is not valid — please ask for it again" default: var ae *answerError if errors.As(err, &ae) { diff --git a/internal/handler/booking_email_test.go b/internal/handler/booking_email_test.go new file mode 100644 index 0000000..43f8b25 --- /dev/null +++ b/internal/handler/booking_email_test.go @@ -0,0 +1,122 @@ +package handler_test + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/calnode/calnode/internal/handler" +) + +// bookWithEmail posts a booking with the given raw email value and returns the response. +func bookWithEmail(t *testing.T, h *handler.Handler, slug, hhmm, email string) *httptest.ResponseRecorder { + t.Helper() + body := fmt.Sprintf(`{"event_type_slug":%q,"start_at":"2026-06-15T%s:00Z","name":"Bob","email":%q}`, + slug, hhmm, email) + req := httptest.NewRequest(http.MethodPost, "/v1/bookings", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + h.CreateBooking(rec, req) + return rec +} + +// organizerEmail reads back the stored organizer address for the workspace's single +// booking. The public create response carries no attendees, so this goes through the +// admin list, which does. +func organizerEmail(t *testing.T, h *handler.Handler, apiKey string) string { + t.Helper() + req := authReq(http.MethodGet, "/v1/bookings", "", apiKey) + rec := httptest.NewRecorder() + h.RequireAuth(h.ListBookings)(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("GET /v1/bookings: %d - %s", rec.Code, rec.Body.String()) + } + var out struct { + Items []struct { + Attendees []struct { + Email string `json:"email"` + } `json:"attendees"` + } `json:"items"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatalf("decode bookings: %v", err) + } + if len(out.Items) != 1 || len(out.Items[0].Attendees) != 1 { + t.Fatalf("want exactly one booking with one organizer; got %+v", out.Items) + } + return out.Items[0].Attendees[0].Email +} + +// TestCreateBooking_rejectsMalformedEmail: the booker's address ends up in an outbound +// message's To: header, so it is checked at intake rather than at the mailer alone. The +// emptiness check above it only proves the field was filled in. +func TestCreateBooking_rejectsMalformedEmail(t *testing.T) { + h, key, _ := setupWorkspace(t) + slug, _ := seedEventTypeHTTP(t, h, key) + + for _, tc := range []struct { + name string + email string + }{ + {"not an address", "not-an-address"}, + {"header injection", "a@b.example\r\nBcc: x@y.example"}, + {"two addresses", "a@b.example, c@d.example"}, + } { + t.Run(tc.name, func(t *testing.T) { + rec := bookWithEmail(t, h, slug, "09:00", tc.email) + if rec.Code != http.StatusBadRequest { + t.Fatalf("got %d - %s; want 400", rec.Code, rec.Body.String()) + } + var resp struct { + Error string `json:"error"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode error body: %v", err) + } + if resp.Error != "email must be a valid email address" { + t.Errorf("error = %q; want the plain intake sentence", resp.Error) + } + }) + } + + // Nothing was stored on any of those attempts. + req := authReq(http.MethodGet, "/v1/bookings", "", key) + rec := httptest.NewRecorder() + h.RequireAuth(h.ListBookings)(rec, req) + if strings.Contains(rec.Body.String(), "b.example") { + t.Errorf("a rejected booking was persisted: %s", rec.Body.String()) + } +} + +// TestCreateBooking_normalisesDisplayNameEmail: a pasted `Bob ` is +// stored as the bare address. Deliberate — every later reader (the hourly throttle, the +// per-invitee cap, the To: header) treats the stored value as a plain address. +func TestCreateBooking_normalisesDisplayNameEmail(t *testing.T) { + h, key, _ := setupWorkspace(t) + slug, _ := seedEventTypeHTTP(t, h, key) + + rec := bookWithEmail(t, h, slug, "09:00", " Bob ") + if rec.Code != http.StatusCreated { + t.Fatalf("got %d - %s; want 201", rec.Code, rec.Body.String()) + } + if got := organizerEmail(t, h, key); got != "bob@example.com" { + t.Errorf("stored organizer email = %q; want %q", got, "bob@example.com") + } +} + +// TestCreateBooking_ordinaryEmailUnchanged: normalisation must not rewrite the common case. +func TestCreateBooking_ordinaryEmailUnchanged(t *testing.T) { + h, key, _ := setupWorkspace(t) + slug, _ := seedEventTypeHTTP(t, h, key) + + rec := bookWithEmail(t, h, slug, "09:00", "Bob.Booker@example.com") + if rec.Code != http.StatusCreated { + t.Fatalf("got %d - %s; want 201", rec.Code, rec.Body.String()) + } + if got := organizerEmail(t, h, key); got != "Bob.Booker@example.com" { + t.Errorf("stored organizer email = %q; want it unchanged", got) + } +} diff --git a/internal/handler/booking_handler.go b/internal/handler/booking_handler.go index 6b7abe3..fc7a5f9 100644 --- a/internal/handler/booking_handler.go +++ b/internal/handler/booking_handler.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "net/http" + "net/mail" "net/url" "strconv" "strings" @@ -24,6 +25,31 @@ import ( "github.com/calnode/calnode/internal/zoom" ) +// errInvalidBookerEmail is returned by normalizeBookerEmail for an address net/mail +// cannot parse. Callers map it to their own protocol (400 on the REST path, a short +// retry hint for the assistant). +var errInvalidBookerEmail = errors.New("email must be a valid email address") + +// normalizeBookerEmail validates a booker-supplied address and returns the bare address +// to store. Every booking path ends with that value in an outbound message's To: header, +// so it must be something a header can hold: an unparsed address is the one raw input the +// mailer used to pass through verbatim, and a CR/LF inside it would end the To: line. +// +// Returning a.Address rather than the input also normalises `"Bob" ` to +// `bob@example.com`, which is what the rest of the system already assumes it holds — the +// per-invitee cap, the hourly throttle and the manage-link lookups all compare the stored +// value as a plain address. +// +// mail.ParseAddress is the whole rule. No length or domain heuristics: they reject valid +// addresses, and the property that matters here is parseability, not plausibility. +func normalizeBookerEmail(raw string) (string, error) { + a, err := mail.ParseAddress(strings.TrimSpace(raw)) + if err != nil { + return "", errInvalidBookerEmail + } + return a.Address, nil +} + // maxBookingsPerEmailPerHour caps how many bookings one email address can create // across the workspace in a rolling hour — a per-identity backstop to the per-IP // rate limit, for the openly-public booking page. @@ -479,6 +505,16 @@ func (h *Handler) loadBookableEventType(ctx context.Context, slug string) (*book // booking.ErrDoubleBooked, booking.ErrBookingLimitReached, errNoHostAvailable) for callers to // map to their own protocol. func (h *Handler) createBookingForSlug(ctx context.Context, slug string, startAt time.Time, organizer booking.Attendee, rawAnswers []booking.Answer) (*booking.Booking, error) { + // Both callers take the address from something they do not control — an LLM's + // extraction from booker chat, or an MCP client's tool arguments — so it is checked + // here, once, rather than in each of them. (The REST handler does not come through + // this core; it normalises at its own intake, before its throttle reads the value.) + email, err := normalizeBookerEmail(organizer.Email) + if err != nil { + return nil, err + } + organizer.Email = email + et, err := h.loadBookableEventType(ctx, slug) if err != nil { return nil, err @@ -722,6 +758,15 @@ func (h *Handler) CreateBooking(w http.ResponseWriter, r *http.Request) { h.writeError(w, http.StatusBadRequest, "event_type_slug, start_at, name, and email are required") return } + // Normalise before anything reads req.Email — the hourly throttle, the Stripe + // Checkout session, the attendee row and the confirmation email all take it from + // here, and they must all see the same bare address. + email, err := normalizeBookerEmail(req.Email) + if err != nil { + h.writeError(w, http.StatusBadRequest, err.Error()) + return + } + req.Email = email if req.Timezone == "" { req.Timezone = "UTC" } diff --git a/internal/mailer/ics_test.go b/internal/mailer/ics_test.go index bcb3a21..c0faa02 100644 --- a/internal/mailer/ics_test.go +++ b/internal/mailer/ics_test.go @@ -113,7 +113,7 @@ func TestBuildRaw_multipartWithICS(t *testing.T) { s := smtpForTest() d := testBookingData() d.AttachICS = true - raw := string(s.buildRaw(Message{ + raw := string(mustBuildRaw(t, s, Message{ To: []string{"bob@example.com"}, Subject: "Booking confirmed", Text: "body text", diff --git a/internal/mailer/mailer_test.go b/internal/mailer/mailer_test.go index 67c5d46..1c1a946 100644 --- a/internal/mailer/mailer_test.go +++ b/internal/mailer/mailer_test.go @@ -2,6 +2,7 @@ package mailer import ( "context" + "errors" "net/url" "strings" "sync" @@ -244,6 +245,17 @@ func smtpForTest() *SMTP { return &SMTP{from: "noreply@example.com", fromName: "Calnode"} } +// mustBuildRaw builds msg and fails the test if the recipients are refused — for the +// cases below, which are about header encoding rather than recipient validation. +func mustBuildRaw(t *testing.T, s *SMTP, msg Message) []byte { + t.Helper() + raw, err := s.buildRaw(msg) + if err != nil { + t.Fatalf("buildRaw: %v", err) + } + return raw +} + func TestBuildRaw_subjectInjectionPrevented(t *testing.T) { s := smtpForTest() msg := Message{ @@ -251,7 +263,7 @@ func TestBuildRaw_subjectInjectionPrevented(t *testing.T) { Subject: "Evil\r\nBcc: attacker@evil.com", Text: "body", } - raw := string(s.buildRaw(msg)) + raw := string(mustBuildRaw(t, s, msg)) if strings.Contains(raw, "Bcc: attacker@evil.com") { t.Error("header injection: injected Bcc header found in raw message") @@ -273,7 +285,7 @@ func TestBuildRaw_nonASCIISubjectEncoded(t *testing.T) { Subject: "Réunion d'équipe", Text: "body", } - raw := string(s.buildRaw(msg)) + raw := string(mustBuildRaw(t, s, msg)) // The raw Subject: line must not contain bare UTF-8 bytes (> 0x7E). for _, line := range strings.Split(raw, "\r\n") { @@ -298,7 +310,7 @@ func TestBuildRaw_pureASCIISubjectNotEncoded(t *testing.T) { Subject: "Booking confirmed: 30-min call", Text: "body", } - raw := string(s.buildRaw(msg)) + raw := string(mustBuildRaw(t, s, msg)) for _, line := range strings.Split(raw, "\r\n") { if strings.HasPrefix(line, "Subject:") { @@ -313,7 +325,7 @@ func TestBuildRaw_pureASCIISubjectNotEncoded(t *testing.T) { func TestBuildRaw_fromNameFormatted(t *testing.T) { s := smtpForTest() msg := Message{To: []string{"x@example.com"}, Subject: "Hi", Text: "body"} - raw := string(s.buildRaw(msg)) + raw := string(mustBuildRaw(t, s, msg)) if !strings.Contains(raw, "Calnode") { t.Error("From: header missing sender name") @@ -322,3 +334,60 @@ func TestBuildRaw_fromNameFormatted(t *testing.T) { t.Error("From: header missing sender address") } } + +// TestBuildRaw_recipientAddresses covers the To: header, which is the one header field +// assembled from caller-supplied input without an encoder in front of it. Subject and the +// From display name go through mime.QEncoding and attachment filenames through %q; a +// recipient that mail.ParseAddress refuses used to be appended verbatim, so a CR/LF inside +// it would have ended the To: line and started a header of the sender's choosing. +func TestBuildRaw_recipientAddresses(t *testing.T) { + for _, tc := range []struct { + name string + to []string + // want is the expected To: header line. Empty means buildRaw must refuse. + want string + }{ + {"plain address", []string{"bob@example.com"}, "To: "}, + {"display name is normalised into the header", []string{"Bob "}, `To: "Bob" `}, + {"two recipients", []string{"bob@example.com", "eve@example.com"}, "To: , "}, + {"CRLF in the address", []string{"a@b.example\r\nBcc: attacker@example.com"}, ""}, + {"not an address at all", []string{"not-an-address"}, ""}, + {"no recipient", nil, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + raw, err := smtpForTest().buildRaw(Message{To: tc.to, Subject: "Hi", Text: "body"}) + + if tc.want == "" { + if err == nil { + t.Fatalf("buildRaw accepted %q; want a refusal", tc.to) + } + if !errors.Is(err, ErrInvalidRecipient) { + t.Errorf("error = %v; want it to wrap ErrInvalidRecipient", err) + } + if raw != nil { + t.Errorf("buildRaw returned %d bytes alongside its error; want none", len(raw)) + } + // The offending value is attacker-controlled and this error is logged, + // so it must not carry the raw bytes back out. + if strings.Contains(err.Error(), "attacker@example.com") { + t.Errorf("error %q quotes the rejected address back", err) + } + return + } + + if err != nil { + t.Fatalf("buildRaw(%q): %v", tc.to, err) + } + var got string + for _, line := range strings.Split(string(raw), "\r\n") { + if strings.HasPrefix(line, "To:") { + got = line + break + } + } + if got != tc.want { + t.Errorf("To: header = %q; want %q", got, tc.want) + } + }) + } +} diff --git a/internal/mailer/smtp.go b/internal/mailer/smtp.go index 26a478b..5fb2d94 100644 --- a/internal/mailer/smtp.go +++ b/internal/mailer/smtp.go @@ -82,7 +82,11 @@ func newDialers(deadline time.Time, host string) (net.Dialer, tls.Dialer) { func (s *SMTP) Send(ctx context.Context, msg Message) error { addr := net.JoinHostPort(s.host, s.port) - raw := s.buildRaw(msg) + // Built before anything is dialed, so an unusable recipient costs no connection. + raw, err := s.buildRaw(msg) + if err != nil { + return err + } // Bounds the whole conversation, not just the dial (see defaultSMTPTimeout). // Whichever is EARLIER of "ctx's own deadline" and "now + the default" wins, so a @@ -174,7 +178,13 @@ func (s *SMTP) Send(ctx context.Context, msg Message) error { return nil } -func (s *SMTP) buildRaw(msg Message) []byte { +// ErrInvalidRecipient marks a recipient address that net/mail cannot parse. buildRaw +// refuses to write such an address into the To: header, so Send returns this instead of +// sending anything. The offending value is deliberately left out of the message: it is +// caller-supplied, may carry CR/LF, and this error is logged. +var ErrInvalidRecipient = errors.New("mailer: invalid recipient address") + +func (s *SMTP) buildRaw(msg Message) ([]byte, error) { from := mail.Address{Name: s.fromName, Address: s.from} // mime.QEncoding.Encode returns the string unchanged when it is pure ASCII @@ -184,16 +194,22 @@ func (s *SMTP) buildRaw(msg Message) []byte { // and satisfying RFC 2047 at the same time. subject := mime.QEncoding.Encode("utf-8", msg.Subject) - // Validate and normalise To addresses so the To: header line is properly - // quoted. Delivery uses c.Rcpt() (separate SMTP command) so a To: header - // formatting error cannot redirect mail. + // Validate and normalise To addresses so the To: header line is properly quoted. + // An address that does not parse is REFUSED rather than passed through: writing one + // verbatim is what would let a CR/LF inside it close the To: line and start a header + // of the attacker's choosing. net/smtp's Rcpt happens to reject CR/LF too, but that + // is one call away in the standard library and protects only this transport, so the + // guarantee is made here, where the header is actually assembled. + if len(msg.To) == 0 { + return nil, fmt.Errorf("%w: message has no recipient", ErrInvalidRecipient) + } toFormatted := make([]string, 0, len(msg.To)) - for _, addr := range msg.To { - if a, err := mail.ParseAddress(addr); err == nil { - toFormatted = append(toFormatted, a.String()) - } else { - toFormatted = append(toFormatted, addr) + for i, addr := range msg.To { + a, err := mail.ParseAddress(addr) + if err != nil { + return nil, fmt.Errorf("%w: recipient %d", ErrInvalidRecipient, i) } + toFormatted = append(toFormatted, a.String()) } var buf bytes.Buffer @@ -209,7 +225,7 @@ func (s *SMTP) buildRaw(msg Message) []byte { if !hasHTML && !hasAtt { buf.WriteString("Content-Type: text/plain; charset=utf-8\r\n\r\n") buf.WriteString(msg.Text) - return buf.Bytes() + return buf.Bytes(), nil } // writeBody emits the message body at the current MIME level: either a single @@ -242,7 +258,7 @@ func (s *SMTP) buildRaw(msg Message) []byte { // No attachments: the body is the whole message. if !hasAtt { writeBody(&buf) - return buf.Bytes() + return buf.Bytes(), nil } // multipart/mixed: the body part (text or multipart/alternative) followed by @@ -262,7 +278,7 @@ func (s *SMTP) buildRaw(msg Message) []byte { buf.WriteString("\r\n") } fmt.Fprintf(&buf, "--%s--\r\n", mixed) - return buf.Bytes() + return buf.Bytes(), nil } // base64Wrap base64-encodes b and wraps it at 76 characters per line (RFC 2045). diff --git a/internal/mailer/smtp_test.go b/internal/mailer/smtp_test.go index f32ebbb..9b8fbf8 100644 --- a/internal/mailer/smtp_test.go +++ b/internal/mailer/smtp_test.go @@ -2,7 +2,9 @@ package mailer import ( "context" + "errors" "net" + "sync/atomic" "testing" "time" ) @@ -95,3 +97,49 @@ func TestSMTP_Send_appliesDefaultTimeoutWithNoCtxDeadline(t *testing.T) { t.Errorf("Send took %v to fail; want it bounded by defaultSMTPTimeout (300ms), not hanging", elapsed) } } + +// TestSMTP_Send_invalidRecipientNeverDials is the half a buildRaw unit test cannot show: +// the refusal has to happen before a connection is opened, not after. +// +// The address below is stopped today by net/smtp too — Client.Rcpt runs validateLine and +// rejects any CR or LF, so the exchange would abort at RCPT TO before DATA. That is +// incidental protection living one call away in the standard library, it covers only this +// transport, and it costs a dial, an EHLO and an AUTH first. The listener here accepts and +// counts connections, so a regression that moves the check back after the dial fails loudly. +func TestSMTP_Send_invalidRecipientNeverDials(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + t.Cleanup(func() { ln.Close() }) //nolint:errcheck + + var accepted atomic.Int32 + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + accepted.Add(1) + conn.Close() //nolint:errcheck + } + }() + + host, port, err := net.SplitHostPort(ln.Addr().String()) + if err != nil { + t.Fatalf("split addr: %v", err) + } + s := NewSMTP(host, port, "", "", false, false, "from@test.local", "Test") + + err = s.Send(context.Background(), Message{ + To: []string{"a@b.example\r\nBcc: attacker@example.com"}, + Subject: "test", + Text: "body", + }) + if !errors.Is(err, ErrInvalidRecipient) { + t.Fatalf("Send error = %v; want ErrInvalidRecipient", err) + } + if n := accepted.Load(); n != 0 { + t.Errorf("Send opened %d connection(s) before refusing the recipient; want 0", n) + } +}