Skip to content

Commit f4e16c5

Browse files
waleedlatif1waleedlatif1
committed
fix(logs): keep provenance sidecars intact through log PII redaction
Log-stage PII masking walks every string leaf of the execution state, so a match inside an encrypted provenance entry rewrote the ciphertext and the display read failed closed with the final output withheld. Restore the finalOutput/workflowInput sidecars, loop/parallel scope provenance, and sourceExecutionId from the pre-redaction state alongside the fields already restored. An undecryptable stored envelope now logs one attributed error per read instead of three.
1 parent 0f755fe commit f4e16c5

4 files changed

Lines changed: 197 additions & 53 deletions

File tree

‎apps/sim/executor/utils/resolved-secret-trace-registry.ts‎

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,18 @@ export interface ImportResolvedSecretTraceProvenanceOptions {
417417
* `source-provenance-incomplete` from a symptom into an address.
418418
*/
419419
origin?: string
420+
/**
421+
* Takes the import's entry decrypt-failure summary in place of the registry's own error line,
422+
* for a caller that reports the failure itself with the context the registry lacks.
423+
*/
424+
onDecryptFailure?: (failure: ProvenanceDecryptFailure) => void
425+
}
426+
427+
/** Entry decrypt failures of one provenance import, summarised. */
428+
export interface ProvenanceDecryptFailure {
429+
error: string
430+
failedEntryCount: number
431+
totalEntryCount: number
420432
}
421433

