Skip to content

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

Merged
cliffhall merged 2 commits into
v2/mainfrom
v2/fix/5508-git-remote-redact-query-fragment
Nov 12, 2026
Merged

cliffhall merged 2 commits into
v2/mainfrom
v2/fix/5508-git-remote-redact-query-fragment

Conversation

@cliffhall

@cliffhall cliffhall commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

Closes #5508

Description

git_remote promised that credentials embedded in a URL are shown as <redacted>, but redact_url only redacted the URL's userinfo. A token in a query parameter or the fragment came back as configured.

redact_url now also redacts, for every scheme:// URL (including the inner URL of a <transport>::<url> remote-helper address):

  • every query value, whatever the parameter's name (?private_token=<redacted>&ref=<redacted>). No list of credential-like names is complete, so none is kept; the names stay so the URL still reads as what it is. A parameter with no = (?<token>) is redacted whole.
  • a non-empty fragment (#<redacted>). The fragment starts at the first #, which also ends the query.

Two things are deliberately kept as configured, and the tool description and README now say so:

  • Path segments (https://host/token/SECRET/r.git): a token there cannot be told apart from a repository name, which is the boundary the issue allows for.
  • file:// URLs' ? and #, which are characters of a local file name, not a query or fragment. Their userinfo is still redacted as before.

A URL where a ? or # comes before the userinfo's @ is shown as scheme://<redacted>. The userinfo runs to the last @ before the first /, so in https://host?t=a@SECRET the ? might start a query holding an @, or might be in a password (https://user:p?ss@host.example, the edge case the reporter found while probing the prototype on the issue). No split of it is safe, so nothing after the scheme is shown (found in Copilot round 1).

Server Details

  • Server: git
  • Changed: git_remote output (redact_url), the tool's description (and its list_tools.json snapshot), src/git/README.md

Motivation and Context

See #5508. Reported by @reservoir-cmyk. Thanks also to @SalyyS1, whose prototype on the issue explored the same space; this PR is an independent implementation that redacts every query value rather than filtering by parameter name.

How Has This Been Tested?

  • Unit and protocol tests (src/git/tests/test_remote.py): 23 new redact_url cases (query values, bare and empty parameters, empty query and fragment, # before ?, userinfo + query + fragment, @ inside a query value, SSH, ?/# before the userinfo's @, with and without a path, a remote-helper URL, a path-segment token, file://), plus a test that drives git_remote through the MCP client with the issue's two examples. Full git suite: 365 passed; server.py coverage 100% lines, 99.4% branches. npm run local:gate exit 0.

  • Inspector CLI 2.12.0, both eras, before (origin/v2/main) and after (this branch). The repository has origin fetching https://git.example.com/g/r.git?private_token=SECRET1, pushing https://git.example.com/g/r.git#SECRET2, and pathtok at https://git.example.com/token/SECRET3/r.git:

    npx -y @modelcontextprotocol/inspector@2.12.0 --cli uv --directory <checkout>/src/git run mcp-server-git -- \
      --protocol-era <legacy|modern> --method tools/call --tool-name git_remote --tool-arg repo_path=<repo> --format json

    Before, legacy (2025-11-25) and modern (2026-07-28, _meta carries io.modelcontextprotocol/serverInfo), identical:

    origin	https://git.example.com/g/r.git?private_token=SECRET1 (fetch)
    origin	https://git.example.com/g/r.git#SECRET2 (push)
    pathtok	https://git.example.com/token/SECRET3/r.git (fetch)
    pathtok	https://git.example.com/token/SECRET3/r.git (push)
    

    After, legacy and modern, identical:

    origin	https://git.example.com/g/r.git?private_token=<redacted> (fetch)
    origin	https://git.example.com/g/r.git#<redacted> (push)
    pathtok	https://git.example.com/token/SECRET3/r.git (fetch)
    pathtok	https://git.example.com/token/SECRET3/r.git (push)
    
  • LLM client: Claude Code 2.1.322, headless, with only this branch's server configured (--strict-mcp-config). Prompt: "Use the git server's git_remote tool on the repository at <repo> and reply with exactly the text it returned, verbatim." It called git_remote and returned the "After" listing above. Claude Code does not show which protocol era it negotiated, so the run cannot be labelled with one.

Breaking Changes

None. No configuration changes; git_remote output redacts more.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Protocol Documentation
  • My changes follow MCP security best practices
  • I have updated the server's README accordingly
  • I have added a changeset (npm run changeset) if this changes what a TypeScript server publishes (not applicable: a Python server)
  • I have tested this with an LLM client
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling (not applicable: a pure string transform with no failure mode)
  • I have documented all environment variables and configuration options (not applicable: none added)

🤖 Generated with Claude Code

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 <redacted>" 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 <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall cliffhall added the v2 label Nov 12, 2026
@changeset-bot

changeset-bot Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 0bd4cf9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

git: MCP interface diff

Base: a9558c5f216b (Merge pull request #5531 from modelcontextprotocol/v2/docs/5483-docker-stale-comments)
Head: the build in this checkout, at 5ff47bfbde61 (Merge 0bd4cf9 into 9914133)

Diffed because it was named (--server).

⚠️ Interface changes detected in 1 part(s) of the interface. Review them below to confirm they are intended.

Tools Prompts Resources Resource templates
Base 14 2 0 0
Head 14 2 0 0

tools

--- base/tools.json
+++ target/tools.json

- tools[git_remote].description: "Lists the repository's remotes with their fetch and push URLs, like `git remote -v`. Credentials emb..."
+ tools[git_remote].description: "Lists the repository's remotes with their fetch and push URLs, like `git remote -v`. Credentials emb..."

Updated by this run for 0bd4cf90ad004fbcaaa1e2646e1f84e02a4cee13.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The URL redactor still leaks credential content in pathless URLs whose query or fragment contains @, requiring conservative redaction and regression tests for these cases.

1 open issue
What changed in this PR

This PR addresses #5508 by extending the Git server’s git_remote URL redaction beyond userinfo to query values and fragments.

Changes:

  • Redacts query values, bare query parameters, and non-empty fragments, including inside remote-helper URLs.
  • Preserves path segments and file:// filename characters, with documented exceptions.
  • Adds regression tests and updates the tool-description snapshot.
File Description
src/​git/​tests/​test_remote.py Adds URL edge cases and a protocol-level redaction test.
src/​git/​tests/​snapshots/​list_tools.json Updates the expected git_remote description.
src/​git/​src/​mcp_server_git/​server.py Implements query and fragment redaction and revises tool wording.
src/​git/​README.md Documents expanded redaction and its boundaries.

Review effort: Balanced

Comment thread src/git/src/mcp_server_git/server.py Resolved
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://<redacted>.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall

Copy link
Copy Markdown
Member Author

Copilot round 1: one finding, fixed in 0bd4cf9.

  • Pathless URLs can leak credentials containing @ in queries or fragments (server.py): a real defect in what this PR adds. The userinfo runs to the last @ before the first /, so in https://example.com?token=left@SECRET it swallowed the ? and the text after the @ was shown as the host. A URL whose userinfo would hold a ? or # is now shown as scheme://<redacted>, SSH included, with five regression cases. The PR body's description and test notes are updated to match. Gate exit 0, git suite 365 passed.

@github-actions

Copy link
Copy Markdown
Contributor

everything: MCP interface diff

Base: a9558c5f216b (Merge pull request #5531 from modelcontextprotocol/v2/docs/5483-docker-stale-comments)
Head: the build in this checkout, at 5ff47bfbde61 (Merge 0bd4cf9 into 9914133)

Diffed because it was named (--server).

✅ No interface changes detected.

Tools Prompts Resources Resource templates
Base 22 4 8 5
Head 22 4 8 5

Updated by this run for 0bd4cf90ad004fbcaaa1e2646e1f84e02a4cee13.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues remain for the change extending Git remote URL redaction to query values and fragments.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@cliffhall

Copy link
Copy Markdown
Member Author

Copilot round 2: clean (no open issues; the round-1 finding is shown as resolved). The review loop has converged.

@cliffhall
cliffhall merged commit 1f8badb into v2/main Nov 12, 2026
49 of 51 checks passed
@cliffhall
cliffhall deleted the v2/fix/5508-git-remote-redact-query-fragment branch November 12, 2026 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

git: git_remote redacts credentials only in the URL's userinfo, not in its query, fragment or path

2 participants