[high] fix(email): detect an attachment by its filename, not its disposition

[elhoim]

BLUF

• Priority: high.
• An emailed .exe delivered as Content-Disposition: inline — or with no disposition header at all — was reported as a message with no attachments, and has_suspicious_attachment stayed False.
• That check is the security purpose of _parse_email_message. It was blind to exactly the delivery shapes an attacker can choose freely.
• Root cause: a part was collected only when it declared Content-Disposition: attachment.
• Fix: treat any leaf part carrying a filename as an attachment, in addition to any part that declares the attachment disposition.
• Scope: src/mulder/server/tools/email.py only. No new abstraction, no change to any other tool.

The bug

    for part in msg.walk():
        if part.get_content_disposition() == "attachment":
            filename = part.get_filename() or "unnamed"
            attachments.append(filename)
            ext = Path(filename).suffix.lower()
            if ext in _SUSPICIOUS_EXTENSIONS:
                has_suspicious = True

Content-Disposition is a hint from the sender, not a property of the data. A part like

Content-Type: application/octet-stream; name="invoice.exe"
Content-Disposition: inline

carries an executable and is skipped entirely. So is a part with no disposition header at all, which several older senders emit. In both cases attachments comes back [] and has_suspicious_attachment comes back False — the message is reported as clean.

inline is not an exotic choice: it is what mail clients emit for anything they might render, so it does not even look anomalous in transit.

The fix

    for part in msg.walk():
        if part.is_multipart():
            continue
        filename = part.get_filename()
        disposition = part.get_content_disposition()
        # An attachment is anything carrying a filename, not only what
        # declares `Content-Disposition: attachment`. Malicious payloads
        # arrive as `inline`, or with no disposition header at all, and both
        # were previously invisible -- including to the suspicious-extension
        # check, which is the point of this function.
        if disposition == "attachment" or filename:
            name = filename or "unnamed"
            attachments.append(name)
            if Path(name).suffix.lower() in _SUSPICIOUS_EXTENSIONS:
                has_suspicious = True

The is_multipart() skip matters: without it a multipart/alternative container could be counted. Body parts carry no filename, so a plain or HTML body is never mistaken for an attachment — pinned by a test.

Widening detection does not widen suspicion: an inline logo.png is now correctly listed as an attachment and correctly left non-suspicious. That is also pinned by a test, so the fix cannot be mistaken for making every message look dangerous.

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, HTML-only messages having no searchable body, and Cc never being searched. Each is a separate root cause and is being submitted as its own PR.

Attachment filenames are left undecoded here: a name sent as =?utf-8?B?…?= will still be listed encoded. That is the RFC 2047 concern and belongs with the header-decoding fix, not this one.

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.

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_inline_payload_is_detected                 - AssertionError: assert [] == ['invoice.exe']
FAILED test_an_inline_payload_trips_the_suspicious_check  - assert False is True
FAILED test_a_payload_with_no_disposition_header_is_detected - AssertionError: assert [] == ['invoice.exe']
FAILED test_a_benign_inline_image_is_listed_but_not_suspicious - AssertionError: assert [] == ['logo.png']
4 failed, 3 passed

The second line is the security consequence in one assertion: a real .exe in the message, and the suspicious-attachment flag reporting False.

The three that pass on both trees are the narrowness tests — a declared attachment still works, a body-only message gains no phantom attachment, and multipart/alternative body parts are not counted — 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 attachment detection, 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