aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-09-10 10:28:07 +0200
committerDanilo M. <danix@danix.xyz>2026-09-10 10:28:07 +0200
commit5f9bbeb509773e5280995b39b44cf9be9c29ece2 (patch)
treeb5670ea4ef424921407ce1049d0ab2bc235561a9
parentbe392fe195f8f15dbe263b4c39d609bce95a8615 (diff)
downloadabusectl-5f9bbeb509773e5280995b39b44cf9be9c29ece2.tar.gz
abusectl-5f9bbeb509773e5280995b39b44cf9be9c29ece2.zip
feat: read the reporter identity from config
The reporter's identity is the one identifier this tool discloses deliberately, so it comes from config only and parse never supplies it. An empty value is absent, the same rule the rest of the config follows. Three departures from the plan, each a defect in its code: A non-string value is REJECTED, not dropped. The plan filtered on isinstance(value, str), so name = 42 or email = ["a@b"] silently vanished and read back as not-configured. This file already learned that lesson from ipaddress.ip_network(42) returning a valid-looking 0.0.0.42/32: a wrong value that reads as plausible is worse than an error. Dropping the email would strip the reply address from every report while the user believed they were identified, so the typo is reported against the file that holds it, the way a non-string trusted_relays entry already is. The value is stored STRIPPED. The plan tested value.strip() for truthiness but stored the original, so name = " A Reporter " reached the From display name as "From: A Reporter <...>", verbatim and unquoted. Only the three keys the spec names are carried across, and text_part no longer subscripts them. Each key is individually skippable and config drops a skipped one, so a partial identity is the normal shape, yet text_part read identity['name'] and identity['org'] directly: a config naming only an email raised KeyError on a case that had parsed perfectly. The "Reported by:" line is now joined from the parts present, so a missing org leaves no stray comma. A wholly unconfigured identity still raises in build(); how to refuse that belongs to the cli task, not here. The dataclass field defaults to an empty dict rather than editing every construction site, and the mutable-dict-in-a-frozen-dataclass is left as is: report only reads it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LByBnw83xr9YP85nskzkyE
-rw-r--r--abusectl/config.py60
-rw-r--r--abusectl/report.py11
-rw-r--r--tests/test_config.py170
-rw-r--r--tests/test_report.py26
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.