Skip to content

Commit b58e594

Browse files
fix(powerbi): harden partial results and selector hydration
1 parent 62cac44 commit b58e594

18 files changed

Lines changed: 600 additions & 100 deletions

File tree

‎apps/docs/content/docs/integrations/powerbi.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ Sim requests these scopes when someone connects a Power BI account. On a self-ho
209209
| ----- | ----------- |
210210
| `https://analysis.windows.net/powerbi/api/Workspace.Read.All` | View Power BI workspaces |
211211
| `https://analysis.windows.net/powerbi/api/Report.Read.All` | View Power BI reports |
212-
| `https://analysis.windows.net/powerbi/api/Dataset.ReadWrite.All` | Read and query Power BI semantic models, request refreshes, and view refresh history |
212+
| `https://analysis.windows.net/powerbi/api/Dataset.ReadWrite.All` | Read, create, update, refresh, and delete Power BI semantic models within your account’s permissions |
213213
| `openid` | Standard authentication |
214214
| `profile` | Access profile information |
215215
| `email` | Access email address |

‎apps/sim/executor/execution/block-executor.test.ts‎

Lines changed: 143 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ import { uploadsMock } from '@sim/testing/mocks/uploads.mock'
55
import { DrizzleQueryError } from 'drizzle-orm/errors'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
77
import { clearLargeValueCacheForTests } from '@/lib/execution/payloads/cache'
8-
import { createLargeArrayManifest } from '@/lib/execution/payloads/large-array-manifest'
8+
import {
9+
createLargeArrayManifest,
10+
isLargeArrayManifest,
11+
readLargeArrayManifestSlice,
12+
} from '@/lib/execution/payloads/large-array-manifest'
913
import { isLargeValueRef } from '@/lib/execution/payloads/large-value-ref'
1014
import { buildTraceSpans } from '@/lib/logs/execution/trace-spans/trace-spans'
1115
import { validateBlockType } from '@/ee/access-control/utils/permission-check'
@@ -14,7 +18,7 @@ import type { DAGNode } from '@/executor/dag/builder'
1418
import { BlockExecutor } from '@/executor/execution/block-executor'
1519
import { ExecutionState } from '@/executor/execution/state'
1620
import type { BlockHandler, ExecutionContext } from '@/executor/types'
17-
import { attachTrustedExecutionCost } from '@/executor/utils/errors'
21+
import { attachToolFailureOutput, attachTrustedExecutionCost } from '@/executor/utils/errors'
1822
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
1923
import { VariableResolver } from '@/executor/variables/resolver'
2024
import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types'
@@ -129,6 +133,143 @@ describe('BlockExecutor', () => {
129133
expect(context.mcpBlockId).toBeUndefined()
130134
})
131135

