Skip to content

Commit 777b3e9

Browse files
waleedlatif1waleedlatif1
authored
fix(projects): revoke banned users' keys first and create imported workflows atomically (#8657)
* fix(projects): revoke banned users' keys first and create imported workflows atomically - disableUserResources deletes the user's API keys before archiving their workspaces, so a failed archive can no longer leave keys behind - Admin and superuser imports create the row, its state and variables in one transaction through createWorkflowWithState (name deduplicated under the workspace lock), so a concurrent archive can't archive the row before its state lands, and same-name imports retry instead of failing - Workspace DELETE records the Project archive audit before its not-found guard, so a retried partial deletion still lands in the audit trail * fix(projects): fail the import when state persistence reports failure, and cover its rollback * test(projects): type the import fixtures and write a report artifact
1 parent 3064219 commit 777b3e9

7 files changed

Lines changed: 253 additions & 136 deletions

File tree

‎apps/sim/app/api/superuser/import-workflow/route.ts‎

Lines changed: 22 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,9 @@ import { loadCopilotChatMessages } from '@/lib/mothership/chat/lifecycle'
1313
import { appendCopilotChatMessages } from '@/lib/mothership/chat/messages-store'
1414
import { verifyEffectiveSuperUser } from '@/lib/permissions/super-user'
1515
import { parseWorkflowJson } from '@/lib/workflows/operations/import-export'
16-
import { insertNewWorkflowRow } from '@/lib/workflows/persistence/new-workflow-row'
17-
import {
18-
loadWorkflowFromNormalizedTables,
19-
saveWorkflowToNormalizedTables,
20-
} from '@/lib/workflows/persistence/utils'
16+
import { createWorkflowWithState } from '@/lib/workflows/orchestration/workflow-lifecycle'
17+
import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/persistence/utils'
2118
import { sanitizeForExport } from '@/lib/workflows/sanitization/json-sanitizer'
22-
import { deduplicateWorkflowName } from '@/lib/workflows/utils'
2319

2420
const logger = createLogger('SuperUserImportWorkflow')
2521

@@ -131,40 +127,30 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
131127

132128
// Create new workflow record
133129
const newWorkflowId = generateId()
134-
const dedupedName = await deduplicateWorkflowName(
135-
`[Debug Import] ${sourceWorkflow.name}`,
136-
targetWorkspaceId,
137-
null
138-
)
139-
140-
await db.transaction((tx) =>
141-
insertNewWorkflowRow(tx, {
142-
id: newWorkflowId,
143-
userId: session.user.id,
144-
workspaceId: targetWorkspaceId,
145-
folderId: null,
146-
name: dedupedName,
147-
description: sourceWorkflow.description,
148-
variables: sourceWorkflow.variables || {},
149-
})
150-
)
151130

