Cannot pip install SVN dependency with authentication using SVN 1.8+

[johnthagen]

Environment

• pip version: 19.0.3
• Python version: 3.6.7
• OS: Ubuntu 18.04 x64 LTS

Description

pip Subversion docs

Starting with SVN 1.8, SVN is non-interactive by default. Before that, it will prompt for a password when the user performs a svn checkout.

The problem is that when pip calls out to svn checkout it is not interactive, and will not allow the user to enter their password. One solution is to store SVN passwords, but that may not be allowed by company rules or that may simply not be desirable for security reasons.

For some more context see

• Add --force-interactive arg to prompt password gedex/wp-tools#33
• https://groups.google.com/forum/#!topic/subversion_users/fQ4K5GShGPU
• https://stackoverflow.com/questions/26066979

The solution seems to be:

1. If SVN version is <1.8, work as it does now (no extra arguments needed).
2. If SVN version is >=1.8, add the --force-interactive command line flag.

Some context from svn checkout --help

  --non-interactive        : do no interactive prompting (default is to prompt
                             only if standard input is a terminal device)
  --force-interactive      : do interactive prompting even if standard input
                             is not a terminal device

Perhaps another solution would be to make svn think it's being called from a terminal device when called from pip?

Using environment variables is another potential option, but it runs into the same fundamental issue: users have to store their password (and in this case in plaintext). This doesn't seem to be appropriate for user workstations that could be shared.

Subversion versions on popular supported Linux distros:

• Ubuntu 18.04 LTS: 1.9.7
• Ubuntu 16.04 LTS: 1.9.3
• RHEL 7 / CentOS 7: 1.7.14
• RHEL 6 / CentOS 6: 1.6.11

Expected behavior

On SVN 1.7, 1.8, and 1.9, when pip installs an SVN dependency, it prompts the user for their password if they have not saved it locally.

How to Reproduce

1. Install Subversion >=1.8.

• If using Ubuntu 18.04, sudo apt install subversion. svn --version will return 1.9.7.

2. Make a venv:

$ python3 -m venv venv
$ source venv/bin/activate
(venv) $ pip install pip==19.0.3

3. Don't save your SVN password and try to pip install an SVN URL from a URL that requires authentication. pip's invocation of svn will not prompt for a password, so it will always fail to install.

(venv) $ pip install svn+https://my-svn-server.com/project
Collecting svn+https://my-svn-server.com/project
   Checking out https://my-svn-server.com/project /tmp/pip-req-build-1g_qavb
svn: E170013: Unable to connect to repository at URL 'https://https://my-svn-server.com/project'
svn: E215004: No more credentials or we tried too many times.
Authentication failed.
Command "svn checkout -q https://my-svn-server.com/project /tmp/pip-req-build-1g_qavb" failed with error code 1 in None

References

• pip/src/pip/_internal/vcs/subversion.py

  Line 58 in e5353f2

  cmd_args = ['checkout', '-q'] + rev_options.to_args() + [url, dest]

