Skip to content

Commit 808bc0d

Browse files
fix(table): storage-validate read-path predicates on GET rows and find
Bugbot round 4: the native predicate path on GET /rows ran only the shape check before wire translation, and find ran none before its toLegacyFilter downgrade — so a typo'd field compiled to a clause matching nothing and read back as a plausible empty page, where the bulk write paths 400. GET now runs validateStoragePredicate post-translation (name-keyed JWT callers validate their translated form, same recipe as resolveBulkFilter); find reuses the grammar-aware tableFilterError gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
1 parent c66ecef commit 808bc0d

4 files changed

Lines changed: 54 additions & 3 deletions

File tree

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,16 @@ import { NextRequest } from 'next/server'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
77
import type { TableDefinition } from '@/lib/table'
88

9-
const { mockCheckAccess, mockFindRowMatches } = vi.hoisted(() => ({
9+
const { mockCheckAccess, mockFindRowMatches, mockTableFilterError } = vi.hoisted(() => ({
1010
mockCheckAccess: vi.fn(),
1111
mockFindRowMatches: vi.fn(),
12+
mockTableFilterError: vi.fn(),
1213
}))
1314

1415
vi.mock('@/app/api/table/utils', async () => {
1516
const { NextResponse } = await import('next/server')
1617
return {
18+
tableFilterError: mockTableFilterError,
1719
checkAccess: mockCheckAccess,
1820
accessError: (result: { status: number }) =>
1921
NextResponse.json(
@@ -67,6 +69,7 @@ describe('GET /api/table/[tableId]/rows/find', () => {
6769
authType: 'session',
6870
})
6971
mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() })
72+
mockTableFilterError.mockReturnValue(null)
7073
mockFindRowMatches.mockResolvedValue({
7174
matches: [{ ordinal: 4, rowId: 'row_4', column: 'name' }],
7275
truncated: false,
@@ -120,6 +123,21 @@ describe('GET /api/table/[tableId]/rows/find', () => {
120123
)
121124
})
122125

126+
it('short-circuits with the filter gate response before searching', async () => {
127+
const { NextResponse } = await import('next/server')
128+
mockTableFilterError.mockReturnValueOnce(
129+
NextResponse.json({ error: 'Unknown filter column "nope"' }, { status: 400 })
130+
)
131+
const res = await callGet({
132+
workspaceId: 'workspace-1',
133+
q: 'foo',
134+
filter: JSON.stringify({ all: [{ field: 'nope', op: 'eq', value: 1 }] }),
135+
})
136+
expect(res.status).toBe(400)
137+
expect((await res.json()).error).toMatch(/Unknown filter column/)
138+
expect(mockFindRowMatches).not.toHaveBeenCalled()
139+
})
140+
123141
it('maps a TableQueryValidationError to 400', async () => {
124142
const { TableQueryValidationError } = await import('@/lib/table/errors')
125143
mockFindRowMatches.mockRejectedValueOnce(new TableQueryValidationError('Invalid field name'))

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import type { Filter, Sort, SortSpec, TablePredicate } from '@/lib/table'
99
import { TableQueryValidationError } from '@/lib/table/errors'
1010
import { toLegacyFilter, toLegacySort } from '@/lib/table/query-builder/converters'
1111
import { findRowMatches } from '@/lib/table/rows/service'
12-
import { accessError, checkAccess } from '@/app/api/table/utils'
12+
import { accessError, checkAccess, tableFilterError } from '@/app/api/table/utils'
1313

1414
const logger = createLogger('TableRowsFindAPI')
1515

@@ -59,6 +59,15 @@ export const GET = withRouteHandler(
5959
return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 })
6060
}
6161

62+
// Same gate as the bulk/async routes: the predicate branch rejects unknown
63+
// storage keys (a typo'd field must 400, not return zero matches), the
64+
// legacy branch keeps its compile check.
65+
const filterError = tableFilterError(
66+
validated.filter as Filter | TablePredicate | undefined,
67+
table.schema.columns
68+
)
69+
if (filterError) return filterError
70+
6271
const { matches, truncated } = await findRowMatches(
6372
table,
6473
{

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,25 @@ describe('GET /api/table/[tableId]/rows', () => {
262262
expect(options.predicate).toEqual({ all: [{ field: 'col_aaa', op: 'eq', value: 'Ada' }] })
263263
expect(options.sort).toEqual({ col_bbb: 'asc' })
264264
})
265+
266+
/**
267+
* Storage validation runs post-translation, mirroring bulk PUT/DELETE: a
268+
* typo'd field must 400, not compile to a clause that matches nothing and
269+
* read back as a plausible empty page.
270+
*/
271+
it('400s a predicate naming an unknown column instead of returning an empty page', async () => {
272+
authAs('session')
273+
274+
const res = await callGet({
275+
workspaceId: 'workspace-1',
276+
filter: JSON.stringify({ all: [{ field: 'col_nope', op: 'eq', value: 'Ada' }] }),
277+
})
278+
279+
expect(res.status).toBe(400)
280+
const body = await res.json()
281+
expect(body.error).toMatch(/Unknown filter column "col_nope"/)
282+
expect(mockQueryRows).not.toHaveBeenCalled()
283+
})
265284
})
266285

267286
describe('PUT/DELETE /api/table/[tableId]/rows — predicate filters', () => {

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,12 @@ export const GET = withRouteHandler(
308308
// Shape-check first: nothing upstream validates this branch, and
309309
// an unchecked hybrid node would silently widen the result.
310310
validatePredicateShape(validated.filter as TablePredicate)
311-
return wire.predicateIn(validated.filter as TablePredicate)
311+
const translated = wire.predicateIn(validated.filter as TablePredicate)
312+
// Post-translation storage check, mirroring the bulk PUT/DELETE
313+
// paths: a typo'd field must 400, not compile to a clause that
314+
// matches nothing and read back as an empty page.
315+
validateStoragePredicate(translated, (table.schema as TableSchema).columns)
316+
return translated
312317
})(),
313318
}
314319
: { filter: validated.filter ? wire.filterIn(validated.filter as Filter) : undefined }),

0 commit comments

Comments
 (0)