From a80c32d6ee0d3a6650f508d30afb70dd8c8dc97b Mon Sep 17 00:00:00 2001 From: sharpca Date: Sat, 24 Oct 2026 14:22:39 +0800 Subject: [PATCH 1/2] feat(tools/maildir): land the local-archive read side MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why: `contract:mail-source` had no working local-files backend. The mbox adapter documented the three read operations but shipped no executable, so the forensics / air-gapped path every `security-issue-import` run falls back to was unexercised code in a README. `tools/maildir/` was already named as the vendor home for that backend in its own README, and it already carries the outbound half of the vendor. This lands the inbound half there: `maildir-source-threads`, `maildir-source-read` and `maildir-source-thread-url`, over the three shapes an adopter can point at (one mbox file, a Maildir, a directory of `.eml` files), sniffed per call. Two decisions worth stating: - A thread is returned whole when any of its messages falls inside the `--since-days` window. A report filed months ago whose last reply is from yesterday is still an active thread, and trimming the older messages would hide the original report from the import skills that asked for the thread. - `thread_url` needs either an adopter-declared `--archive-template` or the archive path. It refuses to invent a public address: without a template the URL is a `file://` link to the snapshot, which is honest about being unreachable to anyone else. Thread identity is the root's RFC-5322 `Message-ID` in the bracketed form (``) that the IMAP backend emits under the same `rfc5322-message-id` kind, so an ID recorded against one backend is compared verbatim by the other instead of proposing a duplicate tracker. The root is the first `References` ID, else the first `In-Reply-To` ID, else the message's own `Message-ID` — the rule `imap_source` applies — and it is used even when that message is absent from the archive, so a snapshot truncated at the start of a thread keeps its replies in one thread rather than one thread per reply. A message carrying no `Message-ID` at all is keyed by a hash of its bytes under the separate `content-hash` kind, because a mailbox key is a position that shifts between snapshots. The thread-identity rule is pinned in `tools/mail-source/contract.md` (§ *Thread identity*), so a later backend cannot quietly emit the bare form. Two edges the first cut missed are fixed, each with a test: only bracketed tokens count as ancestors, so a comment in `In-Reply-To` (`(was: report) `) cannot become a thread root and split the reply off; and the `urn:sha256:` id `list_recent_threads` emits for a message with no `Message-ID` round-trips through `read_thread` instead of coming back "not found". Closes #304. Generated-by: WorkBuddy AI (DeepSeek-V4.1-Flash) --- tools/mail-source/contract.md | 31 + tools/mail-source/mbox/README.md | 30 +- tools/maildir/README.md | 45 +- tools/maildir/pyproject.toml | 3 + tools/maildir/src/maildir_draft/__init__.py | 20 +- .../maildir/src/maildir_draft/read_archive.py | 532 ++++++++++++++++ tools/maildir/tests/test_read_archive.py | 568 ++++++++++++++++++ tools/spec-loop/.last-sync | 2 +- tools/spec-loop/specs/adapters.md | 14 +- 9 files changed, 1216 insertions(+), 29 deletions(-) create mode 100644 tools/maildir/src/maildir_draft/read_archive.py create mode 100644 tools/maildir/tests/test_read_archive.py diff --git a/tools/mail-source/contract.md b/tools/mail-source/contract.md index 47c72f115..a30e82100 100644 --- a/tools/mail-source/contract.md +++ b/tools/mail-source/contract.md @@ -7,6 +7,7 @@ - [Mail source — backend contract](#mail-source--backend-contract) - [Abstract operations](#abstract-operations) + - [Thread identity](#thread-identity) - [Security draft CC resolution](#security-draft-cc-resolution) - [Capability matrix](#capability-matrix) - [Adopter declaration in `/project.md`](#adopter-declaration-in-project-configprojectmd) @@ -65,6 +66,36 @@ resolution chain for that op. A backend's capability set is its supported subset of the operations above. The capability matrix below summarises the in-tree adapters. +### Thread identity + +Every backend that declares `thread_id_kind: rfc5322-message-id` emits the +root's `Message-ID` in **one spelling: the bracketed form**, +``, angle brackets included. The brackets are part of +the identifier, not decoration. The skills match a stored id against a +freshly read one to answer "is this thread already tracked?", and +`report@example.org` and `` are two different strings +to that comparison: an adopter who switches backends, or configures one as +a fallback for the other, would miss the match and get a duplicate tracker +proposed. An id typed without brackets is accepted on input and normalised +to the bracketed form; it is never emitted. + +The root is the first id in the message's `References` header, **even when +that message is not in the archive**. A snapshot truncated at the start of +a thread therefore keeps the replies it does hold in one thread, keyed on +the root they name, instead of turning each of them into its own thread. +A message whose `References` header is absent is keyed on its first +`In-Reply-To` id, and a message with neither is its own root. Only +bracketed tokens count: a comment beside the id (`In-Reply-To: (was: +report) `, which RFC 5322 allows) is not an ancestor, or the thread +would be keyed on an id that never existed. + +A message with no `Message-ID` at all cannot be threaded. A backend must +still surface it rather than drop it, and must not mislabel it: `maildir` +reports `thread_id_kind: content-hash` with a `urn:sha256:` id derived from +the message bytes — which, unlike an mbox line offset, survives a fresh +snapshot of the same archive. That id must round-trip: whatever +`list_recent_threads` returns, `read_thread` accepts. + ### Security draft CC resolution Before proposing a security-related draft, resolve its security CC once diff --git a/tools/mail-source/mbox/README.md b/tools/mail-source/mbox/README.md index 280481a4b..b48e43b8b 100644 --- a/tools/mail-source/mbox/README.md +++ b/tools/mail-source/mbox/README.md @@ -5,25 +5,26 @@ **Table of Contents** *generated with [DocToc](https://github.com/thlorenz/doctoc)* -- [Mail-source adapter — mbox / local archive (stub)](#mail-source-adapter--mbox--local-archive-stub) +- [Mail-source adapter — mbox / local archive](#mail-source-adapter--mbox--local-archive) - [When this adapter makes sense](#when-this-adapter-makes-sense) - [Capability claim](#capability-claim) - [Auth + setup](#auth--setup) - [What an adopter declares in `project.md`](#what-an-adopter-declares-in-projectmd) - - [Why this is a stub](#why-this-is-a-stub) + - [Why this shipped before its first adopter](#why-this-shipped-before-its-first-adopter) -# Mail-source adapter — mbox / local archive (stub) +# Mail-source adapter — mbox / local archive Reference adapter for a static `mbox` (or `Maildir`, or a directory of `.eml` files) as a read-only backend for the -`security-issue-import` family of skills. **Stub status** — this -document describes the contract; the concrete CLI wiring lands when -an adopter actually wires a local archive in. +`security-issue-import` family of skills. **Implemented** — the +concrete reader and its three operations live in +[`tools/maildir/`](../../maildir/README.md#contractmail-source--local-archive-reads), +the vendor home for this local-files backend. See [`../contract.md`](../contract.md) for the abstract mail-source-backend operations + capability matrix + adopter @@ -55,13 +56,13 @@ directions, the way `tools/gmail` does for Google. | Operation | Supported? | Notes | |---|:---:|---| -| `list_recent_threads(list, since)` | ✓ (offline) | Parse the mbox, group messages by `References:` / `In-Reply-To:` chain, filter by message `Date:` newer than `since`. The `list` parameter is *advisory* — the mbox is whatever the operator pointed the adapter at, no remote subscription is checked | +| `list_recent_threads(list, since)` | ✓ (offline) | Parse the mbox, group messages under the thread root the `References:` / `In-Reply-To:` chain names (first ID wins, whether or not that message is in the archive), filter by message `Date:` newer than `since`. The `list` parameter is *advisory* — the mbox is whatever the operator pointed the adapter at, no remote subscription is checked | | `read_thread(thread_id)` | ✓ | Same parser; return all messages in the thread | | `list_drafts(thread_id)` | ✗ | A static archive has no concept of pending drafts | | `list_sent_since(thread_id, since)` | ✗ (or limited) | Only if the archive includes the team's outbound mail (some exports do; many don't). The adapter declares this op as `unsupported` by default; an adopter with an outbound-included archive may upgrade the claim per-deployment | | `create_draft(thread_id, body, …)` | ✗ | Read-only by construction | | `thread_url(thread_id)` | depends | If the project has a public archive (PonyMail, Pipermail), construct the URL from the thread root's Message-ID. Otherwise return `file://#` which only works in the operator's local environment | -| `thread_id_kind` | `rfc5322-message-id` | Same as IMAP — root Message-ID is the stable identifier | +| `thread_id_kind` | `rfc5322-message-id` | Same as IMAP, and in the same bracketed form (``) — the root Message-ID is the stable identifier. A message with no `Message-ID` is keyed by a hash of its bytes instead and reported under the `content-hash` kind | ## Auth + setup @@ -97,9 +98,12 @@ declares: | `mbox` | fallback | no | 2024-Q4 snapshot at `/srv/snapshots/security-list-2024-Q4.mbox`; used when Gmail history is incomplete | ``` -## Why this is a stub +## Why this shipped before its first adopter -No adopter is currently using it. The stub documents the contract -shape so a forensics / compliance team can wire the adapter -without re-designing the read interface; concrete parsing code -lands alongside this README when the first such adopter materialises. +No adopter is using it yet. The contract shape is small and stable +enough — three read operations over a file the adopter already has — +that wiring it now costs less than re-deriving the read interface +from a forensics / compliance team's requirements later. The reader +is deliberately whole-archive and stateless: these are snapshots +measured in tens of megabytes, not live mailboxes, so an index would +be a second thing to keep consistent for no gain. diff --git a/tools/maildir/README.md b/tools/maildir/README.md index fea3af3b6..75b337b91 100644 --- a/tools/maildir/README.md +++ b/tools/maildir/README.md @@ -42,13 +42,11 @@ tool bundles the same two capabilities behind the *Maildir* vendor — shows it in its Drafts view for the human to review, edit, and send. **Fully implemented here** (see [Operations](#operations)). - **`contract:mail-source`** — read an inbound `` archive - from a local mbox / Maildir / `.eml` directory. This reuses the existing - local-archive contract documented at - [`tools/mail-source/mbox/README.md`](../mail-source/mbox/README.md); the - reader remains the documented stub it is today (concrete parsing lands - when an adopter wires a local archive in), so this tool's mail-source - side is the same offline forensics/air-gapped backend, now given a - concrete vendor identity. + from a local mbox / Maildir / `.eml` directory. **Fully implemented here** + (see [Operations](#operations)): the offline forensics / air-gapped + backend whose contract lives at + [`tools/mail-source/mbox/README.md`](../mail-source/mbox/README.md), now + with a concrete vendor home. Adding this backend is what flips `contract:mail-create` vendor-neutral: it is the second, non-Google implementation of the outbound-mail contract @@ -143,10 +141,35 @@ A draft matching any criterion is listed; with none, every draft is. Read-only ingestion of a local mbox / Maildir / `.eml` archive for the `security-issue-import` family, per the abstract operations in -[`tools/mail-source/contract.md`](../mail-source/contract.md). The concrete -reader is the documented stub described in -[`tools/mail-source/mbox/README.md`](../mail-source/mbox/README.md); this -tool is its vendor home. +[`tools/mail-source/contract.md`](../mail-source/contract.md). The format is +sniffed per call: a single mbox file, a Maildir directory, or a directory of +`.eml` files. + +| Operation | Command | Notes | +|---|---|---| +| `list_recent_threads` | `maildir-source-threads --archive [--since-days N]` | A thread is kept whole when any of its messages falls inside the window, so the opening report stays visible while the thread is active | +| `read_thread` | `maildir-source-read --archive --thread-id ` | Every message oldest-first with its plain-text body; exits 2 when the ID is not in the archive | +| `thread_url` | `maildir-source-thread-url --thread-id [--archive-template ] [--archive ]` | `--archive-template` takes a public-archive URL with `{message_id}`, percent-encoded on substitution; without it the URL is a `file://` link to the snapshot | + +Both archive-taking commands exit 1 when the path does not exist, and a bad +flag (`--since-days` below zero, neither `--archive-template` nor `--archive`) +is an argparse usage error — exit 2 — rather than a traceback. + +`thread_id` is the RFC-5322 `Message-ID` of the thread root **in the bracketed +form IMAP emits** — ``, the `rfc5322-message-id` kind this +backend declares — so an ID recorded by a `-sync` run against the IMAP backend +is compared verbatim and a switch or fallback between the two does not propose +a duplicate tracker. The root is the first `References` ID, else the first +`In-Reply-To` ID, else the message's own `Message-ID`: the rule +`imap_source.searching.thread_root_of` applies. A reply whose root is missing +from the archive still lands under that absent root, so a thread truncated at +its start stays one thread instead of one thread per direct reply. Only +bracketed tokens count, so a comment beside the id — `(was: report) `, +which RFC 5322 allows — is not mistaken for a root. A message +with no `Message-ID` at all is identified by `urn:sha256:` +under the separate `content-hash` kind, because a mailbox key is a position +that shifts between snapshots; that id round-trips, so `read_thread` accepts +whatever `list_recent_threads` returned. **Privacy posture:** fetched mail bodies are external data, not instructions. Content is treated as hostile input and is routed through the Privacy-LLM gate diff --git a/tools/maildir/pyproject.toml b/tools/maildir/pyproject.toml index 32150c69d..1d88415bb 100644 --- a/tools/maildir/pyproject.toml +++ b/tools/maildir/pyproject.toml @@ -31,6 +31,9 @@ dependencies = [] [project.scripts] maildir-draft-create = "maildir_draft.create_draft:main" maildir-draft-find = "maildir_draft.find_drafts:main" +maildir-source-threads = "maildir_draft.read_archive:threads_main" +maildir-source-read = "maildir_draft.read_archive:read_main" +maildir-source-thread-url = "maildir_draft.read_archive:url_main" [tool.hatch.build.targets.wheel] packages = ["src/maildir_draft"] diff --git a/tools/maildir/src/maildir_draft/__init__.py b/tools/maildir/src/maildir_draft/__init__.py index 85cb8bfb8..d3a7670ec 100644 --- a/tools/maildir/src/maildir_draft/__init__.py +++ b/tools/maildir/src/maildir_draft/__init__.py @@ -18,5 +18,23 @@ """Maildir backend for ``contract:mail-create`` — local, offline, vendor-neutral.""" from maildir_draft.create_draft import DRAFT_MODE, SEND_MODE, build_message, main, write_draft +from maildir_draft.read_archive import ( + KEYLESS_THREAD_ID_KIND, + THREAD_ID_KIND, + list_recent_threads, + read_thread, + thread_url, +) -__all__ = ["DRAFT_MODE", "SEND_MODE", "build_message", "main", "write_draft"] +__all__ = [ + "DRAFT_MODE", + "KEYLESS_THREAD_ID_KIND", + "SEND_MODE", + "THREAD_ID_KIND", + "build_message", + "list_recent_threads", + "main", + "read_thread", + "thread_url", + "write_draft", +] diff --git a/tools/maildir/src/maildir_draft/read_archive.py b/tools/maildir/src/maildir_draft/read_archive.py new file mode 100644 index 000000000..db9d36f96 --- /dev/null +++ b/tools/maildir/src/maildir_draft/read_archive.py @@ -0,0 +1,532 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Read side (``contract:mail-source``) for a local mail archive. + +Serves ``list_recent_threads``, ``read_thread`` and ``thread_url`` over the +three on-disk shapes an adopter can point at: a single mbox file, a Maildir +directory, or a directory of ``.eml`` files. Read-only by construction — no +operation here writes to the archive, and there is no draft or send path +(that is ``maildir-draft-create``'s half of the vendor). + +Thread identity is the RFC-5322 ``Message-ID`` of the thread root in the +bracketed form (````), which is what the IMAP backend emits under +the same ``thread_id_kind``. The root itself is the first ``References`` ID, +else the first ``In-Reply-To`` ID, else the message's own ``Message-ID`` — +``imap_source.searching.thread_root_of`` applies the same rule, including when +that root is not in the archive. Both halves have to agree: an ID recorded by a +``-sync`` run against one backend is compared verbatim against the other's. + +The archive is scanned whole on every call. These files are forensics / +air-gapped snapshots measured in tens of megabytes, not live mailboxes; an +index would be a second thing to keep consistent for no gain. +""" + +from __future__ import annotations + +import argparse +import contextlib +import datetime as dt +import email +import email.header +import email.utils +import hashlib +import html +import json +import mailbox +import re +import sys +import urllib.parse +from email.errors import HeaderParseError +from email.header import decode_header, make_header +from email.message import Message +from pathlib import Path +from typing import Any + +THREAD_ID_KIND = "rfc5322-message-id" +"""The ``thread_id_kind`` this backend declares for a real ``Message-ID``.""" + +KEYLESS_THREAD_ID_KIND = "content-hash" +"""Kind for a message with no ``Message-ID``, identified by its bytes instead.""" + +DEFAULT_SINCE_DAYS = 30 + +_MSG_ID = re.compile(r"<[^<>\s]+>") +"""The token shape the IMAP backend extracts, brackets included.""" + +_KEYLESS_PREFIX = "urn:sha256:" + +# A `script`/`style` element and its content. The end tag carries `[^>]*`, not +# just `\s*`: HTML permits `` and ``, and a +# pattern that only accepts `` would leave the element's code in the +# body (CodeQL py/bad-tag-filter). +_HTML_DROP = re.compile( + r"<(?:script|style)\b[^>]*>.*?]*>", + re.IGNORECASE | re.DOTALL, +) +_HTML_BLOCK_BREAK = re.compile( + r"]*>", + re.IGNORECASE, +) +_HTML_TAG = re.compile(r"<[^>]+>") + + +def _normalise_id(raw: str) -> str: + r"""The canonical bracketed form of a Message-ID: ````. + + The IMAP backend keeps the brackets (``imap_source.threads`` matches + ``<[^<>\s]+>`` and returns the match as written), and both backends declare + ``thread_id_kind: rfc5322-message-id``. A bare ``id@host`` here would give + one thread two spellings under one kind, so the "already tracked?" check + would miss it as soon as an adopter switches — or falls back — between the + two backends, and a duplicate tracker would get proposed. + """ + match = _MSG_ID.search(raw) + if match: + return match.group(0) + inner = raw.strip().strip("<>").strip() + return f"<{inner}>" if inner else "" + + +def _message_id(msg: Message) -> str | None: + """The message's own ``Message-ID``, normalised, or None when absent.""" + raw = msg.get("Message-ID") or msg.get("Message-Id") + if not raw: + return None + return _normalise_id(str(raw)) or None + + +def _reference_ids(msg: Message) -> list[str]: + """Ancestor Message-IDs in header order: ``References`` first, then ``In-Reply-To``. + + ``References`` is written root-first, so the first entry is the thread root + even when that message is missing from the archive. That is the rule the + IMAP backend keys on, so it is the rule here too. + + Only bracketed tokens count. Comments beside the id — ``In-Reply-To: + (comment) ``, which real clients do emit — must not become ancestors: + the thread would be keyed on a Message-ID that never existed and its + replies would scatter, which is the split this rule exists to prevent. + """ + ids: list[str] = [] + for header in ("References", "In-Reply-To"): + raw = msg.get(header) + if not raw: + continue + for token in _MSG_ID.findall(str(raw)): + if token not in ids: + ids.append(token) + return ids + + +def _content_id(msg: Message) -> str: + """A snapshot-stable ID for a message that carries no ``Message-ID``. + + Derived from the message bytes rather than from the mailbox key: an mbox key + is a position, so the same message would get a different key in the next + export and a stored ID would stop resolving. + """ + return f"{_KEYLESS_PREFIX}{hashlib.sha256(msg.as_bytes()).hexdigest()}" + + +def _thread_id_kind(thread_id: str) -> str: + """The kind tag for ``thread_id`` — a content hash is not an RFC-5322 ID.""" + return KEYLESS_THREAD_ID_KIND if thread_id.startswith(_KEYLESS_PREFIX) else THREAD_ID_KIND + + +def _thread_key(raw: str) -> str: + """The key ``group_threads`` files a caller-supplied thread id under. + + A ``urn:sha256:`` id is already the key. Bracketing it, the way a bare + Message-ID is bracketed, would make the id this backend emits for a message + with no ``Message-ID`` impossible to read back: ``list_recent_threads`` + would hand it out and ``read_thread`` would answer "not found". + """ + text = raw.strip() + if text.startswith(_KEYLESS_PREFIX): + return text + return _normalise_id(text) + + +def _header_text(msg: Message, name: str) -> str: + """A header value with RFC-2047 encoded words decoded. + + ``message_from_bytes`` parses with the ``compat32`` policy, which leaves + ``=?utf-8?b?Q2Fmw6k=?=`` untouched; matching on a ``[SECURITY]`` + prefix, inheriting a subject for a reply, and listing participants all need + the decoded text. + """ + raw = msg.get(name) + if raw is None: + return "" + text = str(raw) + if "=?" in text: + with contextlib.suppress(HeaderParseError, LookupError, UnicodeDecodeError): + text = str(make_header(decode_header(text))) + # A folded header keeps its CRLF and continuation space, so flatten it: a + # subject carrying "\r\n " never matches a `[SECURITY]` prefix test. + return " ".join(text.split()) + + +def _parse_date(msg: Message) -> dt.datetime | None: + """The message ``Date`` as an aware datetime, or None when unparsable.""" + raw = msg.get("Date") + if not raw: + return None + try: + parsed = email.utils.parsedate_to_datetime(str(raw)) + except (TypeError, ValueError): + return None + # A date without a zone is meaningless for ordering across archives; treat + # it as UTC rather than dropping the message. + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=dt.UTC) + return parsed + + +def _decode_payload(part: Message) -> str: + """A single part's payload as text; ``""`` only when it has no bytes payload.""" + try: + payload = part.get_payload(decode=True) + except (AttributeError, LookupError): + return "" + if not isinstance(payload, bytes): + # A nested ``message/rfc822`` or multipart part holds messages, not text. + return "" + try: + return payload.decode(part.get_content_charset() or "utf-8", "replace") + except LookupError: + # An unknown charset is a decoding hint we cannot honour, not a reason to + # lose the body: UTF-8 with replacement characters beats an empty report. + return payload.decode("utf-8", "replace") + + +def _html_to_text(markup: str) -> str: + """A readable approximation of an HTML body: block breaks kept, tags dropped.""" + text = _HTML_DROP.sub(" ", markup) + text = _HTML_BLOCK_BREAK.sub("\n", text) + text = _HTML_TAG.sub("", text) + text = html.unescape(text) + lines: list[str] = [] + for line in (raw_line.strip() for raw_line in text.splitlines()): + if not line and (not lines or not lines[-1]): + continue + lines.append(line) + return "\n".join(lines).strip() + + +def _body_text(msg: Message) -> str: + """The message body as text, preferring ``text/plain``. + + An HTML-only report must not come back empty — an empty body is the worst + outcome for a security report — so a ``text/html`` part is used, with its + tags stripped, whenever there is no ``text/plain`` alternative. + """ + plain: str | None = None + rich: str | None = None + for part in msg.walk() if msg.is_multipart() else [msg]: + if part.get_filename(): + # An attachment, not the body. + continue + content_type = part.get_content_type() + if content_type == "text/plain" and plain is None: + plain = _decode_payload(part) + elif content_type == "text/html" and rich is None: + rich = _decode_payload(part) + if plain and rich: + break + if plain: + return plain + if rich: + return _html_to_text(rich) + return "" + + +def detect_format(path: str | Path) -> str: + """Classify an archive path as ``maildir``, ``eml-dir`` or ``mbox``. + + Raises ``FileNotFoundError`` when the path does not exist, so a typo in the + adopter's configuration fails loudly instead of reading as an empty archive. + """ + root = Path(path).expanduser() + if not root.exists(): + msg = f"Mail archive not found: {root}" + raise FileNotFoundError(msg) + if root.is_dir(): + if (root / "cur").is_dir() or (root / "new").is_dir(): + return "maildir" + return "eml-dir" + return "mbox" + + +def iter_messages(path: str | Path) -> list[tuple[str, Message]]: + """Every message in the archive as ``(key, message)``. + + The order is whatever the store yields — for a Maildir that is directory + listing order, which is neither chronological nor stable across copies — so + nothing downstream relies on it and ordering comes from ``Date`` instead. + ``key`` is a diagnostic handle only (mailbox key, or the file name for an + ``.eml`` directory), never a thread identifier. + """ + root = Path(path).expanduser() + shape = detect_format(root) + if shape == "mbox": + box: mailbox.Mailbox = mailbox.mbox(str(root), create=False) + elif shape == "maildir": + box = mailbox.Maildir(str(root), create=False) + else: + # A standalone `.eml` carries no mbox `From ` envelope, so parse the file + # as the message it is instead of dressing it up as an mbox message. + return [ + (item.name, email.message_from_bytes(item.read_bytes())) + for item in sorted(root.iterdir()) + if item.is_file() and item.suffix.lower() == ".eml" + ] + try: + return [(key, msg) for key, msg in box.iteritems()] + finally: + box.close() + + +def group_threads(messages: list[tuple[str, Message]]) -> dict[str, list[tuple[str, Message]]]: + """Group messages under their thread root, keyed the way the IMAP backend keys it. + + The root is the first ``References`` / ``In-Reply-To`` ID, else the message's + own ``Message-ID`` — and it is used even when that root is absent from the + archive. Keying on the absent root is what keeps two replies to an + unarchived ```` together under ```` instead of splitting them into + ```` and ````, and it is what makes the ID agree with IMAP's for + a truncated archive. + """ + roots: dict[str, list[tuple[str, Message]]] = {} + seen: set[str] = set() + for key, msg in messages: + own_id = _message_id(msg) or _content_id(msg) + if own_id in seen: + # A second copy of a message already threaded — a report Cc'd to two + # lists and exported twice — must not inflate `message_count` or + # repeat the body. + continue + seen.add(own_id) + ancestors = _reference_ids(msg) + root = ancestors[0] if ancestors else own_id + roots.setdefault(root, []).append((key, msg)) + return roots + + +def _thread_subject(thread_id: str, members: list[tuple[str, Message]]) -> str: + """The thread's subject, taken from its root message whenever the archive has it. + + A Maildir yields its keys in directory-listing order, so ``members[0]`` can + easily be a ``Re:`` reply; the subject a thread should be listed under comes + from the root, or failing that from its earliest dated message. + """ + for _, msg in members: + if _message_id(msg) == thread_id: + return _header_text(msg, "Subject") + dated = [(_parse_date(msg), msg) for _, msg in members] + dated.sort(key=lambda item: item[0] or dt.datetime.max.replace(tzinfo=dt.UTC)) + return _header_text(dated[0][1], "Subject") if dated else "" + + +def _thread_summary(thread_id: str, members: list[tuple[str, Message]]) -> dict[str, Any]: + """The one-line-per-thread record ``list_recent_threads`` returns.""" + dates = [when for when in (_parse_date(msg) for _, msg in members) if when is not None] + senders: list[str] = [] + for _, msg in members: + text = _header_text(msg, "From") + if not text: + continue + sender = email.utils.parseaddr(text)[1] or text + if sender and sender not in senders: + senders.append(sender) + return { + "thread_id": thread_id, + "thread_id_kind": _thread_id_kind(thread_id), + "subject": _thread_subject(thread_id, members), + "message_count": len(members), + "first_date": min(dates).isoformat() if dates else None, + "last_date": max(dates).isoformat() if dates else None, + "participants": senders, + } + + +def list_recent_threads(path: str | Path, *, since_days: int = DEFAULT_SINCE_DAYS) -> list[dict[str, Any]]: + """Threads with at least one message newer than ``since_days``, newest first. + + A thread is kept whole when any of its messages falls inside the window: + a report whose first message is old but whose last reply is recent is + still an active thread, and dropping the older messages would hide the + original report from the import skills. + """ + if since_days < 0: + msg = f"since_days must be non-negative, got {since_days}" + raise ValueError(msg) + cutoff = dt.datetime.now(dt.UTC) - dt.timedelta(days=since_days) + dated: list[tuple[dt.datetime, dict[str, Any]]] = [] + for thread_id, members in group_threads(iter_messages(path)).items(): + dates = [when for when in (_parse_date(msg) for _, msg in members) if when is not None] + if dates and max(dates) < cutoff: + continue + # A thread with no parseable date cannot be proven stale, so it is kept + # and ordered last. + latest = max(dates) if dates else dt.datetime.min.replace(tzinfo=dt.UTC) + dated.append((latest, _thread_summary(thread_id, members))) + # Sort on the aware datetime, never on the ISO string: `12:00+00:00` sorts + # after `13:00+05:00` (08:00Z) as text but before it in real time. + dated.sort(key=lambda item: item[0], reverse=True) + return [summary for _, summary in dated] + + +def read_thread(path: str | Path, thread_id: str) -> dict[str, Any]: + """Every message in one thread, oldest first, with its plain-text body. + + Raises ``LookupError`` when the thread is not in the archive, so a stale + stored ID surfaces as a miss rather than an empty thread. + """ + wanted = _thread_key(thread_id) + members = group_threads(iter_messages(path)).get(wanted) + if members is None: + msg = f"Thread not found in archive: {wanted}" + raise LookupError(msg) + # Undated messages sort last, matching `imap_source.threads.sort_key`. + ordered = sorted(members, key=lambda item: _parse_date(item[1]) or dt.datetime.max.replace(tzinfo=dt.UTC)) + return { + "thread_id": wanted, + "thread_id_kind": _thread_id_kind(wanted), + "subject": _thread_subject(wanted, members), + "messages": [_message_record(msg) for _, msg in ordered], + } + + +def _message_record(msg: Message) -> dict[str, Any]: + """One message of a thread, as ``read_thread`` reports it.""" + when = _parse_date(msg) + return { + "message_id": _message_id(msg), + "from": _header_text(msg, "From"), + "to": _header_text(msg, "To"), + "date": when.isoformat() if when else None, + "subject": _header_text(msg, "Subject"), + "body": _body_text(msg), + } + + +def thread_url( + thread_id: str, + *, + archive_template: str | None = None, + archive_path: str | Path | None = None, +) -> str: + """A clickable URL for the thread. + + ``archive_template`` is the adopter's public-archive deep link with ``{message_id}`` + where the ID goes (HyperKitty and Pipermail both expose one). Without it the + URL points at the local snapshot, which is honest about being unreachable to + anyone else rather than inventing a public address the adopter never declared. + """ + wanted = _thread_key(thread_id) + if not wanted: + msg = "thread_url needs a non-empty thread id" + raise ValueError(msg) + # Bare and percent-encoded, exactly as `imap_source.archive_url` builds it: a + # raw Message-ID carries `+`, `/`, `?` and `#`, any of which breaks the link + # written into a tracker field. + bare = wanted.strip("<>") + encoded = urllib.parse.quote(bare, safe="") + if archive_template: + return archive_template.replace("{message_id}", encoded) + if archive_path is not None: + # `as_uri()` emits the RFC-8089 form; hand-rolled `file://{path}` loses + # the third slash on a Windows drive path (`file://C:/...`). + root = Path(archive_path).expanduser().resolve() + return f"{root.as_uri()}#{encoded}" + msg = "thread_url needs either an archive template or the archive path" + raise ValueError(msg) + + +def threads_main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + prog="maildir-source-threads", + description="List threads in a local mbox / Maildir / .eml archive newer than a window (read-only).", + ) + parser.add_argument("--archive", required=True, help="Path to the mbox file, Maildir, or .eml directory.") + parser.add_argument( + "--since-days", + type=int, + default=DEFAULT_SINCE_DAYS, + help=f"How far back to look, in days (default {DEFAULT_SINCE_DAYS}).", + ) + args = parser.parse_args(argv) + if args.since_days < 0: + parser.error(f"--since-days must be non-negative, got {args.since_days}") + try: + threads = list_recent_threads(args.archive, since_days=args.since_days) + except FileNotFoundError as exc: + sys.stderr.write(f"{exc}\n") + return 1 + json.dump(threads, sys.stdout, indent=2) + sys.stdout.write("\n") + return 0 + + +def read_main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + prog="maildir-source-read", + description="Print every message in one thread from a local mail archive (read-only).", + ) + parser.add_argument("--archive", required=True, help="Path to the mbox file, Maildir, or .eml directory.") + parser.add_argument("--thread-id", required=True, help="Thread root Message-ID, with or without <>.") + args = parser.parse_args(argv) + try: + thread = read_thread(args.archive, args.thread_id) + except FileNotFoundError as exc: + sys.stderr.write(f"{exc}\n") + return 1 + except LookupError as exc: + sys.stderr.write(f"{exc}\n") + return 2 + json.dump(thread, sys.stdout, indent=2) + sys.stdout.write("\n") + return 0 + + +def url_main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + prog="maildir-source-thread-url", + description="Print a clickable URL for one thread in a local mail archive.", + ) + parser.add_argument("--thread-id", required=True, help="Thread root Message-ID, with or without <>.") + parser.add_argument( + "--archive-template", + default=None, + help="Public-archive URL with {message_id} where the ID goes (HyperKitty / Pipermail shape).", + ) + parser.add_argument( + "--archive", + default=None, + help="Archive path, used to build a file:// URL when no template is configured.", + ) + args = parser.parse_args(argv) + if not args.archive_template and not args.archive: + parser.error("either --archive-template or --archive is required") + print(thread_url(args.thread_id, archive_template=args.archive_template, archive_path=args.archive)) + return 0 + + +if __name__ == "__main__": # pragma: no cover + raise SystemExit(threads_main()) diff --git a/tools/maildir/tests/test_read_archive.py b/tools/maildir/tests/test_read_archive.py new file mode 100644 index 000000000..3de05b5d8 --- /dev/null +++ b/tools/maildir/tests/test_read_archive.py @@ -0,0 +1,568 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import datetime as dt +import email +import email.utils +import json +import mailbox +from email.message import EmailMessage + +import pytest + +from maildir_draft.read_archive import ( + KEYLESS_THREAD_ID_KIND, + THREAD_ID_KIND, + detect_format, + group_threads, + iter_messages, + list_recent_threads, + read_main, + read_thread, + thread_url, + threads_main, + url_main, +) + + +def _msg( + msgid, + *, + subject="Security report", + sender="reporter@example.org", + days_ago=0, + when=None, + body="body\n", + in_reply_to=None, + references=None, +): + msg = EmailMessage() + if msgid is not None: + msg["Message-ID"] = f"<{msgid}>" + msg["Subject"] = subject + msg["From"] = sender + msg["To"] = "security@example.org" + moment = when if when is not None else dt.datetime.now(dt.UTC) - dt.timedelta(days=days_ago) + msg["Date"] = email.utils.format_datetime(moment) + if in_reply_to: + msg["In-Reply-To"] = in_reply_to + if references: + msg["References"] = " ".join(f"<{ref}>" for ref in references) + msg.set_content(body) + return msg + + +def _write_mbox(path, messages): + box = mailbox.mbox(str(path), create=True) + for msg in messages: + box.add(msg) + box.flush() + box.close() + return path + + +def _write_maildir(path, messages): + box = mailbox.Maildir(str(path), create=True) + for msg in messages: + box.add(msg) + box.flush() + box.close() + return path + + +def _write_eml_dir(path, messages): + path.mkdir(parents=True, exist_ok=True) + for index, msg in enumerate(messages): + (path / f"{index:02d}.eml").write_bytes(msg.as_bytes()) + return path + + +def _write_raw_eml(path, raws): + """Write hand-built RFC-822 documents, for shapes `EmailMessage` will not compose.""" + path.mkdir(parents=True, exist_ok=True) + for index, raw in enumerate(raws): + (path / f"{index:02d}.eml").write_bytes(raw.encode("utf-8")) + return path + + +def test_detect_format_reads_every_supported_shape(tmp_path): + assert detect_format(_write_mbox(tmp_path / "archive.mbox", [_msg("a@example.org")])) == "mbox" + assert detect_format(_write_maildir(tmp_path / "Maildir", [_msg("a@example.org")])) == "maildir" + assert detect_format(_write_eml_dir(tmp_path / "emls", [_msg("a@example.org")])) == "eml-dir" + + +def test_detect_format_rejects_a_missing_path(tmp_path): + with pytest.raises(FileNotFoundError): + detect_format(tmp_path / "nope") + + +@pytest.mark.parametrize("writer", [_write_mbox, _write_maildir, _write_eml_dir]) +def test_every_shape_threads_the_same_way(tmp_path, writer): + archive = writer( + tmp_path / "archive", + [ + _msg("root@example.org", days_ago=5), + _msg("reply@example.org", days_ago=4, in_reply_to=""), + _msg("other@example.org", days_ago=3, subject="Different report"), + ], + ) + + threads = group_threads(iter_messages(archive)) + + assert sorted(threads) == ["", ""] + assert len(threads[""]) == 2 + + +def test_thread_ids_use_the_bracketed_form_the_imap_backend_emits(tmp_path): + r"""One `thread_id_kind` must mean one spelling, or the two backends disagree. + + `imap_source` keeps the brackets (`<[^<>\s]+>`), so a bare `root@host` here + would give the same thread two IDs and a switch between backends would + propose a duplicate tracker. + """ + archive = _write_mbox(tmp_path / "archive.mbox", [_msg("root@example.org")]) + + threads = list_recent_threads(archive, since_days=30) + + assert threads[0]["thread_id"] == "" + assert threads[0]["thread_id_kind"] == THREAD_ID_KIND + assert read_thread(archive, "root@example.org")["thread_id"] == "" + + +def test_references_chain_puts_the_reply_under_the_archive_root(tmp_path): + """A reply that only carries References still lands in its thread.""" + archive = _write_mbox( + tmp_path / "archive.mbox", + [ + _msg("root@example.org", days_ago=9), + _msg("mid@example.org", days_ago=8, in_reply_to=""), + _msg("leaf@example.org", days_ago=7, references=["root@example.org", "mid@example.org"]), + ], + ) + + threads = group_threads(iter_messages(archive)) + + assert list(threads) == [""] + assert len(threads[""]) == 3 + + +def test_replies_to_a_root_missing_from_the_archive_stay_in_one_thread(tmp_path): + """The archive is truncated at the start of the thread; the replies still group. + + Keying each reply on its own Message-ID split them into `r1@h` and `r2@h`, + and IMAP keys on the first References ID whether or not that message is + present, so the two backends also disagreed on the ID. + """ + archive = _write_mbox( + tmp_path / "archive.mbox", + [ + _msg("r1@example.org", days_ago=2, in_reply_to=""), + _msg("r2@example.org", days_ago=1, references=["absent@example.org", "r1@example.org"]), + ], + ) + + threads = group_threads(iter_messages(archive)) + + assert list(threads) == [""] + assert len(threads[""]) == 2 + + +def test_a_comment_beside_the_referenced_id_is_not_an_ancestor(tmp_path): + """`In-Reply-To: (was: report) ` must key on ``, not on `<(was:`. + + RFC 5322 allows a comment in a structured header. Splitting the value on + whitespace turns that comment into a Message-ID that never existed, and the + reply lands in a thread of its own — the same split the References rule + exists to prevent. + """ + archive = _write_mbox( + tmp_path / "archive.mbox", + [ + _msg("root@example.org", days_ago=2), + _msg("reply@example.org", days_ago=1, in_reply_to="(was: report) "), + ], + ) + + threads = group_threads(iter_messages(archive)) + + assert list(threads) == [""] + assert len(threads[""]) == 2 + + +def test_list_recent_threads_keeps_a_thread_whole_when_its_reply_is_recent(tmp_path): + """The old opening report stays visible while the thread is active.""" + archive = _write_mbox( + tmp_path / "archive.mbox", + [ + _msg("root@example.org", days_ago=200), + _msg("reply@example.org", days_ago=1, in_reply_to=""), + _msg("stale@example.org", days_ago=200, subject="Old and closed"), + ], + ) + + threads = list_recent_threads(archive, since_days=30) + + assert [t["thread_id"] for t in threads] == [""] + assert threads[0]["message_count"] == 2 + assert threads[0]["thread_id_kind"] == THREAD_ID_KIND + + +def test_list_recent_threads_sorts_newest_first(tmp_path): + archive = _write_mbox( + tmp_path / "archive.mbox", + [ + _msg("older@example.org", days_ago=5), + _msg("newer@example.org", days_ago=1), + ], + ) + + threads = list_recent_threads(archive, since_days=30) + + assert [t["thread_id"] for t in threads] == ["", ""] + + +def test_newest_first_survives_messages_in_different_time_zones(tmp_path): + """`13:00+05:00` is 08:00Z, so it is older than `12:00+00:00` despite sorting later as text.""" + moment = dt.datetime.now(dt.UTC) + east = (moment - dt.timedelta(hours=4)).astimezone(dt.timezone(dt.timedelta(hours=5))) + archive = _write_mbox( + tmp_path / "archive.mbox", + [ + _msg("east@example.org", when=east), + _msg("utc@example.org", when=moment), + ], + ) + + threads = list_recent_threads(archive, since_days=30) + + assert [t["thread_id"] for t in threads] == ["", ""] + assert threads[1]["last_date"].endswith("+05:00") + + +def test_duplicate_message_ids_are_counted_once(tmp_path): + """The same report exported twice must not double `message_count` or the body.""" + archive = _write_mbox( + tmp_path / "archive.mbox", + [ + _msg("root@example.org", days_ago=2, body="original\n"), + _msg("root@example.org", days_ago=1, body="duplicate copy\n"), + ], + ) + + threads = list_recent_threads(archive, since_days=30) + + assert threads[0]["message_count"] == 1 + assert [m["body"] for m in read_thread(archive, "")["messages"]] == ["original\n"] + + +def test_read_thread_returns_oldest_first_with_bodies(tmp_path): + archive = _write_mbox( + tmp_path / "archive.mbox", + [ + _msg("reply@example.org", days_ago=1, in_reply_to="", body="second\n"), + _msg("root@example.org", days_ago=2, body="first\n"), + ], + ) + + thread = read_thread(archive, "") + + assert [m["body"] for m in thread["messages"]] == ["first\n", "second\n"] + assert [m["message_id"] for m in thread["messages"]] == ["", ""] + + +def test_the_thread_subject_comes_from_the_root_not_the_first_file(tmp_path): + """Maildir keys come back in listing order, so `members[0]` can be the `Re:` reply.""" + archive = _write_maildir( + tmp_path / "Maildir", + [ + _msg( + "reply@example.org", days_ago=1, in_reply_to="", subject="Re: Heap overflow" + ), + _msg("root@example.org", days_ago=2, subject="Heap overflow"), + ], + ) + + threads = list_recent_threads(archive, since_days=30) + + assert threads[0]["subject"] == "Heap overflow" + assert read_thread(archive, "")["subject"] == "Heap overflow" + + +def test_read_thread_raises_for_an_unknown_id(tmp_path): + archive = _write_mbox(tmp_path / "archive.mbox", [_msg("root@example.org")]) + + with pytest.raises(LookupError): + read_thread(archive, "absent@example.org") + + +def test_encoded_headers_are_decoded(tmp_path): + """`compat32` leaves `=?utf-8?q?...?=` as-is, which breaks `[SECURITY]` matching.""" + archive = _write_mbox( + tmp_path / "archive.mbox", + [ + _msg( + "root@example.org", + subject="[SECURITY] Café parser overflow", + sender="Jöhn Döe ", + ) + ], + ) + + thread = read_thread(archive, "") + + assert thread["subject"] == "[SECURITY] Café parser overflow" + assert thread["messages"][0]["from"] == "Jöhn Döe " + assert thread["messages"][0]["subject"] == "[SECURITY] Café parser overflow" + + +def test_an_html_only_message_still_yields_its_body(tmp_path): + """An empty body is the worst outcome for a security report.""" + archive = _write_raw_eml( + tmp_path / "emls", + [ + "Message-ID: \r\n" + "Subject: HTML only\r\n" + "From: reporter@example.org\r\n" + "Date: Sat, 25 Oct 2026 10:00:00 +0000\r\n" + "MIME-Version: 1.0\r\n" + 'Content-Type: multipart/alternative; boundary="B"\r\n' + "\r\n" + "--B\r\n" + 'Content-Type: text/html; charset="utf-8"\r\n' + "\r\n" + "

