Skip to content

Commit 625f48a

Browse files
authored
Harden CLI proxy GraphQL path handling with explicit /api/graphql regression coverage (#4278)
The CLI proxy mode was observed returning 404s for GraphQL-backed `gh` commands when requests hit `/api/graphql` via `GH_HOST`. This PR adds focused regression coverage to ensure GHES-style and GH_HOST-prefixed GraphQL paths are normalized and forwarded through the GraphQL codepath. - **Problem scope** - Protect GraphQL request handling for CLI proxy mode where `gh` may send: - `/api/graphql` (GHES-style) - `/api/v3/graphql` (GH_HOST-prefixed) - **Test updates** - Expanded `TestServeHTTP_GraphQLPreservesQueryString` in `internal/proxy/handler_test.go` to explicitly cover: - `/api/graphql` → forwarded as `/graphql` - `/api/v3/graphql` → forwarded as `/graphql` - Kept existing query-string forwarding assertions and clarified subtest names. - **Regression intent** - Locks in expected normalization/forwarding behavior for GraphQL endpoint variants used by `gh` CLI, reducing risk of future 404 regressions in proxy mode. ```go tests := []struct { path string wantPath string }{ {path: "/api/graphql", wantPath: "/graphql"}, {path: "/api/v3/graphql", wantPath: "/graphql"}, {path: "/api/graphql?foo=bar", wantPath: "/graphql?foo=bar"}, } ``` > [!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-build2075567792/b509/launcher.test /tmp/go-build2075567792/b509/launcher.test -test.testlogfile=/tmp/go-build2075567792/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true til/net.go til/util.go x_amd64/compile` (dns block) > - Triggering command: `/tmp/go-build1045036780/b513/launcher.test /tmp/go-build1045036780/b513/launcher.test -test.testlogfile=/tmp/go-build1045036780/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s n-me�� /tmp/go-build2075567792/b468/_pkg_.a -trimpath 5567792/b444/vet.cfg -p github.com/segme/tmp/go-build3264678706/b424/vet.cfg -lang=go1.17 /opt/hostedtoolcache/go/1.25.9/x64/pkg/tool/linu-buildtags -ato�� -bool -buildtags x_amd64/vet -errorsas -ifaceassert -nilfunc x_amd64/vet` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build2075567792/b491/config.test /tmp/go-build2075567792/b491/config.test -test.testlogfile=/tmp/go-build2075567792/b491/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true te_group.go ternal/wasip1/clgoogle.golang.org/grpc/balancer/pickfirst x_amd64/compile` (dns block) > - Triggering command: `/tmp/go-build1045036780/b495/config.test /tmp/go-build1045036780/b495/config.test -test.testlogfile=/tmp/go-build1045036780/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s pkg/�� /tmp/go-build2075567792/b192/_pkg_.a -trimpath x_amd64/vet -p net/http/httptes-unsafeptr=false -lang=go1.25 x_amd64/vet -p github.com/segmentio/asm/base64 -trimpath iginal -I /tmp/go-build207/tmp/go-build3264678706/b428/vet.cfg -I iginal` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build2075567792/b509/launcher.test /tmp/go-build2075567792/b509/launcher.test -test.testlogfile=/tmp/go-build2075567792/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true til/net.go til/util.go x_amd64/compile` (dns block) > - Triggering command: `/tmp/go-build1045036780/b513/launcher.test /tmp/go-build1045036780/b513/launcher.test -test.testlogfile=/tmp/go-build1045036780/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s n-me�� /tmp/go-build2075567792/b468/_pkg_.a -trimpath 5567792/b444/vet.cfg -p github.com/segme/tmp/go-build3264678706/b424/vet.cfg -lang=go1.17 /opt/hostedtoolcache/go/1.25.9/x64/pkg/tool/linu-buildtags -ato�� -bool -buildtags x_amd64/vet -errorsas -ifaceassert -nilfunc x_amd64/vet` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build2075567792/b509/launcher.test /tmp/go-build2075567792/b509/launcher.test -test.testlogfile=/tmp/go-build2075567792/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true til/net.go til/util.go x_amd64/compile` (dns block) > - Triggering command: `/tmp/go-build1045036780/b513/launcher.test /tmp/go-build1045036780/b513/launcher.test -test.testlogfile=/tmp/go-build1045036780/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s n-me�� /tmp/go-build2075567792/b468/_pkg_.a -trimpath 5567792/b444/vet.cfg -p github.com/segme/tmp/go-build3264678706/b424/vet.cfg -lang=go1.17 /opt/hostedtoolcache/go/1.25.9/x64/pkg/tool/linu-buildtags -ato�� -bool -buildtags x_amd64/vet -errorsas -ifaceassert -nilfunc x_amd64/vet` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build2075567792/b518/mcp.test /tmp/go-build2075567792/b518/mcp.test -test.testlogfile=/tmp/go-build2075567792/b518/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true rt/assertion_com-errorsas rt/assertion_for-ifaceassert x_amd64/compile -p bufio -lang=go1.25 x_amd64/compile -o @v1.43.0/interna-errorsas @v1.43.0/interna-ifaceassert x_amd64/asm -p ions =0 x_amd64/asm` (dns block) > - Triggering command: `/tmp/go-build1045036780/b522/mcp.test /tmp/go-build1045036780/b522/mcp.test -test.testlogfile=/tmp/go-build1045036780/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s -ato�� -bool -buildtags x_amd64/vet -errorsas -ifaceassert -nilfunc x_amd64/vet -ato�� 04 -buildtags x_amd64/vet -errorsas -ifaceassert -nilfunc 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 cebccbe + fcfed11 commit 625f48a

1 file changed

Lines changed: 4 additions & 2 deletions

File tree

internal/proxy/handler_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,10 @@ 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: "/graphql?foo=bar"},
199-
{name: "gh host prefixed graphql path", path: "/api/v3/graphql?foo=bar", wantPath: "/graphql?foo=bar"},
198+
{name: "ghes api graphql path", path: "/api/graphql", wantPath: "/graphql"},
199+
{name: "ghes api graphql path with query", path: "/api/graphql?foo=bar", wantPath: "/graphql?foo=bar"},
200+
{name: "gh host prefixed graphql path", path: "/api/v3/graphql", wantPath: "/graphql"},
201+
{name: "gh host prefixed graphql path with query", path: "/api/v3/graphql?foo=bar", wantPath: "/graphql?foo=bar"},
200202
}
201203

202204
for _, tt := range tests {

0 commit comments

Comments
 (0)