From 51a443c81d82c58f0dfe8bca779799f82a504e93 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Fri, 14 Aug 2026 13:10:55 +0200 Subject: [PATCH 1/2] fix(security): enforce HTTPS for gh-host/GITHUB_HOST to prevent cleartext credentials GHES hosts accepted an http:// scheme, which was interpolated into every REST/GraphQL/upload/raw/authorization URL. Authenticated requests would then carry the bearer token/PAT over cleartext http, exposing it to network interception and replay. Add a central HTTPS check in parseAPIHost so no deployment can build authenticated URLs over http, mirroring the existing GHEC behaviour. Permit http only for loopback hosts (localhost, 127.0.0.1, ::1) so local development against a dev server still works. Closes github/copilot-mcp-core#1815 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 2 +- pkg/http/oauth/oauth_test.go | 9 +++++---- pkg/utils/api.go | 37 ++++++++++++++++++++++++++++++++++++ pkg/utils/api_test.go | 28 ++++++++++++++++++++++++++- 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 6585ab30f6..32f8eb82bc 100644 --- a/README.md +++ b/README.md @@ -247,7 +247,7 @@ To keep your GitHub PAT secure and reusable across different MCP hosts: The flag `--gh-host` and the environment variable `GITHUB_HOST` can be used to set the hostname for GitHub Enterprise Server or GitHub Enterprise Cloud with data residency. -- For GitHub Enterprise Server, prefix the hostname with the `https://` URI scheme, as it otherwise defaults to `http://`, which GitHub Enterprise Server does not support. +- For GitHub Enterprise Server, prefix the hostname with the `https://` URI scheme. HTTPS is required and enforced: non-HTTPS hosts are refused so that credentials are never sent over cleartext (the only exception is a loopback host such as `http://localhost` for local development). - For GitHub Enterprise Cloud with data residency, use `https://YOURSUBDOMAIN.ghe.com` as the hostname. ``` json diff --git a/pkg/http/oauth/oauth_test.go b/pkg/http/oauth/oauth_test.go index f39ef39b87..1c2aa5c7c1 100644 --- a/pkg/http/oauth/oauth_test.go +++ b/pkg/http/oauth/oauth_test.go @@ -691,10 +691,11 @@ func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { expectedStatusCode: http.StatusOK, }, { - name: "GHES with http scheme returns the correct authorization server URL", - host: "http://ghe.example.com", - expectedURL: "http://ghe.example.com/login/oauth", - expectedStatusCode: http.StatusOK, + name: "GHES with http scheme is rejected to avoid cleartext credentials", + host: "http://ghe.example.com", + expectedURL: "", + expectedError: true, + errorContains: "host must use https", }, { name: "custom authorization server in config takes precedence", diff --git a/pkg/utils/api.go b/pkg/utils/api.go index 95dfbd1d5b..090c1850a1 100644 --- a/pkg/utils/api.go +++ b/pkg/utils/api.go @@ -235,6 +235,13 @@ func parseAPIHost(s string) (APIHost, error) { return APIHost{}, fmt.Errorf("host must have a scheme (http or https): %s", s) } + // Enforce HTTPS centrally so no deployment (GHES in particular) can build + // authenticated REST/GraphQL/upload/raw URLs over cleartext http, which + // would leak the bearer token/PAT to anyone on the network. + if err := requireSecureScheme(u); err != nil { + return APIHost{}, err + } + switch classifyHost(u) { case HostTypeDotcom: return newDotcomHost() @@ -245,6 +252,36 @@ func parseAPIHost(s string) (APIHost, error) { } } +// requireSecureScheme rejects hosts that would carry credentials over cleartext. +// Every REST/GraphQL/upload/raw/authorization URL is derived from this host and +// used for authenticated requests, so an http scheme would expose the bearer +// token/PAT to network interception and replay. http is permitted only for +// loopback hosts so that local development against a dev server still works. +func requireSecureScheme(u *url.URL) error { + if u.Scheme == "https" { + return nil + } + if u.Scheme == "http" && isLoopbackHost(u.Hostname()) { + return nil + } + return fmt.Errorf( + "host must use https to avoid sending credentials over cleartext: %s (http is only permitted for loopback hosts such as localhost, 127.0.0.1, or ::1)", + u.Scheme+"://"+u.Hostname(), + ) +} + +// isLoopbackHost reports whether hostname is a loopback address. Only exact +// loopback names/addresses qualify, so credentials are never sent in cleartext +// to a remote host. +func isLoopbackHost(hostname string) bool { + switch strings.ToLower(hostname) { + case "localhost", "127.0.0.1", "::1": + return true + default: + return false + } +} + // HostType identifies which GitHub deployment a host refers to. Tools use this // to skip capabilities that only exist on some deployments. type HostType int diff --git a/pkg/utils/api_test.go b/pkg/utils/api_test.go index 40fcb8f26a..7aa762a9b1 100644 --- a/pkg/utils/api_test.go +++ b/pkg/utils/api_test.go @@ -13,6 +13,7 @@ func TestParseAPIHost(t *testing.T) { input string wantRestURL string wantErr bool + errContains string }{ { name: "empty string defaults to dotcom", @@ -59,13 +60,38 @@ func TestParseAPIHost(t *testing.T) { input: "github.com", wantErr: true, }, + { + name: "http GHES rejected to avoid cleartext credentials", + input: "http://ghes.example.com", + wantErr: true, + errContains: "host must use https", + }, + { + name: "http loopback allowed for local development", + input: "http://localhost", + wantRestURL: "http://localhost/api/v3/", + }, + { + name: "http 127.0.0.1 loopback allowed for local development", + input: "http://127.0.0.1", + wantRestURL: "http://127.0.0.1/api/v3/", + }, + { + name: "http remote host rejected", + input: "http://notgithub.com", + wantErr: true, + errContains: "host must use https", + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { host, err := parseAPIHost(tc.input) if tc.wantErr { - assert.Error(t, err) + require.Error(t, err) + if tc.errContains != "" { + assert.Contains(t, err.Error(), tc.errContains) + } return } require.NoError(t, err) From 1e692b6f0b778cd1b54ea3361facc75dc65bd289 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Fri, 14 Aug 2026 13:22:26 +0200 Subject: [PATCH 2/2] fix: preserve authority for loopback GHES hosts Address review: the loopback exception accepted http://localhost:3000 and http://[::1], but newGHESHost built URLs from u.Hostname(), which drops the port (silently retargeting the dev server to port 80) and strips IPv6 brackets (producing an unusable URL such as http://::1/api/v3/). Derive the base-host REST/GraphQL/upload/raw/authorization URLs from u.Host so the port and IPv6 brackets are preserved. Subdomain-isolation URLs keep using the bare hostname, since a label cannot be prepended to a host:port or an IP literal. Add tests for the ::1 case and for port preservation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/utils/api.go | 18 +++++++++++++----- pkg/utils/api_test.go | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/pkg/utils/api.go b/pkg/utils/api.go index 090c1850a1..4a8d14e4ef 100644 --- a/pkg/utils/api.go +++ b/pkg/utils/api.go @@ -145,12 +145,20 @@ func newGHESHost(hostname string) (APIHost, error) { return APIHost{}, fmt.Errorf("failed to parse GHES URL: %w", err) } - restURL, err := url.Parse(fmt.Sprintf("%s://%s/api/v3/", u.Scheme, u.Hostname())) + // Preserve the full authority (host, port, and IPv6 brackets) for the + // base-host URLs. u.Hostname() drops the port and strips IPv6 brackets, + // which would silently retarget a loopback dev server to port 80 and produce + // an unusable URL for [::1]. The subdomain-isolation URLs below still derive + // from the bare hostname, since a label cannot be prepended to a host:port or + // an IP literal. + authority := u.Host + + restURL, err := url.Parse(fmt.Sprintf("%s://%s/api/v3/", u.Scheme, authority)) if err != nil { return APIHost{}, fmt.Errorf("failed to parse GHES REST URL: %w", err) } - gqlURL, err := url.Parse(fmt.Sprintf("%s://%s/api/graphql", u.Scheme, u.Hostname())) + gqlURL, err := url.Parse(fmt.Sprintf("%s://%s/api/graphql", u.Scheme, authority)) if err != nil { return APIHost{}, fmt.Errorf("failed to parse GHES GraphQL URL: %w", err) } @@ -165,7 +173,7 @@ func newGHESHost(hostname string) (APIHost, error) { uploadURL, err = url.Parse(fmt.Sprintf("%s://uploads.%s/", u.Scheme, u.Hostname())) } else { // Without subdomain isolation: https://hostname/api/uploads/ - uploadURL, err = url.Parse(fmt.Sprintf("%s://%s/api/uploads/", u.Scheme, u.Hostname())) + uploadURL, err = url.Parse(fmt.Sprintf("%s://%s/api/uploads/", u.Scheme, authority)) } if err != nil { return APIHost{}, fmt.Errorf("failed to parse GHES Upload URL: %w", err) @@ -177,13 +185,13 @@ func newGHESHost(hostname string) (APIHost, error) { rawURL, err = url.Parse(fmt.Sprintf("%s://raw.%s/", u.Scheme, u.Hostname())) } else { // Without subdomain isolation: https://hostname/raw/ - rawURL, err = url.Parse(fmt.Sprintf("%s://%s/raw/", u.Scheme, u.Hostname())) + rawURL, err = url.Parse(fmt.Sprintf("%s://%s/raw/", u.Scheme, authority)) } if err != nil { return APIHost{}, fmt.Errorf("failed to parse GHES Raw URL: %w", err) } - authorizationServerURL, err := url.Parse(fmt.Sprintf("%s://%s/login/oauth", u.Scheme, u.Hostname())) + authorizationServerURL, err := url.Parse(fmt.Sprintf("%s://%s/login/oauth", u.Scheme, authority)) if err != nil { return APIHost{}, fmt.Errorf("failed to parse GHES Authorization Server URL: %w", err) } diff --git a/pkg/utils/api_test.go b/pkg/utils/api_test.go index 7aa762a9b1..baa1eb30ce 100644 --- a/pkg/utils/api_test.go +++ b/pkg/utils/api_test.go @@ -76,6 +76,21 @@ func TestParseAPIHost(t *testing.T) { input: "http://127.0.0.1", wantRestURL: "http://127.0.0.1/api/v3/", }, + { + name: "http loopback preserves port for local development", + input: "http://localhost:3000", + wantRestURL: "http://localhost:3000/api/v3/", + }, + { + name: "http ipv6 loopback preserves brackets", + input: "http://[::1]", + wantRestURL: "http://[::1]/api/v3/", + }, + { + name: "http ipv6 loopback preserves brackets and port", + input: "http://[::1]:8080", + wantRestURL: "http://[::1]:8080/api/v3/", + }, { name: "http remote host rejected", input: "http://notgithub.com",