fix(api): check folder containment before lock state; reject malformed version cursors

assertFolderMutable walks a folder's ancestor chain without filtering on
workspace, so inspecting it before containment let a caller tell a locked
folder in someone else's workspace (423) from a nonexistent one (400).
Create and update now assert containment first, matching the ordering
import-workflow.ts already uses.

A version cursor that decodes to JSON without a numeric version filtered
every row out and returned an empty page with nextCursor null, which reads
as a clean end-of-list. Malformed cursors are now a 400.

Author: TheodoreSpeaks

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

@@ -17,8 +17,10 @@ const {
   mockPerformDeleteWorkflow,
   mockAssertWorkflowMutable,
   mockAssertFolderMutable,
+  mockAssertFolderInWorkspace,
   WorkflowLockedErrorMock,
   FolderLockedErrorMock,
+  FolderNotFoundErrorMock,
 } = vi.hoisted(() => ({
   mockCheckRateLimit: vi.fn(),
   mockResolveWorkspaceAccess: vi.fn(),
@@ -27,12 +29,16 @@ const {
   mockPerformDeleteWorkflow: vi.fn(),
   mockAssertWorkflowMutable: vi.fn(),
   mockAssertFolderMutable: vi.fn(),
+  mockAssertFolderInWorkspace: vi.fn(),
   WorkflowLockedErrorMock: class WorkflowLockedError extends Error {
     status = 423
   },
   FolderLockedErrorMock: class FolderLockedError extends Error {
     status = 423
   },
+  FolderNotFoundErrorMock: class FolderNotFoundError extends Error {
+    status = 400
+  },
 }))
 
 vi.mock('@/app/api/v1/middleware', () => ({
@@ -49,8 +55,10 @@ vi.mock('@sim/platform-authz/workflow', () => ({
   getActiveWorkflowRecord: mockGetActiveWorkflowRecord,
   assertWorkflowMutable: mockAssertWorkflowMutable,
   assertFolderMutable: mockAssertFolderMutable,
+  assertFolderInWorkspace: mockAssertFolderInWorkspace,
   WorkflowLockedError: WorkflowLockedErrorMock,
   FolderLockedError: FolderLockedErrorMock,
+  FolderNotFoundError: FolderNotFoundErrorMock,
 }))
 
 vi.mock('@/lib/workflows/input-format', () => ({
@@ -139,6 +147,7 @@ describe('PATCH /api/v2/workflows/[id]', () => {
     mockGetActiveWorkflowRecord.mockResolvedValue(WORKFLOW_RECORD)
     mockAssertWorkflowMutable.mockResolvedValue(undefined)
     mockAssertFolderMutable.mockResolvedValue(undefined)
+    mockAssertFolderInWorkspace.mockResolvedValue(undefined)
     mockPerformUpdateWorkflow.mockResolvedValue({ success: true, workflow: UPDATED })
   })
 
@@ -196,6 +205,40 @@ describe('PATCH /api/v2/workflows/[id]', () => {
     expect(mockPerformUpdateWorkflow).not.toHaveBeenCalled()
   })
 
+  it('400s a folder outside the workspace without ever reading its lock state', async () => {
+    mockAssertFolderInWorkspace.mockRejectedValue(
+      new FolderNotFoundErrorMock('Target folder not found')
+    )
+    const res = await callPatch({ folderId: 'fld-other-workspace' })
+
+    expect(res.status).toBe(400)
+    expect((await res.json()).error.code).toBe('BAD_REQUEST')
+    // Containment runs first, so a locked foreign folder cannot be told apart
+    // from a nonexistent one by its status code.
+    expect(mockAssertFolderMutable).not.toHaveBeenCalled()
+    expect(mockPerformUpdateWorkflow).not.toHaveBeenCalled()
+  })
+
+  it('checks folder containment against the workflow workspace before mutability', async () => {
+    const order: string[] = []
+    mockAssertFolderInWorkspace.mockImplementation(async () => {
+      order.push('containment')
+    })
+    mockAssertFolderMutable.mockImplementation(async () => {
+      order.push('mutability')
+    })
+
+    await callPatch({ folderId: 'fld-1' })
+
+    expect(order).toEqual(['containment', 'mutability'])
+    expect(mockAssertFolderInWorkspace).toHaveBeenCalledWith('fld-1', 'workspace-1')
+  })
+
+  it('skips the containment check on a rename that does not move the workflow', async () => {
+    await callPatch({ name: 'Support Agent v2' })
+    expect(mockAssertFolderInWorkspace).not.toHaveBeenCalled()
+  })
+
   it('409s when the target name is taken in the destination folder', async () => {
     mockPerformUpdateWorkflow.mockResolvedValue({
       success: false,

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

@@ -2,9 +2,11 @@ import { db } from '@sim/db'
 import { workflowBlocks } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import {
+  assertFolderInWorkspace,
   assertFolderMutable,
   assertWorkflowMutable,
   FolderLockedError,
+  FolderNotFoundError,
   getActiveWorkflowRecord,
   WorkflowLockedError,
 } from '@sim/platform-authz/workflow'
@@ -140,6 +142,13 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Rout
     )
     if (access) return v2Error('NOT_FOUND', 'Workflow not found')
 
+    /**
+     * Ownership before lock state: `assertFolderMutable` walks the folder's
+     * ancestor chain without filtering on workspace, so checking it first would
+     * let a caller distinguish a locked folder in someone else's workspace
+     * (423) from one that simply does not exist (400).
+     */
+    if (folderId) await assertFolderInWorkspace(folderId, workflowData.workspaceId)
     await assertWorkflowMutable(id)
     if (folderId !== undefined) await assertFolderMutable(folderId)
 
@@ -180,6 +189,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Rout
 
     return v2Data(item, { rateLimit })
   } catch (error) {
+    if (error instanceof FolderNotFoundError) return v2Error('BAD_REQUEST', error.message)
     if (error instanceof WorkflowLockedError || error instanceof FolderLockedError) {
       return v2Error('LOCKED', error.message)
     }

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

@@ -160,6 +160,23 @@ describe('GET /api/v2/workflows/[id]/versions', () => {
     expect(mockListWorkflowVersions).toHaveBeenCalledWith('wf-1')
   })
 
+  it('400s a structurally invalid cursor instead of silently truncating the list', async () => {
+    // Decodes to valid JSON with no numeric `version` — the shape that would
+    // otherwise filter every row out and report a clean end-of-list.
+    const bogus = Buffer.from(JSON.stringify({ offset: 2 })).toString('base64')
+    const res = await callGet(`?cursor=${encodeURIComponent(bogus)}`)
+
+    expect(res.status).toBe(400)
+    expect((await res.json()).error.code).toBe('BAD_REQUEST')
+    expect(mockListWorkflowVersions).not.toHaveBeenCalled()
+  })
+
+  it('400s a cursor that is not decodable at all', async () => {
+    const res = await callGet('?cursor=not-a-cursor')
+    expect(res.status).toBe(400)
+    expect(mockListWorkflowVersions).not.toHaveBeenCalled()
+  })
+
   it('pages with a version-keyed cursor', async () => {
     const first = await callGet('?limit=2')
     const firstBody = await first.json()

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

@@ -64,10 +64,20 @@ export const GET = withRouteHandler(
       const access = await resolveWorkspaceAccess(rateLimit, userId, workflowData.workspaceId)
       if (access) return v2Error('NOT_FOUND', 'Workflow not found')
 
+      /**
+       * A cursor that decodes to anything other than a version number is
+       * rejected rather than ignored: comparing every row against a missing
+       * `version` yields an empty page with `nextCursor: null`, which reads to
+       * the caller as a clean end-of-list while versions are still pending.
+       */
+      const after = cursor ? decodeCursor<WorkflowVersionCursor>(cursor) : null
+      if (cursor && (!after || !Number.isInteger(after.version) || after.version < 1)) {
+        return v2Error('BAD_REQUEST', 'Invalid cursor')
+      }
+
       const { versions: rows } = await listWorkflowVersions(id)
 
-      const cursorData = cursor ? decodeCursor<WorkflowVersionCursor>(cursor) : null
-      const remaining = cursorData ? rows.filter((row) => row.version < cursorData.version) : rows
+      const remaining = after ? rows.filter((row) => row.version < after.version) : rows
 
       const hasMore = remaining.length > limit
       const page = remaining.slice(0, limit)

apps/sim/app/api/v2/workflows/route.test.ts

@@ -13,15 +13,21 @@ const {
   mockResolveWorkspaceAccess,
   mockPerformCreateWorkflow,
   mockAssertFolderMutable,
+  mockAssertFolderInWorkspace,
   FolderLockedErrorMock,
+  FolderNotFoundErrorMock,
 } = vi.hoisted(() => ({
   mockCheckRateLimit: vi.fn(),
   mockResolveWorkspaceAccess: vi.fn(),
   mockPerformCreateWorkflow: vi.fn(),
   mockAssertFolderMutable: vi.fn(),
+  mockAssertFolderInWorkspace: vi.fn(),
   FolderLockedErrorMock: class FolderLockedError extends Error {
     status = 423
   },
+  FolderNotFoundErrorMock: class FolderNotFoundError extends Error {
+    status = 400
+  },
 }))
 
 vi.mock('@/app/api/v1/middleware', () => ({
@@ -35,7 +41,9 @@ vi.mock('@/lib/workflows/orchestration', () => ({
 
 vi.mock('@sim/platform-authz/workflow', () => ({
   assertFolderMutable: mockAssertFolderMutable,
+  assertFolderInWorkspace: mockAssertFolderInWorkspace,
   FolderLockedError: FolderLockedErrorMock,
+  FolderNotFoundError: FolderNotFoundErrorMock,
 }))
 
 vi.mock('@/app/api/v2/lib/gate', () => ({
@@ -98,6 +106,7 @@ describe('POST /api/v2/workflows', () => {
     mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK)
     mockResolveWorkspaceAccess.mockResolvedValue(null)
     mockAssertFolderMutable.mockResolvedValue(undefined)
+    mockAssertFolderInWorkspace.mockResolvedValue(undefined)
     mockPerformCreateWorkflow.mockResolvedValue({ success: true, workflow: CREATED })
   })
 
@@ -157,6 +166,41 @@ describe('POST /api/v2/workflows', () => {
     expect(mockPerformCreateWorkflow).not.toHaveBeenCalled()
   })
 
+  it('400s a folder outside the workspace without ever reading its lock state', async () => {
+    mockAssertFolderInWorkspace.mockRejectedValue(
+      new FolderNotFoundErrorMock('Target folder not found')
+    )
+    const res = await callPost({ ...VALID_BODY, folderId: 'fld-other-workspace' })
+
+    expect(res.status).toBe(400)
+    expect((await res.json()).error.code).toBe('BAD_REQUEST')
+    // Containment runs first, so a locked foreign folder cannot be told apart
+    // from a nonexistent one by its status code.
+    expect(mockAssertFolderMutable).not.toHaveBeenCalled()
+    expect(mockPerformCreateWorkflow).not.toHaveBeenCalled()
+  })
+
+  it('checks folder containment before mutability', async () => {
+    const order: string[] = []
+    mockAssertFolderInWorkspace.mockImplementation(async () => {
+      order.push('containment')
+    })
+    mockAssertFolderMutable.mockImplementation(async () => {
+      order.push('mutability')
+    })
+
+    await callPost({ ...VALID_BODY, folderId: 'fld-1' })
+
+    expect(order).toEqual(['containment', 'mutability'])
+    expect(mockAssertFolderInWorkspace).toHaveBeenCalledWith('fld-1', 'workspace-1')
+  })
+
+  it('skips the containment check when no folder is supplied', async () => {
+    await callPost(VALID_BODY)
+    expect(mockAssertFolderInWorkspace).not.toHaveBeenCalled()
+    expect(mockAssertFolderMutable).toHaveBeenCalledWith(null)
+  })
+
   it('409s when the name is already taken in the target folder', async () => {
     mockPerformCreateWorkflow.mockResolvedValue({
       success: false,

apps/sim/app/api/v2/workflows/route.ts

@@ -1,7 +1,12 @@
 import { db } from '@sim/db'
 import { workflow } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
-import { assertFolderMutable, FolderLockedError } from '@sim/platform-authz/workflow'
+import {
+  assertFolderInWorkspace,
+  assertFolderMutable,
+  FolderLockedError,
+  FolderNotFoundError,
+} from '@sim/platform-authz/workflow'
 import { getErrorMessage } from '@sim/utils/errors'
 import { generateId } from '@sim/utils/id'
 import { and, asc, eq, gt, isNull, or } from 'drizzle-orm'
@@ -180,6 +185,13 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
     const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'write')
     if (access) return v2WorkspaceAccessError(access)
 
+    /**
+     * Ownership before lock state: `assertFolderMutable` walks the folder's
+     * ancestor chain without filtering on workspace, so checking it first would
+     * let a caller distinguish a locked folder in someone else's workspace
+     * (423) from one that simply does not exist (400).
+     */
+    if (folderId) await assertFolderInWorkspace(folderId, workspaceId)
     await assertFolderMutable(folderId ?? null)
 
     const result = await performCreateWorkflow({
@@ -212,6 +224,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
 
     return v2Data(item, { rateLimit, status: 201 })
   } catch (error) {
+    if (error instanceof FolderNotFoundError) return v2Error('BAD_REQUEST', error.message)
     if (error instanceof FolderLockedError) return v2Error('LOCKED', error.message)
 
     logger.error(`[${requestId}] Workflow create error`, {