diff --git a/.changeset/team-scoped-trigger-uniqueness.md b/.changeset/team-scoped-trigger-uniqueness.md new file mode 100644 index 0000000..c327c27 --- /dev/null +++ b/.changeset/team-scoped-trigger-uniqueness.md @@ -0,0 +1,5 @@ +--- +"@mantle/engine": patch +--- + +Make workflow trigger uniqueness multi-tenant-safe. Both the cron uniqueness index (`team_id, workflow_name, schedule`) and the webhook path index (`team_id, path`) are now scoped by team, so two teams can each register a workflow with the same name/schedule or the same webhook path instead of the second team's apply failing with a unique-constraint violation. Webhook lookup stays scoped to the caller's authenticated team (the `/hooks/` endpoint is authenticated), so a caller can only trigger its own team's webhooks even when another team uses the same path. Email triggers have no uniqueness index and are unaffected. Fixes #164. diff --git a/packages/engine/internal/db/migrations/022_team_scoped_trigger_uniqueness.sql b/packages/engine/internal/db/migrations/022_team_scoped_trigger_uniqueness.sql new file mode 100644 index 0000000..ad44061 --- /dev/null +++ b/packages/engine/internal/db/migrations/022_team_scoped_trigger_uniqueness.sql @@ -0,0 +1,35 @@ +-- +goose Up +-- 022_team_scoped_trigger_uniqueness: trigger uniqueness must be per-team. +-- +-- Workflow names are team-scoped (workflow_definitions is UNIQUE(team_id, name, +-- version)) and the /hooks/ endpoint is authenticated, so trigger lookups always +-- run under the caller's team. The original uniqueness indexes were global, so +-- once trigger registration went live a second team registering the same cron +-- (workflow_name, schedule) or the same webhook path would fail its apply with a +-- unique-constraint violation. Scope both by team_id; the caller's team +-- disambiguates at lookup time. Email triggers have no uniqueness index. +DROP INDEX IF EXISTS idx_workflow_triggers_cron; +CREATE UNIQUE INDEX idx_workflow_triggers_cron + ON workflow_triggers (team_id, workflow_name, schedule) + WHERE type = 'cron'; + +DROP INDEX IF EXISTS idx_workflow_triggers_webhook_path; +CREATE UNIQUE INDEX idx_workflow_triggers_webhook_path + ON workflow_triggers (team_id, path) + WHERE type = 'webhook' AND enabled = true; + +-- +goose Down +-- NOTE: this rollback recreates the original GLOBAL unique indexes. If two teams +-- have registered the same cron (workflow_name, schedule) or webhook path while +-- the team-scoped indexes were in effect, recreating the global indexes will +-- fail with a duplicate-key violation. That is an accepted rollback limitation: +-- an operator must resolve the cross-team duplicates before downgrading. +DROP INDEX IF EXISTS idx_workflow_triggers_webhook_path; +CREATE UNIQUE INDEX idx_workflow_triggers_webhook_path + ON workflow_triggers (path) + WHERE type = 'webhook' AND enabled = true; + +DROP INDEX IF EXISTS idx_workflow_triggers_cron; +CREATE UNIQUE INDEX idx_workflow_triggers_cron + ON workflow_triggers (workflow_name, schedule) + WHERE type = 'cron'; diff --git a/packages/engine/internal/server/trigger.go b/packages/engine/internal/server/trigger.go index f7308d8..3d52a11 100644 --- a/packages/engine/internal/server/trigger.go +++ b/packages/engine/internal/server/trigger.go @@ -43,14 +43,19 @@ func ListCronTriggers(ctx context.Context, db *sql.DB) ([]TriggerRecord, error) return scanTriggers(rows) } -// LookupWebhookTrigger finds the trigger matching a webhook path, scoped to the team. +// LookupWebhookTrigger finds the enabled webhook trigger matching a path, +// scoped to the caller's team. The /hooks/ endpoint is authenticated +// (AuthMiddleware), so the caller's team is always present in ctx and forms a +// tenant boundary: a caller may only trigger webhooks registered by its own +// team, even if another team registered the same path. Webhook path uniqueness +// is therefore team-scoped (see migration 022), not global. func LookupWebhookTrigger(ctx context.Context, db *sql.DB, path string) (*TriggerRecord, error) { teamID := auth.TeamIDFromContext(ctx) var t TriggerRecord err := db.QueryRowContext(ctx, - `SELECT id, workflow_name, workflow_version, type, COALESCE(schedule, ''), COALESCE(path, ''), COALESCE(secret, ''), enabled + `SELECT id, workflow_name, workflow_version, type, COALESCE(schedule, ''), COALESCE(path, ''), COALESCE(secret, ''), enabled, team_id FROM workflow_triggers WHERE type = 'webhook' AND path = $1 AND enabled = true AND team_id = $2`, path, teamID, - ).Scan(&t.ID, &t.WorkflowName, &t.WorkflowVersion, &t.Type, &t.Schedule, &t.Path, &t.Secret, &t.Enabled) + ).Scan(&t.ID, &t.WorkflowName, &t.WorkflowVersion, &t.Type, &t.Schedule, &t.Path, &t.Secret, &t.Enabled, &t.TeamID) if err == sql.ErrNoRows { return nil, nil } diff --git a/packages/engine/internal/server/trigger_test.go b/packages/engine/internal/server/trigger_test.go new file mode 100644 index 0000000..98e7401 --- /dev/null +++ b/packages/engine/internal/server/trigger_test.go @@ -0,0 +1,74 @@ +package server + +import ( + "context" + "testing" + + "github.com/dvflw/mantle/internal/auth" +) + +// TestLookupWebhookTrigger_TeamScoped verifies that webhook path uniqueness and +// lookup are per-team: two teams may register the same path, and a lookup under +// one team's context resolves only that team's trigger — never the other's. The +// /hooks/ endpoint is authenticated, so the caller's team is a tenant boundary; +// a caller must not be able to trigger another team's webhook that happens to +// share a path. +func TestLookupWebhookTrigger_TeamScoped(t *testing.T) { + db, _ := setupWebhookTest(t) + ctx := context.Background() + + teamA := auth.DefaultTeamID + teamB := "00000000-0000-0000-0000-0000000000b2" + if _, err := db.ExecContext(ctx, + `INSERT INTO teams (id, name) VALUES ($1, 'team-b')`, teamB); err != nil { + t.Fatalf("creating team-b: %v", err) + } + + // Both teams register the same webhook path pointing at their own workflow. + for _, tc := range []struct{ team, wf string }{ + {teamA, "wf-a"}, + {teamB, "wf-b"}, + } { + if _, err := db.ExecContext(ctx, + `INSERT INTO workflow_triggers (workflow_name, workflow_version, type, path, team_id) + VALUES ($1, 1, 'webhook', '/hooks/shared', $2)`, tc.wf, tc.team); err != nil { + t.Fatalf("seeding webhook trigger for %s: %v", tc.team, err) + } + } + + // Each caller resolves only its own team's trigger. + for _, tc := range []struct{ team, wantWF string }{ + {teamA, "wf-a"}, + {teamB, "wf-b"}, + } { + callerCtx := auth.WithUser(ctx, &auth.User{TeamID: tc.team}) + tr, err := LookupWebhookTrigger(callerCtx, db, "/hooks/shared") + if err != nil { + t.Fatalf("lookup for %s: %v", tc.team, err) + } + if tr == nil { + t.Fatalf("lookup for %s: expected trigger, got nil", tc.team) + } + if tr.WorkflowName != tc.wantWF { + t.Errorf("team %s resolved workflow %q, want %q", tc.team, tr.WorkflowName, tc.wantWF) + } + if tr.TeamID != tc.team { + t.Errorf("team %s resolved team_id %q, want %q", tc.team, tr.TeamID, tc.team) + } + } + + // A caller from a team with no matching path gets nothing — no cross-team leak. + teamC := "00000000-0000-0000-0000-0000000000c3" + if _, err := db.ExecContext(ctx, + `INSERT INTO teams (id, name) VALUES ($1, 'team-c')`, teamC); err != nil { + t.Fatalf("creating team-c: %v", err) + } + callerCtx := auth.WithUser(ctx, &auth.User{TeamID: teamC}) + tr, err := LookupWebhookTrigger(callerCtx, db, "/hooks/shared") + if err != nil { + t.Fatalf("lookup for team-c: %v", err) + } + if tr != nil { + t.Errorf("team-c should resolve no trigger, got %+v", tr) + } +} diff --git a/packages/engine/internal/server/webhook.go b/packages/engine/internal/server/webhook.go index c92b7ba..b6921e9 100644 --- a/packages/engine/internal/server/webhook.go +++ b/packages/engine/internal/server/webhook.go @@ -86,6 +86,9 @@ func (h *WebhookHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { "workflow", trigger.WorkflowName, "version", trigger.WorkflowVersion) + // The /hooks/ endpoint is authenticated, so r.Context() already carries the + // caller's team — the same team the trigger was looked up under. Running with + // it keeps workflow/credential resolution scoped to that tenant. execID, err := h.server.executeWorkflow(r.Context(), trigger.WorkflowName, trigger.WorkflowVersion, inputs) if err != nil { h.server.Logger.Error("webhook: execution failed", diff --git a/packages/engine/internal/workflow/triggers_test.go b/packages/engine/internal/workflow/triggers_test.go index 3641c0d..81d8bd1 100644 --- a/packages/engine/internal/workflow/triggers_test.go +++ b/packages/engine/internal/workflow/triggers_test.go @@ -4,6 +4,8 @@ import ( "context" "database/sql" "testing" + + "github.com/dvflw/mantle/internal/auth" ) // countTriggers returns the number of workflow_triggers rows for a workflow, @@ -190,6 +192,63 @@ func TestSave_BackfillsTriggersOnUnchangedApply(t *testing.T) { } } +// TestSave_CronTriggersUniquePerTeam verifies that two teams can each register +// a workflow with the same name and cron schedule. Before the uniqueness index +// was team-scoped, the second team's apply failed with a unique-constraint +// violation on (workflow_name, schedule). +func TestSave_CronTriggersUniquePerTeam(t *testing.T) { + database := setupTestDB(t) + ctx := context.Background() + + // A second team — both workflow_definitions.team_id and + // workflow_triggers.team_id are foreign keys to teams(id). + teamB := "00000000-0000-0000-0000-0000000000b2" + if _, err := database.ExecContext(ctx, + `INSERT INTO teams (id, name) VALUES ($1, 'team-b')`, teamB); err != nil { + t.Fatalf("creating team-b: %v", err) + } + + yaml := []byte(`name: shared-name +triggers: + - type: cron + schedule: "*/5 * * * *" +steps: + - name: s + action: http/request + params: + method: GET + url: "https://example.com" +`) + result, err := ParseBytes(yaml) + if err != nil { + t.Fatalf("ParseBytes: %v", err) + } + + // Default team. + if _, err := Save(ctx, database, result, yaml); err != nil { + t.Fatalf("Save (default team): %v", err) + } + // team-b — same workflow name and schedule. + ctxB := auth.WithUser(ctx, &auth.User{TeamID: teamB}) + if _, err := Save(ctxB, database, result, yaml); err != nil { + t.Fatalf("Save (team-b): %v", err) + } + + // Each team owns exactly one cron trigger for the workflow. + for _, tid := range []string{auth.DefaultTeamID, teamB} { + var n int + if err := database.QueryRowContext(ctx, + `SELECT COUNT(*) FROM workflow_triggers + WHERE workflow_name = 'shared-name' AND type = 'cron' AND team_id = $1`, tid, + ).Scan(&n); err != nil { + t.Fatalf("counting triggers for %s: %v", tid, err) + } + if n != 1 { + t.Errorf("team %s cron triggers = %d, want 1", tid, n) + } + } +} + // TestSave_BackfillPreservesDisabledState verifies that backfilling triggers // for a currently-disabled workflow inserts them disabled, so a pruned // workflow does not start firing on a re-apply.