classification
Title: disable executing code in .pth files
Type: Stage:
Components: Installation Versions: Python 3.6
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: brett.cannon, christian.heimes, eric.snow, lemburg, minrk, ncoghlan, paul.moore, r.david.murray, tdsmith, vstinner
Priority: normal Keywords: patch

Created on 2015-06-29 19:30 by minrk, last changed 2015-07-02 16:18 by eric.snow. This issue is now closed.

Files
File name Uploaded Description Edit
0001-disable-executing-code-in-.pth-files.patch minrk, 2015-06-29 19:30 disable executing code in .pth files review
Messages (15)
msg245959 - (view) Author: Min RK (minrk) * Date: 2015-06-29 19:30
.pth files currently allow execution of arbitrary code, triggered by lines starting with `import`. This is a rarely understood, and often misbehaving feature. easy_install has used this feature to ensure that its packages are highest priority (even higher than stdlib). This is one of the unfortunate behaviors that pip undoes from easy_install, in part due to the problems it can cause. There is currently a proposal in setuptools to stop using this, even for easy_install.

The attached patch removes support for executing code in .pth files, throwing an ImportWarning if any such attempts at import are seen.

General question that might result in rejecting this patch:

Are there any good/valid use cases for .pth files being able to execute arbitrary code at interpreter start time?

If this is accepted, some implementation questions:

1. if the feature is removed in 3.6, should a DeprecationWarning be added to 3.5?
2. Is ImportWarning the right warning class (or should there even be a warning)?
msg245961 - (view) Author: Tim Smith (tdsmith) * Date: 2015-06-29 19:57
In Homebrew we occasionally use .pth files to call site.addsitedir. This is useful when we want to add a directory to sys.path that contains .pth files that also need to be processed (for example, when adding a directory to sys.path that contains namespace packages). It would be helpful to retain a mechanism for adding site dirs from a .pth file.

Homebrew also writes temporary probe .pths containing "import site; site.homebrew_was_here = True" in order to check whether .pth files are processed in a particular path, though we could equivalently write a temporary path to a .pth file and verify that it ends up in sys.path.

Finally, Homebrew asks users to write .pth files with imports in some places where we could use usercustomize instead.
msg245964 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-06-29 20:18
I'm guessing this feature has been around too long and is used in too many ways to remove.  I believe it is effectively disabled by -I (if it is not, that would be a valid feature request).  However, since setuptools was the pioneer in this area, if setuptools is no longer using it, perhaps a deprecation would be possible.  I think it would probably have to happen over more than one release, to insure enough time to implement any features needed to replace the deprecated functionality.
msg245965 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2015-06-29 20:27
On 29.06.2015 21:30, Min RK wrote:
> 
> .pth files currently allow execution of arbitrary code, triggered by lines starting with `import`. This is a rarely understood, and often misbehaving feature. easy_install has used this feature to ensure that its packages are highest priority (even higher than stdlib). This is one of the unfortunate behaviors that pip undoes from easy_install, in part due to the problems it can cause. There is currently a proposal in setuptools to stop using this, even for easy_install.
> 
> The attached patch removes support for executing code in .pth files, throwing an ImportWarning if any such attempts at import are seen.

Such a change will require a PEP, since it's an essential feature
that has been documented for a very long time:

https://docs.python.org/3.5/library/site.html

and is used by a lot of existing setuptools installations, which
would break if Python were to remove support for this.

The PEP would also need to address the reasons for removing the
feature, e.g. explain possible attack vectors, confusion caused
by this, etc.

You can then reference this patch in the PEP.

Thanks,
-- 
Marc-Andre Lemburg
eGenix.com
msg245967 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-06-29 20:41
> The attached patch removes support for executing code in .pth files

This change will basically break all Python applications. Don't do that.

If you believe that we can smoothly move to a world without .pth files, you should propose an overall plan, step by step. It implies to add an option to Python to disable .pth files. It might be enabled by the -I command line option.

https://docs.python.org/dev/using/cmdline.html#cmdoption-I
msg245983 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-06-30 08:20
As others have noted, we're not going to change this default in the standard CPython executable (due to the degree of disruption involved), and the -S and -I switches already effectively turn it off (by disabling site module processing entirely)

However, it could be reasonable to consider offering a way to disable *just* the execution of arbitrary code from .pth files, while otherwise allowing the site module to run.
msg245996 - (view) Author: Min RK (minrk) * Date: 2015-06-30 18:52
Thanks for the feedback, I thought it might be a long shot. I will go back to removing the *use* of the feature everywhere I can find it, since it is so problematic and rarely, if ever, desirable.

