fix(security): scope workflow run dispatch and MCP run logs to the token's tags
This commit is contained in:
@@ -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"})
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user