From fa905e41fd0f7e7c1e9f5e91b5f5501620e4a0a7 Mon Sep 17 00:00:00 2001 From: jwalith4418-Mj Date: Tue, 10 Nov 2026 23:39:12 +0530 Subject: [PATCH] Keep aggregate identifiers and reasons within their schema (#950) * Keep aggregate identifiers and reasons within their schema AI assistance: OpenAI Codex. * Expand the #948 CHANGELOG entry and tighten its tests RFC 9990 Appendix A and RFC 7489 Appendix C define only header_from, envelope_from and envelope_to under identifiers, and only type and comment in a reason. Cite them in the test, check that envelope_from and envelope_to survive next to a dropped nested unknown element, and cover an unknown key on the legacy identities path. The CHANGELOG entry now names the affected outputs and the compatibility change. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Sean Whalen <44679+seanthegeek@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 4 ++++ parsedmarc/__init__.py | 23 +++++++++++-------- tests/test_init.py | 52 +++++++++++++++++++++++++++++++++++++++--- 3 files changed, 66 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7947b0b..dac1acda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - **Python 3.10 is no longer supported, and Python 3.15 is.** Python 3.10 reached end of life in October 2026; `requires-python` is now `>=3.11`. Python 3.15 is now tested in CI and listed as supported. +### Bug fixes + +- **Unknown child elements under an aggregate record's `` and inside a `` are no longer copied into the parsed report** ([#948](https://github.com/domainaware/parsedmarc/issues/948)). `_parse_report_record()` copied both mappings whole, so an element that RFC 9990 Appendix A and RFC 7489 Appendix C do not define reached the JSON, Kafka, S3 and webhook outputs with whatever structure the report gave it. Only `header_from`, `envelope_from` and `envelope_to`, and a reason's `type` and `comment`, are kept. **Compatibility:** a consumer of those outputs that read such an extra key no longer receives it. + ## 11.2.2 ### Security diff --git a/parsedmarc/__init__.py b/parsedmarc/__init__.py index 894213dd..50dc41cc 100644 --- a/parsedmarc/__init__.py +++ b/parsedmarc/__init__.py @@ -829,14 +829,16 @@ def _parse_report_record( reasons = policy_evaluated["reason"] else: reasons = [policy_evaluated["reason"]] + normalized_reasons = [] for reason in reasons: # `comment` and `type` are strings in both schemas, but a sender can # give them child elements (a dict) or repeat them (a list), and the # CSV writer joins them as strings (GHSA-4272-4r8g-6c5h). They follow # the rule of every other string field: see _scalar(). - reason["comment"] = _scalar(reason.get("comment")) - reason["type"] = _scalar(reason.get("type")) - reason_type = reason["type"] + reason_type = _scalar(reason.get("type")) + normalized_reasons.append( + {"type": reason_type, "comment": _scalar(reason.get("comment"))} + ) if is_rfc_9990 and reason_type in RFC_7489_REMOVED_POLICY_OVERRIDE_TYPES: logger.warning( "Policy override reason type %r was removed in RFC 9990; " @@ -855,15 +857,16 @@ def _parse_report_record( reason_type, sorted(RFC_9990_POLICY_OVERRIDE_TYPES), ) - new_policy_evaluated["policy_override_reasons"] = reasons + new_policy_evaluated["policy_override_reasons"] = normalized_reasons new_record["policy_evaluated"] = new_policy_evaluated - if "identities" in record: - new_record["identifiers"] = record["identities"].copy() - else: - new_record["identifiers"] = record["identifiers"].copy() - _scalar_fields( - new_record["identifiers"], ("header_from", "envelope_from", "envelope_to") + raw_identifiers = ( + record["identities"] if "identities" in record else record["identifiers"] ) + new_record["identifiers"] = { + key: _scalar(value) + for key, value in raw_identifiers.items() + if key in ("header_from", "envelope_from", "envelope_to") + } new_record["auth_results"] = {"dkim": [], "spf": []} if type(new_record["identifiers"]["header_from"]) is str: lowered_from = new_record["identifiers"]["header_from"].lower() diff --git a/tests/test_init.py b/tests/test_init.py index 09499bb9..afce0af9 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -1309,6 +1309,45 @@ class Test(unittest.TestCase): record, config=parsedmarc.ParserConfig(offline=True) ) + def testAggregateRecordProjectsSchemaFields(self): + """Unknown identifier and override-reason children stay out of the output. + + RFC 9990 Appendix A and RFC 7489 Appendix C define only header_from, + envelope_from and envelope_to under identifiers, and only type and + comment in a reason, so unknown children, nested or not, are not + copied into the parsed output (#948). + """ + with open( + "samples/aggregate/!example.com!1538204542!1538463818.xml", + encoding="utf-8", + ) as sample: + xml = sample.read() + xml = xml.replace( + "", + "1" + "bounce.example.com" + "example.net", + 1, + ).replace( + "", + "othertest" + "1", + 1, + ) + record = parsedmarc.parse_aggregate_report_xml(xml, offline=True)["records"][0] + self.assertEqual( + record["identifiers"], + { + "header_from": "example.com", + "envelope_from": "bounce.example.com", + "envelope_to": "example.net", + }, + ) + self.assertEqual( + record["policy_evaluated"]["policy_override_reasons"], + [{"type": "other", "comment": "test"}], + ) + def testParseReportRecordMissingDkimSpf(self): """Record with missing dkim/spf auth results defaults correctly""" record = { @@ -1382,7 +1421,7 @@ class Test(unittest.TestCase): self.assertIsNone(reasons[1]["comment"]) def testParseReportRecordIdentities(self): - """'identities' key is mapped to 'identifiers'""" + """'identities' key is mapped to 'identifiers'; an unknown key is dropped""" record = { "row": { "source_ip": "192.0.2.1", @@ -1396,14 +1435,21 @@ class Test(unittest.TestCase): "identities": { "header_from": "Example.COM", "envelope_from": "example.com", + "foo": {"x": "1"}, }, "auth_results": {"dkim": [], "spf": []}, } result = parsedmarc._parse_report_record( record, config=parsedmarc.ParserConfig(offline=True) ) - self.assertIn("identifiers", result) - self.assertEqual(result["identifiers"]["header_from"], "example.com") + self.assertEqual( + result["identifiers"], + { + "header_from": "example.com", + "envelope_from": "example.com", + "envelope_to": None, + }, + ) def testParseReportRecordDkimDefaults(self): """DKIM result defaults: selector='none', result='none' when missing"""