> it's an essential feature that has been documented for a very long time
> https://docs.python.org/3.5/library/site.html

The entirety of the documentation of this feature appears to be this sentence on that page:

> Lines starting with import (followed by space or tab) are executed.

No explanation or examples are given, nor any reasoning about the feature or why one might use it.

> This change will basically break all Python applications

This surprises me. Can you elaborate? I have not seen an application rely on executing code in .pth files.

> If you believe that we can smoothly move to a world without .pth files, 
> you should propose an overall plan, step by step.

I have no desire to remove .pth files. .pth files are a fine way to add locations to sys.path. It's .pth files *executing arbitrary code* that's the problem, very surprising, and a source of many errors (especially how it is used in setuptools).
msg245998 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2015-06-30 20:36
On 30.06.2015 20:52, Min RK wrote:
> 
> Thanks for the feedback, I thought it might be a long shot. I will go back to removing the *use* of the feature everywhere I can find it, since it is so problematic and rarely, if ever, desirable.

Could you please post an example of where the feature is
problematic ?

>> it's an essential feature that has been documented for a very long time
>> https://docs.python.org/3.5/library/site.html
> 
> The entirety of the documentation of this feature appears to be this sentence on that page:
>
>> Lines starting with import (followed by space or tab) are executed.

Yep, but it's enough to be in active use.
msg245999 - (view) Author: Min RK (minrk) * Date: 2015-06-30 20:49
> Could you please post an example of where the feature is problematic ?

setuptools/easy_install is the major one, which effectively does `sys.path[:0] = pth_contents`, breaking import priority. This has been known to result in adding `/usr/lib/pythonX.Y/dist-packages` to the front of sys.path, having higher priority that the stdlib or `--user` -installed packages (I helped a user deal with a completely broken installation that was a result of exactly this last week). The result can often be that `pip list` doesn't accurately describe the versions of packages that are imported. It also causes `pip install -e` to result in completely different import priority from `pip install`, which doesn't use easy-install.pth. Removing the code execution from `easy-install.pth` solves all of these problems.
msg246000 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2015-06-30 21:15
On 30.06.2015 22:49, Min RK wrote:
> 
>> Could you please post an example of where the feature is problematic ?
> 
> setuptools/easy_install is the major one, which effectively does `sys.path[:0] = pth_contents`, breaking import priority. This has been known to result in adding `/usr/lib/pythonX.Y/dist-packages` to the front of sys.path, having higher priority that the stdlib or `--user` -installed packages (I helped a user deal with a completely broken installation that was a result of exactly this last week). The result can often be that `pip list` doesn't accurately describe the versions of packages that are imported. It also causes `pip install -e` to result in completely different import priority from `pip install`, which doesn't use easy-install.pth. Removing the code execution from `easy-install.pth` solves all of these problems.

Ok, so you regard the way that setuptools uses the feature as
problematic. I'm sure some people will agree (including myself),
but this is not a good reason to remove the feature altogether.

Just because a feature can be misused doesn't make it a bad
feature. Otherwise, we'd have quite a few things we'd have to
remove from Python :-)

I'd suggest to try to fix the setuptools uses of the feature
instead - in a way that doesn't break all setuptools installations,
of course. Perhaps you could submit a fix for this to the
setuptools maintainers instead.

Thanks,
-- 
Marc-Andre Lemburg
eGenix.com
msg246003 - (view) Author: Min RK (minrk) * Date: 2015-06-30 22:16
> Just because a feature can be misused doesn't make it a bad feature.

That's fair. I'm just not aware of any uses of this feature that aren't misuses, hence the patch.

> Perhaps you could submit a fix for this to the setuptools maintainers instead.

Yes, that's definite the right thing to do, and in fact the first thing I did. It looks like that patch is likely to be merged; it is certainly much less disruptive. That's where I started, then I decided to bring it up to Python itself after reading up on the exploited feature, as it seemed to me like a feature with no use other than misuse.

Thanks for your time.
msg246005 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2015-06-30 22:30
On 01.07.2015 00:16, Min RK wrote:
> 
>> Just because a feature can be misused doesn't make it a bad feature.
> 
> That's fair. I'm just not aware of any uses of this feature that aren't misuses, hence the patch.

I don't remember the details of why this feature was added,
but can imagine that it was supposed to enable installation
of new importers via .pth files.

