Skip to content

Commit 0888ee0

Browse files
fix(api): validate v2 cursor key values and compare timestamps at ms precision
Two review findings, fixed at the root by making a keyset key own its cursor codec instead of hand-writing a decoder per sort. Cursor key values are caller-controlled, and matching the sort stamp and key count was not enough: an unparseable timestamp or a non-numeric size reached the query as an Invalid Date or NaN and surfaced as a 500. Each key now type- checks its own value and rejects a cursor it cannot hold, which both routes render as the documented 400. Timestamp keys now order and compare on date_trunc('milliseconds', col). Postgres keeps microseconds and defaultNow() populates them, but a cursor value round-trips through a millisecond-only JS Date — comparing the raw column against the truncated value re-admitted the page's own last row, duplicating it and stalling pagination outright at a page size of one. Reachable today via workspace_files.updated_at, which insertFileMetadata leaves to defaultNow().
1 parent 9bd5751 commit 0888ee0

9 files changed

Lines changed: 339 additions & 164 deletions

File tree

apps/sim/app/api/v2/files/route.test.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ vi.mock('@/app/api/v2/lib/gate', () => ({
3636

3737
vi.mock('@/lib/uploads/contexts/workspace', () => ({
3838
queryWorkspaceFiles: mockQueryWorkspaceFiles,
39-
workspaceFileCursorKeyCount: () => 2,
4039
uploadWorkspaceFile: mockUploadWorkspaceFile,
4140
getWorkspaceFile: mockGetWorkspaceFile,
4241
FileConflictError: class FileConflictError extends Error {},
@@ -288,6 +287,20 @@ describe('GET /api/v2/files', () => {
288287
expect(mockQueryWorkspaceFiles).not.toHaveBeenCalled()
289288
})
290289

290+
it('400s when the cursor carries values the sort cannot hold', async () => {
291+
mockQueryWorkspaceFiles.mockRejectedValue(
292+
new OrchestrationError('validation', 'cursor does not match the requested sortBy/sortOrder.')
293+
)
294+
const cursor = Buffer.from(
295+
JSON.stringify({ sort: 'uploadedAt:asc', keys: ['not-a-date', 'wf_1'] })
296+
).toString('base64')
297+
298+
const res = await callList(`workspaceId=${WS}&cursor=${encodeURIComponent(cursor)}`)
299+
300+
expect(res.status).toBe(400)
301+
expect((await res.json()).error.code).toBe('BAD_REQUEST')
302+
})
303+
291304
it('terminates pagination when the query reports no further keys', async () => {
292305
mockQueryWorkspaceFiles.mockResolvedValue({ files: [buildRecord()], nextKeys: null })
293306

apps/sim/app/api/v2/files/route.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import {
1818
getWorkspaceFile,
1919
queryWorkspaceFiles,
2020
uploadWorkspaceFile,
21-
workspaceFileCursorKeyCount,
2221
} from '@/lib/uploads/contexts/workspace'
2322
import { checkRateLimit, resolveWorkspaceAccess } from '@/app/api/v1/middleware'
2423
import { toV2File } from '@/app/api/v2/files/utils'
@@ -83,7 +82,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
8382
if (access) return v2WorkspaceAccessError(access)
8483

8584
const sort = cursorSortKey(sortBy, sortOrder)
86-
const decoded = decodeSortedCursor(cursor, sort, workspaceFileCursorKeyCount(sortBy))
85+
const decoded = decodeSortedCursor(cursor, sort)
8786
if (decoded.status === 'invalid') return v2CursorSortError()
8887

8988
const { files, nextKeys } = await queryWorkspaceFiles(workspaceId, {
@@ -101,6 +100,10 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
101100

102101
return v2CursorList(items, nextCursor, { rateLimit })
103102
} catch (error) {
103+
// A cursor that doesn't fit the requested sort arrives classified as `validation` → 400.
104+
const classified = v2CaughtOrchestrationError(error)
105+
if (classified) return classified
106+
104107
logger.error('Error listing files', { error: getErrorMessage(error, 'Unknown error') })
105108
return v2Error('INTERNAL_ERROR', 'Internal server error')
106109
}

apps/sim/app/api/v2/lib/response.ts

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { NextResponse } from 'next/server'
22
import type { ZodError } from 'zod'
3-
import type { CursorKey } from '@/lib/api/list-query'
3+
import { type CursorKey, INVALID_CURSOR_MESSAGE } from '@/lib/api/list-query'
44
import { getValidationErrorMessage, serializeZodIssues } from '@/lib/api/server'
55
import { asOrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types'
66
import type { RateLimitResult, WorkspaceAccessError } from '@/app/api/v1/middleware'
@@ -190,36 +190,26 @@ export type DecodedSortedCursor =
190190
* Reads a keyset cursor back, refusing one that does not belong to the
191191
* requested sort. Resuming a `name`-ordered cursor under `createdAt` would
192192
* compare the wrong column and silently duplicate or skip rows, so a mismatch
193-
* is a client error rather than a best-effort page. A malformed cursor — bad
194-
* base64, or the wrong number of keys for `keyCount` — is rejected for the same
195-
* reason: ignoring it would restart from page one while the caller believes it
196-
* is paging forward, and a short key list would compare a column against
197-
* `undefined`.
193+
* is a client error rather than a best-effort page. A cursor that isn't valid
194+
* base64-JSON is rejected for the same reason: ignoring it would restart from
195+
* page one while the caller believes it is paging forward.
196+
*
197+
* This checks the envelope only. The key VALUES are caller-controlled too, and
198+
* are type-checked against the sort's keys by `keysetAfter`, which is where a
199+
* bad arity or an unparseable timestamp is caught.
198200
*/
199-
export function decodeSortedCursor(
200-
cursor: string | undefined,
201-
sort: string,
202-
keyCount: number
203-
): DecodedSortedCursor {
201+
export function decodeSortedCursor(cursor: string | undefined, sort: string): DecodedSortedCursor {
204202
if (!cursor) return { status: 'absent' }
205203
const decoded = decodeCursor<Partial<SortedCursorPayload>>(cursor)
206-
if (
207-
!decoded ||
208-
decoded.sort !== sort ||
209-
!Array.isArray(decoded.keys) ||
210-
decoded.keys.length !== keyCount
211-
) {
204+
if (!decoded || decoded.sort !== sort || !Array.isArray(decoded.keys)) {
212205
return { status: 'invalid' }
213206
}
214207
return { status: 'ok', keys: decoded.keys }
215208
}
216209

217-
/** The 400 for a cursor that does not match the request's sort. */
210+
/** The 400 for a cursor that cannot be resumed under the request's sort. */
218211
export function v2CursorSortError(): NextResponse {
219-
return v2Error(
220-
'BAD_REQUEST',
221-
'cursor does not match the requested sortBy/sortOrder. Restart pagination without a cursor after changing the sort.'
222-
)
212+
return v2Error('BAD_REQUEST', INVALID_CURSOR_MESSAGE)
223213
}
224214

225215
const V2_CODE_BY_ORCHESTRATION_ERROR: Record<OrchestrationErrorCode, V2ErrorCode> = {

apps/sim/app/api/v2/workflows/route.test.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,13 @@ const lastConditions = () =>
7070

7171
const lastOrderBy = () => dbChainMockFns.orderBy.mock.calls.at(-1) ?? []
7272

73+
/**
74+
* Timestamp keys order on `date_trunc('milliseconds', col)` rather than the raw
75+
* column, so the mocked `sql` fragment carries the column in its interpolated
76+
* values rather than being the column itself.
77+
*/
78+
const truncatedColumnOf = (entry: { column: { values?: unknown[] } }) => entry.column?.values?.[0]
79+
7380
describe('GET /api/v2/workflows', () => {
7481
beforeEach(() => {
7582
vi.clearAllMocks()
@@ -133,11 +140,11 @@ describe('GET /api/v2/workflows', () => {
133140

134141
await callList(`workspaceId=${WS}`)
135142

136-
expect(lastOrderBy()).toEqual([
137-
{ type: 'asc', column: schemaMock.workflow.sortOrder },
138-
{ type: 'asc', column: schemaMock.workflow.createdAt },
139-
{ type: 'asc', column: schemaMock.workflow.id },
140-
])
143+
const orderBy = lastOrderBy()
144+
expect(orderBy.map((e: { type: string }) => e.type)).toEqual(['asc', 'asc', 'asc'])
145+
expect(orderBy[0].column).toBe(schemaMock.workflow.sortOrder)
146+
expect(truncatedColumnOf(orderBy[1])).toBe(schemaMock.workflow.createdAt)
147+
expect(orderBy[2].column).toBe(schemaMock.workflow.id)
141148
})
142149

143150
it('orders by the requested field and direction', async () => {

apps/sim/app/api/v2/workflows/route.ts

Lines changed: 30 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,15 @@ import {
1111
v2ListWorkflowsContract,
1212
} from '@/lib/api/contracts/v2/workflows'
1313
import {
14-
cursorDate,
15-
type KeysetSort,
14+
encodeKeyset,
15+
type KeysetKey,
1616
keysetAfter,
17+
keysetColumns,
1718
listOrderBy,
19+
numberKey,
1820
searchFilter,
21+
textKey,
22+
timestampKey,
1923
} from '@/lib/api/list-query'
2024
import { parseRequest } from '@/lib/api/server'
2125
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -57,37 +61,20 @@ type WorkflowRow = {
5761
* `sortOrder` freely, and dropping `createdAt` from the tiebreak would reshuffle
5862
* every workspace's default list.
5963
*/
64+
const workflowId = textKey<WorkflowRow>(workflow.id, (row) => row.id)
65+
const workflowCreatedAt = timestampKey<WorkflowRow>(workflow.createdAt, (row) => row.createdAt)
66+
6067
const WORKFLOW_SORTS = {
61-
position: {
62-
keys: [workflow.sortOrder, workflow.createdAt, workflow.id],
63-
encode: (row) => [row.sortOrder, cursorDate.encode(row.createdAt), row.id],
64-
decode: ([sortOrder, createdAt, id]) => [
65-
Number(sortOrder),
66-
cursorDate.decode(createdAt),
67-
String(id),
68-
],
69-
},
70-
name: {
71-
keys: [workflow.name, workflow.id],
72-
encode: (row) => [row.name, row.id],
73-
decode: ([name, id]) => [String(name), String(id)],
74-
},
75-
createdAt: {
76-
keys: [workflow.createdAt, workflow.id],
77-
encode: (row) => [cursorDate.encode(row.createdAt), row.id],
78-
decode: ([createdAt, id]) => [cursorDate.decode(createdAt), String(id)],
79-
},
80-
updatedAt: {
81-
keys: [workflow.updatedAt, workflow.id],
82-
encode: (row) => [cursorDate.encode(row.updatedAt), row.id],
83-
decode: ([updatedAt, id]) => [cursorDate.decode(updatedAt), String(id)],
84-
},
85-
runCount: {
86-
keys: [workflow.runCount, workflow.id],
87-
encode: (row) => [row.runCount, row.id],
88-
decode: ([runCount, id]) => [Number(runCount), String(id)],
89-
},
90-
} satisfies Record<V2WorkflowSortBy, KeysetSort<WorkflowRow>>
68+
position: [
69+
numberKey<WorkflowRow>(workflow.sortOrder, (row) => row.sortOrder),
70+
workflowCreatedAt,
71+
workflowId,
72+
],
73+
name: [textKey<WorkflowRow>(workflow.name, (row) => row.name), workflowId],
74+
createdAt: [workflowCreatedAt, workflowId],
75+
updatedAt: [timestampKey<WorkflowRow>(workflow.updatedAt, (row) => row.updatedAt), workflowId],
76+
runCount: [numberKey<WorkflowRow>(workflow.runCount, (row) => row.runCount), workflowId],
77+
} satisfies Record<V2WorkflowSortBy, readonly KeysetKey<WorkflowRow>[]>
9178

9279
export const GET = withRouteHandler(async (request: NextRequest) => {
9380
const requestId = generateId().slice(0, 8)
@@ -117,19 +104,22 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
117104
if (access) return v2WorkspaceAccessError(access)
118105

119106
const sortKey = cursorSortKey(params.sortBy, params.sortOrder)
120-
const sort: KeysetSort<WorkflowRow> = WORKFLOW_SORTS[params.sortBy]
121-
const decoded = decodeSortedCursor(params.cursor, sortKey, sort.keys.length)
107+
const keys: readonly KeysetKey<WorkflowRow>[] = WORKFLOW_SORTS[params.sortBy]
108+
const decoded = decodeSortedCursor(params.cursor, sortKey)
122109
if (decoded.status === 'invalid') return v2CursorSortError()
123110

111+
// `null` here is a cursor whose values don't fit this sort — a client error, not an empty page.
112+
const resumeAfter =
113+
decoded.status === 'ok' ? keysetAfter(keys, decoded.keys, params.sortOrder) : undefined
114+
if (resumeAfter === null) return v2CursorSortError()
115+
124116
const conditions = [
125117
eq(workflow.workspaceId, params.workspaceId),
126118
isNull(workflow.archivedAt),
127119
params.folderId ? eq(workflow.folderId, params.folderId) : undefined,
128120
params.deployedOnly ? eq(workflow.isDeployed, true) : undefined,
129121
searchFilter(workflow.name, params.search),
130-
decoded.status === 'ok'
131-
? keysetAfter(sort.keys, sort.decode(decoded.keys), params.sortOrder)
132-
: undefined,
122+
resumeAfter,
133123
]
134124

135125
const rows = await db
@@ -149,14 +139,15 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
149139
})
150140
.from(workflow)
151141
.where(and(...conditions))
152-
.orderBy(...listOrderBy(sort.keys, params.sortOrder))
142+
.orderBy(...listOrderBy(keysetColumns(keys), params.sortOrder))
153143
.limit(params.limit + 1)
154144

155145
const hasMore = rows.length > params.limit
156146
const data = rows.slice(0, params.limit)
157147

158148
const last = data.at(-1)
159-
const nextCursor = hasMore && last ? encodeSortedCursor(sortKey, sort.encode(last)) : null
149+
const nextCursor =
150+
hasMore && last ? encodeSortedCursor(sortKey, encodeKeyset(keys, last)) : null
160151

161152
const formatted: V2WorkflowListItem[] = data.map((w) => ({
162153
id: w.id,

0 commit comments

Comments
 (0)