mirror of
https://github.com/simstudioai/sim.git
synced 2026-10-16 06:39:56 +08:00
fix(webhooks): centralize the path-delivery rule for trigger providers (#6428)
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.
This commit is contained in:
@@ -20,6 +20,7 @@ import {
|
||||
ADMISSION_ERROR_DESCRIPTOR,
|
||||
ADMISSION_RETRY_AFTER_SECONDS,
|
||||
} from '@/lib/core/admission/transient-failure'
|
||||
import { INTERNAL_TRIGGER_PROVIDERS, POLLING_PROVIDERS } from '@/triggers/constants'
|
||||
|
||||
vi.mock('@/lib/core/security/encryption', () => encryptionMock)
|
||||
|
||||
@@ -638,7 +639,8 @@ describe('Webhook Trigger API Route', () => {
|
||||
})
|
||||
|
||||
describe('Non-path trigger providers', () => {
|
||||
it.each(['sim', 'table', 'tiktok'])(
|
||||
/** Sourced from the registries so a newly added trigger is covered automatically. */
|
||||
it.each([...INTERNAL_TRIGGER_PROVIDERS, ...POLLING_PROVIDERS, 'tiktok'])(
|
||||
'rejects HTTP deliveries to %s trigger paths with 404',
|
||||
async (provider) => {
|
||||
testData.webhooks.push({
|
||||
@@ -657,6 +659,7 @@ describe('Webhook Trigger API Route', () => {
|
||||
|
||||
expect(response.status).toBe(404)
|
||||
expect(queueWebhookExecutionMock).not.toHaveBeenCalled()
|
||||
expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled()
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@@ -15,7 +15,6 @@ import {
|
||||
verifyProviderAuth,
|
||||
} from '@/lib/webhooks/processor'
|
||||
import { acceptsPathWebhookDelivery } from '@/lib/webhooks/providers'
|
||||
import { isInternalTriggerProvider } from '@/triggers/constants'
|
||||
|
||||
const logger = createLogger('WebhookTriggerAPI')
|
||||
|
||||
@@ -100,11 +99,8 @@ async function handleWebhookPost(
|
||||
// Find all webhooks for this path (multiple webhooks in one workflow may share a path)
|
||||
const allWebhooksForPath = await findAllWebhooksForPath({ requestId, path })
|
||||
|
||||
/** Exclude in-process triggers and providers that own an app-level ingress route. */
|
||||
const webhooksForPath = allWebhooksForPath.filter(
|
||||
({ webhook: foundWebhook }) =>
|
||||
!isInternalTriggerProvider(foundWebhook.provider) &&
|
||||
acceptsPathWebhookDelivery(foundWebhook.provider)
|
||||
const webhooksForPath = allWebhooksForPath.filter(({ webhook: foundWebhook }) =>
|
||||
acceptsPathWebhookDelivery(foundWebhook.provider)
|
||||
)
|
||||
|
||||
if (allWebhooksForPath.length > 0 && webhooksForPath.length === 0) {
|
||||
|
||||
@@ -296,7 +296,7 @@ export async function loadTargetWebhookPathsByBlock(
|
||||
const byBlock = new Map<string, ForkTargetWebhook>()
|
||||
for (const row of rows) {
|
||||
if (!row.blockId || !row.path) continue
|
||||
if (isPollingWebhookProvider(row.provider ?? '') || isInternalTriggerProvider(row.provider)) {
|
||||
if (isPollingWebhookProvider(row.provider) || isInternalTriggerProvider(row.provider)) {
|
||||
continue
|
||||
}
|
||||
// One live path-based row per block within a version - `path_deployment_unique` enforces it.
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
export { getProviderHandler } from '@/lib/webhooks/providers/registry'
|
||||
|
||||
import { getProviderHandler } from '@/lib/webhooks/providers/registry'
|
||||
import { isInternalTriggerProvider, isPollingWebhookProvider } from '@/triggers/constants'
|
||||
|
||||
/**
|
||||
* Extract a provider-specific unique identifier from the webhook body for idempotency.
|
||||
@@ -14,8 +15,16 @@ export function extractProviderIdentifierFromBody(provider: string, body: unknow
|
||||
return handler.extractIdempotencyId?.(body) ?? null
|
||||
}
|
||||
|
||||
/** Returns whether a provider accepts deliveries through the generic per-webhook path route. */
|
||||
/**
|
||||
* Whether a provider accepts deliveries through the generic per-webhook path route.
|
||||
*
|
||||
* False for triggers Sim fires itself - internal (table row, workspace events) and polling
|
||||
* (pulled from `/api/webhooks/poll/[provider]`) - and for providers that own an app-level
|
||||
* ingress route: their rows still register a path but declare no `verifyAuth`, so anyone
|
||||
* holding the block ID could otherwise forge events.
|
||||
*/
|
||||
export function acceptsPathWebhookDelivery(provider: string | null): boolean {
|
||||
if (!provider) return true
|
||||
if (isInternalTriggerProvider(provider) || isPollingWebhookProvider(provider)) return false
|
||||
return getProviderHandler(provider).ingressMode !== 'provider'
|
||||
}
|
||||
|
||||
@@ -63,8 +63,8 @@ export const POLLING_PROVIDERS = new Set([
|
||||
'rss',
|
||||
])
|
||||
|
||||
export function isPollingWebhookProvider(provider: string): boolean {
|
||||
return POLLING_PROVIDERS.has(provider)
|
||||
export function isPollingWebhookProvider(provider: string | null): boolean {
|
||||
return provider !== null && POLLING_PROVIDERS.has(provider)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user