Without this feature it would not be possible to add entries to
sys.path via .pth files which can only be handled by non-standard
importers.

>> Perhaps you could submit a fix for this to the setuptools maintainers instead.
> 
> Yes, that's definite the right thing to do, and in fact the first thing I did. It looks like that patch is likely to be merged; it is certainly much less disruptive. That's where I started, then I decided to bring it up to Python itself after reading up on the exploited feature, as it seemed to me like a feature with no use other than misuse.

Thanks, that's certainly a good way forward :-)
msg246076 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2015-07-02 13:51
On 30 June 2015 at 23:30, M.-A. Lemburg <mal@egenix.com> wrote:
> I don't remember the details of why this feature was added,
> but can imagine that it was supposed to enable installation
> of new importers via .pth files.

I don't know for certain if this feature was already available when
importers were added, but there was certainly never any thought of
adding importers via .pth files when we designed them. We expected
importers to be added to sys.(meta_)path by explicit code run at
application startup, before the imports that needed it.

I'm glad to hear that setuptools is reconsidering its import priority
hacking. And I also have never seen any use of executable code in .pth
files that wasn't at best a dubious hack. But removing the feature (as
opposed to getting packages to stop misusing the feature) is too much
of a compatibility break.

It might be worth putting together a documentation patch that expands
what the docs say about the feature. Pointing out that it's easy to
misuse, and advising caution, might be a reasonable thing to do.
(Adding an example of a reasonable use of the feature would be even
better, but that would require thinking of a reasonable use!!! :-))
msg246082 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-07-02 14:59
Note that the idea of replacing .pth files came up a couple years ago:

https://mail.python.org/pipermail/import-sig/2013-July/000645.html

That proposal didn't go anywhere basically because there were more important things to work on. :)
msg246087 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-07-02 16:18
FYI, support for .pth has been around since at least Python 2.0.  However, support for imports in .pth files was added in 2.1:

changeset:   15815:868d2acf745808c9033f57cd5829d97a69ecf56e
branch:      legacy-trunk
user:        Martin v. Löwis <martin@v.loewis.de>
date:        Thu Jan 11 13:02:43 2001 +0000
summary:     Patch #103134: Support import lines in pth files.

From what I understand .pth file weren't meant to be used in the way they are.  Some folks are just really good at exploiting unintended behaviors based on an implementation. :)  We've been stuck with the "feature" ever since.

In this case the implementation happened to be a bit lax in how it evaluated each line.  It should have been strict about allowing only single import statements.  Instead it just made sure the line started with "import" and then exec'ed it.  A check for ";" would have been sufficient.

That said, I don't fault Martin (or whoever actually wrote it) at all. The implementation doesn't really bother me.  Sure, it could have been more careful, but honestly how could anyone be expected to anticipate the consequence here.

Ultimately, folks that looked closely enough at the source to figure out the hack would have had enough context to know the intent.  They should have opened a bug report rather that take advantage of the loophole.  If their need was sufficient they could have easily proposed an explicit mechanism to get what they wanted.
History
Date User Action Args
2015-07-02 16:18:34eric.snowsetmessages: + msg246087
2015-07-02 14:59:12eric.snowsetmessages: + msg246082
2015-07-02 13:51:09paul.mooresetnosy: + paul.moore
messages: + msg246076
2015-06-30 22:30:19lemburgsetmessages: + msg246005
2015-06-30 22:16:18minrksetmessages: + msg246003
2015-06-30 21:15:27lemburgsetmessages: + msg246000
2015-06-30 20:49:45minrksetmessages: + msg245999
2015-06-30 20:36:24lemburgsetmessages: + msg245998
2015-06-30 18:52:18minrksetstatus: open -> closed
resolution: rejected
messages: + msg245996
2015-06-30 08:42:20christian.heimessetnosy: + christian.heimes
2015-06-30 08:20:54ncoghlansetmessages: + msg245983
2015-06-29 20:41:57vstinnersetnosy: + vstinner
messages: + msg245967
2015-06-29 20:27:16lemburgsetnosy: + lemburg
messages: + msg245965
2015-06-29 20:18:47r.david.murraysetnosy: + r.david.murray
messages: + msg245964
2015-06-29 19:57:36tdsmithsetnosy: + tdsmith
messages: + msg245961
2015-06-29 19:49:45ned.deilysetnosy: + brett.cannon, ncoghlan, eric.snow
2015-06-29 19:30:39minrkcreate