From 312c53e297b7dd0a7beaafcc1e1419e7350d62eb Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 12 Sep 2026 20:07:52 -0700 Subject: [PATCH 1/6] fix(webhooks): authorize credential references on webhook upsert POST /api/webhooks persisted client-supplied providerConfig verbatim, while subscription handlers and pollers resolve providerConfig.credentialId by id alone and mint tokens as the credential's owner. Authorize credentialId for the acting user within the workflow's workspace before subscribing or saving, require it to be a literal id, and never accept a client-supplied userId, which the polling token resolver falls back to. --- apps/sim/app/api/webhooks/route.test.ts | 218 ++++++++++++++++++------ apps/sim/app/api/webhooks/route.ts | 43 ++++- 2 files changed, 204 insertions(+), 57 deletions(-) diff --git a/apps/sim/app/api/webhooks/route.test.ts b/apps/sim/app/api/webhooks/route.test.ts index 85c318c1ca2..c22221417a3 100644 --- a/apps/sim/app/api/webhooks/route.test.ts +++ b/apps/sim/app/api/webhooks/route.test.ts @@ -19,6 +19,7 @@ import { import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ + authorizeCredentialUseForAuth: vi.fn(), configurePolling: vi.fn(), createExternalWebhookSubscription: vi.fn(), findConflictingWebhookPathOwner: vi.fn(), @@ -31,6 +32,9 @@ vi.mock('@sim/audit', () => auditMock) vi.mock('@/lib/permission-groups/config-scope.server', () => permissionGroupScopeMock) vi.mock('@/lib/core/telemetry', () => telemetryMock) vi.mock('@/lib/posthog/server', () => posthogServerMock) +vi.mock('@/lib/auth/credential-access', () => ({ + authorizeCredentialUseForAuth: mocks.authorizeCredentialUseForAuth, +})) vi.mock('@/lib/webhooks/env-resolver', () => ({ resolveEnvVarsInObject: mocks.resolveEnvVarsInObject, })) @@ -353,46 +357,72 @@ describe('POST /api/webhooks polling configuration', () => { }) }) -describe('POST /api/webhooks triggers.webhook gate', () => { - beforeEach(() => { - vi.clearAllMocks() - resetDbChainMock() - authMockFns.mockGetSession.mockResolvedValue({ - user: { id: 'actor-1', name: 'Actor', email: 'actor@example.com' }, - session: { id: 'session-1' }, - }) - workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ - allowed: true, - status: 200, - workflow: { id: 'workflow-1' }, - workspacePermission: 'write', - }) - workflowAuthzMockFns.mockAssertWorkflowMutable.mockResolvedValue(undefined) - permissionGroupScopeMockFns.mockResolvePermissionGroupConfig.mockResolvedValue(null) - mocks.findConflictingWebhookPathOwner.mockResolvedValue(null) - mocks.resolveEnvVarsInObject.mockImplementation(async (config) => config) - mocks.shouldRecreateExternalWebhookSubscription.mockReturnValue(false) - mocks.getProviderHandler.mockReturnValue({}) - mocks.createExternalWebhookSubscription.mockResolvedValue({ - updatedProviderConfig: {}, - externalSubscriptionCreated: false, - }) +/** Mocks an actor with write access to `workflow-1` and no provider side effects. */ +function setupUpsertMocks(): void { + vi.clearAllMocks() + resetDbChainMock() + authMockFns.mockGetSession.mockResolvedValue({ + user: { id: 'actor-1', name: 'Actor', email: 'actor@example.com' }, + session: { id: 'session-1' }, }) - - function upsertRequest() { - return createMockRequest('POST', { + workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, + workflow: { id: 'workflow-1' }, + workspacePermission: 'write', + }) + workflowAuthzMockFns.mockAssertWorkflowMutable.mockResolvedValue(undefined) + permissionGroupScopeMockFns.mockResolvePermissionGroupConfig.mockResolvedValue(null) + mocks.findConflictingWebhookPathOwner.mockResolvedValue(null) + mocks.resolveEnvVarsInObject.mockImplementation(async (config) => config) + mocks.shouldRecreateExternalWebhookSubscription.mockReturnValue(false) + mocks.getProviderHandler.mockReturnValue({}) + mocks.createExternalWebhookSubscription.mockResolvedValue({ + updatedProviderConfig: {}, + externalSubscriptionCreated: false, + }) +} + +function upsertRequest(providerConfig: Record = {}) { + return createMockRequest('POST', { + workflowId: 'workflow-1', + path: 'inbound-orders', + provider: 'generic', + providerConfig, + }) +} + +/** The reads the create path makes, in the order the handler issues them. */ +function queueCreatePathRows(): void { + queueTableRows(workflow, [{ id: 'workflow-1', userId: 'actor-1', workspaceId: 'workspace-1' }]) + queueTableRows(webhook, []) +} + +/** The reads the update path makes: the path claim, then the existing row. */ +function queueUpdatePathRows( + isActive: boolean, + providerConfig: Record = {} +): void { + queueTableRows(workflow, [{ id: 'workflow-1', userId: 'actor-1', workspaceId: 'workspace-1' }]) + queueTableRows(webhook, [{ id: 'webhook-1' }]) + queueTableRows(webhook, [ + { + id: 'webhook-1', workflowId: 'workflow-1', + blockId: 'block-1', path: 'inbound-orders', provider: 'generic', - providerConfig: {}, - }) - } + providerConfig, + isActive, + }, + ]) + dbChainMockFns.returning.mockImplementationOnce(async () => [ + { id: 'webhook-1', workflowId: 'workflow-1', path: 'inbound-orders', isActive: true }, + ]) +} - /** The reads the create path makes, in the order the handler issues them. */ - function queueCreatePathRows(): void { - queueTableRows(workflow, [{ id: 'workflow-1', userId: 'actor-1', workspaceId: 'workspace-1' }]) - queueTableRows(webhook, []) - } +describe('POST /api/webhooks triggers.webhook gate', () => { + beforeEach(setupUpsertMocks) /** * Making a workflow reachable from an inbound webhook is the only external @@ -421,26 +451,6 @@ describe('POST /api/webhooks triggers.webhook gate', () => { expect(mocks.createExternalWebhookSubscription).toHaveBeenCalledTimes(1) }) - /** The reads the update path makes: the path claim, then the existing row. */ - function queueUpdatePathRows(isActive: boolean): void { - queueTableRows(workflow, [{ id: 'workflow-1', userId: 'actor-1', workspaceId: 'workspace-1' }]) - queueTableRows(webhook, [{ id: 'webhook-1' }]) - queueTableRows(webhook, [ - { - id: 'webhook-1', - workflowId: 'workflow-1', - blockId: 'block-1', - path: 'inbound-orders', - provider: 'generic', - providerConfig: {}, - isActive, - }, - ]) - dbChainMockFns.returning.mockImplementationOnce(async () => [ - { id: 'webhook-1', workflowId: 'workflow-1', path: 'inbound-orders', isActive: true }, - ]) - } - /** * The upsert always writes `isActive: true`, so re-saving a dormant webhook is * the same transition `PATCH /api/webhooks/[id]` gates — a workflow becoming @@ -488,3 +498,101 @@ describe('POST /api/webhooks triggers.webhook gate', () => { expect(dbChainMockFns.set).toHaveBeenCalledWith(expect.objectContaining({ isActive: true })) }) }) + +describe('POST /api/webhooks credential references', () => { + beforeEach(setupUpsertMocks) + + /** + * Subscription setup and polling mint tokens as the credential's owner, so a + * reference the actor cannot use must be refused before either runs. + */ + it('refuses a credential the actor cannot use', async () => { + mocks.authorizeCredentialUseForAuth.mockResolvedValue({ + ok: false, + error: 'Credential is not accessible from this workflow workspace', + }) + queueCreatePathRows() + + const response = await POST(upsertRequest({ credentialId: 'victim-credential' })) + + expect(response.status).toBe(403) + expect(mocks.authorizeCredentialUseForAuth).toHaveBeenCalledWith( + expect.objectContaining({ success: true, userId: 'actor-1' }), + { credentialId: 'victim-credential', workflowId: 'workflow-1' } + ) + expect(mocks.createExternalWebhookSubscription).not.toHaveBeenCalled() + expect(dbChainMockFns.values).not.toHaveBeenCalled() + }) + + it('refuses a credential id supplied through an env-var reference', async () => { + mocks.resolveEnvVarsInObject.mockImplementation(async (config) => ({ + ...config, + credentialId: 'victim-credential', + })) + queueCreatePathRows() + + const response = await POST(upsertRequest({ credentialId: '{{CREDENTIAL}}' })) + + expect(response.status).toBe(400) + expect(mocks.authorizeCredentialUseForAuth).not.toHaveBeenCalled() + expect(mocks.createExternalWebhookSubscription).not.toHaveBeenCalled() + }) + + it('saves a credential the actor can use in the workflow workspace', async () => { + mocks.authorizeCredentialUseForAuth.mockResolvedValue({ ok: true, workspaceId: 'workspace-1' }) + queueCreatePathRows() + + const response = await POST(upsertRequest({ credentialId: 'own-credential' })) + + expect(response.status).toBe(201) + expect(mocks.createExternalWebhookSubscription).toHaveBeenCalledTimes(1) + expect(dbChainMockFns.values).toHaveBeenCalledWith( + expect.objectContaining({ providerConfig: { credentialId: 'own-credential' } }) + ) + }) + + /** The polling token resolver mints `providerConfig.userId`'s token when no credential is set. */ + it('drops a client-supplied userId before subscribing or saving', async () => { + queueCreatePathRows() + + const response = await POST( + upsertRequest({ userId: 'victim-user', eventType: 'record.created' }) + ) + + expect(response.status).toBe(201) + expect(mocks.createExternalWebhookSubscription).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ providerConfig: { eventType: 'record.created' } }), + expect.anything(), + 'actor-1', + expect.anything() + ) + expect(dbChainMockFns.values).toHaveBeenCalledWith( + expect.objectContaining({ providerConfig: { eventType: 'record.created' } }) + ) + }) + + /** + * A re-save that omits the identity fields keeps the ones the server stored, + * so an existing integration is not broken by the client never sending them. + */ + it('keeps the stored credential and server-set userId on a re-save that omits them', async () => { + queueUpdatePathRows(true, { credentialId: 'stored-credential', userId: 'credential-owner' }) + + const response = await POST( + upsertRequest({ userId: 'victim-user', eventType: 'record.created' }) + ) + + expect(response.status).toBe(200) + expect(mocks.authorizeCredentialUseForAuth).not.toHaveBeenCalled() + expect(dbChainMockFns.set).toHaveBeenCalledWith( + expect.objectContaining({ + providerConfig: { + eventType: 'record.created', + credentialId: 'stored-credential', + userId: 'credential-owner', + }, + }) + ) + }) +}) diff --git a/apps/sim/app/api/webhooks/route.ts b/apps/sim/app/api/webhooks/route.ts index e96cb78b084..be098817f91 100644 --- a/apps/sim/app/api/webhooks/route.ts +++ b/apps/sim/app/api/webhooks/route.ts @@ -9,11 +9,14 @@ import { } from '@sim/platform-authz/workflow' import { getErrorMessage } from '@sim/utils/errors' import { generateId, generateShortId } from '@sim/utils/id' +import { omit } from '@sim/utils/object' import { and, desc, eq, inArray, isNull, or } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { listWebhooksContract, upsertWebhookContract } from '@/lib/api/contracts/webhooks' import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' +import { authorizeCredentialUseForAuth } from '@/lib/auth/credential-access' +import { AuthType } from '@/lib/auth/hybrid' import { PlatformEvents } from '@/lib/core/telemetry' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -374,13 +377,49 @@ export const POST = withRouteHandler(async (request: NextRequest) => { let savedWebhook: any = null let existingWebhook: any = null - const originalProviderConfig = providerConfig || {} + /** + * `userId` is server-owned: the polling token resolver falls back to that + * user's own OAuth account when no credential is set. + */ + const originalProviderConfig: Record = omit(providerConfig || {}, ['userId']) let resolvedProviderConfig = await resolveEnvVarsInObject( originalProviderConfig, userId, workflowRecord.workspaceId || undefined ) + /** + * Subscription handlers and pollers look `credentialId` up by id alone and + * mint tokens as its owner, so the actor must be able to use it in the + * workflow's workspace before anything is subscribed or saved. + */ + const requestedCredentialId = originalProviderConfig.credentialId + if (requestedCredentialId != null && requestedCredentialId !== '') { + /** The row stores the unresolved text, so only a literal id is what gets authorized. */ + if ( + typeof requestedCredentialId !== 'string' || + resolvedProviderConfig.credentialId !== requestedCredentialId + ) { + return NextResponse.json( + { error: 'providerConfig.credentialId must be a literal credential id' }, + { status: 400 } + ) + } + const credentialAccess = await authorizeCredentialUseForAuth( + { success: true, userId, authType: AuthType.SESSION }, + { credentialId: requestedCredentialId, workflowId } + ) + if (!credentialAccess.ok) { + logger.warn(`[${requestId}] Webhook credential reference denied`, { + userId, + workflowId, + credentialId: requestedCredentialId, + reason: credentialAccess.error, + }) + return NextResponse.json({ error: credentialAccess.error }, { status: 403 }) + } + } + let externalSubscriptionCreated = false const createTempWebhookData = (providerConfigOverride = resolvedProviderConfig) => ({ id: targetWebhookId || generateShortId(), @@ -389,7 +428,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { providerConfig: providerConfigOverride, }) - const userProvided = originalProviderConfig as Record + const userProvided = originalProviderConfig const configToSave: Record = { ...userProvided } if (targetWebhookId) { From 5f649ec85f913e009eb0e11b59126d56b2e1ed4c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 12 Sep 2026 20:15:50 -0700 Subject: [PATCH 2/6] fix(webhooks): authorize the stored credential and drop stored userId on re-save --- apps/sim/app/api/webhooks/route.test.ts | 38 ++++++++----- apps/sim/app/api/webhooks/route.ts | 72 ++++++++++++++----------- 2 files changed, 67 insertions(+), 43 deletions(-) diff --git a/apps/sim/app/api/webhooks/route.test.ts b/apps/sim/app/api/webhooks/route.test.ts index c22221417a3..dce67600c5c 100644 --- a/apps/sim/app/api/webhooks/route.test.ts +++ b/apps/sim/app/api/webhooks/route.test.ts @@ -573,26 +573,38 @@ describe('POST /api/webhooks credential references', () => { }) /** - * A re-save that omits the identity fields keeps the ones the server stored, - * so an existing integration is not broken by the client never sending them. + * A re-save that omits `credentialId` still acts with the stored credential + * (polling setup and subscription cleanup read it), so that credential is + * authorized and kept, while a stored `userId` is never carried forward. */ - it('keeps the stored credential and server-set userId on a re-save that omits them', async () => { - queueUpdatePathRows(true, { credentialId: 'stored-credential', userId: 'credential-owner' }) + it('authorizes and keeps the stored credential on a re-save that omits it', async () => { + mocks.authorizeCredentialUseForAuth.mockResolvedValue({ ok: true, workspaceId: 'workspace-1' }) + queueUpdatePathRows(true, { credentialId: 'stored-credential', userId: 'stored-user' }) - const response = await POST( - upsertRequest({ userId: 'victim-user', eventType: 'record.created' }) - ) + const response = await POST(upsertRequest({ eventType: 'record.created' })) expect(response.status).toBe(200) - expect(mocks.authorizeCredentialUseForAuth).not.toHaveBeenCalled() + expect(mocks.authorizeCredentialUseForAuth).toHaveBeenCalledWith(expect.anything(), { + credentialId: 'stored-credential', + workflowId: 'workflow-1', + }) expect(dbChainMockFns.set).toHaveBeenCalledWith( expect.objectContaining({ - providerConfig: { - eventType: 'record.created', - credentialId: 'stored-credential', - userId: 'credential-owner', - }, + providerConfig: { eventType: 'record.created', credentialId: 'stored-credential' }, }) ) }) + + it('refuses a re-save whose stored credential the actor cannot use', async () => { + mocks.authorizeCredentialUseForAuth.mockResolvedValue({ + ok: false, + error: 'You do not have access to this credential.', + }) + queueUpdatePathRows(true, { credentialId: 'stored-credential' }) + + const response = await POST(upsertRequest({ eventType: 'record.created' })) + + expect(response.status).toBe(403) + expect(dbChainMockFns.set).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/app/api/webhooks/route.ts b/apps/sim/app/api/webhooks/route.ts index be098817f91..eb453c5b5c9 100644 --- a/apps/sim/app/api/webhooks/route.ts +++ b/apps/sim/app/api/webhooks/route.ts @@ -379,7 +379,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => { let existingWebhook: any = null /** * `userId` is server-owned: the polling token resolver falls back to that - * user's own OAuth account when no credential is set. + * user's own OAuth account when no credential is set. It is neither accepted + * from the client nor carried forward from a stored row; Gmail and Outlook + * polling setup derive it again from the credential after the save. */ const originalProviderConfig: Record = omit(providerConfig || {}, ['userId']) let resolvedProviderConfig = await resolveEnvVarsInObject( @@ -388,36 +390,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => { workflowRecord.workspaceId || undefined ) - /** - * Subscription handlers and pollers look `credentialId` up by id alone and - * mint tokens as its owner, so the actor must be able to use it in the - * workflow's workspace before anything is subscribed or saved. - */ - const requestedCredentialId = originalProviderConfig.credentialId - if (requestedCredentialId != null && requestedCredentialId !== '') { - /** The row stores the unresolved text, so only a literal id is what gets authorized. */ - if ( - typeof requestedCredentialId !== 'string' || - resolvedProviderConfig.credentialId !== requestedCredentialId - ) { - return NextResponse.json( - { error: 'providerConfig.credentialId must be a literal credential id' }, - { status: 400 } - ) - } - const credentialAccess = await authorizeCredentialUseForAuth( - { success: true, userId, authType: AuthType.SESSION }, - { credentialId: requestedCredentialId, workflowId } + /** The row stores the unresolved text, so only a literal credential id can be authorized. */ + if (resolvedProviderConfig.credentialId !== originalProviderConfig.credentialId) { + return NextResponse.json( + { error: 'providerConfig.credentialId must be a literal credential id' }, + { status: 400 } ) - if (!credentialAccess.ok) { - logger.warn(`[${requestId}] Webhook credential reference denied`, { - userId, - workflowId, - credentialId: requestedCredentialId, - reason: credentialAccess.error, - }) - return NextResponse.json({ error: credentialAccess.error }, { status: 403 }) - } } let externalSubscriptionCreated = false @@ -440,6 +418,39 @@ export const POST = withRouteHandler(async (request: NextRequest) => { existingWebhook = existingRows[0] || null } + /** + * Subscription handlers, pollers, and subscription cleanup look `credentialId` + * up by id alone and mint tokens as its owner. A save acts with the requested + * credential or, when the request omits it, the stored one, so that credential + * must be usable by the actor in the workflow's workspace before anything is + * subscribed, cleaned up, or saved. + */ + const effectiveCredentialId = + 'credentialId' in originalProviderConfig + ? originalProviderConfig.credentialId + : existingWebhook?.providerConfig?.credentialId + if (effectiveCredentialId != null && effectiveCredentialId !== '') { + if (typeof effectiveCredentialId !== 'string') { + return NextResponse.json( + { error: 'providerConfig.credentialId must be a literal credential id' }, + { status: 400 } + ) + } + const credentialAccess = await authorizeCredentialUseForAuth( + { success: true, userId, authType: AuthType.SESSION }, + { credentialId: effectiveCredentialId, workflowId } + ) + if (!credentialAccess.ok) { + logger.warn(`[${requestId}] Webhook credential reference denied`, { + userId, + workflowId, + credentialId: effectiveCredentialId, + reason: credentialAccess.error, + }) + return NextResponse.json({ error: credentialAccess.error }, { status: 403 }) + } + } + /** * permission-group-enforced: triggers.webhook — a raw upsert handler with no * application operation to declare the capability on, so it is asserted @@ -519,6 +530,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { userProvided ) } + configToSave.userId = undefined try { if (targetWebhookId) { From caf468fa0051715c35ea838fc279e370dcfe6433 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 12 Sep 2026 20:22:42 -0700 Subject: [PATCH 3/6] fix(webhooks): authorize both requested and stored credentials on upsert --- apps/sim/app/api/webhooks/route.test.ts | 39 +++++++++++++++++++++++++ apps/sim/app/api/webhooks/route.ts | 26 +++++++++-------- 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/apps/sim/app/api/webhooks/route.test.ts b/apps/sim/app/api/webhooks/route.test.ts index dce67600c5c..d688721f2c4 100644 --- a/apps/sim/app/api/webhooks/route.test.ts +++ b/apps/sim/app/api/webhooks/route.test.ts @@ -607,4 +607,43 @@ describe('POST /api/webhooks credential references', () => { expect(response.status).toBe(403) expect(dbChainMockFns.set).not.toHaveBeenCalled() }) + + /** + * Clearing `credentialId` does not stop the save from acting with the stored + * credential: a recreate still cleans up the previous subscription with it. + */ + it.each([null, ''])( + 'still authorizes the stored credential when a re-save sends credentialId %j', + async (credentialId) => { + mocks.authorizeCredentialUseForAuth.mockResolvedValue({ + ok: false, + error: 'You do not have access to this credential.', + }) + mocks.shouldRecreateExternalWebhookSubscription.mockReturnValue(true) + queueUpdatePathRows(true, { credentialId: 'stored-credential' }) + + const response = await POST(upsertRequest({ credentialId })) + + expect(response.status).toBe(403) + expect(mocks.authorizeCredentialUseForAuth).toHaveBeenCalledWith(expect.anything(), { + credentialId: 'stored-credential', + workflowId: 'workflow-1', + }) + expect(mocks.createExternalWebhookSubscription).not.toHaveBeenCalled() + expect(dbChainMockFns.set).not.toHaveBeenCalled() + } + ) + + it('authorizes both credentials when a re-save replaces the stored one', async () => { + mocks.authorizeCredentialUseForAuth.mockResolvedValue({ ok: true, workspaceId: 'workspace-1' }) + queueUpdatePathRows(true, { credentialId: 'stored-credential' }) + + const response = await POST(upsertRequest({ credentialId: 'new-credential' })) + + expect(response.status).toBe(200) + expect(mocks.authorizeCredentialUseForAuth.mock.calls.map(([, params]) => params)).toEqual([ + { credentialId: 'new-credential', workflowId: 'workflow-1' }, + { credentialId: 'stored-credential', workflowId: 'workflow-1' }, + ]) + }) }) diff --git a/apps/sim/app/api/webhooks/route.ts b/apps/sim/app/api/webhooks/route.ts index eb453c5b5c9..645ba6f941a 100644 --- a/apps/sim/app/api/webhooks/route.ts +++ b/apps/sim/app/api/webhooks/route.ts @@ -420,17 +420,19 @@ export const POST = withRouteHandler(async (request: NextRequest) => { /** * Subscription handlers, pollers, and subscription cleanup look `credentialId` - * up by id alone and mint tokens as its owner. A save acts with the requested - * credential or, when the request omits it, the stored one, so that credential - * must be usable by the actor in the workflow's workspace before anything is - * subscribed, cleaned up, or saved. + * up by id alone and mint tokens as its owner. A save can act with both the + * requested credential and the stored one — the stored one is merged back when + * the request omits it, or used to clean up the previous subscription — so + * each must be usable by the actor in the workflow's workspace before anything + * is subscribed, cleaned up, or saved. */ - const effectiveCredentialId = - 'credentialId' in originalProviderConfig - ? originalProviderConfig.credentialId - : existingWebhook?.providerConfig?.credentialId - if (effectiveCredentialId != null && effectiveCredentialId !== '') { - if (typeof effectiveCredentialId !== 'string') { + const credentialIds = new Set( + [originalProviderConfig.credentialId, existingWebhook?.providerConfig?.credentialId].filter( + (id) => id != null && id !== '' + ) + ) + for (const credentialId of credentialIds) { + if (typeof credentialId !== 'string') { return NextResponse.json( { error: 'providerConfig.credentialId must be a literal credential id' }, { status: 400 } @@ -438,13 +440,13 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } const credentialAccess = await authorizeCredentialUseForAuth( { success: true, userId, authType: AuthType.SESSION }, - { credentialId: effectiveCredentialId, workflowId } + { credentialId, workflowId } ) if (!credentialAccess.ok) { logger.warn(`[${requestId}] Webhook credential reference denied`, { userId, workflowId, - credentialId: effectiveCredentialId, + credentialId, reason: credentialAccess.error, }) return NextResponse.json({ error: credentialAccess.error }, { status: 403 }) From ab90ec131863f1c9a6216b7edab4115d24972688 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 12 Sep 2026 20:28:51 -0700 Subject: [PATCH 4/6] fix(webhooks): authorize the stored credential only when the save uses it --- apps/sim/app/api/webhooks/route.test.ts | 24 ++++++++++++++- apps/sim/app/api/webhooks/route.ts | 40 ++++++++++++++----------- 2 files changed, 45 insertions(+), 19 deletions(-) diff --git a/apps/sim/app/api/webhooks/route.test.ts b/apps/sim/app/api/webhooks/route.test.ts index d688721f2c4..332c7dd88b5 100644 --- a/apps/sim/app/api/webhooks/route.test.ts +++ b/apps/sim/app/api/webhooks/route.test.ts @@ -634,8 +634,30 @@ describe('POST /api/webhooks credential references', () => { } ) - it('authorizes both credentials when a re-save replaces the stored one', async () => { + /** Rotation without recreation never touches the old credential, so it needs no access to it. */ + it('rotates the credential without access to the stored one when nothing is recreated', async () => { + mocks.authorizeCredentialUseForAuth.mockImplementation(async (_auth, { credentialId }) => + credentialId === 'new-credential' + ? { ok: true, workspaceId: 'workspace-1' } + : { ok: false, error: 'You do not have access to this credential.' } + ) + queueUpdatePathRows(true, { credentialId: 'stored-credential' }) + + const response = await POST(upsertRequest({ credentialId: 'new-credential' })) + + expect(response.status).toBe(200) + expect(mocks.authorizeCredentialUseForAuth.mock.calls.map(([, params]) => params)).toEqual([ + { credentialId: 'new-credential', workflowId: 'workflow-1' }, + ]) + expect(dbChainMockFns.set).toHaveBeenCalledWith( + expect.objectContaining({ providerConfig: { credentialId: 'new-credential' } }) + ) + }) + + /** Recreation cleans up the previous subscription with the stored credential. */ + it('authorizes both credentials when a rotation recreates the subscription', async () => { mocks.authorizeCredentialUseForAuth.mockResolvedValue({ ok: true, workspaceId: 'workspace-1' }) + mocks.shouldRecreateExternalWebhookSubscription.mockReturnValue(true) queueUpdatePathRows(true, { credentialId: 'stored-credential' }) const response = await POST(upsertRequest({ credentialId: 'new-credential' })) diff --git a/apps/sim/app/api/webhooks/route.ts b/apps/sim/app/api/webhooks/route.ts index 645ba6f941a..62dd3ec6962 100644 --- a/apps/sim/app/api/webhooks/route.ts +++ b/apps/sim/app/api/webhooks/route.ts @@ -418,18 +418,32 @@ export const POST = withRouteHandler(async (request: NextRequest) => { existingWebhook = existingRows[0] || null } + const shouldRecreateSubscription = + existingWebhook && + shouldRecreateExternalWebhookSubscription({ + previousProvider: existingWebhook.provider as string, + nextProvider: provider, + previousConfig: ((existingWebhook.providerConfig as Record) || + {}) as Record, + nextConfig: resolvedProviderConfig, + }) + /** * Subscription handlers, pollers, and subscription cleanup look `credentialId` - * up by id alone and mint tokens as its owner. A save can act with both the - * requested credential and the stored one — the stored one is merged back when - * the request omits it, or used to clean up the previous subscription — so - * each must be usable by the actor in the workflow's workspace before anything - * is subscribed, cleaned up, or saved. + * up by id alone and mint tokens as its owner, so every credential this save + * acts with must be usable by the actor in the workflow's workspace before + * anything is subscribed, cleaned up, or saved. That is the requested + * credential, plus the stored one when the save uses it: merged back because + * the request omits `credentialId`, or used to clean up the previous + * subscription on recreation. */ + const usesStoredCredential = + existingWebhook && (shouldRecreateSubscription || !('credentialId' in originalProviderConfig)) const credentialIds = new Set( - [originalProviderConfig.credentialId, existingWebhook?.providerConfig?.credentialId].filter( - (id) => id != null && id !== '' - ) + [ + originalProviderConfig.credentialId, + usesStoredCredential ? existingWebhook.providerConfig?.credentialId : undefined, + ].filter((id) => id != null && id !== '') ) for (const credentialId of credentialIds) { if (typeof credentialId !== 'string') { @@ -492,16 +506,6 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } } - const shouldRecreateSubscription = - existingWebhook && - shouldRecreateExternalWebhookSubscription({ - previousProvider: existingWebhook.provider as string, - nextProvider: provider, - previousConfig: ((existingWebhook.providerConfig as Record) || - {}) as Record, - nextConfig: resolvedProviderConfig, - }) - if (!existingWebhook || shouldRecreateSubscription) { try { const result = await createExternalWebhookSubscription( From 17128ac3a05996ee32221a06cbdbba0466d0c33f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 14 Sep 2026 11:46:41 -0700 Subject: [PATCH 5/6] fix(webhooks): check credentials after the permission-group gate --- apps/sim/app/api/webhooks/route.test.ts | 40 +++++++++++ apps/sim/app/api/webhooks/route.ts | 94 ++++++++++++------------- 2 files changed, 87 insertions(+), 47 deletions(-) diff --git a/apps/sim/app/api/webhooks/route.test.ts b/apps/sim/app/api/webhooks/route.test.ts index 332c7dd88b5..879cb61f583 100644 --- a/apps/sim/app/api/webhooks/route.test.ts +++ b/apps/sim/app/api/webhooks/route.test.ts @@ -605,6 +605,7 @@ describe('POST /api/webhooks credential references', () => { const response = await POST(upsertRequest({ eventType: 'record.created' })) expect(response.status).toBe(403) + expect(mocks.createExternalWebhookSubscription).not.toHaveBeenCalled() expect(dbChainMockFns.set).not.toHaveBeenCalled() }) @@ -668,4 +669,43 @@ describe('POST /api/webhooks credential references', () => { { credentialId: 'stored-credential', workflowId: 'workflow-1' }, ]) }) + + /** + * Recreation cleans up the previous subscription with the stored credential even + * when the request omits it, and a `userId` echoed back by the provider is not saved. + */ + it('authorizes the stored credential and drops userId when an omitting re-save recreates', async () => { + mocks.authorizeCredentialUseForAuth.mockResolvedValue({ ok: true, workspaceId: 'workspace-1' }) + mocks.shouldRecreateExternalWebhookSubscription.mockReturnValue(true) + mocks.createExternalWebhookSubscription.mockResolvedValue({ + updatedProviderConfig: { externalId: 'subscription-2', userId: 'stored-user' }, + externalSubscriptionCreated: true, + }) + queueUpdatePathRows(true, { credentialId: 'stored-credential', userId: 'stored-user' }) + + const response = await POST(upsertRequest({ eventType: 'record.created' })) + + expect(response.status).toBe(200) + expect(mocks.authorizeCredentialUseForAuth).toHaveBeenCalledWith(expect.anything(), { + credentialId: 'stored-credential', + workflowId: 'workflow-1', + }) + const savedConfig = dbChainMockFns.set.mock.calls.at(-1)?.[0].providerConfig + expect(savedConfig.userId).toBeUndefined() + expect(savedConfig).toEqual({ eventType: 'record.created', externalId: 'subscription-2' }) + }) + + /** The permission-group refusal keeps answering first, before any credential lookup. */ + it('refuses a withheld creation before authorizing its credential', async () => { + permissionGroupScopeMockFns.mockResolvePermissionGroupConfig.mockResolvedValue({ + ...DEFAULT_PERMISSION_GROUP_CONFIG, + disableWebhookTriggers: true, + }) + queueCreatePathRows() + + const response = await POST(upsertRequest({ credentialId: 'victim-credential' })) + + expect(response.status).toBe(403) + expect(mocks.authorizeCredentialUseForAuth).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/app/api/webhooks/route.ts b/apps/sim/app/api/webhooks/route.ts index 62dd3ec6962..050836fc1da 100644 --- a/apps/sim/app/api/webhooks/route.ts +++ b/apps/sim/app/api/webhooks/route.ts @@ -390,14 +390,6 @@ export const POST = withRouteHandler(async (request: NextRequest) => { workflowRecord.workspaceId || undefined ) - /** The row stores the unresolved text, so only a literal credential id can be authorized. */ - if (resolvedProviderConfig.credentialId !== originalProviderConfig.credentialId) { - return NextResponse.json( - { error: 'providerConfig.credentialId must be a literal credential id' }, - { status: 400 } - ) - } - let externalSubscriptionCreated = false const createTempWebhookData = (providerConfigOverride = resolvedProviderConfig) => ({ id: targetWebhookId || generateShortId(), @@ -418,6 +410,45 @@ export const POST = withRouteHandler(async (request: NextRequest) => { existingWebhook = existingRows[0] || null } + /** + * permission-group-enforced: triggers.webhook — a raw upsert handler with no + * application operation to declare the capability on, so it is asserted + * here. + * + * Creation and reactivation, because both end with a workflow newly + * reachable from an inbound webhook: this upsert always writes + * `isActive: true`, so re-saving a dormant webhook turns it back on exactly + * as `PATCH /api/webhooks/[id]` would, and that route already gates the same + * transition. + * + * Re-saving an already-active webhook is not gated. It changes the config of + * an endpoint that is already reachable and adds no exposure, and refusing + * it would strand a member unable to repair a live integration — the same + * reason inbound delivery is never gated. Inbound delivery runs with no + * session to resolve a group against, and refusing there would break live + * integrations at the provider rather than in Sim. Removing existing + * exposure stays a deliberate act of deleting or deactivating the webhook. + */ + if (!existingWebhook || existingWebhook.isActive === false) { + const withheld = workflowRecord.workspaceId + ? await isWorkspaceCapabilityWithheld( + userId, + workflowRecord.workspaceId, + 'triggers.webhook' + ) + : false + if (withheld) { + logger.warn( + `[${requestId}] Webhook ${existingWebhook ? 'reactivation' : 'creation'} blocked by permission group`, + { + userId, + workflowId, + } + ) + return NextResponse.json({ error: capabilityRefusal('triggers.webhook') }, { status: 403 }) + } + } + const shouldRecreateSubscription = existingWebhook && shouldRecreateExternalWebhookSubscription({ @@ -428,6 +459,14 @@ export const POST = withRouteHandler(async (request: NextRequest) => { nextConfig: resolvedProviderConfig, }) + /** The row stores the unresolved text, so only a literal credential id can be authorized. */ + if (resolvedProviderConfig.credentialId !== originalProviderConfig.credentialId) { + return NextResponse.json( + { error: 'providerConfig.credentialId must be a literal credential id' }, + { status: 400 } + ) + } + /** * Subscription handlers, pollers, and subscription cleanup look `credentialId` * up by id alone and mint tokens as its owner, so every credential this save @@ -467,45 +506,6 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } } - /** - * permission-group-enforced: triggers.webhook — a raw upsert handler with no - * application operation to declare the capability on, so it is asserted - * here. - * - * Creation and reactivation, because both end with a workflow newly - * reachable from an inbound webhook: this upsert always writes - * `isActive: true`, so re-saving a dormant webhook turns it back on exactly - * as `PATCH /api/webhooks/[id]` would, and that route already gates the same - * transition. - * - * Re-saving an already-active webhook is not gated. It changes the config of - * an endpoint that is already reachable and adds no exposure, and refusing - * it would strand a member unable to repair a live integration — the same - * reason inbound delivery is never gated. Inbound delivery runs with no - * session to resolve a group against, and refusing there would break live - * integrations at the provider rather than in Sim. Removing existing - * exposure stays a deliberate act of deleting or deactivating the webhook. - */ - if (!existingWebhook || existingWebhook.isActive === false) { - const withheld = workflowRecord.workspaceId - ? await isWorkspaceCapabilityWithheld( - userId, - workflowRecord.workspaceId, - 'triggers.webhook' - ) - : false - if (withheld) { - logger.warn( - `[${requestId}] Webhook ${existingWebhook ? 'reactivation' : 'creation'} blocked by permission group`, - { - userId, - workflowId, - } - ) - return NextResponse.json({ error: capabilityRefusal('triggers.webhook') }, { status: 403 }) - } - } - if (!existingWebhook || shouldRecreateSubscription) { try { const result = await createExternalWebhookSubscription( From b076d8688f048cdb5427c1a41fad7c0283020311 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 14 Sep 2026 12:13:01 -0700 Subject: [PATCH 6/6] improvement(webhooks): validate credential id shape once before authorization --- apps/sim/app/api/webhooks/route.test.ts | 26 ++++++-------- apps/sim/app/api/webhooks/route.ts | 45 +++++++++++-------------- 2 files changed, 30 insertions(+), 41 deletions(-) diff --git a/apps/sim/app/api/webhooks/route.test.ts b/apps/sim/app/api/webhooks/route.test.ts index 879cb61f583..9399aa0415f 100644 --- a/apps/sim/app/api/webhooks/route.test.ts +++ b/apps/sim/app/api/webhooks/route.test.ts @@ -357,6 +357,9 @@ describe('POST /api/webhooks polling configuration', () => { }) }) +const CREDENTIAL_ALLOWED = { ok: true, workspaceId: 'workspace-1' } +const CREDENTIAL_DENIED = { ok: false, error: 'You do not have access to this credential.' } + /** Mocks an actor with write access to `workflow-1` and no provider side effects. */ function setupUpsertMocks(): void { vi.clearAllMocks() @@ -539,7 +542,7 @@ describe('POST /api/webhooks credential references', () => { }) it('saves a credential the actor can use in the workflow workspace', async () => { - mocks.authorizeCredentialUseForAuth.mockResolvedValue({ ok: true, workspaceId: 'workspace-1' }) + mocks.authorizeCredentialUseForAuth.mockResolvedValue(CREDENTIAL_ALLOWED) queueCreatePathRows() const response = await POST(upsertRequest({ credentialId: 'own-credential' })) @@ -578,7 +581,7 @@ describe('POST /api/webhooks credential references', () => { * authorized and kept, while a stored `userId` is never carried forward. */ it('authorizes and keeps the stored credential on a re-save that omits it', async () => { - mocks.authorizeCredentialUseForAuth.mockResolvedValue({ ok: true, workspaceId: 'workspace-1' }) + mocks.authorizeCredentialUseForAuth.mockResolvedValue(CREDENTIAL_ALLOWED) queueUpdatePathRows(true, { credentialId: 'stored-credential', userId: 'stored-user' }) const response = await POST(upsertRequest({ eventType: 'record.created' })) @@ -596,10 +599,7 @@ describe('POST /api/webhooks credential references', () => { }) it('refuses a re-save whose stored credential the actor cannot use', async () => { - mocks.authorizeCredentialUseForAuth.mockResolvedValue({ - ok: false, - error: 'You do not have access to this credential.', - }) + mocks.authorizeCredentialUseForAuth.mockResolvedValue(CREDENTIAL_DENIED) queueUpdatePathRows(true, { credentialId: 'stored-credential' }) const response = await POST(upsertRequest({ eventType: 'record.created' })) @@ -616,10 +616,7 @@ describe('POST /api/webhooks credential references', () => { it.each([null, ''])( 'still authorizes the stored credential when a re-save sends credentialId %j', async (credentialId) => { - mocks.authorizeCredentialUseForAuth.mockResolvedValue({ - ok: false, - error: 'You do not have access to this credential.', - }) + mocks.authorizeCredentialUseForAuth.mockResolvedValue(CREDENTIAL_DENIED) mocks.shouldRecreateExternalWebhookSubscription.mockReturnValue(true) queueUpdatePathRows(true, { credentialId: 'stored-credential' }) @@ -638,9 +635,7 @@ describe('POST /api/webhooks credential references', () => { /** Rotation without recreation never touches the old credential, so it needs no access to it. */ it('rotates the credential without access to the stored one when nothing is recreated', async () => { mocks.authorizeCredentialUseForAuth.mockImplementation(async (_auth, { credentialId }) => - credentialId === 'new-credential' - ? { ok: true, workspaceId: 'workspace-1' } - : { ok: false, error: 'You do not have access to this credential.' } + credentialId === 'new-credential' ? CREDENTIAL_ALLOWED : CREDENTIAL_DENIED ) queueUpdatePathRows(true, { credentialId: 'stored-credential' }) @@ -657,7 +652,7 @@ describe('POST /api/webhooks credential references', () => { /** Recreation cleans up the previous subscription with the stored credential. */ it('authorizes both credentials when a rotation recreates the subscription', async () => { - mocks.authorizeCredentialUseForAuth.mockResolvedValue({ ok: true, workspaceId: 'workspace-1' }) + mocks.authorizeCredentialUseForAuth.mockResolvedValue(CREDENTIAL_ALLOWED) mocks.shouldRecreateExternalWebhookSubscription.mockReturnValue(true) queueUpdatePathRows(true, { credentialId: 'stored-credential' }) @@ -675,7 +670,7 @@ describe('POST /api/webhooks credential references', () => { * when the request omits it, and a `userId` echoed back by the provider is not saved. */ it('authorizes the stored credential and drops userId when an omitting re-save recreates', async () => { - mocks.authorizeCredentialUseForAuth.mockResolvedValue({ ok: true, workspaceId: 'workspace-1' }) + mocks.authorizeCredentialUseForAuth.mockResolvedValue(CREDENTIAL_ALLOWED) mocks.shouldRecreateExternalWebhookSubscription.mockReturnValue(true) mocks.createExternalWebhookSubscription.mockResolvedValue({ updatedProviderConfig: { externalId: 'subscription-2', userId: 'stored-user' }, @@ -691,7 +686,6 @@ describe('POST /api/webhooks credential references', () => { workflowId: 'workflow-1', }) const savedConfig = dbChainMockFns.set.mock.calls.at(-1)?.[0].providerConfig - expect(savedConfig.userId).toBeUndefined() expect(savedConfig).toEqual({ eventType: 'record.created', externalId: 'subscription-2' }) }) diff --git a/apps/sim/app/api/webhooks/route.ts b/apps/sim/app/api/webhooks/route.ts index 050836fc1da..eb3942c7262 100644 --- a/apps/sim/app/api/webhooks/route.ts +++ b/apps/sim/app/api/webhooks/route.ts @@ -459,38 +459,33 @@ export const POST = withRouteHandler(async (request: NextRequest) => { nextConfig: resolvedProviderConfig, }) - /** The row stores the unresolved text, so only a literal credential id can be authorized. */ - if (resolvedProviderConfig.credentialId !== originalProviderConfig.credentialId) { - return NextResponse.json( - { error: 'providerConfig.credentialId must be a literal credential id' }, - { status: 400 } - ) - } - /** * Subscription handlers, pollers, and subscription cleanup look `credentialId` * up by id alone and mint tokens as its owner, so every credential this save * acts with must be usable by the actor in the workflow's workspace before - * anything is subscribed, cleaned up, or saved. That is the requested - * credential, plus the stored one when the save uses it: merged back because - * the request omits `credentialId`, or used to clean up the previous - * subscription on recreation. + * anything is subscribed, cleaned up, or saved: the requested one, and the + * stored one when it is merged back (the request omits `credentialId`) or + * cleans up the previous subscription (recreation). */ const usesStoredCredential = existingWebhook && (shouldRecreateSubscription || !('credentialId' in originalProviderConfig)) - const credentialIds = new Set( - [ - originalProviderConfig.credentialId, - usesStoredCredential ? existingWebhook.providerConfig?.credentialId : undefined, - ].filter((id) => id != null && id !== '') - ) - for (const credentialId of credentialIds) { - if (typeof credentialId !== 'string') { - return NextResponse.json( - { error: 'providerConfig.credentialId must be a literal credential id' }, - { status: 400 } - ) - } + const credentialIds = [ + originalProviderConfig.credentialId, + usesStoredCredential ? existingWebhook.providerConfig?.credentialId : undefined, + ].filter((id) => id != null && id !== '') + + /** The row stores the unresolved text, so only a literal credential id can be authorized. */ + if ( + resolvedProviderConfig.credentialId !== originalProviderConfig.credentialId || + !credentialIds.every((id): id is string => typeof id === 'string') + ) { + return NextResponse.json( + { error: 'providerConfig.credentialId must be a literal credential id' }, + { status: 400 } + ) + } + + for (const credentialId of new Set(credentialIds)) { const credentialAccess = await authorizeCredentialUseForAuth( { success: true, userId, authType: AuthType.SESSION }, { credentialId, workflowId }