152-
// Save using existing persistence logic
153-
const saveResult = await saveWorkflowToNormalizedTables(newWorkflowId, importedData, {
154-
/**
155-
* Actorless. The superuser debug import is a platform-operator tool for
156-
* reproducing a customer's workflow, not a member authoring one, so no
157-
* workspace permission group governs it.
158-
*/
159-
workspaceId: null,
160-
subjectUserId: null,
131+
const created = await createWorkflowWithState({
132+
id: newWorkflowId,
133+
userId: session.user.id,
134+
workspaceId: targetWorkspaceId,
135+
folderId: null,
136+
name: `[Debug Import] ${sourceWorkflow.name}`,
137+
description: sourceWorkflow.description,
138+
variables: sourceWorkflow.variables || {},
139+
state: importedData,
140+
governance: {
141+
/**
142+
* Actorless. The superuser debug import is a platform-operator tool for
143+
* reproducing a customer's workflow, not a member authoring one, so no
144+
* workspace permission group governs it.
145+
*/
146+
workspaceId: null,
147+
subjectUserId: null,
148+
},
161149
})
162150

163-
if (!saveResult.success) {
164-
// Clean up the workflow record if save failed
165-
await db.delete(workflow).where(eq(workflow.id, newWorkflowId))
151+
if (!created.success) {
166152
return NextResponse.json(
167-
{ error: `Failed to save workflow state: ${saveResult.error}` },
153+
{ error: `Failed to save workflow state: ${created.error}` },
168154
{ status: 500 }
169155
)
170156
}

‎apps/sim/app/api/v1/admin/workflows/import/route.ts‎

Lines changed: 18 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616

1717
import { db } from '@sim/db'
18-
import { workflow, workspace } from '@sim/db/schema'
18+
import { workspace } from '@sim/db/schema'
1919
import { createLogger } from '@sim/logger'
2020
import {
2121
assertFolderInWorkspace,
@@ -31,10 +31,8 @@ import { parseRequest } from '@/lib/api/server'
3131
import { asOrchestrationError } from '@/lib/core/orchestration/types'
3232
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
3333
import { parseWorkflowJson } from '@/lib/workflows/operations/import-export'
34-
import { insertNewWorkflowRow } from '@/lib/workflows/persistence/new-workflow-row'
34+
import { createWorkflowWithState } from '@/lib/workflows/orchestration/workflow-lifecycle'
3535
import { prepareWorkflowStateForPersistence } from '@/lib/workflows/persistence/prepare-state'
36-
import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils'
37-
import { deduplicateWorkflowName } from '@/lib/workflows/utils'
3836
import { normalizeImportedVariables } from '@/lib/workflows/variables/parse'
3937
import { withAdminAuth } from '@/app/api/v1/admin/middleware'
4038
import {
@@ -114,18 +112,6 @@ export const POST = withRouteHandler(
114112
)
115113

116114
const workflowId = generateId()
117-
const dedupedName = await deduplicateWorkflowName(workflowName, workspaceId, folderId || null)
118-
119-
await db.transaction((tx) =>
120-
insertNewWorkflowRow(tx, {
121-
id: workflowId,
122-
userId: workspaceData.ownerId,
123-
workspaceId,
124-
folderId: folderId || null,
125-
name: dedupedName,
126-
description: workflowDescription,
127-
})
128-
)
129115

130116
/**
131117
* Same normalization the editor and the v1 import API run, via the one
@@ -138,13 +124,16 @@ export const POST = withRouteHandler(
138124
logger.warn('Admin API: normalized imported workflow with warnings', { warnings })
139125
}
140126

141-
const saveResult = await saveWorkflowToNormalizedTables(
142-
workflowId,
143-
{
144-
...workflowData,
145-
...preparedState,
146-
},
147-
{
127+
const created = await createWorkflowWithState({
128+
id: workflowId,
129+
userId: workspaceData.ownerId,
130+
workspaceId,
131+
folderId: folderId || null,
132+
name: workflowName,
133+
description: workflowDescription,
134+
variables: normalizeImportedVariables(workflowData.variables),
135+
state: { ...workflowData, ...preparedState },
136+
governance: {
148137
/**
149138
* Actorless. This is the platform-admin surface: the caller is a Sim
150139
* operator restoring data, not a member of the target workspace, so no
@@ -153,29 +142,20 @@ export const POST = withRouteHandler(
153142
*/
154143
workspaceId: null,
155144
subjectUserId: null,
156-
}
157-
)
158-
159-
if (!saveResult.success) {
160-
await db.delete(workflow).where(eq(workflow.id, workflowId))
161-
return internalErrorResponse(`Failed to save workflow state: ${saveResult.error}`)
162-
}
145+
},
146+
})
163147

164-
const variablesRecord = normalizeImportedVariables(workflowData.variables)
165-
if (Object.keys(variablesRecord).length > 0) {
166-
await db
167-
.update(workflow)
168-
.set({ variables: variablesRecord, updatedAt: new Date() })
169-
.where(eq(workflow.id, workflowId))
148+
if (!created.success) {
149+
return internalErrorResponse(`Failed to save workflow state: ${created.error}`)
170150
}
171151

172152
logger.info(
173-
`Admin API: Imported workflow ${workflowId} (${dedupedName}) into workspace ${workspaceId}`
153+
`Admin API: Imported workflow ${workflowId} (${created.workflow.name}) into workspace ${workspaceId}`
174154
)
175155

176156
const response: ImportSuccessResponse = {
177157
workflowId,
178-
name: dedupedName,
158+
name: created.workflow.name,
179159
success: true,
180160
}
181161

‎apps/sim/app/api/v1/admin/workspaces/[id]/import/route.ts‎

Lines changed: 19 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
*/
2525

2626
import { db } from '@sim/db'
27-
import { folder as folderTable, workflow } from '@sim/db/schema'
27+
import { folder as folderTable } from '@sim/db/schema'
2828
import { createLogger } from '@sim/logger'
2929
import { getErrorMessage, getPostgresErrorCode } from '@sim/utils/errors'
3030
import { generateId } from '@sim/utils/id'
@@ -45,10 +45,8 @@ import {
4545
extractWorkflowsFromZip,
4646
parseWorkflowJson,
4747
} from '@/lib/workflows/operations/import-export'
48-
import { insertNewWorkflowRow } from '@/lib/workflows/persistence/new-workflow-row'
48+
import { createWorkflowWithState } from '@/lib/workflows/orchestration/workflow-lifecycle'
4949
import { prepareWorkflowStateForPersistence } from '@/lib/workflows/persistence/prepare-state'
50-
import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils'
51-
import { deduplicateWorkflowName } from '@/lib/workflows/utils'
5250
import { normalizeImportedVariables } from '@/lib/workflows/variables/parse'
5351
import { getWorkspaceWithOwner } from '@/lib/workspaces/permissions/utils'
5452
import { withAdminAuthParams } from '@/app/api/v1/admin/middleware'
@@ -348,18 +346,6 @@ async function importSingleWorkflow(
348346
}
349347

350348
const workflowId = generateId()
351-
const dedupedName = await deduplicateWorkflowName(workflowName, workspaceId, targetFolderId)
352-
353-
await db.transaction((tx) =>
354-
insertNewWorkflowRow(tx, {
355-
id: workflowId,
356-
userId: ownerId,
357-
workspaceId,
358-
folderId: targetFolderId,
359-
name: dedupedName,
360-
description: workflowData.metadata?.description || 'Imported via Admin API',
361-
})
362-
)
363349

364350
/**
365351
* Same normalization the editor, the v1 import API and the single-workflow
@@ -370,16 +356,19 @@ async function importSingleWorkflow(
370356
*/
371357
const { state: preparedState, warnings } = prepareWorkflowStateForPersistence(workflowData)
372358
if (warnings.length > 0) {
373-
logger.warn(`Admin API: normalized "${dedupedName}" with warnings`, { warnings })
359+
logger.warn(`Admin API: normalized "${workflowName}" with warnings`, { warnings })
374360
}
375361

376-
const saveResult = await saveWorkflowToNormalizedTables(
377-
workflowId,
378-
{
379-
...workflowData,
380-
...preparedState,
381-
},
382-
{
362+
const created = await createWorkflowWithState({
363+
id: workflowId,
364+
userId: ownerId,
365+
workspaceId,
366+
folderId: targetFolderId,
367+
name: workflowName,
368+
description: workflowData.metadata?.description || 'Imported via Admin API',
369+
variables: normalizeImportedVariables(workflowData.variables),
370+
state: { ...workflowData, ...preparedState },
371+
governance: {
383372
/**
384373
* Actorless. This is the platform-admin surface: the caller is a Sim
385374
* operator restoring data, not a member of the target workspace, so no
@@ -388,35 +377,21 @@ async function importSingleWorkflow(
388377
*/
389378
workspaceId: null,
390379
subjectUserId: null,
391-
}
392-
)
380+
},
381+
})
393382

394-
if (!saveResult.success) {
395-
await db.delete(workflow).where(eq(workflow.id, workflowId))
383+
if (!created.success) {
396384
return {
397385
workflowId: '',
398-
name: dedupedName,
386+
name: workflowName,
399387
success: false,
400-
error: `Failed to save state: ${saveResult.error}`,
388+
error: `Failed to save state: ${created.error}`,
401389
}
402390
}
403391

404-
/**
405-
* Previously guarded on `Array.isArray`, which silently dropped every
406-
* variable in the current record form — the exact shape this workspace's
407-
* own export emits — so an export/import round trip lost all of them.
408-
*/
409-
const variablesRecord = normalizeImportedVariables(workflowData.variables)
410-
if (Object.keys(variablesRecord).length > 0) {
411-
await db
412-
.update(workflow)
413-
.set({ variables: variablesRecord, updatedAt: new Date() })
414-
.where(eq(workflow.id, workflowId))
415-
}
416-
417392
return {
418393
workflowId,
419-
name: dedupedName,
394+
name: created.workflow.name,
420395
success: true,
421396
}
422397
} catch (error) {

‎apps/sim/app/api/workspaces/[id]/route.ts‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,22 @@ export const DELETE = withRouteHandler(
285285
requestId: `workspace-${workspaceId}`,
286286
})
287287

288+
/** Recorded first: a retry that finishes an earlier partial deletion still archives it. */
289+
if (archiveResult.archivedProject) {
290+
recordAudit({
291+
workspaceId,
292+
actorId: session.user.id,
293+
actorName: session.user.name,
294+
actorEmail: session.user.email,
295+
action: AuditAction.PROJECT_ARCHIVED,
296+
resourceType: AuditResourceType.PROJECT,
297+
resourceId: archiveResult.archivedProject.id,
298+
resourceName: archiveResult.archivedProject.name,
299+
description: `Archived Project "${archiveResult.archivedProject.name}" with its last active environment`,
300+
request,
301+
})
302+
}
303+
288304
if (!archiveResult.archived && !workspaceRecord) {
289305
return NextResponse.json({ error: 'Workspace not found' }, { status: 404 })
290306
}
@@ -307,20 +323,6 @@ export const DELETE = withRouteHandler(
307323
},
308324
request,
309325
})
310-
if (archiveResult.archivedProject) {
311-
recordAudit({
312-
workspaceId,
313-
actorId: session.user.id,
314-
actorName: session.user.name,
315-
actorEmail: session.user.email,
316-
action: AuditAction.PROJECT_ARCHIVED,
317-
resourceType: AuditResourceType.PROJECT,
318-
resourceId: archiveResult.archivedProject.id,
319-
resourceName: archiveResult.archivedProject.name,
320-
description: `Archived Project "${archiveResult.archivedProject.name}" with its last active environment`,
321-
request,
322-
})
323-
}
324326

325327
captureServerEvent(
326328
session.user.id,

‎apps/sim/lib/workflows/lifecycle.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,9 @@ export async function disableUserResources(userId: string): Promise<void> {
294294

295295
const { archiveWorkspace } = await import('@/lib/workspaces/lifecycle')
296296

297+
/** Revoked first: the keys are the ban's access boundary, and an archive may fail. */
298+
await db.delete(apiKey).where(eq(apiKey.userId, userId))
299+
297300
const ownedWorkspaces = await db
298301
.select({ id: workspace.id })
299302
.from(workspace)
@@ -302,7 +305,6 @@ export async function disableUserResources(userId: string): Promise<void> {
302305
for (const row of ownedWorkspaces) {
303306
await archiveWorkspace(row.id, { requestId, expectedOwnerId: userId })
304307
}
305-
await db.delete(apiKey).where(eq(apiKey.userId, userId))
306308

307309
logger.info(
308310
`[${requestId}] Disabled resources for user ${userId}: archived ${ownedWorkspaces.length} workspaces, deleted API keys`

0 commit comments

Comments
 (0)