Skip to content

Commit c59e8c1

Browse files
lorenzozaneeIrynaKulakova
lorenzozanee
and
IrynaKulakova
authored
fix: encode repository resource URIs as UTF-8 (#3437)
Co-authored-by: Iryna Kulakova <52420926+IrynaKulakova@users.noreply.github.com>
1 parent f727c8b commit c59e8c1

3 files changed

Lines changed: 225 additions & 28 deletions

File tree

‎pkg/github/repositories_test.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,67 @@ func Test_GetFileContents(t *testing.T) {
582582
}
583583
}
584584

585+
func Test_GetFileContents_UsesUTF8ResourceURI(t *testing.T) {
586+
content := []byte("package example")
587+
sha := strings.Repeat("a", 40)
588+
result, _ := runRepositoryReadFixture(t, repositoryReadFixture{
589+
contents: &github.RepositoryContent{
590+
Name: new("Þfoo.go"),
591+
Path: new("test/Þfoo.go"),
592+
SHA: new(gitBlobSHA(content)),
593+
Type: new("file"),
594+
Content: new(string(content)),
595+
Size: new(len(content)),
596+
},
597+
}, map[string]any{
598+
"owner": "owner",
599+
"repo": "repo",
600+
"path": "test/Þfoo.go",
601+
"sha": sha,
602+
})
603+
604+
resource := getResourceResult(t, result)
605+
require.Equal(t, "repo://owner/repo/sha/"+sha+"/contents/test/%C3%9Efoo.go", resource.URI)
606+
}
607+
608+
func Test_GetFileContents_LargeFileUsesUTF8ResourceURI(t *testing.T) {
609+
sha := strings.Repeat("a", 40)
610+
result, _ := runRepositoryReadFixture(t, repositoryReadFixture{
611+
contents: &github.RepositoryContent{
612+
Name: new("Þlarge.bin"),
613+
Path: new("test/Þlarge.bin"),
614+
SHA: new(sha),
615+
Type: new("file"),
616+
Size: new(2 * 1024 * 1024),
617+
DownloadURL: new("https://raw.example.com/owner/repo/" + sha + "/test/%C3%9Elarge.bin"),
618+
},
619+
trees: map[string]*github.Tree{
620+
sha: {Entries: []*github.TreeEntry{{
621+
Path: new("test"),
622+
Mode: new("040000"),
623+
Type: new("tree"),
624+
SHA: new("b"),
625+
}}},
626+
"b": {Entries: []*github.TreeEntry{{
627+
Path: new("Þlarge.bin"),
628+
Mode: new("100644"),
629+
Type: new("blob"),
630+
SHA: new(sha),
631+
}}},
632+
},
633+
}, map[string]any{
634+
"owner": "owner",
635+
"repo": "repo",
636+
"path": "test/Þlarge.bin",
637+
"sha": sha,
638+
})
639+
640+
require.Len(t, result.Content, 2)
641+
resource, ok := result.Content[1].(*mcp.ResourceLink)
642+
require.True(t, ok)
643+
require.Equal(t, "repo://owner/repo/sha/"+sha+"/contents/test/%C3%9Elarge.bin", resource.URI)
644+
}
645+
585646
func Test_GetFileContents_SymlinkDisclosure(t *testing.T) {
586647
commitSHA := strings.Repeat("c", 40)
587648
args := func(path string) map[string]any {

‎pkg/github/repository_resource.go‎

Lines changed: 78 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"path/filepath"
1111
"strconv"
1212
"strings"
13+
"unicode/utf8"
1314

1415
"github.com/github/github-mcp-server/v2/pkg/inventory"
1516
"github.com/github/github-mcp-server/v2/pkg/octicons"
@@ -247,53 +248,102 @@ func RepositoryResourceContentsHandler(resourceURITemplate *uritemplate.Template
247248
}
248249
}
249250

250-
// expandRepoResourceURI builds a resource URI using the appropriate URI template
251-
// based on the provided parameters (sha, ref, or default).
251+
func escapeRepoResourceURIComponent(value string) (string, error) {
252+
if !utf8.ValidString(value) {
253+
return "", errors.New("invalid UTF-8 in repository resource URI")
254+
}
255+
256+
const hex = "0123456789ABCDEF"
257+
var escaped strings.Builder
258+
escaped.Grow(len(value))
259+
for i := range len(value) {
260+
b := value[i]
261+
if (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') ||
262+
(b >= '0' && b <= '9') || b == '-' || b == '.' || b == '_' || b == '~' {
263+
escaped.WriteByte(b)
264+
continue
265+
}
266+
escaped.WriteByte('%')
267+
escaped.WriteByte(hex[b>>4])
268+
escaped.WriteByte(hex[b&0x0f])
269+
}
270+
return escaped.String(), nil
271+
}
272+
252273
func expandRepoResourceURI(owner, repo, sha, ref string, pathParts []string) (string, error) {
253-
baseValues := uritemplate.Values{
254-
"owner": uritemplate.String(owner),
255-
"repo": uritemplate.String(repo),
256-
"path": uritemplate.List(pathParts...),
274+
owner, err := escapeRepoResourceURIComponent(owner)
275+
if err != nil {
276+
return "", err
277+
}
278+
repo, err = escapeRepoResourceURIComponent(repo)
279+
if err != nil {
280+
return "", err
281+
}
282+
283+
path := make([]string, len(pathParts))
284+
for i, part := range pathParts {
285+
path[i], err = escapeRepoResourceURIComponent(part)
286+
if err != nil {
287+
return "", err
288+
}
289+
}
290+
291+
base := "repo://" + owner + "/" + repo
292+
withPath := func(uri string) string {
293+
uri += "/contents"
294+
if len(path) > 0 {
295+
uri += "/" + strings.Join(path, "/")
296+
}
297+
return uri
257298
}
258299

259300
switch {
260301
case sha != "":
261-
baseValues["sha"] = uritemplate.String(sha)
262-
return repositoryResourceCommitContentURITemplate.Expand(baseValues)
302+
sha, err = escapeRepoResourceURIComponent(sha)
303+
if err != nil {
304+
return "", err
305+
}
306+
return withPath(base + "/sha/" + sha), nil
263307

264308
case ref != "":
265-
// Parse ref to determine which template to use
266309
switch {
267310
case strings.HasPrefix(ref, "refs/heads/"):
268-
branch := strings.TrimPrefix(ref, "refs/heads/")
269-
baseValues["branch"] = uritemplate.String(branch)
270-
return repositoryResourceBranchContentURITemplate.Expand(baseValues)
311+
branch, err := escapeRepoResourceURIComponent(strings.TrimPrefix(ref, "refs/heads/"))
312+
if err != nil {
313+
return "", err
314+
}
315+
return withPath(base + "/refs/heads/" + branch), nil
271316

272317
case strings.HasPrefix(ref, "refs/tags/"):
273-
tag := strings.TrimPrefix(ref, "refs/tags/")
274-
baseValues["tag"] = uritemplate.String(tag)
275-
return repositoryResourceTagContentURITemplate.Expand(baseValues)
318+
tag, err := escapeRepoResourceURIComponent(strings.TrimPrefix(ref, "refs/tags/"))
319+
if err != nil {
320+
return "", err
321+
}
322+
return withPath(base + "/refs/tags/" + tag), nil
276323

277324
case strings.HasPrefix(ref, "refs/pull/") && strings.HasSuffix(ref, "/head"):
278-
// Extract PR number from "refs/pull/{number}/head"
279-
prPart := strings.TrimPrefix(ref, "refs/pull/")
280-
prNumber := strings.TrimSuffix(prPart, "/head")
281-
baseValues["prNumber"] = uritemplate.String(prNumber)
282-
return repositoryResourcePrContentURITemplate.Expand(baseValues)
325+
prNumber, err := escapeRepoResourceURIComponent(strings.TrimSuffix(strings.TrimPrefix(ref, "refs/pull/"), "/head"))
326+
if err != nil {
327+
return "", err
328+
}
329+
return withPath(base + "/refs/pull/" + prNumber + "/head"), nil
283330

284331
case looksLikeSHA(ref):
285-
// ref is actually a SHA (e.g., from resolveGitReference)
286-
baseValues["sha"] = uritemplate.String(ref)
287-
return repositoryResourceCommitContentURITemplate.Expand(baseValues)
332+
sha, err := escapeRepoResourceURIComponent(ref)
333+
if err != nil {
334+
return "", err
335+
}
336+
return withPath(base + "/sha/" + sha), nil
288337

289338
default:
290-
// For other refs (like a branch name without refs/heads/ prefix),
291-
// treat it as a branch
292-
baseValues["branch"] = uritemplate.String(ref)
293-
return repositoryResourceBranchContentURITemplate.Expand(baseValues)
339+
branch, err := escapeRepoResourceURIComponent(ref)
340+
if err != nil {
341+
return "", err
342+
}
343+
return withPath(base + "/refs/heads/" + branch), nil
294344
}
295345

296346
default:
297-
return repositoryResourceContentURITemplate.Expand(baseValues)
347+
return withPath(base), nil
298348
}
299349
}

‎pkg/github/repository_resource_test.go‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,92 @@ const (
3232
resourceResponseTypeText
3333
)
3434

35+
func TestExpandRepoResourceURIUsesUTF8PercentEncoding(t *testing.T) {
36+
sha := strings.Repeat("a", 40)
37+
tests := []struct {
38+
name string
39+
sha string
40+
ref string
41+
path []string
42+
expected string
43+
}{
44+
{
45+
name: "sha path",
46+
sha: sha,
47+
path: []string{"test", "Þfoo.go"},
48+
expected: "repo://owner/repo/sha/" + sha +
49+
"/contents/test/%C3%9Efoo.go",
50+
},
51+
{
52+
name: "branch path",
53+
ref: "refs/heads/main",
54+
path: []string{"ملف.txt"},
55+
expected: "repo://owner/repo/refs/heads/main/contents/" +
56+
"%D9%85%D9%84%D9%81.txt",
57+
},
58+
{
59+
name: "unicode branch and path",
60+
ref: "refs/heads/分支",
61+
path: []string{"日本語 ファイル.md"},
62+
expected: "repo://owner/repo/refs/heads/%E5%88%86%E6%94%AF/contents/" +
63+
"%E6%97%A5%E6%9C%AC%E8%AA%9E%20%E3%83%95%E3%82%A1%E3%82%A4%E3%83%AB.md",
64+
},
65+
{
66+
name: "unicode tag",
67+
ref: "refs/tags/v1-Þ",
68+
path: []string{"README.md"},
69+
expected: "repo://owner/repo/refs/tags/v1-%C3%9E/contents/README.md",
70+
},
71+
{
72+
name: "pull request ref",
73+
ref: "refs/pull/42/head",
74+
path: []string{"𝄞.txt"},
75+
expected: "repo://owner/repo/refs/pull/42/head/contents/%F0%9D%84%9E.txt",
76+
},
77+
{
78+
name: "reserved path characters",
79+
path: []string{"a%b", "query?value"},
80+
expected: "repo://owner/repo/contents/a%25b/query%3Fvalue",
81+
},
82+
{
83+
name: "ascii path remains unchanged",
84+
path: []string{"docs", "README.md"},
85+
expected: "repo://owner/repo/contents/docs/README.md",
86+
},
87+
}
88+
89+
for _, tc := range tests {
90+
t.Run(tc.name, func(t *testing.T) {
91+
actual, err := expandRepoResourceURI("owner", "repo", tc.sha, tc.ref, tc.path)
92+
require.NoError(t, err)
93+
require.Equal(t, tc.expected, actual)
94+
})
95+
}
96+
}
97+
98+
func TestRepositoryResourceContentsDecodesUTF8Path(t *testing.T) {
99+
base, _ := url.Parse("https://raw.example.com/")
100+
sha := strings.Repeat("a", 40)
101+
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
102+
GetRawReposContentsByOwnerByRepoBySHAByPath: func(w http.ResponseWriter, r *http.Request) {
103+
require.True(t, strings.HasSuffix(r.URL.Path, "/test/Þfoo.go"))
104+
w.Header().Set("Content-Type", "text/plain")
105+
_, err := w.Write([]byte("package x"))
106+
require.NoError(t, err)
107+
},
108+
}))
109+
rawClient, err := raw.NewClient(client, base)
110+
require.NoError(t, err)
111+
ctx := ContextWithDeps(context.Background(), BaseDeps{Client: client, RawClient: rawClient})
112+
handler := RepositoryResourceContentsHandler(repositoryResourceCommitContentURITemplate)
113+
114+
result, err := handler(ctx, &mcp.ReadResourceRequest{Params: &mcp.ReadResourceParams{
115+
URI: "repo://owner/repo/sha/" + sha + "/contents/test/%C3%9Efoo.go",
116+
}})
117+
require.NoError(t, err)
118+
require.Equal(t, "package x", result.Contents[0].Text)
119+
}
120+
35121
func Test_repositoryResourceContents(t *testing.T) {
36122
base, _ := url.Parse("https://raw.example.com/")
37123
tests := []struct {

0 commit comments

Comments
 (0)