Skip to content

fix(triggers): stop workflow copies reusing or wiping a block's webhook identity - #6090

Closed
waleedlatif1 wants to merge 2 commits into
stagingfrom
worktree-webhook-copy-paste-url
Closed

fix(triggers): stop workflow copies reusing or wiping a block's webhook identity#6090
waleedlatif1 wants to merge 2 commits into
stagingfrom
worktree-webhook-copy-paste-url

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

Two related bugs, same root cause: clearing trigger runtime subblocks by id alone, without checking whether the block actually owns a webhook.

1. Clone paths didn't clear the webhook identity (the reported bug)

  • Copy/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 — hitting the path_deployment_unique index as an opaque 500 instead of an actionable error. Blocks templating any workflow that needs more than one webhook.
  • PR fix(trigger-dup): on duplicate trigger should not point at old webhook row #1784 fixed this for duplicate in Oct 2025, but the guard was deleted by three later paste refactors (feat(copy-paste): allow cross workflow selection, paste, move for blocks #2649, improvement(canvas): add multi-block select, add batch handle, enabled, and edge operations #2738, fix(workflow): use panel-aware viewport center for paste and block placement #3024) and had no regression test. This adds one.
  • Clear only the webhook identity (webhookId, triggerPath) on clones, so deploy falls back to the clone's freshly generated block id. Trigger configuration (triggerConfig, triggerId) is preserved — triggerConfig can be a legacy trigger's only config home, so clearing it would silently drop setup.
  • Clear both the subBlocks structure and the sub-block value map — the value map is authoritative in mergeSubblockStateWithValues, so clearing only the structure leaves the source's path in effect.
  • Cover duplicateBlock (action-bar duplicate), 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: findConflictingWebhookPathOwner ignores same-workflow owners and claimWebhookPath's CAS accepts a same-workflow re-claim.

