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 <noreply@anthropic.com>
This commit is contained in:
1 parent
7f8d9b7ff1
commit
c06691a056
7 files changed
+279
-6
No files matched your search
@@ -0,0 +1 @@
|
|||||||
|
DROP TABLE IF EXISTS backfills;
|
||||||
@@ -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
|
||||||
|
);
|
||||||
@@ -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)
|
||||||
|
}
|
||||||
@@ -248,6 +248,11 @@ func run(cfg config) {
|
|||||||
}
|
}
|
||||||
router.SetAdminTlsAPI(tlsStatus, cfg.PrimaryMailHost, cfg.TLSMode, cfg.JwtSecret, cfg.CertPollInterval)
|
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
|
// 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
|
// its own transaction and hands them to the hub, which pushes them to the
|
||||||
// browser (PLAN.md sections 9.4 and 14.2).
|
// browser (PLAN.md sections 9.4 and 14.2).
|
||||||
|
|||||||
@@ -69,18 +69,53 @@ func ExtractMessageHeaders(payload []byte) MessageHeaders {
|
|||||||
}
|
}
|
||||||
|
|
||||||
contentType := msg.Header.Get("Content-Type")
|
contentType := msg.Header.Get("Content-Type")
|
||||||
mediaType, params, err := mime.ParseMediaType(contentType)
|
mediaType, _, err := mime.ParseMediaType(contentType)
|
||||||
if err == nil && strings.HasPrefix(mediaType, "multipart/") {
|
if err != nil {
|
||||||
// mixed and related carry parts that are not the body; alternative is
|
mediaType = ""
|
||||||
// just the same text twice, so it does not count as an attachment.
|
|
||||||
out.HasAttachments = mediaType != "multipart/alternative"
|
|
||||||
_ = params
|
|
||||||
}
|
}
|
||||||
|
out.HasAttachments = carriesAttachment(mediaType, msg.Header.Get("Content-Disposition"))
|
||||||
|
|
||||||
out.Snippet = buildSnippet(msg, mediaType)
|
out.Snippet = buildSnippet(msg, mediaType)
|
||||||
return out
|
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.
|
// buildSnippet takes the opening of the body as a plain-text preview.
|
||||||
func buildSnippet(msg *mail.Message, mediaType string) string {
|
func buildSnippet(msg *mail.Message, mediaType string) string {
|
||||||
// Only a flat text body is previewed. Walking MIME parts here would mean
|
// Only a flat text body is previewed. Walking MIME parts here would mean
|
||||||
|
|||||||
@@ -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: <noreply-dmarc-support@google.com>\r\n" +
|
||||||
|
"To: <dmarc@example.test>\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: <a@b.test>\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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -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
|
||||||
|
}
|
||||||
Reference in new issue
Block a user