From 6fdfa315406170a0fbc4898123e0bfda67cb9070 Mon Sep 17 00:00:00 2001 From: Yunare Maia Date: Mon, 5 Oct 2026 02:14:59 +0000 Subject: [PATCH 1/2] fix(util): redact URL credentials without corrupting the host remove_password_if_present() redacted credentials by substituting them into url.netloc with str.replace(), which rewrites every occurrence anywhere in the netloc -- including the host. https://git@github.com/user/repo.git -> https://*****@*****hub.com/user/repo.git "git" is the most common Git username and a substring of "github.com", so this hits the common case. An empty username or password makes it worse: str.replace("", "*****") matches at every position, so https://:token@fakerepo.example.com/testrepo comes back shredded into a URL roughly ten times longer, which is what lands in GitCommandError messages when a fetch fails. Rebuild the netloc from its userinfo instead, so only the credentials are replaced. The existing test already covered the empty-password case but only asserted the password is absent, which any mangling satisfies. The function is the single redaction path used for logged command lines and for exception messages (git/cmd.py, git/exc.py, git/repo/base.py), so this fixes all of them at once. --- git/util.py | 12 ++++++++---- test/test_util.py | 13 +++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/git/util.py b/git/util.py index 017a94ad3..deebdc8d0 100644 --- a/git/util.py +++ b/git/util.py @@ -658,10 +658,14 @@ 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, "*****")) + # Redact the userinfo as a whole rather than substituting the + # username/password into the netloc: a substring replace also hits + # the host ("git" in "github.com") and an empty username or password + # matches at every position. + _, at, hostinfo = url.netloc.rpartition("@") + if at: + 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..5b72cb432 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -828,6 +828,19 @@ 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"] + def test_mode_str_to_int_accepts_bytes(): assert mode_str_to_int("100644") == 0o100644 From 0f89e3621943a34c49d702061308ac90320780f5 Mon Sep 17 00:00:00 2001 From: Byron Date: Mon, 5 Oct 2026 04:42:18 +0200 Subject: [PATCH 2/2] review - add some edge cases to the tests. Assisted-by: GPT 6.1 Sol Co-authored-by: GPT 6.1 Sol --- git/util.py | 13 +++++-------- test/test_util.py | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/git/util.py b/git/util.py index deebdc8d0..75677b7d3 100644 --- a/git/util.py +++ b/git/util.py @@ -658,14 +658,11 @@ def remove_password_if_present(cmdline: Sequence[str]) -> List[str]: if url.password is None and url.username is None: continue - # Redact the userinfo as a whole rather than substituting the - # username/password into the netloc: a substring replace also hits - # the host ("git" in "github.com") and an empty username or password - # matches at every position. - _, at, hostinfo = url.netloc.rpartition("@") - if at: - redacted = "*****:*****" if url.password is not None else "*****" - url = url._replace(netloc=f"{redacted}@{hostinfo}") + # 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 5b72cb432..ca8e006f3 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -841,6 +841,21 @@ def test_remove_empty_password_keeps_host_intact(self): 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