Skip to content

DMARC branch coverage round 3 + fix apex-fallback exception masking#248

Merged
seanthegeek merged 1 commit into
mainfrom
coverage/dmarc-branch-coverage
May 20, 2026
Merged

DMARC branch coverage round 3 + fix apex-fallback exception masking#248
seanthegeek merged 1 commit into
mainfrom
coverage/dmarc-branch-coverage

Conversation

@seanthegeek
Copy link
Copy Markdown
Contributor

Summary

Pushes DMARC branch coverage from 80% to 98% with focused tests for the apex-fallback path, the DNS tree walk, the report-authorization helpers, and the rua/ruf URI processing. Also fixes one bug surfaced by the new tests.

File Before After
dmarc.py 80% 98%
Project total 91% 93%

Tests added

  • TestQueryDmarcRecordEdges_query_dmarc_record apex fallback (NoAnswer / NXDOMAIN / generic Exception on the apex query, and a generic Exception at the selector wrapping as DMARCError).
  • TestQueryDmarcRecordTreeWalk — walk succeeds at a parent, continues past per-step DMARCRecordNotFound, re-raises other DMARCError, NXDOMAIN on the apex TXT lookup, and the short vs long not-found messages.
  • TestCheckWildcardDmarcReportAuthorization — found, missing, unrelated records (raise vs ignore), DNSException returning False.
  • TestVerifyDmarcReportDestination — same-base-domain short circuit, wildcard short circuit, specific authorization present, missing record, unrelated records reaching the catch-all.
  • TestParseDmarcRecordReportBranches — size-limit warnings, cross-domain destinations calling verify_dmarc_report_destination, missing MX records, DNSException on MX lookup, and the >2-URIs best-practice warning for both rua and ruf.
  • TestGetDmarcRecord — both include_tag_descriptions branches.
  • TestCheckDmarcErrorWithTargetcheck_dmarc flattens error.data["target"] onto the result.

Bug fix in checkdmarc/dmarc.py

_query_dmarc_record's apex-fallback path had the same masking pattern as the pre-#246 BIMI code:

try:
    records = query_dns(domain, "TXT", ...)
    for record in records:
        if record.startswith(txt_prefix):
            raise DMARCRecordInWrongLocation(...)
except dns.resolver.NoAnswer:
    pass
except dns.resolver.NXDOMAIN:
    raise DMARCRecordNotFound(...)
except Exception as error:
    raise DMARCRecordNotFound(error)   # catches and converts DMARCRecordInWrongLocation

So a DMARC record placed at the apex (instead of _dmarc.<domain>) surfaced to callers as DMARCRecordNotFound — they couldn't distinguish "wrong location" from "no record found at all". Added an except DMARCError: raise clause before the catch-all so the specific DMARCRecordInWrongLocation propagates. testApexFallbackWrongLocation demonstrates the previously-broken path.

Test plan

  • GITHUB_ACTIONS=true python -m pytest tests/ passes (396 passed, 12 skipped)
  • ruff check, ruff format --check, pyright (latest, 1.1.409) all clean
  • CI green on this PR

🤖 Generated with Claude Code

tests/test_dmarc.py adds coverage for:
- _query_dmarc_record apex-fallback paths (NoAnswer/NXDOMAIN/Exception
  on the apex query, and a generic Exception at the selector wrapping
  as DMARCError)
- query_dmarc_record DNS tree walk (success at the parent, walk
  continuing past per-step DMARCRecordNotFound, walk re-raising other
  DMARCError, NXDOMAIN looking up apex TXT, and the short/long
  not-found message variants)
- check_wildcard_dmarc_report_authorization (found, missing, unrelated
  raises / ignored, DNSException returning False)
- verify_dmarc_report_destination (same-base-domain short circuit,
  wildcard short circuit, specific authorization record present,
  missing record raises, unrelated records reach the catch-all)
- parse_dmarc_record rua/ruf branches: size-limit warnings, cross-
  domain destinations calling verify_dmarc_report_destination, missing
  MX records, DNSException on the MX lookup, and the >2-URIs
  best-practice warning for both rua and ruf
- get_dmarc_record both with/without include_tag_descriptions
- check_dmarc flattening error.data['target'] onto the result

Fix in checkdmarc/dmarc.py: _query_dmarc_record's apex-fallback path
had the same masking bug as the pre-#246 BIMI code — a broad
``except Exception as error: raise DMARCRecordNotFound(error)`` clause
caught the DMARCRecordInWrongLocation raised earlier in the same
inner try and re-raised it as DMARCRecordNotFound, so callers
couldn't tell "record at the wrong location" from "no record found".
Added an ``except DMARCError: raise`` clause to let our own specific
exceptions propagate while still converting unexpected exceptions to
DMARCRecordNotFound. The new testApexFallbackWrongLocation
demonstrates the previously-broken path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@seanthegeek seanthegeek merged commit 1428c25 into main May 20, 2026
1 check passed
@seanthegeek seanthegeek mentioned this pull request May 20, 2026
3 tasks
seanthegeek added a commit that referenced this pull request May 20, 2026
* Release 5.15.5

- Bump version to 5.15.5 with CHANGELOG entry covering the five bug
  fixes that landed since 5.15.4 (BIMI exception masking + lps tag
  in #246, DMARC apex-fallback exception masking in #248, SMTP cache
  truthiness in #244, SPF redirect ``error`` shadowing in #247),
  the test reorg, and the coverage jump from 67% to 96%.
- Add Codecov to CI: switch the test step to ``pytest --cov
  --cov-report=xml --junitxml=junit.xml``, upload coverage with
  ``codecov/codecov-action@v5``, and upload junit results with
  ``codecov/test-results-action@v1``. Same minimal codecov.yml as
  parsedmarc (project status informational, patch enforced).
- Force the latest ruff and pyright in CI so lint/typecheck rules
  track upstream and surface new issues promptly. Pyright Python
  wrapper additionally uses PYRIGHT_PYTHON_FORCE_VERSION=latest so it
  pulls the most recent node-pyright bundle at run time, not the one
  baked into the wheel.
- Add Codecov badge to README in the same position parsedmarc uses,
  between the build badge and the PyPI badge.
- Add pytest-cov to requirements.txt; gitignore junit.xml.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Fix CI: use ``python -m pytest`` so the in-repo package is importable

The previous step's ``pytest --cov ...`` invocation invoked pytest via
its entry-point script, which does not add cwd to sys.path. Since
checkdmarc isn't pip-installed in CI (the workflow only ``pip install
-r requirements.txt``s the dependencies), ``import checkdmarc.*``
inside every test module failed with ModuleNotFoundError at collection
time.

``python -m pytest`` matches the original ``coverage run -m pytest``
invocation by putting cwd on sys.path[0], so the in-repo package
loads.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Correct CHANGELOG: lps grammar accepted some values, not none

The previous wording claimed the grammar "rejected any lps= value",
but BIMI_TAG_VALUE_REGEX_STRING accepted values matching ``bimi1``,
an HTTPS URL, ``personal``, or ``brand`` — so ``lps=brand`` (etc.)
did parse on 5.15.4. What was broken was the canonical
comma-separated local-part form (``lps=news,billing,support``), which
raised BIMISyntaxError, and the parser branch that processed lps
never wrote the split selectors back to ``tags[tag]["value"]``.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Sean Whalen <seanthegeek@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@seanthegeek seanthegeek deleted the coverage/dmarc-branch-coverage branch May 20, 2026 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant