[high] fix(email): decode RFC 2047 headers before searching or reporting them - #122
Draft
elhoim wants to merge 1 commit into
Draft
[high] fix(email): decode RFC 2047 headers before searching or reporting them#122elhoim wants to merge 1 commit into
elhoim wants to merge 1 commit into
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BLUF
Subjectis 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.From, so a sender display name was shown and searched encoded.SubjectandFromwere read straight off the message withmsg.get(...)and never decoded.email.header.decode_header/make_header, falling back to the raw value if the encoded word is malformed.src/mulder/server/tools/email.pyonly. No new abstraction, no change to any other tool.The bug
A header carrying anything outside ASCII is transmitted encoded:
Read raw, that string is what lands in the result, in the report, and in what
_matches_searchsearches.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
applied to
SubjectandFrom.The fallback returns the raw header rather than dropping it, so a malformed encoded word costs fidelity but never evidence.
HeaderParseErroris in that tuple because of a test rather than a guess: a malformed base64 payload raises it out ofdecode_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_recipientssplitting on a comma inside a quoted display name, HTML-only messages having no searchable body,Ccnever being searched, and attachments detected only viaContent-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 withgit 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 intomaintoday, and none conflicts with open #96.Three of them — this one,
fix/pst-recipient-parsingandfix/pst-header-decoding— each add afrom 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.email.pyrestored toorigin/mainand the new tests kept, 3 of 6 fail: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_exitare not reintroduced — this is a self-contained fix to header decoding, per the review on #32:Branched fresh from current
main(2e5432c); the closed branch was not revised in place.🤖 Generated with Claude Code