Fix missing-file error paths falling through in find_unknown_base_reverse_dns.py (#903)

Two `if not os.path.exists(path): print(f"Error: ...")` checks printed
the intended error message but had no sys.exit(1) after it, so execution
fell through into the subsequent open() call on the same missing path
and raised an unhandled FileNotFoundError instead of the clean error
exit the code clearly intended. Sibling duplicate-entry checks in the
same function already did print-then-sys.exit(1); these two now match.

Sites fixed (both in _main()):
- the nested load_list() helper's missing-file check (used for
  known_unknown_base_reverse_dns.txt and psl_overrides.txt)
- the base_reverse_dns_map.csv missing-file check

Both sites now have regression tests in a new TestFindUnknownBaseReverseDNS
class:
- the load_list() site: calling _main() in a temp directory with no
  known_unknown_base_reverse_dns.txt now raises SystemExit(1) instead of
  FileNotFoundError.
- the base_reverse_dns_map.csv site: _load_as_name_index() does its
  external work entirely through maxminddb.open_database(), the actual
  SDK boundary, so that call is mocked to a context manager over an
  empty iterable rather than loading the real ~23MB bundled MMDB or
  mocking an internal helper. Calling _main() in a temp directory with
  no base_reverse_dns_map.csv now raises SystemExit(1) instead of
  FileNotFoundError.

Both tests capture stdout and assert on the specific "Error: ... does
not exist" message, pinning the exit to the intended site rather than
any sys.exit(1) in the function. Cleanup uses two separate addCleanup
calls (rmtree registered before chdir, so LIFO order runs chdir first)
instead of one lambda wrapping both, so rmtree still runs even if
chdir were to raise.

Also fixed an adjacent prose bug a few lines from the second site: the
"is in known_unknown... and base_reverse_dns_map..." error message was
missing a space after "Error:" and was a backslash-continued f-string
that embedded the source's literal indentation in the printed output.
It now prints as a single clean line, consistent with the file's other
error messages.

Logged the user-facing symptom (clean error replaced by a
FileNotFoundError traceback) under CHANGELOG.md's Unreleased/Bug fixes
section, matching the project's precedent of logging maintainer-tooling
fixes (e.g. the sortlists.py entry).

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

CHANGELOG.md

