Skip to content

Commit df7a868

Browse files
committed
fix(triggers): stop cloned trigger blocks reusing the source's webhook URL
Before a deploy, a trigger block's webhook URL is DERIVED — use-webhook-management falls back to the block's own id, which paste already regenerates, so cloning an undeployed trigger yields a fresh URL. Deploy then registers the webhook at `path = triggerPath || block.id` and writes that literal string back into the source block's `triggerPath`. From then on the URL is STORED, and copy/paste copies it verbatim, so the clone renders the source's URL. Clear the two identity fields (`webhookId`, `triggerPath`) on in-canvas clones so the clone falls back to deriving a path from its new block id. - Clear both the subBlocks structure AND the sub-block value map: the value map is authoritative in mergeSubblockStateWithValues, and `triggerPath` normally lives only there since no trigger declares it as a subblock. - Gate on `blockOwnsWebhookIdentity`, never on the subblock id: Discord, Attio, and Vercel action blocks expose a REQUIRED user-entered `webhookId` that must survive a copy. The predicate mirrors deploy's `resolveTriggerId` so both sides agree on which blocks own a webhook, and fails closed on unregistered types. - `triggerConfig`/`triggerId` are deliberately preserved — they are user configuration, and `triggerConfig` can be a legacy trigger's only config home. Scoped to the clone paths. No deploy-side, server-side, or import/export behavior changes.
1 parent edef789 commit df7a868

6 files changed

Lines changed: 325 additions & 4 deletions

File tree

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, type Mock, vi } from 'vitest'
5+
6+
vi.mock('@/blocks', () => ({ getBlock: vi.fn(), getAllBlocks: vi.fn(() => []) }))
7+
vi.mock('@/triggers', () => ({ getTrigger: vi.fn(), isTriggerValid: vi.fn() }))
8+
9+
import { blockOwnsWebhookIdentity } from '@/lib/workflows/triggers/trigger-utils'
10+
import { getBlock } from '@/blocks'
11+
import { isTriggerValid } from '@/triggers'
12+
13+
/** A dedicated trigger block, as the real registry describes `generic_webhook`. */
14+
const TRIGGER_BLOCK = {
15+
category: 'triggers',
16+
subBlocks: [{ id: 'webhookUrlDisplay', mode: 'trigger' }],
17+
}
18+
19+
/**
20+
* Discord: `category: 'tools'`, NO `triggers` config and NO `mode: 'trigger'` subblocks, but a
21+
* REQUIRED user-entered `webhookId` action field.
22+
*/
23+
const ACTION_BLOCK_WITH_WEBHOOK_ID = {
24+
category: 'tools',
25+
subBlocks: [{ id: 'webhookId', required: true }],
26+
}
27+
28+
/** Attio/Vercel: `category: 'tools'` that DO declare `triggers.enabled`. */
29+
const TRIGGER_CAPABLE_TOOL = {
30+
category: 'tools',
31+
subBlocks: [{ id: 'webhookId', required: true }],
32+
triggers: { enabled: true, available: ['attio_webhook'] },
33+
}
34+
35+
describe('blockOwnsWebhookIdentity', () => {
36+
beforeEach(() => {
37+
vi.clearAllMocks()
38+
;(isTriggerValid as Mock).mockReturnValue(true)
39+
})
40+
41+
/**
42+
* The production-dominant case. A standalone Webhook block is `category: 'triggers'` and its
43+
* `trigger_mode` column defaults to false, so a predicate keyed on `triggerMode` alone would
44+
* answer "no" for exactly the block the reported bug travels through.
45+
*/
46+
it('is true for a dedicated trigger block even with triggerMode false', () => {
47+
;(getBlock as Mock).mockReturnValue(TRIGGER_BLOCK)
48+
49+
expect(blockOwnsWebhookIdentity({ type: 'generic_webhook', triggerMode: false })).toBe(true)
50+
})
51+
52+
it('is false for a dedicated trigger block whose type is not a registered trigger', () => {
53+
;(getBlock as Mock).mockReturnValue(TRIGGER_BLOCK)
54+
;(isTriggerValid as Mock).mockReturnValue(false)
55+
56+
expect(blockOwnsWebhookIdentity({ type: 'retired_trigger', triggerMode: false })).toBe(false)
57+
})
58+
59+
/**
60+
* The regression this predicate exists to prevent: `triggerMode` is persisted raw, so it can be
61+
* true on a block with no trigger capability. Clearing there would wipe Discord's required
62+
* user-entered `webhookId` on every copy.
63+
*/
64+
it('is false for an action block with no trigger capability, even when triggerMode is true', () => {
65+
;(getBlock as Mock).mockReturnValue(ACTION_BLOCK_WITH_WEBHOOK_ID)
66+
67+
expect(blockOwnsWebhookIdentity({ type: 'discord', triggerMode: true })).toBe(false)
68+
})
69+
70+
it('is false for a trigger-capable tool used in action mode', () => {
71+
;(getBlock as Mock).mockReturnValue(TRIGGER_CAPABLE_TOOL)
72+
73+
expect(blockOwnsWebhookIdentity({ type: 'attio', triggerMode: false })).toBe(false)
74+
})
75+
76+
it('is true for a trigger-capable tool switched into trigger mode', () => {
77+
;(getBlock as Mock).mockReturnValue(TRIGGER_CAPABLE_TOOL)
78+
79+
expect(blockOwnsWebhookIdentity({ type: 'attio', triggerMode: true })).toBe(true)
80+
})
81+
82+
/**
83+
* Fails CLOSED. Callers strip on true, and clearing a field we cannot classify is recoverable,
84+
* whereas keeping a stale `triggerPath` risks the clone answering on the source's URL.
85+
*/
86+
it('is true for an unregistered block type', () => {
87+
;(getBlock as Mock).mockReturnValue(undefined)
88+
89+
expect(blockOwnsWebhookIdentity({ type: 'deleted_block_type', triggerMode: false })).toBe(true)
90+
})
91+
92+
it('treats a missing triggerMode the same as false', () => {
93+
;(getBlock as Mock).mockReturnValue(ACTION_BLOCK_WITH_WEBHOOK_ID)
94+
95+
expect(blockOwnsWebhookIdentity({ type: 'discord' })).toBe(false)
96+
})
97+
})

