Skip to content

Commit 785c619

Browse files
improvement(provenance): name the block behind an unprojected input root (#6890)
* fix(provenance): name the block behind an unprojected input root 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. * 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.
1 parent f5728fa commit 785c619

3 files changed

Lines changed: 116 additions & 7 deletions

File tree

apps/sim/executor/handlers/generic/generic-handler.ts

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,16 @@ export class GenericBlockHandler implements BlockHandler {
185185
try {
186186
finalInputs[key] = JSON.parse(value.trim())
187187
} catch (error) {
188+
/**
189+
* The failure class, not the thrown message. This parses a resolved input, so the
190+
* string may be a secret, and V8 quotes the text it rejected back into the
191+
* message — `Unexpected token 's', "sk-live-EX"... is not valid JSON`. That
192+
* prefix is enough to leak. The field name and its declared type are already in
193+
* the message above, and `SyntaxError` is the only class `JSON.parse` throws, so
194+
* nothing diagnostic is lost.
195+
*/
188196
logger.warn(`Failed to parse ${inputType} field "${key}":`, {
189-
error: toError(error).message,
197+
error: toError(error).name,
190198
})
191199
}
192200
}
@@ -199,8 +207,11 @@ export class GenericBlockHandler implements BlockHandler {
199207
boundary && boundary.paths.length > 0 && registry?.hasResolvedInputProjections()
200208
? registry.projectResolvedInputSelections(inputs)
201209
: undefined
202-
if (projectedInputs?.complete === false)
203-
registry?.markIncomplete('structural-input-projection-incomplete')
210+
if (projectedInputs?.complete === false) {
211+
registry?.markIncomplete('structural-input-projection-incomplete', {
212+
detail: { blockType, ...(tool ? { tool: tool.id } : {}) },
213+
})
214+
}
204215

205216
if (projectedInputs?.complete && boundary && tool && registry) {
206217
for (const projection of projectedInputs.values) {
@@ -220,7 +231,7 @@ export class GenericBlockHandler implements BlockHandler {
220231
...blockConfig.tools.config.params(projectedFinalInputs),
221232
}
222233
}
223-
} catch {
234+
} catch (error) {
224235
const structuredProjection = createStructuredModelProjection(
225236
tool,
226237
finalInputs,
@@ -234,7 +245,21 @@ export class GenericBlockHandler implements BlockHandler {
234245
continue
235246
}
236247
if (boundary.requiredProjectionRoots.has(projection.path[0])) {
237-
registry.markIncomplete('structural-input-root-unprojected')
248+
/**
249+
* `config.params` threw on the projected inputs — the copy where a secret has been
250+
* replaced by its placeholder — and no structured projection could recover it. The
251+
* reason alone said only that this happened somewhere, which is not enough to find
252+
* the block. The failure class rather than the thrown message, because a coercion
253+
* that rejects a value tends to quote it, and this input may hold a secret.
254+
*/
255+
registry.markIncomplete('structural-input-root-unprojected', {
256+
detail: {
257+
blockType,
258+
tool: tool.id,
259+
inputPath: projection.path.join('.'),
260+
failure: toError(error).name,
261+
},
262+
})
238263
}
239264
continue
240265
}

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1574,6 +1574,57 @@ describe('incompleteness diagnostics', () => {
15741574
)
15751575
})
15761576

1577+
/**
1578+
* `reason` says what tripped; without this the line says nothing about where, which is the
1579+
* difference between a signal you can act on and one you can only count.
1580+
*/
1581+
it('carries a caller-supplied structural detail onto the reported line', () => {
1582+
const registry = new ResolvedSecretTraceRegistry([], scope)
1583+
1584+
registry.markIncomplete('structural-input-root-unprojected', {
1585+
detail: { blockType: 'api', tool: 'http_request', inputPath: 'body.payload' },
1586+
})
1587+
1588+
expect(mockLogger.error).toHaveBeenCalledWith(
1589+
'Resolved secret registry marked incomplete',
1590+
expect.objectContaining({
1591+
reason: 'structural-input-root-unprojected',
1592+
blockType: 'api',
1593+
tool: 'http_request',
1594+
inputPath: 'body.payload',
1595+
})
1596+
)
1597+
})
1598+
1599+
/** A detail key must never displace the fields every one of these lines is read by. */
1600+
/**
1601+
* The detail type names its fields, so none of these is expressible without a cast. The runtime
1602+
* guarantee is asserted anyway because the payload is assembled in two places — `reason` is
1603+
* added a level above, where the caller's spread order cannot reach it — and a line whose
1604+
* `reason` disagrees with the level it was logged at is worse than one carrying no detail.
1605+
*/
1606+
it('does not let a detail shadow the canonical fields', () => {
1607+
const registry = new ResolvedSecretTraceRegistry([], scope)
1608+
1609+
registry.markIncomplete('structural-input-root-unprojected', {
1610+
detail: {
1611+
reason: 'spoofed',
1612+
origin: 'spoofed',
1613+
scopeWorkspaceId: 'spoofed',
1614+
activeEntryCount: 'spoofed',
1615+
} as never,
1616+
})
1617+
1618+
expect(mockLogger.error).toHaveBeenCalledWith(
1619+
'Resolved secret registry marked incomplete',
1620+
expect.objectContaining({
1621+
reason: 'structural-input-root-unprojected',
1622+
scopeWorkspaceId: 'workspace-1',
1623+
activeEntryCount: 0,
1624+
})
1625+
)
1626+
})
1627+
15771628
it('names the guard that tripped rather than reporting unspecified', () => {
15781629
const registry = new ResolvedSecretTraceRegistry([], scope)
15791630

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

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,13 @@ function reportIncompleteness(
141141
details: Record<string, unknown>
142142
): void {
143143
if (BY_DESIGN_INCOMPLETENESS_REASONS.has(reason)) return
144-
if (ORIGINATING_FAULT_REASONS.has(reason)) logger.error(message, { reason, ...details })
145-
else logger.warn(message, { reason, ...details })
144+
/**
145+
* `reason` is written last so no detail can displace it. It is the field these lines are
146+
* queried and alerted on, and it also selects the level above — a payload whose `reason` says
147+
* one thing while the level was chosen from another is worse than no detail at all.
148+
*/
149+
if (ORIGINATING_FAULT_REASONS.has(reason)) logger.error(message, { ...details, reason })
150+
else logger.warn(message, { ...details, reason })
146151
}
147152

148153
/**
@@ -300,6 +305,32 @@ interface MarkIncompleteContext {
300305
* production latch naming no guard at all.
301306
*/
302307
origin?: string
308+
detail?: MarkIncompleteDetail
309+
}
310+
311+
/**
312+
* Structural facts locating where a guard tripped. `reason` says what went wrong and this says
313+
* where, which is the difference between a line you can act on and one you can only count.
314+
*
315+
* Named fields rather than an open record, for the reason `reason` itself is a closed union: a
316+
* shape a caller can extend freely cannot be aggregated, and — because these merge into the
317+
* reported payload — an open record also lets a caller land a key that a reader takes to mean
318+
* something else, `origin` and `reason` being the two that carry the most weight here.
319+
*
320+
* Names and types only — never a value, and never a caught error's message. Code that throws while
321+
* coercing an input routinely quotes that input back (`JSON.parse` names the text it rejected), and
322+
* an input reaching one of these guards may still hold a resolved secret. That is the same promise
323+
* `reason` already makes about this log, restated where it is easy to break.
324+
*/
325+
interface MarkIncompleteDetail {
326+
/** Block type id, e.g. `api`. */
327+
blockType?: string
328+
/** Tool id, e.g. `http_request`. */
329+
tool?: string
330+
/** Dotted input path within the block's inputs, e.g. `body.payload`. */
331+
inputPath?: string
332+
/** Error class only, e.g. `SyntaxError` — never the thrown message. */
333+
failure?: string
303334
}
304335

305336
export interface ImportResolvedSecretTraceProvenanceOptions {
@@ -1817,6 +1848,8 @@ export class ResolvedSecretTraceRegistry {
18171848
this.modelEgressRevision += 1
18181849
if (this.staged) return
18191850
reportIncompleteness('Resolved secret registry marked incomplete', reason, {
1851+
/** Spread first so a caller's detail can never shadow the fields every line is read by. */
1852+
...(context.detail ?? {}),
18201853
...(context.origin ? { origin: context.origin } : {}),
18211854
scopeWorkspaceId: this.scope?.workspaceId,
18221855
activeEntryCount: this.activeEntries.size,

0 commit comments

Comments
 (0)