From cf84d38c621747d4675589b20295a4afe0907d36 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 12 Aug 2026 00:58:31 -0700 Subject: [PATCH 1/2] test(export): pin the table and tool-param loss the sanitized export accepts #6591 enabled `redactOpaqueCredentialInputs` on the workflow export path, closing a real leak. It also made export lossy for tables and unauthoritative tool params, and nothing pinned that trade in either direction. Adds round-trip fixtures (an api block with two table sub-blocks, an agent block with a custom tool) plus assertions for the current loss, records the security/usability trade on the flag that governs it, and deletes a duplicate `sanitizeForExport` in credential-extractor that omitted the redaction flag and had zero production importers. No behavior change. --- .../api/v1/admin/folders/[id]/export/route.ts | 9 +- .../app/api/v1/workflows/[id]/export/route.ts | 6 +- .../credentials/credential-extractor.test.ts | 64 +++++++------ .../credentials/credential-extractor.ts | 42 ++++++--- .../workflows/operations/export-workflow.ts | 52 ++++++----- .../import-export-roundtrip.test.ts | 90 ++++++++++++++++++- 6 files changed, 190 insertions(+), 73 deletions(-) diff --git a/apps/sim/app/api/v1/admin/folders/[id]/export/route.ts b/apps/sim/app/api/v1/admin/folders/[id]/export/route.ts index 498a77d0a38..6c3893f16e8 100644 --- a/apps/sim/app/api/v1/admin/folders/[id]/export/route.ts +++ b/apps/sim/app/api/v1/admin/folders/[id]/export/route.ts @@ -1,10 +1,15 @@ /** * GET /api/v1/admin/folders/[id]/export * - * Export a folder and all its contents (workflows + subfolders) as a ZIP file or JSON (raw, unsanitized for admin backup/restore). + * Export a folder and all its contents (workflows + subfolders) as a ZIP file or JSON. + * + * The two formats are NOT equivalent. `json` emits the raw stored state for admin + * backup/restore. `zip` runs every workflow through `sanitizeForExport`, which withholds + * credentials and several other classes of sub-block value, so a ZIP does not restore + * faithfully — use `json` when the export has to. * * Query Parameters: - * - format: 'zip' (default) or 'json' + * - format: 'zip' (default, sanitized) or 'json' (raw) * * Response: * - ZIP file download (Content-Type: application/zip) diff --git a/apps/sim/app/api/v1/workflows/[id]/export/route.ts b/apps/sim/app/api/v1/workflows/[id]/export/route.ts index f7ee32c26cb..6f2c2310169 100644 --- a/apps/sim/app/api/v1/workflows/[id]/export/route.ts +++ b/apps/sim/app/api/v1/workflows/[id]/export/route.ts @@ -24,8 +24,10 @@ export const revalidate = 0 * GET /api/v1/workflows/[id]/export * * Exports a workflow as a portable JSON envelope that - * `POST /api/v1/workflows/import` accepts verbatim. Payload assembly and the - * sanitization guarantees are documented on the shared + * `POST /api/v1/workflows/import` accepts without further editing. It is not a + * byte-for-byte clone: the envelope is secret-sanitized, so several classes of + * sub-block value import as empty and must be re-entered. Payload assembly and + * the authoritative list of what is withheld are documented on the shared * {@link buildWorkflowExportPayload}; this route authenticates and renders the * v1 envelope. */ diff --git a/apps/sim/lib/workflows/credentials/credential-extractor.test.ts b/apps/sim/lib/workflows/credentials/credential-extractor.test.ts index 977f9d3d20e..ccdd1be166d 100644 --- a/apps/sim/lib/workflows/credentials/credential-extractor.test.ts +++ b/apps/sim/lib/workflows/credentials/credential-extractor.test.ts @@ -4,7 +4,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { EXPORT_PRESERVED_RESOURCE_TYPES, - sanitizeForExport, sanitizeWorkflowForSharing, } from '@/lib/workflows/credentials/credential-extractor' import { WORKFLOW_SEARCH_SUBBLOCK_RESOURCE_TYPES } from '@/lib/workflows/search-replace/resources/registry' @@ -46,6 +45,17 @@ function stateWithSubBlock(type: string, value: unknown): Partial } as unknown as Partial } +/** + * The exact option set `json-sanitizer`'s `sanitizeForExport` passes, copied rather than imported + * so this suite keeps testing `credential-extractor` without pulling in its own consumer. + * + * The copy cannot drift unnoticed: `import-export-roundtrip` calls the real `sanitizeForExport` + * and asserts the same withholding, so dropping an option there turns that suite red. Every + * export-shaped assertion below must use this constant — one that quietly omitted + * `redactOpaqueCredentialInputs` would describe a configuration no export surface runs. + */ +const EXPORT_OPTIONS = { preserveEnvVars: true, redactOpaqueCredentialInputs: true } as const + function sanitizedValue(type: string, value: unknown): unknown { vi.mocked(getBlock).mockReturnValue({ name: 'Test', @@ -53,7 +63,7 @@ function sanitizedValue(type: string, value: unknown): unknown { subBlocks: [{ id: 'field', title: 'Field', type }], outputs: {}, } as never) - const sanitized = sanitizeForExport(stateWithSubBlock(type, value)) + const sanitized = sanitizeWorkflowForSharing(stateWithSubBlock(type, value), EXPORT_OPTIONS) return sanitized.blocks?.b1?.subBlocks?.field?.value } @@ -98,19 +108,22 @@ describe('export sanitizer resource coverage', () => { it('clears tableId by key on a block with no registry config', () => { vi.mocked(getBlock).mockReturnValue(undefined as never) - const sanitized = sanitizeForExport({ - blocks: { - b1: { - id: 'b1', - type: 'unknown-block', - name: 'Test', - position: { x: 0, y: 0 }, - subBlocks: { tableId: { id: 'tableId', type: 'short-input', value: 'tbl_abc' } }, - outputs: {}, - enabled: true, + const sanitized = sanitizeWorkflowForSharing( + { + blocks: { + b1: { + id: 'b1', + type: 'unknown-block', + name: 'Test', + position: { x: 0, y: 0 }, + subBlocks: { tableId: { id: 'tableId', type: 'short-input', value: 'tbl_abc' } }, + outputs: {}, + enabled: true, + }, }, - }, - } as unknown as Partial) + } as unknown as Partial, + EXPORT_OPTIONS + ) expect(sanitized.blocks?.b1?.subBlocks?.tableId?.value).toBeNull() }) @@ -133,10 +146,10 @@ describe('export sanitizer resource coverage', () => { outputs: {}, } as never) - const sanitized = sanitizeWorkflowForSharing(stateWithSubBlock('tool-input', value), { - preserveEnvVars: true, - redactOpaqueCredentialInputs: true, - }) + const sanitized = sanitizeWorkflowForSharing( + stateWithSubBlock('tool-input', value), + EXPORT_OPTIONS + ) expect(sanitized.blocks?.b1?.subBlocks?.field?.value).toEqual([ { @@ -167,10 +180,10 @@ describe('export sanitizer resource coverage', () => { outputs: {}, } as never) - const sanitized = sanitizeWorkflowForSharing(stateWithSubBlock('tool-input', value), { - preserveEnvVars: true, - redactOpaqueCredentialInputs: true, - }) + const sanitized = sanitizeWorkflowForSharing( + stateWithSubBlock('tool-input', value), + EXPORT_OPTIONS + ) expect(sanitized.blocks?.b1?.subBlocks?.field?.value).toEqual([ { @@ -182,7 +195,7 @@ describe('export sanitizer resource coverage', () => { ]) }) - it('withholds opaque table values from public snapshots', () => { + it('withholds opaque table values from public snapshots and exports', () => { const value = [ { Key: 'Authorization', Value: 'Bearer plaintext-secret' }, { Key: 'API_TOKEN', Value: '{{API_TOKEN}}' }, @@ -194,10 +207,7 @@ describe('export sanitizer resource coverage', () => { outputs: {}, } as never) - const sanitized = sanitizeWorkflowForSharing(stateWithSubBlock('table', value), { - preserveEnvVars: true, - redactOpaqueCredentialInputs: true, - }) + const sanitized = sanitizeWorkflowForSharing(stateWithSubBlock('table', value), EXPORT_OPTIONS) expect(sanitized.blocks?.b1?.subBlocks?.field?.value).toBeNull() }) diff --git a/apps/sim/lib/workflows/credentials/credential-extractor.ts b/apps/sim/lib/workflows/credentials/credential-extractor.ts index c4afe70251d..f1d5d661687 100644 --- a/apps/sim/lib/workflows/credentials/credential-extractor.ts +++ b/apps/sim/lib/workflows/credentials/credential-extractor.ts @@ -84,11 +84,17 @@ const WORKSPACE_SPECIFIC_FIELDS = new Set([ ]) /** - * Sub-block values whose interior cannot be projected safely for a read-only snapshot API. + * Sub-block values whose interior cannot be projected safely once the payload leaves the + * workspace. * * Tables are arbitrary key/value rows used for authorization headers and sandbox environment - * variables. Their cells carry no password metadata, so public snapshots must withhold the whole - * value. Tool inputs are handled separately through the search-replace parameter codecs. + * variables. Their cells carry no password metadata — nothing distinguishes + * `Authorization: Bearer sk-…` from `Content-Type: application/json` — so the whole value is + * withheld. Tool inputs are handled separately through the search-replace parameter codecs. + * + * Which surfaces withhold these values, what that costs, and the shape a future relaxation must + * take are recorded on {@link WorkflowSanitizationOptions.redactOpaqueCredentialInputs}, the flag + * that governs both this set and the tool-input branch. */ const OPAQUE_CREDENTIAL_BEARING_TYPES: ReadonlySet = new Set(['table']) @@ -269,6 +275,26 @@ interface SanitizedWorkflowState { interface WorkflowSanitizationOptions { preserveEnvVars?: boolean + /** + * Withhold values whose interior cannot be projected safely once the payload leaves the + * workspace — whole `table` values (see {@link OPAQUE_CREDENTIAL_BEARING_TYPES}) and every + * `tool-input` parameter with no authoritative codec metadata. + * + * Governed surfaces are every caller that passes this flag: the public execution-snapshot + * projection, the pinned deployment-version read, and — since #6591 — workflow export, which + * reaches the in-app Export as JSON button, the folder and multi-select ZIPs, and the v1/v2 + * export APIs. + * + * The accepted cost on the export surface is that an export is lossy for tables and does not + * round-trip: non-secret configuration (api `params`, cloudwatch dimensions, response `headers`, + * sts `tags`) is withheld alongside the secrets, and a whole-`{{ENV_VAR}}` reference inside a + * cell is withheld too, unlike the same reference in a `password: true` field. Withholding was + * chosen over per-cell heuristics because the sub-blocks that motivate the loss — every header + * table and the `browser_use`/`stagehand`/`daytona` variable tables — are exactly the ones a + * pasted bearer token lands in, and an export file leaves the trust boundary. Relaxing this + * needs a per-sub-block opt-in that fails closed for tables added later, not a wider default; + * `import-export-roundtrip` pins the current loss so the trade cannot be reversed silently. + */ redactOpaqueCredentialInputs?: boolean } @@ -436,13 +462,3 @@ export function sanitizeCredentials( ): SanitizedWorkflowState { return sanitizeWorkflowForSharing(state, { preserveEnvVars: false }) } - -/** - * Sanitize workflow state for export (preserves env vars) - * Convenience wrapper for workflow export - */ -export function sanitizeForExport( - state: Partial | null | undefined -): SanitizedWorkflowState { - return sanitizeWorkflowForSharing(state, { preserveEnvVars: true }) -} diff --git a/apps/sim/lib/workflows/operations/export-workflow.ts b/apps/sim/lib/workflows/operations/export-workflow.ts index e2bfdd159a4..74bfec7cc06 100644 --- a/apps/sim/lib/workflows/operations/export-workflow.ts +++ b/apps/sim/lib/workflows/operations/export-workflow.ts @@ -6,33 +6,6 @@ import { } from '@/lib/workflows/sanitization/json-sanitizer' import { parseWorkflowVariables } from '@/lib/workflows/variables/parse' -/** - * Server-only assembly of the public workflow-export payload, shared by the v1 - * and v2 export routes so both surfaces emit byte-identical envelopes. - * - * Unlike the admin export (`/api/v1/admin/workflows/[id]/export`), which emits - * the raw state for backup/restore, this runs the payload through - * `sanitizeForExport`, which nulls five classes of sub-block value: - * - `password: true` fields, unless the value is a whole `{{ENV_VAR}}` - * reference, which is preserved so the import resolves it in the target - * workspace; - * - `oauth-input` credentials; - * - sensitive nested `tool-input` params and params without authoritative metadata; - * - opaque credential-bearing values such as arbitrary table cells; - * - **workspace-scoped bindings** — selector fields and id-keyed fields that - * point at rows that do not exist in another workspace, cleared rather than - * carried across as dangling ids. - * - * The last class means an export is **not** a byte-for-byte clone even when - * re-imported into the same workspace: those bindings come back empty and must - * be re-selected. This matches the in-app export. - * - * Workflow **variables** are emitted as stored: they are plaintext workflow - * configuration readable by anyone with workspace read (the same permission the - * export routes require); secrets belong in environment variables, which travel - * as unresolved `{{ENV_VAR}}` references. - */ - /** The subset of the workflow record the export payload reads. */ export interface ExportableWorkflowRecord { id: string @@ -111,6 +84,31 @@ function toExportedEdge(edge: Edge): WorkflowExportEdge { * Loads the workflow's normalized state, sanitizes it, and assembles the * portable export envelope. Returns `null` when the workflow has no persisted * normalized state (the caller renders its own 404). + * + * Server-only assembly of the public workflow-export payload, shared by the v1 + * and v2 export routes so both surfaces emit byte-identical envelopes. + * + * Unlike the admin export (`/api/v1/admin/workflows/[id]/export`), which emits + * the raw state for backup/restore, this runs the payload through + * `sanitizeForExport`, which nulls five classes of sub-block value: + * - `password: true` fields, unless the value is a whole `{{ENV_VAR}}` + * reference, which is preserved so the import resolves it in the target + * workspace; + * - `oauth-input` credentials; + * - sensitive nested `tool-input` params and params without authoritative metadata; + * - opaque credential-bearing values such as arbitrary table cells; + * - **workspace-scoped bindings** — selector fields and id-keyed fields that + * point at rows that do not exist in another workspace, cleared rather than + * carried across as dangling ids. + * + * The last two classes mean an export is **not** a byte-for-byte clone even when + * re-imported into the same workspace: those bindings and every table come back + * empty and must be re-entered. This matches the in-app export. + * + * Workflow **variables** are emitted as stored: they are plaintext workflow + * configuration readable by anyone with workspace read (the same permission the + * export routes require); secrets belong in environment variables, which travel + * as unresolved `{{ENV_VAR}}` references. */ export async function buildWorkflowExportPayload( workflowData: ExportableWorkflowRecord diff --git a/apps/sim/lib/workflows/operations/import-export-roundtrip.test.ts b/apps/sim/lib/workflows/operations/import-export-roundtrip.test.ts index 3735775f21c..90b5a552384 100644 --- a/apps/sim/lib/workflows/operations/import-export-roundtrip.test.ts +++ b/apps/sim/lib/workflows/operations/import-export-roundtrip.test.ts @@ -67,6 +67,51 @@ function makeSourceState() { enabled: true, data: { parentId: 'par1', extent: 'parent' }, }, + apiCall: { + id: 'apiCall', + type: 'api', + name: 'Call', + position: { x: 400, y: 200 }, + subBlocks: { + url: { id: 'url', type: 'short-input', value: 'https://example.com/v1/items' }, + headers: { + id: 'headers', + type: 'table', + value: [ + { id: 'r1', cells: { Key: 'Content-Type', Value: 'application/json' } }, + { id: 'r2', cells: { Key: 'Authorization', Value: '{{TABLE_ONLY_TOKEN}}' } }, + ], + }, + params: { + id: 'params', + type: 'table', + value: [{ id: 'r3', cells: { Key: 'limit', Value: '10' } }], + }, + }, + outputs: {}, + enabled: true, + }, + agent: { + id: 'agent', + type: 'agent', + name: 'Agent', + position: { x: 600, y: 200 }, + subBlocks: { + tools: { + id: 'tools', + type: 'tool-input', + value: [ + { + type: 'custom-tool', + title: 'My Tool', + params: { endpoint: 'https://example.com/hook', greeting: 'hi' }, + }, + ], + }, + }, + outputs: {}, + enabled: true, + }, } as Record, edges: [ { id: 'e1', source: 'starter', target: 'loop1', sourceHandle: 'source', targetHandle: null }, @@ -87,7 +132,8 @@ function makeSourceState() { describe('workflow export -> import round trip', () => { const exported = sanitizeForExport(makeSourceState() as any) - const { data: reimported, errors } = parseWorkflowJson(JSON.stringify(exported)) + const exportedJson = JSON.stringify(exported) + const { data: reimported, errors } = parseWorkflowJson(exportedJson) it('re-imports without validation errors', () => { expect(errors).toEqual([]) @@ -96,13 +142,15 @@ describe('workflow export -> import round trip', () => { it('preserves every block, including children inside loop and parallel containers', () => { expect(Object.keys(exported.state.blocks).sort()).toEqual([ + 'agent', + 'apiCall', 'childInLoop', 'childInPar', 'loop1', 'par1', 'starter', ]) - expect(Object.keys(reimported!.blocks)).toHaveLength(5) + expect(Object.keys(reimported!.blocks)).toHaveLength(7) }) it('preserves every edge', () => { @@ -137,6 +185,44 @@ describe('workflow export -> import round trip', () => { }) }) + /** + * The round trip is deliberately NOT lossless, and these assertions exist so that stops being a + * surprise. `sanitizeForExport` passes `redactOpaqueCredentialInputs`, which withholds whole + * `table` values and every parameter of a tool with no authoritative codec metadata — see the + * trade recorded on `OPAQUE_CREDENTIAL_BEARING_TYPES` in `credential-extractor`. + * + * Both classes carry secrets no per-field rule can find (a bearer token pasted into a header + * cell, a key in a custom tool's params) and both also carry ordinary configuration, which goes + * with them. Whoever revisits that trade has to edit these expectations, which is the point: the + * absence of a `table` fixture here is why the change reached a release unnoticed. + */ + it('withholds table values, including whole {{ENV_VAR}} cells, on the way out', () => { + const subBlocks = exported.state.blocks.apiCall.subBlocks as Record + + expect(subBlocks.headers.value).toBeNull() + expect(subBlocks.params.value).toBeNull() + expect(subBlocks.url.value).toBe('https://example.com/v1/items') + expect(exportedJson).not.toContain('{{API_TOKEN}}') + }) + + it('withholds every parameter of a tool with no authoritative codec metadata', () => { + const tools = exported.state.blocks.agent.subBlocks.tools.value as { + params: Record + }[] + + expect(tools[0].params).toEqual({ endpoint: null, greeting: null }) + expect(tools[0].title).toBe('My Tool') + }) + + it('re-imports the withheld sub-blocks as empty rather than dropping or corrupting them', () => { + const reimportedApi = Object.values(reimported!.blocks).find( + (b: any) => b.type === 'api' + ) as any + + expect(reimportedApi.subBlocks.headers.value).toBeNull() + expect(reimportedApi.subBlocks.url.value).toBe('https://example.com/v1/items') + }) + it('does not hang on a block name containing regex metacharacters', () => { const hostile = makeSourceState() hostile.blocks.starter.name = 'a*a*a*a*a*a*a*a*b' From b6a3b387768348a5e161d3fcab360cd01193a598 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 12 Aug 2026 01:42:46 -0700 Subject: [PATCH 2/2] test(export): make the env-ref leak sweep load-bearing and drop test any The sweep asserted against a token the fixture no longer contained, so it passed vacuously. Both the fixture and the assertion now read one symbol, which is the only form that cannot drift. Also types the re-imported block lookup instead of casting through any. --- .../import-export-roundtrip.test.ts | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/apps/sim/lib/workflows/operations/import-export-roundtrip.test.ts b/apps/sim/lib/workflows/operations/import-export-roundtrip.test.ts index 90b5a552384..4daf38ff634 100644 --- a/apps/sim/lib/workflows/operations/import-export-roundtrip.test.ts +++ b/apps/sim/lib/workflows/operations/import-export-roundtrip.test.ts @@ -15,6 +15,14 @@ import { describe, expect, it } from 'vitest' import { parseWorkflowJson } from '@/lib/workflows/operations/import-export' import { sanitizeForExport } from '@/lib/workflows/sanitization/json-sanitizer' +/** + * A whole-`{{ENV_VAR}}` reference that appears in exactly one place: a table cell. The leak sweep + * below greps the serialized envelope for it, so it has to be a value no other fixture uses and + * the same symbol has to feed both the fixture and the assertion — spelling the string twice is + * how that sweep silently starts passing vacuously. + */ +const TABLE_ONLY_ENV_REF = '{{TABLE_ONLY_TOKEN}}' + function makeSourceState() { return { blocks: { @@ -79,7 +87,7 @@ function makeSourceState() { type: 'table', value: [ { id: 'r1', cells: { Key: 'Content-Type', Value: 'application/json' } }, - { id: 'r2', cells: { Key: 'Authorization', Value: '{{TABLE_ONLY_TOKEN}}' } }, + { id: 'r2', cells: { Key: 'Authorization', Value: TABLE_ONLY_ENV_REF } }, ], }, params: { @@ -202,7 +210,7 @@ describe('workflow export -> import round trip', () => { expect(subBlocks.headers.value).toBeNull() expect(subBlocks.params.value).toBeNull() expect(subBlocks.url.value).toBe('https://example.com/v1/items') - expect(exportedJson).not.toContain('{{API_TOKEN}}') + expect(exportedJson).not.toContain(TABLE_ONLY_ENV_REF) }) it('withholds every parameter of a tool with no authoritative codec metadata', () => { @@ -215,12 +223,11 @@ describe('workflow export -> import round trip', () => { }) it('re-imports the withheld sub-blocks as empty rather than dropping or corrupting them', () => { - const reimportedApi = Object.values(reimported!.blocks).find( - (b: any) => b.type === 'api' - ) as any + const reimportedApi = Object.values(reimported!.blocks).find((b) => b.type === 'api') - expect(reimportedApi.subBlocks.headers.value).toBeNull() - expect(reimportedApi.subBlocks.url.value).toBe('https://example.com/v1/items') + expect(reimportedApi).toBeDefined() + expect(reimportedApi?.subBlocks.headers.value).toBeNull() + expect(reimportedApi?.subBlocks.url.value).toBe('https://example.com/v1/items') }) it('does not hang on a block name containing regex metacharacters', () => {