Skip to content

Commit 99acea6

Browse files
authored
Fix subdomain isolation URL parsing (#1218)
* adding better response * adding check for subdomain isolation and return raw resp for better better llm response * adding subdomain for uploads too * remove unnecessary comments * better error message * fix linter
1 parent ea4d842 commit 99acea6

File tree

2 files changed

+49
-4
lines changed

2 files changed

+49
-4
lines changed

internal/ghmcp/server.go

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"os/signal"
1313
"strings"
1414
"syscall"
15+
"time"
1516

1617
"github.com/github/github-mcp-server/pkg/errors"
1718
"github.com/github/github-mcp-server/pkg/github"
@@ -363,11 +364,30 @@ func newGHESHost(hostname string) (apiHost, error) {
363364
return apiHost{}, fmt.Errorf("failed to parse GHES GraphQL URL: %w", err)
364365
}
365366

366-
uploadURL, err := url.Parse(fmt.Sprintf("%s://%s/api/uploads/", u.Scheme, u.Hostname()))
367+
// Check if subdomain isolation is enabled
368+
// See https://docs.github.com/en/enterprise-server@3.17/admin/configuring-settings/hardening-security-for-your-enterprise/enabling-subdomain-isolation#about-subdomain-isolation
369+
hasSubdomainIsolation := checkSubdomainIsolation(u.Scheme, u.Hostname())
370+
371+
var uploadURL *url.URL
372+
if hasSubdomainIsolation {
373+
// With subdomain isolation: https://uploads.hostname/
374+
uploadURL, err = url.Parse(fmt.Sprintf("%s://uploads.%s/", u.Scheme, u.Hostname()))
375+
} else {
376+
// Without subdomain isolation: https://hostname/api/uploads/
377+
uploadURL, err = url.Parse(fmt.Sprintf("%s://%s/api/uploads/", u.Scheme, u.Hostname()))
378+
}
367379
if err != nil {
368380
return apiHost{}, fmt.Errorf("failed to parse GHES Upload URL: %w", err)
369381
}
370-
rawURL, err := url.Parse(fmt.Sprintf("%s://%s/raw/", u.Scheme, u.Hostname()))
382+
383+
var rawURL *url.URL
384+
if hasSubdomainIsolation {
385+
// With subdomain isolation: https://raw.hostname/
386+
rawURL, err = url.Parse(fmt.Sprintf("%s://raw.%s/", u.Scheme, u.Hostname()))
387+
} else {
388+
// Without subdomain isolation: https://hostname/raw/
389+
rawURL, err = url.Parse(fmt.Sprintf("%s://%s/raw/", u.Scheme, u.Hostname()))
390+
}
371391
if err != nil {
372392
return apiHost{}, fmt.Errorf("failed to parse GHES Raw URL: %w", err)
373393
}
@@ -380,6 +400,29 @@ func newGHESHost(hostname string) (apiHost, error) {
380400
}, nil
381401
}
382402

403+
// checkSubdomainIsolation detects if GitHub Enterprise Server has subdomain isolation enabled
404+
// by attempting to ping the raw.<host>/_ping endpoint on the subdomain. The raw subdomain must always exist for subdomain isolation.
405+
func checkSubdomainIsolation(scheme, hostname string) bool {
406+
subdomainURL := fmt.Sprintf("%s://raw.%s/_ping", scheme, hostname)
407+
408+
client := &http.Client{
409+
Timeout: 5 * time.Second,
410+
// Don't follow redirects - we just want to check if the endpoint exists
411+
//nolint:revive // parameters are required by http.Client.CheckRedirect signature
412+
CheckRedirect: func(req *http.Request, via []*http.Request) error {
413+
return http.ErrUseLastResponse
414+
},
415+
}
416+
417+
resp, err := client.Get(subdomainURL)
418+
if err != nil {
419+
return false
420+
}
421+
defer resp.Body.Close()
422+
423+
return resp.StatusCode == http.StatusOK
424+
}
425+
383426
// Note that this does not handle ports yet, so development environments are out.
384427
func parseAPIHost(s string) (apiHost, error) {
385428
if s == "" {

pkg/github/repositories.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,8 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t
542542

543543
// If the path is (most likely) not to be a directory, we will
544544
// first try to get the raw content from the GitHub raw content API.
545+
546+
var rawAPIResponseCode int
545547
if path != "" && !strings.HasSuffix(path, "/") {
546548
// First, get file info from Contents API to retrieve SHA
547549
var fileSHA string
@@ -631,8 +633,8 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t
631633
return mcp.NewToolResultResource(fmt.Sprintf("successfully downloaded binary file (SHA: %s)", fileSHA), result), nil
632634
}
633635
return mcp.NewToolResultResource("successfully downloaded binary file", result), nil
634-
635636
}
637+
rawAPIResponseCode = resp.StatusCode
636638
}
637639

638640
if rawOpts.SHA != "" {
@@ -677,7 +679,7 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t
677679
if err != nil {
678680
return mcp.NewToolResultError(fmt.Sprintf("failed to marshal resolved refs: %s", err)), nil
679681
}
680-
return mcp.NewToolResultText(fmt.Sprintf("Path did not point to a file or directory, but resolved git ref to %s with possible path matches: %s", resolvedRefs, matchingFilesJSON)), nil
682+
return mcp.NewToolResultError(fmt.Sprintf("Resolved potential matches in the repository tree (resolved refs: %s, matching files: %s), but the raw content API returned an unexpected status code %d.", string(resolvedRefs), string(matchingFilesJSON), rawAPIResponseCode)), nil
681683
}
682684

683685
return mcp.NewToolResultError("Failed to get file contents. The path does not point to a file or directory, or the file does not exist in the repository."), nil

0 commit comments

Comments
 (0)