diff --git a/pkg/cli/engine_secrets.go b/pkg/cli/engine_secrets.go index 95d540673db..660c1683267 100644 --- a/pkg/cli/engine_secrets.go +++ b/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 diff --git a/pkg/cli/github.go b/pkg/cli/github.go index 3f9b3a6a90c..5fa96c7506f 100644 --- a/pkg/cli/github.go +++ b/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() +} diff --git a/pkg/cli/import_url_fetcher.go b/pkg/cli/import_url_fetcher.go index 6a09e6744a0..c1b7d5c46e3 100644 --- a/pkg/cli/import_url_fetcher.go +++ b/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 diff --git a/pkg/cli/import_url_fetcher_test.go b/pkg/cli/import_url_fetcher_test.go index cffd67ddd1a..f2b69ce58fc 100644 --- a/pkg/cli/import_url_fetcher_test.go +++ b/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,6 +398,9 @@ 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) @@ -393,6 +408,48 @@ func TestAttachImportAuthHeader_GitHubAlongsideGHE(t *testing.T) { 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) { diff --git a/pkg/cli/init.go b/pkg/cli/init.go index 08dab407caf..31915616001 100644 --- a/pkg/cli/init.go +++ b/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) { diff --git a/pkg/parser/github.go b/pkg/parser/github.go index a721db65f47..e3448866670 100644 --- a/pkg/parser/github.go +++ b/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. diff --git a/pkg/parser/github_host_test.go b/pkg/parser/github_host_test.go index b2601e8afd9..d1e83c48985 100644 --- a/pkg/parser/github_host_test.go +++ b/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) }) } }