Skip to content

Commit b33f8e8

Browse files
authored
fix(executor): give the workflow agent tool the caller's env and PII policy (#6611)
A workflow attached as an Agent (or Pi) tool ran its entire child execution with an empty environment-variable map and no block-output redaction policy. Mechanism. `tools/index.ts` short-circuits `workflow_executor` into `runWorkflowTool`, which builds its synthetic parent `ExecutionContext` with `buildCustomBlockExecutionContext`. That builder was written for the custom-block (deploy-as-block) path and hardcoded `environmentVariables: {}` — safe there only because `WorkflowBlockHandler.executeCore` re-derives the publisher's env inside `if (isCustomBlock)`. The workflow-tool path's synthetic block carries `metadata.id: 'workflow_input'`, so `isCustomBlock` is false, the re-derivation is skipped, and `{}` flows through `childEnvVarValues` into the sub-Executor. `DAGExecutor` has no fallback and `EnvResolver` returns the raw reference on a miss, so a child block field of `Bearer {{MY_API_KEY}}` was transmitted to the third party verbatim and 401'd — silently, with the variable name disclosed. The same builder never set `piiBlockOutputRedaction`, so `block-executor`'s in-flight masking was disabled for every child block of orgs that had explicitly enabled that stage. Both landed as unnoticed side effects of #5273, whose stated goals were admission slots, log rows, cost roll-up and structured errors; #6539 later patched a third dropped field on the same context without noticing these two. Fix. Thread both values through the runner `options` bag — never `params._context`, which spreads model-reachable `contextParams._context` first and would let a model inject its own env map or disable redaction. `executeTool` reads them off the trusted `executionContext`, which also covers the Pi block, whose tool loop calls `executeTool` with `executionContext: ctx` on the identical path. `environmentVariables` is required rather than optional-with-a-default. Silent omission is precisely the failure mode here and in #6539; making it required turns the next caller's omission into a compile error. `runCustomBlockTool` now passes `{}` explicitly, so that path is unchanged at runtime. `piiBlockOutputRedaction` stays optional deliberately: `undefined` is its correct value for the many tenants with no policy, whereas `{}` for env is a wrong identity rather than a default. The builder's TSDoc states both halves of that asymmetry. Identity semantics — this restores function but does not restore main's identity. On main this tool was an HTTP hop into execution-core, which derived the env from the CHILD workflow's owner, so the child got the child owner's personal env plus the child workspace's env. Forwarding the caller's map gives the child the PARENT CALLER's personal env: a different identity, not a subset. That is the deliberate choice, because it is byte-identical to the long-standing canvas workflow block, it is bounded to one workspace by `assertChildWorkflowInWorkspace` on this branch, and it is the only variant consistent with the parent `resolvedSecretTraceRegistry` this path already forwards. The narrow case that worked on main and still will not: a same-workspace child owned by another member that relied on THAT member's personal environment variable. The `deployed_block_executor` call site deliberately gets neither value: custom blocks skip the same-workspace assert and run cross-workspace under the publisher's identity, so the consumer's env and redaction rules are the wrong tenant's. A test pins that so a later refactor cannot unify the branches silently. Tests. Three suites pin the fix itself (runner, builder, `executeTool` dispatch) and go red without it. A fourth case in `workflow-handler.test.ts` pins the last hop — `ctx.environmentVariables` -> `childEnvVarValues` -> the sub-Executor's `envVarValues`, plus `piiBlockOutputRedaction` — on the NON-custom branch. That hop is untouched staging code, so that case passes either way by construction; it exists so a future change to the branch that distinguishes the two paths cannot silently undo this fix downstream of the builder. Out of scope, deliberately: `enforceCredentialAccess` is dropped by the same synthetic context, but on main this path ran under an internal JWT with `useAuthenticatedUserAsActor === false`, so forwarding the parent's value would TIGHTEN behavior versus main and could break currently-working child runs mid-release. It needs its own deliberate change — and it now compounds with this one, since the child runs with the parent's decrypted env while credential-access enforcement stays off. The `input` redaction stage (masking the LLM-authored inputMapping) is also not restored — `ExecutionContext` has no field for it and the canvas workflow block never had it either. Re-enabling masking inside child runs is a live behavior change for affected tenants: `redactObjectStrings` runs with `onFailure: 'throw'`, so a child agent tool call that currently succeeds unmasked can now fail closed, which is main's semantic restored. This belongs in the release note.
1 parent 8f85ded commit b33f8e8

7 files changed

