Skip to content

Commit 095fc99

Browse files
committed
fix(provenance): store an absence apart from a refusal
Relaxing a stored `unknown` assumed it always meant "nobody recorded this". It did not. The sidecar collapsed two opposite claims into one value: bytes nobody recorded, and bytes a writer refused on purpose — a child of an archive whose parent provably held secrets, a generated asset whose safety decision came back false, a transcode whose scanner knew secrets were present and could not locate them. Relaxing the second would have walked known-secret-bearing content through every model boundary at once. The decision type already told them apart: a registry that never ran is `safe: true`, while every refusal is `safe: false`. Only storage lost it. Writers now persist `unrecorded` and `unknown` separately, readers relax the first and refuse the second, and the constraint is widened to accept it. Only workspace files store this distinction, deliberately. Every other durable surface produces a non-exact sidecar from one condition — an incomplete incoming bundle or registry — which is always an absence. Files are the only surface that derives one stored object from another, so they are the only one that can refuse on purpose. The shared policy is unchanged and uniform: read an absence and audit it, refuse a taint. Also aligns the metadata batch classifier, which answered `unknown` where the single-file reader answers `unrecorded`, leaving two classifiers describing one policy differently.
1 parent 11cc0f0 commit 095fc99

9 files changed

Lines changed: 20256 additions & 25 deletions

File tree

apps/sim/lib/copilot/request/tools/files.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,12 @@ describe('maybeWriteOutputToFile', () => {
578578
)
579579
})
580580

