Skip to content

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

Open
Siddhartha-singh01 wants to merge 1 commit intosimstudioai:mainfrom
Siddhartha-singh01:fix/workflow-pagination
Open

fix(api): add limit/offset pagination to GET workflows#3443
Siddhartha-singh01 wants to merge 1 commit intosimstudioai:mainfrom
Siddhartha-singh01:fix/workflow-pagination

Conversation

@Siddhartha-singh01
Copy link

Summary

Fixes a bug where fetching workspaces drops the server due to excessive memory exhaustion.
The GET /api/workflows endpoint was basically fetching every single workflow without any bounds or limits. On larger workspaces, this risks causing OOM crashes or freezing the frontend.
I just added standard offset/limit pagination pulling directly from the URL parameters so that the UI can paginate gracefully.

Fixes #3435

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Other: _________

Testing

Tested pulling workflows with and without the URL limit/offset parameters locally. The query gracefully degrades to the default fetch if no params are provided, so it doesn't break any existing UI logic anywhere else.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@cursor
Copy link

cursor bot commented Mar 6, 2026

PR Summary

Low Risk
Low risk, isolated to the workflows listing query and remains backward compatible when pagination params are omitted. Main risk is unexpected client behavior if new params are used with invalid values or inconsistent paging expectations.

Overview
Adds optional pagination to GET /api/workflows. The endpoint now reads limit and page from the query string and applies LIMIT/OFFSET to the underlying Drizzle query.

The workflow listing logic was refactored to build a shared baseQuery (workspace-scoped or permission-scoped) and then conditionally apply pagination before executing, reducing the chance of large unbounded fetches.

Written by Cursor Bugbot for commit faf058b. This will update automatically on new commits. Configure here.

@vercel
Copy link

vercel bot commented Mar 6, 2026

@Siddhartha-singh01 is attempting to deploy a commit to the Sim Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 6, 2026

Greptile Summary

This PR adds page/limit query-parameter pagination to the GET /api/workflows endpoint to prevent memory exhaustion when workspaces contain many workflows. The approach is straightforward — parse page and limit from the URL, build a typed Drizzle base query (unchanged), then chain .limit() and .offset() conditionally before executing.

Key observations:

  • Missing maximum limit cap: The most important gap in the fix — without an upper bound (e.g. Math.min(limit, 500)), an authenticated caller can still send ?limit=999999999 and reproduce the original OOM scenario. The fix should enforce a server-side cap.
  • any typing on baseQuery: Drizzle queries are strongly typed; using any discards those guarantees and may hide type errors in future refactors.
  • Negative / zero page handling: Values like page=-1 silently fall back to page-1 behaviour without signalling an error to the caller.
  • No UI changes: The PR does not update any client-side fetch call to pass pagination params, so the default (unlimited) behaviour is preserved for all existing callers — the fix only helps consumers who explicitly opt in to pagination.
  • No tests added: The checklist reflects this; adding at least a unit test covering the limit/offset path would improve confidence.

Confidence Score: 2/5

  • The fix partially addresses the OOM risk but leaves a critical gap — no server-side cap on limit — meaning the original vulnerability is still exploitable by a crafted request.
  • Score of 2 reflects that the core logic is directionally correct but the absence of a maximum limit bound means the stated goal (preventing memory exhaustion) is not fully achieved. Until a cap is enforced, the endpoint remains vulnerable to the same class of problem it was meant to fix.
  • apps/sim/app/api/workflows/route.ts — specifically the pagination block at lines 100–105 where no upper bound is enforced on the limit parameter.

Important Files Changed

Filename Overview
apps/sim/app/api/workflows/route.ts Adds page/limit query-param pagination to the GET /api/workflows handler, but lacks an upper-bound cap on limit, leaving the OOM risk partially unaddressed; also introduces a loosely-typed any for the Drizzle query builder.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Client: GET /api/workflows] --> B[Parse page + limit params]
    B --> C{workspaceId present?}
    C -- Yes --> D[Verify workspace exists]
    D --> E[Verify user membership]
    E --> F[Build query for single workspace]
    C -- No --> G[Fetch permitted workspace IDs]
    G --> H{Any workspaceIds?}
    H -- No --> I[Return empty array]
    H -- Yes --> J[Build query for all permitted workspaces]
    F --> K{limit param valid?}
    J --> K
    K -- Yes --> L[Apply .limit]
    L --> M{page param > 1?}
    M -- Yes --> N[Apply .offset]
    M -- No --> O[Execute query]
    N --> O
    K -- No --> O
    O --> P[Return paginated workflows]
Loading

Last reviewed commit: faf058b

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

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)
}
}

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

let baseQuery: any
Copy link
Contributor

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.

Comment on lines +65 to +68
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
Copy link
Contributor

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.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

if (page && !isNaN(page) && page > 1) {
baseQuery = baseQuery.offset((page - 1) * limit)
}
}
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing pagination on GET /api/workflows causes memory exhaustion / DoS risk on large workspaces

1 participant