From f0b531d9df51e3586c5704117aa24d6d68b39c00 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sun, 23 Aug 2026 13:01:09 -0700 Subject: [PATCH] improvement(provenance): aggregate and attribute unrecorded durable reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fail-open on unrecorded durable provenance rests on one compensating control: the audit entry telling the people who own the secrets that a read proceeded unvouched. An audit of all four surfaces found the control incomplete in exactly the places this closes, and confirmed the policy itself sound — so nothing here changes what any read or write does, only what gets recorded about it. Knowledge was the one surface with no audit trail at all: the per-record import reports without a workspace, and the report skips the audit row when it cannot name one, so fail-open knowledge reads emitted one error log line per record and zero audit entries. Both importers now count unrecorded records while the surface is open and report once per read with the workspace, actor, and count — the shape memory and tables already use. The search read reports once across chunks and rendered metadata, and only when the registry did not latch, since a latched read never reaches a model. Fault returns stay silent; those reads fail closed. Memory had the one silent local degrade: a record whose canonical hash outgrows its bounds, or whose entries fail normalization, was stored unknown with nothing logged anywhere — the table writer logs its equivalent. The binding now logs the cause at error where it is decided. An incoming unknown stays silent; its producer already reported. The memory list contract gains the page ceiling every other list already has (max 1000, matching the table convention); no caller in the repo passes a limit at all, and the route is internal-auth only. Workspace-file audit rows now carry the acting user where the caller already holds one — copilot vfs, the agent and mothership handlers, and the provider attachment filter. Everywhere else, including principals with no user to name, the actor stays null, which the report type has always permitted. Two comments catch up with the code: the file sidecar stores three statuses since the absence/taint split, and the mounted-file scanner's scan-overflow-to-taint is deliberate where the registry scan over-approximates — that scan only narrows an already-sound candidate set, while this one decides whether egress redaction would suffice for bytes the same matcher just failed on. --- .../app/api/knowledge/secret-provenance.ts | 2 + .../app/api/memory/secret-provenance.test.ts | 9 ++ .../executor/handlers/agent/agent-handler.ts | 1 + .../handlers/mothership/mothership-handler.ts | 2 +- apps/sim/lib/api/contracts/memory.ts | 8 +- apps/sim/lib/copilot/tools/handlers/vfs.ts | 1 + .../mounted-file-secret-provenance.ts | 8 ++ apps/sim/lib/knowledge/application/search.ts | 26 +++- .../lib/knowledge/secret-provenance.test.ts | 120 +++++++++++++++++- apps/sim/lib/knowledge/secret-provenance.ts | 65 ++++++++-- apps/sim/lib/memory/secret-provenance.test.ts | 107 +++++++++++++++- apps/sim/lib/memory/secret-provenance.ts | 17 +++ .../workspace-file-secret-provenance.test.ts | 39 ++++++ .../workspace-file-secret-provenance.ts | 47 +++++-- apps/sim/providers/index.ts | 1 + 15 files changed, 422 insertions(+), 31 deletions(-) diff --git a/apps/sim/app/api/knowledge/secret-provenance.ts b/apps/sim/app/api/knowledge/secret-provenance.ts index 48a0ff71496..27210da78ba 100644 --- a/apps/sim/app/api/knowledge/secret-provenance.ts +++ b/apps/sim/app/api/knowledge/secret-provenance.ts @@ -248,6 +248,8 @@ export async function finalizeKnowledgePersistedResponse(options: { registry, documents: options.documents, chunks: options.chunks, + ...(options.workspaceId ? { workspaceId: options.workspaceId } : {}), + actorUserId: options.userId, }) return finalizeKnowledgeRegistryResponse({ request: options.request, diff --git a/apps/sim/app/api/memory/secret-provenance.test.ts b/apps/sim/app/api/memory/secret-provenance.test.ts index 866c9474af6..8132ed5e5ad 100644 --- a/apps/sim/app/api/memory/secret-provenance.test.ts +++ b/apps/sim/app/api/memory/secret-provenance.test.ts @@ -18,6 +18,7 @@ vi.mock('@/lib/execution/durable-secret-provenance-enforcement', () => ({ reportUnrecordedDurableProvenance: mockReport, })) +import { memoryListQuerySchema } from '@/lib/api/contracts/memory' import { AuthType } from '@/lib/auth/hybrid' import { PRIVATE_SECRET_PROVENANCE_BUNDLE_V1, @@ -341,3 +342,11 @@ describe('memory write secret provenance', () => { expect(mockReport).not.toHaveBeenCalled() }) }) + +describe('memory list query contract', () => { + it('rejects a limit past the page ceiling and keeps the default below it', () => { + expect(memoryListQuerySchema.safeParse({ limit: '2000' }).success).toBe(false) + expect(memoryListQuerySchema.parse({})).toMatchObject({ limit: 50 }) + expect(memoryListQuerySchema.parse({ limit: '1000' })).toMatchObject({ limit: 1000 }) + }) +}) diff --git a/apps/sim/executor/handlers/agent/agent-handler.ts b/apps/sim/executor/handlers/agent/agent-handler.ts index 9256c3e67d3..eb0b38e2603 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.ts @@ -1680,6 +1680,7 @@ export class AgentBlockHandler implements BlockHandler { identity, registry: ctx.resolvedSecretTraceRegistry, view: 'opaque', + ...(ctx.userId ? { actorUserId: ctx.userId } : {}), }) if (!safe) { unsafeGeneratedDocumentFiles.add(`${file.key}:${file.id}`) diff --git a/apps/sim/executor/handlers/mothership/mothership-handler.ts b/apps/sim/executor/handlers/mothership/mothership-handler.ts index 3676f8b8f90..21f715e603a 100644 --- a/apps/sim/executor/handlers/mothership/mothership-handler.ts +++ b/apps/sim/executor/handlers/mothership/mothership-handler.ts @@ -689,7 +689,7 @@ async function buildMothershipFileAttachments( ) const modelSafe = await areModelSafeWorkspaceFileKeys( userFiles.map((file) => file.key).filter((key): key is string => Boolean(key)), - { workspaceId: ctx.workspaceId } + { workspaceId: ctx.workspaceId, ...(ctx.userId ? { actorUserId: ctx.userId } : {}) } ) if (!modelSafe) throw new Error(MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE) diff --git a/apps/sim/lib/api/contracts/memory.ts b/apps/sim/lib/api/contracts/memory.ts index 0551959bc59..7064bf47934 100644 --- a/apps/sim/lib/api/contracts/memory.ts +++ b/apps/sim/lib/api/contracts/memory.ts @@ -34,7 +34,13 @@ export const agentMemoryDataSchemaContract = agentMemoryDataSchema export const memoryListQuerySchema = z.object({ workspaceId: z.string().optional(), query: z.string().nullable().optional(), - limit: z.coerce.number().int().min(1).optional().default(50), + limit: z.coerce + .number() + .int() + .min(1) + .max(1000, 'Cannot list more than 1000 memories per request') + .optional() + .default(50), }) export const memoryMessageSchema = z diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.ts b/apps/sim/lib/copilot/tools/handlers/vfs.ts index 894df5ea709..41cdcd2ae75 100644 --- a/apps/sim/lib/copilot/tools/handlers/vfs.ts +++ b/apps/sim/lib/copilot/tools/handlers/vfs.ts @@ -126,6 +126,7 @@ async function canReturnWorkspaceFileValue( registry: context.resolvedSecretTraceRegistry, view: provenanceView, value, + actorUserId: context.userId, })) ) { return false diff --git a/apps/sim/lib/execution/mounted-file-secret-provenance.ts b/apps/sim/lib/execution/mounted-file-secret-provenance.ts index 001496e1d5a..44ddfddc870 100644 --- a/apps/sim/lib/execution/mounted-file-secret-provenance.ts +++ b/apps/sim/lib/execution/mounted-file-secret-provenance.ts @@ -89,6 +89,14 @@ export async function createMountedFileSecretProvenanceScanner( return { hasSecrets, + /** + * A scan that cannot finish yields `unknown` — a taint — where the registry's per-value scan + * over-approximates instead. The asymmetry is deliberate: that scan only narrows a candidate + * set that is already a sound answer, while this one decides whether egress redaction of these + * entries would suffice for these bytes — a claim that cannot be made for content the same + * matcher just failed on. Reaching the event bound takes an eight-plus-character literal + * occurring ~a million times, so only degenerate content pays the refusal. + */ scan(buffer) { const matched = new Map() try { diff --git a/apps/sim/lib/knowledge/application/search.ts b/apps/sim/lib/knowledge/application/search.ts index 310447d060b..c4d492820ac 100644 --- a/apps/sim/lib/knowledge/application/search.ts +++ b/apps/sim/lib/knowledge/application/search.ts @@ -15,6 +15,10 @@ import { OrchestrationError } from '@/lib/core/orchestration/types' import { PlatformEvents } from '@/lib/core/telemetry' import { generateRequestId } from '@/lib/core/utils/request' import { importDurableSecretProvenance } from '@/lib/execution/durable-secret-provenance' +import { + isDurableSecretProvenanceEnforced, + reportUnrecordedDurableProvenance, +} from '@/lib/execution/durable-secret-provenance-enforcement' import { defineAuthorizedKnowledgeUseCase } from '@/lib/knowledge/application/authorized-knowledge-use-case' import { KnowledgeUsageLimitExceededError, @@ -488,6 +492,8 @@ export const searchKnowledge = defineAuthorizedKnowledgeUseCase({ } }) if (registry && provenanceSnapshot) { + const knowledgeEnforced = isDurableSecretProvenanceEnforced('knowledge') + let unrecordedCount = provenanceSnapshot.unrecordedCount for (const [documentId, document] of Object.entries(provenanceSnapshot.documentMetadata)) { const renderedMetadata = results .filter((result) => result.documentId === documentId) @@ -496,18 +502,34 @@ export const searchKnowledge = defineAuthorizedKnowledgeUseCase({ sourceUrl: result.sourceUrl, metadata: result.metadata, })) + if (renderedMetadata.length === 0) continue + if (document.provenance.status === 'unknown' && !knowledgeEnforced) unrecordedCount += 1 if ( - renderedMetadata.length > 0 && !(await importDurableSecretProvenance( registry, document.provenance, renderedMetadata, - 'knowledge' + 'knowledge', + { reportUnrecorded: false } )) ) { registry.markIncomplete('knowledge-result-provenance-unavailable') } } + /** + * One entry for the whole search — chunks and rendered metadata are one read. Skipped when + * the registry latched: a latched read never reaches a model, and this entry exists to say a + * fail-open read went ahead unvouched. + */ + if (unrecordedCount > 0 && !registry.isPermanentlyIncomplete()) { + reportUnrecordedDurableProvenance({ + surface: 'knowledge', + cause: 'durable-provenance-unknown', + affectedCount: unrecordedCount, + workspaceId: context.workspaceId, + actorUserId: userId, + }) + } } const cost = baseCost ? { diff --git a/apps/sim/lib/knowledge/secret-provenance.test.ts b/apps/sim/lib/knowledge/secret-provenance.test.ts index c804025d8c3..08d4ba6bb36 100644 --- a/apps/sim/lib/knowledge/secret-provenance.test.ts +++ b/apps/sim/lib/knowledge/secret-provenance.test.ts @@ -1,24 +1,34 @@ /** * @vitest-environment node */ -import { document } from '@sim/db/schema' +import { document, embedding } from '@sim/db/schema' import { queueTableRows, resetDbChainMock } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' import { hashDurableSecretProvenanceValue } from '@/lib/execution/durable-secret-provenance' import { createKnowledgeDocumentSourceValue, + importKnowledgePersistedResponseSecretProvenance, + importKnowledgeSearchResultSecretProvenance, loadKnowledgeDocumentSecretRegistry, readBoundKnowledgeDocumentSecretProvenance, } from '@/lib/knowledge/secret-provenance' +import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' -const { mockDecryptSecret } = vi.hoisted(() => ({ +const { mockDecryptSecret, mockIsEnforced, mockReport } = vi.hoisted(() => ({ mockDecryptSecret: vi.fn(), + mockIsEnforced: vi.fn(() => false), + mockReport: vi.fn(), })) vi.mock('@/lib/core/security/encryption', () => ({ decryptSecret: mockDecryptSecret, })) +vi.mock('@/lib/execution/durable-secret-provenance-enforcement', () => ({ + isDurableSecretProvenanceEnforced: mockIsEnforced, + reportUnrecordedDurableProvenance: mockReport, +})) + const DOCUMENT_SOURCE = createKnowledgeDocumentSourceValue({ filename: 'source.pdf', fileUrl: '/api/files/serve/workspace%2Fworkspace-1%2Fsource.pdf?context=workspace', @@ -39,6 +49,7 @@ describe('knowledge durable secret provenance', () => { resetDbChainMock() queueTableRows(document, [DOCUMENT_ROW]) mockDecryptSecret.mockResolvedValue({ decrypted: 'tracked-secret' }) + mockIsEnforced.mockReturnValue(false) }) it('uses the same explicit source shape for joined rows and persisted writes', () => { @@ -130,3 +141,108 @@ describe('knowledge durable secret provenance', () => { }) }) }) + +describe('knowledge unrecorded-read reporting', () => { + const SCOPE = { userId: 'user-1', workspaceId: 'workspace-1' } + const UNRECORDED_DOCUMENT_ROW = { + id: 'doc-1', + ...DOCUMENT_SOURCE, + secretProvenanceVersion: 1, + provenanceSourceHash: null, + status: 'unknown', + entries: null, + } + const UNRECORDED_CHUNK_ROW = { + id: 'chunk-1', + documentId: 'doc-1', + content: 'chunk text', + chunkHash: 'stale', + secretProvenanceVersion: 1, + provenanceContentHash: null, + status: 'unknown', + entries: null, + } + + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mockIsEnforced.mockReturnValue(false) + }) + + it('reports one aggregated entry per read, naming workspace, actor, and count', async () => { + queueTableRows(document, [UNRECORDED_DOCUMENT_ROW]) + queueTableRows(embedding, [UNRECORDED_CHUNK_ROW]) + const registry = new ResolvedSecretTraceRegistry([], SCOPE) + + await expect( + importKnowledgePersistedResponseSecretProvenance({ + registry, + documents: [{ id: 'doc-1', source: DOCUMENT_SOURCE, value: {} }], + chunks: [{ id: 'chunk-1', documentId: 'doc-1', content: 'chunk text', value: {} }], + workspaceId: 'workspace-1', + actorUserId: 'user-1', + }) + ).resolves.toBe(true) + + expect(registry.isPermanentlyIncomplete()).toBe(false) + expect(mockReport).toHaveBeenCalledTimes(1) + expect(mockReport).toHaveBeenCalledWith({ + surface: 'knowledge', + cause: 'durable-provenance-unknown', + affectedCount: 2, + workspaceId: 'workspace-1', + actorUserId: 'user-1', + }) + }) + + /** A fault return fails the read closed, so no unvouched record reached anything to report. */ + it('reports nothing when the read fails closed on a missing row', async () => { + queueTableRows(document, []) + const registry = new ResolvedSecretTraceRegistry([], SCOPE) + + await expect( + importKnowledgePersistedResponseSecretProvenance({ + registry, + documents: [{ id: 'doc-1', source: DOCUMENT_SOURCE, value: {} }], + workspaceId: 'workspace-1', + actorUserId: 'user-1', + }) + ).resolves.toBe(false) + + expect(mockReport).not.toHaveBeenCalled() + }) + + it('latches without reporting once the surface is enforced', async () => { + mockIsEnforced.mockReturnValue(true) + queueTableRows(document, [UNRECORDED_DOCUMENT_ROW]) + const registry = new ResolvedSecretTraceRegistry([], SCOPE) + + await expect( + importKnowledgePersistedResponseSecretProvenance({ + registry, + documents: [{ id: 'doc-1', source: DOCUMENT_SOURCE, value: {} }], + workspaceId: 'workspace-1', + actorUserId: 'user-1', + }) + ).resolves.toBe(false) + + expect(registry.isPermanentlyIncomplete()).toBe(true) + expect(mockReport).not.toHaveBeenCalled() + }) + + /** The search read spans chunks and rendered metadata, so its caller owns the one report. */ + it('returns the unrecorded count from a search import instead of reporting it', async () => { + queueTableRows(embedding, [{ ...UNRECORDED_CHUNK_ROW, documentId: DOCUMENT_ROW.id }]) + queueTableRows(document, [DOCUMENT_ROW]) + const registry = new ResolvedSecretTraceRegistry([], SCOPE) + + const snapshot = await importKnowledgeSearchResultSecretProvenance({ + registry, + results: [{ id: 'chunk-1', documentId: DOCUMENT_ROW.id, content: 'chunk text' }], + }) + + expect(snapshot.imported).toBe(true) + expect(snapshot.unrecordedCount).toBe(1) + expect(mockReport).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/knowledge/secret-provenance.ts b/apps/sim/lib/knowledge/secret-provenance.ts index 7ef6a8522b0..6a708951da8 100644 --- a/apps/sim/lib/knowledge/secret-provenance.ts +++ b/apps/sim/lib/knowledge/secret-provenance.ts @@ -18,6 +18,10 @@ import { mergeDurableSecretProvenance, normalizeDurableSecretProvenanceEntries, } from '@/lib/execution/durable-secret-provenance' +import { + isDurableSecretProvenanceEnforced, + reportUnrecordedDurableProvenance, +} from '@/lib/execution/durable-secret-provenance-enforcement' import { ResolvedSecretTraceRegistry, type ResolvedSecretTraceScopeV1, @@ -449,9 +453,21 @@ export async function importKnowledgePersistedResponseSecretProvenance(options: content: string value: unknown }[] + /** Names the workspace in the aggregated unrecorded-read audit entry; legacy KBs have none. */ + workspaceId?: string + /** Whose access authorized the read, for the same entry. */ + actorUserId?: string }): Promise { const documents = options.documents ?? [] const chunks = options.chunks ?? [] + /** + * Counted here and reported once at the end of the proceed path, the shape the memory and table + * surfaces use: the per-record import knows no workspace, so its report never produced the + * workspace-visible audit entry, and it logged once per record. A fault return skips the report — + * that read fails closed, so no unvouched record reached anything. + */ + const knowledgeEnforced = isDurableSecretProvenanceEnforced('knowledge') + let unrecordedCount = 0 const documentIds = [...new Set(documents.map((item) => item.id))] const chunkIds = [...new Set(chunks.map((item) => item.id))] const [documentRows, chunkRows] = await Promise.all([ @@ -493,8 +509,11 @@ export async function importKnowledgePersistedResponseSecretProvenance(options: readBoundKnowledgeDocumentSecretProvenance({ ...row, source }), source ) + if (provenance.status === 'unknown' && !knowledgeEnforced) unrecordedCount += 1 if ( - !(await importDurableSecretProvenance(options.registry, provenance, item.value, 'knowledge')) + !(await importDurableSecretProvenance(options.registry, provenance, item.value, 'knowledge', { + reportUnrecorded: false, + })) ) { return false } @@ -507,13 +526,25 @@ export async function importKnowledgePersistedResponseSecretProvenance(options: return false } const provenance = readBoundKnowledgeEmbeddingSecretProvenance(row) + if (provenance.status === 'unknown' && !knowledgeEnforced) unrecordedCount += 1 if ( - !(await importDurableSecretProvenance(options.registry, provenance, item.value, 'knowledge')) + !(await importDurableSecretProvenance(options.registry, provenance, item.value, 'knowledge', { + reportUnrecorded: false, + })) ) { return false } } + if (unrecordedCount > 0) { + reportUnrecordedDurableProvenance({ + surface: 'knowledge', + cause: 'durable-provenance-unknown', + affectedCount: unrecordedCount, + ...(options.workspaceId ? { workspaceId: options.workspaceId } : {}), + actorUserId: options.actorUserId ?? null, + }) + } return !options.registry.isPermanentlyIncomplete() } @@ -523,6 +554,13 @@ export async function importKnowledgeSearchResultSecretProvenance(options: { results: readonly { id: string; documentId: string; content: string }[] }): Promise<{ imported: boolean + /** + * Chunks whose stored provenance was unrecorded and whose import proceeded fail-open. The caller + * folds this into one read-level audit report — it owns the workspace and the metadata imports + * that share the same read, and it reports nothing when the registry latched, since a latched + * read never reaches a model. + */ + unrecordedCount: number documentMetadata: Record< string, { @@ -539,7 +577,9 @@ export async function importKnowledgeSearchResultSecretProvenance(options: { } > }> { - if (options.results.length === 0) return { imported: true, documentMetadata: {} } + if (options.results.length === 0) { + return { imported: true, unrecordedCount: 0, documentMetadata: {} } + } const embeddingIds = [...new Set(options.results.map((result) => result.id))] const documentIds = [...new Set(options.results.map((result) => result.documentId))] const [chunks, documents] = await Promise.all([ @@ -554,30 +594,36 @@ export async function importKnowledgeSearchResultSecretProvenance(options: { ), ]) const chunkById = new Map(chunks.map((row) => [row.id, row])) - if (chunkById.size !== embeddingIds.length) return { imported: false, documentMetadata: {} } + if (chunkById.size !== embeddingIds.length) { + return { imported: false, unrecordedCount: 0, documentMetadata: {} } + } + const knowledgeEnforced = isDurableSecretProvenanceEnforced('knowledge') + let unrecordedCount = 0 for (const result of options.results) { const row = chunkById.get(result.id) if (!row || row.documentId !== result.documentId || row.content !== result.content) { - return { imported: false, documentMetadata: {} } + return { imported: false, unrecordedCount: 0, documentMetadata: {} } } const provenance = readBoundKnowledgeEmbeddingSecretProvenance(row) + if (provenance.status === 'unknown' && !knowledgeEnforced) unrecordedCount += 1 if ( !(await importDurableSecretProvenance( options.registry, provenance, result.content, - 'knowledge' + 'knowledge', + { reportUnrecorded: false } )) ) { - return { imported: false, documentMetadata: {} } + return { imported: false, unrecordedCount: 0, documentMetadata: {} } } } const documentById = new Map(documents.map((row) => [row.id, row])) if (documentById.size !== documentIds.length) { - return { imported: false, documentMetadata: {} } + return { imported: false, unrecordedCount: 0, documentMetadata: {} } } if (options.results.some((result) => !documentById.has(result.documentId))) { - return { imported: false, documentMetadata: {} } + return { imported: false, unrecordedCount: 0, documentMetadata: {} } } const documentMetadata: Record< string, @@ -616,6 +662,7 @@ export async function importKnowledgeSearchResultSecretProvenance(options: { } return { imported: !options.registry.isPermanentlyIncomplete(), + unrecordedCount, documentMetadata, } } diff --git a/apps/sim/lib/memory/secret-provenance.test.ts b/apps/sim/lib/memory/secret-provenance.test.ts index 0057511672e..979e528bdc1 100644 --- a/apps/sim/lib/memory/secret-provenance.test.ts +++ b/apps/sim/lib/memory/secret-provenance.test.ts @@ -1,10 +1,50 @@ /** * @vitest-environment node */ -import { describe, expect, it } from 'vitest' -import { readBoundMemorySecretProvenance } from '@/lib/memory/secret-provenance' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockLogger } = vi.hoisted(() => ({ + mockLogger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, +})) + +vi.mock('@sim/logger', () => ({ + createLogger: () => mockLogger, +})) + +import type { DbTransaction } from '@/lib/db/types' +import { + readBoundMemorySecretProvenance, + replaceMemorySecretProvenanceInTx, +} from '@/lib/memory/secret-provenance' + +interface TxStub { + tx: DbTransaction + inserted: Record[] +} + +function createTxStub(): TxStub { + const inserted: Record[] = [] + const tx = { + insert: () => ({ + values: (value: Record) => { + inserted.push(value) + return { onConflictDoUpdate: async () => undefined } + }, + }), + update: () => ({ + set: () => ({ + where: () => ({ returning: async () => [{ id: 'memory-1' }] }), + }), + }), + } + return { tx: tx as unknown as DbTransaction, inserted } +} describe('memory secret provenance', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + it('treats a marker-null row as legacy even when an old sidecar remains', () => { expect( readBoundMemorySecretProvenance({ @@ -16,4 +56,67 @@ describe('memory secret provenance', () => { }) ).toEqual({ status: 'exact', entries: [] }) }) + + it('binds exact provenance silently when nothing degrades', async () => { + const { tx, inserted } = createTxStub() + + await replaceMemorySecretProvenanceInTx(tx, 'memory-1', [{ role: 'user', content: 'hello' }], { + status: 'exact', + entries: [{ name: 'SECRET', encryptedValue: 'encrypted' }], + }) + + expect(inserted[0]).toMatchObject({ status: 'exact' }) + expect(mockLogger.error).not.toHaveBeenCalled() + }) + + /** + * The one degrade decided in this function: exact provenance arrived and the binding could not + * hold it. Every later read proceeds unvouched, so the cause must be on record at write time. + */ + it('logs the cause when exact provenance degrades because the record cannot be hashed', async () => { + const { tx, inserted } = createTxStub() + + await replaceMemorySecretProvenanceInTx( + tx, + 'memory-1', + { unhashable: () => undefined }, + { + status: 'exact', + entries: [{ name: 'SECRET', encryptedValue: 'encrypted' }], + } + ) + + expect(inserted[0]).toMatchObject({ status: 'unknown', contentHash: 'unavailable' }) + expect(mockLogger.error).toHaveBeenCalledWith( + 'Memory write persisted unrecorded secret provenance', + { surface: 'memory', cause: 'hash-unavailable', memoryId: 'memory-1' } + ) + }) + + it('logs the cause when exact entries cannot be normalized', async () => { + const { tx, inserted } = createTxStub() + + await replaceMemorySecretProvenanceInTx(tx, 'memory-1', [{ role: 'user', content: 'hello' }], { + status: 'exact', + entries: [{ encryptedValue: '' }], + }) + + expect(inserted[0]).toMatchObject({ status: 'unknown' }) + expect(mockLogger.error).toHaveBeenCalledWith( + 'Memory write persisted unrecorded secret provenance', + { surface: 'memory', cause: 'entries-unnormalizable', memoryId: 'memory-1' } + ) + }) + + /** An incoming unknown was degraded by its producer, which already reported it. */ + it('stays silent when the incoming provenance is already unknown', async () => { + const { tx, inserted } = createTxStub() + + await replaceMemorySecretProvenanceInTx(tx, 'memory-1', [{ role: 'user', content: 'hello' }], { + status: 'unknown', + }) + + expect(inserted[0]).toMatchObject({ status: 'unknown' }) + expect(mockLogger.error).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/lib/memory/secret-provenance.ts b/apps/sim/lib/memory/secret-provenance.ts index ff22e3d6fa4..29c650c9c98 100644 --- a/apps/sim/lib/memory/secret-provenance.ts +++ b/apps/sim/lib/memory/secret-provenance.ts @@ -1,4 +1,5 @@ import { memory, memorySecretProvenance } from '@sim/db/schema' +import { createLogger } from '@sim/logger' import { and, eq, isNull, or } from 'drizzle-orm' import type { DbTransaction } from '@/lib/db/types' import { @@ -8,6 +9,8 @@ import { normalizeDurableSecretProvenanceEntries, } from '@/lib/execution/durable-secret-provenance' +const logger = createLogger('MemorySecretProvenance') + interface MemorySecretProvenanceRow { secretProvenanceVersion: number | null data: unknown @@ -49,6 +52,20 @@ export async function replaceMemorySecretProvenanceInTx( ? normalizeDurableSecretProvenanceEntries(provenance.entries) : [] const status = contentHash && provenance.status === 'exact' && entries ? 'exact' : 'unknown' + /** + * The one degrade that happens here rather than upstream: exact provenance arrived, and this + * binding could not hold it — the record outgrew the content hash's bounds, or the entries the + * envelope bounds. Every later read of the row proceeds unvouched, so the cause is logged where + * it was decided, the shape the table writer uses. An incoming `unknown` stays silent; its + * producer already reported. + */ + if (provenance.status === 'exact' && status === 'unknown') { + logger.error('Memory write persisted unrecorded secret provenance', { + surface: 'memory', + cause: contentHash ? 'entries-unnormalizable' : 'hash-unavailable', + memoryId, + }) + } await tx .insert(memorySecretProvenance) .values({ diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-secret-provenance.test.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-secret-provenance.test.ts index 637ae819005..a372277641e 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-secret-provenance.test.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-secret-provenance.test.ts @@ -1432,6 +1432,45 @@ describe('workspace file secret provenance', () => { ) }) + /** The audit row names who read past the absence when the caller can say; null otherwise. */ + it('carries the actor into the unrecorded-read report when the caller supplies one', async () => { + queueTableRows(workspaceFiles, [ + { + id: 'unrecorded-id', + key: 'unrecorded-key', + workspaceId: 'workspace-1', + context: 'workspace', + fileContentUpdatedAt: CONTENT_UPDATED_AT, + secretProvenanceVersion: 1, + provenanceContentUpdatedAt: CONTENT_UPDATED_AT, + status: 'unrecorded', + entries: [], + }, + ]) + + await expect( + isModelSafeWorkspaceFileKey('unrecorded-key', { actorUserId: 'user-1' }) + ).resolves.toBe(true) + expect(mockReport).toHaveBeenCalledWith(expect.objectContaining({ actorUserId: 'user-1' })) + + mockReport.mockClear() + queueTableRows(workspaceFiles, [ + { + id: 'unrecorded-id', + key: 'unrecorded-key', + workspaceId: 'workspace-1', + context: 'workspace', + fileContentUpdatedAt: CONTENT_UPDATED_AT, + secretProvenanceVersion: 1, + provenanceContentUpdatedAt: CONTENT_UPDATED_AT, + status: 'unrecorded', + entries: [], + }, + ]) + await expect(isModelSafeWorkspaceFileKey('unrecorded-key')).resolves.toBe(true) + expect(mockReport).toHaveBeenCalledWith(expect.objectContaining({ actorUserId: null })) + }) + /** * The row has to be a recorded absence, not a refusal. A stored `unknown` is refused whatever the * flag says, so asserting against one would pass with enforcement off and prove nothing about the diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-secret-provenance.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-secret-provenance.ts index a4a2cec7f50..17b632bba1a 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-secret-provenance.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-secret-provenance.ts @@ -36,12 +36,13 @@ export const MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE = /** * What can be said about the secrets a file's bytes carry. * - * Note the deliberate mismatch with storage: the sidecar's `status` column holds only - * `'exact' | 'unknown'`, and a stored `'unknown'` maps to `'unrecorded'` here, not to the - * `'unknown'` below. Storage is recording what the writer could vouch for; this union is recording - * what a reader can conclude, and "the writer said it could not vouch" and "there is nothing usable - * to read" are different conclusions that must not share a branch — only the first is an absence a - * policy may relax. `'unrecorded'` is the name the shared vocabulary already uses for it + * The sidecar's `status` column stores all three values, and reader and storage still mean + * different things by two of them. Stored `'unrecorded'` is the writer saying nobody vouched for + * these bytes; stored `'unknown'` is a writer that knew secrets were in scope and could not map + * them. This union records what a *reader* can conclude, so a missing row, moved version, or stale + * or malformed sidecar also lands on `'unknown'` — "the writer refused" and "there is nothing + * usable to read" share a conclusion but not a stored value. Only `'unrecorded'` is an absence a + * policy may relax; it is the name the shared vocabulary already uses * (`reportUnrecordedDurableProvenance`, the `secret_provenance.unrecorded` audit action). */ export type WorkspaceFileSecretProvenance = @@ -820,7 +821,11 @@ export async function getBoundWorkspaceFileSecretProvenanceByMetadata( * absence this covers. Closing the surface again is a matter of naming it in * `DURABLE_SECRET_PROVENANCE_ENFORCED_SURFACES`. */ -function mayReadUnrecordedWorkspaceFile(workspaceId: string | undefined, count = 1): boolean { +function mayReadUnrecordedWorkspaceFile( + workspaceId: string | undefined, + count = 1, + actorUserId?: string +): boolean { if (isDurableSecretProvenanceEnforced('workspace-file')) return false if (count > 0) { reportUnrecordedDurableProvenance({ @@ -828,6 +833,7 @@ function mayReadUnrecordedWorkspaceFile(workspaceId: string | undefined, count = cause: 'durable-provenance-unknown', ...(count > 1 ? { affectedCount: count } : {}), ...(workspaceId ? { workspaceId } : {}), + actorUserId: actorUserId ?? null, }) } return true @@ -845,10 +851,14 @@ export async function importWorkspaceFileSecretProvenanceForModelView(args: { registry?: ResolvedSecretTraceRegistry view: 'complete' | 'derived' | 'opaque' value?: unknown + /** Whose access authorized the read, for the unrecorded-read audit entry; null when unnameable. */ + actorUserId?: string }): Promise { const provenance = await getBoundWorkspaceFileSecretProvenance(args.workspaceId, args.identity) if (provenance.status === 'unknown') return false - if (provenance.status === 'unrecorded') return mayReadUnrecordedWorkspaceFile(args.workspaceId) + if (provenance.status === 'unrecorded') { + return mayReadUnrecordedWorkspaceFile(args.workspaceId, 1, args.actorUserId) + } if (provenance.entries.length === 0) return true if (args.view === 'opaque' || !args.registry) return false @@ -881,10 +891,14 @@ export async function importWorkspaceFileSecretProvenanceForRuntime(args: { workspaceId: string identity: WorkspaceFileSecretProvenanceIdentity registry?: ResolvedSecretTraceRegistry + /** Whose access authorized the read, for the unrecorded-read audit entry; null when unnameable. */ + actorUserId?: string }): Promise { const provenance = await getBoundWorkspaceFileSecretProvenance(args.workspaceId, args.identity) if (provenance.status === 'unknown') return false - if (provenance.status === 'unrecorded') return mayReadUnrecordedWorkspaceFile(args.workspaceId) + if (provenance.status === 'unrecorded') { + return mayReadUnrecordedWorkspaceFile(args.workspaceId, 1, args.actorUserId) + } if (provenance.entries.length === 0) return true if (!args.registry) return false @@ -904,7 +918,7 @@ export async function filterModelSafeWorkspaceFileAttachments< TAttachment extends WorkspaceFileAttachmentIdentity, >( attachments: readonly TAttachment[], - options: { workspaceId?: string } = {} + options: { workspaceId?: string; actorUserId?: string } = {} ): Promise { if (attachments.length === 0) return [] if (attachments.length > PROVENANCE_MAX_ENTRIES) { @@ -936,7 +950,9 @@ export async function filterModelSafeWorkspaceFileAttachments< return !isDurableSecretProvenanceEnforced('workspace-file') }) /** One report for the whole set of attachments, which is one read, rather than one per file. */ - if (unrecorded > 0) mayReadUnrecordedWorkspaceFile(options.workspaceId, unrecorded) + if (unrecorded > 0) { + mayReadUnrecordedWorkspaceFile(options.workspaceId, unrecorded, options.actorUserId) + } return kept } @@ -993,7 +1009,7 @@ async function loadModelSafeWorkspaceFileRows( */ export async function isModelSafeWorkspaceFileKey( key: string, - options: { workspaceId?: string } = {} + options: { workspaceId?: string; actorUserId?: string } = {} ): Promise { return areModelSafeWorkspaceFileKeys([key], options) } @@ -1005,7 +1021,7 @@ export async function isModelSafeWorkspaceFileKey( */ export async function areModelSafeWorkspaceFileKeys( keys: readonly string[], - options: { workspaceId?: string } = {} + options: { workspaceId?: string; actorUserId?: string } = {} ): Promise { const uniqueKeys = [...new Set(keys.filter((key) => key.length > 0))] if (uniqueKeys.length === 0) return true @@ -1023,5 +1039,8 @@ export async function areModelSafeWorkspaceFileKeys( if (classification === 'unrecorded') unrecorded += 1 } /** One report for the batch, not one per key: a caller checking many keys is one read. */ - return unrecorded === 0 || mayReadUnrecordedWorkspaceFile(options.workspaceId, unrecorded) + return ( + unrecorded === 0 || + mayReadUnrecordedWorkspaceFile(options.workspaceId, unrecorded, options.actorUserId) + ) } diff --git a/apps/sim/providers/index.ts b/apps/sim/providers/index.ts index 4d20d166c6a..66a58507ef9 100644 --- a/apps/sim/providers/index.ts +++ b/apps/sim/providers/index.ts @@ -45,6 +45,7 @@ async function omitUnsafeProviderFileAttachments( try { safeAttachments = await filterModelSafeWorkspaceFileAttachments(attachments, { workspaceId: request.workspaceId, + ...(request.userId ? { actorUserId: request.userId } : {}), }) } catch (error) { logger.error('Workspace file secret provenance could not be verified', {