diff --git a/server/internal/api/monitors.go b/server/internal/api/monitors.go index c9ce559..13f84a1 100644 --- a/server/internal/api/monitors.go +++ b/server/internal/api/monitors.go @@ -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 diff --git a/server/internal/api/serverscope.go b/server/internal/api/serverscope.go index 11ea229..51a6d36 100644 --- a/server/internal/api/serverscope.go +++ b/server/internal/api/serverscope.go @@ -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 diff --git a/server/internal/api/types.go b/server/internal/api/types.go index 1864282..a8c65ed 100644 --- a/server/internal/api/types.go +++ b/server/internal/api/types.go @@ -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 { diff --git a/server/internal/api/workflows.go b/server/internal/api/workflows.go index e90e42e..fcceb7d 100644 --- a/server/internal/api/workflows.go +++ b/server/internal/api/workflows.go @@ -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 diff --git a/server/internal/mcp/tools_health.go b/server/internal/mcp/tools_health.go index 1122df5..0d2c33f 100644 --- a/server/internal/mcp/tools_health.go +++ b/server/internal/mcp/tools_health.go @@ -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"` diff --git a/server/internal/mcp/tools_work.go b/server/internal/mcp/tools_work.go index 0507181..b087dc8 100644 --- a/server/internal/mcp/tools_work.go +++ b/server/internal/mcp/tools_work.go @@ -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 }, }) diff --git a/server/internal/models/monitor.go b/server/internal/models/monitor.go index 7bf7085..d890f86 100644 --- a/server/internal/models/monitor.go +++ b/server/internal/models/monitor.go @@ -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"` diff --git a/server/internal/services/monitors.go b/server/internal/services/monitors.go index 19b8395..0c18781 100644 --- a/server/internal/services/monitors.go +++ b/server/internal/services/monitors.go @@ -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() diff --git a/server/internal/services/tokenscope.go b/server/internal/services/tokenscope.go index 1a86b64..fcb6912 100644 --- a/server/internal/services/tokenscope.go +++ b/server/internal/services/tokenscope.go @@ -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.