Skip to content

Commit 3c916d2

Browse files
SamMorrowDrumsmanthanghasadiyaCopilot
committed
fix: guard CompletionsHandler against nil params/ref
A malformed completion/complete request with missing or empty parameters caused a nil pointer dereference in CompletionsHandler, panicking the process. Reject such requests with a clear error before dispatching on Ref.Type. Reported by @manthanghasadiya (GHSA-w4q6-qw23-4rg7). Co-authored-by: manthanghasadiya <68530736+manthanghasadiya@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d4e1231 commit 3c916d2

2 files changed

Lines changed: 27 additions & 0 deletions

File tree

pkg/github/server.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,9 @@ func NewServer(version, name, title string, opts *mcp.ServerOptions) *mcp.Server
204204

205205
func CompletionsHandler(getClient GetClientFn) func(ctx context.Context, req *mcp.CompleteRequest) (*mcp.CompleteResult, error) {
206206
return func(ctx context.Context, req *mcp.CompleteRequest) (*mcp.CompleteResult, error) {
207+
if req == nil || req.Params == nil || req.Params.Ref == nil {
208+
return nil, fmt.Errorf("invalid request: missing ref parameter")
209+
}
207210
switch req.Params.Ref.Type {
208211
case "ref/resource":
209212
if strings.HasPrefix(req.Params.Ref.URI, "repo://") {

pkg/github/server_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,3 +349,27 @@ func TestResolveEnabledToolsets(t *testing.T) {
349349
})
350350
}
351351
}
352+
353+
func TestCompletionsHandler_RejectsMissingRef(t *testing.T) {
354+
getClient := func(_ context.Context) (*gogithub.Client, error) {
355+
return &gogithub.Client{}, nil
356+
}
357+
handler := CompletionsHandler(getClient)
358+
359+
tests := []struct {
360+
name string
361+
req *mcp.CompleteRequest
362+
}{
363+
{name: "nil request", req: nil},
364+
{name: "nil params", req: &mcp.CompleteRequest{}},
365+
{name: "nil ref", req: &mcp.CompleteRequest{Params: &mcp.CompleteParams{}}},
366+
}
367+
for _, tc := range tests {
368+
t.Run(tc.name, func(t *testing.T) {
369+
result, err := handler(context.Background(), tc.req)
370+
require.Error(t, err)
371+
assert.Nil(t, result)
372+
assert.Contains(t, err.Error(), "missing ref parameter")
373+
})
374+
}
375+
}

0 commit comments

Comments
 (0)