diff --git a/apps/sim/executor/handlers/generic/generic-handler.test.ts b/apps/sim/executor/handlers/generic/generic-handler.test.ts index 660d1b67d2d..2c3dd174f78 100644 --- a/apps/sim/executor/handlers/generic/generic-handler.test.ts +++ b/apps/sim/executor/handlers/generic/generic-handler.test.ts @@ -105,6 +105,55 @@ describe('GenericBlockHandler', () => { expect(result).toEqual(expectedOutput) }) + /** + * `table_insert_row` posts row data to an internal API and declares no `modelInput` — nothing on + * that path reaches a model, and its provenance travels in the private bundle. Marking its + * `secretProvenance` roots as required-to-project made a projection failure fatal for a tool with + * no way to project, and the Table block's `parseJSON` throws once a placeholder stands where the + * JSON object was. The whole run's registry latched, costing provenance for every later boundary + * including the table write that prompted it. + */ + it('keeps vouching when a bundle-only tool cannot project and its block params throw', async () => { + mockTool.request.secretProvenance = { + request: () => [{ key: '0', inputPaths: [['data', 'apiKey']] }], + response: { incomplete: 'propagate' }, + } as never + mockGetBlock.mockReturnValue({ + tools: { + access: ['some_custom_tool'], + config: { + tool: () => 'some_custom_tool', + params: (params: Record) => { + /** + * Throws only on the projected copy. Real blocks reach this by validating or parsing a + * field a placeholder now sits in — the Table block runs `parseJSON` over `data` — and + * which shape breaks does not matter to the invariant under test. + */ + if (typeof params.data === 'string' && params.data.includes('{{')) { + throw new Error('cannot coerce a projected input') + } + return { data: params.data } + }, + }, + }, + inputs: { data: { type: 'json', description: 'Row data' } }, + } as never) + + /** Valid JSON, so the block's first `params` call over the real inputs succeeds. */ + const rowJson = '{"apiKey":"x"}' + const registry = new ResolvedSecretTraceRegistry([ + { name: 'ROW_SECRET', plaintext: rowJson, encryptedValue: 'encrypted-row-secret' }, + ]) + registry.recordResolvedAtInputPath('ROW_SECRET', rowJson, ['data']) + registry.recordResolvedInputProjection(['data'], rowJson, '{{ROW_SECRET}}') + mockContext.resolvedSecretTraceRegistry = registry + + await handler.execute(mockContext, mockBlock, { data: rowJson }) + + expect(registry.isComplete()).toBe(true) + expect(registry.getIncompletenessDiagnostics()).toBeUndefined() + }) + it('preserves exact secret provenance when block params rename a selected input', async () => { mockTool.request.modelInput = { mode: 'private-provenance', diff --git a/apps/sim/executor/handlers/generic/generic-handler.ts b/apps/sim/executor/handlers/generic/generic-handler.ts index dc7d5d28031..f8dcbb42aaa 100644 --- a/apps/sim/executor/handlers/generic/generic-handler.ts +++ b/apps/sim/executor/handlers/generic/generic-handler.ts @@ -47,11 +47,25 @@ function selectBlockBoundaryPaths( if (path[0]) requiredProjectionRoots.add(path[0]) } } + /** + * Tracked, but never required to project. + * + * A `secretProvenance` selection is the opposite mechanism to a projected model input: the + * value travels to an internal API unchanged, with its provenance alongside it in the private + * bundle, precisely so nothing has to be substituted. `table_insert_row` posts row data to the + * table API and declares no `modelInput` at all — there is no model egress on that path. + * + * Requiring those roots anyway made a projection failure fatal for tools that have no way to + * project: `createStructuredModelProjection` rescues only a `mode: 'project'` tool with an + * `applyProjected`, so for the twenty-odd `secretProvenance`-only tools it returns undefined on + * its first check. The Table block's `params` runs `parseJSON` on the projected `data` string, + * which throws once a placeholder stands where the JSON was, and the whole run's registry + * latched — costing provenance for every later boundary, including the table write itself. + * + * A root is required to project when a model will see it, which is what `modelInput` declares. + */ for (const selection of tool.request.secretProvenance?.request?.(params) ?? []) { paths.push(...selection.inputPaths) - for (const path of selection.inputPaths) { - if (path[0]) requiredProjectionRoots.add(path[0]) - } } const uniquePaths = new Map() diff --git a/apps/sim/executor/utils/resolved-secret-projection-refusal.test.ts b/apps/sim/executor/utils/resolved-secret-projection-refusal.test.ts index 16924a314dd..0dc4cdf3b92 100644 --- a/apps/sim/executor/utils/resolved-secret-projection-refusal.test.ts +++ b/apps/sim/executor/utils/resolved-secret-projection-refusal.test.ts @@ -79,6 +79,34 @@ describe('refuseResolvedSecretProjection', () => { ) }) + /** + * The reason says what tripped; without the location a reader still has to go hunting for which + * block. Nested rather than flattened because the guard's own `inputPath` and the refusal's name + * different places once a latch has travelled. + */ + it('reports where the first guard tripped, not only what it was', () => { + const registry = new ResolvedSecretTraceRegistry([], scope) + registry.markIncomplete('structural-input-root-unprojected', { + detail: { blockType: 'table', tool: 'table_insert_row', inputPath: 'data' }, + }) + + expect(() => + refuseResolvedSecretProjection({ + site: 'agent.toolCallCrossing', + message: 'Tool call could not be safely projected', + registry, + inputPath: 'messages', + }) + ).toThrow() + + expect(refusalRecords()[0][1]).toEqual( + expect.objectContaining({ + inputPath: 'messages', + detail: { blockType: 'table', tool: 'table_insert_row', inputPath: 'data' }, + }) + ) + }) + it('names a by-design origin that was silenced when it was marked', () => { const registry = createIncompleteResolvedSecretTraceRegistry(scope) expect(mockLogger.error).not.toHaveBeenCalled() diff --git a/apps/sim/executor/utils/resolved-secret-projection-refusal.ts b/apps/sim/executor/utils/resolved-secret-projection-refusal.ts index 24a573225ea..a11184c333b 100644 --- a/apps/sim/executor/utils/resolved-secret-projection-refusal.ts +++ b/apps/sim/executor/utils/resolved-secret-projection-refusal.ts @@ -71,6 +71,16 @@ function reportRefusal({ site, registry, inputPath }: ResolvedSecretProjectionRe ...(diagnostics.scopeWorkspaceId ? { scopeWorkspaceId: diagnostics.scopeWorkspaceId } : {}), + /** + * Where the first guard tripped — the block, tool and input path that cost the run its + * completeness, which is what a reader needs to go and fix. + * + * Nested rather than spread flat: its `inputPath` names where the guard tripped, while + * this line's own `inputPath` names where the refusal happened. Those are different + * places whenever a latch travels, and flattening would silently overwrite one with the + * other. + */ + ...(diagnostics.detail ? { detail: diagnostics.detail } : {}), } : {}), }) 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 bdc9a301cdd..99fa2953ea3 100644 --- a/apps/sim/executor/utils/resolved-secret-trace-registry.test.ts +++ b/apps/sim/executor/utils/resolved-secret-trace-registry.test.ts @@ -1625,6 +1625,56 @@ describe('incompleteness diagnostics', () => { ) }) + /** + * A run that failed before producing provenance hands the crossing `undefined`. That is the + * expected shape of a failed crossing, not a guard catching something wrong, so it reports at + * warn under its own name instead of joining the originating faults as a would-be breach. + */ + it('separates a crossing that carried no provenance from one that was rejected', async () => { + const absent = new ResolvedSecretTraceRegistry([], scope) + await absent.importCrossingProvenance(undefined, 'value', { + trusted: true, + origin: 'someSurface.failedRunCrossing', + }) + + expect(mockLogger.error).not.toHaveBeenCalled() + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ reason: 'value-provenance-absent' }) + ) + }) + + it('still reports a rejected crossing as a fault', async () => { + const rejected = new ResolvedSecretTraceRegistry([], scope) + await rejected.importCrossingProvenance({ not: 'a bundle' }, 'value', { trusted: true }) + + expect(mockLogger.error).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ reason: 'value-provenance-untrusted' }) + ) + }) + + /** + * A refusal is usually frames from its cause, which is what this struct exists to bridge — but it + * carried only *what* went wrong, so a downstream reporter printed a reason with no location. + */ + it('carries the first guard location through to the diagnostics a refusal reports', () => { + const registry = new ResolvedSecretTraceRegistry([], scope) + + registry.markIncomplete('structural-input-root-unprojected', { + detail: { blockType: 'table', tool: 'table_insert_row', inputPath: 'data' }, + }) + registry.markIncomplete('inherited-incomplete-source', { + detail: { blockType: 'later', tool: 'later_tool' }, + }) + + expect(registry.getIncompletenessDiagnostics()?.detail).toEqual({ + blockType: 'table', + tool: 'table_insert_row', + inputPath: 'data', + }) + }) + it('names the guard that tripped rather than reporting unspecified', () => { const registry = new ResolvedSecretTraceRegistry([], scope) diff --git a/apps/sim/executor/utils/resolved-secret-trace-registry.ts b/apps/sim/executor/utils/resolved-secret-trace-registry.ts index 0c496745d5b..c45fe035f2f 100644 --- a/apps/sim/executor/utils/resolved-secret-trace-registry.ts +++ b/apps/sim/executor/utils/resolved-secret-trace-registry.ts @@ -41,6 +41,11 @@ export type ResolvedSecretIncompletenessReason = | 'inherited-incomplete-input-path' | 'tool-call-scope-mismatch' | 'value-provenance-untrusted' + /** + * A crossing carried no provenance at all, which is what a run that failed before producing any + * looks like. Distinct from `untrusted`: nothing was rejected, there was nothing to reject. + */ + | 'value-provenance-absent' | 'value-provenance-import-failed' | 'value-provenance-filter-incomplete' | 'durable-provenance-unknown' @@ -174,6 +179,14 @@ export interface ResolvedSecretIncompletenessDiagnostics { readonly activeEntryCount: number /** Correlates a refusal with the guard that caused it; never carries user or secret material. */ readonly scopeWorkspaceId?: string + /** + * Where the first guard tripped, carried alongside the reason that named it. + * + * A refusal is often frames away from its cause, which is why this struct exists — but it only + * ever carried *what* went wrong, so a downstream reporter printed a reason with no location and + * a reader had to join to the registry's own line to find the block. + */ + readonly detail?: MarkIncompleteDetail } export const ANONYMOUS_SECRET_TRACE_REPLACEMENT = OPAQUE_RESOLVED_SECRET_REPLACEMENT @@ -322,7 +335,7 @@ interface MarkIncompleteContext { * an input reaching one of these guards may still hold a resolved secret. That is the same promise * `reason` already makes about this log, restated where it is easy to break. */ -interface MarkIncompleteDetail { +export interface MarkIncompleteDetail { /** Block type id, e.g. `api`. */ blockType?: string /** Tool id, e.g. `http_request`. */ @@ -854,6 +867,8 @@ export class ResolvedSecretTraceRegistry { private readonly incompletenessReasons = new Set() /** Import callers that cost this registry its completeness; bounded by {@link MAX_RETAINED_ORIGINS}. */ private readonly incompletenessOrigins = new Set() + /** First guard's location; later ones describe propagation, not the cause. */ + private incompletenessDetail: MarkIncompleteDetail | undefined private activeProvenanceEntryBytes = 0 private complete = true private pendingActivations = 0 @@ -1586,8 +1601,18 @@ export class ResolvedSecretTraceRegistry { value: unknown, options: { trusted: boolean; inputPath?: ResolvedSecretInputPath; origin?: string } ): Promise { + /** + * Absence and distrust are different facts and are reported as such. A run that failed before + * producing provenance hands this `undefined`, which is the expected shape of a failed + * crossing, not a guard catching something wrong — reporting it as a fault put a recurring + * by-design state at error level with a name that reads like a breach. + */ if (!options.trusted || !isResolvedSecretTraceProvenanceV1(provenance)) { - this.markInputPathIncomplete(options.inputPath, 'value-provenance-untrusted', options.origin) + const reason = + options.trusted && provenance === undefined + ? 'value-provenance-absent' + : 'value-provenance-untrusted' + this.markInputPathIncomplete(options.inputPath, reason, options.origin) return { success: false, matched: false } } @@ -1802,6 +1827,7 @@ export class ResolvedSecretTraceRegistry { incompleteInputPathCount: this.incompleteInputPaths.size, activeEntryCount: this.activeEntries.size, ...(this.scope?.workspaceId ? { scopeWorkspaceId: this.scope.workspaceId } : {}), + ...(this.incompletenessDetail ? { detail: this.incompletenessDetail } : {}), } } @@ -1823,6 +1849,7 @@ export class ResolvedSecretTraceRegistry { private inheritIncompletenessReasonsFrom(source: ResolvedSecretTraceRegistry): void { for (const reason of source.incompletenessReasons) this.recordIncompletenessReason(reason) for (const origin of source.incompletenessOrigins) this.recordIncompletenessOrigin(origin) + this.incompletenessDetail ??= source.incompletenessDetail } isPermanentlyIncomplete(): boolean { @@ -1843,6 +1870,7 @@ export class ResolvedSecretTraceRegistry { if (context.source) this.inheritIncompletenessReasonsFrom(context.source) this.recordIncompletenessReason(reason) if (context.origin) this.recordIncompletenessOrigin(context.origin) + this.incompletenessDetail ??= context.detail if (!this.complete) return this.complete = false this.modelEgressRevision += 1