From 251ebbe1dcda8bf70af9455a619c423466f48bb3 Mon Sep 17 00:00:00 2001 From: Yusuf Mirza Altay Date: Thu, 12 Mar 2026 02:29:03 +0300 Subject: [PATCH 1/8] fix(workflows): tighten shared workflow access validation --- .../api/workflows/[id]/deployed/route.test.ts | 62 +++++++++++++++ .../app/api/workflows/[id]/deployed/route.ts | 12 ++- .../api/workflows/[id]/status/route.test.ts | 78 +++++++++++++++++++ .../app/api/workflows/[id]/status/route.ts | 5 +- apps/sim/app/api/workflows/middleware.ts | 18 ++++- 5 files changed, 167 insertions(+), 8 deletions(-) create mode 100644 apps/sim/app/api/workflows/[id]/deployed/route.test.ts create mode 100644 apps/sim/app/api/workflows/[id]/status/route.test.ts diff --git a/apps/sim/app/api/workflows/[id]/deployed/route.test.ts b/apps/sim/app/api/workflows/[id]/deployed/route.test.ts new file mode 100644 index 00000000000..5d346a442b3 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/deployed/route.test.ts @@ -0,0 +1,62 @@ +/** + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mockValidateWorkflowAccess = vi.fn() +const mockVerifyInternalToken = vi.fn() +const mockLoadDeployedWorkflowState = vi.fn() + +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ warn: vi.fn(), error: vi.fn() }), +})) + +vi.mock('@/app/api/workflows/middleware', () => ({ + validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args), +})) + +vi.mock('@/lib/auth/internal', () => ({ + verifyInternalToken: (...args: unknown[]) => mockVerifyInternalToken(...args), +})) + +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: () => 'req-123', +})) + +vi.mock('@/lib/workflows/persistence/utils', () => ({ + loadDeployedWorkflowState: (...args: unknown[]) => mockLoadDeployedWorkflowState(...args), +})) + +import { GET } from '@/app/api/workflows/[id]/deployed/route' + +describe('Workflow deployed-state route', () => { + beforeEach(() => { + vi.clearAllMocks() + mockVerifyInternalToken.mockResolvedValue({ valid: false }) + mockLoadDeployedWorkflowState.mockResolvedValue({ + blocks: {}, + edges: [], + loops: {}, + parallels: {}, + variables: [], + }) + }) + + it('uses hybrid workflow access when request is not internal bearer auth', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ workflow: { id: 'wf-1' } }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deployed', { + headers: { 'x-api-key': 'test-key' }, + }) + const response = await GET(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(200) + expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(req, 'wf-1', { + requireDeployment: false, + action: 'read', + allowInternalSecret: false, + }) + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/deployed/route.ts b/apps/sim/app/api/workflows/[id]/deployed/route.ts index 347e77eacb9..28e4b1d1d24 100644 --- a/apps/sim/app/api/workflows/[id]/deployed/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployed/route.ts @@ -3,7 +3,7 @@ import type { NextRequest, NextResponse } from 'next/server' import { verifyInternalToken } from '@/lib/auth/internal' import { generateRequestId } from '@/lib/core/utils/request' import { loadDeployedWorkflowState } from '@/lib/workflows/persistence/utils' -import { validateWorkflowPermissions } from '@/lib/workflows/utils' +import { validateWorkflowAccess } from '@/app/api/workflows/middleware' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' const logger = createLogger('WorkflowDeployedStateAPI') @@ -31,9 +31,13 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ } if (!isInternalCall) { - const { error } = await validateWorkflowPermissions(id, requestId, 'read') - if (error) { - const response = createErrorResponse(error.message, error.status) + const validation = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'read', + allowInternalSecret: false, + }) + if (validation.error) { + const response = createErrorResponse(validation.error.message, validation.error.status) return addNoCacheHeaders(response) } } diff --git a/apps/sim/app/api/workflows/[id]/status/route.test.ts b/apps/sim/app/api/workflows/[id]/status/route.test.ts new file mode 100644 index 00000000000..2b22b73d368 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/status/route.test.ts @@ -0,0 +1,78 @@ +/** + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mockValidateWorkflowAccess = vi.fn() +const mockLoadWorkflowFromNormalizedTables = vi.fn() +const mockHasWorkflowChanged = vi.fn() +const mockDbSelect = vi.fn() +const mockDbFrom = vi.fn() +const mockDbWhere = vi.fn() +const mockDbLimit = vi.fn() +const mockDbOrderBy = vi.fn() + +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), +})) + +vi.mock('@/app/api/workflows/middleware', () => ({ + validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args), +})) + +vi.mock('@/lib/workflows/persistence/utils', () => ({ + loadWorkflowFromNormalizedTables: (...args: unknown[]) => mockLoadWorkflowFromNormalizedTables(...args), +})) + +vi.mock('@/lib/workflows/comparison', () => ({ + hasWorkflowChanged: (...args: unknown[]) => mockHasWorkflowChanged(...args), +})) + +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: () => 'req-123', +})) + +vi.mock('@sim/db', () => ({ + db: { + select: mockDbSelect, + }, + workflow: { variables: 'variables', id: 'id' }, + workflowDeploymentVersion: { state: 'state', workflowId: 'workflowId', isActive: 'isActive', createdAt: 'createdAt' }, +})) + +vi.mock('drizzle-orm', () => ({ + and: vi.fn(), + desc: vi.fn(), + eq: vi.fn(), +})) + +import { GET } from '@/app/api/workflows/[id]/status/route' + +describe('Workflow status route', () => { + beforeEach(() => { + vi.clearAllMocks() + mockDbSelect.mockReturnValue({ from: mockDbFrom }) + mockDbFrom.mockReturnValue({ where: mockDbWhere }) + mockDbWhere.mockReturnValue({ limit: mockDbLimit, orderBy: mockDbOrderBy }) + mockDbOrderBy.mockReturnValue({ limit: mockDbLimit }) + }) + + it('uses hybrid workflow access for read auth', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'wf-1', isDeployed: false, deployedAt: null, isPublished: false }, + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/status', { + headers: { 'x-api-key': 'test-key' }, + }) + const response = await GET(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(200) + expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(req, 'wf-1', { + requireDeployment: false, + action: 'read', + }) + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/status/route.ts b/apps/sim/app/api/workflows/[id]/status/route.ts index f53fbe05e4d..7bc8eb059fd 100644 --- a/apps/sim/app/api/workflows/[id]/status/route.ts +++ b/apps/sim/app/api/workflows/[id]/status/route.ts @@ -17,7 +17,10 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ try { const { id } = await params - const validation = await validateWorkflowAccess(request, id, false) + const validation = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'read', + }) if (validation.error) { logger.warn(`[${requestId}] Workflow access validation failed: ${validation.error.message}`) return createErrorResponse(validation.error.message, validation.error.status) diff --git a/apps/sim/app/api/workflows/middleware.ts b/apps/sim/app/api/workflows/middleware.ts index d6260002fe3..f6e6fbdfb69 100644 --- a/apps/sim/app/api/workflows/middleware.ts +++ b/apps/sim/app/api/workflows/middleware.ts @@ -17,12 +17,24 @@ export interface ValidationResult { auth?: AuthResult } +export interface WorkflowAccessOptions { + requireDeployment?: boolean + action?: 'read' | 'write' | 'admin' + allowInternalSecret?: boolean +} + export async function validateWorkflowAccess( request: NextRequest, workflowId: string, - requireDeployment = true + options: boolean | WorkflowAccessOptions = true ): Promise { try { + const normalizedOptions: WorkflowAccessOptions = + typeof options === 'boolean' ? { requireDeployment: options } : options + const requireDeployment = normalizedOptions.requireDeployment ?? true + const action = normalizedOptions.action ?? 'read' + const allowInternalSecret = normalizedOptions.allowInternalSecret ?? requireDeployment + const workflow = await getWorkflowById(workflowId) if (!workflow) { return { @@ -57,7 +69,7 @@ export async function validateWorkflowAccess( const authorization = await authorizeWorkflowByWorkspacePermission({ workflowId, userId: auth.userId, - action: 'read', + action, }) if (!authorization.allowed) { return { @@ -82,7 +94,7 @@ export async function validateWorkflowAccess( } const internalSecret = request.headers.get('X-Internal-Secret') - if (env.INTERNAL_API_SECRET && internalSecret === env.INTERNAL_API_SECRET) { + if (allowInternalSecret && env.INTERNAL_API_SECRET && internalSecret === env.INTERNAL_API_SECRET) { return { workflow } } From 5656f745675872016d94fe492bb9badb40542815 Mon Sep 17 00:00:00 2001 From: Yusuf Mirza Altay Date: Thu, 12 Mar 2026 02:29:23 +0300 Subject: [PATCH 2/8] fix(workflows): allow app auth on workflow lifecycle actions --- .../api/workflows/[id]/deploy/route.test.ts | 139 ++++++++++++++++++ .../app/api/workflows/[id]/deploy/route.ts | 59 ++++++-- apps/sim/app/api/workflows/[id]/route.test.ts | 52 +++++++ apps/sim/app/api/workflows/[id]/route.ts | 23 ++- 4 files changed, 256 insertions(+), 17 deletions(-) create mode 100644 apps/sim/app/api/workflows/[id]/deploy/route.test.ts diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.test.ts b/apps/sim/app/api/workflows/[id]/deploy/route.test.ts new file mode 100644 index 00000000000..cec634e0f74 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/deploy/route.test.ts @@ -0,0 +1,139 @@ +/** + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mockValidateWorkflowAccess = vi.fn() +const mockDbSelect = vi.fn() +const mockDbFrom = vi.fn() +const mockDbWhere = vi.fn() +const mockDbLimit = vi.fn() +const mockDbOrderBy = vi.fn() +const mockDeployWorkflow = vi.fn() +const mockUndeployWorkflow = vi.fn() +const mockCleanupWebhooksForWorkflow = vi.fn() +const mockRemoveMcpToolsForWorkflow = vi.fn() + +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), +})) + +vi.mock('@/lib/workflows/utils', () => ({ + validateWorkflowPermissions: vi.fn(), +})) + +vi.mock('@/app/api/workflows/middleware', () => ({ + validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args), +})) + +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: () => 'req-123', +})) + +vi.mock('@sim/db', () => ({ + db: { select: mockDbSelect }, + workflow: { variables: 'variables', id: 'id' }, + workflowDeploymentVersion: { state: 'state', workflowId: 'workflowId', isActive: 'isActive', createdAt: 'createdAt', id: 'id' }, +})) + +vi.mock('drizzle-orm', () => ({ + and: vi.fn(), + desc: vi.fn(), + eq: vi.fn(), +})) + +vi.mock('@/lib/workflows/persistence/utils', () => ({ + loadWorkflowFromNormalizedTables: vi.fn().mockResolvedValue(null), + deployWorkflow: (...args: unknown[]) => mockDeployWorkflow(...args), + undeployWorkflow: (...args: unknown[]) => mockUndeployWorkflow(...args), +})) + +vi.mock('@/lib/workflows/comparison', () => ({ + hasWorkflowChanged: vi.fn().mockReturnValue(false), +})) + +vi.mock('@/lib/workflows/schedules', () => ({ + cleanupDeploymentVersion: vi.fn(), + createSchedulesForDeploy: vi.fn(), + validateWorkflowSchedules: vi.fn().mockReturnValue({ isValid: true }), +})) + +vi.mock('@/lib/webhooks/deploy', () => ({ + cleanupWebhooksForWorkflow: (...args: unknown[]) => mockCleanupWebhooksForWorkflow(...args), + restorePreviousVersionWebhooks: vi.fn(), + saveTriggerWebhooksForDeploy: vi.fn(), +})) + +vi.mock('@/lib/mcp/workflow-mcp-sync', () => ({ + removeMcpToolsForWorkflow: (...args: unknown[]) => mockRemoveMcpToolsForWorkflow(...args), + syncMcpToolsForWorkflow: vi.fn(), +})) + +vi.mock('@/lib/audit/log', () => ({ + AuditAction: {}, + AuditResourceType: {}, + recordAudit: vi.fn(), +})) + +import { POST, DELETE } from '@/app/api/workflows/[id]/deploy/route' + +describe('Workflow deploy route', () => { + beforeEach(() => { + vi.clearAllMocks() + mockDbSelect.mockReturnValue({ from: mockDbFrom }) + mockDbFrom.mockReturnValue({ where: mockDbWhere }) + mockDbWhere.mockReturnValue({ limit: mockDbLimit, orderBy: mockDbOrderBy }) + mockDbOrderBy.mockReturnValue({ limit: mockDbLimit }) + mockDbLimit.mockResolvedValue([]) + mockCleanupWebhooksForWorkflow.mockResolvedValue(undefined) + mockRemoveMcpToolsForWorkflow.mockResolvedValue(undefined) + }) + + it('allows API-key auth for deploy using hybrid auth userId', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' }, + auth: { success: true, userId: 'api-user', authType: 'api_key' }, + }) + mockDeployWorkflow.mockResolvedValue({ + success: true, + deployedAt: '2024-01-01T00:00:00Z', + deploymentVersionId: 'dep-1', + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deploy', { + method: 'POST', + headers: { 'x-api-key': 'test-key' }, + }) + const response = await POST(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.isDeployed).toBe(true) + expect(mockDeployWorkflow).toHaveBeenCalledWith({ + workflowId: 'wf-1', + deployedBy: 'api-user', + workflowName: 'Test Workflow', + }) + }) + + it('allows API-key auth for undeploy using hybrid auth userId', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' }, + auth: { success: true, userId: 'api-user', authType: 'api_key' }, + }) + mockUndeployWorkflow.mockResolvedValue({ success: true }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deploy', { + method: 'DELETE', + headers: { 'x-api-key': 'test-key' }, + }) + const response = await DELETE(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.isDeployed).toBe(false) + expect(mockUndeployWorkflow).toHaveBeenCalledWith({ workflowId: 'wf-1' }) + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.ts b/apps/sim/app/api/workflows/[id]/deploy/route.ts index 1dd8798a3f2..cbaf5c4a868 100644 --- a/apps/sim/app/api/workflows/[id]/deploy/route.ts +++ b/apps/sim/app/api/workflows/[id]/deploy/route.ts @@ -21,6 +21,7 @@ import { validateWorkflowSchedules, } from '@/lib/workflows/schedules' import { validateWorkflowPermissions } from '@/lib/workflows/utils' +import { validateWorkflowAccess } from '@/app/api/workflows/middleware' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' import type { WorkflowState } from '@/stores/workflows/workflow/types' @@ -29,20 +30,47 @@ const logger = createLogger('WorkflowDeployAPI') export const dynamic = 'force-dynamic' export const runtime = 'nodejs' +async function validateLifecycleAdminAccess( + request: NextRequest, + workflowId: string, + requestId: string +) { + const hybridAccess = await validateWorkflowAccess(request, workflowId, { + requireDeployment: false, + action: 'admin', + }) + + if (hybridAccess.error) { + return hybridAccess + } + + if (hybridAccess.auth?.authType === 'session') { + return await validateWorkflowPermissions(workflowId, requestId, 'admin') + } + + return { + error: null, + auth: hybridAccess.auth, + session: null, + workflow: hybridAccess.workflow, + } +} + export async function GET(request: NextRequest, { params }: { params: Promise<{ id: string }> }) { const requestId = generateRequestId() const { id } = await params try { - const { error, workflow: workflowData } = await validateWorkflowPermissions( - id, - requestId, - 'read' - ) - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'read', + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) } + const workflowData = access.workflow + if (!workflowData.isDeployed) { logger.info(`[${requestId}] Workflow is not deployed: ${id}`) return createSuccessResponse({ @@ -115,15 +143,16 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ try { const { + auth, error, session, workflow: workflowData, - } = await validateWorkflowPermissions(id, requestId, 'admin') + } = await validateLifecycleAdminAccess(request, id, requestId) if (error) { return createErrorResponse(error.message, error.status) } - const actorUserId: string | null = session?.user?.id ?? null + const actorUserId: string | null = session?.user?.id ?? auth?.userId ?? null if (!actorUserId) { logger.warn(`[${requestId}] Unable to resolve actor user for workflow deployment: ${id}`) return createErrorResponse('Unable to determine deploying user', 400) @@ -309,7 +338,7 @@ export async function PATCH(request: NextRequest, { params }: { params: Promise< const { id } = await params try { - const { error, session } = await validateWorkflowPermissions(id, requestId, 'admin') + const { error, session } = await validateLifecycleAdminAccess(request, id, requestId) if (error) { return createErrorResponse(error.message, error.status) } @@ -356,14 +385,20 @@ export async function DELETE( try { const { + auth, error, session, workflow: workflowData, - } = await validateWorkflowPermissions(id, requestId, 'admin') + } = await validateLifecycleAdminAccess(request, id, requestId) if (error) { return createErrorResponse(error.message, error.status) } + const actorUserId = session?.user?.id ?? auth?.userId ?? null + if (!actorUserId) { + return createErrorResponse('Unable to determine undeploying user', 400) + } + // Clean up external webhook subscriptions before undeploying await cleanupWebhooksForWorkflow(id, workflowData as Record, requestId) @@ -385,7 +420,7 @@ export async function DELETE( recordAudit({ workspaceId: workflowData?.workspaceId || null, - actorId: session!.user.id, + actorId: actorUserId, actorName: session?.user?.name, actorEmail: session?.user?.email, action: AuditAction.WORKFLOW_UNDEPLOYED, diff --git a/apps/sim/app/api/workflows/[id]/route.test.ts b/apps/sim/app/api/workflows/[id]/route.test.ts index d886e27d466..91d2b3f7a62 100644 --- a/apps/sim/app/api/workflows/[id]/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/route.test.ts @@ -24,6 +24,7 @@ const mockAuthorizeWorkflowByWorkspacePermission = vi.fn() const mockDbDelete = vi.fn() const mockDbUpdate = vi.fn() const mockDbSelect = vi.fn() +const mockValidateWorkflowAccess = vi.fn() /** * Helper to set mock auth state consistently across getSession and hybrid auth. @@ -32,9 +33,16 @@ function mockGetSession(session: { user: { id: string } } | null) { if (session) { mockCheckHybridAuth.mockResolvedValue({ success: true, userId: session.user.id }) mockCheckSessionOrInternalAuth.mockResolvedValue({ success: true, userId: session.user.id }) + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'workflow-123', workspaceId: 'workspace-456' }, + auth: { success: true, userId: session.user.id, authType: 'session' }, + }) } else { mockCheckHybridAuth.mockResolvedValue({ success: false }) mockCheckSessionOrInternalAuth.mockResolvedValue({ success: false }) + mockValidateWorkflowAccess.mockResolvedValue({ + error: { message: 'Unauthorized', status: 401 }, + }) } } @@ -63,6 +71,10 @@ vi.mock('@/lib/workflows/persistence/utils', () => ({ mockLoadWorkflowFromNormalizedTables(workflowId), })) +vi.mock('@/app/api/workflows/middleware', () => ({ + validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args), +})) + vi.mock('@/lib/workflows/utils', () => ({ getWorkflowById: (workflowId: string) => mockGetWorkflowById(workflowId), authorizeWorkflowByWorkspacePermission: (params: { @@ -358,6 +370,46 @@ describe('Workflow By ID API Route', () => { expect(data.success).toBe(true) }) + it('should allow API-key-backed deletion when workflow access is validated', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'other-user', + name: 'Test Workflow', + workspaceId: 'workspace-456', + } + + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: mockWorkflow, + auth: { success: true, userId: 'api-user-1', authType: 'api_key' }, + }) + mockGetWorkflowById.mockResolvedValue(mockWorkflow) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, + workflow: mockWorkflow, + workspacePermission: 'admin', + }) + mockDbSelect.mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockResolvedValue([{ id: 'workflow-123' }, { id: 'workflow-456' }]), + }), + }) + mockDbDelete.mockReturnValue({ + where: vi.fn().mockResolvedValue([{ id: 'workflow-123' }]), + }) + setupGlobalFetchMock({ ok: true }) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { + method: 'DELETE', + headers: { 'x-api-key': 'test-key' }, + }) + const params = Promise.resolve({ id: 'workflow-123' }) + + const response = await DELETE(req, { params }) + + expect(response.status).toBe(200) + }) + it('should prevent deletion of the last workflow in workspace', async () => { const mockWorkflow = { id: 'workflow-123', diff --git a/apps/sim/app/api/workflows/[id]/route.ts b/apps/sim/app/api/workflows/[id]/route.ts index 19d89e8eeb7..0bbe4da02a4 100644 --- a/apps/sim/app/api/workflows/[id]/route.ts +++ b/apps/sim/app/api/workflows/[id]/route.ts @@ -5,12 +5,13 @@ import { and, eq, isNull, ne } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log' -import { AuthType, checkHybridAuth, checkSessionOrInternalAuth } from '@/lib/auth/hybrid' +import { AuthType, checkHybridAuth } from '@/lib/auth/hybrid' import { env } from '@/lib/core/config/env' import { PlatformEvents } from '@/lib/core/telemetry' import { generateRequestId } from '@/lib/core/utils/request' import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/persistence/utils' import { authorizeWorkflowByWorkspacePermission, getWorkflowById } from '@/lib/workflows/utils' +import { validateWorkflowAccess } from '@/app/api/workflows/middleware' const logger = createLogger('WorkflowByIdAPI') @@ -146,13 +147,25 @@ export async function DELETE( const { id: workflowId } = await params try { - const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) - if (!auth.success || !auth.userId) { + const validation = await validateWorkflowAccess(request, workflowId, { + requireDeployment: false, + action: 'admin', + }) + if (validation.error) { logger.warn(`[${requestId}] Unauthorized deletion attempt for workflow ${workflowId}`) - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + return NextResponse.json({ error: validation.error.message }, { status: validation.error.status }) } - const userId = auth.userId + const auth = validation.auth + const userId = auth?.userId + + if (!userId) { + logger.warn(`[${requestId}] Missing user identity for workflow deletion ${workflowId}`) + return NextResponse.json( + { error: 'Workflow deletion requires a user-backed session or API key identity' }, + { status: 400 } + ) + } const authorization = await authorizeWorkflowByWorkspacePermission({ workflowId, From 7465b6fb680460989e6ecd5de3aaa81e901da9d8 Mon Sep 17 00:00:00 2001 From: Yusuf Mirza Altay Date: Thu, 12 Mar 2026 14:41:21 +0300 Subject: [PATCH 3/8] fix(workflows): support app auth on deployment routes --- .../[version]/revert/route.test.ts | 119 +++++++++++++++++ .../deployments/[version]/revert/route.ts | 35 ++++- .../[id]/deployments/[version]/route.test.ts | 124 ++++++++++++++++++ .../[id]/deployments/[version]/route.ts | 39 ++++-- .../workflows/[id]/deployments/route.test.ts | 84 ++++++++++++ .../api/workflows/[id]/deployments/route.ts | 11 +- 6 files changed, 393 insertions(+), 19 deletions(-) create mode 100644 apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts create mode 100644 apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts create mode 100644 apps/sim/app/api/workflows/[id]/deployments/route.test.ts diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts new file mode 100644 index 00000000000..e2f74d5cb3b --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts @@ -0,0 +1,119 @@ +/** + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mockValidateWorkflowAccess = vi.fn() +const mockDbSelect = vi.fn() +const mockDbFrom = vi.fn() +const mockDbWhere = vi.fn() +const mockDbLimit = vi.fn() +const mockDbUpdate = vi.fn() +const mockDbSet = vi.fn() +const mockDbWhereUpdate = vi.fn() +const mockSaveWorkflowToNormalizedTables = vi.fn() +const mockSyncMcpToolsForWorkflow = vi.fn() +const mockFetch = vi.fn() + +vi.stubGlobal('fetch', mockFetch) + +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), +})) + +vi.mock('@/app/api/workflows/middleware', () => ({ + validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args), +})) + +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: () => 'req-123', +})) + +vi.mock('@/lib/core/config/env', () => ({ + env: { INTERNAL_API_SECRET: 'internal-secret', SOCKET_SERVER_URL: 'http://localhost:3002' }, +})) + +vi.mock('@sim/db', () => ({ + db: { + select: mockDbSelect, + update: mockDbUpdate, + }, + workflow: { id: 'id' }, + workflowDeploymentVersion: { + state: 'state', + workflowId: 'workflowId', + isActive: 'isActive', + version: 'version', + }, +})) + +vi.mock('drizzle-orm', () => ({ + and: vi.fn(), + eq: vi.fn(), +})) + +vi.mock('@/lib/workflows/persistence/utils', () => ({ + saveWorkflowToNormalizedTables: (...args: unknown[]) => mockSaveWorkflowToNormalizedTables(...args), +})) + +vi.mock('@/lib/mcp/workflow-mcp-sync', () => ({ + syncMcpToolsForWorkflow: (...args: unknown[]) => mockSyncMcpToolsForWorkflow(...args), +})) + +vi.mock('@/lib/audit/log', () => ({ + AuditAction: { WORKFLOW_DEPLOYMENT_REVERTED: 'WORKFLOW_DEPLOYMENT_REVERTED' }, + AuditResourceType: { WORKFLOW: 'WORKFLOW' }, + recordAudit: vi.fn(), +})) + +import { POST } from '@/app/api/workflows/[id]/deployments/[version]/revert/route' + +describe('Workflow deployment version revert route', () => { + beforeEach(() => { + vi.clearAllMocks() + mockDbSelect.mockReturnValue({ from: mockDbFrom }) + mockDbFrom.mockReturnValue({ where: mockDbWhere }) + mockDbWhere.mockReturnValue({ limit: mockDbLimit }) + mockDbLimit.mockResolvedValue([ + { + state: { + blocks: { 'block-1': { id: 'block-1', type: 'start_trigger', name: 'Start' } }, + edges: [], + loops: {}, + parallels: {}, + }, + }, + ]) + mockDbUpdate.mockReturnValue({ set: mockDbSet }) + mockDbSet.mockReturnValue({ where: mockDbWhereUpdate }) + mockDbWhereUpdate.mockResolvedValue(undefined) + mockSaveWorkflowToNormalizedTables.mockResolvedValue({ success: true }) + mockFetch.mockResolvedValue({ ok: true }) + }) + + it('allows API-key auth for revert using hybrid auth userId', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' }, + auth: { success: true, userId: 'api-user', authType: 'api_key' }, + }) + + const req = new NextRequest( + 'http://localhost:3000/api/workflows/wf-1/deployments/3/revert', + { + method: 'POST', + headers: { 'x-api-key': 'test-key' }, + } + ) + const response = await POST(req, { params: Promise.resolve({ id: 'wf-1', version: '3' }) }) + + expect(response.status).toBe(200) + expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(req, 'wf-1', { + requireDeployment: false, + action: 'admin', + }) + expect(mockSaveWorkflowToNormalizedTables).toHaveBeenCalled() + expect(mockSyncMcpToolsForWorkflow).toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts index 6050bb4b253..74925c069cd 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts @@ -7,11 +7,28 @@ import { env } from '@/lib/core/config/env' import { generateRequestId } from '@/lib/core/utils/request' import { syncMcpToolsForWorkflow } from '@/lib/mcp/workflow-mcp-sync' import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils' -import { validateWorkflowPermissions } from '@/lib/workflows/utils' +import { validateWorkflowAccess } from '@/app/api/workflows/middleware' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' const logger = createLogger('RevertToDeploymentVersionAPI') +async function validateDeploymentVersionAdminAccess(request: NextRequest, workflowId: string) { + const access = await validateWorkflowAccess(request, workflowId, { + requireDeployment: false, + action: 'admin', + }) + + if (access.error) { + return access + } + + return { + error: null, + auth: access.auth, + workflow: access.workflow, + } +} + export const dynamic = 'force-dynamic' export const runtime = 'nodejs' @@ -24,14 +41,20 @@ export async function POST( try { const { + auth, error, - session, workflow: workflowRecord, - } = await validateWorkflowPermissions(id, requestId, 'admin') + } = await validateDeploymentVersionAdminAccess(request, id) if (error) { return createErrorResponse(error.message, error.status) } + const actorUserId = auth?.userId + if (!actorUserId) { + logger.warn(`[${requestId}] Unable to resolve actor user for workflow deployment revert: ${id}`) + return createErrorResponse('Unable to determine reverting user', 400) + } + const versionSelector = version === 'active' ? null : Number(version) if (version !== 'active' && !Number.isFinite(versionSelector)) { return createErrorResponse('Invalid version', 400) @@ -114,12 +137,12 @@ export async function POST( recordAudit({ workspaceId: workflowRecord?.workspaceId ?? null, - actorId: session!.user.id, + actorId: actorUserId, action: AuditAction.WORKFLOW_DEPLOYMENT_REVERTED, resourceType: AuditResourceType.WORKFLOW, resourceId: id, - actorName: session!.user.name ?? undefined, - actorEmail: session!.user.email ?? undefined, + actorName: auth?.userName ?? undefined, + actorEmail: auth?.userEmail ?? undefined, resourceName: workflowRecord?.name ?? undefined, description: `Reverted workflow to deployment version ${version}`, request, diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts new file mode 100644 index 00000000000..4d2045b9530 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts @@ -0,0 +1,124 @@ +/** + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mockValidateWorkflowAccess = vi.fn() +const mockDbSelect = vi.fn() +const mockDbFrom = vi.fn() +const mockDbWhere = vi.fn() +const mockDbLimit = vi.fn() +const mockSaveTriggerWebhooksForDeploy = vi.fn() +const mockCreateSchedulesForDeploy = vi.fn() +const mockActivateWorkflowVersion = vi.fn() +const mockSyncMcpToolsForWorkflow = vi.fn() + +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), +})) + +vi.mock('@/app/api/workflows/middleware', () => ({ + validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args), +})) + +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: () => 'req-123', +})) + +vi.mock('@sim/db', () => ({ + db: { + select: mockDbSelect, + update: vi.fn(() => ({ set: vi.fn(() => ({ where: vi.fn(() => ({ returning: vi.fn() })) })) }), + }, + workflowDeploymentVersion: { + id: 'id', + state: 'state', + workflowId: 'workflowId', + version: 'version', + isActive: 'isActive', + name: 'name', + description: 'description', + }, +})) + +vi.mock('drizzle-orm', () => ({ + and: vi.fn(), + eq: vi.fn(), +})) + +vi.mock('@/lib/webhooks/deploy', () => ({ + restorePreviousVersionWebhooks: vi.fn(), + saveTriggerWebhooksForDeploy: (...args: unknown[]) => mockSaveTriggerWebhooksForDeploy(...args), +})) + +vi.mock('@/lib/workflows/persistence/utils', () => ({ + activateWorkflowVersion: (...args: unknown[]) => mockActivateWorkflowVersion(...args), +})) + +vi.mock('@/lib/workflows/schedules', () => ({ + cleanupDeploymentVersion: vi.fn(), + createSchedulesForDeploy: (...args: unknown[]) => mockCreateSchedulesForDeploy(...args), + validateWorkflowSchedules: vi.fn(() => ({ isValid: true })), +})) + +vi.mock('@/lib/mcp/workflow-mcp-sync', () => ({ + syncMcpToolsForWorkflow: (...args: unknown[]) => mockSyncMcpToolsForWorkflow(...args), +})) + +vi.mock('@/lib/audit/log', () => ({ + AuditAction: { WORKFLOW_DEPLOYMENT_ACTIVATED: 'WORKFLOW_DEPLOYMENT_ACTIVATED' }, + AuditResourceType: { WORKFLOW: 'WORKFLOW' }, + recordAudit: vi.fn(), +})) + +import { PATCH } from '@/app/api/workflows/[id]/deployments/[version]/route' + +describe('Workflow deployment version route', () => { + beforeEach(() => { + vi.clearAllMocks() + mockDbSelect.mockReturnValue({ from: mockDbFrom }) + mockDbFrom.mockReturnValue({ where: mockDbWhere }) + mockDbWhere.mockReturnValue({ limit: mockDbLimit }) + mockDbLimit + .mockResolvedValueOnce([ + { + id: 'dep-3', + state: { + blocks: { 'block-1': { id: 'block-1', type: 'start_trigger', name: 'Start' } }, + }, + }, + ]) + .mockResolvedValueOnce([{ id: 'dep-2' }]) + mockSaveTriggerWebhooksForDeploy.mockResolvedValue({ success: true, warnings: [] }) + mockCreateSchedulesForDeploy.mockResolvedValue({ success: true }) + mockActivateWorkflowVersion.mockResolvedValue({ + success: true, + deployedAt: '2024-01-17T12:00:00.000Z', + }) + }) + + it('allows API-key auth for activation using hybrid auth userId', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' }, + auth: { success: true, userId: 'api-user', authType: 'api_key' }, + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deployments/3', { + method: 'PATCH', + headers: { 'content-type': 'application/json', 'x-api-key': 'test-key' }, + body: JSON.stringify({ isActive: true }), + }) + const response = await PATCH(req, { params: Promise.resolve({ id: 'wf-1', version: '3' }) }) + + expect(response.status).toBe(200) + expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(req, 'wf-1', { + requireDeployment: false, + action: 'admin', + }) + expect(mockSaveTriggerWebhooksForDeploy).toHaveBeenCalledWith( + expect.objectContaining({ userId: 'api-user' }) + ) + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts index 56802840e95..c9ec9bd2d81 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts @@ -13,12 +13,33 @@ import { createSchedulesForDeploy, validateWorkflowSchedules, } from '@/lib/workflows/schedules' -import { validateWorkflowPermissions } 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' const logger = createLogger('WorkflowDeploymentVersionAPI') +async function validateDeploymentVersionLifecycleAccess( + request: NextRequest, + workflowId: string, + action: 'read' | 'write' | 'admin' +) { + const access = await validateWorkflowAccess(request, workflowId, { + requireDeployment: false, + action, + }) + + if (access.error) { + return access + } + + return { + error: null, + auth: access.auth, + workflow: access.workflow, + } +} + const patchBodySchema = z .object({ name: z @@ -53,9 +74,9 @@ export async function GET( const { id, version } = await params try { - const { error } = await validateWorkflowPermissions(id, requestId, 'read') - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateDeploymentVersionLifecycleAccess(request, id, 'read') + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) } const versionNum = Number(version) @@ -108,10 +129,10 @@ export async function PATCH( // Activation requires admin permission, other updates require write const requiredPermission = isActive ? 'admin' : 'write' const { + auth, error, - session, workflow: workflowData, - } = await validateWorkflowPermissions(id, requestId, requiredPermission) + } = await validateDeploymentVersionLifecycleAccess(request, id, requiredPermission) if (error) { return createErrorResponse(error.message, error.status) } @@ -123,7 +144,7 @@ export async function PATCH( // Handle activation if (isActive) { - const actorUserId = session?.user?.id + const actorUserId = auth?.userId if (!actorUserId) { logger.warn(`[${requestId}] Unable to resolve actor user for deployment activation: ${id}`) return createErrorResponse('Unable to determine activating user', 400) @@ -301,8 +322,8 @@ export async function PATCH( recordAudit({ workspaceId: workflowData?.workspaceId, actorId: actorUserId, - actorName: session?.user?.name, - actorEmail: session?.user?.email, + actorName: auth?.userName, + actorEmail: auth?.userEmail, action: AuditAction.WORKFLOW_DEPLOYMENT_ACTIVATED, resourceType: AuditResourceType.WORKFLOW, resourceId: id, diff --git a/apps/sim/app/api/workflows/[id]/deployments/route.test.ts b/apps/sim/app/api/workflows/[id]/deployments/route.test.ts new file mode 100644 index 00000000000..d1d860c5238 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/deployments/route.test.ts @@ -0,0 +1,84 @@ +/** + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mockValidateWorkflowAccess = vi.fn() +const mockDbSelect = vi.fn() +const mockDbFrom = vi.fn() +const mockDbLeftJoin = vi.fn() +const mockDbWhere = vi.fn() +const mockDbOrderBy = vi.fn() + +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), +})) + +vi.mock('@/app/api/workflows/middleware', () => ({ + validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args), +})) + +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: () => 'req-123', +})) + +vi.mock('@sim/db', () => ({ + db: { select: mockDbSelect }, + user: { name: 'name', id: 'id' }, + workflowDeploymentVersion: { + id: 'id', + version: 'version', + name: 'name', + description: 'description', + isActive: 'isActive', + createdAt: 'createdAt', + createdBy: 'createdBy', + workflowId: 'workflowId', + }, +})) + +vi.mock('drizzle-orm', () => ({ + desc: vi.fn(), + eq: vi.fn(), +})) + +import { GET } from '@/app/api/workflows/[id]/deployments/route' + +describe('Workflow deployments list route', () => { + beforeEach(() => { + vi.clearAllMocks() + mockDbSelect.mockReturnValue({ from: mockDbFrom }) + mockDbFrom.mockReturnValue({ leftJoin: mockDbLeftJoin }) + mockDbLeftJoin.mockReturnValue({ where: mockDbWhere }) + mockDbWhere.mockReturnValue({ orderBy: mockDbOrderBy }) + mockDbOrderBy.mockResolvedValue([ + { + id: 'dep-1', + version: 3, + name: 'Current active deployment', + description: 'Latest deployed state', + isActive: true, + createdAt: '2024-01-16T12:00:00.000Z', + createdBy: 'admin-api', + deployedBy: null, + }, + ]) + }) + + it('uses hybrid workflow access for read auth', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ workflow: { id: 'wf-1' } }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deployments', { + headers: { 'x-api-key': 'test-key' }, + }) + const response = await GET(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(200) + expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(req, 'wf-1', { + requireDeployment: false, + action: 'read', + }) + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/deployments/route.ts b/apps/sim/app/api/workflows/[id]/deployments/route.ts index ac2e7e1015f..f4980b07c7a 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/route.ts @@ -3,7 +3,7 @@ import { createLogger } from '@sim/logger' import { desc, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' import { generateRequestId } from '@/lib/core/utils/request' -import { validateWorkflowPermissions } from '@/lib/workflows/utils' +import { validateWorkflowAccess } from '@/app/api/workflows/middleware' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' const logger = createLogger('WorkflowDeploymentsListAPI') @@ -16,9 +16,12 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ const { id } = await params try { - const { error } = await validateWorkflowPermissions(id, requestId, 'read') - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'read', + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) } const rawVersions = await db From 9da2b0416da358bccd47df6f582bd28d050475cf Mon Sep 17 00:00:00 2001 From: Yusuf Mirza Altay Date: Fri, 13 Mar 2026 10:16:14 +0300 Subject: [PATCH 4/8] fix: harden workflow API auth routes Co-authored-by: GitHub Copilot --- .../api/workflows/[id]/deploy/route.test.ts | 107 ++++++++++++++---- .../app/api/workflows/[id]/deploy/route.ts | 47 +++++++- .../[version]/revert/route.test.ts | 45 +++++--- .../[id]/deployments/[version]/route.test.ts | 56 ++++++--- .../workflows/[id]/deployments/route.test.ts | 33 ++++-- apps/sim/app/api/workflows/[id]/route.ts | 21 +++- .../api/workflows/[id]/status/route.test.ts | 41 ++++--- 7 files changed, 269 insertions(+), 81 deletions(-) diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.test.ts b/apps/sim/app/api/workflows/[id]/deploy/route.test.ts index cec634e0f74..71eb19db532 100644 --- a/apps/sim/app/api/workflows/[id]/deploy/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/deploy/route.test.ts @@ -5,16 +5,43 @@ import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -const mockValidateWorkflowAccess = vi.fn() -const mockDbSelect = vi.fn() -const mockDbFrom = vi.fn() -const mockDbWhere = vi.fn() -const mockDbLimit = vi.fn() -const mockDbOrderBy = vi.fn() -const mockDeployWorkflow = vi.fn() -const mockUndeployWorkflow = vi.fn() -const mockCleanupWebhooksForWorkflow = vi.fn() -const mockRemoveMcpToolsForWorkflow = vi.fn() +const { + mockCleanupWebhooksForWorkflow, + mockDbLimit, + mockDbOrderBy, + mockDbFrom, + mockDbSelect, + mockDbSet, + mockDbUpdate, + mockDbWhere, + mockCreateSchedulesForDeploy, + mockDeployWorkflow, + mockLoadWorkflowFromNormalizedTables, + mockRemoveMcpToolsForWorkflow, + mockSaveTriggerWebhooksForDeploy, + mockSyncMcpToolsForWorkflow, + mockUndeployWorkflow, + mockValidatePublicApiAllowed, + mockValidateWorkflowAccess, +} = vi.hoisted(() => ({ + mockCleanupWebhooksForWorkflow: vi.fn(), + mockDbLimit: vi.fn(), + mockDbOrderBy: vi.fn(), + mockDbFrom: vi.fn(), + mockDbSelect: vi.fn(), + mockDbSet: vi.fn(), + mockDbUpdate: vi.fn(), + mockDbWhere: vi.fn(), + mockCreateSchedulesForDeploy: vi.fn(), + mockDeployWorkflow: vi.fn(), + mockLoadWorkflowFromNormalizedTables: vi.fn(), + mockRemoveMcpToolsForWorkflow: vi.fn(), + mockSaveTriggerWebhooksForDeploy: vi.fn(), + mockSyncMcpToolsForWorkflow: vi.fn(), + mockUndeployWorkflow: vi.fn(), + mockValidatePublicApiAllowed: vi.fn(), + mockValidateWorkflowAccess: vi.fn(), +})) vi.mock('@sim/logger', () => ({ createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), @@ -33,19 +60,23 @@ vi.mock('@/lib/core/utils/request', () => ({ })) vi.mock('@sim/db', () => ({ - db: { select: mockDbSelect }, + db: { select: mockDbSelect, update: mockDbUpdate }, workflow: { variables: 'variables', id: 'id' }, workflowDeploymentVersion: { state: 'state', workflowId: 'workflowId', isActive: 'isActive', createdAt: 'createdAt', id: 'id' }, })) -vi.mock('drizzle-orm', () => ({ - and: vi.fn(), - desc: vi.fn(), - eq: vi.fn(), -})) +vi.mock('drizzle-orm', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + and: vi.fn(), + desc: vi.fn(), + eq: vi.fn(), + } +}) vi.mock('@/lib/workflows/persistence/utils', () => ({ - loadWorkflowFromNormalizedTables: vi.fn().mockResolvedValue(null), + loadWorkflowFromNormalizedTables: (...args: unknown[]) => mockLoadWorkflowFromNormalizedTables(...args), deployWorkflow: (...args: unknown[]) => mockDeployWorkflow(...args), undeployWorkflow: (...args: unknown[]) => mockUndeployWorkflow(...args), })) @@ -56,19 +87,19 @@ vi.mock('@/lib/workflows/comparison', () => ({ vi.mock('@/lib/workflows/schedules', () => ({ cleanupDeploymentVersion: vi.fn(), - createSchedulesForDeploy: vi.fn(), + createSchedulesForDeploy: (...args: unknown[]) => mockCreateSchedulesForDeploy(...args), validateWorkflowSchedules: vi.fn().mockReturnValue({ isValid: true }), })) vi.mock('@/lib/webhooks/deploy', () => ({ cleanupWebhooksForWorkflow: (...args: unknown[]) => mockCleanupWebhooksForWorkflow(...args), restorePreviousVersionWebhooks: vi.fn(), - saveTriggerWebhooksForDeploy: vi.fn(), + saveTriggerWebhooksForDeploy: (...args: unknown[]) => mockSaveTriggerWebhooksForDeploy(...args), })) vi.mock('@/lib/mcp/workflow-mcp-sync', () => ({ removeMcpToolsForWorkflow: (...args: unknown[]) => mockRemoveMcpToolsForWorkflow(...args), - syncMcpToolsForWorkflow: vi.fn(), + syncMcpToolsForWorkflow: (...args: unknown[]) => mockSyncMcpToolsForWorkflow(...args), })) vi.mock('@/lib/audit/log', () => ({ @@ -77,7 +108,12 @@ vi.mock('@/lib/audit/log', () => ({ recordAudit: vi.fn(), })) -import { POST, DELETE } from '@/app/api/workflows/[id]/deploy/route' +vi.mock('@/ee/access-control/utils/permission-check', () => ({ + PublicApiNotAllowedError: class PublicApiNotAllowedError extends Error {}, + validatePublicApiAllowed: (...args: unknown[]) => mockValidatePublicApiAllowed(...args), +})) + +import { DELETE, PATCH, POST } from '@/app/api/workflows/[id]/deploy/route' describe('Workflow deploy route', () => { beforeEach(() => { @@ -87,8 +123,20 @@ describe('Workflow deploy route', () => { mockDbWhere.mockReturnValue({ limit: mockDbLimit, orderBy: mockDbOrderBy }) mockDbOrderBy.mockReturnValue({ limit: mockDbLimit }) mockDbLimit.mockResolvedValue([]) + mockDbUpdate.mockReturnValue({ set: mockDbSet }) + mockDbSet.mockReturnValue({ where: mockDbWhere }) mockCleanupWebhooksForWorkflow.mockResolvedValue(undefined) + mockCreateSchedulesForDeploy.mockResolvedValue({ success: true }) + mockLoadWorkflowFromNormalizedTables.mockResolvedValue({ + blocks: { 'block-1': { id: 'block-1', type: 'start_trigger', name: 'Start' } }, + edges: [], + loops: {}, + parallels: {}, + }) + mockSaveTriggerWebhooksForDeploy.mockResolvedValue({ success: true, warnings: [] }) mockRemoveMcpToolsForWorkflow.mockResolvedValue(undefined) + mockSyncMcpToolsForWorkflow.mockResolvedValue(undefined) + mockValidatePublicApiAllowed.mockResolvedValue(undefined) }) it('allows API-key auth for deploy using hybrid auth userId', async () => { @@ -136,4 +184,21 @@ describe('Workflow deploy route', () => { expect(data.isDeployed).toBe(false) expect(mockUndeployWorkflow).toHaveBeenCalledWith({ workflowId: 'wf-1' }) }) + + it('checks public API restrictions against hybrid auth userId', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' }, + auth: { success: true, userId: 'api-user', authType: 'api_key' }, + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deploy', { + method: 'PATCH', + headers: { 'content-type': 'application/json', 'x-api-key': 'test-key' }, + body: JSON.stringify({ isPublicApi: true }), + }) + const response = await PATCH(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(200) + expect(mockValidatePublicApiAllowed).toHaveBeenCalledWith('api-user') + }) }) diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.ts b/apps/sim/app/api/workflows/[id]/deploy/route.ts index cbaf5c4a868..17006b91f7c 100644 --- a/apps/sim/app/api/workflows/[id]/deploy/route.ts +++ b/apps/sim/app/api/workflows/[id]/deploy/route.ts @@ -2,6 +2,7 @@ import { db, workflow, workflowDeploymentVersion } from '@sim/db' import { createLogger } from '@sim/logger' import { and, desc, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' +import type { AuthResult } from '@/lib/auth/hybrid' import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log' import { generateRequestId } from '@/lib/core/utils/request' import { removeMcpToolsForWorkflow, syncMcpToolsForWorkflow } from '@/lib/mcp/workflow-mcp-sync' @@ -30,22 +31,57 @@ const logger = createLogger('WorkflowDeployAPI') export const dynamic = 'force-dynamic' export const runtime = 'nodejs' +type LifecycleAdminAccessResult = { + error: { message: string; status: number } | null | undefined + auth: AuthResult | null | undefined + session: + | Awaited>['session'] + | null + | undefined + workflow: + | Awaited>['workflow'] + | Awaited>['workflow'] + | null + | undefined +} + async function validateLifecycleAdminAccess( request: NextRequest, workflowId: string, requestId: string -) { +): Promise { const hybridAccess = await validateWorkflowAccess(request, workflowId, { requireDeployment: false, action: 'admin', }) if (hybridAccess.error) { - return hybridAccess + return { + error: hybridAccess.error, + auth: hybridAccess.auth, + session: null, + workflow: hybridAccess.workflow, + } } if (hybridAccess.auth?.authType === 'session') { - return await validateWorkflowPermissions(workflowId, requestId, 'admin') + const sessionAccess = await validateWorkflowPermissions(workflowId, requestId, 'admin') + const auth: AuthResult | null = sessionAccess.session?.user?.id + ? { + success: true, + userId: sessionAccess.session.user.id, + userName: sessionAccess.session.user.name, + userEmail: sessionAccess.session.user.email, + authType: 'session', + } + : null + + return { + error: sessionAccess.error, + auth, + session: sessionAccess.session, + workflow: sessionAccess.workflow, + } } return { @@ -338,7 +374,7 @@ export async function PATCH(request: NextRequest, { params }: { params: Promise< const { id } = await params try { - const { error, session } = await validateLifecycleAdminAccess(request, id, requestId) + const { auth, error, session } = await validateLifecycleAdminAccess(request, id, requestId) if (error) { return createErrorResponse(error.message, error.status) } @@ -354,8 +390,9 @@ export async function PATCH(request: NextRequest, { params }: { params: Promise< const { validatePublicApiAllowed, PublicApiNotAllowedError } = await import( '@/ee/access-control/utils/permission-check' ) + const actorUserId = session?.user?.id ?? auth?.userId try { - await validatePublicApiAllowed(session?.user?.id) + await validatePublicApiAllowed(actorUserId) } catch (err) { if (err instanceof PublicApiNotAllowedError) { return createErrorResponse('Public API access is disabled', 403) diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts index e2f74d5cb3b..c985a253707 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts @@ -5,16 +5,29 @@ import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -const mockValidateWorkflowAccess = vi.fn() -const mockDbSelect = vi.fn() -const mockDbFrom = vi.fn() -const mockDbWhere = vi.fn() -const mockDbLimit = vi.fn() -const mockDbUpdate = vi.fn() -const mockDbSet = vi.fn() -const mockDbWhereUpdate = vi.fn() -const mockSaveWorkflowToNormalizedTables = vi.fn() -const mockSyncMcpToolsForWorkflow = vi.fn() +const { + mockDbFrom, + mockDbLimit, + mockDbSelect, + mockDbSet, + mockDbUpdate, + mockDbWhere, + mockDbWhereUpdate, + mockSaveWorkflowToNormalizedTables, + mockSyncMcpToolsForWorkflow, + mockValidateWorkflowAccess, +} = vi.hoisted(() => ({ + mockDbFrom: vi.fn(), + mockDbLimit: vi.fn(), + mockDbSelect: vi.fn(), + mockDbSet: vi.fn(), + mockDbUpdate: vi.fn(), + mockDbWhere: vi.fn(), + mockDbWhereUpdate: vi.fn(), + mockSaveWorkflowToNormalizedTables: vi.fn(), + mockSyncMcpToolsForWorkflow: vi.fn(), + mockValidateWorkflowAccess: vi.fn(), +})) const mockFetch = vi.fn() vi.stubGlobal('fetch', mockFetch) @@ -49,10 +62,14 @@ vi.mock('@sim/db', () => ({ }, })) -vi.mock('drizzle-orm', () => ({ - and: vi.fn(), - eq: vi.fn(), -})) +vi.mock('drizzle-orm', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + and: vi.fn(), + eq: vi.fn(), + } +}) vi.mock('@/lib/workflows/persistence/utils', () => ({ saveWorkflowToNormalizedTables: (...args: unknown[]) => mockSaveWorkflowToNormalizedTables(...args), diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts index 4d2045b9530..510550204e6 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts @@ -5,15 +5,35 @@ import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -const mockValidateWorkflowAccess = vi.fn() -const mockDbSelect = vi.fn() -const mockDbFrom = vi.fn() -const mockDbWhere = vi.fn() -const mockDbLimit = vi.fn() -const mockSaveTriggerWebhooksForDeploy = vi.fn() -const mockCreateSchedulesForDeploy = vi.fn() -const mockActivateWorkflowVersion = vi.fn() -const mockSyncMcpToolsForWorkflow = vi.fn() +const { + mockActivateWorkflowVersion, + mockCreateSchedulesForDeploy, + mockDbFrom, + mockDbLimit, + mockDbReturning, + mockDbSelect, + mockDbSet, + mockDbUpdate, + mockDbWhere, + mockDbWhereUpdate, + mockSaveTriggerWebhooksForDeploy, + mockSyncMcpToolsForWorkflow, + mockValidateWorkflowAccess, +} = vi.hoisted(() => ({ + mockActivateWorkflowVersion: vi.fn(), + mockCreateSchedulesForDeploy: vi.fn(), + mockDbFrom: vi.fn(), + mockDbLimit: vi.fn(), + mockDbReturning: vi.fn(), + mockDbSelect: vi.fn(), + mockDbSet: vi.fn(), + mockDbUpdate: vi.fn(), + mockDbWhere: vi.fn(), + mockDbWhereUpdate: vi.fn(), + mockSaveTriggerWebhooksForDeploy: vi.fn(), + mockSyncMcpToolsForWorkflow: vi.fn(), + mockValidateWorkflowAccess: vi.fn(), +})) vi.mock('@sim/logger', () => ({ createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), @@ -30,7 +50,7 @@ vi.mock('@/lib/core/utils/request', () => ({ vi.mock('@sim/db', () => ({ db: { select: mockDbSelect, - update: vi.fn(() => ({ set: vi.fn(() => ({ where: vi.fn(() => ({ returning: vi.fn() })) })) }), + update: mockDbUpdate, }, workflowDeploymentVersion: { id: 'id', @@ -43,10 +63,14 @@ vi.mock('@sim/db', () => ({ }, })) -vi.mock('drizzle-orm', () => ({ - and: vi.fn(), - eq: vi.fn(), -})) +vi.mock('drizzle-orm', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + and: vi.fn(), + eq: vi.fn(), + } +}) vi.mock('@/lib/webhooks/deploy', () => ({ restorePreviousVersionWebhooks: vi.fn(), @@ -91,6 +115,10 @@ describe('Workflow deployment version route', () => { }, ]) .mockResolvedValueOnce([{ id: 'dep-2' }]) + mockDbUpdate.mockReturnValue({ set: mockDbSet }) + mockDbSet.mockReturnValue({ where: mockDbWhereUpdate }) + mockDbWhereUpdate.mockReturnValue({ returning: mockDbReturning }) + mockDbReturning.mockResolvedValue([]) mockSaveTriggerWebhooksForDeploy.mockResolvedValue({ success: true, warnings: [] }) mockCreateSchedulesForDeploy.mockResolvedValue({ success: true }) mockActivateWorkflowVersion.mockResolvedValue({ diff --git a/apps/sim/app/api/workflows/[id]/deployments/route.test.ts b/apps/sim/app/api/workflows/[id]/deployments/route.test.ts index d1d860c5238..2b760dc7eb5 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/route.test.ts @@ -5,12 +5,21 @@ import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -const mockValidateWorkflowAccess = vi.fn() -const mockDbSelect = vi.fn() -const mockDbFrom = vi.fn() -const mockDbLeftJoin = vi.fn() -const mockDbWhere = vi.fn() -const mockDbOrderBy = vi.fn() +const { + mockDbFrom, + mockDbLeftJoin, + mockDbOrderBy, + mockDbSelect, + mockDbWhere, + mockValidateWorkflowAccess, +} = vi.hoisted(() => ({ + mockDbFrom: vi.fn(), + mockDbLeftJoin: vi.fn(), + mockDbOrderBy: vi.fn(), + mockDbSelect: vi.fn(), + mockDbWhere: vi.fn(), + mockValidateWorkflowAccess: vi.fn(), +})) vi.mock('@sim/logger', () => ({ createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), @@ -39,10 +48,14 @@ vi.mock('@sim/db', () => ({ }, })) -vi.mock('drizzle-orm', () => ({ - desc: vi.fn(), - eq: vi.fn(), -})) +vi.mock('drizzle-orm', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + desc: vi.fn(), + eq: vi.fn(), + } +}) import { GET } from '@/app/api/workflows/[id]/deployments/route' diff --git a/apps/sim/app/api/workflows/[id]/route.ts b/apps/sim/app/api/workflows/[id]/route.ts index 0bbe4da02a4..7e3e831b558 100644 --- a/apps/sim/app/api/workflows/[id]/route.ts +++ b/apps/sim/app/api/workflows/[id]/route.ts @@ -375,13 +375,26 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{ const { id: workflowId } = await params try { - const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) - if (!auth.success || !auth.userId) { + const validation = await validateWorkflowAccess(request, workflowId, { + requireDeployment: false, + action: 'write', + }) + if (validation.error) { logger.warn(`[${requestId}] Unauthorized update attempt for workflow ${workflowId}`) - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + return NextResponse.json( + { error: validation.error.message }, + { status: validation.error.status } + ) } - const userId = auth.userId + const userId = validation.auth?.userId + if (!userId) { + logger.warn(`[${requestId}] Missing user identity for workflow update ${workflowId}`) + return NextResponse.json( + { error: 'Workflow update requires a user-backed session or API key identity' }, + { status: 400 } + ) + } const body = await request.json() const updates = UpdateWorkflowSchema.parse(body) diff --git a/apps/sim/app/api/workflows/[id]/status/route.test.ts b/apps/sim/app/api/workflows/[id]/status/route.test.ts index 2b22b73d368..8e2304737a0 100644 --- a/apps/sim/app/api/workflows/[id]/status/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/status/route.test.ts @@ -5,14 +5,25 @@ import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -const mockValidateWorkflowAccess = vi.fn() -const mockLoadWorkflowFromNormalizedTables = vi.fn() -const mockHasWorkflowChanged = vi.fn() -const mockDbSelect = vi.fn() -const mockDbFrom = vi.fn() -const mockDbWhere = vi.fn() -const mockDbLimit = vi.fn() -const mockDbOrderBy = vi.fn() +const { + mockDbFrom, + mockDbLimit, + mockDbOrderBy, + mockDbSelect, + mockDbWhere, + mockHasWorkflowChanged, + mockLoadWorkflowFromNormalizedTables, + mockValidateWorkflowAccess, +} = vi.hoisted(() => ({ + mockDbFrom: vi.fn(), + mockDbLimit: vi.fn(), + mockDbOrderBy: vi.fn(), + mockDbSelect: vi.fn(), + mockDbWhere: vi.fn(), + mockHasWorkflowChanged: vi.fn(), + mockLoadWorkflowFromNormalizedTables: vi.fn(), + mockValidateWorkflowAccess: vi.fn(), +})) vi.mock('@sim/logger', () => ({ createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), @@ -42,11 +53,15 @@ vi.mock('@sim/db', () => ({ workflowDeploymentVersion: { state: 'state', workflowId: 'workflowId', isActive: 'isActive', createdAt: 'createdAt' }, })) -vi.mock('drizzle-orm', () => ({ - and: vi.fn(), - desc: vi.fn(), - eq: vi.fn(), -})) +vi.mock('drizzle-orm', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + and: vi.fn(), + desc: vi.fn(), + eq: vi.fn(), + } +}) import { GET } from '@/app/api/workflows/[id]/status/route' From c1de6059ddb1e7ccdd2e22049692bcd02ff5c441 Mon Sep 17 00:00:00 2001 From: Yusuf Mirza Altay Date: Fri, 13 Mar 2026 10:42:29 +0300 Subject: [PATCH 5/8] fix: harden workflow API auth follow-ups Co-authored-by: GitHub Copilot --- .../api/workflows/[id]/deploy/route.test.ts | 40 +++++++++-- .../app/api/workflows/[id]/deploy/route.ts | 66 ++++--------------- .../api/workflows/[id]/deployed/route.test.ts | 1 - .../app/api/workflows/[id]/deployed/route.ts | 1 - apps/sim/app/api/workflows/[id]/route.test.ts | 54 ++++++--------- apps/sim/app/api/workflows/[id]/route.ts | 41 +----------- apps/sim/app/api/workflows/middleware.ts | 2 +- 7 files changed, 71 insertions(+), 134 deletions(-) diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.test.ts b/apps/sim/app/api/workflows/[id]/deploy/route.test.ts index 71eb19db532..0513937ed96 100644 --- a/apps/sim/app/api/workflows/[id]/deploy/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/deploy/route.test.ts @@ -7,6 +7,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const { mockCleanupWebhooksForWorkflow, + mockRecordAudit, mockDbLimit, mockDbOrderBy, mockDbFrom, @@ -23,8 +24,10 @@ const { mockUndeployWorkflow, mockValidatePublicApiAllowed, mockValidateWorkflowAccess, + mockValidateWorkflowPermissions, } = vi.hoisted(() => ({ mockCleanupWebhooksForWorkflow: vi.fn(), + mockRecordAudit: vi.fn(), mockDbLimit: vi.fn(), mockDbOrderBy: vi.fn(), mockDbFrom: vi.fn(), @@ -41,6 +44,7 @@ const { mockUndeployWorkflow: vi.fn(), mockValidatePublicApiAllowed: vi.fn(), mockValidateWorkflowAccess: vi.fn(), + mockValidateWorkflowPermissions: vi.fn(), })) vi.mock('@sim/logger', () => ({ @@ -48,7 +52,7 @@ vi.mock('@sim/logger', () => ({ })) vi.mock('@/lib/workflows/utils', () => ({ - validateWorkflowPermissions: vi.fn(), + validateWorkflowPermissions: (...args: unknown[]) => mockValidateWorkflowPermissions(...args), })) vi.mock('@/app/api/workflows/middleware', () => ({ @@ -105,7 +109,7 @@ vi.mock('@/lib/mcp/workflow-mcp-sync', () => ({ vi.mock('@/lib/audit/log', () => ({ AuditAction: {}, AuditResourceType: {}, - recordAudit: vi.fn(), + recordAudit: (...args: unknown[]) => mockRecordAudit(...args), })) vi.mock('@/ee/access-control/utils/permission-check', () => ({ @@ -142,7 +146,13 @@ describe('Workflow deploy route', () => { it('allows API-key auth for deploy using hybrid auth userId', async () => { mockValidateWorkflowAccess.mockResolvedValue({ workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' }, - auth: { success: true, userId: 'api-user', authType: 'api_key' }, + auth: { + success: true, + userId: 'api-user', + userName: 'API Key User', + userEmail: 'api@example.com', + authType: 'api_key', + }, }) mockDeployWorkflow.mockResolvedValue({ success: true, @@ -164,12 +174,26 @@ describe('Workflow deploy route', () => { deployedBy: 'api-user', workflowName: 'Test Workflow', }) + expect(mockValidateWorkflowPermissions).not.toHaveBeenCalled() + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: 'api-user', + actorName: 'API Key User', + actorEmail: 'api@example.com', + }) + ) }) it('allows API-key auth for undeploy using hybrid auth userId', async () => { mockValidateWorkflowAccess.mockResolvedValue({ workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' }, - auth: { success: true, userId: 'api-user', authType: 'api_key' }, + auth: { + success: true, + userId: 'api-user', + userName: 'API Key User', + userEmail: 'api@example.com', + authType: 'api_key', + }, }) mockUndeployWorkflow.mockResolvedValue({ success: true }) @@ -183,6 +207,14 @@ describe('Workflow deploy route', () => { const data = await response.json() expect(data.isDeployed).toBe(false) expect(mockUndeployWorkflow).toHaveBeenCalledWith({ workflowId: 'wf-1' }) + expect(mockValidateWorkflowPermissions).not.toHaveBeenCalled() + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: 'api-user', + actorName: 'API Key User', + actorEmail: 'api@example.com', + }) + ) }) it('checks public API restrictions against hybrid auth userId', async () => { diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.ts b/apps/sim/app/api/workflows/[id]/deploy/route.ts index 17006b91f7c..5c0a2529146 100644 --- a/apps/sim/app/api/workflows/[id]/deploy/route.ts +++ b/apps/sim/app/api/workflows/[id]/deploy/route.ts @@ -21,7 +21,6 @@ import { createSchedulesForDeploy, validateWorkflowSchedules, } from '@/lib/workflows/schedules' -import { validateWorkflowPermissions } from '@/lib/workflows/utils' import { validateWorkflowAccess } from '@/app/api/workflows/middleware' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' import type { WorkflowState } from '@/stores/workflows/workflow/types' @@ -34,21 +33,12 @@ export const runtime = 'nodejs' type LifecycleAdminAccessResult = { error: { message: string; status: number } | null | undefined auth: AuthResult | null | undefined - session: - | Awaited>['session'] - | null - | undefined - workflow: - | Awaited>['workflow'] - | Awaited>['workflow'] - | null - | undefined + workflow: Awaited>['workflow'] | null | undefined } async function validateLifecycleAdminAccess( request: NextRequest, - workflowId: string, - requestId: string + workflowId: string ): Promise { const hybridAccess = await validateWorkflowAccess(request, workflowId, { requireDeployment: false, @@ -59,35 +49,13 @@ async function validateLifecycleAdminAccess( return { error: hybridAccess.error, auth: hybridAccess.auth, - session: null, workflow: hybridAccess.workflow, } } - if (hybridAccess.auth?.authType === 'session') { - const sessionAccess = await validateWorkflowPermissions(workflowId, requestId, 'admin') - const auth: AuthResult | null = sessionAccess.session?.user?.id - ? { - success: true, - userId: sessionAccess.session.user.id, - userName: sessionAccess.session.user.name, - userEmail: sessionAccess.session.user.email, - authType: 'session', - } - : null - - return { - error: sessionAccess.error, - auth, - session: sessionAccess.session, - workflow: sessionAccess.workflow, - } - } - return { error: null, auth: hybridAccess.auth, - session: null, workflow: hybridAccess.workflow, } } @@ -178,17 +146,12 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ const { id } = await params try { - const { - auth, - error, - session, - workflow: workflowData, - } = await validateLifecycleAdminAccess(request, id, requestId) + const { auth, error, workflow: workflowData } = await validateLifecycleAdminAccess(request, id) if (error) { return createErrorResponse(error.message, error.status) } - const actorUserId: string | null = session?.user?.id ?? auth?.userId ?? null + const actorUserId: string | null = auth?.userId ?? null if (!actorUserId) { logger.warn(`[${requestId}] Unable to resolve actor user for workflow deployment: ${id}`) return createErrorResponse('Unable to determine deploying user', 400) @@ -329,8 +292,8 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ recordAudit({ workspaceId: workflowData?.workspaceId || null, actorId: actorUserId, - actorName: session?.user?.name, - actorEmail: session?.user?.email, + actorName: auth?.userName, + actorEmail: auth?.userEmail, action: AuditAction.WORKFLOW_DEPLOYED, resourceType: AuditResourceType.WORKFLOW, resourceId: id, @@ -374,7 +337,7 @@ export async function PATCH(request: NextRequest, { params }: { params: Promise< const { id } = await params try { - const { auth, error, session } = await validateLifecycleAdminAccess(request, id, requestId) + const { auth, error } = await validateLifecycleAdminAccess(request, id) if (error) { return createErrorResponse(error.message, error.status) } @@ -390,7 +353,7 @@ export async function PATCH(request: NextRequest, { params }: { params: Promise< const { validatePublicApiAllowed, PublicApiNotAllowedError } = await import( '@/ee/access-control/utils/permission-check' ) - const actorUserId = session?.user?.id ?? auth?.userId + const actorUserId = auth?.userId try { await validatePublicApiAllowed(actorUserId) } catch (err) { @@ -421,17 +384,12 @@ export async function DELETE( const { id } = await params try { - const { - auth, - error, - session, - workflow: workflowData, - } = await validateLifecycleAdminAccess(request, id, requestId) + const { auth, error, workflow: workflowData } = await validateLifecycleAdminAccess(request, id) if (error) { return createErrorResponse(error.message, error.status) } - const actorUserId = session?.user?.id ?? auth?.userId ?? null + const actorUserId = auth?.userId ?? null if (!actorUserId) { return createErrorResponse('Unable to determine undeploying user', 400) } @@ -458,8 +416,8 @@ export async function DELETE( recordAudit({ workspaceId: workflowData?.workspaceId || null, actorId: actorUserId, - actorName: session?.user?.name, - actorEmail: session?.user?.email, + actorName: auth?.userName, + actorEmail: auth?.userEmail, action: AuditAction.WORKFLOW_UNDEPLOYED, resourceType: AuditResourceType.WORKFLOW, resourceId: id, diff --git a/apps/sim/app/api/workflows/[id]/deployed/route.test.ts b/apps/sim/app/api/workflows/[id]/deployed/route.test.ts index 5d346a442b3..6d903f62411 100644 --- a/apps/sim/app/api/workflows/[id]/deployed/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/deployed/route.test.ts @@ -56,7 +56,6 @@ describe('Workflow deployed-state route', () => { expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(req, 'wf-1', { requireDeployment: false, action: 'read', - allowInternalSecret: false, }) }) }) diff --git a/apps/sim/app/api/workflows/[id]/deployed/route.ts b/apps/sim/app/api/workflows/[id]/deployed/route.ts index 28e4b1d1d24..8a536784522 100644 --- a/apps/sim/app/api/workflows/[id]/deployed/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployed/route.ts @@ -34,7 +34,6 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ const validation = await validateWorkflowAccess(request, id, { requireDeployment: false, action: 'read', - allowInternalSecret: false, }) if (validation.error) { const response = createErrorResponse(validation.error.message, validation.error.status) diff --git a/apps/sim/app/api/workflows/[id]/route.test.ts b/apps/sim/app/api/workflows/[id]/route.test.ts index 91d2b3f7a62..aa450f1a068 100644 --- a/apps/sim/app/api/workflows/[id]/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/route.test.ts @@ -408,6 +408,7 @@ describe('Workflow By ID API Route', () => { const response = await DELETE(req, { params }) expect(response.status).toBe(200) + expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled() }) it('should prevent deletion of the last workflow in workspace', async () => { @@ -448,22 +449,11 @@ describe('Workflow By ID API Route', () => { }) it.concurrent('should deny deletion for non-admin users', async () => { - const mockWorkflow = { - id: 'workflow-123', - userId: 'other-user', - name: 'Test Workflow', - workspaceId: 'workspace-456', - } - - mockGetSession({ user: { id: 'user-123' } }) - - mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ - allowed: false, - status: 403, - message: 'Unauthorized: Access denied to admin this workflow', - workflow: mockWorkflow, - workspacePermission: null, + mockValidateWorkflowAccess.mockResolvedValue({ + error: { + message: 'Unauthorized: Access denied to admin this workflow', + status: 403, + }, }) const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { @@ -532,6 +522,7 @@ describe('Workflow By ID API Route', () => { expect(response.status).toBe(200) const data = await response.json() expect(data.workflow.name).toBe('Updated Workflow') + expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled() }) it('should allow users with write permission to update workflow', async () => { @@ -579,24 +570,13 @@ describe('Workflow By ID API Route', () => { }) it('should deny update for users with only read permission', async () => { - const mockWorkflow = { - id: 'workflow-123', - userId: 'other-user', - name: 'Test Workflow', - workspaceId: 'workspace-456', - } - const updateData = { name: 'Updated Workflow' } - mockGetSession({ user: { id: 'user-123' } }) - - mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ - allowed: false, - status: 403, - message: 'Unauthorized: Access denied to write this workflow', - workflow: mockWorkflow, - workspacePermission: 'read', + mockValidateWorkflowAccess.mockResolvedValue({ + error: { + message: 'Unauthorized: Access denied to write this workflow', + status: 403, + }, }) const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { @@ -765,7 +745,10 @@ describe('Workflow By ID API Route', () => { const updatedWorkflow = { ...mockWorkflow, folderId: 'folder-2', updatedAt: new Date() } - mockGetSession({ user: { id: 'user-123' } }) + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: mockWorkflow, + auth: { success: true, userId: 'user-123', authType: 'session' }, + }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ allowed: true, @@ -807,7 +790,10 @@ describe('Workflow By ID API Route', () => { workspaceId: 'workspace-456', } - mockGetSession({ user: { id: 'user-123' } }) + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: mockWorkflow, + auth: { success: true, userId: 'user-123', authType: 'session' }, + }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ allowed: true, diff --git a/apps/sim/app/api/workflows/[id]/route.ts b/apps/sim/app/api/workflows/[id]/route.ts index 7e3e831b558..6b9b711e1a5 100644 --- a/apps/sim/app/api/workflows/[id]/route.ts +++ b/apps/sim/app/api/workflows/[id]/route.ts @@ -158,6 +158,7 @@ export async function DELETE( const auth = validation.auth const userId = auth?.userId + const workflowData = validation.workflow if (!userId) { logger.warn(`[${requestId}] Missing user identity for workflow deletion ${workflowId}`) @@ -167,30 +168,11 @@ export async function DELETE( ) } - const authorization = await authorizeWorkflowByWorkspacePermission({ - workflowId, - userId, - action: 'admin', - }) - const workflowData = authorization.workflow || (await getWorkflowById(workflowId)) - if (!workflowData) { logger.warn(`[${requestId}] Workflow ${workflowId} not found for deletion`) return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) } - const canDelete = authorization.allowed - - if (!canDelete) { - logger.warn( - `[${requestId}] User ${userId} denied permission to delete workflow ${workflowId}` - ) - return NextResponse.json( - { error: authorization.message || 'Access denied' }, - { status: authorization.status || 403 } - ) - } - // Check if this is the last workflow in the workspace if (workflowData.workspaceId) { const totalWorkflowsInWorkspace = await db @@ -388,6 +370,7 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{ } const userId = validation.auth?.userId + const workflowData = validation.workflow if (!userId) { logger.warn(`[${requestId}] Missing user identity for workflow update ${workflowId}`) return NextResponse.json( @@ -399,31 +382,11 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{ const body = await request.json() const updates = UpdateWorkflowSchema.parse(body) - // Fetch the workflow to check ownership/access - const authorization = await authorizeWorkflowByWorkspacePermission({ - workflowId, - userId, - action: 'write', - }) - const workflowData = authorization.workflow || (await getWorkflowById(workflowId)) - if (!workflowData) { logger.warn(`[${requestId}] Workflow ${workflowId} not found for update`) return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) } - const canUpdate = authorization.allowed - - if (!canUpdate) { - logger.warn( - `[${requestId}] User ${userId} denied permission to update workflow ${workflowId}` - ) - return NextResponse.json( - { error: authorization.message || 'Access denied' }, - { status: authorization.status || 403 } - ) - } - const updateData: Record = { updatedAt: new Date() } if (updates.name !== undefined) updateData.name = updates.name if (updates.description !== undefined) updateData.description = updates.description diff --git a/apps/sim/app/api/workflows/middleware.ts b/apps/sim/app/api/workflows/middleware.ts index f6e6fbdfb69..bfcaed902c2 100644 --- a/apps/sim/app/api/workflows/middleware.ts +++ b/apps/sim/app/api/workflows/middleware.ts @@ -33,7 +33,7 @@ export async function validateWorkflowAccess( typeof options === 'boolean' ? { requireDeployment: options } : options const requireDeployment = normalizedOptions.requireDeployment ?? true const action = normalizedOptions.action ?? 'read' - const allowInternalSecret = normalizedOptions.allowInternalSecret ?? requireDeployment + const allowInternalSecret = normalizedOptions.allowInternalSecret ?? false const workflow = await getWorkflowById(workflowId) if (!workflow) { From aab58cba792901e9f990382a4171576174220493 Mon Sep 17 00:00:00 2001 From: Yusuf Mirza Altay Date: Fri, 13 Mar 2026 10:57:01 +0300 Subject: [PATCH 6/8] fix: align workflow audit actors for API keys Co-authored-by: GitHub Copilot --- .../api/workflows/[id]/deploy/route.test.ts | 12 ++++-------- .../sim/app/api/workflows/[id]/deploy/route.ts | 17 +++++++++++++---- .../deployments/[version]/revert/route.test.ts | 11 ++++++++++- .../[id]/deployments/[version]/revert/route.ts | 7 +++++-- .../[id]/deployments/[version]/route.test.ts | 11 ++++++++++- .../[id]/deployments/[version]/route.ts | 7 +++++-- apps/sim/lib/audit/actor-metadata.ts | 18 ++++++++++++++++++ 7 files changed, 65 insertions(+), 18 deletions(-) create mode 100644 apps/sim/lib/audit/actor-metadata.ts diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.test.ts b/apps/sim/app/api/workflows/[id]/deploy/route.test.ts index 0513937ed96..1474d557df5 100644 --- a/apps/sim/app/api/workflows/[id]/deploy/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/deploy/route.test.ts @@ -149,8 +149,6 @@ describe('Workflow deploy route', () => { auth: { success: true, userId: 'api-user', - userName: 'API Key User', - userEmail: 'api@example.com', authType: 'api_key', }, }) @@ -178,8 +176,8 @@ describe('Workflow deploy route', () => { expect(mockRecordAudit).toHaveBeenCalledWith( expect.objectContaining({ actorId: 'api-user', - actorName: 'API Key User', - actorEmail: 'api@example.com', + actorName: undefined, + actorEmail: undefined, }) ) }) @@ -190,8 +188,6 @@ describe('Workflow deploy route', () => { auth: { success: true, userId: 'api-user', - userName: 'API Key User', - userEmail: 'api@example.com', authType: 'api_key', }, }) @@ -211,8 +207,8 @@ describe('Workflow deploy route', () => { expect(mockRecordAudit).toHaveBeenCalledWith( expect.objectContaining({ actorId: 'api-user', - actorName: 'API Key User', - actorEmail: 'api@example.com', + actorName: undefined, + actorEmail: undefined, }) ) }) diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.ts b/apps/sim/app/api/workflows/[id]/deploy/route.ts index 5c0a2529146..d8821aa1099 100644 --- a/apps/sim/app/api/workflows/[id]/deploy/route.ts +++ b/apps/sim/app/api/workflows/[id]/deploy/route.ts @@ -3,6 +3,7 @@ import { createLogger } from '@sim/logger' import { and, desc, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' import type { AuthResult } from '@/lib/auth/hybrid' +import { getAuditActorMetadata } from '@/lib/audit/actor-metadata' import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log' import { generateRequestId } from '@/lib/core/utils/request' import { removeMcpToolsForWorkflow, syncMcpToolsForWorkflow } from '@/lib/mcp/workflow-mcp-sync' @@ -60,6 +61,10 @@ async function validateLifecycleAdminAccess( } } +function getLifecycleAuditActor(auth: AuthResult | null | undefined) { + return getAuditActorMetadata(auth) +} + export async function GET(request: NextRequest, { params }: { params: Promise<{ id: string }> }) { const requestId = generateRequestId() const { id } = await params @@ -289,11 +294,13 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ // Sync MCP tools with the latest parameter schema await syncMcpToolsForWorkflow({ workflowId: id, requestId, context: 'deploy' }) + const { actorName, actorEmail } = getLifecycleAuditActor(auth) + recordAudit({ workspaceId: workflowData?.workspaceId || null, actorId: actorUserId, - actorName: auth?.userName, - actorEmail: auth?.userEmail, + actorName, + actorEmail, action: AuditAction.WORKFLOW_DEPLOYED, resourceType: AuditResourceType.WORKFLOW, resourceId: id, @@ -413,11 +420,13 @@ export async function DELETE( // Silently fail } + const { actorName, actorEmail } = getLifecycleAuditActor(auth) + recordAudit({ workspaceId: workflowData?.workspaceId || null, actorId: actorUserId, - actorName: auth?.userName, - actorEmail: auth?.userEmail, + actorName, + actorEmail, action: AuditAction.WORKFLOW_UNDEPLOYED, resourceType: AuditResourceType.WORKFLOW, resourceId: id, diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts index c985a253707..8eaf6128be5 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts @@ -13,6 +13,7 @@ const { mockDbUpdate, mockDbWhere, mockDbWhereUpdate, + mockRecordAudit, mockSaveWorkflowToNormalizedTables, mockSyncMcpToolsForWorkflow, mockValidateWorkflowAccess, @@ -24,6 +25,7 @@ const { mockDbUpdate: vi.fn(), mockDbWhere: vi.fn(), mockDbWhereUpdate: vi.fn(), + mockRecordAudit: vi.fn(), mockSaveWorkflowToNormalizedTables: vi.fn(), mockSyncMcpToolsForWorkflow: vi.fn(), mockValidateWorkflowAccess: vi.fn(), @@ -82,7 +84,7 @@ vi.mock('@/lib/mcp/workflow-mcp-sync', () => ({ vi.mock('@/lib/audit/log', () => ({ AuditAction: { WORKFLOW_DEPLOYMENT_REVERTED: 'WORKFLOW_DEPLOYMENT_REVERTED' }, AuditResourceType: { WORKFLOW: 'WORKFLOW' }, - recordAudit: vi.fn(), + recordAudit: (...args: unknown[]) => mockRecordAudit(...args), })) import { POST } from '@/app/api/workflows/[id]/deployments/[version]/revert/route' @@ -132,5 +134,12 @@ describe('Workflow deployment version revert route', () => { }) expect(mockSaveWorkflowToNormalizedTables).toHaveBeenCalled() expect(mockSyncMcpToolsForWorkflow).toHaveBeenCalled() + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: 'api-user', + actorName: undefined, + actorEmail: undefined, + }) + ) }) }) diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts index 74925c069cd..af80f30daec 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts @@ -2,6 +2,7 @@ import { db, workflow, workflowDeploymentVersion } from '@sim/db' import { createLogger } from '@sim/logger' import { and, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' +import { getAuditActorMetadata } from '@/lib/audit/actor-metadata' import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log' import { env } from '@/lib/core/config/env' import { generateRequestId } from '@/lib/core/utils/request' @@ -135,14 +136,16 @@ export async function POST( logger.error('Error sending workflow reverted event to socket server', e) } + const { actorName, actorEmail } = getAuditActorMetadata(auth) + recordAudit({ workspaceId: workflowRecord?.workspaceId ?? null, actorId: actorUserId, action: AuditAction.WORKFLOW_DEPLOYMENT_REVERTED, resourceType: AuditResourceType.WORKFLOW, resourceId: id, - actorName: auth?.userName ?? undefined, - actorEmail: auth?.userEmail ?? undefined, + actorName, + actorEmail, resourceName: workflowRecord?.name ?? undefined, description: `Reverted workflow to deployment version ${version}`, request, diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts index 510550204e6..74c07587948 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts @@ -16,6 +16,7 @@ const { mockDbUpdate, mockDbWhere, mockDbWhereUpdate, + mockRecordAudit, mockSaveTriggerWebhooksForDeploy, mockSyncMcpToolsForWorkflow, mockValidateWorkflowAccess, @@ -30,6 +31,7 @@ const { mockDbUpdate: vi.fn(), mockDbWhere: vi.fn(), mockDbWhereUpdate: vi.fn(), + mockRecordAudit: vi.fn(), mockSaveTriggerWebhooksForDeploy: vi.fn(), mockSyncMcpToolsForWorkflow: vi.fn(), mockValidateWorkflowAccess: vi.fn(), @@ -94,7 +96,7 @@ vi.mock('@/lib/mcp/workflow-mcp-sync', () => ({ vi.mock('@/lib/audit/log', () => ({ AuditAction: { WORKFLOW_DEPLOYMENT_ACTIVATED: 'WORKFLOW_DEPLOYMENT_ACTIVATED' }, AuditResourceType: { WORKFLOW: 'WORKFLOW' }, - recordAudit: vi.fn(), + recordAudit: (...args: unknown[]) => mockRecordAudit(...args), })) import { PATCH } from '@/app/api/workflows/[id]/deployments/[version]/route' @@ -148,5 +150,12 @@ describe('Workflow deployment version route', () => { expect(mockSaveTriggerWebhooksForDeploy).toHaveBeenCalledWith( expect.objectContaining({ userId: 'api-user' }) ) + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: 'api-user', + actorName: undefined, + actorEmail: undefined, + }) + ) }) }) diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts index c9ec9bd2d81..859aa0bac34 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts @@ -3,6 +3,7 @@ import { createLogger } from '@sim/logger' import { and, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' import { z } from 'zod' +import { getAuditActorMetadata } from '@/lib/audit/actor-metadata' import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log' import { generateRequestId } from '@/lib/core/utils/request' import { syncMcpToolsForWorkflow } from '@/lib/mcp/workflow-mcp-sync' @@ -319,11 +320,13 @@ export async function PATCH( } } + const { actorName, actorEmail } = getAuditActorMetadata(auth) + recordAudit({ workspaceId: workflowData?.workspaceId, actorId: actorUserId, - actorName: auth?.userName, - actorEmail: auth?.userEmail, + actorName, + actorEmail, action: AuditAction.WORKFLOW_DEPLOYMENT_ACTIVATED, resourceType: AuditResourceType.WORKFLOW, resourceId: id, diff --git a/apps/sim/lib/audit/actor-metadata.ts b/apps/sim/lib/audit/actor-metadata.ts new file mode 100644 index 00000000000..2ea44c3d41f --- /dev/null +++ b/apps/sim/lib/audit/actor-metadata.ts @@ -0,0 +1,18 @@ +import type { AuthResult } from '@/lib/auth/hybrid' + +export function getAuditActorMetadata(auth: AuthResult | null | undefined): { + actorName: string | undefined + actorEmail: string | undefined +} { + if (auth?.authType !== 'session') { + return { + actorName: undefined, + actorEmail: undefined, + } + } + + return { + actorName: auth.userName ?? undefined, + actorEmail: auth.userEmail ?? undefined, + } +} From f2a4847b1c9807ba6f7396ecda0157785dcf25c0 Mon Sep 17 00:00:00 2001 From: test Date: Fri, 13 Mar 2026 11:51:31 +0300 Subject: [PATCH 7/8] fix: align workflow audit metadata helpers Use the shared audit actor helper consistently so workflow deletion matches deploy behavior and remove the redundant deploy wrapper raised in review. --- apps/sim/app/api/workflows/[id]/deploy/route.ts | 10 +++------- apps/sim/app/api/workflows/[id]/route.test.ts | 15 ++++++++++++++- apps/sim/app/api/workflows/[id]/route.ts | 12 +++++++++--- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.ts b/apps/sim/app/api/workflows/[id]/deploy/route.ts index d8821aa1099..7b6f2c243c4 100644 --- a/apps/sim/app/api/workflows/[id]/deploy/route.ts +++ b/apps/sim/app/api/workflows/[id]/deploy/route.ts @@ -2,9 +2,9 @@ import { db, workflow, workflowDeploymentVersion } from '@sim/db' import { createLogger } from '@sim/logger' import { and, desc, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' -import type { AuthResult } from '@/lib/auth/hybrid' import { getAuditActorMetadata } from '@/lib/audit/actor-metadata' import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log' +import type { AuthResult } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' import { removeMcpToolsForWorkflow, syncMcpToolsForWorkflow } from '@/lib/mcp/workflow-mcp-sync' import { @@ -61,10 +61,6 @@ async function validateLifecycleAdminAccess( } } -function getLifecycleAuditActor(auth: AuthResult | null | undefined) { - return getAuditActorMetadata(auth) -} - export async function GET(request: NextRequest, { params }: { params: Promise<{ id: string }> }) { const requestId = generateRequestId() const { id } = await params @@ -294,7 +290,7 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ // Sync MCP tools with the latest parameter schema await syncMcpToolsForWorkflow({ workflowId: id, requestId, context: 'deploy' }) - const { actorName, actorEmail } = getLifecycleAuditActor(auth) + const { actorName, actorEmail } = getAuditActorMetadata(auth) recordAudit({ workspaceId: workflowData?.workspaceId || null, @@ -420,7 +416,7 @@ export async function DELETE( // Silently fail } - const { actorName, actorEmail } = getLifecycleAuditActor(auth) + const { actorName, actorEmail } = getAuditActorMetadata(auth) recordAudit({ workspaceId: workflowData?.workspaceId || null, diff --git a/apps/sim/app/api/workflows/[id]/route.test.ts b/apps/sim/app/api/workflows/[id]/route.test.ts index aa450f1a068..861129bb555 100644 --- a/apps/sim/app/api/workflows/[id]/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/route.test.ts @@ -380,7 +380,13 @@ describe('Workflow By ID API Route', () => { mockValidateWorkflowAccess.mockResolvedValue({ workflow: mockWorkflow, - auth: { success: true, userId: 'api-user-1', authType: 'api_key' }, + auth: { + success: true, + userId: 'api-user-1', + authType: 'api_key', + userName: 'API Key Actor', + userEmail: null, + }, }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ @@ -409,6 +415,13 @@ describe('Workflow By ID API Route', () => { expect(response.status).toBe(200) expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled() + expect(auditMock.recordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: 'api-user-1', + actorName: undefined, + actorEmail: undefined, + }) + ) }) it('should prevent deletion of the last workflow in workspace', async () => { diff --git a/apps/sim/app/api/workflows/[id]/route.ts b/apps/sim/app/api/workflows/[id]/route.ts index 6b9b711e1a5..3c0a7977b88 100644 --- a/apps/sim/app/api/workflows/[id]/route.ts +++ b/apps/sim/app/api/workflows/[id]/route.ts @@ -4,6 +4,7 @@ import { createLogger } from '@sim/logger' import { and, eq, isNull, ne } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' +import { getAuditActorMetadata } from '@/lib/audit/actor-metadata' import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log' import { AuthType, checkHybridAuth } from '@/lib/auth/hybrid' import { env } from '@/lib/core/config/env' @@ -153,7 +154,10 @@ export async function DELETE( }) if (validation.error) { logger.warn(`[${requestId}] Unauthorized deletion attempt for workflow ${workflowId}`) - return NextResponse.json({ error: validation.error.message }, { status: validation.error.status }) + return NextResponse.json( + { error: validation.error.message }, + { status: validation.error.status } + ) } const auth = validation.auth @@ -323,11 +327,13 @@ export async function DELETE( // Don't fail the deletion if Socket.IO notification fails } + const { actorName, actorEmail } = getAuditActorMetadata(auth) + recordAudit({ workspaceId: workflowData.workspaceId || null, actorId: userId, - actorName: auth.userName, - actorEmail: auth.userEmail, + actorName, + actorEmail, action: AuditAction.WORKFLOW_DELETED, resourceType: AuditResourceType.WORKFLOW, resourceId: workflowId, From a264ee942cdd8a2262078a5e242c5dd9e1f4d932 Mon Sep 17 00:00:00 2001 From: test Date: Fri, 13 Mar 2026 12:08:55 +0300 Subject: [PATCH 8/8] refactor: inline workflow access wrappers Call validateWorkflowAccess directly in workflow deployment lifecycle routes and clean up the related test helper formatting raised in review. --- .../app/api/workflows/[id]/deploy/route.ts | 66 ++++++++----------- .../deployments/[version]/revert/route.ts | 37 ++++------- .../[id]/deployments/[version]/route.ts | 42 ++++-------- apps/sim/app/api/workflows/[id]/route.test.ts | 1 + 4 files changed, 52 insertions(+), 94 deletions(-) diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.ts b/apps/sim/app/api/workflows/[id]/deploy/route.ts index 7b6f2c243c4..bd5fc9c418e 100644 --- a/apps/sim/app/api/workflows/[id]/deploy/route.ts +++ b/apps/sim/app/api/workflows/[id]/deploy/route.ts @@ -4,7 +4,6 @@ import { and, desc, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' import { getAuditActorMetadata } from '@/lib/audit/actor-metadata' import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log' -import type { AuthResult } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' import { removeMcpToolsForWorkflow, syncMcpToolsForWorkflow } from '@/lib/mcp/workflow-mcp-sync' import { @@ -31,36 +30,6 @@ const logger = createLogger('WorkflowDeployAPI') export const dynamic = 'force-dynamic' export const runtime = 'nodejs' -type LifecycleAdminAccessResult = { - error: { message: string; status: number } | null | undefined - auth: AuthResult | null | undefined - workflow: Awaited>['workflow'] | null | undefined -} - -async function validateLifecycleAdminAccess( - request: NextRequest, - workflowId: string -): Promise { - const hybridAccess = await validateWorkflowAccess(request, workflowId, { - requireDeployment: false, - action: 'admin', - }) - - if (hybridAccess.error) { - return { - error: hybridAccess.error, - auth: hybridAccess.auth, - workflow: hybridAccess.workflow, - } - } - - return { - error: null, - auth: hybridAccess.auth, - workflow: hybridAccess.workflow, - } -} - export async function GET(request: NextRequest, { params }: { params: Promise<{ id: string }> }) { const requestId = generateRequestId() const { id } = await params @@ -147,11 +116,17 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ const { id } = await params try { - const { auth, error, workflow: workflowData } = await validateLifecycleAdminAccess(request, id) - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'admin', + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) } + const auth = access.auth + const workflowData = access.workflow + const actorUserId: string | null = auth?.userId ?? null if (!actorUserId) { logger.warn(`[${requestId}] Unable to resolve actor user for workflow deployment: ${id}`) @@ -340,11 +315,16 @@ export async function PATCH(request: NextRequest, { params }: { params: Promise< const { id } = await params try { - const { auth, error } = await validateLifecycleAdminAccess(request, id) - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'admin', + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) } + const auth = access.auth + const body = await request.json() const { isPublicApi } = body @@ -387,11 +367,17 @@ export async function DELETE( const { id } = await params try { - const { auth, error, workflow: workflowData } = await validateLifecycleAdminAccess(request, id) - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'admin', + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) } + const auth = access.auth + const workflowData = access.workflow + const actorUserId = auth?.userId ?? null if (!actorUserId) { return createErrorResponse('Unable to determine undeploying user', 400) diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts index af80f30daec..58d52d0b01c 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts @@ -13,23 +13,6 @@ import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/ const logger = createLogger('RevertToDeploymentVersionAPI') -async function validateDeploymentVersionAdminAccess(request: NextRequest, workflowId: string) { - const access = await validateWorkflowAccess(request, workflowId, { - requireDeployment: false, - action: 'admin', - }) - - if (access.error) { - return access - } - - return { - error: null, - auth: access.auth, - workflow: access.workflow, - } -} - export const dynamic = 'force-dynamic' export const runtime = 'nodejs' @@ -41,18 +24,22 @@ export async function POST( const { id, version } = await params try { - const { - auth, - error, - workflow: workflowRecord, - } = await validateDeploymentVersionAdminAccess(request, id) - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'admin', + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) } + const auth = access.auth + const workflowRecord = access.workflow + const actorUserId = auth?.userId if (!actorUserId) { - logger.warn(`[${requestId}] Unable to resolve actor user for workflow deployment revert: ${id}`) + logger.warn( + `[${requestId}] Unable to resolve actor user for workflow deployment revert: ${id}` + ) return createErrorResponse('Unable to determine reverting user', 400) } diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts index 859aa0bac34..b6450b4beab 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts @@ -20,27 +20,6 @@ import type { BlockState } from '@/stores/workflows/workflow/types' const logger = createLogger('WorkflowDeploymentVersionAPI') -async function validateDeploymentVersionLifecycleAccess( - request: NextRequest, - workflowId: string, - action: 'read' | 'write' | 'admin' -) { - const access = await validateWorkflowAccess(request, workflowId, { - requireDeployment: false, - action, - }) - - if (access.error) { - return access - } - - return { - error: null, - auth: access.auth, - workflow: access.workflow, - } -} - const patchBodySchema = z .object({ name: z @@ -75,7 +54,10 @@ export async function GET( const { id, version } = await params try { - const access = await validateDeploymentVersionLifecycleAccess(request, id, 'read') + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'read', + }) if (access.error) { return createErrorResponse(access.error.message, access.error.status) } @@ -129,15 +111,17 @@ export async function PATCH( // Activation requires admin permission, other updates require write const requiredPermission = isActive ? 'admin' : 'write' - const { - auth, - error, - workflow: workflowData, - } = await validateDeploymentVersionLifecycleAccess(request, id, requiredPermission) - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: requiredPermission, + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) } + const auth = access.auth + const workflowData = access.workflow + const versionNum = Number(version) if (!Number.isFinite(versionNum)) { return createErrorResponse('Invalid version', 400) diff --git a/apps/sim/app/api/workflows/[id]/route.test.ts b/apps/sim/app/api/workflows/[id]/route.test.ts index 861129bb555..e48ed9bd6e8 100644 --- a/apps/sim/app/api/workflows/[id]/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/route.test.ts @@ -40,6 +40,7 @@ function mockGetSession(session: { user: { id: string } } | null) { } else { mockCheckHybridAuth.mockResolvedValue({ success: false }) mockCheckSessionOrInternalAuth.mockResolvedValue({ success: false }) + mockValidateWorkflowAccess.mockResolvedValue({ error: { message: 'Unauthorized', status: 401 }, })