amorfo77/parsedmarc
1
0 Fork 0
mirror of https://github.com/domainaware/parsedmarc.git synced 2026-11-07 01:09:26 +00:00
Code Issues Projects Releases Wiki Activity
1,683 Commits 6 Branches 71 Tags
bc17e81c59ba0764a7b8bb682e69f047c097ecd1
Commit Graph
68 Commits
Author SHA1 Message Date
Sean WhalenandClaude Opus 5.5 33ca4ecf71 Merge commit from fork 
With [smtp] ssl = False (the default) or [imap] ssl = False, mailsuite
upgraded the connection only if the server's plain-text EHLO/CAPABILITY
reply advertised STARTTLS, and otherwise authenticated in plain text. An
on-path attacker who stripped STARTTLS received the credentials and the
emailed results (CWE-319).

- Require mailsuite >= 2.3.4 (GHSA-2x4j-7g3q-wr6f), whose default
  refuses to authenticate over a connection that was not upgraded.
- Add [imap] and [smtp] starttls (unset = require STARTTLS; false =
  explicit plain-text opt-out, for loopback servers such as DavMail),
  with a startup warning when the opt-out is set and ssl is False.
- Add a starttls argument to email_results().
- Document both options; set starttls = False in the DavMail example.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-31 20:36:09 -04:00
Sean WhalenandClaude Opus 5.5 2986c58b0f Redact httpx2 URLs whenever httpx2 is installed (#939) 
_HTTP_URL_CLASSES took httpx2 from dns.query, so it only matched
httpx2.URL when dnspython 2.9+ had imported httpx2. dnspython 2.9
requires Python >= 3.11, so on Python 3.10 CI gets dnspython 2.8 while
httpx2 is still installed (hatch in the [build] extra requires it), and
httpx2.URL arguments on the httpx2 logger went unmasked. Three
TestHttpx2LogRedaction tests failed on 3.10 as a result.

httpx2 is now imported directly behind a ModuleNotFoundError guard.
The new tests re-execute cli.py with dns.query.httpx2 removed and with
httpx2 unimportable, so the regression is caught on every dnspython
version. Also drops the import of dns.query, which nothing uses now.

Follow-up to GHSA-hjr3-rx4c-24cv (unreleased).

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-31 19:32:04 -04:00
Sean WhalenClaude Opus 5.5Sean Whalen
 		48ea47adee Merge commit from fork 
* Stop logging credentials from configured URLs and the IPinfo token (GHSA-hjr3-rx4c-24cv)

With --debug, the Splunk HEC, Elasticsearch and OpenSearch client-init
lines and opensearch.set_hosts() logged configured URLs verbatim, so
userinfo passwords reached the console and log_file (CWE-532).

Triage found more paths of the same class:
- the httpx "HTTP Request:" line, forwarded at --verbose/--debug,
  contains the full URL (str(httpx.URL) keeps userinfo and query), so
  webhook and HEC URL credentials and the IPinfo ?token= were logged
  on every request;
- the PSL overrides, IP database and reverse DNS map downloaders logged
  their URL at DEBUG and str(HTTPStatusError), which embeds the full
  URL, at WARNING;
- libpq echoes the DSN or a password fragment for some malformed
  PostgreSQL connection strings, logged as an output-client error.

Add redact_url_credentials() (string methods only; urlsplit misparses
scheme-less "user:pw@host:port") and use it at each log site; add a
filter on the httpx logger that masks userinfo and the token query
value; build download-failure warnings from the status code and the
redacted request URL instead of the exception text; replace psycopg
ProgrammingError detail from a connection_string with a fixed message.
Docs now recommend separate user/password settings over URL
credentials.

Reported by helixnebulatech.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Mask credentials in the Loki URL in the log and LokiError text

The Loki output (#926) landed on master after this fix was written. Its
url keeps userinfo (copy_with(path=...) preserves it), and the --debug
"Initializing Loki client" line and both LokiError messages, which the
CLI logs on a failed push, included it as-is: the same CWE-532 path as
the Splunk HEC line. Mask it with redact_url_credentials() at all three
sites.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Withhold InvalidURL details, mask malformed URLs, and cover httpx2 and DoH nameservers

Follow-up for GHSA-hjr3-rx4c-24cv (CWE-532):

- httpx.InvalidURL quotes the component it rejected, so a password with
  an unencoded "/", "?" or "#" (RFC 3986 3.2: the authority ends there)
  leaked as "Invalid port: 'p4ssw0rd'", and its chained ValueError repeats
  it. Loki, Splunk HEC, the webhook and the three download loaders now
  report a fixed "details withheld" message raised from None.
  InvalidURL is not an httpx.HTTPError, so a malformed psl_overrides_url
  crashed startup; the loaders now treat it as a download failure.
- redact_url_credentials() masks through the last "@" when the authority
  has no "@" but one follows it, so the --debug lines no longer print
  such passwords; the userinfo lookup is shared with the new nameserver
  masking.
- dnspython 2.9 DoH logs its request line on the httpx2 logger, which was
  forwarded but not filtered; the filter now covers httpx2 and
  httpx2.URL.
- [general] nameservers / --nameservers credentials are masked in the
  DNS pre-flight error, reverse DNS debug errors and per-report errors,
  and unusable https:// / tls:// ports are rejected before any query.
- Docs recommend token/username settings for [loki] and [splunk_hec];
  AGENTS.md gains rules for keeping credentials out of logs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Sean Whalen <seanthegeek@users.noreply.github.com>
 2026-10-31 17:14:00 -04:00
Sean WhalenandClaude Opus 5.5 56e5a09923 Add opt-in Elasticsearch/OpenSearch data streams (#937) 
* Add opt-in Elasticsearch/OpenSearch data streams (#842)

New use_data_streams option in [elasticsearch] and [opensearch] saves
each report type to a data stream named like today's index without the
date suffix, instead of daily or monthly indexes. parsedmarc puts an
exact-name index template (priority 501, @timestamp mapped as date,
<stream>@custom component template on Elasticsearch) and creates the
stream; retention is left to operator-managed ILM/ISM policies, with
documented examples. Dated indexes are untouched and still searched for
duplicates and matched by the dashboards. Every document now carries
@timestamp and is saved with op_type=create in both modes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Fix data stream prose found by cross-check review (layer 1)

Aggregate @timestamp is each record's begin date, not the report's; the
cluster only rolls backing indices over under an attached policy; move
the Elasticsearch 8.7 note after the template it refers to; clarify the
name-collision paragraph, OpenSearch-side comments, the migrate_indexes
docstring, and the test-module headers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Tighten names and wiring tests found by cross-check review (layer 2)

Split the use_data_streams CLI passthrough test per backend so a call
site reading the other section's flag fails; name the dated-mode tests
for both changes they assert; compute index_date only in dated mode in
every save function.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Align data stream docs with Elastic/OpenSearch references (layer 3)

Say the stream must exist before reindexing into it (ES reindex API:
auto-creation needs a data-stream template); an existing stream's write
index needs the ILM policy attached directly since nothing rolls it over
otherwise; target the stream name in the _settings example; note
OpenSearch's leading-dot rule for stream names and Fleet's template
priorities.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Cover the Loki client-initialization error wrapper

_parse_config accepts a [loki] token alongside username/password; only
the LokiClient constructor rejects the combination. Exercise that path
through _init_output_clients with the real constructor so the
RuntimeError("Loki: ...") wrapper Codecov flagged as uncovered is tested.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-31 15:37:37 -04:00
Sean WhalenandClaude Opus 5.5 70f96d0734 Fix DNS over HTTPS under dnspython 2.9 and the misleading pre-flight error (#935) (#936) 
* Fix DNS over HTTPS under dnspython 2.9 and the pre-flight error (#935)

dnspython 2.9 moved its [doh] extra from httpx to httpx2, and
dns.query.https() rejects a session of any other class before sending
anything (dnspython 2.9.1 dns/query.py:551: `if session and not
isinstance(session, httpx2.Client): raise ValueError("session parameter
must be an httpx2.Client")`; 2.8.0 checks httpx.Client at :566).
_get_doh_session() always built an httpx.Client, so every https://
nameserver failed and the resolver retried it until LifetimeTimeout.

The shared DoH client is now built from the Client class of the module
dnspython itself imported (dns.query.httpx2 on 2.9+, dns.query.httpx on
2.7/2.8), so it matches dnspython's isinstance check even when httpx2 is
installed beside dnspython 2.8. httpx2 keeps httpx's
`allow_env_proxies = trust_env and transport is None` (_client.py:658),
so the #880 proxy behavior carries over.

The nameservers pre-flight check called get_reverse_dns(), which turns
every DNS error into None, so the failure above was reported as "no PTR
record for 1.1.1.1". It now calls query_dns() directly and reports
"no PTR record" only for NXDOMAIN/NoAnswer, and the resolver's actual
error otherwise.

Reported by florianutz.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address review of the DNS over HTTPS fix (#935)

- Add the httpx2 and httpcore2 loggers to the dependency loggers so
  --debug shows DoH traffic under dnspython 2.9+.
- Report an invalid dns_test_address by name and value in the
  pre-flight check instead of "Text input is malformed.", without
  attempting a query.
- Rewrap an over-long docstring line in _nameservers_to_resolver_input.
- Soften the test docstring about the no-[doh]-extra fallback.
- Correct the pyproject comment on what the [doh] extra installs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Rewrap the dnspython [doh] comment in pyproject.toml (#935)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-31 15:16:26 -04:00
Sebastian SchubertClaude Opus 5.5Sean Whalen
 		768bf75480 Bastischubert/loki output (#926) 
* feat: add grafana loki output

Push aggregate, failure and SMTP TLS reports as JSON log lines to the
Loki push API. Static labels and labels taken from report fields are
configurable, host defaults to the FQDN. The timestamp is either the
ingest time or the report begin time. Supports basic auth, bearer
tokens, X-Scope-OrgID and batching. Push errors are reported as output
errors so messages are not archived.

Signed-off-by: Sebastian Schubert <basti@schubert.digital>

* feat(dashboards): add grafana dashboard for the loki output

Signed-off-by: Sebastian Schubert <basti@schubert.digital>

* Address Copilot review on the Loki output

Responds to Copilot's review of #926:

- Treat any push status other than 200/204 as a failure. Loki returns
  block_ingestion_status_code (260 by default) when block_ingestion_until
  blocks ingestion, and the logs are discarded (Loki configuration
  reference); the old >= 300 check counted that as a successful save, so
  the CLI could archive mailbox messages whose reports were never stored.
- Read aggregate begin_date as UTC for timestamp = report_begin. Aggregate
  CSV rows carry the record's interval_begin, which the parser builds as
  naive UTC wall-clock time (to_utc=True); reading it as local time shifted
  entries by the host's UTC offset (+4h on America/New_York).
- Group the Published Policies dashboard dedup by org_name as well as
  report_id: report IDs are only unique per reporter (the PostgreSQL key
  is UNIQUE (org_name, report_id, ...)).
- Drop the legend sum from SMTP TLS Sessions Over Time: its [1d] lookback
  with the automatic step yields overlapping rolling totals, so the sum
  counted the same sessions repeatedly.

Adds regression tests for each, including a non-UTC timezone test against
a real sample report.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address second Copilot review on the Loki output

Responds to Copilot's second review of #926 (previously-missed findings):

- Strip each [loki] label_fields entry. _str_to_list() only lstrips, so
  "report_type , domain" produced "report_type " and LokiClient refused to
  start with "Invalid Loki label name 'report_type '".
- Wipe Loki on RESEED=1 even when Elasticsearch is empty. The wipe sat
  inside the ES-has-docs guard, so after the ES container was recreated a
  reseed appended a second copy of the samples to Loki, which does not
  deduplicate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address third Copilot review on the Loki output

Responds to Copilot's third review of #926 (previously-missed findings):

- Keep a base URL's query string when appending the push path. The path
  was string-concatenated, so https://gw.example.org?route=logs became
  ...?route=logs/loki/api/v1/push, hitting the root path with a corrupted
  query. The path is now set on the parsed httpx.URL.
- Push each report's entries in timestamp order. With timestamp =
  report_begin, a normalized multi-day aggregate report yields rows
  source-major (every day of one source, then day 1 of the next), all in
  one stream. Loki rejects entries older than the newest line minus half
  of max_chunk_age (1h by default) with too_far_behind (Loki configuration
  docs, "Accept out-of-order writes"), so later sources were partly
  rejected. The whole entry list is now sorted before batching.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(loki): do not follow redirects and drop forensic alias

httpx re-sends a 301/302 POST as GET, so a redirected push could return 200 without storing anything, and the mailbox messages were archived. Redirects are no longer followed, so a 3xx raises LokiError.

Remove save_forensic_report_to_loki: the module is new, so no caller needs the legacy alias.

Signed-off-by: Sebastian Schubert <basti@schubert.digital>

* fix(dashboards): use the existing Loki datasource uid in the dev bootstrap

When a datasource named Loki already exists with another uid, the dashboard import still bound every panel to dmarc-loki. Read the uid from the lookup response instead.

Signed-off-by: Sebastian Schubert <basti@schubert.digital>

---------

Signed-off-by: Sebastian Schubert <basti@schubert.digital>
Co-authored-by: Sean Whalen <44679+seanthegeek@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-31 10:13:37 -04:00
liangzaihuanyvClaude Opus 5.5Sean Whalen
 		2f89507e5d Fix aggregate report deduplication across policy domains (#934) 
* Fix aggregate report deduplication across policy domains

Scope aggregate-report identity to the reporting organization, Report-ID,
and policy domain, matching RFC 9990 section 3.5.1. Encode the fields as
a JSON array and compare ASCII domain letters without changing returned
report data. Preserve accepted empty-domain reports and save-confirmed
cache semantics.

Add real MIME/mbox and dry-run Maildir regressions for distinct domains,
same-domain repeats, shared configurations, boundary collisions, parallel
parsing, and false/raising save callbacks.

The implementation and tests were prepared with AI assistance, including
a separate review of the scoped changes and test evidence.

* Share the domain-scoped dedup key with the CLI direct-file path

Responds to Copilot review on #934: the CLI's direct-file path still
wrote `{org_name}_{report_id}` keys into the same seen-aggregate-report
cache that the mbox/mailbox classifier now reads with a JSON
[org, report_id, policy_domain] key, so a report passed both as a file
and inside an mbox in one run was saved twice, and direct files from one
reporter sharing a Report-ID for different domains (RFC 9990 §3.5.1)
were still collapsed.

Extract the key into `_aggregate_report_dedup_key()` and use it from
both paths; the direct-file path now writes through
`parser_config.seen_aggregate_report_ids` like the mbox path. Adds
end-to-end `_main()` regression tests for mixed file+mbox input and for
domain-scoped direct-file dedup.

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>
 2026-10-30 22:46:52 -04:00
Sean WhalenandClaude Opus 5.5 7d4661c2af Merge commit from fork 
save_report_to_s3() put the sender-controlled report ID into the object
key and org_name/org_email/report_id into user-defined metadata verbatim.
botocore rejects non-ASCII metadata client-side (ParamValidationError) and
a bare CR/LF with "Invalid header value", which the CLI counts as an S3
output error, so one report held back its whole mailbox batch. "/" and
".." in the report ID added key segments, and keys had no sender identity.

- Percent-encode the report ID into one key segment ("/", the AWS
  "characters to avoid", "%", control and non-ASCII characters) and
  shorten it so the key stays within S3's 1,024-byte limit.
- Percent-encode metadata values (reversible with unquote) and cap each at
  256 characters, under S3's 2 KB metadata limit.
- Append the first 16 hex characters of a SHA-256 over the report type and
  its identifying, report-sourced fields (never enrichment), so reporters
  that reuse one report ID no longer overwrite their own reports and a
  retry still overwrites the same object.
- Fix failure reports, which raised KeyError on report_metadata (absent
  from FailureReport in types.py) since S3 output was added in 7.0.0.

Sources: https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html,
https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingMetadata.html,
botocore handlers.validate_ascii_metadata.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-30 20:50:01 -04:00
Sean WhalenandClaude Sonnet 5.5 7908907f2c Require mailsuite >= 2.3.3 and simplify the Maildir poison-message test (#933) 
mailsuite 2.3.3 (GHSA-9jx3-p8wm-g8vw) returns stored bytes from fetch_message() and renames the file in move_message(), so poison messages now always land in the Invalid folder.

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
 2026-10-27 05:33:35 -04:00
Sean WhalenandClaude Opus 5.5 6b7c203126 Merge commit from fork 
* Fix three security advisories (DOCTYPE amplification, poison mailbox message, byte-length filenames)

GHSA-m47j-wp78-fqjg: reject any DOCTYPE in aggregate report XML. An
internal subset's <!ATTLIST> default attributes are applied by expat to
every matching element, so 64 KB of XML (K=1000, N=10000) took 4.0 s and
1.4 GB here. xmltodict now gets an expat shim whose parsers raise on
StartDoctypeDecl; the lxml recovery path checks docinfo and serializes
only the root element.

GHSA-qxwv-2vwj-v828: parse_report_email() now raises only ParserError
subclasses (HeaderWriteError / RecursionError from re-serializing a part
used to escape). Content errors raised while fetching a message
(email.errors.MessageError, RecursionError) are returned as ParserError
and the message is disposed of as invalid; transport errors still
propagate. A failed invalid-message move/delete is logged instead of
aborting the batch, because mailsuite's Maildir move_message()
re-serializes the message too. mbox reads use get_bytes() instead of the
re-serializing get_string().

GHSA-j53g-34vj-g25q: get_filename_safe_string() bounds names to 100
UTF-8 bytes as well as 100 characters; filesystems limit a name to 255
bytes, so 84 CJK characters + ".eml" failed with ENAMETOOLONG.

Reported by helixnebulatech.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address review: recursion margins, mbox wording, DOCTYPE check order

- Nest the RecursionError poison 400 levels deep (POISON_NESTING_LEVELS)
  instead of 250: the smallest failing depth is ~247 from a shallow
  stack, 245 in a fork worker and ~222 at +100 frames, so 250 left a
  margin of a few levels. The shallow case uses 50 levels and now asserts
  InvalidDMARCReport ("is not a valid report"); the deep case asserts
  "recursion" in the message.
- Poison-loop tests skip a benign poison per message (continue) and
  assert at least one poison was exercised, so they cannot pass vacuously.
- lxml recovery: check for a DOCTYPE before serializing the root, and
  comment why the str() around tostring(encoding="unicode") stays
  (pyright, no lxml stubs).
- CHANGELOG: describe what the walk actually serializes, drop the
  depth-dependent 245 figure, and say plainly that an mbox run still
  stops at such a message, now with a ParserError naming it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Make the Maildir poison-message test interpreter-independent

Whether MaildirConnection.move_message() can store the control-character
message depends on the interpreter: upstream CPython (3.12.9) runs the
CVE-2024-6923 generated-header check only in the text Generator used by
as_string(), so mailbox.Maildir.add()'s BytesGenerator stores it, while
Ubuntu's patched 3.12.3 also checks in BytesGenerator._write_headers and
the move fails. The deeply nested message fails the move everywhere.

The test now decides per message, by trying the same Maildir round trip,
whether the message lands in Archive/Invalid with no mailbox error logged
or stays in INBOX with the move error logged; both branches assert the
batch completed with the aggregate archived. CHANGELOG and the
_dispose_invalid_message docstring now say which messages stay put.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-26 22:05:36 -04:00
NefelibateLGClaude Fable 5.1ZCode ScoutSean Whalen
 		c7a9d294b5 fix: restore parent IP-lookup state in pool workers under spawn/forkserver (#930) 
* fix: restore parent IP-lookup state in pool workers under spawn/forkserver

parsedmarc.utils keeps the IPinfo Lite REST API token
(_IPINFO_API_TOKEN, configure_ipinfo_api) and the selected IP database
path (_IP_DB_PATH, load_ip_db) in process-global variables, and only
the parent process sets them. Under the fork start method a pool worker
inherits them ("All resources of the parent are inherited by the child
process", https://docs.python.org/3/library/multiprocessing.html), but
under spawn (macOS/Windows default) and forkserver (the Linux default
from Python 3.14) it does not ("The child process will only inherit
those resources necessary to run the process object's run() method").
A worker there started from scratch: its IPinfo API lookups were
silently skipped, and _get_ip_database_path() fell back to the bundled
MMDB instead of the downloaded or cached copy the parent selected, so
enrichment differed from the operator's configuration and from the same
run under fork. The CLI parses file inputs through the process pool
even with -n 1, so every file-based CLI run on such platforms was
affected.

parallel_map() now snapshots both globals in the parent and restores
them in every worker via its pool initializer, so IP enrichment matches
the parent's configuration under every start method. The initializer
previously only reconstructed logging and is renamed _init_worker
accordingly.

Fixes #928

Co-authored-by: ZCode Scout <noreply@zcode.local>
Signed-off-by: NefelibateLG <3453510037@qq.com>

* Maintainer edits: Unreleased changelog, PSL overrides, forkserver coverage

- CHANGELOG: move the entry under "## Unreleased" (feature/fix PRs never
  pick a version number) and describe the psl_overrides gap.
- Restore parsedmarc.utils.psl_overrides in pool workers as a separate
  initarg, in place ([:] =), never rebound. A fresh spawn/forkserver
  worker loads the bundled psl_overrides.txt at import and only switches
  to the operator's path/URL on its first reverse DNS map load, so
  get_base_domain() calls before that load (the first aggregate report's
  org_name, the first source IP's reverse DNS name) used the bundled list.
- Cross-reference _init_worker and cli._utils_globals_snapshot(), which
  enumerate the same three utils globals.
- Extend TestWorkerIPState to forkserver (where available) and to the
  PSL overrides list; without the restore, both subtests fail with the
  worker reporting the bundled list.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Signed-off-by: NefelibateLG <3453510037@qq.com>
Co-authored-by: ZCode Scout <noreply@zcode.local>
Co-authored-by: Sean Whalen <44679+seanthegeek@users.noreply.github.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
 2026-10-26 21:54:22 -04:00
Sean WhalenandClaude Fable 5.1 16a9016e2a Neutralize CSV formulas and escape control characters in logged strings (#931) 
* Neutralize CSV formulas and escape control characters in logged strings

CSV fields starting with =, +, -, @, tab, or CR get a leading apostrophe
(OWASP CSV Injection, CWE-1236). Sender-controlled subject/From and a few
aggregate-parser values are passed through the new sanitize_for_log()
before they reach error messages and logs.

Closes #929

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Sanitize more sender-controlled log values; fix sanitize_for_log docs

Escape the SMTP TLS policy-type in its error and org_name/report_id in the
duplicate-report debug lines, correct the sanitize_for_log docstring, and
drop the unreachable return in parse_report_email().

Closes #929

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Test the From-header escaping with a raw control character

mail-parser 4.8.0, which CI resolves, no longer decodes an RFC 2047
encoded word in the From header (4.6.2 did), so the previous test saw the
encoded word rather than the CR LF it was meant to observe. A raw ESC in
the display name survives both versions.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
 2026-10-26 21:34:23 -04:00
David UandClaude Opus 5.5 65a7bfb7f9 Fix "Aggregate Report Details" Grafana panel timing out on PostgreSQL (#923) 
PostgreSQL does not create an index on the referencing side of a foreign
key (docs, "Foreign Keys"), and create_tables() indexed only two of the
schema's seven foreign-key columns. The panel's per-record DKIM/SPF
subqueries on record_id therefore ran two sequential scans per row and
timed out on a 413k-record database.

Index the five remaining foreign-key columns, and add a test that every
REFERENCES column in the schema has an index.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-22 18:13:06 -04:00
Sean WhalenandClaude Fable 5.1 14aa287dc1 Fix ReDoS in failure-report, schema-tag, and timestamp regexes (11.0.4) (#921) 
Three regular expressions that run on attacker-controlled input
backtracked super-linearly (CWE-1333):

GHSA-7grx-mj6g-qxmc (affects 8.2.0 through 11.0.3, reported by
mjatin2007): text_report_regex, used by the Exim/cPanel plain-text
failure-report fallback, was \s*([a-zA-Z\s]+):\s(.+). The leading \s*
and the \s inside the class overlap and the pattern is unanchored, so a
whitespace run is retried from every offset and every split: cubic,
about 8x per doubling (400 spaces 0.06 s, 1600 spaces 3.3 s on CPython
3.12). A ~100 KB message would stall the watch loop for days and be
re-parsed after every restart. The pattern is now anchored to a line
start with disjoint classes.

GHSA-4h9r-c435-j834 (affects 4.1.7 through 11.0.3, found while auditing
every regex in the package): xml_schema_regex was </??xs:schema.*>,
whose greedy .*> rescans to end of line for every <xs:schema
occurrence: quadratic, 1 MB in 19 s. It runs on every attachment that
decodes to "<" after extract_report() has inflated it up to the 100 MiB
cap, which 194 KB of gzip reaches. Now </?xs:schema[^<>]*>. This also
fixes the greedy match deleting a minified single-line report from the
schema tag to the last ">" on the line.

human_timestamp_to_datetime() stripped RFC 5322 comments with
\s*\(.*\)\s*, quadratic on whitespace (200,000 spaces 18.6 s) and
reached through every Date header; filed as a bug fix because MTAs cap
header size. Comments are now removed by a depth-counting scan, which is
linear and handles the nested comments and quoted-pairs RFC 5322
section 3.2.2 allows (a character-class regex would have regressed
them).

Adds regression tests sized so the old patterns fail the time bound in
minutes rather than hanging CI, a "Regular expressions on untrusted
input" section in AGENTS.md, and the 11.0.4 changelog and version bump.

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
 2026-10-18 12:01:32 -04:00
Sean WhalenandClaude Opus 5.5 e077136b60 Fix blank SMTP TLS failure-detail fields crashing saves; read RFC 8460 additional-information (11.0.3) (#916) 
* Treat blank SMTP TLS failure-detail fields as absent; read RFC 8460 additional-information

Some reporters send optional failure-details fields as empty strings
(e.g. "sending-mta-ip": "" on sts-policy-fetch-error). The parser copied
them verbatim, so the PostgreSQL save failed on the INET columns
("invalid input syntax for type inet") and the Elasticsearch/OpenSearch
saves failed Ip() validation, discarding the whole report. Blank or
whitespace-only optional values are now omitted.

RFC 8460 section 4.4 names the URI key "additional-information"; the
parser only read "additional-info-uri" (the schema's value placeholder),
so conformant reports lost the URI. The RFC key is now read, with the old
key kept as a fallback.

Bump version to 11.0.3.

Closes #915

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

* Import parse_report_file directly in the ES/OpenSearch tests

Addresses github-code-quality's "Module is imported with 'import' and
'import from'" findings on tests/test_elastic.py and
tests/test_opensearch.py.

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

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
 2026-09-24 10:42:34 -04:00
Sean WhalenandClaude Fable 5.1 b41230dc07 Make the SIGHUP configuration reload atomic (#910) 
* fix: make the SIGHUP configuration reload atomic

The reload built the replacement output clients, then immediately closed
the old ones and swapped them in -- and only afterwards ran seven more
fallible steps: load_reverse_dns_map() (which reloads the PSL overrides
first), load_ip_db(), configure_ipinfo_api(), the setattr loop onto opts,
_build_parser_config(), the watch-parameter int() conversions, and the
log-level/FileHandler refresh. An exception in any of them was caught and
logged as "Config reload failed, continuing with previous config", which
was false: the new clients were already live and receiving reports, the
old ones were closed and unrecoverable, opts could be half-applied, and
both loaders empty their target before reading anything -- so a bad map
path emptied REVERSE_DNS_MAP until some later uncached lookup happened to
refill it from the previous configuration's paths (both
get_service_from_reverse_dns_base_domain() and get_ip_address_info()
reload a map they find empty), and a bad PSL overrides path emptied
parsedmarc.utils.psl_overrides without touching the map, so nothing
triggered that lazy reload and base domains were folded without the
overrides until a restart.

The reload is now three phases. Phase 1 stages every fallible step off to
the side, in this order: _load_config/_parse_config, _init_output_clients,
the derived values (_build_parser_config and the three watch-parameter
int()s), load_reverse_dns_map into a fresh dict, load_ip_db,
configure_ipinfo_api. The derived values run early because they write
nothing at all, and the two loaders that write process-wide state run
last, so as little as possible can fail behind them -- load_ip_db()'s
download in particular rewrites the shared MMDB cache file before it
assigns _IP_DB_PATH, and that is the one phase-1 side effect no snapshot
can undo (what it leaves is the same refresh startup performs).

The parsedmarc.utils globals the loaders assign are snapshotted by a new
_utils_globals_snapshot(). Enumerated from the loaders' bodies, those are
psl_overrides (cleared and repopulated in place, no `global` statement --
the list object itself is the state), _IP_DB_PATH (the only global
load_ip_db assigns) and _IPINFO_API_TOKEN (the only one
configure_ipinfo_api assigns, and assigned before the probe that can
fail). _LAST_LOGGED_IP_DB_PATH is deliberately not covered: it is
assigned by _get_ip_database_path(), which no loader the reload calls
reaches, and it only decides whether the database path is logged again.

Phase 2 commits with assignments only, REVERSE_DNS_MAP in place -- cli.py
imports that name from parsedmarc, so rebinding it here would desync this
module from parsedmarc.REVERSE_DNS_MAP and from the config.REVERSE_DNS_MAP
that ParserConfig.__setstate__ rebinds to in a worker, and the
ParserConfig staged in phase 1 already holds the pre-reload object. Phase
3 closes the replaced clients last and best effort, so a dying connection
cannot undo the commit.

On failure the replacement clients are closed, then the search backends'
default alias is restored -- that order, because a fully built
_ElasticsearchHandle releases the alias as it closes (see #906 and #902)
-- and then the utils globals are put back. REVERSE_DNS_MAP, opts,
clients, parser_config and the three watch parameters are never written
before the commit, so there is nothing to undo for them; the per-step end
state is traced in a comment at the rollback site, including which
restores are load-bearing at which depth and which are defensive.
configure_ipinfo_api's sys.exit(1) on an invalid token still bypasses all
of this: SystemExit is not an Exception, and the process is leaving.

Closing the replaced log file was the one statement in the commit block
that could still raise (a full disk, a stale mount); it is now caught and
reported after the handler is detached, since by that point the reloaded
configuration is live and rolling back to report a failed reload would be
a lie.

Nothing runs concurrently with the swap: the reload happens in the main
thread between watch_inbox() calls, after the mailbox backend has returned
at a batch boundary, so no save callback is in flight, and parallel.py
creates and tears down its ProcessPoolExecutor inside a single
parallel_map() call -- its workers are separate processes with their own
copies of REVERSE_DNS_MAP and the utils globals either way. The previous
code mutated the same state at the same point, so this is not a change in
exposure.

#906 made _init_output_clients() itself all-or-nothing; this does the same
for the whole reload. The docs already promised that a failed reload
leaves "the previous configuration fully active"; that promise is now
true, and usage.md spells out what it covers and the one thing it does
not.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix: open the replacement log file during reload staging, not after commit

The logging refresh was the one fallible step left after the commit:
phase 2 closed and removed the old FileHandler, then tried to open the
new log_file inside a try/except that only warned. On an unwritable
path the old handler was already gone, file logging was dead,
opts.active_log_file was set to the new path anyway, and the reload was
logged as successful. Because the old-versus-new comparison uses
active_log_file, a later SIGHUP with the same path -- after the operator
fixed permissions -- was a no-op, so only a restart recovered file
logging.

The replacement FileHandler is now constructed in phase 1, after the
watch-parameter conversions and before the loaders (constructing it
opens the file immediately, and that is the only thing in the refresh
that can fail on a bad path). The rollback closes it if a later step
fails; phase 2 only attaches it. An unwritable log_file is therefore a
reload failure, with the previous log file still attached and receiving
the traceback. Startup keeps its warning-only behaviour: there is no
previous configuration to fall back to there.

Found by Copilot's review of #910.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix: retry a log file that could not be opened at startup on the next reload

Startup recorded opts.active_log_file = opts.log_file even when opening
the FileHandler failed (a warning there, since there is no previous
configuration to keep). The SIGHUP reload only stages a replacement
handler when the new config's log_file differs from active_log_file, so
a reload naming the same path never retried the open: file logging
stayed dead until restart, even after the operator fixed the directory
or permissions.

active_log_file now starts as None and is set only after the handler is
attached, so an unchanged log_file that failed at startup is retried by
the next reload, and one that opened at startup is not reopened.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix: make the rollback's staged log handler close best-effort; correct the docs on what aborts a reload

FileHandler.close() can raise while flushing or closing (a full disk, a
stale network mount). In the reload's rollback that raise escaped the
except block: the search-alias and utils-globals restores never ran,
the original staging failure was replaced, and the watcher exited. The
close is now logged and swallowed like the commit path's close of the
replaced handler, so the restores that follow it always run.

usage.md said "unreachable output destinations" abort a reload. That is
true of PostgreSQL, Kafka, and TCP/TLS syslog, whose clients connect in
their constructors, but not of Elasticsearch or OpenSearch: set_hosts()
builds the client without connecting, and migrate_indexes() catches
every cluster call and warns, so a reload pointing at an unreachable
search cluster still commits and fails on the first save. The list now
names what actually raises during staging and calls out the exception.

Both found by Copilot's second review of #910.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
 2026-09-12 13:45:53 -04:00
Sean WhalenandClaude Fable 5.1 316910e1ea fix: never close a caller-supplied file object in parse_report_file/extract_report (#909) 
parse_report_file() and extract_report() both closed a file-like object
the caller passed in once they finished reading it (on the success path
only). A function must not close a handle it did not open: the caller
may still want to seek(0) and retry, log tell(), or reuse the handle.

- parse_report_file(): the caller-supplied-object branch no longer calls
  .close(). A path is still opened and closed via `with`; bytes are still
  wrapped in a function-owned BytesIO that the function closes.
- extract_report(): the seekable-stream branch previously aliased the
  caller's stream into `file_object` and then unconditionally closed it
  in `finally`, on the success path and on the exception path alike. An
  `owns_file_object` flag now tracks whether `file_object` is a buffer
  this function created (str/bytes input, or a copy made from a
  non-seekable caller stream) versus an alias of the caller's own
  seekable stream; only the former is closed.
- parse_aggregate_report_file() forwards its input straight to
  extract_report() and does no closing of its own, so it inherits the
  fix; its docstring now says so and points to extract_report()'s for the
  exact post-call position.

Both extract_report()'s and parse_report_file()'s docstrings now state
the contract explicitly. extract_report()'s docstring initially claimed
a successful call leaves a seekable caller stream "positioned at 0" and
described the seek(0) as seeking "back" to the caller's prior position;
neither is true. The unconditional stream.seek(0) after reading the
6-byte header runs regardless of where the caller's stream was
positioned, and the subsequent content read (member/gzip/text) advances
the stream again, so a successful call typically leaves it at EOF, not
0 (measured: XML 866, gzip 428, zip 901/974). The docstring now matches
parse_report_file()'s wording: left open, positioned wherever the
function's own reads left it.

Added testParseReportFileLeavesCallerHandleOpenOnSuccess,
testExtractReportLeavesSeekableCallerStreamOpen (success path), and
testExtractReportLeavesSeekableCallerStreamOpenOnError (exception path,
completing the "never closed on success or failure" claim, which had
gone untested on its exception half) -- asserting real BytesIO `.closed`
state and, for the success cases, that the handle is still readable
after seek(0). All three fail against the pre-fix code; verified with
`parsedmarc.__file__` that the import resolved to this worktree's
package, not a stale venv install, before trusting the failure/pass
results. Updated testParseReportFileLeavesCallerHandleOpenOnReadError's
docstring, which described the old "closed on success" behavior as
intentional.

Also added testParseAggregateReportFileLeavesCallerStreamOpen: the
contract paragraph added to parse_aggregate_report_file()'s docstring
was previously untested through that entry point -- its existing
stream-based tests only exercise extract_report() and parse_report_file()
directly, while parse_aggregate_report_file() itself was only ever
called with bytes. The new test passes a caller-owned BytesIO through
parse_aggregate_report_file() and asserts it is still open after both a
successful parse and an InvalidAggregateReport raised on garbage
content. It fails against the pre-fix code (AssertionError: True is not
false on the success-path assertion); verified via `parsedmarc.__file__`
that the import resolved to this worktree's package before trusting the
result.

CHANGELOG: added an Unreleased/Changes entry (behavior change for
library callers, naming all three affected functions) and corrected the
existing Unreleased/Bug fixes entry for parse_report_file's path-close
fix, whose closing sentence about caller-supplied objects this PR makes
false and whose cross-reference to "the Changes section below" pointed
the wrong direction (Changes is above Bug fixes).

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
 2026-09-12 11:43:46 -04:00
Sean WhalenandClaude Fable 5.1 550244c6d7 fix: roll back the search backends' default alias when output init fails (#906) 
_init_output_clients() is now all-or-nothing against the Elasticsearch
and OpenSearch connection registries: either it returns the fully built
client dict, or it closes everything it built and leaves both "default"
aliases naming exactly what they named on entry.

elastic.set_hosts()/opensearch.set_hosts() end in
connections.create_connection(), which registers the new client under
the process-wide "default" alias the moment it is constructed
(elasticsearch/dsl/connections.py:81-88 and
opensearchpy/connection/connections.py:87-95, as installed: 8.19.3 /
3.2.0). Everything that runs after that -- the index migration, and
every output configured later -- can still fail. On the SIGHUP reload
path _main() builds the replacement clients before closing the old ones
and, when the build raises, logs "Config reload failed, continuing with
previous config" and keeps the old opts. But the alias had already been
handed to the new client, so every save -- elastic.py's Search and
Document.save() calls, which resolve the "default" alias through
elasticsearch/dsl/_sync/document.py:99-100 -- reached the new cluster
while the index prefixes/suffixes and index_prefix_domain_map still came
from the old configuration. The half-built client was never
closed either, nor was any client built earlier in the same failed call
(that half also leaked on every attempt of the startup retry loop).

The handler tears down first and restores second -- with the teardown in
a try/finally, since a second Ctrl-C landing in it propagates straight
through _close_output_clients, which swallows only Exception -- and the
two steps are not interchangeable. With an _ElasticsearchHandle already in `clients`,
teardown closes its client and releases the alias -- which still names
that client -- and the restore then re-registers the previous client
into an unset alias. Restoring first would put the previous client back
and only then close the handle, which rests the whole rollback on the
handle declining to touch an alias that no longer names its own client:
true only since #902, and a property of the handle rather than of this
function. Tearing down first keeps the guarantee local. Both failure
points are traced in the comment on the handler.

Closing the discarded client is best-effort, and closing it twice is
safe: Elasticsearch.close() -> Transport.close() closes each node's
urllib3 pool (elastic_transport/_transport.py:499-504,
_node/_http_urllib3.py:224-228, and urllib3 pool close() is a no-op once
cleared), and OpenSearch's connection close() guards on `if self.pool`
(opensearchpy/connection/http_urllib3.py:323-328). Taking the snapshot
cannot itself open a connection: get_connection() lazily builds a client
from kwargs left behind by configure() (dsl/connections.py:90-115), and
parsedmarc never calls configure().

The guard catches BaseException rather than Exception because
migrate_indexes() wraps every cluster call in `except Exception` and
logs a warning (elastic.py:824-829, and again in each of the per-index
loops that follow), so what escapes the Elasticsearch block after
set_hosts() is, in practice, a KeyboardInterrupt landing in one of those
calls --
which is what the regression tests inject at the SDK transport
boundary. For the same reason, closing the discarded client swallows
BaseException: an interrupt there must not cost the alias its hand-back,
and the original failure is re-raised afterwards either way.

The alias is not the only module-level state set_hosts() writes:
elastic.set_hosts() also assigns elastic._SERVERLESS (elastic.py:620-622)
-- before it constructs the client, so it can be stale even on a failure
that never reached the registry -- and create_indexes() consults it
(elastic.py:652-659) to decide whether to strip the shard settings
Serverless rejects. It is snapshotted and restored alongside the alias.
opensearch.py has no equivalent (no `global` statement in the file).

Shape: the existing body keeps its indentation as _build_output_clients(),
which fills a `clients` dict the caller passes in and is explicitly not
transactional; the rollback lives in a short _init_output_clients()
wrapper that owns that dict on both paths. Passing the dict in is what
lets the wrapper close what was already built after the build raises. The
public name is unchanged, so the tests and _main() call sites are
untouched.

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
 2026-09-11 08:40:36 -04:00
Sean WhalenandClaude Opus 5 0f3b973be5 fix: close the search client a handle owns, not the default alias (#902) 
_ElasticsearchHandle.close() and _OpenSearchHandle.close() re-resolved
the client library's "default" connection alias at close time. That is
wrong on the SIGHUP reload path, which is not a shutdown path:
_main's reload block calls _init_output_clients() -- and therefore
set_hosts() -> connections.create_connection() -- before
_close_output_clients() on the old clients, deliberately, so a broken
new config leaves the old clients running. By then the alias names the
*new* client, so the old handle closed the new client and then dropped
the alias entirely. Every later save raised
KeyError("There is no connection with alias 'default'.") until
parsedmarc was restarted, and the original client was leaked.

Verified against the installed SDK sources rather than the docs
(AGENTS.md, "Verify bug claims against authoritative sources", point 3):

  elasticsearch/dsl/connections.py (elasticsearch 9.x) and
  opensearchpy/connection/connections.py (opensearch-py 3.x) both define

      def create_connection(self, alias="default", **kwargs):
          conn = self._conns[alias] = <Client>(**kwargs)

  -- so create_connection() overwrites whatever the alias held, and
  returns the very object it stored (elasticsearch.dsl passes it through
  _with_user_agent(), which mutates headers and returns the same object,
  so identity is preserved) --

      def remove_connection(self, alias):
          errors = 0
          for d in (self._conns, self._kwargs):
              try:
                  del d[alias]
              except KeyError:
                  errors += 1
          if errors == 2:
              raise KeyError(f"There is no connection with alias {alias!r}.")

  -- i.e. it deletes the alias outright, with no regard for which client
  currently holds it, and

      def get_connection(self, alias="default"):
          if not isinstance(alias, str):
              return alias            # (opensearch-py; elasticsearch.dsl
                                      #  returns _with_user_agent(alias))
          try:
              return self._conns[alias]
          ...
          raise KeyError(f"There is no connection with alias {alias!r}.")

  -- a str only comes *back* out of get_connection() when a str was
  passed *in* as the alias, which these call sites never do; the client
  stored under "default" is always the object create_connection() built.
  The old `isinstance(conn, str)` guard was therefore dead code and is
  removed rather than carried forward (AGENTS.md: delete unreachable
  branches, don't hide them). add_connection(alias, conn) stores an
  arbitrary object as-is, which is what the new tests use.

set_hosts() now returns the client it registered, _init_output_clients()
passes that client to the handle, and close() closes that object and
gives up the alias only when the registry's current "default" *is* that
same object (identity check). The init/close order in the reload block
is unchanged.

tests/test_cli.py gains TestSearchBackendHandles, which drives the real
connection registries (the SDK boundary) with fake client objects, so no
network is touched: per backend, a reload-shaped test asserts the old
client is closed, the new one is not, and the alias still resolves to the
new client; a shutdown-shaped test asserts the alias is released when it
is still ours; and a best-effort test asserts a raising close() neither
propagates nor blocks the alias release. Each test restores whatever the
process-wide registry held beforehand. All six fail against the unfixed
handles.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-09 19:07:29 -04:00
Sean WhalenandClaude Opus 5 77045c2df0 chore: address CodeQL code-quality findings (no behavior change) (#900) 
- py/empty-except: add a short comment to each of the 10 flagged
  `except <Type>: pass` blocks explaining why swallowing the
  exception is correct there (best-effort cleanup/close, or
  intentional fall-through to the next report format).
- py/unnecessary-lambda: replace `map(lambda x: parse_email_address(x), ...)`
  with `map(parse_email_address, ...)` at the 5 flagged sites in
  parsedmarc/utils.py; parse_email_address takes exactly one
  positional argument, so this is behavior-preserving. Ran
  `ruff format` afterward, which collapsed 3 of the now-shorter
  calls onto single lines.
- py/imprecise-assert: replace `assertTrue(a > b)` / `assertTrue(a >= b)`
  with `assertGreater(a, b)` / `assertGreaterEqual(a, b)` at the 9
  flagged test sites; none carried a custom assertion message.
- .vscode/settings.json: drop three cSpell whitelist entries that are
  pure misspellings ("passsword", "httpasswd", "unparasable") and
  appear nowhere else in the tracked tree, so a recurrence of the typo
  they used to hide (see #888) would be caught again. The correctly
  spelled "htpasswd" and "unparseable" entries are untouched.

Verified clean: ruff check, ruff format --check, pyright (0
errors/warnings), and pytest tests/ (989 tests collected and passing
before and after, GITHUB_ACTIONS=true).

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-09 19:06:34 -04:00
Sean WhalenandClaude Opus 5 d185bd0526 Fix file descriptor leak in parse_report_file (CodeQL py/file-not-closed) (#899) 
parse_report_file's path branch opened the file with open(file_path, "rb")
and then, on a later line, called file_object.read() followed by
file_object.close() with no exception handling between them. An exception
from read() (e.g. an OSError from the underlying storage) skipped close()
and left the descriptor to be released only when Python's garbage
collector eventually finalized the object, rather than closed
deterministically (CPython's io.IOBase.__del__ closes an unclosed file on
finalization: https://docs.python.org/3/library/io.html). This is the
pattern CodeQL's py/file-not-closed query flags, found in a local
code-quality scan.

The path branch now opens the file with a `with` block, so the handle is
closed on both the success and exception paths. The fix is scoped to only
that branch: a BytesIO created internally from bytes/bytearray/memoryview
input, and a file-like object supplied by the caller, are both closed
only after a successful read and left open if read() raises, exactly as
before this commit. Widening that to close a caller-supplied handle on
the exception path too would be a behavior change outside this fix's
scope.

Also closes tests/test_init.py's TestGetDmarcReportsFromMailboxMaildir
._deliver, which did open(source, "rb").read() with no close() at all;
it now uses a `with` block.

Two regression tests cover the narrowed contract:
- testParseReportFileClosesHandleOnReadError patches builtins.open (the
  SDK boundary) with a fake handle implementing the context-manager
  protocol, and asserts close() is called when read() raises. Verified
  against origin/master's version of parsedmarc/__init__.py: the test
  fails with "AssertionError: False is not true" (close was never
  called), confirming it catches the leak.
- testParseReportFileLeavesCallerHandleOpenOnReadError passes a
  caller-supplied fake handle directly as input_ and asserts close() is
  NOT called when read() raises. Verified against a naive whole-block
  try/finally (wrapping close() around all three branches instead of
  only the path branch): the test fails with "AssertionError: True is
  not false" (close was called), confirming it catches the scope
  widening this PR must avoid.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-09 18:44:38 -04:00
Sean WhalenandClaude Opus 5 95ddc8a323 Fix missing-file error paths falling through in find_unknown_base_reverse_dns.py (#903) 
Two `if not os.path.exists(path): print(f"Error: ...")` checks printed
the intended error message but had no sys.exit(1) after it, so execution
fell through into the subsequent open() call on the same missing path
and raised an unhandled FileNotFoundError instead of the clean error
exit the code clearly intended. Sibling duplicate-entry checks in the
same function already did print-then-sys.exit(1); these two now match.

Sites fixed (both in _main()):
- the nested load_list() helper's missing-file check (used for
  known_unknown_base_reverse_dns.txt and psl_overrides.txt)
- the base_reverse_dns_map.csv missing-file check

Both sites now have regression tests in a new TestFindUnknownBaseReverseDNS
class:
- the load_list() site: calling _main() in a temp directory with no
  known_unknown_base_reverse_dns.txt now raises SystemExit(1) instead of
  FileNotFoundError.
- the base_reverse_dns_map.csv site: _load_as_name_index() does its
  external work entirely through maxminddb.open_database(), the actual
  SDK boundary, so that call is mocked to a context manager over an
  empty iterable rather than loading the real ~23MB bundled MMDB or
  mocking an internal helper. Calling _main() in a temp directory with
  no base_reverse_dns_map.csv now raises SystemExit(1) instead of
  FileNotFoundError.

Both tests capture stdout and assert on the specific "Error: ... does
not exist" message, pinning the exit to the intended site rather than
any sys.exit(1) in the function. Cleanup uses two separate addCleanup
calls (rmtree registered before chdir, so LIFO order runs chdir first)
instead of one lambda wrapping both, so rmtree still runs even if
chdir were to raise.

Also fixed an adjacent prose bug a few lines from the second site: the
"is in known_unknown... and base_reverse_dns_map..." error message was
missing a space after "Error:" and was a backslash-continued f-string
that embedded the source's literal indentation in the printed output.
It now prints as a single clean line, consistent with the file's other
error messages.

Logged the user-facing symptom (clean error replaced by a
FileNotFoundError traceback) under CHANGELOG.md's Unreleased/Bug fixes
section, matching the project's precedent of logging maintainer-tooling
fixes (e.g. the sortlists.py entry).

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-09-09 22:44:15 +00:00
Sean WhalenandClaude Fable 5.1 09c88ca2a3 11.0.1 release: harden failure-report sample filenames and cap decompressed report size (#895) 
* Harden failure-report sample filenames and cap decompressed report size

Fixes the two open security advisories.

GHSA-c284-w5m6-jhjm (path traversal, affects 9.0.6 through 11.0.0):
save_output() named each failure report's message sample after the
sample's Subject header, falling back to the raw subject whenever
sanitizing it produced an empty string. A subject of only path
separators and dots -- "../../../" or "/" -- sanitizes to "", so the raw
value reached os.path.join() and the .eml landed outside the samples
directory or at an absolute path. That subject comes from a message that
failed authentication, so any sender a monitored mailbox accepts
controls it. The name is now sanitized at write time and falls back to
"sample", and the caller-supplied filename_safe_subject key is no longer
trusted. get_filename_safe_string() also strips NUL (which would
otherwise make open() raise ValueError: embedded null byte and hold back
the whole mailbox batch), truncates before stripping trailing
characters, and strips trailing spaces along with trailing dots, since
Windows drops both when creating a file; its docstring now states the
guarantees callers depend on.

GHSA-43qf-f35w-2x4r (unbounded decompression, affects all versions
through 11.0.0): extract_report() inflated gzip with one unbounded
zlib.decompress() and read zip members with an unbounded .read(). The
attachment's content is chosen by its sender, and deflate reaches about
1000:1 on degenerate input, so a 100 KB attachment inflated to 100 MB
with a ~209 MiB peak. Extraction now stops at
MAX_DECOMPRESSED_REPORT_SIZE (100 MiB) and raises ParserError. The gzip
path moves to a zlib.decompressobj() bounded by max_length, which does
not raise on a stream that ends early the way the one-shot call did, so
the helper checks decompressor.eof itself; a stream with trailing bytes
after the gzip member still extracts, matching the old behavior. Both
behaviors were observed against the unmodified code first and are
pinned by tests.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Strip NUL from CSV fields so Python 3.10's csv writer accepts them

Python 3.10's csv writer raises _csv.Error: need to escape, but no
escapechar set on any field containing NUL (CPython issue 97503, a 3.10
regression fixed in 3.11+). Failure report text fields (subject, user
agent, authentication results, addresses, etc.) come from untrusted
mail, so a NUL byte in one made parsed_failure_reports_to_csv() -- and
therefore save_output() -- raise on 3.10, which the CLI's
except (OSError, ValueError) around save_output does not catch. NUL is
now stripped from every CSV field on all Python versions via a shared
_csv_safe() helper, applied in all three CSV writers, so output is
identical regardless of interpreter version.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* 11.0.1 release: bump version and finalize changelog

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
 2026-09-03 15:51:44 -04:00
Sean WhalenandClaude Fable 5 07b00a931b Warn on unmatched CLI paths; send Kafka failure/SMTP TLS reports per-report (#890) 
Fixes the two remaining bugs and the nits uncovered during the review
session:

- The CLI logs a warning for each file_path argument that matches no
  files, instead of silently succeeding with empty results — a typo'd
  path in a cron job previously 'worked' forever while processing
  nothing (_expand_file_path_args dropped it as a zero-match glob).
  Deliberately a warning, not an error: a glob legitimately matching
  nothing must not break existing workflows, and mailbox-only runs
  stay silent.

- save_failure_reports_to_kafka and save_smtp_tls_reports_to_kafka now
  send one message per report, mirroring the aggregate saver's
  per-slice sends. Every released version documented per-record sends,
  but the code sent the whole list as one message, which a large batch
  — failure reports carry message samples — could push past Kafka's
  default 1MB message limit. Consumer-visible: these topics now carry
  individual report objects, not one JSON array per batch.

- The CLI module docstring and argparse description now mention SMTP
  TLS reports; usage.md's CLI-help mirror regenerated.

- The elastic/opensearch to_header display-name tests now assert the
  joined 'RT <rcpt@example.com>' string reaches the saved document
  (autospec save), plus the exactly-once call, instead of only that
  save ran.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-08-28 18:23:07 -04:00
Sean WhalenandClaude Fable 5 2d76de9ca6 Fix ignored number_of_replicas and add a --dns-timeout alias (#889) 
Two fixes from the #888 review cycle's flagged items:

- [elasticsearch]/[opensearch] number_of_replicas is no longer ignored
  when number_of_shards is not also set. The parser read replicas only
  inside the shards branch — accidental nesting introduced in the
  6.4.0-era code (1c9a6c4) — while docs/source/usage.md lists the two
  options independently and elastic.py/opensearch.py accept them as
  independent parameters with independent defaults (shards=1,
  replicas=0). Regression tests cover the replicas-only case for both
  sections.

- The CLI accepts --dns-timeout as an alias of --dns_timeout, which is
  kept unchanged for backward compatibility (public since 6.0.0;
  --dns-retries, added in 9.7.1, already hyphenated). The end-to-end
  test exercises both spellings through a real _main() run, so dropping
  either option string fails the suite. usage.md's CLI-help block is
  regenerated to match the new --help output.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-08-28 17:56:14 -04:00
Sean WhalenClaude Fable 5Copilot
 		07bca1ad28 Make the output and mailbox integrations optional extras (#888) 
* Make the output and mailbox integrations optional extras (#883)

Breaking change for the next major release: pip install parsedmarc now
installs the parsing core plus a working core CLI (file, IMAP, Maildir,
and mbox input; CSV/JSON, Splunk HEC, webhook, and syslog output).
Everything else moves behind an extra: elastic, opensearch, kafka, s3,
gelf, loganalytics, msgraph, and gmail, joining the existing postgresql
extra, with an umbrella [all] that deliberately excludes postgresql
(psycopg's binary wheels do not exist on every platform, so
parsedmarc[all] must never fail to install there).

cli.py imports the six SDK-dependent output modules behind the #884
TYPE_CHECKING/try-except guard; a configured section whose extra is
missing fails fast with a ConfigurationError naming the section and the
exact pip install command — including the msgraph and gmail_api mailbox
sections (detected via parsedmarc.mail's placeholder classes) and
postgresql (checked before the constructor so the startup retry loop
does not retry a missing dependency for a minute). The Azure/kiota Graph
error types fall back to never-raised sentinel classes.

The Docker image installs [all,postgresql], so container users see no
change. CI lint installs [build,all,postgresql]; the unit-test job
installs [build,all], deliberately without postgresql so
test_postgres.py's absent-psycopg arm stays exercised. The
never-imported dateparser dependency is dropped in favor of declaring
python-dateutil, which utils.py actually imports; pytz moves to the
build extra for the one test that uses it.

Verified live: a no-extras wheel install imports, parses samples, and
reports the install hint for each gated section; a [all] install
restores every integration; the Docker image builds with every SDK
importable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Patch psycopg presence in the PostgreSQL CLI wiring tests

CI's unit-test job deliberately installs [build,all] without the
postgresql extra, so parsedmarc.cli.postgres.psycopg is None there and
the new missing-extra presence check correctly made _main exit 1 before
the wiring under test ran. The tests simulate the SDK being available
(PostgreSQLClient is mocked at the SDK boundary), so the module-level
psycopg handle is now patched present in setUp. Verified against a
simulated psycopg-absent environment as well as the local full install.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address Copilot review: narrow guards to ModuleNotFoundError, fix docs

- The optional-integration and Graph error-type import guards now catch
  ModuleNotFoundError instead of ImportError, so only a genuinely absent
  package reads as a missing extra; a broken-but-present SDK fails
  loudly with its real error instead of masquerading as one. The test
  blocker raises ModuleNotFoundError accordingly — the exact exception a
  missing package produces.
- _missing_extra_hint docstring no longer calls every gated integration
  an output module (it also serves the msgraph/gmail_api mailbox
  sections).
- Fix the pre-existing passsword typo in usage.md's kafka section; the
  INI key the code reads is password (cli.py _parse_config).

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Quote extras specs in copy-paste install commands

From Copilot's second review round: zsh treats an unquoted .[build,all]
as a glob and fails with 'no matches found', so the commands shown in
AGENTS.md, CONTRIBUTING.md, dashboards/README.md, and the bootstrap
script's comment are now quoted. The CI workflows keep the unquoted
form: they run under bash, which passes unmatched globs through
literally. The suggestion to change the 'Choosing what to install'
heading level was rejected — it is a subsection of 'Installing
parsedmarc', matching the file's existing hierarchy.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Fix upgrade command in the changelog

* Documentation review: accuracy, spelling, grammar, and clarity pass

A full prose review of docs/source, README, CONTRIBUTING, and the
dashboards README, with every accuracy claim verified against the code
before changing it. Highlights:

- usage.md: documented six missing [general] options (the CSV/JSON
  filename options, prettify_json, normalize_timespan_threshold_hours),
  the required kafka smtp_tls_topic, [imap] timeout/max_retries, and
  the postgresql env-var prefix; corrected the maildir_path default
  (None, not INBOX — cli.py Namespace defaults), the mailbox
  check_timeout option name, the systemd restart interval (RestartSec
  is 5m), and merged the duplicate silent entry; quoted every
  copy-paste extras spec for zsh safety.
- elasticsearch.md: fixed an invalid openssl command (rsa:4096 -nodes),
  the dashboards filename (opensearch_dashboards.ndjson, matching the
  file the link serves), and assorted grammar.
- davmail.md: the service-enable command now enables davmail.service
  (was parsedmarc.service — a copy-paste error that left DavMail
  unenabled), plus a view typo and DavMail capitalization.
- output.md: the example schema reference is RFC 7489 Appendix C
  (7480 is RDAP). kibana.md: SPF relies on the SMTP envelope, not
  session headers (RFC 7208). dmarc.md: DKM -> DKIM.
- README: the intro now also names the OpenSearch/Grafana stack,
  matching the feature list. CONTRIBUTING: pre-PR checks now include
  ruff format --check and pyright, matching CI's lint job.
- dashboards/README: the service table and seed description now include
  the PostgreSQL backend the compose stack runs.

Sample data blocks, the CLI-help mirror block, and released CHANGELOG
entries were deliberately left untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Docstring review: accuracy, spelling, grammar, and clarity pass

Every docstring in parsedmarc/, parsedmarc/mail/, the maps maintainer
scripts, and the test suite reviewed with each claim verified against
the code it documents. Text-only — no behavior changes. Highlights:

- Copy-paste errors corrected: parsed_smtp_tls_reports_to_csv and
  splunk/loganalytics save functions described aggregate or failure
  reports they do not handle; LogAnalyticsException claimed to be an
  Elasticsearch error.
- Docstring/behavior mismatches: parse_report_email's report_type
  enumeration omitted smtp_tls; parse_failure_report typed msg_date as
  str (it is datetime); strip_attachment_payloads claimed payloads are
  replaced with None (the key is deleted); kafkaclient's failure and
  SMTP TLS savers claimed per-record slicing while sending the whole
  list in one message (docstrings now describe reality — whether
  slicing was intended is flagged for follow-up); the postgres savers
  claimed to take parse_report_file's return value but receive the
  inner report dict; elastic/opensearch save functions' Raises listed
  only AlreadySaved.
- None-as-semantic-state documented where missing (get_base_domain,
  get_ip_address_country), enumeration completeness fixed
  (get_ip_address_info's 9 result keys, maps script outputs, TSV
  columns), and the stale 44-industry-types count corrected to the
  46 the authoritative README list defines.
- Test docstrings aligned with what the tests actually assert,
  including two that overstated coverage of the elastic/opensearch
  address-list tests.
- Two argparse help strings fixed: file_path now names SMTP TLS report
  files alongside aggregate and failure, mirrored into usage.md's
  CLI-help block; --offline's doubled spaces removed (rendered help
  unchanged).
- elasticsearch.md's security claim corrected against Elastic's docs:
  security is enabled and auto-configured on first startup since 8.0
  (not "8.7 secure mode"), so the settings are verified, not
  hand-written.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
 2026-08-28 17:33:18 -04:00
Sean WhalenandClaude Fable 5 cd864f4cfc Add DNS over HTTPS and DNS over TLS support via the nameservers option (#886) 
Each entry in the existing nameservers option now selects its own
transport (#880): an IP address means plain DNS on port 53 exactly as
before, an https:// URL means DoH, and tls://ip[:port][#hostname] means
DoT, with the optional #hostname naming the TLS certificate identity
(systemd-resolved syntax). Forms can be mixed in one list, and no new
configuration option is involved.

DoH queries go through a shared per-process httpx client passed to
dns.query.https as session=, which is what makes them honor HTTP_PROXY/
HTTPS_PROXY/NO_PROXY and SSL_CERT_FILE — the motivating proxy-only
corporate network case. dnspython's stock DoH path cannot do this: it
builds its httpx client around a custom transport, and httpx only reads
proxy environment variables when no transport is supplied
(allow_env_proxies = trust_env and transport is None). The client is
rebuilt when the PID changes so fork-based worker pools never share a
parent's sockets.

The dnspython requirement becomes dnspython[doh]>=2.7.0 — the extra
supplies the httpx/h2 floors DoH needs, and 2.7.0 is the floor verified
against the dns.nameserver and dns.query.https(session=...) APIs used.

The startup DNS pre-flight check now exercises whichever transports are
configured, so a malformed DoH/DoT entry raises ConfigurationError
before any mailbox work begins. Malformed tls:// entries — including
the plausible slash-for-# typo tls://9.9.9.9/dns.quad9.net, which would
otherwise silently drop the certificate identity — are rejected at
configuration time naming the entry.

Verified live: DoH A/PTR queries against Cloudflare and DoT against
Quad9 (tls://9.9.9.9#dns.quad9.net), plus a full CLI run over a sample
report with encrypted-DNS-only nameservers.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-08-28 15:03:51 -04:00
ilyushaClaude Fable 5Sean WhalenCopilot Autofix powered by AI
 		ffdb220af9 Import optional mailbox connections lazily in parsedmarc.mail (#884) 
* Import optional mailbox connections lazily in parsedmarc.mail

* Fix docstring grammar

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Cover the lazy-import guards with tests and harden the AuthMethod placeholder

Maintainer follow-up to the review of #884:

- Add tests/test_mail.py: a sys.meta_path blocker simulates installs
  without the gmail/msgraph stacks, exercising both except branches.
  CI always has the extras installed, so without a simulated absence
  the guarded paths would never execute and the blocking codecov patch
  gate would fail. Includes isolation-safe purge/restore of sys.modules
  so test ordering cannot leak placeholder state into other suites.
- Replace the AuthMethod = None sentinel with a placeholder that raises
  the msgraph extra's ImportError on non-dunder attribute access,
  iteration, call, and subscript (every Enum usage shape), instead of
  surfacing cryptic NoneType AttributeError/TypeError.
- Raise a fresh ImportError chained to the stored original in the
  connection placeholders, so repeated constructions do not keep
  appending frames to one shared exception object's traceback.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Sean Whalen <44679+seanthegeek@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-08-28 13:16:08 -04:00
Sean WhalenandClaude Fable 5 758d1ffe49 Decode failure report MIME parts per their Content-Transfer-Encoding (#885) 
* Decode failure report MIME parts per their Content-Transfer-Encoding

Fixes #882.

parse_report_email() read every MIME part's payload without asking the
standard library to decode it, leaving any transfer encoding in place:

- A quoted-printable text/rfc822-headers sample part kept its RFC 2045
  §6.7 soft line breaks, which split long headers without RFC 5322
  folding whitespace. The sample's From header became unparseable and,
  with no Reported-Domain field in the report, the whole failure report
  was discarded with "TypeError: 'NoneType' object is not subscriptable".
- A quoted-printable message/feedback-report part parsed "successfully"
  with silently corrupted values (e.g. "dmarc=3Dfail").

A new _decode_mime_payload() helper decodes quoted-printable and base64
parts only, applied to the message/feedback-report and sample branches;
all other branches still receive the raw payload because they do their
own base64/magic-byte handling. Parts with a 7bit/8bit/absent CTE are
returned as-is: the message is parsed from a str, so compat32's
get_payload(decode=True) would round-trip the already-correct text
through raw-unicode-escape and corrupt non-ASCII characters. For nested
message/* parts (the stdlib nests every message/* subtype, so
decode=True returns None), the encoding is undone by hand, including
removing the header/body separator the Generator inserts when the
still-encoded text stops looking like headers mid-block
(MissingHeaderBodySeparatorDefect) — without that, values were truncated
at the first soft line break.

Also fixed in the process, per the same-PR rule for bugs found while
writing tests:

- feedback_report_regex captured the CR of CRLF line endings (RFC 5322
  §2.1 mandates CRLF; re.MULTILINE's "$" matches before the LF, not the
  CR). Previously masked because the base64 branch decoded through a
  bytes repr and stripped literal "\r" escapes.
- A report with no Reported-Domain field and no parseable sample From
  domain now raises InvalidFailureReport with a clear message instead of
  the opaque TypeError; reported_domain is a required str in the
  FailureReport contract (types.py) consumed unconditionally by the
  Elasticsearch/OpenSearch outputs, so defaulting it to None is not an
  option. InvalidFailureReport raised inside parse_failure_report() now
  propagates without the "Unexpected error:" re-wrap.

CLI output over the whole sample corpus is byte-identical before and
after (PYTHONHASHSEED=0, n_procs=1).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Use RFC 5322 header folding instead of implicit string concatenation

The hand-built test message's long Content-Type header was split across
two adjacent string literals inside a list, which reads like a missing
comma (flagged by code review). Fold the header with a tab continuation
line instead — truer to the wire format the builder exists to produce.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Reword test builder docstring to claim only what it does

Copilot review: the builder joins lines with "\n" and embeds CRLF inside
the feedback-report block, so it does not preserve on-the-wire bytes
exactly. What matters for the test is only that the non-ASCII sample
text stays unencoded; say that instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-08-28 12:14:07 -04:00
Sean WhalenandClaude Opus 5 b304cf8639 Cover index_prefix_domain_map and index_suffix in index migrations (#868) (#869) 
* Cover index_prefix_domain_map and index_suffix in index migrations (#868)

The startup Elasticsearch/OpenSearch index migrations built their target
index names from the [elasticsearch]/[opensearch] index_prefix and
index_suffix options alone, while the save path also honors
general.index_prefix_domain_map. A multi-tenant deployment therefore ran
the backfill guard `count` against dmarc_aggregate*/smtp_tls*, patterns
matching none of its real <tenant>_dmarc_aggregate-* indexes. The query
passes allow_no_indices=True, so a zero-match wildcard returns count 0 --
indistinguishable from "already backfilled" -- and the backfill was
skipped silently, with no log line at any level.

Resolve migration index names through a new _migration_index_names()
helper that widens both configurable axes: one name per tenant prefix in
the map plus the unprefixed name (unmapped domains are still saved
unprefixed), and, when an index_suffix is set, the unsuffixed name
alongside the suffixed one so history predating the suffix is covered. A
configured index_prefix still wins outright and suppresses the map
fan-out, matching save-time precedence. The key normalization is now
shared with get_index_prefix() via _normalize_index_prefix(), so the
names parsedmarc migrates cannot drift from the ones it writes. The
resolved lists are logged at DEBUG, and the SIGHUP reload path passes the
freshly parsed map, so a newly onboarded tenant is covered without a
restart.

Also repair the legacy published_policy.fo migration, which has been
unable to complete since mapping types were removed in Elasticsearch 7
(and never existed in OpenSearch): it read the field mapping in the
type-keyed response shape, so the check always fell through, and its
put_mapping() call passed a doc_type argument neither current client
accepts. It now reads either response shape, uses each client's current
signature, and takes its index names in a separate legacy_fo_indexes
argument -- exact names, since 5.0.0 introduced date-suffixed index names
in the same release that fixed the fo declaration, but prefixed and
suffixed where configured, since both options date back to 4.1.0. Tenant
prefixes are excluded: index_prefix_domain_map arrived in 8.19.0, and
this migration renames the index it rebuilds. The Elasticsearch copy,
removed as unreachable during the #806 client migration, is restored now
that the cause is understood.

Finally, reject an index_prefix_domain_map YAML file that is not a
mapping of string tenant names to lists of domains, instead of raising
mid-save on a non-string key or silently matching the wrong domains on a
scalar value -- `in` on a str is a substring test, so "example.co"
matches "example.com"
(https://docs.python.org/3/reference/expressions.html#membership-test-operations).

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

* Fix OpenSearch capitalization and spacing in the usage docs

Copilot review of #869: the `index_prefix_domain_map` option line spelled
"OpenSearch" as "Opensearch" and had a doubled space before the type. Both
predate this branch but sit inside a hunk it rewrites. Also wrapped the line
to match the continuation-indent style of every other option in the list, and
corrected the same misspelling in the multi-tenant section a few lines above
the paragraph this branch added.

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

* Require index_prefix_domain_map domain lists to hold strings

Copilot review of #869: the shape check verified that each value was a
list but not what the list contained, so `tenant_a: [42]` passed and then
never compared equal to any domain the save path looks up -- the same
silent-misbehavior class the check exists to reject, and a contradiction
of the "any other shape is rejected at startup" claim in the docs.

Check the list's items too, and reword the error message, comment,
CHANGELOG and docs to state the rule the check now enforces: a mapping of
tenant names to lists of domain names, all strings.

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

* Make the legacy fo migration retry-safe, and nest its mapping body

Copilot review of #869, verified against live Elasticsearch 8.19 and
OpenSearch 3 containers rather than mocks.

Retry safety (a real defect): an attempt interrupted between creating the
-v2 index and deleting the original left debris that made create() fail
with "resource already exists" on every later startup, inside the same try
that swallows the error -- so the index was never migrated and the debris
document survived. Reproduced on a live cluster. Discard a leftover target
first; that is safe precisely because reaching this point means the
original still holds the data, since it is deleted only once the reindex
has succeeded.

Mapping body: the reviewer's concern that a dotted key under `properties`
risks a runtime failure does not hold -- both clusters accept it and
produce a byte-identical mapping, with the dot expanded into
published_policy -> properties -> fo. Switch to the nested object form
anyway, since dot expansion is conditional on the object's `subobjects`
setting and this shape never is, and derive the object/leaf names from the
dotted constant so the write cannot drift from the field the read looks up.

Both fo-migration suites now build per-name Index mocks. A single shared
mock cannot express "the original exists but its migration target does
not", which is the ordinary case and the one the retry fix turns on; the
happy path now also asserts the target index is never deleted.

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

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-07-27 20:48:00 -04:00
Sean WhalenandClaude Fable 5 5c7f4ba048 Clear shared IP cache in parallel test setUp for isolation (#866) 
tests/test_parallel.py's parity test compares sequential
parse_report_file(path, offline=True) results in the parent process
against results from cold worker processes. When the full suite runs
locally (GITHUB_ACTIONS unset), tests/test_init.py runs first with real
DNS lookups and warms the shared module-level
parsedmarc.IP_ADDRESS_CACHE; get_ip_address_info consults the cache
before honoring offline, so the sequential baseline returned
DNS-enriched entries (e.g. reverse_dns='smtp7.cardinal.com') while the
workers correctly returned None, failing the test. CI never sees this
because it runs offline from the start.

Clear the cache in _ParallelTestCase.setUp so baselines and workers
both start cold. Test-only change; the cache-before-offline ordering in
utils.py is intentional (a cache hit makes no network queries).

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-26 18:41:08 -04:00
Sean WhalenandClaude Fable 5 35e51218b9 Don't archive/delete mailbox messages until output destinations confirm the save (#863) 
* Don't archive/delete mailbox messages until output destinations confirm the save (#242)

get_dmarc_reports_from_mailbox() gains a keyword-only save_callback,
invoked once per fetched batch before any message is deleted or moved.
A False return (or an exception) marks the batch unsaved: its messages
stay in the reports folder for retry, and the aggregate-report dedup
keys staged for that batch are dropped so the retry reparses instead of
skipping. watch_inbox() passes its callback through as save_callback,
so watch mode gets the same protection.

To bound duplicate delivery to destinations that don't deduplicate, a
new [mailbox] max_unsaved_retries option (default 2) caps retries: a
message whose batch has failed the initial attempt plus that many
retries moves to {archive_folder}/Unsaved -- never deleted, whatever
the delete options say. Counts are process-local, so the cap applies
across watch-mode checks; one-shot runs retry indefinitely, which is
the safe direction. A raising callback (the CLI's, under
fail_on_output_error) is counted against the cap the same way before
the exception is re-raised, since mailsuite's IMAP and Maildir watch
loops swallow exceptions and keep checking.

CLI: process_reports() now returns its output-error list and the
mailbox_save_callback adapter feeds that verdict to the library;
save_output() failures (the one uncaught destination) are recorded like
every other sink's; file/mbox-derived reports are saved in a separate
pass from mailbox batches so nothing is saved twice; the combined
results fed to email_results() are filtered by index_prefix_domain_map
explicitly, restoring the SMTP TLS filtering the emailed summary lost
when saving moved into per-batch callbacks.

Credit to @mkilijanek for the original approach in #823.

Fixes #242

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address automated review feedback on #863

- Keep a message's failure counter until its move to Unsaved actually
  succeeds. The counter was popped when the message was classified
  over-cap, before the move was attempted, so a failed move handed the
  still-in-place message a fresh set of under-cap retries and duplicate
  deliveries; now the next failed save classifies it over-cap again and
  re-attempts the move. Regression-tested at cap 1, where the reset
  would observably leave the message in the INBOX instead of moving it.
- Use sys.exit(1) instead of the site-dependent exit() built-in in the
  two new ParserError handlers.
- Close the sample files opened by the new Maildir tests via context
  managers.
- Keep the docs' section-link text on one line and fix the adjacent
  pre-existing "a IMAP" typo.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address Copilot re-review feedback on #863

- Document mailbox_save_callback's raise path: with fail_on_output_error
  it raises ParserError via process_reports() instead of returning
  False, and the library counts that as an unsaved batch too.
- Use str(error_) instead of error_.__str__() in the new File output
  handler.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Reject negative max_unsaved_retries with a ValueError

The option is user-configurable (INI/env/kwarg); a negative value
silently behaved like 0. Both get_dmarc_reports_from_mailbox() and
watch_inbox() now validate it at the door alongside the existing
test/delete guard. watch_inbox() validates before entering the watch
loop, because a ValueError raised inside a check would be swallowed
and endlessly retried by the IMAP and Maildir backends' per-check
exception handling instead of surfacing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Fix two pre-existing bugs surfaced by review of #863

- MAGIC_JSON was b"\7b" -- the octal escape \7 (BEL) plus a literal
  "b", not 0x7B, the "{" every RFC 8259 JSON object begins with -- so
  extract_report() rejected plain uncompressed JSON. Every in-tree
  caller pre-guarded with its own zip/gzip or "{" check, masking the
  dead branch; the practical impact was extract_report() as a public
  API and application/tlsrpt+gzip attachments whose payload is really
  uncompressed JSON. Now b"\x7b".

- get_index_prefix() unconditionally indexed policies[0], while
  parse_smtp_tls_report_json() accepts a report whose policies list is
  empty -- an IndexError crash whenever index_prefix_domain_map was
  configured. An empty-policies report has no domain to map, so it is
  now treated as unmappable and excluded from prefix-mapped output.
  This code was moved into filter_smtp_tls_reports_for_index_prefix()
  by this PR, which widened its exposure to the email_results() path.

Both regression tests were verified to fail against the unfixed code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Rewrap the max_unsaved_retries docstring paragraph

The ValueError sentence was inserted without reflowing, leaving a
112-character line in a docstring wrapped at ~76; ruff formats code,
not docstring prose, so it slipped through lint.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-26 17:07:03 -04:00
Sean WhalenandClaude Fable 5 ae5f4c7915 Add per-report-type mailbox delete options (#858) 
* Add per-report-type mailbox delete options (#256)

Add four new [mailbox] options — delete_aggregate, delete_failure,
delete_smtp_tls, and delete_invalid — each defaulting to the value of
the overall delete option and individually overridable, so e.g.
delete = True with delete_failure = False deletes processed aggregate
and SMTP TLS report messages while archiving failure reports, and
delete_invalid = False keeps unparseable messages in the Invalid
archive subfolder for debugging.

get_dmarc_reports_from_mailbox() and watch_inbox() gained matching
bool | None keyword arguments (None = inherit from delete), resolved
once up front; the delete/test mutual-exclusion guard now checks the
effective per-type flags. The Gmail deletion-scope guard covers any
effective flag and forces all five options off when the scope is
missing. PARSEDMARC_MAILBOX_DELETE_* env vars work automatically.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Document per-type delete kwargs as bool | None with explicit inheritance

Copilot review: the docstrings typed the four per-report-type delete
parameters as plain bool, but None (the default) is the inheritance
mechanism — a library caller couldn't tell from the docs that None,
not False, means "inherit delete".

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Cover disposal error branches; scope the config= docstring claim

Codecov flagged the delete-error handler in the new per-type disposal
loop as the one uncovered patch line. Add two Maildir tests driving a
backend whose first delete/move call raises: the error is logged, the
affected message stays in the INBOX, and disposal continues to the
next report type. The move-error branch gets the symmetric test from
the same harness.

Copilot review: the config= docstring paragraph claimed all keyword
arguments listed above it are ignored when config= is provided, which
now falsely included the four per-type delete options. Scope the claim
to the parsing/enrichment arguments ParserConfig actually carries and
state that mailbox-handling arguments always apply.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-26 13:31:37 -04:00
Sean WhalenandClaude Fable 5 264b9a4556 Add [general] archive_directory to archive processed local files (#570) (#856) 
* Ignore Claude Code agent worktrees under .claude/worktrees/

Untracked repo snapshots from agent sessions were making repo-root
ruff check . fail on stale code and cluttering git status. ruff
respects .gitignore, so ignoring the directory fixes both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Add [general] archive_directory to archive processed local files (#570)

Move successfully processed report files given as local path arguments
into <archive_directory>/<year>/<month>/<Aggregate|Failure|SMTP-TLS>/,
dated from the parsed report's own metadata (aggregate begin_date,
failure arrival_date_utc, SMTP TLS begin_date) rather than the
filename. Files that fail to parse as a report (ParserError) go to
<archive_directory>/Invalid/; other failures (e.g. transient I/O
errors) leave the file in place so a later run can retry it.

Existing destination files are never overwritten: each candidate name
is claimed atomically (O_CREAT | O_EXCL) and collisions get a numeric
suffix before the extension. Files already inside the archive
directory are excluded from processing (compared via realpath so
symlinked spellings still match), so the archive can safely live
inside an input directory, as the issue requests. mbox files and
mailbox modes are unaffected; mailbox modes keep [mailbox]
archive_folder.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address review feedback on docstrings and the placeholder-cleanup except

- _move_file_to_archive's docstring no longer claims the copy2 fallback
  replaces the placeholder atomically; only the same-filesystem os.rename
  path is atomic. The fallback is a plain copy-and-overwrite, which is
  still collision-safe because the placeholder already claimed the name.
- The empty except OSError in the placeholder cleanup now carries a
  comment explaining that it is deliberate best-effort cleanup and the
  re-raised move failure is the actionable error.
- test_general_archive_directory_unset_leaves_attribute_absent's
  docstring now describes what the assertion actually tests (the
  attribute staying absent from a bare Namespace) and moves the
  real-CLI None-default behavior to a parenthetical.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Cover the two defensive exception branches Codecov flagged

Codecov's patch report flagged four uncovered lines, all in the two
platform-dependent exception branches of the archive helpers:

- _exclude_archived_paths's except ValueError branch. Per the Python
  docs for os.path.commonpath, ValueError is raised when paths "are on
  the different drives" (Windows) or mix absolute and relative
  pathnames; both inputs are realpath()-resolved so only the
  different-drives case remains, which Linux CI can't produce
  naturally. The new test simulates the raise and asserts the
  non-comparable path is kept for parsing rather than excluded.

- _move_file_to_archive's except OSError placeholder-cleanup branch.
  The new test fails the move with a non-OSError type and the cleanup
  with OSError, then asserts the move error is what propagates (the
  cleanup error is swallowed, not allowed to mask it) and that the
  zero-byte placeholder survives its failed cleanup.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Tag arrival_date_utc as UTC when parsing the archive date

human_timestamp_to_datetime's docstring names arrival_date_utc as
exactly the kind of known-UTC naive string that should be parsed with
assume_utc=True; _archive_subdir_for_result was parsing it naive.

The flag is scoped to the failure branch because the shared call also
handles the other two report types: aggregate begin_date is a
local-time wall-clock string (timestamp_to_human uses
datetime.fromtimestamp), so tagging it UTC would be wrong, and SMTP TLS
begin_date carries an RFC 3339 offset, making assume_utc a no-op.

No observable behavior change: only the wall-clock year/month fields
are read and assume_utc never shifts wall-clock time, so no new test
can honestly distinguish the two versions — this aligns the call with
its dependency's documented contract and hardens against a future edit
adding a real timezone conversion.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Create the archive placeholder with mode 0o600

os.open's mode parameter defaults to 0o777 (masked by the umask), so
the O_CREAT|O_EXCL placeholder in _move_file_to_archive was created
executable and group-accessible on typical umasks (0o775 under umask
002). Normally it's replaced immediately, but a placeholder that
outlives a failed move+cleanup persisted with those permissions.

Pass 0o600 explicitly. The mode never reaches the real archived file:
os.rename replaces the placeholder's inode outright, and the copy2
fallback's copystat overwrites the mode with the source file's. The
leftover-placeholder regression test now also asserts the surviving
placeholder has no owner-exec or group/other bits (umask-independent,
since the umask only clears bits); the assertion fails against the
unfixed default-mode call.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-26 11:56:01 -04:00
Sean WhalenandClaude Fable 5 4e80047e68 Centralize configuration handling with ParserConfig (#503) (#851) 
* Centralize configuration handling with ParserConfig (#503)

Add parsedmarc/config.py with ParserConfig, a frozen dataclass carrying
every parsing/enrichment option plus the three shared caches (IP address
info, seen aggregate report IDs, reverse DNS map). All eight public
parsing/mailbox functions accept a keyword-only config= argument; when
provided, the individual option keyword arguments are ignored in favor
of the config's values, and every existing per-option keyword argument
keeps working unchanged. The three hand-copied parse_kwargs dicts and
the dns_timeout<->timeout rename chain are gone; the CLI builds one
ParserConfig per run (rebuilt on SIGHUP) and passes it everywhere.

Explicitly constructed configs own fresh isolated caches;
dataclasses.replace() shares them; pickling drops cache contents and
rebinds the unpickling process's module defaults, preserving the
per-worker cache behavior of n_procs parallel parsing. The module
globals IP_ADDRESS_CACHE / SEEN_AGGREGATE_REPORT_IDS / REVERSE_DNS_MAP
remain, identity-preserved, as re-exports of the default caches.

Bug fixes that ride along, each with a regression test:

- One-shot mailbox runs now honor [general] dns_timeout/dns_retries;
  the CLI call site never forwarded dns_timeout, so the library's
  stray 6.0 default silently applied.
- Lazily-triggered reverse DNS map loads (get_ip_address_info /
  get_service_from_reverse_dns_base_domain, including in n_procs
  workers) now thread psl_overrides_path/psl_overrides_url through to
  load_reverse_dns_map instead of clobbering operator-configured PSL
  overrides with the bundled defaults.
- get_dmarc_reports_from_mailbox() and watch_inbox() dns_timeout
  defaults unified to DEFAULT_DNS_TIMEOUT (2.0s) from a stray 6.0, and
  normalize_timespan_threshold_hours to the float 24.0 used everywhere
  else.

Closes #503

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address CI and review feedback on #851

- Wrap the IMAPConnection example in usage.md so ruff format is clean
  over the docs code blocks (CI runs ruff format --check on the whole
  repo; the local runs were scoped to parsedmarc/ and tests/ and
  missed it).
- Fix the pre-existing "URL ro a reverse DNS map" docstring typo in
  get_service_from_reverse_dns_base_domain, caught by Copilot on the
  adjacent hunk.
- Import parsedmarc.config once, as an aliased plain import, in
  tests/test_config.py instead of mixing import and import-from of the
  same module (flagged by code quality scanning); the aliased module
  import also keeps pyright able to resolve the submodule attribute
  access.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address second Copilot review round on #851

- Add parse_aggregate_report_file() to the library entry-point list in
  usage.md; the following paragraph describes the config= contract for
  "each of these functions", so the list must name all eight
  config-accepting entry points.
- ParserConfig.__setstate__ now initializes every non-cache field to
  its class default before applying the pickled state, so a config
  serialized by an older parsedmarc version (whose state predates
  fields added later) unpickles with the newer fields at their
  defaults instead of unset entirely (__init__ never runs during
  unpickling, so an absent field would raise AttributeError on first
  access). Covered by a regression test that feeds __setstate__ a
  partial state dict.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-25 19:27:09 -04:00
Sean WhalenandClaude Fable 5 48445c639e Extend n_procs parallel parsing to mbox and mailbox sources (#147) (#849) 
* Extend n_procs parallel parsing to mbox and mailbox sources (#147)

n_procs previously applied only to report files passed directly as CLI
arguments; messages from mbox files and mailbox connections (IMAP,
Microsoft Graph, Gmail API, Maildir) were always parsed sequentially.

A new parsedmarc.parallel module provides a shared bounded-window
ProcessPoolExecutor helper (parallel_map) used by all three input paths.
Only parsing fans out to a reused worker pool; message fetching, report
deduplication, mailbox archiving/deletion, and output stay sequential in
the main process. The submission window keeps at most ~2*n_procs
messages in flight, so memory stays bounded even for huge mboxes, and
the mailbox path fetches messages lazily on the connection-owning main
thread. keep_alive never crosses the process boundary - the main
process sends periodic IMAP keepalives while workers parse - and with
n_procs > 1, invalid-message disposition happens after the parse phase,
mirroring the existing deferred bulk archive moves.

get_dmarc_reports_from_mbox, get_dmarc_reports_from_mailbox (including
its tail-recursive re-check), and watch_inbox gain an n_procs keyword
argument (default 1); sequential behavior at the default is unchanged.

Replacing the CLI's hand-rolled Pipe/Process batching also fixes two
defects in the direct-file path: a child process that died from a
non-ParserError exception left the parent blocked forever on
conn.recv(), and the hard batch barrier let one slow file idle every
other worker slot. Workers are now a reused pool (no fresh interpreter
per file), with worker logging reconstructed via a spawn-safe pool
initializer instead of fork inheritance.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address Copilot and code-quality review feedback on #849

- parallel_map now validates n_procs >= 1 itself with a clear error
  instead of surfacing ProcessPoolExecutor's max_workers error later.
  The check raises eagerly at the call (the generator body moved into an
  inner function) rather than on first iteration, with a regression test.
- Aligned parallel_map's should_stop docstring with the implementation:
  queued-but-unstarted jobs are cancelled, while in-flight jobs are
  waited on and their results yielded, so the stop can block briefly but
  never discards completed work.
- The parallel mailbox path keeps fetched message ids in a deque popped
  as each in-order result arrives, so the id queue stays bounded by the
  submission window instead of growing to message_limit.
- Closed the three sample-file handles the new tests opened without a
  context manager.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address second round of Copilot feedback on #849

- configure_logging no longer stacks duplicate FileHandlers when called
  again with the same log_file (compared by FileHandler.baseFilename,
  which stores the absolute path): a duplicate wrote every record twice
  and leaked a file descriptor per call, e.g. across SIGHUP config
  reloads. Latent in the pre-extraction cli._configure_logging too.
  Regression tests in the new tests/test_log.py.
- Renamed the CHANGELOG's premature "10.4.0" heading to "Unreleased",
  matching the project convention where in-progress entries accumulate
  under Unreleased and the release PR renames the section and bumps
  parsedmarc/constants.py together (as in the 10.3.0 release).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-25 17:14:40 -04:00
Sean WhalenandClaude Fable 5 0a95a0ceb2 Upgrade ruff to 0.16.0 and pyright to 1.1.411; convert to f-strings (#847) 
* Upgrade ruff to 0.16.0 and pyright to 1.1.411; convert to f-strings

ruff 0.16.0 expanded the default lint rule selection well beyond the
long-standing E4/E7/E9/F, so [tool.ruff.lint] now selects the rule set
explicitly: the pre-0.16 defaults plus the modern-type-hint UP rules and
the two f-string rules (UP030/UP032). All 352 UP030/UP032 findings were
auto-fixed; conversions requiring Python 3.12 f-string quote reuse were
conservatively left as .format() by ruff (verified: the whole package and
test suite byte-compile under CPython 3.10.20, the oldest CI version).
Adopting the other newly-default rule families (BLE, SIM, C4, DTZ, I, ...)
is deferred as a deliberate per-family decision.

ruff format with 0.16.0 also now formats Python code fences in Markdown,
which reformatted one block in parsedmarc/resources/maps/AGENTS.md.

ruff check, ruff format --check, pyright (0 errors/warnings), and the
full test suite (775 passed) are green on the new versions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address Copilot review findings on the f-string conversion

- Rewrite messages that used backslash line continuations inside string
  literals, which embedded the source indentation as literal whitespace
  in the logged/raised text: the duplicate search-error messages in the
  Elasticsearch and OpenSearch outputs, the 'since'-option warning (which
  also implicitly concatenated "24hrs" and "SMTP" with no separator) and
  the IMAP 'since' debug line, and the missing-org_name KeyError message.
- Build the Splunk HEC newline-delimited payloads by appending to a list
  and joining once instead of quadratic string concatenation in a loop.

Declined: switching the webhook output's logger.error to
logger.exception — the single-line ERROR without a traceback is the
deliberate house pattern for batch-resilient sinks, and changing log
verbosity is out of scope for this refactor PR.

ruff check/format, pyright (0 errors/warnings), 775 tests, and a
CPython 3.10 compileall all pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-25 13:46:36 -04:00
Sean WhalenandClaude Fable 5 23c5ea9ad6 Fix DKIM/SPF and SMTP TLS detail-table cross-products in dashboards (#169) (#839) 
* Fix DKIM/SPF alignment detail cross-product in dashboards (#169)

Elasticsearch and OpenSearch dynamic-map the dkim_results/spf_results
object arrays as `object` (create_indexes never registers the DSL
document mappings), so Lucene flattens each array into independent
multi-valued fields and stacked terms aggregations on
dkim_results.selector/.domain/.result return every combination of
values across a report's signatures — each phantom row repeating the
full message count.

Aggregate documents now also carry dkim_results_combined and
spf_results_combined: one "selector / domain / result"
("scope / domain / result") string per auth result, composed in
add_dkim_result/add_spf_result. The Kibana/OpenSearch Dashboards and
Grafana (Elasticsearch) alignment-detail tables aggregate those
instead, and the Splunk detail panels pair the values with
mvzip/mvexpand. A documented idempotent _update_by_query backfills
documents saved by older versions; the query matches only documents
that have auth results and lack the combined fields, because an
`exists` query cannot see an empty array.

Also corrects the dead _SPFResult.results (plural) declaration to
`result` (the save path always wrote the singular key), fixes the
result parameter annotations on add_dkim_result/add_spf_result, and
removes the Grafana dmarcian.com DKIM-checker data link, which
required the separate domain/selector columns.

The SMTP TLS visualizations have the same class of defect and are
tracked separately.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address Copilot review findings on #839

Reword the combined-field regression test docstrings: the DKIM/SPF
auth results are dynamic-mapped as plain `object`, not the `nested`
mapping type the previous wording implied — the distinction is the
crux of the fix. Also drop the inert renameByName entries Copilot
flagged on the Grafana Overview and DKIM Alignment Details panels,
which referenced fields those panels' queries no longer (or never)
produced.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Add PR #839 review lessons to AGENTS.md

Extend the "Review passes cover prose" section with two rules from
the #839 Copilot findings: docstrings/comments get the same
text-level review pass as docs and dashboard labels (with suspicion
for dual-use terms like "nested" near Elasticsearch code), and
inert config entries inside hunks a PR already rewrites should be
cleaned rather than preserved to minimize the diff.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Reflow create_indexes comments flagged by Copilot

The line wrap placed "#169" directly after the comment marker, so the
raw source read "# #169;". Reword so the issue reference stays on one
line.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Extend the hunk-proofreading rule with rendered-text wraps

Fold the PR #839 second-round Copilot lesson into the existing rule:
proofread how wrapped lines render (comment markers, punctuation at
wrap points), not just the wording itself.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Rename Overview combined-result column labels (Copilot round 3)

The Overview table's "DKIM Auth Result" / "SPF Auth Result" labels
were kept when the columns switched to the combined
"selector / domain / result" values, leaving the headers misleading.
Rename them to match the detail panels' convention and retarget the
byName width overrides that matched the old labels, widening them for
the longer values.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Document per-signature row semantics in the alignment tables

A message carrying multiple DKIM signatures appears once per signature
in the details tables, so summing the messages column across rows can
exceed the total message count. State that explicitly rather than
leaving readers to infer it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address Copilot round-4 findings on dashboards

Fix three pre-existing saved-object title typos in the OpenSearch
ndjson (leading space on "Aggregate DMARC passed DMARC", trailing
space on "Aggregate DMARC reporting organizations", double space in
"map  of message sources by country"), in both the top-level title
and the embedded visState title.

Normalize the Splunk DKIM details placeholders: the base search's
fillnull renders wholly-missing DKIM fields as the literal string
"null", so unsigned mail showed "null / null / null" while the SPF
panel shows "none". Rewrite the values to "none" after the signature
split, where the fields are single-valued and the mvzip pairing
cannot be disturbed. Verified against the dev Splunk that no
truncation or mis-pairing occurs either way, since fillnull
guarantees the fields are never actually null.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Backfill combined DKIM/SPF fields automatically at startup

migrate_indexes() now backfills dkim_results_combined and
spf_results_combined on aggregate documents saved by older versions,
so ES/OS users get historical data in the reworked alignment tables
without running the documented _update_by_query by hand. The backfill
is submitted as a non-blocking background task
(wait_for_completion=false, conflicts=proceed) guarded by a cheap
count query, making repeated startups a fast no-op once an index is
backfilled; any cluster error is logged as a warning and retried at
the next startup rather than raised. The manual command remains
documented for users who upgrade dashboards without pointing the new
parsedmarc at the cluster or who want to control write-load timing.

The legacy published_policy.fo long-to-text reindex migration in the
OpenSearch module is kept ahead of the new backfill, for clusters
upgraded from very old data.

Verified end-to-end against the live dev environment: a real CLI
startup backfilled 9 stripped OpenSearch documents (logged with task
ID) while the already-backfilled Elasticsearch side stayed silent,
and a second startup was silent on both engines.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Match backfill guard on either domain or result subfield

End-to-end upgrade testing (real parsedmarc 10.2.4 ingest, then a
branch startup) surfaced that an exists query cannot see an empty
string: a text field with no tokens is invisible to exists. The
parsers we audited never store an auth result with an empty or
missing domain — they drop such entries entirely, so the previous
domain-only guard was sufficient for their data — but the storage
shape of every historical parsedmarc version can't be audited, so
the guard (and the documented manual command) now matches either
the domain or the result subfield per protocol. Matching either
costs nothing and cannot skip a document that has something to
backfill.

Verified by recomputing expected combined values from _source for
all 2,299 documents on both engines: every document with stored
auth results has exactly the recomputed pairs, and the legacy fo
migration correctly did not fire on typeless indexes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Add auth-result filter controls to the Kibana/OSD aggregate dashboard

The combined per-signature columns fixed the #169 cross-product but left
no way to click-filter by an individual selector, domain, or result. Add
an "Aggregate DMARC auth result filters" input_control_vis panel above
the SPF/DKIM details tables with six option-list dropdowns (DKIM
selector/domain/result, SPF scope/domain/result) that emit ordinary
dashboard-wide filter pills. Works on both Kibana 8.19 and OpenSearch
Dashboards 3, verified by driving the controls in both UIs against the
issue's two-signature repro report.

Documented in kibana.md, including the flat-mapping caveat: combining
two component filters matches documents where any signature satisfies
each condition individually.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Scale Grafana source-country map markers with message volume

The "Map of Message Source Countries" panel drew fixed 5 px dark-green
markers at 50% opacity — nearly invisible on the dark basemap, so the
panel read as empty even when data was flowing (verified via the query
API). Markers now scale with Sum(message_count) (min 4, max 30 px) at
0.8 opacity in a higher-contrast green. Pre-existing issue; the
identically-styled failure-dashboard map panel is intentionally left
untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Correct the nested-mapping rationale in the create_indexes comments

The comments claimed Kibana/OSD/Grafana "cannot terms-aggregate fields
inside a nested mapping" — too absolute. Fact-checked empirically and
against primary docs: Kibana/OSD visual editors (Lens and classic
Visualize) do not support nested fields, but Vega panels can run nested
aggregations (they just cannot render tables, per Elastic's docs), and
Grafana >= 9.4 has a nested bucket aggregation (grafana/grafana#62301)
but no reverse_nested, so parent-level metrics like Sum(message_count)
return 0 inside per-signature buckets (reproduced live). Conclusion
unchanged: the dynamic object mapping stays load-bearing for the
shipped dashboards. PR #839's body was updated with the same
correction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Pin ruff exactly, matching the existing pyright pin rationale

CI installs the [build] extra fresh on every run, and ruff was the one
lint tool left unpinned. ruff 0.16.0 (released this week) began
flagging this codebase's str.format() house style, so every PR started
failing lint on lines it never touched. Pin to 0.15.21 — the version
the codebase is clean under — with the same bump-deliberately comment
pyright carries. Upgrading to 0.16 and converting to f-strings can be
its own PR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address Copilot findings: harden migrate_indexes, normalize panel titles

Three unresolved review threads, all verified against cli.py's
re-raising init handler before fixing:

- elastic.py/opensearch.py: connections.get_connection() sat outside
  migrate_indexes()'s try/except, so a connection-registration failure
  would abort startup despite the docstring's promise that migration
  errors are caught and logged. Now caught, logged, and skipped until
  the next startup.
- opensearch.py: the legacy published_policy.fo migration loop did
  unguarded network I/O (exists/get_field_mapping/reindex/delete), so a
  transient cluster error aborted startup on the OpenSearch path while
  the identical situation on the Elasticsearch path was logged and
  survived. Each index's migration attempt is now wrapped, warns, and
  moves on.
- opensearch_dashboards.ndjson: normalized two pre-existing panel
  titles in the aggregate dashboard's panelsJSON ("Reporting
  organizations " trailing space, "Map  of message sources by country"
  double space).

Regression tests assert migrate_indexes never propagates connection or
per-index cluster errors on either backend.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Clarify that Nested() on auth-result fields is in-memory shape only

Copilot flagged that _AggregateReportDoc declares dkim_results and
spf_results with Nested(...) while the create_indexes comment insists
the stored mapping must stay dynamic `object`. Both are true: the
Nested declaration only shapes the DSL's in-memory document building
and is never installed as a mapping, because create_indexes skips
Index.document() registration. Say so at both sites, in both backends,
so nobody "fixes" the mismatch by registering the mapping — which
would install real nested mappings and blank the dashboards.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Refer to the filter panel by its displayed title in docs and CHANGELOG

The dashboard convention is a short panel display title backed by a
long-form saved-object name ("SPF details" / "Aggregate DMARC SPF
details"), and the new controls panel follows it. The docs and
CHANGELOG named the panel by its saved-object title, which is not what
a user sees on the dashboard; use the displayed "Auth result filters"
instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Fix misspelled column label in the failure email samples table

The "DMARC failure email samples" visualization labeled its
authentication_results column "autentication_results". The underlying
field reference was already correct; only the user-facing customLabel
was misspelled. A sweep of every title and customLabel in the ndjson
found no other misspellings.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Extend the combined-field fix to SMTP TLS documents

SMTP TLS reports have the same cross-product defect as the DKIM/SPF
alignment tables (issue #169), one level deeper: policies is an object
array and each policy's failure_details is an object array inside it,
so stacked terms aggregations on their subfields fabricate rows.

Documents now also carry policies_combined ("domain / type" per policy)
and failure_details_combined ("domain / type / result / sending mta /
receiving ip / mx" per failure detail), composed at save time with the
same "none" fallbacks as the aggregate fields. migrate_indexes() gains
smtp_tls_indexes and backfills old documents with the same guarded,
non-blocking update_by_query pattern; cli.py wires the index name in on
both backends, and the manual _update_by_query command is documented.

Also fixes two adjacent dead fields: add_failure_details stored
additional_information_uri under the wrong constructor kwarg
(additional_information), and receiving_mx_hostname had no declaration
despite always being stored.

Verified live on ES 8.19 and OpenSearch 3: a two-policy repro report
yields exactly 2 policy rows and 2 failure-detail rows via the combined
fields where the old stacked aggregations return 4 of each; the startup
backfill converted the 4 pre-existing sample documents on both engines
with zero recompute mismatches.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Rework the SMTP TLS dashboards onto the combined fields

Kibana/OSD: "SMTP TLS domains" replaces its stacked policy_domain ×
policy_type terms with one terms agg on policies_combined.keyword;
"SMTP TLS failure details" replaces six stacked terms spanning both
array levels with one on failure_details_combined.keyword; the
smtp_tls* index-pattern field cache gains the new fields. The
"reporting organizations" table only buckets on doc-level org_name and
needed no change.

Splunk: the base search now expands policies at the JSON level (spath +
mvexpand) so policy fields are scalars per event, and the failure
details panel expands the second level the same way — sums are the
detail's own failed_session_count, correctly paired. Verified via the
search REST API: a two-policy repro returns exactly one row per real
failure detail with per-detail counts.

kibana.md documents the per-policy/per-detail row semantics and the
honest caveat that session-count sums remain per report document.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Persist additional_info_uri from parsed SMTP TLS failure details

Copilot caught that the savers read additional_information_uri from
the parsed failure-detail dict, but the parser's key is
additional_info_uri (SMTPTLSFailureDetailsOptional in types.py, set in
parse_smtp_tls_report_json), so the URI was never persisted — the
read-side half of the dead-field bug whose write-side half (wrong
constructor kwarg) was fixed earlier. Read the parser's key first,
keeping the long-form key as a fallback for dicts built by other
callers. Regression test proven to fail on the unfixed savers.

Also restructured the expected combined-string test values into named
locals so no implicit string concatenation sits inside a list literal.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Say "inner doc", not "nested doc", in the singular-key test docstrings

Final review sweep: in this codebase "nested" is reserved for the
Elasticsearch mapping type, and these InnerDoc-serialization
docstrings used it colloquially — the same dual-use-term trap
documented in AGENTS.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-25 12:29:40 -04:00
Sean WhalenandClaude Fable 5 864f11e2be Accept directory paths as file_path arguments, add -r/--recursive (#843) 
A directory given as a file_path argument now expands to the report
files inside it with shell-glob semantics (dotfile entries excluded,
subdirectories skipped). The new -r/--recursive flag descends into
subdirectories and enables '**' recursion in glob patterns. Directory
names containing glob metacharacters are escaped before expansion.
Fixes the file_path help string's stray trailing apostrophe and
refreshes the stale --help block in docs/source/usage.md.

Closes #397

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-25 12:09:42 -04:00
Sean WhalenandClaude Fable 5 4fa8cfb1e7 Skip the results email when no reports were parsed (#837) 
* Skip the results email when no reports were parsed (#200)

The [smtp] (and Microsoft Graph) results email was sent unconditionally
whenever the transport was configured, so an empty run — an empty inbox,
or one where every message was invalid — still emailed a zip of
headers-only CSVs. The email step is now skipped with an INFO log when
the run produced no aggregate, failure, or SMTP TLS reports.

The regression test was verified to fail against the unfixed code
(send_email was called once with a headers-only zip). Six existing tests
that asserted on the email path with all-empty mocked results now feed a
real parsed sample aggregate report through the actual zip/CSV-building
code instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Add Graph-path regression test for the empty-run email guard

Copilot review on #837 pointed out that only the SMTP transport had a
skip regression test, so a refactor narrowing the guard to the SMTP
branch could silently reintroduce headers-only zips via Microsoft
Graph. The new test was verified to fail against exactly that
narrowing (send_message called once with a headers-only zip).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-21 20:49:41 -04:00
Sanjay SanthanamClaude Fable 5Sean Whalen
 		e98e12acb0 Normalize aggregate result words to lowercase (#835) 
* fix: normalize aggregate result words

Normalize policy-evaluated and authentication result values so mixed-case reporter output does not create duplicate categories. Add a regression covering the existing uppercase sample.

* Move changelog entry to the Unreleased section

The entry landed in the released 10.2.4 section because the branch was
cut before the Unreleased heading existed on master; merged master and
moved it under Unreleased -> Bug fixes, matching the house entry style.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Apply review feedback: normalize disposition, share a guarded helper

- policy_evaluated disposition gets the same lowercase normalization as
  dkim/spf and the auth result words; the RFC 7489 Appendix C /
  RFC 9990 disposition enum is lowercase (none/quarantine/reject), and
  mixed-case values split the Message Disposition categories in every
  dashboard the same way Pass/pass did.
- All four normalization sites now share _normalize_result_word(),
  which guards with isinstance(str): xmltodict returns a dict for
  attribute-bearing elements, so the previously unguarded .lower()
  calls on auth results could raise AttributeError on malformed input
  that used to pass through.
- The #520 fixture's disposition is now mixed-case (None) and the test
  asserts it parses as "none"; verified the assertion fails against the
  pre-fix parser. Test docstring cites the RFC authority per the
  project's testing standards.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Sean Whalen <44679+seanthegeek@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-21 20:02:24 -04:00
supaeasy d9f6532841 Default missing Feedback-Type and Authentication-Results in failure reports (#332) (#831) 
Some Exim/cPanel-based gateways send DMARC failure reports without a
machine-readable message/feedback-report part. parse_report_email()'s
plain-text fallback synthesizes a minimal feedback report with only
Arrival-Date and Source-IP, but the Elasticsearch/OpenSearch outputs
access feedback_type and authentication_results with hard key lookups,
so every such report was archived but never indexed, failing with
"Failure report missing required field: 'feedback_type'".

parse_failure_report() now defaults feedback_type to auth-failure
(RFC 5965 3.1) and authentication_results to None (RFC 6591 3.1) with
logged warnings, matching the existing handling of the REQUIRED
Auth-Failure and Identity-Alignment fields. Adds a sanitized sample
and a regression test asserting the sink-required keys are present.
 2026-07-20 15:21:55 -04:00
Sean WhalenandClaude Fable 5 df9bf82e04 Post-review follow-ups for Graph send (#825/#826) and requests-to-httpx migration (#827) 
Follow-ups from the review of PR #825 (whose implementation had already
landed on master via #826's stacked merge):

- Honor the documented [smtp] attachment and [smtp] message options.
  Both were parsed into opts but never passed to either summary-email
  transport (also broken in released 10.2.2), so a configured custom
  attachment filename or message body was silently ignored. Both the
  SMTP and Microsoft Graph transports now receive them, and the missing
  smtp_attachment Namespace default is added (also covers SIGHUP
  reload, which rebuilds opts from the CLI Namespace).
- Don't mislabel non-Graph mailbox errors as Microsoft Graph failures:
  the shared mailbox-fetch and watch handlers now log a generic
  "Mailbox Error" with traceback when the connection isn't Graph.
- Declare microsoft-kiota-abstractions as a direct dependency (imported
  directly in cli.py for Graph error handling; previously transitive).

Migrate all runtime HTTP from requests to httpx (webhook client, Splunk
HEC client, and the PSL-overrides / IP-database / reverse-DNS-map /
IPinfo-API fetches in utils.py):

- follow_redirects=True everywhere to preserve requests' default
  redirect-following; httpx does not follow redirects by default.
- The PSL-overrides and reverse-DNS-map fetches gain a 60s timeout
  (previously none), matching the IP-database fetch.
- response.ok -> response.is_success; requests.RequestException ->
  httpx.HTTPError; raw string bodies use content= (httpx's data= is
  form-encoding only); Splunk HEC verification moves to client
  construction (httpx has no per-request verify).
- requests drops out of [project] dependencies and moves to the [build]
  extra for the out-of-wheel maintainer script collect_domain_info.py,
  which deliberately stays on requests/urllib3 for its permissive-TLS
  adapter.
- Remove the requests-era module-level
  urllib3.disable_warnings(InsecureRequestWarning) in splunk.py; httpx
  doesn't route through urllib3, so its only remaining effect was
  globally silencing insecure-TLS warnings from other urllib3-based
  components as an import side effect. Nothing imports urllib3 directly
  anymore, so it also leaves [project] dependencies.

Tests: config-to-transport wiring for attachment/message on both
transports (including defaults), non-Graph errors keep the generic log
line, webhook/Splunk payload assertions moved to content=, and Splunk
verify asserted at httpx.Client construction. 736 passed; ruff and
pyright clean.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-14 16:11:47 -04:00
KiliandClaude Sonnet 5 31c928d6fc Refresh Microsoft Graph docs: national clouds, examples, troubleshooting (#826) 
* Send report summary via Microsoft Graph; make Graph failures observable

Two related fixes shipped together:

Send via Graph: the periodic DMARC summary email can now be sent
through the already-authenticated Microsoft Graph mailbox connection
(MSGraphConnection.send_message(), /users/{mailbox}/sendMail) instead
of only SMTP. Triggered when [msgraph] is configured and [smtp] has a
`to` value but no `host` -- SMTP is always preferred when `host` is
set, with no automatic fallback to Graph on SMTP failure. Reuses the
same connection used for reading; no new send-only config mode.
email_results()'s SMTP behavior is unchanged; a new
email_results_via_msgraph() shares its content-building logic via a
new _build_report_email_content() helper. Graph's sendMail always
sends as the authenticated mailbox, so [smtp] from is ignored on this
path -- documented, along with the required Mail.Send permissions and
a caveat that delegated auth flows (UsernamePassword/DeviceCode) don't
currently request that scope, so app-only auth is the supported path
for sending. Tracks #472.

Observable Graph failures: MSGraphConnection construction, mailbox
fetch, message send, and --watch failures now catch
ClientAuthenticationError/APIError/httpx.HTTPError specifically and
log one clear ERROR line naming the mailbox, tenant, auth method, and
the Graph request-id/client-request-id when available, instead of a
bare "MS Graph Error"/"Mailbox Error" with no context. Full traceback
still preserved at --debug. --watch previously had no Graph-specific
error handling at all -- a Graph error there crashed with a raw
uncaught traceback; it now exits the same way as the other three
sites. No new config options.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Refresh Microsoft Graph docs: national clouds, examples, troubleshooting

The [msgraph] docs were accurate but missed guidance the community has
been asking for:

- graph_url now lists the actual national/sovereign-cloud endpoint
  values (GCC High, DoD, China/21Vianet), with an explicit warning
  that setting it alone is not sufficient -- the Entra ID auth
  endpoint isn't independently configurable in parsedmarc or
  mailsuite, so it always hits the global login.microsoftonline.com.
- A minimal working [msgraph] example for every auth method
  (UsernamePassword, DeviceCode, ClientSecret, Certificate,
  ClientAssertion) -- previously only Certificate had one, entangled
  with the SMTP-sending example.
- A reading-permission matrix alongside the existing sending one, so
  every auth method x own/shared-mailbox combination is explicit in
  one place for both directions.
- An accurate note on the parsedmarc-named token cache: it's a
  deliberate backward-compatibility choice from the 9.11.0 mailsuite
  extraction (mailsuite's own default cache name differs), not a
  migration users need to act on.
- A troubleshooting table for four error scenarios, verified against
  source rather than assumed: admin consent and folder-resolution
  failures are still live and documented with real fixes; the
  event-loop and ISO-timestamp errors are historical, already fixed
  below this project's dependency/version floor.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
 2026-07-14 13:34:56 -04:00
Sean WhalenandClaude Fable 5 40509f801b Migrate Elasticsearch output to the elasticsearch-py 8.x client (#822) 
* Migrate Elasticsearch output to the elasticsearch-py 8.x client (#806)

The mandatory elasticsearch<7.14.0 + elasticsearch-dsl==7.4.0 pins
transitively forced urllib3<2 (EOL 1.26.x) onto every install. The old
<7.14.0 cap only existed to dodge the client product check that broke
OpenSearch users (#452, #653) — obsolete now that parsedmarc has a
dedicated [opensearch] backend on opensearch-py.

- Depend on elasticsearch>=8.18,<9 and drop elasticsearch-dsl entirely
  (the DSL ships inside the client as elasticsearch.dsl since 8.18.0).
  The 8.x client's elastic-transport allows urllib3>=1.26.2,<3, so
  installs can now resolve urllib3 2.x. The 8.x line supports both
  Elasticsearch 8.x and 9.x servers; ES 7.x servers are no longer
  supported, and OpenSearch users pointing [elasticsearch] at an
  OpenSearch cluster must switch to the [opensearch] section.
- set_hosts() now builds 8.x connection kwargs (scheme-qualified host
  URLs, request_timeout, basic_auth) while keeping the function
  signature and every INI option unchanged.
- migrate_indexes() is now a documented no-op kept for API
  compatibility: its only migration (re-typing published_policy.fo
  from long to text) applied exclusively to indices carrying the
  legacy ES 6-era "doc" mapping type, which cannot exist on any
  server the 8.x client can reach.
- The elasticsearch.dsl 8.x stubs use dataclass_transform and don't
  surface pre-8.x-style bare `name = Text()` fields as constructor
  parameters; each Document/InnerDoc class now carries a
  TYPE_CHECKING-only `__init__(*args, **kwargs)` declaration matching
  the real runtime signature, which also made nine pre-existing
  pyright ignores unnecessary.

Verified with ruff, pyright (0 errors/0 warnings), the full pytest
suite (718 passed), and a CLI run over the bundled samples; CI's live
elasticsearch:8.19.7 service exercises the new client end-to-end.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Use pass instead of ... in TYPE_CHECKING __init__ stubs

CodeQL flags an ellipsis-only body as "Statement has no effect" (12
alerts on PR #822); pass is equivalent at runtime and to the type
checker and keeps the alerts from resurfacing on every future scan.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-13 13:50:25 -04:00
Sean WhalenandClaude Sonnet 5 01b85ab7de Fix aggregate-report timestamp skew on non-UTC hosts (#819) (#821) 
record["interval_begin"]/["interval_end"] are UTC wall-clock strings
(produced in __init__.py by strftime() on a datetime already converted
via human_timestamp_to_datetime(..., to_utc=True)), but the
Elasticsearch and OpenSearch per-record save loops and the Splunk HEC
aggregate-report event builder re-parsed them without assume_utc=True,
so on non-UTC hosts they were misinterpreted as local time and shifted
by the host's UTC offset -- shifting the stored date_begin/date_end,
the daily/monthly index date, and the Splunk event time. Verified by
parsing a real sample report under TZ=Europe/Warsaw vs TZ=UTC and by
reproducing the exact skew via human_timestamp_to_unix_timestamp().

This is the same class of bug fixed for arrival_date_utc in #811/#812
(commit cdda5da); the assume_utc keyword already exists on
human_timestamp_to_datetime()/human_timestamp_to_unix_timestamp() and
is reused here rather than reimplemented.

The issue also proposed changes to the report-level begin_date/end_date
parses in elastic.py/opensearch.py (~line 455) and to postgres.py.
Both were investigated and left unchanged: the report-level strings
are genuinely host-local time (from timestamp_to_human() ->
datetime.fromtimestamp()), so their existing no-assume_utc round-trip
is already correct on a single host -- postgres.py already gets this
right via two distinct helpers (_naive_local_to_timestamptz vs
_ensure_utc_suffix). Adding assume_utc to the report-level parses
would introduce a skew rather than fix one.

Fixes https://github.com/domainaware/parsedmarc/issues/819

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
 2026-07-13 09:59:35 -04:00
KiliMISAPOR LABClaude Sonnet 5
 		f33951c0bb Fix host-dependent flakiness in testMissingEverythingRaisesFileNotFoundError (#820) 
The system-path fallback list in _get_ip_database_path() checks real
absolute paths like /usr/share/GeoIP/GeoLite2-Country.mmdb. On a host
that actually has one installed there, the fallback succeeds and no
FileNotFoundError is raised, regardless of the test's mocked bundled
path -- the code is behaving correctly, the test just wasn't isolated
from the host filesystem. Patch os.path.exists to force every system
path to look absent so the test is host-independent.

Co-authored-by: MISAPOR LAB <misapor@lab.misapor.pl>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
 2026-07-13 09:32:32 -04:00
Sean WhalenandClaude Fable 5 746da77de5 Raise test coverage: utils.py, elastic.py, and opensearch.py to 100% (#816) 
* Raise test coverage: utils, elastic, and opensearch to 100%

Coverage of the shipped library rises from 88% to 90%, with
parsedmarc/utils.py 86% -> 100% and elastic.py / opensearch.py
99% -> 100%. All new tests assert on observable behaviour and mock
only at SDK boundaries (dnspython Resolver.resolve, requests.get,
subprocess.check_call, elasticsearch_dsl/opensearchpy Document.save).

New tests cover: query_dns transient-error retries, the load_ip_db
download/cache/bundled fallback chain, the IPinfo API token probe and
per-request MMDB fallbacks, _normalize_ip_record schema handling,
reverse-DNS-map invalid-CSV fallback, caller-provided reverse DNS
maps, Outlook MSG conversion (missing msgconvert and success paths),
parse_email Cc/Bcc/attachment-hash branches, aggregate-XML edge cases
(bytes input, repeated policy_published, unknown RFC 9990 override
types, missing org_name, attribute-only <email>), extract_report on
non-seekable streams, and the _AggregateReportDoc.save() override
that derives passed_dmarc.

Bugs found by the new tests, fixed in the same PR per the testing
standards:

- parse_email() crashed with KeyError: 'Headers' on messages whose
  From header is present but unparseable (e.g. a bare "From:" line):
  the fallback read parsed_email["Headers"], but the parsed headers
  are stored under lowercase "headers" (assigned a few lines up in
  the same function), so the key never exists. At the CLI surface
  this made any failure report whose embedded sample had an empty
  From: header fail to parse ("Missing value: 'Headers'").
- configure_ipinfo_api(probe=True) logged "IPinfo API configured"
  when the probe could not reach the API, contradicting its own
  docstring ("other errors are logged and the token is still
  accepted"): _ipinfo_api_lookup() returns None on network errors
  instead of raising, so the probe's exception handler was
  unreachable. The probe now checks the lookup result and warns on
  failure; 401/403 still raises InvalidIPinfoAPIKey.

Dead code deleted rather than padded with tests:

- _SMTPTLSReportDoc.add_policy() in elastic.py and opensearch.py
  (the save paths construct _SMTPTLSPolicyDoc directly).
- The no-op "for failure_index in failure_indexes: pass" loop in
  both migrate_indexes() implementations (parameter still accepted).
- The importlib.resources ImportError fallback in utils.py, which
  re-imported the same module and is unreachable on Python >= 3.10.
- The "Invalid report content" guard in extract_report(): every
  input branch assigns file_object or raises first (confirmed by
  pyright narrowing with the guard removed).

Also widens parse_aggregate_report_xml's annotation to str | bytes
to match its existing runtime behaviour (bytes are decoded with
errors ignored).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Use assertGreater for the reverse-DNS-map fallback size check

Addresses the github-code-quality bot finding on PR #816: assertTrue
with a comparison inside can't show the operands on failure, while
assertGreater reports both values and the failed relation. No change
to test behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-07-11 18:21:04 -04:00
KiliClaude Fable 5MISAPOR LABSean Whalen
 		3fda55d385 Make Microsoft Graph connection activity observable (#815) 
* Make Microsoft Graph connection activity observable

parsedmarc only configured its own logger, so all Graph connection
activity was silently dropped even with --debug: the mailbox layer
logs under mailsuite.mailbox.graph, token acquisition under
azure.identity (including the AADSTS error codes that distinguish a
local config problem from an Exchange Online / Entra ID one), and
HTTP traffic under httpx/msgraph — none of which had a handler or
level set. _main() also logged nothing around the MSGraphConnection
call, so a hang left no trace at all.

Three changes, all parsedmarc-side (no mailsuite changes needed):

- Log a redacted connection summary at INFO before connecting (auth
  method, tenant ID, client ID, mailbox, Graph URL) plus a --debug
  detail line with certificate path, token-file path, and set/not-set
  flags for secrets. Secret values are never logged; a regression
  test asserts they don't appear in captured output.
- Log a timing line after the connection object is initialized.
- Propagate parsedmarc's --verbose/--debug level and handlers to the
  dependency loggers (mailsuite, azure, msgraph, httpx, httpcore) via
  _configure_dependency_logging(), synced to exactly the parsedmarc
  logger's handlers so SIGHUP log-file swaps neither duplicate output
  nor write to closed handlers. At the default level dependency
  loggers sit at WARNING, so their warnings keep surfacing (formatted)
  without new noise.

All four new tests fail on the unfixed code (verified by stashing the
cli.py change).

Fixes https://github.com/domainaware/parsedmarc/issues/814.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Disable propagation on dependency loggers; document kiota's absence

Set propagate=False on the dependency loggers when syncing handlers, so
a stray logging.basicConfig() anywhere in the process cannot
double-print every dependency record through the root logger — the
function already owns these loggers' handler lists, and this makes that
ownership complete. Asserted alongside the existing level/handler checks.

kiota_http and its sibling packages were considered for
_DEPENDENCY_LOGGERS but verified to not use Python logging at all
(their observability is OpenTelemetry tracing), so a comment now
records why they are absent rather than leaving the omission to be
"fixed" later.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: MISAPOR LAB <misapor@lab.misapor.pl>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Sean Whalen <44679+seanthegeek@users.noreply.github.com>
 2026-07-09 20:46:39 -04:00
KiliClaude Fable 5MISAPOR LABSean Whalen
 		7fed72798a Stop system GeoIP files from shadowing the bundled IPinfo database (#813) 
* Stop system GeoIP files from shadowing the bundled IPinfo database

_get_ip_database_path() searched well-known system paths (including
/usr/share/GeoIP/GeoLite2-Country.mmdb and CWD-relative names) before
the database parsedmarc manages, so on any host with a distro GeoIP
package installed every lookup silently used a country-only — and
often years-old — database instead of the bundled IPinfo Lite one.
That disabled ASN enrichment entirely (asn/as_name/as_domain were None
for every IP) and with it the ASN-fallback path into the reverse-DNS
map, with no signal beyond a generic "IP database is more than a
month old" warning. Verified live on a Fedora host whose distro
GeoLite2-Country.mmdb dated to December 2019.

New precedence: explicit ip_db_path -> _IP_DB_PATH selected by
load_ip_db() (downloaded/cached/bundled) -> the bundled copy -> system
paths as a true last resort (only consulted when the bundled data
file is missing). The selected file is logged at debug level so a
--debug run shows which database answered.

The automatic system-path pickup was documented behavior, so
installation.md now tells MaxMind GeoLite2 users to set ip_db_path
explicitly, with a migration note.

Both new regression tests reproduce the shadowing portably via a decoy
CWD GeoLite2-Country.mmdb (the fallback list includes relative names),
and fail on the unfixed code (verified by stashing the source change).

Fixes https://github.com/domainaware/parsedmarc/issues/810.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address review: accurate fallback comment, dedup selection log, cover fallback tiers

- Correct the nothing-found-anywhere comment: the os.stat() age check
  raises FileNotFoundError before the caller's open_database() would.
- Log "Using IP database at ..." only when the selected path changes
  instead of on every uncached IP lookup, so --debug runs over large
  batches aren't flooded; tracked via _LAST_LOGGED_IP_DB_PATH, reset in
  the test fixture for order-independence.
- Cover the previously untested branches of _get_ip_database_path:
  system-path fallback when the bundled database is missing, the
  FileNotFoundError when nothing exists anywhere, the stale-database
  warning, and the log-once-per-path behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: MISAPOR LAB <misapor@lab.misapor.pl>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Sean Whalen <44679+seanthegeek@users.noreply.github.com>
 2026-07-09 20:07:00 -04:00