From a5dc83a0a429e18daf603e672e8684229eeae2ed Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 20 Aug 2026 10:24:36 -0700 Subject: [PATCH 1/2] fix(provenance): name the block behind an unprojected input root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit structural-input-root-unprojected fires when a block's config.params throws on the projected inputs — the copy where a secret has been replaced by its placeholder — and no structured projection recovers it. It was reported with the reason and nothing else, so a line told you this had happened somewhere without naming the block, and the caught error was discarded by a bare catch. markIncomplete now takes a structural detail, and this guard passes the block type, tool, input path, and failure class. Names and types only. A coercion that rejects a value tends to quote it, and an input reaching this guard may still hold a resolved secret. Which is also why the json-parse warning a few lines above no longer logs the thrown message: V8 quotes the text it rejected back into it — Unexpected token 's', "sk-live-EX"... is not valid JSON — and that prefix is enough to leak. The field name and its declared type are already in the message, and SyntaxError is the only class JSON.parse throws. --- .../handlers/generic/generic-handler.ts | 35 +++++++++++++++--- .../resolved-secret-trace-registry.test.ts | 36 +++++++++++++++++++ .../utils/resolved-secret-trace-registry.ts | 13 +++++++ 3 files changed, 79 insertions(+), 5 deletions(-) diff --git a/apps/sim/executor/handlers/generic/generic-handler.ts b/apps/sim/executor/handlers/generic/generic-handler.ts index 2e713b0fddf..ad549e278d8 100644 --- a/apps/sim/executor/handlers/generic/generic-handler.ts +++ b/apps/sim/executor/handlers/generic/generic-handler.ts @@ -185,8 +185,16 @@ export class GenericBlockHandler implements BlockHandler { try { finalInputs[key] = JSON.parse(value.trim()) } catch (error) { + /** + * The failure class, not the thrown message. This parses a resolved input, so the + * string may be a secret, and V8 quotes the text it rejected back into the + * message — `Unexpected token 's', "sk-live-EX"... is not valid JSON`. That + * prefix is enough to leak. The field name and its declared type are already in + * the message above, and `SyntaxError` is the only class `JSON.parse` throws, so + * nothing diagnostic is lost. + */ logger.warn(`Failed to parse ${inputType} field "${key}":`, { - error: toError(error).message, + error: toError(error).name, }) } } @@ -199,8 +207,11 @@ export class GenericBlockHandler implements BlockHandler { boundary && boundary.paths.length > 0 && registry?.hasResolvedInputProjections() ? registry.projectResolvedInputSelections(inputs) : undefined - if (projectedInputs?.complete === false) - registry?.markIncomplete('structural-input-projection-incomplete') + if (projectedInputs?.complete === false) { + registry?.markIncomplete('structural-input-projection-incomplete', { + detail: { blockType, ...(tool ? { tool: tool.id } : {}) }, + }) + } if (projectedInputs?.complete && boundary && tool && registry) { for (const projection of projectedInputs.values) { @@ -220,7 +231,7 @@ export class GenericBlockHandler implements BlockHandler { ...blockConfig.tools.config.params(projectedFinalInputs), } } - } catch { + } catch (error) { const structuredProjection = createStructuredModelProjection( tool, finalInputs, @@ -234,7 +245,21 @@ export class GenericBlockHandler implements BlockHandler { continue } if (boundary.requiredProjectionRoots.has(projection.path[0])) { - registry.markIncomplete('structural-input-root-unprojected') + /** + * `config.params` threw on the projected inputs — the copy where a secret has been + * replaced by its placeholder — and no structured projection could recover it. The + * reason alone said only that this happened somewhere, which is not enough to find + * the block. The failure class rather than the thrown message, because a coercion + * that rejects a value tends to quote it, and this input may hold a secret. + */ + registry.markIncomplete('structural-input-root-unprojected', { + detail: { + blockType, + tool: tool.id, + inputPath: projection.path.join('.'), + failure: toError(error).name, + }, + }) } continue } 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 f79ad9c7290..abb726efc57 100644 --- a/apps/sim/executor/utils/resolved-secret-trace-registry.test.ts +++ b/apps/sim/executor/utils/resolved-secret-trace-registry.test.ts @@ -1574,6 +1574,42 @@ describe('incompleteness diagnostics', () => { ) }) + /** + * `reason` says what tripped; without this the line says nothing about where, which is the + * difference between a signal you can act on and one you can only count. + */ + it('carries a caller-supplied structural detail onto the reported line', () => { + const registry = new ResolvedSecretTraceRegistry([], scope) + + registry.markIncomplete('structural-input-root-unprojected', { + detail: { blockType: 'api', tool: 'http_request', inputPath: 'body.payload' }, + }) + + expect(mockLogger.error).toHaveBeenCalledWith( + 'Resolved secret registry marked incomplete', + expect.objectContaining({ + reason: 'structural-input-root-unprojected', + blockType: 'api', + tool: 'http_request', + inputPath: 'body.payload', + }) + ) + }) + + /** A detail key must never displace the fields every one of these lines is read by. */ + it('does not let a detail shadow the canonical fields', () => { + const registry = new ResolvedSecretTraceRegistry([], scope) + + registry.markIncomplete('structural-input-root-unprojected', { + detail: { scopeWorkspaceId: 'spoofed', activeEntryCount: 'spoofed' }, + }) + + expect(mockLogger.error).toHaveBeenCalledWith( + 'Resolved secret registry marked incomplete', + expect.objectContaining({ scopeWorkspaceId: 'workspace-1', activeEntryCount: 0 }) + ) + }) + 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 30abd837e86..ecd797c87f6 100644 --- a/apps/sim/executor/utils/resolved-secret-trace-registry.ts +++ b/apps/sim/executor/utils/resolved-secret-trace-registry.ts @@ -300,6 +300,17 @@ interface MarkIncompleteContext { * production latch naming no guard at all. */ origin?: string + /** + * Structural facts locating where a guard tripped: the block, the tool, the input path, the class + * of failure. `reason` says what went wrong and this says where, which is the difference between + * a line you can act on and one you can only count. + * + * Names and types only — never a value, and never a caught error's message. Code that throws + * while coercing an input routinely quotes that input back (`JSON.parse` names the text it + * rejected), and 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. + */ + detail?: Record } export interface ImportResolvedSecretTraceProvenanceOptions { @@ -1817,6 +1828,8 @@ export class ResolvedSecretTraceRegistry { this.modelEgressRevision += 1 if (this.staged) return reportIncompleteness('Resolved secret registry marked incomplete', reason, { + /** Spread first so a caller's detail can never shadow the fields every line is read by. */ + ...(context.detail ?? {}), ...(context.origin ? { origin: context.origin } : {}), scopeWorkspaceId: this.scope?.workspaceId, activeEntryCount: this.activeEntries.size, From 672d441aa151d84584d16c4c437cc83b3304cf4f Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 20 Aug 2026 10:34:59 -0700 Subject: [PATCH 2/2] fix(provenance): keep a detail from displacing the reason it explains The detail merged into the incompleteness payload could shadow `reason`. Spreading it first at the call site protected only the fields added there; `reason` is added a level up in reportIncompleteness, which built `{ reason, ...details }`, so a detail carrying that key replaced the guard literal on the line while the level was still selected from the real one. `origin` was reachable the same way whenever no importer origin was set. Write `reason` last, which protects every caller of that reporter rather than the one that prompted this, and close the detail to named fields so neither key is expressible without a cast. --- .../resolved-secret-trace-registry.test.ts | 19 +++++++- .../utils/resolved-secret-trace-registry.ts | 46 +++++++++++++------ 2 files changed, 50 insertions(+), 15 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 abb726efc57..bdc9a301cdd 100644 --- a/apps/sim/executor/utils/resolved-secret-trace-registry.test.ts +++ b/apps/sim/executor/utils/resolved-secret-trace-registry.test.ts @@ -1597,16 +1597,31 @@ describe('incompleteness diagnostics', () => { }) /** A detail key must never displace the fields every one of these lines is read by. */ + /** + * The detail type names its fields, so none of these is expressible without a cast. The runtime + * guarantee is asserted anyway because the payload is assembled in two places — `reason` is + * added a level above, where the caller's spread order cannot reach it — and a line whose + * `reason` disagrees with the level it was logged at is worse than one carrying no detail. + */ it('does not let a detail shadow the canonical fields', () => { const registry = new ResolvedSecretTraceRegistry([], scope) registry.markIncomplete('structural-input-root-unprojected', { - detail: { scopeWorkspaceId: 'spoofed', activeEntryCount: 'spoofed' }, + detail: { + reason: 'spoofed', + origin: 'spoofed', + scopeWorkspaceId: 'spoofed', + activeEntryCount: 'spoofed', + } as never, }) expect(mockLogger.error).toHaveBeenCalledWith( 'Resolved secret registry marked incomplete', - expect.objectContaining({ scopeWorkspaceId: 'workspace-1', activeEntryCount: 0 }) + expect.objectContaining({ + reason: 'structural-input-root-unprojected', + scopeWorkspaceId: 'workspace-1', + activeEntryCount: 0, + }) ) }) diff --git a/apps/sim/executor/utils/resolved-secret-trace-registry.ts b/apps/sim/executor/utils/resolved-secret-trace-registry.ts index ecd797c87f6..0c496745d5b 100644 --- a/apps/sim/executor/utils/resolved-secret-trace-registry.ts +++ b/apps/sim/executor/utils/resolved-secret-trace-registry.ts @@ -141,8 +141,13 @@ function reportIncompleteness( details: Record ): void { if (BY_DESIGN_INCOMPLETENESS_REASONS.has(reason)) return - if (ORIGINATING_FAULT_REASONS.has(reason)) logger.error(message, { reason, ...details }) - else logger.warn(message, { reason, ...details }) + /** + * `reason` is written last so no detail can displace it. It is the field these lines are + * queried and alerted on, and it also selects the level above — a payload whose `reason` says + * one thing while the level was chosen from another is worse than no detail at all. + */ + if (ORIGINATING_FAULT_REASONS.has(reason)) logger.error(message, { ...details, reason }) + else logger.warn(message, { ...details, reason }) } /** @@ -300,17 +305,32 @@ interface MarkIncompleteContext { * production latch naming no guard at all. */ origin?: string - /** - * Structural facts locating where a guard tripped: the block, the tool, the input path, the class - * of failure. `reason` says what went wrong and this says where, which is the difference between - * a line you can act on and one you can only count. - * - * Names and types only — never a value, and never a caught error's message. Code that throws - * while coercing an input routinely quotes that input back (`JSON.parse` names the text it - * rejected), and 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. - */ - detail?: Record + detail?: MarkIncompleteDetail +} + +/** + * Structural facts locating where a guard tripped. `reason` says what went wrong and this says + * where, which is the difference between a line you can act on and one you can only count. + * + * Named fields rather than an open record, for the reason `reason` itself is a closed union: a + * shape a caller can extend freely cannot be aggregated, and — because these merge into the + * reported payload — an open record also lets a caller land a key that a reader takes to mean + * something else, `origin` and `reason` being the two that carry the most weight here. + * + * Names and types only — never a value, and never a caught error's message. Code that throws while + * coercing an input routinely quotes that input back (`JSON.parse` names the text it rejected), and + * 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 { + /** Block type id, e.g. `api`. */ + blockType?: string + /** Tool id, e.g. `http_request`. */ + tool?: string + /** Dotted input path within the block's inputs, e.g. `body.payload`. */ + inputPath?: string + /** Error class only, e.g. `SyntaxError` — never the thrown message. */ + failure?: string } export interface ImportResolvedSecretTraceProvenanceOptions {