Skip to content

Commit 9fc6586

Browse files
authored
fix(v2): hold create to the rules update enforces, and bind the last two cursors (#6684)
Three defects, one shape: a rule applied to one path and not its sibling. Two were found by probing the live surface after the previous fixes deployed, and the third by reading for the pattern. Creating a workflow group through the public surface validated almost nothing the update path validates. An enrichment group could name an enrichment the registry does not define, or an output the enrichment does not have, or carry no output id at all — each a 201 storing a column no run can ever write, discovered only when the caller later tried to edit the group and got the 400 create should have given. The workflow half was the same: a fabricated block-and-path coordinate was stored on create and refused on update. Create now runs the same two registry helpers and the same workflow-output check the update path uses. The discriminator there is the backing workflow id, not the declared type. The workflow sidebar creates enrichment-template groups labelled `enrichment` while backed by a real workflow and carrying no enrichment id, so keying on the label would have refused the first-party create path outright. A group's producer type could also be relabelled after the fact into a state creation refuses. Nothing rejected it and nothing could repair it, since the update body carries no enrichment id to supply. Relabelling an enrichment group as workflow-backed is the harmful direction: it keeps the enrichment id while moving the group onto the workflow branch with an empty workflow id, so every cell run fails. An update may now only restate the type the group already has. The workflow-version and workspace-member lists were the last two paged reads minting cursors with no route identity, so a token from one parent resumed another at a position that silently skips rows — the defect the previous change closed everywhere else. Both now wrap their domain token with the same scope binding, and the pagination guardrail gained a declaration of every nested list's parent path param, because the old one recorded only query filters and so could not tell an unfiltered list from a forgotten parent.
1 parent 4ff339e commit 9fc6586

9 files changed

Lines changed: 592 additions & 43 deletions

File tree

apps/docs/openapi-v2-tables.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6806,7 +6806,7 @@
68066806
"enum": ["live", "deployed"]
68076807
},
68086808
"type": {
6809-
"description": "Replacement workflow-group producer type.",
6809+
"description": "Workflow-group producer type. Must match the group's stored type — a group's producer cannot be changed after creation.",
68106810
"type": "string",
68116811
"enum": ["manual", "enrichment"]
68126812
},

apps/sim/app/api/v2/workflows/[id]/versions/route.test.ts

Lines changed: 79 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
} from '@sim/testing'
1313
import { NextRequest } from 'next/server'
1414
import { beforeEach, describe, expect, it, vi } from 'vitest'
15-
import { UNREADABLE_CURSOR_MESSAGE } from '@/lib/api/cursor-binding'
15+
import { REFILTERED_CURSOR_MESSAGE, UNREADABLE_CURSOR_MESSAGE } from '@/lib/api/cursor-binding'
1616

