From 8cd01271bc494d983cc1ebd8846e408f885842fd Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 29 Jul 2026 21:44:37 -0700 Subject: [PATCH 1/2] fix(triggers): stop copy/paste and duplicate reusing a trigger block's webhook URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/sim/lib/webhooks/deploy.test.ts | 71 ++++++++++ apps/sim/lib/webhooks/deploy.ts | 20 +++ .../lib/workflows/triggers/trigger-utils.ts | 16 +++ apps/sim/stores/workflows/utils.test.ts | 132 ++++++++++++++++++ apps/sim/stores/workflows/utils.ts | 47 ++++++- .../stores/workflows/workflow/store.test.ts | 64 +++++++++ apps/sim/stores/workflows/workflow/store.ts | 9 ++ apps/sim/triggers/constants.ts | 12 ++ 8 files changed, 369 insertions(+), 2 deletions(-) diff --git a/apps/sim/lib/webhooks/deploy.test.ts b/apps/sim/lib/webhooks/deploy.test.ts index da0f6f4a4f0..16e15c253a8 100644 --- a/apps/sim/lib/webhooks/deploy.test.ts +++ b/apps/sim/lib/webhooks/deploy.test.ts @@ -49,6 +49,7 @@ import { buildProviderConfig, resolveTriggerCredentialId, resolveWebhookConfigForBlock, + validateTriggerWebhookConfigForDeploy, } from '@/lib/webhooks/deploy' import { getBlock } from '@/blocks' import { getTrigger } from '@/triggers' @@ -359,3 +360,73 @@ describe('resolveWebhookConfigForBlock — slack_oauth routing', () => { expect(mockFetchSlackTeamId).not.toHaveBeenCalled() }) }) + +describe('validateTriggerWebhookConfigForDeploy — duplicate path guard', () => { + const pathTrigger = trigger([{ id: 'eventType', mode: 'trigger', required: false }]) + + function triggerBlockWithPath(id: string, name: string, triggerPath: string): BlockState { + return { + id, + name, + type: 'generic_webhook', + enabled: true, + triggerMode: true, + subBlocks: { triggerPath: { value: triggerPath } }, + } as unknown as BlockState + } + + beforeEach(() => { + ;(getBlock as Mock).mockReturnValue({ category: 'triggers' }) + ;(getTrigger as Mock).mockReturnValue(pathTrigger) + }) + + /** + * The cross-workflow guards do not cover two blocks in ONE workflow sharing a path: + * findConflictingWebhookPathOwner ignores same-workflow owners and claimWebhookPath's CAS + * accepts a same-workflow re-claim. Without this guard it surfaces as an opaque 500 from the + * `path_deployment_unique` index. + */ + it('rejects two trigger blocks in one workflow sharing a triggerPath', async () => { + const result = await validateTriggerWebhookConfigForDeploy({ + 'block-a': triggerBlockWithPath('block-a', 'Webhook 1', 'shared-path'), + 'block-b': triggerBlockWithPath('block-b', 'Webhook 2', 'shared-path'), + }) + + expect(result.success).toBe(false) + if (result.success) return + expect(result.error.status).toBe(400) + expect(result.error.message).toContain('shared-path') + expect(result.error.message).toContain('Webhook 1') + expect(result.error.message).toContain('Webhook 2') + }) + + it('allows distinct paths', async () => { + const result = await validateTriggerWebhookConfigForDeploy({ + 'block-a': triggerBlockWithPath('block-a', 'Webhook 1', 'path-a'), + 'block-b': triggerBlockWithPath('block-b', 'Webhook 2', 'path-b'), + }) + + expect(result.success).toBe(true) + }) + + it('allows multiple blocks with no explicit path (each falls back to its block id)', async () => { + const result = await validateTriggerWebhookConfigForDeploy({ + 'block-a': triggerBlockWithPath('block-a', 'Webhook 1', ''), + 'block-b': triggerBlockWithPath('block-b', 'Webhook 2', ''), + }) + + expect(result.success).toBe(true) + }) + + it('ignores a disabled block that shares a path', async () => { + const disabled = triggerBlockWithPath('block-b', 'Webhook 2', 'shared-path') + ;(disabled as { enabled: boolean }).enabled = false + + const result = await validateTriggerWebhookConfigForDeploy({ + 'block-a': triggerBlockWithPath('block-a', 'Webhook 1', 'shared-path'), + 'block-b': disabled, + }) + + expect(result.success).toBe(true) + }) +}) diff --git a/apps/sim/lib/webhooks/deploy.ts b/apps/sim/lib/webhooks/deploy.ts index 7bb32655690..68fb9ac2078 100644 --- a/apps/sim/lib/webhooks/deploy.ts +++ b/apps/sim/lib/webhooks/deploy.ts @@ -73,11 +73,31 @@ export async function validateTriggerWebhookConfigForDeploy( blocks: Record ): Promise { const triggerBlocks = Object.values(blocks || {}).filter((b) => b && b.enabled !== false) + const blockNameByPath = new Map() for (const block of triggerBlocks) { const triggerId = resolveTriggerId(block) if (!triggerId || !isTriggerValid(triggerId)) continue + // Two blocks in ONE workflow claiming the same path is unreachable through the UI, but the + // cross-workflow guards do not cover it (findConflictingWebhookPathOwner ignores same-workflow + // owners; claimWebhookPath's CAS accepts a same-workflow re-claim). Without this it lands on + // the `path_deployment_unique` index as an opaque 500 instead of an actionable message. + const pathValue = getSubBlockValue(block, 'triggerPath') + if (typeof pathValue === 'string' && pathValue.length > 0) { + const owner = blockNameByPath.get(pathValue) + if (owner) { + return { + success: false, + error: { + message: `Webhook path "${pathValue}" is used by more than one trigger block ("${owner}" and "${block.name}"). Each trigger needs its own path.`, + status: 400, + }, + } + } + blockNameByPath.set(pathValue, block.name) + } + const triggerDef = getTrigger(triggerId) const { providerConfig, missingFields } = buildProviderConfig(block, triggerId, triggerDef) diff --git a/apps/sim/lib/workflows/triggers/trigger-utils.ts b/apps/sim/lib/workflows/triggers/trigger-utils.ts index 4f55dd0bbb7..5d56e2390fb 100644 --- a/apps/sim/lib/workflows/triggers/trigger-utils.ts +++ b/apps/sim/lib/workflows/triggers/trigger-utils.ts @@ -199,6 +199,22 @@ export function getAllTriggerBlocks(): TriggerInfo[] { }) } +/** + * Whether a block instance is actually acting as a trigger, so its + * {@link TRIGGER_RUNTIME_SUBBLOCK_IDS} values are webhook runtime metadata rather than user + * configuration. + * + * Deliberately narrower than {@link hasTriggerCapability}: several `category: 'tools'` blocks + * declare `triggers.enabled` AND expose a required user-entered `webhookId` action field + * (Discord, Attio, Vercel). Gating on capability would treat that field as runtime metadata and + * silently clear it. Mirrors the deploy-side `resolveTriggerId` gate so both sides agree on which + * blocks own a webhook. + */ +export function isBlockActingAsTrigger(block: BlockState): boolean { + if (block.triggerMode === true) return true + return getBlock(block.type)?.category === 'triggers' +} + /** * Check if a block has trigger capability (contains trigger mode subblocks) */ diff --git a/apps/sim/stores/workflows/utils.test.ts b/apps/sim/stores/workflows/utils.test.ts index 68966905752..c4aad938bb2 100644 --- a/apps/sim/stores/workflows/utils.test.ts +++ b/apps/sim/stores/workflows/utils.test.ts @@ -857,3 +857,135 @@ describe('regenerateBlockIds', () => { expect(names).toContain('Agent 2') }) }) + +describe('regenerateBlockIds — trigger webhook identity', () => { + const positionOffset = { x: 50, y: 50 } + const sourceId = 'webhook-source' + const deployedPath = 'abc123deployedpath' + + /** + * Regression: PR #1784 stripped `webhookId`/`triggerPath` on duplicate, but three later + * paste refactors deleted the guard. A clone that keeps `triggerPath` renders the source's + * webhook URL and collides with it on `path_deployment_unique` at deploy. + */ + it('clears webhook runtime values on a pasted trigger block', () => { + const blocksToCopy = { + [sourceId]: createBlock({ + id: sourceId, + type: 'generic_webhook', + name: 'Webhook 1', + triggerMode: true, + subBlocks: { + triggerPath: { id: 'triggerPath', type: 'short-input', value: deployedPath }, + webhookId: { id: 'webhookId', type: 'short-input', value: 'wh_original' }, + }, + }), + } + + const result = regenerateBlockIds( + blocksToCopy, + [], + {}, + {}, + { [sourceId]: { triggerPath: deployedPath, webhookId: 'wh_original' } }, + positionOffset, + {}, + getUniqueBlockName + ) + + const newId = Object.keys(result.blocks)[0] + expect(newId).not.toBe(sourceId) + + // Both sources must be cleared: the value map wins in mergeSubblockState. + expect(result.blocks[newId].subBlocks.triggerPath?.value).toBeNull() + expect(result.blocks[newId].subBlocks.webhookId?.value).toBeNull() + expect(result.subBlockValues[newId].triggerPath).toBeNull() + expect(result.subBlockValues[newId].webhookId).toBeNull() + }) + + /** + * Discord, Attio, and Vercel action blocks expose a REQUIRED user-entered `webhookId` + * subblock. Clearing by subblock id alone would silently wipe it, so the clear is gated on + * the block actually acting as a trigger. + */ + it('preserves the user-entered webhookId on a non-trigger action block', () => { + const discordId = 'discord-1' + const blocksToCopy = { + [discordId]: createBlock({ + id: discordId, + type: 'discord', + name: 'Discord 1', + triggerMode: false, + subBlocks: { + webhookId: { id: 'webhookId', type: 'short-input', value: '1234567890' }, + }, + }), + } + + const result = regenerateBlockIds( + blocksToCopy, + [], + {}, + {}, + { [discordId]: { webhookId: '1234567890' } }, + positionOffset, + {}, + getUniqueBlockName + ) + + const newId = Object.keys(result.blocks)[0] + expect(result.blocks[newId].subBlocks.webhookId?.value).toBe('1234567890') + expect(result.subBlockValues[newId].webhookId).toBe('1234567890') + }) + + /** + * Only the webhook IDENTITY is cleared. Trigger configuration — including `triggerConfig`, + * which can be the only home for a legacy trigger's fields — must survive the copy, or pasting + * a configured trigger would silently lose its setup. + */ + it('preserves trigger configuration and ordinary values on a cloned trigger block', () => { + const blocksToCopy = { + [sourceId]: createBlock({ + id: sourceId, + type: 'generic_webhook', + name: 'Webhook 1', + triggerMode: true, + subBlocks: { + triggerPath: { id: 'triggerPath', type: 'short-input', value: deployedPath }, + triggerConfig: { id: 'triggerConfig', type: 'short-input', value: { labelIds: ['x'] } }, + triggerId: { id: 'triggerId', type: 'short-input', value: 'generic_webhook' }, + token: { id: 'token', type: 'short-input', value: 'user-secret' }, + requireAuth: { id: 'requireAuth', type: 'switch', value: true }, + }, + }), + } + + const result = regenerateBlockIds( + blocksToCopy, + [], + {}, + {}, + { + [sourceId]: { + triggerPath: deployedPath, + triggerConfig: { labelIds: ['x'] }, + triggerId: 'generic_webhook', + token: 'user-secret', + requireAuth: true, + }, + }, + positionOffset, + {}, + getUniqueBlockName + ) + + const newId = Object.keys(result.blocks)[0] + expect(result.subBlockValues[newId].triggerPath).toBeNull() + expect(result.subBlockValues[newId].triggerConfig).toEqual({ labelIds: ['x'] }) + expect(result.subBlockValues[newId].triggerId).toBe('generic_webhook') + expect(result.subBlockValues[newId].token).toBe('user-secret') + expect(result.subBlockValues[newId].requireAuth).toBe(true) + expect(result.blocks[newId].subBlocks.triggerConfig?.value).toEqual({ labelIds: ['x'] }) + expect(result.blocks[newId].subBlocks.token?.value).toBe('user-secret') + }) +}) diff --git a/apps/sim/stores/workflows/utils.ts b/apps/sim/stores/workflows/utils.ts index eaa60d714bd..a3709062e7f 100644 --- a/apps/sim/stores/workflows/utils.ts +++ b/apps/sim/stores/workflows/utils.ts @@ -8,7 +8,10 @@ import { remapConditionBlockIds, remapConditionEdgeHandle } from '@/lib/workflow import { isDynamicHandleSubblock } from '@/lib/workflows/dynamic-handle-topology' import { createDefaultInputFormatField } from '@/lib/workflows/input-format' import { buildDefaultCanonicalModes } from '@/lib/workflows/subblocks/visibility' -import { hasTriggerCapability } from '@/lib/workflows/triggers/trigger-utils' +import { + hasTriggerCapability, + isBlockActingAsTrigger, +} from '@/lib/workflows/triggers/trigger-utils' import { getBlock } from '@/blocks' import { escapeRegExp, normalizeName } from '@/executor/constants' import { useWorkflowRegistry } from '@/stores/workflows/registry/store' @@ -22,7 +25,7 @@ import type { SubBlockState, WorkflowState, } from '@/stores/workflows/workflow/types' -import { TRIGGER_RUNTIME_SUBBLOCK_IDS } from '@/triggers/constants' +import { TRIGGER_RUNTIME_SUBBLOCK_IDS, WEBHOOK_IDENTITY_SUBBLOCK_IDS } from '@/triggers/constants' /** Threshold to detect viewport-based offsets vs small duplicate offsets */ const LARGE_OFFSET_THRESHOLD = 300 @@ -251,6 +254,36 @@ function updateValueReferences(value: unknown, nameMap: Map): un return value } +/** + * Clears the {@link WEBHOOK_IDENTITY_SUBBLOCK_IDS} a cloned trigger block must not inherit, across + * BOTH the sub-block structure and the sub-block value map. + * + * Both are required. `mergeSubblockStateWithValues` treats the value map as authoritative — a + * `null` there overrides the structure — but only creates an entry for a structure-less key when + * the value is non-null. So nulling the map covers the common case (`triggerPath` is written by + * `useWebhookManagement` into the store only, never declared as a subblock), and clearing the + * structure covers blocks whose subBlocks were hydrated from a merge. Either way the clone + * resolves no path and deploy falls back to its freshly generated block id. + * + * Gated on {@link isBlockActingAsTrigger}, NOT on the subblock id alone: Discord, Attio, and + * Vercel action blocks expose a required user-entered `webhookId` field that must survive a copy. + * + * Mutates both arguments in place; both are expected to be clone-owned copies. + */ +export function stripWebhookIdentityForClone( + block: BlockState, + subBlocks: Record, + subBlockValues: Record +): void { + if (!isBlockActingAsTrigger(block)) return + + for (const subBlockId of WEBHOOK_IDENTITY_SUBBLOCK_IDS) { + const subBlock = subBlocks[subBlockId] + if (subBlock) subBlocks[subBlockId] = { ...subBlock, value: null } + if (subBlockId in subBlockValues) subBlockValues[subBlockId] = null + } +} + function updateBlockReferences( blocks: Record, nameMap: Map, @@ -565,6 +598,16 @@ export function regenerateBlockIds( }) }) + // Clones must not inherit the source's webhook identity: a pasted trigger block that keeps + // `triggerPath` renders the original's webhook URL and collides with it on deploy + // (`path_deployment_unique`). Deliberately a separate gated pass rather than + // `updateBlockReferences`' id-based `clearTriggerRuntimeValues` flag, so the import/export + // path that uses that flag keeps its exact current behavior. + Object.entries(newBlocks).forEach(([blockId, block]) => { + // An absent value-map entry needs no clearing: deploy treats absent and null identically. + stripWebhookIdentityForClone(block, block.subBlocks, newSubBlockValues[blockId] ?? {}) + }) + return { blocks: newBlocks, edges: newEdges, diff --git a/apps/sim/stores/workflows/workflow/store.test.ts b/apps/sim/stores/workflows/workflow/store.test.ts index f5072971faf..a1a57666526 100644 --- a/apps/sim/stores/workflows/workflow/store.test.ts +++ b/apps/sim/stores/workflows/workflow/store.test.ts @@ -492,6 +492,70 @@ describe('workflow store', () => { expect(blocks[duplicatedId].position.x).not.toBe(0) } }) + + /** + * Regression: a duplicated trigger block that keeps the source's `triggerPath` renders the + * original's webhook URL and collides with it on `path_deployment_unique` at deploy. + */ + it('should clear webhook runtime values when duplicating a trigger block', () => { + const { duplicateBlock } = useWorkflowStore.getState() + useWorkflowRegistry.setState({ activeWorkflowId: 'wf-1' }) + useWorkflowStore.setState({ currentWorkflowId: 'wf-1' }) + + addBlock( + 'original', + 'generic_webhook', + 'Webhook 1', + { x: 0, y: 0 }, + undefined, + undefined, + undefined, + { + triggerMode: true, + } + ) + useSubBlockStore.setState({ + workflowValues: { + 'wf-1': { + original: { triggerPath: 'deployed-path-abc', webhookId: 'wh_original' }, + }, + }, + }) + + duplicateBlock('original') + + const { blocks } = useWorkflowStore.getState() + const duplicatedId = Object.keys(blocks).find((id) => id !== 'original') + expect(duplicatedId).toBeDefined() + if (!duplicatedId) return + + const values = useSubBlockStore.getState().workflowValues['wf-1'] + expect(values[duplicatedId].triggerPath).toBeNull() + expect(values[duplicatedId].webhookId).toBeNull() + // The source keeps its identity. + expect(values.original.triggerPath).toBe('deployed-path-abc') + }) + + it('should preserve a user-entered webhookId when duplicating a non-trigger block', () => { + const { duplicateBlock } = useWorkflowStore.getState() + useWorkflowRegistry.setState({ activeWorkflowId: 'wf-1' }) + useWorkflowStore.setState({ currentWorkflowId: 'wf-1' }) + + addBlock('original', 'discord', 'Discord 1', { x: 0, y: 0 }) + useSubBlockStore.setState({ + workflowValues: { 'wf-1': { original: { webhookId: '1234567890' } } }, + }) + + duplicateBlock('original') + + const { blocks } = useWorkflowStore.getState() + const duplicatedId = Object.keys(blocks).find((id) => id !== 'original') + expect(duplicatedId).toBeDefined() + if (!duplicatedId) return + + const values = useSubBlockStore.getState().workflowValues['wf-1'] + expect(values[duplicatedId].webhookId).toBe('1234567890') + }) }) describe('batchUpdatePositions', () => { diff --git a/apps/sim/stores/workflows/workflow/store.ts b/apps/sim/stores/workflows/workflow/store.ts index 11008b1658f..22d2a8b7d63 100644 --- a/apps/sim/stores/workflows/workflow/store.ts +++ b/apps/sim/stores/workflows/workflow/store.ts @@ -17,6 +17,7 @@ import { getUniqueBlockName, mergeSubblockState, remapConditionIds, + stripWebhookIdentityForClone, } from '@/stores/workflows/utils' import type { Position, @@ -600,6 +601,14 @@ export const useWorkflowStore = create()( newId ) + // A duplicated trigger block must not inherit the source's webhook identity, or it + // renders the original's URL and collides with it on deploy. + stripWebhookIdentityForClone( + block, + newSubBlocks as Record, + clonedSubBlockValues + ) + const newState = { blocks: { ...get().blocks, diff --git a/apps/sim/triggers/constants.ts b/apps/sim/triggers/constants.ts index d7126c12b8e..d4a23972acc 100644 --- a/apps/sim/triggers/constants.ts +++ b/apps/sim/triggers/constants.ts @@ -30,6 +30,18 @@ export const TRIGGER_RUNTIME_SUBBLOCK_IDS: string[] = [ 'triggerId', ] +/** + * The subset of {@link TRIGGER_RUNTIME_SUBBLOCK_IDS} that identifies one specific deployed webhook + * row. A cloned trigger block must not inherit these: keeping `triggerPath` makes the clone render + * the source's public URL and collide with it on the `path_deployment_unique` index at deploy. + * + * Deliberately excludes `triggerConfig` and `triggerId`, which are user configuration a clone + * SHOULD keep. `triggerConfig` in particular can be the only home for a legacy trigger's fields + * (it is migrated into individual subblocks by `populateTriggerFieldsFromConfig`, which only runs + * once a webhook row is loaded) so clearing it could silently drop configuration. + */ +export const WEBHOOK_IDENTITY_SUBBLOCK_IDS: string[] = ['webhookId', 'triggerPath'] + /** * Synthesized read-only field exposing a webhook trigger block's public URL in the * copilot's read view of workflow state (see sanitizeForCopilot). The URL is derived From 791cedec83c85ac3f9186e5b1a894cc8670197a8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 29 Jul 2026 21:55:44 -0700 Subject: [PATCH 2/2] fix(triggers): stop workflow copy paths wiping a user-entered webhookId MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trigger-runtime strip on the workflow-level copy paths was keyed on subblock id alone. Discord, Attio, and Vercel action blocks expose a REQUIRED user-entered `webhookId` field, so duplicating, forking, or importing a workflow blanked it while leaving the `webhookToken` it pairs with — a silently half-configured copy that fails at runtime with an error pointing nowhere near the copy that caused it. - Gate the TRIGGER_RUNTIME_SUBBLOCK_IDS strip on isBlockActingAsTrigger in sanitizeSubBlocksForDuplicate (server duplicate + workspace forking) and in updateBlockReferences (export/import). - Make sanitizeSubBlocksForDuplicate's flag a required parameter so every call site must decide rather than inheriting a silent default. - Widen isBlockActingAsTrigger to accept a DB block row as well as BlockState. Trigger blocks are unaffected — they still lose their webhook identity on a copy. Display-only system subblocks are still stripped unconditionally; those ids exist only on trigger definitions. Verified `scheduleInfo` is the only other collision and it belongs to the schedule block, which is category: 'triggers'. --- .../lib/copy/copy-workflows.ts | 6 +- .../lib/workflows/persistence/duplicate.ts | 6 +- .../persistence/remap-internal-ids.test.ts | 76 +++++++++++++++++++ .../persistence/remap-internal-ids.ts | 20 ++++- .../lib/workflows/triggers/trigger-utils.ts | 5 +- apps/sim/stores/workflows/utils.test.ts | 74 +++++++++++++++++- apps/sim/stores/workflows/utils.ts | 5 +- 7 files changed, 184 insertions(+), 8 deletions(-) diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts index 0e6ac294fb2..1e111c6d6bc 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts @@ -13,6 +13,7 @@ import { } from '@/lib/workflows/persistence/remap-internal-ids' import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils' import type { CanonicalModeOverrides } from '@/lib/workflows/subblocks/visibility' +import { isBlockActingAsTrigger } from '@/lib/workflows/triggers/trigger-utils' import { deriveForkBlockId, type ForkBlockIdResolver, @@ -436,7 +437,10 @@ export async function copyWorkflowStateIntoTarget( // double-cast-allowed: SubBlockState is structurally a SubBlockRecord entry but lacks the open index signature SubBlockRecord declares const sourceSubBlocks = (block.subBlocks ?? {}) as unknown as SubBlockRecord - const sanitizedSource = sanitizeSubBlocksForDuplicate(sourceSubBlocks) + const sanitizedSource = sanitizeSubBlocksForDuplicate( + sourceSubBlocks, + isBlockActingAsTrigger(block) + ) let subBlocks: SubBlockRecord = sanitizedSource // Tracks the block's live `canonicalModes` through this pass, so a `tool-input` reindex // (a dropped custom-tool/MCP entry shifts later tools' array positions) is visible to every diff --git a/apps/sim/lib/workflows/persistence/duplicate.ts b/apps/sim/lib/workflows/persistence/duplicate.ts index 3d4600e9811..e697e96a9a3 100644 --- a/apps/sim/lib/workflows/persistence/duplicate.ts +++ b/apps/sim/lib/workflows/persistence/duplicate.ts @@ -22,6 +22,7 @@ import { type SubBlockRecord, sanitizeSubBlocksForDuplicate, } from '@/lib/workflows/persistence/remap-internal-ids' +import { isBlockActingAsTrigger } from '@/lib/workflows/triggers/trigger-utils' import { deduplicateWorkflowName } from '@/lib/workflows/utils' import type { Variable } from '@/stores/variables/types' import type { LoopConfig, ParallelConfig } from '@/stores/workflows/workflow/types' @@ -304,7 +305,10 @@ export async function duplicateWorkflow( typeof updatedSubBlocks === 'object' && !Array.isArray(updatedSubBlocks) ) { - updatedSubBlocks = sanitizeSubBlocksForDuplicate(updatedSubBlocks as SubBlockRecord) + updatedSubBlocks = sanitizeSubBlocksForDuplicate( + updatedSubBlocks as SubBlockRecord, + isBlockActingAsTrigger(block) + ) } if ( varIdMapping.size > 0 && diff --git a/apps/sim/lib/workflows/persistence/remap-internal-ids.test.ts b/apps/sim/lib/workflows/persistence/remap-internal-ids.test.ts index b0069c858bd..1d8c311df67 100644 --- a/apps/sim/lib/workflows/persistence/remap-internal-ids.test.ts +++ b/apps/sim/lib/workflows/persistence/remap-internal-ids.test.ts @@ -8,6 +8,7 @@ import { remapConditionIdsInSubBlocks, remapWorkflowReferencesInSubBlocks, type SubBlockRecord, + sanitizeSubBlocksForDuplicate, } from '@/lib/workflows/persistence/remap-internal-ids' describe('remapWorkflowReferencesInSubBlocks', () => { @@ -463,3 +464,78 @@ describe('coerceObjectArray', () => { expect(coerceObjectArray(42)).toEqual({ array: null, wasString: false }) }) }) + +describe('sanitizeSubBlocksForDuplicate', () => { + /** + * Regression: the strip was keyed on subblock id alone, so duplicating or forking a workflow + * blanked Discord/Attio/Vercel's REQUIRED user-entered `webhookId` while leaving the + * `webhookToken` it pairs with — a silently half-configured copy. + */ + it('keeps a user-entered webhookId on a block that is not acting as a trigger', () => { + const discord: SubBlockRecord = { + operation: { type: 'dropdown', value: 'discord_execute_webhook' }, + webhookId: { type: 'short-input', value: '1234567890' }, + webhookToken: { type: 'short-input', value: 'tok_abc' }, + content: { type: 'long-input', value: 'hello' }, + } + + const out = sanitizeSubBlocksForDuplicate(discord, false) + + expect(out.webhookId).toEqual({ type: 'short-input', value: '1234567890' }) + expect(out.webhookToken).toEqual({ type: 'short-input', value: 'tok_abc' }) + expect(out.content).toBeDefined() + expect(out.operation).toBeDefined() + }) + + it('keeps prefixed keys on a block that is not acting as a trigger', () => { + const out = sanitizeSubBlocksForDuplicate( + { + webhookId_custom: { type: 'short-input', value: 'x' }, + triggerConfig: { type: 'short-input', value: { labelIds: ['a'] } }, + }, + false + ) + + expect(out.webhookId_custom).toBeDefined() + expect(out.triggerConfig).toBeDefined() + }) + + it('still strips webhook runtime identity on a real trigger block', () => { + const out = sanitizeSubBlocksForDuplicate( + { + webhookId: { type: 'short-input', value: 'wh_original' }, + triggerPath: { type: 'short-input', value: 'deployed-path' }, + triggerConfig: { type: 'short-input', value: { labelIds: ['a'] } }, + triggerId: { type: 'short-input', value: 'generic_webhook' }, + requireAuth: { type: 'switch', value: true }, + }, + true + ) + + expect(out.webhookId).toBeUndefined() + expect(out.triggerPath).toBeUndefined() + expect(out.triggerConfig).toBeUndefined() + expect(out.triggerId).toBeUndefined() + expect(out.requireAuth).toBeDefined() + }) + + it('strips display-only system subblocks regardless of trigger status', () => { + for (const isTrigger of [true, false]) { + const out = sanitizeSubBlocksForDuplicate( + { + webhookUrlDisplay: { type: 'short-input', value: 'https://x' }, + samplePayload: { type: 'code', value: '{}' }, + scheduleInfo: { type: 'schedule-info', value: null }, + triggerCredentials: { type: 'oauth-input', value: 'cred-1' }, + }, + isTrigger + ) + + expect(out.webhookUrlDisplay).toBeUndefined() + expect(out.samplePayload).toBeUndefined() + expect(out.scheduleInfo).toBeUndefined() + // triggerCredentials is deliberately preserved so a copy keeps its OAuth connection. + expect(out.triggerCredentials).toBeDefined() + } + }) +}) diff --git a/apps/sim/lib/workflows/persistence/remap-internal-ids.ts b/apps/sim/lib/workflows/persistence/remap-internal-ids.ts index 09217823190..a4b591de935 100644 --- a/apps/sim/lib/workflows/persistence/remap-internal-ids.ts +++ b/apps/sim/lib/workflows/persistence/remap-internal-ids.ts @@ -47,11 +47,25 @@ export function isSystemSubBlockKey(key: string, ids: Set | string[]): b return idList.some((id) => key === id || key.startsWith(`${id}_`)) } -/** Strip trigger-runtime and non-credential system subblocks for a fresh copy. */ -export function sanitizeSubBlocksForDuplicate(subBlocks: SubBlockRecord): SubBlockRecord { +/** + * Strip trigger-runtime and non-credential system subblocks for a fresh copy. + * + * `isActingAsTrigger` is required, not optional, so every call site must decide: the + * {@link TRIGGER_RUNTIME_SUBBLOCK_IDS} strip is only correct for a block that actually owns a + * webhook. Discord, Attio, and Vercel action blocks expose a REQUIRED user-entered `webhookId` + * field, and stripping it left the copy half-configured (`webhookToken` survives while the id it + * pairs with vanishes). Resolve the flag with `isBlockActingAsTrigger(block)`. + * + * The display-only system subblocks are stripped unconditionally: those ids exist only on trigger + * definitions, so the strip is a no-op for anything else. + */ +export function sanitizeSubBlocksForDuplicate( + subBlocks: SubBlockRecord, + isActingAsTrigger: boolean +): SubBlockRecord { const sanitized: SubBlockRecord = {} for (const [key, subBlock] of Object.entries(subBlocks)) { - if (isSystemSubBlockKey(key, TRIGGER_RUNTIME_SUBBLOCK_IDS)) continue + if (isActingAsTrigger && isSystemSubBlockKey(key, TRIGGER_RUNTIME_SUBBLOCK_IDS)) continue if (isSystemSubBlockKey(key, DUPLICATE_STRIPPED_SYSTEM_SUBBLOCK_IDS)) continue sanitized[key] = subBlock } diff --git a/apps/sim/lib/workflows/triggers/trigger-utils.ts b/apps/sim/lib/workflows/triggers/trigger-utils.ts index 5d56e2390fb..8e51c4adc1e 100644 --- a/apps/sim/lib/workflows/triggers/trigger-utils.ts +++ b/apps/sim/lib/workflows/triggers/trigger-utils.ts @@ -210,7 +210,10 @@ export function getAllTriggerBlocks(): TriggerInfo[] { * silently clear it. Mirrors the deploy-side `resolveTriggerId` gate so both sides agree on which * blocks own a webhook. */ -export function isBlockActingAsTrigger(block: BlockState): boolean { +export function isBlockActingAsTrigger(block: { + type: string + triggerMode?: boolean | null +}): boolean { if (block.triggerMode === true) return true return getBlock(block.type)?.category === 'triggers' } diff --git a/apps/sim/stores/workflows/utils.test.ts b/apps/sim/stores/workflows/utils.test.ts index c4aad938bb2..ce0b800dacb 100644 --- a/apps/sim/stores/workflows/utils.test.ts +++ b/apps/sim/stores/workflows/utils.test.ts @@ -8,7 +8,7 @@ import { import type { Edge } from 'reactflow' import { describe, expect, it } from 'vitest' import { normalizeName } from '@/executor/constants' -import { getUniqueBlockName, regenerateBlockIds } from './utils' +import { getUniqueBlockName, regenerateBlockIds, regenerateWorkflowIds } from './utils' describe('normalizeName', () => { it.concurrent('should convert to lowercase', () => { @@ -989,3 +989,75 @@ describe('regenerateBlockIds — trigger webhook identity', () => { expect(result.blocks[newId].subBlocks.token?.value).toBe('user-secret') }) }) + +describe('regenerateWorkflowIds — trigger webhook identity', () => { + function stateWith(block: Record) { + return { + blocks: { 'block-1': { id: 'block-1', position: { x: 0, y: 0 }, outputs: {}, ...block } }, + edges: [], + loops: {}, + parallels: {}, + } as never + } + + /** + * Regression: import/export cleared runtime ids by subblock id alone, so an imported Discord + * block lost its REQUIRED user-entered `webhookId` while keeping the `webhookToken` it pairs + * with. Trigger blocks (below) must still be cleared. + */ + it('keeps a user-entered webhookId on a non-trigger action block', () => { + const out = regenerateWorkflowIds( + stateWith({ + type: 'discord', + name: 'Discord 1', + triggerMode: false, + subBlocks: { + operation: { id: 'operation', type: 'dropdown', value: 'discord_execute_webhook' }, + webhookId: { id: 'webhookId', type: 'short-input', value: '1234567890' }, + webhookToken: { id: 'webhookToken', type: 'short-input', value: 'tok_abc' }, + }, + }) + ) + + const block = Object.values(out.blocks)[0] + expect(block.subBlocks.webhookId?.value).toBe('1234567890') + expect(block.subBlocks.webhookToken?.value).toBe('tok_abc') + }) + + it('still clears runtime identity on a block in trigger mode', () => { + const out = regenerateWorkflowIds( + stateWith({ + type: 'generic_webhook', + name: 'Webhook 1', + triggerMode: true, + subBlocks: { + webhookId: { id: 'webhookId', type: 'short-input', value: 'wh_original' }, + triggerPath: { id: 'triggerPath', type: 'short-input', value: 'deployed-path' }, + requireAuth: { id: 'requireAuth', type: 'switch', value: true }, + }, + }) + ) + + const block = Object.values(out.blocks)[0] + expect(block.subBlocks.webhookId?.value).toBeNull() + expect(block.subBlocks.triggerPath?.value).toBeNull() + expect(block.subBlocks.requireAuth?.value).toBe(true) + }) + + it('leaves runtime identity alone when clearTriggerRuntimeValues is opted out', () => { + const out = regenerateWorkflowIds( + stateWith({ + type: 'generic_webhook', + name: 'Webhook 1', + triggerMode: true, + subBlocks: { + triggerPath: { id: 'triggerPath', type: 'short-input', value: 'deployed-path' }, + }, + }), + { clearTriggerRuntimeValues: false } + ) + + const block = Object.values(out.blocks)[0] + expect(block.subBlocks.triggerPath?.value).toBe('deployed-path') + }) +}) diff --git a/apps/sim/stores/workflows/utils.ts b/apps/sim/stores/workflows/utils.ts index a3709062e7f..a27b9371cf0 100644 --- a/apps/sim/stores/workflows/utils.ts +++ b/apps/sim/stores/workflows/utils.ts @@ -290,9 +290,12 @@ function updateBlockReferences( clearTriggerRuntimeValues = false ): void { Object.entries(blocks).forEach(([_, block]) => { + // The clear is only correct for a block that actually owns a webhook — Discord, Attio, and + // Vercel action blocks expose a REQUIRED user-entered `webhookId` field that must survive. + const clearRuntimeValues = clearTriggerRuntimeValues && isBlockActingAsTrigger(block) if (block.subBlocks) { Object.entries(block.subBlocks).forEach(([subBlockId, subBlock]) => { - if (clearTriggerRuntimeValues && TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(subBlockId)) { + if (clearRuntimeValues && TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(subBlockId)) { block.subBlocks[subBlockId] = { ...subBlock, value: null } return }