Skip to content

Commit fc09a99

Browse files
committed
remove CrossOriginProtection from ServerConfig
1 parent 6baed53 commit fc09a99

File tree

3 files changed

+42
-94
lines changed

3 files changed

+42
-94
lines changed

pkg/http/handler.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,11 +223,16 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
223223
return
224224
}
225225

226+
// Bypass cross-origin protection: this server uses bearer tokens (not
227+
// cookies), so Sec-Fetch-Site CSRF checks are unnecessary. See PR #XXXX.
228+
crossOriginProtection := http.NewCrossOriginProtection()
229+
crossOriginProtection.AddInsecureBypassPattern("/")
230+
226231
mcpHandler := mcp.NewStreamableHTTPHandler(func(_ *http.Request) *mcp.Server {
227232
return ghServer
228233
}, &mcp.StreamableHTTPOptions{
229234
Stateless: true,
230-
CrossOriginProtection: h.config.CrossOriginProtection,
235+
CrossOriginProtection: crossOriginProtection,
231236
})
232237

233238
mcpHandler.ServeHTTP(w, r)

pkg/http/handler_test.go

Lines changed: 36 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -665,97 +665,53 @@ func buildStaticInventoryFromTools(cfg *ServerConfig, tools []inventory.ServerTo
665665
func TestCrossOriginProtection(t *testing.T) {
666666
jsonRPCBody := `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-03-26","capabilities":{},"clientInfo":{"name":"test","version":"0.1"}}}`
667667

668-
newHandler := func(t *testing.T, crossOriginProtection *http.CrossOriginProtection) http.Handler {
669-
t.Helper()
670-
671-
apiHost, err := utils.NewAPIHost("https://api.githubcopilot.com")
672-
require.NoError(t, err)
673-
674-
handler := NewHTTPMcpHandler(
675-
context.Background(),
676-
&ServerConfig{
677-
Version: "test",
678-
CrossOriginProtection: crossOriginProtection,
679-
},
680-
nil,
681-
translations.NullTranslationHelper,
682-
slog.Default(),
683-
apiHost,
684-
WithInventoryFactory(func(_ *http.Request) (*inventory.Inventory, error) {
685-
return inventory.NewBuilder().Build()
686-
}),
687-
WithGitHubMCPServerFactory(func(_ *http.Request, _ github.ToolDependencies, _ *inventory.Inventory, _ *github.MCPServerConfig) (*mcp.Server, error) {
688-
return mcp.NewServer(&mcp.Implementation{Name: "test", Version: "0.0.1"}, nil), nil
689-
}),
690-
WithScopeFetcher(allScopesFetcher{}),
691-
)
692-
693-
r := chi.NewRouter()
694-
handler.RegisterMiddleware(r)
695-
handler.RegisterRoutes(r)
696-
return r
697-
}
668+
apiHost, err := utils.NewAPIHost("https://api.githubcopilot.com")
669+
require.NoError(t, err)
670+
671+
handler := NewHTTPMcpHandler(
672+
context.Background(),
673+
&ServerConfig{
674+
Version: "test",
675+
},
676+
nil,
677+
translations.NullTranslationHelper,
678+
slog.Default(),
679+
apiHost,
680+
WithInventoryFactory(func(_ *http.Request) (*inventory.Inventory, error) {
681+
return inventory.NewBuilder().Build()
682+
}),
683+
WithGitHubMCPServerFactory(func(_ *http.Request, _ github.ToolDependencies, _ *inventory.Inventory, _ *github.MCPServerConfig) (*mcp.Server, error) {
684+
return mcp.NewServer(&mcp.Implementation{Name: "test", Version: "0.0.1"}, nil), nil
685+
}),
686+
WithScopeFetcher(allScopesFetcher{}),
687+
)
688+
689+
r := chi.NewRouter()
690+
handler.RegisterMiddleware(r)
691+
handler.RegisterRoutes(r)
698692

699693
tests := []struct {
700-
name string
701-
crossOriginProtection *http.CrossOriginProtection
702-
secFetchSite string
703-
origin string
704-
expectedStatusCode int
694+
name string
695+
secFetchSite string
696+
origin string
705697
}{
706698
{
707-
name: "SDK default rejects cross-site when no bypass configured",
708-
secFetchSite: "cross-site",
709-
origin: "https://evil.example.com",
710-
expectedStatusCode: http.StatusForbidden,
711-
},
712-
{
713-
name: "SDK default allows same-origin request",
714-
secFetchSite: "same-origin",
715-
expectedStatusCode: http.StatusOK,
699+
name: "cross-site request with bearer token succeeds",
700+
secFetchSite: "cross-site",
701+
origin: "https://example.com",
716702
},
717703
{
718-
name: "SDK default allows request without Sec-Fetch-Site (native client)",
719-
secFetchSite: "",
720-
expectedStatusCode: http.StatusOK,
704+
name: "same-origin request succeeds",
705+
secFetchSite: "same-origin",
721706
},
722707
{
723-
name: "bypass protection allows cross-site request",
724-
crossOriginProtection: func() *http.CrossOriginProtection {
725-
p := http.NewCrossOriginProtection()
726-
p.AddInsecureBypassPattern("/")
727-
return p
728-
}(),
729-
secFetchSite: "cross-site",
730-
origin: "https://example.com",
731-
expectedStatusCode: http.StatusOK,
732-
},
733-
{
734-
name: "bypass protection still allows same-origin request",
735-
crossOriginProtection: func() *http.CrossOriginProtection {
736-
p := http.NewCrossOriginProtection()
737-
p.AddInsecureBypassPattern("/")
738-
return p
739-
}(),
740-
secFetchSite: "same-origin",
741-
expectedStatusCode: http.StatusOK,
742-
},
743-
{
744-
name: "bypass protection allows request without Sec-Fetch-Site (native client)",
745-
crossOriginProtection: func() *http.CrossOriginProtection {
746-
p := http.NewCrossOriginProtection()
747-
p.AddInsecureBypassPattern("/")
748-
return p
749-
}(),
750-
secFetchSite: "",
751-
expectedStatusCode: http.StatusOK,
708+
name: "native client without Sec-Fetch-Site succeeds",
709+
secFetchSite: "",
752710
},
753711
}
754712

755713
for _, tt := range tests {
756714
t.Run(tt.name, func(t *testing.T) {
757-
h := newHandler(t, tt.crossOriginProtection)
758-
759715
req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(jsonRPCBody))
760716
req.Header.Set("Content-Type", "application/json")
761717
req.Header.Set("Accept", "application/json, text/event-stream")
@@ -768,9 +724,9 @@ func TestCrossOriginProtection(t *testing.T) {
768724
}
769725

770726
rr := httptest.NewRecorder()
771-
h.ServeHTTP(rr, req)
727+
r.ServeHTTP(rr, req)
772728

773-
assert.Equal(t, tt.expectedStatusCode, rr.Code, "unexpected status code; body: %s", rr.Body.String())
729+
assert.Equal(t, http.StatusOK, rr.Code, "unexpected status code; body: %s", rr.Body.String())
774730
})
775731
}
776732
}

