DMARCbis support: new tags, removed tag warnings, DNS tree walk, bug fixes

[Copilot]

• Fix docstring in testDMARCSyntaxError — accurately describes testing InvalidDMARCTagValue for invalid fo value
• Fix docstring in testSPFMultipleRecords — accurately describes testing SPFError
• Fix docstring in testSPFRedirectWithMacro — accurately describes the redirect counting as 1 DNS lookup
• Change removed_tags in dmarc.py from set to tuple for deterministic warning ordering
• Patch query_dns in testDMARCbisTreeWalkDiscovery to remove network dependency and skipUnless decorator
• Normalize domain at start of query_dmarc_record() using normalize_domain(domain).rstrip(".") to handle zero-width/unicode characters and trailing dots consistently

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[seanthegeek]

@copilot Improve tests by reviewing all tests and adding tests as need to ensure complete code coverage of the entire project, not just this PR.

[Copilot]

@copilot Improve tests by reviewing all tests and adding tests as need to ensure complete code coverage of the entire project, not just this PR.

Added 92 new tests across all modules (153 total, up from 61). Coverage improved from 45% to 58%. Key module improvements: dmarc.py 60%→82%, mta_sts.py 32%→71%, smtp_tls_reporting.py 38%→69%, soa.py 43%→89%, __init__.py 21%→54%.

Also found and fixed 5 pre-existing bugs — all missing raise keywords that silently discarded exceptions:

• mta_sts.py: 4 instances in parse_mta_sts_policy() (duplicate key, invalid version, invalid mode, non-integer max_age) + fixed duplicate key detection to use seen_keys set instead of checking pre-populated dict
• smtp_tls_reporting.py: 1 instance in parse_smtp_tls_reporting_record() (missing rua tag)

See commits a84f310 and f0fded7.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• vmc.digicert.com
  • Triggering command: /usr/bin/python python -m pytest tests.py -v --tb=short (dns block)
  • Triggering command: /usr/bin/python python -m pytest tests.py --cov=checkdmarc --cov-report=term-missing (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Copilot]

Pull request overview

Adds DMARCbis parsing/query support alongside RFC 7489 behavior, fixes several pre-existing parsing/exception bugs across modules, and significantly expands unit test coverage to cover DMARC/MTA-STS/TLSRPT/SOA/SPF and utility helpers.

Changes:

• Extend DMARC tag support (e.g., np, psd, t), adjust p handling, and implement DMARCbis tree-walk discovery in query_dmarc_record
• Fix missing raise statements in MTA-STS policy parsing and SMTP TLS Reporting record parsing; fix DMARC error message formatting and tag description population
• Add broad unit tests for new DMARCbis behaviors and many previously uncovered edge cases

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
tests.py	Adds extensive new unit tests across DMARC, SPF, MTA-STS, TLSRPT, SOA, and utility helpers
checkdmarc/smtp_tls_reporting.py	Fixes missing raise for required rua tag
checkdmarc/mta_sts.py	Fixes missing raise statements and corrects duplicate-key detection logic
checkdmarc/dmarc.py	Adds DMARCbis tags/behavior, tree-walk DMARC discovery, and fixes tag description/value handling

Comments suppressed due to low confidence (5)

tests.py:797

• The docstring says this test raises DMARCSyntaxError, but the assertion expects InvalidDMARCTagValue. Update the docstring (or the expected exception) so the test name/description matches the behavior being validated.

    def testDMARCSyntaxError(self):
        """A DMARC syntax error raises DMARCSyntaxError"""
        dmarc_record = "v=DMARC1; p=reject; fo=invalid_value"
        domain = "example.com"
        self.assertRaises(
            checkdmarc.dmarc.InvalidDMARCTagValue,
            checkdmarc.dmarc.parse_dmarc_record,
            dmarc_record,
            domain,
        )

tests.py:1197

• The docstring says multiple SPF TXT records raise SPFRecordNotFound, but this test asserts SPFError. Adjust the docstring to reflect the actual expected exception type (or assert the more specific exception if that’s the intended contract).

    def testSPFMultipleRecords(self):
        """Multiple SPF TXT records raise SPFRecordNotFound (wraps MultipleSPFRTXTRecords)"""
        with patch("checkdmarc.spf.query_dns") as mock_dns:
            mock_dns.side_effect = [
                [],  # SPF type records
                ["v=spf1 -all", "v=spf1 +all"],  # Two SPF TXT records
            ]
            self.assertRaises(
                checkdmarc.spf.SPFError,
                checkdmarc.spf.query_spf_record,
                "example.com",
            )

