Skip to content

Commit 5decc01

Browse files
committed
fix(api): make workflow import atomic and clarify what export redacts
Writes the imported graph and its variables in a single transaction and deletes the shell workflow row on any failure, so a caller that receives an error is never left with a partially imported workflow. Previously a throw from the variables update returned 500 while leaving the workflow behind with an empty variables map. Also narrows the export route's sanitization claim: workflow variables are emitted as stored, matching GET /api/v1/workflows/[id] and the in-app export. They are plaintext configuration readable at the same permission level this route requires; secrets belong in environment variables, which travel as unresolved references.
1 parent 6266b52 commit 5decc01

4 files changed

Lines changed: 82 additions & 20 deletions

File tree

apps/docs/openapi.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,7 +1245,7 @@
12451245
"get": {
12461246
"operationId": "exportWorkflow",
12471247
"summary": "Export Workflow",
1248-
"description": "Export a workflow as a portable JSON envelope that POST /api/v1/workflows/import accepts verbatim. Stored credentials and secret field values are redacted from the payload, while `{{ENV_VAR}}` references and block positions are preserved. Requires read permission on the workflow's workspace. Returns 404 when the workflow does not exist or you do not have access to it.",
1248+
"description": "Export a workflow as a portable JSON envelope that POST /api/v1/workflows/import accepts verbatim. Credential and password field values stored on blocks are redacted from the payload, while `{{ENV_VAR}}` references and block positions are preserved. Workflow variables are returned as stored \u2014 they are plaintext configuration readable by anyone with workspace read, and secrets belong in environment variables, which travel as unresolved references. Requires read permission on the workflow's workspace. Returns 404 when the workflow does not exist or you do not have access to it.",
12491249
"tags": ["Workflows"],
12501250
"x-codeSamples": [
12511251
{
@@ -7575,15 +7575,15 @@
75757575
},
75767576
"variables": {
75777577
"type": "object",
7578-
"description": "Workflow-level variables, keyed by variable id.",
7578+
"description": "Workflow-level variables, keyed by variable id. Emitted as stored \u2014 treat as plaintext configuration, not a secret store.",
75797579
"additionalProperties": true,
75807580
"example": {}
75817581
}
75827582
}
75837583
},
75847584
"WorkflowExport": {
75857585
"type": "object",
7586-
"description": "A portable workflow export envelope. Pass this object straight back to POST /api/v1/workflows/import to recreate the workflow. Stored credentials and secret field values are redacted; `{{ENV_VAR}}` references are preserved.",
7586+
"description": "A portable workflow export envelope. Pass this object straight back to POST /api/v1/workflows/import to recreate the workflow. Credential and password field values stored on blocks are redacted; `{{ENV_VAR}}` references and workflow variables are preserved as stored.",
75877587
"properties": {
75887588
"version": {
75897589
"type": "string",

apps/sim/app/api/v1/workflows/[id]/export/route.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,16 @@ function toExportedEdge(edge: Edge): ExportedEdge {
6767
*
6868
* Unlike the admin export (`/api/v1/admin/workflows/[id]/export`), which emits
6969
* the raw state for backup/restore, this surface runs the payload through
70-
* `sanitizeForExport`: stored credentials and secret sub-block values are
71-
* stripped while `{{ENV_VAR}}` references and block positions are preserved.
70+
* `sanitizeForExport`: block sub-block values declared `password: true` or
71+
* `oauth-input` are nulled, while `{{ENV_VAR}}` references and block positions
72+
* are preserved.
73+
*
74+
* Workflow **variables** are emitted as stored, matching `GET
75+
* /api/v1/workflows/[id]` and the in-app export. Variables are plaintext
76+
* workflow configuration readable by anyone with workspace read (the same
77+
* permission this route requires); secrets belong in environment variables,
78+
* which travel as unresolved `{{ENV_VAR}}` references. Redacting them here
79+
* would break import round-trips without narrowing access.
7280
*/
7381
export const GET = withRouteHandler(
7482
async (request: NextRequest, context: { params: Promise<{ id: string }> }) => {

apps/sim/app/api/v1/workflows/import/route.test.ts

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ vi.mock('@sim/db', () => ({
7575
})),
7676
})),
7777
delete: mockDbDelete,
78-
update: mockDbUpdate,
78+
transaction: vi.fn(async (fn: (tx: unknown) => Promise<void>) => fn({ update: mockDbUpdate })),
7979
},
8080
}))
8181

@@ -223,7 +223,11 @@ describe('POST /api/v1/workflows/import', () => {
223223
createdAt: CREATED_AT.toISOString(),
224224
updatedAt: CREATED_AT.toISOString(),
225225
})
226-
expect(mockSaveWorkflowToNormalizedTables).toHaveBeenCalledWith('wf-new', expect.anything())
226+
expect(mockSaveWorkflowToNormalizedTables).toHaveBeenCalledWith(
227+
'wf-new',
228+
expect.anything(),
229+
expect.anything()
230+
)
227231
})
228232

