Skip to content

Commit ee7b3fb

Browse files
committed
fix: keep webhook lookup team-scoped; team-scope webhook path index
Corrects the webhook approach from the previous commit. The /hooks/ endpoint is behind AuthMiddleware, so every webhook request carries the caller's authenticated team — it is not an anonymous public endpoint. Looking webhooks up by path alone (and running under the matched row's team) let an authenticated caller from team A trigger team B's webhook under B's credentials, a cross-tenant escalation (flagged in review). - LookupWebhookTrigger stays scoped to the caller's team; the handler runs the workflow under the request context (the caller's team), as before. - To still let two teams register the same webhook path, migration 022 now team-scopes the webhook path index too: (team_id, path). The caller's team disambiguates at lookup, so there is no ambiguity or leak. - Documented the down-migration limitation: recreating the global unique indexes fails if cross-team duplicates exist (operator must resolve them before downgrading). - Test rewritten: two teams register the same path; each caller resolves only its own trigger; a third team resolves nothing (no cross-team leak). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WnKgCRfMvFcZAkoVbr8k79
1 parent 2289607 commit ee7b3fb

5 files changed

Lines changed: 88 additions & 58 deletions

File tree

.changeset/team-scoped-trigger-uniqueness.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
"@mantle/engine": patch
33
---
44

5-
Make workflow trigger uniqueness multi-tenant-safe. The cron trigger uniqueness index is now scoped by team (`team_id, workflow_name, schedule`), so two teams can each register a workflow with the same name and schedule instead of the second team's apply failing with a unique-constraint violation. Webhook paths remain a global routing key (an inbound `POST /hooks/<path>` carries no team identity), and the webhook lookup is no longer team-scoped: it resolves by path and runs the workflow under the owning team from the matched trigger row, so webhooks registered by non-default teams now route correctly. Fixes #164.
5+
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.

packages/engine/internal/db/migrations/022_team_scoped_trigger_uniqueness.sql

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,34 @@
11
-- +goose Up
2-
-- 022_team_scoped_trigger_uniqueness: cron trigger uniqueness must be per-team.
2+
-- 022_team_scoped_trigger_uniqueness: trigger uniqueness must be per-team.
33
--
44
-- Workflow names are team-scoped (workflow_definitions is UNIQUE(team_id, name,
5-
-- version)), so two teams may legitimately own a workflow of the same name on
6-
-- the same schedule. The original index on (workflow_name, schedule) was global,
7-
-- so the second team's apply failed with a unique-constraint violation once
8-
-- trigger registration went live. Scope the uniqueness by team.
9-
--
10-
-- The webhook path index is intentionally left global: an inbound
11-
-- POST /hooks/<path> request carries no team identity, so the path is the global
12-
-- routing key and must be unique across all teams. Email triggers have no
13-
-- uniqueness index and are unaffected.
5+
-- version)) and the /hooks/ endpoint is authenticated, so trigger lookups always
6+
-- run under the caller's team. The original uniqueness indexes were global, so
7+
-- once trigger registration went live a second team registering the same cron
8+
-- (workflow_name, schedule) or the same webhook path would fail its apply with a
9+
-- unique-constraint violation. Scope both by team_id; the caller's team
10+
-- disambiguates at lookup time. Email triggers have no uniqueness index.
1411
DROP INDEX IF EXISTS idx_workflow_triggers_cron;
1512
CREATE UNIQUE INDEX idx_workflow_triggers_cron
1613
ON workflow_triggers (team_id, workflow_name, schedule)
1714
WHERE type = 'cron';
1815

16+
DROP INDEX IF EXISTS idx_workflow_triggers_webhook_path;
17+
CREATE UNIQUE INDEX idx_workflow_triggers_webhook_path
18+
ON workflow_triggers (team_id, path)
19+
WHERE type = 'webhook' AND enabled = true;
20+
1921
-- +goose Down
22+
-- NOTE: this rollback recreates the original GLOBAL unique indexes. If two teams
23+
-- have registered the same cron (workflow_name, schedule) or webhook path while
24+
-- the team-scoped indexes were in effect, recreating the global indexes will
25+
-- fail with a duplicate-key violation. That is an accepted rollback limitation:
26+
-- an operator must resolve the cross-team duplicates before downgrading.
27+
DROP INDEX IF EXISTS idx_workflow_triggers_webhook_path;
28+
CREATE UNIQUE INDEX idx_workflow_triggers_webhook_path
29+
ON workflow_triggers (path)
30+
WHERE type = 'webhook' AND enabled = true;
31+
2032
DROP INDEX IF EXISTS idx_workflow_triggers_cron;
2133
CREATE UNIQUE INDEX idx_workflow_triggers_cron
2234
ON workflow_triggers (workflow_name, schedule)

