From 571bbf741b961f8ddaa82b3e7c4eb74cd9ea8c17 Mon Sep 17 00:00:00 2001 From: Sean Whalen <44679+seanthegeek@users.noreply.github.com> Date: Sat, 11 Jul 2026 15:52:47 -0400 Subject: [PATCH] Guarantee non-empty xml_schema: identifies the report, not MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The XML root element (anchored at the top of parse_aggregate_report_xml) is what identifies an aggregate report; is optional metadata per RFC 7489 Appendix C. The "draft" fallback only covered a fully absent : an empty (which xmltodict parses as None) or a whitespace-only value produced xml_schema=None, which the flat-row serializer coerced to "" — and any consumer detecting aggregate rows by a non-empty xml_schema, such as the Google SecOps CBN parser's report-type cascade, silently dropped the row. The fallback now applies unless carries non-empty text (attribute-wrapped values are unwrapped via _text like other elements). The regression test fails on the previous code with xml_schema=None != "draft". Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 7 +++ google_secops_parser/README.md | 5 ++- google_secops_parser/parsedmarc.conf | 8 ++-- parsedmarc/__init__.py | 12 ++++- tests/test_init.py | 67 ++++++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12195c81..99668115 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,13 @@ `policy_strings` / `mx_host_patterns` from an earlier policy were reused for a later policy that did not define them, because the row template dict was built once per report instead of once per policy. +- **`xml_schema` can no longer be empty on aggregate rows.** The `` + root element is what identifies an aggregate report; `` is optional + metadata (RFC 7489 Appendix C) that reporters omit or leave empty in the + wild. An empty or whitespace-only `` previously produced + `xml_schema=None` (an empty string in the flat CSV/JSON rows), breaking + consumers that detect aggregate rows by a non-empty `xml_schema`. The + "draft" fallback now applies unless `` carries non-empty text. ## 10.2.1 diff --git a/google_secops_parser/README.md b/google_secops_parser/README.md index ff4d4ed6..6242cd27 100644 --- a/google_secops_parser/README.md +++ b/google_secops_parser/README.md @@ -57,7 +57,10 @@ The `or` fallbacks matter: text-format failure reports have no `Feedback-Type` field (parsedmarc emits no `feedback_type` key for them, but always computes `arrival_date_utc`), and SMTP TLS failure-detail rows from parsedmarc versions older than this parser lack `policy_type` (every RFC 8460 failure detail has a -`result_type`). +`result_type`). `xml_schema` needs no fallback: the `` XML root +element is what identifies an aggregate report, and parsedmarc guarantees a +non-empty `xml_schema` on every aggregate row ("draft" whenever `` +is missing, empty, or whitespace). `EMAIL_TRANSACTION` and `GENERIC_EVENT` are both valid `metadata.event_type` values. Note that **`GENERIC_EVENT` events only appear in raw-log and UDM diff --git a/google_secops_parser/parsedmarc.conf b/google_secops_parser/parsedmarc.conf index e9f8c70c..a2304a84 100644 --- a/google_secops_parser/parsedmarc.conf +++ b/google_secops_parser/parsedmarc.conf @@ -241,9 +241,11 @@ filter { # policy_type alone is not enough for streams from parsedmarc versions # that omitted policy_domain/policy_type on SMTP TLS failure-detail rows; # result_type is required in every RFC 8460 failure detail. - # xml_schema is aggregate-only and parsedmarc defaults it to "draft" when - # the report omits (parsedmarc/__init__.py), so it survives a - # missing version. It is preferred over: header_from (can be empty when a + # xml_schema is aggregate-only and guaranteed non-empty: the + # XML root element is what identifies an aggregate report, and parsedmarc + # falls back to "draft" whenever is missing, empty, or + # whitespace (parsedmarc/__init__.py), so every aggregate row carries a + # usable value. It is preferred over: header_from (can be empty when a # record carries no identifiers), adkim (a defaulted policy field), domain # (a generic name), and dmarc_aligned (a boolean that only becomes testable # after the convert in step 1b -- detection should not depend on that). diff --git a/parsedmarc/__init__.py b/parsedmarc/__init__.py index d964f5a1..e9a6109c 100644 --- a/parsedmarc/__init__.py +++ b/parsedmarc/__init__.py @@ -848,9 +848,19 @@ def parse_aggregate_report_xml( report_metadata["email"], ) report_metadata["email"] = unwrapped + # The root element (anchored above) is what identifies an + # aggregate report; is only metadata about the schema + # revision. Reports in the wild omit entirely or send an + # empty (which xmltodict parses as None) or one carrying + # attributes (a dict). Only a non-empty text value may override the + # "draft" default: xml_schema must never be empty, because flat-row + # consumers (e.g. the Google SecOps parser's report-type detection) + # rely on it being present on every aggregate row. schema = "draft" if "version" in report: - schema = report["version"] + version = _text(report["version"]) + if isinstance(version, str) and version.strip(): + schema = version.strip() new_report: dict[str, Any] = { "xml_schema": schema, "xml_namespace": xml_namespace, diff --git a/tests/test_init.py b/tests/test_init.py index 6ea50912..48751f36 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -1497,6 +1497,73 @@ class Test(unittest.TestCase): self.assertIsNone(pp["testing"]) self.assertIsNone(pp["discovery_method"]) + def testEmptyVersionElementFallsBackToDraftSchema(self): + """An empty must not blank out xml_schema. + + The root element is what identifies an aggregate report; + is optional metadata (RFC 7489 Appendix C) that reporters + omit or leave empty in the wild. xmltodict parses an empty + as None, which previously became xml_schema=None and an + empty string in the flat rows — breaking downstream consumers that + detect aggregate rows by a non-empty xml_schema (the Google SecOps + parser's report-type cascade). + """ + xml = """ + + + + TestOrg + test@example.com + test-empty-version + 17040672001704153599 + + + example.com +

none

+ + + + 192.0.2.1 + 1 + nonepasspass + + example.com + example.compass + + """ + report = parsedmarc.parse_aggregate_report_xml(xml, offline=True) + self.assertEqual(report["xml_schema"], "draft") + rows = parsedmarc.parsed_aggregate_reports_to_csv_rows(report) + self.assertEqual(rows[0]["xml_schema"], "draft") + + def testWhitespaceVersionElementFallsBackToDraftSchema(self): + """A whitespace-only value also falls back to 'draft'.""" + xml = """ + + + + TestOrg + test@example.com + test-ws-version + 17040672001704153599 + + + example.com +

none

+ + + + 192.0.2.1 + 1 + nonepasspass + + example.com + example.compass + + """ + report = parsedmarc.parse_aggregate_report_xml(xml, offline=True) + self.assertEqual(report["xml_schema"], "draft") + def testMagicXmlTagDetection(self): """XML without declaration (starting with '<') is extracted""" xml_no_decl = b"Ta@b.comr117040672001704153599example.com

none

192.0.2.11nonepasspassexample.comexample.compass"