disable executing code in .pth files

[minrk]

BPO	24534
Nosy	@malemburg, @brettcannon, @pfmoore, @ncoghlan, @vstinner, @tiran, @bitdancer, @ericsnowcurrently, @minrk, @tdsmith
Files	0001-disable-executing-code-in-.pth-files.patch: disable executing code in .pth files

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2015-06-30.18:52:18.737>
created_at = <Date 2015-06-29.19:30:39.094>
labels = ['expert-installation']
title = 'disable executing code in .pth files'
updated_at = <Date 2015-07-02.16:18:34.516>
user = 'https://github.com/minrk'

bugs.python.org fields:

activity = <Date 2015-07-02.16:18:34.516>
actor = 'eric.snow'
assignee = 'none'
closed = True
closed_date = <Date 2015-06-30.18:52:18.737>
closer = 'minrk'
components = ['Installation']
creation = <Date 2015-06-29.19:30:39.094>
creator = 'minrk'
dependencies = []
files = ['39836']
hgrepos = []
issue_num = 24534
keywords = ['patch']
message_count = 15.0
messages = ['245959', '245961', '245964', '245965', '245967', '245983', '245996', '245998', '245999', '246000', '246003', '246005', '246076', '246082', '246087']
nosy_count = 10.0
nosy_names = ['lemburg', 'brett.cannon', 'paul.moore', 'ncoghlan', 'vstinner', 'christian.heimes', 'r.david.murray', 'eric.snow', 'minrk', 'tdsmith']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue24534'
versions = ['Python 3.6']

[minrk]

.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)?

[tdsmith]

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.

[bitdancer]

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.

[malemburg]

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

[vstinner]

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

[ncoghlan]

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.

[minrk]

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).

[malemburg]

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.

[minrk]

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.

[malemburg]

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

[minrk]

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.

[malemburg]

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 :-)

[pfmoore]

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!!! :-))