Skip to content

Commit f83c491

Browse files
committed
fix(provenance): stop requiring a projection of roots no model sees
A secretProvenance selection is the opposite mechanism to a projected model input: the value reaches an internal API unchanged, with its provenance alongside it in the private bundle, precisely so nothing has to be substituted. selectBlockBoundaryPaths marked those roots required to project anyway. That made a projection failure fatal for tools with 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. table_insert_row declares no modelInput at all and posts row data to the table API; when the Table block's params threw on its projected data the whole run's registry latched, costing provenance for every later boundary — including the table write that prompted it, whose rows were then stored unknown and re-reported on every later read. Track those paths, require none of them. A root is required to project when a model will see it, which is what modelInput declares. Also separate a crossing that carried no provenance from one that was rejected, so a run that failed before producing any stops reporting a by-design state as an originating fault; and carry the first guard's location into the diagnostics a refusal reports, so a downstream reporter names where rather than only what.
1 parent 6a5e250 commit f83c491

4 files changed

Lines changed: 146 additions & 5 deletions

File tree

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,55 @@ describe('GenericBlockHandler', () => {
105105
expect(result).toEqual(expectedOutput)
106106
})
107107

108+
/**
109+
* `table_insert_row` posts row data to an internal API and declares no `modelInput` — nothing on
110+
* that path reaches a model, and its provenance travels in the private bundle. Marking its
111+
* `secretProvenance` roots as required-to-project made a projection failure fatal for a tool with
112+
* no way to project, and the Table block's `parseJSON` throws once a placeholder stands where the
113+
* JSON object was. The whole run's registry latched, costing provenance for every later boundary
114+
* including the table write that prompted it.
115+
*/
116+
it('keeps vouching when a bundle-only tool cannot project and its block params throw', async () => {
117+
mockTool.request.secretProvenance = {
118+
request: () => [{ key: '0', inputPaths: [['data', 'apiKey']] }],
119+
response: { incomplete: 'propagate' },
120+
} as never
121+
mockGetBlock.mockReturnValue({
122+
tools: {
123+
access: ['some_custom_tool'],
124+
config: {
125+
tool: () => 'some_custom_tool',
126+
params: (params: Record<string, unknown>) => {
127+
/**
128+
* Throws only on the projected copy. Real blocks reach this by validating or parsing a
129+
* field a placeholder now sits in — the Table block runs `parseJSON` over `data` — and
130+
* which shape breaks does not matter to the invariant under test.
131+
*/
132+
if (typeof params.data === 'string' && params.data.includes('{{')) {
133+
throw new Error('cannot coerce a projected input')
134+
}
135+
return { data: params.data }
136+
},
137+
},
138+
},
139+
inputs: { data: { type: 'json', description: 'Row data' } },
140+
} as never)
141+
142+
/** Valid JSON, so the block's first `params` call over the real inputs succeeds. */
143+
const rowJson = '{"apiKey":"x"}'
144+
const registry = new ResolvedSecretTraceRegistry([
145+
{ name: 'ROW_SECRET', plaintext: rowJson, encryptedValue: 'encrypted-row-secret' },
146+
])
147+
registry.recordResolvedAtInputPath('ROW_SECRET', rowJson, ['data'])
148+
registry.recordResolvedInputProjection(['data'], rowJson, '{{ROW_SECRET}}')
149+
mockContext.resolvedSecretTraceRegistry = registry
150+
151+
await handler.execute(mockContext, mockBlock, { data: rowJson })
152+
153+
expect(registry.isComplete()).toBe(true)
154+
expect(registry.getIncompletenessDiagnostics()).toBeUndefined()
155+
})
156+
108157
it('preserves exact secret provenance when block params rename a selected input', async () => {
109158
mockTool.request.modelInput = {
110159
mode: 'private-provenance',

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,25 @@ function selectBlockBoundaryPaths(
4747
if (path[0]) requiredProjectionRoots.add(path[0])
4848
}
4949
}
50+
/**
51+
* Tracked, but never required to project.
52+
*
53+
* A `secretProvenance` selection is the opposite mechanism to a projected model input: the
54+
* value travels to an internal API unchanged, with its provenance alongside it in the private
55+
* bundle, precisely so nothing has to be substituted. `table_insert_row` posts row data to the
56+
* table API and declares no `modelInput` at all — there is no model egress on that path.
57+
*
58+
* Requiring those roots anyway made a projection failure fatal for tools that have no way to
59+
* project: `createStructuredModelProjection` rescues only a `mode: 'project'` tool with an
60+
* `applyProjected`, so for the twenty-odd `secretProvenance`-only tools it returns undefined on
61+
* its first check. The Table block's `params` runs `parseJSON` on the projected `data` string,
62+
* which throws once a placeholder stands where the JSON was, and the whole run's registry
63+
* latched — costing provenance for every later boundary, including the table write itself.
64+
*
65+
* A root is required to project when a model will see it, which is what `modelInput` declares.
66+
*/
5067
for (const selection of tool.request.secretProvenance?.request?.(params) ?? []) {
5168
paths.push(...selection.inputPaths)
52-
for (const path of selection.inputPaths) {
53-
if (path[0]) requiredProjectionRoots.add(path[0])
54-
}
5569
}
5670

5771
const uniquePaths = new Map<string, ResolvedSecretInputPath>()

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1625,6 +1625,56 @@ describe('incompleteness diagnostics', () => {
16251625
)
16261626
})
16271627

1628+
/**
1629+
* A run that failed before producing provenance hands the crossing `undefined`. That is the
1630+
* expected shape of a failed crossing, not a guard catching something wrong, so it reports at
1631+
* warn under its own name instead of joining the originating faults as a would-be breach.
1632+
*/
1633+
it('separates a crossing that carried no provenance from one that was rejected', async () => {
1634+
const absent = new ResolvedSecretTraceRegistry([], scope)
1635+
await absent.importCrossingProvenance(undefined, 'value', {
1636+
trusted: true,
1637+
origin: 'someSurface.failedRunCrossing',
1638+
})
1639+
1640+
expect(mockLogger.error).not.toHaveBeenCalled()
1641+
expect(mockLogger.warn).toHaveBeenCalledWith(
1642+
expect.any(String),
1643+
expect.objectContaining({ reason: 'value-provenance-absent' })
1644+
)
1645+
})
1646+
1647+
it('still reports a rejected crossing as a fault', async () => {
1648+
const rejected = new ResolvedSecretTraceRegistry([], scope)
1649+
await rejected.importCrossingProvenance({ not: 'a bundle' }, 'value', { trusted: true })
1650+
1651+
expect(mockLogger.error).toHaveBeenCalledWith(
1652+
expect.any(String),
1653+
expect.objectContaining({ reason: 'value-provenance-untrusted' })
1654+
)
1655+
})
1656+
1657+
/**
1658+
* A refusal is usually frames from its cause, which is what this struct exists to bridge — but it
1659+
* carried only *what* went wrong, so a downstream reporter printed a reason with no location.
1660+
*/
1661+
it('carries the first guard location through to the diagnostics a refusal reports', () => {
1662+
const registry = new ResolvedSecretTraceRegistry([], scope)
1663+
1664+
registry.markIncomplete('structural-input-root-unprojected', {
1665+
detail: { blockType: 'table', tool: 'table_insert_row', inputPath: 'data' },
1666+
})
1667+
registry.markIncomplete('inherited-incomplete-source', {
1668+
detail: { blockType: 'later', tool: 'later_tool' },
1669+
})
1670+
1671+
expect(registry.getIncompletenessDiagnostics()?.detail).toEqual({
1672+
blockType: 'table',
1673+
tool: 'table_insert_row',
1674+
inputPath: 'data',
1675+
})
1676+
})
1677+
16281678
it('names the guard that tripped rather than reporting unspecified', () => {
16291679
const registry = new ResolvedSecretTraceRegistry([], scope)
16301680

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

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ export type ResolvedSecretIncompletenessReason =
4141
| 'inherited-incomplete-input-path'
4242
| 'tool-call-scope-mismatch'
4343
| 'value-provenance-untrusted'
44+
/**
45+
* A crossing carried no provenance at all, which is what a run that failed before producing any
46+
* looks like. Distinct from `untrusted`: nothing was rejected, there was nothing to reject.
47+
*/
48+
| 'value-provenance-absent'
4449
| 'value-provenance-import-failed'
4550
| 'value-provenance-filter-incomplete'
4651
| 'durable-provenance-unknown'
@@ -174,6 +179,14 @@ export interface ResolvedSecretIncompletenessDiagnostics {
174179
readonly activeEntryCount: number
175180
/** Correlates a refusal with the guard that caused it; never carries user or secret material. */
176181
readonly scopeWorkspaceId?: string
182+
/**
183+
* Where the first guard tripped, carried alongside the reason that named it.
184+
*
185+
* A refusal is often frames away from its cause, which is why this struct exists — but it only
186+
* ever carried *what* went wrong, so a downstream reporter printed a reason with no location and
187+
* a reader had to join to the registry's own line to find the block.
188+
*/
189+
readonly detail?: MarkIncompleteDetail
177190
}
178191

179192
export const ANONYMOUS_SECRET_TRACE_REPLACEMENT = OPAQUE_RESOLVED_SECRET_REPLACEMENT
@@ -322,7 +335,7 @@ interface MarkIncompleteContext {
322335
* an input reaching one of these guards may still hold a resolved secret. That is the same promise
323336
* `reason` already makes about this log, restated where it is easy to break.
324337
*/
325-
interface MarkIncompleteDetail {
338+
export interface MarkIncompleteDetail {
326339
/** Block type id, e.g. `api`. */
327340
blockType?: string
328341
/** Tool id, e.g. `http_request`. */
@@ -854,6 +867,8 @@ export class ResolvedSecretTraceRegistry {
854867
private readonly incompletenessReasons = new Set<ResolvedSecretIncompletenessReason>()
855868
/** Import callers that cost this registry its completeness; bounded by {@link MAX_RETAINED_ORIGINS}. */
856869
private readonly incompletenessOrigins = new Set<string>()
870+
/** First guard's location; later ones describe propagation, not the cause. */
871+
private incompletenessDetail: MarkIncompleteDetail | undefined
857872
private activeProvenanceEntryBytes = 0
858873
private complete = true
859874
private pendingActivations = 0
@@ -1586,8 +1601,18 @@ export class ResolvedSecretTraceRegistry {
15861601
value: unknown,
15871602
options: { trusted: boolean; inputPath?: ResolvedSecretInputPath; origin?: string }
15881603
): Promise<ImportResolvedSecretTraceProvenanceForValueResult> {
1604+
/**
1605+
* Absence and distrust are different facts and are reported as such. A run that failed before
1606+
* producing provenance hands this `undefined`, which is the expected shape of a failed
1607+
* crossing, not a guard catching something wrong — reporting it as a fault put a recurring
1608+
* by-design state at error level with a name that reads like a breach.
1609+
*/
15891610
if (!options.trusted || !isResolvedSecretTraceProvenanceV1(provenance)) {
1590-
this.markInputPathIncomplete(options.inputPath, 'value-provenance-untrusted', options.origin)
1611+
const reason =
1612+
options.trusted && provenance === undefined
1613+
? 'value-provenance-absent'
1614+
: 'value-provenance-untrusted'
1615+
this.markInputPathIncomplete(options.inputPath, reason, options.origin)
15911616
return { success: false, matched: false }
15921617
}
15931618

@@ -1802,6 +1827,7 @@ export class ResolvedSecretTraceRegistry {
18021827
incompleteInputPathCount: this.incompleteInputPaths.size,
18031828
activeEntryCount: this.activeEntries.size,
18041829
...(this.scope?.workspaceId ? { scopeWorkspaceId: this.scope.workspaceId } : {}),
1830+
...(this.incompletenessDetail ? { detail: this.incompletenessDetail } : {}),
18051831
}
18061832
}
18071833

@@ -1823,6 +1849,7 @@ export class ResolvedSecretTraceRegistry {
18231849
private inheritIncompletenessReasonsFrom(source: ResolvedSecretTraceRegistry): void {
18241850
for (const reason of source.incompletenessReasons) this.recordIncompletenessReason(reason)
18251851
for (const origin of source.incompletenessOrigins) this.recordIncompletenessOrigin(origin)
1852+
this.incompletenessDetail ??= source.incompletenessDetail
18261853
}
18271854

18281855
isPermanentlyIncomplete(): boolean {
@@ -1843,6 +1870,7 @@ export class ResolvedSecretTraceRegistry {
18431870
if (context.source) this.inheritIncompletenessReasonsFrom(context.source)
18441871
this.recordIncompletenessReason(reason)
18451872
if (context.origin) this.recordIncompletenessOrigin(context.origin)
1873+
this.incompletenessDetail ??= context.detail
18461874
if (!this.complete) return
18471875
this.complete = false
18481876
this.modelEgressRevision += 1

0 commit comments

Comments
 (0)