From 1ffca31b84fb454c8bd2988f7c18cc688578af49 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 11 Nov 2026 23:25:21 -0500 Subject: [PATCH 1/2] fix(git): redact query values and the fragment in git_remote URLs redact_url only redacted a URL's userinfo, so a token in a query parameter (?private_token=...) or the fragment was returned as is, short of the tool's "credentials are shown as " contract. Every query value and a non-empty fragment are now redacted too, for any scheme:// URL but file://, whose ? and # belong to a file name. A token in a path segment cannot be told from a repository name, so the tool description and README now say path segments are kept. Closes #5508 Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- src/git/README.md | 2 +- src/git/src/mcp_server_git/server.py | 70 ++++++++++++++++++++----- src/git/tests/snapshots/list_tools.json | 2 +- src/git/tests/test_remote.py | 62 +++++++++++++++++++++- 4 files changed, 119 insertions(+), 17 deletions(-) diff --git a/src/git/README.md b/src/git/README.md index fab7aa5527..c6809d4c66 100644 --- a/src/git/README.md +++ b/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, `\t (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 ``. 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 ``, 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 ``, and so is every query value (`?private_token=`, keeping the parameter names) and a non-empty fragment (`#`). 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 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 ``, 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 diff --git a/src/git/src/mcp_server_git/server.py b/src/git/src/mcp_server_git/server.py index cd1d874ece..71da3e9493 100644 --- a/src/git/src/mcp_server_git/server.py +++ b/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[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,69 @@ def git_branch( 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 `=` (`?`) 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 `` (#1611). + """`url` with any credential in it replaced by `` (#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://@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 - (`::`) 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 + remote-helper address (`::`) 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 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 +1515,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 " - "" + "like `git remote -v`. Credentials embedded in a URL's userinfo, " + "query values or fragment are shown as ; path segments " + "are shown as configured" ), annotations=READ_ONLY, structured_output=False, diff --git a/src/git/tests/snapshots/list_tools.json b/src/git/tests/snapshots/list_tools.json index 6ef36c2dcd..12953ca0f2 100644 --- a/src/git/tests/snapshots/list_tools.json +++ b/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 ", + "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 ; path segments are shown as configured", "inputSchema": { "properties": { "repo_path": { diff --git a/src/git/tests/test_remote.py b/src/git/tests/test_remote.py index 9d193d50f0..0cd7bea8f6 100644 --- a/src/git/tests/test_remote.py +++ b/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,53 @@ async def test_not_a_repository(tmp_path: Path): ), ("ext::ssh://git:pw@example.com/r", "ext::ssh://git:@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=", + ), + ( + "https://example.com/r?a=1&token=SECRET&&flag", + "https://example.com/r?a=&token=&&", + ), + ("https://example.com/r?SECRET", "https://example.com/r?"), + ("https://example.com/r?", "https://example.com/r?"), + ( + "https://git.example.com/g/r.git#SECRET", + "https://git.example.com/g/r.git#", + ), + ("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#"), + ( + "https://example.com/r?a=b#c", + "https://example.com/r?a=#", + ), + # Userinfo, query and fragment together; an '@' inside a query value. + ( + "https://u:p@example.com/r?t=x#y", + "https://@example.com/r?t=#", + ), + ("https://example.com/r?e=a@b.com", "https://example.com/r?e="), + ("ssh://git@example.com/r?t=x", "ssh://git@example.com/r?t="), + # No path: a '?' or '#' in the userinfo is still inside the userinfo. + ("https://user:p?ss@example.com", "https://@example.com"), + ("https://user:p#ss@example.com", "https://@example.com"), + ("https://example.com?t=SECRET", "https://example.com?t="), + # A remote-helper address: the inner URL's query is redacted. + ( + "https::https://example.com/r?t=SECRET", + "https::https://example.com/r?t=", + ), + # 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://@host/srv/r?x"), # A password holding a line separator other than '\n'. ("https://u:s3cret\u2028tok@example.com/r", "https://@example.com/r"), ], @@ -194,3 +241,16 @@ async def test_remote_helper_url_is_redacted(repo: git.Repo): "origin\thttps::https://@example.com/r.git (fetch)\n" "origin\thttps::https://@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= (fetch)\n" + "origin\thttps://git.example.com/g/r.git# (push)" + ) From 0bd4cf90ad004fbcaaa1e2646e1f84e02a4cee13 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 11 Nov 2026 23:42:39 -0500 Subject: [PATCH 2/2] fix(git): redact a URL whose userinfo would hold a '?' or '#' whole The userinfo runs to the last '@' before the first '/', so in a URL like https://host?t=a@SECRET it swallowed the query delimiter and the part of the credential after the '@' was shown as the host. A '?' or '#' there may be in a password or may start a query or fragment, so no split of it is safe: such a URL is now shown as scheme://. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- src/git/README.md | 2 +- src/git/src/mcp_server_git/server.py | 10 ++++++++-- src/git/tests/test_remote.py | 13 ++++++++++--- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/git/README.md b/src/git/README.md index c6809d4c66..5dcfe50686 100644 --- a/src/git/README.md +++ b/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, `\t (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 ``, and so is every query value (`?private_token=`, keeping the parameter names) and a non-empty fragment (`#`). 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 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 ``, 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 ``, and so is every query value (`?private_token=`, keeping the parameter names) and a non-empty fragment (`#`). 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://`. 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 ``, 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 diff --git a/src/git/src/mcp_server_git/server.py b/src/git/src/mcp_server_git/server.py index 71da3e9493..8e54e641a4 100644 --- a/src/git/src/mcp_server_git/server.py +++ b/src/git/src/mcp_server_git/server.py @@ -966,8 +966,9 @@ def redact_url(url: str) -> str: 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 - remote-helper address (`::`) has its inner URL redacted. + 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://`. A remote-helper address (`::`) 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. """ @@ -983,6 +984,11 @@ def redact_url(url: str) -> str: head, rest = url[: authority.end()], url[authority.end() :] else: 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 diff --git a/src/git/tests/test_remote.py b/src/git/tests/test_remote.py index 0cd7bea8f6..f887cfa56c 100644 --- a/src/git/tests/test_remote.py +++ b/src/git/tests/test_remote.py @@ -147,9 +147,16 @@ async def test_not_a_repository(tmp_path: Path): ), ("https://example.com/r?e=a@b.com", "https://example.com/r?e="), ("ssh://git@example.com/r?t=x", "ssh://git@example.com/r?t="), - # No path: a '?' or '#' in the userinfo is still inside the userinfo. - ("https://user:p?ss@example.com", "https://@example.com"), - ("https://user:p#ss@example.com", "https://@example.com"), + # 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://"), + ("https://user:p#ss@example.com", "https://"), + ("https://example.com?token=left@SECRET", "https://"), + ("https://example.com#left@SECRET", "https://"), + ("ssh://example.com?token=left@SECRET", "ssh://"), + ("https://example.com?t=a@SECRET/x", "https://"), + ("https://u:p?ss@example.com/r", "https://"), ("https://example.com?t=SECRET", "https://example.com?t="), # A remote-helper address: the inner URL's query is redacted. (