aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--abusectl/cli.py55
-rw-r--r--abusectl/init.py67
-rw-r--r--docs/BACKLOG.md28
-rw-r--r--tests/test_init.py138
4 files changed, 282 insertions, 6 deletions
diff --git a/abusectl/cli.py b/abusectl/cli.py
index ed31a4b..36a12ec 100644
--- a/abusectl/cli.py
+++ b/abusectl/cli.py
@@ -192,6 +192,59 @@ def _ask_relays() -> list[str]:
return candidates
+def _ask_reporter_email() -> str:
+ """Ask for the reply address, re-asking on an answer that cannot be one.
+
+ Validated HERE rather than after the next question, the shape of defect
+ the first hand test found four of. Skippable: an empty answer returns
+ empty and the key is simply absent from the file.
+
+ ponytail: the ceiling is deliberately low. This is not an RFC 5322
+ validator and must not become one, because the failures those catch are
+ not the failures that happen: what a typo actually costs is a report
+ whose reply address bounces, and the answers that produce that are the
+ ones with no @ at all (a name pasted in), a spelled-out "at", or a
+ stray space from a copy-paste. Anything past that would start rejecting
+ addresses that work.
+ """
+ while True:
+ answer = _ask(
+ "\nYour email for abuse desks to reply to (Enter to skip): "
+ )
+ if not answer:
+ return ""
+ if answer.count("@") == 1 and all(part.strip() for part in answer.split("@")):
+ if not any(c.isspace() for c in answer):
+ return answer
+ print(" That does not look like an address abuse desks could reply to.")
+ print(" Expected something like you@example.org, or Enter to skip.")
+
+
+def _ask_reporter() -> dict:
+ """Ask for the identity that goes IN the reports, all three skippable."""
+ print("\nabusectl puts your identity in the reports it sends, so an abuse")
+ print("desk can tell who reported and reply to you. This is the one thing")
+ print("the tool discloses on purpose; everything from the message itself")
+ print("is redacted. Leave any of these blank to skip it.")
+
+ answers = {
+ "reporter_name": _ask("\nYour name (Enter to skip): "),
+ "reporter_org": _ask("Your organisation (Enter to skip): "),
+ "reporter_email": _ask_reporter_email(),
+ }
+
+ # Said here rather than discovered later. The address is the one field a
+ # report cannot be built without, since it becomes the From, and skipping
+ # it is a legitimate choice at setup: the file is hand-editable and this
+ # run may only be about the relays. What is not acceptable is finding out
+ # at `report` time with nothing explaining why.
+ if not answers["reporter_email"]:
+ print("\n No reply address set. `abusectl report` needs one, so add")
+ print(" email under [reporter] before your first report, or re-run init.")
+
+ return answers
+
+
def _prompt_answers(sample: Path | None) -> dict:
"""Ask the interactive questions and return an answers dict for init.build.
@@ -204,7 +257,7 @@ def _prompt_answers(sample: Path | None) -> dict:
trusted_relays = _ask_relays()
cases = _ask(f"\nCases directory [{config.DEFAULT_CASES}]: ")
- return {"trusted_relays": trusted_relays, "cases": cases}
+ return {"trusted_relays": trusted_relays, "cases": cases, **_ask_reporter()}
def _cmd_init(args) -> int:
diff --git a/abusectl/init.py b/abusectl/init.py
index d10f6ed..626dd41 100644
--- a/abusectl/init.py
+++ b/abusectl/init.py
@@ -158,8 +158,16 @@ def _quoted(value: str, field: str) -> str:
return f'"{value}"'
+# The answer key each [reporter] field is asked under.
+_REPORTER_FIELDS = (
+ ("name", "reporter_name"),
+ ("org", "reporter_org"),
+ ("email", "reporter_email"),
+)
+
+
def build(answers: dict) -> str:
- """Render answers as config TOML for the [general] table."""
+ """Render answers as config TOML for the [general] and [reporter] tables."""
relays = _validate_relays(answers.get("trusted_relays", []))
lines = [
@@ -180,6 +188,30 @@ def build(answers: dict) -> str:
if isinstance(cases, str) and cases.strip():
lines.append(f"cases = {_quoted(cases, 'cases')}")
+ # Written STRIPPED, matching config.load(), which strips on read. A file
+ # whose text differs from what every consumer sees is a seam worth not
+ # having, and the name becomes a From display name where surrounding
+ # whitespace would survive verbatim into a header.
+ reporter = []
+ for key, answer_key in _REPORTER_FIELDS:
+ value = answers.get(answer_key, "")
+ if isinstance(value, str) and value.strip():
+ reporter.append(f"{key} = {_quoted(value.strip(), key)}")
+
+ # Emitted only when something was answered. An empty [reporter] reads as
+ # configured-with-nothing, the same trap as api_key = "": absent reads as
+ # not-configured and the part that wants it can say so plainly.
+ if reporter:
+ lines.extend([
+ "",
+ "# The one identity abusectl discloses DELIBERATELY: it goes in the",
+ "# reports you send, so an abuse desk can reply to you. Everything",
+ "# else the tool touches is redacted; this is not.",
+ "",
+ "[reporter]",
+ ])
+ lines.extend(reporter)
+
lines.extend([
"",
"# Later parts of abusectl add further sections here:",
@@ -238,12 +270,34 @@ def back_up(path: pathlib.Path) -> pathlib.Path | None:
return backup_path
-def _preserved_sections(path: pathlib.Path) -> str:
- """Return the raw text of every top-level table except [general].
+def _rendered_sections(text: str) -> set:
+ """Return the names of the top-level tables actually present in text.
+
+ Read back off the rendered TOML rather than assumed from the answers:
+ build() emits [reporter] only when something was answered, and the
+ question preservation must answer is what this run WROTE, not what it
+ could have written.
+ """
+ names = set()
+ for line in text.splitlines():
+ stripped = line.strip()
+ if stripped.startswith("[") and stripped.endswith("]"):
+ names.add(stripped.lstrip("[").rstrip("]").strip())
+ return names
+
+
+def _preserved_sections(path: pathlib.Path, built: set) -> str:
+ """Return the raw text of every top-level table build() did not render.
Carrying raw lines rather than re-rendering means a comment, or a field
this build does not understand, also survives an init that only sets
the relays.
+
+ A table build() DID render is dropped instead: emitting it twice makes
+ the file unreadable, since tomllib refuses a duplicate table and the
+ whole config, [misp] key included, goes with it. `built` names those,
+ and it comes from the rendered text rather than a hand-kept list, so a
+ section added to the builder cannot be forgotten here.
"""
if not path.exists():
return ""
@@ -256,7 +310,7 @@ def _preserved_sections(path: pathlib.Path) -> str:
for line in lines:
stripped = line.strip()
if stripped.startswith("[") and stripped.endswith("]"):
- skipping = stripped.lstrip("[").rstrip("]").strip() == "general"
+ skipping = stripped.lstrip("[").rstrip("]").strip() in built
if skipping:
continue
if not skipping:
@@ -278,8 +332,11 @@ def write(path: pathlib.Path, answers: dict, force: bool = False) -> pathlib.Pat
if path.exists() and not force:
raise FileExistsError(f"{path} already exists; pass --force to overwrite")
- preserved = _preserved_sections(path)
text = build(answers)
+ # Only what this run actually rendered is dropped from the old file, so
+ # an init that skips every reporter question keeps the identity the user
+ # set by hand instead of silently deleting it.
+ preserved = _preserved_sections(path, _rendered_sections(text))
if preserved:
text = text.rstrip("\n") + "\n\n" + preserved + "\n"
diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md
index 780cbf5..63ea52c 100644
--- a/docs/BACKLOG.md
+++ b/docs/BACKLOG.md
@@ -134,3 +134,31 @@ to name the field differently, the fix is one string and one test.
a field an x-arf parser skips rather than acts on wrongly. Worth doing before
the first real report is filed, so a desk running x-arf tooling gets what it
expects.
+
+## 5. `report.build()` raises KeyError on an identity with no email
+
+**Observed.** `report.build(manifest, destination, identity)` reads
+`identity.get("name", "")` defensively but `identity["email"]` directly, so an
+identity carrying a name and no address raises `KeyError: 'email'` rather than
+saying what is missing. Reproduced through the public API against the real
+config reader: `config.load()` on a file whose `[reporter]` sets `name` and
+omits `email` returns `{"name": "A Reporter"}`, and that dict raises.
+
+**Cause.** Every key in `[reporter]` is independently optional, by the same
+skipped-answer-is-absent rule the rest of the config follows, but the report
+builder treats one of them as required without checking. Task 9 fixed the
+config half of this seam; the report half was not reachable from a config file
+until `init` grew the three prompts, and now it is: skipping the email
+question while answering the name produces exactly this shape.
+
+**Approach.** Refuse before building, not after: raise a named error saying no
+reply address is configured. Filling in an empty addr-spec instead would be
+worse, since a `From:` with no address produces a report that is sent and
+cannot be replied to, which defeats the reason the identity is disclosed at
+all. `init` now warns at the prompt when the address is skipped, so the
+remaining gap is a hand-edited config and the error message it deserves.
+
+**Constraints.** Belongs with the `report` subcommand rather than the builder
+alone, since where the check lives decides whether the user sees an exit code
+and a sentence or a traceback. Not a leak: the failure is loud and nothing is
+sent.
diff --git a/tests/test_init.py b/tests/test_init.py
index 619d6ba..46b1a26 100644
--- a/tests/test_init.py
+++ b/tests/test_init.py
@@ -59,6 +59,144 @@ class TestBuildConfig(unittest.TestCase):
"cases": 'x" \ntrusted_relays = ["0.0.0.0/0"]\n#'})
+class TestReporterSection(unittest.TestCase):
+ def test_the_identity_becomes_a_reporter_table(self):
+ text = init.build({
+ "trusted_relays": ["192.0.2.0/24"],
+ "reporter_name": "A Reporter",
+ "reporter_org": "Example Ltd",
+ "reporter_email": "abuse@example.org",
+ })
+ parsed = tomllib.loads(text)
+ self.assertEqual(parsed["reporter"], {
+ "name": "A Reporter",
+ "org": "Example Ltd",
+ "email": "abuse@example.org",
+ })
+
+ def test_a_fully_skipped_identity_emits_no_table_at_all(self):
+ # Not an empty [reporter]: an empty table reads as configured, and
+ # the reader would then report an identity of nothing rather than
+ # saying plainly that none is set.
+ text = init.build({
+ "trusted_relays": ["192.0.2.0/24"],
+ "reporter_name": "",
+ "reporter_org": "",
+ "reporter_email": "",
+ })
+ self.assertNotIn("[reporter]", text)
+ self.assertNotIn("reporter", tomllib.loads(text))
+
+ def test_a_skipped_answer_is_absent_not_empty(self):
+ # Same rule as the cases path: "" reads as configured-and-broken.
+ text = init.build({
+ "trusted_relays": ["192.0.2.0/24"],
+ "reporter_name": "A Reporter",
+ "reporter_org": "",
+ "reporter_email": " ",
+ })
+ reporter = tomllib.loads(text)["reporter"]
+ self.assertEqual(reporter, {"name": "A Reporter"})
+
+ def test_values_are_written_stripped(self):
+ # config.load() strips on read, so writing unstripped would make the
+ # file disagree with what every consumer sees.
+ text = init.build({
+ "trusted_relays": ["192.0.2.0/24"],
+ "reporter_name": " A Reporter ",
+ })
+ self.assertEqual(tomllib.loads(text)["reporter"]["name"], "A Reporter")
+
+ def test_a_quote_in_an_identity_cannot_break_out_of_the_toml(self):
+ with self.assertRaises(ValueError):
+ init.build({
+ "trusted_relays": ["192.0.2.0/24"],
+ "reporter_name": 'x"\nemail = "attacker@example.invalid"',
+ })
+
+ def test_the_identity_loads_back_through_config(self):
+ # The seam report.build() consumes: what init writes must arrive as
+ # the dict shape the reader hands over, keys and all.
+ with tempfile.TemporaryDirectory() as tmp:
+ path = pathlib.Path(tmp) / "config.toml"
+ path.write_text(init.build({
+ "trusted_relays": ["192.0.2.0/24"],
+ "reporter_name": "A Reporter",
+ "reporter_org": "Example Ltd",
+ "reporter_email": "abuse@example.org",
+ }), encoding="utf-8")
+ self.assertEqual(config.load(path).reporter, {
+ "name": "A Reporter",
+ "org": "Example Ltd",
+ "email": "abuse@example.org",
+ })
+
+ def test_a_partial_identity_loads_back_with_only_what_was_given(self):
+ with tempfile.TemporaryDirectory() as tmp:
+ path = pathlib.Path(tmp) / "config.toml"
+ path.write_text(init.build({
+ "trusted_relays": ["192.0.2.0/24"],
+ "reporter_email": "abuse@example.org",
+ }), encoding="utf-8")
+ self.assertEqual(config.load(path).reporter,
+ {"email": "abuse@example.org"})
+
+
+class TestReporterCarryAcross(unittest.TestCase):
+ def test_an_identity_set_by_hand_survives_a_relays_only_rewrite(self):
+ # AGENTS.md: sections build() does not produce are carried across
+ # verbatim. Skipping all three answers must not delete an identity
+ # the user set earlier; there would be no warning that it went.
+ with tempfile.TemporaryDirectory() as tmp:
+ path = pathlib.Path(tmp) / "config.toml"
+ path.write_text(
+ '[general]\ntrusted_relays = ["10.0.0.0/8"]\n\n'
+ '[reporter]\nname = "A Reporter"\nemail = "abuse@example.org"\n',
+ encoding="utf-8",
+ )
+ init.write(path, {"trusted_relays": ["192.0.2.0/24"]}, force=True)
+ self.assertEqual(config.load(path).reporter,
+ {"name": "A Reporter", "email": "abuse@example.org"})
+
+ def test_a_new_identity_replaces_the_old_one_without_duplicating_it(self):
+ # A section the builder DOES produce must not also be carried across:
+ # two [reporter] tables in one file is not merely untidy, tomllib
+ # refuses the whole file and the config becomes unreadable.
+ with tempfile.TemporaryDirectory() as tmp:
+ path = pathlib.Path(tmp) / "config.toml"
+ path.write_text(
+ '[general]\ntrusted_relays = ["10.0.0.0/8"]\n\n'
+ '[reporter]\nname = "Old Name"\n',
+ encoding="utf-8",
+ )
+ init.write(path, {
+ "trusted_relays": ["192.0.2.0/24"],
+ "reporter_name": "New Name",
+ }, force=True)
+
+ self.assertEqual(path.read_text().count("[reporter]"), 1)
+ self.assertEqual(config.load(path).reporter, {"name": "New Name"})
+
+ def test_an_unrelated_section_still_survives_alongside_an_identity(self):
+ with tempfile.TemporaryDirectory() as tmp:
+ path = pathlib.Path(tmp) / "config.toml"
+ path.write_text(
+ '[general]\ntrusted_relays = ["10.0.0.0/8"]\n\n'
+ '[reporter]\nname = "Old Name"\n\n'
+ '[misp]\napi_key = "kept"\n',
+ encoding="utf-8",
+ )
+ init.write(path, {
+ "trusted_relays": ["192.0.2.0/24"],
+ "reporter_name": "New Name",
+ }, force=True)
+
+ rewritten = path.read_text()
+ self.assertIn("kept", rewritten)
+ self.assertNotIn("Old Name", rewritten)
+ self.assertEqual(config.load(path).reporter, {"name": "New Name"})
+
+
class TestProviderTable(unittest.TestCase):
def test_a_known_provider_resolves_to_ranges(self):
self.assertTrue(init.provider_relays("gmail"))