fix(workflows): harden deployment auth and audit actor metadata

Author: PlaneInABottle

apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts

@@ -7,6 +7,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 const {
   mockActivateWorkflowVersion,
+  mockAuthorizeWorkflowByWorkspacePermission,
   mockCreateSchedulesForDeploy,
   mockDbFrom,
   mockDbLimit,
@@ -22,6 +23,7 @@ const {
   mockValidateWorkflowAccess,
 } = vi.hoisted(() => ({
   mockActivateWorkflowVersion: vi.fn(),
+  mockAuthorizeWorkflowByWorkspacePermission: vi.fn(),
   mockCreateSchedulesForDeploy: vi.fn(),
   mockDbFrom: vi.fn(),
   mockDbLimit: vi.fn(),
@@ -93,6 +95,11 @@ vi.mock('@/lib/mcp/workflow-mcp-sync', () => ({
   syncMcpToolsForWorkflow: (...args: unknown[]) => mockSyncMcpToolsForWorkflow(...args),
 }))
 
+vi.mock('@/lib/workflows/utils', () => ({
+  authorizeWorkflowByWorkspacePermission: (...args: unknown[]) =>
+    mockAuthorizeWorkflowByWorkspacePermission(...args),
+}))
+
 vi.mock('@/lib/audit/log', () => ({
   AuditAction: { WORKFLOW_DEPLOYMENT_ACTIVATED: 'WORKFLOW_DEPLOYMENT_ACTIVATED' },
   AuditResourceType: { WORKFLOW: 'WORKFLOW' },
@@ -127,6 +134,7 @@ describe('Workflow deployment version route', () => {
       success: true,
       deployedAt: '2024-01-17T12:00:00.000Z',
     })
+    mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ allowed: true, status: 200 })
   })
 
   it('uses write permission for metadata-only patch updates', async () => {
@@ -171,8 +179,10 @@ describe('Workflow deployment version route', () => {
       requireDeployment: false,
       action: 'write',
     })
-    expect(mockValidateWorkflowAccess).toHaveBeenNthCalledWith(2, req, 'wf-1', {
-      requireDeployment: false,
+    expect(mockValidateWorkflowAccess).toHaveBeenCalledTimes(1)
+    expect(mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith({
+      workflowId: 'wf-1',
+      userId: 'api-user',
       action: 'admin',
     })
     expect(mockSaveTriggerWebhooksForDeploy).toHaveBeenCalledWith(
@@ -206,14 +216,15 @@ describe('Workflow deployment version route', () => {
   })
 
   it('returns admin auth failure before activation side effects', async () => {
-    mockValidateWorkflowAccess
-      .mockResolvedValueOnce({
-        workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' },
-        auth: { success: true, userId: 'user-1', authType: 'session' },
-      })
-      .mockResolvedValueOnce({
-        error: { message: 'Admin permission required', status: 403 },
-      })
+    mockValidateWorkflowAccess.mockResolvedValue({
+      workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' },
+      auth: { success: true, userId: 'user-1', authType: 'session' },
+    })
+    mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({
+      allowed: false,
+      status: 403,
+      message: 'Admin permission required',
+    })
 
     const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deployments/3', {
       method: 'PATCH',
@@ -223,13 +234,14 @@ describe('Workflow deployment version route', () => {
     const response = await PATCH(req, { params: Promise.resolve({ id: 'wf-1', version: '3' }) })
 
     expect(response.status).toBe(403)
-    expect(mockValidateWorkflowAccess).toHaveBeenCalledTimes(2)
-    expect(mockValidateWorkflowAccess).toHaveBeenNthCalledWith(1, req, 'wf-1', {
+    expect(mockValidateWorkflowAccess).toHaveBeenCalledTimes(1)
+    expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(req, 'wf-1', {
       requireDeployment: false,
       action: 'write',
     })
-    expect(mockValidateWorkflowAccess).toHaveBeenNthCalledWith(2, req, 'wf-1', {
-      requireDeployment: false,
+    expect(mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith({
+      workflowId: 'wf-1',
+      userId: 'user-1',
       action: 'admin',
     })
     expect(mockDbSelect).not.toHaveBeenCalled()

apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts

@@ -14,6 +14,7 @@ import {
   createSchedulesForDeploy,
   validateWorkflowSchedules,
 } from '@/lib/workflows/schedules'
+import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils'
 import { validateWorkflowAccess } from '@/app/api/workflows/middleware'
 import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils'
 import type { BlockState } from '@/stores/workflows/workflow/types'
@@ -116,20 +117,24 @@ export async function PATCH(
     }
 
     const { name, description, isActive } = validation.data
+    const auth = access.auth
+    const workflowData = access.workflow
 
     if (isActive) {
-      const activationAccess = await validateWorkflowAccess(request, id, {
-        requireDeployment: false,
+      if (!auth?.userId) {
+        return createErrorResponse('Unable to determine activating user', 400)
+      }
+
+      const authorization = await authorizeWorkflowByWorkspacePermission({
+        workflowId: id,
+        userId: auth.userId,
         action: 'admin',
       })
-      if (activationAccess.error) {
-        return createErrorResponse(activationAccess.error.message, activationAccess.error.status)
+      if (!authorization.allowed) {
+        return createErrorResponse(authorization.message || 'Access denied', authorization.status)
       }
     }
 
-    const auth = access.auth
-    const workflowData = access.workflow
-
     const versionNum = Number(version)
     if (!Number.isFinite(versionNum)) {
       return createErrorResponse('Invalid version', 400)

apps/sim/app/api/workflows/[id]/route.test.ts

@@ -422,7 +422,7 @@ describe('Workflow By ID API Route', () => {
       expect(auditMock.recordAudit).toHaveBeenCalledWith(
         expect.objectContaining({
           actorId: 'api-user-1',
-          actorName: undefined,
+          actorName: 'API Key Actor',
           actorEmail: undefined,
         })
       )

apps/sim/app/api/workflows/middleware.test.ts

@@ -170,4 +170,39 @@ describe('validateWorkflowAccess', () => {
 
     expect(result).toEqual({ workflow, auth })
   })
+
+  it('returns 404 for deployed access when workflow is missing', async () => {
+    mockGetWorkflowById.mockResolvedValue(null)
+
+    const result = await validateWorkflowAccess(createRequest(), WORKFLOW_ID, {
+      requireDeployment: true,
+    })
+
+    expect(result).toEqual({
+      error: {
+        message: 'Workflow not found',
+        status: 404,
+      },
+    })
+    expect(mockCheckHybridAuth).not.toHaveBeenCalled()
+    expect(mockAuthenticateApiKeyFromHeader).not.toHaveBeenCalled()
+  })
+
+  it('returns 403 for deployed access when workflow has no workspace', async () => {
+    mockGetWorkflowById.mockResolvedValue(createWorkflow({ workspaceId: null, isDeployed: true }))
+
+    const result = await validateWorkflowAccess(createRequest(), WORKFLOW_ID, {
+      requireDeployment: true,
+    })
+
+    expect(result).toEqual({
+      error: {
+        message:
+          'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.',
+        status: 403,
+      },
+    })
+    expect(mockCheckHybridAuth).not.toHaveBeenCalled()
+    expect(mockAuthenticateApiKeyFromHeader).not.toHaveBeenCalled()
+  })
 })

apps/sim/app/api/workflows/middleware.ts

@@ -23,6 +23,30 @@ export interface WorkflowAccessOptions {
   allowInternalSecret?: boolean
 }
 
+async function getValidatedWorkflow(workflowId: string): Promise<ValidationResult> {
+  const workflow = await getWorkflowById(workflowId)
+  if (!workflow) {
+    return {
+      error: {
+        message: 'Workflow not found',
+        status: 404,
+      },
+    }
+  }
+
+  if (!workflow.workspaceId) {
+    return {
+      error: {
+        message:
+          'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.',
+        status: 403,
+      },
+    }
+  }
+
+  return { workflow }
+}
+
 export async function validateWorkflowAccess(
   request: NextRequest,
   workflowId: string,
@@ -46,25 +70,11 @@ export async function validateWorkflowAccess(
         }
       }
 
-      const workflow = await getWorkflowById(workflowId)
-      if (!workflow) {
-        return {
-          error: {
-            message: 'Workflow not found',
-            status: 404,
-          },
-        }
-      }
-
-      if (!workflow.workspaceId) {
-        return {
-          error: {
-            message:
-              'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.',
-            status: 403,
-          },
-        }
+      const workflowResult = await getValidatedWorkflow(workflowId)
+      if (workflowResult.error || !workflowResult.workflow) {
+        return workflowResult
       }
+      const workflow = workflowResult.workflow
 
       const authorization = await authorizeWorkflowByWorkspacePermission({
         workflowId,
@@ -83,25 +93,11 @@ export async function validateWorkflowAccess(
       return { workflow, auth }
     }
 
-    const workflow = await getWorkflowById(workflowId)
-    if (!workflow) {
-      return {
-        error: {
-          message: 'Workflow not found',
-          status: 404,
-        },
-      }
-    }
-
-    if (!workflow.workspaceId) {
-      return {
-        error: {
-          message:
-            'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.',
-          status: 403,
-        },
-      }
+    const workflowResult = await getValidatedWorkflow(workflowId)
+    if (workflowResult.error || !workflowResult.workflow) {
+      return workflowResult
     }
+    const workflow = workflowResult.workflow
 
     if (requireDeployment) {
       if (!workflow.isDeployed) {

apps/sim/lib/audit/actor-metadata.test.ts

@@ -0,0 +1,31 @@
+/**
+ * @vitest-environment node
+ */
+
+import { describe, expect, it } from 'vitest'
+import { getAuditActorMetadata } from '@/lib/audit/actor-metadata'
+import { AuthType } from '@/lib/auth/hybrid'
+
+describe('getAuditActorMetadata', () => {
+  it('preserves actor metadata for API key auth when present', () => {
+    expect(
+      getAuditActorMetadata({
+        success: true,
+        userId: 'api-user',
+        userName: 'API Key Actor',
+        userEmail: 'api@example.com',
+        authType: AuthType.API_KEY,
+      })
+    ).toEqual({
+      actorName: 'API Key Actor',
+      actorEmail: 'api@example.com',
+    })
+  })
+
+  it('returns undefined metadata when auth is missing', () => {
+    expect(getAuditActorMetadata(null)).toEqual({
+      actorName: undefined,
+      actorEmail: undefined,
+    })
+  })
+})

apps/sim/lib/audit/actor-metadata.ts

@@ -4,7 +4,7 @@ export function getAuditActorMetadata(auth: AuthResult | null | undefined): {
   actorName: string | undefined
   actorEmail: string | undefined
 } {
-  if (auth?.authType !== 'session') {
+  if (!auth) {
     return {
       actorName: undefined,
       actorEmail: undefined,