Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 30 additions & 5 deletions apps/sim/executor/handlers/generic/generic-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
}
}
Expand All @@ -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) {
Expand All @@ -220,7 +231,7 @@ export class GenericBlockHandler implements BlockHandler {
...blockConfig.tools.config.params(projectedFinalInputs),
}
}
} catch {
} catch (error) {
const structuredProjection = createStructuredModelProjection(
tool,
finalInputs,
Expand All @@ -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
}
Expand Down
51 changes: 51 additions & 0 deletions apps/sim/executor/utils/resolved-secret-trace-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1574,6 +1574,57 @@ 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. */
/**
* 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: {
reason: 'spoofed',
origin: 'spoofed',
scopeWorkspaceId: 'spoofed',
activeEntryCount: 'spoofed',
} as never,
})

expect(mockLogger.error).toHaveBeenCalledWith(
'Resolved secret registry marked incomplete',
expect.objectContaining({
reason: 'structural-input-root-unprojected',
scopeWorkspaceId: 'workspace-1',
activeEntryCount: 0,
})
)
})

it('names the guard that tripped rather than reporting unspecified', () => {
const registry = new ResolvedSecretTraceRegistry([], scope)

Expand Down
37 changes: 35 additions & 2 deletions apps/sim/executor/utils/resolved-secret-trace-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,13 @@ function reportIncompleteness(
details: Record<string, unknown>
): 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 })
}

/**
Expand Down Expand Up @@ -300,6 +305,32 @@ interface MarkIncompleteContext {
* production latch naming no guard at all.
*/
origin?: string
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 {
Expand Down Expand Up @@ -1817,6 +1848,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 ?? {}),
Comment thread
icecrasher321 marked this conversation as resolved.
...(context.origin ? { origin: context.origin } : {}),
scopeWorkspaceId: this.scope?.workspaceId,
activeEntryCount: this.activeEntries.size,
Expand Down
Loading