Skip to content

fix(security): enforce HTTPS for gh-host/GITHUB_HOST to prevent cleartext credentials - #3069

Merged
SamMorrowDrums merged 2 commits into
mainfrom
sammorrowdrums-ghes-https-enforcement
Aug 14, 2026
Merged

fix(security): enforce HTTPS for gh-host/GITHUB_HOST to prevent cleartext credentials#3069
SamMorrowDrums merged 2 commits into
mainfrom
sammorrowdrums-ghes-https-enforcement

Conversation

@SamMorrowDrums

Copy link
Copy Markdown
Collaborator

Closes github/copilot-mcp-core#1815

The problem

The MCP server accepts an enterprise host via the --gh-host flag / GITHUB_HOST env var. For GitHub Enterprise Server (GHES) hosts, a non-HTTPS (http://) scheme was accepted and interpolated into every derived URL — REST, GraphQL, upload, raw, and the authorization server (fmt.Sprintf("%s://%s/api/v3/", u.Scheme, u.Hostname()), etc.). checkSubdomainIsolation also probed over that scheme.

The result: outbound authenticated requests could carry the bearer token / PAT over cleartext HTTP, exposing it to network interception and replay. newGHECHost already rejected http, but newGHESHost did not.

The fix

Enforce HTTPS centrally in parseAPIHost, so no deployment (GHES included) can ever build authenticated URLs over cleartext, and the behavior is consistent with the existing GHEC rejection — without duplicating logic per host type.

Loopback exception

http is permitted only for loopback hosts (localhost, 127.0.0.1, ::1) so local/dev testing against a dev server still works. This is implemented narrowly via an exact-match isLoopbackHost check and documented with a comment explaining why. Empty-scheme inputs are still rejected as before.

Rejection produces a clear, security-oriented error:

host must use https to avoid sending credentials over cleartext: http://ghe.example.com (http is only permitted for loopback hosts such as localhost, 127.0.0.1, or ::1)

Tests

  • pkg/utils/api_test.go: https GHES accepted (unchanged); http GHES rejected with the security error; http loopback accepted (localhost and 127.0.0.1); http remote host rejected; empty-scheme still rejected; dotcom/GHEC unchanged.
  • pkg/http/oauth/oauth_test.go: the pre-existing case that asserted http GHES was accepted now asserts it is rejected — that test previously codified the vulnerable behavior.

Docs

Reworded the GHES note in README.md: HTTPS is now required/enforced (loopback excepted) rather than "otherwise defaults to http://".

Validation

go build ./..., go test -race ./..., and golangci-lint (via script/lint, 0 issues) all pass. script/generate-docs produces no additional diff. The change is surgical — no unrelated refactoring.

…text 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>
Copilot AI balanced review requested due to automatic review settings August 14, 2026 11:11
@SamMorrowDrums
SamMorrowDrums requested a review from a team as a code owner August 14, 2026 11:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Enforces HTTPS for configured GitHub hosts while retaining a loopback exception for local development.

Changes:

  • Adds centralized scheme validation.
  • Updates host and OAuth tests.
  • Documents the HTTPS requirement.
Show a summary per file
File Description
README.md Documents HTTPS enforcement and loopback exception.
pkg/utils/api.go Validates schemes before constructing API URLs.
pkg/utils/api_test.go Tests HTTP rejection and loopback acceptance.
pkg/http/oauth/oauth_test.go Verifies insecure GHES OAuth hosts are rejected.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread pkg/utils/api.go
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>
@SamMorrowDrums
SamMorrowDrums merged commit 0ea1f77 into main Aug 14, 2026
20 checks passed
@SamMorrowDrums
SamMorrowDrums deleted the sammorrowdrums-ghes-https-enforcement branch August 14, 2026 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants