[high] fix(email): give an HTML-only message a searchable body

[elhoim]

BLUF

• Priority: high.
• A message sent as HTML only — which is most phishing — had body_text: None. A search_term naming a phrase in its body could never match it, and none of its content reached the case DB.
• Two further defects in the same helper: a text/plain attachment was returned as the message body, and an unrecognised charset raised LookupError and lost the body entirely.
• Root cause: body_text came from _get_body(msg, "text/plain") alone, and _get_body walked every MIME part including attachments while decoding without a guard.
• Fix: walk only leaf parts, skip attachments, fall back to UTF-8 on an unknown charset, and fall back to text/html with markup stripped when there is no plain part.
• Scope: src/mulder/server/tools/email.py only. No new abstraction, no change to any other tool.

The bug

        "body_text": _get_body(msg, "text/plain"),

and

    if msg.is_multipart():
        for part in msg.walk():
            if part.get_content_type() == content_type:
                payload = part.get_payload(decode=True)
                if isinstance(payload, bytes):
                    charset = part.get_content_charset() or "utf-8"
                    return payload.decode(charset, errors="replace")

Three separate consequences:

1. HTML-only message → no body. body_text is None, so _matches_search receives None, the body is never searched, and the message is dropped from any keyword search. Its content is also absent from what gets indexed.
2. A text/plain attachment is returned as the body. The walk does not check Content-Disposition, so for a multipart/mixed message whose body is HTML and whose attachment is notes.txt, the attachment text becomes body_text.
3. An unknown charset loses the body. payload.decode(charset, …) raises LookupError — not caught anywhere — for a charset the platform does not know. errors="replace" does not help; the exception is raised before decoding starts.

The fix

    for part in msg.walk():
        if part.is_multipart():
            continue
        # A text/plain *attachment* is evidence, but it is not the body.
        if part.get_content_disposition() == "attachment":
            continue
        if part.get_content_type() != content_type:
            continue
        payload = part.get_payload(decode=True)
        if isinstance(payload, bytes):
            charset = part.get_content_charset() or "utf-8"
            try:
                return payload.decode(charset, errors="replace")
            except LookupError:
                # A charset the platform does not know must not lose the body.
                return payload.decode("utf-8", errors="replace")
    return None

plus _get_searchable_body, which prefers text/plain and falls back to text/html with tags, <script> and <style> blocks removed and the common entities decoded. Script and style content is stripped rather than indexed — neither is evidence, and indexing it would pollute keyword search.

A message that has a text/plain part is completely unaffected; that is pinned by a test.

Deliberately out of scope

fix/email-parsing (closed #78) bundled this with five other independent defects in the same file — the lexicographic date comparison, RFC 2047 header decoding, _parse_recipients splitting on a comma inside a quoted display name, Cc/Bcc never being searched, and attachments detected only via Content-Disposition: attachment. Each is a separate root cause and is being submitted as its own PR.

HTML-to-text here is deliberately a tag strip, not a full renderer: it exists so the words are searchable, not to reproduce layout. Pulling in an HTML parsing dependency for that would be a much larger change than the defect warrants.

Relationship to open PR #96 (fix/readpst-exit-code): that one guards the case where readpst failed. This one is the success path, where messages were extracted and then parsed lossily. Different functions, no overlap; verified conflict-free with git merge-tree.

Verification

• uvx pre-commit run --all-files (new test staged first, so the hooks actually see it) → ruff, ruff-format, mypy all pass.
• Full suite → 861 passed, nothing deselected, nothing skipped.
• Discriminating check — with email.py restored to origin/main and the new tests kept, 4 of 7 fail:

FAILED test_an_html_only_message_has_a_body           - AssertionError: an HTML-only message was left with no body at all
FAILED test_a_keyword_search_finds_an_html_only_message - assert 0 == 1
FAILED test_a_text_attachment_is_not_mistaken_for_the_body - AssertionError: assert 'real body' in 'attached notes'
FAILED test_an_unknown_charset_does_not_lose_the_body  - LookupError: unknown encoding: x-not-a-real-charset
4 failed, 3 passed

The second line is the forensic consequence stated as an assertion: a phishing message containing "wire transfer" in its body, invisible to a search for exactly that phrase. The third and fourth lines are defects 2 and 3, each reproduced directly rather than argued.

The three that pass on both trees are the narrowness tests — plain-text messages unchanged, and markup absent from the body — so they pin the fix rather than the bug.

Context

Recovered from closed PR #78, which bundled six independent defects behind one title. The rejected outcome framework and classify_tool_exit are not reintroduced — this is a self-contained fix to body extraction, per the review on #32:

focused fixes for specific tools that currently swallow failures or lose partial-result information would be welcome.

Branched fresh from current main (2e5432c); the closed branch was not revised in place.

🤖 Generated with Claude Code