diff --git a/pkg/resolvemcp/README.md b/pkg/resolvemcp/README.md index c258c4e..67dc79a 100644 --- a/pkg/resolvemcp/README.md +++ b/pkg/resolvemcp/README.md @@ -16,6 +16,8 @@ import ( handler := resolvemcp.NewHandlerWithGORM(db, resolvemcp.Config{ BaseURL: "http://localhost:8080", BasePath: "/mcp", + // Read-only by default; uncomment to allow writes: + // ReadOnly: resolvemcp.Bool(false), }) securityList, _ := security.NewSecurityList(provider) @@ -444,12 +446,14 @@ The text appears in `list_tables` and `describe_table`. The server also sends a ## Read-only mode -Set `Config.ReadOnly: true` to disable every write: +The server is **read-only unless you enable writes**: `Config.ReadOnly` is a `*bool` and an unset (nil) value means on. To allow inserts, updates and deletes: ```go -handler := resolvemcp.NewHandlerWithGORM(db, resolvemcp.Config{ReadOnly: true}) +handler := resolvemcp.NewHandlerWithGORM(db, resolvemcp.Config{ReadOnly: resolvemcp.Bool(false)}) ``` +While read-only is on: + - The insert, update, delete and annotation tools are not registered, so the agent never sees them. `list_functions`/`call_function` are off too, because a registered function may change data, unless you set `AllowFunctionCalls` (below). - `list_tables` and `describe_table` report only `select`; `describe_table` also sets `read_only: true` and lists no writable columns. - The MCP server instructions (and the exported catalogue) say the server is read-only and tell the agent not to attempt writes. @@ -460,14 +464,14 @@ handler := resolvemcp.NewHandlerWithGORM(db, resolvemcp.Config{ReadOnly: true}) ```go // Read-only server that may still run two named functions resolvemcp.Config{ - ReadOnly: true, + // ReadOnly is on by default AllowFunctionCalls: true, // keep list_functions / call_function on a read-only server AllowedFunctions: []string{"report_totals", "search_customers"}, } ``` -- `AllowFunctionCalls` only matters with `ReadOnly`; without it, functions are always available. Set it only for functions that do not change data. -- `AllowedFunctions` works with or without `ReadOnly`. When empty, every registered function is allowed. When set, only the named functions are listed and callable; any other is reported as `unknown function`, so its existence is not revealed. Per-function `Authorize` still applies on top. +- `AllowFunctionCalls` only matters while read-only is on; with writes enabled (`ReadOnly: resolvemcp.Bool(false)`), functions are always available. Set it only for functions that do not change data. +- `AllowedFunctions` works in either mode. When empty, every registered function is allowed. When set, only the named functions are listed and callable; any other is reported as `unknown function`, so its existence is not revealed. Per-function `Authorize` still applies on top. ## MCP Tools @@ -729,6 +733,7 @@ The handler resolves table names in priority order: ## Breaking changes +- The server is read-only by default. Writes (insert/update/delete), annotations and function calls need `Config{ReadOnly: resolvemcp.Bool(false)}` (function calls can also be kept on a read-only server with `AllowFunctionCalls`). - Per-model tools (`read_/create_/update_/delete_{schema}_{entity}`) and per-model resources are gone; use the meta tools. - `Setup*` / `NewSSEServer` / `NewStreamableHTTPHandler` take a `*security.SecurityList` and require authentication. `OptionalAuth*` helpers were removed; `*Unauthenticated` variants exist for explicit opt-out. - `resolvespec_annotate` is opt-in via `Config.EnableAnnotations`. diff --git a/pkg/resolvemcp/catalog.go b/pkg/resolvemcp/catalog.go index d4e1139..44dd43c 100644 --- a/pkg/resolvemcp/catalog.go +++ b/pkg/resolvemcp/catalog.go @@ -145,8 +145,8 @@ func (h *Handler) BuildCatalog() Catalog { GeneratedAt: time.Now().UTC(), Server: h.name, Version: h.version, - ReadOnly: h.config.ReadOnly, - Guide: guideFor(h.config.ReadOnly, h.config.AllowFunctionCalls), + ReadOnly: h.config.readOnly, + Guide: guideFor(h.config.readOnly, h.config.AllowFunctionCalls), Limits: CatalogLimits{ DefaultLimit: h.config.DefaultLimit, MaxLimit: h.config.MaxLimit, @@ -179,7 +179,7 @@ func (h *Handler) BuildCatalog() Catalog { for mt != nil && (mt.Kind() == reflect.Pointer || mt.Kind() == reflect.Slice) { mt = mt.Elem() } - if !h.config.ReadOnly && mt != nil && mt.Kind() == reflect.Struct { + if !h.config.readOnly && mt != nil && mt.Kind() == reflect.Struct { for k := range reflectionJSONColumns(mt) { writable[k] = true } diff --git a/pkg/resolvemcp/doc.go b/pkg/resolvemcp/doc.go index f1c99d3..4083dcc 100644 --- a/pkg/resolvemcp/doc.go +++ b/pkg/resolvemcp/doc.go @@ -17,6 +17,9 @@ // // The same guide is sent to MCP clients as the server instructions. // +// The server is read-only by default (Config.ReadOnly nil means on); set +// ReadOnly: resolvemcp.Bool(false) to enable the write tools. +// // # Setting it up // // handler := resolvemcp.NewHandlerWithGORM(db, resolvemcp.Config{BaseURL: "http://localhost:8080"}) diff --git a/pkg/resolvemcp/handler.go b/pkg/resolvemcp/handler.go index 93baa20..92851fb 100644 --- a/pkg/resolvemcp/handler.go +++ b/pkg/resolvemcp/handler.go @@ -44,7 +44,7 @@ func NewHandler(db common.Database, registry common.ModelRegistry, cfg Config) * db: db, registry: registry, hooks: NewHookRegistry(), - mcpServer: server.NewMCPServer("resolvemcp", "1.0.0", server.WithInstructions(guideFor(cfg.ReadOnly, cfg.AllowFunctionCalls))), + mcpServer: server.NewMCPServer("resolvemcp", "1.0.0", server.WithInstructions(guideFor(cfg.withDefaults().readOnly, cfg.AllowFunctionCalls))), config: cfg.withDefaults(), confirms: newConfirmStore(), name: "resolvemcp", @@ -57,7 +57,7 @@ func NewHandler(db common.Database, registry common.ModelRegistry, cfg Config) * } } registerMetaTools(h) - if cfg.EnableAnnotations && !cfg.ReadOnly { + if cfg.EnableAnnotations && !h.config.readOnly { registerAnnotationTool(h) } return h diff --git a/pkg/resolvemcp/meta.go b/pkg/resolvemcp/meta.go index e7e454c..c195a24 100644 --- a/pkg/resolvemcp/meta.go +++ b/pkg/resolvemcp/meta.go @@ -56,10 +56,10 @@ func registerMetaTools(h *Handler) { mcp.WithBoolean("include_count", mcp.Description("Also return the total number of matching rows (slower on large tables).")), ), h.handleSelect) - if !h.config.ReadOnly { + if !h.config.readOnly { registerWriteTools(h, tableArg, idArg, filtersArg, dryRunArg, confirmArg) } - if !h.config.ReadOnly || h.config.AllowFunctionCalls { + if !h.config.readOnly || h.config.AllowFunctionCalls { registerFunctionTools(h, readOnly) } } @@ -130,7 +130,7 @@ func (h *Handler) opsFor(r modelregistry.ModelRules) []string { if r.CanRead { ops = append(ops, opSelect) } - if h.config.ReadOnly { + if h.config.readOnly { return ops } if r.CanCreate { @@ -157,7 +157,7 @@ func (h *Handler) resolveTable(args map[string]any, op string) (schema, entity s if _, err := h.registry.GetModelByEntity(schema, entity); err != nil { return "", "", invalidArg("unknown table %q; see list_tables", truncate(table)) } - if op != "" && op != opSelect && h.config.ReadOnly { + if op != "" && op != opSelect && h.config.readOnly { return "", "", NewClientError(CodeForbidden, "this server is read-only: writes are disabled") } if op != "" { @@ -212,7 +212,7 @@ func (h *Handler) handleDescribeTable(_ context.Context, req mcp.CallToolRequest modelType = modelType.Elem() } writable := map[string]bool{} - if !h.config.ReadOnly && modelType != nil && modelType.Kind() == reflect.Struct { + if !h.config.readOnly && modelType != nil && modelType.Kind() == reflect.Struct { for jsonKey := range reflection.BuildJSONToDBColumnMap(modelType) { writable[jsonKey] = true } @@ -251,7 +251,7 @@ func (h *Handler) handleDescribeTable(_ context.Context, req mcp.CallToolRequest "relations": info.relationNames, "writable_columns": writableNames, "operations": h.opsFor(rules), - "read_only": h.config.ReadOnly, + "read_only": h.config.readOnly, "filter_operators": filterOperators, "limits": map[string]any{ "default_limit": h.config.DefaultLimit, diff --git a/pkg/resolvemcp/readonly_test.go b/pkg/resolvemcp/readonly_test.go index 52115ff..659609d 100644 --- a/pkg/resolvemcp/readonly_test.go +++ b/pkg/resolvemcp/readonly_test.go @@ -13,7 +13,7 @@ import ( func newReadOnlyHandler(t *testing.T) *Handler { t.Helper() h := NewHandler(database.NewPgSQLAdapter(nil), modelregistry.NewModelRegistry(), - Config{ReadOnly: true, EnableAnnotations: true}) + Config{EnableAnnotations: true}) if err := h.RegisterModel("public", "items", &docItem{}); err != nil { t.Fatal(err) } @@ -96,7 +96,7 @@ func newFnHandler(t *testing.T, cfg Config) *Handler { } func TestReadOnlyAllowFunctionCalls(t *testing.T) { - h := newFnHandler(t, Config{ReadOnly: true, AllowFunctionCalls: true}) + h := newFnHandler(t, Config{AllowFunctionCalls: true}) tools := h.mcpServer.ListTools() if tools["list_functions"] == nil || tools["call_function"] == nil { t.Error("function tools must be registered") @@ -147,3 +147,22 @@ func TestAllowedFunctions(t *testing.T) { } } } + +func TestReadOnlyDefaultsOnAndCanBeDisabled(t *testing.T) { + if !(Config{}).withDefaults().readOnly { + t.Error("ReadOnly must default to on") + } + if !(Config{ReadOnly: Bool(true)}).withDefaults().readOnly { + t.Error("explicit true") + } + if (Config{ReadOnly: Bool(false)}).withDefaults().readOnly { + t.Error("Bool(false) must enable writes") + } + h := NewHandler(database.NewPgSQLAdapter(nil), modelregistry.NewModelRegistry(), Config{ReadOnly: Bool(false)}) + if h.mcpServer.ListTools()["insert_into_table"] == nil { + t.Error("write tools must register when ReadOnly is Bool(false)") + } + if strings.Contains(h.BuildCatalog().Guide, "READ-ONLY") { + t.Error("guide must not claim read-only") + } +} diff --git a/pkg/resolvemcp/resolvemcp.go b/pkg/resolvemcp/resolvemcp.go index 5174508..b916abc 100644 --- a/pkg/resolvemcp/resolvemcp.go +++ b/pkg/resolvemcp/resolvemcp.go @@ -51,17 +51,20 @@ type Config struct { // host, with at most 32 distinct base URLs cached; prefer setting BaseURL. AllowedHosts []string - // ReadOnly disables every write. The insert, update, delete and annotation tools are not - // registered, list_tables and describe_table report only the select operation (no - // writable columns), a write attempted anyway is refused with a "forbidden" error, and - // the server instructions tell the agent it cannot write. list_functions/call_function - // are also off, because a registered function may change data, unless AllowFunctionCalls - // is set. - ReadOnly bool + // ReadOnly disables every write and is ON when left nil: set it to Bool(false) to allow + // writes. When on, the insert, update, delete and annotation tools are not registered, + // list_tables and describe_table report only the select operation (no writable columns), + // a write attempted anyway is refused with a "forbidden" error, and the server + // instructions tell the agent it cannot write. list_functions/call_function are also + // off, because a registered function may change data, unless AllowFunctionCalls is set. + ReadOnly *bool + + // readOnly is ReadOnly after defaults (nil means true). + readOnly bool // AllowFunctionCalls keeps list_functions and call_function available on a ReadOnly // server. Only set it for functions that do not change data; pair it with - // AllowedFunctions to name them. It has no effect when ReadOnly is false (functions are + // AllowedFunctions to name them. It has no effect when writes are enabled (ReadOnly set to Bool(false)) (functions are // always available then). AllowFunctionCalls bool @@ -77,6 +80,9 @@ type Config struct { EnableAnnotations bool } +// Bool returns a pointer to v, for the optional boolean fields of Config. +func Bool(v bool) *bool { return &v } + // withDefaults fills the zero limit fields. func (c Config) withDefaults() Config { def := func(v *int, d int) { @@ -93,6 +99,7 @@ func (c Config) withDefaults() Config { if c.DefaultLimit > c.MaxLimit { c.DefaultLimit = c.MaxLimit } + c.readOnly = c.ReadOnly == nil || *c.ReadOnly if c.QueryTimeout <= 0 { c.QueryTimeout = 30 * time.Second } diff --git a/pkg/resolvemcp/security_test.go b/pkg/resolvemcp/security_test.go index 96162ae..7035412 100644 --- a/pkg/resolvemcp/security_test.go +++ b/pkg/resolvemcp/security_test.go @@ -191,7 +191,7 @@ func TestAnnotationToolIsOptIn(t *testing.T) { if h.mcpServer.GetTool(annotationToolName) != nil { t.Fatal("annotation tool must be off by default") } - on := NewHandler(h.db, modelregistry.NewModelRegistry(), Config{EnableAnnotations: true}) + on := NewHandler(h.db, modelregistry.NewModelRegistry(), Config{EnableAnnotations: true, ReadOnly: Bool(false)}) if on.mcpServer.GetTool(annotationToolName) == nil { t.Fatal("annotation tool missing when enabled") } diff --git a/pkg/resolvemcp/tx_test.go b/pkg/resolvemcp/tx_test.go index 899ce62..bd914a7 100644 --- a/pkg/resolvemcp/tx_test.go +++ b/pkg/resolvemcp/tx_test.go @@ -29,7 +29,7 @@ func newTxHarness(t *testing.T) (*Handler, sqlmock.Sqlmock, context.Context) { // connection and fails on the context timeout. db.SetMaxOpenConns(1) t.Cleanup(func() { _ = db.Close() }) - h := NewHandler(database.NewPgSQLAdapter(db), modelregistry.NewModelRegistry(), Config{}) + h := NewHandler(database.NewPgSQLAdapter(db), modelregistry.NewModelRegistry(), Config{ReadOnly: Bool(false)}) if err := h.RegisterModel("public", "items", &txItem{}); err != nil { t.Fatal(err) }