diff --git a/git/util.py b/git/util.py index 017a94ad3..75677b7d3 100644 --- a/git/util.py +++ b/git/util.py @@ -658,10 +658,11 @@ def remove_password_if_present(cmdline: Sequence[str]) -> List[str]: if url.password is None and url.username is None: continue - if url.password is not None: - url = url._replace(netloc=url.netloc.replace(url.password, "*****")) - if url.username is not None: - url = url._replace(netloc=url.netloc.replace(url.username, "*****")) + # Match urllib.parse's userinfo boundary. Keeping the raw hostinfo + # preserves hostname case, IPv6 brackets, and port formatting. + _, _, hostinfo = url.netloc.rpartition("@") + redacted = "*****:*****" if url.password is not None else "*****" + url = url._replace(netloc=f"{redacted}@{hostinfo}") new_cmdline[index] = urlunsplit(url) except ValueError: # This is not a valid URL. diff --git a/test/test_util.py b/test/test_util.py index eb520aac6..ca8e006f3 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -828,6 +828,34 @@ def test_remove_password_from_command_line(self): assert authorization not in " ".join(redacted_cmd_6) assert "http.extraHeader=Authorization: *****" in redacted_cmd_6 + def test_remove_password_keeps_host_intact(self): + """Redaction must not touch the host, even when it contains the username.""" + redacted = remove_password_if_present(["git", "clone", "https://git@github.com/user/repo.git"]) + assert redacted == ["git", "clone", "https://*****@github.com/user/repo.git"] + + redacted = remove_password_if_present(["git", "clone", "ssh://git@github.com/u/r.git"]) + assert redacted == ["git", "clone", "ssh://*****@github.com/u/r.git"] + + def test_remove_empty_password_keeps_host_intact(self): + """An empty password must not expand into every position of the netloc.""" + redacted = remove_password_if_present(["git", "clone", "https://:@fakerepo.example.com/testrepo"]) + assert redacted == ["git", "clone", "https://*****:*****@fakerepo.example.com/testrepo"] + + @ddt.data( + ( + "https://user%40example.com:p%40ss@GitHub.COM:00443/repo@name?q=a@b#c@d", + "https://*****:*****@GitHub.COM:00443/repo@name?q=a@b#c@d", + ), + ("//user:pass@[2001:db8::1]:0080/repo", "//*****:*****@[2001:db8::1]:0080/repo"), + ("https://user:p@ss@example.com/repo", "https://*****:*****@example.com/repo"), + ("https://user:@example.com/repo", "https://*****:*****@example.com/repo"), + ("https://@example.com/repo", "https://*****@example.com/repo"), + ("https://example.com/repo@name?q=a@b#c@d", "https://example.com/repo@name?q=a@b#c@d"), + ) + @ddt.unpack + def test_remove_password_preserves_url_components(self, url, expected): + assert remove_password_if_present([url]) == [expected] + def test_mode_str_to_int_accepts_bytes(): assert mode_str_to_int("100644") == 0o100644