Technical review (Opus) and security review (Fable) of 5be109b..5154bba;
reports in the session's scratchpad. Nothing above low from security; one
bug and three risks from the technical review. Migration 0009 is not
deployed, so it was extended.
- Bug: two refusals of one second swapped their NOQUEUE lines (each line
went to the newest lineless row). AttachRefusalLine now matches the
client address too (send_log.client_ip, written by the journal milter -
Postfix's line names the client and the sender, never the login: the
sasl_username part of the pattern never matched and could be fed by HELO,
security F1/F2), takes the nearest row in time and the oldest of equals,
and a line already on a row goes on no other (security F6).
- Waiting lines and recoveries lived only in memory while the saved offset
moved past them: a stop within 30 s lost them. The tracker now never saves
an offset past the oldest waiting item in its file (tracker.hold,
refusalLines.oldest) - for a recovery, the client= line that names its
login - so they are read again; the "once" guards make that harmless.
- A recovered row whose login the tailer did not see was nobody's: its
domain_id falls back to the envelope domain's.
- The fingerprint's file identity is the inode alone: the device number can
change across a host reboot and make the same log look rotated.
- A refused row carries its queue id; UpdateStatus's "newest row" now skips
refused rows, so a later message with that id does not flip it to sent.
- The alive mark: its age is read first and kept from then on, so asking
supervisord for 30 s cannot make it stale; the entrypoint removes it with
the sockets - a container restart is not the check's downtime, and a mark
from before would have counted the whole outage as "≈".
- Security F3: the sender check prints no ";" of the client's in the From:
text of its reply, and the tailer reads the envelope after the first ";"
- Postfix's own text.
- A refused row with a queue id and no stored lines says so (it was told the
mail.log text of 2.2.0); the fact's em dash is the house hyphen.
- Docs: architecture (client address, no login), guide § Health (warns on
either count), help topics log and sender-check (design 1ca87f2).
- e2e: refusals_show_postfixs_line wants the refusal's own sender in the
line; refusal_recovered_from_the_log waits past the tailer's 30 s and
checks that a refusal the panel wrote got no recovered twin.
Not changed, written down: review 9 (mail.log.1 read to its end at that
moment; postlogd may still write to it until the reload), 10 (a log under
512 bytes has no fingerprint, so places fall back to time briefly after a
rotation), 6 (a 451 at MAIL FROM of another milter reads the same - the
client and sender must match too now), security F5/F12 (info). Review 12:
the refused and closed-by-the-panel variants of out-message were shown to
the owner as pictures of the real templates, not drawn in out-message.html:
the guard ties every drawn screen to a page of the engine, and a variant
has the page's outline.
Evidence on the test server for this tree: go vet clean, go test ./... 23
packages ok, make e2e exit 0 (refusal_recovered_from_the_log 70 s).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
65 lines
2.5 KiB
Go
65 lines
2.5 KiB
Go
package main
|
|
|
|
import (
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
// The check's downtime is the time since its last mark, under a Postfix that
|
|
// was running: a new container has no mark, and a Postfix that is not up, or
|
|
// came up later than the mark, was not kept waiting by the check for all of
|
|
// that time.
|
|
func TestSenderCheckDowntime(t *testing.T) {
|
|
now := time.Date(2026, 10, 12, 18, 0, 0, 0, time.UTC)
|
|
var mark time.Time // the alive mark's time; zero is no mark
|
|
set := func(at time.Time) { mark = at }
|
|
postfix := func(started time.Time, running bool) func() postfixState {
|
|
return func() postfixState { return postfixState{started: started, running: running, known: true} }
|
|
}
|
|
silent := func() postfixState { return postfixState{} }
|
|
down := func(ask func() postfixState) (time.Duration, bool) { return senderCheckDowntime(mark, now, ask) }
|
|
|
|
if d, _ := down(postfix(now.Add(-time.Hour), true)); d != 0 {
|
|
t.Errorf("no mark: %s", d)
|
|
}
|
|
set(now.Add(-40 * time.Second))
|
|
if d, est := down(postfix(now.Add(-time.Hour), true)); d != 40*time.Second || est {
|
|
t.Errorf("Postfix up throughout: %s %v, want 40s, not estimated", d, est)
|
|
}
|
|
if d, _ := down(postfix(now.Add(-10*time.Second), true)); d != 10*time.Second {
|
|
t.Errorf("Postfix started within the gap: %s, want 10s", d)
|
|
}
|
|
if d, _ := down(postfix(time.Time{}, false)); d != 0 {
|
|
t.Errorf("Postfix not running: %s", d)
|
|
}
|
|
// supervisord did not answer: the time since the mark, an estimate.
|
|
if d, est := down(silent); d != 40*time.Second || !est {
|
|
t.Errorf("supervisord silent: %s %v, want 40s, estimated", d, est)
|
|
}
|
|
set(now.Add(time.Minute)) // a clock set back
|
|
if d, _ := down(postfix(now.Add(-time.Hour), true)); d != 0 {
|
|
t.Errorf("a mark in the future: %s", d)
|
|
}
|
|
}
|
|
|
|
// Downtime is booked on the days it fell on.
|
|
func TestByDay(t *testing.T) {
|
|
at := func(d, h, m int) time.Time { return time.Date(2026, 10, d, h, m, 0, 0, time.UTC) }
|
|
got := byDay(at(12, 23, 50), at(14, 0, 10))
|
|
want := []dayPart{{at(12, 23, 50), 10 * time.Minute}, {at(13, 0, 0), 24 * time.Hour}, {at(14, 0, 0), 10 * time.Minute}}
|
|
if len(got) != len(want) {
|
|
t.Fatalf("parts = %+v", got)
|
|
}
|
|
for i := range want {
|
|
if !got[i].day.Equal(want[i].day) || got[i].down != want[i].down {
|
|
t.Errorf("part %d = %+v, want %+v", i, got[i], want[i])
|
|
}
|
|
}
|
|
if one := byDay(at(12, 10, 0), at(12, 10, 0).Add(40*time.Second)); len(one) != 1 || one[0].down != 40*time.Second {
|
|
t.Errorf("within a day: %+v", one)
|
|
}
|
|
if none := byDay(at(12, 10, 0), at(12, 10, 0)); len(none) != 0 {
|
|
t.Errorf("no time: %+v", none)
|
|
}
|
|
}
|