Skip to content

Commit ca734c5

Browse files
fix(http): make --read-only flag work in HTTP mode
The --read-only flag and GITHUB_READ_ONLY environment variable were being completely ignored in HTTP mode, causing write tools like create_branch, create_pull_request, and merge_pull_request to remain accessible and functional even when read-only mode was requested. This fix: - Adds ReadOnly field to ServerConfig struct - Passes the --read-only flag from main.go to HTTP config - Creates withGlobalReadonly middleware to enforce readonly mode globally - Adds test coverage for the global readonly configuration Issue #2156
1 parent 1da41fa commit ca734c5

File tree

3 files changed

+69
-0
lines changed

3 files changed

+69
-0
lines changed

cmd/github-mcp-server/main.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ var (
119119
LockdownMode: viper.GetBool("lockdown-mode"),
120120
RepoAccessCacheTTL: &ttl,
121121
ScopeChallenge: viper.GetBool("scope-challenge"),
122+
ReadOnly: viper.GetBool("read-only"),
122123
}
123124

124125
return ghhttp.RunHTTPServer(httpConfig)

pkg/http/handler_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,3 +409,51 @@ func TestHTTPHandlerRoutes(t *testing.T) {
409409
})
410410
}
411411
}
412+
413+
func TestWithGlobalReadonly(t *testing.T) {
414+
tests := []struct {
415+
name string
416+
readonly bool
417+
expectReadonly bool
418+
}{
419+
{
420+
name: "readonly config enables readonly mode",
421+
readonly: true,
422+
expectReadonly: true,
423+
},
424+
{
425+
name: "non-readonly config leaves readonly disabled",
426+
readonly: false,
427+
expectReadonly: false,
428+
},
429+
}
430+
431+
for _, tt := range tests {
432+
t.Run(tt.name, func(t *testing.T) {
433+
cfg := &ServerConfig{
434+
Version: "test",
435+
ReadOnly: tt.readonly,
436+
}
437+
438+
middleware := withGlobalReadonly(cfg)
439+
440+
// Create a handler that will check the context
441+
var contextReadonly bool
442+
next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
443+
contextReadonly = ghcontext.IsReadonly(r.Context())
444+
w.WriteHeader(http.StatusOK)
445+
})
446+
447+
// Apply middleware
448+
handler := middleware(next)
449+
450+
// Create request and execute
451+
req := httptest.NewRequest(http.MethodGet, "/", nil)
452+
rr := httptest.NewRecorder()
453+
handler.ServeHTTP(rr, req)
454+
455+
// Verify the readonly state matches the config
456+
assert.Equal(t, tt.expectReadonly, contextReadonly, "readonly context should match config")
457+
})
458+
}
459+
}

pkg/http/server.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ type ServerConfig struct {
6767
// ScopeChallenge indicates if we should return OAuth scope challenges, and if we should perform
6868
// tool filtering based on token scopes.
6969
ScopeChallenge bool
70+
71+
// ReadOnly indicates if the server should run in read-only mode globally
72+
ReadOnly bool
7073
}
7174

7275
func RunHTTPServer(cfg ServerConfig) error {
@@ -145,6 +148,9 @@ func RunHTTPServer(cfg ServerConfig) error {
145148
// Register Middleware First, needs to be before route registration
146149
handler.RegisterMiddleware(r)
147150

151+
// Apply global readonly setting from server config
152+
r.Use(withGlobalReadonly(&cfg))
153+
148154
// Register MCP server routes
149155
handler.RegisterRoutes(r)
150156
})
@@ -219,3 +225,17 @@ func createHTTPFeatureChecker() inventory.FeatureFlagChecker {
219225
return false, nil
220226
}
221227
}
228+
229+
// withGlobalReadonly is middleware that applies the server's global readonly setting
230+
func withGlobalReadonly(cfg *ServerConfig) func(http.Handler) http.Handler {
231+
return func(next http.Handler) http.Handler {
232+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
233+
if cfg.ReadOnly {
234+
ctx := ghcontext.WithReadonly(r.Context(), true)
235+
next.ServeHTTP(w, r.WithContext(ctx))
236+
} else {
237+
next.ServeHTTP(w, r)
238+
}
239+
})
240+
}
241+
}

0 commit comments

Comments
 (0)