Skip to content

Commit 515f316

Browse files
authored
Fix DIFC proxy GraphQL endpoint rewriting for github.com API base (#4030)
The DIFC proxy was forwarding GraphQL traffic to `/api/graphql` even when upstream was github.com-style, causing 404s and repeated retries in cli-proxy workflows. This updates GraphQL path normalization to target the correct endpoint based on configured API base. - **Endpoint rewrite logic** - Updated proxy forwarding to rewrite GraphQL paths conditionally: - upstream ending in `/api/v3` (GHES) → `/api/graphql` - upstream without `/api/v3` (github.com) → `/graphql` - Applies consistently for incoming GraphQL variants: `/graphql`, `/api/graphql`, `/api/v3/graphql`. - Preserves query strings during rewrite. - **Coverage for dotcom behavior** - Added focused tests for github.com-style API base to assert canonical forwarding to `/graphql` across all supported input path forms. - Kept existing GHES coverage intact. - **Handler expectation alignment** - Updated GraphQL query-string preservation expectation in handler tests to reflect normalized dotcom forwarding path. ```go pathOnly, query, hasQuery := strings.Cut(path, "?") if IsGraphQLPath(pathOnly) { graphqlURL := s.githubAPIURL + "/graphql" if strings.HasSuffix(s.githubAPIURL, "/api/v3") { graphqlURL = strings.TrimSuffix(s.githubAPIURL, "/api/v3") + "/api/graphql" } url = graphqlURL if hasQuery { url += "?" + query } } ``` > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `example.com` > - Triggering command: `/tmp/go-build203165019/b514/launcher.test /tmp/go-build203165019/b514/launcher.test -test.testlogfile=/tmp/go-build203165019/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build203165019/b405/vet.cfg 1.80.0/internal/resolver/delegatingresolver/delegatingresolver.go 0845442/b151/ x_amd64/vet e.go esource/v1` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build203165019/b496/config.test /tmp/go-build203165019/b496/config.test -test.testlogfile=/tmp/go-build203165019/b496/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build203165019/b317/vet.cfg /sdk@v1.43.0/trago1.25.8 /sdk@v1.43.0/tra-c=4 x_amd64/vet ctor idle E=3 x_amd64/vet 0845�� g_.a otection x_amd64/vet --gdwarf-5 rs/otlp/otlptrac-atomic -o x_amd64/vet` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build203165019/b514/launcher.test /tmp/go-build203165019/b514/launcher.test -test.testlogfile=/tmp/go-build203165019/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build203165019/b405/vet.cfg 1.80.0/internal/resolver/delegatingresolver/delegatingresolver.go 0845442/b151/ x_amd64/vet e.go esource/v1` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build203165019/b514/launcher.test /tmp/go-build203165019/b514/launcher.test -test.testlogfile=/tmp/go-build203165019/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build203165019/b405/vet.cfg 1.80.0/internal/resolver/delegatingresolver/delegatingresolver.go 0845442/b151/ x_amd64/vet e.go esource/v1` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build203165019/b523/mcp.test /tmp/go-build203165019/b523/mcp.test -test.testlogfile=/tmp/go-build203165019/b523/testlog.txt -test.paniconexit0 -test.timeout=10m0s 0845�� g_.a GBv9xrrGM x_amd64/vet -p go-sdk/internal/-atomic -lang=go1.24 x_amd64/vet ortc�� cfg 64/src/net/textp-ifaceassert x_amd64/vet . --gdwarf2 --64 x_amd64/vet` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/github/gh-aw-mcpg/settings/copilot/coding_agent) (admins only) > > </details>
2 parents 37a491d + 8963661 commit 515f316

File tree

3 files changed

+50
-3
lines changed

3 files changed

+50
-3
lines changed

internal/proxy/handler_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ func TestServeHTTP_GraphQLPreservesQueryString(t *testing.T) {
195195
wantPath string
196196
}{
197197
{name: "graphql path", path: "/graphql?foo=bar", wantPath: "/graphql?foo=bar"},
198-
{name: "ghes api graphql path", path: "/api/graphql?foo=bar", wantPath: "/api/graphql?foo=bar"},
198+
{name: "ghes api graphql path", path: "/api/graphql?foo=bar", wantPath: "/graphql?foo=bar"},
199199
{name: "gh host prefixed graphql path", path: "/api/v3/graphql?foo=bar", wantPath: "/graphql?foo=bar"},
200200
}
201201

internal/proxy/proxy.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,8 +344,12 @@ func (r *restBackendCaller) CallTool(ctx context.Context, toolName string, args
344344
func (s *Server) forwardToGitHub(ctx context.Context, method, path string, body io.Reader, contentType string, clientAuth string) (*http.Response, error) {
345345
url := s.githubAPIURL + path
346346
pathOnly, query, hasQuery := strings.Cut(path, "?")
347-
if strings.HasSuffix(s.githubAPIURL, "/api/v3") && IsGraphQLPath(pathOnly) {
348-
url = strings.TrimSuffix(s.githubAPIURL, "/api/v3") + "/api/graphql"
347+
if IsGraphQLPath(pathOnly) {
348+
graphqlURL := s.githubAPIURL + "/graphql"
349+
if strings.HasSuffix(s.githubAPIURL, "/api/v3") {
350+
graphqlURL = strings.TrimSuffix(s.githubAPIURL, "/api/v3") + "/api/graphql"
351+
}
352+
url = graphqlURL
349353
if hasQuery {
350354
url += "?" + query
351355
}

internal/proxy/proxy_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -887,3 +887,46 @@ func TestForwardToGitHub_RewritesGraphQLPathForGHESAPIBase(t *testing.T) {
887887
})
888888
}
889889
}
890+
891+
func TestForwardToGitHub_RewritesGraphQLPathForDotComAPIBase(t *testing.T) {
892+
var receivedPath string
893+
var receivedQuery string
894+
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
895+
receivedPath = r.URL.Path
896+
receivedQuery = r.URL.RawQuery
897+
w.WriteHeader(http.StatusOK)
898+
}))
899+
defer upstream.Close()
900+
901+
s := &Server{
902+
githubAPIURL: upstream.URL,
903+
httpClient: upstream.Client(),
904+
}
905+
906+
tests := []struct {
907+
name string
908+
path string
909+
wantPath string
910+
wantQuery string
911+
}{
912+
{name: "plain graphql endpoint", path: "/graphql", wantPath: "/graphql"},
913+
{name: "ghes api graphql endpoint", path: "/api/graphql", wantPath: "/graphql"},
914+
{name: "gh host prefixed graphql endpoint", path: "/api/v3/graphql", wantPath: "/graphql"},
915+
{name: "graphql endpoint with query string", path: "/api/graphql?foo=bar", wantPath: "/graphql", wantQuery: "foo=bar"},
916+
}
917+
918+
for _, tt := range tests {
919+
t.Run(tt.name, func(t *testing.T) {
920+
receivedPath = ""
921+
receivedQuery = ""
922+
923+
resp, err := s.forwardToGitHub(context.Background(), http.MethodPost, tt.path, nil, "application/json", "")
924+
require.NoError(t, err)
925+
require.NotNil(t, resp)
926+
defer resp.Body.Close()
927+
928+
assert.Equal(t, tt.wantPath, receivedPath)
929+
assert.Equal(t, tt.wantQuery, receivedQuery)
930+
})
931+
}
932+
}

0 commit comments

Comments
 (0)