Skip to content

Harden CLI proxy GraphQL path handling with explicit /api/graphql regression coverage#4278

Merged
lpcox merged 2 commits intomainfrom
copilot/fix-difc-proxy-graphql-404
Apr 21, 2026
Merged

Harden CLI proxy GraphQL path handling with explicit /api/graphql regression coverage#4278
lpcox merged 2 commits intomainfrom
copilot/fix-difc-proxy-graphql-404

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 21, 2026

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.
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

Firewall rules blocked me from connecting to one or more addresses (expand for details)

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:

Copilot AI changed the title [WIP] Fix DIFC proxy to handle GraphQL requests Harden CLI proxy GraphQL path handling with explicit /api/graphql regression coverage Apr 21, 2026
Copilot AI requested a review from lpcox April 21, 2026 18:20
@lpcox lpcox marked this pull request as ready for review April 21, 2026 18:21
Copilot AI review requested due to automatic review settings April 21, 2026 18:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds regression coverage to ensure CLI proxy GraphQL requests hitting GHES-style (/api/graphql) and GH_HOST-prefixed (/api/v3/graphql) endpoints are correctly normalized/forwarded through the GraphQL codepath, preserving any query string.

Changes:

  • Expanded TestServeHTTP_GraphQLPreservesQueryString to cover /api/graphql and /api/v3/graphql without query strings.
  • Added explicit subtests for the same paths with query strings to lock in forwarding behavior (…?foo=bar).
  • Clarified subtest naming to distinguish base-path vs query-string cases.
Show a summary per file
File Description
internal/proxy/handler_test.go Extends GraphQL proxy tests to cover /api/graphql and /api/v3/graphql path variants (with/without query strings) and assert correct upstream request URI.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@lpcox lpcox merged commit 625f48a into main Apr 21, 2026
30 checks passed
@lpcox lpcox deleted the copilot/fix-difc-proxy-graphql-404 branch April 21, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DIFC proxy (CLI proxy mode) returns 404 for GraphQL requests (/api/graphql)

3 participants