fix(chat): stop classifying secret-free binary sandbox exports as unknown (#6349)

icecrasher321 · claude · web-flow · commit 5ad820daf636 · 2026-08-06T17:15:05.000-07:00
* fix(execution): stop classifying secret-free binary sandbox exports as unknown * fix(execution): fail closed when files are mounted without a provenance envelope The binary classifier read an absent mounted-file scanner as "no mounted secrets". That is absence of evidence, not evidence of absence: the request contract permits _sandboxFiles without the provenance envelope, so a caller that mounts secret-bearing bytes and omits the envelope would have a derived binary persisted as provably secret-free. Not reachable today — the route is internal-JWT-only and its one file-mounting caller always emits the envelope — but the classification rested on an invariant nothing enforced. - the copilot handler emits the envelope on the same condition that produces the mount, so tables ship one too and the two cannot drift apart - a mount with no verified scanner now counts as secret material in scope, so the classification is never stronger than what the caller attested to Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(execution): treat partial and unscannable mount attestations as unknown Two ways the envelope could read as stronger evidence than it was. The copilot handler preserved `_sandboxFiles` that arrived on the params and then exported provenance from `mountedRegistry`, which knows only about the files it resolved itself. The route would have read that partial envelope as a complete attestation over every mounted byte. The envelope now covers the whole mounted set or is not emitted at all, and a mount with no envelope already fails closed. `hasSecrets` was derived from whether entries produced scannable literals, so an envelope listing entries that all failed to decrypt reported false and let a derived binary be marked exact-empty. It now reflects what the envelope attested to: entries that yield no plaintext make the mount less classifiable, not more. Neither was reachable — `_sandboxFiles` is absent from the copilot tool schema, so nothing can populate the preserved-mount branch — but both had the classification resting on a property nothing enforced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(tools): regenerate stale tool metadata `bun run tool-metadata:check` fails on origin/staging as well as here, so this is not from this branch — #6317 landed the artifact generated from a factory that still built a per-provider apiKey description, and the source was later genericized without regenerating. Regenerating changes exactly the five embeddings entries' apiKey description to the text `tools/embeddings/factory.ts:74` actually produces. The per-provider strings appear nowhere in source. Included here only because the gate is red on every branch cut from staging until someone lands it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(execution): count the runtime payload as secret material in scope An execution with no mounted files and no env secret still carries `params` and `contextVariables` into the sandbox — the runtime payload is serialized into a private-input file, so resolved block outputs and workflow variables land as plaintext regardless of `_sandboxFiles`. The scope predicate only looked at mounts and env secrets, so a binary derived from them was classified exact-empty. The route has no catalog for those values and cannot tell a secret-bearing one from an ordinary one, so they count as in scope. Only an execution with nothing at all in scope earns an exact-empty binary. This narrows where the relaxation applies rather than regressing anything: every binary export was unknown before this branch, so a workflow Function block carrying block references keeps exactly the behavior it has today. The mothership path is unaffected — its tool sets no contextVariables, blockData, or workflowVariables, which is the case this branch exists to fix. Values, not keys, for the params check: `executionParams._context` is set to undefined before the context is built, so a key count reads every execution as carrying params. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Revert "fix(execution): count the runtime payload as secret material in scope" This reverts commit 754e37c. The classifier's secret catalog is the Secrets feature and nothing else: `outputSecretNamesByScanLiteral` and `outputSecretPlaintextsByName` are built only from `envVars`, and mounted-file entries trace back to the same place. `contextVariables`, `blockData`, and `workflowVariables` are ordinary workflow data — resolved block outputs the user already sees in logs — and the text export path does not scan them either. Treating their mere presence as secret material was a heuristic, not a security property, and it created exactly the asymmetry rejected two rounds earlier: a binary derived from a context variable would be `unknown` while a text export of the same bytes stays exact-empty. Stricter than the text path for the same content is not a boundary. It was also nearly inert. `scopeEnvironmentVariables` returns every workspace secret when scope is `all` (the default), so any workflow Function block with secrets configured already trips the env branch. The only slice it changed was executions with no env vars at all, where the workspace has no secret for a context variable to carry. A Secret resolved into an upstream block's output and arriving here through blockData is a real gap, but it is pre-existing, identical for text exports, and belongs at the executor -> route boundary as a provenance envelope for params — not as a presence check in this classifier. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
diff --git a/apps/sim/app/api/function/execute/route.test.ts b/apps/sim/app/api/function/execute/route.test.ts
@@ -858,6 +858,124 @@ describe('Function Execute API Route', () => {
       expect(JSON.stringify(data)).toContain('files/report-secret-value.txt')
     })
 
+    it('classifies a binary export exact-empty when no secret was in scope', async () => {
+      envFlagsMock.isRemoteSandboxEnabled = true
+      mockExecuteInSandbox.mockResolvedValueOnce({
+        result: 'done',
+        stdout: '',
+        sandboxId: 'sandbox-123',
+        exportedFiles: { '/home/user/small.jpg': '/9j/4AAQ' },
+      })
+
+      const response = await POST(
+        createMockRequest('POST', {
+          code: 'print("done")',
+          language: 'python',
+          workspaceId: 'workspace-1',
+          outputs: {
+            files: [
+              {
+                path: 'files/small.jpg',
+                sandboxPath: '/home/user/small.jpg',
+                mimeType: 'image/jpeg',
+              },
+            ],
+          },
+        })
+      )
+
+      expect(response.status).toBe(200)
+      expect(mockWriteWorkspaceFileByPath).toHaveBeenCalledWith(
+        expect.objectContaining({ secretProvenance: { status: 'exact', entries: [] } })
+      )
+    })
+
+    it('keeps a binary export unknown when files were mounted without a provenance envelope', async () => {
+      envFlagsMock.isRemoteSandboxEnabled = true
+      mockExecuteInSandbox.mockResolvedValueOnce({
+        result: 'done',
+        stdout: '',
+        sandboxId: 'sandbox-123',
+        exportedFiles: { '/home/user/small.jpg': '/9j/4AAQ' },
+      })
+
+      const response = await POST(
+        createMockRequest('POST', {
+          code: 'print("done")',
+          language: 'python',
+          workspaceId: 'workspace-1',
+          _sandboxFiles: [{ path: '/home/user/in.bin', content: 'mounted bytes' }],
+          outputs: {
+            files: [
+              {
+                path: 'files/small.jpg',
+                sandboxPath: '/home/user/small.jpg',
+                mimeType: 'image/jpeg',
+              },
+            ],
+          },
+        })
+      )
+
+      expect(response.status).toBe(200)
+      expect(mockWriteWorkspaceFileByPath).toHaveBeenCalledWith(
+        expect.objectContaining({ secretProvenance: { status: 'unknown' } })
+      )
+    })
+
+    it('keeps a binary export unknown when a mounted input file carried a secret', async () => {
+      envFlagsMock.isRemoteSandboxEnabled = true
+      mockExecuteInSandbox.mockResolvedValueOnce({
+        result: 'done',
+        stdout: '',
+        sandboxId: 'sandbox-123',
+        exportedFiles: { '/home/user/small.jpg': '/9j/4AAQ' },
+      })
+
+      const response = await POST(
+        createMockRequest(
+          'POST',
+          {
+            code: 'print("done")',
+            language: 'python',
+            workspaceId: 'workspace-1',
+            outputs: {
+              files: [
+                {
+                  path: 'files/small.jpg',
+                  sandboxPath: '/home/user/small.jpg',
+                  mimeType: 'image/jpeg',
+                },
+              ],
+            },
+            [PRIVATE_SECRET_PROVENANCE_FIELD]: {
+              version: 1,
+              complete: true,
+              selections: [
+                {
+                  key: MOUNTED_WORKSPACE_FILES_PROVENANCE_KEY,
+                  provenance: {
+                    version: 1,
+                    complete: true,
+                    entries: [{ encryptedValue: 'encrypted:mounted-secret' }],
+                    scope: { userId: 'user-123', workspaceId: 'workspace-1' },
+                  },
+                },
+              ],
+            },
+          },
+          {
+            [PRIVATE_SECRET_PROVENANCE_HEADER]: PRIVATE_SECRET_PROVENANCE_BUNDLE_V1,
+          }
+        )
+      )
+
+      expect(response.status).toBe(200)
+      expect(mockWriteWorkspaceFileByPath).toHaveBeenCalledWith(
+        expect.objectContaining({ secretProvenance: { status: 'unknown' } })
+      )
+    })
+
     it('marks binary exports unknown without failing the Function execution', async () => {
       envFlagsMock.isRemoteSandboxEnabled = true
       mockExecuteInSandbox.mockResolvedValueOnce({
diff --git a/apps/sim/app/api/function/execute/route.ts b/apps/sim/app/api/function/execute/route.ts
@@ -83,6 +83,7 @@ import {
   resolveWorkspaceFileReference,
 } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
 import {
+  EXACT_EMPTY_WORKSPACE_FILE_SECRET_PROVENANCE,
   mergeWorkspaceFileSecretProvenance,
   type WorkspaceFileSecretProvenance,
 } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance'
@@ -988,6 +989,7 @@ interface FunctionRouteExecutionContext {
   outputSecretNamesByScanLiteral: Map<string, string[]>
   outputSecretPlaintextsByName: Map<string, string>
   mountedFileSecretProvenanceScanner?: MountedFileSecretProvenanceScanner
+  hasMountedSandboxFiles: boolean
 }
 
 type ResolvedSecretNamesMetadataType =
@@ -1208,13 +1210,42 @@ function activateOutputSecretProvenance(
   }
 }
 
+/**
+ * True when any secret material was in scope for this execution — a mounted environment secret, or
+ * a secret carried by a mounted input file. When false, nothing secret ever reached the sandbox, so
+ * no export of any kind can carry one.
+ *
+ * Mounted bytes are classified from the caller's provenance envelope. Files mounted *without* one
+ * are unclassifiable rather than clean: absence of an envelope is absence of evidence, not evidence
+ * the mount carried nothing. Those fail closed here so the classification can never be stronger
+ * than what the caller actually attested to.
+ */
+function hasSecretMaterialInScope(context: FunctionRouteExecutionContext): boolean {
+  if (context.outputSecretPlaintextsByName.size > 0) return true
+  const scanner = context.mountedFileSecretProvenanceScanner
+  return scanner ? scanner.hasSecrets : context.hasMountedSandboxFiles
+}
+
+/**
+ * Classifies the secret provenance of one exported sandbox file.
+ *
+ * Text exports are scanned for the exact resolved-secret plaintexts in scope. Binary exports cannot
+ * be scanned soundly — re-encoding can carry a secret without leaving a literal substring — so they
+ * are classified only when no secret material was in scope at all; with nothing available to embed,
+ * the bytes are provably secret-free. Otherwise they stay unknown, which fails closed at every
+ * model and runtime boundary that later reads the file.
+ */
 async function getOutputFileSecretProvenance(
   buffer: Buffer,
   isBinary: boolean,
   context: FunctionRouteExecutionContext,
   scope: { userId: string; workspaceId: string }
 ): Promise<WorkspaceFileSecretProvenance> {
-  if (isBinary) return { status: 'unknown' }
+  if (isBinary) {
+    return hasSecretMaterialInScope(context)
+      ? { status: 'unknown' }
+      : EXACT_EMPTY_WORKSPACE_FILE_SECRET_PROVENANCE
+  }
   const mountedFileProvenance = context.mountedFileSecretProvenanceScanner?.scan(buffer) ?? {
     status: 'exact' as const,
     entries: [],
@@ -1999,6 +2030,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
       outputSecretNamesByScanLiteral: new Map(),
       outputSecretPlaintextsByName: new Map(),
       mountedFileSecretProvenanceScanner,
+      hasMountedSandboxFiles: (_sandboxFiles?.length ?? 0) > 0,
     }
     for (const [name, plaintext] of Object.entries(envVars)) {
       if (!plaintext) continue
diff --git a/apps/sim/lib/copilot/tools/handlers/function-execute.test.ts b/apps/sim/lib/copilot/tools/handlers/function-execute.test.ts
@@ -759,6 +759,22 @@ describe('executeFunctionExecute file mounts', () => {
     expect(mockExecuteTool).not.toHaveBeenCalled()
   })
 
+  it('omits the envelope when mounts it cannot attest to are already on the params', async () => {
+    mockExecuteTool.mockResolvedValue({ success: true, output: { result: 'ok' } })
+
+    await executeFunctionExecute(
+      {
+        inputFiles: ['files/data.csv'],
+        _sandboxFiles: [{ path: '/home/user/preserved.bin', content: 'from another resolver' }],
+      },
+      context
+    )
+
+    const call = mockExecuteTool.mock.calls[0]?.[1]
+    expect(call?._sandboxFiles?.length).toBeGreaterThan(1)
+    expect(call?.[PRIVATE_SECRET_PROVENANCE_FIELD]).toBeUndefined()
+  })
+
   it('projects only mounted-file secrets that cross the settled Function result', async () => {
     mockImportWorkspaceFileSecretProvenanceForRuntime.mockImplementation(
       async ({ registry }: { registry?: ResolvedSecretTraceRegistry }) =>
diff --git a/apps/sim/lib/copilot/tools/handlers/function-execute.ts b/apps/sim/lib/copilot/tools/handlers/function-execute.ts
@@ -636,21 +636,30 @@ export async function executeFunctionExecute(
           secretActorUserId ?? context.userId,
           mountedRegistry
         )
+        // Every mount ships its provenance envelope, tables included. The route classifies an
+        // output file from that envelope, so a mount without one is unclassifiable there — it
+        // cannot tell "nothing secret was mounted" from "nobody said". Emitting on the same
+        // condition that produces the mount keeps the two from drifting apart.
         if (resolved.length > 0) {
           const existing = (enrichedParams._sandboxFiles as SandboxFile[]) || []
           enrichedParams._sandboxFiles = [...existing, ...resolved]
-        }
 
-        if (inputFiles.length > 0 || inputDirectories.length > 0) {
-          const provenance = mountedRegistry.exportProvenance()
-          const bundle: PrivateSecretProvenanceBundleV1 = {
-            version: 1,
-            complete: provenance.complete,
-            selections: provenance.complete
-              ? [{ key: MOUNTED_WORKSPACE_FILES_PROVENANCE_KEY, provenance }]
-              : [],
+          // The envelope attests to the WHOLE mounted set or it is not emitted at all. Mounts
+          // that arrived already on the params came from outside this resolver, so
+          // `mountedRegistry` knows nothing about them; an envelope covering only `resolved`
+          // would read at the route as a complete attestation over every mounted byte. With no
+          // envelope the route fails closed instead, which is the honest answer.
+          if (existing.length === 0) {
+            const provenance = mountedRegistry.exportProvenance()
+            const bundle: PrivateSecretProvenanceBundleV1 = {
+              version: 1,
+              complete: provenance.complete,
+              selections: provenance.complete
+                ? [{ key: MOUNTED_WORKSPACE_FILES_PROVENANCE_KEY, provenance }]
+                : [],
+            }
+            enrichedParams[PRIVATE_SECRET_PROVENANCE_FIELD] = bundle
           }
-          enrichedParams[PRIVATE_SECRET_PROVENANCE_FIELD] = bundle
         }
       }
     }
diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts
@@ -292,6 +292,50 @@ describe('vfs handlers oversize policy', () => {
     )
   })
 
+  it('windows against the real line count when a read under-reports totalLines', async () => {
+    const vfs = makeVfs()
+    vfs.readFileContentWithProvenance.mockResolvedValue({
+      // `/extract` synthesizes a whole extracted document but reports totalLines: 1.
+      value: { content: 'page one\npage two\npage three', totalLines: 1 },
+      file: { fileId: 'file-1', key: 'workspace/key-1', context: 'workspace' },
+    })
+    getOrMaterializeVFS.mockResolvedValue(vfs)
+
+    const result = await executeVfsRead(
+      { path: 'files/report.pdf/extract', offset: 1, limit: 2 },
+      GREP_CTX
+    )
+
+    expect(result.success).toBe(true)
+    expect(result.output).toEqual({ content: 'page two\npage three', totalLines: 1 })
+  })
+
+  it('leaves an attachment read unwindowed so its label is never blanked', async () => {
+    const vfs = makeVfs()
+    const imageResult = {
+      content: 'Image: photo.jpeg (157.0KB, image/jpeg, resized for vision)',
+      totalLines: 1,
+      attachment: {
+        type: 'image',
+        name: 'photo.jpeg',
+        source: { type: 'base64', media_type: 'image/jpeg', data: 'AAAA' },
+      },
+    }
+    vfs.readFileContentWithProvenance.mockResolvedValue({
+      value: imageResult,
+      file: { fileId: 'file-1', key: 'workspace/key-1', context: 'workspace' },
+    })
+    getOrMaterializeVFS.mockResolvedValue(vfs)
+
+    const result = await executeVfsRead(
+      { path: 'files/photo.jpeg/content', offset: 1, limit: 100 },
+      GREP_CTX
+    )
+
+    expect(result.success).toBe(true)
+    expect(result.output).toEqual(imageResult)
+  })
+
   it('rejects only the file read when durable provenance cannot be verified', async () => {
     const vfs = makeVfs()
     vfs.readFileContentWithProvenance.mockResolvedValue({
diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.ts b/apps/sim/lib/copilot/tools/handlers/vfs.ts
@@ -285,12 +285,21 @@ export async function executeVfsRead(
     }
     const offset = parseOptionalNumber(params.offset)
     const limit = parseOptionalNumber(params.limit)
+    /**
+     * Applies the caller's line window, clamped against the content's ACTUAL line count rather than
+     * the self-reported `totalLines`. Synthesized results report `totalLines: 1` for content that is
+     * not one line (e.g. `files/x.pdf/extract` returns a whole extracted document), and clamping to
+     * that collapses the read to its first line — or to nothing for any nonzero offset. An
+     * attachment result is skipped outright: its `content` is a one-line label beside the bytes, so
+     * a window can only blank the label while the model still receives the attachment.
+     */
     const applyWindow = <T extends { content: string; totalLines: number }>(result: T): T => {
       if (offset === undefined && limit === undefined) return result
+      if (hasModelAttachment(result)) return result
       const lines = result.content.split('\n')
-      const start = Math.max(0, Math.min(result.totalLines, offset ?? 0))
-      const endRaw = limit !== undefined ? start + Math.max(0, limit) : result.totalLines
-      const end = Math.max(start, Math.min(result.totalLines, endRaw))
+      const start = Math.max(0, Math.min(lines.length, offset ?? 0))
+      const endRaw = limit !== undefined ? start + Math.max(0, limit) : lines.length
+      const end = Math.max(start, Math.min(lines.length, endRaw))
       return {
         ...result,
         content: lines.slice(start, end).join('\n'),
diff --git a/apps/sim/lib/execution/mounted-file-secret-provenance.test.ts b/apps/sim/lib/execution/mounted-file-secret-provenance.test.ts
@@ -52,6 +52,38 @@ describe('mounted file output provenance scanner', () => {
     })
   })
 
+  it('reports whether the mount carried any secret material', async () => {
+    const withSecrets = await createMountedFileSecretProvenanceScanner({
+      version: 1,
+      complete: true,
+      entries: [{ encryptedValue: 'encrypted-a' }],
+      scope: { userId: 'user-1', workspaceId: 'workspace-1' },
+    })
+    expect(withSecrets?.hasSecrets).toBe(true)
+
+    const withoutSecrets = await createMountedFileSecretProvenanceScanner({
+      version: 1,
+      complete: true,
+      entries: [],
+      scope: { userId: 'user-1', workspaceId: 'workspace-1' },
+    })
+    expect(withoutSecrets?.hasSecrets).toBe(false)
+    expect(withoutSecrets?.scan(Buffer.from('anything'))).toEqual({ status: 'exact', entries: [] })
+  })
+
+  it('keeps hasSecrets true when attested entries yield no scannable plaintext', async () => {
+    encryptionMockFns.mockDecryptSecret.mockImplementation(async () => ({ decrypted: '' }))
+
+    const scanner = await createMountedFileSecretProvenanceScanner({
+      version: 1,
+      complete: true,
+      entries: [{ encryptedValue: 'encrypted-a' }],
+      scope: { userId: 'user-1', workspaceId: 'workspace-1' },
+    })
+
+    expect(scanner?.hasSecrets).toBe(true)
+  })
+
   it('fails closed when encrypted provenance is incomplete or cannot be decrypted', async () => {
     await expect(
       createMountedFileSecretProvenanceScanner({ version: 1, complete: false, entries: [] })
diff --git a/apps/sim/lib/execution/mounted-file-secret-provenance.ts b/apps/sim/lib/execution/mounted-file-secret-provenance.ts
