Files
selfpost/internal/logtail/refusals_test.go
T
mixandClaude Opus 5.5 af130e268a findings of the reviews of audit stage 2b
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>
2026-10-14 18:38:22 +03:00

125 lines
5.6 KiB
Go

package logtail
import (
"testing"
"time"
"github.com/mixeme/selfpost/internal/store"
)
type refusalProbe struct {
rows int // rows waiting for a line
lines []string
who []string
recovered []store.SendLogEntry
}
func (r *refusalProbe) RecoverRefusal(e store.SendLogEntry, _ time.Time) (bool, error) {
r.recovered = append(r.recovered, e)
return true, nil
}
func (r *refusalProbe) AttachRefusalLine(clientIP, from string, _ time.Time, line string) (bool, error) {
if r.rows == 0 {
return false, nil
}
r.rows--
r.lines = append(r.lines, line)
r.who = append(r.who, clientIP+"|"+from)
return true, nil
}
// The NOQUEUE line of a refusal at MAIL FROM goes on its row; a line whose
// row is not written yet waits and is tried again; port 25's and other
// lines are not refusals of an application.
func TestRefusalLines(t *testing.T) {
now := time.Date(2026, 10, 14, 12, 0, 0, 0, time.UTC)
st := &refusalProbe{}
r := &refusalLines{st: st}
limit := `2026-10-14T12:00:00+00:00 mail postfix/smtpd[1149]: NOQUEUE: milter-reject: MAIL from unknown[10.77.0.1]: 451 4.7.1 Service unavailable - try again later; from=<billing@sender.e2e.test> proto=ESMTP helo=<e2e-client.e2e.test> sasl_username=<l2app>`
for _, other := range []string{
`2026-10-14T12:00:00+00:00 mail postfix/inbound/smtpd[372]: NOQUEUE: milter-reject: MAIL from unknown[203.0.113.7]: 451 4.7.1 Service unavailable - try again later; from=<a@stranger.example> proto=ESMTP helo=<x>`,
`2026-10-14T12:00:00+00:00 mail postfix/smtpd[11]: NOQUEUE: milter-reject: CONNECT from unknown[10.77.0.1]: 451 4.7.1 Service unavailable - try again later; proto=SMTP`,
`2026-10-14T12:00:00+00:00 mail postfix/smtpd[11]: NOQUEUE: reject: RCPT from unknown[10.77.0.1]: 553 5.7.1 <a@x>: Sender address rejected: not owned by user app1; from=<a@x> to=<b@y> proto=ESMTP helo=<x>`,
} {
r.handle(other, now, Place{})
}
if len(r.pending) != 0 {
t.Fatalf("lines that are not an application's refusal at MAIL wait: %+v", r.pending)
}
r.handle(limit, now, Place{}) // its row is not there yet
if len(r.pending) != 1 {
t.Fatalf("pending %d, want the line waiting", len(r.pending))
}
st.rows = 1 // the milter's row is written
r.retry(now.Add(time.Second))
if len(r.pending) != 0 || len(st.lines) != 1 || st.who[0] != "10.77.0.1|billing@sender.e2e.test" {
t.Fatalf("after the retry: pending %d, attached %v", len(r.pending), st.who)
}
r.handle(limit, now, Place{}) // no row ever comes
r.retry(now.Add(refusalLineWait + time.Second))
if len(r.pending) != 0 {
t.Fatal("a line kept waiting past its time")
}
}
// cleanup's line about a From: refusal is the row's, written by the panel; once
// the panel has had the time to, the line is handed to RecoverRefusal, which
// writes the row only if it is missing - with the login smtpd named for the
// queue id, the sender, the recipient and the From: line as printed.
func TestFromRefusalIsRecoveredAfterTheWait(t *testing.T) {
now := time.Date(2026, 10, 14, 12, 0, 0, 0, time.UTC)
st := &refusalProbe{}
r := &refusalLines{st: st}
r.handle(`2026-10-14T12:00:00+00:00 mail postfix/smtpd[11]: E543D3C75DA0: client=unknown[10.77.0.1], sasl_method=PLAIN, sasl_username=app1`, now, Place{})
r.handle(`2026-10-14T12:00:01+00:00 mail postfix/cleanup[13]: E543D3C75DA0: milter-reject: END-OF-MESSAGE from unknown[10.77.0.1]: 5.7.1 From: "The - Shop" <director@other.e2e.test> - it names another sender of this server; from=<alerts@sender.e2e.test> to=<rcpt@sink.e2e.test> proto=ESMTP helo=<x>`, now, Place{})
r.retry(now.Add(time.Second))
if len(st.recovered) != 0 {
t.Fatal("recovered before the panel had the time to write the row")
}
r.retry(now.Add(refusalLineWait + time.Second))
if len(st.recovered) != 1 {
t.Fatalf("recovered %d, want 1", len(st.recovered))
}
e := st.recovered[0]
want := store.SendLogEntry{QueueID: "E543D3C75DA0", AppLogin: "app1", From: "alerts@sender.e2e.test", To: "rcpt@sink.e2e.test",
Domain: "sender.e2e.test", Reason: store.ReasonFrom, HeaderFrom: `"The - Shop" <director@other.e2e.test>`}
if e != want {
t.Fatalf("recovered %+v, want %+v", e, want)
}
}
// While a line or a recovery waits, the offset saved for its file stays at the
// earliest line it needs - for a recovery the client= line that names its
// login - so a stop before the wait is over reads them again (review of stage
// 2b).
func TestWaitingLinesHoldTheSavedOffset(t *testing.T) {
now := time.Date(2026, 10, 14, 12, 0, 0, 0, time.UTC)
r := &refusalLines{st: &refusalProbe{}}
if _, held := r.oldest("fp"); held {
t.Fatal("nothing waits, yet the offset is held")
}
r.handle(`2026-10-14T12:00:00+00:00 mail postfix/smtpd[11]: Q1: client=unknown[10.77.0.1], sasl_method=PLAIN, sasl_username=app1`, now, Place{FP: "fp", End: 100})
r.handle(`2026-10-14T12:00:01+00:00 mail postfix/cleanup[13]: Q1: milter-reject: END-OF-MESSAGE from unknown[10.77.0.1]: 5.7.1 From: d@other.example - not an address this application sends as; from=<a@sender.example> to=<r@x> proto=ESMTP helo=<x>`, now, Place{FP: "fp", End: 300})
if p, held := r.oldest("fp"); !held || p.End != 100 {
t.Fatalf("held at %+v %v, want the client= line at 100", p, held)
}
if _, held := r.oldest("another"); held {
t.Fatal("a place in another file holds this one")
}
r.retry(now.Add(refusalLineWait + time.Second))
if _, held := r.oldest("fp"); held {
t.Fatal("still held after the wait")
}
// The tracker clamps what it saves to the hold.
cs := &captureStore{}
tr := &tracker{st: cs, path: "x", fp: "fp", hold: func(string) (Place, bool) { return Place{FP: "fp", End: 50}, true }}
tr.record(nil, 400, true)
if cs.state.Offset != 50 {
t.Fatalf("saved %d, want the hold at 50", cs.state.Offset)
}
}