diff --git a/apps/sim/executor/utils/resolved-secret-trace-registry.ts b/apps/sim/executor/utils/resolved-secret-trace-registry.ts index c45fe035f2f..e8d6e44412f 100644 --- a/apps/sim/executor/utils/resolved-secret-trace-registry.ts +++ b/apps/sim/executor/utils/resolved-secret-trace-registry.ts @@ -880,9 +880,10 @@ export class ResolvedSecretTraceRegistry { private readonly scope?: ResolvedSecretTraceScopeV1 private readonly completeProvenanceEnvelopeBytes: number /** - * A staged registry filters one value and is then discarded. Its caller re-reports whatever - * fault it hits against the real input path, so its own summary lines would restate that with - * strictly less context. Entry-level detail still logs — the caller cannot reconstruct it. + * A staged registry filters values for one operation and is then discarded. Its caller owns the + * reporting and says it with strictly more context — the real input path for a value filter, the + * execution for a display read — so the registry's own summary lines would only restate it. + * Entry-level detail still logs — the caller cannot reconstruct it. */ private readonly staged: boolean diff --git a/apps/sim/lib/logs/execution/trace-store.test.ts b/apps/sim/lib/logs/execution/trace-store.test.ts index f6afaf68b1d..c73e65241a4 100644 --- a/apps/sim/lib/logs/execution/trace-store.test.ts +++ b/apps/sim/lib/logs/execution/trace-store.test.ts @@ -3,10 +3,16 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest' -const { decryptSecretMock, materializeLargeValueRefMock, storeLargeValueMock } = vi.hoisted(() => ({ - decryptSecretMock: vi.fn(), - materializeLargeValueRefMock: vi.fn(), - storeLargeValueMock: vi.fn(), +const { decryptSecretMock, materializeLargeValueRefMock, storeLargeValueMock, mockLogger } = + vi.hoisted(() => ({ + decryptSecretMock: vi.fn(), + materializeLargeValueRefMock: vi.fn(), + storeLargeValueMock: vi.fn(), + mockLogger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + })) + +vi.mock('@sim/logger', () => ({ + createLogger: () => mockLogger, })) vi.mock('@/lib/core/security/encryption', () => ({ @@ -536,3 +542,233 @@ describe('projectExecutionDataForDisplay provenance handling', () => { expect(displayData.traceSpans).toEqual([]) }) }) + +describe('stored provenance display reporting', () => { + const REGISTRY_SUMMARY_MESSAGES = [ + 'Resolved secret registry marked incomplete', + 'Resolved secret input path marked incomplete', + ] + + function registrySummaryLines(): unknown[] { + return [...mockLogger.warn.mock.calls, ...mockLogger.error.mock.calls].filter(([message]) => + REGISTRY_SUMMARY_MESSAGES.includes(message as string) + ) + } + + /** + * The stored state was recorded when the run wrote it; a view re-deriving it must say which + * execution it served, once — not restate the latch through registry summaries that name none. + */ + it('reports an incomplete stored envelope once, naming the execution and the parts', async () => { + const displayData = await projectExecutionDataForDisplay( + { + finalOutput: { result: 'value' }, + executionState: { + resolvedSecretTraceProvenance: { version: 1, complete: false, entries: [] }, + finalOutputResolvedSecretTraceProvenance: { version: 1, complete: false, entries: [] }, + }, + }, + CONTEXT + ) + + expect(displayData).not.toHaveProperty('finalOutput') + expect(registrySummaryLines()).toHaveLength(0) + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Stored execution provenance cannot vouch for display content', + expect.objectContaining({ + site: 'traceStore.displayProjection', + executionId: 'execution-1', + workflowId: 'workflow-1', + workspaceId: 'workspace-1', + parts: ['traceSpans', 'finalOutput'], + partCount: 2, + }) + ) + expect(mockLogger.error).not.toHaveBeenCalled() + }) + + it('reports a malformed stored envelope at error, keeping the value withheld', async () => { + const displayData = await projectExecutionDataForDisplay( + { + finalOutput: { result: 'value' }, + executionState: { + resolvedSecretTraceProvenance: { + version: 1, + complete: true, + entries: [], + scope: { userId: 'user-1', workspaceId: 'workspace-1' }, + }, + finalOutputResolvedSecretTraceProvenance: 'garbage', + }, + }, + CONTEXT + ) + + expect(displayData).not.toHaveProperty('finalOutput') + expect(registrySummaryLines()).toHaveLength(0) + expect(mockLogger.error).toHaveBeenCalledWith( + 'Stored execution provenance is malformed', + expect.objectContaining({ + site: 'traceStore.displayProjection', + executionId: 'execution-1', + parts: ['finalOutput'], + }) + ) + }) + + /** A complete envelope whose entries cannot be decrypted withholds content like any fault. */ + it('attributes an undecryptable stored envelope to its execution at error', async () => { + decryptSecretMock.mockRejectedValue(new Error('key rotated')) + + const displayData = await projectExecutionDataForDisplay( + { + finalOutput: { result: 'value' }, + executionState: { + resolvedSecretTraceProvenance: { + version: 1, + complete: true, + entries: [], + scope: { userId: 'user-1', workspaceId: 'workspace-1' }, + }, + finalOutputResolvedSecretTraceProvenance: { + version: 1, + complete: true, + entries: [{ name: 'SECRET', encryptedValue: 'ciphertext' }], + scope: { userId: 'user-1', workspaceId: 'workspace-1' }, + }, + }, + }, + CONTEXT + ) + + expect(displayData).not.toHaveProperty('finalOutput') + expect(registrySummaryLines()).toHaveLength(0) + expect(mockLogger.error).toHaveBeenCalledWith( + 'Stored execution provenance could not be decrypted', + expect.objectContaining({ + site: 'traceStore.displayProjection', + executionId: 'execution-1', + parts: ['finalOutput'], + }) + ) + }) + + it('reports a malformed block-output envelope at error, withholding the output', async () => { + const result = await materializeExecutionDataForDisplayWithBlockOutputs( + { + executionState: { + resolvedSecretTraceProvenance: { + version: 1, + complete: true, + entries: [], + scope: { userId: 'user-1', workspaceId: 'workspace-1' }, + }, + blockStates: { + 'block-1': { output: { value: 1 }, resolvedSecretTraceProvenance: 'garbage' }, + }, + }, + }, + CONTEXT, + ['block-1'] + ) + + expect(result.blockOutputs.has('block-1')).toBe(false) + expect(registrySummaryLines()).toHaveLength(0) + expect(mockLogger.error).toHaveBeenCalledWith( + 'Stored execution provenance is malformed', + expect.objectContaining({ + site: 'traceStore.blockOutputs', + executionId: 'execution-1', + parts: ['blockOutput:block-1'], + }) + ) + }) + + it('stays silent when every stored envelope is complete', async () => { + const displayData = await projectExecutionDataForDisplay( + { + finalOutput: { result: 'direct-literal' }, + executionState: { + resolvedSecretTraceProvenance: { + version: 1, + complete: true, + entries: [], + scope: { userId: 'user-1', workspaceId: 'workspace-1' }, + }, + finalOutputResolvedSecretTraceProvenance: { + version: 1, + complete: true, + entries: [], + scope: { userId: 'user-1', workspaceId: 'workspace-1' }, + }, + }, + }, + CONTEXT + ) + + expect(displayData.finalOutput).toEqual({ result: 'direct-literal' }) + expect(mockLogger.warn).not.toHaveBeenCalled() + expect(mockLogger.error).not.toHaveBeenCalled() + }) + + /** The block entry point runs both display functions; each names its own site for the envelope. */ + it('attributes an incomplete run envelope under both sites on a block-outputs read', async () => { + await materializeExecutionDataForDisplayWithBlockOutputs( + { + finalOutput: { result: 'value' }, + executionState: { + resolvedSecretTraceProvenance: { version: 1, complete: false, entries: [] }, + blockStates: { + 'block-1': { output: { value: 1 } }, + }, + }, + }, + CONTEXT, + ['block-1'] + ) + + expect(registrySummaryLines()).toHaveLength(0) + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Stored execution provenance cannot vouch for display content', + expect.objectContaining({ site: 'traceStore.displayProjection', parts: ['traceSpans'] }) + ) + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Stored execution provenance cannot vouch for display content', + expect.objectContaining({ site: 'traceStore.blockOutputs', parts: ['run'] }) + ) + }) + + it('reports incomplete block-output envelopes once for the whole block read', async () => { + const result = await materializeExecutionDataForDisplayWithBlockOutputs( + { + executionState: { + resolvedSecretTraceProvenance: { + version: 1, + complete: true, + entries: [], + scope: { userId: 'user-1', workspaceId: 'workspace-1' }, + }, + blockStates: { + 'block-1': { + output: { value: 1 }, + resolvedSecretTraceProvenance: { version: 1, complete: false, entries: [] }, + }, + }, + }, + }, + CONTEXT, + ['block-1'] + ) + + expect(result.blockOutputs.has('block-1')).toBe(false) + expect(registrySummaryLines()).toHaveLength(0) + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Stored execution provenance cannot vouch for display content', + expect.objectContaining({ + site: 'traceStore.blockOutputs', + executionId: 'execution-1', + parts: ['blockOutput:block-1'], + }) + ) + }) +}) diff --git a/apps/sim/lib/logs/execution/trace-store.ts b/apps/sim/lib/logs/execution/trace-store.ts index fc3306d597c..afaed357d39 100644 --- a/apps/sim/lib/logs/execution/trace-store.ts +++ b/apps/sim/lib/logs/execution/trace-store.ts @@ -300,11 +300,13 @@ export async function materializeExecutionDataForDisplayWithBlockOutputs( return { executionData: displayData, blockOutputs: new Map() } } - const runRegistry = await importResolvedSecretTraceRegistry( + const runImport = await importStoredDisplayEnvelope( materialized[RESOLVED_SECRET_PROVENANCE_KEY] ?? executionState?.[RESOLVED_SECRET_PROVENANCE_KEY], 'traceStore.blockOutputRunProvenance' ) + const provenanceFaults = new Map() + if (runImport.fault) provenanceFaults.set('run', runImport.fault) const blockOutputs = new Map() const projectionStore = createReadOnlyProjectionStore(context) @@ -312,13 +314,15 @@ export async function materializeExecutionDataForDisplayWithBlockOutputs( const blockState = readRecord(blockStates[blockId]) if (!blockState || blockState.output === undefined) continue - const hasExactProvenance = Object.hasOwn(blockState, RESOLVED_SECRET_PROVENANCE_KEY) - const registry = hasExactProvenance - ? await importResolvedSecretTraceRegistry( - blockState[RESOLVED_SECRET_PROVENANCE_KEY], - 'traceStore.blockOutputExactProvenance' - ) - : runRegistry + let registry = runImport.registry + if (Object.hasOwn(blockState, RESOLVED_SECRET_PROVENANCE_KEY)) { + const blockImport = await importStoredDisplayEnvelope( + blockState[RESOLVED_SECRET_PROVENANCE_KEY], + 'traceStore.blockOutputExactProvenance' + ) + if (blockImport.fault) provenanceFaults.set(`blockOutput:${blockId}`, blockImport.fault) + registry = blockImport.registry + } const now = new Date().toISOString() const [projected] = await projectTraceSpansForSecrets( [ @@ -338,6 +342,7 @@ export async function materializeExecutionDataForDisplayWithBlockOutputs( blockOutputs.set(blockId, projected.output.value) } } + reportStoredDisplayProvenanceFaults('traceStore.blockOutputs', context, provenanceFaults) return { executionData: displayData, blockOutputs } } @@ -346,15 +351,98 @@ function readRecord(value: unknown): Record | undefined { return isRecordLike(value) ? (value as Record) : undefined } -async function importResolvedSecretTraceRegistry( +type StoredDisplayProvenanceFault = 'incomplete' | 'malformed' | 'undecryptable' + +interface StoredDisplayEnvelopeImport { + registry: ResolvedSecretTraceRegistry | undefined + fault: StoredDisplayProvenanceFault | undefined +} + +/** + * Staged: display registries filter stored values for one materialization and are discarded, and + * their own mark-time summaries name no execution — the read boundary reports instead, through + * {@link reportStoredDisplayProvenanceFaults}. A stored envelope's incompleteness is not an event + * on this path; it was recorded when the run wrote it, and every later view re-derives it. + * + * The fault is classified where the import happens so every consumer reports the same way: an + * absent envelope is not a fault (truncation has its own warning), a present value that does not + * parse is `malformed`, a parsed envelope that cannot vouch is `incomplete`, and a complete + * envelope whose registry latched during import — entry decryption is the only latch on this + * trusted path — is `undecryptable`. Projection withholds the guarded values in all three cases. + */ +async function importStoredDisplayEnvelope( provenance: unknown, origin: string -): Promise { - if (!isResolvedSecretTraceProvenanceV1(provenance)) return undefined +): Promise { + if (provenance === undefined) return { registry: undefined, fault: undefined } + if (!isResolvedSecretTraceProvenanceV1(provenance)) { + return { registry: undefined, fault: 'malformed' } + } - const registry = new ResolvedSecretTraceRegistry([], provenance.scope) + const registry = new ResolvedSecretTraceRegistry([], provenance.scope, { staged: true }) await registry.importProvenance(provenance, { trusted: true, origin }) - return registry + const fault = !provenance.complete + ? 'incomplete' + : registry.isPermanentlyIncomplete() + ? 'undecryptable' + : undefined + return { registry, fault } +} + +const MAX_REPORTED_PROVENANCE_FAULT_PARTS = 20 + +const STORED_PROVENANCE_FAULT_REPORTS = { + incomplete: { + level: 'warn', + message: 'Stored execution provenance cannot vouch for display content', + }, + malformed: { level: 'error', message: 'Stored execution provenance is malformed' }, + /** The entry-level decrypt error already logs its counts; this adds the execution it hit. */ + undecryptable: { level: 'error', message: 'Stored execution provenance could not be decrypted' }, +} as const satisfies Record< + StoredDisplayProvenanceFault, + { level: 'warn' | 'error'; message: string } +> + +/** + * One attributed line per fault kind per display function, in place of one registry summary per + * envelope per view. + * + * The registry summaries these replace carried counts and a workspace but no execution id, so a + * reader repeatedly materializing the same stored rows produced an unattributable stream — the + * lines could not say which executions to go look at. Severity follows the registry reason each + * fault replaces: incomplete at warn (a stored state being re-read), malformed and undecryptable + * at error (faults wherever they are met). + * + * A block-outputs read runs the display projection first, so a faulted run envelope appears once + * under each site — `traceSpans` guarding the span projection, `run` as the block fallback. Two + * sites reading the same envelope are two facts about the view; collapsing them would couple the + * display functions to share reporting state for one line less. + */ +function reportStoredDisplayProvenanceFaults( + site: string, + context: TraceStoreReadContext, + faults: ReadonlyMap +): void { + if (faults.size === 0) return + const details = { + site, + executionId: context.executionId, + ...(context.workflowId ? { workflowId: context.workflowId } : {}), + ...(context.workspaceId ? { workspaceId: context.workspaceId } : {}), + } + for (const [kind, report] of Object.entries(STORED_PROVENANCE_FAULT_REPORTS) as [ + StoredDisplayProvenanceFault, + (typeof STORED_PROVENANCE_FAULT_REPORTS)[StoredDisplayProvenanceFault], + ][]) { + const parts = [...faults].filter(([, fault]) => fault === kind).map(([part]) => part) + if (parts.length === 0) continue + logger[report.level](report.message, { + ...details, + parts: parts.slice(0, MAX_REPORTED_PROVENANCE_FAULT_PARTS), + partCount: parts.length, + }) + } } function createReadOnlyProjectionStore(context: TraceStoreReadContext) { @@ -395,7 +483,10 @@ export async function projectExecutionDataForDisplay( return projectLegacyExecutionDataForDisplay(executionData) } - const registry = await importResolvedSecretTraceRegistry(provenance, 'traceStore.spanProvenance') + const provenanceFaults = new Map() + const runImport = await importStoredDisplayEnvelope(provenance, 'traceStore.spanProvenance') + const registry = runImport.registry + if (runImport.fault) provenanceFaults.set('traceSpans', runImport.fault) /** * Compaction drops `executionState`, and with it the only copy of the @@ -436,16 +527,18 @@ export async function projectExecutionDataForDisplay( continue } - const exactProvenance = executionState[provenanceKey] - const exactRegistry = isResolvedSecretTraceProvenanceV1(exactProvenance) - ? new ResolvedSecretTraceRegistry([], exactProvenance.scope) - : new ResolvedSecretTraceRegistry() - if (isResolvedSecretTraceProvenanceV1(exactProvenance)) { - await exactRegistry.importProvenance(exactProvenance, { - trusted: true, - origin: 'traceStore.exactProvenance', - }) - } else { + const exactImport = await importStoredDisplayEnvelope( + executionState[provenanceKey], + 'traceStore.exactProvenance' + ) + if (exactImport.fault) provenanceFaults.set(valueKey, exactImport.fault) + /** + * The exact value must project against SOME registry, so an unusable envelope gets a latched + * one — the projection then withholds the value rather than passing it through unguarded. + */ + let exactRegistry = exactImport.registry + if (!exactRegistry) { + exactRegistry = new ResolvedSecretTraceRegistry([], undefined, { staged: true }) exactRegistry.markIncomplete('untrusted-provenance', { origin: 'traceStore.exactProvenance' }) } @@ -467,6 +560,7 @@ export async function projectExecutionDataForDisplay( exactValueProjections.set(valueKey, projected.output.value) } } + reportStoredDisplayProvenanceFaults('traceStore.displayProjection', context, provenanceFaults) const envelope: Record = {} for (const key of LOG_DISPLAY_CONTENT_KEYS) {