gh-74453: Add stronger security warning to os.path.commonprefix #144401
Conversation
sethmlarson
added
type-security
Doc/library/os.path.rst
Outdated
| (``''``). | ||
|
|
||
| .. note:: | ||
| .. danger:: |
There was a problem hiding this comment.
We do not often use "danger" but rather prefer using warning (I believe we have something about it in the devguide)
There was a problem hiding this comment.
Thanks! I've updated to warning instead of danger.
There was a problem hiding this comment.
I believe we have something about it in the devguide
Out of curiosity, which section? I was unable to find any on these.
There was a problem hiding this comment.
There was a problem hiding this comment.
Yes here. It is just so that we do not have a proliferation of different boxes (while Sphinx and docutils provide lots of boxes we tend to only use a few of them). In addition other security warnings were usually indicated through a warning.
Though if we actually use danger/important instead of warning, feel free to revert my suggestion (from what I remember we mostly used warning)
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
LGTM.
IMO we should still consider deprecating os.path.commonprefix() and add string.commonprefix() instead. But this doc change is a good start :-)
|
@vstinner Thanks! I'm working on a PR that deprecates the function and moves it to |
encukou
added
awaiting merge
needs backport to 3.10
encukou
added
needs backport to 3.12
|
Thanks @sethmlarson for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10. |
|
Thanks @sethmlarson for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12. |
|
Thanks @sethmlarson for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11. |
|
Thanks @sethmlarson for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
|
Thanks @sethmlarson for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
…pythonGH-144401) (cherry picked from commit 4e15b8d) Co-authored-by: Seth Michael Larson <seth@python.org>
|
GH-144426 is a backport of this pull request to the 3.10 branch. |
|
GH-144427 is a backport of this pull request to the 3.12 branch. |
…pythonGH-144401) (cherry picked from commit 4e15b8d) Co-authored-by: Seth Michael Larson <seth@python.org>
|
GH-144428 is a backport of this pull request to the 3.11 branch. |
|
GH-144429 is a backport of this pull request to the 3.13 branch. |
|
GH-144430 is a backport of this pull request to the 3.14 branch. |
…pythonGH-144401) (cherry picked from commit 4e15b8d) Co-authored-by: Seth Michael Larson <seth@python.org>
|
I've created the follow-up PR which deprecates |
5 participants
The first part of closing #74453, this documentation update I believe is less controversial than a deprecation. I'm recommending backporting this warning, as all Python versions supported today have
commonpath().The mix-up that
commonprefixis acceptable for generating a path prefix (versus a string prefix) occurred at least once in a critical packaging tool: https://www.cve.org/CVERecord?id=CVE-2026-1703 Given its usage (40K+ hits on GitHub) I suspect this is not the only occurrence.📚 Documentation preview 📚: https://cpython-previews--144401.org.readthedocs.build/