diff options
| author | Danilo M. <danix@danix.xyz> | 2026-09-09 19:15:44 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-09-09 19:15:44 +0200 |
| commit | d8161dae1fce1b5e5ffd757f411e1123b2f5dfa5 (patch) | |
| tree | e38578dfb59beeb300c577dcb8bf7837337e7afd /tests | |
| parent | 7275db9f81e9995318441807aa0a80022c84d55f (diff) | |
| download | abusectl-d8161dae1fce1b5e5ffd757f411e1123b2f5dfa5.tar.gz abusectl-d8161dae1fce1b5e5ffd757f411e1123b2f5dfa5.zip | |
fix: fold a role mailbox's case, and test the half that was not tested
"Abuse@Host.Invalid" and "abuse@host.invalid" produced two destinations and
two mails to one desk, which is the duplicate-mail behaviour the grouping
exists to prevent.
The root cause is the test, not the grouping. _group_key folded the domain
correctly, but the test written for it used a lowercase local part in both
of its inputs, so it varied only the half that worked and passed while the
other half was never exercised. A test that varies one half of its input
proves nothing about the other.
The local part now folds for the RFC 2142 role mailboxes. That RFC mandates
those names and requires them matched case-insensitively, so no host runs
"Abuse@" and "abuse@" as different desks and treating them as two costs a
duplicate mail with nothing on the other side of the trade. This reverses
the earlier decision only for the standardised names: any other local part
is still left as published, because for a named mailbox folding could merge
two desks a host genuinely distinguishes, and a dropped desk is worse than a
duplicate. Each half now folds on the strength of its own standard.
Both folds are mutation-checked and killed by distinct tests, so neither
masks the other, and the role fold is checked across all five role names.
Also pins two behaviours that held only by luck: the caller's contacts are
not modified, and a destination's ioc list is not aliased to the contact's.
Both are true today because the grouping starts a fresh list, and nothing
would have caught an implementation that reused the contact's.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xj1ayFRSUQ2u7cwb3S4axE
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/test_report.py | 103 |
1 files changed, 98 insertions, 5 deletions
diff --git a/tests/test_report.py b/tests/test_report.py index c72974e..5649b0b 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -1,3 +1,4 @@ +import copy import unittest from abusectl import report @@ -124,6 +125,23 @@ class Grouping(unittest.TestCase): self.assertEqual(destinations[0]["iocs"], ["ioc-1", "ioc-2"]) self.assertEqual(destinations[0]["target"], "abuse@Host.Invalid") + def test_the_domain_folds_under_a_local_part_that_does_not(self): + """Isolate the domain fold from the role fold. + + The version of this test that first shipped used a lowercase + local part throughout, so it exercised only the domain and passed + while a capitalised role name produced two destinations. + """ + contacts = [ + {"iocs": ["ioc-1"], "query": "198.51.100.7", + "abuse": ["J.Smith@Host.Invalid"], "source": "rdap"}, + {"iocs": ["ioc-2"], "query": "example.invalid", + "abuse": ["J.Smith@host.invalid"], "source": "rdap"}, + ] + destinations = report.email_destinations(contacts) + self.assertEqual(len(destinations), 1) + self.assertEqual(destinations[0]["iocs"], ["ioc-1", "ioc-2"]) + def test_two_spellings_of_one_desk_share_an_id(self): """The id derives from the same normalised form the grouping uses. @@ -134,11 +152,31 @@ class Grouping(unittest.TestCase): self.assertEqual(report.email_destination_id("abuse@Host.Invalid"), report.email_destination_id("abuse@host.invalid")) - def test_two_local_part_cases_stay_two_destinations(self): - """Only the receiving host knows whether its local parts fold. + def test_a_role_mailbox_folds_in_both_halves(self): + """"Abuse@Host.Invalid" and "abuse@host.invalid" are one desk. + + The case that first shipped folded the domain only, so a jCard + publishing the role name capitalised produced two destinations and + two mails to one desk. RFC 2142 mandates the role mailboxes and + requires them case-insensitive, so no host runs "Abuse@" and + "abuse@" as different desks. + """ + contacts = [ + {"iocs": ["ioc-1"], "query": "198.51.100.7", + "abuse": ["Abuse@Host.Invalid"], "source": "rdap"}, + {"iocs": ["ioc-2"], "query": "example.invalid", + "abuse": ["abuse@host.invalid"], "source": "rdap"}, + ] + destinations = report.email_destinations(contacts) + self.assertEqual(len(destinations), 1) + self.assertEqual(destinations[0]["iocs"], ["ioc-1", "ioc-2"]) + self.assertEqual(destinations[0]["target"], "Abuse@Host.Invalid") + + def test_one_contact_publishing_a_role_mailbox_twice_folds_it(self): + """The same fold applies within one contact's own abuse list. - Folding them here would silently drop a desk that a host genuinely - distinguishes; not folding them costs a duplicate mail at worst. + rdap.abuse_addresses dedupes case-sensitively, so a jCard with two + vcard rows spelling the role differently delivers both here. """ contacts = [ {"iocs": ["ioc-1"], "query": "198.51.100.7", @@ -146,8 +184,32 @@ class Grouping(unittest.TestCase): "source": "rdap"}, ] destinations = report.email_destinations(contacts) + self.assertEqual(len(destinations), 1) + self.assertEqual(destinations[0]["iocs"], ["ioc-1"]) + + def test_every_rfc2142_role_this_tool_can_meet_folds(self): + for role in ("abuse", "postmaster", "security", "noc", "hostmaster"): + with self.subTest(role=role): + self.assertEqual( + report.email_destination_id(f"{role.title()}@host.invalid"), + report.email_destination_id(f"{role}@host.invalid")) + + def test_a_personal_local_part_is_left_alone(self): + """Only the receiving host knows whether ITS local parts fold. + + A named mailbox is not a standardised role, so folding it could + silently merge two desks a host genuinely distinguishes and drop + one of them. Two mails to one desk is the lesser failure, and the + role names above are where the duplicate actually happens. + """ + contacts = [ + {"iocs": ["ioc-1"], "query": "198.51.100.7", + "abuse": ["J.Smith@host.invalid", "j.smith@host.invalid"], + "source": "rdap"}, + ] + destinations = report.email_destinations(contacts) self.assertEqual(sorted(d["target"] for d in destinations), - ["Abuse@host.invalid", "abuse@host.invalid"]) + ["J.Smith@host.invalid", "j.smith@host.invalid"]) self.assertNotEqual(destinations[0]["id"], destinations[1]["id"]) def test_a_destination_starts_with_no_body(self): @@ -157,6 +219,37 @@ class Grouping(unittest.TestCase): ] self.assertIsNone(report.email_destinations(contacts)[0]["body"]) + def test_the_contacts_passed_in_are_not_modified(self): + """The caller's contacts are the manifest's own array. + + case.py is the only writer of a manifest, so a grouping pass that + edited what it was handed would write through it from outside. + """ + contacts = [ + {"iocs": ["ioc-1"], "query": "198.51.100.7", + "abuse": ["abuse@host.invalid"], "source": "rdap"}, + ] + before = copy.deepcopy(contacts) + report.email_destinations(contacts) + self.assertEqual(contacts, before) + + def test_a_destinations_ioc_list_is_its_own(self): + """Not aliased to the contact's list it was built from. + + Holds today because the grouping starts a fresh list, but nothing + else pins it: an implementation that reused contact["iocs"] for a + single-contact destination would pass every other test here and + leave a destination and a contact sharing one list in a manifest + about to be written. + """ + contacts = [ + {"iocs": ["ioc-1"], "query": "198.51.100.7", + "abuse": ["abuse@host.invalid"], "source": "rdap"}, + ] + destination = report.email_destinations(contacts)[0] + destination["iocs"].append("ioc-2") + self.assertEqual(contacts[0]["iocs"], ["ioc-1"]) + def test_the_same_contacts_produce_the_same_ids_twice(self): """Ids must not depend on dict iteration luck or set ordering.""" contacts = [ |
