fix(workflows): tighten shared workflow access validation

Author: PlaneInABottle

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

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

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',
+    })
+  })
+})

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)

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<ValidationResult> {
   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 }
       }