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"""