@@ -7,6 +7,10 @@
 - **The prebuilt Docker image (`ghcr.io/domainaware/parsedmarc`) is roughly 40% smaller to pull** ([#893](https://github.com/domainaware/parsedmarc/pull/893)). The runtime stage copied the built wheel out of the build stage and deleted it again at the end of the next `RUN`, but a `RUN` can only write a whiteout over a layer an earlier instruction already committed: the wheel shipped in every published image and every `docker pull` downloaded it (10,713,473 bytes of the 11.0.0 image, on both architectures). The wheel is now bind-mounted from the build stage instead, and a bind mount is never committed to a layer. `pip install` also runs with `--no-cache-dir`, which drops a further ~99 MB of pip's download cache that the image had been carrying in the same layer as `site-packages`. Measured on linux/amd64: 272,138,724 compressed bytes across six layers before, 163,757,439 across five after.
 - **Bare `exit(...)` calls are now `sys.exit(...)` throughout the CLI and the maintainer map-tooling scripts.** `exit` is installed into `builtins` by the `site` module, not just for interactive sessions, so it isn't guaranteed to exist under `python -S` or an embedded interpreter that skips `site` — the failure paths that called it would raise `NameError` instead of actually exiting. `sys.exit` is always available and was already used elsewhere in `cli.py` and `find_unknown_base_reverse_dns.py`. Also removed the dead, immediately-recomputed `index_date` stores in the Elasticsearch and OpenSearch aggregate-report savers; behavior is unchanged.
 
+### Bug fixes
+
+- `find_unknown_base_reverse_dns.py`'s missing-file checks for `base_reverse_dns_map.csv` and the `known_unknown`/PSL-override lists printed a clean error message but fell through into an unhandled `FileNotFoundError` traceback instead of exiting.
+
 ## 11.0.1
 
 ### Security

parsedmarc/resources/maps/find_unknown_base_reverse_dns.py

@@ -168,6 +168,7 @@ def _main():
     def load_list(file_path, list_var):
         if not os.path.exists(file_path):
             print(f"Error: {file_path} does not exist")
+            sys.exit(1)
         print(f"Loading {file_path}")
         with open(file_path) as f:
             for line in f.readlines():
@@ -188,6 +189,7 @@ def _main():
     print(f"Indexed {len(as_name_index)} as_names from the MMDB")
     if not os.path.exists(base_reverse_dns_map_file_path):
         print(f"Error: {base_reverse_dns_map_file_path} does not exist")
+        sys.exit(1)
     print(f"Loading {base_reverse_dns_map_file_path}")
     with open(base_reverse_dns_map_file_path) as f:
         for row in csv.DictReader(f):
@@ -201,8 +203,8 @@ def _main():
                 known_domains.append(domain)
             if domain in known_unknown_domains and known_domains:
                 print(
-                    f"Error:{domain} is in {known_unknown_list_file_path} and \
-                        {base_reverse_dns_map_file_path}"
+                    f"Error: {domain} is in {known_unknown_list_file_path} "
+                    f"and {base_reverse_dns_map_file_path}"
                 )
                 sys.exit(1)
     if not os.path.exists(args.input):

tests/test_maps.py

@@ -4,7 +4,14 @@ These scripts are maintainer-only batch tooling — they do not ship in the
 wheel — but they still need regression coverage because they enforce the
 privacy and integrity rules for the reverse-DNS map data files."""
 
+import contextlib
+import io
+import os
+import shutil
+import sys
+import tempfile
 import unittest
+from unittest import mock
 
 
 class TestMapScriptsIPDetection(unittest.TestCase):
@@ -53,6 +60,116 @@ class TestMapScriptsIPDetection(unittest.TestCase):
         self.assertEqual(cdi._apply_psl_override("example.com", ov), "example.com")
 
 
+class TestFindUnknownBaseReverseDNS(unittest.TestCase):
+    """Missing-file error paths in find_unknown_base_reverse_dns.py's
+    ``_main()``.
+
+    Both sites below printed the intended ``"Error: ... does not exist"``
+    message but had no ``sys.exit(1)`` after it, so execution fell through
+    into the next ``open()`` call on the same missing path and raised an
+    unhandled ``FileNotFoundError`` instead of the clean, intended exit.
+    """
+
+    def test_missing_known_unknown_list_exits_cleanly(self):
+        """A missing known_unknown_base_reverse_dns.txt must print the
+        intended error message and exit(1), not fall through into an
+        unhandled FileNotFoundError from the subsequent open() call.
+
+        Regression test: the nested ``load_list()`` helper in ``_main()``
+        printed ``f"Error: {file_path} does not exist"`` but had no
+        ``sys.exit(1)`` after it, so execution fell through into
+        ``print(f"Loading {file_path}")`` and then ``open(file_path)`` on
+        the same missing path, raising an unhandled ``FileNotFoundError``
+        instead of the clean, intended error exit. The sibling
+        duplicate-entry check a few lines below (``domain in list_var``)
+        already did print-then-``sys.exit(1)``; this verifies the
+        missing-file check now matches that style. Capturing stdout and
+        asserting on the exact message (rather than only the exit code)
+        pins the exit to this specific site, not just any ``sys.exit(1)``
+        in the function.
+        """
+        import parsedmarc.resources.maps.find_unknown_base_reverse_dns as fu
+
+        old_cwd = os.getcwd()
+        tmp_dir = tempfile.mkdtemp()
+        # Register rmtree first so, under LIFO cleanup ordering, chdir back
+        # to old_cwd always runs before rmtree removes tmp_dir -- and, since
+        # unittest runs each addCleanup independently, rmtree still runs
+        # even if os.chdir were to raise.
+        self.addCleanup(shutil.rmtree, tmp_dir, ignore_errors=True)
+        self.addCleanup(os.chdir, old_cwd)
+        os.chdir(tmp_dir)
+
+        stdout = io.StringIO()
+        with mock.patch.object(sys, "argv", ["find_unknown_base_reverse_dns.py"]):
+            with contextlib.redirect_stdout(stdout):
+                with self.assertRaises(SystemExit) as cm:
+                    fu._main()
+        self.assertEqual(cm.exception.code, 1)
+        self.assertIn(
+            "Error: known_unknown_base_reverse_dns.txt does not exist",
+            stdout.getvalue(),
+        )
+
+    def test_missing_base_reverse_dns_map_exits_cleanly(self):
+        """A missing base_reverse_dns_map.csv must print the intended error
+        message and exit(1), not fall through into an unhandled
+        FileNotFoundError from the subsequent open() call.
+
+        Regression test for the second site fixed alongside the
+        ``load_list()`` one above: ``_main()`` printed
+        ``f"Error: {base_reverse_dns_map_file_path} does not exist"`` but
+        had no ``sys.exit(1)`` after it. Reaching this check requires
+        getting past the MMDB load first. ``_load_as_name_index`` does its
+        external work entirely through ``maxminddb.open_database()`` --
+        the actual SDK boundary per AGENTS.md's "mock at SDK boundaries"
+        rule -- so that call is mocked to a context manager over an empty
+        iterable instead of loading the real, ~23MB bundled MMDB or
+        mocking an internal helper of this codebase.
+        """
+        import parsedmarc.resources.maps.find_unknown_base_reverse_dns as fu
+
+        old_cwd = os.getcwd()
+        tmp_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, tmp_dir, ignore_errors=True)
+        self.addCleanup(os.chdir, old_cwd)
+
+        maps_dir = os.path.join(tmp_dir, "maps")
+        ipinfo_dir = os.path.join(tmp_dir, "ipinfo")
+        os.makedirs(maps_dir)
+        os.makedirs(ipinfo_dir)
+        # Both lists are loaded before the MMDB check and must exist, but
+        # their content isn't exercised by this test.
+        open(os.path.join(maps_dir, "known_unknown_base_reverse_dns.txt"), "w").close()
+        open(os.path.join(maps_dir, "psl_overrides.txt"), "w").close()
+        # A placeholder for the MMDB: only os.path.exists() touches this
+        # path directly, since maxminddb.open_database() itself is mocked
+        # below and never actually reads the file.
+        open(os.path.join(ipinfo_dir, "ipinfo_lite.mmdb"), "w").close()
+        # base_reverse_dns_map.csv is deliberately NOT created -- that's
+        # the missing-file condition under test.
+
+        os.chdir(maps_dir)
+
+        class _EmptyMMDBReader:
+            def __enter__(self):
+                return iter(())
+
+            def __exit__(self, *exc_info):
+                return False
+
+        stdout = io.StringIO()
+        with mock.patch.object(sys, "argv", ["find_unknown_base_reverse_dns.py"]):
+            with mock.patch("maxminddb.open_database", return_value=_EmptyMMDBReader()):
+                with contextlib.redirect_stdout(stdout):
+                    with self.assertRaises(SystemExit) as cm:
+                        fu._main()
+        self.assertEqual(cm.exception.code, 1)
+        self.assertIn(
+            "Error: base_reverse_dns_map.csv does not exist", stdout.getvalue()
+        )
+
+
 class TestDetectPSLOverrides(unittest.TestCase):
     """Cluster detection, brand-tail extraction, and full-pipeline behaviour
     for `detect_psl_overrides.py`."""