Add security warning to mailbox docs

[StanFromIreland]

CC @bitdancer, you should have gotten a series of notifications just there. I suggest adding a security warning, they collectively are the motivation for this change.

[read-the-docs-community]

Documentation build overview

📚 cpython-previews | 🛠️ Build #33028261 | 📁 Comparing 3006e53 against main (3a1df78)

  🔍 Preview build  

10 files changed · ± 10 modified

± Modified

• c-api/perfmaps.html
• deprecations/index.html
• deprecations/pending-removal-in-3.20.html
• library/ast.html
• library/mailbox.html
• whatsnew/3.12.html
• whatsnew/3.13.html
• whatsnew/3.14.html
• whatsnew/3.15.html
• whatsnew/changelog.html

[bitdancer]

Well, you could probably add a variation on this warning to every module in the stdlib that operates on files, so at the moment I'm not convinced. I haven't read over the security issues, though, so we'll see. I don't think the message as is is all that useful, though. I certainly don't know what it is trying to tell me.

I suggest we hold on this for a bit for further discussion.

[gpshead]

The message is passively trying to say: We will not treat these problems in mailbox as security issues because it is nobody's priority to attempt to maintain mailbox for such purposes.

We could just say that instead.

Realistically: Why do we even have this module in the stdlib at all?

[StanFromIreland]

The message is passively trying to say: We will not treat these problems in mailbox as security issues because it is nobody's priority to attempt to maintain mailbox for such purposes.

We could just say that instead.

Exactly! Hmm, we imply the same for various other modules and APIs but generally use the fancy wording.

Realistically: Why do we even have this module in the stdlib at all?

I think the AA batteries we included have expanded to D batteries. ;-)

[gpshead]

fwiw mailbox is a leaf node module (nothing else in the stdlib depends on it AFAICT) so while I wouldn't actually go so far as to say something as snarky as my "isn't a priority" in the docs, wording like your proposal laying out security expectations is good to have.

for this specific module, given the reports we've been receiving on PSRT maybe we could be more specific and offer some advice to users. The security reports that we're not considering serious have been around the mailbox files/directory-trees being on attacker controlled file systems. Not about the contents of an individual email stored within. "externally modified data" in your phrasing is maybe a bit vague and could imply message contents are involved?

---

on the slippery slops docs warning topic: We have a general theme of these messages being spread out throughout our docs near the APIs they apply to. Messaging like this is intended to meet the user where they're at when looking up how to use something.

Our other security what is and isn't an issue docs like the devguide and security.md can also point to things in a more "do or do not report x/y/z" manner for specific packages.

[StanFromIreland]

fwiw mailbox is a leaf node module (nothing else in the stdlib depends on it AFAICT)

Oh indeed. It really should have been a PyPI module, if only it had existed at the time.

Not about the contents of an individual email stored within. "externally modified data" in your phrasing is maybe a bit vague and could imply message contents are involved?

Maybe:

... it does not protect against mailbox files or directory trees controlled or modified by untrusted users.

Or something like:

... it does not protect against untrusted paths or externally modified mailbox files or directory trees.

Messaging like this is intended to meet the user where they're at when looking up how to use something.

In this case, the warning applies to the entire module, so I think putting warnings under each API would be too duplicative.

[gpshead]

.. warning::

   The :mod:`!mailbox` module is designed for mailboxes under the sole
   control of the application and the local user.  It is **not** hardened
   for use in hostile environments.  As such, the following are not treated
   as security vulnerabilities:

   * Passing untrusted input to :mod:`!mailbox` APIs.  Values such as
     message keys and folder names are used as filesystem paths, so
     unsanitized input may read, write, or delete files outside the
     mailbox.  Sanitizing these inputs is the caller's responsibility;
     at a minimum, reject any value containing ``".."``, ``"/"``,
     ``"\"``, or ``":"``.

   * Using a mailbox whose files or directories reside on a filesystem
     that other users can modify.  Symlink and other races there can
     redirect mailbox operations to arbitrary files.

Maybe? though that also seems verbose. (that's from me going back and forth with Claude a few times given our desire to clarify the reports at hand and wanting to make the advice somewhat actionable)

We might accept PRs helping flag some such inputs via ValueError's in the future but the important thing is we're not claiming to be comprehensive.

[StanFromIreland]

It is certainly more detailed, although “As such, the following are not treated as security vulnerabilities:” feels too specific. I think the docs should explicitly state the general (lack of a) support boundary.

Also, I think we should avoid suggestions like "at a minimum, reject any value containing "..", "/", "\", or ":"." It is not a complete cross-platform path-safety rule and could become misleading.

[gpshead]

yeah, agreed. even listing the characters to avoid screams "so why isn't that a ValueError already?"... the reason likely being nobody proactively maintains the module. and avoiding characters instead of specifying what is allowed is backwards... MH is integer keyed message ids by definition for example.

[StanFromIreland]

Suggested change

	externally modified data.
	externally modified mailbox files or directory trees.

Seeing the complications adding suggestions introduces, WDYT about this? Based on usage on GitHub (which overall is tiny!), almost all users are using the module for small little local scripts, and won't care about such considerations anyhow.

[bedevere-app]

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

[StanFromIreland]

I'll defer to RDM, closing.