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

Tutorial suggests dangerous (for packages with deps) install-from-testpypi step #559

Closed
awwad opened this issue Sep 28, 2018 · 13 comments
Closed
Assignees
Labels
type: task Something that needs to be done that is not a bug or feature

Comments

@awwad
Copy link

awwad commented Sep 28, 2018

The tutorial recommends (permalink) that you test your upload to test.pypi.org by trying to install it.

If your package has any dependencies, this is dangerous, as project registrations on pypi.org and test.pypi.org are decoupled, and only a fraction of the packages registered on pypi.org are currently registered on test.pypi.org.

It would be easy for an attacker to register a popular package name that is currently unregistered on test.pypi.org (e.g. docutils, futures, s3transfer, urllib3, ...) and upload attack code in setup.py, leading to execution of arbitrary code on maintainer systems. At the moment, a random package named rsa is currently listed on test.pypi.org that seems to have nothing to do with the rsa on pypi.org. Admittedly, the potential number of affected folks would probably be low. :)

Aside from this, the installation has a good shot at failing for packages with dependencies anyway, as the required packages or versions have a good chance of not being on test.pypi.org. So this dangerous step may not be worth much in any case.

@webknjaz
Copy link
Member

Good point. It's an interesting attack vector.

P.S. May I suggest that for security things there must be some non-public channel used? (I'm not aware of such though)

@pradyunsg
Copy link
Member

[email protected] is probably fine for this since the Packaging-WG folks would be notified for things like this.

The appropriate fix for this would be to do one of the following:

  • add --no-deps to the pip install
  • pip download --index-url=<test-pypi> and then pip install downloaded-artifact

The latter has the advantage of also installing dependencies from PyPI.

@webknjaz
Copy link
Member

webknjaz commented Oct 6, 2018

Is there a way to say "hey pip, only fetch this package from that index, use default index by default"?

@pradyunsg
Copy link
Member

Is there a way to say "hey pip, only fetch this package from that index, use default index by default"?

No. pip treats all indexes as equivalent internally.

@webknjaz
Copy link
Member

webknjaz commented Nov 8, 2018

So no way to map packages to indexes? Meaning that whichever index comes first in the list wins for that dist name?

@xavfernandez
Copy link
Member

No. pip collects all the available versions from all indexes then select among those versions found the "best" one.

@webknjaz
Copy link
Member

webknjaz commented Nov 9, 2018

What if there's the same dist with the same version across multiple indexes. Which one is the "best"?

@pfmoore
Copy link
Member

pfmoore commented Nov 9, 2018

Any of them. Same dist and same version is how pip considers things identical. If you have a package which has wheels/sdists with the same version but different content, that package is broken.

@flowerbug
Copy link

flowerbug commented Dec 10, 2018

It seems to me that the common sense approach is to only allow references to either:

  • packages you have uploaded to test.pypi.org yourself

  • references to other packages must be satisfied by pypi.org (temporarily copy them and delete after a few days IMO).

this leaves no attack vector open other than what is on pypi.org itself.

@awwad
Copy link
Author

awwad commented Dec 10, 2018

It seems to me that the common sense approach is to only allow references to either:

  • packages you have uploaded to test.pypi.org yourself
  • references to other packages must be satisfied by pypi.org (temporarily copy them and delete after a few days IMO).

this leaves no attack vector open other than what is on pypi.org itself.

Yes, I think that's probably right.... To the extent that it's difficult to do, you may want to skip the first bullet point.

@theacodes
Copy link
Member

@ncoghlan this discussion has bloomed beyond the packaging tutorial, it should be moved to pypa/packging-problems. Can you move it?

@theacodes theacodes added the type: task Something that needs to be done that is not a bug or feature label Jan 22, 2019
@pradyunsg
Copy link
Member

pradyunsg commented Jan 22, 2019

From https://help.github.com/articles/transferring-an-issue-to-another-repository/:

To transfer an open issue to another repository, you must have admin permissions on the repository the issue is in and the repository you're transferring the issue to.

I think only the Organization owners can, since they're the only ones with the admin permissions on pypa/packaging-problems (AFAICT no pypa team has admin permissions on packaging-problems)

@ncoghlan
Copy link
Member

@theacodes I think there's a tutorial level fix for this specific issue, which is to include --no-deps in the suggested installation test: https://pip.pypa.io/en/stable/reference/pip_install/#install-no-deps

Then there can be a note explaining that because test.pypi doesn't mirror or otherwise redirect to the real PyPI it's better to use '--no-deps' since it's guaranteed free from unwanted side effects, and trying to install deps from there probably wouldn't work anyway.

(tangent: now that the warehouse migration is complete, perhaps we should be asking @ewdurbin if the regularly scheduled purge of the test instance could instead be replaced by overwriting the test database with a snapshot of the production one? That would fix a bunch of usability issues and comprehensively plug this particular hole, since all the projects on the test instance would be owned by the same accounts that own them on the production instance)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task Something that needs to be done that is not a bug or feature
Projects
None yet
Development

No branches or pull requests

8 participants