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

Author: waleedlatif1

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()
+  })
 })

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(