From aa8f09a034cc863e761b629465e988219ca691ea Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 11 Aug 2026 15:44:05 -0700 Subject: [PATCH 1/3] chore(api): remove the unused public API route builder and dead endpoint labels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `withPublicApiRouteHandler` and 27 `ApiEndpoint` union members landed together in #5273, but the v2 surface shipped on `defineV2JsonRoute` + `v2RateLimits` instead. The builder had no production caller — only its own test — and the v2 rate limiter never reads an `ApiEndpoint` label, so those members were never emitted to telemetry by symbol or by string literal. Remaining members are exactly the labels a v1 route passes to `checkRateLimit` or `authenticateRequest`. Drops the now-unreachable `hasZodUsage` branch from the API validation audit; no ratchet metric moves (route total stays 1093). --- .../app/api/public-api-route-handler.test.ts | 272 ------------------ apps/sim/app/api/public-api-route-handler.ts | 79 ----- apps/sim/app/api/v1/middleware.ts | 37 +-- scripts/check-api-validation-contracts.ts | 10 - 4 files changed, 7 insertions(+), 391 deletions(-) delete mode 100644 apps/sim/app/api/public-api-route-handler.test.ts delete mode 100644 apps/sim/app/api/public-api-route-handler.ts diff --git a/apps/sim/app/api/public-api-route-handler.test.ts b/apps/sim/app/api/public-api-route-handler.test.ts deleted file mode 100644 index 33757c33864..00000000000 --- a/apps/sim/app/api/public-api-route-handler.test.ts +++ /dev/null @@ -1,272 +0,0 @@ -/** - * @vitest-environment node - */ -import { NextRequest, NextResponse } from 'next/server' -import { beforeEach, describe, expect, it, vi } from 'vitest' -import { z } from 'zod' -import { defineRouteContract } from '@/lib/api/contracts' -import { recordRateLimitSnapshot } from '@/lib/api/server/rate-limit-context' - -const { - mockCheckRateLimit, - mockGate, - mockHandler, - mockLoggerError, - mockLoggerInfo, - requestContextState, -} = vi.hoisted(() => ({ - mockCheckRateLimit: vi.fn(), - mockGate: vi.fn(), - mockHandler: vi.fn(), - mockLoggerError: vi.fn(), - mockLoggerInfo: vi.fn(), - requestContextState: { - current: undefined as { requestId: string; method?: string; path?: string } | undefined, - }, -})) - -vi.mock('@sim/logger', () => ({ - createLogger: () => ({ - info: (...arguments_: unknown[]) => - mockLoggerInfo(requestContextState.current?.requestId, ...arguments_), - warn: vi.fn(), - error: (...arguments_: unknown[]) => - mockLoggerError(requestContextState.current?.requestId, ...arguments_), - }), - getRequestContext: () => requestContextState.current, - runWithRequestContext: async ( - context: { requestId: string; method?: string; path?: string }, - callback: () => T | Promise - ): Promise => { - requestContextState.current = context - try { - return await callback() - } finally { - requestContextState.current = undefined - } - }, -})) - -vi.mock('@/lib/core/utils/request', () => ({ - generateRequestId: () => requestContextState.current?.requestId ?? 'outer-request-id', -})) - -vi.mock('@/app/api/v1/middleware', () => ({ - checkRateLimit: mockCheckRateLimit, -})) - -vi.mock('@/app/api/v2/lib/gate', () => ({ - v2ApiGateError: mockGate, -})) - -import { withPublicApiRouteHandler } from '@/app/api/public-api-route-handler' - -const RATE_LIMIT = { - allowed: true, - limit: 400, - remaining: 399, - resetAt: new Date('2026-08-06T20:00:00.000Z'), - userId: 'user-1', - keyType: 'personal' as const, -} - -const queryContract = defineRouteContract({ - method: 'POST', - path: '/api/test/:itemId', - params: z.object({ itemId: z.string().min(1) }), - query: z.object({ limit: z.coerce.number().int().positive() }), - body: z.object({ name: z.string().min(1) }), - response: { mode: 'json', schema: z.object({ ok: z.boolean() }) }, -}) - -const listContract = defineRouteContract({ - method: 'GET', - path: '/api/test', - query: z.object({ workspaceId: z.string().min(1) }), - response: { mode: 'json', schema: z.object({ ok: z.boolean() }) }, -}) - -const POST = withPublicApiRouteHandler({ - contract: queryContract, - rateLimitEndpoint: 'table-rows', - parseOptions: { - maxBodyBytes: 32, - payloadTooLargeResponse: () => - NextResponse.json({ error: 'Custom payload limit response' }, { status: 413 }), - }, - handler: async (arguments_) => { - mockHandler(arguments_) - return NextResponse.json({ ok: true }) - }, -}) - -const GET = withPublicApiRouteHandler({ - contract: listContract, - rateLimitEndpoint: 'tables', - handler: async (arguments_) => { - mockHandler(arguments_) - return NextResponse.json({ ok: true }) - }, -}) - -const FAILING_GET = withPublicApiRouteHandler({ - contract: listContract, - rateLimitEndpoint: 'tables', - handler: async () => { - throw new Error('handler failed') - }, -}) - -function postRequest(body: string): NextRequest { - return new NextRequest('http://localhost:3000/api/test/item-1?limit=10', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body, - }) -} - -function listRequest(query = 'workspaceId=workspace-1'): NextRequest { - return new NextRequest(`http://localhost:3000/api/test?${query}`) -} - -describe('withPublicApiRouteHandler', () => { - beforeEach(() => { - vi.clearAllMocks() - mockGate.mockResolvedValue(null) - mockCheckRateLimit.mockImplementation(async (request: NextRequest) => { - recordRateLimitSnapshot(request, RATE_LIMIT) - return RATE_LIMIT - }) - }) - - it.each([ - ['authentication failure', 401], - ['rate-limit denial', 429], - ])('short-circuits %s before reading or parsing the body', async (_label, status) => { - mockCheckRateLimit.mockImplementation(async (request: NextRequest) => { - if (status === 401) { - return { - allowed: false, - limit: 0, - remaining: 0, - resetAt: new Date('2026-08-06T20:00:00.000Z'), - error: 'API key required', - } - } - - recordRateLimitSnapshot(request, RATE_LIMIT) - return { ...RATE_LIMIT, allowed: false, remaining: 0, retryAfterMs: 30_000 } - }) - const request = postRequest('{not valid json') - - const response = await POST(request, { params: { itemId: 'item-1' } }) - - expect(response.status).toBe(status) - expect(request.bodyUsed).toBe(false) - expect(mockHandler).not.toHaveBeenCalled() - expect(mockCheckRateLimit).toHaveBeenCalledWith(request, 'table-rows') - expect(mockGate).not.toHaveBeenCalled() - if (status === 401) { - expect(response.headers.get('X-RateLimit-Limit')).toBe('0') - } else { - expect(response.headers.get('Retry-After')).toBe('30') - expect(response.headers.get('X-RateLimit-Limit')).toBe('400') - } - }) - - it('checks the v2 rollout gate before reading or parsing the body', async () => { - mockGate.mockResolvedValue(NextResponse.json({ error: 'Not found' }, { status: 404 })) - const request = postRequest('{not valid json') - - const response = await POST(request, { params: { itemId: 'item-1' } }) - - expect(response.status).toBe(404) - expect(request.bodyUsed).toBe(false) - expect(mockGate).toHaveBeenCalledWith('user-1') - expect(mockHandler).not.toHaveBeenCalled() - }) - - it('fails fast when an allowed rate-limit result has no user ID', async () => { - mockCheckRateLimit.mockResolvedValue({ ...RATE_LIMIT, userId: undefined }) - - const response = await GET(listRequest()) - - expect(response.status).toBe(500) - expect(mockGate).not.toHaveBeenCalled() - expect(mockHandler).not.toHaveBeenCalled() - }) - - it('returns a contract validation response after authentication', async () => { - const response = await POST(postRequest(JSON.stringify({ name: '' })), { - params: { itemId: 'item-1' }, - }) - - expect(response.status).toBe(400) - expect(response.headers.get('X-RateLimit-Limit')).toBe('400') - expect(mockHandler).not.toHaveBeenCalled() - }) - - it('forwards the body-size parse option', async () => { - const response = await POST(postRequest(JSON.stringify({ name: 'x'.repeat(40) })), { - params: { itemId: 'item-1' }, - }) - - expect(response.status).toBe(413) - expect(response.headers.get('X-RateLimit-Remaining')).toBe('399') - await expect(response.json()).resolves.toEqual({ error: 'Custom payload limit response' }) - expect(mockHandler).not.toHaveBeenCalled() - }) - - it('provides parsed params, query, body, and auth to the handler', async () => { - const request = postRequest(JSON.stringify({ name: 'Ada' })) - const response = await POST(request, { params: Promise.resolve({ itemId: 'item-1' }) }) - - expect(response.status).toBe(200) - expect(mockHandler).toHaveBeenCalledWith({ - request, - input: { - params: { itemId: 'item-1' }, - query: { limit: 10 }, - body: { name: 'Ada' }, - headers: undefined, - }, - auth: { - requestId: 'outer-request-id', - userId: 'user-1', - rateLimit: RATE_LIMIT, - }, - }) - expect(response.headers.get('x-request-id')).toBe('outer-request-id') - expect(response.headers.get('X-RateLimit-Reset')).toBe(RATE_LIMIT.resetAt.toISOString()) - expect(mockLoggerInfo).toHaveBeenCalledWith( - 'outer-request-id', - 'OK', - expect.objectContaining({ status: 200 }) - ) - }) - - it('supports direct invocation without a route context', async () => { - const request = listRequest() - const response = await GET(request) - - expect(response.status).toBe(200) - expect(mockHandler.mock.calls[0][0].input.query).toEqual({ workspaceId: 'workspace-1' }) - expect(mockCheckRateLimit).toHaveBeenCalledWith(request, 'tables') - }) - - it('keeps rate-limit and request headers on unhandled endpoint errors', async () => { - const response = await FAILING_GET(listRequest()) - - expect(response.status).toBe(500) - await expect(response.json()).resolves.toEqual({ - error: { code: 'INTERNAL_ERROR', message: 'Internal server error' }, - }) - expect(response.headers.get('x-request-id')).toBe('outer-request-id') - expect(response.headers.get('X-RateLimit-Limit')).toBe('400') - expect(mockLoggerError).toHaveBeenCalledWith( - 'outer-request-id', - 'Unhandled route error', - expect.objectContaining({ error: 'handler failed' }) - ) - }) -}) diff --git a/apps/sim/app/api/public-api-route-handler.ts b/apps/sim/app/api/public-api-route-handler.ts deleted file mode 100644 index af25a4fe3b4..00000000000 --- a/apps/sim/app/api/public-api-route-handler.ts +++ /dev/null @@ -1,79 +0,0 @@ -import type { NextRequest, NextResponse } from 'next/server' -import type { AnyApiRouteContract } from '@/lib/api/contracts' -import { type ParsedRequest, type ParseRequestOptions, parseRequest } from '@/lib/api/server' -import { generateRequestId } from '@/lib/core/utils/request' -import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { type ApiEndpoint, type AuthorizedRequest, checkRateLimit } from '@/app/api/v1/middleware' -import { v2ApiGateError } from '@/app/api/v2/lib/gate' -import { v2Error, v2RateLimitError, v2ValidationError } from '@/app/api/v2/lib/response' - -interface PublicApiRouteContext { - params?: - | Promise> - | Record -} - -interface PublicApiRouteHandlerArguments { - request: NextRequest - input: ParsedRequest - auth: AuthorizedRequest -} - -interface PublicApiRouteHandlerOptions { - contract: C - rateLimitEndpoint: ApiEndpoint - parseOptions?: ParseRequestOptions - handler: ( - arguments_: PublicApiRouteHandlerArguments - ) => Promise | NextResponse | Response -} - -type PublicApiNextRouteHandler = ( - request: NextRequest, - context?: PublicApiRouteContext -) => Promise - -/** - * Wraps an API-key-authenticated public route with request context, rate - * limiting, authentication, and contract parsing before invoking the route's - * authorization and business logic. Unexpected endpoint errors are logged once - * by the shared route handler and rendered as the canonical v2 500 envelope. - */ -export function withPublicApiRouteHandler({ - contract, - rateLimitEndpoint, - parseOptions, - handler, -}: PublicApiRouteHandlerOptions): PublicApiNextRouteHandler { - const wrapped = withRouteHandler( - async (request, context) => { - const requestId = generateRequestId() - const rateLimit = await checkRateLimit(request, rateLimitEndpoint) - if (!rateLimit.allowed) return v2RateLimitError(rateLimit) - - if (!rateLimit.userId) { - throw new Error('Allowed public API request is missing a user ID') - } - const userId = rateLimit.userId - const gate = await v2ApiGateError(userId) - if (gate) return gate - - const parsed = await parseRequest(contract, request, context ?? {}, { - validationErrorResponse: v2ValidationError, - ...parseOptions, - }) - if (!parsed.success) return parsed.response - - return handler({ - request, - input: parsed.data, - auth: { requestId, userId, rateLimit }, - }) - }, - { - unhandledErrorResponse: () => v2Error('INTERNAL_ERROR', 'Internal server error'), - } - ) - - return async (request, context) => wrapped(request, context) -} diff --git a/apps/sim/app/api/v1/middleware.ts b/apps/sim/app/api/v1/middleware.ts index 7efe9175314..6a9db50ec44 100644 --- a/apps/sim/app/api/v1/middleware.ts +++ b/apps/sim/app/api/v1/middleware.ts @@ -20,9 +20,13 @@ const logger = createLogger('V1Middleware') const rateLimiter = new RateLimiter() /** - * Endpoint labels for public API auth/rate-limit telemetry. Version-neutral: the - * v1 and v2 public surfaces share the same `authenticateV1Request` + `api-endpoint` - * rate bucket, so the label is only a log/metric dimension, not a policy switch. + * Endpoint labels for v1 public API auth/rate-limit telemetry. The label is only + * a log/metric dimension, not a policy switch — every label resolves to the same + * `authenticateV1Request` + `api-endpoint` rate bucket. + * + * The v2 surface does not use these labels: v2 routes are built with + * `defineV2JsonRoute` and rate-limited through `v2RateLimits`. Add a member only + * when a route actually passes it to `checkRateLimit` / `authenticateRequest`. */ export type ApiEndpoint = | 'logs' @@ -31,8 +35,6 @@ export type ApiEndpoint = | 'workflow-detail' | 'workflow-deploy' | 'workflow-rollback' - | 'workflow-versions' - | 'workflow-version-detail' | 'workflow-export' | 'workflow-import' | 'audit-logs' @@ -40,37 +42,12 @@ export type ApiEndpoint = | 'table-detail' | 'table-rows' | 'table-row-detail' - | 'table-rows-find' | 'table-columns' - | 'table-views' - | 'table-view-detail' - | 'table-groups' - | 'table-enrichment' - | 'table-import' - | 'table-export' - | 'table-jobs' | 'files' | 'file-detail' - | 'file-share' - | 'file-content' - | 'file-move' - | 'file-bulk-delete' | 'knowledge' | 'knowledge-detail' | 'knowledge-search' - | 'copilot-chat' - | 'billing-usage' - | 'mcp-servers' - | 'mcp-server-detail' - | 'skills' - | 'skill-detail' - | 'custom-tools' - | 'custom-tool-detail' - | 'credentials' - | 'secrets' - | 'secret-detail' - | 'workspaces' - | 'workspace-members' export interface RateLimitResult { allowed: boolean diff --git a/scripts/check-api-validation-contracts.ts b/scripts/check-api-validation-contracts.ts index efc7947b7e6..501b1089dd1 100644 --- a/scripts/check-api-validation-contracts.ts +++ b/scripts/check-api-validation-contracts.ts @@ -146,9 +146,6 @@ const RAW_JSON_BASELINE_ROUTES = new Set([ ]) const CONTRACT_IMPORT_PATTERN = /\bfrom\s+['"]@\/lib\/api\/contracts(?:\/[^'"]*)?['"]/ -const PUBLIC_API_ROUTE_HANDLER_IMPORT_PATTERN = - /\bimport\s*\{[^}]*\bwithPublicApiRouteHandler\b[^}]*\}\s*from\s*['"]@\/app\/api\/public-api-route-handler['"]/ -const PUBLIC_API_ROUTE_HANDLER_USAGE_PATTERN = /\bwithPublicApiRouteHandler\s*\(/ const DECLARATIVE_ROUTE_BUILDER_IMPORT_PATTERN = /\bimport\s*\{[^}]*(?:\bdefineInternalJsonRoute\b|\bdefineV2JsonRoute\b|\bdefineInternalBinaryRoute\b|\bdefineV2BinaryRoute\b)[^}]*\}\s*from\s*['"]@\/lib\/api\/server\/routes['"]/ const DECLARATIVE_ROUTE_BUILDER_USAGE_PATTERN = @@ -723,13 +720,6 @@ function hasZodUsage(relativePath: string, content: string): boolean { ) { return true } - if ( - CONTRACT_IMPORT_PATTERN.test(content) && - PUBLIC_API_ROUTE_HANDLER_IMPORT_PATTERN.test(content) && - PUBLIC_API_ROUTE_HANDLER_USAGE_PATTERN.test(content) - ) { - return true - } if ( CONTRACT_IMPORT_PATTERN.test(content) && DECLARATIVE_ROUTE_BUILDER_IMPORT_PATTERN.test(content) && From 46f9c2f250330a0a518a9bb3f802a0cd203a27f8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 11 Aug 2026 16:20:44 -0700 Subject: [PATCH 2/3] fix(folders): bound the workflow folderId-branch path index reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `createWorkflow` and `updateWorkflow` each resolve a folder two ways inside one function. The folderPath branch goes through `resolveWorkflowFolderPath`, which loads the path index with `maxRows: MAX_FOLDERS_PER_WORKSPACE`; the folderId branch loaded it with no bound at all, issuing a `SELECT` over every active folder row in the workspace. In `updateWorkflow` the unbounded read and the bounded fallback sit thirty lines apart in the same function. Passes the cap at both sites, matching the read sites that already opt in. Exceeding it throws `FolderCollectionLimitExceededError` rather than truncating, because a partial path index resolves real folder paths to `undefined` and re-roots resources at the workspace root. `maxRows` deliberately stays opt-in rather than becoming the default. Folder creation does not refuse at the same ceiling on every path — `POST /api/folders` goes through the `createFolder` name/parentId variant, which passes no `maxFolderRows`, so the count guard in `executeCreateFolderAtPath` never runs and a workspace can already hold more than `MAX_FOLDERS_PER_WORKSPACE` folders. Defaulting the bound would make every path-index consumer throw for a state the product allows to exist. Reconciling reader and writer is a separate change with a user-facing limit, not a chore. --- apps/sim/lib/folders/queries.test.ts | 20 ++++++++++ apps/sim/lib/folders/queries.ts | 10 +++++ .../workflows/application/create-workflow.ts | 5 ++- .../workflows/application/update-workflow.ts | 4 +- .../application/workflow-crud.test.ts | 40 +++++++++++++++++++ 5 files changed, 77 insertions(+), 2 deletions(-) diff --git a/apps/sim/lib/folders/queries.test.ts b/apps/sim/lib/folders/queries.test.ts index dac83c35df7..ef662d282ac 100644 --- a/apps/sim/lib/folders/queries.test.ts +++ b/apps/sim/lib/folders/queries.test.ts @@ -202,6 +202,26 @@ describe('folder queries', () => { expect(dbChainMockFns.limit).toHaveBeenCalledWith(3) }) + /** + * The bound stays opt-in. Folder creation does not refuse at + * `MAX_FOLDERS_PER_WORKSPACE` on every path — `POST /api/folders` passes no + * `maxFolderRows` — so a workspace already over the cap must still be + * readable. Defaulting the bound would turn every path-index consumer into + * a hard failure for a state the product allows to exist. + */ + it('leaves the read unbounded when no maxRows is given', async () => { + queueTableRows(schemaMock.folder, [ + ROW, + { ...ROW, id: 'f-2', name: 'Archive' }, + { ...ROW, id: 'f-3', name: 'Drafts' }, + ]) + + const index = await loadActiveFolderPathIndex('ws-1', 'workflow') + + expect(dbChainMockFns.limit).not.toHaveBeenCalled() + expect(index.rowById.size).toBe(3) + }) + it('fails before returning an oversized folder list', async () => { queueTableRows(schemaMock.folder, [ROW, { ...ROW, id: 'f-2' }, { ...ROW, id: 'f-3' }]) diff --git a/apps/sim/lib/folders/queries.ts b/apps/sim/lib/folders/queries.ts index 27128143d75..b32e4d39a56 100644 --- a/apps/sim/lib/folders/queries.ts +++ b/apps/sim/lib/folders/queries.ts @@ -169,6 +169,16 @@ interface ListActiveFolderRowsOptions { maxRows?: number } +/** + * Materializes the workspace's active folder tree for one resource type. + * + * `maxRows` is opt-in: omitting it reads every active folder row. Callers that + * pass it get a throw of `FolderCollectionLimitExceededError` rather than a + * truncated index, because a partial path index resolves real folder paths to + * `undefined` and re-roots resources at the workspace root. The bound is not a + * default because folder creation does not enforce the same ceiling on every + * path, so a workspace can hold more rows than the cap and must still be read. + */ export async function loadActiveFolderPathIndex( workspaceId: string, resourceType: FolderResourceType, diff --git a/apps/sim/lib/workflows/application/create-workflow.ts b/apps/sim/lib/workflows/application/create-workflow.ts index f1123885e2e..5e9db561f28 100644 --- a/apps/sim/lib/workflows/application/create-workflow.ts +++ b/apps/sim/lib/workflows/application/create-workflow.ts @@ -4,6 +4,7 @@ import { createLogger } from '@sim/logger' import { assertFolderMutable, FolderLockedError } from '@sim/platform-authz/workflow' import { OrchestrationError } from '@/lib/core/orchestration/types' import { PlatformEvents } from '@/lib/core/telemetry' +import { MAX_FOLDERS_PER_WORKSPACE } from '@/lib/folders/constants' import { loadActiveFolderPathIndex } from '@/lib/folders/queries' import { notifyWorkflowUpdated } from '@/lib/realtime/notify' import { defineAuthorizedWorkflowUseCase } from '@/lib/workflows/application/authorized-workflow-use-case' @@ -40,7 +41,9 @@ export const createWorkflow = defineAuthorizedWorkflowUseCase({ ? await resolveWorkflowFolderPath(context.workspaceId, input.folderPath ?? '/') : { folderId: input.folderId, - index: await loadActiveFolderPathIndex(context.workspaceId, 'workflow'), + index: await loadActiveFolderPathIndex(context.workspaceId, 'workflow', undefined, { + maxRows: MAX_FOLDERS_PER_WORKSPACE, + }), } if (resolution.folderId && !resolution.index.pathById.has(resolution.folderId)) { throw new OrchestrationError('not_found', 'Folder not found') diff --git a/apps/sim/lib/workflows/application/update-workflow.ts b/apps/sim/lib/workflows/application/update-workflow.ts index d697250a93f..9002d9839df 100644 --- a/apps/sim/lib/workflows/application/update-workflow.ts +++ b/apps/sim/lib/workflows/application/update-workflow.ts @@ -146,7 +146,9 @@ async function executeWorkflowUpdate(args: { input.folderId !== undefined ? { folderId: input.folderId, - index: await loadActiveFolderPathIndex(context.workspaceId, 'workflow'), + index: await loadActiveFolderPathIndex(context.workspaceId, 'workflow', undefined, { + maxRows: MAX_FOLDERS_PER_WORKSPACE, + }), } : input.folderPath === undefined ? undefined diff --git a/apps/sim/lib/workflows/application/workflow-crud.test.ts b/apps/sim/lib/workflows/application/workflow-crud.test.ts index 24e730cb69a..f8911354272 100644 --- a/apps/sim/lib/workflows/application/workflow-crud.test.ts +++ b/apps/sim/lib/workflows/application/workflow-crud.test.ts @@ -95,6 +95,7 @@ vi.mock('@/lib/core/telemetry', () => ({ PlatformEvents: { workflowCreated: mocks.workflowCreated }, })) +import { MAX_FOLDERS_PER_WORKSPACE } from '@/lib/folders/constants' import { createWorkflow } from '@/lib/workflows/application/create-workflow' import { deleteWorkflow } from '@/lib/workflows/application/delete-workflow' import { listWorkflowVersions } from '@/lib/workflows/application/list-workflow-versions' @@ -365,6 +366,45 @@ describe('authorized workflow CRUD and version reads', () => { expect(mocks.updateRecord).toHaveBeenCalledTimes(1) }) + /** + * Both use cases resolve a folder two ways in one function. The folderPath + * branch goes through `resolveWorkflowFolderPath`, which bounds its path + * index at `MAX_FOLDERS_PER_WORKSPACE`; the folderId branch loads the index + * directly and must pass the same cap rather than issuing an unbounded + * `SELECT` over every folder row in the workspace. + */ + it.each([ + [ + 'createWorkflow', + () => + createWorkflow.execute({ + principal: personalPrincipal, + input: { workspaceId: WORKSPACE_ID, name: workflowRecord.name, folderId: 'folder-1' }, + }), + ], + [ + 'updateWorkflow', + () => + updateWorkflow.execute({ + principal: personalPrincipal, + input: { workflowId: WORKFLOW_ID, folderId: 'folder-1' }, + }), + ], + ])('bounds the %s folderId-branch path index at the workspace cap', async (_name, run) => { + mocks.loadFolderIndex.mockResolvedValue({ + rowById: new Map(), + pathById: new Map([['folder-1', '/Reports']]), + idByPath: new Map([['/Reports', 'folder-1']]), + }) + + await run() + + expect(mocks.loadFolderIndex).toHaveBeenCalledTimes(1) + expect(mocks.loadFolderIndex).toHaveBeenCalledWith(WORKSPACE_ID, 'workflow', undefined, { + maxRows: MAX_FOLDERS_PER_WORKSPACE, + }) + }) + it('does not audit an authoritative delete no-op', async () => { mocks.deleteRecord.mockResolvedValue({ success: true, From 23b779277146c2b0cf62a1b589c95dbbeda713a9 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 11 Aug 2026 15:44:35 -0700 Subject: [PATCH 3/3] fix(api): reject an undecodable offset cursor on v2 table rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/v2/tables/{tableId}/rows coerced an undecodable pagination cursor to offset 0 and re-served page one. A client paging forward reads that as a fresh first page and can loop over it forever. Every sibling v2 cursor list — logs, files, workflows, workflow runs, workflow versions, workspace members, tables, knowledge documents — already rejects with a validation error instead. Extracts the offset-cursor decode both offset-paginated v2 routes had inlined into `decodeOffsetCursor`, next to the existing `decodeSortedCursor`, so the reject-don't-restart rule has one home. --- .../api/v2/knowledge/[id]/documents/route.ts | 31 ++++++----------- apps/sim/app/api/v2/lib/response.ts | 24 ++++++++++++- .../v2/tables/[tableId]/rows/route.test.ts | 34 +++++++++++++++++-- .../app/api/v2/tables/[tableId]/rows/route.ts | 4 +-- 4 files changed, 67 insertions(+), 26 deletions(-) diff --git a/apps/sim/app/api/v2/knowledge/[id]/documents/route.ts b/apps/sim/app/api/v2/knowledge/[id]/documents/route.ts index 99e9b85d811..62871ad988f 100644 --- a/apps/sim/app/api/v2/knowledge/[id]/documents/route.ts +++ b/apps/sim/app/api/v2/knowledge/[id]/documents/route.ts @@ -30,7 +30,7 @@ import { uploadWorkspaceFile } from '@/lib/uploads/contexts/workspace' import { MAX_KNOWLEDGE_DOCUMENT_FILE_SIZE } from '@/lib/uploads/shared/types' import { validateFileType } from '@/lib/uploads/utils/validation' import { serializeDate } from '@/app/api/v1/knowledge/utils' -import { decodeCursor, encodeCursor } from '@/app/api/v2/lib/response' +import { decodeOffsetCursor, encodeCursor } from '@/app/api/v2/lib/response' export const dynamic = 'force-dynamic' export const revalidate = 0 @@ -72,25 +72,16 @@ export const GET = defineV2JsonRoute({ operation: knowledgeOperations.listDocuments, rateLimit: v2RateLimits.publicApi, errorPolicy: v2KnowledgeErrorPolicies.concealKnowledgeBaseAuthorization, - mapInput: ({ params, query }) => { - const decodedCursor = query.cursor ? decodeCursor<{ offset: number }>(query.cursor) : null - if ( - query.cursor && - (!decodedCursor || !Number.isInteger(decodedCursor.offset) || decodedCursor.offset < 0) - ) { - throw new OrchestrationError('validation', 'Invalid cursor') - } - return { - knowledgeBaseId: params.id, - assertedWorkspaceId: query.workspaceId, - enabledFilter: query.enabledFilter, - search: query.search, - limit: query.limit, - offset: decodedCursor?.offset ?? 0, - sortBy: query.sortBy, - sortOrder: query.sortOrder, - } - }, + mapInput: ({ params, query }) => ({ + knowledgeBaseId: params.id, + assertedWorkspaceId: query.workspaceId, + enabledFilter: query.enabledFilter, + search: query.search, + limit: query.limit, + offset: decodeOffsetCursor(query.cursor), + sortBy: query.sortBy, + sortOrder: query.sortOrder, + }), useCase: listKnowledgeDocuments, present: ({ documents, pagination }) => ({ data: documents.map(toV2DocumentSummary), diff --git a/apps/sim/app/api/v2/lib/response.ts b/apps/sim/app/api/v2/lib/response.ts index 475f73cb5c5..4bb6221704a 100644 --- a/apps/sim/app/api/v2/lib/response.ts +++ b/apps/sim/app/api/v2/lib/response.ts @@ -2,7 +2,11 @@ import { NextResponse } from 'next/server' import type { ZodError } from 'zod' import { type CursorKey, INVALID_CURSOR_MESSAGE } from '@/lib/api/list-query' import { getValidationErrorMessage, serializeZodIssues } from '@/lib/api/server' -import { asOrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types' +import { + asOrchestrationError, + OrchestrationError, + type OrchestrationErrorCode, +} from '@/lib/core/orchestration/types' import type { RateLimitResult, WorkspaceAccessError } from '@/app/api/v1/middleware' /** @@ -158,6 +162,24 @@ export function decodeCursor>(cursor: string): T | n } } +/** + * Reads back an offset cursor minted by `encodeCursor({ offset })`. + * + * An absent cursor means page one. A cursor that is not valid base64-JSON, or + * that does not carry a non-negative integer `offset`, is rejected rather than + * coerced to 0: silently restarting at page one while the caller believes it is + * paging forward makes a paging client loop over the first page forever. The v2 + * error policies render the thrown validation error as the canonical 400. + */ +export function decodeOffsetCursor(cursor: string | undefined): number { + if (!cursor) return 0 + const offset = decodeCursor<{ offset?: unknown }>(cursor)?.offset + if (typeof offset !== 'number' || !Number.isInteger(offset) || offset < 0) { + throw new OrchestrationError('validation', 'Invalid cursor') + } + return offset +} + /** * The sort a keyset cursor was minted under, as it is written into the cursor * payload. Comparing the whole string is what makes a mid-pagination sort diff --git a/apps/sim/app/api/v2/tables/[tableId]/rows/route.test.ts b/apps/sim/app/api/v2/tables/[tableId]/rows/route.test.ts index 82e9c837f43..c73dcd52e2b 100644 --- a/apps/sim/app/api/v2/tables/[tableId]/rows/route.test.ts +++ b/apps/sim/app/api/v2/tables/[tableId]/rows/route.test.ts @@ -111,8 +111,36 @@ describe('/api/v2/tables/[tableId]/rows', () => { }) }) - it('retains malformed GET cursor fallback compatibility', async () => { - const req = request('GET', undefined, `?workspaceId=${WORKSPACE_ID}&limit=25&cursor=malformed`) + /** + * Coercing an undecodable cursor to offset 0 re-served page one while the + * client believed it was paging forward, which loops a paging client forever. + * Every sibling v2 cursor list rejects instead, so this one does too. + */ + it.each([ + ['undecodable base64-JSON', 'malformed'], + ['a payload with no offset', Buffer.from(JSON.stringify({ o: 5 })).toString('base64')], + ['a non-integer offset', Buffer.from(JSON.stringify({ offset: 1.5 })).toString('base64')], + ['a negative offset', Buffer.from(JSON.stringify({ offset: -1 })).toString('base64')], + ])('rejects a GET cursor with %s instead of restarting pagination', async (_label, cursor) => { + const req = request( + 'GET', + undefined, + `?workspaceId=${WORKSPACE_ID}&limit=25&cursor=${encodeURIComponent(cursor)}` + ) + const response = await GET(req, CONTEXT) + + expect(response.status).toBe(400) + expect((await response.json()).error).toMatchObject({ message: 'Invalid cursor' }) + expect(mocks.listRows).not.toHaveBeenCalled() + }) + + it('resumes at the encoded offset for a well-formed cursor', async () => { + const cursor = Buffer.from(JSON.stringify({ offset: 50 })).toString('base64') + const req = request( + 'GET', + undefined, + `?workspaceId=${WORKSPACE_ID}&limit=25&cursor=${encodeURIComponent(cursor)}` + ) const response = await GET(req, CONTEXT) expect(response.status).toBe(200) @@ -122,7 +150,7 @@ describe('/api/v2/tables/[tableId]/rows', () => { tableId: 'table-1', assertedWorkspaceId: WORKSPACE_ID, limit: 25, - offset: 0, + offset: 50, }, request: req, }) diff --git a/apps/sim/app/api/v2/tables/[tableId]/rows/route.ts b/apps/sim/app/api/v2/tables/[tableId]/rows/route.ts index 6586ec88e1f..86e6560456b 100644 --- a/apps/sim/app/api/v2/tables/[tableId]/rows/route.ts +++ b/apps/sim/app/api/v2/tables/[tableId]/rows/route.ts @@ -14,7 +14,7 @@ import { updateTableRows, } from '@/lib/table/application/rows' import { namedRowMapper } from '@/lib/table/cell-format' -import { decodeCursor, encodeCursor } from '@/app/api/v2/lib/response' +import { decodeOffsetCursor, encodeCursor } from '@/app/api/v2/lib/response' import { toApiRow } from '@/app/api/v2/tables/utils' export const dynamic = 'force-dynamic' @@ -30,7 +30,7 @@ export const GET = defineV2JsonRoute({ tableId: params.tableId, assertedWorkspaceId: query.workspaceId, limit: query.limit, - offset: query.cursor ? (decodeCursor<{ offset: number }>(query.cursor)?.offset ?? 0) : 0, + offset: decodeOffsetCursor(query.cursor), }), useCase: listTableRows, present: ({ table, rows, nextOffset }) => {