mirror of
https://github.com/domainaware/parsedmarc.git
synced 2026-09-10 16:08:00 +00:00
77045c2df0f7da5083892917dccc402902f2a525
1646
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  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>
|
||
|
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> |
||
|
Sean WhalenandClaude Opus 5
|
95ddc8a323 |
Fix missing-file error paths falling through in find_unknown_base_reverse_dns.py (#903)
Two `if not os.path.exists(path): print(f"Error: ...")` checks printed the intended error message but had no sys.exit(1) after it, so execution fell through into the subsequent open() call on the same missing path and raised an unhandled FileNotFoundError instead of the clean error exit the code clearly intended. Sibling duplicate-entry checks in the same function already did print-then-sys.exit(1); these two now match. Sites fixed (both in _main()): - the nested load_list() helper's missing-file check (used for known_unknown_base_reverse_dns.txt and psl_overrides.txt) - the base_reverse_dns_map.csv missing-file check Both sites now have regression tests in a new TestFindUnknownBaseReverseDNS class: - the load_list() site: calling _main() in a temp directory with no known_unknown_base_reverse_dns.txt now raises SystemExit(1) instead of FileNotFoundError. - the base_reverse_dns_map.csv site: _load_as_name_index() does its external work entirely through maxminddb.open_database(), the actual SDK boundary, so that call is mocked to a context manager over an empty iterable rather than loading the real ~23MB bundled MMDB or mocking an internal helper. Calling _main() in a temp directory with no base_reverse_dns_map.csv now raises SystemExit(1) instead of FileNotFoundError. Both tests capture stdout and assert on the specific "Error: ... does not exist" message, pinning the exit to the intended site rather than any sys.exit(1) in the function. Cleanup uses two separate addCleanup calls (rmtree registered before chdir, so LIFO order runs chdir first) instead of one lambda wrapping both, so rmtree still runs even if chdir were to raise. Also fixed an adjacent prose bug a few lines from the second site: the "is in known_unknown... and base_reverse_dns_map..." error message was missing a space after "Error:" and was a backslash-continued f-string that embedded the source's literal indentation in the printed output. It now prints as a single clean line, consistent with the file's other error messages. Logged the user-facing symptom (clean error replaced by a FileNotFoundError traceback) under CHANGELOG.md's Unreleased/Bug fixes section, matching the project's precedent of logging maintainer-tooling fixes (e.g. the sortlists.py entry). Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Sean WhalenandClaude Opus 5
|
4298603713 |
Fix CodeQL findings: use sys.exit(), remove dead index_date stores (#898)
py/use-of-exit-or-quit (29 sites): replace bare `exit(...)` calls with `sys.exit(...)` in cli.py (20 sites), sortlists.py (4 sites), and find_unknown_base_reverse_dns.py (5 sites). `exit` is injected into the interactive namespace by the `site` module, so it is not guaranteed to exist under `python -S` or an embedded interpreter — the very error paths that called it would raise NameError instead of exiting cleanly. `sys.exit` is always available and was already used elsewhere in these files. sortlists.py did not import `sys`; the import was added to its existing stdlib import block. find_unknown_base_reverse_dns.py already imported `sys` (used at line 71), so only the call sites changed there. Checked tests/test_cli.py for anything patching `exit` directly — none do; the SystemExit-based assertions exercise the same behavior for either spelling, so no test changes were needed. py/multiple-definition (4 sites): removed the dead `index_date` computation in save_aggregate_report_to_elasticsearch() (elastic.py:944-947) and save_aggregate_report_to_opensearch() (opensearch.py:875-878). Both blocks derived index_date from the report-level begin_date, but the per-record loop immediately below unconditionally recomputes index_date from each record's own begin_date before the only read of the variable (the index name build near the end of the loop body). Verified by reading both functions that no code between the outer assignment and the loop's recompute reads index_date, and that the recompute is unconditional on every path that reaches the read, before deleting. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
 
|
e1c2e42d5d |
Dockerfile: bind-mount the wheel so it stops shipping in every published image (#893)
* Dockerfile: bind-mount the wheel instead of COPYing it The runtime stage COPYs the built wheel out of the build stage and the RUN that installs it rm -rf's it again. A RUN cannot remove what an earlier instruction already committed -- it writes a whiteout on top -- so the COPY layer ships in every pull. Measured on ghcr.io/domainaware/parsedmarc:11.0.0: on amd64 layer 4 is 10,713,473 bytes of a 267,653,434-byte image (4.0%); on arm64 10,713,473 of 270,400,579 (3.96%). Layer 5 carries tmp/.wh.dist. The 10.5.0 and 10.0.0 tags carry the same layer. Bind mounts are not committed to layers, so the rm -rf /tmp/dist clause is no longer needed and is removed. A # syntax=docker/dockerfile:1 directive is added so RUN --mount is guaranteed available. * Add --no-cache-dir option to pip install Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Dockerfile: drop the syntax directive; document the size win `RUN --mount=type=bind,from=<stage>` has been stable in BuildKit's built-in Dockerfile frontend since 2020, and the Docker workflow builds through docker/setup-buildx-action + docker/build-push-action, so the mount works without `# syntax=docker/dockerfile:1`. The directive only adds an unpinned Docker Hub round trip before the Dockerfile can be read (`resolve image config for docker-image://docker.io/docker/dockerfile:1`), on a floating tag, and it does not help the legacy non-BuildKit builder, which rejects `RUN --mount` either way. Verified on linux/amd64 with BuildKit (buildx 0.37.0, Docker 29.8.0): building with and without the directive produced five layers totalling 163,757,439 and 163,757,152 compressed bytes respectively -- a 287-byte gzip nondeterminism, otherwise identical. The master baseline built six layers totalling 272,138,724 bytes, whose wheel layer of 10,713,475 bytes reproduces the published 11.0.0 image's 10,713,473 to within two bytes. The comment block is trimmed to state the invariant rather than narrate what the code used to do, and CHANGELOG.md gains an entry: the change is user-visible, and the ~40% reduction is mostly the pip download cache that --no-cache-dir removes, not the wheel layer the PR was opened for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Sean Whalen <44679+seanthegeek@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
 github-actions[bot]andseanthegeek
|
de88cd893f |
chore: update IPinfo Lite MMDB (#896)
Co-authored-by: seanthegeek <44679+seanthegeek@users.noreply.github.com> |
||
|
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>11.0.1 |
||
|
github-actions[bot]andseanthegeek
|
823b0a1811 |
chore: update IPinfo Lite MMDB (#894)
Co-authored-by: seanthegeek <44679+seanthegeek@users.noreply.github.com> |
||
|
Sean WhalenandClaude Fable 5
|
15e7db1811 |
11.0.0 release: bump version and finalize changelog (#892)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>11.0.0 |
||
|
Sean WhalenandClaude Fable 5
|
0d832ff682 |
Make dashboard-dev-bootstrap.sh work with Docker or Podman (#891)
The bootstrap script hardcoded `docker compose`, locking out contributors on Podman-based systems. It now auto-detects a working container engine (Docker preferred when both are usable) and its Compose implementation (`docker compose`/`docker-compose`, `podman compose`/`podman-compose`), selectable explicitly with --backend docker|podman or the CONTAINER_BACKEND environment variable (the flag wins). Detection requires a live `<engine> info` call, so a leftover docker CLI with no running daemon does not shadow a working podman, and both compose forms are probed with `version` so a broken install fails at startup instead of partway through the run. Adds -h/--help documenting the flag and the script's env knobs. Every container invocation already flowed through the single COMPOSE array, so the rest of the script is unchanged. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Sean WhalenandClaude Fable 5
|
07b00a931b |
Warn on unmatched CLI paths; send Kafka failure/SMTP TLS reports per-report (#890)
Fixes the two remaining bugs and the nits uncovered during the review session: - The CLI logs a warning for each file_path argument that matches no files, instead of silently succeeding with empty results — a typo'd path in a cron job previously 'worked' forever while processing nothing (_expand_file_path_args dropped it as a zero-match glob). Deliberately a warning, not an error: a glob legitimately matching nothing must not break existing workflows, and mailbox-only runs stay silent. - save_failure_reports_to_kafka and save_smtp_tls_reports_to_kafka now send one message per report, mirroring the aggregate saver's per-slice sends. Every released version documented per-record sends, but the code sent the whole list as one message, which a large batch — failure reports carry message samples — could push past Kafka's default 1MB message limit. Consumer-visible: these topics now carry individual report objects, not one JSON array per batch. - The CLI module docstring and argparse description now mention SMTP TLS reports; usage.md's CLI-help mirror regenerated. - The elastic/opensearch to_header display-name tests now assert the joined 'RT <rcpt@example.com>' string reaches the saved document (autospec save), plus the exactly-once call, instead of only that save ran. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Sean WhalenandClaude Fable 5
|
2d76de9ca6 |
Fix ignored number_of_replicas and add a --dns-timeout alias (#889)
Two fixes from the #888 review cycle's flagged items:
- [elasticsearch]/[opensearch] number_of_replicas is no longer ignored
when number_of_shards is not also set. The parser read replicas only
inside the shards branch — accidental nesting introduced in the
6.4.0-era code (
|
||
|
|
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
|
52be8850b2 |
10.5.0 release: bump version and finalize changelog (#887)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>10.5.0 |
||
|
Sean WhalenandClaude Fable 5
|
cd864f4cfc |
Add DNS over HTTPS and DNS over TLS support via the nameservers option (#886)
Each entry in the existing nameservers option now selects its own transport (#880): an IP address means plain DNS on port 53 exactly as before, an https:// URL means DoH, and tls://ip[:port][#hostname] means DoT, with the optional #hostname naming the TLS certificate identity (systemd-resolved syntax). Forms can be mixed in one list, and no new configuration option is involved. DoH queries go through a shared per-process httpx client passed to dns.query.https as session=, which is what makes them honor HTTP_PROXY/ HTTPS_PROXY/NO_PROXY and SSL_CERT_FILE — the motivating proxy-only corporate network case. dnspython's stock DoH path cannot do this: it builds its httpx client around a custom transport, and httpx only reads proxy environment variables when no transport is supplied (allow_env_proxies = trust_env and transport is None). The client is rebuilt when the PID changes so fork-based worker pools never share a parent's sockets. The dnspython requirement becomes dnspython[doh]>=2.7.0 — the extra supplies the httpx/h2 floors DoH needs, and 2.7.0 is the floor verified against the dns.nameserver and dns.query.https(session=...) APIs used. The startup DNS pre-flight check now exercises whichever transports are configured, so a malformed DoH/DoT entry raises ConfigurationError before any mailbox work begins. Malformed tls:// entries — including the plausible slash-for-# typo tls://9.9.9.9/dns.quad9.net, which would otherwise silently drop the certificate identity — are rejected at configuration time naming the entry. Verified live: DoH A/PTR queries against Cloudflare and DoT against Quad9 (tls://9.9.9.9#dns.quad9.net), plus a full CLI run over a sample report with encrypted-DNS-only nameservers. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
ffdb220af9 |
Import optional mailbox connections lazily in parsedmarc.mail (#884)
* Import optional mailbox connections lazily in parsedmarc.mail * Fix docstring grammar Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Cover the lazy-import guards with tests and harden the AuthMethod placeholder Maintainer follow-up to the review of #884: - Add tests/test_mail.py: a sys.meta_path blocker simulates installs without the gmail/msgraph stacks, exercising both except branches. CI always has the extras installed, so without a simulated absence the guarded paths would never execute and the blocking codecov patch gate would fail. Includes isolation-safe purge/restore of sys.modules so test ordering cannot leak placeholder state into other suites. - Replace the AuthMethod = None sentinel with a placeholder that raises the msgraph extra's ImportError on non-dunder attribute access, iteration, call, and subscript (every Enum usage shape), instead of surfacing cryptic NoneType AttributeError/TypeError. - Raise a fresh ImportError chained to the stored original in the connection placeholders, so repeated constructions do not keep appending frames to one shared exception object's traceback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Sean Whalen <44679+seanthegeek@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
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> |
||
|
github-actions[bot]andseanthegeek
|
d52fca130d |
chore: update IPinfo Lite MMDB (#881)
Co-authored-by: seanthegeek <44679+seanthegeek@users.noreply.github.com> |
||
|
Sean WhalenandClaude Fable 5
|
6358c36401 |
chore: update GitHub Actions to latest major versions (#879)
- actions/checkout v5 -> v7 - actions/setup-python v6 -> v7 - actions/configure-pages v5 -> v6 - actions/upload-pages-artifact v3 -> v5 - actions/deploy-pages v4 -> v5 - actions/upload-artifact v4 -> v7 - actions/download-artifact v4 -> v8 - docker/setup-qemu-action v3 -> v4 - docker/setup-buildx-action v3 -> v4 - docker/metadata-action v5 -> v6 - docker/login-action v3 -> v4 - docker/build-push-action v6 -> v7 - codecov/codecov-action v5 -> v7 - peter-evans/create-pull-request v7 -> v8 codecov/test-results-action (already replaced by codecov-action with report_type: test_results) and pypa/gh-action-pypi-publish@release/v1 (moving branch, still the current major) need no change. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Sean WhalenandClaude Fable 5
|
e48521bb47 |
CI: replace deprecated codecov/test-results-action with codecov-action@v5 (#877)
codecov/test-results-action@v1 emits two warnings on every test run: its bundled actions/github-script pin targets deprecated Node.js 20, and Codecov deprecated the action itself in favor of running codecov/codecov-action@v5 with report_type: test_results. Upload the JUnit results through the same codecov-action already used for coverage. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Sean WhalenandClaude Fable 5
|
8e200181e7 |
10.4.3 release: bump mailsuite floor to >=2.3.1 (#876)
mailsuite 2.3.1 fixes mailsuite.utils.parse_email() raising TypeError
instead of ValueError("Not an email") on unparseable (non-email) input
under mail-parser 4.6.2 (seanthegeek/mailsuite#61), which parsedmarc's
floors have required since 10.4.2.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
10.4.3
|
||
|
Sean WhalenandClaude Fable 5
|
2352603eda |
10.4.2 release (#875)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>10.4.2 |
||
|
Sean WhalenandClaude Fable 5
|
400f3d319c |
Automate releases and docs deployment (#874)
Port mailsuite's tag-triggered release pipeline: - Add release.yml: pushing a version tag runs the full CI suite (python-tests.yml via workflow_call), then builds the package (the tag must match the version in parsedmarc/constants.py, checked with `hatch version`), publishes to PyPI via Trusted Publishing, creates the GitHub Release with notes from the tag's CHANGELOG.md section and the built distributions attached, pushes the multi-arch Docker image, and deploys the Sphinx docs - Add docs.yml: reusable docs build/deploy to GitHub Pages, also runnable on demand (workflow_dispatch) for documentation-only changes between releases - docker.yml: add a workflow_call trigger with a push_image input, since a GitHub Release created with the workflow's own GITHUB_TOKEN emits no `release: published` event; release.yml calls it directly instead - Remove the legacy build.sh / publish-docs.sh manual process - AGENTS.md: CRITICAL rule that releases require explicit maintainer permission, plus docs for the new release flow and its one-time repo/PyPI configuration prerequisites - Bump the mailsuite floor to >=2.3.0 (raises the transitive mail-parser floor to >=4.6.2 and cryptography to >=50.0.0) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
github-actions[bot]andseanthegeek
|
7acfaa0cea |
chore: update IPinfo Lite MMDB (#873)
Co-authored-by: seanthegeek <44679+seanthegeek@users.noreply.github.com> |
||
 ericericsw
|
c730a1aefa |
Correcting the incorrect interval name (#872)
Correcting the incorrect interval name: fixed_interval->interval Maintaining the fixed_interval setting will cause errors in the dashboard output. |
||
|
github-actions[bot]andseanthegeek
|
5b2dfa7d1a |
chore: update IPinfo Lite MMDB (#871)
Co-authored-by: seanthegeek <44679+seanthegeek@users.noreply.github.com> |
||
|
github-actions[bot]andseanthegeek
|
6f56b92c44 |
chore: update IPinfo Lite MMDB (#870)
Co-authored-by: seanthegeek <44679+seanthegeek@users.noreply.github.com> |
||
|
Sean WhalenandClaude Opus 5
|
b304cf8639 |
Cover index_prefix_domain_map and index_suffix in index migrations (#868) (#869)
* Cover index_prefix_domain_map and index_suffix in index migrations (#868) The startup Elasticsearch/OpenSearch index migrations built their target index names from the [elasticsearch]/[opensearch] index_prefix and index_suffix options alone, while the save path also honors general.index_prefix_domain_map. A multi-tenant deployment therefore ran the backfill guard `count` against dmarc_aggregate*/smtp_tls*, patterns matching none of its real <tenant>_dmarc_aggregate-* indexes. The query passes allow_no_indices=True, so a zero-match wildcard returns count 0 -- indistinguishable from "already backfilled" -- and the backfill was skipped silently, with no log line at any level. Resolve migration index names through a new _migration_index_names() helper that widens both configurable axes: one name per tenant prefix in the map plus the unprefixed name (unmapped domains are still saved unprefixed), and, when an index_suffix is set, the unsuffixed name alongside the suffixed one so history predating the suffix is covered. A configured index_prefix still wins outright and suppresses the map fan-out, matching save-time precedence. The key normalization is now shared with get_index_prefix() via _normalize_index_prefix(), so the names parsedmarc migrates cannot drift from the ones it writes. The resolved lists are logged at DEBUG, and the SIGHUP reload path passes the freshly parsed map, so a newly onboarded tenant is covered without a restart. Also repair the legacy published_policy.fo migration, which has been unable to complete since mapping types were removed in Elasticsearch 7 (and never existed in OpenSearch): it read the field mapping in the type-keyed response shape, so the check always fell through, and its put_mapping() call passed a doc_type argument neither current client accepts. It now reads either response shape, uses each client's current signature, and takes its index names in a separate legacy_fo_indexes argument -- exact names, since 5.0.0 introduced date-suffixed index names in the same release that fixed the fo declaration, but prefixed and suffixed where configured, since both options date back to 4.1.0. Tenant prefixes are excluded: index_prefix_domain_map arrived in 8.19.0, and this migration renames the index it rebuilds. The Elasticsearch copy, removed as unreachable during the #806 client migration, is restored now that the cause is understood. Finally, reject an index_prefix_domain_map YAML file that is not a mapping of string tenant names to lists of domains, instead of raising mid-save on a non-string key or silently matching the wrong domains on a scalar value -- `in` on a str is a substring test, so "example.co" matches "example.com" (https://docs.python.org/3/reference/expressions.html#membership-test-operations). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fix OpenSearch capitalization and spacing in the usage docs Copilot review of #869: the `index_prefix_domain_map` option line spelled "OpenSearch" as "Opensearch" and had a doubled space before the type. Both predate this branch but sit inside a hunk it rewrites. Also wrapped the line to match the continuation-indent style of every other option in the list, and corrected the same misspelling in the multi-tenant section a few lines above the paragraph this branch added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Require index_prefix_domain_map domain lists to hold strings Copilot review of #869: the shape check verified that each value was a list but not what the list contained, so `tenant_a: [42]` passed and then never compared equal to any domain the save path looks up -- the same silent-misbehavior class the check exists to reject, and a contradiction of the "any other shape is rejected at startup" claim in the docs. Check the list's items too, and reword the error message, comment, CHANGELOG and docs to state the rule the check now enforces: a mapping of tenant names to lists of domain names, all strings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Make the legacy fo migration retry-safe, and nest its mapping body Copilot review of #869, verified against live Elasticsearch 8.19 and OpenSearch 3 containers rather than mocks. Retry safety (a real defect): an attempt interrupted between creating the -v2 index and deleting the original left debris that made create() fail with "resource already exists" on every later startup, inside the same try that swallows the error -- so the index was never migrated and the debris document survived. Reproduced on a live cluster. Discard a leftover target first; that is safe precisely because reaching this point means the original still holds the data, since it is deleted only once the reindex has succeeded. Mapping body: the reviewer's concern that a dotted key under `properties` risks a runtime failure does not hold -- both clusters accept it and produce a byte-identical mapping, with the dot expanded into published_policy -> properties -> fo. Switch to the nested object form anyway, since dot expansion is conditional on the object's `subobjects` setting and this shape never is, and derive the object/leaf names from the dotted constant so the write cannot drift from the field the read looks up. Both fo-migration suites now build per-name Index mocks. A single shared mock cannot express "the original exists but its migration target does not", which is the ordinary case and the one the retry fix turns on; the happy path now also asserts the target index is never deleted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>10.4.1 |
||
|
github-actions[bot]andseanthegeek
|
7500d48c9e |
chore: update IPinfo Lite MMDB (#867)
Co-authored-by: seanthegeek <44679+seanthegeek@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>10.4.0 |
||
|
Sean Whalen
|
483ae8112d | 10.4.0 release | ||
|
Sean WhalenandClaude Fable 5
|
a5a5b45a50 |
Unify DMARC pass/fail chart naming on "DMARC compliance" (#865)
* Unify DMARC pass/fail chart naming on "DMARC compliance" The same metric had three names across the dashboards: the pass/fail pie chart was "Passed DMARC" (OpenSearch, Splunk) and "DMARC Passage" (Grafana), while the per-domain table column added in #834 was "% DMARC Compliant". Converge on the compliance wording, which already anchored the "Message volume and DMARC compliance by from domain" panels in every dashboard family: - Pie chart: "DMARC compliance" ("DMARC Compliance" in Grafana, which title-cases panel names; OpenSearch and Splunk use sentence case). - Line chart: "DMARC passage over time" -> "DMARC compliance over time" everywhere, so the pie rename doesn't leave the same inconsistency one panel down. - Splunk filter dropdown label: "Passed DMARC" -> "DMARC compliant". - Grafana Guide panel text and docs/source/kibana.md prose updated to match, plus a pre-existing typo fix ("pie charts. you"). Field names, tokens, and queries (passed_dmarc / dmarc_passed) are untouched; only human-facing titles, labels, and prose changed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix "pie charts. you" typo in the Grafana Guide panel text The period-for-comma typo fixed in docs/source/kibana.md also existed in the Grafana Guide panel markdown, in both duplicated copies of the guide text (content and options.content). Caught by Copilot review on PR #865. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Sync the Grafana Guide panel walkthrough with the current panels The Guide text described a three-column table layout with a from-domain list "on the right" and directed readers to a "Message From Header" table, but the dashboard has two table columns (Reporting Organisations on the left; Top 2000 Message Sources by Reverse DNS on the right with Message volume and DMARC compliance by from domain below it) and no Message From Header table. Point the walkthrough at the real panel titles and mention the per-domain compliance percentage the table shows, mirroring the kibana.md walkthrough updated in #834. Both duplicated copies of the guide markdown (content and options.content) remain identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Scope the CHANGELOG rename claim to dashboards that have the charts The bullet said the pass/fail charts were renamed "across the ... Grafana dashboards", but the PostgreSQL Grafana variant has no pass/fail pie or over-time chart, so the claim quantified over a dashboard it doesn't apply to. Name the affected dashboards explicitly and note the PostgreSQL variant is unchanged. Caught by Copilot review on PR #865. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Sean WhalenandClaude Fable 5
|
641c64ad02 |
Add PR #863 review lessons to AGENTS.md (#864)
Five additions distilled from the #863 review cycle (mailbox save-callback / #242), filed under the existing themes: - Review prose as prose: adjacent code (constants, literals with escape-rule surprises) is fair game like adjacent prose. - Check claims against what they range over: a contract that signals failure two ways owes both the same safety bookkeeping; bookkeeping paired with a side effect must follow it, not precede it. - Honest tests: a test named for an exclusive claim ("only"/"never"/ "exactly once") must observe both halves. - Verify what CI enforces: an end-to-end run must execute the working tree, not a stale installed copy. Lessons that were instances of existing rules (enumeration counting, adjacent typos, reflow-don't-argue) were deliberately not duplicated. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
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> |
||
|
Sean WhalenandClaude Fable 5
|
f1f31542ad |
Prune obsolete permission allowlist entries (#862)
Remove the four exact-match pytest rules referencing the retired tests.py monolith and the one-off py_compile rule; the pytest/ruff prefix rules added in #861 cover all current invocation forms. Follow-up to Copilot review feedback on #861. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Sean WhalenandClaude Fable 5
|
a03b5074ee |
Allowlist the current test/lint/type-check command forms (#861)
The existing pytest allow rules all reference the retired tests.py monolith and never match anymore, so every test run prompts. Add the forms actually in use (pytest/.venv/bin/pytest, with and without the GITHUB_ACTIONS=true offline prefix), the venv-path pyright that auto-allow doesn't recognize, and the two exact pinned python -m ruff check forms. Derived from a 50-session transcript scan; read-only or test-scoped commands only. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Sean WhalenandClaude Fable 5
|
dbfc8a0a11 |
Remove the inert enter-auto-mode clause from the model split (#860)
Permission mode is controlled by the Claude Code harness (the plan-approval dialog, Shift+Tab, launch flags), not by the model — no CLAUDE.md instruction, hook, or setting can switch modes after plan approval, so the clause was dead text that read as a promise the model can't keep. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Sean WhalenandClaude Fable 5
|
29a4dd5149 |
Consolidate AGENTS.md review lessons thematically (#859)
* Document the review lessons from PR #858's review cycle Three rules distilled from the misses: plain-type docstrings are wrong when None is a semantic state; "pre-existing" triage stops applying when the diff extends the set a claim quantifies over; and verify CI's gates (patch coverage), not just its commands — with ad-hoc checks built to fail loudly on empty matches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Use the standard unhyphenated "ad hoc" and "post hoc" Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Consolidate the review lessons thematically instead of chronologically The review section had grown by accretion — a new block of rules per review cycle (#834, #839, #849, #851, #858), each retelling its incident at length, with the same principles recurring under different PR numbers. Regroup all fifteen rules into five themes (prose as prose; nothing is pre-verified; claims vs. what they range over; verify what CI enforces; fresh-context review), keeping every rule and every concrete incident compressed to one clause with its PR number. Cuts the section from ~1,830 to ~1,180 words with no rule lost. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Make mid-incident glue the fifth disguise, matching the stated count Copilot review: "Nothing is pre-verified" announced four disguises but listed four bullets plus a trailing unbulleted rule — ambiguous by this file's own enumeration-counting standard. Fold mid-incident glue into the enumeration (its mechanism is the same: code that escapes review because of how it was produced) and restore the same-scrutiny-as-a- subagent clause dropped during consolidation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Pluralize the review-discipline intro; standardize on American spelling Copilot review round: the intro used distributive singulars against a five-PR enumeration, and the file mixed "behaviour" (older Testing standards text) with "behavior" (newer sections). Standardized the whole file to American spelling per maintainer preference. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
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> |
||
|
Sean WhalenandClaude Fable 5
|
264b9a4556 |
Add [general] archive_directory to archive processed local files (#570) (#856)
* Ignore Claude Code agent worktrees under .claude/worktrees/ Untracked repo snapshots from agent sessions were making repo-root ruff check . fail on stale code and cluttering git status. ruff respects .gitignore, so ignoring the directory fixes both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add [general] archive_directory to archive processed local files (#570) Move successfully processed report files given as local path arguments into <archive_directory>/<year>/<month>/<Aggregate|Failure|SMTP-TLS>/, dated from the parsed report's own metadata (aggregate begin_date, failure arrival_date_utc, SMTP TLS begin_date) rather than the filename. Files that fail to parse as a report (ParserError) go to <archive_directory>/Invalid/; other failures (e.g. transient I/O errors) leave the file in place so a later run can retry it. Existing destination files are never overwritten: each candidate name is claimed atomically (O_CREAT | O_EXCL) and collisions get a numeric suffix before the extension. Files already inside the archive directory are excluded from processing (compared via realpath so symlinked spellings still match), so the archive can safely live inside an input directory, as the issue requests. mbox files and mailbox modes are unaffected; mailbox modes keep [mailbox] archive_folder. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address review feedback on docstrings and the placeholder-cleanup except - _move_file_to_archive's docstring no longer claims the copy2 fallback replaces the placeholder atomically; only the same-filesystem os.rename path is atomic. The fallback is a plain copy-and-overwrite, which is still collision-safe because the placeholder already claimed the name. - The empty except OSError in the placeholder cleanup now carries a comment explaining that it is deliberate best-effort cleanup and the re-raised move failure is the actionable error. - test_general_archive_directory_unset_leaves_attribute_absent's docstring now describes what the assertion actually tests (the attribute staying absent from a bare Namespace) and moves the real-CLI None-default behavior to a parenthetical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Cover the two defensive exception branches Codecov flagged Codecov's patch report flagged four uncovered lines, all in the two platform-dependent exception branches of the archive helpers: - _exclude_archived_paths's except ValueError branch. Per the Python docs for os.path.commonpath, ValueError is raised when paths "are on the different drives" (Windows) or mix absolute and relative pathnames; both inputs are realpath()-resolved so only the different-drives case remains, which Linux CI can't produce naturally. The new test simulates the raise and asserts the non-comparable path is kept for parsing rather than excluded. - _move_file_to_archive's except OSError placeholder-cleanup branch. The new test fails the move with a non-OSError type and the cleanup with OSError, then asserts the move error is what propagates (the cleanup error is swallowed, not allowed to mask it) and that the zero-byte placeholder survives its failed cleanup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Tag arrival_date_utc as UTC when parsing the archive date human_timestamp_to_datetime's docstring names arrival_date_utc as exactly the kind of known-UTC naive string that should be parsed with assume_utc=True; _archive_subdir_for_result was parsing it naive. The flag is scoped to the failure branch because the shared call also handles the other two report types: aggregate begin_date is a local-time wall-clock string (timestamp_to_human uses datetime.fromtimestamp), so tagging it UTC would be wrong, and SMTP TLS begin_date carries an RFC 3339 offset, making assume_utc a no-op. No observable behavior change: only the wall-clock year/month fields are read and assume_utc never shifts wall-clock time, so no new test can honestly distinguish the two versions — this aligns the call with its dependency's documented contract and hardens against a future edit adding a real timezone conversion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Create the archive placeholder with mode 0o600 os.open's mode parameter defaults to 0o777 (masked by the umask), so the O_CREAT|O_EXCL placeholder in _move_file_to_archive was created executable and group-accessible on typical umasks (0o775 under umask 002). Normally it's replaced immediately, but a placeholder that outlives a failed move+cleanup persisted with those permissions. Pass 0o600 explicitly. The mode never reaches the real archived file: os.rename replaces the placeholder's inode outright, and the copy2 fallback's copystat overwrites the mode with the source file's. The leftover-placeholder regression test now also asserts the surviving placeholder has no owner-exec or group/other bits (umask-independent, since the umask only clears bits); the assertion fails against the unfixed default-mode call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Sean WhalenandClaude Fable 5
|
f3689c3125 |
Use Opus for large implementations in the CLAUDE.md model split (#857)
* Use Opus for large implementations in the CLAUDE.md model split Sonnet remains the default implementer for routine, well-scoped changes; large multi-file features or refactors now use Opus, decided at planning time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Define the "complex" Opus trigger with concrete criteria Addresses Copilot review feedback on #857: the heading promised Opus for "large or complex work" but the body only defined the large multi-file case. Add concrete examples of contained-but-complex changes (subtle parsing/encoding logic, concurrency, paired protocols). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Sean WhalenandClaude Fable 5
|
a421747a79 |
Add a Domain policy dropdown filter to the Splunk aggregate dashboard (#854) (#855)
Adds a dropdown that filters every panel on the published DMARC policy (published_policy.p) or subdomain policy (published_policy.sp), mirroring the Message disposition dropdown's choices and default and sitting immediately before it, per the issue spec. Both fields are always present on parsedmarc-written events (sp defaults to p in the parser), so the wildcard default matches everything, consistent with the other raw-search filters. Also fixes the Splunk docs page's XML-files link, which still pointed at the pre-#736 splunk/ path and 404ed. Closes #854 Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Sean WhalenandClaude Fable 5
|
b02a7972b5 |
Clarify the two-directory code span in the #851 verification lesson (#853)
Copilot flagged on #852 (post-merge) that the code span `parsedmarc/ tests/` reads as a single path when it names two directories. Spell them out as separate spans. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Sean WhalenandClaude Fable 5
|
2084e5d162 |
Document the review lessons from PR #851's review cycle (#852)
PR #851 was the first cycle run with all prior review guidance loaded in context. The author's own seam-checklist pass caught three real defects before the PR opened, but a fresh-context reviewer still caught four more across two rounds. Three new rules drawn from the shapes of those misses: - A fix made during review is new code with zero review coverage; touching one direction of a paired protocol obligates re-deriving the inverse direction, including version-skew inputs. - Verification means CI's literal commands from the repo root, not a plausible subset; fix noise via config exclusions, don't narrow the command. - Count enumerations in prose against the code-defined set they enumerate. 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
|
4f4733003c |
Document the review lessons from PR #849's review cycle (#850)
A fresh automated reviewer caught five defects across two rounds of PR #849 that the authoring model's own review passes missed. Three new review rules capture the shape of those misses: moved code is new code (a verbatim extraction carried a latent FileHandler-dedup asymmetry past review because "pure move" felt pre-verified), an extracted helper is new API surface (the promoted pool helper was missing input validation its old call site had made unnecessary, and its docstring drifted from its stop behavior), and end with a fresh-context review rather than a self re-read (the author's cold re-read is never cold). Also documents the CHANGELOG release convention in the Releases section: feature/fix PRs accumulate entries under "## Unreleased" and never pick a version number; the release PR renames the heading and bumps parsedmarc/constants.py together. PR #849 initially guessed a "10.4.0" heading and had to walk it back. 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> |
||
|
Sean WhalenandClaude Fable 5
|
c95d4666dc |
Release 10.3.0 (#848)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>10.3.0 |
||
|
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> |
||
|
Sean WhalenandClaude Fable 5
|
df086e7242 |
Fix mbox-run progress display and document n_procs scope (#147) (#846)
* Fix mbox-run progress display and document n_procs scope (#147) An mbox-only run showed a misleading, permanently-stuck `0it` progress bar: the CLI's tqdm bar only tracks report files passed directly as arguments (mbox paths are split out first), and per-message mbox progress is only logged at INFO, which --silent / config-file runs hide. The empty bar is no longer created when there are no direct file arguments, and get_dmarc_reports_from_mbox() now wraps its message loop in a tqdm bar that auto-disables on non-TTY output (tqdm's disable=None), so interactive mbox imports show real per-message progress while tests, cron jobs, and piped runs stay clean. Also documents that n_procs parallel parsing applies only to report files passed directly on the command line — messages from mbox files and mailbox connections (IMAP, Microsoft Graph, Gmail API, Maildir) are always processed sequentially — and fixes the pre-existing "Number of process" typo in that entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Gate the CLI progress bar on stderr, where tqdm renders it Addresses Copilot review feedback on #846: tqdm writes to stderr by default, but the guard checked sys.stdout.isatty(), so a run with stderr redirected to a log file would write bar escapes into the log, and a run with stdout redirected (e.g. piping the JSON output) hid the bar even though stderr could display it. This also matches the mbox bar's disable=None auto-check, which keys off tqdm's own output stream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |