fix(util): redact URL credentials in config values on the command line

`remove_password_if_present` is applied to every command line that
GitPython logs or stores in `GitCommandError` and the other
`CommandError` subclasses. It redacted an argument only when the whole
argument was a URL with userinfo, or when it was an
`http.extraHeader=Authorization: ...` assignment. Two common forms were
missed:

- A URL that is the value of a configuration assignment, such as
  `-c http.proxy=http://user:pass@proxy:3128` or
  `-c remote.origin.url=https://user:token@host/repo`. The text before
  `://` contains `=`, so `urlsplit` finds no scheme and no userinfo, and
  the argument was kept verbatim.
- The `--config=<key>=<value>` form, which is what `transform_kwargs`
  produces for a `config=...` keyword argument. The leading `--config=`
  defeated both checks, so even
  `--config=http.extraHeader=Authorization: Bearer <token>` was printed
  in full.

For example, after
`repo.git.set_persistent_git_options(c="http.proxy=http://user:secret@proxy:3128")`
every failing command raised a `GitCommandError` whose message contained
`user:secret`, and the same text reached the log when command tracing is
on.

The URL redaction moves into a small `_redact_url_credentials` helper.
`remove_password_if_present` strips a leading `--config=` before the
extra header check, and when the whole argument is not a URL with
userinfo it tries the assignment after `--config=` (which covers
`url.<base>.insteadOf` keys that carry credentials) and then the value
after the first `=`. Only the userinfo is replaced, so the key and the
rest of the URL stay readable, and arguments without a URL are returned
unchanged. Arguments that were already redacted produce the same output
as before.

Adds `test_remove_password_from_config_values` to `test/test_util.py`.
Its five redaction cases fail before this change and pass after it, and
the existing redaction tests still pass. `ruff check`, `ruff format
--check` and `mypy` are clean.

Author: Keerthana-64

git/util.py

@@ -652,42 +652,60 @@ def expand_path(p: Union[None, PathLike], expand_vars: bool = True) -> Optional[
         return None
 
 
+def _redact_url_credentials(text: str) -> Optional[str]:
+    """Return `text` with the userinfo of a URL redacted, or ``None`` if it has none."""
+    try:
+        url = urlsplit(text)
+    except ValueError:
+        # This is not a valid URL.
+        return None
+    if url.password is None and url.username is None:
+        return None
+
+    # 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 "*****"
+    return urlunsplit(url._replace(netloc=f"{redacted}@{hostinfo}"))
+
+
 def remove_password_if_present(cmdline: Sequence[str]) -> List[str]:
     """Redact credentials in URLs and HTTP Authorization extra headers in a command line.
 
+    URLs are also redacted when they are the value of a configuration assignment, such
+    as ``http.proxy=<url>``, and assignments are recognized in both the ``-c <key>=<value>``
+    and the ``--config=<key>=<value>`` form.
+
     If nothing is found, this just returns the command line as-is.
 
     This should be used for every log line that print a command line, as well as
     exception messages.
     """
     new_cmdline = []
-    for index, to_parse in enumerate(cmdline):
-        new_cmdline.append(to_parse)
-        config_key, separator, header = to_parse.partition("=")
-        header_name, colon, _ = header.partition(":")
+    for to_parse in cmdline:
+        option = "--config=" if to_parse.startswith("--config=") else ""
+        assignment = to_parse[len(option) :]
+        config_key, separator, value = assignment.partition("=")
+        header_name, colon, _ = value.partition(":")
         if (
             separator
             and colon
             and config_key.lower().endswith(".extraheader")
             and header_name.strip().lower() == "authorization"
         ):
-            new_cmdline[index] = "%s%s%s%s *****" % (config_key, separator, header_name, colon)
-            continue
-        try:
-            url = urlsplit(to_parse)
-            # Remove password from the URL if present.
-            if url.password is None and url.username is None:
-                continue
-
-            # 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.
+            new_cmdline.append("%s%s%s%s%s *****" % (option, config_key, separator, header_name, colon))
             continue
+
+        redacted = _redact_url_credentials(to_parse)
+        if redacted is None and option:
+            redacted = _redact_url_credentials(assignment)
+            if redacted is not None:
+                redacted = option + redacted
+        if redacted is None and separator:
+            redacted = _redact_url_credentials(value)
+            if redacted is not None:
+                redacted = option + config_key + separator + redacted
+        new_cmdline.append(to_parse if redacted is None else redacted)
     return new_cmdline
 
 

test/test_util.py

@@ -856,6 +856,33 @@ def test_remove_empty_password_keeps_host_intact(self):
     def test_remove_password_preserves_url_components(self, url, expected):
         assert remove_password_if_present([url]) == [expected]
 
+    @ddt.data(
+        (
+            ["-c", "http.proxy=http://user:pass@proxy.example.com:3128"],
+            ["-c", "http.proxy=http://*****:*****@proxy.example.com:3128"],
+        ),
+        (
+            ["-c", "remote.origin.url=https://user:pass@example.com/repo"],
+            ["-c", "remote.origin.url=https://*****:*****@example.com/repo"],
+        ),
+        (
+            ["--config=http.proxy=http://user:pass@proxy.example.com:3128"],
+            ["--config=http.proxy=http://*****:*****@proxy.example.com:3128"],
+        ),
+        (
+            ["--config=url.https://user:pass@example.com/.insteadOf=https://example.com/"],
+            ["--config=url.https://*****:*****@example.com/.insteadOf=https://example.com/"],
+        ),
+        (
+            ["--config=http.extraHeader=Authorization: Bearer pass"],
+            ["--config=http.extraHeader=Authorization: *****"],
+        ),
+        (["-c", "user.name=pass"], ["-c", "user.name=pass"]),
+    )
+    @ddt.unpack
+    def test_remove_password_from_config_values(self, cmdline, expected):
+        assert remove_password_if_present(cmdline) == expected
+
 
 def test_mode_str_to_int_accepts_bytes():
     assert mode_str_to_int("100644") == 0o100644