pkg/http/server.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,6 @@ type ServerConfig struct {
8787

8888
// InsidersMode indicates if we should enable experimental features.
8989
InsidersMode bool
90-
91-
// CrossOriginProtection configures the SDK's cross-origin request protection.
92-
// If nil and using RunHTTPServer, cross-origin requests are allowed (auto-bypass).
93-
// If nil and using the handler as a library, the SDK default (reject) applies.
94-
CrossOriginProtection *http.CrossOriginProtection
9590
}
9691

9792
func RunHTTPServer(cfg ServerConfig) error {
@@ -165,14 +160,6 @@ func RunHTTPServer(cfg ServerConfig) error {
165160
serverOptions = append(serverOptions, WithScopeFetcher(scopeFetcher))
166161
}
167162

168-
// Bypass cross-origin protection: this server uses bearer tokens, not
169-
// cookies, so CSRF checks are unnecessary.
170-
if cfg.CrossOriginProtection == nil {
171-
p := http.NewCrossOriginProtection()
172-
p.AddInsecureBypassPattern("/")
173-
cfg.CrossOriginProtection = p
174-
}
175-
176163
r := chi.NewRouter()
177164
handler := NewHTTPMcpHandler(ctx, &cfg, deps, t, logger, apiHost, append(serverOptions, WithFeatureChecker(featureChecker), WithOAuthConfig(oauthCfg))...)
178165
oauthHandler, err := oauth.NewAuthHandler(oauthCfg, apiHost)

0 commit comments

Comments
 (0)