diff --git a/CHANGELOG.md b/CHANGELOG.md index a7bb538..466e92f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ### Added +- A page per delivery (`/deliveries/{id}`), reached from the *Details* link on + every send-log row. It carries what the log itself no longer shows — the + sending domain, the application the message was submitted under, the Postfix + queue id to search the system log for, when the status was last reported — + so the journal can grow fields without the table having to find columns for + them. *Back* returns to the page and filters the row was opened from, rebuilt + from the log's own parameters only. + - A **DNS** badge in the domain list, one per row, carrying the same ok/warn/error/unknown vocabulary as the rest of the panel: the worst of that domain's DKIM, SPF and DMARC checks, so a domain whose records were never @@ -44,6 +52,17 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ### Changed +- The delivery log lists what identifies a message and nothing else: time, + sender, recipient, subject, status. Domain and application, which were a + column each, remain the log's two filters and now appear per message on the + delivery page. The two dropped columns were the widest thing in the table + after the addresses, and both repeat down the page whenever a filter is set. +- Subjects are decoded for display as well as on the way in, so the rows the + journal-milter recorded before it decoded them itself — the ones an operator + is most likely to still be reading — show their text rather than + `=?utf-8?Q?…?=`. The decoder moved to `internal/mailhdr` and is shared by the + milter and the panel; it is idempotent, so a row decoded once passes through + unchanged. - The panel's navigation is a column down the left edge instead of a bar across the top. As a bar it did not fit on one row — six page entries and the session block against the panel's width — and had to be split into two, diff --git a/README.md b/README.md index 02edb38..613870d 100644 --- a/README.md +++ b/README.md @@ -207,9 +207,13 @@ service healthy and will mail be accepted?" (level 2) are configured here. *Export domain* writes a single-domain archive; *Import a domain* on the Backup page reads one back in. - **Deliveries** (`/deliveries`) — searchable send log with server-side filters - by domain and application. Each row shows status `queued` (accepted, not yet + by domain and application. A row identifies its message and nothing more — + time, sender, recipient, subject and status `queued` (accepted, not yet delivered), `sent` (handed off successfully), or `rejected` (refused — for - example by a level-2 rate limit). Retention is controlled by + example by a level-2 rate limit); *Details* opens that row's own page + (`/deliveries/{id}`) with the sending domain, the application it was + submitted under, the Postfix queue id to search the system log for, and when + the status was last reported. Retention is controlled by `SEND_LOG_RETENTION_DAYS`. - **Mail queue** (`/mail-queue`) — live view of messages Postfix is still trying to deliver or deferring. diff --git a/docs/architecture.md b/docs/architecture.md index 4ba89ad..5e2bfce 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -136,6 +136,7 @@ unless noted. | `/status` | Process, cert, socket, PTR checks; machine CPU/memory/network | | `/domains`, `/domains/*` | Domain and application CRUD, DKIM, L2 limits | | `/deliveries` | Send log with filters | +| `/deliveries/{id}` | One send-log row in full | | `/mail-queue` | Postfix queue view | | `/system-log` | `mail.log` tail | | `/reload` | Reload OpenDKIM + Postfix maps | diff --git a/internal/mailhdr/subject.go b/internal/mailhdr/subject.go new file mode 100644 index 0000000..a722090 --- /dev/null +++ b/internal/mailhdr/subject.go @@ -0,0 +1,38 @@ +// Package mailhdr turns raw mail header values into display text. It sits +// between the journal-milter, which reads headers off the wire, and the panel, +// which shows them: both need the same answer for the same header, and the +// panel needs it for rows the milter wrote before this decoding existed. +package mailhdr + +import ( + "mime" + "strings" +) + +// SubjectMaxRunes caps what the journal keeps of a subject. A Subject header +// may legally run to hundreds of characters; the log only needs enough to +// recognise the message, and the panel shows one row per recipient. +const SubjectMaxRunes = 200 + +// DecodeSubject turns a raw Subject header into display text. Anything +// non-ASCII arrives as RFC 2047 encoded-words (=?utf-8?Q?=D0=9F…?=), which the +// panel would otherwise show verbatim: unreadable, and — being one unbreakable +// run — wide enough to push the send-log table out of its card. Go's decoder +// covers the UTF-8 and ASCII charsets senders use in practice; for anything +// else (windows-1251, koi8-r) it fails and the raw header is kept, which is no +// worse than not decoding at all. Truncation is applied after decoding so the +// cap counts characters of the subject, not bytes of its encoding. +// +// It is idempotent: already-decoded text contains no encoded-words, so a second +// pass returns it unchanged. That is what lets the panel decode on the way out +// as well as the milter on the way in. +func DecodeSubject(v string) string { + if dec, err := (&mime.WordDecoder{}).DecodeHeader(v); err == nil { + v = dec + } + v = strings.TrimSpace(v) + if r := []rune(v); len(r) > SubjectMaxRunes { + v = string(r[:SubjectMaxRunes]) + "…" + } + return v +} diff --git a/internal/mailhdr/subject_test.go b/internal/mailhdr/subject_test.go new file mode 100644 index 0000000..89e799a --- /dev/null +++ b/internal/mailhdr/subject_test.go @@ -0,0 +1,38 @@ +package mailhdr + +import ( + "strings" + "testing" +) + +func TestDecodeSubject(t *testing.T) { + long := strings.Repeat("я", SubjectMaxRunes+10) + + for _, tc := range []struct { + name, raw, want string + }{ + {"plain", "Hello there", "Hello there"}, + {"utf8 q", "=?utf-8?Q?=D0=9F=D1=80=D0=BE=D0=B2=D0=B5=D1=80=D0=BA=D0=B0?=", "Проверка"}, + {"utf8 b, folded across two words", "=?utf-8?B?0J/RgNC40LLQtdGC?=\r\n =?utf-8?B?INC80LjRgA==?=", "Привет мир"}, + // No decoder for the legacy single-byte charsets: keep the header as + // sent rather than losing the subject entirely. + {"unknown charset", "=?windows-1251?B?z/Do4uXy?=", "=?windows-1251?B?z/Do4uXy?="}, + {"too long", long, strings.Repeat("я", SubjectMaxRunes) + "…"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := DecodeSubject(tc.raw); got != tc.want { + t.Fatalf("DecodeSubject(%q) = %q, want %q", tc.raw, got, tc.want) + } + }) + } +} + +// The panel decodes on the way out as well as the milter on the way in, so +// every already-decoded row in the send log passes through a second time. +func TestDecodeSubjectIsIdempotent(t *testing.T) { + for _, s := range []string{"Проверка", "Hello there", "", "=?windows-1251?B?z/Do4uXy?="} { + if got := DecodeSubject(DecodeSubject(s)); got != DecodeSubject(s) { + t.Fatalf("second pass over %q changed it to %q", s, got) + } + } +} diff --git a/internal/milter/milter.go b/internal/milter/milter.go index 6c0807c..d15a062 100644 --- a/internal/milter/milter.go +++ b/internal/milter/milter.go @@ -14,7 +14,6 @@ package milter import ( "context" "log" - "mime" "net" "net/textproto" "strings" @@ -22,6 +21,7 @@ import ( "github.com/emersion/go-milter" + "github.com/mixeme/selfpost/internal/mailhdr" "github.com/mixeme/selfpost/internal/store" ) @@ -101,35 +101,11 @@ func (s *session) RcptTo(rcpt string, m *milter.Modifier) (milter.Response, erro // Header captures the Subject. Only the first Subject header is kept. func (s *session) Header(name, value string, m *milter.Modifier) (milter.Response, error) { if s.subject == "" && textproto.CanonicalMIMEHeaderKey(name) == "Subject" { - s.subject = decodeSubject(value) + s.subject = mailhdr.DecodeSubject(value) } return milter.RespContinue, nil } -// subjectMaxRunes caps what the journal keeps of a subject. A Subject header -// may legally run to hundreds of characters; the log only needs enough to -// recognise the message, and the panel shows one row per recipient. -const subjectMaxRunes = 200 - -// decodeSubject turns the raw Subject header into display text. Anything -// non-ASCII arrives as RFC 2047 encoded-words (=?utf-8?Q?=D0=9F…?=), which the -// panel would otherwise show verbatim: unreadable, and — being one unbreakable -// run — wide enough to push the send-log table out of its card. Go's decoder -// covers the UTF-8 and ASCII charsets senders use in practice; for anything -// else (windows-1251, koi8-r) it fails and the raw header is kept, which is no -// worse than before. Truncation is applied after decoding so the cap counts -// characters of the subject, not bytes of its encoding. -func decodeSubject(v string) string { - if dec, err := (&mime.WordDecoder{}).DecodeHeader(v); err == nil { - v = dec - } - v = strings.TrimSpace(v) - if r := []rune(v); len(r) > subjectMaxRunes { - v = string(r[:subjectMaxRunes]) + "…" - } - return v -} - // Body fires at end-of-message, when the queue-id macro {i} is set and the // message is about to be committed to the queue. This is where the "queued" // rows are written. We accept (this milter is done) without ever rejecting. diff --git a/internal/milter/milter_test.go b/internal/milter/milter_test.go index 76f3cc6..ac35fa4 100644 --- a/internal/milter/milter_test.go +++ b/internal/milter/milter_test.go @@ -3,7 +3,6 @@ package milter import ( "errors" "net" - "strings" "testing" "time" @@ -115,30 +114,15 @@ func TestSessionRecordsRowPerRecipient(t *testing.T) { } // A subject in any non-ASCII alphabet reaches the milter as RFC 2047 -// encoded-words; the journal stores the text, not the encoding. +// encoded-words; the journal stores the text, not the encoding. The decoding +// itself is mailhdr's; what this checks is that Header runs it. func TestHeaderDecodesEncodedSubject(t *testing.T) { - long := strings.Repeat("я", subjectMaxRunes+10) - - for _, tc := range []struct { - name, raw, want string - }{ - {"plain", "Hello there", "Hello there"}, - {"utf8 q", "=?utf-8?Q?=D0=9F=D1=80=D0=BE=D0=B2=D0=B5=D1=80=D0=BA=D0=B0?=", "Проверка"}, - {"utf8 b, folded across two words", "=?utf-8?B?0J/RgNC40LLQtdGC?=\r\n =?utf-8?B?INC80LjRgA==?=", "Привет мир"}, - // No decoder for the legacy single-byte charsets: keep the header as - // sent rather than losing the subject entirely. - {"unknown charset", "=?windows-1251?B?z/Do4uXy?=", "=?windows-1251?B?z/Do4uXy?="}, - {"too long", long, strings.Repeat("я", subjectMaxRunes) + "…"}, - } { - t.Run(tc.name, func(t *testing.T) { - s := &session{rec: &fakeRecorder{}} - if _, err := s.Header("Subject", tc.raw, mods(nil)); err != nil { - t.Fatalf("Header: %v", err) - } - if s.subject != tc.want { - t.Fatalf("subject = %q, want %q", s.subject, tc.want) - } - }) + s := &session{rec: &fakeRecorder{}} + if _, err := s.Header("Subject", "=?utf-8?Q?=D0=9F=D1=80=D0=BE=D0=B2=D0=B5=D1=80=D0=BA=D0=B0?=", mods(nil)); err != nil { + t.Fatalf("Header: %v", err) + } + if s.subject != "Проверка" { + t.Fatalf("subject = %q, want %q", s.subject, "Проверка") } } diff --git a/internal/store/sendlog.go b/internal/store/sendlog.go index 5abeda1..1d9952e 100644 --- a/internal/store/sendlog.go +++ b/internal/store/sendlog.go @@ -1,6 +1,8 @@ package store import ( + "database/sql" + "errors" "fmt" "strings" "time" @@ -104,6 +106,37 @@ type SendLogRow struct { Subject string Status string CreatedAt time.Time + // UpdatedAt is when the status last changed — the log-tailer's report of + // the delivery attempt. It equals CreatedAt while a row is still queued. + UpdatedAt time.Time +} + +// ErrSendLogNotFound is returned by GetSendLog for an id that no longer exists. +// Send-log rows are pruned on the retention window, so a bookmarked delivery +// disappearing is expected, not a fault. +var ErrSendLogNotFound = errors.New("send-log entry not found") + +// GetSendLog returns a single send-log row by id, for the delivery detail page +// the log links each row to. +func (s *Store) GetSendLog(id int64) (SendLogRow, error) { + var ( + row SendLogRow + createdAt, updatedAt string + ) + err := s.db.QueryRow( + `SELECT id, queue_id, domain, app_login, from_addr, to_addr, subject, status, created_at, updated_at + FROM send_log WHERE id = ?`, id, + ).Scan(&row.ID, &row.QueueID, &row.Domain, &row.AppLogin, &row.From, &row.To, + &row.Subject, &row.Status, &createdAt, &updatedAt) + if errors.Is(err, sql.ErrNoRows) { + return SendLogRow{}, ErrSendLogNotFound + } + if err != nil { + return SendLogRow{}, fmt.Errorf("get send_log %d: %w", id, err) + } + row.CreatedAt, _ = time.Parse(time.RFC3339, createdAt) + row.UpdatedAt, _ = time.Parse(time.RFC3339, updatedAt) + return row, nil } // SendLogFilter narrows QuerySendLog/CountSendLog by domain and/or @@ -119,7 +152,7 @@ func (s *Store) QuerySendLog(filter SendLogFilter, limit, offset int) ([]SendLog where, args := sendLogWhere(filter) args = append(args, limit, offset) rows, err := s.db.Query( - `SELECT id, queue_id, domain, app_login, from_addr, to_addr, subject, status, created_at + `SELECT id, queue_id, domain, app_login, from_addr, to_addr, subject, status, created_at, updated_at FROM send_log`+where+` ORDER BY id DESC LIMIT ? OFFSET ?`, @@ -133,14 +166,15 @@ func (s *Store) QuerySendLog(filter SendLogFilter, limit, offset int) ([]SendLog var out []SendLogRow for rows.Next() { var ( - row SendLogRow - createdAt string + row SendLogRow + createdAt, updatedAt string ) if err := rows.Scan(&row.ID, &row.QueueID, &row.Domain, &row.AppLogin, - &row.From, &row.To, &row.Subject, &row.Status, &createdAt); err != nil { + &row.From, &row.To, &row.Subject, &row.Status, &createdAt, &updatedAt); err != nil { return nil, fmt.Errorf("scan send_log row: %w", err) } row.CreatedAt, _ = time.Parse(time.RFC3339, createdAt) + row.UpdatedAt, _ = time.Parse(time.RFC3339, updatedAt) out = append(out, row) } return out, rows.Err() diff --git a/internal/web/handlers_monitor.go b/internal/web/handlers_monitor.go index 1d353a2..a141f46 100644 --- a/internal/web/handlers_monitor.go +++ b/internal/web/handlers_monitor.go @@ -4,9 +4,11 @@ import ( "errors" "io/fs" "net/http" + "net/url" "strconv" "github.com/mixeme/selfpost/internal/logtail" + "github.com/mixeme/selfpost/internal/mailhdr" "github.com/mixeme/selfpost/internal/postfix" "github.com/mixeme/selfpost/internal/store" ) @@ -50,6 +52,58 @@ func (s *Server) handleDeliveriesRows(w http.ResponseWriter, r *http.Request) { s.renderFragment(w, http.StatusOK, "deliveries_rows", data) } +// handleDelivery renders one send-log row in full. The log itself carries only +// what identifies a message at a glance — when, who to and from, what about, +// how it ended — and every remaining field (domain, application, queue id, when +// the status was last reported) lives here, one page per row, so widening the +// journal never costs the table a column. +func (s *Server) handleDelivery(w http.ResponseWriter, r *http.Request) { + id, err := strconv.ParseInt(r.PathValue("id"), 10, 64) + if err != nil || id <= 0 { + http.NotFound(w, r) + return + } + row, err := s.store.GetSendLog(id) + if err != nil { + // A row pruned on the retention window is gone, not broken. + if errors.Is(err, store.ErrSendLogNotFound) { + http.NotFound(w, r) + return + } + logf("panel: delivery %d: %v", id, err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + row.Subject = mailhdr.DecodeSubject(row.Subject) + s.render(w, http.StatusOK, "delivery", map[string]any{ + "Title": "SelfPost — delivery", + "User": currentUser(r), + "Active": "deliveries", + "Row": row, + // Where the row came from, so "Back" returns to the page and filters + // the operator was looking at rather than the top of an unfiltered log. + "BackURL": deliveriesBackURL(r), + }) +} + +// deliveriesBackURL rebuilds the delivery-log URL a detail page was opened +// from. Only the log's own parameters are carried over, and each is re-encoded +// by url.Values, so nothing a visitor appends to the link can travel back into +// the page as markup or as a different destination. +func deliveriesBackURL(r *http.Request) string { + q := r.URL.Query() + back := url.Values{} + for _, k := range []string{"domain", "app", "p"} { + if v := q.Get(k); v != "" { + back.Set(k, v) + } + } + if len(back) == 0 { + return "/deliveries" + } + return "/deliveries?" + back.Encode() +} + // sendLogData reads the domain/app filters and page number off the query // string, queries the store, and assembles everything the template needs // (filter dropdown options plus the current selection, rows, and pagination). @@ -69,6 +123,12 @@ func (s *Server) sendLogData(r *http.Request) (map[string]any, error) { if err != nil { return nil, err } + // Decode on the way out as well as on the way in: rows the journal-milter + // wrote before it decoded subjects itself still hold the raw header, and + // they are the ones an operator is most likely to be looking at. + for i := range rows { + rows[i].Subject = mailhdr.DecodeSubject(rows[i].Subject) + } domains, err := s.store.ListDomains() if err != nil { return nil, err diff --git a/internal/web/handlers_monitor_test.go b/internal/web/handlers_monitor_test.go index 4bee126..47afc7a 100644 --- a/internal/web/handlers_monitor_test.go +++ b/internal/web/handlers_monitor_test.go @@ -1,8 +1,14 @@ package web import ( + "net/http" + "net/http/httptest" "path/filepath" + "strconv" + "strings" "testing" + + "github.com/mixeme/selfpost/internal/store" ) // After log rotation renames mail.log away, Postfix takes about a second to @@ -19,3 +25,137 @@ func TestReadLogTailMissingFileIsNotAnError(t *testing.T) { t.Errorf("errText = %q, want empty (missing file is not an error)", errText) } } + +// The delivery log is a list of messages, not a dump of the journal: it shows +// when, from, to, subject and status, and links each row to the page carrying +// the rest. A column added back here is one the table has no width for. +func TestDeliveryLogShowsOnlyTheIdentifyingColumns(t *testing.T) { + s, row := serverWithDelivery(t) + + out := getBody(t, s.handleDeliveries, "/deliveries") + for _, want := range []string{ + row.CreatedAt.Format("2006-01-02 15:04:05"), + "noreply@bs.example.ru", "public@example.ru", + "Проверка", ">sent<", `href="/deliveries/` + itoa(row.ID), + } { + if !strings.Contains(out, want) { + t.Errorf("delivery log is missing %q:\n%s", want, out) + } + } + // Domain and application stay available as filters; what the table must not + // carry is a column of them per row. + for _, unwanted := range []string{"
| Time | Domain | App | From | To | Subject | Status | |
|---|---|---|---|---|---|---|---|
| Time | From | To | Subject | Status | |||
| {{.CreatedAt.Format "2006-01-02 15:04:05"}} | -{{.Domain}} | -{{.AppLogin}} | {{.From}} | {{.To}} | {{.Subject}} | {{.Status}} | +Details |