Skip to content

Commit ee0ea1e

Browse files
committed
fix(tables): validate reference targets
1 parent 4041952 commit ee0ea1e

11 files changed

Lines changed: 269 additions & 21 deletions

File tree

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/workflow-group-meta-cell.test.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ vi.mock('@/lib/table/column-types', () => ({
4646
columnTypeOf: (column: ColumnDefinition) => ({
4747
icon: () => null,
4848
label: column.type === 'reference' ? 'Reference' : 'Text',
49-
hasConfiguration: column.type === 'reference',
5049
}),
5150
}))
5251

apps/sim/lib/table/__tests__/column-type-registry.test.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,6 @@ describe('registry shape', () => {
4949

5050
expect(definition.label).toBe('Reference')
5151
expect(definition.icon).toBe(TableIcon)
52-
expect(definition.requiresConfigurationOnCreate).toBe(true)
53-
expect(definition.hasConfiguration).toBe(true)
5452
expect(definition.ownedMetadata).toEqual(['referenceTableId'])
5553
expect(definition.jsonbCast).toBeNull()
5654
})

apps/sim/lib/table/column-types/reference.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ export const referenceColumnType: ColumnTypeDefinition = {
1010
jsonbCast: null,
1111
storesOpaqueIds: false,
1212
supportsUnique: true,
13-
requiresConfigurationOnCreate: true,
14-
hasConfiguration: true,
1513
sampleValue: 'row_123',
1614
ownedMetadata: ['referenceTableId'],
1715
workflowInputType: 'string',
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
5+
import { hasMockCondition, schemaMock } from '@sim/testing'
6+
import { describe, expect, it, vi } from 'vitest'
7+
import { assertColumnReferencesInWorkspace } from '@/lib/table/column-types/registry.server'
8+
import type { DbTransaction } from '@/lib/table/planner'
9+
10+
function transactionWithTargets(targetIds: string[]) {
11+
const where = vi.fn().mockResolvedValue(targetIds.map((id) => ({ id })))
12+
const from = vi.fn(() => ({ where }))
13+
const select = vi.fn(() => ({ from }))
14+
return {
15+
trx: { select } as unknown as DbTransaction,
16+
select,
17+
where,
18+
}
19+
}
20+
21+
describe('assertColumnReferencesInWorkspace', () => {
22+
it('skips the database when no column type references a table', async () => {
23+
const { trx, select } = transactionWithTargets([])
24+
25+
await assertColumnReferencesInWorkspace(trx, 'ws_1', [
26+
{ id: 'col_name', name: 'Name', type: 'string' },
27+
])
28+
29+
expect(select).not.toHaveBeenCalled()
30+
})
31+
32+
it('accepts active Reference targets returned for the workspace', async () => {
33+
const { trx, select, where } = transactionWithTargets(['tbl_accounts', 'tbl_companies'])
34+
35+
await assertColumnReferencesInWorkspace(trx, 'ws_1', [
36+
{
37+
id: 'col_account',
38+
name: 'Account',
39+
type: 'reference',
40+
referenceTableId: 'tbl_accounts',
41+
},
42+
{
43+
id: 'col_company',
44+
name: 'Company',
45+
type: 'reference',
46+
referenceTableId: 'tbl_companies',
47+
},
48+
{
49+
id: 'col_duplicate',
50+
name: 'Duplicate',
51+
type: 'reference',
52+
referenceTableId: 'tbl_accounts',
53+
},
54+
])
55+
56+
expect(select).toHaveBeenCalledOnce()
57+
const condition = where.mock.calls[0][0]
58+
expect(hasMockCondition(condition, (node) => node.type === 'eq' && node.right === 'ws_1')).toBe(
59+
true
60+
)
61+
expect(
62+
hasMockCondition(
63+
condition,
64+
(node) =>
65+
node.type === 'inArray' &&
66+
node.column === schemaMock.userTableDefinitions.id &&
67+
Array.isArray(node.values) &&
68+
node.values.length === 2
69+
)
70+
).toBe(true)
71+
expect(
72+
hasMockCondition(
73+
condition,
74+
(node) =>
75+
node.type === 'isNull' && node.column === schemaMock.userTableDefinitions.archivedAt
76+
)
77+
).toBe(true)
78+
})
79+
80+
it('conceals missing, archived, and cross-workspace targets as not found', async () => {
81+
const { trx } = transactionWithTargets(['tbl_accounts'])
82+
83+
await expect(
84+
assertColumnReferencesInWorkspace(trx, 'ws_1', [
85+
{
86+
id: 'col_account',
87+
name: 'Account',
88+
type: 'reference',
89+
referenceTableId: 'tbl_accounts',
90+
},
91+
{
92+
id: 'col_company',
93+
name: 'Company',
94+
type: 'reference',
95+
referenceTableId: 'tbl_unavailable',
96+
},
97+
])
98+
).rejects.toMatchObject({
99+
code: 'not_found',
100+
message: 'Reference table "tbl_unavailable" not found in this workspace',
101+
})
102+
})
103+
})

apps/sim/lib/table/column-types/registry.server.ts

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@
1111
* under any other type. `currency` needs only the inbound one.
1212
*/
1313

14-
import { userTableRows } from '@sim/db/schema'
15-
import { and, eq, sql } from 'drizzle-orm'
14+
import { userTableDefinitions, userTableRows } from '@sim/db/schema'
15+
import { and, eq, inArray, isNull, sql } from 'drizzle-orm'
16+
import { OrchestrationError } from '@/lib/core/orchestration/types'
1617
import { COLUMN_TYPE_REGISTRY } from '@/lib/table/column-types/registry'
1718
import type { ColumnType } from '@/lib/table/column-types/types'
1819
import type {
@@ -21,7 +22,7 @@ import type {
2122
} from '@/lib/table/column-types/types.server'
2223
import type { DbTransaction } from '@/lib/table/planner'
2324
import { updateTableRowsWithDerivedSecretProvenance } from '@/lib/table/rows/secret-provenance'
24-
import type { JsonValue, SelectOption } from '@/lib/table/types'
25+
import type { ColumnDefinition, JsonValue, SelectOption } from '@/lib/table/types'
2526

2627
/**
2728
* Rewrites a column's cells from stored option **ids** to option **names**, for
@@ -290,7 +291,51 @@ export const COLUMN_TYPE_SERVER_REGISTRY: Record<ColumnType, ColumnTypeServerEnt
290291
migrateSelectCellsToNames(trx, tableId, workspaceId, columnKey, previous.options ?? []),
291292
},
292293
currency: COLUMN_TYPE_REGISTRY.currency,
293-
reference: COLUMN_TYPE_REGISTRY.reference,
294+
reference: {
295+
...COLUMN_TYPE_REGISTRY.reference,
296+
referencedTableIds: (column) =>
297+
typeof column.referenceTableId === 'string' ? [column.referenceTableId] : [],
298+
},
299+
}
300+
301+
/**
302+
* Validates every table ID referenced by column metadata in one query.
303+
*
304+
* This intentionally validates only the target table. Cell values remain
305+
* opaque row-ID strings and are never checked for existence.
306+
*/
307+
export async function assertColumnReferencesInWorkspace(
308+
trx: DbTransaction,
309+
workspaceId: string,
310+
columns: readonly ColumnDefinition[]
311+
): Promise<void> {
312+
const referencedTableIds = [
313+
...new Set(
314+
columns.flatMap(
315+
(column) => COLUMN_TYPE_SERVER_REGISTRY[column.type].referencedTableIds?.(column) ?? []
316+
)
317+
),
318+
]
319+
if (referencedTableIds.length === 0) return
320+
321+
const targets = await trx
322+
.select({ id: userTableDefinitions.id })
323+
.from(userTableDefinitions)
324+
.where(
325+
and(
326+
eq(userTableDefinitions.workspaceId, workspaceId),
327+
inArray(userTableDefinitions.id, referencedTableIds),
328+
isNull(userTableDefinitions.archivedAt)
329+
)
330+
)
331+
const foundIds = new Set(targets.map((target) => target.id))
332+
const missingId = referencedTableIds.find((id) => !foundIds.has(id))
333+
if (missingId) {
334+
throw new OrchestrationError(
335+
'not_found',
336+
`Reference table "${missingId}" not found in this workspace`
337+
)
338+
}
294339
}
295340

296341
/** The inbound migration for a target type, if it has one. */

apps/sim/lib/table/column-types/types.server.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
/**
2-
* The server-only half of a column type: rewriting stored cells when a column
3-
* is converted into or out of this type.
2+
* The server-only half of a column type: database-backed definition checks and
3+
* stored-cell rewrites for conversion into or out of the type.
44
*
55
* Separate from `types.ts` so the client-safe definition never references a
6-
* drizzle transaction type. Mirrors `connectors/`'s `ConnectorMeta` /
6+
* Drizzle transaction type. Mirrors `connectors/`'s `ConnectorMeta` /
77
* `ConnectorConfig` split.
88
*/
99

@@ -31,6 +31,12 @@ export interface ColumnCellMigrationContext {
3131
export type ColumnCellMigration = (context: ColumnCellMigrationContext) => Promise<void>
3232

3333
export interface ColumnTypeServerDefinition {
34+
/**
35+
* Table IDs named by this column's type-specific metadata. The server
36+
* registry uses this to validate cross-table references in one batch before
37+
* a schema is persisted. Omitted by types that do not reference tables.
38+
*/
39+
readonly referencedTableIds?: (column: ColumnDefinition) => readonly string[]
3440
/**
3541
* Rewrites cells into this type's canonical storage shape when a column is
3642
* converted **to** it. Omitted when the stored bytes are already correct.

apps/sim/lib/table/column-types/types.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
* `scripts/check-client-boundary-imports.ts` only forbids calling a
1313
* `'use client'` export from a server surface). It must NOT reach `@sim/db`,
1414
* `drizzle-orm`, or `next/server` — the tables grid imports it directly.
15-
* - `ColumnTypeServerDefinition` (in `types.server.ts`) adds the one genuinely
16-
* server-only concern: rewriting stored cells inside a transaction.
15+
* - `ColumnTypeServerDefinition` (in `types.server.ts`) adds database-backed
16+
* definition checks and stored-cell rewrites inside a transaction.
1717
*
1818
* This mirrors `connectors/types.ts`'s `ConnectorMeta` / `ConnectorConfig`
1919
* split and its `registry.ts` / `registry.server.ts` pair.

apps/sim/lib/table/columns/reference-metadata.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,21 @@ import type { TableDefinition } from '@/lib/table/types'
88

99
const mocks = vi.hoisted(() => ({
1010
withLockedTable: vi.fn(),
11+
assertColumnReferencesInWorkspace: vi.fn(),
12+
migrationFrom: vi.fn(),
13+
migrationTo: vi.fn(),
14+
writeBackCoercedCells: vi.fn(),
1115
set: vi.fn(),
1216
where: vi.fn(),
1317
}))
1418

1519
vi.mock('@/lib/table/service', () => ({ withLockedTable: mocks.withLockedTable }))
20+
vi.mock('@/lib/table/column-types/registry.server', () => ({
21+
assertColumnReferencesInWorkspace: mocks.assertColumnReferencesInWorkspace,
22+
migrationFrom: mocks.migrationFrom,
23+
migrationTo: mocks.migrationTo,
24+
writeBackCoercedCells: mocks.writeBackCoercedCells,
25+
}))
1626

1727
import {
1828
addTableColumn,
@@ -50,6 +60,10 @@ function tableWithReference(referenceTableId = 'tbl_accounts'): TableDefinition
5060
describe('reference column metadata persistence', () => {
5161
beforeEach(() => {
5262
vi.clearAllMocks()
63+
mocks.assertColumnReferencesInWorkspace.mockResolvedValue(undefined)
64+
mocks.migrationFrom.mockReturnValue(undefined)
65+
mocks.migrationTo.mockReturnValue(undefined)
66+
mocks.writeBackCoercedCells.mockResolvedValue(undefined)
5367
mocks.where.mockResolvedValue(undefined)
5468
mocks.set.mockReturnValue({ where: mocks.where })
5569
})
@@ -87,6 +101,11 @@ describe('reference column metadata persistence', () => {
87101
type: 'reference',
88102
referenceTableId: 'tbl_accounts',
89103
})
104+
expect(mocks.assertColumnReferencesInWorkspace).toHaveBeenCalledWith(
105+
expect.anything(),
106+
'ws_1',
107+
[expect.objectContaining({ referenceTableId: 'tbl_accounts' })]
108+
)
90109
})
91110

92111
it('retains the supplied target when converting a column to reference', async () => {
@@ -107,6 +126,11 @@ describe('reference column metadata persistence', () => {
107126
type: 'reference',
108127
referenceTableId: 'tbl_accounts',
109128
})
129+
expect(mocks.assertColumnReferencesInWorkspace).toHaveBeenCalledWith(
130+
expect.anything(),
131+
'ws_1',
132+
[expect.objectContaining({ referenceTableId: 'tbl_accounts' })]
133+
)
110134
})
111135

112136
it('changes a reference target without reading or rewriting rows', async () => {
@@ -122,6 +146,11 @@ describe('reference column metadata persistence', () => {
122146
)
123147

124148
expect(updated.schema.columns[0]).toMatchObject({ referenceTableId: 'tbl_companies' })
149+
expect(mocks.assertColumnReferencesInWorkspace).toHaveBeenCalledWith(
150+
expect.anything(),
151+
'ws_1',
152+
[expect.objectContaining({ referenceTableId: 'tbl_companies' })]
153+
)
125154
expect(trx.select).not.toHaveBeenCalled()
126155
expect(trx.execute).not.toHaveBeenCalled()
127156
expect(trx.update).toHaveBeenCalledOnce()
@@ -144,6 +173,24 @@ describe('reference column metadata persistence', () => {
144173
expect(trx.update).not.toHaveBeenCalled()
145174
})
146175

176+
it('leaves the source schema unchanged when the target table is unavailable', async () => {
177+
const trx = useTable(tableWithReference())
178+
mocks.assertColumnReferencesInWorkspace.mockRejectedValueOnce({ code: 'not_found' })
179+
180+
await expect(
181+
updateColumnReference(
182+
{
183+
tableId: 'tbl_people',
184+
columnName: 'col_account',
185+
referenceTableId: 'tbl_missing',
186+
},
187+
'req_1'
188+
)
189+
).rejects.toMatchObject({ code: 'not_found' })
190+
191+
expect(trx.update).not.toHaveBeenCalled()
192+
})
193+
147194
it('returns the locked table unchanged when the target is already set', async () => {
148195
const table = tableWithReference()
149196
const trx = useTable(table)

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
valueForTypeConversion,
3131
} from '@/lib/table/column-types'
3232
import {
33+
assertColumnReferencesInWorkspace,
3334
migrationFrom,
3435
migrationTo,
3536
writeBackCoercedCells,
@@ -196,6 +197,7 @@ export async function addTableColumn(
196197
`Invalid column: ${columnValidation.errors.join('; ')}`
197198
)
198199
}
200+
await assertColumnReferencesInWorkspace(trx, table.workspaceId, [newColumn])
199201

200202
const newColumnId = getColumnId(newColumn)
201203

@@ -964,6 +966,7 @@ export async function updateColumnType(
964966
isSelectType,
965967
targetMultiple: !!targetMultiple,
966968
})
969+
await assertColumnReferencesInWorkspace(trx, table.workspaceId, [convertedColumn])
967970
const renamedColumns = schema.columns.map((c, i) => (i === columnIndex ? convertedColumn : c))
968971
const updatedColumns = renamedColumns.map((c, i) =>
969972
i === columnIndex ? applyPendingRename(renamedColumns, columnIndex, data.newName) : c
@@ -1480,8 +1483,8 @@ export async function updateColumnCurrency(
14801483
* Changes the table targeted by a `reference` column.
14811484
*
14821485
* Cells already store plain row-ID strings, so changing the target updates only
1483-
* the column schema. The target is deliberately not loaded or validated here;
1484-
* dangling table and row IDs are valid reference values for now.
1486+
* the column schema. The target must be an active table in the same workspace;
1487+
* stored row IDs remain opaque strings and are not checked for existence.
14851488
*/
14861489
export async function updateColumnReference(
14871490
data: UpdateColumnReferenceData,
@@ -1520,6 +1523,7 @@ export async function updateColumnReference(
15201523
`Invalid column: ${columnValidation.errors.join('; ')}`
15211524
)
15221525
}
1526+
await assertColumnReferencesInWorkspace(trx, table.workspaceId, [updatedColumn])
15231527

15241528
const constrained = await applyConstraints(
15251529
trx,

0 commit comments

Comments
 (0)