Skip to content

Commit fb67a7d

Browse files
djeshkovdfw0000
djeshkov
and
dfw0000
authored
fix: UA-cluster pass must not duplicate per-IP pass IPs (#8)
The UA-cluster pass wrote cluster member IPs to blocked-ua-clusters.conf while only deduping against its own file. An IP caught by both the per-IP pass and the UA-cluster pass ended up in both blocked-ips.conf and blocked-ua-clusters.conf. Since nginx/blacklist.conf includes both files in the same geo block, nginx logged a "duplicate network" warning for every such IP on each reload (300+ warnings observed in production). Fix in cmd_ua_cluster: - Read blocked_ips_conf and skip any cluster member IP the per-IP pass already owns (the per-IP pass runs first; the UA-cluster pass yields). - Drop stale entries already in blocked-ua-clusters.conf that the per-IP pass now owns — cleans duplicates accumulated before this fix on the next write. - Force the write when only stale dups were dropped (no new/extended). Also fixes a latent same-run bug: an IP appearing in two clusters was appended to `auto` twice because `auto_by_cidr` was not updated after an append. The index is now kept in sync. Adds tests/test_ua_cluster_dedup.py — 3 regression tests; 2 fail on the pre-fix code (verified by stash-and-run). Co-authored-by: Daniil Zheshkov <dan@cloudzen.it>
1 parent 6817b7f commit fb67a7d

2 files changed

Lines changed: 152 additions & 4 deletions

File tree

‎autoblock‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1427,9 +1427,26 @@ def cmd_ua_cluster(cfg, dry_run=False, show_only=False):
14271427

14281428
manual, auto = read_blocked(cfg["blocked_ua_cluster_conf"])
14291429
manual_set = manual_cidrs(manual)
1430+
1431+
# Dedup against the per-IP pass output. An IP already blocked by the per-IP
1432+
# pass must NOT also be written here: both files are included in the same
1433+
# nginx geo block, and a CIDR present in two included files makes nginx log
1434+
# a "duplicate network" warning on every reload. The per-IP pass owns such
1435+
# IPs (it runs first); the UA-cluster pass yields. We both skip them when
1436+
# adding AND drop any stale entries already in our file that the per-IP
1437+
# pass now owns — that cleans duplicates accumulated before this fix.
1438+
per_ip_cidrs = set()
1439+
per_ip_path = cfg.get("blocked_ips_conf")
1440+
if per_ip_path and Path(per_ip_path).exists():
1441+
pm, pa = read_blocked(per_ip_path)
1442+
per_ip_cidrs = manual_cidrs(pm) | {e["cidr"] for e in pa}
1443+
1444+
pre_dedup = len(auto)
1445+
auto = [e for e in auto if e["cidr"] not in per_ip_cidrs]
1446+
dropped_dups = pre_dedup - len(auto)
14301447
auto_by_cidr = {e["cidr"]: e for e in auto}
14311448

1432-
added, extended = [], []
1449+
added, extended, skipped_per_ip = [], [], 0
14331450
for c in clusters:
14341451
ua_tag = re.sub(r"\s+", "_", c["ua"])[:40]
14351452
full_reason = (f"ua-cluster score{c['score']} {c['ip_count']}ips "
@@ -1441,23 +1458,29 @@ def cmd_ua_cluster(cfg, dry_run=False, show_only=False):
14411458
for ip in c["ips"]:
14421459
if ip in manual_set:
14431460
continue
1461+
if ip in per_ip_cidrs:
1462+
skipped_per_ip += 1
1463+
continue
14441464
if ip in auto_by_cidr:
14451465
auto_by_cidr[ip]["expires"] = c["expires"]
14461466
auto_by_cidr[ip]["reason"] = full_reason
14471467
extended.append(ip)
14481468
else:
14491469
auto.append({"cidr": ip, "added": c["added"],
14501470
"expires": c["expires"], "reason": full_reason})
1471+
auto_by_cidr[ip] = auto[-1]
14511472
added.append(ip)
14521473

1453-
logging.info("UA-cluster: %d cluster(s) → %d new IPs, %d extended",
1454-
len(clusters), len(added), len(extended))
1474+
logging.info("UA-cluster: %d cluster(s) → %d new IPs, %d extended, "
1475+
"%d skipped (owned by per-IP pass), %d stale dups dropped",
1476+
len(clusters), len(added), len(extended),
1477+
skipped_per_ip, dropped_dups)
14551478

14561479
if dry_run:
14571480
logging.info("--dry-run: UA-cluster %d new, %d extended — NOT applied",
14581481
len(added), len(extended))
14591482
return
1460-
if not added and not extended:
1483+
if not added and not extended and not dropped_dups:
14611484
return
14621485

14631486
write_blocked(cfg["blocked_ua_cluster_conf"], manual, auto)

‎tests/test_ua_cluster_dedup.py‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
"""Regression test for the UA-cluster ↔ per-IP dedup fix.
2+
3+
An IP blocked by the per-IP pass must not also be written to the UA-cluster
4+
block file: both files are included in the same nginx geo block, and a CIDR
5+
present in two included files makes nginx log a "duplicate network" warning
6+
on every reload.
7+
8+
The UA-cluster pass must:
9+
1. skip cluster member IPs already owned by the per-IP pass, and
10+
2. drop stale entries already in its own file that the per-IP pass now owns
11+
(cleans duplicates accumulated before the fix).
12+
"""
13+
import textwrap
14+
15+
16+
def _make_cfg(tmp_path, ua_file, ips_file):
17+
return {
18+
"blocked_ua_cluster_conf": str(ua_file),
19+
"blocked_ips_conf": str(ips_file),
20+
"asn_db": str(tmp_path / "nonexistent-asn.tsv.gz"),
21+
"ua_cluster_threshold": 7,
22+
"ua_cluster_min_ips": 30,
23+
}
24+
25+
26+
def test_ua_cluster_skips_and_drops_per_ip_owned_ips(autoblock_mod, tmp_path, monkeypatch):
27+
ua_file = tmp_path / "blocked-ua-clusters.conf"
28+
ips_file = tmp_path / "blocked-ips.conf"
29+
30+
# per-IP pass already owns 1.1.1.1 and 9.9.9.9
31+
ips_file.write_text(textwrap.dedent(f"""\
32+
{autoblock_mod.AUTO_BEGIN_MARKER}
33+
1.1.1.1 1; # auto added=2026-05-19T00:00:00Z expires=2026-05-26T00:00:00Z reason=per-ip
34+
9.9.9.9 1; # auto added=2026-05-19T00:00:00Z expires=2026-05-26T00:00:00Z reason=per-ip
35+
{autoblock_mod.AUTO_END_MARKER}
36+
"""))
37+
38+
# UA-cluster file already has a STALE duplicate (1.1.1.1, now owned by
39+
# per-IP) plus a legitimate own entry (2.2.2.2)
40+
ua_file.write_text(textwrap.dedent(f"""\
41+
{autoblock_mod.AUTO_BEGIN_MARKER}
42+
1.1.1.1 1; # auto added=2026-05-18T00:00:00Z expires=2026-05-25T00:00:00Z reason=ua-cluster
43+
2.2.2.2 1; # auto added=2026-05-18T00:00:00Z expires=2026-05-25T00:00:00Z reason=ua-cluster
44+
{autoblock_mod.AUTO_END_MARKER}
45+
"""))
46+
47+
# One cluster whose members overlap per-IP (1.1.1.1, 9.9.9.9) and add new
48+
# IPs (2.2.2.2 extend, 3.3.3.3 new)
49+
cluster = {
50+
"ua": "Mozilla/5.0 Firefox/133.0", "score": 12, "ip_count": 200,
51+
"hosting_ratio": 0.95, "reasons": "noassets,noref,host95%",
52+
"ips": ["1.1.1.1", "9.9.9.9", "2.2.2.2", "3.3.3.3"],
53+
"added": "2026-05-19T10:00:00Z", "expires": "2026-05-26T10:00:00Z",
54+
}
55+
56+
monkeypatch.setattr(autoblock_mod, "find_blockable_ua_clusters",
57+
lambda *a, **k: [cluster])
58+
monkeypatch.setattr(autoblock_mod, "nginx_reload", lambda: True)
59+
60+
cfg = _make_cfg(tmp_path, ua_file, ips_file)
61+
autoblock_mod.cmd_ua_cluster(cfg, dry_run=False)
62+
63+
manual, auto = autoblock_mod.read_blocked(str(ua_file))
64+
cidrs = {e["cidr"] for e in auto}
65+
66+
# per-IP-owned IPs must NOT be in the UA-cluster file
67+
assert "1.1.1.1" not in cidrs, "stale per-IP-owned dup must be dropped"
68+
assert "9.9.9.9" not in cidrs, "per-IP-owned IP must be skipped, not added"
69+
# UA-cluster's own IPs stay
70+
assert "2.2.2.2" in cidrs, "UA-cluster's own IP must be kept"
71+
assert "3.3.3.3" in cidrs, "new non-overlapping cluster IP must be added"
72+
73+
74+
def test_ua_cluster_no_per_ip_file_is_safe(autoblock_mod, tmp_path, monkeypatch):
75+
"""When blocked_ips_conf does not exist, dedup is a no-op — all cluster
76+
IPs are written normally."""
77+
ua_file = tmp_path / "blocked-ua-clusters.conf"
78+
ips_file = tmp_path / "does-not-exist.conf"
79+
80+
cluster = {
81+
"ua": "Mozilla/5.0 Firefox/133.0", "score": 12, "ip_count": 200,
82+
"hosting_ratio": 0.95, "reasons": "noassets,noref",
83+
"ips": ["4.4.4.4", "5.5.5.5"],
84+
"added": "2026-05-19T10:00:00Z", "expires": "2026-05-26T10:00:00Z",
85+
}
86+
monkeypatch.setattr(autoblock_mod, "find_blockable_ua_clusters",
87+
lambda *a, **k: [cluster])
88+
monkeypatch.setattr(autoblock_mod, "nginx_reload", lambda: True)
89+
90+
cfg = _make_cfg(tmp_path, ua_file, ips_file)
91+
autoblock_mod.cmd_ua_cluster(cfg, dry_run=False)
92+
93+
_, auto = autoblock_mod.read_blocked(str(ua_file))
94+
cidrs = {e["cidr"] for e in auto}
95+
assert cidrs == {"4.4.4.4", "5.5.5.5"}
96+
97+
98+
def test_ua_cluster_same_ip_two_clusters_not_duplicated(autoblock_mod, tmp_path, monkeypatch):
99+
"""An IP appearing in two clusters in one run must be written once, not
100+
twice (the auto_by_cidr index must be updated as entries are appended)."""
101+
ua_file = tmp_path / "blocked-ua-clusters.conf"
102+
ips_file = tmp_path / "no-per-ip.conf"
103+
104+
clusters = [
105+
{"ua": "UA-A", "score": 10, "ip_count": 50, "hosting_ratio": 0.9,
106+
"reasons": "x", "ips": ["7.7.7.7"],
107+
"added": "2026-05-19T10:00:00Z", "expires": "2026-05-26T10:00:00Z"},
108+
{"ua": "UA-B", "score": 10, "ip_count": 50, "hosting_ratio": 0.9,
109+
"reasons": "x", "ips": ["7.7.7.7"]}, # same IP, second cluster
110+
]
111+
# second cluster reuses added/expires from first via .get fallback in code;
112+
# provide them to be safe
113+
clusters[1]["added"] = "2026-05-19T10:00:00Z"
114+
clusters[1]["expires"] = "2026-05-26T10:00:00Z"
115+
116+
monkeypatch.setattr(autoblock_mod, "find_blockable_ua_clusters",
117+
lambda *a, **k: clusters)
118+
monkeypatch.setattr(autoblock_mod, "nginx_reload", lambda: True)
119+
120+
cfg = _make_cfg(tmp_path, ua_file, ips_file)
121+
autoblock_mod.cmd_ua_cluster(cfg, dry_run=False)
122+
123+
_, auto = autoblock_mod.read_blocked(str(ua_file))
124+
cidrs = [e["cidr"] for e in auto]
125+
assert cidrs.count("7.7.7.7") == 1, "IP in two clusters must appear once"

0 commit comments

Comments
 (0)