diff options
author | Michael Mercier <michael.mercier@ryax.tech> | 2021-03-16 10:00:51 +0100 |
---|---|---|
committer | Michael Mercier <michael.mercier@ryax.tech> | 2021-03-16 10:16:34 +0100 |
commit | ffddedf5467df993b7a42fbd15afacb901bca6d7 (patch) | |
tree | b43ffc85ba78261e9446e5a9446de82054d707dc | |
parent | 50cbafc690e5692a16148dbde9de680be70ddbd1 (diff) | |
download | gitpython-ffddedf5467df993b7a42fbd15afacb901bca6d7.tar.gz |
Use copy and not inplace remove password + working case test
-rw-r--r-- | git/cmd.py | 16 | ||||
-rw-r--r-- | git/util.py | 6 | ||||
-rw-r--r-- | test/test_repo.py | 5 | ||||
-rw-r--r-- | test/test_util.py | 7 |
4 files changed, 21 insertions, 13 deletions
@@ -405,7 +405,7 @@ class Git(LazyMixin): if status != 0: errstr = read_all_from_possibly_closed_stream(self.proc.stderr) log.debug('AutoInterrupt wait stderr: %r' % (errstr,)) - raise GitCommandError(self.args, status, errstr) + raise GitCommandError(remove_password_if_present(self.args), status, errstr) # END status handling return status # END auto interrupt @@ -638,7 +638,7 @@ class Git(LazyMixin): :param env: A dictionary of environment variables to be passed to `subprocess.Popen`. - + :param max_chunk_size: Maximum number of bytes in one chunk of data passed to the output_stream in one invocation of write() method. If the given number is not positive then @@ -706,7 +706,7 @@ class Git(LazyMixin): if is_win: cmd_not_found_exception = OSError if kill_after_timeout: - raise GitCommandError(command, '"kill_after_timeout" feature is not supported on Windows.') + raise GitCommandError(redacted_command, '"kill_after_timeout" feature is not supported on Windows.') else: if sys.version_info[0] > 2: cmd_not_found_exception = FileNotFoundError # NOQA # exists, flake8 unknown @UndefinedVariable @@ -721,7 +721,7 @@ class Git(LazyMixin): if istream: istream_ok = "<valid stream>" log.debug("Popen(%s, cwd=%s, universal_newlines=%s, shell=%s, istream=%s)", - command, cwd, universal_newlines, shell, istream_ok) + redacted_command, cwd, universal_newlines, shell, istream_ok) try: proc = Popen(command, env=env, @@ -737,7 +737,7 @@ class Git(LazyMixin): **subprocess_kwargs ) except cmd_not_found_exception as err: - raise GitCommandNotFound(command, err) from err + raise GitCommandNotFound(redacted_command, err) from err if as_process: return self.AutoInterrupt(proc, command) @@ -787,7 +787,7 @@ class Git(LazyMixin): watchdog.cancel() if kill_check.isSet(): stderr_value = ('Timeout: the command "%s" did not complete in %d ' - 'secs.' % (" ".join(command), kill_after_timeout)) + 'secs.' % (" ".join(redacted_command), kill_after_timeout)) if not universal_newlines: stderr_value = stderr_value.encode(defenc) # strip trailing "\n" @@ -811,7 +811,7 @@ class Git(LazyMixin): proc.stderr.close() if self.GIT_PYTHON_TRACE == 'full': - cmdstr = " ".join(command) + cmdstr = " ".join(redacted_command) def as_text(stdout_value): return not output_stream and safe_decode(stdout_value) or '<OUTPUT_STREAM>' @@ -827,7 +827,7 @@ class Git(LazyMixin): # END handle debug printing if with_exceptions and status != 0: - raise GitCommandError(command, status, stderr_value, stdout_value) + raise GitCommandError(redacted_command, status, stderr_value, stdout_value) if isinstance(stdout_value, bytes) and stdout_as_string: # could also be output_stream stdout_value = safe_decode(stdout_value) diff --git a/git/util.py b/git/util.py index 80985df4..907c6998 100644 --- a/git/util.py +++ b/git/util.py @@ -349,7 +349,9 @@ def remove_password_if_present(cmdline): This should be used for every log line that print a command line. """ + new_cmdline = [] for index, to_parse in enumerate(cmdline): + new_cmdline.append(to_parse) try: url = urlsplit(to_parse) # Remove password from the URL if present @@ -358,11 +360,11 @@ def remove_password_if_present(cmdline): edited_url = url._replace( netloc=url.netloc.replace(url.password, "*****")) - cmdline[index] = urlunsplit(edited_url) + new_cmdline[index] = urlunsplit(edited_url) except ValueError: # This is not a valid URL pass - return cmdline + return new_cmdline #} END utilities diff --git a/test/test_repo.py b/test/test_repo.py index ac4c6660..8dc17833 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -240,7 +240,6 @@ class TestRepo(TestBase): @with_rw_directory def test_leaking_password_in_clone_logs(self, rw_dir): - """Check that the password is not printed on the logs""" password = "fakepassword1234" try: Repo.clone_from( @@ -249,6 +248,10 @@ class TestRepo(TestBase): to_path=rw_dir) except GitCommandError as err: assert password not in str(err), "The error message '%s' should not contain the password" % err + # Working example from a blank private project + Repo.clone_from( + url="https://gitlab+deploy-token-392045:mLWhVus7bjLsy8xj8q2V@gitlab.com/mercierm/test_git_python", + to_path=rw_dir) @with_rw_repo('HEAD') def test_max_chunk_size(self, repo): diff --git a/test/test_util.py b/test/test_util.py index a4963264..ddc5f628 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -325,7 +325,6 @@ class TestUtils(TestBase): self.assertEqual(t1._name, t2._name) def test_remove_password_from_command_line(self): - """Check that the password is not printed on the logs""" password = "fakepassword1234" url_with_pass = "https://fakeuser:{}@fakerepo.example.com/testrepo".format(password) url_without_pass = "https://fakerepo.example.com/testrepo" @@ -334,6 +333,10 @@ class TestUtils(TestBase): cmd_2 = ["git", "clone", "-v", url_without_pass] cmd_3 = ["no", "url", "in", "this", "one"] - assert password not in remove_password_if_present(cmd_1) + redacted_cmd_1 = remove_password_if_present(cmd_1) + assert password not in " ".join(redacted_cmd_1) + # Check that we use a copy + assert cmd_1 is not redacted_cmd_1 + assert password in " ".join(cmd_1) assert cmd_2 == remove_password_if_present(cmd_2) assert cmd_3 == remove_password_if_present(cmd_3) |