fix: scope monitor runner and workflow targets to the caller's fleet
GET /api/monitors and GET /api/monitors/:id returned Monitor.Runner unfiltered; for an agent-pushed monitor that field is literally a server ID, so a restricted token learned which out-of-scope server a monitor runs on directly, not merely that one exists. services.RedactMonitorRunner replaces Runner with models.RunnerRestricted when it names a server outside the caller's scope, resolved once via the new services.VisibleServerIDs rather than per monitor. The monitor itself is still returned — a restricted operator may legitimately need to see that it exists and is up or down — only the runner field goes neutral; omitting the monitor entirely was considered and rejected as more surprising than one field changing. Runner "server" (control-plane-run) is never touched. The MCP list_monitors/get_monitor_status projections never had a Runner field to begin with, so REST and the tool surface already agreed; a comment now records why. GET /api/workflows and GET /api/workflows/:id returned Workflow.TargetServerIDs unfiltered — directly naming out-of-scope servers, worse than a count. services.FilterVisibleServerIDs narrows the list to what VisibleServerIDs admits and reports hidden (no count) when at least one target was dropped; WorkflowResponse wraps *models.Workflow with a scoped TargetServerIDs and a TargetsRestricted flag. TargetTags is left untouched — the tag vocabulary is already ruled acceptable to expose. The MCP list_workflows/get_workflow tools get the identical treatment: list_workflows' target count is now based on the filtered ID list, and get_workflow's workflowDetail carries the same TargetsRestricted flag, so a model that sees a filtered target list and then has run_workflow refuse the same workflow for out-of-scope targets is not left concluding the refusal invented a problem the list never mentioned. All four routes recorded in serverScopedRoutes as true; none is boot-enforced, for the same substring-filter reason as the key routes added in the previous round.
This commit is contained in:
@@ -39,6 +39,16 @@ func listMonitors(c *gin.Context) {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
|
||||
visible, restricted, err := services.VisibleServerIDs(auth.InstanceID(c), auth.ServerScope(c))
|
||||
if err != nil {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
for i := range monitors {
|
||||
monitors[i] = services.RedactMonitorRunner(monitors[i], visible, restricted)
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, monitors)
|
||||
}
|
||||
|
||||
@@ -103,7 +113,15 @@ func getMonitor(c *gin.Context) {
|
||||
c.JSON(http.StatusNotFound, gin.H{"error": "monitor not found"})
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, m)
|
||||
|
||||
visible, restricted, err := services.VisibleServerIDs(auth.InstanceID(c), auth.ServerScope(c))
|
||||
if err != nil {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
redacted := services.RedactMonitorRunner(*m, visible, restricted)
|
||||
|
||||
c.JSON(http.StatusOK, &redacted)
|
||||
}
|
||||
|
||||
// updateMonitor godoc
|
||||
|
||||
@@ -87,6 +87,35 @@ var serverScopedRoutes = map[string]bool{
|
||||
// declare-by-default inversion already recorded as a follow-up above.
|
||||
"GET /api/keys": true,
|
||||
|
||||
// listMonitors/getMonitor redact models.Monitor.Runner to
|
||||
// models.RunnerRestricted via services.RedactMonitorRunner when it names
|
||||
// a server outside the caller's scope — Runner is literally a server ID
|
||||
// for an agent-pushed monitor, so left unfiltered it discloses one
|
||||
// directly, worse than the key list's count. The monitor itself is still
|
||||
// returned: a restricted operator may legitimately need to see that it
|
||||
// exists and is up or down, so only the runner field goes neutral rather
|
||||
// than the whole monitor disappearing from the list. Runner "server"
|
||||
// (control-plane-run) is never touched — it names no server. Neither
|
||||
// path carries any of serverTouchingRoutes' substrings, so like the two
|
||||
// key routes above these entries are not boot-enforced.
|
||||
"GET /api/monitors": true,
|
||||
"GET /api/monitors/:id": true,
|
||||
|
||||
// listWorkflows/getWorkflow narrow Workflow.TargetServerIDs to what the
|
||||
// caller's scope admits via services.VisibleServerIDs +
|
||||
// FilterVisibleServerIDs, wrapped in WorkflowResponse so the JSON field
|
||||
// name is unchanged. TargetTags is left untouched — the tag vocabulary
|
||||
// itself is ruled acceptable to expose, unlike a resolved server ID.
|
||||
// TargetsRestricted is set (with no count) whenever at least one target
|
||||
// was dropped, so a caller is told some targets are hidden rather than
|
||||
// silently shown a shorter list that happens to match what
|
||||
// run_workflow's all-or-nothing scope refusal then also acts on — the
|
||||
// two cannot appear to disagree with each other. Neither path carries
|
||||
// any of serverTouchingRoutes' substrings, so like the routes above
|
||||
// these entries are not boot-enforced.
|
||||
"GET /api/workflows": true,
|
||||
"GET /api/workflows/: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
|
||||
|
||||
@@ -114,6 +114,23 @@ type AgentVersionResponse struct {
|
||||
Version string `json:"version"`
|
||||
}
|
||||
|
||||
// WorkflowResponse is a workflow with its TargetServerIDs narrowed to what
|
||||
// the acting token's scope admits — the explicit field shadows the embedded
|
||||
// one for JSON marshalling, matching the pattern KeyDetailResponse already
|
||||
// uses. TargetTags is not filtered: the tag vocabulary itself is ruled
|
||||
// acceptable to expose, and only the resolved ID list can name a specific
|
||||
// out-of-scope server.
|
||||
//
|
||||
// TargetsRestricted is set, with no count, when at least one target was
|
||||
// dropped, so a caller reading this alongside run_workflow's all-or-nothing
|
||||
// out-of-scope refusal sees why: the refusal is not inventing a problem the
|
||||
// list never mentioned.
|
||||
type WorkflowResponse struct {
|
||||
*models.Workflow
|
||||
TargetServerIDs []string `json:"target_server_ids"`
|
||||
TargetsRestricted bool `json:"targets_restricted,omitempty"`
|
||||
}
|
||||
|
||||
type UpdateAgentResponse struct {
|
||||
Message string `json:"message"`
|
||||
Version string `json:"version"`
|
||||
@@ -121,14 +138,14 @@ type UpdateAgentResponse struct {
|
||||
|
||||
type AuditEventsResponse struct {
|
||||
Events []models.AuditEvent `json:"events"`
|
||||
Total int64 `json:"total"`
|
||||
Total int64 `json:"total"`
|
||||
}
|
||||
|
||||
// --- tokens ---
|
||||
|
||||
type ListTokensResponse struct {
|
||||
Tokens []models.APIToken `json:"tokens"`
|
||||
All bool `json:"all"`
|
||||
All bool `json:"all"`
|
||||
}
|
||||
|
||||
type TokenScopesResponse struct {
|
||||
@@ -215,8 +232,8 @@ type RunWorkflowResponse struct {
|
||||
}
|
||||
|
||||
type ScheduleResponse struct {
|
||||
Schedule models.Schedule `json:"schedule"`
|
||||
NextRunAt *time.Time `json:"next_run_at"`
|
||||
Schedule models.Schedule `json:"schedule"`
|
||||
NextRunAt *time.Time `json:"next_run_at"`
|
||||
}
|
||||
|
||||
type OccurrencesResponse struct {
|
||||
|
||||
@@ -442,7 +442,20 @@ func listWorkflows(c *gin.Context) {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, wfs)
|
||||
|
||||
visible, restricted, err := services.VisibleServerIDs(auth.InstanceID(c), auth.ServerScope(c))
|
||||
if err != nil {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
|
||||
out := make([]WorkflowResponse, 0, len(wfs))
|
||||
for i := range wfs {
|
||||
w := wfs[i]
|
||||
ids, hidden := services.FilterVisibleServerIDs(w.TargetServerIDs, visible, restricted)
|
||||
out = append(out, WorkflowResponse{Workflow: &w, TargetServerIDs: ids, TargetsRestricted: hidden})
|
||||
}
|
||||
c.JSON(http.StatusOK, out)
|
||||
}
|
||||
|
||||
// createWorkflow godoc
|
||||
@@ -490,7 +503,15 @@ func getWorkflow(c *gin.Context) {
|
||||
c.JSON(http.StatusNotFound, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, w)
|
||||
|
||||
visible, restricted, err := services.VisibleServerIDs(auth.InstanceID(c), auth.ServerScope(c))
|
||||
if err != nil {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
ids, hidden := services.FilterVisibleServerIDs(w.TargetServerIDs, visible, restricted)
|
||||
|
||||
c.JSON(http.StatusOK, WorkflowResponse{Workflow: w, TargetServerIDs: ids, TargetsRestricted: hidden})
|
||||
}
|
||||
|
||||
// updateWorkflow godoc
|
||||
|
||||
@@ -12,6 +12,14 @@ import (
|
||||
// monitorSummary carries state and identity. A model asking "what is broken"
|
||||
// needs the state and the name; the target URL, expected status, keyword,
|
||||
// runner and channel list are configuration it did not ask for.
|
||||
//
|
||||
// Runner in particular is not merely omitted as noise: for an agent-pushed
|
||||
// monitor it is literally a server ID, and REST's listMonitors/getMonitor
|
||||
// redact it to models.RunnerRestricted when that server is outside the
|
||||
// caller's scope. This projection never had a runner field to redact — the
|
||||
// same outcome, reached by never including it rather than by filtering it
|
||||
// out, so this tool and get_monitor_status cannot disagree with the REST
|
||||
// surface about what a restricted token learns.
|
||||
type monitorSummary struct {
|
||||
ID string `json:"id"`
|
||||
Name string `json:"name"`
|
||||
|
||||
@@ -37,6 +37,12 @@ type workflowDetail struct {
|
||||
Targets []string `json:"target_server_ids,omitempty"`
|
||||
Tags map[string]string `json:"target_tags,omitempty"`
|
||||
Schedule string `json:"schedule,omitempty"`
|
||||
// TargetsRestricted is set, with no count, when Targets omits at least
|
||||
// one server ID outside this token's scope — mirroring
|
||||
// WorkflowResponse's REST field, so a model reading this alongside a
|
||||
// run_workflow refusal for the same workflow is not left to conclude the
|
||||
// refusal invented a problem this tool never mentioned.
|
||||
TargetsRestricted bool `json:"targets_restricted,omitempty"`
|
||||
}
|
||||
|
||||
// ---- runs ----
|
||||
@@ -159,13 +165,24 @@ func init() {
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("could not list workflows: %w", err)
|
||||
}
|
||||
|
||||
visible, restricted, err := services.VisibleServerIDs(c.InstanceID, c.TokenScope)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("could not resolve this token's server scope: %w", err)
|
||||
}
|
||||
|
||||
limit := pageLimit(args)
|
||||
out := make([]workflowSummary, 0, limit)
|
||||
for _, w := range workflows {
|
||||
if len(out) == limit {
|
||||
break
|
||||
}
|
||||
targets := len(w.TargetServerIDs)
|
||||
// The tag count is exposed as-is (the tag vocabulary is not
|
||||
// restricted); the ID count is narrowed to what this token
|
||||
// can see so it cannot itself disclose that out-of-scope
|
||||
// targets exist, the same leak the REST list closes.
|
||||
ids, _ := services.FilterVisibleServerIDs(w.TargetServerIDs, visible, restricted)
|
||||
targets := len(ids)
|
||||
if len(w.TargetTags) > 0 {
|
||||
targets = len(w.TargetTags)
|
||||
}
|
||||
@@ -207,13 +224,21 @@ func init() {
|
||||
if w.Schedule != nil && w.Schedule.Enabled {
|
||||
schedule = w.Schedule.Cron
|
||||
}
|
||||
|
||||
visible, restricted, err := services.VisibleServerIDs(c.InstanceID, c.TokenScope)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("could not resolve this token's server scope: %w", err)
|
||||
}
|
||||
targets, hidden := services.FilterVisibleServerIDs(w.TargetServerIDs, visible, restricted)
|
||||
|
||||
return workflowDetail{
|
||||
ID: w.WorkflowID,
|
||||
Name: w.Name,
|
||||
Steps: steps,
|
||||
Targets: w.TargetServerIDs,
|
||||
Tags: w.TargetTags,
|
||||
Schedule: schedule,
|
||||
ID: w.WorkflowID,
|
||||
Name: w.Name,
|
||||
Steps: steps,
|
||||
Targets: targets,
|
||||
Tags: w.TargetTags,
|
||||
Schedule: schedule,
|
||||
TargetsRestricted: hidden,
|
||||
}, nil
|
||||
},
|
||||
})
|
||||
|
||||
@@ -21,6 +21,14 @@ const (
|
||||
|
||||
const RunnerServer = "server"
|
||||
|
||||
// RunnerRestricted replaces a monitor's runner in an API response when the
|
||||
// real value is a server ID the acting token's scope does not admit. The
|
||||
// monitor itself is still returned — a restricted operator may legitimately
|
||||
// need to see its name and state — only where it runs is hidden, the same
|
||||
// way a workflow's target list can omit an ID without the whole workflow
|
||||
// disappearing from a list.
|
||||
const RunnerRestricted = "restricted"
|
||||
|
||||
type MonitorTarget struct {
|
||||
URL string `bson:"url,omitempty" json:"url,omitempty"`
|
||||
Host string `bson:"host,omitempty" json:"host,omitempty"`
|
||||
|
||||
@@ -53,6 +53,28 @@ func SpecFor(m *models.Monitor) checker.Spec {
|
||||
}
|
||||
}
|
||||
|
||||
// RedactMonitorRunner replaces m.Runner with models.RunnerRestricted when it
|
||||
// names a server outside the caller's scope, so GET /monitors and GET
|
||||
// /monitors/:id can keep listing the monitor itself — name, type, state,
|
||||
// whether it exists at all — as the first-class object it is, without
|
||||
// disclosing which out-of-scope server it happens to run on. Omitting the
|
||||
// monitor entirely was considered and rejected: a restricted operator has a
|
||||
// legitimate reason to see that a monitor exists and is up or down even when
|
||||
// they cannot manage the host it checks from, and hiding it wholesale is a
|
||||
// bigger surprise than one field going neutral.
|
||||
//
|
||||
// Runner == models.RunnerServer (control-plane-run) is never touched: it
|
||||
// names no server at all, so there is nothing to redact.
|
||||
func RedactMonitorRunner(m models.Monitor, visible map[string]bool, restricted bool) models.Monitor {
|
||||
if !restricted || m.Runner == models.RunnerServer {
|
||||
return m
|
||||
}
|
||||
if !visible[m.Runner] {
|
||||
m.Runner = models.RunnerRestricted
|
||||
}
|
||||
return m
|
||||
}
|
||||
|
||||
func ListMonitors(instanceID string) ([]models.Monitor, error) {
|
||||
ctx, cancel := monCtx()
|
||||
defer cancel()
|
||||
|
||||
@@ -23,6 +23,59 @@ func ServerInTokenScope(srv models.Server, sel map[string]string) bool {
|
||||
return true
|
||||
}
|
||||
|
||||
// VisibleServerIDs resolves the servers tokenScope admits into a membership
|
||||
// set, for a caller that needs to test many IDs against the caller's scope in
|
||||
// one pass — redacting a monitor's runner, filtering a workflow's target list
|
||||
// — rather than resolving one server at a time the way GetServerScoped does.
|
||||
//
|
||||
// restricted is false for an empty tokenScope, matching ServerInTokenScope's
|
||||
// own rule that an empty selector is unrestricted rather than "sees nothing".
|
||||
// ids is then nil, and callers must treat (nil, false) as "everything
|
||||
// visible", never as "nothing visible" — the zero value of a map read is
|
||||
// false, which would silently invert the rule for every unrestricted caller
|
||||
// if this contract were not honoured.
|
||||
func VisibleServerIDs(instanceID string, tokenScope map[string]string) (ids map[string]bool, restricted bool, err error) {
|
||||
if len(tokenScope) == 0 {
|
||||
return nil, false, nil
|
||||
}
|
||||
servers, err := ListServers(instanceID)
|
||||
if err != nil {
|
||||
return nil, true, err
|
||||
}
|
||||
ids = make(map[string]bool, len(servers))
|
||||
for _, s := range servers {
|
||||
if ServerInTokenScope(s, tokenScope) {
|
||||
ids[s.ServerID] = true
|
||||
}
|
||||
}
|
||||
return ids, true, nil
|
||||
}
|
||||
|
||||
// FilterVisibleServerIDs narrows ids to the ones visible admits, using the
|
||||
// (ids, restricted) pair VisibleServerIDs returns. An unrestricted caller
|
||||
// (restricted false) gets ids back unchanged and hidden is always false.
|
||||
//
|
||||
// hidden reports only whether at least one id was dropped — never how many —
|
||||
// because the point of surfacing it at all is to let a caller say "some
|
||||
// targets are not visible to you" without the count itself becoming the leak
|
||||
// this exists to close. A workflow that targets both an in-scope and an
|
||||
// out-of-scope host should not read as "0 hidden" or "3 hidden"; either
|
||||
// number is information about a fleet outside the caller's scope.
|
||||
func FilterVisibleServerIDs(ids []string, visible map[string]bool, restricted bool) (filtered []string, hidden bool) {
|
||||
if !restricted {
|
||||
return ids, false
|
||||
}
|
||||
filtered = make([]string, 0, len(ids))
|
||||
for _, id := range ids {
|
||||
if visible[id] {
|
||||
filtered = append(filtered, id)
|
||||
} else {
|
||||
hidden = true
|
||||
}
|
||||
}
|
||||
return filtered, hidden
|
||||
}
|
||||
|
||||
// IntersectSelectors merges the caller's token restriction with a selector the
|
||||
// request asked for. ok is false when the two can never both hold, which means
|
||||
// the request resolves to no servers rather than to an error.
|
||||
|
||||
Reference in New Issue
Block a user