diff --git a/server/internal/mcp/audit.go b/server/internal/mcp/audit.go index 572489e..217b93e 100644 --- a/server/internal/mcp/audit.go +++ b/server/internal/mcp/audit.go @@ -125,3 +125,14 @@ func LogFailure(c Caller, t Tool, args map[string]any, err error) { logEvent(c.InstanceID, "mcp.tool_failed", c.TokenName, "", "", fmt.Sprintf("tool %s (%s) failed: %v", t.Name, SummariseArgs(args), err)) } + +// LogCreated records a definition an agent added. +// +// It is a distinct event type rather than another mcp.tool_call row because of +// the question a human will actually ask, which is "what has this agent added +// to my instance" — an answer buried among hundreds of read rows is not an +// answer. +func LogCreated(c Caller, kind, id, name string) { + logEvent(c.InstanceID, "mcp.created", c.TokenName, "", "", + fmt.Sprintf("created %s %q (%s)", kind, name, id)) +} diff --git a/server/internal/mcp/tools_create.go b/server/internal/mcp/tools_create.go new file mode 100644 index 0000000..c547576 --- /dev/null +++ b/server/internal/mcp/tools_create.go @@ -0,0 +1,256 @@ +package mcp + +import ( + "context" + "fmt" + + "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/models" + "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/services" +) + +// SourceMCP marks a definition an agent wrote. models.WorkflowStep already +// carries a Source field for exactly this kind of provenance, so the UI can +// badge agent-authored steps without a schema change. +const SourceMCP = "mcp" + +// buildStep validates the arguments and returns the step to create. +// +// It is pure so that every refusal below is testable without a database, and +// separate from the handler so the handler is only plumbing. +func buildStep(args map[string]any) (models.WorkflowStep, error) { + name := stringArg(args, "name") + interpreter := stringArg(args, "interpreter") + script := stringArg(args, "script") + + if name == "" { + return models.WorkflowStep{}, fmt.Errorf("name is required") + } + if interpreter == "" { + return models.WorkflowStep{}, fmt.Errorf("interpreter is required, for example bash or powershell") + } + if script == "" { + return models.WorkflowStep{}, fmt.Errorf("script is required") + } + if len(stringSliceArg(args, "secret_refs")) > 0 { + return models.WorkflowStep{}, fmt.Errorf( + "a step created through MCP cannot reference secrets; " + + "add the secret reference in the Vantage UI after reviewing the script") + } + + return models.WorkflowStep{ + Name: name, + Description: stringArg(args, "description"), + Interpreter: interpreter, + Script: script, + Source: SourceMCP, + }, nil +} + +// buildWorkflow validates the arguments and returns the workflow to create. +func buildWorkflow(args map[string]any) (models.Workflow, error) { + name := stringArg(args, "name") + if name == "" { + return models.Workflow{}, fmt.Errorf("name is required") + } + if _, scheduled := args["schedule"]; scheduled { + return models.Workflow{}, fmt.Errorf( + "a workflow created through MCP cannot be scheduled; " + + "create it, review it, then set a schedule in the Vantage UI") + } + + stepIDs := stringSliceArg(args, "step_ids") + if len(stepIDs) == 0 { + return models.Workflow{}, fmt.Errorf("step_ids must name at least one existing step; create steps first with create_step") + } + + // Order comes from the array order rather than from a field, because step + // order is the whole meaning of a workflow and is not worth asking a model + // to restate correctly. OnFailure defaults to "stop", the same default the + // workflow runner falls back to when a saved ref leaves it blank (see + // resolveInlineStep/resolveLibStep in workflow_runner.go). + steps := make([]models.WorkflowStepRef, 0, len(stepIDs)) + for i, id := range stepIDs { + steps = append(steps, models.WorkflowStepRef{ + StepID: id, + Order: i, + OnFailure: "stop", + }) + } + + return models.Workflow{ + Name: name, + TargetServerIDs: stringSliceArg(args, "server_ids"), + TargetTags: tagArg(args), + Steps: steps, + }, nil +} + +// buildMonitor validates the arguments and returns the monitor to create. +func buildMonitor(args map[string]any) (models.Monitor, error) { + name := stringArg(args, "name") + monitorType := stringArg(args, "type") + if name == "" { + return models.Monitor{}, fmt.Errorf("name is required") + } + if monitorType == "" { + return models.Monitor{}, fmt.Errorf("type is required") + } + + rawTarget, ok := args["target"].(map[string]any) + if !ok || len(rawTarget) == 0 { + return models.Monitor{}, fmt.Errorf("target is required") + } + target := models.MonitorTarget{ + URL: stringArg(rawTarget, "url"), + Host: stringArg(rawTarget, "host"), + Method: stringArg(rawTarget, "method"), + Keyword: stringArg(rawTarget, "keyword"), + } + if n, ok := rawTarget["port"].(float64); ok { + target.Port = int(n) + } + if n, ok := rawTarget["expected_status"].(float64); ok { + target.ExpectedStatus = int(n) + } + if n, ok := rawTarget["tls_warn_days"].(float64); ok { + target.TLSWarnDays = int(n) + } + if b, ok := rawTarget["insecure"].(bool); ok { + target.Insecure = b + } + + switch monitorType { + case models.MonitorHTTP, models.MonitorTLS: + if target.URL == "" { + return models.Monitor{}, fmt.Errorf("target.url is required for a %s monitor", monitorType) + } + case models.MonitorTCP, models.MonitorICMP: + if target.Host == "" { + return models.Monitor{}, fmt.Errorf("target.host is required for a %s monitor", monitorType) + } + if monitorType == models.MonitorTCP && target.Port == 0 { + return models.Monitor{}, fmt.Errorf("target.port is required for a tcp monitor") + } + default: + return models.Monitor{}, fmt.Errorf("unknown monitor type %q", monitorType) + } + + interval := 60 + if n, ok := args["interval_sec"].(float64); ok && int(n) > 0 { + interval = int(n) + } + + return models.Monitor{ + Name: name, + Group: stringArg(args, "group"), + Type: monitorType, + Target: target, + IntervalSec: interval, + // Never armed on creation. A monitor that started enabled would begin + // alerting real people the moment a model invented it, and creating + // must stay a separate decision from acting. + Enabled: false, + }, nil +} + +func init() { + All().Register(Tool{ + Name: "create_step", + Write: true, + Scope: "workflows:write", + Description: "Create a reusable workflow step: a named script with an interpreter. " + + "The step is SAVED to this Vantage instance but is not run by creating it — " + + "add it to a workflow with create_workflow, then run that with run_workflow. " + + "Steps created this way cannot reference secrets.", + Handler: func(ctx context.Context, c Caller, args map[string]any) (any, error) { + step, err := buildStep(args) + if err != nil { + return nil, err + } + created, err := services.CreateStep(c.InstanceID, step) + if err != nil { + return nil, fmt.Errorf("could not create the step: %w", err) + } + LogCreated(c, "step", created.StepID, created.Name) + return map[string]any{ + "step_id": created.StepID, + "name": created.Name, + "note": "Saved but not run. Reference this step_id from create_workflow.", + }, nil + }, + }) + + All().Register(Tool{ + Name: "create_workflow", + Write: true, + Scope: "workflows:write", + Description: "Create a workflow from existing step IDs, in the order given, targeting " + + "servers by ID or by tags. The workflow is SAVED but not run and cannot be " + + "created with a schedule; run it explicitly with run_workflow.", + Handler: func(ctx context.Context, c Caller, args map[string]any) (any, error) { + wf, err := buildWorkflow(args) + if err != nil { + return nil, err + } + + // Targets are stored, not resolved at run time, so a workflow + // cannot be saved pointing at servers this token cannot reach. + // run_workflow later refuses to run a saved workflow unless the + // scoped view of its targets covers every server the unscoped + // resolution would touch; this mirrors that same all-or-nothing + // check at creation time so a workflow this token could not run + // is never created in the first place. + if len(wf.TargetServerIDs) > 0 || len(wf.TargetTags) > 0 { + allTargets, err := services.ResolveTargets(c.InstanceID, wf.TargetServerIDs, wf.TargetTags) + if err != nil { + return nil, fmt.Errorf("this workflow matches no servers") + } + scopedTargets, err := services.ResolveTargetsScoped(c.InstanceID, wf.TargetServerIDs, wf.TargetTags, c.TokenScope) + if err != nil || len(scopedTargets) != len(allTargets) { + return nil, fmt.Errorf("%w: no servers visible to this token matched the requested targets", ErrOutOfScope) + } + if err := CheckFanOut(len(scopedTargets), args); err != nil { + return nil, err + } + } + + created, err := services.CreateWorkflow(c.InstanceID, wf) + if err != nil { + return nil, fmt.Errorf("could not create the workflow: %w", err) + } + LogCreated(c, "workflow", created.WorkflowID, created.Name) + return map[string]any{ + "workflow_id": created.WorkflowID, + "name": created.Name, + "steps": len(created.Steps), + "note": "Saved but not run and not scheduled. Call run_workflow to run it.", + }, nil + }, + }) + + All().Register(Tool{ + Name: "create_monitor", + Write: true, + Scope: "monitors:write", + Description: "Create a monitor. It is SAVED DISABLED and will not check anything or " + + "send any alert until a human enables it in the Vantage UI, so proposing a " + + "monitor is safe.", + Handler: func(ctx context.Context, c Caller, args map[string]any) (any, error) { + m, err := buildMonitor(args) + if err != nil { + return nil, err + } + created, err := services.CreateMonitor(c.InstanceID, &m) + if err != nil { + return nil, fmt.Errorf("could not create the monitor: %w", err) + } + LogCreated(c, "monitor", created.MonitorID, created.Name) + return map[string]any{ + "monitor_id": created.MonitorID, + "name": created.Name, + "enabled": false, + "note": "Created disabled. Enable it in Vantage to start checking.", + }, nil + }, + }) +} diff --git a/server/internal/mcp/tools_create_test.go b/server/internal/mcp/tools_create_test.go new file mode 100644 index 0000000..b8651ba --- /dev/null +++ b/server/internal/mcp/tools_create_test.go @@ -0,0 +1,195 @@ +package mcp + +import ( + "strings" + "testing" +) + +func TestCreationToolsAreRegisteredAsWrites(t *testing.T) { + for _, name := range []string{"create_step", "create_workflow", "create_monitor"} { + tool, ok := All().Lookup(name) + if !ok { + t.Errorf("tool %q is not registered", name) + continue + } + if !tool.Write { + t.Errorf("tool %q is not marked as a write", name) + } + } +} + +// An agent may add a definition. It may never alter or remove one a human +// wrote, and the cheapest guard against that is the tool simply not existing. +func TestNoUpdateOrDeleteTools(t *testing.T) { + for _, tool := range All().Tools() { + n := tool.Name + if strings.HasPrefix(n, "update_") && n != "update_agent" { + t.Errorf("tool %q edits an existing definition", n) + } + if strings.HasPrefix(n, "delete_") || strings.HasPrefix(n, "remove_") { + t.Errorf("tool %q deletes a definition", n) + } + } +} + +// Composing a script around a secret reference is how a credential ends up +// echoed into a log. +func TestCreateStepRejectsSecretRefs(t *testing.T) { + step, err := buildStep(map[string]any{ + "name": "leaky", + "interpreter": "bash", + "script": "echo hello", + "secret_refs": []any{"prod/db"}, + }) + if err == nil { + t.Fatalf("buildStep accepted secret_refs, got %+v", step) + } + if !strings.Contains(err.Error(), "secret") { + t.Errorf("error %q does not explain the refusal", err) + } +} + +func TestCreateStepRequiresScriptAndInterpreter(t *testing.T) { + if _, err := buildStep(map[string]any{"name": "x", "script": "echo hi"}); err == nil { + t.Error("buildStep accepted a step with no interpreter") + } + if _, err := buildStep(map[string]any{"name": "x", "interpreter": "bash"}); err == nil { + t.Error("buildStep accepted a step with no script") + } +} + +// Steps a model wrote are badged in the UI, so a human can tell at a glance +// what came from an agent. +func TestCreatedStepIsMarkedAgentAuthored(t *testing.T) { + step, err := buildStep(map[string]any{ + "name": "patch", "interpreter": "bash", "script": "apt-get update", + }) + if err != nil { + t.Fatal(err) + } + if step.Source != "mcp" { + t.Errorf("Source = %q, want %q", step.Source, "mcp") + } +} + +// Creating and acting stay two decisions: a created workflow cannot arrive +// already scheduled. +func TestCreateWorkflowRefusesASchedule(t *testing.T) { + _, err := buildWorkflow(map[string]any{ + "name": "nightly", + "step_ids": []any{"step-1"}, + "schedule": map[string]any{"cron": "0 3 * * *"}, + }) + if err == nil { + t.Fatal("buildWorkflow accepted a schedule") + } + if !strings.Contains(err.Error(), "schedule") { + t.Errorf("error %q does not explain the refusal", err) + } +} + +func TestCreateWorkflowRequiresSteps(t *testing.T) { + if _, err := buildWorkflow(map[string]any{"name": "empty"}); err == nil { + t.Error("buildWorkflow accepted a workflow with no steps") + } +} + +// Step order is the whole meaning of a workflow, so it comes from the array +// order rather than from a field a model has to get right. +func TestBuildWorkflowNumbersStepsInOrder(t *testing.T) { + wf, err := buildWorkflow(map[string]any{ + "name": "three", + "step_ids": []any{"a", "b", "c"}, + }) + if err != nil { + t.Fatal(err) + } + if len(wf.Steps) != 3 { + t.Fatalf("got %d steps, want 3", len(wf.Steps)) + } + for i, ref := range wf.Steps { + if ref.Order != i { + t.Errorf("step %d has Order %d", i, ref.Order) + } + } + if wf.Steps[0].StepID != "a" || wf.Steps[2].StepID != "c" { + t.Errorf("step order does not follow the argument order: %+v", wf.Steps) + } +} + +// A monitor that starts enabled would begin alerting real people the moment a +// model invented it. +func TestCreatedMonitorIsDisabled(t *testing.T) { + m, err := buildMonitor(map[string]any{ + "name": "api health", "type": "http", "target": map[string]any{"url": "https://example.com"}, + }) + if err != nil { + t.Fatal(err) + } + if m.Enabled { + t.Error("created monitor is enabled; it must wait for a human") + } +} + +func TestCreateMonitorRequiresNameAndType(t *testing.T) { + if _, err := buildMonitor(map[string]any{"type": "http"}); err == nil { + t.Error("buildMonitor accepted a monitor with no name") + } + if _, err := buildMonitor(map[string]any{"name": "x"}); err == nil { + t.Error("buildMonitor accepted a monitor with no type") + } +} + +// A monitor with no target argument at all cannot be checked, so it is +// refused the same way a missing name or type is. +func TestCreateMonitorRequiresTarget(t *testing.T) { + if _, err := buildMonitor(map[string]any{"name": "x", "type": "http"}); err == nil { + t.Error("buildMonitor accepted a monitor with no target") + } +} + +// The target argument decodes into the real models.MonitorTarget shape, not +// a passthrough map, so an http monitor without a URL is rejected here rather +// than surfacing a confusing failure the first time it is checked. +func TestCreateMonitorHTTPRequiresURL(t *testing.T) { + if _, err := buildMonitor(map[string]any{ + "name": "x", "type": "http", "target": map[string]any{"method": "GET"}, + }); err == nil { + t.Error("buildMonitor accepted an http monitor with no target url") + } +} + +func TestCreateMonitorTCPRequiresHostAndPort(t *testing.T) { + if _, err := buildMonitor(map[string]any{ + "name": "x", "type": "tcp", "target": map[string]any{"host": "example.com"}, + }); err == nil { + t.Error("buildMonitor accepted a tcp monitor with no port") + } +} + +func TestCreateMonitorDecodesTargetFields(t *testing.T) { + m, err := buildMonitor(map[string]any{ + "name": "api health", "type": "http", + "target": map[string]any{ + "url": "https://example.com/health", + "method": "GET", + "expected_status": float64(200), + "keyword": "ok", + }, + }) + if err != nil { + t.Fatal(err) + } + if m.Target.URL != "https://example.com/health" { + t.Errorf("Target.URL = %q", m.Target.URL) + } + if m.Target.Method != "GET" { + t.Errorf("Target.Method = %q", m.Target.Method) + } + if m.Target.ExpectedStatus != 200 { + t.Errorf("Target.ExpectedStatus = %d", m.Target.ExpectedStatus) + } + if m.Target.Keyword != "ok" { + t.Errorf("Target.Keyword = %q", m.Target.Keyword) + } +}