diff --git a/CHANGELOG.md b/CHANGELOG.md index 45a07ab3..4e5c8962 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/docs/source/usage.md b/docs/source/usage.md index 2d97dd29..aafc48fd 100644 --- a/docs/source/usage.md +++ b/docs/source/usage.md @@ -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 diff --git a/parsedmarc/cli.py b/parsedmarc/cli.py index 406ff4a1..426be8e7 100644 --- a/parsedmarc/cli.py +++ b/parsedmarc/cli.py @@ -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: diff --git a/tests/test_cli.py b/tests/test_cli.py index 097bfa44..7634ea8b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -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": [], } @@ -1928,7 +1943,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 +2024,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 +2069,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 +3747,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 +3792,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 +3822,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