diff --git a/docs/content/docs/guides/warmup.mdx b/docs/content/docs/guides/warmup.mdx index a3718d54..5bcdcf31 100644 --- a/docs/content/docs/guides/warmup.mdx +++ b/docs/content/docs/guides/warmup.mdx @@ -19,6 +19,8 @@ With warmup enabled, Warmbly repeatedly picks a partner mailbox (avoiding recent Every warmup send carries a hidden verification token so the receiver can confirm it is genuine warmup traffic. Warmup mail is plaintext and never carries open or click pixels, because it needs to look like ordinary personal email. +The token travels in a message header, and some providers do not pass custom headers on to the recipient. Microsoft in particular strips them in transit and replaces the message identifier, so a warmup email sent from an Outlook or Microsoft 365 mailbox arrives carrying no header at all. Warmbly therefore also records what each send was addressed to, and the message identifier the provider assigned to it, and matches an incoming warmup email on those when the header is gone. Verification does not depend on any one provider behaving well, so warmup from every mailbox type counts, stays out of your unibox, and gets the same engagement. + Enabling warmup controls **outbound** scheduling only. An active mailbox with warmup off stays available as a recipient-only participant, and never starts sending just because it received something. ### Where the content comes from diff --git a/docs/content/docs/learn/email-warmup.mdx b/docs/content/docs/learn/email-warmup.mdx index 1ee93e70..e7513e02 100644 --- a/docs/content/docs/learn/email-warmup.mdx +++ b/docs/content/docs/learn/email-warmup.mdx @@ -37,6 +37,8 @@ Warmup recipients should be other warmed, healthy mailboxes: not free trial mail Every warmup message Warmbly sends carries a verification token. Missing, malformed or replayed tokens are tracked per mailbox and feed a rolling spam score. Above the threshold, the mailbox is removed from the pool until it requalifies. +A token carried in a header only works if the header survives delivery, and not every provider passes custom headers through. Warmbly records the subject and the provider-assigned message identifier for each send as well, so a warmup email whose header was stripped in transit is still recognized at the recipient rather than being treated as ordinary mail. + ## What bad warmup looks like - **Synthetic blasting**: sending 200 identical messages a day to the same handful of recipients. Looks great on a vendor dashboard. Looks awful to Gmail. diff --git a/internal/app/consumer/event_new_email.go b/internal/app/consumer/event_new_email.go index e16163eb..007deae7 100644 --- a/internal/app/consumer/event_new_email.go +++ b/internal/app/consumer/event_new_email.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "net/mail" "slices" "strings" "time" @@ -39,6 +40,11 @@ func (s *JobsService) HandleNewEmail(ctx context.Context, e *models.JobEventNewE if handled { return nil // Don't add to unibox } + } else if s.handleUnmarkedWarmupEmail(ctx, e) { + // Warmup whose verify header did not survive delivery. Every Microsoft + // mailbox sends this way, so without this branch its warmup mail is + // filed as ordinary inbox mail at every recipient. + return nil } // Normal email processing @@ -146,8 +152,73 @@ func (s *JobsService) handleWarmupEmail(ctx context.Context, e *models.JobEventN return false, nil } - // Valid! Consume the token - s.WarmupRepo.ConsumeWarmupToken(ctx, tokenUUID) + s.acceptWarmupEmail(ctx, e, token) + return true, nil +} + +// handleUnmarkedWarmupEmail verifies warmup mail that arrived without its +// verify header. Microsoft Graph drops custom headers in transit (and +// re-stamps the Message-ID), so mail sent from an Outlook or Microsoft 365 +// mailbox reaches every recipient carrying no marker at all; matched only on +// the header it would count for nobody and be filed as ordinary inbox mail. +// +// Unlike the header path a miss here is not suspicious — almost every message +// that reaches this point is simply ordinary mail — so nothing is recorded as +// an invalid attempt. +func (s *JobsService) handleUnmarkedWarmupEmail(ctx context.Context, e *models.JobEventNewEmail) bool { + if s.WarmupRepo == nil || e.Message == nil { + return false + } + token, err := s.WarmupRepo.FindDeliveredWarmupToken( + ctx, + e.Message.EmailID, + firstSenderAddress(e.Message.FromAddr), + e.Message.MessageID, + e.Message.Subject, + ) + if err != nil { + CaptureError(e.UserID, e.Message.EmailID, fmt.Errorf("unmarked warmup lookup: %w", err)) + return false + } + if token == nil { + return false + } + log.Debug(). + Str("token", token.Token.String()). + Str("email_account_id", e.Message.EmailID.String()). + Msg("verified warmup mail that arrived without its verify header") + s.acceptWarmupEmail(ctx, e, token) + return true +} + +// firstSenderAddress pulls the bare address out of the first From value +// ("Name " or a bare address). +func firstSenderAddress(from []string) string { + for _, raw := range from { + raw = strings.TrimSpace(raw) + if raw == "" { + continue + } + if addr, err := mail.ParseAddress(raw); err == nil { + return strings.TrimSpace(addr.Address) + } + if i := strings.LastIndex(raw, "<"); i >= 0 { + if j := strings.Index(raw[i:], ">"); j > 0 { + return strings.TrimSpace(raw[i+1 : i+j]) + } + } + if strings.Contains(raw, "@") { + return raw + } + } + return "" +} + +// acceptWarmupEmail consumes a verified token and runs everything that follows +// from a warmup email having arrived. Shared by both verification paths so the +// header and the header-less route cannot drift apart. +func (s *JobsService) acceptWarmupEmail(ctx context.Context, e *models.JobEventNewEmail, token *models.WarmupToken) { + s.WarmupRepo.ConsumeWarmupToken(ctx, token.Token) // Record the receipt so a later deletion or spam-flag of THIS message can be // attributed back to warmup and to the sender. Verified warmup mail is not @@ -172,7 +243,6 @@ func (s *JobsService) handleWarmupEmail(ctx context.Context, e *models.JobEventN // Perform warmup actions s.performWarmupActions(ctx, e) - return true, nil } // performWarmupActions publishes warmup action events to the worker. Action diff --git a/internal/app/consumer/event_send_result.go b/internal/app/consumer/event_send_result.go index ed5fe15e..d1f24580 100644 --- a/internal/app/consumer/event_send_result.go +++ b/internal/app/consumer/event_send_result.go @@ -41,13 +41,27 @@ func (s *JobsService) HandleEmailSent(ctx context.Context, result models.SendEma log.Warn().Str("task_id", result.TaskID.String()).Msg("email sent result for unknown task") return nil } - if result.MessageID != "" && task.MessageID == "" { + // The worker reports the Message-ID the provider put on the wire, which is + // not always the one the control plane minted: Graph re-stamps it. Take the + // worker's answer whenever it differs, because everything that matches a + // send back to us later (campaign reply threading, warmup reply candidates) + // keys on what the recipient actually received. + if result.MessageID != "" && result.MessageID != task.MessageID { if err := s.TaskRepo.UpdateTaskMessageID(ctx, task.ID, result.MessageID); err != nil { log.Warn().Err(err).Str("task_id", task.ID.String()).Msg("could not record worker message id") } } - if task.TaskType == "campaign" { + switch task.TaskType { + case "campaign": s.repairCampaignSendStamp(ctx, task) + case "warmup": + // Without this the recipient has nothing to match a warmup email + // against when the verify header did not survive delivery. + if s.WarmupRepo != nil && result.MessageID != "" { + if err := s.WarmupRepo.RecordWarmupTokenDelivery(ctx, task.ID, result.MessageID); err != nil { + log.Warn().Err(err).Str("task_id", task.ID.String()).Msg("could not record the delivered warmup message id") + } + } } return nil } diff --git a/internal/app/consumer/warmup_verification_live_test.go b/internal/app/consumer/warmup_verification_live_test.go new file mode 100644 index 00000000..0fdc06b3 --- /dev/null +++ b/internal/app/consumer/warmup_verification_live_test.go @@ -0,0 +1,411 @@ +package jobs + +import ( + "context" + "os" + "testing" + "time" + + "github.com/google/uuid" + + "github.com/warmbly/warmbly/internal/config" + "github.com/warmbly/warmbly/internal/infrastructure/db" + "github.com/warmbly/warmbly/internal/models" + "github.com/warmbly/warmbly/internal/repository" +) + +// Live checks of warmup verification against a real Postgres. Skipped unless +// WARMBLY_TEST_DB is set: +// +// WARMBLY_TEST_DB=postgres://warmbly:warmbly@localhost:15432/warmbly_dev?sslmode=disable \ +// go test ./internal/app/consumer/ -run Live -v +// +// Issue #193: Microsoft Graph drops custom headers in transit and re-stamps the +// Message-ID, so warmup sent from an Outlook mailbox arrives at every recipient +// carrying no verify header and under an id Warmbly never minted. Matched only +// on the header it counted for nobody and was filed in the recipient's unibox +// as ordinary mail. What is worth proving here is the recipient-side +// resolution, which is all SQL. + +type warmupFixture struct { + senderUser, senderOrg, sender uuid.UUID + partnerUser, partnerOrg, partner uuid.UUID + senderEmail, partnerEmail string + task uuid.UUID + subject, mintedMessageID, sentMsgID string +} + +func newWarmupFixture(t *testing.T, handle *db.DB) *warmupFixture { + t.Helper() + ctx := context.Background() + pool := handle.Pool + f := &warmupFixture{ + senderUser: uuid.New(), senderOrg: uuid.New(), sender: uuid.New(), + partnerUser: uuid.New(), partnerOrg: uuid.New(), partner: uuid.New(), + task: uuid.New(), + subject: "quick learning question", + mintedMessageID: "<" + uuid.NewString() + "@outlook.com>", + sentMsgID: "", + } + f.senderEmail = "wsender-" + f.sender.String()[:8] + "@outlook.com" + f.partnerEmail = "wpartner-" + f.partner.String()[:8] + "@test.local" + + exec := func(sql string, args ...any) { + t.Helper() + if _, err := pool.Exec(ctx, sql, args...); err != nil { + t.Fatalf("fixture %q: %v", sql[:min(60, len(sql))], err) + } + } + mailbox := func(user, org, id uuid.UUID, email, provider string) { + exec(`INSERT INTO users (id, email, first_name, last_name) VALUES ($1, $2, 'Warm', 'Up')`, + user, "wu-"+user.String()[:8]+"@test.local") + exec(`INSERT INTO organizations (id, name, slug, owner_user_id) VALUES ($1, 'Warmup Live', $2, $3)`, + org, "wu-"+org.String()[:8], user) + exec(`INSERT INTO email_accounts (id, user_id, organization_id, email, name, + signature_plain, signature_html, provider, status, campaign_limit, min_wait_time, timezone) + VALUES ($1, $2, $3, $4, 'Warmup', '', '', $5, 'active', 50, 600, 'UTC')`, + id, user, org, email, provider) + } + mailbox(f.senderUser, f.senderOrg, f.sender, f.senderEmail, "outlook") + mailbox(f.partnerUser, f.partnerOrg, f.partner, f.partnerEmail, "smtp_imap") + + // completed_at is backdated past the reply path's 45-minute human-timing + // floor so GetLatestReplyCandidate can see this send. + now := time.Now() + exec(`INSERT INTO tasks (id, task_type, email_account_id, status, scheduled_at, completed_at, message_id) + VALUES ($1, 'warmup', $2, 'completed', $3, NOW() - INTERVAL '2 hours', $4)`, + f.task, f.sender, now, f.mintedMessageID) + + t.Cleanup(func() { + c := context.Background() + for _, s := range []struct { + sql string + arg any + }{ + {`DELETE FROM unibox_emails WHERE email_id = $1`, f.partner}, + {`DELETE FROM warmup_received WHERE sender_account_id = $1`, f.sender}, + {`DELETE FROM warmup_tokens WHERE sender_account_id = $1`, f.sender}, + {`DELETE FROM tasks WHERE email_account_id = $1`, f.sender}, + {`DELETE FROM email_accounts WHERE id = $1`, f.sender}, + {`DELETE FROM email_accounts WHERE id = $1`, f.partner}, + {`DELETE FROM organizations WHERE id = $1`, f.senderOrg}, + {`DELETE FROM organizations WHERE id = $1`, f.partnerOrg}, + {`DELETE FROM users WHERE id = $1`, f.senderUser}, + {`DELETE FROM users WHERE id = $1`, f.partnerUser}, + } { + if _, err := pool.Exec(c, s.sql, s.arg); err != nil { + t.Errorf("cleanup %q: %v", s.sql, err) + } + } + }) + return f +} + +// mintToken writes the token the warmup task would have created. +func (f *warmupFixture) mintToken(t *testing.T, repo repository.WarmupRepository) uuid.UUID { + t.Helper() + token := uuid.New() + err := repo.CreateWarmupToken(context.Background(), &models.WarmupToken{ + Token: token, + TaskID: f.task, + SenderAccountID: f.sender, + RecipientAccountID: f.partner, + ConversationTheme: "learning", + ContentSource: models.WarmupContentSourceStatic, + Subject: f.subject, + ExpiresAt: time.Now().Add(7 * 24 * time.Hour), + }) + if err != nil { + t.Fatalf("create warmup token: %v", err) + } + return token +} + +// arrival is the inbound event the recipient's worker produces. Outlook-sent +// warmup has no verify pseudo-flag and a Message-ID Warmbly never minted. +func (f *warmupFixture) arrival(messageID string, flags []string) *models.JobEventNewEmail { + return &models.JobEventNewEmail{ + UserID: f.partnerUser, + Message: &models.EmailMessageStoreData{ + ID: uuid.New(), + EmailID: f.partner, + MessageID: messageID, + Subject: f.subject, + FromAddr: []string{"Warm Up <" + f.senderEmail + ">"}, + ToAddr: []string{f.partnerEmail}, + Flags: flags, + }, + } +} + +func liveWarmupService(t *testing.T) (*JobsService, *db.DB) { + t.Helper() + dsn := os.Getenv("WARMBLY_TEST_DB") + if dsn == "" { + t.Skip("WARMBLY_TEST_DB not set") + } + handle, err := db.New(context.Background(), dsn) + if err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { handle.Pool.Close() }) + return &JobsService{ + TaskRepo: repository.NewTaskRepository(handle.Pool), + WarmupRepo: repository.NewWarmupRepository(handle.Pool), + UniboxRepository: repository.NewUniboxRepository(handle), + EmailRepository: repository.NewEmailRepostory(handle, nil), + }, handle +} + +// The regression: a warmup email that arrives with no verify header must still +// be verified, consumed and kept out of the recipient's unibox. +func TestLiveWarmupVerifiesByDeliveredMessageIDWhenTheHeaderIsStripped(t *testing.T) { + s, handle := liveWarmupService(t) + ctx := context.Background() + f := newWarmupFixture(t, handle) + token := f.mintToken(t, s.WarmupRepo) + + // The worker answers the send with the id Exchange stamped, not ours. + if err := s.HandleEmailSent(ctx, models.SendEmailResult{ + TaskID: f.task, Success: true, MessageID: f.sentMsgID, + }); err != nil { + t.Fatalf("HandleEmailSent: %v", err) + } + + // The task now carries what the recipient will actually see, so warmup + // reply threading and campaign reply matching can find it. + task, err := s.TaskRepo.GetTask(ctx, f.task) + if err != nil || task == nil { + t.Fatalf("get task: %v", err) + } + if task.MessageID != f.sentMsgID { + t.Errorf("task message_id = %q, want the delivered id %q", task.MessageID, f.sentMsgID) + } + + stored, err := s.WarmupRepo.FindWarmupToken(ctx, token) + if err != nil || stored == nil { + t.Fatalf("find token: %v", err) + } + if stored.SentMessageID != f.sentMsgID { + t.Errorf("token sent_message_id = %q, want %q", stored.SentMessageID, f.sentMsgID) + } + + // Arrival at the partner: no verify flag anywhere. + if !s.handleUnmarkedWarmupEmail(ctx, f.arrival(f.sentMsgID, nil)) { + t.Fatal("warmup arriving without its verify header must still be verified") + } + if stored, _ = s.WarmupRepo.FindWarmupToken(ctx, token); stored == nil || stored.ConsumedAt == nil { + t.Error("the token must be consumed, or the send counts for nobody") + } +} + +// The Message-ID is the strongest key, but it depends on the worker's send +// result having landed first. When it has not, the pending token for this +// exact sender/recipient/subject still resolves. +func TestLiveWarmupVerifiesByPairAndSubjectBeforeTheSendResultLands(t *testing.T) { + s, handle := liveWarmupService(t) + ctx := context.Background() + f := newWarmupFixture(t, handle) + token := f.mintToken(t, s.WarmupRepo) + + // No HandleEmailSent yet: sent_message_id is still empty. + if !s.handleUnmarkedWarmupEmail(ctx, f.arrival("<"+uuid.NewString()+"@outlook.com>", nil)) { + t.Fatal("a pending token for this pair and subject must resolve") + } + stored, err := s.WarmupRepo.FindWarmupToken(ctx, token) + if err != nil || stored == nil || stored.ConsumedAt == nil { + t.Errorf("token should be consumed: %+v (err %v)", stored, err) + } +} + +// The pair fallback must not swallow real mail. A different subject from the +// same sender is ordinary mail and belongs in the unibox. +func TestLiveWarmupPairFallbackIgnoresADifferentSubject(t *testing.T) { + s, handle := liveWarmupService(t) + ctx := context.Background() + f := newWarmupFixture(t, handle) + f.mintToken(t, s.WarmupRepo) + + e := f.arrival("<"+uuid.NewString()+"@outlook.com>", nil) + e.Message.Subject = "your invoice is overdue" + if s.handleUnmarkedWarmupEmail(ctx, e) { + t.Error("mail with a different subject must not claim a pending warmup token") + } +} + +// ...and neither must mail from someone who is not the token's sender. +func TestLiveWarmupPairFallbackIgnoresADifferentSender(t *testing.T) { + s, handle := liveWarmupService(t) + ctx := context.Background() + f := newWarmupFixture(t, handle) + f.mintToken(t, s.WarmupRepo) + + e := f.arrival("<"+uuid.NewString()+"@outlook.com>", nil) + e.Message.FromAddr = []string{"Someone Else "} + if s.handleUnmarkedWarmupEmail(ctx, e) { + t.Error("mail from a different sender must not claim a pending warmup token") + } +} + +// A token that was already consumed cannot be claimed twice, so a duplicate +// delivery of the same warmup message falls through to normal processing +// rather than firing the engagement plan again. +func TestLiveWarmupConsumedTokenIsNotReclaimed(t *testing.T) { + s, handle := liveWarmupService(t) + ctx := context.Background() + f := newWarmupFixture(t, handle) + token := f.mintToken(t, s.WarmupRepo) + if err := s.WarmupRepo.ConsumeWarmupToken(ctx, token); err != nil { + t.Fatalf("consume: %v", err) + } + if s.handleUnmarkedWarmupEmail(ctx, f.arrival("<"+uuid.NewString()+"@outlook.com>", nil)) { + t.Error("an already-consumed token must not be claimed again") + } +} + +// The header path is untouched: providers that keep the header still verify +// through it, and the header-less lookup is not consulted. +func TestLiveWarmupStillVerifiesThroughTheHeaderWhenItSurvives(t *testing.T) { + s, handle := liveWarmupService(t) + ctx := context.Background() + f := newWarmupFixture(t, handle) + token := f.mintToken(t, s.WarmupRepo) + + e := f.arrival(f.mintedMessageID, []string{config.WarmupVerifyHeader + ":" + token.String()}) + handled, err := s.handleWarmupEmail(ctx, e, token.String()) + if err != nil { + t.Fatalf("handleWarmupEmail: %v", err) + } + if !handled { + t.Fatal("a valid verify header must still verify") + } + stored, _ := s.WarmupRepo.FindWarmupToken(ctx, token) + if stored == nil || stored.ConsumedAt == nil { + t.Error("the token must be consumed") + } +} + +// The symptom the issue reports: unverified warmup mail becomes an ordinary +// unibox entry in the recipient's inbox. This drives the whole handler, so it +// proves the message never reaches the unibox at all. +func TestLiveWarmupEmailWithNoHeaderNeverReachesTheUnibox(t *testing.T) { + s, handle := liveWarmupService(t) + ctx := context.Background() + f := newWarmupFixture(t, handle) + f.mintToken(t, s.WarmupRepo) + + e := f.arrival("<"+uuid.NewString()+"@outlook.com>", nil) + if err := s.HandleNewEmail(ctx, e); err != nil { + t.Fatalf("HandleNewEmail: %v", err) + } + var stored int + if err := handle.Pool.QueryRow(ctx, + `SELECT COUNT(*) FROM unibox_emails WHERE email_id = $1`, f.partner).Scan(&stored); err != nil { + t.Fatalf("count unibox: %v", err) + } + if stored != 0 { + t.Errorf("verified warmup mail must not be filed in the unibox, found %d entries", stored) + } + + // A message with no pending token is ordinary mail and still lands. + other := f.arrival("<"+uuid.NewString()+"@outlook.com>", nil) + other.Message.Subject = "actual customer question" + if err := s.HandleNewEmail(ctx, other); err != nil { + t.Fatalf("HandleNewEmail (ordinary): %v", err) + } + if err := handle.Pool.QueryRow(ctx, + `SELECT COUNT(*) FROM unibox_emails WHERE email_id = $1`, f.partner).Scan(&stored); err != nil { + t.Fatalf("count unibox: %v", err) + } + if stored != 1 { + t.Errorf("ordinary mail must still be filed in the unibox, found %d entries", stored) + } +} + +func TestFirstSenderAddress(t *testing.T) { + for _, tc := range []struct{ in, want string }{ + {"Warm Up ", "a@b.com"}, + {"a@b.com", "a@b.com"}, + {`"Up, Warm" `, "a@b.com"}, + {"", "a@b.com"}, + {"not an address", ""}, + {"", ""}, + } { + if got := firstSenderAddress([]string{tc.in}); got != tc.want { + t.Errorf("firstSenderAddress(%q) = %q, want %q", tc.in, got, tc.want) + } + } + if got := firstSenderAddress(nil); got != "" { + t.Errorf("firstSenderAddress(nil) = %q, want empty", got) + } +} + +// The other half of the Message-ID rewrite: a partner replying to this send +// threads on whatever the reply candidate reports. Reporting the id we minted +// points In-Reply-To at a message the Outlook mailbox has never held, so the +// reply lands as a new conversation and warmup stops looking conversational. +func TestLiveWarmupReplyCandidateUsesTheDeliveredMessageID(t *testing.T) { + s, handle := liveWarmupService(t) + ctx := context.Background() + f := newWarmupFixture(t, handle) + f.mintToken(t, s.WarmupRepo) + + if err := s.HandleEmailSent(ctx, models.SendEmailResult{ + TaskID: f.task, Success: true, MessageID: f.sentMsgID, + }); err != nil { + t.Fatalf("HandleEmailSent: %v", err) + } + + candidate, err := s.WarmupRepo.GetLatestReplyCandidate(ctx, f.sender, f.partner) + if err != nil { + t.Fatalf("GetLatestReplyCandidate: %v", err) + } + if candidate == nil { + t.Fatal("the send should be replyable") + } + if candidate.MessageID != f.sentMsgID { + t.Errorf("reply candidate message id = %q, want the delivered id %q", candidate.MessageID, f.sentMsgID) + } +} + +// An expired token is not claimable: a stale pending row must not swallow mail +// that arrives days later. +func TestLiveWarmupExpiredTokenIsNotClaimed(t *testing.T) { + s, handle := liveWarmupService(t) + ctx := context.Background() + f := newWarmupFixture(t, handle) + token := f.mintToken(t, s.WarmupRepo) + if _, err := handle.Pool.Exec(ctx, + `UPDATE warmup_tokens SET expires_at = NOW() - INTERVAL '1 hour' WHERE token = $1`, token); err != nil { + t.Fatalf("expire token: %v", err) + } + if s.handleUnmarkedWarmupEmail(ctx, f.arrival("<"+uuid.NewString()+"@outlook.com>", nil)) { + t.Error("an expired token must not be claimed") + } +} + +// The pair fallback is time-boxed, so an old pending token cannot claim mail +// that happens to repeat the subject much later. +func TestLiveWarmupPairFallbackIsTimeBoxed(t *testing.T) { + s, handle := liveWarmupService(t) + ctx := context.Background() + f := newWarmupFixture(t, handle) + token := f.mintToken(t, s.WarmupRepo) + if _, err := handle.Pool.Exec(ctx, + `UPDATE warmup_tokens SET created_at = NOW() - INTERVAL '5 days' WHERE token = $1`, token); err != nil { + t.Fatalf("backdate token: %v", err) + } + if s.handleUnmarkedWarmupEmail(ctx, f.arrival("<"+uuid.NewString()+"@outlook.com>", nil)) { + t.Error("the pair fallback must not reach back past its window") + } + + // The Message-ID key has no such window: it is exact, so it still resolves. + if err := s.HandleEmailSent(ctx, models.SendEmailResult{ + TaskID: f.task, Success: true, MessageID: f.sentMsgID, + }); err != nil { + t.Fatalf("HandleEmailSent: %v", err) + } + if !s.handleUnmarkedWarmupEmail(ctx, f.arrival(f.sentMsgID, nil)) { + t.Error("an exact delivered Message-ID must still resolve an older token") + } +} diff --git a/internal/app/worker/wmail/send.go b/internal/app/worker/wmail/send.go index 6187bb61..2c77f153 100644 --- a/internal/app/worker/wmail/send.go +++ b/internal/app/worker/wmail/send.go @@ -223,9 +223,11 @@ func (w *WMail) sendViaGmail(ctx context.Context, req *SendRequest, bodyHTML str return result } -// sendViaGraph sends an email through Microsoft Graph (RAW MIME sendMail). -// sendMail returns 202 with no provider id, so ProviderMsgID is the RFC -// Message-ID we minted, mirroring the SMTP path. +// sendViaGraph sends an email through Microsoft Graph. Graph re-stamps the +// Message-ID we mint, so the client creates the message as a draft, reads the +// id Exchange assigned, and sends that; the result carries that id rather than +// ours so warmup verification and campaign reply threading match what the +// recipient actually received. func (w *WMail) sendViaGraph(ctx context.Context, req *SendRequest, bodyHTML string) *SendResult { result := &SendResult{ Success: false, @@ -249,12 +251,14 @@ func (w *WMail) sendViaGraph(ctx context.Context, req *SendRequest, bodyHTML str // and no local thread record. parent := parentReference(req) - // Warmup token + RFC 8058 one-click unsubscribe headers; RAW MIME carries - // them verbatim (the JSON message shape cannot). + // Warmup token + RFC 8058 one-click unsubscribe headers. RAW MIME is the + // only Graph shape that can carry them, though Exchange still drops + // custom headers in transit, which is why warmup verification does not + // depend on the token header arriving. customHeaders := buildSendHeaders(req) attachments := toGraphAttachments(req.Attachments) - err := w.GraphData.Client.SendMessage( + sentMessageID, err := w.GraphData.Client.SendMessage( ctx, req.To, req.Cc, @@ -282,9 +286,14 @@ func (w *WMail) sendViaGraph(ctx context.Context, req *SendRequest, bodyHTML str return result } + // Report the id Exchange stamped, not the one we minted: it is the only + // value that matches what the recipient received. + if sentMessageID == "" { + sentMessageID = req.MessageID + } result.Success = true - result.MessageID = req.MessageID - result.ProviderMsgID = req.MessageID // Graph sendMail returns no id + result.MessageID = sentMessageID + result.ProviderMsgID = sentMessageID // Graph send returns no separate provider id return result } diff --git a/internal/client/msgraph/send.go b/internal/client/msgraph/send.go index 879fb3a1..668c60d7 100644 --- a/internal/client/msgraph/send.go +++ b/internal/client/msgraph/send.go @@ -3,26 +3,35 @@ package msgraph import ( "context" "encoding/base64" + "encoding/json" "fmt" "io" "net/http" "strings" + "github.com/rs/zerolog/log" "github.com/warmbly/warmbly/internal/errx" "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/pkg/mailhdr" ) -// SendMessage sends a message through Graph's MIME sendMail endpoint. We build a -// full RFC 5322 message ourselves and POST it base64-encoded with -// Content-Type: text/plain, because only the MIME path lets us set a custom -// Message-ID, In-Reply-To/References for threading, and arbitrary headers (the -// warmup verification token, RFC 8058 one-click unsubscribe) that the JSON -// message shape restricts. Graph auto-files the message in Sent Items. +// SendMessage sends a message through Graph and returns the RFC 5322 +// Message-ID the recipient will actually see. // -// sendMail returns 202 with no id, so there is no provider message id to return; -// callers record the RFC Message-ID they supplied (inbound correlation keys on -// that anyway). customHeaders is variadic to mirror goog.Client.SendMessage. +// It creates the message as a draft and then sends the draft, rather than +// posting to /me/sendMail, because Exchange stamps its own internetMessageId +// when the item is created and discards the Message-ID we supply. Mail sent +// with sendMail therefore reaches recipients under an id Warmbly has never +// seen, which leaves warmup tokens unverifiable and campaign replies +// unthreadable. Creating the draft first lets us read that id before the +// message leaves, so the draft MIME deliberately carries no Message-ID of our +// own. messageID is only used by the sendMail fallback below. +// +// The body is still built as RFC 5322 MIME, which is the only Graph shape that +// can carry In-Reply-To/References and arbitrary headers. Note that Exchange +// drops custom headers in transit even so: the verify header rides along for +// the providers that keep it, and is not what verification depends on. +// customHeaders is variadic to mirror goog.Client.SendMessage. func (c *Client) SendMessage( ctx context.Context, to, cc, bcc []string, @@ -31,16 +40,56 @@ func (c *Client) SendMessage( parent *models.EmailMessageData, attachments []Attachment, customHeaders ...map[string]string, -) error { +) (string, error) { + raw, err := buildMIME(sendHeaders(c.GetAddress(), to, cc, bcc, "", subject, parent, customHeaders...), bodyPlain, bodyHTML, attachments) + if err != nil { + return "", fmt.Errorf("build mime: %w", err) + } + + draftID, assignedID, err := c.createDraft(ctx, raw) + if err != nil { + // Nothing was created, so nothing can be double-sent: fall back to the + // single-shot path with our own Message-ID. The send still lands; only + // the id we learn is lost. + log.Warn().Err(err).Str("email", c.Email).Msg("graph draft creation failed; sending without a readable message id") + fallback, berr := buildMIME(sendHeaders(c.GetAddress(), to, cc, bcc, messageID, subject, parent, customHeaders...), bodyPlain, bodyHTML, attachments) + if berr != nil { + return "", fmt.Errorf("build mime: %w", berr) + } + if serr := c.sendMIME(ctx, fallback); serr != nil { + return "", serr + } + return messageID, nil + } + + if assignedID == "" { + assignedID = c.draftMessageID(ctx, draftID) + } + + if err := c.sendDraft(ctx, draftID); err != nil { + // The draft is still sitting in Drafts; leaving it there would show up + // in the customer's own mail client as an unsent message. + c.discardDraft(ctx, draftID) + return "", err + } + return assignedID, nil +} + +// sendHeaders assembles the top-level RFC 5322 headers for an outbound +// message. A blank messageID omits the header entirely, which is what the +// draft path wants: Exchange then assigns the id and we read it back. +func sendHeaders(from string, to, cc, bcc []string, messageID, subject string, parent *models.EmailMessageData, customHeaders ...map[string]string) []hdr { hdrs := []hdr{ - {"From", c.GetAddress()}, + {"From", from}, {"To", mailhdr.AddressList(to)}, // RFC 2047: a non-ASCII subject or display name has to be encoded or // it reaches the recipient as mojibake. No-op for plain ASCII. {"Subject", mailhdr.Subject(subject)}, - {"Message-ID", messageID}, {"MIME-Version", "1.0"}, } + if messageID != "" { + hdrs = append(hdrs, hdr{"Message-ID", messageID}) + } if len(cc) > 0 { hdrs = append(hdrs, hdr{"Cc", mailhdr.AddressList(cc)}) } @@ -58,13 +107,87 @@ func (c *Client) SendMessage( hdrs = append(hdrs, hdr{k, v}) } } + return hdrs +} - raw, err := buildMIME(hdrs, bodyPlain, bodyHTML, attachments) +// createDraft posts the MIME message as a draft and returns the Graph item id +// plus the internetMessageId Exchange assigned to it. +func (c *Client) createDraft(ctx context.Context, raw []byte) (string, string, error) { + encoded := base64.StdEncoding.EncodeToString(raw) + resp, err := c.do(ctx, http.MethodPost, graphBase+"/me/messages", "text/plain", []byte(encoded)) if err != nil { - return fmt.Errorf("build mime: %w", err) + return "", "", errx.ErrMailServerUnreachable } + defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return "", "", HandleError(resp) + } + var created struct { + ID string `json:"id"` + InternetMessageID string `json:"internetMessageId"` + } + if err := json.NewDecoder(resp.Body).Decode(&created); err != nil { + return "", "", fmt.Errorf("decode draft: %w", err) + } + if created.ID == "" { + return "", "", fmt.Errorf("graph created a draft without an id") + } + return created.ID, created.InternetMessageID, nil +} - // MIME sendMail: the request body is the base64 of the RFC 5322 message. +// draftMessageID re-reads the assigned internetMessageId. Returns "" rather +// than an error: an unknown id costs verification accuracy, not the send. +func (c *Client) draftMessageID(ctx context.Context, draftID string) string { + var msg struct { + InternetMessageID string `json:"internetMessageId"` + } + if err := c.doJSON(ctx, http.MethodGet, c.messageURL(draftID)+"?$select=internetMessageId", nil, &msg); err != nil { + log.Warn().Err(err).Str("email", c.Email).Msg("could not read the message id Graph assigned to the draft") + return "" + } + return msg.InternetMessageID +} + +// sendDraft submits a created draft. +func (c *Client) sendDraft(ctx context.Context, draftID string) error { + resp, err := c.do(ctx, http.MethodPost, c.messageURL(draftID)+"/send", "", nil) + if err != nil { + return errx.ErrMailServerUnreachable + } + defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return HandleError(resp) + } + _, _ = io.Copy(io.Discard, resp.Body) + return nil +} + +// discardDraft removes a draft whose send failed, so it does not sit in the +// customer's Drafts folder as a message they never wrote. +// +// It confirms the item is still a draft first. A send that failed ambiguously +// (a lost response) may in fact have gone out, and deleting the wrong item +// would take the customer's Sent Items copy with it. Best effort otherwise: +// the send has already failed and will be retried. +func (c *Client) discardDraft(ctx context.Context, draftID string) { + var msg struct { + IsDraft bool `json:"isDraft"` + } + if err := c.doJSON(ctx, http.MethodGet, c.messageURL(draftID)+"?$select=isDraft", nil, &msg); err != nil || !msg.IsDraft { + return + } + resp, err := c.do(ctx, http.MethodDelete, c.messageURL(draftID), "", nil) + if err != nil { + return + } + defer resp.Body.Close() + _, _ = io.Copy(io.Discard, resp.Body) +} + +// sendMIME is the single-shot /me/sendMail path, kept as the fallback for when +// a draft cannot be created. Graph auto-files the message in Sent Items and +// returns 202 with no id. +func (c *Client) sendMIME(ctx context.Context, raw []byte) error { encoded := base64.StdEncoding.EncodeToString(raw) resp, err := c.do(ctx, http.MethodPost, graphBase+"/me/sendMail", "text/plain", []byte(encoded)) if err != nil { diff --git a/internal/client/msgraph/send_test.go b/internal/client/msgraph/send_test.go new file mode 100644 index 00000000..2f7355ce --- /dev/null +++ b/internal/client/msgraph/send_test.go @@ -0,0 +1,228 @@ +package msgraph + +import ( + "context" + "encoding/base64" + "io" + "net/http" + "strings" + "testing" + + "github.com/warmbly/warmbly/internal/models" +) + +// sendRT answers the three calls the draft send path makes and records the +// method, path and decoded MIME of each. +type sendRT struct { + createStatus int + sendStatus int + assignedID string + isDraft string // "true" unless a test says the item already left + + calls []string + draftRaw string + sendRaw string +} + +func (s *sendRT) RoundTrip(req *http.Request) (*http.Response, error) { + path := req.Method + " " + req.URL.Path + s.calls = append(s.calls, path) + + body := "" + if req.Body != nil { + b, _ := io.ReadAll(req.Body) + if decoded, err := base64.StdEncoding.DecodeString(string(b)); err == nil { + body = string(decoded) + } + } + + json := func(status int, payload string) (*http.Response, error) { + return &http.Response{ + StatusCode: status, + Body: io.NopCloser(strings.NewReader(payload)), + Header: http.Header{"Content-Type": []string{"application/json"}}, + }, nil + } + + switch { + case req.Method == http.MethodPost && strings.HasSuffix(req.URL.Path, "/sendMail"): + s.sendRaw = body + return json(202, `{}`) + case req.Method == http.MethodPost && strings.HasSuffix(req.URL.Path, "/me/messages"): + s.draftRaw = body + if s.createStatus != 0 && s.createStatus >= 300 { + return json(s.createStatus, `{"error":{"code":"ErrorAccessDenied","message":"no"}}`) + } + return json(201, `{"id":"DRAFT_ID","internetMessageId":"`+s.assignedID+`"}`) + case req.Method == http.MethodPost && strings.HasSuffix(req.URL.Path, "/send"): + if s.sendStatus != 0 && s.sendStatus >= 300 { + return json(s.sendStatus, `{"error":{"code":"ErrorItemNotFound","message":"gone"}}`) + } + return json(202, `{}`) + case req.Method == http.MethodGet: + return json(200, `{"internetMessageId":"`+s.assignedID+`","isDraft":`+s.isDraft+`}`) + case req.Method == http.MethodDelete: + return json(204, ``) + } + return json(404, `{}`) +} + +func (s *sendRT) did(method, suffix string) bool { + for _, c := range s.calls { + if strings.HasPrefix(c, method+" ") && strings.HasSuffix(c, suffix) { + return true + } + } + return false +} + +func newSendClient(rt *sendRT) *Client { + if rt.isDraft == "" { + rt.isDraft = "true" + } + return &Client{Email: "sender@outlook.com", hc: &http.Client{Transport: rt}, folderIDs: map[string]string{}} +} + +func send(c *Client, headers map[string]string, parent *models.EmailMessageData) (string, error) { + return c.SendMessage( + context.Background(), + []string{"partner@example.com"}, nil, nil, + "", + "quick learning question", + "body", "", + parent, + nil, + headers, + ) +} + +// The whole point of the change: the caller learns the Message-ID Exchange +// stamped, not the one we minted, because that is the only value the recipient +// ever sees. +func TestSendMessageReturnsTheMessageIDExchangeAssigned(t *testing.T) { + rt := &sendRT{assignedID: ""} + got, err := send(newSendClient(rt), map[string]string{"X-Mailtrace-Verify": "tok"}, nil) + if err != nil { + t.Fatalf("SendMessage: %v", err) + } + if got != "" { + t.Errorf("message id = %q, want the id Graph assigned to the draft", got) + } + if !rt.did("POST", "/me/messages") || !rt.did("POST", "/send") { + t.Errorf("expected a draft create followed by a send, got %v", rt.calls) + } + if rt.did("POST", "/sendMail") { + t.Error("sendMail must not be used when a draft can be created") + } +} + +// The draft must not carry a Message-ID of ours: Exchange would keep ours on +// the item and re-stamp it at submission, so we would read back an id the +// recipient never sees. +func TestSendMessageDraftCarriesNoMintedMessageID(t *testing.T) { + rt := &sendRT{assignedID: ""} + if _, err := send(newSendClient(rt), nil, nil); err != nil { + t.Fatalf("SendMessage: %v", err) + } + if strings.Contains(rt.draftRaw, "Message-ID:") { + t.Errorf("draft MIME should not set Message-ID:\n%s", rt.draftRaw) + } + if !strings.Contains(rt.draftRaw, "Subject: quick learning question") { + t.Errorf("draft MIME lost the subject:\n%s", rt.draftRaw) + } +} + +// Threading and the verify header still have to reach the draft; Exchange +// keeps In-Reply-To even though it drops the custom header in transit. +func TestSendMessageDraftKeepsThreadingAndCustomHeaders(t *testing.T) { + rt := &sendRT{assignedID: ""} + _, err := send(newSendClient(rt), map[string]string{"X-Mailtrace-Verify": "tok"}, &models.EmailMessageData{MessageID: "parent@x"}) + if err != nil { + t.Fatalf("SendMessage: %v", err) + } + for _, want := range []string{"In-Reply-To: ", "References: ", "X-Mailtrace-Verify: tok"} { + if !strings.Contains(rt.draftRaw, want) { + t.Errorf("draft MIME missing %q:\n%s", want, rt.draftRaw) + } + } +} + +// A mailbox that cannot create drafts must still send. Nothing was created, so +// falling back cannot double-send. +func TestSendMessageFallsBackToSendMailWhenTheDraftCannotBeCreated(t *testing.T) { + rt := &sendRT{createStatus: 403} + got, err := send(newSendClient(rt), nil, nil) + if err != nil { + t.Fatalf("SendMessage: %v", err) + } + if got != "" { + t.Errorf("fallback should report the minted id, got %q", got) + } + if !rt.did("POST", "/sendMail") { + t.Errorf("expected the sendMail fallback, got %v", rt.calls) + } + if !strings.Contains(rt.sendRaw, "Message-ID: ") { + t.Errorf("fallback MIME must carry our Message-ID:\n%s", rt.sendRaw) + } +} + +// A draft whose send is refused would otherwise sit in the customer's Drafts +// folder as a message they never wrote. +func TestSendMessageDiscardsTheDraftWhenTheSendFails(t *testing.T) { + rt := &sendRT{assignedID: "", sendStatus: 404} + if _, err := send(newSendClient(rt), nil, nil); err == nil { + t.Fatal("a refused send must return an error") + } + if !rt.did("DELETE", "/me/messages/DRAFT_ID") { + t.Errorf("expected the draft to be discarded, got %v", rt.calls) + } + if rt.did("POST", "/sendMail") { + t.Error("a created draft must never fall back to sendMail; that would send twice") + } +} + +// Graph does not always return internetMessageId on the create response. +func TestSendMessageRereadsTheAssignedIDWhenCreateOmitsIt(t *testing.T) { + // create answers with no internetMessageId; the follow-up GET has it. + rt := &sendRT{assignedID: ""} + c := newSendClient(rt) + c.hc = &http.Client{Transport: &rereadRT{sendRT: rt, reread: ""}} + got, err := send(c, nil, nil) + if err != nil { + t.Fatalf("SendMessage: %v", err) + } + if got != "" { + t.Errorf("message id = %q, want the re-read id", got) + } +} + +// rereadRT answers the create with no internetMessageId and the follow-up GET +// with one. +type rereadRT struct { + *sendRT + reread string +} + +func (r *rereadRT) RoundTrip(req *http.Request) (*http.Response, error) { + if req.Method == http.MethodGet { + r.calls = append(r.calls, req.Method+" "+req.URL.Path) + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader(`{"internetMessageId":"` + r.reread + `"}`)), + Header: http.Header{"Content-Type": []string{"application/json"}}, + }, nil + } + return r.sendRT.RoundTrip(req) +} + +// An ambiguous send failure may mean the message went out after all. Deleting +// the item then would take the customer's Sent Items copy with it. +func TestSendMessageLeavesAnItemThatIsNoLongerADraft(t *testing.T) { + rt := &sendRT{assignedID: "", sendStatus: 500, isDraft: "false"} + if _, err := send(newSendClient(rt), nil, nil); err == nil { + t.Fatal("a refused send must return an error") + } + if rt.did("DELETE", "/me/messages/DRAFT_ID") { + t.Error("an item that already left the Drafts folder must not be deleted") + } +} diff --git a/internal/infrastructure/db/migrations/000094_warmup_delivery_verification.down.sql b/internal/infrastructure/db/migrations/000094_warmup_delivery_verification.down.sql new file mode 100644 index 00000000..fe4f7e23 --- /dev/null +++ b/internal/infrastructure/db/migrations/000094_warmup_delivery_verification.down.sql @@ -0,0 +1,6 @@ +DROP INDEX IF EXISTS public.idx_warmup_tokens_pending_recipient; +DROP INDEX IF EXISTS public.idx_warmup_tokens_sent_message_id; + +ALTER TABLE public.warmup_tokens + DROP COLUMN IF EXISTS subject, + DROP COLUMN IF EXISTS sent_message_id; diff --git a/internal/infrastructure/db/migrations/000094_warmup_delivery_verification.up.sql b/internal/infrastructure/db/migrations/000094_warmup_delivery_verification.up.sql new file mode 100644 index 00000000..9ae4f389 --- /dev/null +++ b/internal/infrastructure/db/migrations/000094_warmup_delivery_verification.up.sql @@ -0,0 +1,24 @@ +-- Warmup verification can no longer assume the custom verify header survives +-- delivery. Microsoft Graph drops custom headers in transit and re-stamps the +-- Message-ID, so warmup sent from an Outlook mailbox reached every recipient +-- unmarked: the token was never consumed and the mail landed in the +-- recipient's unibox as ordinary mail. +-- +-- sent_message_id records the Message-ID the provider actually put on the +-- wire (read back from the created draft on Graph, our own elsewhere), and +-- subject records what was sent, so the recipient can still resolve the token +-- from an inbound message that carries no header. + +ALTER TABLE public.warmup_tokens + ADD COLUMN IF NOT EXISTS sent_message_id text NOT NULL DEFAULT '', + ADD COLUMN IF NOT EXISTS subject text NOT NULL DEFAULT ''; + +-- Recipient-side lookup by the delivered Message-ID. +CREATE INDEX IF NOT EXISTS idx_warmup_tokens_sent_message_id + ON public.warmup_tokens (sent_message_id) + WHERE consumed_at IS NULL AND sent_message_id <> ''; + +-- Recipient-side fallback: the pending tokens for one recipient, newest first. +CREATE INDEX IF NOT EXISTS idx_warmup_tokens_pending_recipient + ON public.warmup_tokens (recipient_account_id, sender_account_id, created_at DESC) + WHERE consumed_at IS NULL; diff --git a/internal/models/warmup.go b/internal/models/warmup.go index 58f2ebc9..379d4315 100644 --- a/internal/models/warmup.go +++ b/internal/models/warmup.go @@ -19,10 +19,16 @@ type WarmupToken struct { ContentSource string `json:"content_source"` ConversationID *uuid.UUID `json:"conversation_id,omitempty"` // ConversationTurn is zero for the opening and increments for each reply. - ConversationTurn int `json:"conversation_turn"` - CreatedAt time.Time `json:"created_at"` - ConsumedAt *time.Time `json:"consumed_at,omitempty"` - ExpiresAt time.Time `json:"expires_at"` + ConversationTurn int `json:"conversation_turn"` + // Subject is what was actually sent, and SentMessageID is the Message-ID + // the provider put on the wire (which is not the one we minted on Graph). + // Both exist so the recipient can still resolve this token when the verify + // header did not survive delivery. + Subject string `json:"subject"` + SentMessageID string `json:"sent_message_id"` + CreatedAt time.Time `json:"created_at"` + ConsumedAt *time.Time `json:"consumed_at,omitempty"` + ExpiresAt time.Time `json:"expires_at"` } // WarmupEmailAction represents actions to perform on a detected warmup email. diff --git a/internal/repository/pg_warmup.go b/internal/repository/pg_warmup.go index e1b45bac..06fedea4 100644 --- a/internal/repository/pg_warmup.go +++ b/internal/repository/pg_warmup.go @@ -3,9 +3,12 @@ package repository import ( "context" "database/sql" + "errors" + "strings" "time" "github.com/google/uuid" + "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/warmbly/warmbly/internal/models" ) @@ -138,6 +141,13 @@ type WarmupRepository interface { GetWarmupToken(ctx context.Context, tokenID uuid.UUID) (*models.WarmupToken, error) FindWarmupToken(ctx context.Context, tokenID uuid.UUID) (*models.WarmupToken, error) ConsumeWarmupToken(ctx context.Context, tokenID uuid.UUID) error + // RecordWarmupTokenDelivery stamps the Message-ID the provider actually put + // on the wire onto the send's token, which is what the recipient can match + // when the verify header did not survive delivery. + RecordWarmupTokenDelivery(ctx context.Context, taskID uuid.UUID, messageID string) error + // FindDeliveredWarmupToken resolves the pending token for an inbound + // message that carries no verify header. + FindDeliveredWarmupToken(ctx context.Context, recipientAccountID uuid.UUID, senderAddress, messageID, subject string) (*models.WarmupToken, error) RecordInvalidTokenAttempt(ctx context.Context, accountID uuid.UUID, attemptedToken string) error CountRecentInvalidAttempts(ctx context.Context, accountID uuid.UUID, since time.Time) (int, error) @@ -788,8 +798,8 @@ func (r *warmupRepository) GetOrCreateDailyStats(ctx context.Context, accountID // CreateWarmupToken creates a warmup verification token func (r *warmupRepository) CreateWarmupToken(ctx context.Context, token *models.WarmupToken) error { query := ` - INSERT INTO warmup_tokens (token, task_id, sender_account_id, recipient_account_id, conversation_theme, content_source, conversation_id, conversation_turn, expires_at) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) + INSERT INTO warmup_tokens (token, task_id, sender_account_id, recipient_account_id, conversation_theme, content_source, conversation_id, conversation_turn, subject, expires_at) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) ` _, err := r.db.Exec(ctx, query, token.Token, @@ -800,6 +810,7 @@ func (r *warmupRepository) CreateWarmupToken(ctx context.Context, token *models. token.ContentSource, token.ConversationID, token.ConversationTurn, + token.Subject, token.ExpiresAt, ) return err @@ -807,61 +818,17 @@ func (r *warmupRepository) CreateWarmupToken(ctx context.Context, token *models. // GetWarmupToken retrieves a valid (unconsumed, unexpired) warmup token func (r *warmupRepository) GetWarmupToken(ctx context.Context, tokenID uuid.UUID) (*models.WarmupToken, error) { - query := ` - SELECT token, task_id, sender_account_id, recipient_account_id, COALESCE(conversation_theme, ''), COALESCE(content_source, ''), conversation_id, conversation_turn, created_at, consumed_at, expires_at + query := `SELECT ` + warmupTokenColumns + ` FROM warmup_tokens - WHERE token = $1 AND consumed_at IS NULL AND expires_at > NOW() - ` - - t := &models.WarmupToken{} - err := r.db.QueryRow(ctx, query, tokenID).Scan( - &t.Token, - &t.TaskID, - &t.SenderAccountID, - &t.RecipientAccountID, - &t.ConversationTheme, - &t.ContentSource, - &t.ConversationID, - &t.ConversationTurn, - &t.CreatedAt, - &t.ConsumedAt, - &t.ExpiresAt, - ) - - if err == sql.ErrNoRows { - return nil, nil - } - - return t, err + WHERE token = $1 AND consumed_at IS NULL AND expires_at > NOW()` + return scanWarmupToken(r.db.QueryRow(ctx, query, tokenID)) } func (r *warmupRepository) FindWarmupToken(ctx context.Context, tokenID uuid.UUID) (*models.WarmupToken, error) { - query := ` - SELECT token, task_id, sender_account_id, recipient_account_id, COALESCE(conversation_theme, ''), COALESCE(content_source, ''), conversation_id, conversation_turn, created_at, consumed_at, expires_at + query := `SELECT ` + warmupTokenColumns + ` FROM warmup_tokens - WHERE token = $1 - ` - - t := &models.WarmupToken{} - err := r.db.QueryRow(ctx, query, tokenID).Scan( - &t.Token, - &t.TaskID, - &t.SenderAccountID, - &t.RecipientAccountID, - &t.ConversationTheme, - &t.ContentSource, - &t.ConversationID, - &t.ConversationTurn, - &t.CreatedAt, - &t.ConsumedAt, - &t.ExpiresAt, - ) - - if err == sql.ErrNoRows { - return nil, nil - } - - return t, err + WHERE token = $1` + return scanWarmupToken(r.db.QueryRow(ctx, query, tokenID)) } // ConsumeWarmupToken marks a warmup token as consumed @@ -871,6 +838,97 @@ func (r *warmupRepository) ConsumeWarmupToken(ctx context.Context, tokenID uuid. return err } +// warmupTokenColumns is the shared select list for a warmup token row. +const warmupTokenColumns = `token, task_id, sender_account_id, recipient_account_id, + COALESCE(conversation_theme, ''), COALESCE(content_source, ''), conversation_id, + conversation_turn, COALESCE(subject, ''), COALESCE(sent_message_id, ''), + created_at, consumed_at, expires_at` + +func scanWarmupToken(row pgx.Row) (*models.WarmupToken, error) { + t := &models.WarmupToken{} + err := row.Scan( + &t.Token, + &t.TaskID, + &t.SenderAccountID, + &t.RecipientAccountID, + &t.ConversationTheme, + &t.ContentSource, + &t.ConversationID, + &t.ConversationTurn, + &t.Subject, + &t.SentMessageID, + &t.CreatedAt, + &t.ConsumedAt, + &t.ExpiresAt, + ) + if errors.Is(err, pgx.ErrNoRows) { + return nil, nil + } + if err != nil { + return nil, err + } + return t, nil +} + +// RecordWarmupTokenDelivery stamps the delivered Message-ID onto the send's +// token. Graph re-stamps the Message-ID we mint, so on Outlook this is the +// only value a recipient can match the send against. +func (r *warmupRepository) RecordWarmupTokenDelivery(ctx context.Context, taskID uuid.UUID, messageID string) error { + if messageID == "" { + return nil + } + _, err := r.db.Exec(ctx, + `UPDATE warmup_tokens SET sent_message_id = $2 WHERE task_id = $1`, + taskID, messageID) + return err +} + +// FindDeliveredWarmupToken resolves the token for warmup mail that arrived +// without its verify header, which is every send from a Microsoft mailbox +// (Graph strips custom headers in transit). +// +// Two keys: the Message-ID the provider stamped on the send, and the pending +// token for this exact sender/recipient pair whose subject matches. Both are +// scoped to unconsumed, unexpired tokens addressed to this recipient, of which +// there are only ever a handful, so this is one indexed lookup on a path every +// inbound message takes. +// +// The pair key is deliberately narrow. Warmup partners are other Warmbly +// mailboxes, so without the subject and the two-day window a real email +// between two pool members could claim a pending token and vanish from the +// recipient's unibox. +func (r *warmupRepository) FindDeliveredWarmupToken(ctx context.Context, recipientAccountID uuid.UUID, senderAddress, messageID, subject string) (*models.WarmupToken, error) { + messageID = strings.Trim(strings.TrimSpace(messageID), "<>") + senderAddress = strings.TrimSpace(senderAddress) + subject = strings.TrimSpace(subject) + if messageID == "" && (senderAddress == "" || subject == "") { + return nil, nil + } + + const matchesMessageID = `($2 <> '' AND wt.sent_message_id <> '' AND btrim(wt.sent_message_id, '<>') = $2)` + query := `SELECT ` + warmupTokenColumns + ` + FROM warmup_tokens wt + WHERE wt.recipient_account_id = $1 + AND wt.consumed_at IS NULL + AND wt.expires_at > NOW() + AND ( + ` + matchesMessageID + ` + OR ( + $3 <> '' AND $4 <> '' + AND wt.created_at > NOW() - INTERVAL '2 days' + AND wt.subject <> '' + AND lower(btrim(wt.subject)) = lower($4) + AND EXISTS ( + SELECT 1 FROM email_accounts ea + WHERE ea.id = wt.sender_account_id AND lower(ea.email) = lower($3) + ) + ) + ) + ORDER BY ` + matchesMessageID + ` DESC, wt.created_at DESC + LIMIT 1` + return scanWarmupToken(r.db.QueryRow(ctx, query, recipientAccountID, messageID, senderAddress, subject)) +} + // RecordInvalidTokenAttempt records an invalid warmup token attempt func (r *warmupRepository) RecordInvalidTokenAttempt(ctx context.Context, accountID uuid.UUID, attemptedToken string) error { query := ` diff --git a/internal/tasks/email_task.go b/internal/tasks/email_task.go index 779ef6ae..8c0cefd9 100644 --- a/internal/tasks/email_task.go +++ b/internal/tasks/email_task.go @@ -333,7 +333,10 @@ func (s *tasksService) HandleEmailTask(task *proto.ProcessTask) *errx.Error { ContentSource: contentSource, ConversationID: conversationID, ConversationTurn: conversationTurn, - ExpiresAt: time.Now().Add(7 * 24 * time.Hour), + // The recipient falls back to matching on the subject when the verify + // header does not survive delivery (Graph strips it). + Subject: subject, + ExpiresAt: time.Now().Add(7 * 24 * time.Hour), } if err := s.warmupRepo.CreateWarmupToken(ctx, tokenRecord); err != nil { log.Warn().Err(err).Str("task_id", taskID.String()).Str("email_account_id", account.ID.String()).Msg("Failed to create warmup token")