diff options
| -rw-r--r-- | abusectl/config.py | 60 | ||||
| -rw-r--r-- | abusectl/report.py | 11 | ||||
| -rw-r--r-- | tests/test_config.py | 170 | ||||
| -rw-r--r-- | tests/test_report.py | 26 |
4 files changed, 260 insertions, 7 deletions
diff --git a/abusectl/config.py b/abusectl/config.py index 38f5254..26c45f1 100644 --- a/abusectl/config.py +++ b/abusectl/config.py @@ -18,8 +18,13 @@ A key that was skipped at setup is ABSENT from the file, never an empty string: api_key = "" reads as configured-and-broken and produces a confusing auth error much later, while an absent key reads as not-configured and the part that wants it can say so plainly. Later parts of this tool add MISP and -vendor API keys under their own tables in this same file; this module only -reads [general] and leaves other sections alone. +vendor API keys under their own tables in this same file; this module reads +[general] and [reporter] and leaves other sections alone. + +[reporter] holds the one identifier this tool discloses DELIBERATELY. The +distinction from every identifier it refuses to publish is provenance, so it +is enforced by provenance: the identity comes from config only and parse +never supplies it. Not-configured is a distinct condition, not a crash. This parser refuses to guess which mail relays are the user's own, because guessing wrong means @@ -32,7 +37,7 @@ import ipaddress import os import pathlib import tomllib -from dataclasses import dataclass +from dataclasses import dataclass, field class NotConfigured(Exception): @@ -57,10 +62,22 @@ def path() -> pathlib.Path: return pathlib.Path(base) / "abusectl" / "config.toml" +# The reporting identity, and nothing else, is carried out of [reporter]. +# Only these three keys are kept: they are what the spec names and what a +# report body consumes, so an unknown key cannot reach a document sent to a +# third party by way of a config the user hand-edited. +REPORTER_KEYS = ("name", "org", "email") + + @dataclass(frozen=True) class Config: trusted_relays: list[str] cases: pathlib.Path + # Empty when no identity is configured. A dict in a frozen dataclass is + # still mutable, and that is left alone deliberately: report.build() + # only reads it, and a defensive copy here would buy nothing while + # suggesting a guarantee this class does not make. + reporter: dict = field(default_factory=dict) def load(from_path: pathlib.Path | None = None) -> Config: @@ -101,4 +118,39 @@ def load(from_path: pathlib.Path | None = None) -> Config: raw_cases = general.get("cases", "") cases = pathlib.Path(raw_cases).expanduser() if raw_cases.strip() else DEFAULT_CASES - return Config(trusted_relays=trusted_relays, cases=cases) + raw_reporter = data.get("reporter", {}) + # reporter = "A Reporter" is valid TOML, and .items() on it raises + # AttributeError naming nothing the user can find in their file. Same + # shape of hand-edit as the unbracketed trusted_relays above. + if not isinstance(raw_reporter, dict): + raise ValueError( + f"reporter in {source} must be a table, got " + f"{type(raw_reporter).__name__}" + ) + + reporter = {} + for key in REPORTER_KEYS: + if key not in raw_reporter: + continue + value = raw_reporter[key] + # Rejected, not dropped. ipaddress.ip_network(42) returning a + # valid-looking network is the trap this file already learned from: + # a wrong value that reads as plausible is worse than an error. Here + # a dropped value reads as not-configured, and this is the ONE + # identifier the tool discloses deliberately, so a typo that + # silently removes the reply address from every report must be + # reported against the file that holds it. + if not isinstance(value, str): + raise ValueError( + f"reporter.{key} in {source} must be a string, got " + f"{type(value).__name__}: {value!r}" + ) + # A skipped answer is ABSENT, never an empty string: "" reads as + # configured-and-broken much later, absent reads as not-configured + # and the part that wants it can say so plainly. Stored stripped + # because the name becomes a From display name, where surrounding + # whitespace survives verbatim into the header. + if value.strip(): + reporter[key] = value.strip() + + return Config(trusted_relays=trusted_relays, cases=cases, reporter=reporter) diff --git a/abusectl/report.py b/abusectl/report.py index 20eb58a..6bf3c54 100644 --- a/abusectl/report.py +++ b/abusectl/report.py @@ -529,9 +529,16 @@ def text_part(manifest: dict, destination: dict, identity: dict) -> str: for key, value in sorted(auth.items())), " ") lines += ["", *_REDACTION_NOTE, ""] + # Each [reporter] key is individually skippable and config DROPS a + # skipped one rather than storing "", so a partial identity is the + # normal shape here. Subscripting raised KeyError on a case that had + # parsed perfectly, and joining unconditionally left a stray comma with + # nothing on one side of it. + who = ", ".join(str(identity[key]) for key in ("name", "org") + if identity.get(key)) lines += _wrap_value( - f"Reported by: {identity['name']}, {identity['org']} " - f"<{identity['email']}>", "") + f"Reported by: {who + ' ' if who else ''}" + f"<{identity.get('email', '')}>", "") lines.append("Generated by abusectl.") return "\n".join(lines) + "\n" diff --git a/tests/test_config.py b/tests/test_config.py index 6fa8619..1815993 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -20,7 +20,25 @@ import tempfile import unittest from unittest import mock -from abusectl import config +from abusectl import config, report + +# Minimal, kept local rather than imported from test_report: these tests are +# about the shape config produces, and they must not start failing because a +# report fixture grew a field. +_MANIFEST = { + "format": 1, + "case_id": "2026-09-07-aaaa", + "iocs": [ + {"id": "ioc-1", "type": "ipv4", "value": "203.0.113.42", + "origin": "received-chain", "confidence": "boundary-hop"}, + ], + "headers": [("From", '"Example Bank" <phish@sender.invalid>')], + "contacts": [ + {"iocs": ["ioc-1"], "query": "203.0.113.42", + "abuse": ["abuse@host.invalid"], "source": "rdap"}, + ], +} +_DESTINATION = {"id": "d1", "target": "abuse@host.invalid", "iocs": ["ioc-1"]} class TestConfig(unittest.TestCase): @@ -105,6 +123,156 @@ class TestConfig(unittest.TestCase): with self.assertRaises(ValueError): config.load(path) + def test_the_reporter_identity_is_read(self): + path = self._write( + "[general]\n" + 'trusted_relays = ["192.0.2.0/24"]\n' + "\n" + "[reporter]\n" + 'name = "A Reporter"\n' + 'org = "Example Consulting"\n' + 'email = "reporter@example.org"\n' + ) + loaded = config.load(path) + self.assertEqual(loaded.reporter["name"], "A Reporter") + self.assertEqual(loaded.reporter["org"], "Example Consulting") + self.assertEqual(loaded.reporter["email"], "reporter@example.org") + + def test_an_absent_reporter_section_is_an_empty_dict_not_a_crash(self): + path = self._write('[general]\ntrusted_relays = ["192.0.2.0/24"]\n') + self.assertEqual(config.load(path).reporter, {}) + + def test_an_empty_value_is_treated_as_absent(self): + # Same rule as cases and as every key init writes: skipped is ABSENT, + # never "". An empty org must not reach a report as a stray comma. + path = self._write( + "[general]\n" + 'trusted_relays = ["192.0.2.0/24"]\n' + "\n" + "[reporter]\n" + 'name = "A Reporter"\n' + 'org = ""\n' + 'email = "reporter@example.org"\n' + ) + reporter = config.load(path).reporter + self.assertNotIn("org", reporter) + self.assertEqual(reporter["name"], "A Reporter") + + def test_a_whitespace_only_value_is_treated_as_absent(self): + path = self._write( + "[general]\n" + 'trusted_relays = ["192.0.2.0/24"]\n' + "\n" + "[reporter]\n" + 'org = " "\n' + 'email = "reporter@example.org"\n' + ) + self.assertNotIn("org", config.load(path).reporter) + + def test_a_reporter_value_is_stored_stripped(self): + # The name becomes a From display name. Leading and trailing space + # survives into the header verbatim, which is sloppy at best and + # affects folding at worst. + path = self._write( + "[general]\n" + 'trusted_relays = ["192.0.2.0/24"]\n' + "\n" + "[reporter]\n" + 'name = " A Reporter "\n' + 'email = " reporter@example.org "\n' + ) + reporter = config.load(path).reporter + self.assertEqual(reporter["name"], "A Reporter") + self.assertEqual(reporter["email"], "reporter@example.org") + + def test_a_reporter_value_that_is_not_a_string_is_rejected_clearly(self): + # Not dropped. Dropping reads as not-configured, and this is the one + # identity the tool discloses deliberately: a typo that silently + # removes the reply address must be reported against the file that + # holds it, the same way a non-string trusted_relays entry is. + path = self._write( + "[general]\n" + 'trusted_relays = ["192.0.2.0/24"]\n' + "\n" + "[reporter]\n" + "name = 42\n" + ) + with self.assertRaises(ValueError) as caught: + config.load(path) + self.assertIn("must be a string", str(caught.exception)) + self.assertIn("name", str(caught.exception)) + + def test_a_non_string_email_is_rejected_rather_than_dropped(self): + path = self._write( + "[general]\n" + 'trusted_relays = ["192.0.2.0/24"]\n' + "\n" + "[reporter]\n" + 'email = ["reporter@example.org"]\n' + ) + with self.assertRaises(ValueError): + config.load(path) + + def test_a_string_reporter_section_is_rejected_clearly(self): + # reporter = "A Reporter" is valid TOML and would otherwise raise + # AttributeError naming nothing the user can find in their file. It + # has to precede [general]: a bare key written after a table header + # belongs to that table, not to the document. + path = self._write( + 'reporter = "me"\n\n[general]\ntrusted_relays = ["192.0.2.0/24"]\n' + ) + with self.assertRaises(ValueError) as caught: + config.load(path) + self.assertIn("must be a table", str(caught.exception)) + + def test_an_unknown_reporter_key_is_ignored(self): + # Ignored rather than refused: unlike a manifest format, an unknown + # key here loses nothing. Keeping only the three the spec names is + # what stops it reaching a report body. + path = self._write( + "[general]\n" + 'trusted_relays = ["192.0.2.0/24"]\n' + "\n" + "[reporter]\n" + 'email = "reporter@example.org"\n' + 'phone = "+1 555 0100"\n' + ) + reporter = config.load(path).reporter + self.assertNotIn("phone", reporter) + self.assertEqual(reporter["email"], "reporter@example.org") + + def test_a_configured_identity_builds_a_report_body(self): + # The round trip that only shows up much later otherwise: the dict + # config produces must be the shape report.build() consumes. + path = self._write( + "[general]\n" + 'trusted_relays = ["192.0.2.0/24"]\n' + "\n" + "[reporter]\n" + 'name = "A Reporter"\n' + 'org = "Example Consulting"\n' + 'email = "reporter@example.org"\n' + ) + body = report.build(_MANIFEST, _DESTINATION, config.load(path).reporter) + self.assertIn("A Reporter <reporter@example.org>", body) + self.assertIn( + "Reported by: A Reporter, Example Consulting " + "<reporter@example.org>", body) + + def test_an_identity_with_only_an_email_still_builds_a_body(self): + # Every key is individually skippable per the spec, so config drops + # the skipped ones and build() must survive their absence rather + # than raising KeyError on a case that parsed fine. + path = self._write( + "[general]\n" + 'trusted_relays = ["192.0.2.0/24"]\n' + "\n" + "[reporter]\n" + 'email = "reporter@example.org"\n' + ) + body = report.build(_MANIFEST, _DESTINATION, config.load(path).reporter) + self.assertIn("reporter@example.org", body) + def test_the_config_path_follows_xdg_config_home(self): with mock.patch.dict(os.environ, {"XDG_CONFIG_HOME": "/tmp/xdg-probe"}): self.assertEqual( diff --git a/tests/test_report.py b/tests/test_report.py index e87956f..9159dc7 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -640,6 +640,32 @@ class TextPart(unittest.TestCase): self.assertLessEqual(len(line), 72, line) self.assertIn(long_subject, report.unwrap(text)) + def test_an_identity_missing_a_name_or_org_still_builds(self): + """Each [reporter] key is individually skippable. + + config drops a skipped one rather than storing "", so a partial + identity is the normal shape here, not a malformed one. Subscripting + it raised KeyError on a case that had parsed perfectly. + """ + destination = report.email_destinations(MANIFEST["contacts"])[0] + + text = report.text_part(MANIFEST, destination, + {"email": "reporter@example.org"}) + self.assertIn("Reported by: <reporter@example.org>", text) + self.assertNotIn(",", report.unwrap(text).split("Reported by:")[1] + .splitlines()[0]) + + text = report.text_part(MANIFEST, destination, + {"name": "A Reporter", + "email": "reporter@example.org"}) + self.assertIn("Reported by: A Reporter <reporter@example.org>", text) + + text = report.text_part(MANIFEST, destination, + {"org": "Example Consulting", + "email": "reporter@example.org"}) + self.assertIn("Reported by: Example Consulting " + "<reporter@example.org>", text) + def test_a_long_reporter_identity_is_not_truncated(self): """The identity is the one thing disclosed deliberately. |