581-
it('preserves legacy writes without a registry and marks their provenance unknown', async () => {
581+
/**
582+
* No registry means no recorder ran, so nothing was written down about these bytes — an absence,
583+
* which the surface's policy may relax, and distinct from the taint a refused safety decision
584+
* produces below.
585+
*/
586+
it('preserves legacy writes without a registry and marks their provenance unrecorded', async () => {
582587
const result = await maybeWriteOutputToFile(
583588
RunFunction.id,
584589
{ outputs: { files: [{ path: 'files/report.json', mode: 'overwrite' }] } },
@@ -589,7 +594,7 @@ describe('maybeWriteOutputToFile', () => {
589594
expect(result.success).toBe(true)
590595
expect(mockWriteWorkspaceFileByPath).toHaveBeenCalledWith(
591596
expect.anything(),
592-
expect.objectContaining({ secretProvenance: { status: 'unknown' } })
597+
expect.objectContaining({ secretProvenance: { status: 'unrecorded' } })
593598
)
594599
})
595600

apps/sim/lib/execution/durable-secret-provenance-enforcement.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,19 @@ const logger = createLogger('DurableSecretProvenanceEnforcement')
99
*
1010
* Named by the call site rather than derived, so the surface survives refactors and stays
1111
* greppable — the same convention the projection-refusal `site` strings use.
12+
*
13+
* One policy governs all of them: an **absence** — nobody recorded what these bytes carry — may be
14+
* read, with an audit entry naming the surface, because a value nobody recorded says exactly what
15+
* an untracked one does. A **taint** — a writer that knew secrets were present and could not map
16+
* them to this output — is refused, and no policy relaxes it.
17+
*
18+
* Only `workspace-file` stores the difference, and that is deliberate rather than drift. On the
19+
* other surfaces every non-exact sidecar comes from one condition, an incomplete incoming bundle
20+
* or registry, which is always an absence; two stored statuses say everything there is to say.
21+
* Files are the only surface that derives one stored object from another — an archive extracted
22+
* into children, a generated asset, a transcoded output — so they are the only one that can refuse
23+
* on purpose, and the only one with two claims to keep apart. Adding a third status elsewhere would
24+
* encode a distinction that surface cannot make; removing it here would collapse one that matters.
1225
*/
1326
export const DURABLE_SECRET_PROVENANCE_SURFACES = [
1427
'memory',

apps/sim/lib/uploads/contexts/workspace/workspace-file-secret-provenance.test.ts

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -153,23 +153,49 @@ describe('workspace file secret provenance', () => {
153153
})
154154

155155
/**
156-
* The union carries three states; the column's CHECK constraint accepts two. Forwarding the
157-
* status verbatim would send `'unrecorded'` to the database as a value it rejects, aborting the
158-
* enclosing transaction rather than writing a bad row.
156+
* Absence and taint are different claims and must round-trip separately. Collapsing them is what
157+
* made a file a writer deliberately refused indistinguishable from one nobody recorded, and so
158+
* eligible for the same relaxation.
159159
*/
160-
it('narrows an unrecorded initialization to a status the column accepts', async () => {
160+
it('persists an unrecorded initialization as its own status', async () => {
161161
await initializeWorkspaceFileSecretProvenanceInTx(
162162
dbChainMock.db as unknown as DbTransaction,
163163
'file-1',
164164
CONTENT_UPDATED_AT,
165165
{ status: 'unrecorded' }
166166
)
167167

168+
expect(dbChainMockFns.values).toHaveBeenCalledWith(
169+
expect.objectContaining({ fileId: 'file-1', status: 'unrecorded', entries: [] })
170+
)
171+
})
172+
173+
it('persists a refused write as unknown, not as an absence', async () => {
174+
await replaceWorkspaceFileSecretProvenanceInTx(
175+
dbChainMock.db as unknown as DbTransaction,
176+
'file-1',
177+
CONTENT_UPDATED_AT,
178+
{ status: 'unknown' }
179+
)
180+
168181
expect(dbChainMockFns.values).toHaveBeenCalledWith(
169182
expect.objectContaining({ fileId: 'file-1', status: 'unknown', entries: [] })
170183
)
171184
})
172185

186+
it('persists an unrecorded replacement as its own status', async () => {
187+
await replaceWorkspaceFileSecretProvenanceInTx(
188+
dbChainMock.db as unknown as DbTransaction,
189+
'file-1',
190+
CONTENT_UPDATED_AT,
191+
{ status: 'unrecorded' }
192+
)
193+
194+
expect(dbChainMockFns.values).toHaveBeenCalledWith(
195+
expect.objectContaining({ fileId: 'file-1', status: 'unrecorded', entries: [] })
196+
)
197+
})
198+
173199
it('rejects a marker write that cannot bind the exact tracked content version', async () => {
174200
dbChainMockFns.returning.mockResolvedValueOnce([])
175201

@@ -305,6 +331,17 @@ describe('workspace file secret provenance', () => {
305331
status: 'unknown',
306332
entries: [],
307333
},
334+
{
335+
id: 'unrecorded-id',
336+
key: 'unrecorded-key',
337+
workspaceId: 'workspace-1',
338+
context: 'workspace',
339+
fileContentUpdatedAt: CONTENT_UPDATED_AT,
340+
secretProvenanceVersion: 1,
341+
provenanceContentUpdatedAt: CONTENT_UPDATED_AT,
342+
status: 'unrecorded',
343+
entries: [],
344+
},
308345
{
309346
id: 'other-workspace-id',
310347
key: 'other-workspace-key',
@@ -348,6 +385,7 @@ describe('workspace file secret provenance', () => {
348385
{ id: 'wrong-id', key: 'safe-key' },
349386
{ id: 'safe-id', key: 'tainted-key' },
350387
{ id: 'unknown-id', key: 'unknown-key' },
388+
{ id: 'unrecorded-id', key: 'unrecorded-key' },
351389
{ id: 'other-workspace-id', key: 'other-workspace-key' },
352390
{ id: 'pre-marker-sidecar-id', key: 'pre-marker-sidecar-key' },
353391
{ id: 'synthetic-execution-id', key: 'untracked-context-key' },
@@ -362,8 +400,12 @@ describe('workspace file secret provenance', () => {
362400
{ key: 'safe-key' },
363401
{ id: 'file-1700000000000', key: 'safe-key' },
364402
{ id: 'wrong-id', key: 'safe-key' },
365-
/** Unrecorded, so kept — the untracked keys below say the same thing and always were. */
366-
{ id: 'unknown-id', key: 'unknown-key' },
403+
/**
404+
* Unrecorded, so kept — the untracked keys below say the same thing and always were. The
405+
* stored `unknown` above is dropped: a writer refused those bytes on purpose, which is a
406+
* different claim from nobody having recorded them, and no policy relaxes it.
407+
*/
408+
{ id: 'unrecorded-id', key: 'unrecorded-key' },
367409
{ id: 'pre-marker-sidecar-id', key: 'pre-marker-sidecar-key' },
368410
{ id: 'synthetic-execution-id', key: 'untracked-context-key' },
369411
{ id: 'legacy-id', key: 'legacy-key' },
@@ -720,7 +762,7 @@ describe('workspace file secret provenance', () => {
720762
fileContentUpdatedAt: CONTENT_UPDATED_AT,
721763
secretProvenanceVersion: 1,
722764
provenanceContentUpdatedAt: CONTENT_UPDATED_AT,
723-
status: 'unknown',
765+
status: 'unrecorded',
724766
entries: [],
725767
},
726768
])
@@ -1331,7 +1373,7 @@ describe('workspace file secret provenance', () => {
13311373
fileContentUpdatedAt: CONTENT_UPDATED_AT,
13321374
secretProvenanceVersion: 1,
13331375
provenanceContentUpdatedAt: CONTENT_UPDATED_AT,
1334-
status: 'unknown',
1376+
status: 'unrecorded',
13351377
entries: [],
13361378
},
13371379
])

apps/sim/lib/uploads/contexts/workspace/workspace-file-secret-provenance.ts

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,12 @@ export async function createWorkspaceFileSecretProvenanceFromRegistry(
242242
representations: readonly WorkspaceFileSecretProvenanceRepresentation[] = [],
243243
representationsComplete = true
244244
): Promise<WorkspaceFileSecretProvenanceWriteDecision> {
245-
if (!registry) return { safe: true, provenance: { status: 'unknown' } }
245+
/**
246+
* No registry means no recorder ran, so nothing was written down about these bytes. That is an
247+
* absence, and it is the one thing this surface's policy may relax — distinct from the taint
248+
* every `safe: false` below produces, which a caller persists as `unknown` and no policy relaxes.
249+
*/
250+
if (!registry) return { safe: true, provenance: { status: 'unrecorded' } }
246251
const sourceProvenance = registry.exportCommittedProvenanceForValue(sourceValue)
247252
const persistedProvenance = Object.is(sourceValue, persistedValue)
248253
? sourceProvenance
@@ -422,12 +427,19 @@ export async function replaceWorkspaceFileSecretProvenanceInTx(
422427
return
423428
}
424429

430+
/**
431+
* Absence and taint are different claims and must not share a stored value. Collapsing them here
432+
* is what let a file the writer deliberately refused — a child of a secret-bearing archive, a
433+
* generated asset whose safety decision was `false` — become indistinguishable from one nobody
434+
* recorded, and therefore eligible for the same relaxation.
435+
*/
436+
const status = provenance.status === 'unrecorded' ? 'unrecorded' : 'unknown'
425437
await tx
426438
.insert(workspaceFileSecretProvenance)
427-
.values({ fileId, contentUpdatedAt, status: 'unknown', entries: [], updatedAt: new Date() })
439+
.values({ fileId, contentUpdatedAt, status, entries: [], updatedAt: new Date() })
428440
.onConflictDoUpdate({
429441
target: workspaceFileSecretProvenance.fileId,
430-
set: { contentUpdatedAt, status: 'unknown', entries: [], updatedAt: new Date() },
442+
set: { contentUpdatedAt, status, entries: [], updatedAt: new Date() },
431443
})
432444
await markWorkspaceFileSecretProvenanceTrackedInTx(tx, fileId, contentUpdatedAt)
433445
}
@@ -449,12 +461,13 @@ export async function initializeWorkspaceFileSecretProvenanceInTx(
449461
): Promise<void> {
450462
const isExact = provenance.status === 'exact'
451463
const entries = isExact ? serializeExactEntriesForStorage(provenance.entries) : []
464+
const status = isExact ? 'exact' : provenance.status === 'unrecorded' ? 'unrecorded' : 'unknown'
452465
await tx
453466
.insert(workspaceFileSecretProvenance)
454467
.values({
455468
fileId,
456469
contentUpdatedAt,
457-
status: isExact ? 'exact' : 'unknown',
470+
status,
458471
entries,
459472
updatedAt: new Date(),
460473
})
@@ -686,10 +699,12 @@ export async function getBoundWorkspaceFileSecretProvenance(
686699
row.provenanceContentUpdatedAt?.getTime() === row.fileContentUpdatedAt.getTime()
687700
if (!bindingIsCurrent || !isValidStoredEntries(row.entries)) return { status: 'unknown' }
688701
/**
689-
* The one shape the surface's policy may relax: a sidecar bound to this exact content that says
690-
* nobody recorded what it carries — the same statement the untracked file above makes.
702+
* The one shape the surface's policy may relax: a sidecar bound to this exact content recording
703+
* that nobody vouched for it — the same statement the untracked file above makes. A stored
704+
* `unknown` is the opposite claim, written by a writer that refused these bytes on purpose, and
705+
* stays refused.
691706
*/
692-
if (row.status === 'unknown') return { status: 'unrecorded' }
707+
if (row.status === 'unrecorded') return { status: 'unrecorded' }
693708
if (row.status !== 'exact') return { status: 'unknown' }
694709
return { status: 'exact', entries: deserializeExactEntriesFromStorage(row.entries) }
695710
}
@@ -739,12 +754,25 @@ export async function getBoundWorkspaceFileSecretProvenanceByMetadata(
739754
if (
740755
row.secretProvenanceVersion !== 1 ||
741756
row.provenanceContentUpdatedAt?.getTime() !== row.fileContentUpdatedAt.getTime() ||
742-
row.status !== 'exact' ||
743757
!isValidStoredEntries(row.entries)
744758
) {
745759
result.set(row.id, { status: 'unknown' })
746760
continue
747761
}
762+
/**
763+
* Same answer the single-file reader gives the same row. Collapsing a recorded absence into
764+
* `unknown` here would leave two classifiers describing one policy differently, and the
765+
* caller that eventually distinguishes them would get a different verdict depending on which
766+
* one it happened to call.
767+
*/
768+
if (row.status === 'unrecorded') {
769+
result.set(row.id, { status: 'unrecorded' })
770+
continue
771+
}
772+
if (row.status !== 'exact') {
773+
result.set(row.id, { status: 'unknown' })
774+
continue
775+
}
748776
result.set(row.id, {
749777
status: 'exact',
750778
entries: deserializeExactEntriesFromStorage(row.entries),
@@ -909,7 +937,7 @@ function classifyModelSafeWorkspaceFileRow(
909937
const bindingIsCurrent =
910938
row.provenanceContentUpdatedAt?.getTime() === row.fileContentUpdatedAt.getTime()
911939
if (!bindingIsCurrent || !isValidStoredEntries(row.entries)) return 'unsafe'
912-
if (row.status === 'unknown') return 'unrecorded'
940+
if (row.status === 'unrecorded') return 'unrecorded'
913941
if (row.status !== 'exact') return 'unsafe'
914942
return row.entries.length === 0 ? 'safe' : 'unsafe'
915943
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
-- migration-safe: replaces the status CHECK with a strictly wider one, adding 'unrecorded' to the existing ('exact', 'unknown'). Every stored row and every write the currently-deployed code can make already satisfies the replacement, so dropping it opens no window in which an old write is rejected, and it is re-added in the same transaction below.
2+
ALTER TABLE "workspace_file_secret_provenance" DROP CONSTRAINT "workspace_file_secret_provenance_status_check";--> statement-breakpoint
3+
ALTER TABLE "workspace_file_secret_provenance" ADD CONSTRAINT "workspace_file_secret_provenance_status_check" CHECK ("workspace_file_secret_provenance"."status" IN ('exact', 'unknown', 'unrecorded')) NOT VALID;--> statement-breakpoint
4+
ALTER TABLE "workspace_file_secret_provenance" VALIDATE CONSTRAINT "workspace_file_secret_provenance_status_check";

0 commit comments

Comments
 (0)