• pip/src/pip/_internal/vcs/__init__.py

  Line 546 in e5353f2

  def run_command(

• pip/src/pip/_internal/utils/misc.py

  Line 722 in e5353f2

  proc = subprocess.Popen(

[johnthagen]

It seems like the most "proper" way would be to make Popen calls to SVN with a psuedo tty, so that SVN behaves as if it were called from the terminal directly.

• pty
• https://gist.github.com/thomasballinger/7979808
• https://stackoverflow.com/a/43012138/1398841

[johnthagen]

There does seem to be precedent for special casing based on Subversion version:

pip/src/pip/_internal/vcs/subversion.py

Line 157 in e5353f2

else: # subversion >= 1.7 does not have the 'entries' file

pip/src/pip/_internal/vcs/subversion.py

Line 44 in e5353f2

# directory --force fixes this, but was only added in svn 1.5

pip/src/pip/_internal/vcs/subversion.py

Lines 174 to 175 in e5353f2

	try:
	# subversion >= 1.7

[cjerdonek]

My initial thought on this is that passing --force-interactive would be better than trying to use a pseudo-terminal.

Also, it seems like this shouldn't be done by default, but perhaps only if we can be sure pip is being used interactively (e.g. if sys.stdout.isatty() returns True) -- a bit like how svn is described as behaving.

[pfmoore]

It's worth pointing out that ptys aren't an option on Windows.

[johnthagen]

@cjerdonek Yeah, I agree that a pseudo-terminal would be a more complex solution.

The only issue with trying to pass --force-interactive regardless of SVN version would be that SVN 1.7 and 1.6 don't come with this option, so doing that would break compatibility with RHEL/CentOS 6 and RHEL/CentOS 7. (Gotta love ancient supported toolchains...)

We could work around this with svn --version, but I don't know if that is a bridge too far for a tool like pip.

I think this could be a proposed solution (in pseudocode):

svn_version = get_version()  # `svn --version`
if svn_version >= 1.8 and sys.stdout.isatty():
    svn_options.append('--force-interactive')
else:
    # Do nothing
    pass

Thoughts?

[cjerdonek]

While this might look easy, I think this might take a bit more work in certain ways to fully get right. Some thoughts:

1. If pip will be in a mode expecting interaction, we'd want to be sure that the output to the user (e.g. command prompts) is flushed as it is currently going through logging code (and also accept stdin). This will add additional complications to call_subprocess().
2. If we need to call out to svn to get the version, I think we'd only want to do that once (lazily), and then cache the result. (The Subversion class isn't structured in the way that I'd personally like for this yet, but it's close.)
3. To do this properly, we would probably want to do what SVN is doing and provide command-line options to force the behavior one way or the other (e.g. if the user is redirecting stdout but still wants interactivity). Similar to (2), the VersionControl class (and subclasses) isn't really set up yet to accept command-line options.
4. Lastly, I don't think we'd need to worry about providing new functionality like this for older versions of SVN.

[johnthagen]

Lastly, I don't think we'd need to worry about providing new functionality like this for older versions of SVN.

Okay, so just so I'm clear, you mean not supporting Subversion 1.6 and 1.7 (RHEL 6/7 and CentOS 6/7)? So we would push the --force-interactive flag without regard for what version of Subversion they are running (since 1.6/1.7 don't have the flag?

# CentOS 7
$ svn --version
svn, version 1.7.14 (r1542130)
   compiled Apr 11 2018, 02:40:28

$ svn --force-interactive
svn: invalid option: --force-interactive

I don't really disagree in principle (it's a burden to support old tools), but just wanted to be clear that pip would then no longer be able to work with the platform installed Subversion on RHEL 6/7 and CentOS 6/7. RHEL/CentOS 7 will be supported until 2024 😕.

