Expand parser GitHub host unit coverage and harden host-repo assertions (#46926)

Author: Copilot

pkg/cli/engine_secrets.go

@@ -408,19 +408,6 @@ func buildGenericPATCreationURL() string {
 	return buildPATCreationURL(nil)
 }
 
-// isAnyGitHubHostEnvVarSet returns true when any of the environment variables
-// consumed by getGitHubHost() is explicitly set.  When at least one is present
-// the caller has made an explicit host choice and the git-remote fallback should
-// not be consulted.
-func isAnyGitHubHostEnvVarSet() bool {
-	for _, envVar := range []string{"GITHUB_SERVER_URL", "GITHUB_ENTERPRISE_HOST", "GITHUB_HOST", "GH_HOST"} {
-		if os.Getenv(envVar) != "" { //nolint:osgetenvlibrary
-			return true
-		}
-	}
-	return false
-}
-
 func buildPATCreationURL(values url.Values) string {
 	hostURL := getGitHubHost()
 	// Only consult the git remote when the caller has not made an explicit host

pkg/cli/github.go

@@ -33,3 +33,10 @@ func getGitHubHostForRepo(repo string) string {
 	// For all other repositories, use the configured GitHub host
 	return getGitHubHost()
 }
+
+// isAnyGitHubHostEnvVarSet returns true when at least one of the environment
+// variables consulted by getGitHubHost is explicitly set to a non-empty value.
+// Delegates to parser.IsAnyGitHubHostEnvVarSet() for the shared implementation.
+func isAnyGitHubHostEnvVarSet() bool {
+	return parser.IsAnyGitHubHostEnvVarSet()
+}

pkg/cli/import_url_fetcher.go

@@ -8,7 +8,6 @@ import (
 	"mime"
 	"net/http"
 	"net/url"
-	"os"
 	"strings"
 	"time"
 
@@ -305,26 +304,22 @@ func logResponseBodyVerbose(resp *http.Response) {
 }
 
 func importAuthGHHost() string {
-	ghHost := os.Getenv("GH_HOST") //nolint:osgetenvlibrary
-	if ghHost == "" {
+	// Use unified resolution (GITHUB_SERVER_URL > GITHUB_ENTERPRISE_HOST > GITHUB_HOST > GH_HOST).
+	// Return "" when no host env var is set so that callers do not add a
+	// redundant entry for github.com, which is already in defaultImportAuthHosts.
+	if !isAnyGitHubHostEnvVarSet() {
 		return ""
 	}
-	// GH_HOST may carry a scheme prefix; extract just the hostname.
-	if u, parseErr := url.Parse(ghHost); parseErr == nil && u.Host != "" {
-		return strings.ToLower(u.Hostname())
-	}
-	// No scheme present — treat the whole value as a bare hostname (possibly
-	// with port). Strip any accidental scheme prefix or trailing path.
-	bare := strings.TrimPrefix(ghHost, "https://")
-	bare = strings.TrimPrefix(bare, "http://")
-	if idx := strings.IndexByte(bare, '/'); idx != -1 {
-		bare = bare[:idx]
-	}
-	parsed, err := url.Parse("https://" + bare)
-	if err == nil && parsed.Host != "" {
-		return strings.ToLower(parsed.Hostname())
+	// getGitHubHost (defined in this package) returns a normalized https://… URL;
+	// url.Parse is used only to extract the hostname so that port numbers are stripped.
+	resolved := getGitHubHost()
+	u, parseErr := url.Parse(resolved)
+	if parseErr != nil || u.Hostname() == "" {
+		// getGitHubHost always returns a well-formed URL; an error here is unexpected.
+		importURLFetcherLog.Printf("importAuthGHHost: unexpected url.Parse failure for %q: %v", resolved, parseErr)
+		return ""
 	}
-	return strings.ToLower(bare)
+	return strings.ToLower(u.Hostname())
 }
 
 // sanitizeHTTPError strips the request URL from a *url.Error (the error type

pkg/cli/import_url_fetcher_test.go

@@ -336,6 +336,9 @@ func TestAttachImportAuthHeader_DocsGitHub_NoToken(t *testing.T) {
 func TestAttachImportAuthHeader_GHE_BareHostname(t *testing.T) {
 	t.Setenv("GITHUB_TOKEN", "ghe-token")
 	t.Setenv("GH_TOKEN", "")
+	t.Setenv("GITHUB_SERVER_URL", "")
+	t.Setenv("GITHUB_ENTERPRISE_HOST", "")
+	t.Setenv("GITHUB_HOST", "")
 	t.Setenv("GH_HOST", "ghe.example.com")
 
 	req, _ := http.NewRequest(http.MethodGet, "https://ghe.example.com/owner/repo/raw/main/wf.md", nil)
@@ -347,6 +350,9 @@ func TestAttachImportAuthHeader_GHE_BareHostname(t *testing.T) {
 func TestAttachImportAuthHeader_GHE_HTTPSScheme(t *testing.T) {
 	t.Setenv("GITHUB_TOKEN", "ghe-token")
 	t.Setenv("GH_TOKEN", "")
+	t.Setenv("GITHUB_SERVER_URL", "")
+	t.Setenv("GITHUB_ENTERPRISE_HOST", "")
+	t.Setenv("GITHUB_HOST", "")
 	t.Setenv("GH_HOST", "https://ghe.example.com")
 
 	req, _ := http.NewRequest(http.MethodGet, "https://ghe.example.com/owner/repo/raw/main/wf.md", nil)
@@ -358,6 +364,9 @@ func TestAttachImportAuthHeader_GHE_HTTPSScheme(t *testing.T) {
 func TestAttachImportAuthHeader_GHE_HTTPSchemePrefix(t *testing.T) {
 	t.Setenv("GITHUB_TOKEN", "ghe-token")
 	t.Setenv("GH_TOKEN", "")
+	t.Setenv("GITHUB_SERVER_URL", "")
+	t.Setenv("GITHUB_ENTERPRISE_HOST", "")
+	t.Setenv("GITHUB_HOST", "")
 	t.Setenv("GH_HOST", "http://ghe.example.com")
 
 	// HTTPS request → token sent.
@@ -375,6 +384,9 @@ func TestAttachImportAuthHeader_GHE_HTTPSchemePrefix(t *testing.T) {
 func TestAttachImportAuthHeader_GHE_DifferentHost(t *testing.T) {
 	t.Setenv("GITHUB_TOKEN", "ghe-token")
 	t.Setenv("GH_TOKEN", "")
+	t.Setenv("GITHUB_SERVER_URL", "")
+	t.Setenv("GITHUB_ENTERPRISE_HOST", "")
+	t.Setenv("GITHUB_HOST", "")
 	t.Setenv("GH_HOST", "ghe.example.com")
 
 	req, _ := http.NewRequest(http.MethodGet, "https://other.example.com/workflow.md", nil)
@@ -386,13 +398,58 @@ func TestAttachImportAuthHeader_GHE_DifferentHost(t *testing.T) {
 func TestAttachImportAuthHeader_GitHubAlongsideGHE(t *testing.T) {
 	t.Setenv("GITHUB_TOKEN", "dual-token")
 	t.Setenv("GH_TOKEN", "")
+	t.Setenv("GITHUB_SERVER_URL", "")
+	t.Setenv("GITHUB_ENTERPRISE_HOST", "")
+	t.Setenv("GITHUB_HOST", "")
 	t.Setenv("GH_HOST", "ghe.example.com")
 
 	req, _ := http.NewRequest(http.MethodGet, "https://github.com/owner/repo/raw/main/wf.md", nil)
 	attachImportAuthHeader(req, "https://github.com/owner/repo/raw/main/wf.md")
 	assert.Equal(t, "Bearer dual-token", req.Header.Get("Authorization"), "github.com must still be allowed when GH_HOST is also set")
 }
 
+// GITHUB_ENTERPRISE_HOST resolves the auth host (unified resolution).
+func TestAttachImportAuthHeader_GHE_EnterpriseHostEnvVar(t *testing.T) {
+	t.Setenv("GITHUB_TOKEN", "ent-token")
+	t.Setenv("GH_TOKEN", "")
+	t.Setenv("GITHUB_SERVER_URL", "")
+	t.Setenv("GITHUB_ENTERPRISE_HOST", "ent.example.com")
+	t.Setenv("GITHUB_HOST", "other.example.com")
+	t.Setenv("GH_HOST", "gh.example.com")
+
+	req, _ := http.NewRequest(http.MethodGet, "https://ent.example.com/owner/repo/raw/main/wf.md", nil)
+	attachImportAuthHeader(req, "https://ent.example.com/owner/repo/raw/main/wf.md")
+	assert.Equal(t, "Bearer ent-token", req.Header.Get("Authorization"), "GITHUB_ENTERPRISE_HOST must resolve as the auth host")
+}
+
+// GITHUB_SERVER_URL resolves the auth host (highest priority).
+func TestAttachImportAuthHeader_GHE_ServerURLEnvVar(t *testing.T) {
+	t.Setenv("GITHUB_TOKEN", "srv-token")
+	t.Setenv("GH_TOKEN", "")
+	t.Setenv("GITHUB_SERVER_URL", "https://srv.example.com")
+	t.Setenv("GITHUB_ENTERPRISE_HOST", "ent.example.com")
+	t.Setenv("GITHUB_HOST", "")
+	t.Setenv("GH_HOST", "")
+
+	req, _ := http.NewRequest(http.MethodGet, "https://srv.example.com/owner/repo/raw/main/wf.md", nil)
+	attachImportAuthHeader(req, "https://srv.example.com/owner/repo/raw/main/wf.md")
+	assert.Equal(t, "Bearer srv-token", req.Header.Get("Authorization"), "GITHUB_SERVER_URL must resolve as the auth host")
+}
+
+// GITHUB_HOST resolves the auth host (third-highest priority, above GH_HOST).
+func TestAttachImportAuthHeader_GHE_GitHubHostEnvVar(t *testing.T) {
+	t.Setenv("GITHUB_TOKEN", "gh-host-token")
+	t.Setenv("GH_TOKEN", "")
+	t.Setenv("GITHUB_SERVER_URL", "")
+	t.Setenv("GITHUB_ENTERPRISE_HOST", "")
+	t.Setenv("GITHUB_HOST", "ghhost.example.com")
+	t.Setenv("GH_HOST", "lowpriority.example.com")
+
+	req, _ := http.NewRequest(http.MethodGet, "https://ghhost.example.com/owner/repo/raw/main/wf.md", nil)
+	attachImportAuthHeader(req, "https://ghhost.example.com/owner/repo/raw/main/wf.md")
+	assert.Equal(t, "Bearer "+"gh-host-token", req.Header.Get("Authorization"), "GITHUB_HOST must resolve as the auth host")
+}
+
 // TestBuildRequestLogString_RedactsAuthorization verifies that the request formatter
 // never exposes the raw token and shows the correct redacted form.
 func TestBuildRequestLogString_RedactsAuthorization(t *testing.T) {

pkg/cli/init.go

@@ -337,32 +337,25 @@ func isGHESHost(host string) bool {
 // detectGHESDeployment returns the GHES host if the current repository's git
 // remote points to a GitHub Enterprise Server instance, or "" if it does not.
 // Detection uses the following sources in priority order:
-//  1. GITHUB_SERVER_URL environment variable (set automatically inside GitHub Actions)
-//  2. GH_HOST environment variable (set by the gh CLI)
-//  3. The hostname extracted from the git origin remote URL
+//  1. GITHUB_SERVER_URL, GITHUB_ENTERPRISE_HOST, GITHUB_HOST, GH_HOST environment variables
+//  2. The hostname extracted from the git origin remote URL
 func detectGHESDeployment() string {
-	// Check GITHUB_SERVER_URL first (set inside GitHub Actions runners)
-	if serverURL := os.Getenv("GITHUB_SERVER_URL"); serverURL != "" { //nolint:osgetenvlibrary
-		// serverURL is like "https://ghes.example.com", extract just the host.
-		host := serverURL
-		for _, scheme := range []string{"https://", "http://"} {
-			host = strings.TrimPrefix(host, scheme)
+	// Check env vars in unified priority order (mirrors GetGitHubHost):
+	// GITHUB_SERVER_URL > GITHUB_ENTERPRISE_HOST > GITHUB_HOST > GH_HOST
+	for _, envVar := range []string{"GITHUB_SERVER_URL", "GITHUB_ENTERPRISE_HOST", "GITHUB_HOST", "GH_HOST"} {
+		rawValue := os.Getenv(envVar) //nolint:osgetenvlibrary
+		if rawValue == "" {
+			continue
 		}
+		host := strings.TrimPrefix(rawValue, "https://")
+		host = strings.TrimPrefix(host, "http://")
 		host = strings.TrimSuffix(host, "/")
 		if isGHESHost(host) {
-			initLog.Printf("Detected GHES deployment from GITHUB_SERVER_URL: %s", host)
+			initLog.Printf("Detected GHES deployment from %s: %s", envVar, host)
 			return host
 		}
 	}
 
-	// Check GH_HOST (set when using the gh CLI against an enterprise instance)
-	if ghHost := os.Getenv("GH_HOST"); ghHost != "" { //nolint:osgetenvlibrary
-		if isGHESHost(ghHost) {
-			initLog.Printf("Detected GHES deployment from GH_HOST: %s", ghHost)
-			return ghHost
-		}
-	}
-
 	// Fall back to detecting the host from the git origin remote
 	host := getHostFromOriginRemote()
 	if isGHESHost(host) {

pkg/parser/github.go

@@ -40,6 +40,19 @@ func GetGitHubHost() string {
 	return defaultHost
 }
 
+// IsAnyGitHubHostEnvVarSet returns true when at least one of the environment
+// variables consulted by GetGitHubHost is explicitly set to a non-empty value.
+// This indicates the caller has made an explicit host choice and automatic
+// fallback heuristics (such as git-remote detection) should not be consulted.
+func IsAnyGitHubHostEnvVarSet() bool {
+	for _, envVar := range []string{"GITHUB_SERVER_URL", "GITHUB_ENTERPRISE_HOST", "GITHUB_HOST", "GH_HOST"} {
+		if os.Getenv(envVar) != "" { //nolint:osgetenvlibrary
+			return true
+		}
+	}
+	return false
+}
+
 // GetGitHubHostForRepo returns the GitHub host URL for a specific repository.
 // Repositories under the github, githubnext, and microsoft organizations are
 // fetched from public GitHub (https://github.com) in cross-host contexts.

pkg/parser/github_host_test.go

@@ -5,9 +5,73 @@ package parser
 import (
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+func TestGetGitHubHost(t *testing.T) {
+	tests := []struct {
+		name           string
+		serverURL      string
+		enterpriseHost string
+		githubHost     string
+		ghHost         string
+		expectedHost   string
+	}{
+		{
+			name:           "GITHUB_SERVER_URL wins over others",
+			serverURL:      "acme.ghe.com/redacted",
+			enterpriseHost: "enterprise.ghe.com",
+			githubHost:     "github-host.ghe.com",
+			ghHost:         "gh-host.ghe.com",
+			expectedHost:   "https://acme.ghe.com/redacted",
+		},
+		{
+			name:           "GITHUB_ENTERPRISE_HOST wins over GITHUB_HOST and GH_HOST",
+			serverURL:      "",
+			enterpriseHost: "acme.ghe.com",
+			githubHost:     "github-host.ghe.com",
+			ghHost:         "gh-host.ghe.com",
+			expectedHost:   "https://acme.ghe.com",
+		},
+		{
+			name:           "GITHUB_HOST wins over GH_HOST",
+			serverURL:      "",
+			enterpriseHost: "",
+			githubHost:     "acme.ghe.com/",
+			ghHost:         "gh-host.ghe.com",
+			expectedHost:   "https://acme.ghe.com",
+		},
+		{
+			name:           "GH_HOST used when others are empty",
+			serverURL:      "",
+			enterpriseHost: "",
+			githubHost:     "",
+			ghHost:         "acme.ghe.com",
+			expectedHost:   "https://acme.ghe.com",
+		},
+		{
+			name:           "all vars empty falls back to github.com",
+			serverURL:      "",
+			enterpriseHost: "",
+			githubHost:     "",
+			ghHost:         "",
+			expectedHost:   "https://github.com",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Setenv("GITHUB_SERVER_URL", tt.serverURL)
+			t.Setenv("GITHUB_ENTERPRISE_HOST", tt.enterpriseHost)
+			t.Setenv("GITHUB_HOST", tt.githubHost)
+			t.Setenv("GH_HOST", tt.ghHost)
+
+			host := GetGitHubHost()
+			require.Equal(t, tt.expectedHost, host)
+		})
+	}
+}
+
 func TestGetGitHubHostForRepo_PublicOrgFallback(t *testing.T) {
 	tests := []struct {
 		name         string
@@ -23,6 +87,13 @@ func TestGetGitHubHostForRepo_PublicOrgFallback(t *testing.T) {
 			gheHost:      "myorg.ghe.com",
 			expectedHost: "https://myorg.ghe.com",
 		},
+		{
+			name:         "empty gheHost falls back to public for non-fallback owner",
+			owner:        "acme",
+			repo:         "repo",
+			gheHost:      "",
+			expectedHost: "https://github.com",
+		},
 		{
 			name:         "github owner uses public host",
 			owner:        "github",
@@ -54,7 +125,7 @@ func TestGetGitHubHostForRepo_PublicOrgFallback(t *testing.T) {
 			t.Setenv("GH_HOST", "")
 
 			host := GetGitHubHostForRepo(tt.owner, tt.repo)
-			assert.Equal(t, tt.expectedHost, host, "GetGitHubHostForRepo(%q, %q)", tt.owner, tt.repo)
+			require.Equal(t, tt.expectedHost, host, "GetGitHubHostForRepo(%q, %q)", tt.owner, tt.repo)
 		})
 	}
 }