2. Workflow-level copy paths over-cleared (found while auditing #1)

  • Discord, Attio, and Vercel action blocks expose a required user-entered webhookId field. 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 strip on isBlockActingAsTrigger in sanitizeSubBlocksForDuplicate (server duplicate + workspace forking) and updateBlockReferences (export/import).
  • sanitizeSubBlocksForDuplicate's flag is a required parameter so every call site must decide rather than inheriting a silent default.

Why isBlockActingAsTrigger and not hasTriggerCapability

Vercel and Attio are category: 'tools' with triggers.enabled, so capability returns true and would defeat the guard. The new predicate is triggerMode === true || category === 'triggers' — exactly deploy's own resolveTriggerId gate, so client and server agree on which blocks own a webhook (including for unregistered block types, where both decline and no webhook is created either way).

Audited all 10 stripped ids for collisions with action-block subblock ids: webhookId is the only real one. scheduleInfo also collides but belongs to the schedule block, which is category: 'triggers'. Display-only system subblocks stay stripped unconditionally — those ids exist only on trigger definitions.

Not changed

Trigger blocks behave identically everywhere — they still lose their webhook identity on every copy path. The copilot path is untouched: edit_workflow already drops runtime subblock ids in both validation.ts and operations.ts and merges rather than replaces, so it cannot change a webhook URL. triggerCredentials remains deliberately preserved so a copy keeps its OAuth connection.

Note: existing already-pasted workflows don't self-heal — they now fail with an actionable 400 instead of a 500, and the duplicate trigger blocks need to be re-added once.

Related pre-existing issue left for a separate PR: the generic webhook's auth token (triggers/generic/webhook.ts:33) is a create-time-only generator, so clones share the source's shared secret.

Type of Change

  • Bug fix

Testing

  • 8870 tests pass across stores + lib + ee + triggers (633 files); tsc --noEmit clean.
  • All gates pass: check:api-validation, check:boundaries, check:realtime-prune, check:client-boundary, check:utils. The boundary + prune checks specifically confirm importing @/blocks into the persistence layer doesn't leak into apps/realtime.
  • Verified the new tests are non-vacuous — disabled each fix in turn and confirmed exactly the intended tests fail (3 for the clone fix, 3 for the gate fix) while all others still pass.
  • 16 new tests: paste/duplicate clear identity, paste/duplicate preserve Discord's webhookId, paste preserves triggerConfig+triggerId+token, 4 deploy-guard cases, 4 sanitizeSubBlocksForDuplicate cases (non-trigger preserved, prefixed keys preserved, trigger still stripped, display-only always stripped), 3 regenerateWorkflowIds cases.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…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.
@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 30, 2026 4:56am

Request Review

@cursor

cursor Bot commented Jul 30, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes copy/paste/duplicate/fork/import and deploy validation for webhook triggers; regression tests cover the main paths, but incorrect gating could still wipe user webhook ids or leave clones with colliding paths.

Overview
Fixes two bugs from stripping trigger subblocks by id alone without checking whether the block actually owns a webhook.

Cloned trigger blocks (paste, duplicate, duplicateBlock) no longer inherit the source’s webhookId and triggerPath. A new stripWebhookIdentityForClone clears only WEBHOOK_IDENTITY_SUBBLOCK_IDS in both subblock structure and the value map (so deploy doesn’t reuse the source URL or hit path_deployment_unique). Trigger setup such as triggerConfig, triggerId, and auth token is kept.

Workflow-level copies (duplicate, fork, import/export) now gate runtime stripping on isBlockActingAsTrigger (triggerMode or category === 'triggers'), so action blocks like Discord/Attio/Vercel keep their required user webhookId instead of half-configured copies.

Deploy validation rejects two enabled triggers in one workflow sharing the same explicit triggerPath with a 400 naming both blocks, instead of an opaque 500 from the unique index.

Reviewed by Cursor Bugbot for commit 791cede. Configure here.

Comment thread apps/sim/lib/webhooks/deploy.ts
@greptile-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes webhook identity handling on workflow copy/duplicate/import so trigger clones drop webhookId/triggerPath while non-trigger action blocks keep user-entered webhookId, and adds a same-workflow duplicate triggerPath deploy guard.

  • Client paste/duplicate and action-bar duplicate strip webhook identity via stripWebhookIdentityForClone gated on isBlockActingAsTrigger.
  • Server duplicate, workspace fork, and import/export gate TRIGGER_RUNTIME_SUBBLOCK_IDS stripping the same way; sanitizeSubBlocksForDuplicate requires an explicit isActingAsTrigger flag.
  • Deploy validation rejects two enabled triggers in one workflow sharing the same raw triggerPath with a 400 naming both blocks.
  • Tests cover clone identity clear, Discord webhookId preservation, sanitize behavior, and deploy duplicate-path cases.

Confidence Score: 3/5

Not fully safe to merge until the same-workflow deploy path guard normalizes triggerPath the same way claimWebhookPath does, so slash/whitespace variants get the actionable 400 instead of a later collision.

The prior deploy-guard finding remains: duplicate paths that differ only by leading/trailing slashes or whitespace still bypass validateTriggerWebhookConfigForDeploy's map and can hit the unique index path without naming both blocks. Clone and non-trigger webhookId preservation changes look consistent with the intended gates and tests.

Files Needing Attention: apps/sim/lib/webhooks/deploy.ts

Important Files Changed

Filename Overview
apps/sim/lib/webhooks/deploy.ts Adds same-workflow duplicate triggerPath guard still keyed on raw path strings without claim-path normalization.
apps/sim/stores/workflows/utils.ts Adds stripWebhookIdentityForClone and gates import/export runtime clear on isBlockActingAsTrigger.
apps/sim/lib/workflows/persistence/remap-internal-ids.ts Requires isActingAsTrigger before stripping trigger runtime subblock ids on duplicate.
apps/sim/lib/workflows/triggers/trigger-utils.ts Adds isBlockActingAsTrigger aligned with deploy resolveTriggerId (triggerMode or category triggers).
apps/sim/stores/workflows/workflow/store.ts duplicateBlock now strips webhook identity on the clone without touching the source.
apps/sim/lib/workflows/persistence/duplicate.ts Passes isBlockActingAsTrigger into sanitizeSubBlocksForDuplicate during workflow duplicate.
apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts Fork/copy path passes isBlockActingAsTrigger into sanitize so action webhookId is kept.

Reviews (2): Last reviewed commit: "fix(triggers): stop workflow copy paths ..." | Re-trigger Greptile

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'.
@waleedlatif1 waleedlatif1 changed the title fix(triggers): stop copy/paste and duplicate reusing a trigger block's webhook URL fix(triggers): stop workflow copies reusing or wiping a block's webhook identity Jul 30, 2026
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 791cede. Configure here.

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

Closing in favor of a clean, minimal reimplementation.

The diagnosis here was right and is now pinned down precisely: before deploy the webhook URL is derived (use-webhook-management.ts:103-109 falls back to blockId, which paste already regenerates), and after deploy it is stored — deploy writes path = triggerPath || block.id and the effect at :151 writes that literal string back into the source block's triggerPath. Paste then copies it verbatim, so the clone renders the source's URL. Deploy is the only thing that populates the field, which is why an undeployed paste rotates correctly.

But this PR accumulated scope that does not belong with that one-line-shaped fix, and an independent audit found the tests were passing for the wrong reason — every "clears" assertion rode the triggerMode: true branch, while a real generic_webhook is category: 'triggers' with trigger_mode defaulting to false. The category branch could be replaced with return false and 251 tests still passed, i.e. the fix was deletable without turning the suite red.

Also dropped as unjustified by the data: the same-workflow deploy guard (production has 0 live duplicate webhook paths, and it introduced a false positive for two slack_oauth blocks, whose effectivePath is always null and which therefore can never collide), and the server/import over-clear gate (real, but 4 Discord blocks production-wide, and it made the unknown-block-type case fail open).

Reimplementing off origin/staging as a single minimal change: clear webhookId/triggerPath on in-canvas clones only, gated so Discord/Attio/Vercel's required user-entered webhookId survives, with tests that exercise the production-dominant category branch and are verified non-vacuous by mutation. No deploy-side and no server-side behavior change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant