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.

Author: waleedlatif1

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<string, unknown> = {}) {
+  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<string, unknown> = {}
+): 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',
+        },
+      })
+    )
+  })
+})

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<string, unknown> = 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<string, unknown>
+    const userProvided = originalProviderConfig
     const configToSave: Record<string, unknown> = { ...userProvided }
 
     if (targetWebhookId) {