apps/sim/lib/workflows/triggers/trigger-utils.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { type StartBlockCandidate, StartBlockPath } from '@/lib/workflows/trigge
55
import { getAllBlocks, getBlock } from '@/blocks'
66
import type { BlockConfig } from '@/blocks/types'
77
import type { BlockState, WorkflowState } from '@/stores/workflows/workflow/types'
8-
import { getTrigger } from '@/triggers'
8+
import { getTrigger, isTriggerValid } from '@/triggers'
99

1010
const logger = createLogger('TriggerUtils')
1111

@@ -199,6 +199,34 @@ export function getAllTriggerBlocks(): TriggerInfo[] {
199199
})
200200
}
201201

202+
/**
203+
* Whether a block instance owns a webhook, so its {@link WEBHOOK_IDENTITY_SUBBLOCK_IDS} hold
204+
* runtime metadata a clone must not inherit rather than user configuration.
205+
*
206+
* Mirrors the deploy-side `resolveTriggerId` gate, and that equivalence is what makes it safe in
207+
* both directions: when this says no, deploy also resolves no trigger and registers no webhook, so
208+
* a stale path left on the block cannot collide with anything.
209+
*
210+
* Deliberately NOT {@link TriggerUtils.isTriggerBlock}, which is `category || triggerMode`. Two
211+
* reasons it cannot be reused here:
212+
* - `triggerMode` is persisted raw (`prepareBlockState` and the copilot's `buildBlockState` both
213+
* compute an `effectiveTriggerMode` for outputs yet store the unfiltered flag), so it can be
214+
* `true` on a block with no trigger capability at all — e.g. Discord, which declares no `triggers`
215+
* config and no `mode: 'trigger'` subblocks but does expose a REQUIRED user-entered `webhookId`
216+
* action field. Treating that as runtime metadata would wipe it on every copy.
217+
* - It also matches the legacy starter block, which owns no webhook.
218+
*/
219+
export function blockOwnsWebhookIdentity(block: Pick<BlockState, 'type' | 'triggerMode'>): boolean {
220+
const blockConfig = getBlock(block.type)
221+
// Unknown type (deprecated/renamed block, or a row written by an older deployment): fail CLOSED.
222+
// Callers strip on `true`, and clearing a field we cannot classify is recoverable, whereas
223+
// keeping a stale `triggerPath` risks the clone silently answering on the source's URL.
224+
if (!blockConfig) return true
225+
if (blockConfig.category === 'triggers' && isTriggerValid(block.type)) return true
226+
if (!block.triggerMode) return false
227+
return hasTriggerCapability(blockConfig)
228+
}
229+
202230
/**
203231
* Check if a block has trigger capability (contains trigger mode subblocks)
204232
*/

