refactor: enforce Copilot table application boundary (#6453)

* refactor: enforce copilot table application boundary

* fix(tables): finish application boundary migration

* fix(tables): restore scoped copilot imports

* fix(tables): compose copilot commands atomically

* fix(tables): preserve workflow group scheduling

* fix(tables): complete fixed copilot composition

* fix(tables): reject enrichment output mutation

* fix(tables): complete authorized application boundary

Author: TheodoreSpeaks

apps/sim/app/api/table/[tableId]/exports/route.ts

@@ -1,39 +1,27 @@
-import { type NextRequest, NextResponse } from 'next/server'
 import { createTableExportResourceContract } from '@/lib/api/contracts/table-transfers'
-import { parseRequest } from '@/lib/api/server'
-import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
-import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import {
-  createTableExportResource,
-  toV2TableExport,
-} from '@/lib/table/orchestration/export-resource'
-import { accessError, checkAccess, orchestrationErrorResponse } from '@/app/api/table/utils'
+  defineInternalJsonRoute,
+  internalPlainOrchestrationErrorPolicy,
+  internalRateLimits,
+} from '@/lib/api/server/routes'
+import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
+import { createTableExportUseCase } from '@/lib/table/application/exports'
+import { tableOperations } from '@/lib/table/application/operations'
+import { toV2TableExport } from '@/lib/table/orchestration/export-resource'
 
-interface TableRouteParams {
-  params: Promise<{ tableId: string }>
-}
-
-export const POST = withRouteHandler(async (request: NextRequest, context: TableRouteParams) => {
-  const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false })
-  if (!auth.success || !auth.userId) {
-    return NextResponse.json({ error: 'Authentication required' }, { status: 401 })
-  }
-  const parsed = await parseRequest(createTableExportResourceContract, request, context)
-  if (!parsed.success) return parsed.response
-  const access = await checkAccess(parsed.data.params.tableId, auth.userId, 'read')
-  if (!access.ok) return accessError(access, 'table-export')
-  if (access.table.workspaceId !== parsed.data.body.workspaceId) {
-    return NextResponse.json({ error: 'Table not found' }, { status: 404 })
-  }
-  try {
-    const record = await createTableExportResource({
-      table: access.table,
-      format: parsed.data.body.format,
-    })
-    return NextResponse.json({ data: toV2TableExport(record, true) }, { status: 201 })
-  } catch (error) {
-    const classified = orchestrationErrorResponse(error)
-    if (classified) return classified
-    throw error
-  }
+export const POST = defineInternalJsonRoute({
+  contract: createTableExportResourceContract,
+  auth: internalTableSessionOrExecutorAuth,
+  operation: tableOperations.createExport,
+  rateLimit: internalRateLimits.none({
+    reason: 'Existing authenticated table export creation has no request-rate policy',
+  }),
+  errorPolicy: internalPlainOrchestrationErrorPolicy,
+  mapInput: ({ params, body }) => ({
+    tableId: params.tableId,
+    workspaceId: body.workspaceId,
+    format: body.format,
+  }),
+  useCase: createTableExportUseCase,
+  present: ({ export: record }) => ({ data: toV2TableExport(record, true) }),
 })

apps/sim/app/api/table/[tableId]/groups/route.test.ts

@@ -1,209 +1,73 @@
 /**
  * @vitest-environment node
  */
-import { hybridAuthMockFns, workflowAuthzMockFns } from '@sim/testing'
-import { NextRequest } from 'next/server'
-import { beforeEach, describe, expect, it, vi } from 'vitest'
-import type { TableDefinition } from '@/lib/table'
+import { describe, expect, it, vi } from 'vitest'
 
-const { mockCheckAccess, mockAddWorkflowGroup, mockUpdateWorkflowGroup } = vi.hoisted(() => ({
-  mockCheckAccess: vi.fn(),
-  mockAddWorkflowGroup: vi.fn(),
-  mockUpdateWorkflowGroup: vi.fn(),
-}))
+interface CapturedDefinition {
+  contract: { method: string; path: string }
+  auth: unknown
+  operation: { id: string }
+  useCase: unknown
+}
 
-vi.mock('@/app/api/table/utils', async () => {
-  const { NextResponse } = await import('next/server')
-  return {
-    accessError: (result: { status: number }) =>
-      NextResponse.json({ error: 'denied' }, { status: result.status }),
-    checkAccess: mockCheckAccess,
-    normalizeColumn: (column: unknown) => column,
-  }
-})
+const mocks = vi.hoisted(() => ({
+  auth: { kind: 'session-or-executor' },
+  definitions: [] as CapturedDefinition[],
+  useCases: {
+    create: { operation: { id: 'tables.groups.create' } },
+    remove: { operation: { id: 'tables.groups.delete' } },
+    update: { operation: { id: 'tables.groups.update' } },
+  },
+}))
 
