From 8e94bf88cd1b8ca441228a20d75681b9b38c8d6d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sun, 9 Aug 2026 17:27:00 -0700 Subject: [PATCH] fix(provenance): record why a resolved-secret registry became incomplete Incompleteness is one-way: once any guard trips, every later model projection in the run fails and the user is left with a single opaque sentence. Every guard could set it and none recorded which, in a file that imported no logger at all, so the cause could not be recovered after the fact. Name each guard with a static reason literal. Originating causes log at error because they permanently fail the run and error is the only level that survives every default the logger falls back to; reasons that merely carry an upstream fault forward log at warn so one fault does not read as several. The decrypt catch no longer discards its cause. No behaviour change. Reasons are static literals and the logged input path is block/field names; no resolved value is recorded. --- .../resolved-secret-trace-registry.test.ts | 186 +++++++++++++++++- .../utils/resolved-secret-trace-registry.ts | 179 ++++++++++++++--- 2 files changed, 337 insertions(+), 28 deletions(-) diff --git a/apps/sim/executor/utils/resolved-secret-trace-registry.test.ts b/apps/sim/executor/utils/resolved-secret-trace-registry.test.ts index 3967555abf9..f8906be8e12 100644 --- a/apps/sim/executor/utils/resolved-secret-trace-registry.test.ts +++ b/apps/sim/executor/utils/resolved-secret-trace-registry.test.ts @@ -1,15 +1,21 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -const { mockDecryptSecret } = vi.hoisted(() => ({ +const { mockDecryptSecret, mockLogger } = vi.hoisted(() => ({ mockDecryptSecret: vi.fn(), + mockLogger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, })) vi.mock('@/lib/core/security/encryption', () => ({ decryptSecret: mockDecryptSecret, })) +vi.mock('@sim/logger', () => ({ + createLogger: () => mockLogger, +})) + import { ANONYMOUS_SECRET_TRACE_REPLACEMENT, + createIncompleteResolvedSecretTraceRegistry, createResolvedSecretTraceRegistry, isResolvedSecretTraceProvenanceV1, RESOLVED_SECRET_TRACE_CHECKPOINT_VERSION, @@ -1279,3 +1285,181 @@ describe('ResolvedSecretTraceRegistry', () => { expect(registry.getModelEgressSnapshot()).toEqual({ complete: false }) }) }) + +describe('incompleteness diagnostics', () => { + const scope = { userId: 'user-1', workspaceId: 'workspace-1' } + + beforeEach(() => { + mockLogger.warn.mockClear() + mockLogger.error.mockClear() + }) + + it('reports an originating incompleteness at error so the default log level cannot hide it', () => { + const registry = new ResolvedSecretTraceRegistry([], scope) + + registry.markIncomplete('projection-mismatch') + + expect(mockLogger.warn).not.toHaveBeenCalled() + expect(mockLogger.error).toHaveBeenCalledWith( + 'Resolved secret registry marked incomplete', + expect.objectContaining({ reason: 'projection-mismatch' }) + ) + }) + + it('reports an inherited incompleteness at warn so one fault does not read as several', () => { + const registry = new ResolvedSecretTraceRegistry([], scope) + + registry.markIncomplete('inherited-incomplete-source') + + expect(mockLogger.error).not.toHaveBeenCalled() + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Resolved secret registry marked incomplete', + expect.objectContaining({ reason: 'inherited-incomplete-source' }) + ) + }) + + it('names the guard that tripped rather than reporting unspecified', () => { + const registry = new ResolvedSecretTraceRegistry([], scope) + + registry.recordResolved('MISSING', 'value-not-in-catalog') + + expect(mockLogger.error).toHaveBeenCalledWith( + 'Resolved secret registry marked incomplete', + expect.objectContaining({ reason: 'unverified-resolved-entry' }) + ) + }) + + it('separates a tool-call scope mismatch from a merged child that was already incomplete', () => { + const scopeMismatch = new ResolvedSecretTraceRegistry([], scope) + const foreignChild = new ResolvedSecretTraceRegistry([], { + userId: 'user-1', + workspaceId: 'workspace-2', + }) + + scopeMismatch.mergeToolCallRegistry(foreignChild) + + expect(mockLogger.error).toHaveBeenCalledWith( + 'Resolved secret registry marked incomplete', + expect.objectContaining({ reason: 'tool-call-scope-mismatch' }) + ) + + mockLogger.warn.mockClear() + mockLogger.error.mockClear() + + const sameScope = new ResolvedSecretTraceRegistry([], scope) + const incompleteChild = new ResolvedSecretTraceRegistry([], scope) + incompleteChild.markIncomplete('projection-mismatch') + mockLogger.error.mockClear() + + sameScope.mergeToolCallRegistry(incompleteChild) + + expect(mockLogger.error).not.toHaveBeenCalled() + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Resolved secret registry marked incomplete', + expect.objectContaining({ reason: 'inherited-incomplete-source' }) + ) + }) + + it('attributes an already-incomplete bundle to its source rather than to the value filter', async () => { + const registry = new ResolvedSecretTraceRegistry([], scope) + + await registry.importProvenanceForValue( + { version: 1, complete: false, entries: [], scope }, + 'x', + { + trusted: true, + inputPath: ['prompt'], + } + ) + + const reasons = mockLogger.error.mock.calls + .concat(mockLogger.warn.mock.calls) + .map(([, details]) => (details as { reason?: string })?.reason) + expect(reasons).toContain('source-provenance-incomplete') + expect(reasons).not.toContain('value-provenance-filter-incomplete') + }) + + it('reports an incoming incomplete bundle at warn, since no catalog was ever on offer', () => { + const registry = new ResolvedSecretTraceRegistry([], scope) + + registry.markIncomplete('source-provenance-incomplete') + + expect(mockLogger.error).not.toHaveBeenCalled() + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Resolved secret registry marked incomplete', + expect.objectContaining({ reason: 'source-provenance-incomplete' }) + ) + }) + + it('stays silent for a registry built incomplete by design, which sits on hot paths', () => { + createIncompleteResolvedSecretTraceRegistry(scope) + + expect(mockLogger.error).not.toHaveBeenCalled() + expect(mockLogger.warn).not.toHaveBeenCalled() + }) + + it('keeps an unaudited caller taking the default reason out of the error stream', () => { + const registry = new ResolvedSecretTraceRegistry([], scope) + + registry.markIncomplete() + + expect(mockLogger.error).not.toHaveBeenCalled() + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Resolved secret registry marked incomplete', + expect.objectContaining({ reason: 'unspecified' }) + ) + }) + + it('reports an incoming incomplete bundle exactly once, from the registry that knows the path', async () => { + const registry = new ResolvedSecretTraceRegistry([], scope) + + await registry.importProvenanceForValue( + { version: 1, complete: false, entries: [], scope }, + 'x', + { trusted: true, inputPath: ['prompt'] } + ) + + const records = mockLogger.error.mock.calls.concat(mockLogger.warn.mock.calls) + expect(records).toHaveLength(1) + expect(records[0]).toEqual([ + 'Resolved secret input path marked incomplete', + expect.objectContaining({ reason: 'source-provenance-incomplete', inputPath: 'prompt' }), + ]) + }) + + it('summarises decrypt failures once per import instead of once per entry', async () => { + mockDecryptSecret.mockRejectedValue(new Error('key rotated')) + const registry = new ResolvedSecretTraceRegistry([], scope) + + await registry.importProvenance( + { + version: 1, + complete: true, + entries: Array.from({ length: 25 }, (_, i) => ({ + name: `SECRET_${i}`, + encryptedValue: `encrypted-${i}`, + })), + scope, + }, + { trusted: true } + ) + + const decryptRecords = mockLogger.error.mock.calls.filter( + ([message]) => message === 'Provenance entries could not be decrypted' + ) + expect(decryptRecords).toHaveLength(1) + expect(decryptRecords[0][1]).toEqual( + expect.objectContaining({ failedEntryCount: 25, totalEntryCount: 25, error: 'key rotated' }) + ) + }) + + it('records no secret material alongside the reason', () => { + const registry = new ResolvedSecretTraceRegistry([], scope) + + registry.recordResolved('MISSING', 'super-secret-value') + + const logged = JSON.stringify(mockLogger.error.mock.calls) + expect(logged).not.toContain('super-secret-value') + expect(logged).not.toContain('MISSING') + }) +}) diff --git a/apps/sim/executor/utils/resolved-secret-trace-registry.ts b/apps/sim/executor/utils/resolved-secret-trace-registry.ts index 6cda63975e7..348a4fba6f9 100644 --- a/apps/sim/executor/utils/resolved-secret-trace-registry.ts +++ b/apps/sim/executor/utils/resolved-secret-trace-registry.ts @@ -1,3 +1,5 @@ +import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' import { decryptSecret } from '@/lib/core/security/encryption' import { isLargeArrayManifest } from '@/lib/execution/payloads/large-array-manifest-metadata' import { isLargeValueRef } from '@/lib/execution/payloads/large-value-ref' @@ -10,6 +12,69 @@ import { } from '@/executor/utils/resolved-secret-matcher' import { getResolvedSecretMatcherCapacityFailure } from '@/executor/utils/resolved-secret-matcher-capacity' +const logger = createLogger('ResolvedSecretTraceRegistry') + +/** + * Why a registry stopped being able to vouch for what it projects. + * + * Incompleteness is one-way and fails every later model projection in the run, surfacing to the + * user as a single opaque sentence. Recording which guard tripped is the only way to tell a + * genuine containment from a matcher that merely could not decide — the reasons are static + * literals and the logged path is block/field names, never a resolved value. + */ +type ResolvedSecretIncompletenessReason = + | 'untrusted-provenance' + | 'source-provenance-incomplete' + | 'entry-decrypt-failed' + | 'unverified-resolved-entry' + | 'projection-mismatch' + | 'unresolved-placeholder' + | 'provenance-capacity-exceeded' + | 'restored-checkpoint-unavailable' + | 'constructed-incomplete' + | 'inherited-incomplete-source' + | 'inherited-incomplete-input-path' + | 'tool-call-scope-mismatch' + | 'value-provenance-untrusted' + | 'value-provenance-import-failed' + | 'value-provenance-filter-incomplete' + | 'unspecified' + +/** + * Reasons that mean something went wrong, rather than that provenance was never on offer. + * + * These log at error: each is a guard tripping on a path that should have succeeded, none is + * reachable on a healthy run, and error is the only level surviving every default the logger falls + * back to — production, test, and a self-hosted chart that sets no `LOG_LEVEL`. + * + * Everything absent from this set logs at warn, which is the deliberate default. Incompleteness is + * also the *designed* state wherever there is no catalog to vouch with, and those paths are hot — a + * webhook execution builds an incomplete registry on every run before replacing it. Defaulting to + * warn keeps a by-design state, an upstream bundle that already declared itself incomplete, a fork + * inheriting a parent that reported moments earlier, or an unaudited caller taking the default + * reason from flooding the error stream. A reason added later without thought stays quiet. + */ +const ORIGINATING_FAULT_REASONS = new Set([ + 'untrusted-provenance', + 'entry-decrypt-failed', + 'unverified-resolved-entry', + 'projection-mismatch', + 'unresolved-placeholder', + 'provenance-capacity-exceeded', + 'tool-call-scope-mismatch', + 'value-provenance-untrusted', + 'value-provenance-import-failed', +]) + +/** + * Incompleteness that is a construction choice rather than an event: `createIncomplete…` states + * outright that no trusted catalog was available. It carries nothing a reader could act on and sits + * on hot paths, so it is not reported at all. + */ +const BY_DESIGN_INCOMPLETENESS_REASONS = new Set([ + 'constructed-incomplete', +]) + export const ANONYMOUS_SECRET_TRACE_REPLACEMENT = OPAQUE_RESOLVED_SECRET_REPLACEMENT export const RESOLVED_SECRET_TRACE_CHECKPOINT_VERSION = 1 @@ -544,15 +609,23 @@ 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. + */ + private readonly staged: boolean constructor( catalogEntries: Iterable = [], - scope?: ResolvedSecretTraceScopeV1 + scope?: ResolvedSecretTraceScopeV1, + options: { staged?: boolean } = {} ) { + this.staged = options.staged === true this.scope = scope ? cloneProvenanceScope(scope) : undefined this.completeProvenanceEnvelopeBytes = serializedProvenanceEnvelopeByteSize(true, this.scope) if (this.completeProvenanceEnvelopeBytes > MAX_SERIALIZED_PROVENANCE_BYTES) { - this.markIncomplete() + this.markIncomplete('provenance-capacity-exceeded') } let catalogEntriesSeen = 0 for (const entry of catalogEntries) { @@ -580,7 +653,7 @@ export class ResolvedSecretTraceRegistry { } this.copyResolvedInputPathsTo(fork) this.copyIncompleteInputPathsTo(fork) - if (!this.complete) fork.markIncomplete() + if (!this.complete) fork.markIncomplete('inherited-incomplete-source') return fork } @@ -591,12 +664,12 @@ export class ResolvedSecretTraceRegistry { ): ResolvedSecretTraceRegistry { const fork = new ResolvedSecretTraceRegistry(this.catalog.values(), this.scope) if (!this.complete) { - fork.markIncomplete() + fork.markIncomplete('inherited-incomplete-source') return fork } if (this.hasIncompleteInputPathOverlapping(paths)) { - fork.markIncomplete() + fork.markIncomplete('inherited-incomplete-input-path') return fork } @@ -620,14 +693,19 @@ export class ResolvedSecretTraceRegistry { fork.addActiveEntry({ ...entry }, { propagated: true }) } } - if (this.isPermanentlyIncomplete()) fork.markIncomplete() + if (this.isPermanentlyIncomplete()) fork.markIncomplete('inherited-incomplete-source') return fork } /** Merges one settled tool-call registry into the turn-scoped registry. */ mergeToolCallRegistry(child: ResolvedSecretTraceRegistry): void { - if (!scopesMatch(this.scope, child.scope) || !child.isComplete()) { - this.markIncomplete() + if (!scopesMatch(this.scope, child.scope)) { + this.markIncomplete('tool-call-scope-mismatch') + return + } + + if (!child.isComplete()) { + this.markIncomplete('inherited-incomplete-source') return } @@ -649,7 +727,7 @@ export class ResolvedSecretTraceRegistry { if (resolvedValue.length === 0) return false const entry = this.getVerifiedResolvedEntry(name, resolvedValue) if (!entry) { - this.markIncomplete() + this.markIncomplete('unverified-resolved-entry') return false } @@ -669,7 +747,7 @@ export class ResolvedSecretTraceRegistry { const entry = this.getVerifiedResolvedEntry(name, resolvedValue) if (!entry) { - this.markInputPathIncomplete(path) + this.markInputPathIncomplete(path, 'unverified-resolved-entry') return false } @@ -790,7 +868,7 @@ export class ResolvedSecretTraceRegistry { typeof projectedValue === 'string' && state.projectedValue !== projectedValue ) { - this.markInputPathIncomplete(path) + this.markInputPathIncomplete(path, 'projection-mismatch') return } for (const entryKey of entryKeys) state.entryKeys.add(entryKey) @@ -846,7 +924,7 @@ export class ResolvedSecretTraceRegistry { if (current.raw !== null && typeof current.raw === 'object') { const standaloneName = canonicalPlaceholderName(current.projected as string) if (!standaloneName || !entryKeysByName.has(standaloneName)) { - this.markInputPathIncomplete(current.path) + this.markInputPathIncomplete(current.path, 'unresolved-placeholder') return } recordProjectedMarkerAcrossRawLeaves( @@ -994,16 +1072,18 @@ export class ResolvedSecretTraceRegistry { options: ImportResolvedSecretTraceProvenanceOptions ): Promise { if (!options.trusted || !isResolvedSecretTraceProvenanceV1(provenance)) { - this.markIncomplete() + this.markIncomplete('untrusted-provenance') return false } if (!provenance.complete) { - this.markIncomplete() + this.markIncomplete('source-provenance-incomplete') } const sameScope = scopesMatch(provenance.scope, this.scope) let importedAll = true + let decryptFailures = 0 + let firstDecryptError: string | undefined for (const entry of provenance.entries) { try { const { decrypted } = await decryptSecret(entry.encryptedValue) @@ -1016,12 +1096,27 @@ export class ResolvedSecretTraceRegistry { }, { propagated: true } ) - } catch { + } catch (error) { importedAll = false - this.markIncomplete() + decryptFailures += 1 + firstDecryptError ??= getErrorMessage(error, 'Unknown error') + this.markIncomplete('entry-decrypt-failed') } } + /** + * Summarised rather than logged per entry: one rotated or corrupt key fails every entry in the + * bundle, and a bundle may carry up to MAX_PROVENANCE_ENTRIES of them. + */ + if (decryptFailures > 0) { + logger.error('Provenance entries could not be decrypted', { + error: firstDecryptError, + failedEntryCount: decryptFailures, + totalEntryCount: provenance.entries.length, + scopeWorkspaceId: this.scope?.workspaceId, + }) + } + return importedAll } @@ -1058,19 +1153,22 @@ export class ResolvedSecretTraceRegistry { options: { trusted: boolean; inputPath?: ResolvedSecretInputPath } ): Promise { if (!options.trusted || !isResolvedSecretTraceProvenanceV1(provenance)) { - this.markInputPathIncomplete(options.inputPath) + this.markInputPathIncomplete(options.inputPath, 'value-provenance-untrusted') return { success: false, matched: false } } - const sourceRegistry = new ResolvedSecretTraceRegistry([], provenance.scope) + const sourceRegistry = new ResolvedSecretTraceRegistry([], provenance.scope, { staged: true }) const sourceImported = await sourceRegistry.importProvenance(provenance, { trusted: true }) const filteredProvenance = sourceRegistry.exportProvenanceForValue(value) if (!sourceImported) { - this.markInputPathIncomplete(options.inputPath) + this.markInputPathIncomplete(options.inputPath, 'value-provenance-import-failed') return { success: false, matched: false } } if (!filteredProvenance.complete) { - this.markInputPathIncomplete(options.inputPath) + this.markInputPathIncomplete( + options.inputPath, + provenance.complete ? 'value-provenance-filter-incomplete' : 'source-provenance-incomplete' + ) return { success: true, matched: false } } const filteredImported = await this.importProvenance(filteredProvenance, { trusted: true }) @@ -1214,10 +1312,20 @@ export class ResolvedSecretTraceRegistry { return !this.complete || this.incompleteInputPaths.size > 0 } - markIncomplete(): void { + markIncomplete(reason: ResolvedSecretIncompletenessReason = 'unspecified'): void { if (!this.complete) return this.complete = false this.modelEgressRevision += 1 + if (this.staged || BY_DESIGN_INCOMPLETENESS_REASONS.has(reason)) return + const details = { + reason, + scopeWorkspaceId: this.scope?.workspaceId, + activeEntryCount: this.activeEntries.size, + incompleteInputPathCount: this.incompleteInputPaths.size, + } + const message = 'Resolved secret registry marked incomplete' + if (ORIGINATING_FAULT_REASONS.has(reason)) logger.error(message, details) + else logger.warn(message, details) } /** @@ -1388,7 +1496,11 @@ export class ResolvedSecretTraceRegistry { matcher = createResolvedSecretMatcher( [...candidatesByScanLiteral.keys()].map((plaintext) => ({ plaintext, replacement: '' })) ) - } catch { + } catch (error) { + logger.error('Provenance filter matcher could not be built', { + error: getErrorMessage(error, 'Unknown error'), + candidateCount: candidatesByScanLiteral.size, + }) return { complete: false } } @@ -1623,15 +1735,28 @@ export class ResolvedSecretTraceRegistry { ) } - private markInputPathIncomplete(path: ResolvedSecretInputPath | undefined): void { + private markInputPathIncomplete( + path: ResolvedSecretInputPath | undefined, + reason: ResolvedSecretIncompletenessReason = 'unspecified' + ): void { if (!path || path.length === 0) { - this.markIncomplete() + this.markIncomplete(reason) return } const key = inputPathKey(path) if (this.incompleteInputPaths.has(key)) return this.incompleteInputPaths.set(key, [...path]) this.modelEgressRevision += 1 + if (this.staged || BY_DESIGN_INCOMPLETENESS_REASONS.has(reason)) return + const details = { + reason, + inputPath: path.join('.'), + scopeWorkspaceId: this.scope?.workspaceId, + activeEntryCount: this.activeEntries.size, + } + const message = 'Resolved secret input path marked incomplete' + if (ORIGINATING_FAULT_REASONS.has(reason)) logger.error(message, details) + else logger.warn(message, details) } private copyIncompleteInputPathsTo( @@ -1672,7 +1797,7 @@ export class ResolvedSecretTraceRegistry { entryBytes > MAX_SERIALIZED_PROVENANCE_BYTES ) { - this.markIncomplete() + this.markIncomplete('provenance-capacity-exceeded') return } this.activeEntries.set(key, entry) @@ -1733,7 +1858,7 @@ export async function createResolvedSecretTraceRegistry( options.restoreTrusted === true && options.restoredCheckpointVersion !== undefined ) { - registry.markIncomplete() + registry.markIncomplete('restored-checkpoint-unavailable') } return registry @@ -1744,6 +1869,6 @@ export function createIncompleteResolvedSecretTraceRegistry( scope?: ResolvedSecretTraceScopeV1 ): ResolvedSecretTraceRegistry { const registry = new ResolvedSecretTraceRegistry([], scope) - registry.markIncomplete() + registry.markIncomplete('constructed-incomplete') return registry }