From 4facfa023b6ef27bd593c75827db7bd2cbda197d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 8 Aug 2026 13:30:50 -0700 Subject: [PATCH 1/2] test(triggers): guard the provider granularity the path route depends on The public trigger route classifies delivery by provider, not by trigger id, so a provider owning both a polling and an HTTP trigger would have its HTTP deliveries rejected. Sim already models dual-delivery services as two providers (slack / slack_app), but nothing asserted it: the existing POLLING_PROVIDERS sync assertions all still pass for a mixed provider. --- apps/sim/triggers/constants.test.ts | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/apps/sim/triggers/constants.test.ts b/apps/sim/triggers/constants.test.ts index 6de36a36a3c..9e2af02f2b5 100644 --- a/apps/sim/triggers/constants.test.ts +++ b/apps/sim/triggers/constants.test.ts @@ -26,6 +26,35 @@ describe('POLLING_PROVIDERS sync with TriggerConfig.polling', () => { expect(missing, `Triggers with polling: true missing from POLLING_PROVIDERS`).toEqual([]) }) + /** + * `acceptsPathWebhookDelivery` gates the public trigger route on the PROVIDER, not the trigger + * id, so a provider owning both families would have its HTTP deliveries rejected as though they + * were forged. Sim already models dual-delivery services as two providers - Slack ships + * `slack` (path) and `slack_app` (shared ingress) rather than one mixed provider. Adding, say, a + * Gmail push trigger under the existing `gmail` provider is the shape this guards against: every + * other assertion here would still pass while real deliveries 404. + */ + it('no provider mixes polling and HTTP triggers', () => { + const byProvider = new Map() + for (const trigger of Object.values(TRIGGER_REGISTRY)) { + const entry = byProvider.get(trigger.provider) ?? { polling: [], http: [] } + entry[trigger.polling === true ? 'polling' : 'http'].push(trigger.id) + byProvider.set(trigger.provider, entry) + } + + const mixed = [...byProvider] + .filter(([, entry]) => entry.polling.length > 0 && entry.http.length > 0) + .map( + ([provider, entry]) => + `${provider}: polling=[${entry.polling.join(', ')}] http=[${entry.http.join(', ')}]` + ) + + expect( + mixed, + 'Split the HTTP triggers onto their own provider - the public trigger route rejects the whole provider' + ).toEqual([]) + }) + it('no POLLING_PROVIDERS entry lacks a polling: true trigger in the registry', () => { const extra: string[] = [] for (const provider of POLLING_PROVIDERS) { From f939e97005f7f5f1dadaafd811fbe9dafce6d89f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 8 Aug 2026 13:38:26 -0700 Subject: [PATCH 2/2] test(triggers): exempt provider-gated triggers from the granularity check Classifying every non-polling trigger as path-delivered was too broad: internal and app-level-ingress providers never serve the public path route either, so mixing polling with those would have failed the invariant despite there being no routing conflict to fix. --- apps/sim/triggers/constants.test.ts | 34 +++++++++++++++++------------ 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/apps/sim/triggers/constants.test.ts b/apps/sim/triggers/constants.test.ts index 9e2af02f2b5..2a5374d69d9 100644 --- a/apps/sim/triggers/constants.test.ts +++ b/apps/sim/triggers/constants.test.ts @@ -2,7 +2,8 @@ * @vitest-environment node */ import { describe, expect, it } from 'vitest' -import { POLLING_PROVIDERS } from '@/triggers/constants' +import { getProviderHandler } from '@/lib/webhooks/providers' +import { isInternalTriggerProvider, POLLING_PROVIDERS } from '@/triggers/constants' import { TRIGGER_REGISTRY } from '@/triggers/registry' describe('POLLING_PROVIDERS sync with TriggerConfig.polling', () => { @@ -27,31 +28,36 @@ describe('POLLING_PROVIDERS sync with TriggerConfig.polling', () => { }) /** - * `acceptsPathWebhookDelivery` gates the public trigger route on the PROVIDER, not the trigger - * id, so a provider owning both families would have its HTTP deliveries rejected as though they - * were forged. Sim already models dual-delivery services as two providers - Slack ships - * `slack` (path) and `slack_app` (shared ingress) rather than one mixed provider. Adding, say, a - * Gmail push trigger under the existing `gmail` provider is the shape this guards against: every - * other assertion here would still pass while real deliveries 404. + * `acceptsPathWebhookDelivery` gates the whole PROVIDER, not the trigger id, so a provider that + * serves the public path route must not also own a polling trigger - membership in + * `POLLING_PROVIDERS` would 404 its real deliveries. Providers gated wholesale for a + * provider-level reason (internal, or an app-level ingress route) never serve that route, so + * mixing is harmless there and they are exempt. Split dual-delivery services into two providers + * instead, as Slack does with `slack` and `slack_app`. */ - it('no provider mixes polling and HTTP triggers', () => { - const byProvider = new Map() + it('no path-delivered provider also owns a polling trigger', () => { + const byProvider = new Map() for (const trigger of Object.values(TRIGGER_REGISTRY)) { - const entry = byProvider.get(trigger.provider) ?? { polling: [], http: [] } - entry[trigger.polling === true ? 'polling' : 'http'].push(trigger.id) + const gatedByProvider = + isInternalTriggerProvider(trigger.provider) || + getProviderHandler(trigger.provider).ingressMode === 'provider' + if (gatedByProvider) continue + + const entry = byProvider.get(trigger.provider) ?? { polling: [], path: [] } + entry[trigger.polling === true ? 'polling' : 'path'].push(trigger.id) byProvider.set(trigger.provider, entry) } const mixed = [...byProvider] - .filter(([, entry]) => entry.polling.length > 0 && entry.http.length > 0) + .filter(([, entry]) => entry.polling.length > 0 && entry.path.length > 0) .map( ([provider, entry]) => - `${provider}: polling=[${entry.polling.join(', ')}] http=[${entry.http.join(', ')}]` + `${provider}: polling=[${entry.polling.join(', ')}] path=[${entry.path.join(', ')}]` ) expect( mixed, - 'Split the HTTP triggers onto their own provider - the public trigger route rejects the whole provider' + 'Split the path-delivered triggers onto their own provider - the public trigger route rejects the whole provider' ).toEqual([]) })