mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(executor): carry child provenance across the workflow agent tool result (#6619)
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.
This commit is contained in:
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<void> {
|
||||
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, unknown> | string
|
||||
@@ -93,7 +127,9 @@ export async function runWorkflowTool(
|
||||
output && typeof output === 'object' && !Array.isArray(output)
|
||||
? (output as Record<string, unknown>)
|
||||
: { 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
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user