422434
export interface ExportResolvedSecretTraceProvenanceForValueOptions {
@@ -966,7 +978,7 @@ export class ResolvedSecretTraceRegistry {
966978
* A staged registry filters values for one operation and is then discarded. Its caller owns the
967979
* reporting and says it with strictly more context — the real input path for a value filter, the
968980
* execution for a display read — so the registry's own summary lines would only restate it.
969-
* Entry-level detail still logs — the caller cannot reconstruct it.
981+
* Entry decrypt failures still log unless the caller takes them through `onDecryptFailure`.
970982
*/
971983
private readonly staged: boolean
972984
/**
@@ -1694,11 +1706,10 @@ export class ResolvedSecretTraceRegistry {
16941706

16951707
const sameScope = scopesMatch(provenance.scope, this.scope)
16961708
let importedAll = true
1697-
let decryptFailures = 0
1698-
let firstDecryptError: string | undefined
1709+
let decryptFailure: ProvenanceDecryptFailure | undefined
16991710
for (const entry of provenance.entries) {
17001711
try {
1701-
const { decrypted } = await decryptSecret(entry.encryptedValue)
1712+
const { decrypted } = await decryptSecret(entry.encryptedValue, { logFailure: false })
17021713
this.addActiveEntry(
17031714
{
17041715
name: entry.name ?? '',
@@ -1710,8 +1721,12 @@ export class ResolvedSecretTraceRegistry {
17101721
)
17111722
} catch (error) {
17121723
importedAll = false
1713-
decryptFailures += 1
1714-
firstDecryptError ??= getErrorMessage(error, 'Unknown error')
1724+
decryptFailure ??= {
1725+
error: getErrorMessage(error, 'Unknown error'),
1726+
failedEntryCount: 0,
1727+
totalEntryCount: provenance.entries.length,
1728+
}
1729+
decryptFailure.failedEntryCount += 1
17151730
this.markIncomplete('entry-decrypt-failed', { origin: options.origin })
17161731
}
17171732
}
@@ -1720,13 +1735,15 @@ export class ResolvedSecretTraceRegistry {
17201735
* Summarised rather than logged per entry: one rotated or corrupt key fails every entry in the
17211736
* bundle, and a bundle may carry up to MAX_PROVENANCE_ENTRIES of them.
17221737
*/
1723-
if (decryptFailures > 0) {
1724-
logger.error('Provenance entries could not be decrypted', {
1725-
error: firstDecryptError,
1726-
failedEntryCount: decryptFailures,
1727-
totalEntryCount: provenance.entries.length,
1728-
scopeWorkspaceId: this.scope?.workspaceId,
1729-
})
1738+
if (decryptFailure) {
1739+
if (options.onDecryptFailure) {
1740+
options.onDecryptFailure(decryptFailure)
1741+
} else {
1742+
logger.error('Provenance entries could not be decrypted', {
1743+
...decryptFailure,
1744+
scopeWorkspaceId: this.scope?.workspaceId,
1745+
})
1746+
}
17301747
}
17311748

17321749
return importedAll

‎apps/sim/lib/logs/execution/logger.test.ts‎

Lines changed: 74 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
billingUsageMonitorMock,
2424
billingUsageMonitorMockFns,
2525
} from '@sim/testing/mocks/billing-usage-monitor.mock'
26+
import { maskClientMock, maskClientMockFns } from '@sim/testing/mocks/mask-client.mock'
2627
import { isPlainRecord } from '@sim/utils/object'
2728
import { afterAll, beforeEach, describe, expect, test, vi } from 'vitest'
2829
import { recordUsage } from '@/lib/billing/core/usage-log'
@@ -95,6 +96,8 @@ vi.mock('@/lib/billing/core/usage', () => billingUsageMock)
9596

9697
vi.mock('@/lib/billing/core/usage-log', () => billingUsageLogMock)
9798

99+
vi.mock('@/lib/guardrails/mask-client', () => maskClientMock)
100+
98101
vi.mock('@/lib/billing/threshold-billing', () => ({
99102
checkAndBillOverageThreshold: vi.fn(() => Promise.resolve()),
100103
checkAndBillPayerOverageThreshold: vi.fn(() => Promise.resolve()),
@@ -270,6 +273,9 @@ describe('ExecutionLogger', () => {
270273
async function completeAndReadWrite(params: {
271274
executionState?: SerializableExecutionState
272275
redactedState?: SerializableExecutionState
276+
/** Runs the real log-stage PII pass instead of standing in for it. */
277+
realRedaction?: boolean
278+
finalOutput?: Record<string, unknown>
273279
}) {
274280
const startedAt = new Date('2026-08-11T00:00:00.000Z')
275281
queueTableRows(workflowExecutionLogs, [
@@ -299,12 +305,14 @@ describe('ExecutionLogger', () => {
299305
costTotalRefined: boolean
300306
}>
301307
}
302-
vi.spyOn(internals, 'applyPiiRedaction').mockImplementation(
303-
async (_workspaceId: string, payload: Record<string, unknown>) =>
304-
Object.hasOwn(params, 'redactedState')
305-
? { ...payload, executionState: params.redactedState }
306-
: payload
307-
)
308+
if (!params.realRedaction) {
309+
vi.spyOn(internals, 'applyPiiRedaction').mockImplementation(
310+
async (_workspaceId: string, payload: Record<string, unknown>) =>
311+
Object.hasOwn(params, 'redactedState')
312+
? { ...payload, executionState: params.redactedState }
313+
: payload
314+
)
315+
}
308316
vi.spyOn(internals, 'recordExecutionUsage').mockResolvedValue({
309317
recordedIncrement: 0,
310318
costTotalRefined: false,
@@ -324,7 +332,7 @@ describe('ExecutionLogger', () => {
324332
baseExecutionCharge: 0,
325333
models: {},
326334
},
327-
finalOutput: { completed: true },
335+
finalOutput: params.finalOutput ?? { completed: true },
328336
traceSpans: [],
329337
...(params.executionState ? { executionState: params.executionState } : {}),
330338
})
@@ -433,6 +441,65 @@ describe('ExecutionLogger', () => {
433441
expect(preserved?.blockStates).toEqual(redactedState.blockStates)
434442
})
435443

444+
test('keeps encrypted provenance sidecars intact through log PII redaction', async () => {
445+
queueTableRows(workspace, [
446+
{
447+
orgSettings: {
448+
piiRedaction: { rules: [{ workspaceId: null, entityTypes: ['PHONE_NUMBER'] }] },
449+
},
450+
},
451+
])
452+
maskClientMockFns.mockMaskPIIBatchViaHttp.mockImplementation(async (texts: string[]) => ({
453+
masked: texts.map((text) => text.replace(/\d{7,}/g, '<PHONE_NUMBER>')),
454+
scrubbedCount: 0,
455+
}))
456+
const sidecar = (encryptedValue: string) => ({
457+
version: 1 as const,
458+
complete: true,
459+
entries: [{ name: 'API_SECRET', encryptedValue }],
460+
})
461+
const finalOutputProvenance = sidecar('9f3c2a5551234567e0:4b18093327716650aa:0c1d')
462+
const workflowInputProvenance = sidecar('1e2f5550001111e0:77a0384460007799bb:9d8e')
463+
const loopInputProvenance = sidecar('3a4b5552223333c0:66b0129930004455cc:5f6a')
464+
const parallelOutputProvenance = sidecar('7c8d5554445555a0:11b0567780009911dd:2e3f')
465+
466+
const written = await completeAndReadWrite({
467+
realRedaction: true,
468+
finalOutput: { reply: 'Call me at 5551234567' },
469+
executionState: {
470+
...EMPTY_STATE,
471+
sourceExecutionId: 'execution-20260811123456789',
472+
resolvedSecretTraceProvenance: RUN_PROVENANCE,
473+
resolvedSecretTraceCheckpointVersion: 1,
474+
finalOutputResolvedSecretTraceProvenance: finalOutputProvenance,
475+
workflowInputResolvedSecretTraceProvenance: workflowInputProvenance,
476+
loopExecutions: {
477+
loop1: {
478+
iteration: 0,
479+
items: ['Call 5559876543'],
480+
inputResolvedSecretTraceProvenance: loopInputProvenance,
481+
},
482+
},
483+
parallelExecutions: {
484+
parallel1: { resolvedSecretTraceProvenance: parallelOutputProvenance },
485+
},
486+
} as SerializableExecutionState,
487+
})
488+
489+
const state = written?.executionState as SerializableExecutionState | undefined
490+
expect(written?.finalOutput).toEqual({ reply: 'Call me at <PHONE_NUMBER>' })
491+
expect(state?.loopExecutions?.loop1?.items).toEqual(['Call <PHONE_NUMBER>'])
492+
expect(state?.finalOutputResolvedSecretTraceProvenance).toEqual(finalOutputProvenance)
493+
expect(state?.workflowInputResolvedSecretTraceProvenance).toEqual(workflowInputProvenance)
494+
expect(state?.loopExecutions?.loop1?.inputResolvedSecretTraceProvenance).toEqual(
495+
loopInputProvenance
496+
)
497+
expect(state?.parallelExecutions?.parallel1?.resolvedSecretTraceProvenance).toEqual(
498+
parallelOutputProvenance
499+
)
500+
expect(state?.sourceExecutionId).toBe('execution-20260811123456789')
501+
})
502+
436503
test('summarizes oversized execution data before storage', () => {
437504
const loggerInstance = new ExecutionLogger() as any
438505
const largePayload = 'x'.repeat(1_100_000)

‎apps/sim/lib/logs/execution/logger.ts‎

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
getPostgresErrorCode,
1616
} from '@sim/utils/errors'
1717
import { generateId } from '@sim/utils/id'
18+
import { filterUndefined, isRecordLike } from '@sim/utils/object'
1819
import { and, eq, inArray, sql } from 'drizzle-orm'
1920
import { checkUsageStatus as checkResolvedUsageStatus } from '@/lib/billing/calculations/usage-monitor'
2021
import { type BillingAttributionSnapshot, toBillingContext } from '@/lib/billing/core/attribution'
@@ -259,6 +260,27 @@ function summarizeTraceSpansForExecutionData(traceSpans?: TraceSpan[]): TraceSpa
259260
})
260261
}
261262

263+
/** Puts each loop or parallel scope's own provenance back onto its masked copy. */
264+
function restoreSubflowScopeProvenance(
265+
redactedScopes: Record<string, unknown> | undefined,
266+
originalScopes: Record<string, unknown> | undefined
267+
): Record<string, unknown> | undefined {
268+
if (!redactedScopes) return redactedScopes
269+
const scopes = { ...redactedScopes }
270+
for (const [scopeId, originalScope] of Object.entries(originalScopes ?? {})) {
271+
const redactedScope = scopes[scopeId]
272+
if (!isRecordLike(redactedScope) || !isRecordLike(originalScope)) continue
273+
scopes[scopeId] = {
274+
...redactedScope,
275+
...filterUndefined({
276+
inputResolvedSecretTraceProvenance: originalScope.inputResolvedSecretTraceProvenance,
277+
resolvedSecretTraceProvenance: originalScope.resolvedSecretTraceProvenance,
278+
}),
279+
}
280+
}
281+
return scopes
282+
}
283+
262284
function summarizeExecutionState(executionState?: SerializableExecutionState) {
263285
if (!executionState) {
264286
return undefined
@@ -777,18 +799,19 @@ export class ExecutionLogger {
777799
})
778800
}
779801

780-
/** Restores server-only lifecycle metadata after broad execution-state PII masking. */
802+
/**
803+
* Restores server-only lifecycle metadata after broad execution-state PII masking. The masker
804+
* walks every string leaf, so it can rewrite a substring of an encrypted provenance entry or an
805+
* id and make it unusable; these fields carry no user content, so the originals go back verbatim.
806+
*/
781807
private preservePrivateExecutionStateMetadata(
782808
redactedState: SerializableExecutionState | undefined,
783809
originalState: SerializableExecutionState | undefined
784810
): SerializableExecutionState | undefined {
785-
if (!redactedState) return redactedState
811+
if (!redactedState || !originalState) return redactedState
786812

787-
const provenance = originalState?.resolvedSecretTraceProvenance
788-
const provenanceCheckpointVersion = originalState?.resolvedSecretTraceCheckpointVersion
789-
const trustedLargeValueAccess = originalState?.trustedLargeValueAccess
790813
const blockStates = { ...redactedState.blockStates }
791-
for (const [blockId, originalBlockState] of Object.entries(originalState?.blockStates ?? {})) {
814+
for (const [blockId, originalBlockState] of Object.entries(originalState.blockStates)) {
792815
const redactedBlockState = blockStates[blockId]
793816
if (redactedBlockState && originalBlockState.resolvedSecretTraceProvenance) {
794817
blockStates[blockId] = {
@@ -798,8 +821,7 @@ export class ExecutionLogger {
798821
}
799822
}
800823
const blockLogs = redactedState.blockLogs.map((log, index) => {
801-
const displayProvenance =
802-
originalState?.blockLogs[index]?.displayResolvedSecretTraceProvenance
824+
const displayProvenance = originalState.blockLogs[index]?.displayResolvedSecretTraceProvenance
803825
return displayProvenance
804826
? { ...log, displayResolvedSecretTraceProvenance: displayProvenance }
805827
: log
@@ -809,17 +831,26 @@ export class ExecutionLogger {
809831
...redactedState,
810832
blockStates,
811833
blockLogs,
812-
...(provenance !== undefined ? { resolvedSecretTraceProvenance: provenance } : {}),
813-
...(originalState?.workflowVariableResolvedSecretTraceProvenance !== undefined
814-
? {
815-
workflowVariableResolvedSecretTraceProvenance:
816-
originalState.workflowVariableResolvedSecretTraceProvenance,
817-
}
818-
: {}),
819-
...(provenanceCheckpointVersion !== undefined
820-
? { resolvedSecretTraceCheckpointVersion: provenanceCheckpointVersion }
821-
: {}),
822-
...(trustedLargeValueAccess !== undefined ? { trustedLargeValueAccess } : {}),
834+
...filterUndefined({
835+
loopExecutions: restoreSubflowScopeProvenance(
836+
redactedState.loopExecutions,
837+
originalState.loopExecutions
838+
),
839+
parallelExecutions: restoreSubflowScopeProvenance(
840+
redactedState.parallelExecutions,
841+
originalState.parallelExecutions
842+
),
843+
sourceExecutionId: originalState.sourceExecutionId,
844+
trustedLargeValueAccess: originalState.trustedLargeValueAccess,
845+
resolvedSecretTraceProvenance: originalState.resolvedSecretTraceProvenance,
846+
workflowVariableResolvedSecretTraceProvenance:
847+
originalState.workflowVariableResolvedSecretTraceProvenance,
848+
workflowInputResolvedSecretTraceProvenance:
849+
originalState.workflowInputResolvedSecretTraceProvenance,
850+
finalOutputResolvedSecretTraceProvenance:
851+
originalState.finalOutputResolvedSecretTraceProvenance,
852+
resolvedSecretTraceCheckpointVersion: originalState.resolvedSecretTraceCheckpointVersion,
853+
}),
823854
}
824855
}
825856

0 commit comments

Comments
 (0)