From c35c204e93bf7146eb92a67a1df2d519f8505e25 Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 20 Oct 2026 09:45:46 +0000 Subject: [PATCH] A ban's earlier_bans counts a ban dropped after an admin's ban (closes #128) earlierBans took the counts in the notes of the netblock's first ban held, so a ban dropped after an admin's ban, which came first, went uncounted. Each ban held now gives a count, its notes' counts plus one for it and each ban held after it, and each cause takes the largest. Counting from the newest ban alone would lose the bans before an admin's ban added to bans.json without notes. Model: opus-5-5 --- internal/bans/admin_test.go | 29 +++++++++++++++++++++++++++++ internal/bans/bans.go | 29 +++++++++++++++++++---------- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/internal/bans/admin_test.go b/internal/bans/admin_test.go index d919772..5afefb3 100644 --- a/internal/bans/admin_test.go +++ b/internal/bans/admin_test.go @@ -61,6 +61,35 @@ func TestAdminsBansAreNeverDroppedAndDoNotCountTowardMaxBans(t *testing.T) { wantBans(t, ledger, map[netip.Prefix]int{adminsOnly: 2, both: 1, second: 0, third: 1}) } +func TestEarlierBansCountABanDroppedAfterAnAdminsBan(t *testing.T) { + t.Parallel() + + rules := defaultRules() + rules.MaxBans = 2 + ledger := bans.New(rules) + netblock := netip.MustParsePrefix("203.0.113.1/32") + other := netip.MustParsePrefix("203.0.113.2/32") + + // An admin bans the netblock for a minute, and it then breaks a limit + // twice, each time after its last ban has ended. + ledger.BanForAdmin(netblock, midnight(), midnight().Add(time.Minute), + "probes for logins") + first, _ := ledger.BanForLimit(netblock, midnight().Add(time.Minute), bans.Notes{}) + second, _ := ledger.BanForLimit(netblock, first.Expires, bans.Notes{}) + + // A ban on another netblock fills the ledger, and the netblock's first + // ban for a limit, its earliest that is not an admin's, goes. + ledger.BanForLimit(other, second.Start, bans.Notes{}) + wantBans(t, ledger, map[netip.Prefix]int{netblock: 2, other: 1}) + + // The netblock's next ban still counts it. + third, _ := ledger.BanForLimit(netblock, second.Expires, bans.Notes{}) + if third.Notes.EarlierBans != (bans.EarlierBans{Limit: 2, Admin: 1}) { + t.Errorf("the third ban for a limit has earlier bans %+v, "+ + "want 2 for a limit and 1 an admin's", third.Notes.EarlierBans) + } +} + func TestReasonOfTheBansSmallwebwafMakes(t *testing.T) { t.Parallel() diff --git a/internal/bans/bans.go b/internal/bans/bans.go index c07b52f..d4e9e88 100644 --- a/internal/bans/bans.go +++ b/internal/bans/bans.go @@ -687,26 +687,35 @@ func (l *Ledger) ban( } // earlierBans returns how many bans a netblock with the bans held, oldest -// first, has had, by cause: the first ban held counts the bans the -// netblock had before that one, since dropped to make room, and each ban -// held adds one. +// first, has had, by cause. Each ban held gives a count: the bans its +// notes count before it, and one for it and each ban held after it. A +// ban's count misses a ban dropped after it to make room, and that of an +// admin's ban added to bans.json without notes misses the bans before it, +// so each cause gets the largest of these counts. func earlierBans(held []Ban) EarlierBans { - earlier := held[0].Notes.EarlierBans + // fromHere counts ban and the bans held after it. + var most, fromHere EarlierBans - for _, ban := range held { + for _, ban := range slices.Backward(held) { switch ban.Cause { case CauseLimit: - earlier.Limit++ + fromHere.Limit++ case CauseAttack: - earlier.Attack++ + fromHere.Attack++ case CauseAdmin: - earlier.Admin++ + fromHere.Admin++ case CauseCrowdSec: - earlier.CrowdSec++ + fromHere.CrowdSec++ } + + notes := ban.Notes.EarlierBans + most.Limit = max(most.Limit, notes.Limit+fromHere.Limit) + most.Attack = max(most.Attack, notes.Attack+fromHere.Attack) + most.Admin = max(most.Admin, notes.Admin+fromHere.Admin) + most.CrowdSec = max(most.CrowdSec, notes.CrowdSec+fromHere.CrowdSec) } - return earlier + return most } // markChanged has Changed receive a value, unless one is waiting already.