kami
da62a2f25e
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.
97 lines
2.9 KiB
Go
97 lines
2.9 KiB
Go
package main
|
|
|
|
import (
|
|
"context"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/kami/maven/internal/config"
|
|
)
|
|
|
|
func TestWireMCPOffWhenUnconfigured(t *testing.T) {
|
|
st := newTestStore(t)
|
|
for name, cfg := range map[string]*config.Config{
|
|
"no block": {},
|
|
"nothing enabled": {MCP: &config.MCPConfig{Servers: []config.MCPServerConfig{
|
|
{Name: "vikunja", URL: "http://192.168.1.104:9100/mcp"},
|
|
}}},
|
|
} {
|
|
t.Run(name, func(t *testing.T) {
|
|
if w := wireMCP(cfg, st); w != nil {
|
|
t.Fatal("MCP must be off unless a server is configured AND enabled")
|
|
}
|
|
})
|
|
}
|
|
// nil wiring must be safe to use everywhere it is reachable.
|
|
var w *mcpWiring
|
|
w.close()
|
|
w.propose(context.Background())
|
|
if w.status() != nil || w.caller() != nil {
|
|
t.Fatal("a nil wiring must report nothing")
|
|
}
|
|
}
|
|
|
|
// Wiring must not dial. Boot used to block for the whole per-server timeout
|
|
// budget on a black-holed endpoint, and on the passkey path that delay landed
|
|
// inside the unlock handler.
|
|
func TestWireMCPDoesNotDial(t *testing.T) {
|
|
st := newTestStore(t)
|
|
w := wireMCP(&config.Config{MCP: &config.MCPConfig{Servers: []config.MCPServerConfig{{
|
|
Name: "dead", Command: "/nonexistent/mcp-server", Enabled: true,
|
|
}}}}, st)
|
|
if w == nil {
|
|
t.Fatal("a configured server should wire")
|
|
}
|
|
defer w.close()
|
|
if s := w.status(); len(s) != 1 || s[0].Err != "" {
|
|
t.Fatalf("wireMCP dialled: %+v", s)
|
|
}
|
|
}
|
|
|
|
// An unreachable server must not stop the daemon, must be reported as down, and
|
|
// must propose nothing.
|
|
func TestWireMCPUnreachableServerIsNotFatal(t *testing.T) {
|
|
st := newTestStore(t)
|
|
w := wireMCP(&config.Config{MCP: &config.MCPConfig{Servers: []config.MCPServerConfig{{
|
|
Name: "dead", Command: "/nonexistent/mcp-server", Enabled: true,
|
|
}}}}, st)
|
|
if w == nil {
|
|
t.Fatal("a configured server should still wire")
|
|
}
|
|
defer w.close()
|
|
w.connect(context.Background())
|
|
st2 := w.status()
|
|
if len(st2) != 1 || st2[0].Connected || st2[0].Err == "" {
|
|
t.Fatalf("status = %+v", st2)
|
|
}
|
|
tools, err := st.ListTools(context.Background(), "")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if len(tools) != 0 {
|
|
t.Fatalf("a server that never answered must propose nothing, got %+v", tools)
|
|
}
|
|
}
|
|
|
|
// A url server whose address is private is refused by webfetch unless that
|
|
// server sets allow_private. This is the guard the whole MCP path rides on, so
|
|
// it is asserted here too, at the wiring level.
|
|
func TestWireMCPPrivateURLRefusedWithoutAllowPrivate(t *testing.T) {
|
|
st := newTestStore(t)
|
|
w := wireMCP(&config.Config{MCP: &config.MCPConfig{Servers: []config.MCPServerConfig{{
|
|
Name: "lan", URL: "http://127.0.0.1:9100/mcp", Enabled: true,
|
|
}}}}, st)
|
|
if w == nil {
|
|
t.Fatal("should wire")
|
|
}
|
|
defer w.close()
|
|
w.connect(context.Background())
|
|
s := w.status()[0]
|
|
if s.Connected {
|
|
t.Fatal("a loopback server must not connect without allow_private")
|
|
}
|
|
if !strings.Contains(s.Err, "private address") {
|
|
t.Fatalf("err = %q, want the private-address refusal", s.Err)
|
|
}
|
|
}
|