From d58a96788c9255d7a447d9205995cd5f4fa6f490 Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Tue, 12 May 2020 21:46:37 +0700 Subject: [PATCH 1/2] Fix leaked URLs with credentials in the output --- piptools/exceptions.py | 10 ++++- piptools/scripts/compile.py | 10 +++-- piptools/utils.py | 4 ++ tests/test_cli_compile.py | 77 +++++++++++++++++++++++++++++-------- tests/test_utils.py | 10 +++++ 5 files changed, 89 insertions(+), 22 deletions(-) diff --git a/piptools/exceptions.py b/piptools/exceptions.py index 5aac84bb3..527897274 100644 --- a/piptools/exceptions.py +++ b/piptools/exceptions.py @@ -1,3 +1,6 @@ +from pip._internal.utils.misc import redact_auth_from_url + + class PipToolsError(Exception): pass @@ -40,11 +43,14 @@ def __str__(self): source_ireqs = getattr(self.ireq, "_source_ireqs", []) lines.extend(" {}".format(ireq) for ireq in source_ireqs) else: + redacted_urls = tuple( + redact_auth_from_url(url) for url in self.finder.index_urls + ) lines.append("No versions found") lines.append( "{} {} reachable?".format( - "Were" if len(self.finder.index_urls) > 1 else "Was", - " or ".join(self.finder.index_urls), + "Were" if len(redacted_urls) > 1 else "Was", + " or ".join(redacted_urls), ) ) return "\n".join(lines) diff --git a/piptools/scripts/compile.py b/piptools/scripts/compile.py index 7bd6e0822..4a9482e25 100755 --- a/piptools/scripts/compile.py +++ b/piptools/scripts/compile.py @@ -9,6 +9,7 @@ from click.utils import safecall from pip._internal.commands import create_command from pip._internal.req.constructors import install_req_from_line +from pip._internal.utils.misc import redact_auth_from_url from .. import click from .._compat import parse_requirements @@ -27,6 +28,7 @@ # Get default values of the pip's options (including options from pip.conf). install_command = create_command("install") pip_defaults = install_command.parser.get_default_values() +default_index_url = redact_auth_from_url(pip_defaults.index_url) @click.command() @@ -63,7 +65,7 @@ @click.option( "-i", "--index-url", - help="Change index URL (defaults to {})".format(pip_defaults.index_url), + help="Change index URL (defaults to {})".format(default_index_url), envvar="PIP_INDEX_URL", ) @click.option( @@ -371,14 +373,14 @@ def cli( log.debug("Using indexes:") with log.indentation(): for index_url in dedup(repository.finder.index_urls): - log.debug(index_url) + log.debug(redact_auth_from_url(index_url)) if repository.finder.find_links: log.debug("") - log.debug("Configuration:") + log.debug("Using links:") with log.indentation(): for find_link in dedup(repository.finder.find_links): - log.debug("-f {}".format(find_link)) + log.debug(redact_auth_from_url(find_link)) try: resolver = Resolver( diff --git a/piptools/utils.py b/piptools/utils.py index 00d1ab1be..b0eca76a6 100644 --- a/piptools/utils.py +++ b/piptools/utils.py @@ -8,6 +8,8 @@ import six from click.utils import LazyFile from pip._internal.req.constructors import install_req_from_line +from pip._internal.utils.misc import redact_auth_from_url +from pip._internal.vcs import is_url from six.moves import shlex_quote from ._compat import PIP_VERSION @@ -365,6 +367,8 @@ def get_compile_command(click_ctx): left_args.append(shlex_quote(arg)) # Append to args the option with a value else: + if isinstance(val, six.string_types) and is_url(val): + val = redact_auth_from_url(val) if option.name == "pip_args": # shlex_quote would produce functional but noisily quoted results, # e.g. --pip-args='--cache-dir='"'"'/tmp/with spaces'"'"'' diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 454649c0d..3d785dbe0 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -107,7 +107,7 @@ def test_find_links_option(runner): out = runner.invoke(cli, ["-v", "-f", "./libs1", "-f", "./libs2"]) # Check that find-links has been passed to pip - assert "Configuration:\n -f ./libs1\n -f ./libs2\n -f ./libs3\n" in out.stderr + assert "Using links:\n ./libs1\n ./libs2\n ./libs3\n" in out.stderr # Check that find-links has been written to a requirements.txt with open("requirements.txt", "r") as req_txt: @@ -143,6 +143,30 @@ def test_extra_index_option(pip_with_index_conf, runner): ) +@pytest.mark.parametrize("option", ("--extra-index-url", "--find-links")) +def test_redacted_urls_in_verbose_output(runner, option): + """ + Test that URLs with sensitive data don't leak to the output. + """ + with open("requirements.in", "w"): + pass + + out = runner.invoke( + cli, + [ + "--no-header", + "--no-index", + "--no-emit-find-links", + "--verbose", + option, + "http://username:password@example.com", + ], + ) + + assert "http://username:****@example.com" in out.stderr + assert "password" not in out.stderr + + def test_trusted_host(pip_conf, runner): with open("requirements.in", "w"): pass @@ -658,21 +682,37 @@ def test_no_candidates_pre(pip_conf, runner): assert "Tried pre-versions:" in out.stderr -def test_default_index_url(pip_with_index_conf): +@pytest.mark.parametrize( + ("url", "expected_url"), + ( + pytest.param("https://example.com", "https://example.com", id="regular url"), + pytest.param( + "https://username:password@example.com", + "https://username:****@example.com", + id="url with credentials", + ), + ), +) +def test_default_index_url(make_pip_conf, url, expected_url): + """ + Test help's output with default index URL. + """ + make_pip_conf( + dedent( + """\ + [global] + index-url = {url} + """.format( + url=url + ) + ) + ) + status, output = invoke([sys.executable, "-m", "piptools", "compile", "--help"]) output = output.decode("utf-8") - # Click's subprocess output has \r\r\n line endings on win py27. Fix it. - output = output.replace("\r\r", "\r") - assert status == 0 - expected = ( - " -i, --index-url TEXT Change index URL (defaults to" - + os.linesep - + " http://example.com)" - + os.linesep - ) - assert expected in output + assert expected_url in output def test_stdin_without_output_file(runner): @@ -995,15 +1035,20 @@ def test_options_in_requirements_file(runner, options): ("cli_options", "expected_message"), ( pytest.param( - ["--index-url", "file:foo"], - "Was file:foo reachable?", + ["--index-url", "scheme://foo"], + "Was scheme://foo reachable?", id="single index url", ), pytest.param( - ["--index-url", "file:foo", "--extra-index-url", "file:bar"], - "Were file:foo or file:bar reachable?", + ["--index-url", "scheme://foo", "--extra-index-url", "scheme://bar"], + "Were scheme://foo or scheme://bar reachable?", id="multiple index urls", ), + pytest.param( + ["--index-url", "scheme://username:password@host"], + "Was scheme://username:****@host reachable?", + id="index url with credentials", + ), ), ) def test_unreachable_index_urls(runner, cli_options, expected_message): diff --git a/tests/test_utils.py b/tests/test_utils.py index 64492679f..6c0a37f38 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -274,6 +274,16 @@ def test_force_text(value, expected_text): ["--pip-args", "--disable-pip-version-check --isolated"], "pip-compile --pip-args='--disable-pip-version-check --isolated'", ), + pytest.param( + ["--extra-index-url", "https://username:password@example.com/"], + "pip-compile --extra-index-url='https://username:****@example.com/'", + id="redact password in index", + ), + pytest.param( + ["--find-links", "https://username:password@example.com/"], + "pip-compile --find-links='https://username:****@example.com/'", + id="redact password in link", + ), ), ) def test_get_compile_command(tmpdir_cwd, cli_args, expected_command): From 302d46bb77d4d2fe31e135801a05e89cf811106f Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Mon, 18 May 2020 11:52:18 +0700 Subject: [PATCH 2/2] Address review comments Move some logic to a function to not waste the global scope. Co-Authored-By: Andy Kluger --- piptools/scripts/compile.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/piptools/scripts/compile.py b/piptools/scripts/compile.py index 4a9482e25..f66901b11 100755 --- a/piptools/scripts/compile.py +++ b/piptools/scripts/compile.py @@ -25,10 +25,15 @@ DEFAULT_REQUIREMENTS_FILE = "requirements.in" DEFAULT_REQUIREMENTS_OUTPUT_FILE = "requirements.txt" -# Get default values of the pip's options (including options from pip.conf). -install_command = create_command("install") -pip_defaults = install_command.parser.get_default_values() -default_index_url = redact_auth_from_url(pip_defaults.index_url) + +def _get_default_option(option_name): + """ + Get default value of the pip's option (including option from pip.conf) + by a given option name. + """ + install_command = create_command("install") + default_values = install_command.parser.get_default_values() + return getattr(default_values, option_name) @click.command() @@ -65,7 +70,9 @@ @click.option( "-i", "--index-url", - help="Change index URL (defaults to {})".format(default_index_url), + help="Change index URL (defaults to {index_url})".format( + index_url=redact_auth_from_url(_get_default_option("index_url")) + ), envvar="PIP_INDEX_URL", ) @click.option(