feat(monitors): validate metric monitors and confine selectors to token scope

server/internal/services/metricrules.go

@@ -1,6 +1,7 @@
 package services
 
 import (
+	"errors"
 	"fmt"
 	"path"
 	"strings"
@@ -9,6 +10,22 @@ import (
 	"gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/models"
 )
 
+// ErrMonitorOutOfScope marks a metric monitor whose selector reaches beyond a
+// restricted token's tag scope, on create, update or delete.
+var ErrMonitorOutOfScope = errors.New("selector is outside this credential's tag scope")
+
+// selectorWithinScope holds when every server the selector can match is also
+// inside the token's scope - true exactly when the selector pins every pair
+// the scope does.
+func selectorWithinScope(sel, tokenScope map[string]string) bool {
+	for k, v := range tokenScope {
+		if sel[k] != v {
+			return false
+		}
+	}
+	return true
+}
+
 const (
 	MetricDiskPct            = "disk_pct"
 	MetricDiskFreeGB         = "disk_free_gb"

server/internal/services/metricscope_test.go

@@ -0,0 +1,27 @@
+package services
+
+import "testing"
+
+// A restricted token must not be able to create a rule that watches servers it
+// cannot see: the per-server table and incident messages would disclose them.
+func TestSelectorWithinScope(t *testing.T) {
+	scope := map[string]string{"env": "prod"}
+	cases := []struct {
+		sel  map[string]string
+		want bool
+	}{
+		{nil, false},
+		{map[string]string{"env": "dev"}, false},
+		{map[string]string{"role": "web"}, false},
+		{map[string]string{"env": "prod"}, true},
+		{map[string]string{"env": "prod", "role": "web"}, true},
+	}
+	for _, c := range cases {
+		if got := selectorWithinScope(c.sel, scope); got != c.want {
+			t.Errorf("sel %v: got %v want %v", c.sel, got, c.want)
+		}
+	}
+	if !selectorWithinScope(nil, nil) {
+		t.Error("an unrestricted credential may use any selector, including the whole fleet")
+	}
+}

server/internal/services/monitors.go

@@ -202,6 +202,18 @@ func CreateMonitor(instanceID string, m *models.Monitor, tokenScope map[string]s
 		m.Runner = models.RunnerServer
 		m.IntervalSec = 0
 	}
+	if m.Type == models.MonitorMetric {
+		if err := validateMetric(m); err != nil {
+			return nil, err
+		}
+		if !selectorWithinScope(m.Target.Selector, tokenScope) {
+			return nil, ErrMonitorOutOfScope
+		}
+		// A metric monitor is evaluated by sweeping stored server inventory,
+		// never run against a single named agent.
+		m.Runner = models.RunnerServer
+		m.IntervalSec = 0
+	}
 	m.State = models.MonitorState{Status: models.StatusPending}
 	if _, err := db.Col("monitors").InsertOne(ctx, m); err != nil {
 		return nil, err
@@ -229,13 +241,31 @@ func UpdateMonitor(instanceID, monitorID string, upd bson.M, tokenScope map[stri
 			return fmt.Errorf("type cannot be changed to or from %s", bad)
 		}
 	}
-	if raw, present := upd["target"]; present && existing.Type == models.MonitorHeartbeat {
+	// Editing any field of a metric monitor the token could not have created is
+	// refused before any write, so a restricted token cannot rename or disable
+	// a fleet-wide rule it cannot fully see.
+	if existing.Type == models.MonitorMetric && !selectorWithinScope(existing.Target.Selector, tokenScope) {
+		return ErrMonitorOutOfScope
+	}
+
+	if raw, present := upd["target"]; present && models.IsPassiveMonitor(existing.Type) {
 		tg, ok := raw.(models.MonitorTarget)
 		if !ok {
-			return fmt.Errorf("target must be an object")
+			return fmt.Errorf("%w: target must be an object", ErrInvalidMonitor)
 		}
-		if err := validateHeartbeat(&tg); err != nil {
-			return err
+		switch existing.Type {
+		case models.MonitorHeartbeat:
+			if err := validateHeartbeat(&tg); err != nil {
+				return err
+			}
+		case models.MonitorMetric:
+			probe := models.Monitor{Type: existing.Type, Target: tg, ForSec: existing.ForSec}
+			if err := validateMetric(&probe); err != nil {
+				return err
+			}
+			if !selectorWithinScope(tg.Selector, tokenScope) {
+				return ErrMonitorOutOfScope
+			}
 		}
 		upd["target"] = tg
 	}
@@ -277,9 +307,26 @@ func UpdateMonitor(instanceID, monitorID string, upd bson.M, tokenScope map[stri
 	return err
 }
 
-func DeleteMonitor(instanceID, monitorID string) error {
+// DeleteMonitor deletes a monitor and its associated data. For a metric
+// monitor, deletion is refused when its selector reaches beyond tokenScope -
+// the same out-of-scope check applied on create and update - so a restricted
+// token cannot remove a fleet-wide rule it could not have made. Other monitor
+// types delete as before, unaffected by tokenScope.
+func DeleteMonitor(instanceID, monitorID string, tokenScope map[string]string) error {
 	ctx, cancel := monCtx()
 	defer cancel()
+
+	existing, err := GetMonitor(instanceID, monitorID)
+	if err != nil {
+		return err
+	}
+	if existing == nil {
+		return nil
+	}
+	if existing.Type == models.MonitorMetric && !selectorWithinScope(existing.Target.Selector, tokenScope) {
+		return ErrMonitorOutOfScope
+	}
+
 	res, err := db.Col("monitors").DeleteOne(ctx, bson.M{"monitor_id": monitorID, "instance_id": instanceID})
 	if err != nil {
 		return err
@@ -291,6 +338,7 @@ func DeleteMonitor(instanceID, monitorID string) error {
 	db.Col("incidents").DeleteMany(ctx, bson.M{"monitor_id": monitorID, "instance_id": instanceID})
 	db.Col("monitor_rollups").DeleteMany(ctx, bson.M{"monitor_id": monitorID, "instance_id": instanceID})
 	db.Col("monitor_samples").DeleteMany(ctx, bson.M{"monitor_id": monitorID, "instance_id": instanceID})
+	db.Col("monitor_server_states").DeleteMany(ctx, bson.M{"monitor_id": monitorID, "instance_id": instanceID})
 	return nil
 }