Skip to content

Commit 8cd0127

Browse files
committed
fix(triggers): stop copy/paste and duplicate reusing a trigger block's webhook URL
Pasting or duplicating a deployed webhook trigger block carried the source's triggerPath and webhookId onto the clone. The clone rendered the original's public URL, and deploy resolved both blocks to one path — landing on the path_deployment_unique index as an opaque 500 rather than an actionable error. PR #1784 fixed this for duplicate, but the guard was deleted by three later paste refactors (#2649, #2738, #3024) and had no regression test. - Clear only the webhook identity (webhookId, triggerPath) on clones, so deploy falls back to the clone's freshly generated block id for the path. Trigger configuration (triggerConfig, triggerId) is preserved — it can be a legacy trigger's only config home. - Clear both the subBlocks structure and the sub-block value map: the value map is authoritative in mergeSubblockStateWithValues. - Gate on isBlockActingAsTrigger, not the subblock id: Discord, Attio, and Vercel action blocks expose a required user-entered webhookId field. Mirrors deploy's resolveTriggerId so both sides agree on which blocks own a webhook. - Cover duplicateBlock, which bypasses regenerateBlockIds entirely. - Reject two same-workflow trigger blocks sharing a path at deploy with a 400 naming both blocks; the existing guards are cross-workflow only. Import/export and server-side duplicate paths are deliberately untouched.
1 parent edef789 commit 8cd0127

8 files changed

Lines changed: 369 additions & 2 deletions

File tree

apps/sim/lib/webhooks/deploy.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import {
4949
buildProviderConfig,
5050
resolveTriggerCredentialId,
5151
resolveWebhookConfigForBlock,
52+
validateTriggerWebhookConfigForDeploy,
5253
} from '@/lib/webhooks/deploy'
5354
import { getBlock } from '@/blocks'
5455
import { getTrigger } from '@/triggers'
@@ -359,3 +360,73 @@ describe('resolveWebhookConfigForBlock — slack_oauth routing', () => {
359360
expect(mockFetchSlackTeamId).not.toHaveBeenCalled()
360361
})
361362
})
363+
364+
describe('validateTriggerWebhookConfigForDeploy — duplicate path guard', () => {
365+
const pathTrigger = trigger([{ id: 'eventType', mode: 'trigger', required: false }])
366+
367+
function triggerBlockWithPath(id: string, name: string, triggerPath: string): BlockState {
368+
return {
369+
id,
370+
name,
371+
type: 'generic_webhook',
372+
enabled: true,
373+
triggerMode: true,
374+
subBlocks: { triggerPath: { value: triggerPath } },
375+
} as unknown as BlockState
376+
}
377+
378+
beforeEach(() => {
379+
;(getBlock as Mock).mockReturnValue({ category: 'triggers' })
380+
;(getTrigger as Mock).mockReturnValue(pathTrigger)
381+
})
382+
383+
/**
384+
* The cross-workflow guards do not cover two blocks in ONE workflow sharing a path:
385+
* findConflictingWebhookPathOwner ignores same-workflow owners and claimWebhookPath's CAS
386+
* accepts a same-workflow re-claim. Without this guard it surfaces as an opaque 500 from the
387+
* `path_deployment_unique` index.
388+
*/
389+
it('rejects two trigger blocks in one workflow sharing a triggerPath', async () => {
390+
const result = await validateTriggerWebhookConfigForDeploy({
391+
'block-a': triggerBlockWithPath('block-a', 'Webhook 1', 'shared-path'),
392+
'block-b': triggerBlockWithPath('block-b', 'Webhook 2', 'shared-path'),
393+
})
394+
395+
expect(result.success).toBe(false)
396+
if (result.success) return
397+
expect(result.error.status).toBe(400)
398+
expect(result.error.message).toContain('shared-path')
399+
expect(result.error.message).toContain('Webhook 1')
400+
expect(result.error.message).toContain('Webhook 2')
401+
})
402+
403+
it('allows distinct paths', async () => {
404+
const result = await validateTriggerWebhookConfigForDeploy({
405+
'block-a': triggerBlockWithPath('block-a', 'Webhook 1', 'path-a'),
406+
'block-b': triggerBlockWithPath('block-b', 'Webhook 2', 'path-b'),
407+
})
408+
409+
expect(result.success).toBe(true)
410+
})
411+
412+
it('allows multiple blocks with no explicit path (each falls back to its block id)', async () => {
413+
const result = await validateTriggerWebhookConfigForDeploy({
414+
'block-a': triggerBlockWithPath('block-a', 'Webhook 1', ''),
415+
'block-b': triggerBlockWithPath('block-b', 'Webhook 2', ''),
416+
})
417+
418+
expect(result.success).toBe(true)
419+
})
420+
421+
it('ignores a disabled block that shares a path', async () => {
422+
const disabled = triggerBlockWithPath('block-b', 'Webhook 2', 'shared-path')
423+
;(disabled as { enabled: boolean }).enabled = false
424+
425+
const result = await validateTriggerWebhookConfigForDeploy({
426+
'block-a': triggerBlockWithPath('block-a', 'Webhook 1', 'shared-path'),
427+
'block-b': disabled,
428+
})
429+
430+
expect(result.success).toBe(true)
431+
})
432+
})