229233
it('derives the name from the export envelope and deduplicates it', async () => {
@@ -313,6 +317,22 @@ describe('POST /api/v1/workflows/import', () => {
313317
expect(mockDbUpdate).not.toHaveBeenCalled()
314318
})
315319

320+
it('writes the graph and variables inside a single transaction', async () => {
321+
mockParseWorkflowJson.mockReturnValue({
322+
data: {
323+
...PARSED_STATE,
324+
variables: { 'var-1': { id: 'var-1', name: 'host', type: 'string', value: 'x' } },
325+
},
326+
errors: [],
327+
})
328+
329+
await POST(makeRequest(validBody()))
330+
331+
const tx = { update: mockDbUpdate }
332+
expect(mockSaveWorkflowToNormalizedTables).toHaveBeenCalledWith('wf-new', expect.anything(), tx)
333+
expect(mockDbUpdate).toHaveBeenCalled()
334+
})
335+
316336
it('deletes the created row and returns 500 when persisting state fails', async () => {
317337
const whereSpy = vi.fn().mockResolvedValue(undefined)
318338
mockDbDelete.mockReturnValue({ where: whereSpy })
@@ -325,6 +345,27 @@ describe('POST /api/v1/workflows/import', () => {
325345
expect(whereSpy).toHaveBeenCalled()
326346
})
327347

348+
it('rolls back the created workflow when the variables write throws', async () => {
349+
const whereSpy = vi.fn().mockResolvedValue(undefined)
350+
mockDbDelete.mockReturnValue({ where: whereSpy })
351+
mockDbUpdate.mockImplementation(() => {
352+
throw new Error('variables write failed')
353+
})
354+
mockParseWorkflowJson.mockReturnValue({
355+
data: {
356+
...PARSED_STATE,
357+
variables: { 'var-1': { id: 'var-1', name: 'host', type: 'string', value: 'x' } },
358+
},
359+
errors: [],
360+
})
361+
362+
const response = await POST(makeRequest(validBody()))
363+
364+
expect(response.status).toBe(500)
365+
expect(mockDbDelete).toHaveBeenCalled()
366+
expect(whereSpy).toHaveBeenCalled()
367+
})
368+
328369
it('unwraps an export response envelope so its metadata still resolves', async () => {
329370
await POST(makeRequest(validBody({ workflow: { data: EXPORT_ENVELOPE, limits: {} } })))
330371

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

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -229,24 +229,37 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
229229

230230
const workflowId = created.workflow.id
231231

232-
const saveResult = await saveWorkflowToNormalizedTables(workflowId, workflowState)
233-
if (!saveResult.success) {
234-
await db.delete(workflow).where(eq(workflow.id, workflowId))
235-
logger.error(`[${requestId}] Failed to persist imported workflow state`, {
232+
const variables = toVariablesRecord(workflowState.variables)
233+
234+
/**
235+
* The graph and the variables are written in one transaction so an import
236+
* can never half-land, and any failure deletes the shell row created above
237+
* — a caller that receives an error must not be left with a partially
238+
* imported workflow in their workspace.
239+
*/
240+
try {
241+
await db.transaction(async (tx) => {
242+
const saveResult = await saveWorkflowToNormalizedTables(workflowId, workflowState, tx)
243+
if (!saveResult.success) {
244+
throw new Error(saveResult.error || 'Failed to save workflow state')
245+
}
246+
247+
if (Object.keys(variables).length > 0) {
248+
await tx
249+
.update(workflow)
250+
.set({ variables, updatedAt: new Date() })
251+
.where(eq(workflow.id, workflowId))
252+
}
253+
})
254+
} catch (error) {
255+
logger.error(`[${requestId}] Failed to persist imported workflow, rolling back`, {
236256
workflowId,
237-
error: saveResult.error,
257+
error: getErrorMessage(error, 'Unknown error'),
238258
})
259+
await db.delete(workflow).where(eq(workflow.id, workflowId))
239260
return NextResponse.json({ error: 'Failed to save workflow state' }, { status: 500 })
240261
}
241262

242-
const variables = toVariablesRecord(workflowState.variables)
243-
if (Object.keys(variables).length > 0) {
244-
await db
245-
.update(workflow)
246-
.set({ variables, updatedAt: new Date() })
247-
.where(eq(workflow.id, workflowId))
248-
}
249-
250263
logger.info(`[${requestId}] Imported workflow ${workflowId} into workspace ${workspaceId}`, {
251264
name: created.workflow.name,
252265
blocksCount: Object.keys(workflowState.blocks).length,

0 commit comments

Comments
 (0)