diff --git a/news/6890.bugfix b/news/6890.bugfix new file mode 100644 index 00000000000..3da0d5bb2fa --- /dev/null +++ b/news/6890.bugfix @@ -0,0 +1,2 @@ +Hide security-sensitive strings like passwords in log messages related to +version control system (aka VCS) command invocations. diff --git a/src/pip/_internal/download.py b/src/pip/_internal/download.py index 159a4433e39..57883f354e3 100644 --- a/src/pip/_internal/download.py +++ b/src/pip/_internal/download.py @@ -48,6 +48,7 @@ display_path, format_size, get_installed_version, + hide_url, netloc_has_port, path_to_url, remove_auth_from_url, @@ -729,8 +730,10 @@ def is_archive_file(name): def unpack_vcs_link(link, location): + # type: (Link, str) -> None vcs_backend = _get_used_vcs_backend(link) - vcs_backend.unpack(location, url=link.url) + assert vcs_backend is not None + vcs_backend.unpack(location, url=hide_url(link.url)) def _get_used_vcs_backend(link): diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index d57804da188..264fade4cfa 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -38,6 +38,7 @@ dist_in_usersite, ensure_dir, get_installed_version, + hide_url, redact_password_from_url, rmtree, ) @@ -813,11 +814,11 @@ def update_editable(self, obtain=True): vc_type, url = self.link.url.split('+', 1) vcs_backend = vcs.get_backend(vc_type) if vcs_backend: - url = self.link.url + hidden_url = hide_url(self.link.url) if obtain: - vcs_backend.obtain(self.source_dir, url=url) + vcs_backend.obtain(self.source_dir, url=hidden_url) else: - vcs_backend.export(self.source_dir, url=url) + vcs_backend.export(self.source_dir, url=hidden_url) else: assert 0, ( 'Unexpected version control type (in %s): %s' diff --git a/src/pip/_internal/utils/misc.py b/src/pip/_internal/utils/misc.py index 9ae5f481c92..3d1ae4709d6 100644 --- a/src/pip/_internal/utils/misc.py +++ b/src/pip/_internal/utils/misc.py @@ -61,6 +61,7 @@ from pip._internal.utils.ui import SpinnerInterface VersionInfo = Tuple[int, int, int] + CommandArgs = List[Union[str, 'HiddenText']] else: # typing's cast() is needed at runtime, but we don't want to import typing. # Thus, we use a dummy no-op version, which we tell mypy to ignore. @@ -749,8 +750,8 @@ def unpack_file( is_svn_page(file_contents(filename))): # We don't really care about this from pip._internal.vcs.subversion import Subversion - url = 'svn+' + link.url - Subversion().unpack(location, url=url) + hidden_url = hide_url('svn+' + link.url) + Subversion().unpack(location, url=hidden_url) else: # FIXME: handle? # FIXME: magic signatures? @@ -764,16 +765,52 @@ def unpack_file( ) +def make_command(*args): + # type: (Union[str, HiddenText, CommandArgs]) -> CommandArgs + """ + Create a CommandArgs object. + """ + command_args = [] # type: CommandArgs + for arg in args: + # Check for list instead of CommandArgs since CommandArgs is + # only known during type-checking. + if isinstance(arg, list): + command_args.extend(arg) + else: + # Otherwise, arg is str or HiddenText. + command_args.append(arg) + + return command_args + + def format_command_args(args): - # type: (List[str]) -> str + # type: (Union[List[str], CommandArgs]) -> str """ Format command arguments for display. """ - return ' '.join(shlex_quote(arg) for arg in args) + # For HiddenText arguments, display the redacted form by calling str(). + # Also, we don't apply str() to arguments that aren't HiddenText since + # this can trigger a UnicodeDecodeError in Python 2 if the argument + # has type unicode and includes a non-ascii character. (The type + # checker doesn't ensure the annotations are correct in all cases.) + return ' '.join( + shlex_quote(str(arg)) if isinstance(arg, HiddenText) + else shlex_quote(arg) for arg in args + ) + + +def reveal_command_args(args): + # type: (Union[List[str], CommandArgs]) -> List[str] + """ + Return the arguments in their raw, unredacted form. + """ + return [ + arg.secret if isinstance(arg, HiddenText) else arg for arg in args + ] def make_subprocess_output_error( - cmd_args, # type: List[str] + cmd_args, # type: Union[List[str], CommandArgs] cwd, # type: Optional[str] lines, # type: List[Text] exit_status, # type: int @@ -815,7 +852,7 @@ def make_subprocess_output_error( def call_subprocess( - cmd, # type: List[str] + cmd, # type: Union[List[str], CommandArgs] show_stdout=False, # type: bool cwd=None, # type: Optional[str] on_returncode='raise', # type: str @@ -882,7 +919,9 @@ def call_subprocess( env.pop(name, None) try: proc = subprocess.Popen( - cmd, stderr=subprocess.STDOUT, stdin=subprocess.PIPE, + # Convert HiddenText objects to the underlying str. + reveal_command_args(cmd), + stderr=subprocess.STDOUT, stdin=subprocess.PIPE, stdout=subprocess.PIPE, cwd=cwd, env=env, ) proc.stdin.close() @@ -1199,6 +1238,52 @@ def redact_password_from_url(url): return _transform_url(url, _redact_netloc)[0] +class HiddenText(object): + def __init__( + self, + secret, # type: str + redacted, # type: str + ): + # type: (...) -> None + self.secret = secret + self.redacted = redacted + + def __repr__(self): + # type: (...) -> str + return ''.format(str(self)) + + def __str__(self): + # type: (...) -> str + return self.redacted + + # This is useful for testing. + def __eq__(self, other): + # type: (Any) -> bool + if type(self) != type(other): + return False + + # The string being used for redaction doesn't also have to match, + # just the raw, original string. + return (self.secret == other.secret) + + # We need to provide an explicit __ne__ implementation for Python 2. + # TODO: remove this when we drop PY2 support. + def __ne__(self, other): + # type: (Any) -> bool + return not self == other + + +def hide_value(value): + # type: (str) -> HiddenText + return HiddenText(value, redacted='****') + + +def hide_url(url): + # type: (str) -> HiddenText + redacted = redact_password_from_url(url) + return HiddenText(url, redacted=redacted) + + def protect_pip_from_modification_on_windows(modifying_pip): """Protection of pip.exe from modification on Windows diff --git a/src/pip/_internal/vcs/bazaar.py b/src/pip/_internal/vcs/bazaar.py index 61b7f41e408..c49eba50e53 100644 --- a/src/pip/_internal/vcs/bazaar.py +++ b/src/pip/_internal/vcs/bazaar.py @@ -5,12 +5,18 @@ from pip._vendor.six.moves.urllib import parse as urllib_parse -from pip._internal.utils.misc import display_path, path_to_url, rmtree +from pip._internal.utils.misc import ( + display_path, + make_command, + path_to_url, + rmtree, +) from pip._internal.utils.typing import MYPY_CHECK_RUNNING from pip._internal.vcs.versioncontrol import VersionControl, vcs if MYPY_CHECK_RUNNING: from typing import Optional, Tuple + from pip._internal.utils.misc import HiddenText from pip._internal.vcs.versioncontrol import AuthInfo, RevOptions @@ -38,7 +44,7 @@ def get_base_rev_args(rev): return ['-r', rev] def export(self, location, url): - # type: (str, str) -> None + # type: (str, HiddenText) -> None """ Export the Bazaar repository at the url to the destination location """ @@ -48,12 +54,12 @@ def export(self, location, url): url, rev_options = self.get_url_rev_options(url) self.run_command( - ['export', location, url] + rev_options.to_args(), + make_command('export', location, url, rev_options.to_args()), show_stdout=False, ) def fetch_new(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None + # type: (str, HiddenText, RevOptions) -> None rev_display = rev_options.to_display() logger.info( 'Checking out %s%s to %s', @@ -61,16 +67,18 @@ def fetch_new(self, dest, url, rev_options): rev_display, display_path(dest), ) - cmd_args = ['branch', '-q'] + rev_options.to_args() + [url, dest] + cmd_args = ( + make_command('branch', '-q', rev_options.to_args(), url, dest) + ) self.run_command(cmd_args) def switch(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None - self.run_command(['switch', url], cwd=dest) + # type: (str, HiddenText, RevOptions) -> None + self.run_command(make_command('switch', url), cwd=dest) def update(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None - cmd_args = ['pull', '-q'] + rev_options.to_args() + # type: (str, HiddenText, RevOptions) -> None + cmd_args = make_command('pull', '-q', rev_options.to_args()) self.run_command(cmd_args, cwd=dest) @classmethod diff --git a/src/pip/_internal/vcs/git.py b/src/pip/_internal/vcs/git.py index 89b25782bf7..65069af7b7b 100644 --- a/src/pip/_internal/vcs/git.py +++ b/src/pip/_internal/vcs/git.py @@ -10,7 +10,7 @@ from pip._internal.exceptions import BadCommand from pip._internal.utils.compat import samefile -from pip._internal.utils.misc import display_path, redact_password_from_url +from pip._internal.utils.misc import display_path, make_command from pip._internal.utils.temp_dir import TempDirectory from pip._internal.utils.typing import MYPY_CHECK_RUNNING from pip._internal.vcs.versioncontrol import ( @@ -21,6 +21,7 @@ if MYPY_CHECK_RUNNING: from typing import Optional, Tuple + from pip._internal.utils.misc import HiddenText from pip._internal.vcs.versioncontrol import AuthInfo, RevOptions @@ -89,7 +90,7 @@ def get_current_branch(cls, location): return None def export(self, location, url): - # type: (str, str) -> None + # type: (str, HiddenText) -> None """Export the Git repository at the url to the destination location""" if not location.endswith('/'): location = location + '/' @@ -138,7 +139,7 @@ def get_revision_sha(cls, dest, rev): @classmethod def resolve_revision(cls, dest, url, rev_options): - # type: (str, str, RevOptions) -> RevOptions + # type: (str, HiddenText, RevOptions) -> RevOptions """ Resolve a revision to a new RevOptions object with the SHA1 of the branch, tag, or ref if found. @@ -172,7 +173,7 @@ def resolve_revision(cls, dest, url, rev_options): # If it looks like a ref, we have to fetch it explicitly. cls.run_command( - ['fetch', '-q', url] + rev_options.to_args(), + make_command('fetch', '-q', url, rev_options.to_args()), cwd=dest, ) # Change the revision to the SHA of the ref we fetched @@ -197,13 +198,10 @@ def is_commit_id_equal(cls, dest, name): return cls.get_revision(dest) == name def fetch_new(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None + # type: (str, HiddenText, RevOptions) -> None rev_display = rev_options.to_display() - logger.info( - 'Cloning %s%s to %s', redact_password_from_url(url), - rev_display, display_path(dest), - ) - self.run_command(['clone', '-q', url, dest]) + logger.info('Cloning %s%s to %s', url, rev_display, display_path(dest)) + self.run_command(make_command('clone', '-q', url, dest)) if rev_options.rev: # Then a specific revision was requested. @@ -213,7 +211,9 @@ def fetch_new(self, dest, url, rev_options): # Only do a checkout if the current commit id doesn't match # the requested revision. if not self.is_commit_id_equal(dest, rev_options.rev): - cmd_args = ['checkout', '-q'] + rev_options.to_args() + cmd_args = make_command( + 'checkout', '-q', rev_options.to_args(), + ) self.run_command(cmd_args, cwd=dest) elif self.get_current_branch(dest) != branch_name: # Then a specific branch was requested, and that branch @@ -228,15 +228,18 @@ def fetch_new(self, dest, url, rev_options): self.update_submodules(dest) def switch(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None - self.run_command(['config', 'remote.origin.url', url], cwd=dest) - cmd_args = ['checkout', '-q'] + rev_options.to_args() + # type: (str, HiddenText, RevOptions) -> None + self.run_command( + make_command('config', 'remote.origin.url', url), + cwd=dest, + ) + cmd_args = make_command('checkout', '-q', rev_options.to_args()) self.run_command(cmd_args, cwd=dest) self.update_submodules(dest) def update(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None + # type: (str, HiddenText, RevOptions) -> None # First fetch changes from the default remote if self.get_git_version() >= parse_version('1.9.0'): # fetch tags in addition to everything else @@ -245,7 +248,7 @@ def update(self, dest, url, rev_options): self.run_command(['fetch', '-q'], cwd=dest) # Then reset to wanted revision (maybe even origin/master) rev_options = self.resolve_revision(dest, url, rev_options) - cmd_args = ['reset', '--hard', '-q'] + rev_options.to_args() + cmd_args = make_command('reset', '--hard', '-q', rev_options.to_args()) self.run_command(cmd_args, cwd=dest) #: update submodules self.update_submodules(dest) diff --git a/src/pip/_internal/vcs/mercurial.py b/src/pip/_internal/vcs/mercurial.py index 18c779934e8..21697ff1584 100644 --- a/src/pip/_internal/vcs/mercurial.py +++ b/src/pip/_internal/vcs/mercurial.py @@ -5,12 +5,13 @@ from pip._vendor.six.moves import configparser -from pip._internal.utils.misc import display_path, path_to_url +from pip._internal.utils.misc import display_path, make_command, path_to_url from pip._internal.utils.temp_dir import TempDirectory from pip._internal.utils.typing import MYPY_CHECK_RUNNING from pip._internal.vcs.versioncontrol import VersionControl, vcs if MYPY_CHECK_RUNNING: + from pip._internal.utils.misc import HiddenText from pip._internal.vcs.versioncontrol import RevOptions @@ -28,7 +29,7 @@ def get_base_rev_args(rev): return [rev] def export(self, location, url): - # type: (str, str) -> None + # type: (str, HiddenText) -> None """Export the Hg repository at the url to the destination location""" with TempDirectory(kind="export") as temp_dir: self.unpack(temp_dir.path, url=url) @@ -38,7 +39,7 @@ def export(self, location, url): ) def fetch_new(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None + # type: (str, HiddenText, RevOptions) -> None rev_display = rev_options.to_display() logger.info( 'Cloning hg %s%s to %s', @@ -46,17 +47,19 @@ def fetch_new(self, dest, url, rev_options): rev_display, display_path(dest), ) - self.run_command(['clone', '--noupdate', '-q', url, dest]) - cmd_args = ['update', '-q'] + rev_options.to_args() - self.run_command(cmd_args, cwd=dest) + self.run_command(make_command('clone', '--noupdate', '-q', url, dest)) + self.run_command( + make_command('update', '-q', rev_options.to_args()), + cwd=dest, + ) def switch(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None + # type: (str, HiddenText, RevOptions) -> None repo_config = os.path.join(dest, self.dirname, 'hgrc') config = configparser.RawConfigParser() try: config.read(repo_config) - config.set('paths', 'default', url) + config.set('paths', 'default', url.secret) with open(repo_config, 'w') as config_file: config.write(config_file) except (OSError, configparser.NoSectionError) as exc: @@ -64,13 +67,13 @@ def switch(self, dest, url, rev_options): 'Could not switch Mercurial repository to %s: %s', url, exc, ) else: - cmd_args = ['update', '-q'] + rev_options.to_args() + cmd_args = make_command('update', '-q', rev_options.to_args()) self.run_command(cmd_args, cwd=dest) def update(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None + # type: (str, HiddenText, RevOptions) -> None self.run_command(['pull', '-q'], cwd=dest) - cmd_args = ['update', '-q'] + rev_options.to_args() + cmd_args = make_command('update', '-q', rev_options.to_args()) self.run_command(cmd_args, cwd=dest) @classmethod diff --git a/src/pip/_internal/vcs/subversion.py b/src/pip/_internal/vcs/subversion.py index a15883fa7fa..2d9ed9a100d 100644 --- a/src/pip/_internal/vcs/subversion.py +++ b/src/pip/_internal/vcs/subversion.py @@ -8,6 +8,7 @@ from pip._internal.utils.logging import indent_log from pip._internal.utils.misc import ( display_path, + make_command, rmtree, split_auth_from_netloc, ) @@ -21,7 +22,8 @@ if MYPY_CHECK_RUNNING: - from typing import List, Optional, Tuple + from typing import Optional, Tuple + from pip._internal.utils.misc import CommandArgs, HiddenText from pip._internal.vcs.versioncontrol import AuthInfo, RevOptions @@ -94,8 +96,8 @@ def get_url_rev_and_auth(cls, url): @staticmethod def make_rev_args(username, password): - # type: (Optional[str], Optional[str]) -> List[str] - extra_args = [] # type: List[str] + # type: (Optional[str], Optional[HiddenText]) -> CommandArgs + extra_args = [] # type: CommandArgs if username: extra_args += ['--username', username] if password: @@ -243,7 +245,7 @@ def get_vcs_version(self): return vcs_version def get_remote_call_options(self): - # type: () -> List[str] + # type: () -> CommandArgs """Return options to be used on calls to Subversion that contact the server. These options are applicable for the following ``svn`` subcommands used @@ -276,7 +278,7 @@ def get_remote_call_options(self): return [] def export(self, location, url): - # type: (str, str) -> None + # type: (str, HiddenText) -> None """Export the svn repository at the url to the destination location""" url, rev_options = self.get_url_rev_options(url) @@ -286,12 +288,14 @@ def export(self, location, url): # Subversion doesn't like to check out over an existing # directory --force fixes this, but was only added in svn 1.5 rmtree(location) - cmd_args = (['export'] + self.get_remote_call_options() + - rev_options.to_args() + [url, location]) + cmd_args = make_command( + 'export', self.get_remote_call_options(), + rev_options.to_args(), url, location, + ) self.run_command(cmd_args, show_stdout=False) def fetch_new(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None + # type: (str, HiddenText, RevOptions) -> None rev_display = rev_options.to_display() logger.info( 'Checking out %s%s to %s', @@ -299,21 +303,26 @@ def fetch_new(self, dest, url, rev_options): rev_display, display_path(dest), ) - cmd_args = (['checkout', '-q'] + - self.get_remote_call_options() + - rev_options.to_args() + [url, dest]) + cmd_args = make_command( + 'checkout', '-q', self.get_remote_call_options(), + rev_options.to_args(), url, dest, + ) self.run_command(cmd_args) def switch(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None - cmd_args = (['switch'] + self.get_remote_call_options() + - rev_options.to_args() + [url, dest]) + # type: (str, HiddenText, RevOptions) -> None + cmd_args = make_command( + 'switch', self.get_remote_call_options(), rev_options.to_args(), + url, dest, + ) self.run_command(cmd_args) def update(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None - cmd_args = (['update'] + self.get_remote_call_options() + - rev_options.to_args() + [dest]) + # type: (str, HiddenText, RevOptions) -> None + cmd_args = make_command( + 'update', self.get_remote_call_options(), rev_options.to_args(), + dest, + ) self.run_command(cmd_args) diff --git a/src/pip/_internal/vcs/versioncontrol.py b/src/pip/_internal/vcs/versioncontrol.py index 6d400d56883..40740e97867 100644 --- a/src/pip/_internal/vcs/versioncontrol.py +++ b/src/pip/_internal/vcs/versioncontrol.py @@ -16,18 +16,23 @@ backup_dir, call_subprocess, display_path, + hide_url, + hide_value, + make_command, rmtree, ) from pip._internal.utils.typing import MYPY_CHECK_RUNNING if MYPY_CHECK_RUNNING: from typing import ( - Any, Dict, Iterable, List, Mapping, Optional, Text, Tuple, Type + Any, Dict, Iterable, List, Mapping, Optional, Text, Tuple, Type, Union ) from pip._internal.utils.ui import SpinnerInterface + from pip._internal.utils.misc import CommandArgs, HiddenText AuthInfo = Tuple[Optional[str], Optional[str]] + __all__ = ['vcs'] @@ -67,7 +72,7 @@ def __init__( self, vc_class, # type: Type[VersionControl] rev=None, # type: Optional[str] - extra_args=None, # type: Optional[List[str]] + extra_args=None, # type: Optional[CommandArgs] ): # type: (...) -> None """ @@ -96,11 +101,11 @@ def arg_rev(self): return self.rev def to_args(self): - # type: () -> List[str] + # type: () -> CommandArgs """ Return the VCS-specific command arguments. """ - args = [] # type: List[str] + args = [] # type: CommandArgs rev = self.arg_rev if rev is not None: args += self.vc_class.get_base_rev_args(rev) @@ -271,7 +276,7 @@ def get_base_rev_args(rev): @classmethod def make_rev_options(cls, rev=None, extra_args=None): - # type: (Optional[str], Optional[List[str]]) -> RevOptions + # type: (Optional[str], Optional[CommandArgs]) -> RevOptions """ Return a RevOptions object. @@ -292,7 +297,7 @@ def _is_local_repository(cls, repo): return repo.startswith(os.path.sep) or bool(drive) def export(self, location, url): - # type: (str, str) -> None + # type: (str, HiddenText) -> None """ Export the repository at the url to the destination location i.e. only download the files, without vcs informations @@ -347,24 +352,27 @@ def get_url_rev_and_auth(cls, url): @staticmethod def make_rev_args(username, password): - # type: (Optional[str], Optional[str]) -> List[str] + # type: (Optional[str], Optional[HiddenText]) -> CommandArgs """ Return the RevOptions "extra arguments" to use in obtain(). """ return [] def get_url_rev_options(self, url): - # type: (str) -> Tuple[str, RevOptions] + # type: (HiddenText) -> Tuple[HiddenText, RevOptions] """ Return the URL and RevOptions object to use in obtain() and in some cases export(), as a tuple (url, rev_options). """ - url, rev, user_pass = self.get_url_rev_and_auth(url) - username, password = user_pass + secret_url, rev, user_pass = self.get_url_rev_and_auth(url.secret) + username, secret_password = user_pass + password = None # type: Optional[HiddenText] + if secret_password is not None: + password = hide_value(secret_password) extra_args = self.make_rev_args(username, password) rev_options = self.make_rev_options(rev, extra_args=extra_args) - return url, rev_options + return hide_url(secret_url), rev_options @staticmethod def normalize_url(url): @@ -384,7 +392,7 @@ def compare_urls(cls, url1, url2): return (cls.normalize_url(url1) == cls.normalize_url(url2)) def fetch_new(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None + # type: (str, HiddenText, RevOptions) -> None """ Fetch a revision from a repository, in the case that this is the first fetch from the repository. @@ -396,7 +404,7 @@ def fetch_new(self, dest, url, rev_options): raise NotImplementedError def switch(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None + # type: (str, HiddenText, RevOptions) -> None """ Switch the repo at ``dest`` to point to ``URL``. @@ -406,7 +414,7 @@ def switch(self, dest, url, rev_options): raise NotImplementedError def update(self, dest, url, rev_options): - # type: (str, str, RevOptions) -> None + # type: (str, HiddenText, RevOptions) -> None """ Update an already-existing repo to the given ``rev_options``. @@ -427,7 +435,7 @@ def is_commit_id_equal(cls, dest, name): raise NotImplementedError def obtain(self, dest, url): - # type: (str, str) -> None + # type: (str, HiddenText) -> None """ Install or update in editable mode the package represented by this VersionControl object. @@ -444,7 +452,7 @@ def obtain(self, dest, url): rev_display = rev_options.to_display() if self.is_repository_directory(dest): existing_url = self.get_remote_url(dest) - if self.compare_urls(existing_url, url): + if self.compare_urls(existing_url, url.secret): logger.debug( '%s in %s exists, and has correct URL (%s)', self.repo_name.title(), @@ -520,7 +528,7 @@ def obtain(self, dest, url): self.switch(dest, url, rev_options) def unpack(self, location, url): - # type: (str, str) -> None + # type: (str, HiddenText) -> None """ Clean up current location and download the url repository (and vcs infos) into location @@ -551,7 +559,7 @@ def get_revision(cls, location): @classmethod def run_command( cls, - cmd, # type: List[str] + cmd, # type: Union[List[str], CommandArgs] show_stdout=True, # type: bool cwd=None, # type: Optional[str] on_returncode='raise', # type: str @@ -566,7 +574,7 @@ def run_command( This is simply a wrapper around call_subprocess that adds the VCS command name, and checks that the VCS is available """ - cmd = [cls.name] + cmd + cmd = make_command(cls.name, *cmd) try: return call_subprocess(cmd, show_stdout, cwd, on_returncode=on_returncode, diff --git a/tests/functional/test_vcs_bazaar.py b/tests/functional/test_vcs_bazaar.py index 6bd61611ac1..af52daa63ca 100644 --- a/tests/functional/test_vcs_bazaar.py +++ b/tests/functional/test_vcs_bazaar.py @@ -6,6 +6,7 @@ import pytest +from pip._internal.utils.misc import hide_url from pip._internal.vcs.bazaar import Bazaar from tests.lib import ( _test_path_to_file_url, @@ -35,7 +36,7 @@ def test_export(script, tmpdir): _vcs_add(script, str(source_dir), vcs='bazaar') export_dir = str(tmpdir / 'export') - url = 'bzr+' + _test_path_to_file_url(source_dir) + url = hide_url('bzr+' + _test_path_to_file_url(source_dir)) Bazaar().export(export_dir, url=url) assert os.listdir(export_dir) == ['test_file'] @@ -59,7 +60,7 @@ def test_export_rev(script, tmpdir): ) export_dir = tmpdir / 'export' - url = 'bzr+' + _test_path_to_file_url(source_dir) + '@1' + url = hide_url('bzr+' + _test_path_to_file_url(source_dir) + '@1') Bazaar().export(str(export_dir), url=url) with open(export_dir / 'test_file', 'r') as f: diff --git a/tests/lib/local_repos.py b/tests/lib/local_repos.py index 4e193e72499..69c60adb3fb 100644 --- a/tests/lib/local_repos.py +++ b/tests/lib/local_repos.py @@ -5,6 +5,7 @@ from pip._vendor.six.moves.urllib import request as urllib_request +from pip._internal.utils.misc import hide_url from pip._internal.vcs import bazaar, git, mercurial, subversion from tests.lib import path_to_url @@ -59,7 +60,8 @@ def _get_vcs_and_checkout_url(remote_repository, directory): destination_path = os.path.join(directory, repository_name) if not os.path.exists(destination_path): - vcs_class().obtain(destination_path, url=remote_repository) + url = hide_url(remote_repository) + vcs_class().obtain(destination_path, url=url) return '%s+%s' % ( vcs, path_to_url('/'.join([directory, repository_name, branch])), diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 8db3384f02b..ea252ceb257 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -37,6 +37,7 @@ ) from pip._internal.utils.hashes import Hashes, MissingHashes from pip._internal.utils.misc import ( + HiddenText, build_url_from_netloc, call_subprocess, egg_link_path, @@ -44,6 +45,9 @@ format_command_args, get_installed_distributions, get_prog, + hide_url, + hide_value, + make_command, make_subprocess_output_error, netloc_has_port, normalize_path, @@ -830,6 +834,9 @@ def test_get_prog(self, monkeypatch, argv, executable, expected): (['pip', 'list'], 'pip list'), (['foo', 'space space', 'new\nline', 'double"quote', "single'quote"], """foo 'space space' 'new\nline' 'double"quote' 'single'"'"'quote'"""), + # Test HiddenText arguments. + (make_command(hide_value('secret1'), 'foo', hide_value('secret2')), + "'****' foo '****'"), ]) def test_format_command_args(args, expected): actual = format_command_args(args) @@ -1356,6 +1363,73 @@ def test_redact_password_from_url(auth_url, expected_url): assert url == expected_url +class TestHiddenText: + + def test_basic(self): + """ + Test str(), repr(), and attribute access. + """ + hidden = HiddenText('my-secret', redacted='######') + assert repr(hidden) == "" + assert str(hidden) == '######' + assert hidden.redacted == '######' + assert hidden.secret == 'my-secret' + + def test_equality_with_str(self): + """ + Test equality (and inequality) with str objects. + """ + hidden = HiddenText('secret', redacted='****') + + # Test that the object doesn't compare equal to either its original + # or redacted forms. + assert hidden != hidden.secret + assert hidden.secret != hidden + + assert hidden != hidden.redacted + assert hidden.redacted != hidden + + def test_equality_same_secret(self): + """ + Test equality with an object having the same secret. + """ + # Choose different redactions for the two objects. + hidden1 = HiddenText('secret', redacted='****') + hidden2 = HiddenText('secret', redacted='####') + + assert hidden1 == hidden2 + # Also test __ne__. This assertion fails in Python 2 without + # defining HiddenText.__ne__. + assert not hidden1 != hidden2 + + def test_equality_different_secret(self): + """ + Test equality with an object having a different secret. + """ + hidden1 = HiddenText('secret-1', redacted='****') + hidden2 = HiddenText('secret-2', redacted='****') + + assert hidden1 != hidden2 + # Also test __eq__. + assert not hidden1 == hidden2 + + +def test_hide_value(): + hidden = hide_value('my-secret') + assert repr(hidden) == "" + assert str(hidden) == '****' + assert hidden.redacted == '****' + assert hidden.secret == 'my-secret' + + +def test_hide_url(): + hidden_url = hide_url('https://user:password@example.com') + assert repr(hidden_url) == "" + assert str(hidden_url) == 'https://user:****@example.com' + assert hidden_url.redacted == 'https://user:****@example.com' + assert hidden_url.secret == 'https://user:password@example.com' + + @pytest.fixture() def patch_deprecation_check_version(): # We do this, so that the deprecation tests are easier to write. diff --git a/tests/unit/test_vcs.py b/tests/unit/test_vcs.py index 66754c667ac..c64ec2797f7 100644 --- a/tests/unit/test_vcs.py +++ b/tests/unit/test_vcs.py @@ -6,6 +6,7 @@ from pip._vendor.packaging.version import parse as parse_version from pip._internal.exceptions import BadCommand +from pip._internal.utils.misc import hide_url, hide_value from pip._internal.vcs import make_vcs_requirement_url from pip._internal.vcs.bazaar import Bazaar from pip._internal.vcs.git import Git, looks_like_hash @@ -342,7 +343,7 @@ def test_subversion__get_url_rev_and_auth(url, expected): @pytest.mark.parametrize('username, password, expected', [ (None, None, []), ('user', None, []), - ('user', 'pass', []), + ('user', hide_value('pass'), []), ]) def test_git__make_rev_args(username, password, expected): """ @@ -355,7 +356,8 @@ def test_git__make_rev_args(username, password, expected): @pytest.mark.parametrize('username, password, expected', [ (None, None, []), ('user', None, ['--username', 'user']), - ('user', 'pass', ['--username', 'user', '--password', 'pass']), + ('user', hide_value('pass'), + ['--username', 'user', '--password', hide_value('pass')]), ]) def test_subversion__make_rev_args(username, password, expected): """ @@ -369,12 +371,15 @@ def test_subversion__get_url_rev_options(): """ Test Subversion.get_url_rev_options(). """ - url = 'svn+https://user:pass@svn.example.com/MyProject@v1.0#egg=MyProject' - url, rev_options = Subversion().get_url_rev_options(url) - assert url == 'https://svn.example.com/MyProject' + secret_url = ( + 'svn+https://user:pass@svn.example.com/MyProject@v1.0#egg=MyProject' + ) + hidden_url = hide_url(secret_url) + url, rev_options = Subversion().get_url_rev_options(hidden_url) + assert url == hide_url('https://svn.example.com/MyProject') assert rev_options.rev == 'v1.0' assert rev_options.extra_args == ( - ['--username', 'user', '--password', 'pass'] + ['--username', 'user', '--password', hide_value('pass')] ) @@ -519,43 +524,48 @@ def assert_call_args(self, args): assert self.call_subprocess_mock.call_args[0][0] == args def test_obtain(self): - self.svn.obtain(self.dest, self.url) - self.assert_call_args( - ['svn', 'checkout', '-q', '--non-interactive', '--username', - 'username', '--password', 'password', - 'http://svn.example.com/', '/tmp/test']) + self.svn.obtain(self.dest, hide_url(self.url)) + self.assert_call_args([ + 'svn', 'checkout', '-q', '--non-interactive', '--username', + 'username', '--password', hide_value('password'), + hide_url('http://svn.example.com/'), '/tmp/test', + ]) def test_export(self): - self.svn.export(self.dest, self.url) - self.assert_call_args( - ['svn', 'export', '--non-interactive', '--username', 'username', - '--password', 'password', 'http://svn.example.com/', - '/tmp/test']) + self.svn.export(self.dest, hide_url(self.url)) + self.assert_call_args([ + 'svn', 'export', '--non-interactive', '--username', 'username', + '--password', hide_value('password'), + hide_url('http://svn.example.com/'), '/tmp/test', + ]) def test_fetch_new(self): - self.svn.fetch_new(self.dest, self.url, self.rev_options) - self.assert_call_args( - ['svn', 'checkout', '-q', '--non-interactive', - 'svn+http://username:password@svn.example.com/', - '/tmp/test']) + self.svn.fetch_new(self.dest, hide_url(self.url), self.rev_options) + self.assert_call_args([ + 'svn', 'checkout', '-q', '--non-interactive', + hide_url('svn+http://username:password@svn.example.com/'), + '/tmp/test', + ]) def test_fetch_new_revision(self): rev_options = RevOptions(Subversion, '123') - self.svn.fetch_new(self.dest, self.url, rev_options) - self.assert_call_args( - ['svn', 'checkout', '-q', '--non-interactive', - '-r', '123', - 'svn+http://username:password@svn.example.com/', - '/tmp/test']) + self.svn.fetch_new(self.dest, hide_url(self.url), rev_options) + self.assert_call_args([ + 'svn', 'checkout', '-q', '--non-interactive', '-r', '123', + hide_url('svn+http://username:password@svn.example.com/'), + '/tmp/test', + ]) def test_switch(self): - self.svn.switch(self.dest, self.url, self.rev_options) - self.assert_call_args( - ['svn', 'switch', '--non-interactive', - 'svn+http://username:password@svn.example.com/', - '/tmp/test']) + self.svn.switch(self.dest, hide_url(self.url), self.rev_options) + self.assert_call_args([ + 'svn', 'switch', '--non-interactive', + hide_url('svn+http://username:password@svn.example.com/'), + '/tmp/test', + ]) def test_update(self): - self.svn.update(self.dest, self.url, self.rev_options) - self.assert_call_args( - ['svn', 'update', '--non-interactive', '/tmp/test']) + self.svn.update(self.dest, hide_url(self.url), self.rev_options) + self.assert_call_args([ + 'svn', 'update', '--non-interactive', '/tmp/test', + ]) diff --git a/tests/unit/test_vcs_mercurial.py b/tests/unit/test_vcs_mercurial.py index f47b3882e31..630619b8236 100644 --- a/tests/unit/test_vcs_mercurial.py +++ b/tests/unit/test_vcs_mercurial.py @@ -6,6 +6,7 @@ from pip._vendor.six.moves import configparser +from pip._internal.utils.misc import hide_url from pip._internal.vcs.mercurial import Mercurial from tests.lib import need_mercurial @@ -24,7 +25,7 @@ def test_mercurial_switch_updates_config_file_when_found(tmpdir): hgrc_path = os.path.join(hg_dir, 'hgrc') with open(hgrc_path, 'w') as f: config.write(f) - hg.switch(tmpdir, 'new_url', options) + hg.switch(tmpdir, hide_url('new_url'), options) config.read(hgrc_path)