Vantage/vantage-app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: close privilege escalation via workflow TargetTags on REST create/update

Browse Source 
CreateWorkflow and UpdateWorkflow validated TargetServerIDs against the
caller's scope but never checked TargetTags, letting a restricted token
save a workflow with an empty ID list and an out-of-scope tag selector,
then reach those hosts once the scheduler fires it as the system with no
restriction of its own. Add validateWorkflowTargetScope, applying the
same all-or-nothing rule the MCP create_workflow tool already enforces:
resolve the full target set unscoped and scoped, refuse unless they
match. Update the PUT /api/workflows/:id/schedule fleetWide comment to
say why it is safe now (targets are constrained at write time) rather
than repeating the falsified claim that scheduling reaches nothing new.
This commit is contained in:
mrhid6
2026-09-09 11:45:39 +00:00
parent 52ac966cba
commit 2e3d2a33f9
3 changed files with 163 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/internal/api/serverscope.go
+23 -12
View File
@@ -216,18 +216,29 @@ var serverScopedRoutes = map[string]scopeDecl{
"GET /api/runs/:runId/servers/:serverId/logs": scoped,
"GET /api/runs/:runId/servers/:serverId/logs/stream": scoped,
// Deleting a workflow, cancelling a run and arming a schedule all act on a
// definition rather than on a server, and none of them returns server
// data. Each can nevertheless reach a definition whose targets a
// restricted token cannot see — a cancel stops work on out-of-scope hosts,
// a schedule arms it there. That reach is real but bounded: the caller
// learns nothing about which hosts are involved (both /workflows listings
// are scoped), and a scope-narrowed variant of "cancel this run" would
// have to either half-cancel a run or refuse one whose targets are mixed,
// neither of which is a better answer than the current one. Recorded as a
// deliberate choice, not an oversight.
"DELETE /api/workflows/:id": fleetWide,
"POST /api/runs/:runId/cancel": fleetWide,
// Deleting a workflow and cancelling a run both act on a definition rather
// than on a server, and neither returns server data. Each can
// nevertheless reach a definition whose targets a restricted token cannot
// see — a cancel stops work on out-of-scope hosts. That reach is real but
// bounded: the caller learns nothing about which hosts are involved (both
// /workflows listings are scoped), and a scope-narrowed variant of
// "cancel this run" would have to either half-cancel a run or refuse one
// whose targets are mixed, neither of which is a better answer than the
// current one. Recorded as a deliberate choice, not an oversight.
"DELETE /api/workflows/:id": fleetWide,
"POST /api/runs/:runId/cancel": fleetWide,
// Arming a schedule applies no scope check of its own, and that is safe
// only because it has nothing left to check: CreateWorkflow and
// UpdateWorkflow (internal/services/workflows.go) already refuse to save
// a workflow whose resolved targets — TargetServerIDs union TargetTags —
// reach outside the acting credential's scope, the same all-or-nothing
// rule the MCP create_workflow tool applies. So by the time a workflow
// exists to be scheduled, its targets were already constrained to
// whichever scope wrote them. The scheduler later fires it with a nil
// token scope, acting as the system rather than as any caller, and that
// is fine precisely because the targets were fixed at write time, not at
// fire time.
"PUT /api/workflows/:id/schedule": fleetWide,
"GET /api/workflows/:id/schedule/preview": exempt,
server/internal/services/workflow_target_scope_test.go
+98
View File
@@ -0,0 +1,98 @@
package services
import (
"testing"
"gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/models"
)
// allowSaveTargets is the pure core of validateWorkflowTargetScope: the
// all-or-nothing rule that CreateWorkflow/UpdateWorkflow now enforce, and
// that create_workflow already enforced on the MCP surface. Proved here
// without a database by resolving over an in-memory fleet the same way
// ResolveTargets/ResolveTargetsScoped do.
func allowSaveTargets(all []models.Server, ids []string, tags, tokenScope map[string]string) bool {
allTargets := UnionTargets(all, ids, tags)
if len(allTargets) == 0 {
// No targets at all, or none matched — nothing to escalate through.
return true
}
if tokenScope == nil {
return true
}
visible := make([]models.Server, 0, len(all))
for _, s := range all {
if ServerInTokenScope(s, tokenScope) {
visible = append(visible, s)
}
}
scopedTargets := UnionTargets(visible, ids, tags)
return len(scopedTargets) == len(allTargets)
}
// This is the exact hole: an empty TargetServerIDs with a TargetTags selector
// reaching outside the caller's scope must be refused at save time, not left
// to surface later when the scheduler fires the workflow with no restriction
// of its own.
func TestSaveTargetsRestrictedCallerCannotReachOutsideScope(t *testing.T) {
fleet := []models.Server{
{ServerID: "stg-1", Tags: map[string]string{"env": "staging"}},
{ServerID: "prod-1", Tags: map[string]string{"env": "production"}},
}
staging := map[string]string{"env": "staging"}
if allowSaveTargets(fleet, nil, map[string]string{"env": "production"}, staging) {
t.Error("staging-scoped caller allowed to save a workflow targeting env=production by tag")
}
if allowSaveTargets(fleet, []string{"prod-1"}, nil, staging) {
t.Error("staging-scoped caller allowed to save a workflow naming an out-of-scope server ID")
}
}
func TestSaveTargetsUnrestrictedCallerUnaffected(t *testing.T) {
fleet := []models.Server{
{ServerID: "stg-1", Tags: map[string]string{"env": "staging"}},
{ServerID: "prod-1", Tags: map[string]string{"env": "production"}},
}
if !allowSaveTargets(fleet, nil, map[string]string{"env": "production"}, nil) {
t.Error("nil (unrestricted) token scope was blocked from a valid target selector")
}
if !allowSaveTargets(fleet, []string{"prod-1"}, nil, nil) {
t.Error("nil (unrestricted) token scope was blocked from naming a server by ID")
}
}
func TestSaveTargetsEqualOrNarrowerSelectorAllowed(t *testing.T) {
fleet := []models.Server{
{ServerID: "stg-1", Tags: map[string]string{"env": "staging", "team": "core"}},
{ServerID: "stg-2", Tags: map[string]string{"env": "staging", "team": "web"}},
}
staging := map[string]string{"env": "staging"}
// Equal selector: everything the caller can already see.
if !allowSaveTargets(fleet, nil, map[string]string{"env": "staging"}, staging) {
t.Error("equal selector refused")
}
// Narrower selector: a strict subset of the caller's scope.
if !allowSaveTargets(fleet, nil, map[string]string{"env": "staging", "team": "core"}, staging) {
t.Error("narrower selector refused")
}
}
// An empty TargetTags selector matches nothing (MatchesTags is deliberately
// exclusive on empty), and a workflow with no targets at all must stay
// creatable — the scope check must not turn that into a refusal by accident.
func TestSaveTargetsNoTargetsAtAllIsUnaffected(t *testing.T) {
fleet := []models.Server{
{ServerID: "stg-1", Tags: map[string]string{"env": "staging"}},
}
staging := map[string]string{"env": "staging"}
if !allowSaveTargets(fleet, nil, nil, staging) {
t.Error("workflow with no targets at all was refused for a restricted caller")
}
if !allowSaveTargets(fleet, nil, nil, nil) {
t.Error("workflow with no targets at all was refused for an unrestricted caller")
}
}
server/internal/services/workflows.go
+42
View File
@@ -261,6 +261,9 @@ func CreateWorkflow(instanceID string, w models.Workflow, tokenScope map[string]
if err := validateTargetServers(instanceID, w.TargetServerIDs, tokenScope); err != nil {
return nil, err
}
if err := validateWorkflowTargetScope(instanceID, w.TargetServerIDs, w.TargetTags, tokenScope); err != nil {
return nil, err
}
normalizeInlineSteps(&w)
if _, err := db.Col("workflows").InsertOne(ctx, w); err != nil {
return nil, err
@@ -280,6 +283,9 @@ func UpdateWorkflow(instanceID, id string, w models.Workflow, tokenScope map[str
if err := validateTargetServers(instanceID, w.TargetServerIDs, tokenScope); err != nil {
return err
}
if err := validateWorkflowTargetScope(instanceID, w.TargetServerIDs, w.TargetTags, tokenScope); err != nil {
return err
}
normalizeInlineSteps(&w)
_, err := db.Col("workflows").UpdateOne(ctx, bson.M{"workflow_id": id, "instance_id": instanceID}, bson.M{"$set": bson.M{
"name": w.Name,
@@ -313,6 +319,42 @@ func validateTargetServers(instanceID string, serverIDs []string, tokenScope map
return nil
}
// validateWorkflowTargetScope refuses to save a workflow whose combined
// targets (IDs union tags) reach outside the acting credential's scope.
//
// validateTargetServers only checks the ID half; a token restricted to
// env=staging could otherwise leave TargetServerIDs empty, set TargetTags to
// {env: production}, and reach those hosts later through the scheduler, which
// fires with no restriction of its own because it acts as the system rather
// than as any caller. This is the same all-or-nothing rule create_workflow
// enforces on the MCP surface: resolve the full target set unscoped and again
// scoped, and refuse unless they match exactly.
//
// A nil tokenScope is unrestricted and always passes. A workflow with no
// targets at all resolves to ErrNoTargets on both sides and is left alone —
// that is an existing, separate concern (validated elsewhere, or simply
// allowed), not a scope violation.
//
// The refusal message matches validateTargetServers' so a restricted caller
// cannot tell "these hosts exist but you cannot have them" apart from "your
// tag selector matches nothing"; it must not become a fleet enumeration
// oracle.
func validateWorkflowTargetScope(instanceID string, serverIDs []string, tags map[string]string, tokenScope map[string]string) error {
if tokenScope == nil {
return nil
}
allTargets, err := ResolveTargets(instanceID, serverIDs, tags)
if err != nil {
// No targets at all, or none matched — nothing to escalate through.
return nil
}
scopedTargets, err := ResolveTargetsScoped(instanceID, serverIDs, tags, tokenScope)
if err != nil || len(scopedTargets) != len(allTargets) {
return fmt.Errorf("target tags match no servers visible to this token")
}
return nil
}
func normalizeInlineSteps(w *models.Workflow) {
for i := range w.Steps {
in := w.Steps[i].Inline