fix(git): redact query values and the fragment in git_remote URLs

src/git/README.md

@@ -113,7 +113,7 @@ Please note that mcp-server-git is currently in early development. The functiona
    - Inputs:
      - `repo_path` (string): Path to the Git repository.
    - Returns: One line per remote and direction, `<name>\t<url> (fetch)` or `(push)` (a partial clone's fetch line ends with its filter, such as ` [blob:none]`), or `No remotes configured`
-   - A credential embedded in a URL is never returned: the whole userinfo of a URL such as `https://user:token@host/...` or `https://token@host/...` is shown as `<redacted>`. An SSH username (`ssh://git@host/...`) is kept, since it names an account rather than authenticating one; a password after it is redacted. A remote-helper address (`https::https://user:token@host/...`) has its inner URL redacted the same way. scp-style addresses (`git@host:path`) and local paths are shown as configured. A line that cannot be parsed is shown as `<redacted>`, and if a configured remote URL (or `insteadOf` rewrite) contains a newline, which git itself refuses, the tool returns an error instead of a listing.
+   - A credential embedded in a URL is never returned: the whole userinfo of a URL such as `https://user:token@host/...` or `https://token@host/...` is shown as `<redacted>`, and so is every query value (`?private_token=<redacted>`, keeping the parameter names) and a non-empty fragment (`#<redacted>`). An SSH username (`ssh://git@host/...`) is kept, since it names an account rather than authenticating one; a password after it is redacted. A remote-helper address (`https::https://user:token@host/...`) has its inner URL redacted the same way. A URL where a `?` or `#` comes before the userinfo's `@` (`https://host?t=a@b`) cannot be split safely, so it is shown as `https://<redacted>`. A token carried as a path segment (`https://host/token/SECRET/r.git`) is not redacted, since it cannot be told apart from a repository name. scp-style addresses (`git@host:path`) and local paths are shown as configured, and a `file://` URL keeps its `?` and `#` parts, which belong to a local file name. A line that cannot be parsed is shown as `<redacted>`, and if a configured remote URL (or `insteadOf` rewrite) contains a newline, which git itself refuses, the tool returns an error instead of a listing.
 
 ### Protocol versions
 

src/git/src/mcp_server_git/server.py

@@ -897,6 +897,10 @@ def git_branch(
     return branch_info
 
 
+# A URL with an authority part, `scheme://...`; anything else (an scp-like
+# address, a local path) is not a URL and carries no userinfo or query.
+_URL_SCHEME = re.compile(r"^(?P<scheme>[A-Za-z][A-Za-z0-9+.-]*)://")
+
 # A URL with an authority part: `scheme://[userinfo@]host...`. The userinfo is
 # everything up to the last '@' before the first '/', which is where git itself
 # stops reading it.
@@ -925,32 +929,75 @@ def git_branch(
 REDACTED = "<redacted>"
 
 
+def _redact_query_and_fragment(rest: str) -> str:
+    """`rest` (a URL after its userinfo) with query values and fragment redacted.
+
+    A credential can ride in a query parameter (`?private_token=...`) or the
+    fragment as easily as in the userinfo (#5508). Every query value is
+    redacted, not only those with credential-like names, since no list of names
+    is complete; the names stay, so the URL still reads as what it is. A
+    parameter with no `=` (`?<token>`) is redacted whole, and so is a non-empty
+    fragment. The fragment starts at the first '#', which also ends the query.
+    """
+    rest, hash_mark, fragment = rest.partition("#")
+    rest, question_mark, query = rest.partition("?")
+    if question_mark:
+        params = []
+        for param in query.split("&"):
+            name, equals, _ = param.partition("=")
+            if equals:
+                params.append(f"{name}={REDACTED}")
+            else:
+                params.append(REDACTED if param else param)
+        rest += "?" + "&".join(params)
+    if hash_mark:
+        rest += "#" + (REDACTED if fragment else "")
+    return rest
+
+
 def redact_url(url: str) -> str:
-    """`url` with any credential in its userinfo replaced by `<redacted>` (#1611).
+    """`url` with any credential in it replaced by `<redacted>` (#1611, #5508).
 
     A remote URL can carry a credential (`https://user:token@host/...`), and
     the tool's output goes to the model, so it must not echo one. Over HTTP(S)
     and every other scheme a token is as often the username as the password
     (`https://<token>@github.com/...`), so the whole userinfo is redacted. An
     SSH username names the account and authenticates nothing, so it stays,
-    with only a password after it redacted. A remote-helper address
-    (`<transport>::<url>`) has its inner URL redacted. An scp-like address
-    (`git@host:path`) or a local path has no userinfo to carry a password and
-    is returned as is.
+    with only a password after it redacted. Query values and the fragment are
+    redacted too (`_redact_query_and_fragment`), except in a `file://` URL,
+    where '?' and '#' are characters of a local file name. A path segment is
+    kept: a token there cannot be told apart from a repository name. A URL
+    whose userinfo would hold a '?' or '#' cannot be split safely and is
+    shown as `scheme://<redacted>`. A remote-helper address (`<transport>::<url>`) has its inner URL redacted.
+    An scp-like address (`git@host:path`) or a local path is not a URL and is
+    returned as is.
     """
     helper = _REMOTE_HELPER.match(url)
     if helper is not None:
         return url[: helper.end()] + redact_url(url[helper.end() :])
+    authority = _URL_SCHEME.match(url)
+    if authority is None:
+        return url
+    scheme = authority["scheme"]
     match = _URL_USERINFO.match(url)
     if match is None:
-        return url
-    scheme, userinfo = match["scheme"], match["userinfo"]
-    if scheme.lower() in _SSH_SCHEMES:
-        user, has_password, _ = userinfo.partition(":")
-        kept = f"{user}:{REDACTED}" if has_password else user
+        head, rest = url[: authority.end()], url[authority.end() :]
     else:
-        kept = REDACTED
-    return f"{scheme}://{kept}@{url[match.end() :]}"
+        userinfo = match["userinfo"]
+        if "?" in userinfo or "#" in userinfo:
+            # Ambiguous: the '?' or '#' may be in a password or may start a
+            # query or fragment holding an '@' (`https://host?t=a@SECRET`), so
+            # no split of it is safe. Only the scheme is shown.
+            return f"{scheme}://{REDACTED}"
+        if scheme.lower() in _SSH_SCHEMES:
+            user, has_password, _ = userinfo.partition(":")
+            kept = f"{user}:{REDACTED}" if has_password else user
+        else:
+            kept = REDACTED
+        head, rest = f"{scheme}://{kept}@", url[match.end() :]
+    if scheme.lower() == "file":
+        return head + rest
+    return head + _redact_query_and_fragment(rest)
 
 
 # The config values `git remote -v` builds its URLs from: each remote's
@@ -1474,8 +1521,9 @@ def branch(
         name=GitTools.REMOTE.value,
         description=(
             "Lists the repository's remotes with their fetch and push URLs, "
-            "like `git remote -v`. Credentials embedded in a URL are shown as "
-            "<redacted>"
+            "like `git remote -v`. Credentials embedded in a URL's userinfo, "
+            "query values or fragment are shown as <redacted>; path segments "
+            "are shown as configured"
         ),
         annotations=READ_ONLY,
         structured_output=False,

src/git/tests/snapshots/list_tools.json

@@ -441,7 +441,7 @@
     },
     {
       "name": "git_remote",
-      "description": "Lists the repository's remotes with their fetch and push URLs, like `git remote -v`. Credentials embedded in a URL are shown as <redacted>",
+      "description": "Lists the repository's remotes with their fetch and push URLs, like `git remote -v`. Credentials embedded in a URL's userinfo, query values or fragment are shown as <redacted>; path segments are shown as configured",
       "inputSchema": {
         "properties": {
           "repo_path": {

src/git/tests/test_remote.py

@@ -3,7 +3,7 @@
 The tool's output goes to the model, and a remote URL can carry a credential
 (`https://user:token@host/...`), so most of these pin `redact_url`'s rule: the
 whole userinfo is redacted except an SSH username, which names an account and
-authenticates nothing. The protocol tests drive the real server through
+authenticates nothing, and so are query values and the fragment (#5508). The protocol tests drive the real server through
 `conftest.connect`; the unit tests cover URL shapes git would not let a test
 configure as easily.
 """
@@ -117,6 +117,60 @@ async def test_not_a_repository(tmp_path: Path):
         ),
         ("ext::ssh://git:pw@example.com/r", "ext::ssh://git:<redacted>@example.com/r"),
         ("https::https://example.com/r.git", "https::https://example.com/r.git"),
+        # Query values and the fragment (#5508): every value, whatever its
+        # parameter's name, with the names kept; a bare parameter whole.
+        (
+            "https://git.example.com/g/r.git?private_token=SECRET",
+            "https://git.example.com/g/r.git?private_token=<redacted>",
+        ),
+        (
+            "https://example.com/r?a=1&token=SECRET&&flag",
+            "https://example.com/r?a=<redacted>&token=<redacted>&&<redacted>",
+        ),
+        ("https://example.com/r?SECRET", "https://example.com/r?<redacted>"),
+        ("https://example.com/r?", "https://example.com/r?"),
+        (
+            "https://git.example.com/g/r.git#SECRET",
+            "https://git.example.com/g/r.git#<redacted>",
+        ),
+        ("https://example.com/r#", "https://example.com/r#"),
+        # The fragment starts at the first '#', which also ends the query.
+        ("https://example.com/r#a?b=c", "https://example.com/r#<redacted>"),
+        (
+            "https://example.com/r?a=b#c",
+            "https://example.com/r?a=<redacted>#<redacted>",
+        ),
+        # Userinfo, query and fragment together; an '@' inside a query value.
+        (
+            "https://u:p@example.com/r?t=x#y",
+            "https://<redacted>@example.com/r?t=<redacted>#<redacted>",
+        ),
+        ("https://example.com/r?e=a@b.com", "https://example.com/r?e=<redacted>"),
+        ("ssh://git@example.com/r?t=x", "ssh://git@example.com/r?t=<redacted>"),
+        # A '?' or '#' before the userinfo's '@' may be in a password or may
+        # start a query or fragment holding an '@': nothing after the scheme
+        # can be shown safely.
+        ("https://user:p?ss@example.com", "https://<redacted>"),
+        ("https://user:p#ss@example.com", "https://<redacted>"),
+        ("https://example.com?token=left@SECRET", "https://<redacted>"),
+        ("https://example.com#left@SECRET", "https://<redacted>"),
+        ("ssh://example.com?token=left@SECRET", "ssh://<redacted>"),
+        ("https://example.com?t=a@SECRET/x", "https://<redacted>"),
+        ("https://u:p?ss@example.com/r", "https://<redacted>"),
+        ("https://example.com?t=SECRET", "https://example.com?t=<redacted>"),
+        # A remote-helper address: the inner URL's query is redacted.
+        (
+            "https::https://example.com/r?t=SECRET",
+            "https::https://example.com/r?t=<redacted>",
+        ),
+        # A path segment cannot be told apart from a repository name: kept.
+        (
+            "https://git.example.com/token/SECRET/r.git",
+            "https://git.example.com/token/SECRET/r.git",
+        ),
+        # A file:// URL names a local file, whose name may hold '?' or '#'.
+        ("file:///srv/r?x#y.git", "file:///srv/r?x#y.git"),
+        ("file://u:p@host/srv/r?x", "file://<redacted>@host/srv/r?x"),
         # A password holding a line separator other than '\n'.
         ("https://u:s3cret\u2028tok@example.com/r", "https://<redacted>@example.com/r"),
     ],
@@ -194,3 +248,16 @@ async def test_remote_helper_url_is_redacted(repo: git.Repo):
         "origin\thttps::https://<redacted>@example.com/r.git (fetch)\n"
         "origin\thttps::https://<redacted>@example.com/r.git (push)"
     )
+
+
+async def test_query_and_fragment_credentials_are_redacted_over_the_wire(
+    repo: git.Repo,
+):
+    # The issue's examples (#5508): a token in a query value or the fragment.
+    repo.create_remote("origin", "https://git.example.com/g/r.git?private_token=S1")
+    repo.git.remote("set-url", "--push", "origin", "https://git.example.com/g/r.git#S2")
+    result = await call_remote(None, repo)
+    assert result == text_result(
+        "origin\thttps://git.example.com/g/r.git?private_token=<redacted> (fetch)\n"
+        "origin\thttps://git.example.com/g/r.git#<redacted> (push)"
+    )