(The Subversion class isn't structured in the way that I'd personally like for this yet, but it's close.)

If we can agree on a way forward I'm willing to try to put together a PR. But if it involves refactoring the Subversion class, a pip maintainer would probably be better suited. If a pip maintainer made the refactorings you suggest, I would be willing give it a shot to add --force-interactive on top when needed.

[cjerdonek]

So we would push the --force-interactive flag without regard for what version of Subversion they are running (since 1.6/1.7 don't have the flag?

I meant passing the flag only for SVN 1.8+ (which is why I also suggested caching the value of getting the version). But sorry, I had missed or forgotten that older versions were interactive by default in the absence of any option. So you can disregard that last bullet. :)

I would be willing give it a shot to add --force-interactive on top when needed.

Okay, great. But yeah, I'd prefer if you waited a bit, if that's okay. I actually have a VCS-related PR queued up (as a follow-up to PR #6356) that I'm now planning to modify in response to this issue. (The PR should become simpler in fact, and should help for this.) I'd also like to think about how call_subprocess() should best be changed for this, because the logging there was just reworked to be simpler, and I'd like to avoid re-introducing too much more complexity there, if possible.

If you want I can describe some of the VCS changes I'd like to see. They aren't super hard but may involve touching a few things.

I'll also think to see what, if any, of this PR can be done in advance of the refactorings.

[cjerdonek]

One thing you could do now in preparation is add a get_vcs_version() class method to the Subversion class that returns a tuple, and add a test for it. There's already a Git.get_git_version() method you could look at for ideas. The test could be marked to run only if SVN is installed, and at least one of the test environments should require that SVN be installed. (You can look at the last Bazaar PR to see how that was done.)

[cjerdonek]

Btw, is there a way to prevent interactivity on older versions of SVN (e.g. to prevent hangs)?

[johnthagen]

Btw, is there a way to prevent interactivity on older versions of SVN (e.g. to prevent hangs)?

# CentOS 7.6.
$ svn --version
svn, version 1.7.14 (r1542130)
   compiled Apr 11 2018, 02:40:28
$ svn checkout --help
...
Global options:
  --username ARG           : specify a username ARG
  --password ARG           : specify a password ARG
  --no-auth-cache          : do not cache authentication tokens
  --non-interactive        : do no interactive prompting

--non-interactive should be what you want, and it is available at least in SVN 1.7.

[johnthagen]

@cjerdonek I submitted #6390, let me know if that is what you were looking for.

[cjerdonek]

--non-interactive should be what you want, and it is available at least in SVN 1.7.

Thanks. A related positive change IMO would be to pass that flag to SVN when SVN is 1.7 and isatty() returns False (the flip-side of the case you're interested in, to prevent hangs when pip isn't being used interactively).

[johnthagen]

A related positive change IMO would be to pass that flag to SVN when SVN is 1.7 and isatty() returns False

@cjerdonek I think we need to be very cautious about this. For example, it's currently very useful that when pip is called from tox on CentOS 7 with SVN 1.7, that svn prompts for a password. I doubt pip is in a tty at that point.

tox -> pip -> svn: prompt user for password.

This allows us to use tox from the terminal against private SVN repos, but not have users store their passwords on the system.

It's not a difficult configuration to make sure svn only prompts you when you want it to prompt you (e.g. for a password you are intending not to store).

[cjerdonek]

For example, it's currently very useful that when pip is called from tox on CentOS 7 with SVN 1.7, that svn prompts for a password. I doubt pip is in a tty at that point.

Okay, good to know. This is also why I said the following above:

3. To do this properly, we would probably want to do what SVN is doing and provide command-line options to force the behavior one way or the other (e.g. if the user is redirecting stdout but still wants interactivity).

By the way, your point would also apply to the SVN 1.8+ case, with the approach for this PR of checking isatty(), right?

[cjerdonek]

For example, it's currently very useful that when pip is called from tox on CentOS 7 with SVN 1.7, that svn prompts for a password.

Actually, can you confirm whether you can really do this with recent versions of pip? It appears as if this isn't possible as of pip 10.0 (since stdin is closed after the call to subprocess.Popen()): #4982

[johnthagen]

By the way, your point would also apply to the SVN 1.8+ case, with the approach for this PR of checking isatty(), right?

Yeah, I'm wondering if we should avoid suppressing interactive in either case based on isatty(). It's a tough one, because on one hand getting svn to ignore superfluous prompts is nice, but if it makes it so it's not possible to input a password at all when run from a tool like tox, that is a problem. I'm not sure how tox calls pip, to know whether it thinks its in a tty or not, but we should look into it when fixing this problem.

Actually, can you confirm whether you can really do this with recent versions of pip? It appears as if this isn't possible as of pip 10.0 (since stdin is closed after the call to subprocess.Popen()): #4982

I can confirm the following configurations all work with stock installed tools on CentOS 7.

• Python 3.6.6 from EPEL
• svn 1.7.14 from the base CentOS 7 repo
• pip 19.0.3
• tox 3.7.0

Configuration

~/.subversion/servers has the following option set so that svn does not prompt the user whether they want to store passwords (which would break pip usage).

store-plaintext-passwords = no

Test 1: pip from terminal

$ pip install svn+https://my-private-svn.com/svn/project/project-1

# Prompts for password, user enters
# pip successfully installs `project-1`

Test 2: pip from terminal with requirements.txt

requirements.txt:

svn+https://my-private-svn.com/svn/project/project-1

$ pip install -r requirements.txt

# Prompts for password, user enters
# pip successfully installs `project-1`

Test 3: tox

requirements.txt:

svn+https://my-private-svn.com/svn/project/project-1

tox.ini:

...
[testenv]
deps =
    -r{toxinidir}/requirements.txt
...

$ tox --recreate
py36 create: ...
py36 installdeps -r /home/user/requirements.txt
# Prompts for password, user enters
# pip successfully installs `project-1`
...

Follow Up

I don't know the intricacies of Linux terminals well, but it would appear that stdin prompts are different in some ways than these password prompts? For one, you don't see what you type when you enter passwords, so is there some other password/authentication-specific Linux command going on here?

[johnthagen]

@cjerdonek After some digging into the Subversion source code (always a fun activity), I found where svn is opening the special device node /dev/tty:

https://svn.apache.org/repos/asf/subversion/trunk/subversion/libsvn_subr/prompt.c

As I understand it, /dev/tty is a special device node that refers to the controlling terminal for a process. This allows svn to prompt for a password directly to the terminal regardless of what other processes have called it.

So I believe this explains why stdin doesn't apply in the case of password prompts from svn.

[johnthagen]

After some more testing, perhaps the tox case not making pip think it's in a tty is not actually a concern.

I wrote a small Python script that checked if stdout, stderr, and stdin .isatty().

# tox.ini
[testenv]
commands = python tty_test.py

$ tox -e py36

All three returned True. This doesn't 100% prove that when pip is called (it is used in the deps section of tox.ini), it thinks it's in a tty, but gives me hope that this may not be an issue.

[cjerdonek]

Thanks for all your research and digging! 👍

[cjerdonek]

@johnthagen Let me know if you want my thoughts on the next steps, or if you want to propose something and I can react to that. I think it might be helpful to do at least one more incremental PR before landing the functionality.

[johnthagen]

For reference, svn options are:

# svn <= 1.7
 --non-interactive        : do no interactive prompting

# svn >= 1.8
  --non-interactive        : do no interactive prompting (default is to prompt
                             only if standard input is a terminal device)
  --force-interactive      : do interactive prompting even if standard input
                             is not a terminal device

Just so we're on the same page, our current pseudocode proposal is:

svn_version = get_vcs_version()  # `svn --version`
if not sys.stdin.isatty():
    # TODO: Test that `tox` `deps` install step runs `pip` in a TTY.
    svn_options.append('--non-interactive')
elif svn_version >= 1.8:
    svn_options.append('--force-interactive')

Or a more conservative proposal is to just address needing to get the password prompt on 1.8+.

svn_version = get_vcs_version()  # `svn --version`
if svn_version >= 1.8 and sys.stdin.isatty():
    svn_options.append('--force-interactive')

The second has less risk of breaking situations were you want to be prompted on 1.7, but has the risk that 1.7 and 1.8 could behave differently from pip. The first approach is probably better (I just wish I had a better handle on when isatty() will be False).

What are your thoughts?

[cjerdonek]

Do you know for sure that --non-interactive is present in all SVN versions <= 1.7?

I was thinking the next PR could define and test a method on the Subversion class that returns the list of global options to use (empty list, ['--non-interactive'], or ['--force-interactive']), but not start including them in the actual invocations yet. (Do you know which SVN invocations these options will need to be passed to, by the way?)

I think the Subversion class's __init__() method should start accepting an optional use_interactive argument (or some other name) that defaults to a value based on sys.stdin.isatty(). This will help with unit-testing the method above. It can also be used later to force certain behavior e.g. via pip command-line options.

I also think the class should grow a private _vcs_version instance attribute to cache the version value from the method you just wrote. It can initially be set to None (unset), with a None version value being stored as some other non-None value (perhaps False). The get_vcs_version() method can be changed to check this instance attribute, and call a class method called something like call_vcs_version() that would contain your current logic. This private instance attribute will also help with unit-testing the method that returns the interactive options, because it will make it easy to test different "versions" of SVN.

[cjerdonek]

I agree with your summary, by the way. Thanks!

[johnthagen]

Do you know for sure that --non-interactive is present in all SVN versions <= 1.7?

According to: https://svn.apache.org/repos/asf/subversion/trunk/CHANGES

Version 0.14.4 [Alpha Interim 4] (released 29 Oct 2002, revision 3553)

• new --non-interactive switch for commandline client

[johnthagen]

(Do you know which SVN invocations these options will need to be passed to, by the way?)

I believe this is where the options would need to be applied:

pip/src/pip/_internal/vcs/subversion.py

Lines 84 to 93 in c8e9caa

	def fetch_new(cls, dest, url, rev_options):
	rev_display = rev_options.to_display()
	logger.info(
	'Checking out %s%s to %s',
	url,
	rev_display,
	display_path(dest),
	)
	cmd_args = ['checkout', '-q'] + rev_options.to_args() + [url, dest]
	cls.run_command(cmd_args)

pip checks out the Subversion project to a folder in /tmp before installing it from that folder.

[cjerdonek]

Regarding the various SVN invocations to consider, there are also invocations of commands like "export", "update", etc. When editable mode is being used, a different directory is used, and in-place updates can occur. I'm guessing at least "update" would also need the options passed.

Re: tox, I did a quick check of the tox code base, and it looks like stdin is never passed to its subprocess calls. Perhaps this means any TTY is preserved (unlike pip which passes stdin=subprocess.PIPE).

[johnthagen]

Can confirm the following commands are using in subversion.py and support --non-interactive and --force-interactive:

• export
• checkout
• switch
• update
• info

[johnthagen]

@cjerdonek With #6439 merged, I think the last thing that needs to be completed is for us to actually add the get_remote_call_options() result to the invocations of svn where they are needed:

pip/src/pip/_internal/vcs/subversion.py

Lines 271 to 278 in 07ce2ab

	These options are applicable for the following ``svn`` subcommands used
	in this class.
	- checkout
	- export
	- info
	- switch
	- update

Since this is when we actually change functionality, I plan to additionally test out the proposed pip egg with this change on some private SVN repos (on SVN 1.7 & 1.8+) to make sure everything is working as we expect before we merge the PR.

How would you like to proceed?

[cjerdonek]

@johnthagen That sounds good (and glad to hear you can test out on private repos). I don't think there will be much code. But if you have any questions in advance, feel free to let me know.

[johnthagen]

@cjerdonek. Do you know which pip version this will land in? I'm planning to test the current master branch and then also the first released version as well to triple check everything.

[cjerdonek]

The next one (19.2).

[cjerdonek]

Also, the next release will probably happen sometime in July (every 3 months).

[johnthagen]

Wanted to report that I tested the master branch at 83d813c on a private SVN repository in the following configurations:

• Ubuntu 18.04 LTS: svn 1.9.7
• CentOS 7: svn 1.7.14

Everything worked correctly, so I'd say everything is good to go for 19.2.

I'll test one final time when 19.2 is released to give a final smoke test as it goes into the wild.

[cjerdonek]

Great! Thank you! 👍