Skip to content

Commit 334575d

Browse files
committed
fix(tables): validate reference targets
1 parent 640b6dc commit 334575d

11 files changed

Lines changed: 265 additions & 16 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
@@ -48,8 +48,6 @@ describe('registry shape', () => {
4848

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

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

295340
/** 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
@@ -29,6 +29,7 @@ import {
2929
TYPE_SPECIFIC_COLUMN_KEYS,
3030
} from '@/lib/table/column-types'
3131
import {
32+
assertColumnReferencesInWorkspace,
3233
migrationFrom,
3334
migrationTo,
3435
writeBackCoercedCells,
@@ -192,6 +193,7 @@ export async function addTableColumn(
192193
`Invalid column: ${columnValidation.errors.join('; ')}`
193194
)
194195
}
196+
await assertColumnReferencesInWorkspace(trx, table.workspaceId, [newColumn])
195197

196198
const newColumnId = getColumnId(newColumn)
197199

@@ -950,6 +952,7 @@ export async function updateColumnType(
950952
isSelectType,
951953
targetMultiple: !!targetMultiple,
952954
})
955+
await assertColumnReferencesInWorkspace(trx, table.workspaceId, [convertedColumn])
953956

954957
let incompatibleCount = 0
955958
let blankCount = 0
@@ -1465,8 +1468,8 @@ export async function updateColumnCurrency(
14651468
* Changes the table targeted by a `reference` column.
14661469
*
14671470
* Cells already store plain row-ID strings, so changing the target updates only
1468-
* the column schema. The target is deliberately not loaded or validated here;
1469-
* dangling table and row IDs are valid reference values for now.
1471+
* the column schema. The target must be an active table in the same workspace;
1472+
* stored row IDs remain opaque strings and are not checked for existence.
14701473
*/
14711474
export async function updateColumnReference(
14721475
data: UpdateColumnReferenceData,
@@ -1505,6 +1508,7 @@ export async function updateColumnReference(
15051508
`Invalid column: ${columnValidation.errors.join('; ')}`
15061509
)
15071510
}
1511+
await assertColumnReferencesInWorkspace(trx, table.workspaceId, [updatedColumn])
15081512

15091513
const constrained = await applyConstraints(
15101514
trx,

0 commit comments

Comments
 (0)