Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow direct urls in install_requires #6301

Closed
themightyoarfish opened this issue Feb 26, 2019 · 12 comments
Closed

Allow direct urls in install_requires #6301

themightyoarfish opened this issue Feb 26, 2019 · 12 comments
Labels
auto-locked Outdated issues that have been locked by automation resolution: wrong project Should be reported elsewhere type: support User Support

Comments

@themightyoarfish
Copy link

What's the problem this feature will solve?
I want to put a package on PyPi which uses the new direct url syntax
pkg@git-repo-url from PEP 508. Uploading to PyPi yields

HTTPError: 400 Client Error: Invalid value for requires_dist. Error: Can't have direct dependency:

It is stated here that this is for "security reasons", but as pointed out later in the thread, this seems nonsensical as setup.py could execute arbitrary code anyway. Are there any other security concerns I am missing?

Describe the solution you'd like
Simply allow direct urls.

I have a package which depends on another package which I don't want to go through the trouble of uploading to PyPi as well. This dependency then breaks installing my package from PyPi.

@uranusjr
Copy link
Member

The restriction is nonsensical for setup.py, but not for dependency specification in general. So pip probably needs some indication when it should allow this (either by their backend or the user).

Another possible way to draw the line is to allow this in pip, but disallow submitting this kind of distros to PyPI.

@themightyoarfish
Copy link
Author

The point of this issue is kind of to discuss if we can't have it on PyPI as well (it works with pip already), since there is no security benefit in disallowing it.

@pfmoore
Copy link
Member

pfmoore commented Mar 19, 2019

There is a security benefit. The actual restriction is that direct URLs cannot occur in the requires-dist metadata element. A wheel (or a project that uses a build backend other than setuptools) does not execute setup.py, so there's no code execution issue there. However, the issue of downloading something from PyPI (which is presumably trusted) which triggers a further download from another arbitrary site that you may not trust, remains.

Also, even for setuptools-based source distributions, restricting direct URLs acts as a layered defense - you still need to review setup.py, but you don't need to review the security of referenced websites as well.

@frankcorneliusmartin
Copy link

frankcorneliusmartin commented Mar 19, 2019

Anyone can publish anything on PyPI, so I would not consider this as trusted anyway? Only after you have verified the publisher you could trust a dependency, or am I missing something here.

@pfmoore
Copy link
Member

pfmoore commented Mar 19, 2019

If you don't trust PyPI, then yes, you have to verify everything (or not use PyPI). But many people do have some level of trust in PyPI - for example, that the "requests" package cannot be suddenly hijacked by a malicious 3rd party.

@frankcorneliusmartin
Copy link

frankcorneliusmartin commented Mar 19, 2019

Yes, I agree that it adds some level of trust, especially long lived/known (or previously used) packages. However if you do not check the contents of setup.py this level of security does no longer hold, as we do not trust everything on PyPI. Therefore making it just as secure as allowing external direct urls, this is true when not considering that it requires some effort to publish (a malicious) dependency on PyPI.

@uranusjr
Copy link
Member

uranusjr commented Mar 19, 2019

The problem is that setup.py is not only used to be directly run on your machine to install the package, but also to create a wheel, either by the developer (to upload to PyPI) or pip (to later install it). As Paul mentioned, the problem is that wheel does not (and should not, I want to add) support URL specs, so if setup(install_requires=...) does, setuptools needs to somehow convert it to a non-URL dependency when duilding the wheel, which is by no means straightforward, if at all possible. And you’d need to convince setuptools maintainers instead of pip on that.

@geryogam
Copy link

geryogam commented Aug 7, 2019

@uranusjr

As Paul mentioned, the problem is that wheel does not (and should not, I want to add) support URL specs,

Not according to @ncoghlan in issue 763: every tool (including wheel) should support direct references and only public index servers MAY prohibit direct references.

@geryogam
Copy link

geryogam commented Aug 7, 2019

@themightyoarfish

We are facing a similar issue at work. We cannot upload Python wheels with direct references in their metadata to a private index server on AzureDevOps using twine (for you it was a public index server, PyPI):

HTTPError: 400 Client Error: Bad Request - Packages with direct (URL) references in Requires-Dist are not allowed.

@chrahunt
Copy link
Member

chrahunt commented Aug 8, 2019

These all seem like they are related specifically to policies on the index servers, unless I'm misunderstanding. Is there any pip command that fails due to direct URL usage in requirements? If so can we have a specific issue created for that with instructions for reproducing?

@geryogam
Copy link

geryogam commented Aug 8, 2019

@chrahunt

Is there any pip command that fails due to direct URL usage in requirements?

No.

These all seem like they are related specifically to policies on the index servers, unless I'm misunderstanding.

Exactly. We use the default Python index server provided by AzureDevOps pipelines. So we need to investigate if we can configure it for handling direct references in the Requires-Dist metadata of wheels.

@chrahunt
Copy link
Member

chrahunt commented Sep 22, 2019

Since this behavior is coming from pypa/warehouse, I will close the issue.

If anyone coming up against this wants to see it changed, I would suggest creating an issue over at the Warehouse repository and linking back here. I couldn't find an existing issue, just the place where the change was introduced: #1013.

If any change is required in pip to support this use case, then we can address it after confirmation that PyPI will start allowing direct references.

@chrahunt chrahunt added the resolution: wrong project Should be reported elsewhere label Sep 22, 2019
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Oct 22, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation resolution: wrong project Should be reported elsewhere type: support User Support
Projects
None yet
Development

No branches or pull requests

7 participants