diff --git a/pkg/github/repository_resource_test.go b/pkg/github/repository_resource_test.go index 8ef31fda90..81088f826d 100644 --- a/pkg/github/repository_resource_test.go +++ b/pkg/github/repository_resource_test.go @@ -118,6 +118,49 @@ func TestRepositoryResourceContentsDecodesUTF8Path(t *testing.T) { require.Equal(t, "package x", result.Contents[0].Text) } +func TestRepositoryResourceContentsPercentInFileName(t *testing.T) { + tests := []struct { + name string + uri string + wantPath string + }{ + { + name: "literal percent sign", + uri: "repo://owner/repo/contents/docs/100%25.txt", + wantPath: "/owner/repo/HEAD/docs/100%.txt", + }, + { + name: "file name that looks like a percent escape", + uri: "repo://owner/repo/contents/%2541.txt", + wantPath: "/owner/repo/HEAD/%41.txt", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + base, _ := url.Parse("https://raw.example.com/") + var gotPath string + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetRawReposContentsByOwnerByRepoByPath: func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + w.Header().Set("Content-Type", "text/plain") + _, err := w.Write([]byte("file body")) + require.NoError(t, err) + }, + })) + rawClient, err := raw.NewClient(client, base) + require.NoError(t, err) + ctx := ContextWithDeps(context.Background(), BaseDeps{Client: client, RawClient: rawClient}) + handler := RepositoryResourceContentsHandler(repositoryResourceContentURITemplate) + + result, err := handler(ctx, &mcp.ReadResourceRequest{Params: &mcp.ReadResourceParams{URI: tc.uri}}) + require.NoError(t, err) + require.Equal(t, "file body", result.Contents[0].Text) + require.Equal(t, tc.wantPath, gotPath) + }) + } +} + func Test_repositoryResourceContents(t *testing.T) { base, _ := url.Parse("https://raw.example.com/") tests := []struct { diff --git a/pkg/raw/raw.go b/pkg/raw/raw.go index 3bf77b29e7..19766a0de2 100644 --- a/pkg/raw/raw.go +++ b/pkg/raw/raw.go @@ -92,7 +92,7 @@ func (c *Client) refURL(owner, repo, ref, path string) (string, error) { if err := rejectPathTraversal(owner, repo, ref, path); err != nil { return "", err } - return c.url.JoinPath(owner, repo, ref, path).String(), nil + return c.url.JoinPath(escapePathSegments(owner, repo, ref, path)...).String(), nil } func (c *Client) URLFromOpts(opts *ContentOpts, owner, repo, path string) (string, error) { @@ -110,7 +110,26 @@ func (c *Client) commitURL(owner, repo, sha, path string) (string, error) { if err := rejectPathTraversal(owner, repo, sha, path); err != nil { return "", err } - return c.url.JoinPath(owner, repo, sha, path).String(), nil + return c.url.JoinPath(escapePathSegments(owner, repo, sha, path)...).String(), nil +} + +// escapePathSegments percent-encodes every "/"-separated segment of the given +// components. The components are decoded text (for example a file path taken +// from a repo:// resource URI), but url.URL.JoinPath treats its arguments as +// already escaped: a literal "%41" would be decoded again (fetching "A"), and +// an invalid escape such as "100%.txt" makes JoinPath silently fall back to +// the base URL. Escaping each segment keeps the file name exactly as given +// while preserving "/" as the path separator. +func escapePathSegments(components ...string) []string { + escaped := make([]string, 0, len(components)) + for _, component := range components { + segments := strings.Split(component, "/") + for i, segment := range segments { + segments[i] = url.PathEscape(segment) + } + escaped = append(escaped, strings.Join(segments, "/")) + } + return escaped } type ContentOpts struct { diff --git a/pkg/raw/raw_test.go b/pkg/raw/raw_test.go index 4269ade1ea..68a2873bf3 100644 --- a/pkg/raw/raw_test.go +++ b/pkg/raw/raw_test.go @@ -285,10 +285,34 @@ func TestUrlFromOpts(t *testing.T) { wantErr: true, }, { - name: "benign percent-encoded filename allowed", + name: "literal percent-escape-like filename kept as typed", opts: nil, owner: "octocat", repo: "hello", path: "%2ehidden", - want: "https://raw.example.com/octocat/hello/HEAD/%2ehidden", + want: "https://raw.example.com/octocat/hello/HEAD/%252ehidden", + }, + { + name: "filename with literal percent sign", + opts: nil, + owner: "octocat", repo: "hello", path: "docs/100%.txt", + want: "https://raw.example.com/octocat/hello/HEAD/docs/100%25.txt", + }, + { + name: "filename that looks like a percent escape is not decoded", + opts: nil, + owner: "octocat", repo: "hello", path: "%41.txt", + want: "https://raw.example.com/octocat/hello/HEAD/%2541.txt", + }, + { + name: "percent sign in ref with sha path", + opts: &ContentOpts{SHA: "abc123"}, + owner: "octocat", repo: "hello", path: "50%off/file.txt", + want: "https://raw.example.com/octocat/hello/abc123/50%25off/file.txt", + }, + { + name: "space hash question mark and unicode are escaped", + opts: &ContentOpts{Ref: "refs/heads/feature/分支"}, + owner: "octocat", repo: "hello", path: "a b/#c?.md", + want: "https://raw.example.com/octocat/hello/refs/heads/feature/%E5%88%86%E6%94%AF/a%20b/%23c%3F.md", }, }