Skip to content

Commit fbca58a

Browse files
committed
fix(streaming): redact tool payloads on selected outputs in public chat
Redaction only ran on the empty-selection branch, but a deployment almost always selects outputs, so it was dead in the case it exists for. Selecting toolCalls streamed the raw arguments and results to a public chat client in a chunk frame, and providerTiming carried thinking content the same way. Both paths now extract from the sanitized block output rather than the raw log: the streamed selected output, which is the reachable vector, and the final envelope. Sanitizing the source rather than per selected path means a newly selectable field cannot reopen the hole.
1 parent 7245f4d commit fbca58a

2 files changed

Lines changed: 78 additions & 3 deletions

File tree

apps/sim/lib/workflows/streaming/streaming.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,44 @@ describe('final envelope tool payloads', () => {
660660
expect(JSON.stringify(final)).not.toContain('72')
661661
})
662662

663+
/**
664+
* A deployment almost always selects outputs, so redaction that only covered
665+
* the empty-selection branch would be dead in the case it exists for.
666+
*/
667+
it('redacts tool payloads when the deployment selects toolCalls directly', async () => {
668+
const stream = await createStreamingResponse({
669+
requestId: 'request-1',
670+
streamConfig: { isSecureMode: true, selectedOutputs: ['block_toolCalls'] },
671+
executeFn: async ({ onBlockComplete }) => {
672+
const toolOnlyOutput = { toolCalls: agentOutput.toolCalls }
673+
await onBlockComplete('block', toolOnlyOutput)
674+
return {
675+
success: true,
676+
output: {},
677+
logs: [
678+
{
679+
blockId: 'block',
680+
output: toolOnlyOutput,
681+
startedAt: new Date().toISOString(),
682+
endedAt: new Date().toISOString(),
683+
durationMs: 1,
684+
success: true,
685+
},
686+
],
687+
} as any
688+
},
689+
})
690+
691+
// The payload rides the chunk frame, not `final`, so assert on the whole
692+
// stream — sanitizing only the envelope would still leak here.
693+
const events = await collectSSEEvents(stream)
694+
const serialized = JSON.stringify(events)
695+
696+
expect(serialized).not.toContain('private')
697+
expect(serialized).not.toContain('72')
698+
expect(serialized).toContain('get_weather')
699+
})
700+
663701
it('keeps tool results for the authenticated workflow API', async () => {
664702
const stream = await createStreamingResponse({
665703
requestId: 'request-1',

apps/sim/lib/workflows/streaming/streaming.ts

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { createLogger } from '@sim/logger'
22
import { getErrorMessage } from '@sim/utils/errors'
3-
import { omit } from '@sim/utils/object'
3+
import { isRecordLike, omit } from '@sim/utils/object'
44
import { createTimeoutAbortController, getTimeoutErrorMessage } from '@/lib/core/execution-limits'
55
import {
66
extractBlockIdFromOutputId,
@@ -366,6 +366,23 @@ async function buildMinimalResult(
366366
return minimalResult
367367
}
368368

369+
/**
370+
* Selected outputs are extracted from the sanitized block output, not the raw
371+
* log. A deployment can select `toolCalls` or `providerTiming` directly, so
372+
* sanitizing per selected path would leave the leak open for whichever path
373+
* was missed; sanitizing the source closes it for every path at once. Cached
374+
* per block because several descriptors can target the same one.
375+
*/
376+
const sanitizedBlockOutputs = new Map<string, Record<string, unknown>>()
377+
const sanitizedOutputFor = (blockId: string, output: Record<string, unknown>) => {
378+
const cached = sanitizedBlockOutputs.get(blockId)
379+
if (cached) return cached
380+
381+
const sanitized = sanitizeOutputForEnvelope(output, envelopeOptions)
382+
sanitizedBlockOutputs.set(blockId, sanitized)
383+
return sanitized
384+
}
385+
369386
let selectedOutputBytes = assertSelectedOutputBytes(minimalResult.output)
370387
for (const descriptor of getSelectedOutputDescriptors(selectedOutputs)) {
371388
const { blockId, path } = descriptor
@@ -406,7 +423,11 @@ async function buildMinimalResult(
406423
getBase64DecodedByteBudget(remainingBytes)
407424
),
408425
}
409-
const value = await extractOutputValue(blockLog.output, path, extractionContext)
426+
const value = await extractOutputValue(
427+
sanitizedOutputFor(blockId, blockLog.output),
428+
path,
429+
extractionContext
430+
)
410431
if (value === undefined) {
411432
continue
412433
}
@@ -692,6 +713,18 @@ export async function createStreamingResponse(
692713
(descriptor) => descriptor.blockId === blockId
693714
)
694715

716+
/**
717+
* A selected output is streamed here and then skipped in the `final`
718+
* envelope, so this is the reachable path for a deployment that selects
719+
* `toolCalls` or `providerTiming` — sanitizing only the envelope would
720+
* leave the payload flowing through the chunk frame instead.
721+
*/
722+
const sanitizedOutput = isRecordLike(output)
723+
? sanitizeOutputForEnvelope(output, {
724+
redactToolPayloads: streamConfig.isSecureMode === true,
725+
})
726+
: output
727+
695728
for (const descriptor of matchingOutputs) {
696729
if (state.selectedOutputError) {
697730
break
@@ -714,7 +747,11 @@ export async function createStreamingResponse(
714747
),
715748
}
716749
const materializationContext = buildMaterializationContext(extractionContext)
717-
const outputValue = await extractOutputValue(output, descriptor.path, extractionContext)
750+
const outputValue = await extractOutputValue(
751+
sanitizedOutput,
752+
descriptor.path,
753+
extractionContext
754+
)
718755

719756
if (outputValue !== undefined) {
720757
const materializedOutput = await materializeInlineExecutionValue(

0 commit comments

Comments
 (0)