fix(webhooks): authorize credential references on webhook upsert (#7800)

* 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.

* fix(webhooks): authorize the stored credential and drop stored userId on re-save

* fix(webhooks): authorize both requested and stored credentials on upsert

* fix(webhooks): authorize the stored credential only when the save uses it

* fix(webhooks): check credentials after the permission-group gate

* improvement(webhooks): validate credential id shape once before authorization

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,75 @@ 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,
-    })
-  })
+const CREDENTIAL_ALLOWED = { ok: true, workspaceId: 'workspace-1' }
+const CREDENTIAL_DENIED = { ok: false, error: 'You do not have access to this credential.' }
 
-  function upsertRequest() {
-    return createMockRequest('POST', {
+/** 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' },
+  })
+  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 +454,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 +501,205 @@ 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(CREDENTIAL_ALLOWED)
+    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 `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('authorizes and keeps the stored credential on a re-save that omits it', async () => {
+    mocks.authorizeCredentialUseForAuth.mockResolvedValue(CREDENTIAL_ALLOWED)
+    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',
+    })
+    expect(dbChainMockFns.set).toHaveBeenCalledWith(
+      expect.objectContaining({
+        providerConfig: { eventType: 'record.created', credentialId: 'stored-credential' },
+      })
+    )
+  })
+
+  it('refuses a re-save whose stored credential the actor cannot use', async () => {
+    mocks.authorizeCredentialUseForAuth.mockResolvedValue(CREDENTIAL_DENIED)
+    queueUpdatePathRows(true, { credentialId: 'stored-credential' })
+
+    const response = await POST(upsertRequest({ eventType: 'record.created' }))
+
+    expect(response.status).toBe(403)
+    expect(mocks.createExternalWebhookSubscription).not.toHaveBeenCalled()
+    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(CREDENTIAL_DENIED)
+      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()
+    }
+  )
+
+  /** 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' ? CREDENTIAL_ALLOWED : CREDENTIAL_DENIED
+    )
+    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(CREDENTIAL_ALLOWED)
+    mocks.shouldRecreateExternalWebhookSubscription.mockReturnValue(true)
+    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' },
+    ])
+  })
+
+  /**
+   * 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(CREDENTIAL_ALLOWED)
+    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).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()
+  })
+})