inbound-antispam-panel: three things the reviews left as found
test / test (push) Canceled after 0s

The owner asked for them to be put right (2026-10-12).

  - A refusal is not logged twice after an unclean stop. The tailer
    saves its place every five seconds, so on start it reads some
    lines again; for the lines it reads on start a row is written only
    if the same row is not there already (store.InsertInboundSpamOnce:
    same moment, client, sender, recipient, decision, engine, detail).
    Lines read as they are written are always new, so two identical
    refusals in one second stay two rows.
  - The Inbound log offers Deny / Allow sender only for a sender the
    lists would take (inbound.Listable): an envelope sender may be
    spelt in ways the form refuses, and the link led to a refusal.
  - A network of one address is stored as the address: 192.0.2.7/32
    and 192.0.2.7 were two entries of one list.

Checked on the test server, uncommitted tree: go vet ./... ok,
go test ./... ok (22 packages), make e2e ok.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
mix
2026-10-12 08:17:23 +03:00
co-authored by Claude Opus 5.5
parent c731a98a0f
commit 89774ab847
14 changed files with 192 additions and 39 deletions
+6 -3
View File
@@ -23,7 +23,8 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
inbound log row; the log's row cap was applied only every six hours; rows of inbound log row; the log's row cap was applied only every six hours; rows of
a connection's later messages had no client address; two changes of the a connection's later messages had no client address; two changes of the
filter lists at once could leave the filter's files behind the page; a held filter lists at once could leave the filter's files behind the page; a held
message was logged twice. Recorded as accepted in message was logged twice; after an unclean stop of the panel the refusals of
its last seconds were logged twice. Recorded as accepted in
[docs/security.md](docs/security.md): the filter's verdict in the log is the [docs/security.md](docs/security.md): the filter's verdict in the log is the
sender's own while the filter is down; an allow entry for a sender address sender's own while the filter is down; an allow entry for a sender address
trusts an address anyone can claim; the list files are readable by anyone on trusts an address anyone can claim; the list files are readable by anyone on
@@ -76,14 +77,16 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
verdict and symbols, or the reply a refusal was given. A user with the verdict and symbols, or the reply a refusal was given. A user with the
*domain* role sees the rows of the inbound domains assigned to them. For the *domain* role sees the rows of the inbound domains assigned to them. For the
global role a row links to the filter lists with the sender filled in: global role a row links to the filter lists with the sender filled in:
*Deny sender* on accepted mail, *Allow sender* on mail the filter refused. *Deny sender* on accepted mail, *Allow sender* on mail the filter refused -
unless the sender is spelt in a way the lists do not take.
- **Inbound › Filter lists** (global role): add and remove allow and deny - **Inbound › Filter lists** (global role): add and remove allow and deny
entries; the page says where the lists go and whether the filter is on to entries; the page says where the lists go and whether the filter is on to
read them. Each entry shows how often it decided a message - *Matched*, or read them. Each entry shows how often it decided a message - *Matched*, or
*never* - and its kind opens a sheet with the note, the day it was added and *never* - and its kind opens a sheet with the note, the day it was added and
the last match, so that entries nothing matches any more can be found and the last match, so that entries nothing matches any more can be found and
removed. The count is read from the inbound log and reaches back as far as removed. The count is read from the inbound log and reaches back as far as
the log does. the log does. A network of one address (`192.0.2.7/32`) is stored as the
address, so it cannot be on a list twice.
- **Server › Settings**: *Keep inbound log rows, days*, shown with the inbound - **Server › Settings**: *Keep inbound log rows, days*, shown with the inbound
relay on. relay on.
- Help: *Inbound log* and *Filter lists*. - Help: *Inbound log* and *Filter lists*.
+3 -3
View File
@@ -97,9 +97,9 @@ smtp/inet` removes Debian's stock smtpd). Outbound delivery still uses the
line, when the recipient is at an inbound domain line, when the recipient is at an inbound domain
([internal/logtail/inbound.go](../internal/logtail/inbound.go)). A line ([internal/logtail/inbound.go](../internal/logtail/inbound.go)). A line
names one recipient, so a message refused at end-of-message is one row names one recipient, so a message refused at end-of-message is one row
whatever its recipient count, and a line read twice (the tailer saves its whatever its recipient count. The tailer saves its place every five seconds,
place every five seconds; an unclean stop can lose those seconds) is two so after an unclean stop it reads some lines again; the lines it reads on
rows. The start are written only if the same row is not there already. The
journal is pruned every six hours: rows journal is pruned every six hours: rows
older than the `inbound_spam_log_retention_days` setting, then down to older than the `inbound_spam_log_retention_days` setting, then down to
10 000. 10 000.
+13
View File
@@ -194,6 +194,14 @@ func (l *Lists) LastWritten() time.Time {
return newest return newest
} }
// Listable reports whether value could be added to a list as an entry of kind.
// The Inbound log asks before it offers a row's sender to the lists: an
// envelope sender may be spelt in ways the lists do not take.
func Listable(kind, value string) bool {
_, err := normalizeListValue(kind, value)
return err == nil
}
// normalizeListValue validates one entry and returns the form it is stored and // normalizeListValue validates one entry and returns the form it is stored and
// written in: lower case for an address or a domain, the canonical text of an // written in: lower case for an address or a domain, the canonical text of an
// address or a network for an IP. // address or a network for an IP.
@@ -250,6 +258,11 @@ func normalizeListValue(kind, raw string) (string, error) {
if p.Bits() == 0 { if p.Bits() == 0 {
return "", invalid("%q is every address there is", v) return "", invalid("%q is every address there is", v)
} }
// A network of one address is that address: 192.0.2.7/32 and
// 192.0.2.7 are one entry, not two that look different.
if p.Bits() == p.Addr().BitLen() {
return p.Addr().String(), nil
}
return p.Masked().String(), nil return p.Masked().String(), nil
} }
a, err := netip.ParseAddr(v) a, err := netip.ParseAddr(v)
+27
View File
@@ -46,6 +46,8 @@ func TestNormalizeListValue(t *testing.T) {
{store.SpamEntryDomain, "sub.evil.example", "sub.evil.example"}, {store.SpamEntryDomain, "sub.evil.example", "sub.evil.example"},
{store.SpamEntryIP, "192.0.2.7", "192.0.2.7"}, {store.SpamEntryIP, "192.0.2.7", "192.0.2.7"},
{store.SpamEntryIP, "192.0.2.77/24", "192.0.2.0/24"}, {store.SpamEntryIP, "192.0.2.77/24", "192.0.2.0/24"},
{store.SpamEntryIP, "192.0.2.7/32", "192.0.2.7"},
{store.SpamEntryIP, "2001:db8::1/128", "2001:db8::1"},
{store.SpamEntryIP, "::ffff:192.0.2.7", "192.0.2.7"}, {store.SpamEntryIP, "::ffff:192.0.2.7", "192.0.2.7"},
{store.SpamEntryIP, "2001:DB8:0:0::1", "2001:db8::1"}, {store.SpamEntryIP, "2001:DB8:0:0::1", "2001:db8::1"},
{store.SpamEntryIP, "2001:db8:1::5/48", "2001:db8:1::/48"}, {store.SpamEntryIP, "2001:db8:1::5/48", "2001:db8:1::/48"},
@@ -368,3 +370,28 @@ func TestListsUsageWithNoEntries(t *testing.T) {
t.Errorf("Usage = %v, %v", usage, err) t.Errorf("Usage = %v, %v", usage, err)
} }
} }
// The same address written as a network of one is the same entry.
func TestListsOneAddressNetworkIsTheAddress(t *testing.T) {
l, _, dir := testLists(t)
if _, err := l.Add(store.SpamListDeny, store.SpamEntryIP, "192.0.2.7", ""); err != nil {
t.Fatalf("Add: %v", err)
}
if _, err := l.Add(store.SpamListDeny, store.SpamEntryIP, "192.0.2.7/32", ""); !errors.Is(err, store.ErrSpamListEntryExists) {
t.Errorf("192.0.2.7/32 after 192.0.2.7: %v, want ErrSpamListEntryExists", err)
}
if got := strings.Join(mapLines(t, dir, "deny_ip.map"), " "); got != "192.0.2.7" {
t.Errorf("deny_ip.map = %q", got)
}
}
func TestListable(t *testing.T) {
for value, want := range map[string]bool{
"billing@vendor.example": true, "srs0=ab=cd=example.org=user@forwarder.example": true,
"o'brien@example.org": false, "a#b@example.org": false, "": false, "not-an-address": false,
} {
if got := Listable(store.SpamEntryAddress, value); got != want {
t.Errorf("Listable(address, %q) = %v, want %v", value, got, want)
}
}
}
+17 -2
View File
@@ -15,6 +15,7 @@ import (
// pruning. *store.Store satisfies it. // pruning. *store.Store satisfies it.
type InboundStore interface { type InboundStore interface {
InsertInboundSpam(e store.InboundSpamEntry) error InsertInboundSpam(e store.InboundSpamEntry) error
InsertInboundSpamOnce(e store.InboundSpamEntry) (bool, error)
IsInboundDomain(name string) (bool, error) IsInboundDomain(name string) (bool, error)
PruneInboundSpam(cutoff time.Time, keep int) (int64, error) PruneInboundSpam(cutoff time.Time, keep int) (int64, error)
} }
@@ -107,7 +108,14 @@ func newInboundJournal(st InboundStore) *inboundJournal {
// handle looks at one mail.log line and writes a journal row when it is a // handle looks at one mail.log line and writes a journal row when it is a
// refusal of mail for an inbound domain. Errors are logged; the tailer goes on. // refusal of mail for an inbound domain. Errors are logged; the tailer goes on.
func (j *inboundJournal) handle(line string, now time.Time) { //
// catchingUp says the line is one of those the tailer reads when it starts,
// from where it last saved its place to the end of the file. It saves its
// place every few seconds, so after an unclean stop some of those lines were
// read before; a row is then written only if it is not there already. Lines
// read as they are written are always new, and two identical refusals in one
// second stay two rows.
func (j *inboundJournal) handle(line string, now time.Time, catchingUp bool) {
at, rest := lineTime(line, now) at, rest := lineTime(line, now)
if m := inboundClientRe.FindStringSubmatch(rest); m != nil { if m := inboundClientRe.FindStringSubmatch(rest); m != nil {
j.remember(m[1]) j.remember(m[1])
@@ -174,7 +182,14 @@ func (j *inboundJournal) handle(line string, now time.Time) {
if e.Decision == store.SpamDecisionReject && strings.HasPrefix(reason, "4") { if e.Decision == store.SpamDecisionReject && strings.HasPrefix(reason, "4") {
e.Decision = store.SpamDecisionTempfail e.Decision = store.SpamDecisionTempfail
} }
if err := j.st.InsertInboundSpam(e); err != nil { if catchingUp {
if wrote, err := j.st.InsertInboundSpamOnce(e); err != nil {
log.Printf("log-tailer: inbound journal: record %s -> %s: %v", from, to, err)
return
} else if !wrote {
return // read before the last stop
}
} else if err := j.st.InsertInboundSpam(e); err != nil {
log.Printf("log-tailer: inbound journal: record %s -> %s: %v", from, to, err) log.Printf("log-tailer: inbound journal: record %s -> %s: %v", from, to, err)
return return
} }
+49 -22
View File
@@ -22,6 +22,17 @@ func (f *fakeInboundStore) InsertInboundSpam(e store.InboundSpamEntry) error {
return nil return nil
} }
// InsertInboundSpamOnce writes unless an equal row is there, like the store's.
func (f *fakeInboundStore) InsertInboundSpamOnce(e store.InboundSpamEntry) (bool, error) {
for _, r := range f.rows {
if r == e {
return false, nil
}
}
f.rows = append(f.rows, e)
return true, nil
}
func (f *fakeInboundStore) IsInboundDomain(name string) (bool, error) { func (f *fakeInboundStore) IsInboundDomain(name string) (bool, error) {
if f.lookupErr != nil { if f.lookupErr != nil {
return false, f.lookupErr return false, f.lookupErr
@@ -50,7 +61,7 @@ const (
// its line. // its line.
func TestInboundRejectByPostfix(t *testing.T) { func TestInboundRejectByPostfix(t *testing.T) {
j, st := inboundFixture() j, st := inboundFixture()
j.handle(unknownRcp, tailNow) j.handle(unknownRcp, tailNow, false)
if len(st.rows) != 1 { if len(st.rows) != 1 {
t.Fatalf("rows = %+v", st.rows) t.Fatalf("rows = %+v", st.rows)
} }
@@ -68,7 +79,7 @@ func TestInboundRejectByPostfix(t *testing.T) {
func TestInboundTempfailByPostfix(t *testing.T) { func TestInboundTempfailByPostfix(t *testing.T) {
j, st := inboundFixture() j, st := inboundFixture()
j.handle(head+"postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from unknown[2001:db8::9]: 450 4.1.8 <a@nx.example>: Sender address rejected: Domain not found; from=<a@nx.example> to=<B@IN.example.com> proto=ESMTP helo=<x>", tailNow) j.handle(head+"postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from unknown[2001:db8::9]: 450 4.1.8 <a@nx.example>: Sender address rejected: Domain not found; from=<a@nx.example> to=<B@IN.example.com> proto=ESMTP helo=<x>", tailNow, false)
if len(st.rows) != 1 || st.rows[0].Decision != store.SpamDecisionTempfail || st.rows[0].ClientIP != "2001:db8::9" || st.rows[0].InboundDomain != "in.example.com" { if len(st.rows) != 1 || st.rows[0].Decision != store.SpamDecisionTempfail || st.rows[0].ClientIP != "2001:db8::9" || st.rows[0].InboundDomain != "in.example.com" {
t.Errorf("rows = %+v", st.rows) t.Errorf("rows = %+v", st.rows)
} }
@@ -81,14 +92,14 @@ func TestInboundMilterVerdictNeedsAPort25QueueID(t *testing.T) {
j, st := inboundFixture() j, st := inboundFixture()
// Opened by a submission port: postfix/smtpd, not postfix/inbound/smtpd. // Opened by a submission port: postfix/smtpd, not postfix/inbound/smtpd.
j.handle(head+"postfix/smtpd[20]: 4A1B2C3D: client=app.example.com[203.0.113.4], sasl_method=PLAIN, sasl_username=app", tailNow) j.handle(head+"postfix/smtpd[20]: 4A1B2C3D: client=app.example.com[203.0.113.4], sasl_method=PLAIN, sasl_username=app", tailNow, false)
j.handle(head+verdict, tailNow) j.handle(head+verdict, tailNow, false)
if len(st.rows) != 0 { if len(st.rows) != 0 {
t.Fatalf("a submission-port verdict was journalled: %+v", st.rows) t.Fatalf("a submission-port verdict was journalled: %+v", st.rows)
} }
j.handle(head+"postfix/inbound/smtpd[41]: 4A1B2C3D: client=mx.example.net[203.0.113.9]", tailNow) j.handle(head+"postfix/inbound/smtpd[41]: 4A1B2C3D: client=mx.example.net[203.0.113.9]", tailNow, false)
j.handle(head+verdict, tailNow) j.handle(head+verdict, tailNow, false)
if len(st.rows) != 1 { if len(st.rows) != 1 {
t.Fatalf("rows = %+v", st.rows) t.Fatalf("rows = %+v", st.rows)
} }
@@ -97,7 +108,7 @@ func TestInboundMilterVerdictNeedsAPort25QueueID(t *testing.T) {
t.Errorf("row = %+v", got) t.Errorf("row = %+v", got)
} }
// One verdict per message: the id is forgotten with it. // One verdict per message: the id is forgotten with it.
j.handle(head+verdict, tailNow) j.handle(head+verdict, tailNow, false)
if len(st.rows) != 1 { if len(st.rows) != 1 {
t.Errorf("the same queue id was journalled twice") t.Errorf("the same queue id was journalled twice")
} }
@@ -109,8 +120,8 @@ func TestInboundMilterDecisions(t *testing.T) {
"milter-discard": {"milter triggers DISCARD action", store.SpamDecisionReject}, "milter-discard": {"milter triggers DISCARD action", store.SpamDecisionReject},
} { } {
j, st := inboundFixture() j, st := inboundFixture()
j.handle(head+"postfix/inbound/smtpd[41]: Q1: client=mx.example.net[203.0.113.9]", tailNow) j.handle(head+"postfix/inbound/smtpd[41]: Q1: client=mx.example.net[203.0.113.9]", tailNow, false)
j.handle(head+"postfix/cleanup[52]: Q1: "+kind+": END-OF-MESSAGE from mx.example.net[203.0.113.9]: "+c.reply+"; from=<a@example.net> to=<b@in.example.com> proto=ESMTP helo=<mx.example.net>", tailNow) j.handle(head+"postfix/cleanup[52]: Q1: "+kind+": END-OF-MESSAGE from mx.example.net[203.0.113.9]: "+c.reply+"; from=<a@example.net> to=<b@in.example.com> proto=ESMTP helo=<mx.example.net>", tailNow, false)
if len(st.rows) != 1 || st.rows[0].Decision != c.decision || st.rows[0].Engine != store.SpamEngineRspamd { if len(st.rows) != 1 || st.rows[0].Decision != c.decision || st.rows[0].Engine != store.SpamEngineRspamd {
t.Errorf("%s: rows = %+v", kind, st.rows) t.Errorf("%s: rows = %+v", kind, st.rows)
} }
@@ -130,7 +141,7 @@ func TestInboundIgnores(t *testing.T) {
"an address with a bracket in it": head + "postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 554 5.7.1 <b@elsewhere.example>: Relay access denied; from=<\"a> to=<v@in.example.com> proto=X helo=<y\"@example.net> to=<b@elsewhere.example> proto=ESMTP helo=<x>", "an address with a bracket in it": head + "postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 554 5.7.1 <b@elsewhere.example>: Relay access denied; from=<\"a> to=<v@in.example.com> proto=X helo=<y\"@example.net> to=<b@elsewhere.example> proto=ESMTP helo=<x>",
} { } {
j, st := inboundFixture() j, st := inboundFixture()
j.handle(line, tailNow) j.handle(line, tailNow, false)
if len(st.rows) != 0 { if len(st.rows) != 0 {
t.Errorf("%s: journalled %+v", name, st.rows) t.Errorf("%s: journalled %+v", name, st.rows)
} }
@@ -139,7 +150,7 @@ func TestInboundIgnores(t *testing.T) {
func TestInboundDetailIsCutAndPrintable(t *testing.T) { func TestInboundDetailIsCutAndPrintable(t *testing.T) {
j, st := inboundFixture() j, st := inboundFixture()
j.handle(head+"postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 550 5.7.1 caf\xc3\xa9 \x1b[31m"+strings.Repeat("x", 400)+"; from=<a@example.net> to=<b@in.example.com> proto=ESMTP helo=<x>", tailNow) j.handle(head+"postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 550 5.7.1 caf\xc3\xa9 \x1b[31m"+strings.Repeat("x", 400)+"; from=<a@example.net> to=<b@in.example.com> proto=ESMTP helo=<x>", tailNow, false)
if len(st.rows) != 1 { if len(st.rows) != 1 {
t.Fatalf("rows = %+v", st.rows) t.Fatalf("rows = %+v", st.rows)
} }
@@ -152,7 +163,7 @@ func TestInboundDetailIsCutAndPrintable(t *testing.T) {
func TestInboundLookupFailureLeavesNoRow(t *testing.T) { func TestInboundLookupFailureLeavesNoRow(t *testing.T) {
j, st := inboundFixture() j, st := inboundFixture()
st.lookupErr = errors.New("boom") st.lookupErr = errors.New("boom")
j.handle(unknownRcp, tailNow) j.handle(unknownRcp, tailNow, false)
if len(st.rows) != 0 { if len(st.rows) != 0 {
t.Errorf("rows = %+v", st.rows) t.Errorf("rows = %+v", st.rows)
} }
@@ -162,8 +173,8 @@ func TestInboundLookupFailureLeavesNoRow(t *testing.T) {
// writes its row; a second one from the log would be the same message twice. // writes its row; a second one from the log would be the same message twice.
func TestInboundHoldIsTheMiltersRow(t *testing.T) { func TestInboundHoldIsTheMiltersRow(t *testing.T) {
j, st := inboundFixture() j, st := inboundFixture()
j.handle(head+"postfix/inbound/smtpd[41]: Q1: client=mx.example.net[203.0.113.9]", tailNow) j.handle(head+"postfix/inbound/smtpd[41]: Q1: client=mx.example.net[203.0.113.9]", tailNow, false)
j.handle(head+"postfix/cleanup[52]: Q1: milter-hold: END-OF-MESSAGE from mx.example.net[203.0.113.9]: milter triggers HOLD action; from=<a@example.net> to=<b@in.example.com> proto=ESMTP helo=<mx.example.net>", tailNow) j.handle(head+"postfix/cleanup[52]: Q1: milter-hold: END-OF-MESSAGE from mx.example.net[203.0.113.9]: milter triggers HOLD action; from=<a@example.net> to=<b@in.example.com> proto=ESMTP helo=<mx.example.net>", tailNow, false)
if len(st.rows) != 0 { if len(st.rows) != 0 {
t.Errorf("rows = %+v", st.rows) t.Errorf("rows = %+v", st.rows)
} }
@@ -174,7 +185,7 @@ func TestInboundHoldIsTheMiltersRow(t *testing.T) {
// which is Postfix's own. // which is Postfix's own.
func TestInboundHeloCannotWriteTheRow(t *testing.T) { func TestInboundHeloCannotWriteTheRow(t *testing.T) {
j, st := inboundFixture() j, st := inboundFixture()
j.handle(head+"postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 550 5.1.1 <nobody@in.example.com>: Recipient address rejected: User unknown in relay recipient table; from=<real@example.net> to=<nobody@in.example.com> proto=ESMTP helo=<x 5.7.1 Refused by this server's deny list; from=<victim@bank.example> to=<ceo@in.example.com> proto=ESMTP helo=<y>", tailNow) j.handle(head+"postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 550 5.1.1 <nobody@in.example.com>: Recipient address rejected: User unknown in relay recipient table; from=<real@example.net> to=<nobody@in.example.com> proto=ESMTP helo=<x 5.7.1 Refused by this server's deny list; from=<victim@bank.example> to=<ceo@in.example.com> proto=ESMTP helo=<y>", tailNow, false)
if len(st.rows) != 1 { if len(st.rows) != 1 {
t.Fatalf("rows = %+v", st.rows) t.Fatalf("rows = %+v", st.rows)
} }
@@ -186,7 +197,7 @@ func TestInboundHeloCannotWriteTheRow(t *testing.T) {
// A HELO name with a closing bracket in it does not lose the line either. // A HELO name with a closing bracket in it does not lose the line either.
j, st = inboundFixture() j, st = inboundFixture()
j.handle(head+"postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 550 5.1.1 <nobody@in.example.com>: no; from=<real@example.net> to=<nobody@in.example.com> proto=ESMTP helo=<a>b>", tailNow) j.handle(head+"postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 550 5.1.1 <nobody@in.example.com>: no; from=<real@example.net> to=<nobody@in.example.com> proto=ESMTP helo=<a>b>", tailNow, false)
if len(st.rows) != 1 || st.rows[0].From != "real@example.net" { if len(st.rows) != 1 || st.rows[0].From != "real@example.net" {
t.Errorf("rows = %+v", st.rows) t.Errorf("rows = %+v", st.rows)
} }
@@ -194,8 +205,8 @@ func TestInboundHeloCannotWriteTheRow(t *testing.T) {
// The same on a verdict line: the filter's reply is not extended by the // The same on a verdict line: the filter's reply is not extended by the
// HELO name, so a stranger cannot make a refusal look like the deny list's. // HELO name, so a stranger cannot make a refusal look like the deny list's.
j, st = inboundFixture() j, st = inboundFixture()
j.handle(head+"postfix/inbound/smtpd[41]: Q1: client=x[203.0.113.9]", tailNow) j.handle(head+"postfix/inbound/smtpd[41]: Q1: client=x[203.0.113.9]", tailNow, false)
j.handle(head+"postfix/cleanup[52]: Q1: milter-reject: END-OF-MESSAGE from x[203.0.113.9]: 5.7.1 Spam message rejected; from=<real@example.net> to=<b@in.example.com> proto=ESMTP helo=<x this server's deny list; from=<spoofed@example.org> to=<b@in.example.com> proto=ESMTP helo=<y>", tailNow) j.handle(head+"postfix/cleanup[52]: Q1: milter-reject: END-OF-MESSAGE from x[203.0.113.9]: 5.7.1 Spam message rejected; from=<real@example.net> to=<b@in.example.com> proto=ESMTP helo=<x this server's deny list; from=<spoofed@example.org> to=<b@in.example.com> proto=ESMTP helo=<y>", tailNow, false)
if len(st.rows) != 1 || st.rows[0].Detail != "5.7.1 Spam message rejected" || st.rows[0].From != "real@example.net" { if len(st.rows) != 1 || st.rows[0].Detail != "5.7.1 Spam message rejected" || st.rows[0].From != "real@example.net" {
t.Errorf("rows = %+v", st.rows) t.Errorf("rows = %+v", st.rows)
} }
@@ -210,9 +221,9 @@ func TestInboundQueueIDIsForgotten(t *testing.T) {
"reused by the submission port": "postfix/smtpd[20]: Q1: client=app.example.com[203.0.113.4], sasl_method=PLAIN, sasl_username=app", "reused by the submission port": "postfix/smtpd[20]: Q1: client=app.example.com[203.0.113.4], sasl_method=PLAIN, sasl_username=app",
} { } {
j, st := inboundFixture() j, st := inboundFixture()
j.handle(head+"postfix/inbound/smtpd[41]: Q1: client=mx.example.net[203.0.113.9]", tailNow) j.handle(head+"postfix/inbound/smtpd[41]: Q1: client=mx.example.net[203.0.113.9]", tailNow, false)
j.handle(head+forget, tailNow) j.handle(head+forget, tailNow, false)
j.handle(head+verdict, tailNow) j.handle(head+verdict, tailNow, false)
if len(st.rows) != 0 || len(j.port25) != 0 { if len(st.rows) != 0 || len(j.port25) != 0 {
t.Errorf("%s: rows %+v, remembered %v", name, st.rows, j.port25) t.Errorf("%s: rows %+v, remembered %v", name, st.rows, j.port25)
} }
@@ -240,7 +251,7 @@ func TestInboundStaleArrivalDoesNotDropALiveID(t *testing.T) {
func TestInboundCutsTheJournalAsItWrites(t *testing.T) { func TestInboundCutsTheJournalAsItWrites(t *testing.T) {
j, st := inboundFixture() j, st := inboundFixture()
for i := 0; i < inboundPruneEvery*2+10; i++ { for i := 0; i < inboundPruneEvery*2+10; i++ {
j.handle(unknownRcp, tailNow) j.handle(unknownRcp, tailNow, false)
} }
if len(st.pruned) != 2 || st.pruned[0] != inboundSpamLogMaxRows { if len(st.pruned) != 2 || st.pruned[0] != inboundSpamLogMaxRows {
t.Errorf("cuts = %v, want two, to %d rows", st.pruned, inboundSpamLogMaxRows) t.Errorf("cuts = %v, want two, to %d rows", st.pruned, inboundSpamLogMaxRows)
@@ -304,3 +315,19 @@ func TestInboundRetentionPrunesAtStart(t *testing.T) {
func contextWithCancel() (context.Context, context.CancelFunc) { func contextWithCancel() (context.Context, context.CancelFunc) {
return context.WithCancel(context.Background()) return context.WithCancel(context.Background())
} }
// The lines the tailer reads when it starts may be ones it read before its
// last stop: a refusal among them is one row, not two. Lines read as they are
// written are always new.
func TestInboundCatchUpDoesNotWriteARowTwice(t *testing.T) {
j, st := inboundFixture()
j.handle(unknownRcp, tailNow, false) // before the stop
j.handle(unknownRcp, tailNow, true) // the same line, read again on start
if len(st.rows) != 1 {
t.Fatalf("rows = %d, want the one refusal", len(st.rows))
}
j.handle(unknownRcp, tailNow, false) // a second refusal in the same second
if len(st.rows) != 2 {
t.Errorf("rows = %d, want two: a line read as it is written is new", len(st.rows))
}
}
+7 -1
View File
@@ -127,7 +127,13 @@ func Run(ctx context.Context, path string, st StatusStore, retention RetentionDa
return follow(ctx, path, &tracker{st: st, path: path}, caughtUp, func(line string) { return follow(ctx, path, &tracker{st: st, path: path}, caughtUp, func(line string) {
if journal != nil { if journal != nil {
journal.handle(line, time.Now().UTC()) catchingUp := true
select {
case <-caughtUp:
catchingUp = false
default:
}
journal.handle(line, time.Now().UTC(), catchingUp)
} }
queueID, recipient, status, ok := parseDelivery(line) queueID, recipient, status, ok := parseDelivery(line)
if !ok { if !ok {
+3 -3
View File
@@ -22,8 +22,8 @@ const (
// persistInterval throttles the offset write. Losing up to this much // persistInterval throttles the offset write. Losing up to this much
// progress on a crash means re-parsing a few lines, which is much cheaper // progress on a crash means re-parsing a few lines, which is much cheaper
// than a database write per poll tick. For a delivery status that changes // than a database write per poll tick. For a delivery status that changes
// nothing (UpdateStatus is idempotent); a refusal in those lines gets a // nothing (UpdateStatus is idempotent), and a refusal among them is not
// second row in the inbound journal. // written to the inbound journal a second time (inboundJournal.handle).
persistInterval = 5 * time.Second persistInterval = 5 * time.Second
) )
@@ -49,7 +49,7 @@ type tracker struct {
// tail written while the panel was down; anything else means the file is not // tail written while the panel was down; anything else means the file is not
// the one the offset referred to (rotated, recreated or truncated in the // the one the offset referred to (rotated, recreated or truncated in the
// meantime), so read it from the start. Re-parsing lines already seen writes // meantime), so read it from the start. Re-parsing lines already seen writes
// the same status onto the same send-log row, and a second inbound journal // the same status onto the same send-log row, and no second inbound journal
// row for a refusal among them. // row for a refusal among them.
func (t *tracker) resume(f *os.File) int64 { func (t *tracker) resume(f *os.File) int64 {
size, err := fileSize(f) size, err := fileSize(f)
+26
View File
@@ -63,6 +63,32 @@ func (s *Store) InsertInboundSpam(e InboundSpamEntry) error {
return nil return nil
} }
// InsertInboundSpamOnce records a journal row unless the same row is there
// already - same moment, client, sender, recipient, decision, engine and
// detail - and says whether it wrote. The log tailer uses it for the lines it
// reads when it starts: after an unclean stop those are lines it may have read
// before, and a refusal read twice is otherwise two rows. e.At must be set; a
// row dated "now" is never the same as another.
func (s *Store) InsertInboundSpamOnce(e InboundSpamEntry) (bool, error) {
at := e.At.UTC().Format(time.RFC3339)
res, err := s.db.Exec(
`INSERT INTO inbound_spam_log
(inbound_domain, client_ip, from_addr, to_addr, subject, decision, engine, detail, created_at)
SELECT ?, ?, ?, ?, ?, ?, ?, ?, ?
WHERE NOT EXISTS (
SELECT 1 FROM inbound_spam_log
WHERE created_at = ? AND client_ip = ? AND from_addr = ? AND to_addr = ?
AND decision = ? AND engine = ? AND detail = ?)`,
e.InboundDomain, e.ClientIP, e.From, e.To, e.Subject, e.Decision, e.Engine, e.Detail, at,
at, e.ClientIP, e.From, e.To, e.Decision, e.Engine, e.Detail,
)
if err != nil {
return false, fmt.Errorf("insert inbound_spam_log once: %w", err)
}
n, _ := res.RowsAffected()
return n > 0, nil
}
// PruneInboundSpam deletes journal rows older than cutoff and then, if more // PruneInboundSpam deletes journal rows older than cutoff and then, if more
// than keep rows are left, the oldest of those. It returns how many went. // than keep rows are left, the oldest of those. It returns how many went.
func (s *Store) PruneInboundSpam(cutoff time.Time, keep int) (int64, error) { func (s *Store) PruneInboundSpam(cutoff time.Time, keep int) (int64, error) {
+26
View File
@@ -310,3 +310,29 @@ func TestPruneInboundSpamCapIsByTime(t *testing.T) {
t.Errorf("left = %+v, %v; want the two newest by time", rows, err) t.Errorf("left = %+v, %v; want the two newest by time", rows, err)
} }
} }
func TestInsertInboundSpamOnce(t *testing.T) {
st := openTestStore(t)
e := InboundSpamEntry{
InboundDomain: "in.example.com", ClientIP: "203.0.113.7", From: "a@example.net", To: "nobody@in.example.com",
Decision: SpamDecisionReject, Engine: SpamEnginePostfix, Detail: "550 5.1.1 no",
At: time.Date(2026, 10, 12, 9, 0, 5, 0, time.UTC),
}
for i, want := range []bool{true, false, false} {
if wrote, err := st.InsertInboundSpamOnce(e); err != nil || wrote != want {
t.Fatalf("attempt %d: wrote %v, %v; want %v", i+1, wrote, err, want)
}
}
// Anything that tells two refusals apart makes a second row.
later, other := e, e
later.At = e.At.Add(time.Second)
other.To = "somebody@in.example.com"
for name, v := range map[string]InboundSpamEntry{"a second later": later, "another recipient": other} {
if wrote, err := st.InsertInboundSpamOnce(v); err != nil || !wrote {
t.Errorf("%s: wrote %v, %v", name, wrote, err)
}
}
if n, err := st.CountInboundSpam(InboundSpamFilter{}); err != nil || n != 3 {
t.Errorf("rows = %d, %v; want 3", n, err)
}
}
@@ -90,7 +90,7 @@ func (h *Handlers) HandleInboundLog(w http.ResponseWriter, r *http.Request) {
table := make([]view.InLogRow, len(rows)) table := make([]view.InLogRow, len(rows))
for i, row := range rows { for i, row := range rows {
table[i] = out.NewRow(view.InLogRowInput{ table[i] = out.NewRow(view.InLogRowInput{
ID: row.ID, At: row.CreatedAt, From: row.From, Client: row.ClientIP, To: row.To, Subject: row.Subject, ID: row.ID, Listable: inbound.Listable(store.SpamEntryAddress, row.From), At: row.CreatedAt, From: row.From, Client: row.ClientIP, To: row.To, Subject: row.Subject,
Decision: row.Decision, Engine: row.Engine, Detail: row.Detail, Decision: row.Decision, Engine: row.Engine, Detail: row.Detail,
}) })
} }
+1 -1
View File
@@ -427,7 +427,7 @@ func inLogFixture() *InLog {
panic(err) panic(err)
} }
id++ id++
return p.NewRow(InLogRowInput{ID: id, At: at, From: from, Client: client, To: to, Subject: subject, return p.NewRow(InLogRowInput{ID: id, Listable: true, At: at, From: from, Client: client, To: to, Subject: subject,
Decision: decision, Engine: engine, Detail: detail}) Decision: decision, Engine: engine, Detail: detail})
} }
return p.WithRows([]InLogRow{ return p.WithRows([]InLogRow{
+5 -2
View File
@@ -85,7 +85,10 @@ type InLogRow struct {
// From may be empty: refused mail is read from mail.log, which has no subject, // From may be empty: refused mail is read from mail.log, which has no subject,
// and a filter that said nothing has no detail. // and a filter that said nothing has no detail.
type InLogRowInput struct { type InLogRowInput struct {
ID int64 ID int64
// Listable says the sender is an address the filter lists would take. A
// row whose sender they would refuse offers no way into them.
Listable bool
At time.Time At time.Time
From string From string
Client string Client string
@@ -149,7 +152,7 @@ func (p *InLog) NewRow(in InLogRowInput) InLogRow {
Rows: whyRows(in.Decision, in.Engine, in.Detail, in.Subject), Rows: whyRows(in.Decision, in.Engine, in.Detail, in.Subject),
}, },
} }
if p.IsGlobal { if p.IsGlobal && in.Listable {
r.Action = senderAction(in.Decision, in.Engine, in.From) r.Action = senderAction(in.Decision, in.Engine, in.From)
} }
return r return r
+8 -1
View File
@@ -147,10 +147,17 @@ func TestInLogSenderAction(t *testing.T) {
// The address in the link is data: it cannot leave the attribute. // The address in the link is data: it cannot leave the attribute.
p := NewInLog(admin(), 90, nil, InLogFilter{}) p := NewInLog(admin(), 90, nil, InLogFilter{})
row := p.NewRow(InLogRowInput{At: time.Now(), From: `"><script>1</script>@x`, To: "b@acme.io", Decision: "accept", Engine: "rspamd"}) // (The handler would not call such a sender listable; the view must not
// depend on that.)
row := p.NewRow(InLogRowInput{Listable: true, At: time.Now(), From: `"><script>1</script>@x`, To: "b@acme.io", Decision: "accept", Engine: "rspamd"})
out := renderSignedIn(t, "in-log", p.WithRows([]InLogRow{row}, 1, 1)) out := renderSignedIn(t, "in-log", p.WithRows([]InLogRow{row}, 1, 1))
pageLacks(t, "Inbound log", out, `<script>1`) pageLacks(t, "Inbound log", out, `<script>1`)
pageHas(t, "Inbound log", out, `value=%22%3E%3Cscript%3E1%3C%2Fscript%3E%40x`) pageHas(t, "Inbound log", out, `value=%22%3E%3Cscript%3E1%3C%2Fscript%3E%40x`)
// A sender the lists would refuse is offered no way into them.
if row := p.NewRow(InLogRowInput{At: time.Now(), From: "o'brien@example.org", To: "b@acme.io", Decision: "accept", Engine: "rspamd"}); row.Action != nil {
t.Errorf("a sender that is not listable is offered %+v", *row.Action)
}
} }
// What a sender wrote is text on the page: an address, a subject or a reply from // What a sender wrote is text on the page: an address, a subject or a reply from