From b5a099c59c3fab278b9dbfc52550edb74911ae8d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 12 Aug 2026 01:59:50 -0700 Subject: [PATCH] fix(executor): carry child provenance across the workflow agent tool result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workflow invoked as an agent tool resolves `{{VAR}}` to the real decrypted value in the child run, but the tool result handed back to the model vendor was projected through a registry that had dropped those entries — so the plaintext crossed to the vendor verbatim. Mechanism. Model-facing projection runs `registry.forkForPropagatedEntries()`, which keeps only entries a result explicitly carried. `EnvResolver` records a resolution without `propagated`, unlike every other boundary that hands a value onward. The child shares the caller's registry object, so `blockLogs`, trace spans and error diagnostics still redact (they read the unforked registry) — only the model fork loses them. The exposure is wider than the child's final output: `mapChildOutputToParent` puts the full `childTraceSpans` — every child block's inputs and outputs — into the returned object, and `postProcessToolOutput` strips only `__`-prefixed keys. The previous implementation ran the child over HTTP: the execute route emitted `__resolvedSecretTraceProvenance`, the tool imported it as `propagated: true`, and `transformResponse` curated the body so `childTraceSpans` never crossed. The in-process branch returns before any of that. Fix. `runWorkflowTool` exports committed provenance for the value it returns and imports it back with `{ trusted: true }`, which marks those entries propagated — the same crossing the custom-block branch already performs in `workflow-handler`. Output-projection only: the returned result is unchanged, and the child executes exactly as before. Redacted values render as `{{NAME}}`, matching the literal the model saw before this regression. Values shorter than `MIN_SUBSTITUTABLE_LITERAL_LENGTH` are still not redacted anywhere — that floor governs detection as well as substitution, and is a documented accepted cost. A test pins the behavior rather than leaving it silent. --- .../workflow/workflow-tool-runner.test.ts | 179 +++++++++++++++++- .../handlers/workflow/workflow-tool-runner.ts | 42 +++- 2 files changed, 218 insertions(+), 3 deletions(-) diff --git a/apps/sim/executor/handlers/workflow/workflow-tool-runner.test.ts b/apps/sim/executor/handlers/workflow/workflow-tool-runner.test.ts index 1a3bd3f54d8..eea9caed21a 100644 --- a/apps/sim/executor/handlers/workflow/workflow-tool-runner.test.ts +++ b/apps/sim/executor/handlers/workflow/workflow-tool-runner.test.ts @@ -3,7 +3,11 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest' -const { mockExecute } = vi.hoisted(() => ({ mockExecute: vi.fn() })) +const { mockExecute, mockDecryptSecret, mockEncryptSecret } = vi.hoisted(() => ({ + mockExecute: vi.fn(), + mockDecryptSecret: vi.fn(), + mockEncryptSecret: vi.fn(), +})) vi.mock('@/executor/handlers/workflow/workflow-handler', () => ({ WorkflowBlockHandler: class { @@ -11,8 +15,19 @@ vi.mock('@/executor/handlers/workflow/workflow-handler', () => ({ }, })) +vi.mock('@/lib/core/security/encryption', () => ({ + decryptSecret: mockDecryptSecret, + encryptSecret: mockEncryptSecret, +})) + +import { projectToolResultForCopilot } from '@/lib/copilot/request/tools/resolved-secret-result' +import type { ToolExecutionResult } from '@/lib/copilot/tool-executor/types' +import { ExecutionState } from '@/executor/execution/state' import type { PiiBlockOutputRedaction } from '@/executor/execution/types' import { runWorkflowTool } from '@/executor/handlers/workflow/workflow-tool-runner' +import type { ExecutionContext } from '@/executor/types' +import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' +import { EnvResolver } from '@/executor/variables/resolvers/env' const PII_POLICY: PiiBlockOutputRedaction = { enabled: true, @@ -61,3 +76,165 @@ describe('runWorkflowTool execution context', () => { expect(ctxArg.environmentVariables).toEqual({ MY_API_KEY: 'trusted' }) }) }) + +const CHILD_SECRET = 'sk-live-7f2c9a41d8be4055b6c3' +const CHILD_SECRET_ENCRYPTED = 'encrypted:CHILD_API_KEY' +const SHORT_SECRET = 'hunter7' +const SHORT_SECRET_ENCRYPTED = 'encrypted:SHORT_KEY' +const SCOPE = { userId: 'user-1', workspaceId: 'ws-1' } + +/** + * Resolves `{{NAME}}` through the real {@link EnvResolver} against the synthetic child context, + * reproducing how a child block records provenance for an environment variable it consumed. + */ +function resolveChildEnvReference( + ctx: ExecutionContext, + reference: string, + inputPath: readonly string[] +): unknown { + return new EnvResolver().resolve(reference, { + executionContext: ctx, + executionState: new ExecutionState(), + currentNodeId: 'child-block', + inputPath, + }) +} + +describe('runWorkflowTool model-facing result provenance', () => { + beforeEach(() => { + vi.clearAllMocks() + mockDecryptSecret.mockImplementation(async (encryptedValue: string) => { + if (encryptedValue === CHILD_SECRET_ENCRYPTED) return { decrypted: CHILD_SECRET } + if (encryptedValue === SHORT_SECRET_ENCRYPTED) return { decrypted: SHORT_SECRET } + throw new Error(`Unexpected encrypted value: ${encryptedValue}`) + }) + }) + + it('keeps a child-resolved environment secret out of the agent-visible tool result', async () => { + const registry = new ResolvedSecretTraceRegistry( + [ + { + name: 'CHILD_API_KEY', + plaintext: CHILD_SECRET, + encryptedValue: CHILD_SECRET_ENCRYPTED, + }, + ], + SCOPE + ) + + mockExecute.mockImplementation(async (ctx: ExecutionContext) => { + const resolved = resolveChildEnvReference(ctx, '{{CHILD_API_KEY}}', ['child-block', 'apiKey']) + return { + success: true, + childWorkflowName: 'Child', + childWorkflowId: 'wf-child', + result: { token: resolved }, + childTraceSpans: [ + { + id: 'span-1', + name: 'API', + type: 'api', + input: { authorization: `Bearer ${resolved}` }, + }, + ], + } + }) + + const result = await runWorkflowTool( + { workflowId: 'wf-child', _context: { workspaceId: 'ws-1', userId: 'user-1' } }, + { + environmentVariables: { CHILD_API_KEY: CHILD_SECRET }, + resolvedSecretTraceRegistry: registry, + } + ) + + expect(JSON.stringify(result.output)).toContain(CHILD_SECRET) + + const projected = projectToolResultForCopilot(result as ToolExecutionResult, registry) + expect(JSON.stringify(projected)).not.toContain(CHILD_SECRET) + expect(projected.output).toEqual({ + success: true, + childWorkflowName: 'Child', + childWorkflowId: 'wf-child', + result: { token: '{{CHILD_API_KEY}}' }, + childTraceSpans: [ + { + id: 'span-1', + name: 'API', + type: 'api', + input: { authorization: 'Bearer {{CHILD_API_KEY}}' }, + }, + ], + }) + }) + + it('keeps a child-resolved environment secret out of a failed tool result', async () => { + const registry = new ResolvedSecretTraceRegistry( + [{ name: 'CHILD_API_KEY', plaintext: CHILD_SECRET, encryptedValue: CHILD_SECRET_ENCRYPTED }], + SCOPE + ) + + mockExecute.mockImplementation(async (ctx: ExecutionContext) => { + const resolved = resolveChildEnvReference(ctx, '{{CHILD_API_KEY}}', ['child-block', 'apiKey']) + throw new Error(`Upstream rejected credential ${String(resolved)}`) + }) + + const result = await runWorkflowTool( + { workflowId: 'wf-child', _context: { workspaceId: 'ws-1', userId: 'user-1' } }, + { + environmentVariables: { CHILD_API_KEY: CHILD_SECRET }, + resolvedSecretTraceRegistry: registry, + } + ) + + const projected = projectToolResultForCopilot(result as ToolExecutionResult, registry) + expect(JSON.stringify(projected)).not.toContain(CHILD_SECRET) + expect(projected.error).toContain('{{CHILD_API_KEY}}') + }) + + it('leaves an unrelated configured secret inert in the agent-visible tool result', async () => { + const registry = new ResolvedSecretTraceRegistry( + [ + { + name: 'CHILD_API_KEY', + plaintext: CHILD_SECRET, + encryptedValue: CHILD_SECRET_ENCRYPTED, + }, + ], + SCOPE + ) + + mockExecute.mockResolvedValue({ success: true, result: { note: 'no secret here' } }) + + const result = await runWorkflowTool( + { workflowId: 'wf-child', _context: { workspaceId: 'ws-1', userId: 'user-1' } }, + { + environmentVariables: { CHILD_API_KEY: CHILD_SECRET }, + resolvedSecretTraceRegistry: registry, + } + ) + + const projected = projectToolResultForCopilot(result as ToolExecutionResult, registry) + expect(projected.output).toEqual({ success: true, result: { note: 'no secret here' } }) + }) + + it('cannot redact a child-resolved value below the substitutable literal length', async () => { + const registry = new ResolvedSecretTraceRegistry( + [{ name: 'SHORT_KEY', plaintext: SHORT_SECRET, encryptedValue: SHORT_SECRET_ENCRYPTED }], + SCOPE + ) + + mockExecute.mockImplementation(async (ctx: ExecutionContext) => { + const resolved = resolveChildEnvReference(ctx, '{{SHORT_KEY}}', ['child-block', 'apiKey']) + return { success: true, result: { token: resolved } } + }) + + const result = await runWorkflowTool( + { workflowId: 'wf-child', _context: { workspaceId: 'ws-1', userId: 'user-1' } }, + { environmentVariables: { SHORT_KEY: SHORT_SECRET }, resolvedSecretTraceRegistry: registry } + ) + + const projected = projectToolResultForCopilot(result as ToolExecutionResult, registry) + expect(JSON.stringify(projected)).toContain(SHORT_SECRET) + }) +}) diff --git a/apps/sim/executor/handlers/workflow/workflow-tool-runner.ts b/apps/sim/executor/handlers/workflow/workflow-tool-runner.ts index dee4a388891..b37675bc849 100644 --- a/apps/sim/executor/handlers/workflow/workflow-tool-runner.ts +++ b/apps/sim/executor/handlers/workflow/workflow-tool-runner.ts @@ -31,6 +31,40 @@ function aggregateChildCost(childTraceSpans: TraceSpan[]): number { return Math.max(0, summary.totalCost - summary.baseExecutionCharge) } +/** + * Records that the child's result carries provenance for the secrets it resolved, so the + * model-facing projection can still find them. + * + * The child executes against the caller's own tool-call registry object, so nothing has to be + * moved between registries — but `EnvResolver` records a resolution without marking it + * propagated, and `forkForPropagatedEntries` (the fork every model boundary projects through) + * keeps only propagated entries. Without this crossing a value the child resolved from an + * environment variable is dropped from the projection registry and reaches the model vendor in + * plaintext. Mirrors the custom-block crossing in `workflow-handler`, and fails closed: an + * unusable envelope marks the registry incomplete, which reduces the result the model sees. + */ +async function markResultProvenanceCrossing( + registry: ResolvedSecretTraceRegistry | undefined, + result: ToolResponse +): Promise { + if (!registry) return + try { + const crossingProvenance = registry.exportCommittedProvenanceForValue({ + output: result.output, + error: result.error, + }) + await registry.importProvenance(crossingProvenance, { + trusted: true, + origin: 'workflowToolRunner.agentResultCrossing', + }) + } catch (error) { + logger.error('Workflow tool result provenance could not be carried across', { + error: getErrorMessage(error, 'Unknown error'), + }) + registry.markIncomplete('value-provenance-import-failed') + } +} + interface WorkflowToolParams { workflowId?: string inputMapping?: Record | string @@ -93,7 +127,9 @@ export async function runWorkflowTool( output && typeof output === 'object' && !Array.isArray(output) ? (output as Record) : { result: output } - return { success: true, output: normalized } + const result: ToolResponse = { success: true, output: normalized } + await markResultProvenanceCrossing(options.resolvedSecretTraceRegistry, result) + return result } catch (error) { const message = getErrorMessage(error, 'Workflow execution failed') const isChildError = ChildWorkflowError.isChildWorkflowError(error) @@ -108,7 +144,7 @@ export async function runWorkflowTool( message, code: structured.code, }) - return { + const result: ToolResponse = { success: false, output: { ...(childCost > 0 ? { cost: { total: childCost } } : {}), @@ -117,5 +153,7 @@ export async function runWorkflowTool( }, error: message, } + await markResultProvenanceCrossing(options.resolvedSecretTraceRegistry, result) + return result } }