From c06691a056639d44fd55d46e7eb9770e20d88c45 Mon Sep 17 00:00:00 2001 From: Lucas Rayan Date: Mon, 24 Aug 2026 12:28:37 -0300 Subject: [PATCH] fix(mail): relatorios DMARC nao mostravam o clipe de anexo has_attachments so era definido para mensagens multipart. Um relatorio agregado DMARC do Google e de parte unica: o corpo E o anexo, um application/zip sem qualquer parte de texto. Ficava guardado como nao tendo anexos, ao lado dos relatorios TLS-RPT do mesmo remetente e na mesma pasta, que trazem uma linha de texto e por isso sao multipart/mixed e apareciam marcados. A regra passa a olhar tambem para a parte de topo: uma disposicao explicita decide (um .txt enviado como a mensagem inteira e anexo, uma parte inline nao e), e sem ela conta como anexo tudo o que nao seja texto nem multipart -- um zip, um PDF, uma mensagem reencaminhada. As mensagens ja guardadas mantinham a marca antiga, e sao justamente aquelas que o utilizador esta a olhar. Ha um backfill que as re-le uma vez, em segundo plano, sem atrasar o arranque. So examina as que estao marcadas como sem anexos: a regra antiga nunca inventou um anexo que nao existisse, portanto uma marca verdadeira nao precisa de segunda opiniao e a varredura fica do tamanho do defeito. A tabela backfills guarda o marcador para nao repetir a cada arranque. Co-Authored-By: Claude Opus 5 --- migrations/0011_backfill_markers.down.sql | 1 + migrations/0011_backfill_markers.up.sql | 12 +++ .../backfill_attachments.go | 77 ++++++++++++++++ .../cmd/lambdamail-protocols/main.go | 5 ++ .../application/usecase/message_headers.go | 47 ++++++++-- .../usecase/message_headers_test.go | 56 ++++++++++++ .../postgres/attachment_backfill.go | 87 +++++++++++++++++++ 7 files changed, 279 insertions(+), 6 deletions(-) create mode 100644 migrations/0011_backfill_markers.down.sql create mode 100644 migrations/0011_backfill_markers.up.sql create mode 100644 services/protocols/cmd/lambdamail-protocols/backfill_attachments.go create mode 100644 services/protocols/internal/infrastructure/postgres/attachment_backfill.go diff --git a/migrations/0011_backfill_markers.down.sql b/migrations/0011_backfill_markers.down.sql new file mode 100644 index 0000000..c28a9a3 --- /dev/null +++ b/migrations/0011_backfill_markers.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS backfills; diff --git a/migrations/0011_backfill_markers.up.sql b/migrations/0011_backfill_markers.up.sql new file mode 100644 index 0000000..00787f7 --- /dev/null +++ b/migrations/0011_backfill_markers.up.sql @@ -0,0 +1,12 @@ +-- Records one-shot data repairs so they run once rather than on every start. +-- +-- The first user is the attachment flag: has_attachments was only ever set for +-- multipart messages, so a single-part attachment - a DMARC aggregate report, +-- whose whole body is a zip - was stored as having none. The fix corrects new +-- deliveries; the messages already filed need re-reading, and re-reading every +-- message on every boot is not something to do forever. +CREATE TABLE IF NOT EXISTS backfills ( + name VARCHAR(64) PRIMARY KEY, + completed_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + detail TEXT +); diff --git a/services/protocols/cmd/lambdamail-protocols/backfill_attachments.go b/services/protocols/cmd/lambdamail-protocols/backfill_attachments.go new file mode 100644 index 0000000..f118c82 --- /dev/null +++ b/services/protocols/cmd/lambdamail-protocols/backfill_attachments.go @@ -0,0 +1,77 @@ +package main + +import ( + "context" + "log" + + "github.com/jackc/pgx/v5/pgxpool" + + "lambdamail/protocols/internal/application/usecase" + "lambdamail/protocols/internal/infrastructure/diskstorage" + "lambdamail/protocols/internal/infrastructure/postgres" +) + +// attachmentBackfillName identifies this repair in the backfills table. +const attachmentBackfillName = "has_attachments_single_part" + +// backfillAttachmentFlags re-reads stored messages and marks the ones whose +// body is itself an attachment. +// +// has_attachments was only set for multipart messages, so a DMARC aggregate +// report - a single-part application/zip with no text at all - was filed as +// having none. Correcting the rule fixes new deliveries; the messages already +// in the mailbox need re-reading, which is what this does, once. +// +// It runs in the background and never blocks startup: this is cosmetic +// metadata, and no message is at risk if it never finishes. +func backfillAttachmentFlags(ctx context.Context, pool *pgxpool.Pool) { + repo := postgres.NewAttachmentBackfill(pool) + + done, err := repo.Done(ctx, attachmentBackfillName) + if err != nil { + log.Printf("attachment backfill: cannot read the marker, skipping: %v", err) + return + } + if done { + return + } + + blobs := diskstorage.NewLocalDiskBlobReader(pool) + + // Bounded: a mailbox with a million read messages should not turn a deploy + // into an hour of disk reads. What is left is picked up on the next start, + // because the marker is only written once a pass changes nothing. + const batch = 5000 + candidates, err := repo.Candidates(ctx, batch) + if err != nil { + log.Printf("attachment backfill: cannot list candidates: %v", err) + return + } + + corrected := 0 + for _, c := range candidates { + payload, err := blobs.ReadByID(ctx, c.BlobID) + if err != nil { + // A blob that cannot be read is not a reason to stop; the flag + // stays as it was and the message is untouched. + continue + } + if !usecase.ExtractMessageHeaders(payload).HasAttachments { + continue + } + if err := repo.SetHasAttachments(ctx, c.ID); err != nil { + log.Printf("attachment backfill: %v", err) + continue + } + corrected++ + } + + if len(candidates) < batch { + // The whole set fit in one pass, so there is nothing left to do. + if err := repo.MarkDone(ctx, attachmentBackfillName, + "corrected single-part attachments"); err != nil { + log.Printf("attachment backfill: %v", err) + } + } + log.Printf("attachment backfill: examined %d message(s), corrected %d", len(candidates), corrected) +} diff --git a/services/protocols/cmd/lambdamail-protocols/main.go b/services/protocols/cmd/lambdamail-protocols/main.go index b90bb80..764a81a 100644 --- a/services/protocols/cmd/lambdamail-protocols/main.go +++ b/services/protocols/cmd/lambdamail-protocols/main.go @@ -248,6 +248,11 @@ func run(cfg config) { } router.SetAdminTlsAPI(tlsStatus, cfg.PrimaryMailHost, cfg.TLSMode, cfg.JwtSecret, cfg.CertPollInterval) + // One-shot repair of has_attachments on messages stored before single-part + // attachments were recognised. In the background: it is display metadata, + // and nothing waits on it. + go backfillAttachmentFlags(ctx, pool) + // Real-time updates: the outbox relay reads the events delivery wrote in // its own transaction and hands them to the hub, which pushes them to the // browser (PLAN.md sections 9.4 and 14.2). diff --git a/services/protocols/internal/application/usecase/message_headers.go b/services/protocols/internal/application/usecase/message_headers.go index dc1cdd8..788e685 100644 --- a/services/protocols/internal/application/usecase/message_headers.go +++ b/services/protocols/internal/application/usecase/message_headers.go @@ -69,18 +69,53 @@ func ExtractMessageHeaders(payload []byte) MessageHeaders { } contentType := msg.Header.Get("Content-Type") - mediaType, params, err := mime.ParseMediaType(contentType) - if err == nil && strings.HasPrefix(mediaType, "multipart/") { - // mixed and related carry parts that are not the body; alternative is - // just the same text twice, so it does not count as an attachment. - out.HasAttachments = mediaType != "multipart/alternative" - _ = params + mediaType, _, err := mime.ParseMediaType(contentType) + if err != nil { + mediaType = "" } + out.HasAttachments = carriesAttachment(mediaType, msg.Header.Get("Content-Disposition")) out.Snippet = buildSnippet(msg, mediaType) return out } +// carriesAttachment decides whether a message has something worth a paperclip, +// from its top-level headers alone. +// +// Looking only for multipart/* missed the case where the body IS the +// attachment: a DMARC aggregate report from Google arrives as a single-part +// application/zip with no text part at all, so it showed up unmarked next to +// TLS-RPT reports - which do carry a text part, and so are multipart/mixed - +// from the same sender in the same folder. +func carriesAttachment(mediaType, disposition string) bool { + // An explicit disposition is the author saying so, and it settles the + // question even for a text type: a .txt sent as the whole message is an + // attachment, an inline part is not. + if disp, _, err := mime.ParseMediaType(disposition); err == nil { + switch strings.ToLower(disp) { + case "attachment": + return true + case "inline": + return false + } + } + + switch { + case strings.HasPrefix(mediaType, "multipart/"): + // mixed and related carry parts that are not the body; alternative is + // just the same text twice. + return mediaType != "multipart/alternative" + case mediaType == "": + // No Content-Type means text/plain (RFC 2045 section 5.2). An + // unparseable one is treated the same way rather than guessed at. + return false + default: + // Anything that is not text and not multipart is a single part that is + // not the body: a zip, a PDF, a forwarded message. + return !strings.HasPrefix(mediaType, "text/") + } +} + // buildSnippet takes the opening of the body as a plain-text preview. func buildSnippet(msg *mail.Message, mediaType string) string { // Only a flat text body is previewed. Walking MIME parts here would mean diff --git a/services/protocols/internal/application/usecase/message_headers_test.go b/services/protocols/internal/application/usecase/message_headers_test.go index 4ba12de..e7ff4c8 100644 --- a/services/protocols/internal/application/usecase/message_headers_test.go +++ b/services/protocols/internal/application/usecase/message_headers_test.go @@ -86,3 +86,59 @@ func TestExtractMessageHeaders_TruncatesWithoutBreakingUTF8(t *testing.T) { } } } + +// A DMARC aggregate report from Google is a single-part message whose body IS +// the attachment: Content-Type application/zip, no text part at all. The flag +// was only ever set for multipart/*, so these arrived without the paperclip +// while TLS-RPT reports - which do carry a text part, so multipart/mixed - got +// one. Same folder, same sender, one marked and one not. +func TestExtractMessageHeaders_SinglePartAttachmentCounts(t *testing.T) { + msg := "From: \r\n" + + "To: \r\n" + + "Subject: Report domain: example.test\r\n" + + "Content-Type: application/zip; name=\"google.com!example.test!1.zip\"\r\n" + + "Content-Disposition: attachment; filename=\"google.com!example.test!1.zip\"\r\n" + + "Content-Transfer-Encoding: base64\r\n" + + "\r\n" + + "UEsDBBQAAAAIAA==\r\n" + + h := ExtractMessageHeaders([]byte(msg)) + + if !h.HasAttachments { + t.Error("a message whose whole body is a zip was reported as having no attachments") + } +} + +func TestExtractMessageHeaders_AttachmentDetection(t *testing.T) { + cases := []struct { + name string + headers string + want bool + }{ + {"plain text", "Content-Type: text/plain; charset=utf-8\r\n", false}, + {"html only", "Content-Type: text/html; charset=utf-8\r\n", false}, + // The same text twice is not an attachment. + {"alternative", "Content-Type: multipart/alternative; boundary=b\r\n", false}, + {"mixed", "Content-Type: multipart/mixed; boundary=b\r\n", true}, + {"related", "Content-Type: multipart/related; boundary=b\r\n", true}, + {"bare zip", "Content-Type: application/zip\r\n", true}, + {"bare pdf", "Content-Type: application/pdf\r\n", true}, + // A forwarded message carried as the whole body. + {"attached message", "Content-Type: message/rfc822\r\n", true}, + // An explicit disposition wins even over a text type: someone sent a + // .txt file as the entire message. + {"text as attachment", "Content-Type: text/plain\r\nContent-Disposition: attachment; filename=\"notes.txt\"\r\n", true}, + {"inline text", "Content-Type: text/plain\r\nContent-Disposition: inline\r\n", false}, + // No Content-Type at all means text/plain (RFC 2045 section 5.2). + {"absent content type", "", false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + msg := "From: \r\nSubject: x\r\n" + tc.headers + "\r\nbody\r\n" + if got := ExtractMessageHeaders([]byte(msg)).HasAttachments; got != tc.want { + t.Errorf("HasAttachments = %v, want %v", got, tc.want) + } + }) + } +} diff --git a/services/protocols/internal/infrastructure/postgres/attachment_backfill.go b/services/protocols/internal/infrastructure/postgres/attachment_backfill.go new file mode 100644 index 0000000..d196a70 --- /dev/null +++ b/services/protocols/internal/infrastructure/postgres/attachment_backfill.go @@ -0,0 +1,87 @@ +package postgres + +import ( + "context" + "fmt" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" +) + +// AttachmentBackfill repairs has_attachments on messages already stored. +// +// The flag was only ever set for multipart messages, so a single-part +// attachment - a DMARC aggregate report, whose entire body is a zip - was +// filed as having none, and showed up without a paperclip beside TLS-RPT +// reports from the same sender that happened to carry a text part. +type AttachmentBackfill struct { + pool *pgxpool.Pool +} + +func NewAttachmentBackfill(pool *pgxpool.Pool) *AttachmentBackfill { + return &AttachmentBackfill{pool: pool} +} + +// Done reports whether this repair has already run. +func (b *AttachmentBackfill) Done(ctx context.Context, name string) (bool, error) { + var exists bool + err := b.pool.QueryRow(ctx, + `SELECT EXISTS (SELECT 1 FROM backfills WHERE name = $1)`, name).Scan(&exists) + if err != nil { + return false, fmt.Errorf("read backfill marker %s: %w", name, err) + } + return exists, nil +} + +func (b *AttachmentBackfill) MarkDone(ctx context.Context, name, detail string) error { + _, err := b.pool.Exec(ctx, + `INSERT INTO backfills (name, detail) VALUES ($1, $2) ON CONFLICT (name) DO NOTHING`, + name, detail) + if err != nil { + return fmt.Errorf("record backfill marker %s: %w", name, err) + } + return nil +} + +// Candidate is one message worth re-reading. +type Candidate struct { + ID uuid.UUID + BlobID uuid.UUID +} + +// Candidates lists messages currently flagged as having no attachment. +// +// Only those: the old rule never claimed an attachment that was not there, so +// a true flag needs no second opinion and the scan stays as small as the bug. +func (b *AttachmentBackfill) Candidates(ctx context.Context, limit int) ([]Candidate, error) { + rows, err := b.pool.Query(ctx, ` + SELECT id, blob_id FROM email_messages + WHERE has_attachments = false AND expunged_at IS NULL + ORDER BY received_at DESC + LIMIT $1 + `, limit) + if err != nil { + return nil, fmt.Errorf("list attachment backfill candidates: %w", err) + } + defer rows.Close() + + out := []Candidate{} + for rows.Next() { + var c Candidate + if err := rows.Scan(&c.ID, &c.BlobID); err != nil { + return nil, err + } + out = append(out, c) + } + return out, rows.Err() +} + +// SetHasAttachments records the recomputed flag. +func (b *AttachmentBackfill) SetHasAttachments(ctx context.Context, messageID uuid.UUID) error { + _, err := b.pool.Exec(ctx, + `UPDATE email_messages SET has_attachments = true WHERE id = $1`, messageID) + if err != nil { + return fmt.Errorf("set has_attachments for %s: %w", messageID, err) + } + return nil +}