Skip to content

Commit 356b79e

Browse files
committed
fix(execution): stop classifying secret-free binary sandbox exports as unknown
1 parent f76d46b commit 356b79e

7 files changed

Lines changed: 197 additions & 7 deletions

File tree

apps/sim/app/api/function/execute/route.test.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,91 @@ describe('Function Execute API Route', () => {
858858
expect(JSON.stringify(data)).toContain('files/report-secret-value.txt')
859859
})
860860

861+
it('classifies a binary export exact-empty when no secret was in scope', async () => {
862+
envFlagsMock.isRemoteSandboxEnabled = true
863+
mockExecuteInSandbox.mockResolvedValueOnce({
864+
result: 'done',
865+
stdout: '',
866+
sandboxId: 'sandbox-123',
867+
exportedFiles: { '/home/user/small.jpg': '/9j/4AAQ' },
868+
})
869+
870+
const response = await POST(
871+
createMockRequest('POST', {
872+
code: 'print("done")',
873+
language: 'python',
874+
workspaceId: 'workspace-1',
875+
outputs: {
876+
files: [
877+
{
878+
path: 'files/small.jpg',
879+
sandboxPath: '/home/user/small.jpg',
880+
mimeType: 'image/jpeg',
881+
},
882+
],
883+
},
884+
})
885+
)
886+
887+
expect(response.status).toBe(200)
888+
expect(mockWriteWorkspaceFileByPath).toHaveBeenCalledWith(
889+
expect.objectContaining({ secretProvenance: { status: 'exact', entries: [] } })
890+
)
891+
})
892+
893+
it('keeps a binary export unknown when a mounted input file carried a secret', async () => {
894+
envFlagsMock.isRemoteSandboxEnabled = true
895+
mockExecuteInSandbox.mockResolvedValueOnce({
896+
result: 'done',
897+
stdout: '',
898+
sandboxId: 'sandbox-123',
899+
exportedFiles: { '/home/user/small.jpg': '/9j/4AAQ' },
900+
})
901+
902+
const response = await POST(
903+
createMockRequest(
904+
'POST',
905+
{
906+
code: 'print("done")',
907+
language: 'python',
908+
workspaceId: 'workspace-1',
909+
outputs: {
910+
files: [
911+
{
912+
path: 'files/small.jpg',
913+
sandboxPath: '/home/user/small.jpg',
914+
mimeType: 'image/jpeg',
915+
},
916+
],
917+
},
918+
[PRIVATE_SECRET_PROVENANCE_FIELD]: {
919+
version: 1,
920+
complete: true,
921+
selections: [
922+
{
923+
key: MOUNTED_WORKSPACE_FILES_PROVENANCE_KEY,
924+
provenance: {
925+
version: 1,
926+
complete: true,
927+
entries: [{ encryptedValue: 'encrypted:mounted-secret' }],
928+
scope: { userId: 'user-123', workspaceId: 'workspace-1' },
929+
},
930+
},
931+
],
932+
},
933+
},
934+
{
935+
[PRIVATE_SECRET_PROVENANCE_HEADER]: PRIVATE_SECRET_PROVENANCE_BUNDLE_V1,
936+
}
937+
)
938+
)
939+
940+
expect(response.status).toBe(200)
941+
expect(mockWriteWorkspaceFileByPath).toHaveBeenCalledWith(
942+
expect.objectContaining({ secretProvenance: { status: 'unknown' } })
943+
)
944+
})
945+
861946
it('marks binary exports unknown without failing the Function execution', async () => {
862947
envFlagsMock.isRemoteSandboxEnabled = true
863948
mockExecuteInSandbox.mockResolvedValueOnce({

apps/sim/app/api/function/execute/route.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ import {
8383
resolveWorkspaceFileReference,
8484
} from '@/lib/uploads/contexts/workspace/workspace-file-manager'
8585
import {
86+
EXACT_EMPTY_WORKSPACE_FILE_SECRET_PROVENANCE,
8687
mergeWorkspaceFileSecretProvenance,
8788
type WorkspaceFileSecretProvenance,
8889
} from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance'
@@ -1208,13 +1209,38 @@ function activateOutputSecretProvenance(
12081209
}
12091210
}
12101211

1212+
/**
1213+
* True when any secret material was in scope for this execution — a mounted environment secret, or
1214+
* a secret carried by a mounted input file. When false, nothing secret ever reached the sandbox, so
1215+
* no export of any kind can carry one.
1216+
*/
1217+
function hasSecretMaterialInScope(context: FunctionRouteExecutionContext): boolean {
1218+
return (
1219+
context.outputSecretPlaintextsByName.size > 0 ||
1220+
(context.mountedFileSecretProvenanceScanner?.hasSecrets ?? false)
1221+
)
1222+
}
1223+
1224+
/**
1225+
* Classifies the secret provenance of one exported sandbox file.
1226+
*
1227+
* Text exports are scanned for the exact resolved-secret plaintexts in scope. Binary exports cannot
1228+
* be scanned soundly — re-encoding can carry a secret without leaving a literal substring — so they
1229+
* are classified only when no secret material was in scope at all; with nothing available to embed,
1230+
* the bytes are provably secret-free. Otherwise they stay unknown, which fails closed at every
1231+
* model and runtime boundary that later reads the file.
1232+
*/
12111233
async function getOutputFileSecretProvenance(
12121234
buffer: Buffer,
12131235
isBinary: boolean,
12141236
context: FunctionRouteExecutionContext,
12151237
scope: { userId: string; workspaceId: string }
12161238
): Promise<WorkspaceFileSecretProvenance> {
1217-
if (isBinary) return { status: 'unknown' }
1239+
if (isBinary) {
1240+
return hasSecretMaterialInScope(context)
1241+
? { status: 'unknown' }
1242+
: EXACT_EMPTY_WORKSPACE_FILE_SECRET_PROVENANCE
1243+
}
12181244
const mountedFileProvenance = context.mountedFileSecretProvenanceScanner?.scan(buffer) ?? {
12191245
status: 'exact' as const,
12201246
entries: [],

apps/sim/lib/copilot/tools/handlers/vfs.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,50 @@ describe('vfs handlers oversize policy', () => {
292292
)
293293
})
294294

295+
it('windows against the real line count when a read under-reports totalLines', async () => {
296+
const vfs = makeVfs()
297+
vfs.readFileContentWithProvenance.mockResolvedValue({
298+
// `/extract` synthesizes a whole extracted document but reports totalLines: 1.
299+
value: { content: 'page one\npage two\npage three', totalLines: 1 },
300+
file: { fileId: 'file-1', key: 'workspace/key-1', context: 'workspace' },
301+
})
302+
getOrMaterializeVFS.mockResolvedValue(vfs)
303+
304+
const result = await executeVfsRead(
305+
{ path: 'files/report.pdf/extract', offset: 1, limit: 2 },
306+
GREP_CTX
307+
)
308+
309+
expect(result.success).toBe(true)
310+
expect(result.output).toEqual({ content: 'page two\npage three', totalLines: 1 })
311+
})
312+
313+
it('leaves an attachment read unwindowed so its label is never blanked', async () => {
314+
const vfs = makeVfs()
315+
const imageResult = {
316+
content: 'Image: photo.jpeg (157.0KB, image/jpeg, resized for vision)',
317+
totalLines: 1,
318+
attachment: {
319+
type: 'image',
320+
name: 'photo.jpeg',
321+
source: { type: 'base64', media_type: 'image/jpeg', data: 'AAAA' },
322+
},
323+
}
324+
vfs.readFileContentWithProvenance.mockResolvedValue({
325+
value: imageResult,
326+
file: { fileId: 'file-1', key: 'workspace/key-1', context: 'workspace' },
327+
})
328+
getOrMaterializeVFS.mockResolvedValue(vfs)
329+
330+
const result = await executeVfsRead(
331+
{ path: 'files/photo.jpeg/content', offset: 1, limit: 100 },
332+
GREP_CTX
333+
)
334+
335+
expect(result.success).toBe(true)
336+
expect(result.output).toEqual(imageResult)
337+
})
338+
295339
it('rejects only the file read when durable provenance cannot be verified', async () => {
296340
const vfs = makeVfs()
297341
vfs.readFileContentWithProvenance.mockResolvedValue({

apps/sim/lib/copilot/tools/handlers/vfs.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -285,12 +285,21 @@ export async function executeVfsRead(
285285
}
286286
const offset = parseOptionalNumber(params.offset)
287287
const limit = parseOptionalNumber(params.limit)
288+
/**
289+
* Applies the caller's line window, clamped against the content's ACTUAL line count rather than
290+
* the self-reported `totalLines`. Synthesized results report `totalLines: 1` for content that is
291+
* not one line (e.g. `files/x.pdf/extract` returns a whole extracted document), and clamping to
292+
* that collapses the read to its first line — or to nothing for any nonzero offset. An
293+
* attachment result is skipped outright: its `content` is a one-line label beside the bytes, so
294+
* a window can only blank the label while the model still receives the attachment.
295+
*/
288296
const applyWindow = <T extends { content: string; totalLines: number }>(result: T): T => {
289297
if (offset === undefined && limit === undefined) return result
298+
if (hasModelAttachment(result)) return result
290299
const lines = result.content.split('\n')
291-
const start = Math.max(0, Math.min(result.totalLines, offset ?? 0))
292-
const endRaw = limit !== undefined ? start + Math.max(0, limit) : result.totalLines
293-
const end = Math.max(start, Math.min(result.totalLines, endRaw))
300+
const start = Math.max(0, Math.min(lines.length, offset ?? 0))
301+
const endRaw = limit !== undefined ? start + Math.max(0, limit) : lines.length
302+
const end = Math.max(start, Math.min(lines.length, endRaw))
294303
return {
295304
...result,
296305
content: lines.slice(start, end).join('\n'),

apps/sim/lib/embeddings/client.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { createLogger } from '@sim/logger'
2-
import { chunkArray } from '@sim/utils'
2+
import { chunkArray } from '@sim/utils/helpers'
33
import { env, envNumber } from '@/lib/core/config/env'
44
import { mapWithConcurrency } from '@/lib/core/utils/concurrency'
55
import {

apps/sim/lib/execution/mounted-file-secret-provenance.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,25 @@ describe('mounted file output provenance scanner', () => {
5252
})
5353
})
5454

55+
it('reports whether the mount carried any secret material', async () => {
56+
const withSecrets = await createMountedFileSecretProvenanceScanner({
57+
version: 1,
58+
complete: true,
59+
entries: [{ encryptedValue: 'encrypted-a' }],
60+
scope: { userId: 'user-1', workspaceId: 'workspace-1' },
61+
})
62+
expect(withSecrets?.hasSecrets).toBe(true)
63+
64+
const withoutSecrets = await createMountedFileSecretProvenanceScanner({
65+
version: 1,
66+
complete: true,
67+
entries: [],
68+
scope: { userId: 'user-1', workspaceId: 'workspace-1' },
69+
})
70+
expect(withoutSecrets?.hasSecrets).toBe(false)
71+
expect(withoutSecrets?.scan(Buffer.from('anything'))).toEqual({ status: 'exact', entries: [] })
72+
})
73+
5574
it('fails closed when encrypted provenance is incomplete or cannot be decrypted', async () => {
5675
await expect(
5776
createMountedFileSecretProvenanceScanner({ version: 1, complete: false, entries: [] })

apps/sim/lib/execution/mounted-file-secret-provenance.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ const MAX_MOUNTED_FILE_SECRET_MATCH_EVENTS = 1_000_000
1111
const ANONYMOUS_MOUNTED_FILE_SECRET_NAME = 'MOUNTED_FILE_SECRET'
1212

1313
export interface MountedFileSecretProvenanceScanner {
14+
/**
15+
* True when the mount actually carried secret material. False means the mounted files were
16+
* classified and contributed nothing, so no mounted secret can reach an output file — which lets
17+
* callers classify content this scanner cannot soundly scan (binary bytes) instead of failing closed.
18+
*/
19+
hasSecrets: boolean
1420
scan(buffer: Buffer): WorkspaceFileSecretProvenance
1521
}
1622

@@ -57,7 +63,7 @@ export async function createMountedFileSecretProvenanceScanner(
5763
}
5864

5965
if (entriesByScanLiteral.size === 0) {
60-
return { scan: () => ({ status: 'exact', entries: [] }) }
66+
return { hasSecrets: false, scan: () => ({ status: 'exact', entries: [] }) }
6167
}
6268

6369
let matcher
@@ -69,10 +75,11 @@ export async function createMountedFileSecretProvenanceScanner(
6975
return undefined
7076
}
7177
if (!matcher) {
72-
return { scan: () => ({ status: 'exact', entries: [] }) }
78+
return { hasSecrets: false, scan: () => ({ status: 'exact', entries: [] }) }
7379
}
7480

7581
return {
82+
hasSecrets: true,
7683
scan(buffer) {
7784
const matched = new Map<string, WorkspaceFileSecretProvenanceEntry>()
7885
try {

0 commit comments

Comments
 (0)