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

pip should warn on git:// protocol #1983

Closed
alex opened this issue Aug 20, 2014 · 9 comments · Fixed by #7938
Closed

pip should warn on git:// protocol #1983

alex opened this issue Aug 20, 2014 · 9 comments · Fixed by #7938
Labels
auto-locked Outdated issues that have been locked by automation C: vcs pip's interaction with version control systems like git, svn and bzr state: awaiting PR Feature discussed, PR is needed type: security Has potential security implications

Comments

@alex
Copy link
Member

alex commented Aug 20, 2014

This is un-authenticated, the same as HTTP, and is thus a MITM vector.

@jezdez
Copy link
Member

jezdez commented Aug 21, 2014

Makes sense, we should do the same for other vcs backends using unencrypted protocols

@dstufft dstufft added the type: security Has potential security implications label Mar 17, 2015
@pradyunsg pradyunsg added state: awaiting PR Feature discussed, PR is needed C: vcs pip's interaction with version control systems like git, svn and bzr labels Oct 2, 2017
@deveshks
Copy link
Contributor

I wanted to take a shot at this PR.

But before that, I wanted to understand what kind of URL does git:// protocol refers to ? Is this from one of the URL's mentioned in the pip install's Git subsection ?

@uranusjr
Copy link
Member

uranusjr commented Mar 29, 2020

@deveshks It’s the Git protocol, which is an ancient (OK this is an exaggeration) standard before “smart HTTP” came about, and was the prevelant choice for read-only remotes (because fetching through HTTP and HTTPS was very slow at the time). To install from it, you’d write something like pip install git+git://example.com/repo.git.

Honestly I feel this is no longer necessary. Nobody really mentions git:// anywhere anymore, and anyone still using the Git protocol at this point probably knows what they’re doing and don’t need the warning anyway.

@uranusjr
Copy link
Member

I think one thing we should do is to change or remove those git+git:// and git:// entries in the documentation to avoid the impression that it is a good idea.

@deveshks
Copy link
Contributor

Hi @uranusjr ,

I did try pip install -e git+git://github.com/python/mypy.git which I assume refers to the Git protocol, and got the error ERROR: Could not detect requirement name for 'git+git://github.com/python/mypy.git', please specify one with #egg=your_package_name, so I think that should be enough to ensure failure if the old protocol is used.

BTW, I also tried pip install -e git://github.com/python/mypy#egg=mypy which refers to the first URL git://git.example.com/MyProject#egg=MyProject and it succeeds.

I also tried pip install -e git+git://github.com/python/mypy#egg=mypy which refers to the second URL git+git://git.example.com/MyProject#egg=MyProject and it succeeds too.

We can remove both URL's from the documentation, but I think we should also raise an exception if the users are still using said URL. What do you think?

@uranusjr
Copy link
Member

I think we should also raise an exception if the users are still using said URL.

I don’t think it’s a good idea to raise an exception. It is a perfectly valid URL, and pip should keep it working. The most pip should do is to emit a warning message, althought that is still unnecessary IMO.

@deveshks
Copy link
Contributor

Sure @uranusjr , I think that makes more sense.

So do I create a PR to just remove the URL's from the documentation, or do I also emit a warning message as well?

@uranusjr
Copy link
Member

@deveshks Let’s do them in two PRs. The documentation one should be straightforward, and discussion on the warning’s wording could delay it unnecessarily.

@deveshks
Copy link
Contributor

deveshks commented Mar 30, 2020

Thanks @uranusjr , have create the first PR for the doc. As for the warning, I think it can be something like

WARNING. You are using <url> which is based on Git protocol, which is unauthenticated. 
We suggest you to use other appropriate vcs urls as per the pip install docs

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label May 5, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2020
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 C: vcs pip's interaction with version control systems like git, svn and bzr state: awaiting PR Feature discussed, PR is needed type: security Has potential security implications
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants