Skip to content

Commit 56d11f5

Browse files
authored
Escape MCP client metadata and preserve revision provenance
Encode upstream client comments as valid HTTP header data, keep explicit build revisions independent of embedded dirty state, and reject null source hashes. Cover protocol wire behaviour and document the version-source contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6dde41da-ec25-4370-b26c-b036d0a53b3c
1 parent e99ab3c commit 56d11f5

5 files changed

Lines changed: 78 additions & 5 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,9 +405,9 @@ For a complete overview of all installation options, see our **[Installation Gui
405405
If you don't have Docker, you can use `go build` to build the binary in the
406406
`cmd/github-mcp-server` directory, and use the `github-mcp-server stdio` command with the `GITHUB_PERSONAL_ACCESS_TOKEN` environment variable set to your token. To specify the output location of the build, use the `-o` flag. You should configure your server to use the built executable as its `command`.
407407

408-
STDIO API requests identify the server as `github-mcp-server/<version>` and retain the upstream MCP client's name/version in parentheses when available. Release builds keep their release version. Source and default Docker builds use `vcs-<full-commit-sha>` (with `-dirty` when embedded VCS metadata records modified source), rather than the placeholder `version` or `dev`. This is a VCS build identifier, not a release number.
408+
STDIO API requests identify the server as `github-mcp-server/<version>` and retain the upstream MCP client's name/version in parentheses when available. Control characters, quotes, backslashes and parentheses in client metadata are escaped so the HTTP header remains valid; ordinary names, versions, spaces and printable Unicode are preserved. Release builds keep their release version. Source and default Docker builds use `vcs-<full-commit-sha>`, rather than the placeholder `version` or `dev`. The `-dirty` marker applies only when both the revision and modified state come from embedded VCS metadata, never to an explicitly supplied `main.commit`. This is a VCS build identifier, not a release number.
409409

410-
Build the complete package with `go build -o github-mcp-server ./cmd/github-mcp-server` from a Git checkout to embed its VCS revision. For builds without VCS metadata, supply the actual release with `-ldflags '-X main.version=<release>'` or the full source revision with `-ldflags '-X main.commit=<sha>'`. STDIO startup reports an error if neither a real release nor a valid revision is available.
410+
Build the complete package with `go build -o github-mcp-server ./cmd/github-mcp-server` from a Git checkout to embed its VCS revision. For builds without VCS metadata, supply the actual release with `-ldflags '-X main.version=<release>'` or the full source revision with `-ldflags '-X main.commit=<sha>'`. Explicit source revisions remain authoritative even if build-context filtering changes embedded VCS metadata. STDIO startup reports an error if neither a real release nor a valid, nonzero revision is available.
411411

412412
For example:
413413

‎cmd/github-mcp-server/version.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ func resolveServerVersion(release, revision string, info *debug.BuildInfo) (stri
4141
}
4242
}
4343
}
44-
if revision == "" || revision == "commit" {
44+
revisionFromVCS := revision == "" || revision == "commit"
45+
if revisionFromVCS {
4546
revision = vcsRevision
4647
if revision == "" && info != nil && !isPlaceholder(info.Main.Version) {
4748
return validateRelease(info.Main.Version)
@@ -56,8 +57,11 @@ func resolveServerVersion(release, revision string, info *debug.BuildInfo) (stri
5657
if _, err := hex.DecodeString(revision); err != nil {
5758
return "", fmt.Errorf("invalid server build revision: %w", err)
5859
}
60+
if strings.Trim(revision, "0") == "" {
61+
return "", fmt.Errorf("invalid server build revision: all-zero hashes do not identify source")
62+
}
5963
resolved := "vcs-" + strings.ToLower(revision)
60-
if dirty {
64+
if revisionFromVCS && dirty {
6165
resolved += "-dirty"
6266
}
6367
return resolved, nil

‎cmd/github-mcp-server/version_test.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ func TestResolveServerVersion(t *testing.T) {
4646
},
4747
{name: "dirty source", release: "version", revision: "commit", info: dirty, want: "vcs-" + sha + "-dirty"},
4848
{name: "Docker revision", release: "dev", revision: sha, want: "vcs-" + sha},
49+
{name: "Docker revision ignores context dirty state", release: "dev", revision: sha, info: dirty, want: "vcs-" + sha},
50+
{name: "linked revision ignores unrelated dirty state", release: "dev", revision: otherSHA, info: dirty, want: "vcs-" + otherSHA},
4951
{name: "linked revision precedence", release: "dev", revision: otherSHA, info: source, want: "vcs-" + otherSHA},
5052
{name: "SHA256", release: "dev", revision: strings.Repeat("ab", 32), want: "vcs-" + strings.Repeat("ab", 32)},
5153
{name: "canonical hex", release: "dev", revision: strings.ToUpper(sha), want: "vcs-" + sha},
@@ -73,3 +75,31 @@ func TestResolveServerVersion(t *testing.T) {
7375
})
7476
}
7577
}
78+
79+
func TestResolveServerVersionRejectsNullRevisions(t *testing.T) {
80+
t.Parallel()
81+
for _, revision := range []string{strings.Repeat("0", 40), strings.Repeat("0", 64)} {
82+
for _, modified := range []string{"false", "true"} {
83+
for _, linked := range []bool{false, true} {
84+
name := revision + "/modified=" + modified
85+
if linked {
86+
name += "/linked"
87+
}
88+
t.Run(name, func(t *testing.T) {
89+
info := &debug.BuildInfo{Settings: []debug.BuildSetting{
90+
{Key: "vcs.revision", Value: revision},
91+
{Key: "vcs.modified", Value: modified},
92+
}}
93+
commit := "commit"
94+
if linked {
95+
commit = revision
96+
info.Settings[0].Value = strings.Repeat("ab", 20)
97+
}
98+
got, err := resolveServerVersion("dev", commit, info)
99+
require.ErrorContains(t, err, "invalid server build revision")
100+
assert.Empty(t, got)
101+
})
102+
}
103+
}
104+
}
105+
}

‎internal/ghmcp/server.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"net/http"
99
"os"
1010
"os/signal"
11+
"strconv"
1112
"strings"
1213
"syscall"
1314
"time"
@@ -443,7 +444,10 @@ func createFeatureChecker(enabledFeatures []string, insidersMode bool) inventory
443444
func stdioUserAgent(cfg github.MCPServerConfig, client *mcp.Implementation) string {
444445
agent := fmt.Sprintf("github-mcp-server/%s", cfg.Version)
445446
if client != nil {
446-
agent += fmt.Sprintf(" (%s/%s)", client.Name, client.Version)
447+
comment := strconv.Quote(client.Name + "/" + client.Version)
448+
comment = comment[1 : len(comment)-1]
449+
comment = strings.NewReplacer("(", `\(`, ")", `\)`).Replace(comment)
450+
agent += " (" + comment + ")"
447451
}
448452
if cfg.InsidersMode {
449453
agent += " (insiders)"

‎internal/ghmcp/server_test.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,41 @@ func TestStdioGraphQLUserAgent(t *testing.T) {
8888
clientInfo: &mcp.Implementation{Name: "test-client", Version: "4.5.6"},
8989
want: "github-mcp-server/v1.2.4 (test-client/4.5.6)",
9090
},
91+
{
92+
name: "HTTP comment delimiters", version: "v1.2.3",
93+
clientInfo: &mcp.Implementation{Name: `Editor (Preview) \client`, Version: `v(1)\build`},
94+
want: `github-mcp-server/v1.2.3 (Editor \(Preview\) \\client/v\(1\)\\build)`,
95+
},
96+
{
97+
name: "modern control characters", version: "v1.2.3",
98+
clientInfo: &mcp.Implementation{Name: "editor(\x00)\\client\r\nX-Fake:1", Version: "1.0\tbeta\x7f"},
99+
want: `github-mcp-server/v1.2.3 (editor\(\x00\)\\client\r\nX-Fake:1/1.0\tbeta\x7f)`,
100+
},
101+
{
102+
name: "SDK control characters", handshake: "sdk", version: "v1.2.3", insiders: true,
103+
clientInfo: &mcp.Implementation{Name: "editor(\x00)\\client\r\nX-Fake:1", Version: "1.0\tbeta\x7f"},
104+
want: `github-mcp-server/v1.2.3 (editor\(\x00\)\\client\r\nX-Fake:1/1.0\tbeta\x7f) (insiders)`,
105+
},
106+
{
107+
name: "legacy control characters", handshake: "legacy", version: "v1.2.3",
108+
clientInfo: &mcp.Implementation{Name: "editor(\x00)\\client\r\nX-Fake:1", Version: "1.0\tbeta\x7f"},
109+
want: `github-mcp-server/v1.2.3 (editor\(\x00\)\\client\r\nX-Fake:1/1.0\tbeta\x7f)`,
110+
},
111+
{
112+
name: "spaces and Unicode preserved", version: "v1.2.3",
113+
clientInfo: &mcp.Implementation{Name: "VS Code \u03b2", Version: "\u03b1 1.0"},
114+
want: "github-mcp-server/v1.2.3 (VS Code \u03b2/\u03b1 1.0)",
115+
},
116+
{
117+
name: "empty client metadata", version: "v1.2.3",
118+
clientInfo: &mcp.Implementation{},
119+
want: "github-mcp-server/v1.2.3 (/)",
120+
},
121+
{
122+
name: "quoted client metadata", version: "v1.2.3",
123+
clientInfo: &mcp.Implementation{Name: `"Editor"`, Version: `v"1`},
124+
want: `github-mcp-server/v1.2.3 (\"Editor\"/v\"1)`,
125+
},
91126
}
92127

93128
for _, tt := range tests {

0 commit comments

Comments
 (0)