Files
Maven/internal/store/tools_test.go
T
kami da62a2f25e mcp: pin what a tool was when it was approved
An allowlist row stores cmd ["mcp", server, tool]. That is a late-bound
reference to a name the far end owns, so the row pins nothing about
behaviour: a server could redefine an enabled read-only list_tasks into
something that writes, and Maven would keep calling it with no confirm
turn and no second approval. Discovery now stores a fingerprint of the
declared shape, name, description, input schema and readOnlyHint, and
compares it on every refresh. A mismatch drops the row back to proposed
and, if it stopped claiming read-only, marks it destructive. destructive
is only ever raised. A row predating the column adopts its fingerprint
silently, because an upgrade is not a redefinition.

Nothing retracted a proposal either, so a tool a connected server no
longer offers stayed enabled and failed at call time with an internal
string. Those rows are withdrawn, with provenance saying why, and only
for servers that are actually connected so a restart does not disarm
what he approved.

Argument binding rested on readOnlyHint, which the same server writes.
A server advertising delete_project as read-only got an unconfirmed
argument-carrying call. Binding now also requires the tool be named in
allow_tools, something local, and refuses a required property the schema
never describes rather than guessing it is a string.

wireMCP dialled synchronously from run, and on the passkey path from
inside the unlock handler, so one black-holed endpoint delayed boot and
the answer to an unlock. The first dial happens on the refresh goroutine
under the daemon context. Two servers whose names flatten to one local
allowlist name no longer share a row.

Found in review of #71.
2026-08-01 14:11:57 +04:00

260 lines
9.0 KiB
Go

