diff --git a/server/internal/api/monitors.go b/server/internal/api/monitors.go index 13f84a1..7428bda 100644 --- a/server/internal/api/monitors.go +++ b/server/internal/api/monitors.go @@ -83,7 +83,7 @@ func createMonitor(c *gin.Context) { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } - created, err := services.CreateMonitor(auth.InstanceID(c), &m) + created, err := services.CreateMonitor(auth.InstanceID(c), &m, auth.ServerScope(c)) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return @@ -186,7 +186,7 @@ func updateMonitor(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": "no fields to update"}) return } - if err := services.UpdateMonitor(auth.InstanceID(c), c.Param("id"), upd); err != nil { + if err := services.UpdateMonitor(auth.InstanceID(c), c.Param("id"), upd, auth.ServerScope(c)); err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } diff --git a/server/internal/api/serverscope.go b/server/internal/api/serverscope.go index 51a6d36..5c181b8 100644 --- a/server/internal/api/serverscope.go +++ b/server/internal/api/serverscope.go @@ -116,6 +116,21 @@ var serverScopedRoutes = map[string]bool{ "GET /api/workflows": true, "GET /api/workflows/:id": true, + // createWorkflow/updateWorkflow validate target_server_ids through + // services.validateTargetServers, which resolves each named ID with + // GetServerScoped — so a restricted token can neither save a workflow + // targeting a host outside its scope (which the scheduler, firing as the + // system, would otherwise run there) nor learn which IDs exist by the + // difference between "target server not found" and a successful save. + "POST /api/workflows": true, + "PUT /api/workflows/:id": true, + + // createMonitor/updateMonitor validate the runner — which is a server ID + // for an agent-pushed monitor — through services.validateRunner, now + // resolving with GetServerScoped for the same two reasons. + "POST /api/monitors": true, + "PUT /api/monitors/:id": true, + // Creating a server has no server to filter yet. "POST /api/servers": false, // The agent's own enrolment routes authenticate as the agent, not as a diff --git a/server/internal/api/workflows.go b/server/internal/api/workflows.go index 58981c2..7289897 100644 --- a/server/internal/api/workflows.go +++ b/server/internal/api/workflows.go @@ -477,7 +477,7 @@ func createWorkflow(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) return } - out, err := services.CreateWorkflow(auth.InstanceID(c), w) + out, err := services.CreateWorkflow(auth.InstanceID(c), w, auth.ServerScope(c)) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return @@ -534,7 +534,7 @@ func updateWorkflow(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) return } - if err := services.UpdateWorkflow(auth.InstanceID(c), c.Param("id"), w); err != nil { + if err := services.UpdateWorkflow(auth.InstanceID(c), c.Param("id"), w, auth.ServerScope(c)); err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } diff --git a/server/internal/mcp/tools_create.go b/server/internal/mcp/tools_create.go index c547576..f2b83fe 100644 --- a/server/internal/mcp/tools_create.go +++ b/server/internal/mcp/tools_create.go @@ -135,6 +135,17 @@ func buildMonitor(args map[string]any) (models.Monitor, error) { return models.Monitor{}, fmt.Errorf("unknown monitor type %q", monitorType) } + // Runner is deliberately not settable from a tool call, and this refusal + // makes that explicit rather than leaving it safe by omission. A runner + // is a server ID: accepting one would let an agent push a check onto a + // named agent, and a silently ignored argument would leave a model + // believing it had. services.CreateMonitor now validates a runner + // through GetServerScoped as well, so this is a second line rather than + // the only one — but the clearer answer belongs here. + if _, present := args["runner"]; present { + return models.Monitor{}, fmt.Errorf("runner cannot be set from here; monitors created this way always run on the control plane") + } + interval := 60 if n, ok := args["interval_sec"].(float64); ok && int(n) > 0 { interval = int(n) @@ -214,7 +225,7 @@ func init() { } } - created, err := services.CreateWorkflow(c.InstanceID, wf) + created, err := services.CreateWorkflow(c.InstanceID, wf, c.TokenScope) if err != nil { return nil, fmt.Errorf("could not create the workflow: %w", err) } @@ -240,7 +251,7 @@ func init() { if err != nil { return nil, err } - created, err := services.CreateMonitor(c.InstanceID, &m) + created, err := services.CreateMonitor(c.InstanceID, &m, c.TokenScope) if err != nil { return nil, fmt.Errorf("could not create the monitor: %w", err) } diff --git a/server/internal/services/monitors.go b/server/internal/services/monitors.go index 0c18781..16f9947 100644 --- a/server/internal/services/monitors.go +++ b/server/internal/services/monitors.go @@ -146,23 +146,28 @@ func getMonitorByID(monitorID string) (*models.Monitor, error) { return &m, nil } -func validateRunner(instanceID, runner string) error { +// validateRunner refuses a monitor whose runner names a server the acting +// credential cannot see. A runner is literally a server ID, so an unscoped +// check here both lets a restricted token push work onto an out-of-scope agent +// and answers a fleet-enumeration question by the difference between "not +// found" and success. GetServerScoped collapses both into not-found. +func validateRunner(instanceID, runner string, tokenScope map[string]string) error { if runner == "" || runner == models.RunnerServer { return nil } - if _, err := GetServer(instanceID, runner); err != nil { + if _, err := GetServerScoped(instanceID, runner, tokenScope); err != nil { return fmt.Errorf("runner server %s not found", runner) } return nil } -func CreateMonitor(instanceID string, m *models.Monitor) (*models.Monitor, error) { +func CreateMonitor(instanceID string, m *models.Monitor, tokenScope map[string]string) (*models.Monitor, error) { ctx, cancel := monCtx() defer cancel() if err := validateChannelIDs(instanceID, m.ChannelIDs); err != nil { return nil, err } - if err := validateRunner(instanceID, m.Runner); err != nil { + if err := validateRunner(instanceID, m.Runner, tokenScope); err != nil { return nil, err } group, err := normaliseGroup(m.Group) @@ -189,7 +194,7 @@ func CreateMonitor(instanceID string, m *models.Monitor) (*models.Monitor, error return m, nil } -func UpdateMonitor(instanceID, monitorID string, upd bson.M) error { +func UpdateMonitor(instanceID, monitorID string, upd bson.M, tokenScope map[string]string) error { ctx, cancel := monCtx() defer cancel() @@ -218,7 +223,7 @@ func UpdateMonitor(instanceID, monitorID string, upd bson.M) error { if !ok { return fmt.Errorf("runner must be a string") } - if err := validateRunner(instanceID, runner); err != nil { + if err := validateRunner(instanceID, runner, tokenScope); err != nil { return err } @@ -446,5 +451,5 @@ func notifyTransition(m *models.Monitor, newStatus, message string) { } }(ch) } - _ = UpdateMonitor(m.InstanceID, m.MonitorID, bson.M{"state.last_notified_at": time.Now()}) + _ = UpdateMonitor(m.InstanceID, m.MonitorID, bson.M{"state.last_notified_at": time.Now()}, nil) } diff --git a/server/internal/services/workflows.go b/server/internal/services/workflows.go index af892fb..f907835 100644 --- a/server/internal/services/workflows.go +++ b/server/internal/services/workflows.go @@ -239,7 +239,7 @@ func GetWorkflow(instanceID, id string) (*models.Workflow, error) { return &w, err } -func CreateWorkflow(instanceID string, w models.Workflow) (*models.Workflow, error) { +func CreateWorkflow(instanceID string, w models.Workflow, tokenScope map[string]string) (*models.Workflow, error) { ctx, cancel := wfCtx() defer cancel() w.InstanceID = instanceID @@ -258,7 +258,7 @@ func CreateWorkflow(instanceID string, w models.Workflow) (*models.Workflow, err if err := ValidateTags(w.TargetTags); err != nil { return nil, err } - if err := validateTargetServers(instanceID, w.TargetServerIDs); err != nil { + if err := validateTargetServers(instanceID, w.TargetServerIDs, tokenScope); err != nil { return nil, err } normalizeInlineSteps(&w) @@ -268,7 +268,7 @@ func CreateWorkflow(instanceID string, w models.Workflow) (*models.Workflow, err return &w, nil } -func UpdateWorkflow(instanceID, id string, w models.Workflow) error { +func UpdateWorkflow(instanceID, id string, w models.Workflow, tokenScope map[string]string) error { ctx, cancel := wfCtx() defer cancel() if err := ValidateWorkflow(w); err != nil { @@ -277,7 +277,7 @@ func UpdateWorkflow(instanceID, id string, w models.Workflow) error { if err := ValidateTags(w.TargetTags); err != nil { return err } - if err := validateTargetServers(instanceID, w.TargetServerIDs); err != nil { + if err := validateTargetServers(instanceID, w.TargetServerIDs, tokenScope); err != nil { return err } normalizeInlineSteps(&w) @@ -291,9 +291,22 @@ func UpdateWorkflow(instanceID, id string, w models.Workflow) error { return err } -func validateTargetServers(instanceID string, serverIDs []string) error { +// validateTargetServers refuses a workflow naming a server the acting +// credential cannot see. +// +// It resolves through GetServerScoped rather than GetServer for two reasons. +// The first is escalation: without it a token restricted to staging could save +// a workflow targeting production and then reach those hosts through the +// scheduler, which fires as the system with no restriction of its own. The +// second is enumeration — "target server X not found" versus a successful save +// is a yes/no oracle over the whole fleet, and the design forbids a restricted +// token learning which IDs exist outside its scope. +// +// Both cases collapse into the same message an ID that genuinely does not +// exist produces, which is what keeps the two indistinguishable. +func validateTargetServers(instanceID string, serverIDs []string, tokenScope map[string]string) error { for _, sid := range serverIDs { - if _, err := GetServer(instanceID, sid); err != nil { + if _, err := GetServerScoped(instanceID, sid, tokenScope); err != nil { return fmt.Errorf("target server %s not found", sid) } }