35 Commits

Include renames

Author	SHA1	Message	Date
3 people	02fcfd6e82	Merge commit from fork ... * Evict expired dedup-cache entries on insert and hash the key (GHSA-9fw6-mwqr-p43m) The seen-aggregate-report cache is an expiringdict.ExpiringDict, which removes an expired entry only when that same key is read again, or drops its oldest entry once it holds max_len (100 million) entries. A report that is never received again is never read again, so in watch mode every distinct aggregate report stayed in memory for the life of the process. The key was the JSON of the sender-controlled reporter, Report-ID and policy domain, so ten 5.6 KB attachments with a 5 MiB report_id held 64 MiB indefinitely. SeenReportCache (parsedmarc/config.py) subclasses ExpiringDict and removes expired entries from the head of its insertion order on every insert; the cache's max_len is now 100,000. _aggregate_report_dedup_key() now returns the SHA-256 hex digest of that JSON, so a key is 64 characters whatever the report contains. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep a re-added dedup-cache key at the tail so the expiry sweep is not blocked OrderedDict refreshes a re-set key's time in place, so a key added again while still stored kept its old position at the head and stopped the insert-time sweep before the expired entries behind it. SeenReportCache now moves such a key to the tail before storing it, keeping insertion order equal to age order. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Bound failed reverse-DNS lookups to 120 seconds per aggregate report (GHSA-rwfh-ww9c-jm9m) Every record of an aggregate report made one blocking PTR lookup of a sender-chosen source IP, with no bound on count or time; an unanswering reverse zone costs the full 8 s DNS lifetime with the default nameservers, and a weak-fallback result is never cached, so one IP repeated in every record was looked up every time. A 3,500-record report could block processing for hours. Lookups that return no PTR now share a 120-second budget per report; once it is spent, the remaining records are enriched from the local IP database only, their results are not cached, and one warning is logged. Each distinct source IP is looked up at most once per report, and lookups that find a PTR are not charged, so large legitimate reports keep their enrichment. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Keep values PostgreSQL rejects out of its output, and let one rejected report not hold the batch (GHSA-fxr7-7h9q-v949) An aggregate count above 2**31 - 1, an SMTP TLS sending-mta-ip or receiving-ip that is not an IP address, and a NUL in a failure report's sample each made the PostgreSQL save raise PostgreSQLError, which marked the whole mailbox batch unsaved, so legitimate reports from other senders were never archived. The parser now skips a record whose count is outside 0..MAX_MESSAGE_COUNT and rejects such an SMTP TLS session count (closing the same rejection in Elasticsearch/OpenSearch), drops an invalid TLS IP with a warning, and the PostgreSQL output replaces NUL with U+FFFD in text, text array and JSONB values. Any other psycopg.DataError raises the new PostgreSQLRejectedReport, which the CLI logs and skips for that one report unless fail_on_output_error is set. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Check an aggregate record's count before its reverse-DNS lookup A record whose count is outside 0..MAX_MESSAGE_COUNT is dropped, so its source IP is no longer looked up first, which could cost up to 8 seconds per record. A regression test mocks dnspython and asserts that the dropped record's address is never queried. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Require TLS for the PostgreSQL output by default and add [postgresql] sslmode and sslrootcert (GHSA-xvp7-7762-gx9g) PostgreSQLClient connected without an sslmode, so libpq used its default, prefer: when the server, or an on-path attacker, declined TLS, it continued over plain TCP and answered a clear-text password request. The client now passes sslmode=require unless the new sslmode argument, an sslmode in the connection string, or PGSSLMODE chooses a mode, and sslrootcert names the CA certificate for verify-ca and verify-full. A warning is logged at startup when a TCP host other than localhost or a loopback address is used with a mode that does not verify the server certificate. A malformed connection string is now rejected by a parse before connecting, with the same withheld-details error. A server without TLS fails with libpq's "server does not support SSL, but SSL was required" until sslmode is set to disable or prefer. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Withhold the hosts URL from Elasticsearch and OpenSearch client-build errors (GHSA-74xv-rmpx-2pjm) A password with an unencoded "/", "?" or "#" in a [elasticsearch] or [opensearch] hosts URL makes elastic_transport raise "Could not parse URL '<url>'" and opensearch-py (via urllib.parse) raise "Port could not be cast to integer value as '<password prefix>'". The startup wrapper repeated that text at ERROR without --debug and chained the original, which the SIGHUP reload's logger.exception printed again. set_hosts() now runs through _set_search_hosts(), which replaces any ValueError with a fixed message raised outside the except block. Other errors in the block keep their text with URL credentials masked and are raised from None. This completes the GHSA-hjr3-rx4c-24cv fix. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Cache the downloaded IP database in a private per-user directory and validate it (GHSA-5f56-8ph8-v9q5) load_ip_db() cached the IPinfo Lite download at the fixed, shared <tmp>/parsedmarc/ipinfo_lite.mmdb, where another local user could plant a symlink to have the download overwrite a file the parsedmarc user can write, or plant a database that was used whenever the download failed. The cache now lives in $XDG_CACHE_HOME/parsedmarc or ~/.cache/parsedmarc (%LOCALAPPDATA%\parsedmarc\cache on Windows), created 0700 and used only if it is a real directory the user owns and can write that is not group- or world-writable; otherwise a per-run mkdtemp() directory is used. The download is written with mkstemp(), opened with maxminddb to validate it, and moved into place with os.replace(). A cached copy is used only if it is a regular file owned by the user. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Skip reports over the Logs Ingestion API size limit and publish the Log Analytics streams independently (GHSA-rg4h-58fm-8pc9) The Azure Monitor ingestion SDK packs whole reports into chunks of up to 1 MiB and never splits one, so a report whose JSON is larger is sent as a call of its own, and the Logs Ingestion API rejects any call over 1 MB (https://learn.microsoft.com/en-us/azure/azure-monitor/fundamentals/service-limits). The rejected upload raised before the failure and SMTP TLS streams were sent, and the CLI held the whole batch for retry and finally Unsaved. publish_json() now measures each report as the SDK does and leaves out one over 1,000,000 bytes with a warning naming it, and publish_results() publishes each stream in its own try and raises one LogAnalyticsException naming every stream that failed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Address review: redact the PostgreSQL warning host, drop the connect error chain, and fix Log Analytics and docs wording libpq ends a URI's userinfo at the first "@", so the rest of a password containing "@" becomes part of the host. The PostgreSQL startup warning now credential-masks every host-like value it logs (host list entries, hostaddr, PGHOST, PGHOSTADDR), checks hostaddr before host as libpq does, and the general connect error is raised from None so the driver's unredacted text no longer reaches a logged traceback. The CLI's output-client Raises docs now say which outputs do not chain. publish_json() returns the number of reports uploaded, and publish_results() logs "Successfully pushed" only for a stream that uploaded something. The [postgresql] URI example uses the Unix socket and explains that TCP to localhost needs TLS on the server or sslmode = disable, the [log_analytics] docs note the SDK's 1 MiB call packing, the reverse-DNS-budget wording says "local IP database and reverse DNS map" everywhere, and the GHSA-fxr7 entry no longer claims the parser rejects every unreadable start-datetime. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Require mailsuite 2.3.7 mailsuite 2.3.7 fixes parse_email() keeping a CRLF message's carriage returns in address header values (seanthegeek/mailsuite#74). The floor is raised in the base dependency and the msgraph, gmail and all extras, and the usage docs' stated requirement is updated to match. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Drop Python 3.10 and add Python 3.15 to the supported versions and CI (#953) Python 3.10 reaches end of life in October 2026. requires-python is now >=3.11, the unused requires_python key under [build-system] (not read by hatchling, and its <3.15 bound read as excluding 3.15) is removed, and Python 3.15 joins the CI matrix, classifiers, and support tables. Docs, AGENTS.md, and comments that named 3.10 as the floor are updated. Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com> * Keep aggregate identifiers and reasons within their schema (#950) * Keep aggregate identifiers and reasons within their schema AI assistance: OpenAI Codex. * Expand the #948 CHANGELOG entry and tighten its tests RFC 9990 Appendix A and RFC 7489 Appendix C define only header_from, envelope_from and envelope_to under identifiers, and only type and comment in a reason. Cite them in the test, check that envelope_from and envelope_to survive next to a dropped nested unknown element, and cover an unknown key on the legacy identities path. The CHANGELOG entry now names the affected outputs and the compatibility change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Sean Whalen <44679+seanthegeek@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> * Reject reports dated in the future and bound what parse_report_file() reads (#954) * Reject reports whose index-naming date is more than two days in the future (#947) The date that names a report's dated Elasticsearch/OpenSearch index (an aggregate report's begin date, a failure report's Arrival-Date, an SMTP TLS report's start-datetime) was never compared with the present, so a report dated 2100 named an index for 2100. Each parser now rejects such a report with its own Invalid*Report; past dates stay unbounded. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Refuse an input larger than the report size cap before parse_report_file() reads it (#949) The 32 MiB cap was applied only after the whole input was read. A path is now checked with os.path.getsize(), bytes-like input with len(), and a file object is read at most one byte past the cap. The error is the ParserError with the message the content check already gives. Three existing tests that fed over-cap content now see the earlier rejection. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Measure a memoryview input by its byte size len() of a memoryview counts items, which are not bytes for every format, so the size cap in parse_report_file() compares nbytes instead. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * Bound the aggregate end date and read file objects to their end Address the Copilot review of #954. A normalized report's per-day records name their indexes from their own interval starts, so the report's end date is bounded like its begin date. A raw stream may return fewer bytes than asked for before its end, so parse_report_file() now reads in a loop until end of file or one byte past the cap. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * Accumulate bounded reads in one buffer Address the second Copilot review of #954. Keeping every chunk in a list cost one bytes object per read, about 85 bytes per byte for a stream that returns a byte at a time (22.7 MiB for 256 KiB of input). One bytearray keeps memory proportional to the bytes read. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com> * Skip a message with an invalid report attachment instead of aborting the mbox (#955) * Skip a message with an invalid report attachment instead of aborting the mbox parse_report_email() raised a bare ParserError when a report attachment was invalid, which get_dmarc_reports_from_mbox() re-raises, so one such message ended the whole file. Raise InvalidDMARCReport (a ParserError subclass), which the mbox loop skips with a warning. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Skip an invalid SMTP TLS attachment in an mbox too Address the Copilot review of #955. An invalid SMTP TLS attachment raised InvalidSMTPTLSReport, which is not an InvalidDMARCReport, so it still stopped the mbox. parse_report_email() now names the message's subject in that error, and both mbox loops skip it with a warning. A message that cannot be parsed at all still raises a plain ParserError. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com> * Address the second review: refuse an unencoded @ in a PostgreSQL URI, charge slow reverse-DNS lookups, and smaller fixes - PostgreSQL: a connection string whose parsed host, hostaddr, port or dbname holds an "@" (the rest of a URI password with an unencoded "@", which libpq splits at the first "@") is refused with a fixed error before any warning is logged or any connection is attempted; _connect parses the string on every path. A leading "@" in a host entry (Linux abstract socket) is still allowed. _log_safe_host stays as defence in depth for the host argument and PGHOST/PGHOSTADDR. - Log Analytics: publish_json catches AzureError, so a ServiceRequestError or ServiceResponseError no longer stops the remaining streams. - MAX_MESSAGE_COUNT comment and CHANGELOG: the bound matches PostgreSQL's INTEGER columns and the Elasticsearch/OpenSearch integer mapping of the SMTP TLS session counts; the aggregate message_count is dynamically mapped as long. - Reverse DNS budget: a lookup taking at least SLOW_REVERSE_DNS_SECONDS (1.0) is charged whether or not it returned a name; the warning now says "failed or were slow". - An IPv6 zone ID in an aggregate record's source IP skips the record before any DNS lookup, and in a failure report's Source-IP raises InvalidFailureReport; one helper serves these and the SMTP TLS path. - IP database cache: a relative candidate directory (HOME unset, no passwd entry) is rejected; offline / always_use_local_file creates no directory and no temporary directory and logs no "downloaded again". - No startup warning for sslmode=require with an existing sslrootcert file, which libpq verifies as verify-ca. - PostgreSQLRejectedReport truncates the driver text to 200 characters. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * Address the third review: refuse a URI with more than one @, bound total reverse-DNS time, strip IPv6 zone IDs - postgres: a URI connection string (postgresql:// or postgres://) with more than one literal "@" is refused on the raw string, before any log line or connect, closing the abstract-socket host-list and query parameter bypasses of the parsed-value check. libpq decodes %40 in a password, host, database name and query value, so a URI never needs a second "@". The parsed-value check remains for keyword/value strings. - Reverse DNS: every online lookup is charged to a new 600 s per-report total budget (MAX_REVERSE_DNS_SECONDS_PER_REPORT); failed or slow ones still also use the 120 s budget. The warning names the limit reached. - An IPv6 zone ID in an aggregate source_ip or a failure report's Source-IP is now stripped with a warning instead of rejecting the record or report; SMTP TLS still drops its optional IP fields. - _rejection_detail truncates the driver text before escaping it. - Docs and CHANGELOG entries updated. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * Withhold everything derived from a PostgreSQL connection string from logs and errors Fourth review of GHSA-xvp7-7762-gx9g: libpq ends a URI's userinfo at the first "@" or "/", so postgresql://u:SECRETX/x@db.invalid/d parses to host "u", port "SECRETX" and dbname "x@db.invalid/d". It has one "@", so the raw-count refusal let it through, and connecting raised 'invalid integer value "SECRETX" for connection option "port"', a password fragment in the error and its traceback. A variant put the fragment in sslmode, which the startup warning printed. Three rounds of shape checks each missed a shape. A. With a connection_string, nothing parsed from it is logged or raised: - A connection failure raises a fixed "PostgreSQL connection failed (details withheld ...)" from None, with " [SQLSTATE xxxxx]" appended only when the driver error carries a well-formed SQLSTATE (^[0-9A-Z]{5}$). A string libpq cannot parse keeps its own fixed message, and any error from the pre-parse is replaced by it. - The startup warning names no host and no value from the string; the mode is printed only when it is one of SSL_MODES. With the separate host/port/user/password/database arguments, the driver's message (credential-masked) and the host in the warning are kept. B. The early refusal stays as a convenience that names the common mistake, mirroring libpq's scan: a URI is refused when the text after "://" holds more than one "@", or an "@" with a "/" before it, and the parsed-value check (_has_misplaced_at) runs on URIs again as well as keyword/value strings. Docstrings, usage.md and the CHANGELOG now say the refusal is a convenience and A is what keeps the password out. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * Wrap every PostgreSQL connect failure and narrow the connection-string refusal - postgres._connect(): everything from parsing the settings to holding a configured connection (pre-parse, ssl kwargs, connect, autocommit, dumper registration) is protected. A non-driver error (psycopg's UnicodeEncodeError carrying the whole built connection string for a password with a lone surrogate; on Python 3.15 the unwrapped UnicodeEncodeError('idna', '<host>') for an over-long host label) is replaced with a fixed message from None: the withheld connection failure message with a connection string, a new fixed "a connection setting could not be passed to the driver" message with the separate settings. KeyboardInterrupt/SystemExit still propagate. - cli._init_output_clients(): the PostgreSQL wrapper no longer chains (from None), and a non-PostgreSQLError gets a fixed message. - _has_unencoded_at() now mirrors libpq's URI scan (userinfo up to the first "@" or "/") instead of counting "@", so a percent-encoded database name (d%40x) and an "@" in a query value after a "/" (a certificate path) are accepted, while every password-spill shape in the table is still refused except two that read like legitimate strings. - _has_misplaced_at() is removed: a keyword/value password is its own keyword, so an "@" in a kv host, hostaddr or dbname is not a password fragment, and libpq does not percent-decode kv values, so the encoding advice was wrong. The refusal message now names the database name. - Docstrings, usage.md and the GHSA-xvp7 CHANGELOG entry updated; tests for real psycopg (surrogate password, long host label), fake-psycopg non-driver errors at connect/autocommit/dumpers for both modes, a KeyboardInterrupt check, the rule table (also checked against the real parser), legitimate strings that must connect, and the CLI wrapper. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: jwalith4418-Mj <jwalith4418@gmail.com>	2026-11-10 18:44:59 -05:00
Sean WhalenandClaude Fable 5.2	bbd7b04417	Skip a message with an invalid report attachment instead of aborting the mbox (#955) ... * Skip a message with an invalid report attachment instead of aborting the mbox parse_report_email() raised a bare ParserError when a report attachment was invalid, which get_dmarc_reports_from_mbox() re-raises, so one such message ended the whole file. Raise InvalidDMARCReport (a ParserError subclass), which the mbox loop skips with a warning. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Skip an invalid SMTP TLS attachment in an mbox too Address the Copilot review of #955. An invalid SMTP TLS attachment raised InvalidSMTPTLSReport, which is not an InvalidDMARCReport, so it still stopped the mbox. parse_report_email() now names the message's subject in that error, and both mbox loops skip it with a warning. A message that cannot be parsed at all still raises a plain ParserError. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>	2026-11-10 17:09:45 -05:00
Sean WhalenandClaude Fable 5.2	d9193eafea	Reject reports dated in the future and bound what parse_report_file() reads (#954) ... * Reject reports whose index-naming date is more than two days in the future (#947) The date that names a report's dated Elasticsearch/OpenSearch index (an aggregate report's begin date, a failure report's Arrival-Date, an SMTP TLS report's start-datetime) was never compared with the present, so a report dated 2100 named an index for 2100. Each parser now rejects such a report with its own Invalid*Report; past dates stay unbounded. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Refuse an input larger than the report size cap before parse_report_file() reads it (#949) The 32 MiB cap was applied only after the whole input was read. A path is now checked with os.path.getsize(), bytes-like input with len(), and a file object is read at most one byte past the cap. The error is the ParserError with the message the content check already gives. Three existing tests that fed over-cap content now see the earlier rejection. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Measure a memoryview input by its byte size len() of a memoryview counts items, which are not bytes for every format, so the size cap in parse_report_file() compares nbytes instead. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * Bound the aggregate end date and read file objects to their end Address the Copilot review of #954. A normalized report's per-day records name their indexes from their own interval starts, so the report's end date is bounded like its begin date. A raw stream may return fewer bytes than asked for before its end, so parse_report_file() now reads in a loop until end of file or one byte past the cap. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * Accumulate bounded reads in one buffer Address the second Copilot review of #954. Keeping every chunk in a list cost one bytes object per read, about 85 bytes per byte for a stream that returns a byte at a time (22.7 MiB for 256 KiB of input). One bytearray keeps memory proportional to the bytes read. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>	2026-11-10 17:09:23 -05:00
3 people	fa905e41fd	Keep aggregate identifiers and reasons within their schema (#950) ... * Keep aggregate identifiers and reasons within their schema AI assistance: OpenAI Codex. * Expand the #948 CHANGELOG entry and tighten its tests RFC 9990 Appendix A and RFC 7489 Appendix C define only header_from, envelope_from and envelope_to under identifiers, and only type and comment in a reason. Cite them in the test, check that envelope_from and envelope_to survive next to a dropped nested unknown element, and cover an unknown key on the legacy identities path. The CHANGELOG entry now names the affected outputs and the compatibility change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Sean Whalen <44679+seanthegeek@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-11-10 13:09:12 -05:00
Sean WhalenandClaude Sonnet 5.5	3311db4e16	Drop Python 3.10 and add Python 3.15 to the supported versions and CI (#953) ... Python 3.10 reaches end of life in October 2026. requires-python is now >=3.11, the unused requires_python key under [build-system] (not read by hatchling, and its <3.15 bound read as excluding 3.15) is removed, and Python 3.15 joins the CI matrix, classifiers, and support tables. Docs, AGENTS.md, and comments that named 3.10 as the floor are updated. Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>	2026-11-10 12:51:04 -05:00
Sean WhalenandClaude Fable 5.1	e8292bef70	Hardening for 11.2.2: attempts file, parse timeout, worker handling, three fixes (#945) ... * Wrap the failure-report duplicate search in the output error The aggregate and SMTP TLS saves turn a failed duplicate-check search into ElasticsearchError or OpenSearchError; the failure-report saves let the search library's exception escape instead. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Refuse date strings too long to be dates human_timestamp_to_datetime() had no length limit; a digit run of a million characters took 12 s in dateutil. Add a 128 character limit (ValueError) and map it to the matching Invalid*Report at the failure Arrival-Date, the message Date header and the failure-report sample Date in the Elasticsearch and OpenSearch saves. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Skip failure sample headers with an empty name The Elasticsearch and OpenSearch failure saves normalise each header name; a name that is empty after that is not a valid field name, so leave the header out and save the rest. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Add a persisted store of unfinished parse attempts A message whose parse kills the process (the out-of-memory killer, a container memory cap) is never moved out of the reports folder, so every later run parses it and dies again. parsedmarc.attempts keeps a small JSON file counting, per message, the parse attempts that started but never finished, so the mailbox loop can set such a message aside after MAX_PARSE_ATTEMPTS. The key hashes the first 64 KiB of the message because mailsuite's IMAP backend does not expose UIDVALIDITY, so a reused UID must not inherit an old count. Writes are atomic and owner only, and a failed write turns the store off instead of stopping a run. Refs GHSA-qcc2-4x3v-v4g7, GHSA-q7gm-8cj9-94hr, GHSA-4272-4r8g-6c5h Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Set aside mailbox messages that keep killing the parse A message whose parse ends the process stays in the reports folder, so every one-shot run or service restart parsed it again and died before reaching the reports behind it. The new [mailbox] attempts_file option records each message in an AttemptStore before it is parsed and clears the entry once the parse returns a report or raises ParserError, so an entry left behind marks a parse that never finished; a message with MAX_PARSE_ATTEMPTS of them is disposed of as invalid without parsing. The entry is cleared before the save, so a failed save is never counted as a crash, and only after a successful move, so a failed move is retried. With n_procs > 1 a dead run had a whole submission window in flight, so counting every message submitted beside the culprit would set innocent reports aside on the third run. A message with one unfinished attempt is therefore parsed alone in the calling process after the rest of the batch, and a second death counts against it only. PARSEDMARC_MAILBOX_ATTEMPTS_FILE names the file directly, so the key joins _DIRECT_FILE_KEYS; without it the variable was read as a Docker-secret reference. Refs GHSA-qcc2-4x3v-v4g7, GHSA-q7gm-8cj9-94hr, GHSA-4272-4r8g-6c5h Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Stop a parallel_map job that runs past a timeout The parse limits added for GHSA-qcc2-4x3v-v4g7 bound every input known to parse slowly, but nothing bounds a parse they do not cover: with n_procs = 1 the rest of the mailbox waits behind it. parallel_map() now takes parse_timeout and timeout_result. A job that runs that long gets timeout_result(job, parse_timeout) in its place; since a ProcessPoolExecutor cannot stop one job, every worker is killed, a new pool is started and the other unfinished jobs are resubmitted. A job's clock starts when its worker reports that it began, through a queue the pool initializer hands each worker. Future.running() is not enough: the executor marks one job more than it has workers as running, so a job queued behind a slow neighbour would be charged for the wait. ParseTimeoutError subclasses InvalidDMARCReport so the mbox and mailbox loops treat a timed-out message as invalid without changes. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Add parse_timeout to the mbox, mailbox and watch functions get_dmarc_reports_from_mbox(), get_dmarc_reports_from_mailbox() and watch_inbox() take parse_timeout and pass it to parallel_map() with the email job's timeout value. A timeout set with n_procs = 1 also takes the parallel_map() branch, since only a parse in a worker process can be stopped. The mailbox function and watch_inbox() reject an invalid value before fetching or watching, as they do for max_unsaved_retries: raised inside a watch check, the error would be retried on every check. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Add the [general] parse_timeout option The CLI reads parse_timeout as seconds, rejects a negative or non-numeric value as a ConfigurationError, defaults it to 0 (no limit), and passes it to the report-file, mbox, mailbox and watch calls. The watch call reads it from opts, so a SIGHUP reload takes a new value. The default stays 0 because the parse limits already bound the known inputs and a timeout routes every n_procs = 1 run through a worker process; the documentation suggests 300 seconds. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the attempts_file option Describe [mailbox] attempts_file in the option list and in a new usage section on messages that crash parsedmarc, say in the Unsaved note that the failed-save counts are the in-memory ones, keep the file in a state directory in the systemd example and in a named volume in the Docker Compose example, and add the Security changelog entry for the retry-loop backstop the three advisories deferred. Refs GHSA-qcc2-4x3v-v4g7, GHSA-q7gm-8cj9-94hr, GHSA-4272-4r8g-6c5h Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * List parse_timeout and attempts_file as orchestration options ParserConfig's docstring and the usage page's ParserConfig notes name the keyword arguments that stay outside the dataclass. The two new options are such arguments, so a library caller looking for them on ParserConfig is told where they go. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Bound a suspect message's parse by parse_timeout With an attempts file, a message that was being parsed when an earlier run died is parsed alone after the rest of its batch, so that a second death is charged to it and not to the messages parsed beside it. That parse ran in the main process, where parse_timeout cannot stop it, so the one message most likely to be slow or fatal was the one the timeout did not cover. With a timeout set, each suspect now gets a worker pool of its own, one message per pool so a crash is still attributed to it alone, and its result is taken the same way as the batch's. Without a timeout the suspect is still parsed in this process. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Restore default signal handlers in parsing workers Under the fork start method a worker inherits the parent's signal handlers, and the CLI installs SIGTERM and SIGINT handlers that only set a flag. When a worker dies, ProcessPoolExecutor terminates the others with SIGTERM and joins them; a worker running the inherited handler ignores that signal, so the teardown waited forever and the run hung instead of failing. The pool initializer now resets both signals to their defaults. The logging tests that call the initializer in the test process put the process's own handlers back afterwards. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Order the Unreleased changelog and tie its entries together The three branches each added to the Unreleased entry. The GHSA-q7gm bullet still promised a follow-up for a persisted failed-attempt marker, which the attempts_file option now is, so it names the option. The attempts_file bullet now says that a suspect message is parsed in a worker of its own when parse_timeout is set, matching the code. The #942 fix moves up beside the other fix already on master, and the worker signal-handler fix gets its own line. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Ignore SIGINT in workers and end the batch when one dies Resetting SIGINT to its default in the workers made a Ctrl-C, which the terminal sends to the whole process group, kill every worker, so the run ended in BrokenProcessPool where the CLI's handler meant to let the batch finish. Workers now ignore SIGINT and keep the default SIGTERM action, which ProcessPoolExecutor needs to tear down a broken pool. A worker that dies for another reason (the out-of-memory killer, a crash) raised BrokenProcessPool out of parallel_map, which no caller handled: the file and mbox paths ended in a traceback. parallel_map now kills the remaining workers, closes the start queue and raises ParserError from it, and the CLI file and mbox paths log it and exit 1 as the mailbox paths already did. The in-flight messages keep their attempts-file entries, so the next run parses each alone. The systemd example gains KillMode=mixed so stopping the service signals only the main process and the batch in progress finishes. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * State the parse_timeout bounds the code actually gives A slow job running beside another that expires loses its partial run and starts again, so P slow jobs cost one more round of parse_timeout than the docstring said, plus the bounded joins of the killed workers, during which no heartbeat is sent. The clocks are also checked only while the parent waits for a result, so "at most one second past the timeout" was not true while the caller is busy between results; the docstring, usage.md and the changelog now say a parse is stopped within one wait slice of the parent next waiting. ParseTimeoutError's docstring says it is the value for every timed-out input, SMTP TLS and unrecognised ones included. A new test closes the results iterator mid-run and checks the workers are killed and the start queue closed; the zero-timeout test now observes which pool ran the job instead of patching _DeadlinePool. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Correct the attempts-file wording and test the parallel test mode Suspect isolation happens whenever the mailbox takes the parallel path, which parse_timeout selects even with n_procs = 1; the docstring, comment, usage.md and changelog now say so. The set-aside log said a message "has crashed parsedmarc {count-1} time(s)", which overstates the crashes once a failed move has added attempts; it now reports the unfinished attempts against the cap. usage.md lists attempts_file among the _file keys read directly from the environment and among the settings a SIGHUP reloads, and says that a failed write clearing an entry after a good parse can lead to a healthy message being set aside after two such failures. The test-mode set-aside test now runs the parallel path too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Measure a date's length after its comments are removed The length guard in human_timestamp_to_datetime() ran before the RFC 5322 comments were stripped. RFC 5322 section 3.2.2 puts no limit on a comment's length, so a valid Date header with a long comment was refused, and through the sample Date path the whole report email with it. The check now runs on the stripped value, which is linear to produce; a long digit run survives stripping, so it is still refused before dateutil sees it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Save parsed reports before exiting on a dead worker A ParserError from parallel_map (a worker process that died) or from an mbox made the CLI exit at once, discarding every report parsed so far; report files already moved under archive_directory were then saved nowhere. The CLI now stops reading input, saves what it gathered, and exits with status 1. Workers ignore SIGINT, so the plain pool path's executor shutdown on an early exit (KeyboardInterrupt, closing the generator) waited for the whole submission window. Any early exit now kills the workers, as the timed path already did. The Ctrl-C test ran under fork only, where a worker inherits the test's own harmless handler, so it passed without the SIGINT line. It now runs under every available start method and signals each worker once it has started a job. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Stop counting a message that is already set aside A message at the attempt limit that stayed in the folder (a failed move, test mode) was recorded again on every later run, so the logged count grew without bound. It is now set aside without being recorded, and the log names the stored count and the limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Describe worker death per mode and the systemd stop window A dead worker ends a one-shot run with status 1 after saving, but in watch mode the IMAP and Maildir watchers log it and check again, while Graph and Gmail exit. KillMode=mixed lets the batch finish only within TimeoutStopSec. Also say that queued messages keep attempt entries too, and re-wrap a parse_timeout note line. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Clear the seen report IDs around the worker-death CLI tests The new save-before-exit test parses a sample in the parent, which records it in the shared dedup set, so a later test of the same sample saw it as a duplicate and sent no results email. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Save file reports when a mailbox fetch ends in a ParserError A worker that died during the one-shot mailbox fetch reached the ParserError handler, which exited before the reports from report files and mbox inputs were saved; files already moved under archive_directory were then kept nowhere. The handler now saves them first. A second test covers the input-error path: no mailbox is opened and the parsed file stays under archive_directory. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Say what a mailbox run keeps when a worker dies Earlier batches are saved and the batch in progress stays in the mailbox. Also say that a set-aside message is not recorded again, so its entry ages out after 30 days, and rewrap two paragraphs. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Say which set-aside entries age out of the attempts file The 30-day drop applies only to an entry that stays: a failed move, or a message left in place in test mode. The sentence read as if every set-aside message kept an entry, contradicting the line before it. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Carry the test attempts path in the message, not the environment test_parallel_path_records_each_attempt_before_submitting_it passed the attempts file's path to its worker job through os.environ. A forkserver worker forks from a server started before the patch, so it never saw the variable and the job raised KeyError (Python 3.14's Linux default). The path now travels in a header line of the message itself, which reaches the worker under every start method. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test the worker-side helpers and the save-failure exits Codecov listed added lines no test reached: the timed pool's initializer and job wrapper, which run only in workers; _kill_workers on an executor without _processes; the plain path's heartbeat; the CLI's progress bar; the mbox loop's stop after a file worker dies; and a failed save before exiting, on the file/mbox path and the mailbox path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Read start tokens every round, not only while waiting With parse_timeout set, the parent read the workers' start tokens only inside its wait loop. When results were already done at harvest time it never waited, so each finished job stayed in the pool's token map with its whole input, and the map grew with the run past the submission window. The tokens are now read at the top of every round, and close() clears the map. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Kill the parsing workers on a second Ctrl-C The second Ctrl-C ends parsedmarc with os._exit(130), which skips the process pool's cleanup. Workers now ignore SIGINT, so under every start method a running worker survived as an orphan and kept parsing, without end when parse_timeout is off. The handler now kills the active child processes first; Python runs it in the main thread between bytecodes, so active_children() and Process.kill() are safe there. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Hand timed results back through files, not the pool's pipe With parse_timeout set, a timeout kills every worker, possibly one that is sending a large result. The executor's management thread then waits for the rest of that message forever; the bounded join returns with the thread alive, and because it is not a daemon the interpreter hangs at exit. Reproduced under fork, forkserver and spawn with a 256-400 MiB result. Workers of the timed pool now pickle each outcome, error values included, to a file in a private directory the pool owns and return None; every message left on a pipe is far below PIPE_BUF, which POSIX writes atomically. The pool without a timeout never kills a running worker and is unchanged. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Report an unwritable temporary directory as a ParserError With parse_timeout set the results travel through a temporary directory. If it could not be created, or a worker could not write its result file, a raw OSError escaped parallel_map, which the CLI does not handle. The pool now raises ParserError naming the cause; a worker that cannot write its file returns the error text through the executor, still a small message, and the parent raises the same ParserError. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep every hand-back failure a short ParserError A result the worker could not pickle (PicklingError, TypeError, RecursionError, MemoryError) escaped through the executor with a traceback of any size, breaking the short-message guarantee the timed pool relies on, and a result file that did not unpickle raised a raw exception in the parent. The worker now catches any failure to write its outcome and returns a short marker, its text cut to 200 characters so the message stays below the 512-byte minimum PIPE_BUF; the parent turns it, and any failure to load a result file, into a ParserError. Also corrects the PIPE_BUF figure in the module docstring, names the temporary directory as TMPDIR, and avoids a second 128 MiB object in a test. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Cap the hand-back error text in bytes and survive a failing __str__ The pipe's atomic-write bound is in bytes, so a 200-character text of multibyte characters could exceed it; the cap now applies to the UTF-8 encoding. An exception whose __str__ raises is named by its type alone, so nothing in the worker's error path can raise through the executor. A failed write's .tmp file is removed with the pool's result directory; the docstring says so. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Match the pickler's message across Python versions in a test The unpicklable-result test asserted "Can't get local object", which is the wording of Python 3.12 and 3.13 only; 3.10, 3.11 and 3.14 say "Can't pickle local object". Assert the part they share. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Cover the worker-side hand-back failures in the test process The three failure branches of _run_timed_job and _take_outcome run in worker processes in the pool tests, where coverage cannot see them; call them directly here with an unwritable directory, an unpicklable result and an unreadable result file. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Assert the unreadable-result message by its text, and format The temporary-directory error carries the OSError's text, not its class name, so the test asserts "Is a directory"; one long test line is wrapped as ruff format wants. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Narrow the marker before unpacking it in two tests pyright cannot narrow Optional through assertIsNotNone; a plain assert does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>	2026-11-09 20:54:27 -05:00
3 people	2ee209005b	Merge commit from fork ... * chore: update IPinfo Lite MMDB (#941) Co-authored-by: seanthegeek <44679+seanthegeek@users.noreply.github.com> * Cap report date range and indexes created per report A report whose <date_range> spans millions of days was normalized into one record per day, exhausting memory, and with Elasticsearch or OpenSearch output created one dated index (one shard) per day, exhausting the cluster's shard budget. Reject a range longer than MAX_REPORT_SPAN_DAYS (366) or negative with InvalidAggregateReport. In both save functions, once a report has used MAX_INDEXES_PER_REPORT (31) distinct dated indexes, route further records to the report's first index, keeping their own dates, and log one warning. Refs GHSA-p85v-578x-3wcf Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Catch output-stage exceptions and validate SMTP TLS report types One report sent to a monitored mailbox could make a configured output raise an exception that process_reports() did not catch. It escaped through the mailbox save callback, _main logged "Mailbox Error" and exited 1, and the batch stayed in the mailbox, so every one-shot run died on the same message and duplicated the legitimate reports in non-deduplicating outputs. Three sender-controlled inputs did it: - A failure report whose sample has an empty From:, To: or Reply-To: (To: undisclosed-recipients:; included): mail-parser gives an empty address list, and the Elasticsearch and OpenSearch saves indexed [0] (IndexError). - An SMTP TLS report with a wrong-typed field: "policy-string": [1] made the CSV writer raise TypeError; "start-datetime": 5 made the Elasticsearch, OpenSearch and Splunk saves raise AttributeError and _ensure_utc_suffix() in the PostgreSQL save raise TypeError. - An aggregate report whose <reason> has no <type> (KeyError in the CSV writer) or a <type> that xmltodict parses to a dict or a list (TypeError in its join). Fix: - Every output destination block in process_reports() now ends with the catch-all the aggregate Elasticsearch block had: the failure and SMTP TLS Elasticsearch/OpenSearch blocks, every PostgreSQL and Splunk HEC block, and the file output (its OSError/ValueError branch is kept). The error goes through log_output_error(), so the batch is reported unsaved and fail_on_output_error still raises ParserError. - parse_smtp_tls_report_json() and its helpers check each field against the type RFC 8460 section 4.4 gives it and raise InvalidSMTPTLSReport otherwise: strings for organization-name, report-id, policy-domain, policy-type, result-type and the optional failure-details fields; contact-info a string or a list of strings; policy-string and a list mx-host-pattern lists of strings; the session counts integers or null; failure-details a list of objects; start-datetime and end-datetime strings of at most 64 characters that the outputs' own conversions accept. dateutil took 12.4 s to fail on a 1,000,000-digit date, hence the length bound. - _parse_report_record() makes a reason's type and comment a string or None (a dict unwrapped with _text(), a list as its str()), and the aggregate CSV writer reads both with .get(). - The Elasticsearch and OpenSearch failure saves flatten From, To and Reply-To with a helper that returns None for an empty list, and leave such a header out of the document and the duplicate query. - The Splunk SMTP TLS save converts begin_date inside its try, so a bad value raises SplunkError; _ensure_utc_suffix() returns a non-string unchanged. Refs GHSA-4272-4r8g-6c5h Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Limit XML nesting depth and node count in aggregate reports parse_aggregate_report_xml() gave the decompressed report (up to the 100 MiB decompression cap) to xmltodict with no limit on depth or element count. xmltodict keeps one stack entry per open element and expat reports missing end tags only at the end of the input, so a few KB of gzipped unclosed <a> tags grew to gigabytes of RSS and the kernel OOM killer ended the process before the message was moved; a wide document of millions of siblings did the same at a smaller scale. A failed first parse also re-parsed the document through lxml recovery, which has no node limit, and a second xmltodict pass. _NoDoctypeExpat.ParserCreate() now returns a proxy around the expat parser. pyexpat parsers cannot be subclassed and xmltodict assigns its handlers after ParserCreate() returns, so the proxy's __setattr__ wraps StartElementHandler to count depth and nodes (elements plus attributes) and EndElementHandler to decrement depth, and forwards every other attribute. Over MAX_XML_DEPTH (100) or MAX_XML_NODES (5,000,000) it raises before xmltodict's handler runs. Both xmltodict.parse() call sites turn that into InvalidAggregateReport naming the limit; the first does so instead of trying lxml recovery. The lxml recovery path now runs a counting pass first, with a parser target that builds no tree and counts elements, attributes and namespace declarations against MAX_XML_NODES; the tree is built only for a document within the limit. Depth there is bounded by libxml2's own limit of 256 (huge_tree stays off) and then by the second xmltodict pass. A 22 MiB wide document behind a malformed XML declaration peaked at +1532 MiB before this pass and +23 MiB after. The largest aggregate sample (909 KB) has 32,019 nodes, so a legitimate report at the 100 MiB cap stays near 3.6 million. Refs GHSA-q7gm-8cj9-94hr Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Create Elasticsearch/OpenSearch indexes with mapping guards create_indexes() and create_data_streams() created indexes and the data stream template with settings only, so the cluster typed every report field from the first document that held it, with date_detection on. A sender could be first into a day's index (its name comes from the report) and poison it: a report_id of "2026-11-09" mapped report_id as a date, a failure sample header named "Subject.x" mapped sample.headers.subject as an object, and samples with invented header names used up index.mapping.total_fields.limit. Every later legitimate report for that index was then rejected and ended up in Unsaved; in data stream mode this lasted until a rollover. Both functions now take a mappings body, and the three save functions pass one per report type. Each turns date_detection off and declares the fields that hold dates (@timestamp, date_range, date_begin, date_end, arrival_date, sample.date) as dates, the type date detection gave them before; on the sample reports every other field keeps its old type. The failure mapping maps sample.headers as an object with dynamic: false and declares, as text with a keyword sub-field (the dynamic string default), every header the shipped dashboards reference plus date, message-id and cc; all headers still stay in _source. A "." in a header name is replaced with "_" before the header is stored. The index is now created through the client's indices.create with the settings and mappings body, because opensearch-py's Index.create cannot take a mappings argument; the data stream template merges the mappings with its @timestamp date mapping. Existing indexes are not changed. The other report fields stay dynamic and object-mapped (no nested), which the dashboards need (issue #169). Refs GHSA-qpcg-29vr-jwm3 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Read the RFC 8460 mx-host key in SMTP TLS reports RFC 8460 section 4.4 keys a policy's MX host patterns "mx-host"; its schema line is "mx-host": mx-host-pattern, where mx-host-pattern is the value type, described as "a JSON array of strings", and the RFC's Appendix B example gives the value as a plain string. The parser read only "mx-host-pattern", so every RFC-conformant report lost the field (samples/smtp_tls/rfc8460.json, the Appendix B example, among them). _parse_smtp_tls_report_policy() now reads "mx-host" as a string or a list of strings, a string becoming a one-element list, and falls back to "mx-host-pattern" (same rules) when "mx-host" is absent, null or empty. Any other type raises InvalidSMTPTLSReport. The output key is still mx_host_patterns, so the PostgreSQL TEXT[] column, the Elasticsearch/OpenSearch field and the CSV column are unchanged. Refs #942 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Reject over-limit messages with the mailsuite guard parse_report_email() and parse_email() handed every message to mail-parser and the email package with no bound on blank lines, header lines, header field size, MIME parts or nested multiparts, so a small crafted message took seconds to minutes and gigabytes to parse while the rest of the mailbox waited. Both now call mailsuite.utils.check_message_limits() first, one linear pass. parse_report_email() turns MessageLimitExceeded into InvalidDMARCReport so the mailbox loop files the message under Invalid; parse_failure_report() turns one from the sample into InvalidFailureReport. The mailsuite floor rises to 2.3.5, which adds the guard. Messages nested deeper than 100 multiparts are now rejected by the guard instead of reaching the recursion handler; the tests that used the 400-level message expect the new message. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Cite RFC 7489 §6.3 for the daily report interval The comment on MAX_REPORT_SPAN_DAYS cited RFC 7489 Appendix A for the one-day aggregate report. The daily interval comes from §6.3: the "ri" tag requests the interval between aggregate reports, and its default is 86400 seconds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Add the 11.2.2 changelog: five advisories and #942 One Security bullet per advisory (GHSA-qcc2, GHSA-q7gm, GHSA-p85v, GHSA-4272, GHSA-qpcg) and a Bug fixes bullet for the RFC 8460 mx-host key. The version in constants.py is bumped by the release pull request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep the date range rejection message and pyright's analysis With the XML limits and the date range limit merged, parse_aggregate_report_xml() became too complex for pyright to analyze ("Code is too complex to analyze"), and the date range rejection, raised inside the function's catch-all, reached the caller as "Unexpected error: Report date range spans ...". The date range check moves to _check_report_span() and the second xmltodict pass to _parse_limited_xml(), which raises the XML limit breach as InvalidAggregateReport; the catch-all now re-raises an InvalidAggregateReport as it is. The tests assert the message is not re-wrapped. Refs GHSA-p85v-578x-3wcf Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Put the over-limit failure samples in a delivery-status body mailsuite 2.3.5's check_message_limits() follows the MIME structure and counts blank lines only inside message/delivery-status bodies, the shape whose parse cost is per blank line; blank lines elsewhere in a body are not limited. The two failure-sample tests used blank lines in a plain body, which the final check accepts, so they now put them in a delivery-status body. parsedmarc's call with the default limits is unchanged. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Describe the final mailsuite check and the index cap in the docs The GHSA-qcc2 changelog bullet now describes check_message_limits() as mailsuite 2.3.5 ships it: limits follow the MIME structure, and body lines outside message/delivery-status bodies are not limited. The changelog section moves to Unreleased, as parsedmarc's AGENTS.md has fix pull requests write it; the release pull request names the version. The Elasticsearch and OpenSearch pages now say that an address header with no address is left out of the failure document, and that a report spanning more than 31 days saves its later records to its first dated index. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Bound records times days in normalization and fix the span message The date range limit bounds how far one record is split, but not how many records are split: 10,000 records over 366 days, each with a count of 1,000,000 (3.3 MB of XML), produced 3,670,000 records in 13.5 s and 1.7 GB, and 30,000 of them (a 44 KB gzip) ran out of memory and reached the caller as "Unexpected error: ". A report whose split would exceed MAX_NORMALIZED_RECORDS (250,000) now keeps its records whole over its own range, with timespan_requires_normalization false and one warning; a large legitimate multi-day report is kept rather than rejected. The record loop moves to _parse_report_records() so pyright can still analyze parse_aggregate_report_xml(). The span message used float division, which raises OverflowError for an <end> of 4,001 digits; it now uses integer division. The index-routing warning escapes the sender's report ID with sanitize_for_log(). The changelog and the Elasticsearch and OpenSearch pages say the index cap is per report, that dated index names still come from report dates, and that data streams are affected by neither. Refs GHSA-p85v-578x-3wcf Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Drop XML attributes from aggregate reports before they are used xmltodict turns an element with an attribute, such as <p a="1">none</p>, into {"@a": "1", "#text": "none"}. Such objects passed through the parser into Elasticsearch and OpenSearch documents for version, p, fo, disposition, envelope_to and other scalar fields, so the first one mapped the field as an object and every later report to that index failed, and hundreds of attributes on one element used up total_fields.limit. The parsed report now goes through one linear pass that replaces an element with only attributes and text by its text and drops attributes from the others; the existing _text() calls now receive strings. The changelog and the Elasticsearch and OpenSearch pages also say that an existing data stream's next backing index gets the guarded mappings, since parsedmarc puts the template again in every process, and the indexed-header comment and changelog name cc as the one header added to those the dashboards reference. Refs GHSA-qpcg-29vr-jwm3 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Reject lone surrogates in SMTP TLS reports and guard the stdout print An SMTP TLS string field holding a lone surrogate (JSON "\ud800") passed every type check and survived json.dumps(ensure_ascii=False), and print(output_str) in process_reports(), outside every try, raised UnicodeEncodeError: parsedmarc exited 1, and in mailbox mode the error escaped the save callback. RFC 8460 section 4 reports are I-JSON, whose RFC 7493 section 2.1 forbids surrogate code points, so the string and string-list checks now reject one as InvalidSMTPTLSReport, and the print records a UnicodeEncodeError as a "Standard output" output error. The _smtp_tls_datetime() docstring no longer says a parsed date cannot fail in any output: dateutil accepts partial dates that PostgreSQL may reject, which is contained as PostgreSQLError. The _reason_text() docstring says a dict with child elements gives None. Refs GHSA-4272-4r8g-6c5h Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Describe mailsuite 2.3.5's address group handling in the changelog The final mailsuite 2.3.5 no longer rejects an address header by its comma count; its parse_email() removes RFC 5322 group syntax from address header values over 8 KiB before parsing them. The boundary limit counts lines that are the boundary of an open multipart. parsedmarc's call with the default limits is unchanged. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test the keep-alive call of the extracted record loop _parse_report_records() holds the record loop moved out of parse_aggregate_report_xml(); its keep-alive branch had no test. Refs GHSA-p85v-578x-3wcf Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test a policy override type with child elements With XML attributes dropped at parse time, a dict reaches _reason_text() only for an element with child elements; the test pins its None result and the CSV's "none". Refs GHSA-4272-4r8g-6c5h Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Check plain-string contact-info and mx-host for lone surrogates The surrogate check covered the string fields and string lists, but a contact-info or mx-host given as one string took a branch that skipped it, and a lone surrogate there still made the stdout print raise. Refs GHSA-4272-4r8g-6c5h Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Say which sample headers the failure document leaves out The GHSA-qpcg changelog bullet said every sample header stays in _source, but the failure save leaves out a From, To or Reply-To header that holds no address, as the Elasticsearch and OpenSearch pages say. Refs GHSA-qpcg-29vr-jwm3 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep sender-derived numbers out of the date range message The rejection message printed the report's span in days, so an <end> of thousands of digits put a number of thousands of digits into the exception text and the log. It now says only that the range is negative or longer than MAX_REPORT_SPAN_DAYS. Refs GHSA-p85v-578x-3wcf Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Store every string-typed aggregate field as a string or null Dropping attributes did not cover child elements: <version><x>1</x></version>, <p><x>1</x></p> or <p>none<x/></p> still reached the parsed report, and the Elasticsearch and OpenSearch documents, as a dict, and the first such document mapped the field as an object. _scalar() now makes each field the schema types as a string a string (an element's own text) or None, applied by _scalarize_report() to version, the report_metadata strings and every policy_published child, and by _parse_report_record() to source_ip, disposition, dkim, spf, the identifiers and every auth_results string. A <count> with no text of its own takes the ValueError path of a non-numeric count. _drop_xml_attributes() stays, so a field missed here still loses its attribute keys. The <email> dict unwrap is gone, since the field can no longer be a dict; a text-only report_metadata still raises the AttributeError that reports a missing section. Comments that described attribute dicts now say the dict shape comes only from child elements. The namespace test, which the new code could not fail, is replaced by one that detects RFC 9990 through an attributed np field. Tests walk the to_dict() of a mocked Elasticsearch and OpenSearch save and allow lists and objects only in the known object fields, so a scalar field added later is covered too. Refs GHSA-qpcg-29vr-jwm3 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Record a closed stdout pipe as an output error and fix two comments The standard-output guard in process_reports() caught only UnicodeEncodeError, so a closed pipe (BrokenPipeError, an OSError) still ended the run; it now records both as a "Standard output" output error. _reason_text()'s docstring said a dict gives None because the element has no text of its own, but mixed content (<type>forwarded<x/></type>) gives the text beside the children; the docstring now says what the code does, and a test pins it. The comment above the reason normalization no longer describes attribute dicts, which no longer reach it. Refs GHSA-4272-4r8g-6c5h Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test the normalization limit at its boundary and reword the index cap The suite passed with the limit's <= changed to < and with _days_touched()'s ceiling changed to a floor. New tests: 1,000 records over 250 days (exactly MAX_NORMALIZED_RECORDS) are split; 683 records over 366 days (249,978) are split and 684 (250,344) are not; 682 records over 366 days starting 1 s after midnight touch 367 days (250,294) and are not split. Each mutant was run against these tests and fails: the < mutant fails the exact-limit test, the floor mutant the after-midnight test. The Elasticsearch and OpenSearch pages now say the index cap applies when a report's records would use more than 31 dated indexes, which is what it counts (with monthly_indexes the span limit keeps a report under it). Refs GHSA-p85v-578x-3wcf Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Match the tests and changelog to mailsuite 2.3.5's final check mailsuite 2.3.5's check_message_limits() now parses the message with the email package's own feed parser under compat32 and counts the parts it creates (each message/delivery-status block among them), their nesting depth and its boundary tests, then limits header field length and count on the parsed tree; the limit names changed. The over-limit failure samples now trip max_parts, and the deeply nested poison message max_depth; the tests assert those texts. The GHSA-qcc2 changelog bullet describes the final check, the epilogue mail-parser parses again, and the changes that affect only mailsuite's own parse_email(). Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test a repeated policy_published and note mixed-content behaviour No test failed when _scalarize_report() stopped coercing the fields of a repeated <policy_published> (the list case): with that mutant, a report with two policy_published elements and <p><x>1</x></p> puts a dict into published_policy.p. The parse test and the Elasticsearch and OpenSearch document-walk tests now include that report; each fails on the mutant. The changelog says that an element with text and a stray child element now yields its text (samples/aggregate/invalid_xml.xml gives header_from "bad" where it gave ""). AGENTS.md and the _normalize_result_word() and _scalarize_report() docstrings describe what runs now: attributes are dropped as soon as the XML is parsed, and _scalar() coerces the string fields, among them the RFC 7489 and RFC 9990 policy_published fields. Refs GHSA-qpcg-29vr-jwm3 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Coerce policy override reasons with the same rule as other strings _reason_text() turned a repeated <type> or <comment> into the str() of the list, while _scalar() turns every other repeated string field into None. One class of input now has one rule: the reason's type and comment go through _scalar(), so a repeated one is None ("none" in CSV), and _reason_text() is removed. The test and the GHSA-4272 changelog bullet say so. Refs GHSA-4272-4r8g-6c5h Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test an aggregate <error> with a child element With attributes dropped and reasons coerced by _scalar(), only <error>, which may repeat, still reaches _text() with a dict; its branch had no test. An <error> with text beside a child element keeps the text, and one with no text is dropped. Refs GHSA-qpcg-29vr-jwm3 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Describe mailsuite's six message limits in the changelog mailsuite 2.3.5's check now counts every line of every header block (folded lines and lines the parser keeps only as defects included) and stops the parse early, and limits the total length of the address headers mail-parser parses to 262,144 characters. The GHSA-qcc2 bullet names the six limits. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Render a report email's parts only where their text is read parse_report_email() did str(payload_obj[0]) for every part walk() visits, before the multipart/ continue. str() of a composite part renders its whole subtree, and walk() visits every level, so a nesting within mailsuite's limits rendered its leaf once per level: 98 nested multiparts around a 20 MiB one-line leaf took 26.6 s, and a chain of 99 message/rfc822 parts around a 10 MiB leaf 11.3 s. A part is now rendered only in the branches that read it; other message/* bodies are skipped, since walk() visits their parts. Both shapes now take under 0.5 s, and every sample under samples/failure/ parses to the same output. A feedback report's sample is taken from the first sample part only: a message/rfc822 nested inside the sample used to replace it, but a feedback report has one sample (RFC 6591 section 3.1). Multipart nesting alone no longer reaches the recursion limit, so the GHSA-qxwv test now reaches the recursion guard through a deeply nested sample, which is still rendered. The changelog describes this and the address headers mailsuite 2.3.5 counts (From, Sender, Reply-To, To, Cc, Bcc, Delivered-To, in every part; mail-parser parses all but Sender). Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Parse aggregate XML without attributes and lower the node limit _drop_xml_attributes() rebuilt the parsed tree while the original was alive, and the counting wrappers of _LimitedExpatParser referred to the proxy, closing a reference cycle through the expat parser that kept xmltodict's tree alive until the cyclic garbage collector ran. Both xmltodict passes now use xml_attribs=False, so no attribute key is ever built (expat still hands the attributes to the start handler, so they are still counted: a test shows one element with 1,000 attributes is 1,001 nodes), _drop_xml_attributes() is removed, and the counts live in an _XMLCounts object the wrappers share, so a parse leaves no cycle (a test asserts gc.collect() finds nothing after one). Peak extra memory for 2,000,000 distinct sibling elements: +999 MiB before, +386 MiB after (master +391 MiB); for one element with 2,000,000 attributes: +1,274 MiB before, +445 MiB after (master +734 MiB). At the old limit of 5,000,000 nodes a report could still need about 3 GB. Measured after the change, the worst shape is distinct elements with text behind a malformed XML declaration (the lxml recovery path): 559 bytes per node, +533 MiB for 999,000 nodes and +1,023 MiB for 2,000,000; on the expat path about 220 bytes per node. MAX_XML_NODES is now 1,000,000, so a report at the limit needs at most about 550 MiB; the largest sample has 32,019 nodes (909 KB), and the limit is a report of about 28 MB. The constant's comment and the changelog state the bound. AGENTS.md describes the attribute handling as it now is, and says that <error>, which may repeat, goes through _text() and stays a list. Refs GHSA-q7gm-8cj9-94hr Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Qualify the mixed-content compatibility note to string fields An element with text beside a stray child element yields its text only for the fields the schema types as strings; a <begin> or <end> with mixed content is still rejected, as before. Refs GHSA-qpcg-29vr-jwm3 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Reject over-long XML tags before any parser reads them expat and libxml2 build a start tag's whole attribute list before they report the element, so the node counter only sees the attributes after they cost memory: one element with 4,999,000 attributes (56 MiB) took 5.25 s and +767 MiB before the node limit rejected it, and a single element is bounded only by the 100 MiB decompression cap. parse_aggregate_report_xml() now scans the decoded text once, before the first parse and so before the lxml recovery path too, and rejects a document in which any text between a "<" and the next ">" (or the end of the text) is longer than MAX_XML_TAG_LENGTH (65,536 characters). The scan uses no regex. It splits the text on ">" one 64 KiB chunk at a time, carrying the unfinished piece from its last "<": a single split of the whole text makes one string per tag, which peaked at +1,318 MiB (1.9 s) for 100 MiB of "<a/>"; the chunked scan needs about 2 MiB and takes 0.6 s on that text, and a test bounds its memory and covers a tag across a chunk boundary. The longest tag in the samples is 108 characters. The same shape is now rejected in under 0.1 s, with a tracemalloc peak under 200 MB in the test, behind a well-formed or a malformed declaration. Refs GHSA-q7gm-8cj9-94hr Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Render a report email's feedback-report part only once The message/* skip exempted message/feedback-report, which was rendered with str() every time walk() reached one: a chain of 99 nested message/feedback-report parts around a 10 MiB leaf passed mailsuite's check in 0.02 s and then took 11.6 s. A feedback report has one message/feedback-report part (RFC 6591 section 3), so, as with the sample, only the first is read and rendered; the chain now takes 0.56 s. The failure samples parse to byte-identical output. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Bound attribute counts by their = signs and cap reports at 32 MiB XML allows a raw > inside a quoted attribute value, so attributes such as x=">" cut a tag into short pieces that pass the tag scan: 4,999,000 of them cost 5.61 s and +781 MiB, and 8,000,000 (98 MiB) 8.9 s and +1,215 MiB, before the node limit fired. Every attribute needs an =, so a document with more = characters than MAX_XML_NODES is now rejected in one str.count() pass, before the tag scan; the samples hold at most 3. The tag scan stays for long tags, and its docstring says it does not bound attribute counts. The stated memory bound was false: text per node is bounded only by the size of the report, so at the node limit with 80 characters per element (94 MiB) the recovery path reached +827 MiB. MAX_DECOMPRESSED_REPORT_SIZE falls from 100 MiB to 32 MiB (the node limit already means a legitimate report is at most about 28 MB, at the samples' 35 nodes per KB, and the largest sample is 909 KB), and parse_aggregate_report_xml() holds an uncompressed report to the same length, so the bound holds for every input. Measured at the cap, 999,000 distinct elements with 14 characters of text (31 MiB) need +587 MiB on the recovery path and +302 MiB on the expat path; the constant comment and the changelog state it. When the first parse fails, the exception's traceback kept the partial xmltodict tree alive while the recovery parse ran; it is cleared first (+603 to +587 MiB, and +263 to +241 MiB for 330,000 elements of 80 characters). A test shows the recovery path leaves the same 18 lxml helper objects for a document ten times larger, so no tree survives a parse there either. Refs GHSA-q7gm-8cj9-94hr Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Let the RFC 6591 feedback part win over legacy report text The guard that reads only the first message/feedback-report part tested feedback_report, which the legacy text/plain branch ("A message claiming to be from you has failed ... detected.") also sets, so a legacy text part followed by a real feedback-report part and sample now took the legacy part and never read the ARF, where master read the ARF. The guard now tests is_feedback_report, and reading the feedback-report part discards a sample a legacy part set before it. The same rule closes an older gap: a text/plain part inside the sample, which is the spoofed message's own text, could hold the legacy phrase and replace the report's Source-IP and its sample. Once a feedback-report part is read, the legacy branch is skipped; a legacy report has no such part. Tests cover both orders and the in-sample case; the failure samples parse to byte-identical output. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Hold every report input to the size cap and free failed parses parse_report_file() kept the aggregate parser's exception, whose traceback holds the parser's frames and the parsed tree, while it tried SMTP TLS and then the email parser: a 31 MiB file at the cap cost +1,046 MiB there against +587 MiB parsing it directly. It now keeps only each failure's text and raises from a fresh exception of the relevant format, so the chain survives without the tree. A file that begins with "<" is not tried as JSON (json.loads() decoded it all) or as an email, one that begins with "{" is not tried as an email (an RFC 822 message begins with a header line), and text over the size cap is never handed to the email parser, which has none: a 200 MiB <x/> file reached +2,801 MiB in 4.6 s and now costs +232 MiB (reading the file) in 0.05 s. extract_report() reads an uncompressed report at most one byte past the cap instead of decoding it whole. The cap counted characters for text and bytes for compressed reports, so a non-ASCII report could be four times the cap in bytes: 999,000 elements of 14 emoji (31.2 Mi characters, 71 MiB of UTF-8) cost +649 MiB. _exceeds_size_cap() measures text as UTF-8 (ASCII by length, other text 1 MiB of characters at a time, without a full copy), and bytes are checked before they are decoded. Clearing e.__traceback__ inside the except block did nothing on Python 3.10, which keeps the handled exception's traceback while the handler runs. The lxml recovery now runs after the except block, in _recover_xml(), so the first parse's frames and partial tree are unreachable when it starts on every supported Python; a test holds a weak reference to the first parse's counter and fails on the old code under Python 3.10.19 and, with the clearing removed, under 3.14. The changelog's examples are re-measured under the 32 MiB cap, and the test docstrings cite the same figures. Refs GHSA-q7gm-8cj9-94hr Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Skip the parts inside a failure report's sample Once the sample part was taken, walk() still visited its descendants, and they reached the SMTP TLS and base64 attachment branches: a sample whose spoofed message carried an SVG or zipped HTML attachment made the email rejected ("Missing field: 'feedback'", "Unable to parse"), and a gzipped aggregate report or a tlsrpt+json attachment inside it was returned as that report instead of the failure report. The sample is the reported message, so after it is taken every part inside it is skipped (by the id() of each part its walk() yields). Tests cover SVG, zipped HTML, JSON, gzipped aggregate XML, tlsrpt+json and PDF attachments; the failure samples parse to byte-identical output. The Bug fixes bullet also says that a legacy text part followed by a feedback-report part with no sample after it is now invalid, with a test. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Cap SMTP TLS reports and test the format skips parse_smtp_tls_report_json() decoded and json.loads()-ed a report of any size; only the gzipped path was capped, and 60 MiB of "[]," cost +1,565 MiB. It now rejects bytes or text over MAX_DECOMPRESSED_REPORT_SIZE first, as InvalidSMTPTLSReport, so every report type is held to the cap. parse_report_file() names the actual reason it skips the email parser (the leading "<" or "{", or the size cap) rather than a combined one, and a size message for bytes no longer says "of UTF-8". New tests kill the mutants the review found surviving: removing the "<" skip for JSON, narrowing the email skip to "<", the bytes check >= for >, and removing the SMTP TLS cap. The constant comment and the changelog state the memory at the cap as measured on the current code: +345 MiB on the lxml recovery path and +302 MiB on expat parsing the text directly, +500 MiB and +452 MiB through parse_report_file(). The changelog says only the exception's cause changed for files skipped by format, not the message. Refs GHSA-q7gm-8cj9-94hr Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Skip every sample candidate and correct the stated measurements sample_parts held only the first sample candidate's parts, so a later message/rfc822 (or one after a text/rfc822-headers sample) was dropped as a sample but its parts were still walked, and a gzipped aggregate report inside it was returned in place of the failure report. Every candidate's parts are now skipped. Tests cover both shapes, and a test that a gzipped aggregate report beside the sample is still returned pins the rule (inside the sample: skipped; beside it: read) and fails when the set is built from the whole message. parse_report_file() no longer builds email reasons for content that begins with "<" or "{", which its message never shows, and the size reason reads "Invalid report email: the report is larger than 32 MiB". The memory figures are re-measured in a fresh process with the input read from disk before the baseline: +399 MiB on the recovery path and +338 to +355 MiB on expat for the 31 MiB shape, +1,505 MiB for 60 MiB of JSON before the SMTP TLS cap; the constant comment, the changelog and the test docstrings state them. RFC 6591 section 3.1 is cited for the sample's place, and application/json is named among the attachments that made a failure report invalid. Refs GHSA-qcc2-4x3v-v4g7 Refs GHSA-q7gm-8cj9-94hr Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Scale the timed tests' bounds to the machine running them The mailsuite fix's first CI run on GitHub's shared runners failed six timed tests: the suite ran four to six times slower there than on a workstation, and checks doing real work took longer than their fixed bounds. Several bounds here have far less margin (0.2 s on a test that takes 0.18 s whole), so they would fail the same way. tests/timing.py adds budget(seconds), which measures a fixed reference parse twice just before the timed call and multiplies the bound by the slowdown against a reference machine; a faster machine keeps the written bound. Every timed assert now compares against budget(<written seconds>) computed before the timer starts. No written bound changed and no assert was removed. Refs GHSA-qcc2-4x3v-v4g7 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>	2026-11-09 14:10:30 -05:00
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
3 people	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 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
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
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	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 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 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
3 people	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	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 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	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
3 people	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
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	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
Sean WhalenandClaude Opus 4.8	a67e8d3ebc	10.2.0 - Explain why a report is invalid instead of "Not a valid report" (#802) ... * Explain why a report is invalid instead of "Not a valid report" The parser catches broadly so one malformed report can't crash a batch, but every failure surfaced as the generic ParserError("Not a valid report"), telling operators nothing about the cause. parse_report_file() now keeps each format parser's specific error as it tries aggregate XML -> SMTP TLS JSON -> report email, and when all three reject the input it content-sniffs the leading byte to surface the single relevant reason (e.g. "Invalid aggregate report: Missing field: 'org_name'", or "Not a recognized report format (...)"). The CLI already logs str(error), so this reaches the user with no cli.py change. Every parser catch site also re-raises with `raise ... from <original>`, preserving the underlying ExpatError / JSONDecodeError / KeyError / archive errors on __cause__ for library callers and tracebacks. The same exception *types* are still raised. Finally, the catch-all "unexpected error" branches append `(raised at <file>:<line>)` from the deepest traceback frame, but only when the parsedmarc logger is at DEBUG level (e.g. the CLI's --debug); normal-level output is unchanged. Bumps the in-progress version to 10.2.0 and documents all three in the CHANGELOG. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Cover the failure-report path in parse_report_file error tests The reason-surfacing tests covered the aggregate and SMTP TLS branches but not failure reports, which reach parse_report_file only via the email path. Add a malformed multipart/report failure email (missing the required Source-IP) and assert the message names the failure format and the missing field rather than collapsing to "Not a valid report". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Cover the new error-reporting lines; drop one unreachable catch Bring the lines added by this PR to full test coverage: - Test _exc_origin() with a never-raised exception (no __traceback__) so the "no frames" guard is exercised. - Test _parse_smtp_tls_failure_details() with a non-dict, which raises TypeError (not KeyError) and exercises the generic catch-all. - Test parse_report_email() with an unparseable Date header, which trips the initial mail-parse catch-all and becomes a ParserError. Two dead lines are removed rather than hidden, per the project's "delete unreachable branches, no # pragma: no cover" rule: - _looks_like_email() looped with a `continue` for blank lines, but every caller passes lstrip()-ed text, so the first line is never blank. Simplified to inspect the first line directly. - parse_report_email()'s `except Exception` after `except InvalidFailureReport` was unreachable: parse_failure_report wraps its entire body and provably raises only InvalidFailureReport, which the preceding handler already catches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix IndexError when backfilling envelope_from from SPF results _parse_report_record() backfills a missing/empty envelope_from from the last SPF auth result's domain. The "envelope_from is None" branch gated on the raw auth_results["spf"] list but indexed the filtered new_record["auth_results"]["spf"] list, which only holds results that have a domain. A reporter sending an SPF result with no domain made the filtered list empty while the raw list was non-empty, so [-1] raised IndexError and the whole record failed to parse. The two near-identical envelope_from backfill branches (missing identifier vs. empty identifier) drifted apart -- only one was updated when the filtered new_record list was introduced -- which is what let them disagree on which list to read. Merge them into a single path, keyed on dict.get("envelope_from") is None, that gates and indexes the same raw list with the "domain" membership guard the missing-identifier branch already used. Regression test: envelope_from=None with an SPF result carrying no domain now parses to envelope_from=None instead of raising. This is the bug that motivated the surrounding error-reporting work in this PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Trim comment * Cover the touched error branches; drop a dead UnicodeDecodeError catch Codecov flagged the pre-existing error branches this PR touched (adding `from e` / `_exc_origin`) as changed-and-uncovered. Most are real malformed-report paths, so add honest tests that drive them with realistic inputs: - parse_smtp_tls_report_json: nested missing key (date-range without start-datetime) -> InvalidSMTPTLSReport chaining a KeyError. - parse_aggregate_report_xml: non-structured report_metadata -> the AttributeError branch ("Report missing required section"). - parse_report_email: valid legacy text/plain failure report (success path), a text report missing its fields, a base64 attachment of malformed aggregate XML, and one of invalid SMTP TLS JSON. - parse_report_file: gzipped junk -> the str branch of the content sniff. The `except UnicodeDecodeError` in extract_report is removed as dead code (no `# pragma: no cover`, per the repo rule): str-mode streams are already rejected by explicit isinstance checks, and every decode() uses errors="ignore", so it can never fire. str-mode still raises ParserError. Also rename the two new failure-report tests from "Forensic" to "Failure" and add an AGENTS.md rule: RUF reports are "failure reports"; "forensic" is reserved for the literal backward-compat alias identifiers only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix pyright errors in the new error-branch tests CI runs pyright over the whole repo (tests included); these slipped through because the local check only covered parsedmarc/__init__.py: - _parse_smtp_tls_failure_details("not a dict") is a deliberate wrong-type test -> targeted `# pyright: ignore[reportArgumentType]`. - result["report"]["source"]["ip_address"] on a ParsedReport TypedDict -> cast(FailureReport, result["report"]) first, matching existing tests. This is what failed lint-docs-build (and, since `test` needs it, skipped the Codecov upload) on the prior commits. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-25 14:53:56 -04:00
Sean WhalenandClaude Fable 5	eaeea4f53d	Make the whole codebase pass pyright cleanly and enforce it in CI (#798) ... * Make the whole codebase pass pyright cleanly and enforce it in CI Fix all 102 pyright (1.1.410, standard mode) errors across the library, tests, and maps scripts, then pin and enforce the zero-errors bar: - postgres.py: make the optional psycopg import TYPE_CHECKING-aware so the module is properly typed while keeping the runtime install-hint fallback; import psycopg.types.json explicitly as psycopg_json (the old psycopg_types.json attribute access only worked because psycopg imports the submodule eagerly); have _connect()/_ensure_connected() return the live connection so save methods use a non-Optional local; type the DDL list as list[LiteralString] to match psycopg's execute() overloads. - kafkaclient.py: resolve the kafka-python 2.x/3.x bootstrap-error fallback statically via TYPE_CHECKING (kafka-python 3.0 removed NoBrokersAvailable), which also fixes _BootstrapError's import resolution in tests. - syslog.py: go through getattr/setattr for SysLogHandler.socket (absent from typeshed); type the save_* methods with the report TypedDicts (single or list, matching cli.py call sites — gelf.py gets the same signatures); raise ValueError when retry_attempts < 1 instead of falling through and registering a None handler (bug fix, with a regression test and a CHANGELOG entry). - elastic.py / opensearch.py: human_result params are Optional[str]. - maps scripts: sort_csv declared a return type but never returned (now -> None); seen_sort_field_values was possibly unbound; convert_to_utf8's src_encoding is Optional[str]. - tests: cast sample-report dict helpers to their TypedDicts; mark deliberate wrong-type calls with targeted pyright ignores; add narrowing asserts for Optional results; access the mocked KafkaProducer through a cast helper; match the mailsuite fetch_message base signature (**kwargs); patch the renamed parsedmarc.postgres.psycopg_json in test_postgres's setUpModule. Enforcement: [tool.pyright] in pyproject.toml (include parsedmarc, tests, docs; standard mode), pyright==1.1.410 pinned in the [build] extra (pinned exactly so a new pyright release can't break CI without a code change), and a "Check types" step in the lint CI job — which now also runs ruff format --check and installs the [postgresql] extra so the optional psycopg import resolves. Documented in AGENTS.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Set session headers via update() instead of replacing the dict requests 2.34 ships inline type annotations, and Session.headers is a CaseInsensitiveDict[str] — assigning a plain dict fails pyright there (the CI runner resolved 2.34.2; the local venv's untyped 2.32.4 hid it). headers.update() is correctly typed against both versions, and is the documented requests idiom: it overrides User-Agent and the client-specific headers while keeping the session's defaults (Accept-Encoding, Connection) instead of wiping them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-06-12 21:33:01 -04:00
Sean WhalenandClaude Opus 4.7	ef2fb84cc0	test: cover parsedmarc's mailbox processing loop end-to-end on a real Maildir (#777) ... AGENTS.md notes get_dmarc_reports_from_mailbox was halted at low coverage because honest testing needed a live IMAP server or mocks so deep they test the mock. mailsuite's MaildirConnection is a real on-disk backend with no network or credentials, so the fetch -> parse/classify -> route loop can now be exercised for real in CI. TestGetDmarcReportsFromMailboxMaildir delivers real sample reports (one aggregate, failure, and SMTP-TLS email) plus an unparseable message into a Maildir INBOX, runs get_dmarc_reports_from_mailbox offline, and asserts on observable results — parsed report counts and which archive subfolder each message physically lands in: - each report type routed to Archive/{Aggregate,Failure,SMTP-TLS}, the junk message to Archive/Invalid, INBOX drained - delete=True removes processed messages instead of archiving them - test=True parses and returns reports but moves nothing and creates no folders setUp resets the module-global SEEN_AGGREGATE_REPORT_IDS dedup cache so test order can't drop an already-"seen" aggregate report, and the maildir lives at a fresh subpath so mailbox.Maildir(create=True) actually builds cur/new/tmp. Lifts parsedmarc/__init__.py from 76% to 82%, honestly. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-21 12:37:24 -04:00
Sean WhalenandClaude Opus 4.7	a6778707d7	Finish forensic→failure rename: archive-folder migration + dashboard/doc cleanup (#776) ... The forensic→failure rename (#659) left a few loose ends and one deliberate hold-back. This closes them. Leftover rename misses (broken paths / stale canonical names): - CONTRIBUTING.md, dashboard-dev-bootstrap.sh: samples/forensic/* → samples/failure/* - dashboard-dev-bootstrap.sh, dashboards/README.md: dmarc_forensic_dashboard.xml → dmarc_failure_dashboard.xml (the file was already renamed; the import path and view name were not) - docs/source/usage.md: PARSEDMARC_GENERAL_SAVE_FORENSIC → ..._SAVE_FAILURE example - samples/parsedmarc.ini: save_forensic → save_failure - pyproject.toml, README.md: canonical "failure" naming (ci.ini intentionally keeps save_forensic to smoke-test the deprecated alias.) Archive subfolder rename + on-startup migration: - New failure reports now archive to <archive>/Failure (was <archive>/Forensic). - _migrate_forensic_archive_folder() runs once on startup (best-effort): renames Forensic→Failure when no Failure folder exists yet, merges the two when both exist, no-ops when there's no legacy folder, and logs-and-skips a mailbox it can't reorganize (warn, don't crash). This consolidates pre- and post-rename failure reports into one folder, replacing the previously documented decision to keep the folder named Forensic to avoid a split archive. Uses the folder-management API (folder_exists / rename_folder / merge_folders) added in mailsuite 2.1.0; the pin is bumped to >=2.1.0. Grafana dashboard (the rename PR updated OSD/Splunk/ES-OS but not Grafana): - Forensic panel titles + the datasource label → Failure; the fo-column display label and its linked byName field-override matcher both → "Failure Policy" (changed together so the column-width override keeps matching). - dev-bootstrap Grafana ES datasource: dmarc_forensic* → dmarc_f* (matches both pre-rename dmarc_forensic* and post-rename dmarc_failure*, like the OSD/Kibana dashboards); RESEED wipe loop now also clears dmarc_failure* indices. - Removed dashboards/grafana/Grafana-DMARC_Reports.json-new_panel.json, an orphan export accidentally committed in #736 and referenced by nothing. Tests (tests/test_init.py): - TestMigrateForensicArchiveFolderMaildir: real on-disk Maildir round-trips via mailsuite's MaildirConnection (no mocks) — rename, merge, no-op, and the full get_dmarc_reports_from_mailbox orchestration. Runs in CI (no network/creds). - TestMigrateForensicArchiveFolderErrorHandling: the one path a real Maildir can't reproduce — a backend that raises mid-operation must warn, not crash. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-21 12:29:40 -04:00
Sean WhalenandClaude Opus 4.7	b7b8383fa4	Expand honest test coverage from 59% to 83%; fix two latent bugs (#775) ... * Expand honest test coverage from 59% to 83%; fix two latent bugs 271 new tests across the output modules, ES/OS clients, CLI config parsing, and the top-level parsing surface. Coverage measured against shipped code only (see [tool.coverage.run] source = ["parsedmarc"] omit = ["*/parsedmarc/resources/maps/*.py"] in pyproject.toml). Per-module results: s3.py 38% → 100% (also fixes SMTP-TLS-to-S3 bug below) gelf.py 40% → 100% syslog.py 46% → 100% kafkaclient.py 34% → 100% splunk.py 24% → 100% loganalytics.py 56% → 100% webhook.py 78% → 100% (also removes redundant try/except) elastic.py 36% → 99% opensearch.py 40% → 99% cli.py 52% → 69% __init__.py 74% → 76% (also fixes append_json bug below) utils.py 84% (unchanged in this PR) TOTAL 59% → 83% The remaining 17% is honest. The biggest unreached blocks are _main() in cli.py and the watch-mode mailbox iteration in __init__.py, both of which would require either standing up live subsystems (real Elasticsearch, real IMAP) or mocking deep enough that the test would verify the mock rather than the code. The PR-A AGENTS.md guidance — "if 90% requires faking it, ship 85% honestly" — applies here. Bugs fixed while writing tests: 1. parsedmarc/s3.py — SMTP-TLS-to-S3 was completely broken. save_report_to_s3 unconditionally read report["report_metadata"] when building S3 object metadata, but RFC 8460 §4.3 SMTP TLS reports are flat (no report_metadata sub-object). The CLI's surrounding try/except silently swallowed the KeyError, so every SMTP-TLS report quietly failed to upload. Also fixes a related issue: parse_smtp_tls_report_json stores begin_date as the raw ISO-8601 string from the report (per the SMTPTLSReport TypedDict and RFC 8460 §4.3), but the S3 code path assumed a datetime with .year / .month / .day attributes. Both fixed; the broken metadata-extraction branch now uses the flat-report fields, and the date branch normalizes via human_timestamp_to_datetime. 2. parsedmarc/__init__.py — append_json corrupted JSON output files on the second write. The original implementation opened files in "a+" mode, then seek()ed backwards to overwrite the trailing "]" with ",\n" before appending more elements. Python's docs are explicit (https://docs.python.org/3/library/functions.html#open): on POSIX, writes in "a"/"a+" mode always go to EOF regardless of seek() position. The result was that the second call produced [...]\n],\n[...] -style corrupted output instead of a single merged array. Replaced with a read-merge-write pattern: load the existing array (if any), append the new elements, rewrite the whole file. The CSV cousin append_csv was not affected — it doesn't seek backwards. 3. parsedmarc/webhook.py — removed redundant try/except blocks in save_aggregate_report_to_webhook / save_failure_report_to_webhook / save_smtp_tls_report_to_webhook. _send_to_webhook already catches every Exception itself, so the outer except blocks were unreachable dead code (covered nothing, defended against nothing, and inflated the source-line count without testing value). Testing approach: mocks at SDK boundaries (boto3 resource, kafka producer, requests session, opensearch/elasticsearch Document/Search, azure LogsIngestionClient). Tests verify the parsedmarc-side transformation logic — document/event construction, index/topic naming, dedup queries, error wrapping — rather than asserting on mock invocations as a proxy for behaviour. Where a branch is defensive against a caller that doesn't exist in the codebase, the test is omitted (commented in code rather than hidden behind a pragma). 547 tests total (was 276), all passing. ruff check + format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Document the two bug fixes from this PR in the 10.0.0 changelog Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Document testing standards in AGENTS.md Adds a "Testing standards" section covering the principles applied in PR-A (split) and PR-B (coverage expansion): - Coverage measures shipped code only — don't reintroduce tests/* to the scope, don't expand omit, don't use # pragma: no cover. - Honest tests assert on observable behaviour, not "the mock was called". Mock at SDK boundaries; parse the payload that gets sent. - "If 90% requires faking it, ship 85% honestly" — coverage is a tool, not a goal. PR-B's deliberate stops at cli.py 69% and __init__.py 76% are the documented precedent for when to halt. - Verify bug claims against the relevant RFC, internal types, installed SDK source, or upstream docs before changing code. Cite the source in the commit message and test docstring (RFC 8460 §4.3 and the Python open() docs for #775's two bug fixes are the pattern to follow). - Bugs found while writing tests are fixed in the same PR; the test doubles as the regression guard. - File layout (tests/test_<module>.py) is non-negotiable; module-level test loggers need fresh-handler setup so test ordering doesn't break assertLogs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Cover the corrupt-file fallback in append_json Codecov flagged 2 missing patch-coverage lines on PR #775: the except (json.JSONDecodeError, OSError) branch in append_json, which falls back to overwriting when the existing file isn't a parseable JSON array. Two new tests in tests/test_init.py:TestAppendJson exercise both paths: - test_corrupt_existing_file_is_overwritten_cleanly: existing file contains invalid JSON; append_json overwrites with the new array. - test_existing_file_with_non_list_root_is_overwritten: existing file parses as {"foo": ...} (dict, not list); the isinstance guard rejects it and we overwrite cleanly. Patch coverage now 100% on the bug fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-20 20:35:22 -04:00
Sean WhalenandClaude Opus 4.7	5b08627eaa	Split tests.py into per-module tests/test_<module>.py (#774) ... * Split tests.py into per-module tests/test_<module>.py The 5174-line tests.py monolith is split into per-module files under tests/, mirroring the checkdmarc layout: tests/test_init.py parsedmarc/__init__.py parsing surface tests/test_cli.py parsedmarc/cli.py + config / env-vars / SIGHUP tests/test_utils.py parsedmarc/utils.py (DNS, IP info, PSL, etc.) tests/test_webhook.py parsedmarc/webhook.py tests/test_kafkaclient.py parsedmarc/kafkaclient.py tests/test_splunk.py parsedmarc/splunk.py tests/test_syslog.py parsedmarc/syslog.py tests/test_loganalytics.py parsedmarc/loganalytics.py tests/test_gelf.py parsedmarc/gelf.py tests/test_s3.py parsedmarc/s3.py tests/test_maps.py parsedmarc/resources/maps/ maintainer scripts The split is purely a redistribution — no test bodies changed, no tests added or removed. All 276 existing tests pass under the new layout. The current tests.py contains two kitchen-sink classes (`Test` at line 54 and `TestEnvVarConfig` at line 2360) holding tests that span many modules. Their methods are routed to the correct per-module file by name prefix; the wholly-thematic classes (TestExtractReport, TestUtilsXxx, TestSighupReload, etc.) move whole. Each target file gets its own `class Test(unittest.TestCase)` for the redistributed kitchen-sink methods, plus the thematic classes verbatim. Wiring updates: - `.github/workflows/python-tests.yml`: `pytest ... tests.py` → `python -m pytest ... tests/` (also switches to `python -m pytest` per the checkdmarc convention so cwd lands on the project root). - `pyproject.toml`: adds `[tool.pytest.ini_options] testpaths = ["tests"]` and `[tool.coverage.run] source = ["parsedmarc"]` with an `omit` for `parsedmarc/resources/maps/*.py`. The maps scripts are maintainer-only batch tooling that ships out of the wheel; excluding them from coverage makes the headline number reflect only installed library code. Runtime coverage on the new layout is 59% (was 45% with maps counted), and PR-B will push it to 90%+. - `AGENTS.md`: documents the new layout and how to run individual files / tests; tells future contributors not to reintroduce a monolithic tests.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Restore 66.9% coverage baseline (count tests/ + parsedmarc) Master's headline 66.9% number on Codecov includes the tests.py file itself (99.35% covered) being measured alongside parsedmarc/*. The original tests.py had no `[tool.coverage.run]` block, so coverage's default — "measure every file imported during the run" — counted the test code as if it were product code. The split commit added `source = ["parsedmarc"]` which suppressed measurement of the test files (correct in principle, since test files aren't shipped code), and that alone made the headline number drop by ~8 percentage points without any actual loss of testing. This commit swaps `source` for an explicit `include = ["parsedmarc/*", "tests/*"]` so both halves are measured the way they were on master. Verified: 276 tests, 66.96% line coverage (effectively unchanged from master's 66.90%). If you want the shipped-code-only number (was the headline that this commit overrides), run `pytest --cov=parsedmarc tests/`. That number is currently 59% and is the focus of the upcoming coverage-expansion PR. Also adds junit.xml to .gitignore so the CI artefact doesn't get accidentally committed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Restrict coverage to shipped code (`source = ["parsedmarc"]`) Reverts the prior commit's `include = ["tests/*"]`. Counting the test files toward coverage was wrong — it conflates "shipped code exercised by tests" with "test code that pytest auto-runs", inflates the headline number, and rewards writing more tests rather than tests that verify more code. Master's apparent 66.9% was an artefact of the old monolithic tests.py having no [tool.coverage.run] block at all; coverage's default behaviour measured every imported file, including the test file itself at ~99% "covered", which added ~8 percentage points to the displayed number without any real testing signal. Restricting to `source = ["parsedmarc"]` plus the existing maps omit gives a meaningful baseline: 59% of shipped code is exercised by the test suite today. That's the number the next PR is targeting to lift to 90%+ before the 10.0.0 release; the Codecov "drop" here is a measurement correction, not a regression. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-20 19:29:09 -04:00