[pre-check] knative/func#3708 rebased (mcp: readonly guard for config add/remove)

pkg/mcp/instructions_warning.md

@@ -8,21 +8,22 @@ The Functions MCP server is currently running in **read-only mode**.
 
 **Available operations:**
 - Create Functions
-- Build Functions
-- Configure Functions (envs, labels, volumes)
+- Build Functions (without pushing the image)
 - Inspect Functions
+- List Function configuration
 - Run/stop Functions locally (not cluster operations, so unaffected by read-only mode)
 
 **Disabled operations:**
+- Configure Functions (envs, labels, volumes add/remove)
 - Deploy to cluster
 - Delete from cluster
 - Push a built image to a registry (`build` with `push` enabled)
 
-These write operations are disabled to prevent unintended cluster modifications.
+These write operations are disabled to prevent unintended local or cluster modifications.
 
 ## Enabling Write Mode
 
-If the user needs to deploy or delete Functions, or push a built image to a registry, you MUST inform them to enable write mode:
+If the user needs to deploy, delete, or configure Functions, or push a built image to a registry, you MUST inform them to enable write mode:
 
 1. Close/exit this application completely
 2. Set the environment variable: `FUNC_ENABLE_MCP_WRITE=true`

pkg/mcp/mcp.go

@@ -30,7 +30,7 @@ const (
 type Server struct {
 	OnInit    func(context.Context) // Invoked when the server is initialized
 	prefix    string                // Command prefix ("func" or "kn func")
-	readonly  atomic.Bool           // disables deploy, delete, and pushing built images when true
+	readonly  atomic.Bool           // disables mutation tools when true
 	executor  executor
 	transport mcp.Transport  // Transport to use (defaults to StdioTransport)
 	impl      *mcp.Server    // implements the protocol

pkg/mcp/tools_config_envs.go

@@ -142,6 +142,10 @@ type ConfigEnvsAddOutput struct {
 }
 
 func (s *Server) configEnvsAddHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigEnvsAddInput) (result *mcp.CallToolResult, output ConfigEnvsAddOutput, err error) {
+	if s.readonly.Load() {
+		err = fmt.Errorf("the server is currently in readonly mode. Please set %s and restart the MCP server", EnvMCPWrite)
+		return
+	}
 	if err = input.validate(); err != nil {
 		return
 	}
@@ -186,6 +190,10 @@ type ConfigEnvsRemoveOutput struct {
 }
 
 func (s *Server) configEnvsRemoveHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigEnvsRemoveInput) (result *mcp.CallToolResult, output ConfigEnvsRemoveOutput, err error) {
+	if s.readonly.Load() {
+		err = fmt.Errorf("the server is currently in readonly mode. Please set %s and restart the MCP server", EnvMCPWrite)
+		return
+	}
 	out, err := s.executor.Execute(ctx, "config", input.Args()...)
 	if err != nil {
 		err = fmt.Errorf("%w\n%s", err, string(out))

pkg/mcp/tools_config_envs_test.go

@@ -3,6 +3,7 @@ package mcp
 import (
 	"context"
 	"errors"
+	"strings"
 	"testing"
 
 	"github.com/modelcontextprotocol/go-sdk/mcp"
@@ -627,3 +628,61 @@ func TestTool_ConfigEnvsRemove_Error(t *testing.T) {
 		t.Fatal("expected error result, got success")
 	}
 }
+
+// TestTool_ConfigEnvsAdd_Readonly ensures the config_envs_add tool is blocked in readonly mode.
+func TestTool_ConfigEnvsAdd_Readonly(t *testing.T) {
+	executor := mock.NewExecutor()
+
+	client, server, err := newTestPair(t, WithExecutor(executor))
+	if err != nil {
+		t.Fatal(err)
+	}
+	server.readonly.Store(true)
+
+	result, err := client.CallTool(t.Context(), &mcp.CallToolParams{
+		Name:      "config_envs_add",
+		Arguments: map[string]any{"path": ".", "name": "KEY", "value": "VAL"},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !result.IsError {
+		t.Fatal("expected error result in readonly mode, got success")
+	}
+	got := resultToString(result)
+	if !strings.Contains(got, "readonly mode") {
+		t.Fatalf("expected readonly-mode error, got: %q", got)
+	}
+	if executor.ExecuteInvoked {
+		t.Fatal("executor should not be invoked in readonly mode")
+	}
+}
+
+// TestTool_ConfigEnvsRemove_Readonly ensures the config_envs_remove tool is blocked in readonly mode.
+func TestTool_ConfigEnvsRemove_Readonly(t *testing.T) {
+	executor := mock.NewExecutor()
+
+	client, server, err := newTestPair(t, WithExecutor(executor))
+	if err != nil {
+		t.Fatal(err)
+	}
+	server.readonly.Store(true)
+
+	result, err := client.CallTool(t.Context(), &mcp.CallToolParams{
+		Name:      "config_envs_remove",
+		Arguments: map[string]any{"path": ".", "name": "KEY"},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !result.IsError {
+		t.Fatal("expected error result in readonly mode, got success")
+	}
+	got := resultToString(result)
+	if !strings.Contains(got, "readonly mode") {
+		t.Fatalf("expected readonly-mode error, got: %q", got)
+	}
+	if executor.ExecuteInvoked {
+		t.Fatal("executor should not be invoked in readonly mode")
+	}
+}

pkg/mcp/tools_config_labels.go

@@ -79,6 +79,10 @@ type ConfigLabelsAddOutput struct {
 }
 
 func (s *Server) configLabelsAddHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigLabelsAddInput) (result *mcp.CallToolResult, output ConfigLabelsAddOutput, err error) {
+	if s.readonly.Load() {
+		err = fmt.Errorf("the server is currently in readonly mode. Please set %s and restart the MCP server", EnvMCPWrite)
+		return
+	}
 	out, err := s.executor.Execute(ctx, "config", input.Args()...)
 	if err != nil {
 		err = fmt.Errorf("%w\n%s", err, string(out))
@@ -120,6 +124,10 @@ type ConfigLabelsRemoveOutput struct {
 }
 
 func (s *Server) configLabelsRemoveHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigLabelsRemoveInput) (result *mcp.CallToolResult, output ConfigLabelsRemoveOutput, err error) {
+	if s.readonly.Load() {
+		err = fmt.Errorf("the server is currently in readonly mode. Please set %s and restart the MCP server", EnvMCPWrite)
+		return
+	}
 	out, err := s.executor.Execute(ctx, "config", input.Args()...)
 	if err != nil {
 		err = fmt.Errorf("%w\n%s", err, string(out))

pkg/mcp/tools_config_labels_test.go

@@ -3,6 +3,7 @@ package mcp
 import (
 	"context"
 	"errors"
+	"strings"
 	"testing"
 
 	"github.com/modelcontextprotocol/go-sdk/mcp"
@@ -242,3 +243,61 @@ func TestTool_ConfigLabelsRemove_Error(t *testing.T) {
 		t.Fatal("expected error result, got success")
 	}
 }
+
+// TestTool_ConfigLabelsAdd_Readonly ensures the config_labels_add tool is blocked in readonly mode.
+func TestTool_ConfigLabelsAdd_Readonly(t *testing.T) {
+	executor := mock.NewExecutor()
+
+	client, server, err := newTestPair(t, WithExecutor(executor))
+	if err != nil {
+		t.Fatal(err)
+	}
+	server.readonly.Store(true)
+
+	result, err := client.CallTool(t.Context(), &mcp.CallToolParams{
+		Name:      "config_labels_add",
+		Arguments: map[string]any{"path": ".", "name": "app", "value": "test"},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !result.IsError {
+		t.Fatal("expected error result in readonly mode, got success")
+	}
+	got := resultToString(result)
+	if !strings.Contains(got, "readonly mode") {
+		t.Fatalf("expected readonly-mode error, got: %q", got)
+	}
+	if executor.ExecuteInvoked {
+		t.Fatal("executor should not be invoked in readonly mode")
+	}
+}
+
+// TestTool_ConfigLabelsRemove_Readonly ensures the config_labels_remove tool is blocked in readonly mode.
+func TestTool_ConfigLabelsRemove_Readonly(t *testing.T) {
+	executor := mock.NewExecutor()
+
+	client, server, err := newTestPair(t, WithExecutor(executor))
+	if err != nil {
+		t.Fatal(err)
+	}
+	server.readonly.Store(true)
+
+	result, err := client.CallTool(t.Context(), &mcp.CallToolParams{
+		Name:      "config_labels_remove",
+		Arguments: map[string]any{"path": ".", "name": "app"},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !result.IsError {
+		t.Fatal("expected error result in readonly mode, got success")
+	}
+	got := resultToString(result)
+	if !strings.Contains(got, "readonly mode") {
+		t.Fatalf("expected readonly-mode error, got: %q", got)
+	}
+	if executor.ExecuteInvoked {
+		t.Fatal("executor should not be invoked in readonly mode")
+	}
+}

pkg/mcp/tools_config_volumes.go

@@ -87,6 +87,10 @@ type ConfigVolumesAddOutput struct {
 }
 
 func (s *Server) configVolumesAddHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigVolumesAddInput) (result *mcp.CallToolResult, output ConfigVolumesAddOutput, err error) {
+	if s.readonly.Load() {
+		err = fmt.Errorf("the server is currently in readonly mode. Please set %s and restart the MCP server", EnvMCPWrite)
+		return
+	}
 	out, err := s.executor.Execute(ctx, "config", input.Args()...)
 	if err != nil {
 		err = fmt.Errorf("%w\n%s", err, string(out))
@@ -128,6 +132,10 @@ type ConfigVolumesRemoveOutput struct {
 }
 
 func (s *Server) configVolumesRemoveHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigVolumesRemoveInput) (result *mcp.CallToolResult, output ConfigVolumesRemoveOutput, err error) {
+	if s.readonly.Load() {
+		err = fmt.Errorf("the server is currently in readonly mode. Please set %s and restart the MCP server", EnvMCPWrite)
+		return
+	}
 	out, err := s.executor.Execute(ctx, "config", input.Args()...)
 	if err != nil {
 		err = fmt.Errorf("%w\n%s", err, string(out))

pkg/mcp/tools_config_volumes_test.go

@@ -3,6 +3,7 @@ package mcp
 import (
 	"context"
 	"errors"
+	"strings"
 	"testing"
 
 	"github.com/modelcontextprotocol/go-sdk/mcp"
@@ -246,3 +247,61 @@ func TestTool_ConfigVolumesRemove_Error(t *testing.T) {
 		t.Fatal("expected error result, got success")
 	}
 }
+
+// TestTool_ConfigVolumesAdd_Readonly ensures the config_volumes_add tool is blocked in readonly mode.
+func TestTool_ConfigVolumesAdd_Readonly(t *testing.T) {
+	executor := mock.NewExecutor()
+
+	client, server, err := newTestPair(t, WithExecutor(executor))
+	if err != nil {
+		t.Fatal(err)
+	}
+	server.readonly.Store(true)
+
+	result, err := client.CallTool(t.Context(), &mcp.CallToolParams{
+		Name:      "config_volumes_add",
+		Arguments: map[string]any{"path": "."},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !result.IsError {
+		t.Fatal("expected error result in readonly mode, got success")
+	}
+	got := resultToString(result)
+	if !strings.Contains(got, "readonly mode") {
+		t.Fatalf("expected readonly-mode error, got: %q", got)
+	}
+	if executor.ExecuteInvoked {
+		t.Fatal("executor should not be invoked in readonly mode")
+	}
+}
+
+// TestTool_ConfigVolumesRemove_Readonly ensures the config_volumes_remove tool is blocked in readonly mode.
+func TestTool_ConfigVolumesRemove_Readonly(t *testing.T) {
+	executor := mock.NewExecutor()
+
+	client, server, err := newTestPair(t, WithExecutor(executor))
+	if err != nil {
+		t.Fatal(err)
+	}
+	server.readonly.Store(true)
+
+	result, err := client.CallTool(t.Context(), &mcp.CallToolParams{
+		Name:      "config_volumes_remove",
+		Arguments: map[string]any{"path": "."},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !result.IsError {
+		t.Fatal("expected error result in readonly mode, got success")
+	}
+	got := resultToString(result)
+	if !strings.Contains(got, "readonly mode") {
+		t.Fatalf("expected readonly-mode error, got: %q", got)
+	}
+	if executor.ExecuteInvoked {
+		t.Fatal("executor should not be invoked in readonly mode")
+	}
+}