apps/sim/stores/workflows/utils.test.ts

Lines changed: 137 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,38 @@ import {
66
createStarterBlock,
77
} from '@sim/testing'
88
import type { Edge } from 'reactflow'
9-
import { describe, expect, it } from 'vitest'
9+
import { describe, expect, it, vi } from 'vitest'
1010
import { normalizeName } from '@/executor/constants'
1111
import { getUniqueBlockName, regenerateBlockIds } from './utils'
1212

13+
// The global `@/blocks/registry` mock returns a config with no `category`, which would make
14+
// `blockOwnsWebhookIdentity` answer "not a trigger" for every type and let these tests pass while
15+
// the production predicate is broken. Override it with the shape the real registry returns.
16+
vi.mock('@/blocks/registry', () => ({
17+
getBlock: vi.fn((type: string) =>
18+
type === 'generic_webhook'
19+
? {
20+
name: 'Webhook',
21+
description: 'Webhook trigger',
22+
icon: () => null,
23+
category: 'triggers',
24+
subBlocks: [{ id: 'webhookUrlDisplay', mode: 'trigger' }],
25+
outputs: {},
26+
}
27+
: {
28+
name: 'Mock Block',
29+
description: 'Mock block description',
30+
icon: () => null,
31+
category: 'tools',
32+
subBlocks: [],
33+
outputs: {},
34+
}
35+
),
36+
getAllBlocks: vi.fn(() => []),
37+
getLatestBlock: vi.fn(() => undefined),
38+
getBlockByToolName: vi.fn(() => undefined),
39+
}))
40+
1341
describe('normalizeName', () => {
1442
it.concurrent('should convert to lowercase', () => {
1543
expect(normalizeName('MyVariable')).toBe('myvariable')
@@ -857,3 +885,111 @@ describe('regenerateBlockIds', () => {
857885
expect(names).toContain('Agent 2')
858886
})
859887
})
888+
889+
describe('regenerateBlockIds — webhook identity on clones', () => {
890+
const positionOffset = { x: 50, y: 50 }
891+
const sourceId = 'webhook-source'
892+
const deployedPath = 'webhook-source'
893+
894+
function pasteOne(block: Record<string, unknown>, values: Record<string, unknown>) {
895+
const blocks = { [sourceId]: createBlock({ id: sourceId, ...block }) as never }
896+
const result = regenerateBlockIds(
897+
blocks,
898+
[],
899+
{},
900+
{},
901+
{ [sourceId]: values },
902+
positionOffset,
903+
{},
904+
getUniqueBlockName
905+
)
906+
const newId = Object.keys(result.blocks)[0]
907+
return { newId, result }
908+
}
909+
910+
/**
911+
* The reported bug, in its production shape: a dedicated trigger block has `category: 'triggers'`
912+
* and `triggerMode: false`. Deploy writes `webhook.path` (its own block id) back into
913+
* `triggerPath`, after which a clone renders the SOURCE's URL instead of deriving its own.
914+
*/
915+
it('clears the webhook identity on a pasted dedicated trigger block', () => {
916+
const { newId, result } = pasteOne(
917+
{
918+
type: 'generic_webhook',
919+
name: 'Webhook 1',
920+
triggerMode: false,
921+
subBlocks: {
922+
triggerPath: { id: 'triggerPath', type: 'short-input', value: deployedPath },
923+
webhookId: { id: 'webhookId', type: 'short-input', value: 'wh_original' },
924+
},
925+
},
926+
{ triggerPath: deployedPath, webhookId: 'wh_original' }
927+
)
928+
929+
expect(newId).not.toBe(sourceId)
930+
// Both sources must be cleared: the value map overrides the structure in mergeSubblockState.
931+
expect(result.blocks[newId].subBlocks.triggerPath?.value).toBeNull()
932+
expect(result.blocks[newId].subBlocks.webhookId?.value).toBeNull()
933+
expect(result.subBlockValues[newId].triggerPath).toBeNull()
934+
expect(result.subBlockValues[newId].webhookId).toBeNull()
935+
})
936+
937+
it('clears it when the path lives only in the value map, not the structure', () => {
938+
const { newId, result } = pasteOne(
939+
{ type: 'generic_webhook', name: 'Webhook 1', triggerMode: false, subBlocks: {} },
940+
{ triggerPath: deployedPath }
941+
)
942+
943+
expect(result.subBlockValues[newId].triggerPath).toBeNull()
944+
})
945+
946+
/**
947+
* Discord/Attio/Vercel expose a REQUIRED user-entered `webhookId`. Clearing by subblock id alone
948+
* would silently blank it, leaving the copy half-configured next to a surviving `webhookToken`.
949+
*/
950+
it('preserves a user-entered webhookId on an action block', () => {
951+
const { newId, result } = pasteOne(
952+
{
953+
type: 'discord',
954+
name: 'Discord 1',
955+
triggerMode: false,
956+
subBlocks: {
957+
webhookId: { id: 'webhookId', type: 'short-input', value: '1234567890' },
958+
webhookToken: { id: 'webhookToken', type: 'short-input', value: 'tok_abc' },
959+
},
960+
},
961+
{ webhookId: '1234567890', webhookToken: 'tok_abc' }
962+
)
963+
964+
expect(result.blocks[newId].subBlocks.webhookId?.value).toBe('1234567890')
965+
expect(result.subBlockValues[newId].webhookId).toBe('1234567890')
966+
expect(result.subBlockValues[newId].webhookToken).toBe('tok_abc')
967+
})
968+
969+
it('preserves trigger configuration on a cloned trigger block', () => {
970+
const { newId, result } = pasteOne(
971+
{
972+
type: 'generic_webhook',
973+
name: 'Webhook 1',
974+
triggerMode: false,
975+
subBlocks: {
976+
triggerPath: { id: 'triggerPath', type: 'short-input', value: deployedPath },
977+
triggerConfig: { id: 'triggerConfig', type: 'short-input', value: { labelIds: ['a'] } },
978+
triggerId: { id: 'triggerId', type: 'short-input', value: 'generic_webhook' },
979+
token: { id: 'token', type: 'short-input', value: 'user-secret' },
980+
},
981+
},
982+
{
983+
triggerPath: deployedPath,
984+
triggerConfig: { labelIds: ['a'] },
985+
triggerId: 'generic_webhook',
986+
token: 'user-secret',
987+
}
988+
)
989+
990+
expect(result.subBlockValues[newId].triggerPath).toBeNull()
991+
expect(result.subBlockValues[newId].triggerConfig).toEqual({ labelIds: ['a'] })
992+
expect(result.subBlockValues[newId].triggerId).toBe('generic_webhook')
993+
expect(result.subBlockValues[newId].token).toBe('user-secret')
994+
})
995+
})

apps/sim/stores/workflows/utils.ts

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import { remapConditionBlockIds, remapConditionEdgeHandle } from '@/lib/workflow
88
import { isDynamicHandleSubblock } from '@/lib/workflows/dynamic-handle-topology'
99
import { createDefaultInputFormatField } from '@/lib/workflows/input-format'
1010
import { buildDefaultCanonicalModes } from '@/lib/workflows/subblocks/visibility'
11-
import { hasTriggerCapability } from '@/lib/workflows/triggers/trigger-utils'
11+
import {
12+
blockOwnsWebhookIdentity,
13+
hasTriggerCapability,
14+
} from '@/lib/workflows/triggers/trigger-utils'
1215
import { getBlock } from '@/blocks'
1316
import { escapeRegExp, normalizeName } from '@/executor/constants'
1417
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
@@ -22,7 +25,7 @@ import type {
2225
SubBlockState,
2326
WorkflowState,
2427
} from '@/stores/workflows/workflow/types'
25-
import { TRIGGER_RUNTIME_SUBBLOCK_IDS } from '@/triggers/constants'
28+
import { TRIGGER_RUNTIME_SUBBLOCK_IDS, WEBHOOK_IDENTITY_SUBBLOCK_IDS } from '@/triggers/constants'
2629

2730
/** Threshold to detect viewport-based offsets vs small duplicate offsets */
2831
const LARGE_OFFSET_THRESHOLD = 300
@@ -251,6 +254,38 @@ function updateValueReferences(value: unknown, nameMap: Map<string, string>): un
251254
return value
252255
}
253256

257+
/**
258+
* Clears the {@link WEBHOOK_IDENTITY_SUBBLOCK_IDS} a cloned trigger block must not inherit, across
259+
* BOTH the sub-block structure and the sub-block value map.
260+
*
261+
* Both are required. `mergeSubblockStateWithValues` treats the value map as authoritative — a `null`
262+
* there overrides the structure — but only materializes an entry for a structure-less key when the
263+
* value is non-null. So nulling the map covers the common shape (`triggerPath` is written into the
264+
* store by `useWebhookManagement` and is never declared as a subblock), and clearing the structure
265+
* covers blocks whose subBlocks were hydrated from a merge. Either way the clone resolves no path,
266+
* and deploy's `triggerPath || block.id` fallback derives a fresh one from its new block id.
267+
*
268+
* Gated on {@link blockOwnsWebhookIdentity}, never on the subblock id alone: Discord, Attio, and
269+
* Vercel action blocks expose a REQUIRED user-entered `webhookId` that must survive a copy.
270+
*
271+
* Mutates both arguments in place; both must be clone-owned copies. `subBlockValues` is optional so
272+
* a caller with no value-map entry passes `undefined` rather than a throwaway object literal whose
273+
* writes would be silently discarded.
274+
*/
275+
export function stripWebhookIdentityForClone(
276+
block: Pick<BlockState, 'type' | 'triggerMode'>,
277+
subBlocks: Record<string, SubBlockState>,
278+
subBlockValues: Record<string, unknown> | undefined
279+
): void {
280+
if (!blockOwnsWebhookIdentity(block)) return
281+
282+
for (const subBlockId of WEBHOOK_IDENTITY_SUBBLOCK_IDS) {
283+
const subBlock = subBlocks[subBlockId]
284+
if (subBlock) subBlocks[subBlockId] = { ...subBlock, value: null }
285+
if (subBlockValues && subBlockId in subBlockValues) subBlockValues[subBlockId] = null
286+
}
287+
}
288+
254289
function updateBlockReferences(
255290
blocks: Record<string, BlockState>,
256291
nameMap: Map<string, string>,
@@ -565,6 +600,10 @@ export function regenerateBlockIds(
565600
})
566601
})
567602

603+
Object.entries(newBlocks).forEach(([blockId, block]) => {
604+
stripWebhookIdentityForClone(block, block.subBlocks, newSubBlockValues[blockId])
605+
})
606+
568607
return {
569608
blocks: newBlocks,
570609
edges: newEdges,

apps/sim/stores/workflows/workflow/store.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
getUniqueBlockName,
1818
mergeSubblockState,
1919
remapConditionIds,
20+
stripWebhookIdentityForClone,
2021
} from '@/stores/workflows/utils'
2122
import type {
2223
Position,
@@ -599,6 +600,11 @@ export const useWorkflowStore = create<WorkflowStore>()(
599600
id,
600601
newId
601602
)
603+
stripWebhookIdentityForClone(
604+
block,
605+
newSubBlocks as Record<string, SubBlockState>,
606+
clonedSubBlockValues
607+
)
602608

603609
const newState = {
604610
blocks: {

0 commit comments

Comments
 (0)