Heap overflow in parser

\r\n" + "--B--\r\n", + ], + ) + + body = read_thread(archive, "")["messages"][0]["body"] + + assert "Heap overflow in parser" in body + assert "<" not in body + + +def test_script_and_style_content_never_reaches_the_body(tmp_path): + """The element is dropped whole, however its end tag is spelled. + + HTML permits ``. A filter that only accepts `` + matches nothing, leaves the element's code in the report body, and — worse + for a security report — puts attacker-supplied script text in front of the + maintainer as if it were the report (CodeQL py/bad-tag-filter). + """ + archive = _write_raw_eml( + tmp_path / "emls", + [ + "Message-ID: \r\n" + "Subject: HTML only\r\n" + "Date: Sat, 25 Oct 2026 10:00:00 +0000\r\n" + "Content-Type: text/html\r\n" + "\r\n" + "`, and a -# pattern that only accepts `` would leave the element's code in the -# body (CodeQL py/bad-tag-filter). -_HTML_DROP = re.compile( - r"<(?:script|style)\b[^>]*>.*?]*>", - re.IGNORECASE | re.DOTALL, -) +# The body is attacker-controlled, so every tag pattern stops at the next `<` +# (`[^<>]`) and an element's end tag is searched for once, forward: a pattern +# that can rescan to the end of the input from each `<` turns a few megabytes of +# `<` or of unclosed ``, and a pattern that only accepts `` would leave +# the element's code in the body (CodeQL py/bad-tag-filter). +_HTML_DROP_CLOSE = {name: re.compile(rf"]*>", re.IGNORECASE) for name in ("script", "style")} _HTML_BLOCK_BREAK = re.compile( - r"]*>", + r"]*>", re.IGNORECASE, ) -_HTML_TAG = re.compile(r"<[^>]+>") +_HTML_TAG = re.compile(r"<[^<>]+>") def _normalise_id(raw: str) -> str: @@ -215,9 +218,28 @@ def _decode_payload(part: Message) -> str: return payload.decode("utf-8", "replace") +def _drop_script_style(markup: str) -> str: + """``markup`` with every ``script`` / ``style`` element and its content removed. + + An element left unclosed runs to the end of the document, as a browser + reads it, so its code never reaches the body. + """ + kept: list[str] = [] + pos = 0 + while (opener := _HTML_DROP_OPEN.search(markup, pos)) is not None: + kept.append(markup[pos : opener.start()]) + kept.append(" ") + closer = _HTML_DROP_CLOSE[opener.group(1).lower()].search(markup, opener.end()) + if closer is None: + return "".join(kept) + pos = closer.end() + kept.append(markup[pos:]) + return "".join(kept) + + def _html_to_text(markup: str) -> str: """A readable approximation of an HTML body: block breaks kept, tags dropped.""" - text = _HTML_DROP.sub(" ", markup) + text = _drop_script_style(markup) text = _HTML_BLOCK_BREAK.sub("\n", text) text = _HTML_TAG.sub("", text) text = html.unescape(text) @@ -276,30 +298,44 @@ def detect_format(path: str | Path) -> str: def iter_messages(path: str | Path) -> list[tuple[str, Message]]: """Every message in the archive as ``(key, message)``. - The order is whatever the store yields — for a Maildir that is directory - listing order, which is neither chronological nor stable across copies — so - nothing downstream relies on it and ordering comes from ``Date`` instead. - ``key`` is a diagnostic handle only (mailbox key, or the file name for an - ``.eml`` directory), never a thread identifier. + The order is the store's — for a Maildir, file-name order within ``cur/`` + and then ``new/``, which is neither chronological nor stable across copies — + so nothing downstream relies on it and ordering comes from ``Date`` instead. + ``key`` is a diagnostic handle only (the mbox key, ``cur/`` or + ``new/`` for a Maildir, the file name for an ``.eml`` directory), never + a thread identifier. """ root = Path(path).expanduser() shape = detect_format(root) if shape == "mbox": - box: mailbox.Mailbox = mailbox.mbox(str(root), create=False) - elif shape == "maildir": - box = mailbox.Maildir(str(root), create=False) + box = mailbox.mbox(str(root), create=False) + try: + return [(key, msg) for key, msg in box.iteritems()] + finally: + box.close() + if shape == "maildir": + # Maildir message files are listed by hand rather than through + # `mailbox.Maildir`, which opens whatever a name in `cur/` or `new/` + # points at. + files = [ + (f"{sub}/{item.name}", item) + for sub in ("cur", "new") + if (root / sub).is_dir() and not (root / sub).is_symlink() + for item in sorted((root / sub).iterdir()) + if not item.name.startswith(".") + ] else: # A standalone `.eml` carries no mbox `From ` envelope, so parse the file # as the message it is instead of dressing it up as an mbox message. - return [ - (item.name, email.message_from_bytes(item.read_bytes())) - for item in sorted(root.iterdir()) - if item.is_file() and item.suffix.lower() == ".eml" - ] - try: - return [(key, msg) for key, msg in box.iteritems()] - finally: - box.close() + files = [(item.name, item) for item in sorted(root.iterdir()) if item.suffix.lower() == ".eml"] + # The archive is a snapshot from elsewhere, so a symlink in it is not a + # message: followed, `x.eml -> ~/.ssh/id_ed25519` would put a local file's + # bytes into the agent's context and from there into a tracker. + return [ + (key, email.message_from_bytes(item.read_bytes())) + for key, item in files + if item.is_file() and not item.is_symlink() + ] def group_threads(messages: list[tuple[str, Message]]) -> dict[str, list[tuple[str, Message]]]: @@ -313,15 +349,19 @@ def group_threads(messages: list[tuple[str, Message]]) -> dict[str, list[tuple[s a truncated archive. """ roots: dict[str, list[tuple[str, Message]]] = {} - seen: set[str] = set() + seen: set[tuple[str, str]] = set() for key, msg in messages: own_id = _message_id(msg) or _content_id(msg) - if own_id in seen: - # A second copy of a message already threaded — a report Cc'd to two - # lists and exported twice — must not inflate `message_count` or - # repeat the body. + # A second copy of a message already threaded — a report Cc'd to two + # lists and exported twice — must not inflate `message_count` or repeat + # the body. The copies differ in their delivery headers, so they are + # matched on the body; a different message reusing the Message-ID (any + # sender can pick one) is kept, never silently dropped in favour of + # whichever copy the directory listing yielded first. + copy = (own_id, hashlib.sha256(_body_text(msg).encode()).hexdigest()) + if copy in seen: continue - seen.add(own_id) + seen.add(copy) ancestors = _reference_ids(msg) root = ancestors[0] if ancestors else own_id roots.setdefault(root, []).append((key, msg)) @@ -331,7 +371,7 @@ def group_threads(messages: list[tuple[str, Message]]) -> dict[str, list[tuple[s def _thread_subject(thread_id: str, members: list[tuple[str, Message]]) -> str: """The thread's subject, taken from its root message whenever the archive has it. - A Maildir yields its keys in directory-listing order, so ``members[0]`` can + A Maildir yields its messages in file-name order, so ``members[0]`` can easily be a ``Re:`` reply; the subject a thread should be listed under comes from the root, or failing that from its earliest dated message. """ diff --git a/tools/maildir/tests/test_read_archive.py b/tools/maildir/tests/test_read_archive.py index 3de05b5d8..65a1415fd 100644 --- a/tools/maildir/tests/test_read_archive.py +++ b/tools/maildir/tests/test_read_archive.py @@ -20,6 +20,7 @@ import email.utils import json import mailbox +import time from email.message import EmailMessage import pytest @@ -255,18 +256,58 @@ def test_newest_first_survives_messages_in_different_time_zones(tmp_path): def test_duplicate_message_ids_are_counted_once(tmp_path): """The same report exported twice must not double `message_count` or the body.""" + first = _msg("root@example.org", days_ago=2, body="original\n") + second = _msg("root@example.org", days_ago=1, body="original\n") + second["List-Id"] = "" + archive = _write_mbox(tmp_path / "archive.mbox", [first, second]) + + threads = list_recent_threads(archive, since_days=30) + + assert threads[0]["message_count"] == 1 + assert [m["body"] for m in read_thread(archive, "")["messages"]] == ["original\n"] + + +def test_a_reused_message_id_does_not_hide_a_different_message(tmp_path): + """Any sender can pick a Message-ID; reusing a report's must not drop the report.""" archive = _write_mbox( tmp_path / "archive.mbox", [ - _msg("root@example.org", days_ago=2, body="original\n"), - _msg("root@example.org", days_ago=1, body="duplicate copy\n"), + _msg("root@example.org", days_ago=2, body="decoy\n"), + _msg("root@example.org", days_ago=1, body="the real report\n"), ], ) - threads = list_recent_threads(archive, since_days=30) + bodies = [m["body"] for m in read_thread(archive, "")["messages"]] + + assert bodies == ["decoy\n", "the real report\n"] - assert threads[0]["message_count"] == 1 - assert [m["body"] for m in read_thread(archive, "")["messages"]] == ["original\n"] + +def test_symlinked_messages_are_not_read(tmp_path): + """A symlink in a snapshot from elsewhere must not pull a local file into the body.""" + secret = tmp_path / "secret.eml" + secret.write_bytes(_msg("secret@example.org", body="local secret\n").as_bytes()) + + emls = tmp_path / "emls" + emls.mkdir() + (emls / "report.eml").write_bytes(_msg("report@example.org").as_bytes()) + (emls / "planted.eml").symlink_to(secret) + + maildir = tmp_path / "maildir" + for sub in ("cur", "tmp"): + (maildir / sub).mkdir(parents=True) + (maildir / "cur" / "1700000000.M1P1.host:2,S").write_bytes(_msg("report@example.org").as_bytes()) + (maildir / "cur" / "1700000001.M1P1.host:2,S").symlink_to(secret) + linked = tmp_path / "linked-new" + linked.mkdir() + (linked / "1700000002.M1P1.host").write_bytes(secret.read_bytes()) + (maildir / "new").symlink_to(linked, target_is_directory=True) + + for archive in (emls, maildir): + ids = [t["thread_id"] for t in list_recent_threads(archive, since_days=30)] + + assert ids == [""], archive + with pytest.raises(LookupError): + read_thread(archive, "") def test_read_thread_returns_oldest_first_with_bodies(tmp_path): @@ -383,6 +424,46 @@ def test_script_and_style_content_never_reaches_the_body(tmp_path): assert "color" not in body +def test_an_unclosed_script_runs_to_the_end_of_the_document(tmp_path): + archive = _write_raw_eml( + tmp_path / "emls", + [ + "Message-ID: \r\n" + "Subject: HTML only\r\n" + "Date: Sat, 25 Oct 2026 10:00:00 +0000\r\n" + "Content-Type: text/html\r\n" + "\r\n" + "

Heap overflow