136+
function createFailedToolExecution(output: Record<string, unknown>) {
137+
const block = createBlock()
138+
const workflow: SerializedWorkflow = {
139+
version: '1',
140+
blocks: [block],
141+
connections: [],
142+
loops: {},
143+
parallels: {},
144+
}
145+
const state = new ExecutionState()
146+
const failure = new Error('query incomplete')
147+
attachToolFailureOutput(failure, output)
148+
const handler: BlockHandler = {
149+
canHandle: () => true,
150+
execute: async () => {
151+
throw failure
152+
},
153+
}
154+
const onBlockComplete = vi.fn(async () => {})
155+
const executor = new BlockExecutor(
156+
[handler],
157+
new VariableResolver(workflow, {}, state),
158+
{ onBlockComplete },
159+
state
160+
)
161+
const ctx = createContext(state)
162+
ctx.piiBlockOutputRedaction = {
163+
enabled: true,
164+
entityTypes: ['EMAIL_ADDRESS'],
165+
language: 'en',
166+
}
167+
const node = createNode(block)
168+
node.outgoingEdges.set('error-edge', { sourceHandle: EDGE.ERROR, target: 'error-handler' })
169+
return { executor, block, state, ctx, node, failure, onBlockComplete }
170+
}
171+
172+
it('masks partial failed tool rows before error-port state and completion output', async () => {
173+
mockMaskBatch.mockImplementation(async (texts: string[]) =>
174+
texts.map((text) => text.replaceAll('alice@example.com', '<EMAIL_ADDRESS>'))
175+
)
176+
const { executor, block, state, ctx, node, onBlockComplete } = createFailedToolExecution({
177+
rows: [{ email: 'alice@example.com', count: 7 }],
178+
rowCount: 1,
179+
incomplete: true,
180+
})
181+
182+
const output = await executor.execute(ctx, node, block)
183+
await vi.waitFor(() => expect(onBlockComplete).toHaveBeenCalledOnce())
184+
185+
const expected = {
186+
rows: [{ email: '<EMAIL_ADDRESS>', count: 7 }],
187+
rowCount: 1,
188+
incomplete: true,
189+
error: 'query incomplete',
190+
}
191+
expect(output).toEqual(expected)
192+
expect(state.getBlockOutput(block.id)).toEqual(expected)
193+
expect(ctx.blockLogs[0]?.output).toEqual(expected)
194+
expect(onBlockComplete.mock.calls[0]?.[3]?.output).toEqual(expected)
195+
expect(ctx.blockLogs[0]).toMatchObject({ success: false, errorHandled: true })
196+
})
197+
198+
it('masks and re-stores partial failed tool manifests under the current execution', async () => {
199+
const items = [{ email: 'alice@example.com', count: 7 }]
200+
const manifest = await createLargeArrayManifest(items, {
201+
workspaceId: 'workspace-1',
202+
workflowId: 'workflow-1',
203+
executionId: 'source-execution',
204+
})
205+
clearLargeValueCacheForTests()
206+
mockDownloadFile.mockResolvedValue(Buffer.from(JSON.stringify(items)))
207+
mockMaskBatch.mockImplementation(async (texts: string[]) =>
208+
texts.map((text) => text.replaceAll('alice@example.com', '<EMAIL_ADDRESS>'))
209+
)
210+
const { executor, block, state, ctx, node, onBlockComplete } = createFailedToolExecution({
211+
rows: manifest,
212+
rowCount: 1,
213+
incomplete: true,
214+
})
215+
ctx.largeValueExecutionIds = ['source-execution']
216+
217+
const output = await executor.execute(ctx, node, block)
218+
await vi.waitFor(() => expect(onBlockComplete).toHaveBeenCalledOnce())
219+
220+
expect(output).toMatchObject({ rowCount: 1, incomplete: true, error: 'query incomplete' })
221+
expect(output.rows).toMatchObject({
222+
preview: [{ email: '<EMAIL_ADDRESS>', count: 7 }],
223+
chunks: [{ ref: { executionId: 'execution-1' } }],
224+
})
225+
if (!isLargeArrayManifest(output.rows)) throw new Error('Expected a masked row manifest')
226+
expect(
227+
await readLargeArrayManifestSlice(output.rows, 0, 1, {
228+
workspaceId: ctx.workspaceId,
229+
workflowId: ctx.workflowId,
230+
executionId: ctx.executionId,
231+
})
232+
).toEqual([{ email: '<EMAIL_ADDRESS>', count: 7 }])
233+
expect(state.getBlockOutput(block.id)).toEqual(output)
234+
expect(ctx.blockLogs[0]?.output).toEqual(output)
235+
expect(onBlockComplete.mock.calls[0]?.[3]?.output).toEqual(output)
236+
})
237+
238+
it('omits failed tool payloads when masking fails while preserving trusted cost', async () => {
239+
const unsafeFailure = 'mask service failed while processing alice@example.com'
240+
mockMaskBatch.mockRejectedValueOnce(new Error(unsafeFailure))
241+
const { executor, block, state, ctx, node, failure, onBlockComplete } =
242+
createFailedToolExecution({
243+
rows: [{ email: 'alice@example.com', count: 7 }],
244+
rowCount: 1,
245+
incomplete: true,
246+
})
247+
const cost = { input: 0.1, output: 0.2, total: 0.3 }
248+
attachTrustedExecutionCost(failure, cost)
249+
250+
const output = await executor.execute(ctx, node, block)
251+
await vi.waitFor(() => expect(onBlockComplete).toHaveBeenCalledOnce())
252+
253+
const expected = {
254+
error: 'PII redaction failed. Partial tool output was omitted.',
255+
cost,
256+
}
257+
expect(output).toEqual(expected)
258+
expect(state.getBlockOutput(block.id)).toEqual(expected)
259+
expect(ctx.blockLogs[0]?.output).toEqual(expected)
260+
expect(ctx.blockLogs[0]?.error).toBe(expected.error)
261+
expect(onBlockComplete.mock.calls[0]?.[3]?.output).toEqual(expected)
262+
expect(ctx.blockLogs[0]).toMatchObject({ success: false, errorHandled: true })
263+
const surfaced = JSON.stringify([
264+
output,
265+
ctx.blockLogs,
266+
onBlockComplete.mock.calls,
267+
blockExecutorBaseLogger.error.mock.calls,
268+
])
269+
expect(surfaced).not.toContain('alice@example.com')
270+
expect(surfaced).not.toContain(unsafeFailure)
271+
})
272+
132273
it('redacts an authorized prior-execution manifest returned by a block under the current execution', async () => {
133274
const items = [{ email: 'alice@example.com', count: 7 }]
134275
const manifest = await createLargeArrayManifest(items, {

‎apps/sim/executor/execution/block-executor.ts‎

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -350,35 +350,7 @@ export class BlockExecutor {
350350
})) as NormalizedBlockOutput
351351
}
352352

353-
if (blockCtx.piiBlockOutputRedaction?.enabled) {
354-
// In-flight redaction before the log/state split below, so both the
355-
// downstream state copy and the persisted log copy are masked.
356-
// `onFailure: 'throw'` aborts the run rather than feeding corrupted/leaked
357-
// data downstream.
358-
const redactionOptions = {
359-
entityTypes: blockCtx.piiBlockOutputRedaction.entityTypes,
360-
language: blockCtx.piiBlockOutputRedaction.language,
361-
customPatterns: blockCtx.piiBlockOutputRedaction.customPatterns,
362-
onFailure: 'throw' as const,
363-
}
364-
// Tools like the function executor offload large outputs to large-value
365-
// refs BEFORE they reach here, and the string walk treats a ref as opaque.
366-
// So hydrate → mask → re-store any refs first, then mask inline strings —
367-
// otherwise PII inside an offloaded output is never redacted.
368-
normalizedOutput = await redactLargeValueRefsInValue(normalizedOutput, {
369-
...redactionOptions,
370-
store: {
371-
workspaceId: blockCtx.workspaceId,
372-
workflowId: blockCtx.workflowId,
373-
executionId: blockCtx.executionId,
374-
largeValueExecutionIds: blockCtx.largeValueExecutionIds,
375-
largeValueKeys: blockCtx.largeValueKeys,
376-
allowLargeValueWorkflowScope: blockCtx.allowLargeValueWorkflowScope,
377-
userId: blockCtx.userId,
378-
},
379-
})
380-
normalizedOutput = await redactObjectStrings(normalizedOutput, redactionOptions)
381-
}
353+
normalizedOutput = await this.redactBlockOutput(normalizedOutput, blockCtx)
382354

383355
const compacted = await compactBlockOutput(normalizedOutput, {
384356
workspaceId: blockCtx.workspaceId,
@@ -622,6 +594,33 @@ export class BlockExecutor {
622594
}
623595
}
624596

597+
private async redactBlockOutput(
598+
output: NormalizedBlockOutput,
599+
ctx: ExecutionContext
600+
): Promise<NormalizedBlockOutput> {
601+
if (!ctx.piiBlockOutputRedaction?.enabled) return output
602+
const options = {
603+
entityTypes: ctx.piiBlockOutputRedaction.entityTypes,
604+
language: ctx.piiBlockOutputRedaction.language,
605+
customPatterns: ctx.piiBlockOutputRedaction.customPatterns,
606+
onFailure: 'throw' as const,
607+
}
608+
// Offloaded content must be hydrated, masked, and re-stored before inline strings.
609+
const redacted = await redactLargeValueRefsInValue(output, {
610+
...options,
611+
store: {
612+
workspaceId: ctx.workspaceId,
613+
workflowId: ctx.workflowId,
614+
executionId: ctx.executionId,
615+
largeValueExecutionIds: ctx.largeValueExecutionIds,
616+
largeValueKeys: ctx.largeValueKeys,
617+
allowLargeValueWorkflowScope: ctx.allowLargeValueWorkflowScope,
618+
userId: ctx.userId,
619+
},
620+
})
621+
return redactObjectStrings(redacted, options)
622+
}
623+
625624
private async handleBlockError(
626625
error: unknown,
627626
ctx: ExecutionContext,
@@ -637,10 +636,10 @@ export class BlockExecutor {
637636
streamingPartialOutput?: Record<string, any>,
638637
completedHandlerCost?: TrustedExecutionCost
639638
): Promise<NormalizedBlockOutput> {
640-
const endedAt = new Date().toISOString()
641-
const duration = performance.now() - startTime
639+
let endedAt = new Date().toISOString()
640+
let duration = performance.now() - startTime
642641
const isDatabaseError = error instanceof DrizzleQueryError
643-
const errorMessage = isDatabaseError ? INTERNAL_DATABASE_ERROR_MESSAGE : normalizeError(error)
642+
let errorMessage = isDatabaseError ? INTERNAL_DATABASE_ERROR_MESSAGE : normalizeError(error)
644643
const hasLogInputs =
645644
inputsForLog && typeof inputsForLog === 'object' && Object.keys(inputsForLog).length > 0
646645
const input = hasLogInputs
@@ -709,8 +708,19 @@ export class BlockExecutor {
709708
}
710709

711710
const trustedExecutionCost = readTrustedExecutionCost(error) ?? completedHandlerCost
711+
let partialOutput = readToolFailureOutput(error)
712+
if (partialOutput) {
713+
try {
714+
partialOutput = await this.redactBlockOutput(partialOutput, ctx)
715+
} catch {
716+
partialOutput = undefined
717+
errorMessage = 'PII redaction failed. Partial tool output was omitted.'
718+
}
719+
}
720+
endedAt = new Date().toISOString()
721+
duration = performance.now() - startTime
712722
const errorOutput: NormalizedBlockOutput = {
713-
...readToolFailureOutput(error),
723+
...partialOutput,
714724
error: errorMessage,
715725
...(trustedExecutionCost ? { cost: trustedExecutionCost } : {}),
716726
}

‎apps/sim/executor/handlers/generic/generic-handler.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { prepareResolvedSecretProjectedInputs } from '@/executor/utils/resolved-
1010
import type { ResolvedSecretInputPath } from '@/executor/utils/resolved-secret-trace-registry'
1111
import type { SerializedBlock } from '@/serializer/types'
1212
import { executeTool } from '@/tools'
13+
import { isToolDiagnosticFailure } from '@/tools/errors'
1314
import { isInternalToolConfig, type ToolConfig } from '@/tools/types'
1415
import { getTool } from '@/tools/utils'
1516

@@ -348,7 +349,7 @@ export class GenericBlockHandler implements BlockHandler {
348349
const error = new Error(errorMessage)
349350

350351
const declaredOutput: Record<string, unknown> = {}
351-
if (isPlainRecord(result.output)) {
352+
if (!isToolDiagnosticFailure(result) && isPlainRecord(result.output)) {
352353
for (const key of Object.keys(tool?.outputs ?? {})) {
353354
if (key !== 'cost' && key !== 'error' && Object.hasOwn(result.output, key)) {
354355
declaredOutput[key] = result.output[key]

‎apps/sim/lib/oauth/utils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export const SCOPE_DESCRIPTIONS: Record<string, string> = {
1515
'https://analysis.windows.net/powerbi/api/Workspace.Read.All': 'View Power BI workspaces',
1616
'https://analysis.windows.net/powerbi/api/Report.Read.All': 'View Power BI reports',
1717
'https://analysis.windows.net/powerbi/api/Dataset.ReadWrite.All':
18-
'Read and query Power BI semantic models, request refreshes, and view refresh history',
18+
'Read, create, update, refresh, and delete Power BI semantic models within your account’s permissions',
1919
'users.profile:write': 'Update Slack user profiles',
2020
'users.profile:read': 'View Slack user profiles',
2121
'channels:join': 'Join public Slack channels',

‎apps/sim/lib/selectors/manifest.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ export const selectorManifest = {
380380
search: true,
381381
detail: true,
382382
}),
383-
'powerbi.workspaces': providerSelector([], { listMode: 'paginated' }),
383+
'powerbi.workspaces': providerSelector([], { listMode: 'paginated', detail: true }),
384384
'powerbi.datasets': providerSelector(['groupId'], {
385385
readiness: { all: ['oauthCredential', 'groupId'] },
386386
detail: true,

‎apps/sim/lib/selectors/server/providers/powerbi.test.ts‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -161,30 +161,47 @@ describe('Power BI selector provider boundary', () => {
161161
}
162162
)
163163

164-
it.each(['datasets', 'reports'] as const)(
165-
'resolves a saved %s selection without loading the full resource list',
164+
it.each(['workspaces', 'datasets', 'reports'] as const)(
165+
'resolves a saved %s selection through one direct encoded provider request',
166166
async (kind) => {
167-
fetchMock.mockResolvedValueOnce(jsonResponse({ id: 'resource-id', name: 'Saved resource' }))
167+
fetchMock.mockResolvedValueOnce(
168+
jsonResponse({
169+
id: 'resource#id',
170+
name: 'Saved resource',
171+
rawProviderSecret: 'must-not-escape',
172+
})
173+
)
168174
const key = `powerbi.${kind}` as const
169175
expect(
170176
await powerBISelectorAttachments[key].execute(
171177
args({ selectorKey: key, request: { kind: 'detail', id: ' resource#id ' } })
172178
)
173-
).toEqual({ kind: 'detail', item: { id: 'resource-id', label: 'Saved resource' } })
174-
expect(new URL(String(fetchMock.mock.calls[0]?.[0])).pathname).toBe(
175-
`/v1.0/myorg/groups/workspace-one/${kind}/resource%23id`
179+
).toEqual({ kind: 'detail', item: { id: 'resource#id', label: 'Saved resource' } })
180+
expect(fetchMock).toHaveBeenCalledTimes(1)
181+
const url = new URL(String(fetchMock.mock.calls[0]?.[0]))
182+
expect(url.origin).toBe('https://api.powerbi.com')
183+
expect(url.pathname).toBe(
184+
kind === 'workspaces'
185+
? '/v1.0/myorg/groups/resource%23id'
186+
: `/v1.0/myorg/groups/workspace-one/${kind}/resource%23id`
176187
)
188+
expect(url.search).toBe('')
189+
expect(url.hash).toBe('')
177190
}
178191
)
179192

180-
it('returns no detail for a deleted semantic model', async () => {
181-
fetchMock.mockResolvedValueOnce(new Response(null, { status: 404 }))
182-
expect(
183-
await powerBISelectorAttachments['powerbi.datasets'].execute(
184-
args({ selectorKey: 'powerbi.datasets', request: { kind: 'detail', id: 'removed-model' } })
185-
)
186-
).toEqual({ kind: 'detail', item: null })
187-
})
193+
it.each(['workspaces', 'datasets'] as const)(
194+
'returns no detail for a deleted %s selection',
195+
async (kind) => {
196+
fetchMock.mockResolvedValueOnce(new Response(null, { status: 404 }))
197+
const key = `powerbi.${kind}` as const
198+
expect(
199+
await powerBISelectorAttachments[key].execute(
200+
args({ selectorKey: key, request: { kind: 'detail', id: 'removed-resource' } })
201+
)
202+
).toEqual({ kind: 'detail', item: null })
203+
}
204+
)
188205

189206
it.each([
190207
{},

‎apps/sim/tools/errors.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,17 @@
11
import { HttpError } from '@/lib/core/utils/http-error'
2+
import type { ToolResponse } from '@/tools/types'
3+
4+
const diagnosticFailures = new WeakSet<ToolResponse>()
5+
6+
/** Catch-generated HTTP diagnostics are not declared resource outputs. */
7+
export function markToolDiagnosticFailure(response: ToolResponse): ToolResponse {
8+
diagnosticFailures.add(response)
9+
return response
10+
}
11+
12+
export function isToolDiagnosticFailure(response: ToolResponse): boolean {
13+
return diagnosticFailures.has(response)
14+
}
215

316
/**
417
* Hosted-key acquisition blocked by the workspace's own rate bucket. Carries a

‎apps/sim/tools/generated/tool-outputs.ts‎

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)