fix(security): scope workflow target and monitor runner validation to the caller's tags
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user