From c47eb3bd1a2e6efd2a3d647b47d35d5090f3c241 Mon Sep 17 00:00:00 2001 From: "elvandlie@gmail.com" Date: Sat, 16 May 2026 18:40:17 +0700 Subject: [PATCH] fix(mcp): use atomic.Bool for Server.readonly to prevent data races The Server.readonly field is a plain bool with no synchronization. While the current stdio transport is single-client, the MCP SDK may dispatch tool calls on separate goroutines internally. Additionally, Start() writes to readonly while handler goroutines may concurrently read it, which is a data race under the Go memory model. Replace bool with sync/atomic.Bool for lock-free thread safety. All reads use Load() and all writes use Store(). Also add test coverage for the readonly rejection path in both the deploy and delete handlers. Signed-off-by: Elvand Lie Signed-off-by: elvandlie@gmail.com --- pkg/mcp/mcp.go | 9 +++++---- pkg/mcp/tools_delete.go | 2 +- pkg/mcp/tools_delete_test.go | 21 ++++++++++++++++++++- pkg/mcp/tools_deploy.go | 2 +- pkg/mcp/tools_deploy_test.go | 21 ++++++++++++++++++++- 5 files changed, 47 insertions(+), 8 deletions(-) diff --git a/pkg/mcp/mcp.go b/pkg/mcp/mcp.go index a0c14496e2..0521bbcecb 100644 --- a/pkg/mcp/mcp.go +++ b/pkg/mcp/mcp.go @@ -4,6 +4,7 @@ import ( "context" "os/exec" "strings" + "sync/atomic" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -22,7 +23,7 @@ const ( type Server struct { OnInit func(context.Context) // Invoked when the server is initialized prefix string // Command prefix ("func" or "kn func") - readonly bool // disables deploy and delete when true + readonly atomic.Bool // disables deploy and delete when true executor executor transport mcp.Transport // Transport to use (defaults to StdioTransport) impl *mcp.Server // implements the protocol @@ -58,7 +59,7 @@ func WithTransport(transport mcp.Transport) Option { // WithReadonly sets the server to readonly mode. func WithReadonly(readonly bool) Option { return func(s *Server) { - s.readonly = readonly + s.readonly.Store(readonly) } } @@ -80,7 +81,7 @@ func New(options ...Option) *Server { Title: title, Version: version}, &mcp.ServerOptions{ - Instructions: instructions(s.readonly), + Instructions: instructions(s.readonly.Load()), HasPrompts: true, HasResources: true, HasTools: true, @@ -146,7 +147,7 @@ func New(options ...Option) *Server { // Start the MCP server using the configured transport func (s *Server) Start(ctx context.Context, writeEnabled bool) error { - s.readonly = !writeEnabled + s.readonly.Store(!writeEnabled) return s.impl.Run(ctx, s.transport) } diff --git a/pkg/mcp/tools_delete.go b/pkg/mcp/tools_delete.go index c70f8e8a8b..27760f864d 100644 --- a/pkg/mcp/tools_delete.go +++ b/pkg/mcp/tools_delete.go @@ -20,7 +20,7 @@ var deleteTool = &mcp.Tool{ } func (s *Server) deleteHandler(ctx context.Context, r *mcp.CallToolRequest, input DeleteInput) (result *mcp.CallToolResult, output DeleteOutput, err error) { - if s.readonly { + if s.readonly.Load() { err = fmt.Errorf("the server is currently in readonly mode. Please set FUNC_ENABLE_MCP_WRITE and restart the client") return } diff --git a/pkg/mcp/tools_delete_test.go b/pkg/mcp/tools_delete_test.go index 52bd8a9741..58f5769821 100644 --- a/pkg/mcp/tools_delete_test.go +++ b/pkg/mcp/tools_delete_test.go @@ -55,7 +55,7 @@ func TestTool_Delete_Args(t *testing.T) { t.Fatal(err) } // Delete requires write mode - enable it for this test - server.readonly = false + server.readonly.Store(false) // Build input arguments from test data inputArgs := buildInputArgs(stringFlags, boolFlags) @@ -76,3 +76,22 @@ func TestTool_Delete_Args(t *testing.T) { t.Fatal("executor was not invoked") } } + +// TestTool_Delete_Readonly ensures the delete tool rejects requests in readonly mode. +func TestTool_Delete_Readonly(t *testing.T) { + client, _, err := newTestPairWithReadonly(t, true) // readonly = true + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "delete", + Arguments: map[string]any{"name": "my-function"}, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatal("expected delete to be rejected in readonly mode") + } +} diff --git a/pkg/mcp/tools_deploy.go b/pkg/mcp/tools_deploy.go index 518052130f..09c7231656 100644 --- a/pkg/mcp/tools_deploy.go +++ b/pkg/mcp/tools_deploy.go @@ -20,7 +20,7 @@ var deployTool = &mcp.Tool{ } func (s *Server) deployHandler(ctx context.Context, r *mcp.CallToolRequest, input DeployInput) (result *mcp.CallToolResult, output DeployOutput, err error) { - if s.readonly { + if s.readonly.Load() { err = fmt.Errorf("the server is currently in readonly mode. Please set FUNC_ENABLE_MCP_WRITE and restart the client") return } diff --git a/pkg/mcp/tools_deploy_test.go b/pkg/mcp/tools_deploy_test.go index 4b8d2e83c4..5ec697622d 100644 --- a/pkg/mcp/tools_deploy_test.go +++ b/pkg/mcp/tools_deploy_test.go @@ -59,7 +59,7 @@ func TestTool_Deploy_Args(t *testing.T) { t.Fatal(err) } // Deploy requires write mode - enable it for this test - server.readonly = false + server.readonly.Store(false) // Build input arguments from test data inputArgs := buildInputArgs(stringFlags, boolFlags) @@ -79,3 +79,22 @@ func TestTool_Deploy_Args(t *testing.T) { t.Fatal("executor was not invoked") } } + +// TestTool_Deploy_Readonly ensures the deploy tool rejects requests in readonly mode. +func TestTool_Deploy_Readonly(t *testing.T) { + client, _, err := newTestPairWithReadonly(t, true) // readonly = true + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "deploy", + Arguments: map[string]any{"path": "."}, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatal("expected deploy to be rejected in readonly mode") + } +}