Skip to content

fix(api): add limit/offset pagination to GET workflows #3443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Siddhartha-singh01 wants to merge 1 commit into
base: main
Choose a base branch
from
+21 −3
Open

fix(api): add limit/offset pagination to GET workflows #3443

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions apps/sim/app/api/workflows/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,18 @@ export async function GET(request: NextRequest) {
}
}

let workflows
const pageParam = url.searchParams.get('page')
const limitParam = url.searchParams.get('limit')
const page = pageParam ? parseInt(pageParam, 10) : undefined
const limit = limitParam ? parseInt(limitParam, 10) : undefined
Comment on lines +65 to +68
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing input validation for negative / zero page numbers

Negative page numbers silently fall through: e.g. page=-5 passes the page > 1 check as false, so the offset clause is never applied — callers get page-1 results regardless. Consider adding an explicit guard:

Suggested change
const pageParam = url.searchParams.get('page')
const limitParam = url.searchParams.get('limit')
const page = pageParam ? parseInt(pageParam, 10) : undefined
const limit = limitParam ? parseInt(limitParam, 10) : undefined
const pageParam = url.searchParams.get('page')
const limitParam = url.searchParams.get('limit')
const page = pageParam ? Math.max(1, parseInt(pageParam, 10)) : undefined
const limit = limitParam ? parseInt(limitParam, 10) : undefined

This ensures that any page value below 1 is clamped to 1 rather than silently returning the first page without surfacing an error to the caller.


let workflows
const orderByClause = [asc(workflow.sortOrder), asc(workflow.createdAt), asc(workflow.id)]

let baseQuery: any
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

baseQuery: any bypasses Drizzle's type safety

Drizzle ORM queries are strongly typed end-to-end. Using any here silently drops those guarantees — e.g. .limit() / .offset() return types are no longer checked, and any future misuse won't be caught at compile time.

A practical alternative is to union the two concrete query types, or extract the shared query-building logic into a typed helper. At minimum, prefer unknown over any so the compiler forces explicit narrowing before use.


if (workspaceId) {
workflows = await db
baseQuery = db
.select()
.from(workflow)
.where(eq(workflow.workspaceId, workspaceId))
Expand All @@ -77,17 +83,29 @@ export async function GET(request: NextRequest) {
.select({ workspaceId: permissions.entityId })
.from(permissions)
.where(and(eq(permissions.userId, userId), eq(permissions.entityType, 'workspace')))

const workspaceIds = workspacePermissionRows.map((row) => row.workspaceId)

if (workspaceIds.length === 0) {
return NextResponse.json({ data: [] }, { status: 200 })
}
workflows = await db

baseQuery = db
.select()
.from(workflow)
.where(inArray(workflow.workspaceId, workspaceIds))
.orderBy(...orderByClause)
}

if (limit && !isNaN(limit) && limit > 0) {
baseQuery = baseQuery.limit(limit)
if (page && !isNaN(page) && page > 1) {
baseQuery = baseQuery.offset((page - 1) * limit)
}
}
Comment on lines +100 to +105
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No upper bound on limit undermines the fix

The entire motivation for this PR is to prevent OOM crashes from unbounded queries. However, because there is no cap on the limit value, an authenticated caller can still pass ?limit=999999999 and fetch the entire table in a single request — the original vulnerability is not fully closed.

A maximum limit (e.g. 100 or 500) should be enforced before the query is built:

Suggested change
if (limit && !isNaN(limit) && limit > 0) {
baseQuery = baseQuery.limit(limit)
if (page && !isNaN(page) && page > 1) {
baseQuery = baseQuery.offset((page - 1) * limit)
}
}
if (limit && !isNaN(limit) && limit > 0) {
const safeLimitMax = 500
const safeLimit = Math.min(limit, safeLimitMax)
baseQuery = baseQuery.limit(safeLimit)
if (page && !isNaN(page) && page > 1) {
baseQuery = baseQuery.offset((page - 1) * safeLimit)
}
}

Copy link

@cursor cursor bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No upper bound on user-supplied limit parameter

Medium Severity

The limit query parameter is user-controlled with no maximum cap, so a request like ?limit=999999999 still fetches all rows — the exact OOM scenario this PR aims to fix. The codebase already enforces MAX_LIMIT = 250 via parsePaginationParams in apps/sim/app/api/v1/admin/types.ts; the same safeguard is missing here.

Fix in Cursor Fix in Web


workflows = await baseQuery

return NextResponse.json({ data: workflows }, { status: 200 })
} catch (error: any) {
const elapsed = Date.now() - startTime
Expand Down