Hide security-sensitive strings in VCS command log messages

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.

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):

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'

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,46 @@ def redact_password_from_url(url):
     return _transform_url(url, _redact_netloc)[0]
 
 
+class HiddenText(object):
+    def __init__(
+        self,
+        secret,         # type: str
+        redacted=None,  # type: Optional[str]
+    ):
+        # type: (...) -> None
+        if redacted is None:
+            redacted = '****'
+
+        self.secret = secret
+        self.redacted = redacted
+
+    def __repr__(self):
+        return '<HiddenText {!r}>'.format(str(self))
+
+    def __str__(self):
+        return self.redacted
+
+    # This is useful for testing.
+    def __eq__(self, other):
+        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)
+
+
+def hide_value(value):
+    # type: (str) -> HiddenText
+    return HiddenText(value)
+
+
+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
 

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,29 +54,31 @@ 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',
             url,
             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

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)