diff --git a/pkg/sources/github/connector.go b/pkg/sources/github/connector.go index 494ab5d18..3f460295b 100644 --- a/pkg/sources/github/connector.go +++ b/pkg/sources/github/connector.go @@ -18,6 +18,8 @@ import ( const ( cloudV3Endpoint = "https://api.github.com" cloudGraphqlEndpoint = "https://api.github.com/graphql" // https://docs.github.com/en/graphql/guides/forming-calls-with-graphql#the-graphql-endpoint + + gheCloudSuffix = ".ghe.com" ) // Connector abstracts over the authenticated ways to interact with GitHub: cloning and API operations. @@ -34,6 +36,81 @@ type Connector interface { Clone(ctx context.Context, repoURL string, args ...string) (string, *gogit.Repository, error) } +// isGHECloud returns true if the endpoint is a GHE.com instance +// (GHEC with data residency). GHE.com uses the same root-level API layout as +// api.github.com, NOT the GHES /api/v3/ layout. +// +// Examples: +// +// https://api.company.ghe.com -> true +// https://company.ghe.com -> true (web URL; user might pass this) +// https://github.mycompany.com -> false (GHES) +// https://api.github.com -> false (regular github.com) +func isGHECloud(endpoint string) bool { + u, err := url.Parse(strings.TrimRight(endpoint, "/")) + if err != nil { + return false + } + return strings.HasSuffix(strings.ToLower(u.Hostname()), gheCloudSuffix) +} + +// normalizeGHECloudAPIEndpoint ensures the endpoint points to the API subdomain +// with a trailing slash (required by go-github's BaseURL). +// +// On GHE.com the web UI lives at SUBDOMAIN.ghe.com while the API lives at +// api.SUBDOMAIN.ghe.com. Users may pass either form. +// +// https://company.ghe.com -> https://api.company.ghe.com/ +// https://company.ghe.com/ -> https://api.company.ghe.com/ +// https://api.company.ghe.com -> https://api.company.ghe.com/ +// https://api.company.ghe.com/ -> https://api.company.ghe.com/ +func normalizeGHECloudAPIEndpoint(endpoint string) (string, error) { + u, err := url.Parse(strings.TrimRight(endpoint, "/")) + if err != nil { + return "", fmt.Errorf("invalid GHE.com endpoint URL: %w", err) + } + + host := u.Hostname() + port := u.Port() + + // If user passed the web URL (company.ghe.com), prepend "api." + if !strings.HasPrefix(strings.ToLower(host), "api.") { + host = "api." + host + } + + if port != "" { + u.Host = host + ":" + port + } else { + u.Host = host + } + + // go-github requires a trailing slash on BaseURL. + u.Path = "/" + + return u.String(), nil +} + +// createGHECloudClient creates a go-github client configured for GHE.com. +// GHE.com serves its REST API at the root of api.SUBDOMAIN.ghe.com (same +// layout as api.github.com), so we must NOT use WithEnterpriseURLs which +// would append /api/v3/. +func createGHECloudClient(httpClient *http.Client, apiEndpoint string) (*github.Client, error) { + apiURL, err := normalizeGHECloudAPIEndpoint(apiEndpoint) + if err != nil { + return nil, err + } + + parsedURL, err := url.Parse(apiURL) + if err != nil { + return nil, fmt.Errorf("failed to parse GHE.com API URL: %w", err) + } + + client := github.NewClient(httpClient) + client.BaseURL = parsedURL + client.UploadURL = parsedURL + return client, nil +} + func newConnector(ctx context.Context, source *Source) (Connector, error) { apiEndpoint := canonicalAPIEndpoint(source.conn.Endpoint) @@ -60,27 +137,44 @@ func createAPIClient(ctx context.Context, httpClient *http.Client, apiEndpoint s ctx.Logger().V(2).Info("Creating API client", "url", apiEndpoint) // If we're using public GitHub, make a regular client. - // Otherwise, make an enterprise client. if strings.EqualFold(apiEndpoint, cloudV3Endpoint) { return github.NewClient(httpClient), nil } + // GHE.com (GHEC with data residency) serves its API at the root level + // (like api.github.com), NOT under /api/v3/. + if isGHECloud(apiEndpoint) { + return createGHECloudClient(httpClient, apiEndpoint) + } + + // GHES (self-hosted) — WithEnterpriseURLs appends /api/v3/ and /api/uploads/. return github.NewClient(httpClient).WithEnterpriseURLs(apiEndpoint, apiEndpoint) } func createGraphqlClient(ctx context.Context, client *http.Client, apiEndpoint string) (*githubv4.Client, error) { var graphqlEndpoint string - if apiEndpoint == cloudV3Endpoint { + switch { + case apiEndpoint == cloudV3Endpoint: graphqlEndpoint = cloudGraphqlEndpoint - } else { - // Use the root endpoint for the host. + + case isGHECloud(apiEndpoint): + // GHE.com: GraphQL lives at the root level, same as api.github.com. + // https://api.SUBDOMAIN.ghe.com/graphql + apiURL, err := normalizeGHECloudAPIEndpoint(apiEndpoint) + if err != nil { + return nil, fmt.Errorf("error normalizing GHE.com endpoint: %w", err) + } + graphqlEndpoint = strings.TrimRight(apiURL, "/") + "/graphql" + + default: + // GHES: GraphQL lives under /api/graphql. // https://docs.github.com/en/enterprise-server@3.11/graphql/guides/introduction-to-graphql parsedURL, err := url.Parse(apiEndpoint) if err != nil { return nil, fmt.Errorf("error parsing URL: %w", err) } - // GitHub Enterprise uses `/api/v3` for the base. (https://github.com/google/go-github/issues/958) + // GitHub Enterprise Server uses `/api/v3` for the base. (https://github.com/google/go-github/issues/958) // Swap it, and anything before `/api`, with GraphQL. before, _ := strings.CutSuffix(parsedURL.Path, "/api/v3") parsedURL.Path = before + "/api/graphql" diff --git a/pkg/sources/github/connector_app.go b/pkg/sources/github/connector_app.go index 0b66c4684..cef1567db 100644 --- a/pkg/sources/github/connector_app.go +++ b/pkg/sources/github/connector_app.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "strconv" + "strings" "sync" "github.com/bradleyfalzon/ghinstallation/v2" @@ -273,8 +274,10 @@ func (c *appConnector) createAPIClientForInstallation(installationID int64) (*gi return nil, fmt.Errorf("could not create app transport for installation %d: %w", installationID, err) } + // NewFromAppsTransport inherits BaseURL from appsTransport, which + // newAppsTransport already set correctly (incl. GHE.com). Don't override it + // with the raw endpoint here or GHE.com token refresh would 401. transport := ghinstallation.NewFromAppsTransport(appsTransport, installationID) - transport.BaseURL = c.apiEndpoint client, err := newGitHubClientWithTransport(c.apiEndpoint, transport) if err != nil { @@ -289,12 +292,41 @@ func newAppsTransport(apiEndpoint string, appID int64, privateKey []byte) (*ghin if err != nil { return nil, err } - appsTransport.BaseURL = apiEndpoint + + // ghinstallation uses BaseURL to build the token exchange URL + // {BaseURL}/app/installations/{id}/access_tokens. For GHE.com this must be + // the api.SUBDOMAIN.ghe.com base with no /api/v3 and no trailing slash + // (which ghinstallation does not expect), otherwise token refresh 401s. + baseURL, err := appsBaseURL(apiEndpoint) + if err != nil { + return nil, err + } + appsTransport.BaseURL = baseURL return appsTransport, nil } +// appsBaseURL returns the BaseURL that ghinstallation transports should use for +// token exchange/refresh. For GHE.com it resolves to the api.* subdomain with +// the trailing slash trimmed; for github.com and GHES it is the endpoint as-is. +func appsBaseURL(apiEndpoint string) (string, error) { + if isGHECloud(apiEndpoint) { + normalizedURL, err := normalizeGHECloudAPIEndpoint(apiEndpoint) + if err != nil { + return "", fmt.Errorf("could not normalize GHE.com endpoint: %w", err) + } + return strings.TrimRight(normalizedURL, "/"), nil + } + return apiEndpoint, nil +} + func newGitHubClientWithTransport(apiEndpoint string, transport http.RoundTripper) (*github.Client, error) { httpClient := common.RetryableHTTPClientTimeout(githubHTTPTimeoutSeconds) httpClient.Transport = transport + + // GHE.com (GHEC with data residency) serves its REST API at the root of + // api.SUBDOMAIN.ghe.com (like api.github.com), NOT under the GHES /api/v3 path. + if isGHECloud(apiEndpoint) { + return createGHECloudClient(httpClient, apiEndpoint) + } return github.NewClient(httpClient).WithEnterpriseURLs(apiEndpoint, apiEndpoint) } diff --git a/pkg/sources/github/connector_token.go b/pkg/sources/github/connector_token.go index fc878c07e..1e4d23417 100644 --- a/pkg/sources/github/connector_token.go +++ b/pkg/sources/github/connector_token.go @@ -49,11 +49,16 @@ func NewTokenConnector(ctx context.Context, apiEndpoint, token, clonePath string return nil, fmt.Errorf("error creating GraphQL client: %w", err) } + // Treat both GHES and GHE.com as "enterprise" for enumeration purposes. + // GHE.com (GHEC with data residency) is a dedicated enterprise environment + // with EMU, so enterprise-level enumeration (e.g., addAllVisibleOrgs) applies. + isEnterprise := !strings.EqualFold(apiEndpoint, cloudV3Endpoint) + return &tokenConnector{ apiClient: apiClient, graphqlClient: graphqlClient, token: token, - isGitHubEnterprise: !strings.EqualFold(apiEndpoint, cloudV3Endpoint), + isGitHubEnterprise: isEnterprise, handleRateLimit: handleRateLimit, authInUrl: authInUrl, clonePath: clonePath, diff --git a/pkg/sources/github/github_test.go b/pkg/sources/github/github_test.go index 64f48b283..906924016 100644 --- a/pkg/sources/github/github_test.go +++ b/pkg/sources/github/github_test.go @@ -366,6 +366,51 @@ func TestAddReposByApp(t *testing.T) { assert.True(t, gock.IsDone()) } +// TestAppConnector_EnterpriseBaseURL verifies that GitHub App authentication +// correctly sets the BaseURL on the internal AppsTransport for token refresh. +// This is critical for GitHub Enterprise Server and GHEC with Data Residency. +// Without this fix, token refresh requests would go to api.github.com instead +// of the configured enterprise endpoint. +func TestAppConnector_EnterpriseBaseURL(t *testing.T) { + privateKey := createPrivateKey() + enterpriseEndpoint := "https://api.example.ghe.com" + + connector, err := NewAppConnector( + context.Background(), + enterpriseEndpoint, + &credentialspb.GitHubApp{ + PrivateKey: privateKey, + InstallationId: "1337", + AppId: "4141", + }) + require.NoError(t, err) + + appConn, ok := connector.(*appConnector) + require.True(t, ok, "connector should be an appConnector") + + // Get the HTTP client's transport, which should be a ghinstallation.Transport + transport := appConn.APIClient().Client().Transport + + // Use reflection to access the Transport's BaseURL field + transportVal := reflect.ValueOf(transport).Elem() + baseURLField := transportVal.FieldByName("BaseURL") + require.True(t, baseURLField.IsValid(), "Transport should have a BaseURL field") + assert.Equal(t, enterpriseEndpoint, baseURLField.String(), + "Transport.BaseURL should be set to enterprise endpoint") + + // Use reflection to access the internal appsTransport and verify its BaseURL + // This is the critical check - the internal AppsTransport is used for token refresh + appsTransportField := transportVal.FieldByName("appsTransport") + require.True(t, appsTransportField.IsValid(), "Transport should have an appsTransport field") + + // Access the BaseURL of the internal AppsTransport + appsTransportVal := appsTransportField.Elem() + appsBaseURLField := appsTransportVal.FieldByName("BaseURL") + require.True(t, appsBaseURLField.IsValid(), "AppsTransport should have a BaseURL field") + assert.Equal(t, enterpriseEndpoint, appsBaseURLField.String(), + "AppsTransport.BaseURL should be set to enterprise endpoint") +} + func TestAddOrgsByUser(t *testing.T) { defer gock.Off() @@ -2737,3 +2782,182 @@ func TestSource_ExcludeArchivedForkInteraction(t *testing.T) { }) } } + +func TestIsGHECloud(t *testing.T) { + tests := []struct { + name string + endpoint string + want bool + }{ + // GHE.com endpoints — should return true + {"api subdomain", "https://api.company.ghe.com", true}, + {"web subdomain", "https://company.ghe.com", true}, + {"api subdomain trailing slash", "https://api.company.ghe.com/", true}, + {"web subdomain trailing slash", "https://company.ghe.com/", true}, + {"uppercase", "https://API.company.GHE.COM", true}, + {"mixed case", "https://Api.company.Ghe.Com/", true}, + {"deep subdomain", "https://api.team.company.ghe.com", true}, + + // GHES endpoints — should return false + {"ghes custom domain", "https://github.mycompany.com", false}, + {"ghes with path", "https://github.mycompany.com/api/v3", false}, + {"ghes internal", "https://github.internal.corp.com", false}, + + // github.com — should return false + {"cloud api", "https://api.github.com", false}, + {"cloud web", "https://github.com", false}, + + // Edge cases — should return false + {"empty string", "", false}, + {"localhost", "http://localhost:8080", false}, + {"malicious suffix", "https://notghe.com", false}, + {"partial match", "https://api.fakeghe.com.evil.com", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isGHECloud(tt.endpoint) + if got != tt.want { + t.Errorf("isGHECloud(%q) = %v, want %v", tt.endpoint, got, tt.want) + } + }) + } +} + +func TestNormalizeGHECloudAPIEndpoint(t *testing.T) { + tests := []struct { + name string + input string + want string + wantErr bool + }{ + { + name: "web URL to API URL", + input: "https://company.ghe.com", + want: "https://api.company.ghe.com/", + }, + { + name: "web URL with trailing slash", + input: "https://company.ghe.com/", + want: "https://api.company.ghe.com/", + }, + { + name: "api URL already correct", + input: "https://api.company.ghe.com", + want: "https://api.company.ghe.com/", + }, + { + name: "api URL with trailing slash", + input: "https://api.company.ghe.com/", + want: "https://api.company.ghe.com/", + }, + { + name: "preserves port", + input: "http://company.ghe.com:8080", + want: "http://api.company.ghe.com:8080/", + }, + { + name: "api with port", + input: "http://api.company.ghe.com:8080", + want: "http://api.company.ghe.com:8080/", + }, + { + name: "strips path", + input: "https://company.ghe.com/some/path", + want: "https://api.company.ghe.com/", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := normalizeGHECloudAPIEndpoint(tt.input) + if (err != nil) != tt.wantErr { + t.Errorf("normalizeGHECloudAPIEndpoint(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("normalizeGHECloudAPIEndpoint(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + +func TestCreateAPIClient_GHECloud_NoAPIV3(t *testing.T) { + // The most critical test: GHE.com clients must NOT have /api/v3/ in their BaseURL. + endpoints := []string{ + "https://api.company.ghe.com", + "https://company.ghe.com", + "https://api.company.ghe.com/", + "https://company.ghe.com/", + } + + for _, endpoint := range endpoints { + t.Run(endpoint, func(t *testing.T) { + client, err := createGHECloudClient(http.DefaultClient, endpoint) + if err != nil { + t.Fatalf("createGHECloudClient(%q) unexpected error: %v", endpoint, err) + } + + baseURL := client.BaseURL.String() + + // Must NOT contain /api/v3 + if strings.Contains(baseURL, "/api/v3") { + t.Errorf("GHE.com client BaseURL must not contain /api/v3/, got: %s", baseURL) + } + + // Must point to api.company.ghe.com + if client.BaseURL.Hostname() != "api.company.ghe.com" { + t.Errorf("expected hostname api.company.ghe.com, got: %s", client.BaseURL.Hostname()) + } + + // Must have trailing slash + if !strings.HasSuffix(baseURL, "/") { + t.Errorf("BaseURL must have trailing slash, got: %s", baseURL) + } + }) + } +} + +func TestCreateGraphqlClient_GHECloud(t *testing.T) { + // GHE.com GraphQL should be at /graphql, NOT /api/graphql. + // We can't easily inspect the URL from the githubv4.Client, + // so we test the URL construction logic indirectly by checking + // the normalized endpoint. + apiURL, err := normalizeGHECloudAPIEndpoint("https://company.ghe.com") + if err != nil { + t.Fatal(err) + } + + graphqlURL := strings.TrimRight(apiURL, "/") + "/graphql" + expected := "https://api.company.ghe.com/graphql" + + if graphqlURL != expected { + t.Errorf("GHE.com GraphQL URL = %q, want %q", graphqlURL, expected) + } +} + +func TestCreateAPIClient_GHES_HasAPIV3(t *testing.T) { + // GHES clients SHOULD have /api/v3/ — make sure we didn't break that. + // Note: WithEnterpriseURLs returns an error for invalid URLs, but for + // valid ones it appends /api/v3/. + client, err := createAPIClient(context.Background(), http.DefaultClient, "https://github.mycompany.com") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !strings.Contains(client.BaseURL.String(), "/api/v3") { + t.Errorf("GHES client BaseURL should contain /api/v3/, got: %s", client.BaseURL.String()) + } +} + +func TestCreateAPIClient_CloudGitHub(t *testing.T) { + // Regular github.com should use the default client (api.github.com). + client, err := createAPIClient(context.Background(), http.DefaultClient, cloudV3Endpoint) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if client.BaseURL.String() != "https://api.github.com/" { + t.Errorf("expected https://api.github.com/, got: %s", client.BaseURL.String()) + } +}