Lines changed: 254 additions & 29 deletions

File tree

apps/sim/executor/handlers/workflow/custom-block-tool-runner.test.ts

Lines changed: 59 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,26 +15,36 @@ vi.mock('@/executor/handlers/workflow/workflow-handler', () => ({
1515
}))
1616

1717
import { ChildWorkflowError } from '@/executor/errors/child-workflow-error'
18+
import type { PiiBlockOutputRedaction } from '@/executor/execution/types'
1819
import {
1920
buildCustomBlockExecutionContext,
2021
runCustomBlockTool,
2122
} from '@/executor/handlers/workflow/custom-block-tool-runner'
2223
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
2324

25+
const PII_POLICY: PiiBlockOutputRedaction = {
26+
enabled: true,
27+
entityTypes: ['EMAIL_ADDRESS'],
28+
language: 'en',
29+
}
30+
2431
const mockRunnerLogger =
2532
vi.mocked(createLogger).mock.results[
2633
vi.mocked(createLogger).mock.calls.findIndex(([name]) => name === 'CustomBlockToolRunner')
2734
].value
2835

2936
describe('buildCustomBlockExecutionContext', () => {
3037
it('carries consumer identity, inherits the call chain, and is fully scaffolded', () => {
31-
const ctx = buildCustomBlockExecutionContext({
32-
workspaceId: 'ws-consumer',
33-
userId: 'u-consumer',
34-
workflowId: 'wf-parent',
35-
callChain: ['wf-parent'],
36-
billingAttribution: { actorUserId: 'u-consumer', workspaceId: 'ws-consumer' } as any,
37-
})
38+
const ctx = buildCustomBlockExecutionContext(
39+
{
40+
workspaceId: 'ws-consumer',
41+
userId: 'u-consumer',
42+
workflowId: 'wf-parent',
43+
callChain: ['wf-parent'],
44+
billingAttribution: { actorUserId: 'u-consumer', workspaceId: 'ws-consumer' } as any,
45+
},
46+
{ environmentVariables: {} }
47+
)
3848

3949
expect(ctx.workspaceId).toBe('ws-consumer')
4050
expect(ctx.userId).toBe('u-consumer')
@@ -59,7 +69,20 @@ describe('buildCustomBlockExecutionContext', () => {
5969
})
6070

6171
it('defaults the call chain to [] when none is provided', () => {
62-
expect(buildCustomBlockExecutionContext({}).callChain).toEqual([])
72+
expect(buildCustomBlockExecutionContext({}, { environmentVariables: {} }).callChain).toEqual([])
73+
})
74+
75+
it('carries the caller-supplied env map and redaction policy verbatim', () => {
76+
const ctx = buildCustomBlockExecutionContext(
77+
{ workspaceId: 'ws-1' },
78+
{
79+
environmentVariables: { MY_API_KEY: 'secret-value' },
80+
piiBlockOutputRedaction: PII_POLICY,
81+
}
82+
)
83+
84+
expect(ctx.environmentVariables).toEqual({ MY_API_KEY: 'secret-value' })
85+
expect(ctx.piiBlockOutputRedaction).toBe(PII_POLICY)
6386
})
6487
})
6588

@@ -135,6 +158,16 @@ describe('runCustomBlockTool', () => {
135158
expect(res.output).toEqual({})
136159
})
137160

161+
it('runs the child with no env and no redaction policy — the custom branch re-derives both', async () => {
162+
mockExecute.mockResolvedValue({ success: true })
163+
164+
await runCustomBlockTool({ blockType: 'custom_block_abc', _context: {} })
165+
166+
const [ctxArg] = mockExecute.mock.calls[0]
167+
expect(ctxArg.environmentVariables).toEqual({})
168+
expect(ctxArg.piiBlockOutputRedaction).toBeUndefined()
169+
})
170+
138171
it('rejects a missing block type without invoking the handler', async () => {
139172
const res = await runCustomBlockTool({ _context: {} })
140173
expect(res.success).toBe(false)
@@ -144,19 +177,25 @@ describe('runCustomBlockTool', () => {
144177

145178
describe('buildCustomBlockExecutionContext invoker identity', () => {
146179
it("adopts the invoking run's ids so correlation names a real execution", () => {
147-
const ctx = buildCustomBlockExecutionContext({
148-
workspaceId: 'ws-1',
149-
executionId: 'agent-execution-id',
150-
requestId: 'agent-request-id',
151-
})
180+
const ctx = buildCustomBlockExecutionContext(
181+
{
182+
workspaceId: 'ws-1',
183+
executionId: 'agent-execution-id',
184+
requestId: 'agent-request-id',
185+
},
186+
{ environmentVariables: {} }
187+
)
152188

153189
expect(ctx.executionId).toBe('agent-execution-id')
154190
expect(ctx.metadata.executionId).toBe('agent-execution-id')
155191
expect(ctx.metadata.requestId).toBe('agent-request-id')
156192
})
157193

158194
it('falls back to generated ids when the caller supplies none', () => {
159-
const ctx = buildCustomBlockExecutionContext({ workspaceId: 'ws-1' })
195+
const ctx = buildCustomBlockExecutionContext(
196+
{ workspaceId: 'ws-1' },
197+
{ environmentVariables: {} }
198+
)
160199

161200
expect(ctx.executionId).toBeTruthy()
162201
expect(ctx.metadata.requestId).toBeTruthy()
@@ -169,14 +208,17 @@ describe('buildCustomBlockExecutionContext cancellation', () => {
169208
const controller = new AbortController()
170209
const ctx = buildCustomBlockExecutionContext(
171210
{ workspaceId: 'ws-1' },
172-
{ abortSignal: controller.signal }
211+
{ environmentVariables: {}, abortSignal: controller.signal }
173212
)
174213

175214
expect(ctx.abortSignal).toBe(controller.signal)
176215
})
177216

178217
it('leaves the signal undefined when the caller has none', () => {
179-
expect(buildCustomBlockExecutionContext({ workspaceId: 'ws-1' }).abortSignal).toBeUndefined()
218+
expect(
219+
buildCustomBlockExecutionContext({ workspaceId: 'ws-1' }, { environmentVariables: {} })
220+
.abortSignal
221+
).toBeUndefined()
180222
})
181223
})
182224

@@ -186,7 +228,7 @@ describe('buildCustomBlockExecutionContext secret provenance', () => {
186228

187229
const ctx = buildCustomBlockExecutionContext(
188230
{ workspaceId: 'ws-1' },
189-
{ resolvedSecretTraceRegistry: registry }
231+
{ environmentVariables: {}, resolvedSecretTraceRegistry: registry }
190232
)
191233

192234
expect(ctx.resolvedSecretTraceRegistry).toBe(registry)

apps/sim/executor/handlers/workflow/custom-block-tool-runner.ts

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { getErrorMessage } from '@sim/utils/errors'
33
import { generateId } from '@sim/utils/id'
44
import { isPlainRecord } from '@sim/utils/object'
55
import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution'
6+
import type { PiiBlockOutputRedaction } from '@/executor/execution/types'
67
import { WorkflowBlockHandler } from '@/executor/handlers/workflow/workflow-handler'
78
import type { ExecutionContext, ExecutorDelegationOrigin } from '@/executor/types'
89
import { projectResolvedSecretDiagnosticContent } from '@/executor/utils/resolved-secret-content-projection'
@@ -40,23 +41,40 @@ interface CustomBlockToolParams {
4041
}
4142

4243
/**
43-
* Build a minimal top-level `ExecutionContext` for running a custom block as an
44-
* agent tool. Every value comes from the server-set `_context` (LLM-proof),
45-
* including the invoking run's execution and request ids so the child's log
46-
* correlation names a real execution. `WorkflowBlockHandler`'s path re-derives owner
47-
* identity, env, and billing from `getCustomBlockAuthority`, so this only needs the
48-
* fields that path reads — `workspaceId` (org-scopes the authority lookup),
49-
* `metadata` (read unconditionally at `executeCore`), and `callChain` (recursion
50-
* depth guard, inherited so it never resets across hops) — plus the non-optional
51-
* scaffolding. Keep in sync with `WorkflowBlockHandler.executeCore`'s custom branch.
44+
* Build a minimal top-level `ExecutionContext` for running a workflow or a custom
45+
* block as an agent tool. Every value comes from the server-set `_context`
46+
* (LLM-proof) or from `options` (not model-reachable at all), including the
47+
* invoking run's execution and request ids so the child's log correlation names a
48+
* real execution. `WorkflowBlockHandler.executeCore` reads `workspaceId` (org-scopes
49+
* the authority lookup), `metadata` (read unconditionally), and `callChain`
50+
* (recursion depth guard, inherited so it never resets across hops), plus the
51+
* non-optional scaffolding.
52+
*
53+
* `environmentVariables` is required rather than defaulted because only the caller
54+
* knows which identity's env the child must run under: the custom-block branch
55+
* re-derives the publisher's env from `getCustomBlockAuthority` and passes `{}`,
56+
* while every other caller must forward the invoking run's map or the child
57+
* resolves `{{VAR}}` to the literal reference string. Silent omission is exactly
58+
* how the workflow-as-agent-tool path shipped with an empty map.
59+
*
60+
* `piiBlockOutputRedaction` stays optional because `undefined` is its correct
61+
* value rather than a wrong identity: most tenants have no policy at all, and the
62+
* custom-block branch omits it deliberately — that child runs cross-workspace
63+
* under the publisher's identity, so the consumer's redaction rules would be the
64+
* wrong tenant's, exactly as the consumer's env would be.
65+
* Keep in sync with `WorkflowBlockHandler.executeCore`.
5266
*/
5367
export function buildCustomBlockExecutionContext(
5468
context: CustomBlockExecutorContext,
5569
options: {
70+
/** The invoking run's decrypted env, or `{}` when the child re-derives its own. */
71+
environmentVariables: Record<string, string>
5672
abortSignal?: AbortSignal
5773
resolvedSecretTraceRegistry?: ResolvedSecretTraceRegistry
5874
executorDelegationOrigin?: ExecutorDelegationOrigin
59-
} = {}
75+
/** The invoking run's in-flight block-output redaction policy. */
76+
piiBlockOutputRedaction?: PiiBlockOutputRedaction
77+
}
6078
): ExecutionContext {
6179
// Prefer the invoking agent run's ids so correlation and cancellation both
6280
// point at a real execution; fall back only when a caller could not supply them.
@@ -75,7 +93,8 @@ export function buildCustomBlockExecutionContext(
7593
// the agent tool loop owns the only signal reaching this path.
7694
abortSignal: options.abortSignal,
7795
resolvedSecretTraceRegistry: options.resolvedSecretTraceRegistry,
78-
environmentVariables: {},
96+
environmentVariables: options.environmentVariables,
97+
piiBlockOutputRedaction: options.piiBlockOutputRedaction,
7998
blockStates: new Map(),
8099
executedBlocks: new Set(),
81100
blockLogs: [],
@@ -121,6 +140,7 @@ export async function runCustomBlockTool(
121140
}
122141

123142
const ctx = buildCustomBlockExecutionContext(params._context ?? {}, {
143+
environmentVariables: {},
124144
abortSignal: options.abortSignal,
125145
resolvedSecretTraceRegistry: options.resolvedSecretTraceRegistry,
126146
})

apps/sim/executor/handlers/workflow/workflow-handler.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,43 @@ describe('WorkflowBlockHandler', () => {
516516
expect(mockResolveBillingAttribution).not.toHaveBeenCalled()
517517
})
518518

519+
it("runs a non-custom child under the parent's env and redaction policy", async () => {
520+
const piiBlockOutputRedaction = {
521+
enabled: true,
522+
entityTypes: ['EMAIL_ADDRESS'],
523+
language: 'en',
524+
}
525+
const ctx = {
526+
...mockContext,
527+
workspaceId: 'workspace-parent',
528+
environmentVariables: { MY_API_KEY: 'parent-secret' },
529+
piiBlockOutputRedaction,
530+
} as unknown as ExecutionContext
531+
532+
mockFetch.mockResolvedValueOnce({
533+
ok: true,
534+
json: () =>
535+
Promise.resolve({
536+
data: {
537+
name: 'Child Workflow',
538+
workspaceId: 'workspace-parent',
539+
state: { blocks: {}, edges: [], loops: {}, parallels: {} },
540+
},
541+
}),
542+
})
543+
mockCreateSnapshot.mockResolvedValue({ snapshot: { id: 'snapshot-1' } })
544+
mockExecutorExecute.mockResolvedValue({ success: true, output: { data: 'ok' } })
545+
546+
await handler.execute(ctx, mockBlock, inputs)
547+
548+
expect(executorOptions).toHaveLength(1)
549+
expect(executorOptions[0].envVarValues).toEqual({ MY_API_KEY: 'parent-secret' })
550+
expect(executorOptions[0].contextExtensions.piiBlockOutputRedaction).toBe(
551+
piiBlockOutputRedaction
552+
)
553+
expect(mockGetPersonalAndWorkspaceEnv).not.toHaveBeenCalled()
554+
})
555+
519556
it('resolves a source-scoped billing attribution for custom block children', async () => {
520557
const consumerAttribution = { actorUserId: 'consumer-1', workspaceId: 'workspace-consumer' }
521558
const sourceAttribution = { actorUserId: 'owner-9', workspaceId: 'workspace-source' }
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const { mockExecute } = vi.hoisted(() => ({ mockExecute: vi.fn() }))
7+
8+
vi.mock('@/executor/handlers/workflow/workflow-handler', () => ({
9+
WorkflowBlockHandler: class {
10+
execute = mockExecute
11+
},
12+
}))
13+
14+
import type { PiiBlockOutputRedaction } from '@/executor/execution/types'
15+
import { runWorkflowTool } from '@/executor/handlers/workflow/workflow-tool-runner'
16+
17+
const PII_POLICY: PiiBlockOutputRedaction = {
18+
enabled: true,
19+
entityTypes: ['EMAIL_ADDRESS'],
20+
language: 'en',
21+
}
22+
23+
describe('runWorkflowTool execution context', () => {
24+
beforeEach(() => {
25+
vi.clearAllMocks()
26+
mockExecute.mockResolvedValue({ success: true })
27+
})
28+
29+
it("runs the child under the invoking run's environment variables", async () => {
30+
await runWorkflowTool(
31+
{ workflowId: 'wf-child', _context: { workspaceId: 'ws-1' } },
32+
{ environmentVariables: { MY_API_KEY: 'secret-value' } }
33+
)
34+
35+
const [ctxArg] = mockExecute.mock.calls[0]
36+
expect(ctxArg.environmentVariables).toEqual({ MY_API_KEY: 'secret-value' })
37+
})
38+
39+
it("forwards the invoking run's block-output redaction policy", async () => {
40+
await runWorkflowTool(
41+
{ workflowId: 'wf-child', _context: { workspaceId: 'ws-1' } },
42+
{ environmentVariables: {}, piiBlockOutputRedaction: PII_POLICY }
43+
)
44+
45+
const [ctxArg] = mockExecute.mock.calls[0]
46+
expect(ctxArg.piiBlockOutputRedaction).toBe(PII_POLICY)
47+
})
48+
49+
it('ignores an env map smuggled in through the model-reachable _context bag', async () => {
50+
const modelSuppliedContext = {
51+
workspaceId: 'ws-1',
52+
environmentVariables: { MY_API_KEY: 'model-injected' },
53+
}
54+
55+
await runWorkflowTool(
56+
{ workflowId: 'wf-child', _context: modelSuppliedContext },
57+
{ environmentVariables: { MY_API_KEY: 'trusted' } }
58+
)
59+
60+
const [ctxArg] = mockExecute.mock.calls[0]
61+
expect(ctxArg.environmentVariables).toEqual({ MY_API_KEY: 'trusted' })
62+
})
63+
})

apps/sim/executor/handlers/workflow/workflow-tool-runner.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { generateId } from '@sim/utils/id'
44
import { calculateCostSummary } from '@/lib/logs/execution/logging-factory'
55
import type { TraceSpan } from '@/lib/logs/types'
66
import { ChildWorkflowError } from '@/executor/errors/child-workflow-error'
7+
import type { PiiBlockOutputRedaction } from '@/executor/execution/types'
78
import {
89
buildCustomBlockExecutionContext,
910
type CustomBlockExecutorContext,
@@ -47,14 +48,21 @@ interface WorkflowToolParams {
4748
* On failure the result carries the structured error + the child executionId
4849
* in `output` so parent workflows can route on `error.code` and report a
4950
* reproducible handle to the workflow's provider.
51+
*
52+
* The child runs under the invoking run's environment variables and block-output
53+
* redaction policy, matching the canvas workflow block — `workflow-handler.ts`
54+
* keeps both from the parent context on the non-custom branch. The handler's
55+
* same-workspace assert bounds that forwarding to a single workspace.
5056
*/
5157
export async function runWorkflowTool(
5258
params: WorkflowToolParams,
5359
options: {
60+
environmentVariables: Record<string, string>
5461
abortSignal?: AbortSignal
5562
resolvedSecretTraceRegistry?: ResolvedSecretTraceRegistry
5663
executorDelegationOrigin?: ExecutorDelegationOrigin
57-
} = {}
64+
piiBlockOutputRedaction?: PiiBlockOutputRedaction
65+
}
5866
): Promise<ToolResponse> {
5967
if (!params.workflowId) {
6068
return { success: false, output: {}, error: 'Missing workflowId' }

0 commit comments

Comments
 (0)