Skip to content

Commit 6fdfa31

Browse files
committed
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.
1 parent 574aec2 commit 6fdfa31

2 files changed

Lines changed: 21 additions & 4 deletions

File tree

‎git/util.py‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -658,10 +658,14 @@ def remove_password_if_present(cmdline: Sequence[str]) -> List[str]:
658658
if url.password is None and url.username is None:
659659
continue
660660

661-
if url.password is not None:
662-
url = url._replace(netloc=url.netloc.replace(url.password, "*****"))
663-
if url.username is not None:
664-
url = url._replace(netloc=url.netloc.replace(url.username, "*****"))
661+
# Redact the userinfo as a whole rather than substituting the
662+
# username/password into the netloc: a substring replace also hits
663+
# the host ("git" in "github.com") and an empty username or password
664+
# matches at every position.
665+
_, at, hostinfo = url.netloc.rpartition("@")
666+
if at:
667+
redacted = "*****:*****" if url.password is not None else "*****"
668+
url = url._replace(netloc=f"{redacted}@{hostinfo}")
665669
new_cmdline[index] = urlunsplit(url)
666670
except ValueError:
667671
# This is not a valid URL.

‎test/test_util.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -828,6 +828,19 @@ def test_remove_password_from_command_line(self):
828828
assert authorization not in " ".join(redacted_cmd_6)
829829
assert "http.extraHeader=Authorization: *****" in redacted_cmd_6
830830

831+
def test_remove_password_keeps_host_intact(self):
832+
"""Redaction must not touch the host, even when it contains the username."""
833+
redacted = remove_password_if_present(["git", "clone", "https://git@github.com/user/repo.git"])
834+
assert redacted == ["git", "clone", "https://*****@github.com/user/repo.git"]
835+
836+
redacted = remove_password_if_present(["git", "clone", "ssh://git@github.com/u/r.git"])
837+
assert redacted == ["git", "clone", "ssh://*****@github.com/u/r.git"]
838+
839+
def test_remove_empty_password_keeps_host_intact(self):
840+
"""An empty password must not expand into every position of the netloc."""
841+
redacted = remove_password_if_present(["git", "clone", "https://:@fakerepo.example.com/testrepo"])
842+
assert redacted == ["git", "clone", "https://*****:*****@fakerepo.example.com/testrepo"]
843+
831844

832845
def test_mode_str_to_int_accepts_bytes():
833846
assert mode_str_to_int("100644") == 0o100644

0 commit comments

Comments
 (0)