-vi.mock('@/lib/table/workflow-groups/service', () => ({
-  addWorkflowGroup: mockAddWorkflowGroup,
-  updateWorkflowGroup: mockUpdateWorkflowGroup,
-  deleteWorkflowGroup: vi.fn(),
+vi.mock('@/lib/api/server/routes', () => ({
+  defineInternalJsonRoute: (definition: CapturedDefinition) => {
+    mocks.definitions.push(definition)
+    return vi.fn()
+  },
+  extendInternalErrorPolicy: vi.fn(() => ({ kind: 'table' })),
+  internalErrorResponse: vi.fn(),
+  internalPlainOrchestrationErrorPolicy: { kind: 'plain' },
+  internalRateLimits: {
+    none: ({ reason }: { reason: string }) => ({ kind: 'none', reason }),
+  },
 }))
 
-import { PATCH, POST } from '@/app/api/table/[tableId]/groups/route'
+vi.mock('@/lib/table/api', () => ({ internalTableSessionOrExecutorAuth: mocks.auth }))
 
-function buildTable(overrides: Partial<TableDefinition> = {}): TableDefinition {
-  return {
-    id: 'tbl_1',
-    name: 'People',
-    description: null,
-    schema: { columns: [] },
-    metadata: null,
-    rowCount: 0,
-    maxRows: 100,
-    workspaceId: 'workspace-1',
-    createdBy: 'user-1',
-    archivedAt: null,
-    createdAt: new Date('2024-01-01'),
-    updatedAt: new Date('2024-01-01'),
-    ...overrides,
-  }
-}
+vi.mock('@/lib/table/application/groups', () => ({
+  createTableGroupUseCase: mocks.useCases.create,
+  deleteTableGroupUseCase: mocks.useCases.remove,
+  updateTableGroupUseCase: mocks.useCases.update,
+}))
 
-function callPost(body: Record<string, unknown>, tableId = 'tbl_1') {
-  const req = new NextRequest(`http://localhost:3000/api/table/${tableId}/groups`, {
-    method: 'POST',
-    body: JSON.stringify(body),
-    headers: { 'Content-Type': 'application/json' },
-  })
-  return POST(req, { params: Promise.resolve({ tableId }) })
-}
+vi.mock('@/app/api/table/utils', () => ({
+  normalizeColumn: vi.fn(),
+}))
 
-function callPatch(body: Record<string, unknown>, tableId = 'tbl_1') {
-  const req = new NextRequest(`http://localhost:3000/api/table/${tableId}/groups`, {
-    method: 'PATCH',
-    body: JSON.stringify(body),
-    headers: { 'Content-Type': 'application/json' },
-  })
-  return PATCH(req, { params: Promise.resolve({ tableId }) })
-}
+import '@/app/api/table/[tableId]/groups/route'
 
-const baseGroup = {
-  id: 'grp_1',
-  workflowId: 'wf_1',
-  outputs: [{ blockId: 'block_1', path: 'result', columnName: 'result' }],
+function definition(method: string): CapturedDefinition {
+  const match = mocks.definitions.find((candidate) => candidate.contract.method === method)
+  if (!match) throw new Error(`Missing ${method} group route definition`)
+  return match
 }
 
-const baseOutputColumns = [{ name: 'result', type: 'string', workflowGroupId: 'grp_1' }]
-
-describe('POST /api/table/[tableId]/groups', () => {
-  beforeEach(() => {
-    vi.clearAllMocks()
-    hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({
-      success: true,
-      userId: 'user-1',
-      authType: 'session',
-    })
-    mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() })
-    workflowAuthzMockFns.mockGetActiveWorkflowContext.mockResolvedValue({
-      workflow: { id: 'wf_1' },
-      workspaceId: 'workspace-1',
-      workspaceOrganizationId: null,
-    })
-    mockAddWorkflowGroup.mockResolvedValue({
-      schema: { columns: baseOutputColumns, workflowGroups: [baseGroup] },
-    })
-  })
-
-  it('rejects a workflowId belonging to a different workspace', async () => {
-    workflowAuthzMockFns.mockGetActiveWorkflowContext.mockResolvedValue({
-      workflow: { id: 'wf_1' },
-      workspaceId: 'other-workspace',
-      workspaceOrganizationId: null,
-    })
-    const res = await callPost({
-      workspaceId: 'workspace-1',
-      group: baseGroup,
-      outputColumns: baseOutputColumns,
-    })
-    expect(res.status).toBe(400)
-    expect(mockAddWorkflowGroup).not.toHaveBeenCalled()
-  })
-
-  it('rejects a nonexistent workflowId', async () => {
-    workflowAuthzMockFns.mockGetActiveWorkflowContext.mockResolvedValue(null)
-    const res = await callPost({
-      workspaceId: 'workspace-1',
-      group: baseGroup,
-      outputColumns: baseOutputColumns,
-    })
-    expect(res.status).toBe(400)
-    expect(mockAddWorkflowGroup).not.toHaveBeenCalled()
-  })
-
-  it('succeeds when the workflow belongs to the same workspace', async () => {
-    const res = await callPost({
-      workspaceId: 'workspace-1',
-      group: baseGroup,
-      outputColumns: baseOutputColumns,
-    })
-    expect(res.status).toBe(200)
-    expect(mockAddWorkflowGroup).toHaveBeenCalled()
-  })
-
-  it('skips the workflow check for enrichment groups without a workflowId', async () => {
-    const res = await callPost({
-      workspaceId: 'workspace-1',
-      group: { ...baseGroup, workflowId: '', enrichmentId: 'enrich_1' },
-      outputColumns: baseOutputColumns,
-    })
-    expect(res.status).toBe(200)
-    expect(workflowAuthzMockFns.mockGetActiveWorkflowContext).not.toHaveBeenCalled()
-    expect(mockAddWorkflowGroup).toHaveBeenCalled()
-  })
-})
-
-describe('PATCH /api/table/[tableId]/groups', () => {
-  beforeEach(() => {
-    vi.clearAllMocks()
-    hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({
-      success: true,
-      userId: 'user-1',
-      authType: 'session',
-    })
-    mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() })
-    workflowAuthzMockFns.mockGetActiveWorkflowContext.mockResolvedValue({
-      workflow: { id: 'wf_1' },
-      workspaceId: 'workspace-1',
-      workspaceOrganizationId: null,
-    })
-    mockUpdateWorkflowGroup.mockResolvedValue({
-      schema: { columns: baseOutputColumns, workflowGroups: [baseGroup] },
-    })
-  })
-
-  it('rejects changing workflowId to one in a different workspace', async () => {
-    workflowAuthzMockFns.mockGetActiveWorkflowContext.mockResolvedValue({
-      workflow: { id: 'wf_2' },
-      workspaceId: 'other-workspace',
-      workspaceOrganizationId: null,
-    })
-    const res = await callPatch({
-      workspaceId: 'workspace-1',
-      groupId: 'grp_1',
-      workflowId: 'wf_2',
-    })
-    expect(res.status).toBe(400)
-    expect(mockUpdateWorkflowGroup).not.toHaveBeenCalled()
-  })
-
-  it('rejects a nonexistent workflowId', async () => {
-    workflowAuthzMockFns.mockGetActiveWorkflowContext.mockResolvedValue(null)
-    const res = await callPatch({
-      workspaceId: 'workspace-1',
-      groupId: 'grp_1',
-      workflowId: 'wf_missing',
-    })
-    expect(res.status).toBe(400)
-    expect(mockUpdateWorkflowGroup).not.toHaveBeenCalled()
-  })
-
-  it('succeeds when changing workflowId to one in the same workspace', async () => {
-    const res = await callPatch({
-      workspaceId: 'workspace-1',
-      groupId: 'grp_1',
-      workflowId: 'wf_1',
-    })
-    expect(res.status).toBe(200)
-    expect(mockUpdateWorkflowGroup).toHaveBeenCalled()
-  })
-
-  it('skips the workflow check when workflowId is not being changed', async () => {
-    const res = await callPatch({
-      workspaceId: 'workspace-1',
-      groupId: 'grp_1',
-      name: 'Renamed group',
-    })
-    expect(res.status).toBe(200)
-    expect(workflowAuthzMockFns.mockGetActiveWorkflowContext).not.toHaveBeenCalled()
-    expect(mockUpdateWorkflowGroup).toHaveBeenCalled()
+describe('/api/table/[tableId]/groups', () => {
+  it('routes every mutation through its session-or-executor application use case', () => {
+    const expected = [
+      ['POST', mocks.useCases.create],
+      ['PATCH', mocks.useCases.update],
+      ['DELETE', mocks.useCases.remove],
+    ] as const
+
+    expect(mocks.definitions).toHaveLength(expected.length)
+    for (const [method, useCase] of expected) {
+      const route = definition(method)
+      expect(route.contract.path).toBe('/api/table/[tableId]/groups')
+      expect(route.auth).toBe(mocks.auth)
+      expect(route.useCase).toBe(useCase)
+      expect(route.operation.id).toBe(useCase.operation.id)
+    }
   })
 })