Add per-report-type mailbox delete options (#858)

* Add per-report-type mailbox delete options (#256)

Add four new [mailbox] options — delete_aggregate, delete_failure,
delete_smtp_tls, and delete_invalid — each defaulting to the value of
the overall delete option and individually overridable, so e.g.
delete = True with delete_failure = False deletes processed aggregate
and SMTP TLS report messages while archiving failure reports, and
delete_invalid = False keeps unparseable messages in the Invalid
archive subfolder for debugging.

get_dmarc_reports_from_mailbox() and watch_inbox() gained matching
bool | None keyword arguments (None = inherit from delete), resolved
once up front; the delete/test mutual-exclusion guard now checks the
effective per-type flags. The Gmail deletion-scope guard covers any
effective flag and forces all five options off when the scope is
missing. PARSEDMARC_MAILBOX_DELETE_* env vars work automatically.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Document per-type delete kwargs as bool | None with explicit inheritance

Copilot review: the docstrings typed the four per-report-type delete
parameters as plain bool, but None (the default) is the inheritance
mechanism — a library caller couldn't tell from the docs that None,
not False, means "inherit delete".

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Cover disposal error branches; scope the config= docstring claim

Codecov flagged the delete-error handler in the new per-type disposal
loop as the one uncovered patch line. Add two Maildir tests driving a
backend whose first delete/move call raises: the error is logged, the
affected message stays in the INBOX, and disposal continues to the
next report type. The move-error branch gets the symmetric test from
the same harness.

Copilot review: the config= docstring paragraph claimed all keyword
arguments listed above it are ignored when config= is provided, which
now falsely included the four per-type delete options. Scope the claim
to the parsing/enrichment arguments ParserConfig actually carries and
state that mailbox-handling arguments always apply.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Sean Whalen
2026-07-26 13:31:37 -04:00
committed by GitHub
co-authored by Claude Fable 5
parent 264b9a4556
commit ae5f4c7915
6 changed files with 725 additions and 66 deletions
+1
View File
@@ -7,6 +7,7 @@
- **`n_procs` parallel parsing now covers messages from mbox files and mailbox connections** ([#147](https://github.com/domainaware/parsedmarc/issues/147)), not just report files passed directly as CLI arguments: IMAP, Microsoft Graph, Gmail API, and Maildir connections, including watch mode. `get_dmarc_reports_from_mbox()`, `get_dmarc_reports_from_mailbox()`, and `watch_inbox()` all gained an `n_procs` keyword argument. Parallel workers are now a reused process pool for the whole run, with a bounded submission window that keeps at most roughly `2 * n_procs` messages in flight at a time so memory stays bounded even for huge mboxes; the main process sends periodic IMAP keepalives while workers parse.
- **Added `ParserConfig`, a single frozen dataclass carrying every parsing/enrichment option** ([#503](https://github.com/domainaware/parsedmarc/issues/503)): offline mode, IP database path, reverse DNS map and PSL overrides paths/URLs, DNS nameservers/timeout/retries, `strip_attachment_payloads`, `normalize_timespan_threshold_hours`, and the three caches the parser and enrichment code share across calls (IP address info, seen aggregate report IDs, and the reverse DNS map). `parse_aggregate_report_xml()`, `parse_aggregate_report_file()`, `parse_failure_report()`, `parse_report_email()`, `parse_report_file()`, `get_dmarc_reports_from_mbox()`, `get_dmarc_reports_from_mailbox()`, and `watch_inbox()` all accept a keyword-only `config=` argument; when it's provided, the individual option keyword arguments are ignored in favor of the config's values, and every existing per-option keyword argument continues to work unchanged when `config=` is omitted. Every explicitly constructed `ParserConfig` owns fresh, isolated caches; omitting `config=` falls back to the existing process-wide shared default caches, unchanged and identity-preserved (`parsedmarc.IP_ADDRESS_CACHE`, `parsedmarc.SEEN_AGGREGATE_REPORT_IDS`, `parsedmarc.REVERSE_DNS_MAP`). Caches never cross multiprocessing worker boundaries: `ParserConfig.__getstate__` drops the three cache fields when a config is pickled for a worker, and each worker accumulates its own from that point on. `keep_alive` and `n_procs` remain separate keyword arguments — they control process/worker orchestration, not parsing or enrichment behavior, so they're deliberately not `ParserConfig` fields. See the new "Using parsedmarc as a library" section of the usage docs for an example.
- **Added a "Domain policy" dropdown filter to the Splunk aggregate DMARC dashboard** ([#854](https://github.com/domainaware/parsedmarc/issues/854)): filters every panel on the published DMARC policy (`published_policy.p`) or subdomain policy (`published_policy.sp`), making it easy to see which domains with a `none` policy are ready to move to `quarantine` or `reject`. It sits immediately before the "Message disposition" dropdown and offers the same choices (`any`/`none`/`quarantine`/`reject`, defaulting to `any`).
- **New `[mailbox]` options `delete_aggregate`, `delete_failure`, `delete_smtp_tls`, and `delete_invalid`** ([#256](https://github.com/domainaware/parsedmarc/issues/256)): control message deletion per report type instead of all-or-nothing. Each defaults to the value of the overall `delete` option, so existing configurations behave exactly as before; set one to override just that type — for example `delete = True` with `delete_failure = False` deletes processed aggregate and SMTP TLS report messages while archiving failure report messages. `delete_invalid` covers messages that could not be parsed, so they can be kept in the `Invalid` archive subfolder for debugging even when everything else is deleted. `get_dmarc_reports_from_mailbox()` and `watch_inbox()` gained matching `delete_aggregate`, `delete_failure`, `delete_smtp_tls`, and `delete_invalid` keyword arguments, where `None` (the default) means "inherit `delete`". The mutual exclusion between deletion and `test` mode is now evaluated against the effective per-type flags: any type that would be deleted still raises a `ValueError` alongside `test`, but `delete = True` with all four per-type options explicitly `False` is now valid with `test` enabled, since nothing would be deleted.
- **New `[general]` option `archive_directory`**: move successfully processed local report files into a dated archive tree (`<archive_directory>/<year>/<month>/<Aggregate|Failure|SMTP-TLS>/`); files that fail to parse as a report go to `<archive_directory>/Invalid/`, while files that fail for other reasons (e.g. transient I/O errors) are left in place so a later run can retry them; existing files are never overwritten (numeric suffix); files already under the archive directory are excluded from later runs. Applies only to report files passed directly as local path arguments ([#570](https://github.com/domainaware/parsedmarc/issues/570)).
### Bug fixes
+23
View File
@@ -216,6 +216,29 @@ The full set of configuration options are:
messages as they arrive or poll MS Graph for new messages
- `delete` - bool: Delete messages after processing them,
instead of archiving them
- `delete_aggregate` - bool: Delete aggregate report messages
after processing them, instead of archiving them
(Default: the value of `delete`)
- `delete_failure` - bool: Delete failure report messages
after processing them, instead of archiving them
(Default: the value of `delete`)
- `delete_smtp_tls` - bool: Delete SMTP TLS report messages
after processing them, instead of archiving them
(Default: the value of `delete`)
- `delete_invalid` - bool: Delete messages that could not be
parsed, instead of archiving them in the `Invalid`
subfolder, where they can be inspected for debugging
(Default: the value of `delete`)
:::{note}
Each of these four options overrides `delete` for one kind of
message only, and the other three keep inheriting `delete`. So
`delete = True` combined with `delete_failure = False` archives
failure report messages while deleting processed aggregate and
SMTP TLS report messages — and unparseable ones, unless
`delete_invalid = False` is set as well.
:::
- `test` - bool: Do not move or delete messages
- `batch_size` - int: Number of messages to read and process
before saving. Default `10`. Use `0` for no limit.
+127 -65
View File
@@ -2243,6 +2243,8 @@ def _dispose_invalid_message(
) -> None:
"""Delete or move an unparseable message, per ``delete``.
Callers pass their effective ``delete_invalid`` value as ``delete``.
Shared, unmodified, by the sequential and parallel branches of
``get_dmarc_reports_from_mailbox`` so both dispose of invalid messages
identically.
@@ -2427,6 +2429,10 @@ def get_dmarc_reports_from_mailbox(
reports_folder: str = "INBOX",
archive_folder: str = "Archive",
delete: bool = False,
delete_aggregate: bool | None = None,
delete_failure: bool | None = None,
delete_smtp_tls: bool | None = None,
delete_invalid: bool | None = None,
test: bool = False,
ip_db_path: str | None = None,
always_use_local_files: bool = False,
@@ -2452,7 +2458,23 @@ def get_dmarc_reports_from_mailbox(
connection: A Mailbox connection object
reports_folder (str): The folder where reports can be found
archive_folder (str): The folder to move processed mail to
delete (bool): Delete messages after processing them
delete (bool): Delete messages after processing them
delete_aggregate (bool | None): Delete aggregate report messages
after processing them, instead of moving them to the
``Aggregate`` archive subfolder; ``None`` (the default) inherits
the value of ``delete``
delete_failure (bool | None): Delete failure report messages after
processing them, instead of moving them to the ``Failure``
archive subfolder; ``None`` (the default) inherits the value of
``delete``
delete_smtp_tls (bool | None): Delete SMTP TLS report messages after
processing them, instead of moving them to the ``SMTP-TLS``
archive subfolder; ``None`` (the default) inherits the value of
``delete``
delete_invalid (bool | None): Delete unparseable messages, instead
of moving them to the ``Invalid`` archive subfolder where they
can be inspected for debugging; ``None`` (the default) inherits
the value of ``delete``
test (bool): Do not move or delete messages after processing them
ip_db_path (str): Path to a MMDB file from IPinfo, MaxMind, or DBIP
always_use_local_files (bool): Do not download files
@@ -2481,15 +2503,34 @@ def get_dmarc_reports_from_mailbox(
interleaved message-by-message as it is when ``n_procs`` is 1.
Not part of ``config``; always applies.
config (ParserConfig): a single object carrying all parsing and
enrichment options plus the caches; when provided, the
individual option keyword arguments listed above are ignored in
favor of the config's values.
enrichment options plus the caches; when provided, it replaces
the individual parsing and enrichment option keyword arguments
listed above (DNS, GeoIP, offline mode, attachment payload
stripping, timespan normalization), whose values are then
ignored. The remaining keyword arguments control mailbox
handling and orchestration rather than parsing (the folder
names, the ``delete`` options, ``test``, ``since``,
``batch_size``); they are not part of ``config`` and always
apply.
Returns:
dict: Lists of ``aggregate_reports``, ``failure_reports``, and ``smtp_tls_reports``
"""
if delete and test:
raise ValueError("delete and test options are mutually exclusive")
# Each per-report-type flag inherits the overall ``delete`` value when it
# is left unset (``None``). Resolve once, up front: every decision below
# reads only these four resolved flags. The raw ``delete`` parameter is
# still forwarded verbatim to the recursive self-call at the end of this
# function, where it is inert because all four resolved flags accompany
# it.
delete_aggregate = delete if delete_aggregate is None else delete_aggregate
delete_failure = delete if delete_failure is None else delete_failure
delete_smtp_tls = delete if delete_smtp_tls is None else delete_smtp_tls
delete_invalid = delete if delete_invalid is None else delete_invalid
if test and (
delete_aggregate or delete_failure or delete_smtp_tls or delete_invalid
):
raise ValueError("delete options and test are mutually exclusive")
if connection is None:
raise ValueError("Must supply a connection")
@@ -2646,7 +2687,10 @@ def get_dmarc_reports_from_mailbox(
if not test:
for invalid_message_id in invalid_msg_ids:
_dispose_invalid_message(
connection, invalid_message_id, delete, invalid_reports_folder
connection,
invalid_message_id,
delete_invalid,
invalid_reports_folder,
)
else:
for i in range(message_limit):
@@ -2678,73 +2722,57 @@ def get_dmarc_reports_from_mailbox(
logger.warning(error.__str__())
if not test:
_dispose_invalid_message(
connection, message_id, delete, invalid_reports_folder
connection, message_id, delete_invalid, invalid_reports_folder
)
if not test:
if delete:
processed_messages = (
aggregate_report_msg_uids + failure_report_msg_uids + smtp_tls_msg_uids
)
number_of_processed_msgs = len(processed_messages)
for i in range(number_of_processed_msgs):
msg_uid = processed_messages[i]
logger.debug(
f"Deleting message {i + 1} of {number_of_processed_msgs}: UID {msg_uid}"
)
try:
connection.delete_message(msg_uid)
except Exception as e:
message = "Error deleting message UID"
e = f"{message} {msg_uid}: {e}"
logger.error(f"Mailbox error: {e}")
else:
if len(aggregate_report_msg_uids) > 0:
log_message = "Moving aggregate report messages from"
logger.debug(
f"{log_message} {reports_folder} to {aggregate_reports_folder}"
)
number_of_agg_report_msgs = len(aggregate_report_msg_uids)
for i in range(number_of_agg_report_msgs):
msg_uid = aggregate_report_msg_uids[i]
# Each report type is disposed of according to its own effective
# delete flag: deleted outright, or moved to its archive subfolder.
for msg_uids, delete_type, destination_folder, label in (
(
aggregate_report_msg_uids,
delete_aggregate,
aggregate_reports_folder,
"aggregate report",
),
(
failure_report_msg_uids,
delete_failure,
failure_reports_folder,
"failure report",
),
(
smtp_tls_msg_uids,
delete_smtp_tls,
smtp_tls_reports_folder,
"SMTP TLS report",
),
):
number_of_msgs = len(msg_uids)
if number_of_msgs == 0:
continue
if not delete_type:
message = f"Moving {label} messages from"
logger.debug(f"{message} {reports_folder} to {destination_folder}")
for i in range(number_of_msgs):
msg_uid = msg_uids[i]
if delete_type:
logger.debug(
f"Moving message {i + 1} of {number_of_agg_report_msgs}: UID {msg_uid}"
f"Deleting message {i + 1} of {number_of_msgs}: UID {msg_uid}"
)
try:
connection.move_message(msg_uid, aggregate_reports_folder)
connection.delete_message(msg_uid)
except Exception as e:
message = "Error moving message UID"
message = "Error deleting message UID"
e = f"{message} {msg_uid}: {e}"
logger.error(f"Mailbox error: {e}")
if len(failure_report_msg_uids) > 0:
message = "Moving failure report messages from"
logger.debug(f"{message} {reports_folder} to {failure_reports_folder}")
number_of_failure_msgs = len(failure_report_msg_uids)
for i in range(number_of_failure_msgs):
msg_uid = failure_report_msg_uids[i]
else:
message = "Moving message"
logger.debug(
f"{message} {i + 1} of {number_of_failure_msgs}: UID {msg_uid}"
f"{message} {i + 1} of {number_of_msgs}: UID {msg_uid}"
)
try:
connection.move_message(msg_uid, failure_reports_folder)
except Exception as e:
e = f"Error moving message UID {msg_uid}: {e}"
logger.error(f"Mailbox error: {e}")
if len(smtp_tls_msg_uids) > 0:
message = "Moving SMTP TLS report messages from"
logger.debug(f"{message} {reports_folder} to {smtp_tls_reports_folder}")
number_of_smtp_tls_uids = len(smtp_tls_msg_uids)
for i in range(number_of_smtp_tls_uids):
msg_uid = smtp_tls_msg_uids[i]
message = "Moving message"
logger.debug(
f"{message} {i + 1} of {number_of_smtp_tls_uids}: UID {msg_uid}"
)
try:
connection.move_message(msg_uid, smtp_tls_reports_folder)
connection.move_message(msg_uid, destination_folder)
except Exception as e:
e = f"Error moving message UID {msg_uid}: {e}"
logger.error(f"Mailbox error: {e}")
@@ -2771,6 +2799,10 @@ def get_dmarc_reports_from_mailbox(
reports_folder=reports_folder,
archive_folder=archive_folder,
delete=delete,
delete_aggregate=delete_aggregate,
delete_failure=delete_failure,
delete_smtp_tls=delete_smtp_tls,
delete_invalid=delete_invalid,
test=test,
results=results,
since=current_time,
@@ -2788,6 +2820,10 @@ def watch_inbox(
reports_folder: str = "INBOX",
archive_folder: str = "Archive",
delete: bool = False,
delete_aggregate: bool | None = None,
delete_failure: bool | None = None,
delete_smtp_tls: bool | None = None,
delete_invalid: bool | None = None,
test: bool = False,
check_timeout: int = 30,
ip_db_path: str | None = None,
@@ -2815,7 +2851,23 @@ def watch_inbox(
callback: The callback function to receive the parsing results
reports_folder (str): The IMAP folder where reports can be found
archive_folder (str): The folder to move processed mail to
delete (bool): Delete messages after processing them
delete (bool): Delete messages after processing them
delete_aggregate (bool | None): Delete aggregate report messages
after processing them, instead of moving them to the
``Aggregate`` archive subfolder; ``None`` (the default) inherits
the value of ``delete``
delete_failure (bool | None): Delete failure report messages after
processing them, instead of moving them to the ``Failure``
archive subfolder; ``None`` (the default) inherits the value of
``delete``
delete_smtp_tls (bool | None): Delete SMTP TLS report messages after
processing them, instead of moving them to the ``SMTP-TLS``
archive subfolder; ``None`` (the default) inherits the value of
``delete``
delete_invalid (bool | None): Delete unparseable messages, instead
of moving them to the ``Invalid`` archive subfolder where they
can be inspected for debugging; ``None`` (the default) inherits
the value of ``delete``
test (bool): Do not move or delete messages after processing them
check_timeout (int): Number of seconds to wait for a IMAP IDLE response
or the number of seconds until the next mail check
@@ -2842,9 +2894,15 @@ def watch_inbox(
parallel. Passed through to ``get_dmarc_reports_from_mailbox``
on each check. Not part of ``config``; always applies.
config (ParserConfig): a single object carrying all parsing and
enrichment options plus the caches; when provided, the
individual option keyword arguments listed above are ignored in
favor of the config's values.
enrichment options plus the caches; when provided, it replaces
the individual parsing and enrichment option keyword arguments
listed above (DNS, GeoIP, offline mode, attachment payload
stripping, timespan normalization), whose values are then
ignored. The remaining keyword arguments control mailbox
handling and orchestration rather than parsing (the folder
names, the ``delete`` options, ``test``, ``since``,
``batch_size``); they are not part of ``config`` and always
apply.
"""
cfg = _resolve_config(
config,
@@ -2866,6 +2924,10 @@ def watch_inbox(
reports_folder=reports_folder,
archive_folder=archive_folder,
delete=delete,
delete_aggregate=delete_aggregate,
delete_failure=delete_failure,
delete_smtp_tls=delete_smtp_tls,
delete_invalid=delete_invalid,
test=test,
batch_size=batch_size,
since=since,
+48 -1
View File
@@ -783,6 +783,22 @@ def _parse_config(config: ConfigParser, opts):
opts.mailbox_watch = bool(mailbox_config.getboolean("watch"))
if "delete" in mailbox_config:
opts.mailbox_delete = bool(mailbox_config.getboolean("delete"))
if "delete_aggregate" in mailbox_config:
opts.mailbox_delete_aggregate = bool(
mailbox_config.getboolean("delete_aggregate")
)
if "delete_failure" in mailbox_config:
opts.mailbox_delete_failure = bool(
mailbox_config.getboolean("delete_failure")
)
if "delete_smtp_tls" in mailbox_config:
opts.mailbox_delete_smtp_tls = bool(
mailbox_config.getboolean("delete_smtp_tls")
)
if "delete_invalid" in mailbox_config:
opts.mailbox_delete_invalid = bool(
mailbox_config.getboolean("delete_invalid")
)
if "test" in mailbox_config:
opts.mailbox_test = bool(mailbox_config.getboolean("test"))
if "batch_size" in mailbox_config:
@@ -2217,6 +2233,13 @@ def _main():
mailbox_archive_folder="Archive",
mailbox_watch=False,
mailbox_delete=False,
# None means "unset": each per-report-type flag inherits mailbox_delete
# in get_dmarc_reports_from_mailbox, so an explicit False (opting one
# type out of a global delete = true) stays distinct from being unset.
mailbox_delete_aggregate=None,
mailbox_delete_failure=None,
mailbox_delete_smtp_tls=None,
mailbox_delete_invalid=None,
mailbox_test=False,
mailbox_batch_size=10,
mailbox_check_timeout=30,
@@ -2686,7 +2709,21 @@ def _main():
exit(1)
if opts.gmail_api_credentials_file:
if opts.mailbox_delete:
# Any effective delete flag needs the deletion scope: the per-report-type
# flags inherit mailbox_delete when unset (None), so this reduces to
# mailbox_delete alone when none of them is set. The scope is a
# mailbox-wide capability grant rather than a per-type one, so when it
# is missing every flag is turned off explicitly.
per_type_delete_opts = (
"mailbox_delete_aggregate",
"mailbox_delete_failure",
"mailbox_delete_smtp_tls",
"mailbox_delete_invalid",
)
if any(
opts.mailbox_delete if getattr(opts, name) is None else getattr(opts, name)
for name in per_type_delete_opts
):
if "https://mail.google.com/" not in opts.gmail_api_scopes:
logger.error(
"Message deletion requires scope"
@@ -2695,6 +2732,8 @@ def _main():
"to acquire proper access."
)
opts.mailbox_delete = False
for name in per_type_delete_opts:
setattr(opts, name, False)
try:
mailbox_connection = GmailConnection(
@@ -2737,6 +2776,10 @@ def _main():
reports = get_dmarc_reports_from_mailbox(
connection=mailbox_connection,
delete=opts.mailbox_delete,
delete_aggregate=opts.mailbox_delete_aggregate,
delete_failure=opts.mailbox_delete_failure,
delete_smtp_tls=opts.mailbox_delete_smtp_tls,
delete_invalid=opts.mailbox_delete_invalid,
batch_size=mailbox_batch_size_value,
reports_folder=opts.mailbox_reports_folder,
archive_folder=opts.mailbox_archive_folder,
@@ -2859,6 +2902,10 @@ def _main():
reports_folder=opts.mailbox_reports_folder,
archive_folder=opts.mailbox_archive_folder,
delete=opts.mailbox_delete,
delete_aggregate=opts.mailbox_delete_aggregate,
delete_failure=opts.mailbox_delete_failure,
delete_smtp_tls=opts.mailbox_delete_smtp_tls,
delete_invalid=opts.mailbox_delete_invalid,
test=opts.mailbox_test,
check_timeout=mailbox_check_timeout_value,
batch_size=mailbox_batch_size_value,
+202
View File
@@ -1552,6 +1552,146 @@ since = 2d
self.assertEqual(mock_watch_inbox.call_args.kwargs.get("since"), "2d")
class TestMailboxPerTypeDeleteOptions(unittest.TestCase):
"""The [mailbox] per-report-type delete options (issue #256) must reach
the library unchanged, including the difference between an explicit
``false`` and an unset option (``None``, meaning "inherit ``delete``")."""
def setUp(self):
from parsedmarc.log import logger as _logger
_logger.disabled = True
self._stdout_patch = patch("sys.stdout", new_callable=io.StringIO)
self._stderr_patch = patch("sys.stderr", new_callable=io.StringIO)
self._stdout_patch.start()
self._stderr_patch.start()
def tearDown(self):
from parsedmarc.log import logger as _logger
_logger.disabled = False
self._stderr_patch.stop()
self._stdout_patch.stop()
def _write_config(self, config_text):
with tempfile.NamedTemporaryFile("w", suffix=".ini", delete=False) as cfg:
cfg.write(config_text)
cfg_path = cfg.name
self.addCleanup(lambda: os.path.exists(cfg_path) and os.remove(cfg_path))
return cfg_path
IMAP_CONFIG = """[general]
silent = true
[imap]
host = imap.example.com
user = user
password = pass
[mailbox]
delete = true
delete_failure = false
"""
def _assert_per_type_delete_kwargs(self, kwargs):
self.assertIs(kwargs.get("delete"), True)
self.assertIs(kwargs.get("delete_failure"), False)
# Unset options stay None so the library applies the inheritance.
self.assertIsNone(kwargs.get("delete_aggregate"))
self.assertIsNone(kwargs.get("delete_smtp_tls"))
self.assertIsNone(kwargs.get("delete_invalid"))
@patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
@patch("parsedmarc.cli.watch_inbox")
@patch("parsedmarc.cli.IMAPConnection")
def testCliPassesPerTypeDeleteToWatchInbox(
self, mock_imap_connection, mock_watch_inbox, mock_get_mailbox_reports
):
mock_imap_connection.return_value = object()
mock_get_mailbox_reports.return_value = {
"aggregate_reports": [],
"failure_reports": [],
"smtp_tls_reports": [],
}
mock_watch_inbox.side_effect = FileExistsError("stop-watch-loop")
cfg_path = self._write_config(self.IMAP_CONFIG + "watch = true\n")
with patch.object(sys, "argv", ["parsedmarc", "-c", cfg_path]):
with self.assertRaises(SystemExit) as system_exit:
parsedmarc.cli._main()
self.assertEqual(system_exit.exception.code, 1)
self._assert_per_type_delete_kwargs(mock_watch_inbox.call_args.kwargs)
@patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
@patch("parsedmarc.cli.IMAPConnection")
def testCliPassesPerTypeDeleteToOneShotMailboxRun(
self, mock_imap_connection, mock_get_mailbox_reports
):
mock_imap_connection.return_value = object()
mock_get_mailbox_reports.return_value = {
"aggregate_reports": [],
"failure_reports": [],
"smtp_tls_reports": [],
}
cfg_path = self._write_config(self.IMAP_CONFIG)
with patch.object(sys, "argv", ["parsedmarc", "-c", cfg_path]):
parsedmarc.cli._main()
self._assert_per_type_delete_kwargs(mock_get_mailbox_reports.call_args.kwargs)
@patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
@patch("parsedmarc.cli.GmailConnection")
def testGmailScopeGuardForcesAllDeleteFlagsOff(
self, mock_gmail_connection, mock_get_mailbox_reports
):
"""The Gmail deletion scope is a mailbox-wide capability grant, so a
per-report-type delete option alone (with ``delete`` unset) is enough
to trip the guard, and a missing scope turns every delete flag off."""
mock_gmail_connection.return_value = MagicMock()
mock_get_mailbox_reports.return_value = {
"aggregate_reports": [],
"failure_reports": [],
"smtp_tls_reports": [],
}
cfg_path = self._write_config("""[general]
silent = true
[gmail_api]
credentials_file = /tmp/gmail-credentials.json
scopes = https://www.googleapis.com/auth/gmail.modify
[mailbox]
delete_aggregate = true
""")
from parsedmarc.log import logger as _logger
_logger.disabled = False
try:
with patch.object(sys, "argv", ["parsedmarc", "-c", cfg_path]):
with self.assertLogs("parsedmarc.log", level="ERROR") as logs:
parsedmarc.cli._main()
finally:
_logger.disabled = True
self.assertTrue(
any("Message deletion requires scope" in line for line in logs.output),
logs.output,
)
kwargs = mock_get_mailbox_reports.call_args.kwargs
for option in (
"delete",
"delete_aggregate",
"delete_failure",
"delete_smtp_tls",
"delete_invalid",
):
with self.subTest(option=option):
self.assertIs(kwargs.get(option), False)
class TestCliParserConfigWiring(unittest.TestCase):
"""Tests that _main() builds a single ParserConfig (via
_build_parser_config) from parsed opts and passes it as ``config=`` to
@@ -4484,6 +4624,68 @@ def _config_with(section: str, settings: dict) -> "ConfigParser":
return cp
class TestParseConfigMailbox(unittest.TestCase):
"""The [mailbox] section, including the per-report-type delete options
(issue #256). An option that is absent from the INI must be left alone
entirely: _main defaults the four per-type options to None so the library
inherits the overall ``delete`` value, and writing False for an absent key
would silently disable that inheritance."""
PER_TYPE_DELETE_OPTS = (
"mailbox_delete_aggregate",
"mailbox_delete_failure",
"mailbox_delete_smtp_tls",
"mailbox_delete_invalid",
)
def test_mailbox_full_section(self):
from parsedmarc.cli import _parse_config
cp = _config_with(
"mailbox",
{
"reports_folder": "Reports",
"archive_folder": "Processed",
"watch": "true",
"delete": "true",
"delete_aggregate": "true",
"delete_failure": "false",
"delete_smtp_tls": "true",
"delete_invalid": "false",
"test": "false",
"batch_size": "25",
"check_timeout": "60",
"since": "3d",
},
)
opts = _opts()
_parse_config(cp, opts)
self.assertEqual(opts.mailbox_reports_folder, "Reports")
self.assertEqual(opts.mailbox_archive_folder, "Processed")
self.assertIs(opts.mailbox_watch, True)
self.assertIs(opts.mailbox_delete, True)
self.assertIs(opts.mailbox_delete_aggregate, True)
# Explicitly false, not None: this type opts out of delete = true.
self.assertIs(opts.mailbox_delete_failure, False)
self.assertIs(opts.mailbox_delete_smtp_tls, True)
self.assertIs(opts.mailbox_delete_invalid, False)
self.assertIs(opts.mailbox_test, False)
self.assertEqual(opts.mailbox_batch_size, 25)
self.assertEqual(opts.mailbox_check_timeout, 60)
self.assertEqual(opts.mailbox_since, "3d")
def test_mailbox_absent_per_type_delete_keys_are_not_set(self):
from parsedmarc.cli import _parse_config
cp = _config_with("mailbox", {"delete": "true"})
opts = _opts()
_parse_config(cp, opts)
self.assertIs(opts.mailbox_delete, True)
for option in self.PER_TYPE_DELETE_OPTS:
with self.subTest(option=option):
self.assertFalse(hasattr(opts, option))
class TestParseConfigGeneral(unittest.TestCase):
"""The [general] section sets dozens of flags. Hit a representative
subset: filenames, save-toggles, DNS settings, output dir."""
+324
View File
@@ -13,6 +13,7 @@ import logging
import mailbox
import os
import unittest
from collections.abc import Callable
from datetime import datetime, timedelta, timezone
from glob import glob
from io import BytesIO
@@ -2963,6 +2964,38 @@ class TestGetDmarcReportsFromMailboxValidation(unittest.TestCase):
)
self.assertIn("mutually exclusive", str(ctx.exception))
def test_inherited_delete_with_test_raises(self):
"""The guard checks the effective per-type flags, so delete=True still
raises alongside test=True when only *one* per-type flag opts out --
the remaining three inherit the deletion."""
with self.assertRaises(ValueError) as ctx:
parsedmarc.get_dmarc_reports_from_mailbox(
connection=MagicMock(), delete=True, delete_aggregate=False, test=True
)
self.assertIn("mutually exclusive", str(ctx.exception))
def test_explicit_per_type_delete_with_test_raises(self):
"""Each per-type delete flag on its own is enough to conflict with
test=True, with the overall delete option left False and the other
three flags explicitly False."""
for flag in (
"delete_aggregate",
"delete_failure",
"delete_smtp_tls",
"delete_invalid",
):
with self.subTest(flag=flag):
with self.assertRaises(ValueError) as ctx:
parsedmarc.get_dmarc_reports_from_mailbox(
connection=MagicMock(),
test=True,
delete_aggregate=flag == "delete_aggregate",
delete_failure=flag == "delete_failure",
delete_smtp_tls=flag == "delete_smtp_tls",
delete_invalid=flag == "delete_invalid",
)
self.assertIn("mutually exclusive", str(ctx.exception))
def test_none_connection_raises(self):
with self.assertRaises(ValueError) as ctx:
parsedmarc.get_dmarc_reports_from_mailbox(
@@ -3071,6 +3104,34 @@ class TestMigrateForensicArchiveFolderMaildir(unittest.TestCase):
self.assertEqual(result["failure_reports"], [])
class _FailingDisposalMaildirConnection(MaildirConnection):
"""A MaildirConnection whose first disposal call fails.
Real mailbox backends can reject an individual delete or move (permission
denied, a UID that vanished, a dropped connection) while the rest of the
batch is still fine. Failing only the *first* call, then behaving
normally, is what makes that observable: the message the failure hit
stays put, and every message after it must still be disposed of.
"""
def __init__(self, *args, fail_delete=False, fail_move=False, **kwargs):
super().__init__(*args, **kwargs)
self._fail_delete = fail_delete
self._fail_move = fail_move
def delete_message(self, message_id):
if self._fail_delete:
self._fail_delete = False
raise RuntimeError("server said no")
super().delete_message(message_id)
def move_message(self, message_id, folder_name):
if self._fail_move:
self._fail_move = False
raise RuntimeError("server said no")
super().move_message(message_id, folder_name)
class TestGetDmarcReportsFromMailboxMaildir(unittest.TestCase):
"""parsedmarc's real mailbox processing loop, end to end on an on-disk
Maildir (mailsuite MaildirConnection, no mocks, offline parsing): fetch
@@ -3184,6 +3245,113 @@ class TestGetDmarcReportsFromMailboxMaildir(unittest.TestCase):
self.assertEqual(conn.fetch_messages("Archive/Failure"), [])
self.assertEqual(conn.fetch_messages("Archive/Aggregate"), [])
def test_delete_aggregate_overrides_delete_false(self):
"""delete_aggregate=True with delete left at its False default: only
the aggregate report message is deleted; the other two report types
(and the unparseable message) are still archived."""
self._deliver(self.AGGREGATE)
self._deliver(self.FAILURE)
self._deliver(self.SMTP_TLS)
self._deliver(self.JUNK)
conn, result = self._run(delete_aggregate=True)
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(conn.fetch_messages("INBOX"), [])
self.assertEqual(conn.fetch_messages("Archive/Aggregate"), [])
self.assertEqual(len(conn.fetch_messages("Archive/Failure")), 1)
self.assertEqual(len(conn.fetch_messages("Archive/SMTP-TLS")), 1)
self.assertEqual(len(conn.fetch_messages("Archive/Invalid")), 1)
def test_delete_failure_false_overrides_delete_true(self):
"""delete=True with delete_failure=False: failure report messages are
kept (archived) while every other message -- aggregate, SMTP TLS, and
the unparseable one, all inheriting delete=True -- is deleted. This is
the motivating case from issue #256."""
self._deliver(self.AGGREGATE)
self._deliver(self.FAILURE)
self._deliver(self.SMTP_TLS)
self._deliver(self.JUNK)
conn, result = self._run(delete=True, delete_failure=False)
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(len(result["failure_reports"]), 1)
self.assertEqual(len(result["smtp_tls_reports"]), 1)
self.assertEqual(conn.fetch_messages("INBOX"), [])
self.assertEqual(len(conn.fetch_messages("Archive/Failure")), 1)
self.assertEqual(conn.fetch_messages("Archive/Aggregate"), [])
self.assertEqual(conn.fetch_messages("Archive/SMTP-TLS"), [])
self.assertEqual(conn.fetch_messages("Archive/Invalid"), [])
def test_delete_invalid_true_deletes_unparseable_messages(self):
"""delete_invalid=True on its own: the unparseable message is deleted
while the parsed failure report is still archived."""
self._deliver(self.FAILURE)
self._deliver(self.JUNK)
conn, result = self._run(delete_invalid=True)
self.assertEqual(len(result["failure_reports"]), 1)
self.assertEqual(conn.fetch_messages("INBOX"), [])
self.assertEqual(conn.fetch_messages("Archive/Invalid"), [])
self.assertEqual(len(conn.fetch_messages("Archive/Failure")), 1)
def test_delete_invalid_false_keeps_unparseable_when_delete_true(self):
"""delete=True with delete_invalid=False: the unparseable message is
kept in Archive/Invalid for debugging while the parsed failure report
message is deleted."""
self._deliver(self.FAILURE)
self._deliver(self.JUNK)
conn, result = self._run(delete=True, delete_invalid=False)
self.assertEqual(len(result["failure_reports"]), 1)
self.assertEqual(conn.fetch_messages("INBOX"), [])
self.assertEqual(len(conn.fetch_messages("Archive/Invalid")), 1)
self.assertEqual(conn.fetch_messages("Archive/Failure"), [])
def test_per_type_delete_flags_parallel(self):
"""Same per-type semantics on the n_procs=2 branch, where
invalid-message disposition happens after the parse phase: the
failure report and the unparseable message are archived (both
explicitly False) while the aggregate report inherits delete=True and
is deleted."""
self._deliver(self.AGGREGATE)
self._deliver(self.FAILURE)
self._deliver(self.JUNK)
conn, result = self._run(
n_procs=2, delete=True, delete_failure=False, delete_invalid=False
)
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(len(result["failure_reports"]), 1)
self.assertEqual(conn.fetch_messages("INBOX"), [])
self.assertEqual(len(conn.fetch_messages("Archive/Failure")), 1)
self.assertEqual(len(conn.fetch_messages("Archive/Invalid")), 1)
self.assertEqual(conn.fetch_messages("Archive/Aggregate"), [])
def test_test_mode_allowed_when_all_delete_flags_explicitly_false(self):
"""The test/delete guard fires on the *effective* per-type flags, so
delete=True alongside all four per-type flags explicitly False is
valid with test=True: nothing would be deleted. The message is parsed
and left in the INBOX."""
self._deliver(self.FAILURE)
conn, result = self._run(
delete=True,
delete_aggregate=False,
delete_failure=False,
delete_smtp_tls=False,
delete_invalid=False,
test=True,
)
self.assertEqual(len(result["failure_reports"]), 1)
self.assertEqual(len(conn.fetch_messages("INBOX")), 1)
self.assertFalse(conn.folder_exists("Archive/Failure"))
def test_test_mode_parses_without_moving_or_creating_folders(self):
"""test=True: the report is parsed and returned, but the message stays
in the INBOX and no archive folders are created/touched."""
@@ -3228,6 +3396,64 @@ class TestGetDmarcReportsFromMailboxMaildir(unittest.TestCase):
self.assertEqual(len(conn.fetch_messages("Archive/Aggregate")), 2)
self.assertEqual(conn.fetch_messages("INBOX"), [])
def test_delete_error_is_logged_and_disposal_continues(self):
"""A backend that rejects one delete must not abort the disposal of
the remaining messages: the error is logged and the loop moves on to
the next report type. The aggregate message's delete fails, so it
stays in the INBOX, while the SMTP TLS message is still deleted."""
self._deliver(self.AGGREGATE)
self._deliver(self.SMTP_TLS)
conn = _FailingDisposalMaildirConnection(
self._maildir, maildir_create=True, fail_delete=True
)
with self.assertLogs("parsedmarc.log", level="ERROR") as cm:
result = parsedmarc.get_dmarc_reports_from_mailbox(
connection=conn, offline=True, delete=True
)
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(len(result["smtp_tls_reports"]), 1)
self.assertTrue(
any(
"Mailbox error: Error deleting message UID" in line
for line in cm.output
),
cm.output,
)
# Only the message whose delete raised is left behind.
self.assertEqual(len(conn.fetch_messages("INBOX")), 1)
self.assertEqual(conn.fetch_messages("Archive/Aggregate"), [])
self.assertEqual(conn.fetch_messages("Archive/SMTP-TLS"), [])
def test_move_error_is_logged_and_disposal_continues(self):
"""The same for the archiving half of the disposal loop: a rejected
move is logged and the loop continues, so the aggregate message stays
in the INBOX while the SMTP TLS message still reaches its archive
subfolder."""
self._deliver(self.AGGREGATE)
self._deliver(self.SMTP_TLS)
conn = _FailingDisposalMaildirConnection(
self._maildir, maildir_create=True, fail_move=True
)
with self.assertLogs("parsedmarc.log", level="ERROR") as cm:
result = parsedmarc.get_dmarc_reports_from_mailbox(
connection=conn, offline=True
)
self.assertEqual(len(result["aggregate_reports"]), 1)
self.assertEqual(len(result["smtp_tls_reports"]), 1)
self.assertTrue(
any(
"Mailbox error: Error moving message UID" in line for line in cm.output
),
cm.output,
)
self.assertEqual(len(conn.fetch_messages("INBOX")), 1)
self.assertEqual(conn.fetch_messages("Archive/Aggregate"), [])
self.assertEqual(len(conn.fetch_messages("Archive/SMTP-TLS")), 1)
class _MidRunArrivalMaildirConnection(MaildirConnection):
"""A MaildirConnection that delivers one extra message into the INBOX
@@ -3301,6 +3527,104 @@ class TestGetDmarcReportsFromMailboxMaildirBatchSizeZeroRecursion(unittest.TestC
self.assertEqual(len(result["smtp_tls_reports"]), 1)
self.assertEqual(conn.fetch_messages("INBOX"), [])
def test_batch_size_zero_recursion_threads_per_type_delete(self):
"""The per-report-type delete flags must be threaded through the
recursive re-check call too. The SMTP TLS report only "arrives" after
the main pass's fetch, so it is disposed of by the recursive call: with
delete_smtp_tls=True it must be deleted, not archived. Dropping the
kwarg from the recursive call archives it instead and fails here."""
conn = _MidRunArrivalMaildirConnection(
self._maildir, maildir_create=True, extra_source=self.SMTP_TLS
)
result = parsedmarc.get_dmarc_reports_from_mailbox(
connection=conn, offline=True, batch_size=0, delete_smtp_tls=True
)
self.assertEqual(len(result["smtp_tls_reports"]), 1)
self.assertEqual(conn.fetch_messages("INBOX"), [])
self.assertEqual(conn.fetch_messages("Archive/SMTP-TLS"), [])
# The types that inherit the (False) delete default are unaffected.
self.assertEqual(len(conn.fetch_messages("Archive/Aggregate")), 1)
self.assertEqual(len(conn.fetch_messages("Archive/Failure")), 1)
class _SingleCheckMaildirConnection(MaildirConnection):
"""A MaildirConnection whose watch() performs exactly one check.
mailsuite's own MaildirConnection.watch() loops forever (sleeping
check_timeout seconds between checks) and swallows every exception the
callback raises, so it cannot be driven from a test as-is. This override
keeps the same signature, invokes watch_inbox's check_callback once, lets
exceptions propagate, and returns -- so the real closure inside
watch_inbox runs against a real on-disk Maildir, with no parsedmarc
internals mocked.
"""
def watch(
self,
check_callback: Callable[[parsedmarc.MailboxConnection], None],
check_timeout: int,
config_reloading: Callable[[], bool] | None = None,
) -> None:
del check_timeout, config_reloading
check_callback(self)
class TestWatchInboxMaildir(unittest.TestCase):
"""watch_inbox builds a check_callback closure that calls
get_dmarc_reports_from_mailbox on every mailbox check. Everything watch
mode does to messages therefore flows through that closure, which is
reached only via the backend's watch() -- so it needs a connection whose
watch() actually runs one check (see _SingleCheckMaildirConnection)."""
FAILURE = "samples/failure/dmarc_ruf_report_linkedin.eml"
JUNK = b"From: noise@example.com\nSubject: not a report\n\nplain text\n"
def setUp(self):
self._tmp = mkdtemp()
self.addCleanup(rmtree, self._tmp, ignore_errors=True)
parsedmarc.SEEN_AGGREGATE_REPORT_IDS.clear()
self._maildir = os.path.join(self._tmp, "Maildir")
inbox = mailbox.Maildir(self._maildir, create=True)
with open(self.FAILURE, "rb") as failure_file:
inbox.add(mailbox.MaildirMessage(failure_file.read()))
inbox.add(mailbox.MaildirMessage(self.JUNK))
inbox.flush()
def test_watch_inbox_forwards_per_type_delete_flags(self):
"""Trip-wire for the per-report-type delete flags on watch_inbox's
inner call: delete=True with delete_failure=False must archive the
failure report message while the unparseable message, inheriting
delete=True, is deleted. Dropping delete_failure from the closure
would silently delete the failure report message instead -- watch
mode ignoring what the caller asked for -- and fail here."""
conn = _SingleCheckMaildirConnection(self._maildir, maildir_create=True)
# watch_inbox passes create_folders=False (in real watch mode the
# archive folders already exist, created by the run's first pass), so
# the destination folder and its parent are seeded here.
conn.create_folder("Archive")
conn.create_folder("Archive/Failure")
received = []
parsedmarc.watch_inbox(
mailbox_connection=conn,
callback=received.append,
offline=True,
delete=True,
delete_failure=False,
)
# The results reach the callback through the same closure.
self.assertEqual(len(received), 1)
self.assertEqual(len(received[0]["failure_reports"]), 1)
self.assertEqual(conn.fetch_messages("INBOX"), [])
self.assertEqual(len(conn.fetch_messages("Archive/Failure")), 1)
# delete_invalid inherited delete=True, so the unparseable message was
# deleted rather than filed in the Invalid subfolder.
self.assertFalse(conn.folder_exists("Archive/Invalid"))
class TestEmailResultsErrorBranches(unittest.TestCase):
"""email_results requires mail_to to be a list — this is enforced