tests.py:1217

• The docstring claims the SPF redirect-with-macro case is accepted "without DNS lookup", but the assertion expects dns_lookups == 1. Clarify the docstring to match what the parser counts as a lookup here (or adjust the expected lookup count if the intention is truly zero DNS work).

    def testSPFRedirectWithMacro(self):
        """SPF redirect with macro is accepted without DNS lookup"""
        spf_record = "v=spf1 redirect=%{d}._spf.example.com"
        domain = "example.com"
        results = checkdmarc.spf.parse_spf_record(spf_record, domain)
        self.assertIsNotNone(results["parsed"]["redirect"])
        self.assertEqual(results["dns_lookups"], 1)

checkdmarc/dmarc.py:1366

• removed_tags is defined as a set, so the order of warnings appended here is not guaranteed. Since warnings are user-visible output (and may be asserted in tests), consider using a tuple/list (e.g., (\"pct\", \"rf\", \"ri\")) to keep warning ordering deterministic.

    # Warn about tags removed in DMARCbis
    removed_tags = {"pct", "rf", "ri"}
    for tag in removed_tags:
        if tag in tags and tags[tag]["explicit"]:

tests.py:594

• This test patches _query_dmarc_record but query_dmarc_record() also performs a real query_dns(domain, 'TXT') call for the root-record warning, which can make the test slow/flaky and actually dependent on DNS/network. Since the intent is to test the tree-walk with mocks, also patch checkdmarc.dmarc.query_dns (e.g., return []) and then the skipUnless(/etc/resolv.conf) gate can be removed.

    @unittest.skipUnless(os.path.exists("/etc/resolv.conf"), "no network")
    def testDMARCbisTreeWalkDiscovery(self):
        """DNS tree walk discovers DMARC records for parent domains"""
        # This tests that the tree walk works by using a mock
        with patch("checkdmarc.dmarc._query_dmarc_record") as mock_query:
            # First call for sub.example.com returns None
            # Walk: example.com returns a record
            mock_query.side_effect = [
                None,  # _dmarc.sub.example.com
                "v=DMARC1; p=reject",  # _dmarc.example.com
            ]
            result = checkdmarc.dmarc.query_dmarc_record("sub.example.com")

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[seanthegeek]

@copilot apply changes based on the comments in this thread

[Copilot]

@copilot apply changes based on the comments in this thread

Both review suggestions were already applied in previous commits:

1. Tree-walk exception handling (dmarc.py:941-947) — separates DMARCRecordNotFound (continue) from other DMARCError (re-raise)
2. Unused dns.exception import removed from tests.py

All 153 tests pass and ruff check is clean.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• vmc.digicert.com
  • Triggering command: /usr/bin/python python -m pytest tests.py -v --tb=short (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

tests.py:1101

• testDMARCTreeWalkLongDomain’s docstring says it verifies the query-limit/starting point for long domains, but the test only asserts that DMARCRecordNotFound is raised. This won’t catch regressions in the tree-walk limit logic. Add assertions on mock_query.call_args_list (e.g., max number of calls and the first parent queried for the long domain) so the test actually verifies the intended behavior.

    def testDMARCTreeWalkLongDomain(self):
        """DNS tree walk limits queries for domains with many labels"""
        with patch("checkdmarc.dmarc._query_dmarc_record") as mock_query:
            with patch("checkdmarc.dmarc.query_dns") as mock_root_dns:
                mock_root_dns.return_value = []
                # For a 9-label domain, it should start from 7 labels (index 2)
                # Calls: original domain, then tree walk from d.e.f.g.example.com down
                mock_query.return_value = None
                domain = "a.b.c.d.e.f.g.example.com"
                self.assertRaises(
                    checkdmarc.dmarc.DMARCRecordNotFound,
                    checkdmarc.dmarc.query_dmarc_record,
                    domain,
                )

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• vmc.digicert.com
  • Triggering command: /usr/bin/python python -m pytest tests.py -v --tb=short (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.