Skip to content

Commit 46f9c2f

Browse files
committed
fix(folders): bound the workflow folderId-branch path index reads
`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.
1 parent aa8f09a commit 46f9c2f

5 files changed

Lines changed: 77 additions & 2 deletions

File tree

apps/sim/lib/folders/queries.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,26 @@ describe('folder queries', () => {
202202
expect(dbChainMockFns.limit).toHaveBeenCalledWith(3)
203203
})
204204

205+
/**
206+
* The bound stays opt-in. Folder creation does not refuse at
207+
* `MAX_FOLDERS_PER_WORKSPACE` on every path — `POST /api/folders` passes no
208+
* `maxFolderRows` — so a workspace already over the cap must still be
209+
* readable. Defaulting the bound would turn every path-index consumer into
210+
* a hard failure for a state the product allows to exist.
211+
*/
212+
it('leaves the read unbounded when no maxRows is given', async () => {
213+
queueTableRows(schemaMock.folder, [
214+
ROW,
215+
{ ...ROW, id: 'f-2', name: 'Archive' },
216+
{ ...ROW, id: 'f-3', name: 'Drafts' },
217+
])
218+
219+
const index = await loadActiveFolderPathIndex('ws-1', 'workflow')
220+
221+
expect(dbChainMockFns.limit).not.toHaveBeenCalled()
222+
expect(index.rowById.size).toBe(3)
223+
})
224+
205225
it('fails before returning an oversized folder list', async () => {
206226
queueTableRows(schemaMock.folder, [ROW, { ...ROW, id: 'f-2' }, { ...ROW, id: 'f-3' }])
207227

apps/sim/lib/folders/queries.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,16 @@ interface ListActiveFolderRowsOptions {
169169
maxRows?: number
170170
}
171171

172+
/**
173+
* Materializes the workspace's active folder tree for one resource type.
174+
*
175+
* `maxRows` is opt-in: omitting it reads every active folder row. Callers that
176+
* pass it get a throw of `FolderCollectionLimitExceededError` rather than a
177+
* truncated index, because a partial path index resolves real folder paths to
178+
* `undefined` and re-roots resources at the workspace root. The bound is not a
179+
* default because folder creation does not enforce the same ceiling on every
180+
* path, so a workspace can hold more rows than the cap and must still be read.
181+
*/
172182
export async function loadActiveFolderPathIndex(
173183
workspaceId: string,
174184
resourceType: FolderResourceType,

apps/sim/lib/workflows/application/create-workflow.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { createLogger } from '@sim/logger'
44
import { assertFolderMutable, FolderLockedError } from '@sim/platform-authz/workflow'
55
import { OrchestrationError } from '@/lib/core/orchestration/types'
66
import { PlatformEvents } from '@/lib/core/telemetry'
7+
import { MAX_FOLDERS_PER_WORKSPACE } from '@/lib/folders/constants'
78
import { loadActiveFolderPathIndex } from '@/lib/folders/queries'
89
import { notifyWorkflowUpdated } from '@/lib/realtime/notify'
910
import { defineAuthorizedWorkflowUseCase } from '@/lib/workflows/application/authorized-workflow-use-case'
@@ -40,7 +41,9 @@ export const createWorkflow = defineAuthorizedWorkflowUseCase({
4041
? await resolveWorkflowFolderPath(context.workspaceId, input.folderPath ?? '/')
4142
: {
4243
folderId: input.folderId,
43-
index: await loadActiveFolderPathIndex(context.workspaceId, 'workflow'),
44+
index: await loadActiveFolderPathIndex(context.workspaceId, 'workflow', undefined, {
45+
maxRows: MAX_FOLDERS_PER_WORKSPACE,
46+
}),
4447
}
4548
if (resolution.folderId && !resolution.index.pathById.has(resolution.folderId)) {
4649
throw new OrchestrationError('not_found', 'Folder not found')

apps/sim/lib/workflows/application/update-workflow.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,9 @@ async function executeWorkflowUpdate(args: {
146146
input.folderId !== undefined
147147
? {
148148
folderId: input.folderId,
149-
index: await loadActiveFolderPathIndex(context.workspaceId, 'workflow'),
149+
index: await loadActiveFolderPathIndex(context.workspaceId, 'workflow', undefined, {
150+
maxRows: MAX_FOLDERS_PER_WORKSPACE,
151+
}),
150152
}
151153
: input.folderPath === undefined
152154
? undefined

apps/sim/lib/workflows/application/workflow-crud.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ vi.mock('@/lib/core/telemetry', () => ({
9595
PlatformEvents: { workflowCreated: mocks.workflowCreated },
9696
}))
9797

98+
import { MAX_FOLDERS_PER_WORKSPACE } from '@/lib/folders/constants'
9899
import { createWorkflow } from '@/lib/workflows/application/create-workflow'
99100
import { deleteWorkflow } from '@/lib/workflows/application/delete-workflow'
100101
import { listWorkflowVersions } from '@/lib/workflows/application/list-workflow-versions'
@@ -365,6 +366,45 @@ describe('authorized workflow CRUD and version reads', () => {
365366
expect(mocks.updateRecord).toHaveBeenCalledTimes(1)
366367
})
367368

369+
/**
370+
* Both use cases resolve a folder two ways in one function. The folderPath
371+
* branch goes through `resolveWorkflowFolderPath`, which bounds its path
372+
* index at `MAX_FOLDERS_PER_WORKSPACE`; the folderId branch loads the index
373+
* directly and must pass the same cap rather than issuing an unbounded
374+
* `SELECT` over every folder row in the workspace.
375+
*/
376+
it.each([
377+
[
378+
'createWorkflow',
379+
() =>
380+
createWorkflow.execute({
381+
principal: personalPrincipal,
382+
input: { workspaceId: WORKSPACE_ID, name: workflowRecord.name, folderId: 'folder-1' },
383+
}),
384+
],
385+
[
386+
'updateWorkflow',
387+
() =>
388+
updateWorkflow.execute({
389+
principal: personalPrincipal,
390+
input: { workflowId: WORKFLOW_ID, folderId: 'folder-1' },
391+
}),
392+
],
393+
])('bounds the %s folderId-branch path index at the workspace cap', async (_name, run) => {
394+
mocks.loadFolderIndex.mockResolvedValue({
395+
rowById: new Map(),
396+
pathById: new Map([['folder-1', '/Reports']]),
397+
idByPath: new Map([['/Reports', 'folder-1']]),
398+
})
399+
400+
await run()
401+
402+
expect(mocks.loadFolderIndex).toHaveBeenCalledTimes(1)
403+
expect(mocks.loadFolderIndex).toHaveBeenCalledWith(WORKSPACE_ID, 'workflow', undefined, {
404+
maxRows: MAX_FOLDERS_PER_WORKSPACE,
405+
})
406+
})
407+
368408
it('does not audit an authoritative delete no-op', async () => {
369409
mocks.deleteRecord.mockResolvedValue({
370410
success: true,

0 commit comments

Comments
 (0)