amorfo77/parsedmarc
1
0
Fork 0
mirror of https://github.com/domainaware/parsedmarc.git synced 2026-07-28 19:34:55 +00:00
Code Issues Projects Releases Wiki Activity

Skip the results email when no reports were parsed (#837)

Browse Source 
* Skip the results email when no reports were parsed (#200)

The [smtp] (and Microsoft Graph) results email was sent unconditionally
whenever the transport was configured, so an empty run — an empty inbox,
or one where every message was invalid — still emailed a zip of
headers-only CSVs. The email step is now skipped with an INFO log when
the run produced no aggregate, failure, or SMTP TLS reports.

The regression test was verified to fail against the unfixed code
(send_email was called once with a headers-only zip). Six existing tests
that asserted on the email path with all-empty mocked results now feed a
real parsed sample aggregate report through the actual zip/CSV-building
code instead.

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

* Add Graph-path regression test for the empty-run email guard

Copilot review on #837 pointed out that only the SMTP transport had a
skip regression test, so a refactor narrowing the guard to the SMTP
branch could silently reintroduce headers-only zips via Microsoft
Graph. The new test was verified to fail against exactly that
narrowing (send_message called once with a headers-only zip).

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-21 20:49:41 -04:00
committed by GitHub
co-authored by Claude Fable 5
parent e98e12acb0
commit 4fa8cfb1e7
4 changed files with 170 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+1
View File
@@ -11,6 +11,7 @@
- **The Elasticsearch/OpenSearch aggregate dashboards' over-time charts (and the Grafana ES dashboard's summary pies and time series) bucketed on the multi-valued `date_range` field**; a date histogram counts a report once per value, double-counting any report whose begin and end dates fall in different buckets. All date histograms and time-range filters now use the single-valued `date_begin`, matching the report-begin semantics of the PostgreSQL (`begin_date`) and Splunk (`_time` = interval begin) dashboards.
- **Aggregate-report policy and authentication result words are now normalized to lowercase** ([#288](https://github.com/domainaware/parsedmarc/issues/288)): reporters that emit mixed-case values such as `Pass` no longer create duplicate result categories in outputs and dashboards.
- **The results email (SMTP and Microsoft Graph) is no longer sent when no reports were parsed** ([#200](https://github.com/domainaware/parsedmarc/issues/200)): previously an empty run — an empty inbox, or one where every message was invalid — still emailed a zip of headers-only CSVs. The email step is now skipped with an INFO log when the run produced no aggregate, failure, or SMTP TLS reports.
## 10.2.4
docs/source/usage.md
+5
View File
@@ -527,6 +527,11 @@ The full set of configuration options are:
- `aggregate_topic` - str: The Kafka topic for aggregate reports
- `failure_topic` - str: The Kafka topic for failure reports
- `smtp`
The results email is only sent when at least one aggregate, failure,
or SMTP TLS report was parsed during the run; an empty run (e.g. an
empty inbox) skips the email instead of sending headers-only CSVs.
- `host` - str: The SMTP hostname. Required unless `[msgraph]` is
configured, in which case omitting it sends the summary via
Microsoft Graph instead — see "Sending the summary email via
parsedmarc/cli.py
+10 -1
View File
@@ -2681,7 +2681,16 @@ def _main():
if isinstance(opts.smtp_to, list)
else _str_to_list(str(opts.smtp_to))
)
if opts.smtp_host:
has_reports = bool(
parsing_results["aggregate_reports"]
or parsing_results["failure_reports"]
or parsing_results["smtp_tls_reports"]
)
if not has_reports and (
opts.smtp_host or (msgraph_connection is not None and smtp_to_value)
):
logger.info("No reports were parsed; skipping the results email")
elif opts.smtp_host:
try:
verify = True
if opts.smtp_skip_certificate_verification:
tests/test_cli.py
+154 -6
View File
@@ -26,6 +26,21 @@ import parsedmarc
import parsedmarc.cli
import parsedmarc.elastic
import parsedmarc.opensearch as opensearch_module
from parsedmarc.types import AggregateReport
SAMPLE_AGGREGATE_REPORT_PATH = (
"samples/aggregate/!example.com!1538204542!1538463818.xml"
)
def _sample_aggregate_reports() -> list[AggregateReport]:
"""Parses a real sample aggregate report so tests that exercise the
real zip/CSV-building code path (email_results / email_results_via_msgraph)
have a fully valid AggregateReport rather than a hand-built partial
dict that would fail in parsed_aggregate_reports_to_csv_rows()."""
result = parsedmarc.parse_report_file(SAMPLE_AGGREGATE_REPORT_PATH, offline=True)
assert result["report_type"] == "aggregate"
return [cast(AggregateReport, result["report"])]
class _BreakLoop(BaseException):
@@ -1892,7 +1907,7 @@ certificate_password = s3cret-cert-pass
"""[smtp] with only to/subject (no host) plus [msgraph] sends the
summary via Microsoft Graph's sendMail."""
mock_get_mailbox_reports.return_value = {
"aggregate_reports": [],
"aggregate_reports": _sample_aggregate_reports(),
"failure_reports": [],
"smtp_tls_reports": [],
}
@@ -1918,6 +1933,44 @@ subject = DMARC Summary
with zipfile.ZipFile(io.BytesIO(payload)) as zf:
self.assertIsNone(zf.testzip())
@patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
@patch("parsedmarc.cli.MSGraphConnection")
def testCliSkipsMsGraphSummaryEmailWhenNothingWasParsed(
self, mock_graph_connection, mock_get_mailbox_reports
):
"""Regression test for the Microsoft Graph half of the #200
empty-run guard: with an empty mailbox (no aggregate, failure,
or SMTP TLS reports), the Graph-sent summary email must not be
sent, and an INFO log line should explain why it was skipped.
A refactor that narrows the skip guard's condition to only the
SMTP branch (dropping the msgraph_connection/smtp_to_value
check) must fail this test."""
mock_get_mailbox_reports.return_value = {
"aggregate_reports": [],
"failure_reports": [],
"smtp_tls_reports": [],
}
# verbose=true is added (CERT_CONFIG only sets silent=true) so the
# logger's effective level is INFO and the skip message is
# actually emitted for assertLogs to capture.
config_text = (
self.CERT_CONFIG.replace("silent = true", "silent = true\nverbose = true")
+ """
[smtp]
to = admin@example.com
subject = DMARC Summary
"""
)
cfg_path = self._write_config(config_text)
with self.assertLogs("parsedmarc.log", level="INFO") as logs:
self._run_main(cfg_path)
mock_graph_connection.return_value.send_message.assert_not_called()
self.assertTrue(
any("skipping the results email" in line for line in logs.output)
)
@patch("parsedmarc.cli.email_results")
@patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
@patch("parsedmarc.cli.MSGraphConnection")
@@ -1928,7 +1981,7 @@ subject = DMARC Summary
set, even with [msgraph] also configured — no fallback, no
dual-send."""
mock_get_mailbox_reports.return_value = {
"aggregate_reports": [],
"aggregate_reports": _sample_aggregate_reports(),
"failure_reports": [],
"smtp_tls_reports": [],
}
@@ -2009,7 +2062,7 @@ to = admin@example.com
"""A Graph sendMail failure gets the same single-ERROR-line
treatment as connection/fetch failures."""
mock_get_mailbox_reports.return_value = {
"aggregate_reports": [],
"aggregate_reports": _sample_aggregate_reports(),
"failure_reports": [],
"smtp_tls_reports": [],
}
@@ -2054,7 +2107,7 @@ subject = DMARC Summary
Graph path, the configured attachment filename and message
body must reach send_message()."""
mock_get_mailbox_reports.return_value = {
"aggregate_reports": [],
"aggregate_reports": _sample_aggregate_reports(),
"failure_reports": [],
"smtp_tls_reports": [],
}
@@ -3732,7 +3785,7 @@ class TestSmtpAttachmentAndMessageWiring(unittest.TestCase):
email_results() rather than being silently dropped."""
mock_imap_connection.return_value = object()
mock_get_mailbox_reports.return_value = {
"aggregate_reports": [],
"aggregate_reports": _sample_aggregate_reports(),
"failure_reports": [],
"smtp_tls_reports": [],
}
@@ -3777,7 +3830,7 @@ message = Custom body text
than being called with no attachment/message context at all."""
mock_imap_connection.return_value = object()
mock_get_mailbox_reports.return_value = {
"aggregate_reports": [],
"aggregate_reports": _sample_aggregate_reports(),
"failure_reports": [],
"smtp_tls_reports": [],
}
@@ -3807,6 +3860,101 @@ to = admin@example.com
)
class TestSkipsResultsEmailWhenNoReportsParsed(unittest.TestCase):
"""#200: a run that parses no reports at all (e.g. an empty inbox,
or a mailbox where every message failed to parse) must not send a
results email with headers-only, data-free CSVs. The email step is
now skipped, with an INFO log line, whenever aggregate_reports,
failure_reports, and smtp_tls_reports are all empty."""
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
def _run_main(self, cfg_path, *cli_args):
with patch.object(sys, "argv", ["parsedmarc", "-c", cfg_path, *cli_args]):
parsedmarc.cli._main()
@patch("parsedmarc.send_email")
@patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
@patch("parsedmarc.cli.IMAPConnection")
def testSkipsResultsEmailWhenNothingWasParsed(
self, mock_imap_connection, mock_get_mailbox_reports, mock_send_email
):
"""Regression test for #200: with an empty inbox (no aggregate,
failure, or SMTP TLS reports), the SMTP results email must not
be sent, and an INFO log line should explain why it was
skipped."""
mock_imap_connection.return_value = object()
mock_get_mailbox_reports.return_value = {
"aggregate_reports": [],
"failure_reports": [],
"smtp_tls_reports": [],
}
config_text = """[general]
silent = true
verbose = true
[imap]
host = imap.example.com
user = test-user
password = test-password
[smtp]
host = smtp.example.com
user = smtp-user
password = smtp-password
from = dmarc@example.com
to = admin@example.com
"""
cfg_path = self._write_config(config_text)
with self.assertLogs("parsedmarc.log", level="INFO") as logs:
self._run_main(cfg_path)
mock_send_email.assert_not_called()
self.assertTrue(
any("skipping the results email" in line for line in logs.output)
)
@patch("parsedmarc.send_email")
def testSendsResultsEmailWithRealPayloadWhenReportsExist(self, mock_send_email):
"""When at least one report is parsed, the results email is
still sent, and the attached zip's aggregate.csv contains actual
data rows (not just a header row)."""
config_text = """[general]
silent = true
offline = true
[smtp]
host = smtp.example.com
user = smtp-user
password = smtp-password
from = dmarc@example.com
to = admin@example.com
"""
cfg_path = self._write_config(config_text)
with patch.object(
sys,
"argv",
["parsedmarc", "-c", cfg_path, SAMPLE_AGGREGATE_REPORT_PATH],
):
parsedmarc.cli._main()
mock_send_email.assert_called_once()
call_kwargs = mock_send_email.call_args.kwargs
attachments = call_kwargs["attachments"]
filename, zip_bytes = attachments[0]
with zipfile.ZipFile(io.BytesIO(zip_bytes)) as zf:
csv_text = zf.read("aggregate.csv").decode("utf-8")
self.assertGreater(len(csv_text.strip().splitlines()), 1)
class TestParseConfigS3(unittest.TestCase):
def test_s3_complete(self):
from parsedmarc.cli import _parse_config