diff --git a/apps/docs/components/icons.tsx b/apps/docs/components/icons.tsx index f21df070063..2417e6acb58 100644 --- a/apps/docs/components/icons.tsx +++ b/apps/docs/components/icons.tsx @@ -780,7 +780,7 @@ export function GitLabIcon(props: SVGProps) { export function SerperIcon(props: SVGProps) { return ( - + ) { export const S3Icon = (props: SVGProps) => ( ) { export function BrainIcon(props: SVGProps) { return ( - - - - - - - - - - + + ) } @@ -2342,8 +2323,8 @@ export function ExtendIcon(props: SVGProps) { export function EvernoteIcon(props: SVGProps) { return ( - - + + ) } @@ -2538,7 +2519,7 @@ export function TelegramIcon(props: SVGProps) { ) { } export function MicrosoftOneDriveIcon(props: SVGProps) { + const id = useId() return ( - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + ) } @@ -5882,12 +6037,22 @@ export function GreenhouseIcon(props: SVGProps) { export function GreptileIcon(props: SVGProps) { return ( - + + + ) diff --git a/apps/sim/app/api/auth/sso/register/route.test.ts b/apps/sim/app/api/auth/sso/register/route.test.ts new file mode 100644 index 00000000000..87f4cdd6744 --- /dev/null +++ b/apps/sim/app/api/auth/sso/register/route.test.ts @@ -0,0 +1,196 @@ +/** + * @vitest-environment node + */ +import { createEnvMock, createMockRequest } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockGetSession, + mockRegisterSSOProvider, + mockHasSSOAccess, + mockValidateUrlWithDNS, + dbState, + memberTable, + ssoProviderTable, +} = vi.hoisted(() => ({ + mockGetSession: vi.fn(), + mockRegisterSSOProvider: vi.fn(), + mockHasSSOAccess: vi.fn(), + mockValidateUrlWithDNS: vi.fn(), + dbState: { members: [] as any[], providers: [] as any[] }, + memberTable: { + userId: 'member.userId', + organizationId: 'member.organizationId', + role: 'member.role', + }, + ssoProviderTable: { + id: 'sso.id', + providerId: 'sso.providerId', + domain: 'sso.domain', + issuer: 'sso.issuer', + userId: 'sso.userId', + organizationId: 'sso.organizationId', + oidcConfig: 'sso.oidcConfig', + samlConfig: 'sso.samlConfig', + }, +})) + +function makeBuilder(rows: any[]): any { + const thenable: any = Promise.resolve(rows) + thenable.where = (condition: any) => { + const values = condition?.values + if (Array.isArray(values) && values.length > 0) { + const target = String(values[values.length - 1]).toLowerCase() + return makeBuilder(rows.filter((r) => String(r.domain ?? '').toLowerCase() === target)) + } + return makeBuilder(rows) + } + thenable.limit = () => Promise.resolve(rows) + thenable.orderBy = () => Promise.resolve(rows) + return thenable +} + +vi.mock('@sim/db', () => ({ + db: { + select: () => ({ + from: (table: unknown) => + makeBuilder(table === memberTable ? dbState.members : dbState.providers), + }), + }, + member: memberTable, + ssoProvider: ssoProviderTable, +})) + +vi.mock('@/lib/auth', () => ({ + getSession: mockGetSession, + auth: { api: { registerSSOProvider: mockRegisterSSOProvider } }, +})) + +vi.mock('@/lib/billing', () => ({ + hasSSOAccess: mockHasSSOAccess, +})) + +vi.mock('@/lib/auth/sso/domain', () => ({ + normalizeSSODomain: (input: unknown): string | null => { + if (typeof input !== 'string') return null + const value = input.trim().toLowerCase() + return /^[a-z0-9-]+(\.[a-z0-9-]+)+$/.test(value) ? value : null + }, +})) + +vi.mock('@/lib/core/security/input-validation.server', () => ({ + validateUrlWithDNS: mockValidateUrlWithDNS, + secureFetchWithPinnedIP: vi.fn(), +})) + +vi.mock('@/lib/core/config/env', () => createEnvMock({ SSO_ENABLED: 'true' })) + +import { POST } from '@/app/api/auth/sso/register/route' + +const OIDC_BODY = { + providerType: 'oidc' as const, + providerId: 'acme-oidc', + issuer: 'https://idp.acme.com', + domain: 'acme.com', + clientId: 'client-id', + clientSecret: 'client-secret', + authorizationEndpoint: 'https://idp.acme.com/authorize', + tokenEndpoint: 'https://idp.acme.com/token', + userInfoEndpoint: 'https://idp.acme.com/userinfo', + jwksEndpoint: 'https://idp.acme.com/jwks', +} + +function request(body: Record) { + return createMockRequest('POST', body) +} + +describe('POST /api/auth/sso/register', () => { + beforeEach(() => { + vi.clearAllMocks() + dbState.members = [] + dbState.providers = [] + mockGetSession.mockResolvedValue({ user: { id: 'u1' } }) + mockHasSSOAccess.mockResolvedValue(true) + mockValidateUrlWithDNS.mockResolvedValue({ isValid: true, resolvedIP: '1.2.3.4' }) + mockRegisterSSOProvider.mockResolvedValue({ providerId: 'acme-oidc' }) + }) + + it('rejects callers without an Enterprise plan', async () => { + mockHasSSOAccess.mockResolvedValue(false) + const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + expect(res.status).toBe(403) + expect(mockRegisterSSOProvider).not.toHaveBeenCalled() + }) + + it('rejects callers who are not an admin/owner of the target org', async () => { + dbState.members = [{ organizationId: 'org1', role: 'member' }] + const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + expect(res.status).toBe(403) + expect(mockRegisterSSOProvider).not.toHaveBeenCalled() + }) + + it('rejects an invalid domain', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + const res = await POST(request({ ...OIDC_BODY, domain: 'not-a-domain', orgId: 'org1' })) + expect(res.status).toBe(400) + expect(mockRegisterSSOProvider).not.toHaveBeenCalled() + }) + + it('rejects a domain already registered by another organization', async () => { + dbState.members = [{ organizationId: 'org-attacker', role: 'owner' }] + dbState.providers = [{ domain: 'acme.com', userId: 'u-victim', organizationId: 'org-victim' }] + const res = await POST(request({ ...OIDC_BODY, orgId: 'org-attacker' })) + const json = await res.json() + expect(res.status).toBe(409) + expect(json.code).toBe('SSO_DOMAIN_ALREADY_REGISTERED') + expect(mockRegisterSSOProvider).not.toHaveBeenCalled() + }) + + it('matches conflicts across casing variants', async () => { + dbState.members = [{ organizationId: 'org-attacker', role: 'owner' }] + dbState.providers = [{ domain: 'ACME.com', userId: 'u-victim', organizationId: 'org-victim' }] + const res = await POST(request({ ...OIDC_BODY, orgId: 'org-attacker' })) + expect(res.status).toBe(409) + expect(mockRegisterSSOProvider).not.toHaveBeenCalled() + }) + + it('registers when the domain is unclaimed', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + expect(res.status).toBe(200) + expect(mockRegisterSSOProvider).toHaveBeenCalledTimes(1) + }) + + it('allows the owning tenant to update its own provider for the same domain', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + dbState.providers = [{ domain: 'acme.com', userId: 'u1', organizationId: 'org1' }] + const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + expect(res.status).toBe(200) + expect(mockRegisterSSOProvider).toHaveBeenCalledTimes(1) + }) + + it('lets an org admin adopt their own user-scoped provider for the same domain', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + dbState.providers = [{ domain: 'acme.com', userId: 'u1', organizationId: null }] + const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + expect(res.status).toBe(200) + expect(mockRegisterSSOProvider).toHaveBeenCalledTimes(1) + }) + + it("still blocks an org admin from claiming another user's user-scoped domain", async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + dbState.providers = [{ domain: 'acme.com', userId: 'someone-else', organizationId: null }] + const res = await POST(request({ ...OIDC_BODY, orgId: 'org1' })) + expect(res.status).toBe(409) + expect(mockRegisterSSOProvider).not.toHaveBeenCalled() + }) + + it('normalizes the domain before persisting it', async () => { + dbState.members = [{ organizationId: 'org1', role: 'owner' }] + const res = await POST(request({ ...OIDC_BODY, domain: 'ACME.com', orgId: 'org1' })) + expect(res.status).toBe(200) + expect(mockRegisterSSOProvider).toHaveBeenCalledTimes(1) + const config = mockRegisterSSOProvider.mock.calls[0][0].body + expect(config.domain).toBe('acme.com') + }) +}) diff --git a/apps/sim/app/api/auth/sso/register/route.ts b/apps/sim/app/api/auth/sso/register/route.ts index 5b285a5bd28..235116fc9e4 100644 --- a/apps/sim/app/api/auth/sso/register/route.ts +++ b/apps/sim/app/api/auth/sso/register/route.ts @@ -1,11 +1,12 @@ import { db, member, ssoProvider } from '@sim/db' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' -import { and, eq } from 'drizzle-orm' +import { and, eq, sql } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { ssoRegistrationContract } from '@/lib/api/contracts/auth' import { getValidationErrorMessage, parseRequest } from '@/lib/api/server' import { auth, getSession } from '@/lib/auth' +import { normalizeSSODomain } from '@/lib/auth/sso/domain' import { hasSSOAccess } from '@/lib/billing' import { env } from '@/lib/core/config/env' import { @@ -51,7 +52,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { if (!parsed.success) return parsed.response const body = parsed.data.body - const { providerId, issuer, domain, providerType, mapping, orgId } = body + const { providerId, issuer, providerType, mapping, orgId } = body if (orgId) { const [membership] = await db @@ -67,6 +68,43 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } } + const domain = normalizeSSODomain(body.domain) + if (!domain) { + return NextResponse.json({ error: 'Enter a valid domain like company.com' }, { status: 400 }) + } + + const isOwnedByCaller = (provider: { + userId: string | null + organizationId: string | null + }): boolean => { + if (provider.userId === session.user.id && !provider.organizationId) return true + return orgId ? provider.organizationId === orgId : false + } + + const existingProviders = await db + .select({ + userId: ssoProvider.userId, + organizationId: ssoProvider.organizationId, + }) + .from(ssoProvider) + .where(sql`lower(${ssoProvider.domain}) = ${domain}`) + const conflictingProvider = existingProviders.find((provider) => !isOwnedByCaller(provider)) + + if (conflictingProvider) { + logger.warn('Rejected SSO registration for domain owned by another tenant', { + domain, + orgId, + userId: session.user.id, + }) + return NextResponse.json( + { + error: 'This domain is already registered for SSO by another organization.', + code: 'SSO_DOMAIN_ALREADY_REGISTERED', + }, + { status: 409 } + ) + } + const headers: Record = {} request.headers.forEach((value, key) => { headers[key] = value diff --git a/apps/sim/app/api/table/[tableId]/export/route.ts b/apps/sim/app/api/table/[tableId]/export/route.ts index 8f9fa34b807..a9f7b002070 100644 --- a/apps/sim/app/api/table/[tableId]/export/route.ts +++ b/apps/sim/app/api/table/[tableId]/export/route.ts @@ -53,7 +53,9 @@ export const GET = withRouteHandler(async (request: NextRequest, { params }: Rou const encoder = new TextEncoder() try { if (format === 'csv') { - controller.enqueue(encoder.encode(`${toCsvRow(columns.map((c) => c.name))}\n`)) + controller.enqueue( + encoder.encode(`${toCsvRow(columns.map((c) => neutralizeCsvFormula(c.name)))}\n`) + ) } else { controller.enqueue(encoder.encode('[')) } @@ -111,10 +113,23 @@ function sanitizeFilename(name: string): string { return cleaned || 'table' } +/** + * Prefixes a single quote to values starting with a spreadsheet formula trigger + * (`=`, `+`, `-`, `@`, tab, CR), neutralizing CSV injection in Excel/Sheets. + */ +function neutralizeCsvFormula(value: string): string { + return /^[=+\-@\t\r]/.test(value) ? `'${value}` : value +} + +/** + * Serializes a cell for CSV. Only string cells are formula-neutralized; numbers, + * booleans, dates, and JSON objects can never form a trigger and pass through verbatim. + */ function formatCsvValue(value: unknown): string { if (value === null || value === undefined) return '' if (value instanceof Date) return value.toISOString() if (typeof value === 'object') return JSON.stringify(value) + if (typeof value === 'string') return neutralizeCsvFormula(value) return String(value) } diff --git a/apps/sim/app/api/webhooks/route.ts b/apps/sim/app/api/webhooks/route.ts index 4740909c2f5..80f34950ed2 100644 --- a/apps/sim/app/api/webhooks/route.ts +++ b/apps/sim/app/api/webhooks/route.ts @@ -27,7 +27,10 @@ import { } from '@/lib/webhooks/provider-subscriptions' import { getProviderHandler } from '@/lib/webhooks/providers' import { mergeNonUserFields } from '@/lib/webhooks/utils' -import { syncWebhooksForCredentialSet } from '@/lib/webhooks/utils.server' +import { + findConflictingWebhookPathOwner, + syncWebhooksForCredentialSet, +} from '@/lib/webhooks/utils.server' import { extractCredentialSetId, isCredentialSetValue } from '@/executor/constants' const logger = createLogger('WebhooksAPI') @@ -330,21 +333,31 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } } if (!targetWebhookId) { - const existingByPath = await db - .select({ id: webhook.id, workflowId: webhook.workflowId }) + const conflictingOwner = await findConflictingWebhookPathOwner({ + path: finalPath, + workflowId, + }) + if (conflictingOwner) { + logger.warn(`[${requestId}] Webhook path conflict: ${finalPath}`) + return NextResponse.json( + { error: 'Webhook path already exists.', code: 'PATH_EXISTS' }, + { status: 409 } + ) + } + + const ownExisting = await db + .select({ id: webhook.id }) .from(webhook) - .where(and(eq(webhook.path, finalPath), isNull(webhook.archivedAt))) - .limit(1) - if (existingByPath.length > 0) { - // If a webhook with the same path exists but belongs to a different workflow, return an error - if (existingByPath[0].workflowId !== workflowId) { - logger.warn(`[${requestId}] Webhook path conflict: ${finalPath}`) - return NextResponse.json( - { error: 'Webhook path already exists.', code: 'PATH_EXISTS' }, - { status: 409 } + .where( + and( + eq(webhook.path, finalPath), + eq(webhook.workflowId, workflowId), + isNull(webhook.archivedAt) ) - } - targetWebhookId = existingByPath[0].id + ) + .limit(1) + if (ownExisting.length > 0) { + targetWebhookId = ownExisting[0].id } } diff --git a/apps/sim/components/icons.tsx b/apps/sim/components/icons.tsx index f21df070063..2417e6acb58 100644 --- a/apps/sim/components/icons.tsx +++ b/apps/sim/components/icons.tsx @@ -780,7 +780,7 @@ export function GitLabIcon(props: SVGProps) { export function SerperIcon(props: SVGProps) { return ( - + ) { export const S3Icon = (props: SVGProps) => ( ) { export function BrainIcon(props: SVGProps) { return ( - - - - - - - - - - + + ) } @@ -2342,8 +2323,8 @@ export function ExtendIcon(props: SVGProps) { export function EvernoteIcon(props: SVGProps) { return ( - - + + ) } @@ -2538,7 +2519,7 @@ export function TelegramIcon(props: SVGProps) { ) { } export function MicrosoftOneDriveIcon(props: SVGProps) { + const id = useId() return ( - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + ) } @@ -5882,12 +6037,22 @@ export function GreenhouseIcon(props: SVGProps) { export function GreptileIcon(props: SVGProps) { return ( - + + + ) diff --git a/apps/sim/lib/auth/sso/domain.test.ts b/apps/sim/lib/auth/sso/domain.test.ts new file mode 100644 index 00000000000..5ca62331a81 --- /dev/null +++ b/apps/sim/lib/auth/sso/domain.test.ts @@ -0,0 +1,38 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { normalizeSSODomain } from '@/lib/auth/sso/domain' + +describe('normalizeSSODomain', () => { + it('lowercases and trims', () => { + expect(normalizeSSODomain(' Company.COM ')).toBe('company.com') + }) + + it('strips protocol, path, query, and port', () => { + expect(normalizeSSODomain('https://company.com/sso?x=1')).toBe('company.com') + expect(normalizeSSODomain('company.com:8443')).toBe('company.com') + }) + + it('strips wildcard, leading @, and email local part', () => { + expect(normalizeSSODomain('*.company.com')).toBe('company.com') + expect(normalizeSSODomain('@company.com')).toBe('company.com') + expect(normalizeSSODomain('user@company.com')).toBe('company.com') + }) + + it('drops a trailing dot', () => { + expect(normalizeSSODomain('company.com.')).toBe('company.com') + }) + + it('treats casing and formatting variants as the same domain', () => { + expect(normalizeSSODomain('Company.COM')).toBe(normalizeSSODomain('company.com')) + expect(normalizeSSODomain('user@Company.com')).toBe(normalizeSSODomain('company.com')) + }) + + it('rejects values that are not registrable domains', () => { + expect(normalizeSSODomain('')).toBeNull() + expect(normalizeSSODomain('localhost')).toBeNull() + expect(normalizeSSODomain('not a domain')).toBeNull() + expect(normalizeSSODomain('company')).toBeNull() + }) +}) diff --git a/apps/sim/lib/auth/sso/domain.ts b/apps/sim/lib/auth/sso/domain.ts new file mode 100644 index 00000000000..bdd0cc1714f --- /dev/null +++ b/apps/sim/lib/auth/sso/domain.ts @@ -0,0 +1,25 @@ +/** + * Normalizes a user-supplied SSO email domain to a canonical, comparable form: + * strips protocol, path, query, port, a leading wildcard/`@`, an email local + * part, and a trailing dot, then lowercases. Returns `null` for inputs that are + * not a registrable domain (e.g. `example.com`), which callers treat as invalid. + */ +export function normalizeSSODomain(input: string): string | null { + if (typeof input !== 'string') return null + + let value = input.trim().toLowerCase() + if (!value) return null + + value = value.replace(/^[a-z][a-z0-9+.-]*:\/\//, '') + value = value.replace(/^\*\./, '').replace(/^@/, '') + value = value.split('/')[0] + value = value.split('?')[0] + value = value.split('@').pop() ?? value + value = value.split(':')[0] + value = value.replace(/\.$/, '') + + if (!/^[a-z0-9-]+(\.[a-z0-9-]+)+$/.test(value)) return null + if (value.split('.').some((label) => label.length === 0 || label.length > 63)) return null + + return value +} diff --git a/apps/sim/lib/webhooks/deploy.ts b/apps/sim/lib/webhooks/deploy.ts index 6f9b97b84f3..f3fa642685b 100644 --- a/apps/sim/lib/webhooks/deploy.ts +++ b/apps/sim/lib/webhooks/deploy.ts @@ -12,7 +12,10 @@ import { shouldRecreateExternalWebhookSubscription, } from '@/lib/webhooks/provider-subscriptions' import { getProviderHandler } from '@/lib/webhooks/providers' -import { syncWebhooksForCredentialSet } from '@/lib/webhooks/utils.server' +import { + findConflictingWebhookPathOwner, + syncWebhooksForCredentialSet, +} from '@/lib/webhooks/utils.server' import { buildCanonicalIndex } from '@/lib/workflows/subblocks/visibility' import { getBlock } from '@/blocks' import type { SubBlockConfig } from '@/blocks/types' @@ -538,6 +541,23 @@ export async function saveTriggerWebhooksForDeploy({ } } + const pathConflict = await findConflictingWebhookPathOwner({ + path: triggerPath, + workflowId, + }) + if (pathConflict) { + logger.warn( + `[${requestId}] Webhook path conflict for "${triggerPath}": already owned by workflow ${pathConflict}` + ) + return { + success: false, + error: { + message: `Webhook path "${triggerPath}" is already in use. Choose a different path.`, + status: 409, + }, + } + } + webhookConfigs.set(block.id, { provider, providerConfig, triggerPath, triggerDef }) if (providerConfig.credentialSetId) { diff --git a/apps/sim/lib/webhooks/processor.test.ts b/apps/sim/lib/webhooks/processor.test.ts index c5f3894707f..381cc6108a0 100644 --- a/apps/sim/lib/webhooks/processor.test.ts +++ b/apps/sim/lib/webhooks/processor.test.ts @@ -10,23 +10,36 @@ import { } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' -const { mockGenerateId, mockEnqueue, mockGetJobQueue, mockShouldExecuteInline } = vi.hoisted( - () => ({ - mockGenerateId: vi.fn(), - mockEnqueue: vi.fn(), - mockGetJobQueue: vi.fn(), - mockShouldExecuteInline: vi.fn(), - }) -) +const { + mockGenerateId, + mockEnqueue, + mockGetJobQueue, + mockShouldExecuteInline, + mockWebhookLookupResult, +} = vi.hoisted(() => ({ + mockGenerateId: vi.fn(), + mockEnqueue: vi.fn(), + mockGetJobQueue: vi.fn(), + mockShouldExecuteInline: vi.fn(), + mockWebhookLookupResult: { rows: [] as Array<{ webhook: any; workflow: any }> }, +})) const mockPreprocessExecution = executionPreprocessingMockFns.mockPreprocessExecution -vi.mock('@sim/db', () => ({ - db: {}, - webhook: {}, - workflow: {}, - workflowDeploymentVersion: {}, -})) +vi.mock('@sim/db', () => { + const selectChain = { + from: () => selectChain, + innerJoin: () => selectChain, + leftJoin: () => selectChain, + where: () => Promise.resolve(mockWebhookLookupResult.rows), + } + return { + db: { select: () => selectChain }, + webhook: {}, + workflow: {}, + workflowDeploymentVersion: {}, + } +}) vi.mock('drizzle-orm', () => ({ and: vi.fn(), @@ -109,7 +122,74 @@ vi.mock('@/triggers/jira/utils', () => ({ isJiraEventMatch: vi.fn().mockReturnValue(true), })) -import { checkWebhookPreprocessing, queueWebhookExecution } from '@/lib/webhooks/processor' +import { + checkWebhookPreprocessing, + findAllWebhooksForPath, + queueWebhookExecution, +} from '@/lib/webhooks/processor' + +describe('findAllWebhooksForPath cross-tenant collision', () => { + beforeEach(() => { + vi.clearAllMocks() + mockWebhookLookupResult.rows = [] + }) + + const makeRow = (workflowId: string, webhookId: string, createdAt: Date) => ({ + webhook: { id: webhookId, workflowId, path: 'shared-path', createdAt }, + workflow: { id: workflowId }, + }) + + it('returns all rows when they belong to a single workflow (credential set fan-out)', async () => { + mockWebhookLookupResult.rows = [ + makeRow('workflow-1', 'wh-a', new Date('2026-01-01')), + makeRow('workflow-1', 'wh-b', new Date('2026-01-02')), + ] + + const results = await findAllWebhooksForPath({ requestId: 'req-1', path: 'shared-path' }) + + expect(results).toHaveLength(2) + expect(results.map((r) => r.webhook.id)).toEqual(['wh-a', 'wh-b']) + }) + + it('drops foreign rows when a path collides across workflows, keeping the earliest owner', async () => { + const victim = makeRow('victim-workflow', 'victim-wh', new Date('2026-01-01')) + const attacker = makeRow('attacker-workflow', 'attacker-wh', new Date('2026-05-01')) + mockWebhookLookupResult.rows = [attacker, victim] + + const results = await findAllWebhooksForPath({ requestId: 'req-2', path: 'shared-path' }) + + expect(results).toHaveLength(1) + expect(results[0].webhook.id).toBe('victim-wh') + expect(results[0].webhook.workflowId).toBe('victim-workflow') + }) + + it("preserves the owner's full credential-set fan-out while dropping a foreign row", async () => { + const victimA = makeRow('victim-workflow', 'victim-wh-a', new Date('2026-01-01')) + const victimB = makeRow('victim-workflow', 'victim-wh-b', new Date('2026-01-03')) + const attacker = makeRow('attacker-workflow', 'attacker-wh', new Date('2026-05-01')) + mockWebhookLookupResult.rows = [victimB, attacker, victimA] + + const results = await findAllWebhooksForPath({ requestId: 'req-5', path: 'shared-path' }) + + expect(results).toHaveLength(2) + expect(results.every((r) => r.webhook.workflowId === 'victim-workflow')).toBe(true) + expect(results.map((r) => r.webhook.id).sort()).toEqual(['victim-wh-a', 'victim-wh-b']) + }) + + it('returns an empty array when no webhooks match', async () => { + mockWebhookLookupResult.rows = [] + + const results = await findAllWebhooksForPath({ requestId: 'req-3', path: 'missing' }) + + expect(results).toEqual([]) + }) + + it('returns an empty array when path is not provided', async () => { + const results = await findAllWebhooksForPath({ requestId: 'req-4' }) + + expect(results).toEqual([]) + }) +}) describe('webhook processor execution identity', () => { beforeEach(() => { diff --git a/apps/sim/lib/webhooks/processor.ts b/apps/sim/lib/webhooks/processor.ts index 15f5fcacf6b..96ce09813b4 100644 --- a/apps/sim/lib/webhooks/processor.ts +++ b/apps/sim/lib/webhooks/processor.ts @@ -305,8 +305,12 @@ async function findWebhookAndWorkflow( } /** - * Find ALL webhooks matching a path. - * Used for credential sets where multiple webhooks share the same path. + * Finds all webhooks matching a path, scoped to a single workflow. + * + * Legitimate fan-out (credential sets) is always within one workflow, but paths + * are user-controlled and only unique per deployment version, so two tenants can + * register the same path. On collision we keep only the workflow that registered + * the path first, so one tenant can never receive another's webhook deliveries. */ export async function findAllWebhooksForPath( options: WebhookProcessorOptions @@ -344,7 +348,31 @@ export async function findAllWebhooksForPath( if (results.length === 0) { logger.warn(`[${options.requestId}] No active webhooks found for path: ${options.path}`) - } else if (results.length > 1) { + return results + } + + const distinctWorkflowIds = new Set(results.map((result) => result.webhook.workflowId)) + + if (distinctWorkflowIds.size > 1) { + const owner = results.reduce((earliest, candidate) => { + const candidateTime = new Date(candidate.webhook.createdAt).getTime() + const earliestTime = new Date(earliest.webhook.createdAt).getTime() + if (candidateTime !== earliestTime) { + return candidateTime < earliestTime ? candidate : earliest + } + return candidate.webhook.id < earliest.webhook.id ? candidate : earliest + }) + const ownerWorkflowId = owner.webhook.workflowId + const ownerResults = results.filter((result) => result.webhook.workflowId === ownerWorkflowId) + + logger.error( + `[${options.requestId}] Cross-tenant webhook path collision for path: ${options.path}. Found ${results.length} active webhooks across ${distinctWorkflowIds.size} workflows. Dispatching only to owner workflow ${ownerWorkflowId} and dropping ${results.length - ownerResults.length} foreign webhook(s).` + ) + + return ownerResults + } + + if (results.length > 1) { logger.info( `[${options.requestId}] Found ${results.length} webhooks for path: ${options.path} (credential set fan-out)` ) diff --git a/apps/sim/lib/webhooks/utils.server.ts b/apps/sim/lib/webhooks/utils.server.ts index a920553f16b..eab8f9cccfe 100644 --- a/apps/sim/lib/webhooks/utils.server.ts +++ b/apps/sim/lib/webhooks/utils.server.ts @@ -10,6 +10,43 @@ import { cleanupExternalWebhook } from '@/lib/webhooks/provider-subscriptions' import { getCredentialsForCredentialSet } from '@/app/api/auth/oauth/utils' import { isPollingWebhookProvider } from '@/triggers/constants' +/** + * Returns the id of a different workflow that already owns an active webhook on + * the given path, or `null` if the path is free or owned by `workflowId`. + * + * Webhook paths are user-controlled and the database only enforces uniqueness + * per deployment version, so this is the single guard against cross-tenant path + * collisions for every webhook creation path. The filter mirrors the runtime + * dispatcher (`findAllWebhooksForPath`): an active, non-archived webhook on a + * non-archived workflow — inactive or archived webhooks never receive + * deliveries, so they must not reserve a path. All matching rows are scanned so + * a same-workflow row can never mask a foreign collision. + */ +export async function findConflictingWebhookPathOwner(params: { + path: string + workflowId: string + tx?: DbOrTx +}): Promise { + const { path, workflowId, tx } = params + const dbCtx = tx ?? db + + const existing = await dbCtx + .select({ workflowId: webhook.workflowId }) + .from(webhook) + .innerJoin(workflow, eq(webhook.workflowId, workflow.id)) + .where( + and( + eq(webhook.path, path), + eq(webhook.isActive, true), + isNull(webhook.archivedAt), + isNull(workflow.archivedAt) + ) + ) + + const conflict = existing.find((row) => row.workflowId !== workflowId) + return conflict ? conflict.workflowId : null +} + /** * Result of syncing webhooks for a credential set */