Skip to content

fix(webhooks): centralize the path-delivery rule for trigger providers - #6428

Merged
waleedlatif1 merged 1 commit into
stagingfrom
worktree-polling-trigger-forgery
Aug 8, 2026
Merged

fix(webhooks): centralize the path-delivery rule for trigger providers#6428
waleedlatif1 merged 1 commit into
stagingfrom
worktree-polling-trigger-forgery

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Fold the internal- and polling-provider exclusions into acceptsPathWebhookDelivery, so one predicate decides which providers the generic per-webhook path route serves instead of the route body re-deriving it
  • Widen isPollingWebhookProvider to accept a nullable provider, matching isInternalTriggerProvider, and drop the resulting ?? '' at both call sites
  • Source the route's rejection coverage from the trigger registries so a newly added internal or polling trigger is covered automatically

Type of Change

  • Bug fix

Testing

bun run type-check, bun run lint, check:api-validation:strict, check:boundaries, and check:trigger-block-cycle pass. Webhook, trigger, and workspace-forking suites pass (923 tests); verified the added coverage fails when the rule is removed.

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)

Fold the internal- and polling-provider exclusions into
acceptsPathWebhookDelivery so the generic per-webhook path route has one
predicate deciding which providers it serves, instead of the route body
re-deriving it. Widen isPollingWebhookProvider to accept a nullable
provider, matching isInternalTriggerProvider, and drop the resulting
`?? ''` at both call sites.

Regression coverage is sourced from the trigger registries so a newly
added internal or polling trigger is covered automatically.
@vercel

vercel Bot commented Aug 8, 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 Aug 8, 2026 7:21pm

Request Review

@cursor

cursor Bot commented Aug 8, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches webhook ingress and trigger forgery boundaries; the change tightens consistency rather than widening exposure, with tests guarding the rule.

Overview
Centralizes which trigger providers may receive HTTP on the generic per-webhook path route into acceptsPathWebhookDelivery, instead of the trigger route filtering internal and polling providers separately.

That helper now returns false for internal triggers (sim, table) and polling providers (those served via /api/webhooks/poll/[provider]), in addition to providers with app-level ingress. The trigger route only applies this single predicate when matching webhooks.

isPollingWebhookProvider now accepts nullable providers (aligned with isInternalTriggerProvider), so fork deploy-bridge no longer uses ?? ''. Route tests iterate INTERNAL_TRIGGER_PROVIDERS and POLLING_PROVIDERS so new registry entries get rejection coverage automatically, and assert dispatch never runs on rejected paths.

Reviewed by Cursor Bugbot for commit b476db4. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR centralizes generic webhook-path eligibility so internal, polling, and provider-owned ingress routes are rejected by one predicate. It also makes polling-provider detection nullable and expands registry-driven route coverage.

  • Moves internal and polling exclusions into acceptsPathWebhookDelivery.
  • Simplifies the generic webhook route to rely on the centralized predicate.
  • Preserves nullable-provider behavior in workspace-fork deployment mapping.
  • Tests every registered internal and polling provider against generic path delivery.

Confidence Score: 5/5

The PR appears safe to merge, with no actionable regressions identified in the centralized provider-delivery classification or nullable helper change.

Current internal and polling providers do not require generic path delivery, registry-based tests enforce that classification, and nullable workspace-fork filtering remains behaviorally unchanged.

Important Files Changed

Filename Overview
apps/sim/lib/webhooks/providers/index.ts Centralizes path-delivery eligibility by excluding internal, polling, and provider-ingress-only providers without introducing an import cycle.
apps/sim/app/api/webhooks/trigger/[path]/route.ts Replaces duplicated route-level exclusions with the centralized provider predicate while preserving rejection behavior.
apps/sim/app/api/webhooks/trigger/[path]/route.test.ts Derives rejection cases from both provider registries and verifies neither queue nor resolved-target dispatch occurs.
apps/sim/triggers/constants.ts Widens polling-provider detection to nullable input while retaining false for null and unchanged behavior for strings.
apps/sim/ee/workspace-forking/lib/copy/deploy-bridge.ts Removes redundant null coercion now that the polling-provider helper directly accepts nullable database values.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Inbound generic webhook path] --> B[Resolve webhook rows by path]
  B --> C{acceptsPathWebhookDelivery}
  C -->|Internal provider| D[Reject with 404]
  C -->|Polling provider| D
  C -->|Provider-owned ingress| D
  C -->|Generic path provider| E[Verify provider authentication]
  E --> F[Dispatch resolved webhook target]
Loading

Reviews (1): Last reviewed commit: "fix(webhooks): centralize the path-deliv..." | Re-trigger Greptile

@waleedlatif1
waleedlatif1 merged commit 1d67e01 into staging Aug 8, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the worktree-polling-trigger-forgery branch August 8, 2026 19:25
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