apps/sim/lib/webhooks/deploy.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,31 @@ export async function validateTriggerWebhookConfigForDeploy(
7373
blocks: Record<string, BlockState>
7474
): Promise<TriggerSaveResult> {
7575
const triggerBlocks = Object.values(blocks || {}).filter((b) => b && b.enabled !== false)
76+
const blockNameByPath = new Map<string, string>()
7677

7778
for (const block of triggerBlocks) {
7879
const triggerId = resolveTriggerId(block)
7980
if (!triggerId || !isTriggerValid(triggerId)) continue
8081

82+
// Two blocks in ONE workflow claiming the same path is unreachable through the UI, but the
83+
// cross-workflow guards do not cover it (findConflictingWebhookPathOwner ignores same-workflow
84+
// owners; claimWebhookPath's CAS accepts a same-workflow re-claim). Without this it lands on
85+
// the `path_deployment_unique` index as an opaque 500 instead of an actionable message.
86+
const pathValue = getSubBlockValue(block, 'triggerPath')
87+
if (typeof pathValue === 'string' && pathValue.length > 0) {
88+
const owner = blockNameByPath.get(pathValue)
89+
if (owner) {
90+
return {
91+
success: false,
92+
error: {
93+
message: `Webhook path "${pathValue}" is used by more than one trigger block ("${owner}" and "${block.name}"). Each trigger needs its own path.`,
94+
status: 400,
95+
},
96+
}
97+
}
98+
blockNameByPath.set(pathValue, block.name)
99+
}
100+
81101
const triggerDef = getTrigger(triggerId)
82102
const { providerConfig, missingFields } = buildProviderConfig(block, triggerId, triggerDef)
83103

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,22 @@ export function getAllTriggerBlocks(): TriggerInfo[] {
199199
})
200200
}
201201

202+
/**
203+
* Whether a block instance is actually acting as a trigger, so its
204+
* {@link TRIGGER_RUNTIME_SUBBLOCK_IDS} values are webhook runtime metadata rather than user
205+
* configuration.
206+
*
207+
* Deliberately narrower than {@link hasTriggerCapability}: several `category: 'tools'` blocks
208+
* declare `triggers.enabled` AND expose a required user-entered `webhookId` action field
209+
* (Discord, Attio, Vercel). Gating on capability would treat that field as runtime metadata and
210+
* silently clear it. Mirrors the deploy-side `resolveTriggerId` gate so both sides agree on which
211+
* blocks own a webhook.
212+
*/
213+
export function isBlockActingAsTrigger(block: BlockState): boolean {
214+
if (block.triggerMode === true) return true
215+
return getBlock(block.type)?.category === 'triggers'
216+
}
217+
202218
/**
203219
* Check if a block has trigger capability (contains trigger mode subblocks)
204220
*/

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

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -857,3 +857,135 @@ describe('regenerateBlockIds', () => {
857857
expect(names).toContain('Agent 2')
858858
})
859859
})
860+
861+
describe('regenerateBlockIds — trigger webhook identity', () => {
862+
const positionOffset = { x: 50, y: 50 }
863+
const sourceId = 'webhook-source'
864+
const deployedPath = 'abc123deployedpath'
865+
866+
/**
867+
* Regression: PR #1784 stripped `webhookId`/`triggerPath` on duplicate, but three later
868+
* paste refactors deleted the guard. A clone that keeps `triggerPath` renders the source's
869+
* webhook URL and collides with it on `path_deployment_unique` at deploy.
870+
*/
871+
it('clears webhook runtime values on a pasted trigger block', () => {
872+
const blocksToCopy = {
873+
[sourceId]: createBlock({
874+
id: sourceId,
875+
type: 'generic_webhook',
876+
name: 'Webhook 1',
877+
triggerMode: true,
878+
subBlocks: {
879+
triggerPath: { id: 'triggerPath', type: 'short-input', value: deployedPath },
880+
webhookId: { id: 'webhookId', type: 'short-input', value: 'wh_original' },
881+
},
882+
}),
883+
}
884+
885+
const result = regenerateBlockIds(
886+
blocksToCopy,
887+
[],
888+
{},
889+
{},
890+
{ [sourceId]: { triggerPath: deployedPath, webhookId: 'wh_original' } },
891+
positionOffset,
892+
{},
893+
getUniqueBlockName
894+
)
895+
896+
const newId = Object.keys(result.blocks)[0]
897+
expect(newId).not.toBe(sourceId)
898+
899+
// Both sources must be cleared: the value map wins in mergeSubblockState.
900+
expect(result.blocks[newId].subBlocks.triggerPath?.value).toBeNull()
901+
expect(result.blocks[newId].subBlocks.webhookId?.value).toBeNull()
902+
expect(result.subBlockValues[newId].triggerPath).toBeNull()
903+
expect(result.subBlockValues[newId].webhookId).toBeNull()
904+
})
905+
906+
/**
907+
* Discord, Attio, and Vercel action blocks expose a REQUIRED user-entered `webhookId`
908+
* subblock. Clearing by subblock id alone would silently wipe it, so the clear is gated on
909+
* the block actually acting as a trigger.
910+
*/
911+
it('preserves the user-entered webhookId on a non-trigger action block', () => {
912+
const discordId = 'discord-1'
913+
const blocksToCopy = {
914+
[discordId]: createBlock({
915+
id: discordId,
916+
type: 'discord',
917+
name: 'Discord 1',
918+
triggerMode: false,
919+
subBlocks: {
920+
webhookId: { id: 'webhookId', type: 'short-input', value: '1234567890' },
921+
},
922+
}),
923+
}
924+
925+
const result = regenerateBlockIds(
926+
blocksToCopy,
927+
[],
928+
{},
929+
{},
930+
{ [discordId]: { webhookId: '1234567890' } },
931+
positionOffset,
932+
{},
933+
getUniqueBlockName
934+
)
935+
936+
const newId = Object.keys(result.blocks)[0]
937+
expect(result.blocks[newId].subBlocks.webhookId?.value).toBe('1234567890')
938+
expect(result.subBlockValues[newId].webhookId).toBe('1234567890')
939+
})
940+
941+
/**
942+
* Only the webhook IDENTITY is cleared. Trigger configuration — including `triggerConfig`,
943+
* which can be the only home for a legacy trigger's fields — must survive the copy, or pasting
944+
* a configured trigger would silently lose its setup.
945+
*/
946+
it('preserves trigger configuration and ordinary values on a cloned trigger block', () => {
947+
const blocksToCopy = {
948+
[sourceId]: createBlock({
949+
id: sourceId,
950+
type: 'generic_webhook',
951+
name: 'Webhook 1',
952+
triggerMode: true,
953+
subBlocks: {
954+
triggerPath: { id: 'triggerPath', type: 'short-input', value: deployedPath },
955+
triggerConfig: { id: 'triggerConfig', type: 'short-input', value: { labelIds: ['x'] } },
956+
triggerId: { id: 'triggerId', type: 'short-input', value: 'generic_webhook' },
957+
token: { id: 'token', type: 'short-input', value: 'user-secret' },
958+
requireAuth: { id: 'requireAuth', type: 'switch', value: true },
959+
},
960+
}),
961+
}
962+
963+
const result = regenerateBlockIds(
964+
blocksToCopy,
965+
[],
966+
{},
967+
{},
968+
{
969+
[sourceId]: {
970+
triggerPath: deployedPath,
971+
triggerConfig: { labelIds: ['x'] },
972+
triggerId: 'generic_webhook',
973+
token: 'user-secret',
974+
requireAuth: true,
975+
},
976+
},
977+
positionOffset,
978+
{},
979+
getUniqueBlockName
980+
)
981+
982+
const newId = Object.keys(result.blocks)[0]
983+
expect(result.subBlockValues[newId].triggerPath).toBeNull()
984+
expect(result.subBlockValues[newId].triggerConfig).toEqual({ labelIds: ['x'] })
985+
expect(result.subBlockValues[newId].triggerId).toBe('generic_webhook')
986+
expect(result.subBlockValues[newId].token).toBe('user-secret')
987+
expect(result.subBlockValues[newId].requireAuth).toBe(true)
988+
expect(result.blocks[newId].subBlocks.triggerConfig?.value).toEqual({ labelIds: ['x'] })
989+
expect(result.blocks[newId].subBlocks.token?.value).toBe('user-secret')
990+
})
991+
})

apps/sim/stores/workflows/utils.ts

Lines changed: 45 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+
hasTriggerCapability,
13+
isBlockActingAsTrigger,
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,36 @@ 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
262+
* `null` there overrides the structure — but only creates an entry for a structure-less key when
263+
* the value is non-null. So nulling the map covers the common case (`triggerPath` is written by
264+
* `useWebhookManagement` into the store only, never declared as a subblock), and clearing the
265+
* structure covers blocks whose subBlocks were hydrated from a merge. Either way the clone
266+
* resolves no path and deploy falls back to its freshly generated block id.
267+
*
268+
* Gated on {@link isBlockActingAsTrigger}, NOT on the subblock id alone: Discord, Attio, and
269+
* Vercel action blocks expose a required user-entered `webhookId` field that must survive a copy.
270+
*
271+
* Mutates both arguments in place; both are expected to be clone-owned copies.
272+
*/
273+
export function stripWebhookIdentityForClone(
274+
block: BlockState,
275+
subBlocks: Record<string, SubBlockState>,
276+
subBlockValues: Record<string, unknown>
277+
): void {
278+
if (!isBlockActingAsTrigger(block)) return
279+
280+
for (const subBlockId of WEBHOOK_IDENTITY_SUBBLOCK_IDS) {
281+
const subBlock = subBlocks[subBlockId]
282+
if (subBlock) subBlocks[subBlockId] = { ...subBlock, value: null }
283+
if (subBlockId in subBlockValues) subBlockValues[subBlockId] = null
284+
}
285+
}
286+
254287
function updateBlockReferences(
255288
blocks: Record<string, BlockState>,
256289
nameMap: Map<string, string>,
@@ -565,6 +598,16 @@ export function regenerateBlockIds(
565598
})
566599
})
567600

601+
// Clones must not inherit the source's webhook identity: a pasted trigger block that keeps
602+
// `triggerPath` renders the original's webhook URL and collides with it on deploy
603+
// (`path_deployment_unique`). Deliberately a separate gated pass rather than
604+
// `updateBlockReferences`' id-based `clearTriggerRuntimeValues` flag, so the import/export
605+
// path that uses that flag keeps its exact current behavior.
606+
Object.entries(newBlocks).forEach(([blockId, block]) => {
607+
// An absent value-map entry needs no clearing: deploy treats absent and null identically.
608+
stripWebhookIdentityForClone(block, block.subBlocks, newSubBlockValues[blockId] ?? {})
609+
})
610+
568611
return {
569612
blocks: newBlocks,
570613
edges: newEdges,

0 commit comments

Comments
 (0)