1717
const mocks = vi.hoisted(() => ({
1818
listVersions: vi.fn(),
@@ -43,6 +43,50 @@ const auth = {
4343
}
4444
const context = { params: Promise.resolve({ id: 'workflow-1' }) }
4545

46+
function contextFor(workflowId: string) {
47+
return { params: Promise.resolve({ id: workflowId }) }
48+
}
49+
50+
function listVersions(workflowId: string, query = '') {
51+
return GET(
52+
new NextRequest(`http://localhost/api/v2/workflows/${workflowId}/versions${query}`),
53+
contextFor(workflowId)
54+
)
55+
}
56+
57+
/**
58+
* A cursor as the route itself mints it, rather than a payload hand-built by
59+
* the test. The binding a cursor carries is the route's to compute, so a test
60+
* that reconstructs it would pass against a route that stopped applying one.
61+
*/
62+
async function mintCursor(workflowId: string): Promise<string> {
63+
mocks.listVersions.mockResolvedValueOnce({
64+
versions: [
65+
{
66+
id: 'version-5',
67+
version: 5,
68+
name: 'Production',
69+
description: null,
70+
isActive: true,
71+
createdAt: new Date('2026-08-01T00:00:00.000Z'),
72+
deployedByName: 'Ada',
73+
latestOperationStatus: 'active',
74+
},
75+
],
76+
hasMore: true,
77+
})
78+
const { nextCursor } = await (await listVersions(workflowId)).json()
79+
expect(typeof nextCursor).toBe('string')
80+
return nextCursor
81+
}
82+
83+
/** A minted cursor's binding, carrying a forged payload inside it. */
84+
async function forgeInsideBinding(workflowId: string, payload: unknown): Promise<string> {
85+
const { scope } = JSON.parse(Buffer.from(await mintCursor(workflowId), 'base64').toString())
86+
const inner = Buffer.from(JSON.stringify(payload)).toString('base64')
87+
return Buffer.from(JSON.stringify({ scope, inner })).toString('base64')
88+
}
89+
4690
describe('GET /api/v2/workflows/[id]/versions', () => {
4791
beforeEach(() => {
4892
vi.clearAllMocks()
@@ -125,33 +169,52 @@ describe('GET /api/v2/workflows/[id]/versions', () => {
125169
['carrying an unknown key', { version: 2, sort: 'name' }],
126170
['missing its key', {}],
127171
])('rejects a forged cursor %s', async (_case, payload) => {
128-
const cursor = Buffer.from(JSON.stringify(payload)).toString('base64')
129-
const response = await GET(
130-
new NextRequest(
131-
`http://localhost/api/v2/workflows/workflow-1/versions?cursor=${encodeURIComponent(cursor)}`
132-
),
133-
context
134-
)
172+
const cursor = await forgeInsideBinding('workflow-1', payload)
173+
mocks.listVersions.mockClear()
174+
175+
const response = await listVersions('workflow-1', `?cursor=${encodeURIComponent(cursor)}`)
135176

136177
expect(response.status).toBe(400)
137178
expect(mocks.listVersions).not.toHaveBeenCalled()
138179
})
139180

140-
it('resumes from a well-formed cursor', async () => {
141-
const cursor = Buffer.from(JSON.stringify({ version: 5 })).toString('base64')
142-
const response = await GET(
143-
new NextRequest(
144-
`http://localhost/api/v2/workflows/workflow-1/versions?cursor=${encodeURIComponent(cursor)}`
145-
),
146-
context
147-
)
181+
/**
182+
* The regression guard for the binding: a list still pages through its OWN
183+
* cursor. A token that no route accepts binds nothing — it just breaks
184+
* pagination.
185+
*/
186+
it('resumes from the cursor it minted', async () => {
187+
const cursor = await mintCursor('workflow-1')
188+
mocks.listVersions.mockClear()
189+
190+
const response = await listVersions('workflow-1', `?cursor=${encodeURIComponent(cursor)}`)
148191

149192
expect(response.status).toBe(200)
150193
expect(mocks.listVersions).toHaveBeenCalledWith(
151194
expect.objectContaining({ input: expect.objectContaining({ afterVersion: 5 }) })
152195
)
153196
})
154197

198+
/**
199+
* A `version` is an ordinal every workflow's history numbers from 1, so an
200+
* unbound token from a sibling workflow decoded cleanly and resumed from a
201+
* position in a history the caller never walked — silently skipping versions.
202+
*/
203+
it('refuses a cursor minted on another workflow', async () => {
204+
const cursor = await mintCursor('workflow-1')
205+
mocks.listVersions.mockClear()
206+
207+
const response = await listVersions('workflow-2', `?cursor=${encodeURIComponent(cursor)}`)
208+
209+
expect(response.status).toBe(400)
210+
expect((await response.json()).error.message).toBe(REFILTERED_CURSOR_MESSAGE)
211+
expect(mocks.listVersions).not.toHaveBeenCalled()
212+
})
213+
214+
it('mints a cursor bound to the workflow that answered', async () => {
215+
expect(await mintCursor('workflow-1')).not.toBe(await mintCursor('workflow-2'))
216+
})
217+
155218
it('rejects an unauthenticated request', async () => {
156219
v2RouteMocks.authenticate.mockRejectedValueOnce(new MockV2ApiKeyUnauthenticatedError())
157220

apps/sim/app/api/v2/workflows/[id]/versions/route.ts

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,45 @@ import {
33
v2ListWorkflowVersionsContract,
44
v2WorkflowVersionCursorSchema,
55
} from '@/lib/api/contracts/v2/workflows'
6-
import { UNREADABLE_CURSOR_MESSAGE } from '@/lib/api/cursor-binding'
6+
import { cursorRoute, cursorScopeKey, UNREADABLE_CURSOR_MESSAGE } from '@/lib/api/cursor-binding'
77
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
88
import { OrchestrationError } from '@/lib/core/orchestration/types'
99
import { v2WorkflowErrorPolicies } from '@/lib/workflows/api'
1010
import { listWorkflowVersions } from '@/lib/workflows/application/list-workflow-versions'
1111
import { workflowOperations } from '@/lib/workflows/application/operations'
12-
import { decodeCursor, encodeCursor } from '@/app/api/v2/lib/response'
12+
import {
13+
decodeCursor,
14+
encodeCursor,
15+
encodeScopedCursor,
16+
readScopedCursor,
17+
} from '@/app/api/v2/lib/response'
1318

1419
export const dynamic = 'force-dynamic'
1520
export const revalidate = 0
1621

22+
/**
23+
* The sequence a version cursor names a position in: this list, on THIS
24+
* workflow.
25+
*
26+
* The payload is a bare `version` ordinal and every workflow numbers its
27+
* history from 1, so an unscoped token minted on one workflow decoded cleanly
28+
* against another and answered 200 from a position the caller never reached.
29+
* The workflow id lives in the path, so the route is the only place that knows
30+
* which history the ordinal counts within.
31+
*/
32+
function versionCursorScope(workflowId: string): string {
33+
return cursorScopeKey(cursorRoute(v2ListWorkflowVersionsContract, { id: workflowId }))
34+
}
35+
1736
export const GET = defineV2JsonRoute({
1837
contract: v2ListWorkflowVersionsContract,
1938
auth: v2ApiKeyAuth,
2039
operation: workflowOperations.listVersions,
2140
rateLimit: v2RateLimits.publicApi,
2241
errorPolicy: v2WorkflowErrorPolicies.concealWorkflowAuthorization,
2342
mapInput: ({ params, query }) => {
24-
const decoded = query.cursor
25-
? v2WorkflowVersionCursorSchema.safeParse(decodeCursor(query.cursor))
26-
: undefined
43+
const inner = readScopedCursor(query.cursor, versionCursorScope(params.id))
44+
const decoded = inner ? v2WorkflowVersionCursorSchema.safeParse(decodeCursor(inner)) : undefined
2745
if (decoded && !decoded.success) {
2846
throw new OrchestrationError('validation', UNREADABLE_CURSOR_MESSAGE)
2947
}
@@ -34,7 +52,7 @@ export const GET = defineV2JsonRoute({
3452
}
3553
},
3654
useCase: listWorkflowVersions,
37-
present: ({ versions, hasMore }) => {
55+
present: ({ versions, hasMore }, { params }) => {
3856
const data: V2WorkflowVersion[] = versions.map((version) => ({
3957
id: version.id,
4058
version: version.version,
@@ -50,7 +68,10 @@ export const GET = defineV2JsonRoute({
5068
data,
5169
nextCursor:
5270
hasMore && data.length > 0
53-
? encodeCursor({ version: data[data.length - 1].version })
71+
? encodeScopedCursor(
72+
versionCursorScope(params.id),
73+
encodeCursor({ version: data[data.length - 1].version })
74+
)
5475
: null,
5576
}
5677
},

apps/sim/app/api/v2/workspaces/[workspaceId]/members/route.ts

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import {
22
v2ListWorkspaceMembersContract,
33
v2WorkspaceMemberCursorSchema,
44
} from '@/lib/api/contracts/v2/workspaces'
5-
import { UNREADABLE_CURSOR_MESSAGE } from '@/lib/api/cursor-binding'
5+
import { cursorRoute, cursorScopeKey, UNREADABLE_CURSOR_MESSAGE } from '@/lib/api/cursor-binding'
66
import {
77
defineV2JsonRoute,
88
v2ApiKeyAuth,
@@ -12,7 +12,26 @@ import {
1212
import { OrchestrationError } from '@/lib/core/orchestration/types'
1313
import { listPublicWorkspaceMembers } from '@/lib/workspaces/application/list-public-workspace-members'
1414
import { workspaceOperations } from '@/lib/workspaces/application/operations'
15-
import { decodeCursor, encodeCursor } from '@/app/api/v2/lib/response'
15+
import {
16+
decodeCursor,
17+
encodeCursor,
18+
encodeScopedCursor,
19+
readScopedCursor,
20+
} from '@/app/api/v2/lib/response'
21+
22+
/**
23+
* The sequence a member cursor names a position in: this roster, on THIS
24+
* workspace.
25+
*
26+
* The payload is a bare email, and the same person is a member of every
27+
* workspace they belong to, so an unscoped token minted on one roster decoded
28+
* cleanly against another and resumed from an email that names a different
29+
* position in it. The workspace id lives in the path, so the route is the only
30+
* place that knows which roster the email indexes.
31+
*/
32+
function memberCursorScope(workspaceId: string): string {
33+
return cursorScopeKey(cursorRoute(v2ListWorkspaceMembersContract, { workspaceId }))
34+
}
1635

1736
/** GET /api/v2/workspaces/[workspaceId]/members — Effective member roster. */
1837
export const GET = defineV2JsonRoute({
@@ -22,9 +41,8 @@ export const GET = defineV2JsonRoute({
2241
rateLimit: v2RateLimits.publicApi,
2342
errorPolicy: v2OrchestrationErrorPolicy,
2443
mapInput: ({ params, query }) => {
25-
const decoded = query.cursor
26-
? v2WorkspaceMemberCursorSchema.safeParse(decodeCursor(query.cursor))
27-
: undefined
44+
const inner = readScopedCursor(query.cursor, memberCursorScope(params.workspaceId))
45+
const decoded = inner ? v2WorkspaceMemberCursorSchema.safeParse(decodeCursor(inner)) : undefined
2846
if (decoded && !decoded.success) {
2947
throw new OrchestrationError('validation', UNREADABLE_CURSOR_MESSAGE)
3048
}
@@ -35,7 +53,7 @@ export const GET = defineV2JsonRoute({
3553
}
3654
},
3755
useCase: listPublicWorkspaceMembers,
38-
present: ({ page }) => ({
56+
present: ({ page }, { params }) => ({
3957
data: page.members.map((member) => ({
4058
email: member.email,
4159
name: member.name,
@@ -44,6 +62,11 @@ export const GET = defineV2JsonRoute({
4462
isExternal: member.isExternal,
4563
joinedAt: member.joinedAt.toISOString(),
4664
})),
47-
nextCursor: page.nextEmail ? encodeCursor({ email: page.nextEmail }) : null,
65+
nextCursor: page.nextEmail
66+
? encodeScopedCursor(
67+
memberCursorScope(params.workspaceId),
68+
encodeCursor({ email: page.nextEmail })
69+
)
70+
: null,
4871
}),
4972
})

0 commit comments

Comments
 (0)