fix(files): refuse an unknown file per read instead of blanking the Chat tool result (#8922)

* fix(files): refuse an unknown file per read instead of blanking the Chat tool result

A Chat CLI text read of a file whose secret provenance is unknown imported
nothing, marked the invocation's registry permanently incomplete, and still
returned the text. The egress projection then withheld the whole tool result,
so one tainted file blanked every match of a workspace-wide `grep --scope files`.
Forwarded version and bulk reads refused the bytes and also poisoned the
registry.

Text reads and forwarded responses now refuse just that file before its bytes
load (403, "File cannot be sent to a model ..."), so grep reports it as
unreadable and returns the clean matches. Direct downloads keep delivering and
marking the invocation incomplete, because a workbench copy must carry the
unknown classification onto anything derived from it.

The delivery observer now receives the canonical file id, so the refusal log
names the file it refused.

* test(files): expect a per-file refusal for an unknown historical version read

* fix(files): attribute each forwarded delivery's refusal to its own file

Author: waleedlatif1

apps/sim/lib/mothership/agent-cli/file-provenance.test.ts

@@ -4,6 +4,7 @@ import { queueTableRows, resetDbChainMock } from '@sim/testing'
 import { createPersonalApiKeyPrincipal } from '@sim/testing/factories/principal.factory'
 import { encryptionMock, encryptionMockFns } from '@sim/testing/mocks/encryption.mock'
 import { fileParsersMock, fileParsersMockFns } from '@sim/testing/mocks/file-parsers.mock'
+import { getMockLogger } from '@sim/testing/mocks/logger.mock'
 import {
   mothershipWorkspaceTargetMock,
   mothershipWorkspaceTargetMockFns,
@@ -53,6 +54,7 @@ vi.mock('@/lib/mothership/agent-cli/scoped-transport', () => ({
 import { createFileReadTransport } from '@/lib/mothership/agent-cli/file-read-transport'
 import { inspectToolResultForCopilot } from '@/lib/mothership/request/tools/resolved-secret-result'
 import { executeSimCli } from '@/lib/mothership/tools/handlers/sim-cli'
+import { reportWorkspaceFileDelivery } from '@/lib/workspace-files/application/file-delivery-observer'
 import { readWorkspaceFileArtifact } from '@/lib/workspace-files/application/read-workspace-file-artifact'
 import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
 
@@ -98,17 +100,48 @@ function registry() {
   return new ResolvedSecretTraceRegistry([], { userId: 'reader', workspaceId: 'workspace' })
 }
 
-function classify(status: 'exact' | 'unknown', fileRevision = revision) {
+type SidecarStatus = 'exact' | 'unknown'
+
+function sidecarRow(status: SidecarStatus, fileRevision = revision, entries: unknown[] = []) {
+  return {
+    fileContentUpdatedAt: fileRevision,
+    secretProvenanceVersion: 1,
+    provenanceContentUpdatedAt: fileRevision,
+    status,
+    entries,
+  }
+}
+
+/** Queues one sidecar per status, in the order the reads consume them. */
+function classify(...statuses: [SidecarStatus, ...SidecarStatus[]]) {
   resetDbChainMock()
-  queueTableRows(workspaceFiles, [
+  for (const status of statuses) queueTableRows(workspaceFiles, [sidecarRow(status)])
+}
+
+type CliInvocation = Parameters<typeof executeSimCli>[0]['request']['invocation']
+
+const readFile: CliInvocation = {
+  kind: 'augmentation',
+  name: 'files read',
+  positionals: ['file'],
+  flags: {},
+}
+
+function grepFiles(pattern: string): CliInvocation {
+  return { kind: 'augmentation', name: 'grep', positionals: [pattern], flags: { scope: 'files' } }
+}
+
+function runCli(invocation: CliInvocation, trace = registry()) {
+  return executeSimCli(
+    { request: { invocation } },
     {
-      fileContentUpdatedAt: fileRevision,
-      secretProvenanceVersion: 1,
-      provenanceContentUpdatedAt: fileRevision,
-      status,
-      entries: [],
-    },
-  ])
+      userId: 'reader',
+      workspaceId: 'workspace',
+      workflowId: '',
+      chatId: 'chat',
+      resolvedSecretTraceRegistry: trace,
+    }
+  )
 }
 
 function readTransport(trace = registry()) {
@@ -278,14 +311,53 @@ describe('file provenance at the actual CLI and model-result boundary', () => {
     expect(mocks.stream).toHaveBeenCalledTimes(1)
   })
 
-  it('withholds extracted text whose canonical revision changed', async () => {
-    classify('exact', new Date(revision.getTime() + 1))
+  it('refuses a forwarded response naming its unknown file, without blanking the invocation', async () => {
+    const refusals = getMockLogger('DurableSecretProvenancePersistence').error
+    refusals.mockClear()
+    const trace = registry()
+    const transport = createFileReadTransport({
+      endpoint: 'https://sim.test',
+      userId: 'reader',
+      registry: trace,
+      invocation: { userId: 'reader', workspaceId: 'workspace', chatId: 'chat' },
+      transport: async () => {
+        await reportWorkspaceFileDelivery({ status: 'exact', entries: [] }, 'clean')
+        await reportWorkspaceFileDelivery({ status: 'unknown' }, 'tainted')
+        return new Response(content)
+      },
+    })
+    const response = await transport(
+      new Request('https://sim.test/api/v2/files/bulk-download?workspaceId=workspace')
+    )
+    expect(response.status).toBe(403)
+    expect(await response.text()).not.toContain(content)
+    expect(trace.isPermanentlyIncomplete()).toBe(false)
+    expect(refusals).toHaveBeenCalledTimes(1)
+    expect(refusals).toHaveBeenCalledWith(
+      'Refusing unavailable durable secret provenance',
+      expect.objectContaining({ resourceId: 'tainted' })
+    )
+  })
+
+  it('refuses extracted text whose canonical revision changed before loading its bytes', async () => {
+    resetDbChainMock()
+    queueTableRows(workspaceFiles, [sidecarRow('exact', new Date(revision.getTime() + 1))])
     const trace = registry()
     const response = await readTransport(trace)(fileRequest('/text'))
+    expect(response.status).toBe(403)
+    expect(await response.text()).not.toContain(content)
+    expect(mocks.buffer).not.toHaveBeenCalled()
+    expect(trace.isPermanentlyIncomplete()).toBe(false)
+  })
+
+  it('still withholds a delivered text read once the registry is incomplete for another reason', async () => {
+    classify('exact')
+    const trace = registry()
+    trace.markIncomplete('provenance-capacity-exceeded')
+    const response = await readTransport(trace)(fileRequest('/text'))
     expect(response.status).toBe(200)
     const output = await response.text()
     expect(output).toContain(content)
-    expect(trace.isPermanentlyIncomplete()).toBe(true)
     expect(
       JSON.stringify(
         inspectToolResultForCopilot({ success: true, output }, trace, 'sim_cli').result
@@ -366,62 +438,83 @@ describe('file provenance at the actual CLI and model-result boundary', () => {
       classify('unknown')
       const grep = async () => {
         const trace = registry()
-        const result = await executeSimCli(
-          {
-            request: {
-              invocation: {
-                kind: 'augmentation',
-                name: 'grep',
-                positionals: [content],
-                flags: { scope: 'files' },
-              },
-            },
-          },
-          {
-            userId: 'reader',
-            workspaceId: 'workspace',
-            workflowId: '',
-            chatId: 'chat',
-            resolvedSecretTraceRegistry: trace,
-          }
+        const result = await runCli(grepFiles(content), trace)
+        expect(result.error).toContain('Search incomplete')
+        expect(JSON.stringify(result.output)).toContain(
+          'files/private.txt (file): could not read text'
         )
-        expect(result.success).toBe(true)
-        expect(JSON.stringify(result.output)).toContain(content)
-        expect(
-          JSON.stringify(inspectToolResultForCopilot(result, trace, 'sim_cli').result)
-        ).not.toContain(content)
+        expect(JSON.stringify(result.output)).toContain('No matches')
+        expect(trace.isPermanentlyIncomplete()).toBe(false)
       }
       if (mode === 'parallel') await Promise.all([grep(), grep()])
       else {
         await grep()
         await grep()
       }
-      expect(mocks.buffer).toHaveBeenCalledTimes(2)
+      expect(mocks.buffer).not.toHaveBeenCalled()
     }
   )
 
-  it('carries unified text reads through the actual CLI transport and secret projection', async () => {
-    classify('unknown')
+  it('returns clean grep matches while refusing only the file whose provenance is unknown', async () => {
+    const tainted = { ...file, id: 'tainted', key: 'workspace/workspace/tainted-key' }
+    const taintedText = 'TAINTED_FILE_CANARY_FOR_LOCAL_TEST'
+    const byId = (id: string) => (id === tainted.id ? tainted : file)
+    mocks.file.mockImplementation(async (_workspaceId: string, id: string) => byId(id))
+    mocks.reference.mockImplementation(async (_workspaceId: string, id: string) => byId(id))
+    mocks.context.mockImplementation(async (id: string) => ({
+      workspaceId: 'workspace',
+      fileId: id,
+      workspaceOrganizationId: null,
+      allowPersonalApiKeys: true,
+      billedAccountUserId: 'owner',
+    }))
+    mocks.buffer.mockImplementation(async (record: { id: string }) =>
+      Buffer.from(record.id === tainted.id ? taintedText : content)
+    )
+    vi.stubGlobal('fetch', async () =>
+      Response.json({
+        data: [file, tainted].map((record) => ({ ...record, updatedAt: revision.toISOString() })),
+        nextCursor: null,
+      })
+    )
+    classify('exact', 'unknown')
+    const refusals = getMockLogger('DurableSecretProvenancePersistence').error
+    refusals.mockClear()
     const trace = registry()
-    const result = await executeSimCli(
-      {
-        request: {
-          invocation: {
-            kind: 'augmentation',
-            name: 'files read',
-            positionals: ['file'],
-            flags: {},
-          },
-        },
-      },
-      {
-        userId: 'reader',
-        workspaceId: 'workspace',
-        workflowId: '',
-        chatId: 'chat',
-        resolvedSecretTraceRegistry: trace,
-      }
+    const result = await runCli(grepFiles('CANARY_FOR_LOCAL_TEST'), trace)
+    const projected = JSON.stringify(inspectToolResultForCopilot(result, trace, 'sim_cli').result)
+    expect(trace.isPermanentlyIncomplete()).toBe(false)
+    expect(projected).not.toContain(taintedText)
+    expect(projected).toContain(content)
+    expect(mocks.buffer).toHaveBeenCalledTimes(1)
+    expect(refusals).toHaveBeenCalledTimes(1)
+    expect(refusals).toHaveBeenCalledWith(
+      'Refusing unavailable durable secret provenance',
+      expect.objectContaining({
+        cause: 'workspace-file-provenance-unavailable',
+        resourceId: tainted.id,
+      })
     )
+  })
+
+  it('refuses an unknown file through the actual CLI text read without loading its bytes', async () => {
+    classify('unknown')
+    const result = await runCli(readFile)
+    expect(result.success).toBe(false)
+    expect(JSON.stringify(result.output)).not.toContain(content)
+    expect(JSON.stringify(result)).toContain('secret provenance is unavailable')
+    expect(mocks.buffer).not.toHaveBeenCalled()
+  })
+
+  it('carries unified text reads through the actual CLI transport and secret projection', async () => {
+    resetDbChainMock()
+    queueTableRows(workspaceFiles, [
+      sidecarRow('exact', revision, [
+        { name: 'FILE_SECRET', encryptedValue: 'fixture-ciphertext', sourceUserId: 'reader' },
+      ]),
+    ])
+    const trace = registry()
+    const result = await runCli(readFile, trace)
     expect(result.success).toBe(true)
     expect(JSON.stringify(result.output)).toContain(content)
     expect(JSON.stringify(result.output)).toContain('representation')

apps/sim/lib/mothership/agent-cli/file-read-transport.ts

@@ -13,12 +13,14 @@ import {
 import { readV2CredentialHeaders } from '@/lib/api/server/routes/v2-credential-headers'
 import { V2_PARSE_DEFAULTS } from '@/lib/api/server/routes/v2-json-route'
 import { parseRequest } from '@/lib/api/server/validation'
+import { OrchestrationError } from '@/lib/core/orchestration/types'
 import {
   type CopilotChatDelegationContext,
   createCopilotChatPrincipal,
 } from '@/lib/mothership/auth/application-delegation'
 import {
   importWorkspaceFileSnapshotProvenance,
+  MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE,
   mergeWorkspaceFileSecretProvenance,
   type WorkspaceFileSecretProvenance,
 } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance'
@@ -52,18 +54,33 @@ export function createFileReadTransport(context: {
     `${basePath}${v2ListFileFoldersContract.path}`,
     `${basePath}${v2BulkDownloadFilesContract.path}`,
   ])
-  const observe = async (workspaceId: string, provenance?: WorkspaceFileSecretProvenance) => {
-    const imported =
-      provenance &&
-      (await importWorkspaceFileSnapshotProvenance({
-        workspaceId,
-        provenance,
-        registry: context.registry,
-        actorUserId: context.userId,
-      }))
-    if (!imported) context.registry?.markIncomplete('workspace-file-provenance-unknown')
-    return imported
-  }
+  /**
+   * Admits one delivery's classification into the invocation registry. A delivery it refuses is
+   * either withheld (text and forwarded responses reach only the model, so refusing them is enough)
+   * or, for a direct download whose bytes can be persisted, delivered with the whole invocation
+   * marked incomplete so anything derived from the copy keeps the unknown classification.
+   */
+  const admit = async (
+    workspaceId: string,
+    provenance: WorkspaceFileSecretProvenance | undefined,
+    resourceId?: string
+  ): Promise<boolean> =>
+    provenance !== undefined &&
+    (await importWorkspaceFileSnapshotProvenance({
+      workspaceId,
+      provenance,
+      registry: context.registry,
+      actorUserId: context.userId,
+      resourceId,
+    }))
+  const refused = () =>
+    v2FileErrorPolicies.concealResourceAuthorization.render(
+      new OrchestrationError('forbidden', MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE)
+    ) ??
+    Response.json(
+      { error: { message: MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE } },
+      { status: 403 }
+    )
 
   const forward: typeof fetch = async (input, init) => {
     const url = new URL(input instanceof Request ? input.url : input)
@@ -72,8 +89,10 @@ export function createFileReadTransport(context: {
       return (context.transport ?? fetch)(input, init)
     }
     let provenance: WorkspaceFileSecretProvenance | undefined
+    const deliveries: { source: WorkspaceFileSecretProvenance | undefined; fileId: string }[] = []
     const response = await observeWorkspaceFileDelivery(
-      async (source) => {
+      async (source, fileId) => {
+        deliveries.push({ source, fileId })
         provenance = mergeWorkspaceFileSecretProvenance(
           provenance ?? { status: 'exact', entries: [] },
           source ?? { status: 'unknown' }
@@ -83,13 +102,20 @@ export function createFileReadTransport(context: {
     )
     if (!provenance) return response
     try {
-      if (!context.registry || !(await observe(context.invocation.workspaceId, provenance))) {
+      if (!context.registry) {
         await response.body?.cancel().catch(() => {})
         return Response.json(
           { error: { message: 'File read provenance is unavailable. Retry the read.' } },
           { status: 503 }
         )
       }
+      /** Each delivery is admitted under its own file id, so a refusal names the file it refused. */
+      for (const { source, fileId } of deliveries) {
+        if (!(await admit(context.invocation.workspaceId, source, fileId))) {
+          await response.body?.cancel().catch(() => {})
+          return refused()
+        }
+      }
       if (response.body) context.trackDownload?.(response.body, provenance)
       return response
     } catch (error) {
@@ -143,8 +169,10 @@ export function createFileReadTransport(context: {
         if (!parsed.success) return parsed.response
         request.signal.throwIfAborted()
         const result = await observeWorkspaceFileDelivery(
-          async (source) => {
-            await observe(parsed.data.query.workspaceId, source)
+          async (source, fileId) => {
+            if (!(await admit(parsed.data.query.workspaceId, source, fileId))) {
+              throw new OrchestrationError('forbidden', MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE)
+            }
           },
           () =>
             readWorkspaceFileText.execute({
@@ -173,8 +201,10 @@ export function createFileReadTransport(context: {
       if (!parsed.success) return parsed.response
       request.signal.throwIfAborted()
       const result = await observeWorkspaceFileDelivery(
-        async (source) => {
-          await observe(parsed.data.query.workspaceId, source)
+        async (source, fileId) => {
+          if (!(await admit(parsed.data.query.workspaceId, source, fileId))) {
+            context.registry?.markIncomplete('workspace-file-provenance-unknown')
+          }
         },
         () =>
           downloadWorkspaceFileStream.execute({