Skip to content

Commit 7e2edf5

Browse files
committed
fix(tables): stop row writes 500ing on a column outside the table schema
createTableWriteProvenanceTargets (added in #6247) required every submitted column to translate to exactly one storage id and threw otherwise. The wire translator has always dropped keys naming no column in the schema, so any internal-JWT write carrying such a key threw an uncaught error and surfaced as a 500 — where the same write previously succeeded, since the write path drops the column identically. Give a dropped column a null column id instead of throwing. It still gets a target, so the bundle completeness check that pairs one selection per submitted column is unchanged, but no provenance is recorded for a value that is never stored.
1 parent 01f4b43 commit 7e2edf5

2 files changed

Lines changed: 241 additions & 8 deletions

File tree

Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { createMockRequest } from '@sim/testing'
5+
import { describe, expect, it } from 'vitest'
6+
import { AuthType } from '@/lib/auth/hybrid'
7+
import {
8+
PRIVATE_SECRET_PROVENANCE_BUNDLE_V1,
9+
PRIVATE_SECRET_PROVENANCE_FIELD,
10+
PRIVATE_SECRET_PROVENANCE_HEADER,
11+
} from '@/lib/execution/private-tool-metadata'
12+
import { rowDataNameToId } from '@/lib/table/column-keys'
13+
import { tableRowSecretProvenanceSelectionKey } from '@/lib/table/secret-provenance-selection'
14+
import type { RowData } from '@/lib/table/types'
15+
import {
16+
createTableWriteProvenanceTargets,
17+
resolveTableWriteSecretProvenance,
18+
} from '@/app/api/table/row-secret-provenance'
19+
20+
const USER_ID = 'user-1'
21+
const WORKSPACE_ID = 'ws-1'
22+
23+
/** Mirrors the internal-JWT wire translator: names → ids, unknown names dropped. */
24+
const ID_BY_NAME = new Map([
25+
['email', 'col_email'],
26+
['company', 'col_company'],
27+
])
28+
29+
const translateNames = (data: RowData): RowData => rowDataNameToId(data, ID_BY_NAME)
30+
const translateIdentity = (data: RowData): RowData => data
31+
32+
function traceProvenance() {
33+
return {
34+
version: 1,
35+
complete: true,
36+
entries: [],
37+
scope: { userId: USER_ID, workspaceId: WORKSPACE_ID },
38+
}
39+
}
40+
41+
function bundleRequest(selectionKeys: string[]) {
42+
const payload = {
43+
[PRIVATE_SECRET_PROVENANCE_FIELD]: {
44+
version: 1,
45+
complete: true,
46+
selections: selectionKeys.map((key) => ({ key, provenance: traceProvenance() })),
47+
},
48+
}
49+
const request = createMockRequest('POST', payload, {
50+
[PRIVATE_SECRET_PROVENANCE_HEADER]: PRIVATE_SECRET_PROVENANCE_BUNDLE_V1,
51+
})
52+
return { request, payload }
53+
}
54+
55+
describe('createTableWriteProvenanceTargets', () => {
56+
it('maps column names to their storage ids', () => {
57+
const targets = createTableWriteProvenanceTargets([{ email: 'a@b.c' }], translateNames)
58+
59+
expect(targets).toEqual([
60+
{
61+
selectionKey: tableRowSecretProvenanceSelectionKey(0, 'email'),
62+
rowKey: '0',
63+
columnId: 'col_email',
64+
},
65+
])
66+
})
67+
68+
it('returns a null column id for a column the wire translator drops', () => {
69+
const targets = createTableWriteProvenanceTargets(
70+
[{ email: 'a@b.c', notAColumn: 'x' }],
71+
translateNames
72+
)
73+
74+
expect(targets).toHaveLength(2)
75+
expect(targets[0].columnId).toBe('col_email')
76+
expect(targets[1]).toEqual({
77+
selectionKey: tableRowSecretProvenanceSelectionKey(0, 'notAColumn'),
78+
rowKey: '0',
79+
columnId: null,
80+
})
81+
})
82+
83+
it('keeps one target per submitted column so bundle selections stay paired', () => {
84+
const targets = createTableWriteProvenanceTargets(
85+
[{ notAColumn: 'x', alsoNotAColumn: 'y' }],
86+
translateNames
87+
)
88+
89+
expect(targets.map((target) => target.columnId)).toEqual([null, null])
90+
})
91+
92+
it('passes column ids through for identity (session) translation', () => {
93+
const targets = createTableWriteProvenanceTargets([{ col_email: 'a@b.c' }], translateIdentity)
94+
95+
expect(targets[0].columnId).toBe('col_email')
96+
})
97+
98+
it('keys targets by row index across multiple rows', () => {
99+
const targets = createTableWriteProvenanceTargets(
100+
[{ email: 'a@b.c' }, { company: 'Acme' }],
101+
translateNames
102+
)
103+
104+
expect(targets.map((target) => target.rowKey)).toEqual(['0', '1'])
105+
expect(targets[1].selectionKey).toBe(tableRowSecretProvenanceSelectionKey(1, 'company'))
106+
})
107+
})
108+
109+
describe('resolveTableWriteSecretProvenance', () => {
110+
it('records no provenance for a dropped column on an unsupported session write', () => {
111+
const rows = [{ email: 'a@b.c', notAColumn: 'x' }]
112+
const result = resolveTableWriteSecretProvenance({
113+
request: createMockRequest('POST', { rows }),
114+
payload: { rows },
115+
authType: AuthType.SESSION,
116+
userId: USER_ID,
117+
workspaceId: WORKSPACE_ID,
118+
targets: createTableWriteProvenanceTargets(rows, translateNames),
119+
rowKeys: ['0'],
120+
})
121+
122+
expect(result.success).toBe(true)
123+
if (!result.success) return
124+
expect(Object.keys(result.provenanceByRowKey?.['0'].columns ?? {})).toEqual(['col_email'])
125+
})
126+
127+
it('accepts a complete bundle that covers a dropped column', () => {
128+
const rows = [{ email: 'a@b.c', notAColumn: 'x' }]
129+
const { request, payload } = bundleRequest([
130+
tableRowSecretProvenanceSelectionKey(0, 'email'),
131+
tableRowSecretProvenanceSelectionKey(0, 'notAColumn'),
132+
])
133+
134+
const result = resolveTableWriteSecretProvenance({
135+
request,
136+
payload,
137+
authType: AuthType.INTERNAL_JWT,
138+
userId: USER_ID,
139+
workspaceId: WORKSPACE_ID,
140+
targets: createTableWriteProvenanceTargets(rows, translateNames),
141+
rowKeys: ['0'],
142+
})
143+
144+
expect(result.success).toBe(true)
145+
if (!result.success) return
146+
expect(Object.keys(result.provenanceByRowKey?.['0'].columns ?? {})).toEqual(['col_email'])
147+
})
148+
149+
it('stores provenance for a fully translatable bundle', () => {
150+
const rows = [{ email: 'a@b.c', company: 'Acme' }]
151+
const { request, payload } = bundleRequest([
152+
tableRowSecretProvenanceSelectionKey(0, 'email'),
153+
tableRowSecretProvenanceSelectionKey(0, 'company'),
154+
])
155+
156+
const result = resolveTableWriteSecretProvenance({
157+
request,
158+
payload,
159+
authType: AuthType.INTERNAL_JWT,
160+
userId: USER_ID,
161+
workspaceId: WORKSPACE_ID,
162+
targets: createTableWriteProvenanceTargets(rows, translateNames),
163+
rowKeys: ['0'],
164+
})
165+
166+
expect(result.success).toBe(true)
167+
if (!result.success) return
168+
expect(Object.keys(result.provenanceByRowKey?.['0'].columns ?? {}).sort()).toEqual([
169+
'col_company',
170+
'col_email',
171+
])
172+
})
173+
174+
it('rejects a bundle whose selection matches no submitted column', () => {
175+
const rows = [{ email: 'a@b.c' }]
176+
const { request, payload } = bundleRequest([tableRowSecretProvenanceSelectionKey(0, 'company')])
177+
178+
const result = resolveTableWriteSecretProvenance({
179+
request,
180+
payload,
181+
authType: AuthType.INTERNAL_JWT,
182+
userId: USER_ID,
183+
workspaceId: WORKSPACE_ID,
184+
targets: createTableWriteProvenanceTargets(rows, translateNames),
185+
rowKeys: ['0'],
186+
})
187+
188+
expect(result.success).toBe(false)
189+
})
190+
191+
it('rejects a bundle whose selection scope does not match the caller', () => {
192+
const rows = [{ email: 'a@b.c' }]
193+
const payload = {
194+
[PRIVATE_SECRET_PROVENANCE_FIELD]: {
195+
version: 1,
196+
complete: true,
197+
selections: [
198+
{
199+
key: tableRowSecretProvenanceSelectionKey(0, 'email'),
200+
provenance: {
201+
...traceProvenance(),
202+
scope: { userId: 'someone-else', workspaceId: WORKSPACE_ID },
203+
},
204+
},
205+
],
206+
},
207+
}
208+
209+
const result = resolveTableWriteSecretProvenance({
210+
request: createMockRequest('POST', payload, {
211+
[PRIVATE_SECRET_PROVENANCE_HEADER]: PRIVATE_SECRET_PROVENANCE_BUNDLE_V1,
212+
}),
213+
payload,
214+
authType: AuthType.INTERNAL_JWT,
215+
userId: USER_ID,
216+
workspaceId: WORKSPACE_ID,
217+
targets: createTableWriteProvenanceTargets(rows, translateNames),
218+
rowKeys: ['0'],
219+
})
220+
221+
expect(result.success).toBe(false)
222+
})
223+
})

apps/sim/app/api/table/row-secret-provenance.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,24 +29,30 @@ type TableWriteProvenanceResult =
2929
interface TableWriteProvenanceTarget {
3030
selectionKey: string
3131
rowKey: string
32-
columnId: string
32+
/** Storage column id, or `null` when the wire translator drops this column. */
33+
columnId: string | null
3334
}
3435

35-
/** Maps tool-facing column names to the stable storage ids used by the sidecar. */
36+
/**
37+
* Maps tool-facing column names to the stable storage ids used by the sidecar.
38+
*
39+
* The wire translator drops keys that name no column in the table schema, and the
40+
* write path drops them identically, so such a column is simply never persisted.
41+
* It still gets a target — callers key one provenance selection per column they
42+
* sent, and the completeness check pairs the two — but with a `null` column id so
43+
* no provenance is recorded for a value that was never stored.
44+
*/
3645
export function createTableWriteProvenanceTargets(
3746
rows: readonly RowData[],
3847
translate: (data: RowData) => RowData
3948
): TableWriteProvenanceTarget[] {
4049
return rows.flatMap((row, rowIndex) =>
4150
Object.entries(row).map(([columnKey, value]) => {
4251
const translatedKeys = Object.keys(translate({ [columnKey]: value }))
43-
if (translatedKeys.length !== 1) {
44-
throw new Error('Table row secret provenance column translation is invalid')
45-
}
4652
return {
4753
selectionKey: tableRowSecretProvenanceSelectionKey(rowIndex, columnKey),
4854
rowKey: String(rowIndex),
49-
columnId: translatedKeys[0],
55+
columnId: translatedKeys.length === 1 ? translatedKeys[0] : null,
5056
}
5157
})
5258
)
@@ -81,6 +87,7 @@ export function resolveTableWriteSecretProvenance(options: {
8187
provenanceByRowKey[rowKey] = { complete: true, columns: {} }
8288
}
8389
for (const target of options.targets) {
90+
if (target.columnId === null) continue
8491
const row = provenanceByRowKey[target.rowKey] ?? { complete: true, columns: {} }
8592
row.columns[target.columnId] = {
8693
version: 1,
@@ -132,11 +139,14 @@ export function resolveTableWriteSecretProvenance(options: {
132139
if (
133140
!target ||
134141
selection.provenance.scope?.userId !== options.userId ||
135-
selection.provenance.scope?.workspaceId !== options.workspaceId ||
136-
Object.hasOwn(provenanceByRowKey[target.rowKey].columns, target.columnId)
142+
selection.provenance.scope?.workspaceId !== options.workspaceId
137143
) {
138144
return { success: false, response: invalidProvenanceResponse() }
139145
}
146+
if (target.columnId === null) continue
147+
if (Object.hasOwn(provenanceByRowKey[target.rowKey].columns, target.columnId)) {
148+
return { success: false, response: invalidProvenanceResponse() }
149+
}
140150
provenanceByRowKey[target.rowKey].columns[target.columnId] = selection.provenance
141151
}
142152
return { success: true, provenanceByRowKey }

0 commit comments

Comments
 (0)