Skip to content

Commit ad29585

Browse files
fix(tables): restore audit provenance and conflict status in orchestration
Moving the audits into the orchestration functions dropped three things the routes had been carrying, and added one the orchestration now owns twice. - The v1 and v2 column-update routes passed `request` to `recordAudit`, so their audit rows recorded the caller's IP and user-agent. The orchestration function had no way to receive it. Every table orchestration function now takes an optional `OrchestrationRequestContext` and every HTTP route forwards it; the copilot and VFS callers, which have no request, omit it. - `classifyTableMutation` matched `TableConflictError` on "already exists" appearing in the message and reported it as `validation`, turning the UI route's 409 on a duplicate table rename into a 400. It now matches the type, the way `performRestoreTable` already did. - `captureServerEvent` ran on every delete while the audit was gated on a row actually being archived, so a repeat delete of an archived table still reported `table_deleted`. Both now hang off the same evidence. - The copilot delete path kept its own `captureServerEvent` from when the service did not emit one, double-counting every copilot table delete. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGzbVDZpe2dEALbu2BUU8a
1 parent fc0e119 commit ad29585

11 files changed

Lines changed: 121 additions & 37 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
117117
userId: authResult.userId,
118118
updates: validated.updates,
119119
requestId,
120+
request,
120121
})
121122
if (!outcome.success || !outcome.table) {
122123
return NextResponse.json(

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ export const PATCH = withRouteHandler(
199199
newName: validated.name,
200200
userId: authResult.userId,
201201
requestId,
202+
request,
202203
})
203204
if (!renameOutcome.success) {
204205
return NextResponse.json(
@@ -224,6 +225,7 @@ export const PATCH = withRouteHandler(
224225
folderId: validated.folderId,
225226
userId: authResult.userId,
226227
requestId,
228+
request,
227229
})
228230
if (!moveOutcome.success) {
229231
return NextResponse.json(
@@ -289,7 +291,12 @@ export const DELETE = withRouteHandler(
289291
return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 })
290292
}
291293

292-
const outcome = await performDeleteTable({ table, userId: authResult.userId, requestId })
294+
const outcome = await performDeleteTable({
295+
table,
296+
userId: authResult.userId,
297+
requestId,
298+
request,
299+
})
293300
if (!outcome.success) {
294301
return NextResponse.json(
295302
{ error: outcome.error ?? 'Failed to delete table' },

apps/sim/app/api/v1/tables/[tableId]/columns/route.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
151151
userId,
152152
updates: validated.updates,
153153
requestId,
154+
request,
154155
})
155156
if (!outcome.success || !outcome.table) {
156157
return NextResponse.json(

apps/sim/app/api/v1/tables/[tableId]/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Tab
140140
return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 })
141141
}
142142

143-
const outcome = await performDeleteTable({ table: result.table, userId, requestId })
143+
const outcome = await performDeleteTable({ table: result.table, userId, requestId, request })
144144
if (!outcome.success) {
145145
return NextResponse.json(
146146
{ error: outcome.error ?? 'Failed to delete table' },

apps/sim/app/api/v2/tables/[tableId]/columns/route.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
138138
userId,
139139
updates: validated.updates,
140140
requestId,
141+
request,
141142
})
142143
if (!outcome.success || !outcome.table) {
143144
return v2ErrorForOrchestration(outcome.errorCode, outcome.error ?? 'Failed to update column')

apps/sim/app/api/v2/tables/[tableId]/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Tab
100100
return v2Error('NOT_FOUND', 'Table not found')
101101
}
102102

103-
const outcome = await performDeleteTable({ table: result.table, userId, requestId })
103+
const outcome = await performDeleteTable({ table: result.table, userId, requestId, request })
104104
if (!outcome.success) {
105105
return v2ErrorForOrchestration(outcome.errorCode, outcome.error ?? 'Failed to delete table')
106106
}

apps/sim/lib/copilot/tools/server/table/user-table.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {
1010
} from '@/lib/copilot/tools/server/base-tool'
1111
import { isTriggerDevEnabled } from '@/lib/core/config/env-flags'
1212
import { runDetached } from '@/lib/core/utils/background'
13-
import { captureServerEvent } from '@/lib/posthog/server'
1413
import {
1514
buildAutoMapping,
1615
COLUMN_TYPES,
@@ -502,12 +501,6 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
502501
if (!deleteOutcome.success) {
503502
return { success: false, message: deleteOutcome.error ?? 'Failed to delete table' }
504503
}
505-
captureServerEvent(
506-
context.userId,
507-
'table_deleted',
508-
{ table_id: tableId, workspace_id: workspaceId },
509-
{ groups: { workspace: workspaceId } }
510-
)
511504
deleted.push(tableId)
512505
}
513506

apps/sim/lib/core/orchestration/types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,12 @@ export function statusForOrchestrationError(code: OrchestrationErrorCode | undef
1212
if (code === 'locked') return 423
1313
return 500
1414
}
15+
16+
/**
17+
* The slice of an HTTP request the audit log reads for client IP and user-agent
18+
* capture. Optional on every orchestration function so the non-HTTP callers —
19+
* copilot tools, background jobs — can omit what they do not have.
20+
*/
21+
export interface OrchestrationRequestContext {
22+
headers: { get(name: string): string | null }
23+
}

apps/sim/lib/table/orchestration/columns.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
22
import { createLogger } from '@sim/logger'
3-
import type { OrchestrationErrorCode } from '@/lib/core/orchestration/types'
3+
import type {
4+
OrchestrationErrorCode,
5+
OrchestrationRequestContext,
6+
} from '@/lib/core/orchestration/types'
47
import { generateRequestId } from '@/lib/core/utils/request'
58
import { columnMatchesRef, getColumnId } from '@/lib/table/column-keys'
69
import { columnTypeById } from '@/lib/table/column-types'
@@ -33,6 +36,8 @@ export interface PerformUpdateTableColumnParams {
3336
currencyCode?: string
3437
}
3538
requestId?: string
39+
/** Forwarded to the audit record for IP / user-agent capture. */
40+
request?: OrchestrationRequestContext
3641
}
3742

3843
export interface PerformUpdateTableColumnResult {
@@ -96,7 +101,7 @@ function fail(error: string, errorCode: OrchestrationErrorCode): PerformUpdateTa
96101
export async function performUpdateTableColumn(
97102
params: PerformUpdateTableColumnParams
98103
): Promise<PerformUpdateTableColumnResult> {
99-
const { table, columnName, userId, updates } = params
104+
const { table, columnName, userId, updates, request } = params
100105
const requestId = params.requestId ?? generateRequestId()
101106
const tableId = table.id
102107

@@ -269,6 +274,7 @@ export async function performUpdateTableColumn(
269274
resourceName: table.name,
270275
description: `Updated column "${columnName}" in table "${table.name}"`,
271276
metadata: { columnName, updates },
277+
...(request ? { request } : {}),
272278
})
273279

274280
return { success: true, table: updated }

apps/sim/lib/table/orchestration/tables.test.ts

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,26 @@
44
import { beforeEach, describe, expect, it, vi } from 'vitest'
55
import type { TableDefinition } from '@/lib/table/types'
66

7-
const { mockDeleteTable, mockDeleteRow, mockCaptureServerEvent, mockRecordAudit } = vi.hoisted(
8-
() => ({
9-
mockDeleteTable: vi.fn(),
10-
mockDeleteRow: vi.fn(),
11-
mockCaptureServerEvent: vi.fn(),
12-
mockRecordAudit: vi.fn(),
13-
})
14-
)
7+
const {
8+
mockDeleteTable,
9+
mockDeleteRow,
10+
mockRenameTable,
11+
mockCaptureServerEvent,
12+
mockRecordAudit,
13+
MockTableConflictError,
14+
} = vi.hoisted(() => ({
15+
mockDeleteTable: vi.fn(),
16+
mockDeleteRow: vi.fn(),
17+
mockRenameTable: vi.fn(),
18+
mockCaptureServerEvent: vi.fn(),
19+
mockRecordAudit: vi.fn(),
20+
MockTableConflictError: class extends Error {
21+
readonly code = 'TABLE_EXISTS' as const
22+
constructor(name: string) {
23+
super(`A table named "${name}" already exists in this workspace`)
24+
}
25+
},
26+
}))
1527

1628
vi.mock('@sim/audit', () => ({
1729
AuditAction: { TABLE_DELETED: 'table.deleted', TABLE_UPDATED: 'table.updated' },
@@ -22,14 +34,19 @@ vi.mock('@sim/audit', () => ({
2234
vi.mock('@/lib/table/service', () => ({
2335
deleteTable: mockDeleteTable,
2436
moveTableToFolder: vi.fn(),
25-
renameTable: vi.fn(),
37+
renameTable: mockRenameTable,
2638
updateTableLocks: vi.fn(),
39+
TableConflictError: MockTableConflictError,
2740
}))
2841
vi.mock('@/lib/table/rows/service', () => ({ deleteRow: mockDeleteRow }))
2942
vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mockCaptureServerEvent }))
3043

3144
import { TableLockedError } from '@/lib/table/mutation-locks'
32-
import { performDeleteTable, performDeleteTableRow } from '@/lib/table/orchestration/tables'
45+
import {
46+
performDeleteTable,
47+
performDeleteTableRow,
48+
performRenameTable,
49+
} from '@/lib/table/orchestration/tables'
3350

3451
const TABLE = { id: 'table-1', name: 'Tasks', workspaceId: 'ws-1' } as unknown as TableDefinition
3552

@@ -56,13 +73,23 @@ describe('performDeleteTable', () => {
5673
)
5774
})
5875

59-
it('does not audit a repeat delete of an already-archived table', async () => {
76+
it('carries request provenance into the audit row', async () => {
77+
mockDeleteTable.mockResolvedValue({ archived: { name: 'Tasks', workspaceId: 'ws-1' } })
78+
const request = new Request('https://sim.ai', { headers: { 'user-agent': 'curl/8' } })
79+
80+
await performDeleteTable({ table: TABLE, userId: 'user-1', request })
81+
82+
expect(mockRecordAudit).toHaveBeenCalledWith(expect.objectContaining({ request }))
83+
})
84+
85+
it('neither audits nor reports a repeat delete of an already-archived table', async () => {
6086
mockDeleteTable.mockResolvedValue({ archived: null })
6187

6288
const result = await performDeleteTable({ table: TABLE, userId: 'user-1' })
6389

6490
expect(result.success).toBe(true)
6591
expect(mockRecordAudit).not.toHaveBeenCalled()
92+
expect(mockCaptureServerEvent).not.toHaveBeenCalled()
6693
})
6794

6895
it('classifies a delete lock as locked and emits no telemetry', async () => {
@@ -75,6 +102,18 @@ describe('performDeleteTable', () => {
75102
})
76103
})
77104

105+
describe('performRenameTable', () => {
106+
beforeEach(() => vi.clearAllMocks())
107+
108+
it('classifies a name collision as a conflict, not bad input', async () => {
109+
mockRenameTable.mockRejectedValue(new MockTableConflictError('Tasks'))
110+
111+
const result = await performRenameTable({ table: TABLE, newName: 'Tasks', userId: 'user-1' })
112+
113+
expect(result).toMatchObject({ success: false, errorCode: 'conflict' })
114+
})
115+
})
116+
78117
describe('performDeleteTableRow', () => {
79118
beforeEach(() => vi.clearAllMocks())
80119

0 commit comments

Comments
 (0)