mirror of
https://github.com/domainaware/parsedmarc.git
synced 2026-11-11 03:11:33 +00:00
d9193eafea124bd4e9acc37fdde4b585d6fbb44d
6
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  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> |
||
 
|
c7a9d294b5 |
fix: restore parent IP-lookup state in pool workers under spawn/forkserver (#930)
* fix: restore parent IP-lookup state in pool workers under spawn/forkserver
parsedmarc.utils keeps the IPinfo Lite REST API token
(_IPINFO_API_TOKEN, configure_ipinfo_api) and the selected IP database
path (_IP_DB_PATH, load_ip_db) in process-global variables, and only
the parent process sets them. Under the fork start method a pool worker
inherits them ("All resources of the parent are inherited by the child
process", https://docs.python.org/3/library/multiprocessing.html), but
under spawn (macOS/Windows default) and forkserver (the Linux default
from Python 3.14) it does not ("The child process will only inherit
those resources necessary to run the process object's run() method").
A worker there started from scratch: its IPinfo API lookups were
silently skipped, and _get_ip_database_path() fell back to the bundled
MMDB instead of the downloaded or cached copy the parent selected, so
enrichment differed from the operator's configuration and from the same
run under fork. The CLI parses file inputs through the process pool
even with -n 1, so every file-based CLI run on such platforms was
affected.
parallel_map() now snapshots both globals in the parent and restores
them in every worker via its pool initializer, so IP enrichment matches
the parent's configuration under every start method. The initializer
previously only reconstructed logging and is renamed _init_worker
accordingly.
Fixes #928
Co-authored-by: ZCode Scout <noreply@zcode.local>
Signed-off-by: NefelibateLG <3453510037@qq.com>
* Maintainer edits: Unreleased changelog, PSL overrides, forkserver coverage
- CHANGELOG: move the entry under "## Unreleased" (feature/fix PRs never
pick a version number) and describe the psl_overrides gap.
- Restore parsedmarc.utils.psl_overrides in pool workers as a separate
initarg, in place ([:] =), never rebound. A fresh spawn/forkserver
worker loads the bundled psl_overrides.txt at import and only switches
to the operator's path/URL on its first reverse DNS map load, so
get_base_domain() calls before that load (the first aggregate report's
org_name, the first source IP's reverse DNS name) used the bundled list.
- Cross-reference _init_worker and cli._utils_globals_snapshot(), which
enumerate the same three utils globals.
- Extend TestWorkerIPState to forkserver (where available) and to the
PSL overrides list; without the restore, both subtests fail with the
worker reporting the bundled list.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
---------
Signed-off-by: NefelibateLG <3453510037@qq.com>
Co-authored-by: ZCode Scout <noreply@zcode.local>
Co-authored-by: Sean Whalen <44679+seanthegeek@users.noreply.github.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
|
||
|
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> |
||
|
Sean WhalenandClaude Fable 5
|
5c7f4ba048 |
Clear shared IP cache in parallel test setUp for isolation (#866)
tests/test_parallel.py's parity test compares sequential parse_report_file(path, offline=True) results in the parent process against results from cold worker processes. When the full suite runs locally (GITHUB_ACTIONS unset), tests/test_init.py runs first with real DNS lookups and warms the shared module-level parsedmarc.IP_ADDRESS_CACHE; get_ip_address_info consults the cache before honoring offline, so the sequential baseline returned DNS-enriched entries (e.g. reverse_dns='smtp7.cardinal.com') while the workers correctly returned None, failing the test. CI never sees this because it runs offline from the start. Clear the cache in _ParallelTestCase.setUp so baselines and workers both start cold. Test-only change; the cache-before-offline ordering in utils.py is intentional (a cache hit makes no network queries). Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
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> |
||
|
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> |