fix(util): redact URL credentials without corrupting the host

[yunaremaia]

Summary

remove_password_if_present() redacts credentials by substituting the username/password into url.netloc with str.replace(). str.replace() rewrites every occurrence anywhere in the string, and the netloc contains the host and port as well as the userinfo — so the host gets corrupted too.

The username git is the most common Git username and is a substring of github.com:

in : https://git@github.com/user/repo.git
out: https://*****@*****hub.com/user/repo.git

An empty username or password makes it considerably worse: urlsplit returns '' (not None) for https://:token@host, and str.replace('', '*****') matches at every position:

in : https://:fakepassword1234@fakerepo.example.com/testrepo
out: https://*****:******...***@*****f*****a*****k*****e*****r*****e*****p*****o...

That second case is the reason this is more than cosmetic: it inflates the message ~10x and makes the remote unidentifiable, at exactly the moment a user is reading a GitCommandError to work out which remote failed.

This 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 the fix is one guard in the shared function rather than a patch per caller.

The fix

Rebuild the netloc from its userinfo rather than substituting into it:

-            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}")

urlsplit() already returns username/password as non-None only when the
netloc carries an @, so by this point in the loop a userinfo boundary is
guaranteed and the rpartition separator never needs a separate guard.

Output is byte-identical to the current behaviour for every non-overlapping input, including all three inputs in the existing test and port/case preservation.

Why the existing test did not catch it

test_remove_password_from_command_line already contains the empty-password case as cmd_3, and it passes today — because it only asserts password not in " ".join(...), which any mangling satisfies. The two new tests assert the full redacted command line, which is what pins the host in place.

Testing

Both new tests fail on main and pass with this change. The pre-existing test_remove_password_from_command_line passes both before and after, so there is no behaviour change for the cases it already covers. ruff check and ruff format --check are clean.

The suite was not run locally (this was prepared on a memory-constrained machine where pip install of the test dependencies is not possible), so the new tests have not been executed against the real package — CI is the verification. What was run locally is remove_password_if_present's body copied verbatim out of this branch with the assertion table below, executed as a standalone script under the stdlib only:

• the six ddt cases in test_remove_password_preserves_url_components
• the reported https://git@github.com/user/repo.git and ssh://git@github.com/u/r.git cases
• the three pre-existing redaction inputs, confirming the host survives unchanged
• the http.*.extraheader=Authorization: input, confirming the header branch is untouched
• the pre-fix code against the two reported inputs, confirming the mangling is real and not a hypothetical

All pass, and the pre-fix code reproduces both reported outputs (https://*****@*****hub.com/... and the shredded host) exactly as described above.

[Byron]

Thanks a lot, good catch!