package store
import (
"context"
"errors"
"testing"
"time"
)
// TestToolLifecycle covers propose → enable → disable, the states the authed
// /tools page drives. Disable must revert an enabled tool to 'proposed' (kept
// in the store, won't run) and be idempotent.
func TestToolLifecycle(t *testing.T) {
s := newTestStore(t)
ctx := context.Background()
now := time.Now()
// Propose with empty scope → defaults to "homelab".
if _, err := s.ProposeTool(ctx, "restart_svc", "restart the service", "", now); err != nil {
t.Fatalf("propose: %v", err)
}
if err := s.EnableTool(ctx, "restart_svc", []string{"systemctl", "restart", "x"}, true, "", now); err != nil {
t.Fatalf("enable: %v", err)
}
tl, _ := s.LookupTool(ctx, "restart_svc")
if tl.Status != "enabled" {
t.Fatalf("after enable: status=%q want enabled", tl.Status)
}
if tl.Scope != "homelab" {
t.Fatalf("after enable: scope=%q want homelab", tl.Scope)
}
// Propose with explicit scope.
if _, err := s.ProposeTool(ctx, "reboot", "reboot the server", "datacenter", now); err != nil {
t.Fatalf("propose with scope: %v", err)
}
if err := s.EnableTool(ctx, "reboot", []string{"reboot"}, true, "datacenter", now); err != nil {
t.Fatalf("enable with scope: %v", err)
}
tl2, _ := s.LookupTool(ctx, "reboot")
if tl2.Scope != "datacenter" {
t.Fatalf("explicit scope: %q want datacenter", tl2.Scope)
}
if err := s.DisableTool(ctx, "restart_svc"); err != nil {
t.Fatalf("disable: %v", err)
}
tl, err := s.LookupTool(ctx, "restart_svc")
if err != nil {
t.Fatalf("lookup after disable: %v", err)
}
if tl.Status != "proposed" {
t.Fatalf("after disable: status=%q want proposed", tl.Status)
}
// idempotent: disabling an already-proposed (or absent) tool is a no-op.
if err := s.DisableTool(ctx, "restart_svc"); err != nil {
t.Fatalf("disable idempotent: %v", err)
}
if err := s.DisableTool(ctx, "does_not_exist"); err != nil {
t.Fatalf("disable absent must be no-op: %v", err)
}
}
// A discovered MCP tool arrives as a proposal that already knows its cmd, so
// enabling it is one click rather than one retyped argv.
func TestProposeMCPTool(t *testing.T) {
s := newTestStore(t)
ctx := context.Background()
now := time.Now()
cmd := []string{"mcp", "vikunja", "list_tasks"}
fresh, err := s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", cmd, false, "mcp vikunja/list_tasks: List tasks", "fp1", now)
if err != nil {
t.Fatal(err)
}
if !fresh {
t.Fatal("first proposal should be new")
}
got, err := s.LookupTool(ctx, "vikunja_list_tasks")
if err != nil {
t.Fatal(err)
}
if got.Status != "proposed" {
t.Fatalf("status = %q — discovery must never enable", got.Status)
}
if len(got.Cmd) != 3 || got.Cmd[0] != "mcp" || got.Cmd[2] != "list_tasks" {
t.Fatalf("cmd = %v", got.Cmd)
}
if got.Scope != "mcp:vikunja" || got.Utterance == "" {
t.Fatalf("provenance lost: %+v", got)
}
// Re-discovery on the next boot is idempotent.
fresh, err = s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", cmd, true, "changed", "fp1", now)
if err != nil {
t.Fatal(err)
}
if fresh {
t.Error("re-proposing an existing row must report nothing new")
}
// And it must not re-arm or rewrite a row a human already acted on.
if err := s.EnableTool(ctx, "vikunja_list_tasks", cmd, false, "mcp:vikunja", now); err != nil {
t.Fatal(err)
}
if _, err := s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", []string{"mcp", "vikunja", "delete_task"}, true, "x", "fp2", now); err != nil {
t.Fatal(err)
}
got, err = s.LookupTool(ctx, "vikunja_list_tasks")
if err != nil {
t.Fatal(err)
}
if got.Status != "enabled" || got.Cmd[2] != "list_tasks" || got.Destructive {
t.Fatalf("an enabled row was modified by discovery: %+v", got)
}
}
func TestProposeMCPToolNeedsCmd(t *testing.T) {
s := newTestStore(t)
if _, err := s.ProposeMCPTool(context.Background(), "x", "mcp:y", nil, false, "", "", time.Now()); !errors.Is(err, ErrToolCmd) {
t.Fatalf("err = %v, want ErrToolCmd", err)
}
}
// A server that redefines a tool Kami already approved must have to ask again.
// The row stores cmd ["mcp", server, tool], a late-bound reference to a name
// the far end owns, so before the fingerprint a server could turn an enabled
// read-only list_tasks into something that writes and Maven would keep running
// it without a confirm turn.
func TestReconcileMCPToolDemotesARedefinedTool(t *testing.T) {
s := newTestStore(t)
ctx := context.Background()
now := time.Now()
cmd := []string{"mcp", "vikunja", "list_tasks"}
if _, err := s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", cmd, false, "read only", "fp1", now); err != nil {
t.Fatal(err)
}
if err := s.EnableTool(ctx, "vikunja_list_tasks", cmd, false, "mcp:vikunja", now); err != nil {
t.Fatal(err)
}
// Same shape ⇒ nothing happens. Discovery runs every minute and must be
// idempotent.
ch, err := s.ReconcileMCPTool(ctx, "vikunja_list_tasks", "fp1", false, "read only", now)
if err != nil {
t.Fatal(err)
}
if ch.Changed {
t.Fatalf("an unchanged tool must not be touched: %+v", ch)
}
if got, _ := s.LookupTool(ctx, "vikunja_list_tasks"); got.Status != "enabled" {
t.Fatalf("status = %q, want it left enabled", got.Status)
}
// It stopped claiming read-only and its schema moved.
ch, err = s.ReconcileMCPTool(ctx, "vikunja_list_tasks", "fp2", true, "now writes", now)
if err != nil {
t.Fatal(err)
}
if !ch.Changed || !ch.Demoted || !ch.Escalated {
t.Fatalf("change = %+v, want changed+demoted+escalated", ch)
}
got, err := s.LookupTool(ctx, "vikunja_list_tasks")
if err != nil {
t.Fatal(err)
}
if got.Status != "proposed" {
t.Errorf("status = %q, want a redefined tool back in the queue", got.Status)
}
if !got.Destructive {
t.Error("a tool that stopped claiming read-only must gain the confirm turn")
}
if got.Utterance != "now writes" {
t.Errorf("utterance = %q, want what the server says today", got.Utterance)
}
// destructive is only ever raised. The server that changed underneath us
// does not get to relax it by claiming read-only next time.
if _, err := s.ReconcileMCPTool(ctx, "vikunja_list_tasks", "fp3", false, "read only again", now); err != nil {
t.Fatal(err)
}
if got, _ = s.LookupTool(ctx, "vikunja_list_tasks"); !got.Destructive {
t.Error("destructive was relaxed by the server")
}
}
// A row written before fingerprints exist simply adopts one. An upgrade is not
// a redefinition and must not disable everything Kami approved.
func TestReconcileMCPToolAdoptsAnEmptyFingerprint(t *testing.T) {
s := newTestStore(t)
ctx := context.Background()
now := time.Now()
cmd := []string{"mcp", "vikunja", "list_tasks"}
if _, err := s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", cmd, false, "x", "", now); err != nil {
t.Fatal(err)
}
if err := s.EnableTool(ctx, "vikunja_list_tasks", cmd, false, "mcp:vikunja", now); err != nil {
t.Fatal(err)
}
ch, err := s.ReconcileMCPTool(ctx, "vikunja_list_tasks", "fp1", false, "x", now)
if err != nil {
t.Fatal(err)
}
if ch.Changed {
t.Fatalf("adopting must be silent: %+v", ch)
}
if got, _ := s.LookupTool(ctx, "vikunja_list_tasks"); got.Status != "enabled" {
t.Fatalf("status = %q, want still enabled after the upgrade", got.Status)
}
// And now it is pinned.
if ch, _ = s.ReconcileMCPTool(ctx, "vikunja_list_tasks", "fp2", false, "y", now); !ch.Changed {
t.Fatal("the adopted fingerprint must be enforced on the next pass")
}
}
func TestReconcileMCPToolUnknownRow(t *testing.T) {
s := newTestStore(t)
if _, err := s.ReconcileMCPTool(context.Background(), "nope", "fp", false, "", time.Now()); !errors.Is(err, ErrToolNotFound) {
t.Fatalf("err = %v, want ErrToolNotFound", err)
}
}
// A tool the server stopped offering must be disarmed and must say why. It used
// to stay enabled and fail at call time with an internal string, and /tools —
// the one place he would look — did not mention it.
func TestWithdrawTool(t *testing.T) {
s := newTestStore(t)
ctx := context.Background()
now := time.Now()
cmd := []string{"mcp", "vikunja", "list_tasks"}
if _, err := s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", cmd, false, "x", "fp1", now); err != nil {
t.Fatal(err)
}
if err := s.EnableTool(ctx, "vikunja_list_tasks", cmd, false, "mcp:vikunja", now); err != nil {
t.Fatal(err)
}
was, err := s.WithdrawTool(ctx, "vikunja_list_tasks", "gone", now)
if err != nil {
t.Fatal(err)
}
if !was {
t.Error("withdrawing an enabled tool must report that it was enabled")
}
got, err := s.LookupTool(ctx, "vikunja_list_tasks")
if err != nil {
t.Fatal(err)
}
if got.Status != "proposed" || got.Utterance != "gone" {
t.Fatalf("row = %+v, want proposed and saying why", got)
}
// Withdrawing again is not an error and does not claim it was enabled.
if was, err = s.WithdrawTool(ctx, "vikunja_list_tasks", "still gone", now); err != nil || was {
t.Fatalf("second withdraw = %v, %v", was, err)
}
if got, _ = s.LookupTool(ctx, "vikunja_list_tasks"); got.Utterance != "still gone" {
t.Errorf("utterance = %q, want the provenance refreshed anyway", got.Utterance)
}
}