packages/engine/internal/server/trigger.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package server
33
import (
44
"context"
55
"database/sql"
6+
7+
"github.com/dvflw/mantle/internal/auth"
68
)
79

810
// TriggerRecord represents a registered trigger in the database.
@@ -41,16 +43,18 @@ func ListCronTriggers(ctx context.Context, db *sql.DB) ([]TriggerRecord, error)
4143
return scanTriggers(rows)
4244
}
4345

44-
// LookupWebhookTrigger finds the enabled webhook trigger for a path. Webhook
45-
// paths are a global routing key — an inbound POST /hooks/<path> carries no team
46-
// identity — so the lookup is not team-scoped and the path index is unique
47-
// across all teams. The owning team is read from the matched row so the caller
48-
// can run the workflow under the correct tenant.
46+
// LookupWebhookTrigger finds the enabled webhook trigger matching a path,
47+
// scoped to the caller's team. The /hooks/ endpoint is authenticated
48+
// (AuthMiddleware), so the caller's team is always present in ctx and forms a
49+
// tenant boundary: a caller may only trigger webhooks registered by its own
50+
// team, even if another team registered the same path. Webhook path uniqueness
51+
// is therefore team-scoped (see migration 022), not global.
4952
func LookupWebhookTrigger(ctx context.Context, db *sql.DB, path string) (*TriggerRecord, error) {
53+
teamID := auth.TeamIDFromContext(ctx)
5054
var t TriggerRecord
5155
err := db.QueryRowContext(ctx,
5256
`SELECT id, workflow_name, workflow_version, type, COALESCE(schedule, ''), COALESCE(path, ''), COALESCE(secret, ''), enabled, team_id
53-
FROM workflow_triggers WHERE type = 'webhook' AND path = $1 AND enabled = true`, path,
57+
FROM workflow_triggers WHERE type = 'webhook' AND path = $1 AND enabled = true AND team_id = $2`, path, teamID,
5458
).Scan(&t.ID, &t.WorkflowName, &t.WorkflowVersion, &t.Type, &t.Schedule, &t.Path, &t.Secret, &t.Enabled, &t.TeamID)
5559
if err == sql.ErrNoRows {
5660
return nil, nil

packages/engine/internal/server/trigger_test.go

Lines changed: 49 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -7,50 +7,68 @@ import (
77
"github.com/dvflw/mantle/internal/auth"
88
)
99

10-
// TestLookupWebhookTrigger_GlobalPathReturnsOwningTeam verifies that a webhook
11-
// registered by a non-default team is found by an unauthenticated inbound
12-
// lookup (which carries no team identity) and reports the owning team, so the
13-
// handler can run the workflow under the correct tenant. Previously the lookup
14-
// was team-scoped to the default team, so non-default-team webhooks never
15-
// routed.
16-
func TestLookupWebhookTrigger_GlobalPathReturnsOwningTeam(t *testing.T) {
10+
// TestLookupWebhookTrigger_TeamScoped verifies that webhook path uniqueness and
11+
// lookup are per-team: two teams may register the same path, and a lookup under
12+
// one team's context resolves only that team's trigger — never the other's. The
13+
// /hooks/ endpoint is authenticated, so the caller's team is a tenant boundary;
14+
// a caller must not be able to trigger another team's webhook that happens to
15+
// share a path.
16+
func TestLookupWebhookTrigger_TeamScoped(t *testing.T) {
1717
db, _ := setupWebhookTest(t)
1818
ctx := context.Background()
1919

20+
teamA := auth.DefaultTeamID
2021
teamB := "00000000-0000-0000-0000-0000000000b2"
2122
if _, err := db.ExecContext(ctx,
2223
`INSERT INTO teams (id, name) VALUES ($1, 'team-b')`, teamB); err != nil {
2324
t.Fatalf("creating team-b: %v", err)
2425
}
25-
if _, err := db.ExecContext(ctx,
26-
`INSERT INTO workflow_triggers (workflow_name, workflow_version, type, path, team_id)
27-
VALUES ('wf', 1, 'webhook', '/hooks/tb', $1)`, teamB); err != nil {
28-
t.Fatalf("seeding webhook trigger: %v", err)
29-
}
3026

31-
// No team in context, as an inbound webhook request would be.
32-
tr, err := LookupWebhookTrigger(ctx, db, "/hooks/tb")
33-
if err != nil {
34-
t.Fatalf("LookupWebhookTrigger: %v", err)
35-
}
36-
if tr == nil {
37-
t.Fatal("expected trigger, got nil")
27+
// Both teams register the same webhook path pointing at their own workflow.
28+
for _, tc := range []struct{ team, wf string }{
29+
{teamA, "wf-a"},
30+
{teamB, "wf-b"},
31+
} {
32+
if _, err := db.ExecContext(ctx,
33+
`INSERT INTO workflow_triggers (workflow_name, workflow_version, type, path, team_id)
34+
VALUES ($1, 1, 'webhook', '/hooks/shared', $2)`, tc.wf, tc.team); err != nil {
35+
t.Fatalf("seeding webhook trigger for %s: %v", tc.team, err)
36+
}
3837
}
39-
if tr.TeamID != teamB {
40-
t.Errorf("TeamID = %q, want %q", tr.TeamID, teamB)
41-
}
42-
if tr.WorkflowName != "wf" {
43-
t.Errorf("WorkflowName = %q, want wf", tr.WorkflowName)
38+
39+
// Each caller resolves only its own team's trigger.
40+
for _, tc := range []struct{ team, wantWF string }{
41+
{teamA, "wf-a"},
42+
{teamB, "wf-b"},
43+
} {
44+
callerCtx := auth.WithUser(ctx, &auth.User{TeamID: tc.team})
45+
tr, err := LookupWebhookTrigger(callerCtx, db, "/hooks/shared")
46+
if err != nil {
47+
t.Fatalf("lookup for %s: %v", tc.team, err)
48+
}
49+
if tr == nil {
50+
t.Fatalf("lookup for %s: expected trigger, got nil", tc.team)
51+
}
52+
if tr.WorkflowName != tc.wantWF {
53+
t.Errorf("team %s resolved workflow %q, want %q", tc.team, tr.WorkflowName, tc.wantWF)
54+
}
55+
if tr.TeamID != tc.team {
56+
t.Errorf("team %s resolved team_id %q, want %q", tc.team, tr.TeamID, tc.team)
57+
}
4458
}
4559

46-
// A default-team context must not change the result — the path is a global
47-
// routing key, not team-scoped.
48-
ctxDefault := auth.WithUser(ctx, &auth.User{TeamID: auth.DefaultTeamID})
49-
tr2, err := LookupWebhookTrigger(ctxDefault, db, "/hooks/tb")
60+
// A caller from a team with no matching path gets nothing — no cross-team leak.
61+
teamC := "00000000-0000-0000-0000-0000000000c3"
62+
if _, err := db.ExecContext(ctx,
63+
`INSERT INTO teams (id, name) VALUES ($1, 'team-c')`, teamC); err != nil {
64+
t.Fatalf("creating team-c: %v", err)
65+
}
66+
callerCtx := auth.WithUser(ctx, &auth.User{TeamID: teamC})
67+
tr, err := LookupWebhookTrigger(callerCtx, db, "/hooks/shared")
5068
if err != nil {
51-
t.Fatalf("LookupWebhookTrigger (default ctx): %v", err)
69+
t.Fatalf("lookup for team-c: %v", err)
5270
}
53-
if tr2 == nil || tr2.TeamID != teamB {
54-
t.Errorf("global lookup should be team-agnostic; got %+v", tr2)
71+
if tr != nil {
72+
t.Errorf("team-c should resolve no trigger, got %+v", tr)
5573
}
5674
}

packages/engine/internal/server/webhook.go

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import (
99
"io"
1010
"net/http"
1111
"strings"
12-
13-
"github.com/dvflw/mantle/internal/auth"
1412
)
1513

1614
// WebhookHandler handles incoming webhook requests and triggers workflow executions.
@@ -86,14 +84,12 @@ func (h *WebhookHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
8684
h.server.Logger.Info("webhook: triggering workflow",
8785
"path", path,
8886
"workflow", trigger.WorkflowName,
89-
"version", trigger.WorkflowVersion,
90-
"team_id", trigger.TeamID)
91-
92-
// Webhook requests are unauthenticated, so the tenant comes from the matched
93-
// trigger row rather than the request context. Run under that team so
94-
// workflow/credential resolution is correctly scoped.
95-
execCtx := auth.WithUser(r.Context(), &auth.User{TeamID: trigger.TeamID})
96-
execID, err := h.server.executeWorkflow(execCtx, trigger.WorkflowName, trigger.WorkflowVersion, inputs)
87+
"version", trigger.WorkflowVersion)
88+
89+
// The /hooks/ endpoint is authenticated, so r.Context() already carries the
90+
// caller's team — the same team the trigger was looked up under. Running with
91+
// it keeps workflow/credential resolution scoped to that tenant.
92+
execID, err := h.server.executeWorkflow(r.Context(), trigger.WorkflowName, trigger.WorkflowVersion, inputs)
9793
if err != nil {
9894
h.server.Logger.Error("webhook: execution failed",
9995
"workflow", trigger.WorkflowName,

0 commit comments

Comments
 (0)