Skip to content

[high] fix(email): decode RFC 2047 headers before searching or reporting them - #122

Draft
elhoim wants to merge 1 commit into
calebevans:mainfrom
elhoim:fix/pst-header-decoding
Draft

[high] fix(email): decode RFC 2047 headers before searching or reporting them#122
elhoim wants to merge 1 commit into
calebevans:mainfrom
elhoim:fix/pst-header-decoding

Conversation

@elhoim

@elhoim elhoim commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

BLUF

  • Priority: high.
  • A message whose Subject is RFC 2047 encoded — anything non-ASCII, and anything a sender simply chooses to encode — was reported to the examiner as raw base64, and a keyword search for the words it contains could never match it.
  • The same applied to From, so a sender display name was shown and searched encoded.
  • Root cause: Subject and From were read straight off the message with msg.get(...) and never decoded.
  • Fix: decode with email.header.decode_header / make_header, falling back to the raw value if the encoded word is malformed.
  • Scope: src/mulder/server/tools/email.py only. No new abstraction, no change to any other tool.

The bug

        "subject": msg.get("Subject", "(no subject)"),
        "sender": msg.get("From", ""),

A header carrying anything outside ASCII is transmitted encoded:

Subject: =?utf-8?B?V2lyZSB0cmFuc2ZlciByZXF1ZXN0?=

Read raw, that string is what lands in the result, in the report, and in what _matches_search searches. search_term="wire transfer" cannot match it. The words are present in the message and absent from everything the tool can see.

This is not confined to non-ASCII senders. Encoding is the sender's choice, and =?utf-8?B?…?= is valid for a pure-ASCII subject too — so it is a one-line way to keep a subject out of an examiner's keyword search.

The fix

def _decode_header_value(raw: str | None) -> str:
    if not raw:
        return ""
    try:
        return str(make_header(decode_header(raw)))
    except (HeaderParseError, UnicodeDecodeError, LookupError, ValueError):
        return raw

applied to Subject and From.

The fallback returns the raw header rather than dropping it, so a malformed encoded word costs fidelity but never evidence. HeaderParseError is in that tuple because of a test rather than a guess: a malformed base64 payload raises it out of decode_header, and without it the first such header would crash the whole PST parse. That test failed on the first draft of this fix and is included.

An ASCII subject passes through unchanged, and a missing subject still reads (no subject) — both pinned by narrowness tests.

Deliberately out of scope

fix/email-parsing (closed #78) bundled this with five other independent defects in the same file — the lexicographic date comparison, _parse_recipients splitting on a comma inside a quoted display name, HTML-only messages having no searchable body, Cc 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.

Attachment filenames and recipient display names are deliberately not decoded here. Both live in code owned by two of those sibling PRs, and decoding them from this branch would create a conflict for no benefit. Each is a one-line follow-up once the set has landed; noting it explicitly so it is not mistaken for an oversight.

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

A note on merge order

This PR is one of six recovered from closed #78, each branched independently off main. All six merge cleanly into main today, and none conflicts with open #96.

Three of them — this one, fix/pst-recipient-parsing and fix/pst-header-decoding — each add a from email... import at the same point in the import block, so whichever merges second will report a conflict there. It is confined to the import lines; no two of the six change the same statement. Resolution is to keep both imports.

Flagging it here rather than leaving it to be discovered at merge time.

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, 3 of 6 fail:
FAILED test_an_encoded_subject_is_decoded    - AssertionError: assert '=?utf-8?B?V2...iByZXF1ZXN0?=' == 'Wire transfer req...
FAILED test_an_encoded_sender_name_is_decoded - AssertionError: assert 'Ünter Müller' in '=?utf-8?B?w5xudGVyIE3DvGxsZXI=?= ...
FAILED test_a_keyword_search_finds_an_encoded_subject - assert 0 == 1
3 failed, 3 passed

The third line is the forensic consequence as an assertion: a message whose subject is "Wire transfer request", invisible to a search for exactly that phrase.

The three that pass on both trees are the narrowness tests — plain subject unchanged, missing subject still (no subject), malformed header still yields something — 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 header decoding, 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

Subject and From were read straight off the message. A non-ASCII header
arrives encoded -- =?utf-8?B?V2lyZSB0cmFuc2ZlciByZXF1ZXN0?= -- so the
report handed the examiner base64 and a keyword search for the words the
subject actually contains could never match it.

Any sender may choose that encoding even for a pure-ASCII subject, so it
also works as a trivial way to hide a subject line from keyword search.

Decode with email.header.decode_header/make_header, falling back to the raw
value when the encoded word is malformed so a broken header loses nothing.
The fallback catches HeaderParseError as well as the decoding errors: a bad
base64 payload raises it out of decode_header, which a test pins.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant