mirror of
https://github.com/domainaware/parsedmarc.git
synced 2026-11-10 19:11:00 +00:00
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 <noreply@anthropic.com> --------- Co-authored-by: Sean Whalen <44679+seanthegeek@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
3 files changed
+66
-13
No files matched your search
@@ -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 `<identifiers>` and inside a `<reason>` 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
|
||||
|
||||
+13
-10
@@ -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()
|
||||
|
||||
+49
-3
@@ -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(
|
||||
"<identifiers>",
|
||||
"<identifiers><foo><x>1</x></foo>"
|
||||
"<envelope_from>bounce.example.com</envelope_from>"
|
||||
"<envelope_to>example.net</envelope_to>",
|
||||
1,
|
||||
).replace(
|
||||
"</policy_evaluated>",
|
||||
"<reason><type>other</type><comment>test</comment>"
|
||||
"<foo><x>1</x></foo></reason></policy_evaluated>",
|
||||
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"""
|
||||
|
||||
Reference in new issue
Block a user