diff --git a/server/internal/api/workflows.go b/server/internal/api/workflows.go index fcceb7d..58981c2 100644 --- a/server/internal/api/workflows.go +++ b/server/internal/api/workflows.go @@ -581,7 +581,7 @@ func deleteWorkflow(c *gin.Context) { // @Security bearerAuth // @Router /workflows/{id}/run [post] func runWorkflow(c *gin.Context) { - runID, err := services.TriggerWorkflow(auth.InstanceID(c), c.Param("id"), actorFromCtx(c)) + runID, err := services.TriggerWorkflow(auth.InstanceID(c), c.Param("id"), actorFromCtx(c), auth.ServerScope(c)) if err != nil { if errors.Is(err, services.ErrNoTargets) { c.JSON(http.StatusBadRequest, gin.H{"error": "this workflow matches no servers"}) diff --git a/server/internal/mcp/tools_work.go b/server/internal/mcp/tools_work.go index b087dc8..64002b4 100644 --- a/server/internal/mcp/tools_work.go +++ b/server/internal/mcp/tools_work.go @@ -314,6 +314,20 @@ func init() { return nil, fmt.Errorf("server %q is not part of run %q", serverID, runID) } + // Membership in the run is not scope: a run started before this + // token was restricted, or by an unrestricted credential, names + // servers this token must not read. The stdout of an + // out-of-scope host is exactly the data the tag restriction + // exists to withhold. + // + // The refusal reuses the membership message verbatim so that + // "in the run but out of your scope" and "not in the run at all" + // are indistinguishable — otherwise the difference between the + // two answers enumerates hosts the token cannot see. + if _, err := services.GetServerScoped(c.InstanceID, serverID, c.TokenScope); err != nil { + return nil, fmt.Errorf("server %q is not part of run %q", serverID, runID) + } + limit := defaultLogLimit if raw, ok := args["limit"].(float64); ok && int(raw) > 0 { limit = int(raw) diff --git a/server/internal/mcp/tools_write.go b/server/internal/mcp/tools_write.go index 2d7be3e..6d14eee 100644 --- a/server/internal/mcp/tools_write.go +++ b/server/internal/mcp/tools_write.go @@ -76,6 +76,15 @@ func init() { if err != nil { return nil, fmt.Errorf("this workflow matches no servers") } + // This all-or-nothing pre-check is no longer the only guard: + // services.TriggerWorkflow now resolves through + // ResolveTargetsScoped itself, so a run started with this token + // can never touch a server outside its scope regardless of what + // happens here. It is kept because its refusal is the clearer + // answer for a model: the service layer would silently run + // against the in-scope subset, while a partially out-of-scope + // workflow is documented here as refused outright, which is + // behaviour a caller relies on. scopedTargets, err := services.ResolveTargetsScoped(c.InstanceID, wf.TargetServerIDs, wf.TargetTags, c.TokenScope) if err != nil || len(scopedTargets) != len(allTargets) { return nil, fmt.Errorf("%w: no servers visible to this token matched the request", ErrOutOfScope) @@ -85,7 +94,7 @@ func init() { return nil, err } - runID, err := services.TriggerWorkflow(c.InstanceID, workflowID, c.TokenName) + runID, err := services.TriggerWorkflow(c.InstanceID, workflowID, c.TokenName, c.TokenScope) if err != nil { return nil, fmt.Errorf("could not start the run: %w", err) } diff --git a/server/internal/services/workflow_runner.go b/server/internal/services/workflow_runner.go index ba3a16d..c1e67e7 100644 --- a/server/internal/services/workflow_runner.go +++ b/server/internal/services/workflow_runner.go @@ -17,12 +17,24 @@ import ( const stepDispatchGrace = 15 * time.Second -func TriggerWorkflow(instanceID, workflowID, actor string) (string, error) { +// TriggerWorkflow starts a run of workflow workflowID. +// +// tokenScope is the acting credential's tag restriction, nil meaning +// unrestricted. It is threaded down to ResolveTargetsScoped rather than being +// applied by the caller, because this is the one place a run's target set is +// decided: a handler that resolved targets itself and then called an unscoped +// trigger would leave the dispatch reaching further than the readout. +// +// A run whose configured targets fall entirely outside the caller's scope +// resolves to nothing and returns ErrNoTargets — the same answer a workflow +// targeting no servers at all gives, so an out-of-scope host stays +// indistinguishable from one that does not exist. +func TriggerWorkflow(instanceID, workflowID, actor string, tokenScope map[string]string) (string, error) { wf, err := GetWorkflow(instanceID, workflowID) if err != nil { return "", err } - targets, err := ResolveTargets(instanceID, wf.TargetServerIDs, wf.TargetTags) + targets, err := ResolveTargetsScoped(instanceID, wf.TargetServerIDs, wf.TargetTags, tokenScope) if err != nil { return "", err } diff --git a/server/internal/workflowsched/sched.go b/server/internal/workflowsched/sched.go index da57cfb..9a8c299 100644 --- a/server/internal/workflowsched/sched.go +++ b/server/internal/workflowsched/sched.go @@ -17,7 +17,7 @@ const tickInterval = 30 * time.Second // imported because services already imports this package for NextOccurrence, // and a package cannot import its own importer. type Deps struct { - TriggerWorkflow func(instanceID, workflowID, actor string) (string, error) + TriggerWorkflow func(instanceID, workflowID, actor string, tokenScope map[string]string) (string, error) LogEvent func(instanceID, eventType, actor, serverID, keyID, details string) } @@ -105,7 +105,13 @@ func process(ctx context.Context, deps Deps, wf models.Workflow, now time.Time) case SkipRunning: recordSkip(ctx, deps, wf, string(SkipRunning), dueAt, now) case Fire: - if _, err := deps.TriggerWorkflow(wf.InstanceID, wf.WorkflowID, "schedule"); err != nil { + // nil tokenScope: the scheduler acts as the system, not as any user. + // A schedule fires the workflow's own saved targets, and there is no + // acting credential whose tag restriction could narrow them — the + // person who armed the schedule is not present at fire time, and + // inheriting a restriction from whoever last saved the workflow would + // make a run's reach depend on an editor's credential. + if _, err := deps.TriggerWorkflow(wf.InstanceID, wf.WorkflowID, "schedule", nil); err != nil { log.Printf("workflowsched: trigger %s: %v", wf.WorkflowID, err) recordSkip(ctx, deps, wf, "error: "+err.Error(), dueAt, now) return