Skip to content

Commit c04a28c

Browse files
fix(tables): snapshot every sandbox mount
1 parent d9c6858 commit c04a28c

2 files changed

Lines changed: 46 additions & 210 deletions

File tree

apps/sim/lib/copilot/tools/handlers/function-execute.test.ts

Lines changed: 3 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import {
1313
const {
1414
mockGetTableById,
1515
mockListTables,
16-
mockQueryRows,
1716
mockGetOrCreateTableSnapshot,
1817
mockDownloadFile,
1918
mockGeneratePresignedDownloadUrl,
@@ -32,12 +31,10 @@ const {
3231
mockMaterializeCopilotCodeSecrets,
3332
mockHasWorkspaceSandboxAccess,
3433
mockImportWorkspaceFileSecretProvenanceForRuntime,
35-
mockLoadTableRowSecretProvenance,
3634
mockIsTableSnapshotSafeForModelMount,
3735
} = vi.hoisted(() => ({
3836
mockGetTableById: vi.fn(),
3937
mockListTables: vi.fn(),
40-
mockQueryRows: vi.fn(),
4138
mockGetOrCreateTableSnapshot: vi.fn(),
4239
mockDownloadFile: vi.fn(),
4340
mockGeneratePresignedDownloadUrl: vi.fn(),
@@ -56,7 +53,6 @@ const {
5653
mockMaterializeCopilotCodeSecrets: vi.fn(),
5754
mockHasWorkspaceSandboxAccess: vi.fn(),
5855
mockImportWorkspaceFileSecretProvenanceForRuntime: vi.fn(),
59-
mockLoadTableRowSecretProvenance: vi.fn(),
6056
mockIsTableSnapshotSafeForModelMount: vi.fn(),
6157
}))
6258

@@ -65,10 +61,8 @@ vi.mock('@/lib/table/service', () => ({
6561
getTableById: mockGetTableById,
6662
listTables: mockListTables,
6763
}))
68-
vi.mock('@/lib/table/rows/service', () => ({ queryRows: mockQueryRows }))
6964
vi.mock('@/lib/table/rows/secret-provenance', () => ({
7065
isTableSnapshotSafeForModelMount: mockIsTableSnapshotSafeForModelMount,
71-
loadTableRowSecretProvenance: mockLoadTableRowSecretProvenance,
7266
}))
7367
vi.mock('@/lib/table/snapshot-cache', () => ({
7468
getOrCreateTableSnapshot: mockGetOrCreateTableSnapshot,
@@ -125,13 +119,12 @@ vi.mock('@/lib/execution/remote-sandbox/workspace-sandboxes', () => ({
125119
import { projectToolResultForCopilot } from '@/lib/copilot/request/tools/resolved-secret-result'
126120
import { executeFunctionExecute } from '@/lib/copilot/tools/handlers/function-execute'
127121
import { executeRunCode } from '@/lib/copilot/tools/handlers/run-code'
128-
import { TABLE_LIMITS } from '@/lib/table/constants'
129122
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
130123

131124
const table = {
132125
id: 'tbl_1',
133126
workspaceId: 'ws_1',
134-
rowCount: TABLE_LIMITS.DEFAULT_QUERY_LIMIT + 1,
127+
rowCount: 1,
135128
schema: { columns: [{ id: 'col_name', name: 'name', type: 'string' }] },
136129
}
137130

@@ -153,13 +146,7 @@ function resetExecutionMocks(): void {
153146
vi.clearAllMocks()
154147
mockExecuteTool.mockReset()
155148
mockMaterializeCopilotCodeSecrets.mockReset()
156-
mockLoadTableRowSecretProvenance.mockReset()
157149
mockIsTableSnapshotSafeForModelMount.mockReset()
158-
mockLoadTableRowSecretProvenance.mockResolvedValue({
159-
version: 1,
160-
complete: true,
161-
entries: [],
162-
})
163150
mockIsTableSnapshotSafeForModelMount.mockResolvedValue(true)
164151
mockListWorkspaceFiles.mockResolvedValue([])
165152
mockListWorkspaceFileFolders.mockResolvedValue([])
@@ -572,76 +559,12 @@ describe('executeFunctionExecute table mounts', () => {
572559
resetExecutionMocks()
573560
mockExecuteTool.mockResolvedValue({ success: true })
574561
mockGetTableById.mockResolvedValue(table)
575-
// Row data is keyed by stable column id at rest, not display name.
576-
mockQueryRows.mockResolvedValue({ rows: [{ data: { col_name: 'Ada' } }] })
577562
mockHasCloudStorage.mockReturnValue(true)
578563
mockGeneratePresignedDownloadUrl.mockResolvedValue('https://s3.example/presigned?sig=abc')
579564
})
580565

581-
it('mounts a table at the inline row limit without truncation', async () => {
582-
mockGetTableById.mockResolvedValue({ ...table, rowCount: TABLE_LIMITS.DEFAULT_QUERY_LIMIT })
583-
584-
await executeFunctionExecute({ inputTables: ['tbl_1'] }, context as never)
585-
586-
expect(mockQueryRows).toHaveBeenCalledWith(
587-
expect.objectContaining({ id: 'tbl_1' }),
588-
{ limit: TABLE_LIMITS.DEFAULT_QUERY_LIMIT },
589-
'copilot-fn-exec'
590-
)
591-
expect(mockGetOrCreateTableSnapshot).not.toHaveBeenCalled()
592-
const files = mountedFiles()
593-
expect(files[0].path).toBe('/home/user/tables/tbl_1.csv')
594-
expect(files[0].content).toBe('name\nAda')
595-
})
596-
597-
it('mounts CSV with display-name headers and id-keyed values, never column ids', async () => {
598-
mockGetTableById.mockResolvedValue({
599-
id: 'tbl_2',
600-
workspaceId: 'ws_1',
601-
rowCount: 2,
602-
schema: {
603-
columns: [
604-
{ id: 'col_name', name: 'name', type: 'string' },
605-
{ id: 'col_company', name: 'company', type: 'string' },
606-
],
607-
},
608-
})
609-
mockQueryRows.mockResolvedValue({
610-
rows: [
611-
{ data: { col_name: 'Ada', col_company: 'Analytical Engine' } },
612-
{ data: { col_name: 'Grace', col_company: 'Navy, Inc' } },
613-
],
614-
})
615-
616-
await executeFunctionExecute({ inputTables: ['tbl_2'] }, context as never)
617-
618-
const csv = mountedFiles()[0].content as string
619-
const lines = csv.split('\n')
620-
expect(lines[0]).toBe('name,company')
621-
expect(lines[1]).toBe('Ada,Analytical Engine')
622-
// Value containing a comma is quoted.
623-
expect(lines[2]).toBe('Grace,"Navy, Inc"')
624-
// No stable column id leaks into the mounted file.
625-
expect(csv).not.toContain('col_name')
626-
expect(csv).not.toContain('col_company')
627-
})
628-
629-
it('reads values by column id for legacy name-keyed rows too', async () => {
630-
// Legacy column with no id: getColumnId falls back to name, so name-keyed data is correct.
631-
mockGetTableById.mockResolvedValue({
632-
id: 'tbl_legacy',
633-
workspaceId: 'ws_1',
634-
rowCount: 1,
635-
schema: { columns: [{ name: 'email', type: 'string' }] },
636-
})
637-
mockQueryRows.mockResolvedValue({ rows: [{ data: { email: 'a@b.com' } }] })
638-
639-
await executeFunctionExecute({ inputTables: ['tbl_legacy'] }, context as never)
640-
641-
expect(mountedFiles()[0].content).toBe('email\na@b.com')
642-
})
643-
644-
it('mounts a table above the inline row limit by presigned snapshot URL', async () => {
566+
it('mounts every table by presigned snapshot URL', async () => {
567+
mockGetTableById.mockResolvedValue({ ...table, rowCount: 0 })
645568
mockGetOrCreateTableSnapshot.mockResolvedValue({
646569
key: 'table-snapshots/ws_1/tbl_1/v5.csv',
647570
size: 9,
@@ -651,7 +574,6 @@ describe('executeFunctionExecute table mounts', () => {
651574
await executeFunctionExecute({ inputTables: ['tbl_1'] }, context as never)
652575

653576
expect(mockGetOrCreateTableSnapshot).toHaveBeenCalledTimes(1)
654-
expect(mockQueryRows).not.toHaveBeenCalled()
655577
expect(mockDownloadFile).not.toHaveBeenCalled()
656578
expect(mockGeneratePresignedDownloadUrl).toHaveBeenCalledWith(
657579
'table-snapshots/ws_1/tbl_1/v5.csv',
@@ -715,35 +637,6 @@ describe('executeFunctionExecute table mounts', () => {
715637
expect(projectToolResultForCopilot(result, parentRegistry)).toEqual({ success: true })
716638
})
717639

718-
it('unknown inline row provenance still mounts and taints model egress', async () => {
719-
mockGetTableById.mockResolvedValue({ ...table, rowCount: TABLE_LIMITS.DEFAULT_QUERY_LIMIT })
720-
mockLoadTableRowSecretProvenance.mockResolvedValue({
721-
version: 1,
722-
complete: false,
723-
entries: [],
724-
})
725-
mockExecuteTool.mockResolvedValue({ success: true, output: { result: 'raw output' } })
726-
const parentRegistry = new ResolvedSecretTraceRegistry([], {
727-
userId: 'u1',
728-
workspaceId: 'ws_1',
729-
})
730-
731-
const result = await executeFunctionExecute(
732-
{ inputTables: ['tbl_1'] },
733-
{ ...context, resolvedSecretTraceRegistry: parentRegistry }
734-
)
735-
736-
expect(mountedFiles()[0].content).toBe('name\nAda')
737-
expect(mockExecuteTool.mock.calls[0]?.[1]?.[PRIVATE_SECRET_PROVENANCE_FIELD]).toEqual({
738-
version: 1,
739-
complete: false,
740-
selections: [],
741-
})
742-
expect(result).toEqual({ success: true, output: { result: 'raw output' } })
743-
expect(parentRegistry.isComplete()).toBe(false)
744-
expect(projectToolResultForCopilot(result, parentRegistry)).toEqual({ success: true })
745-
})
746-
747640
it('throws when a cloud snapshot exceeds the table mount limit', async () => {
748641
mockGetOrCreateTableSnapshot.mockResolvedValue({
749642
key: 'table-snapshots/ws_1/tbl_1/v5.csv',

apps/sim/lib/copilot/tools/handlers/function-execute.ts

Lines changed: 43 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
materializeCopilotCodeSecrets,
1212
} from '@/lib/copilot/tools/secret-mount-materializer.server'
1313
import { decodeVfsPathSegments, encodeVfsPathSegments } from '@/lib/copilot/vfs/path-utils'
14-
import { neutralizeCsvFormula, toCsvRow } from '@/lib/core/utils/csv'
1514
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
1615
import type { PrivateSecretProvenanceBundleV1 } from '@/lib/execution/model-input-provenance'
1716
import {
@@ -20,14 +19,7 @@ import {
2019
} from '@/lib/execution/private-tool-metadata'
2120
import { MAX_PLAN_REQUIRED } from '@/lib/execution/remote-sandbox/workspace-sandboxes'
2221
import { recordSecretUsage } from '@/lib/secrets/usage/record'
23-
import { getColumnId } from '@/lib/table/column-keys'
24-
import { TABLE_LIMITS } from '@/lib/table/constants'
25-
import { formatCsvCell } from '@/lib/table/export-format'
26-
import {
27-
isTableSnapshotSafeForModelMount,
28-
loadTableRowSecretProvenance,
29-
} from '@/lib/table/rows/secret-provenance'
30-
import { queryRows } from '@/lib/table/rows/service'
22+
import { isTableSnapshotSafeForModelMount } from '@/lib/table/rows/secret-provenance'
3123
import { getTableById, listTables } from '@/lib/table/service'
3224
import { getOrCreateTableSnapshot, SNAPSHOT_MAX_BYTES } from '@/lib/table/snapshot-cache'
3325
import {
@@ -310,7 +302,6 @@ export async function resolveInputFiles(
310302
inputFiles?: unknown[],
311303
inputTables?: unknown[],
312304
inputDirectories?: unknown[],
313-
provenanceUserId?: string,
314305
resolvedSecretTraceRegistry?: ResolvedSecretTraceRegistry,
315306
filePrincipal?: Principal
316307
): Promise<SandboxFile[]> {
@@ -487,108 +478,61 @@ export async function resolveInputFiles(
487478
: undefined
488479
const mountPath = sandboxPath || `/home/user/tables/${table.id}.csv`
489480

490-
/** Tables that exceed the bounded inline query mount from a complete versioned snapshot. */
491-
if (table.rowCount > TABLE_LIMITS.DEFAULT_QUERY_LIMIT) {
492-
const snapshot = await getOrCreateTableSnapshot(table, 'copilot-fn-exec')
493-
if (!resolvedSecretTraceRegistry) {
494-
throw new Error(
495-
`Input table "${tableId}" cannot be mounted because its secret provenance is unavailable.`
496-
)
497-
}
498-
try {
499-
const safeForModelMount = await isTableSnapshotSafeForModelMount({
500-
tableId: table.id,
501-
workspaceId,
502-
rowsVersion: snapshot.version,
503-
})
504-
if (!safeForModelMount)
505-
resolvedSecretTraceRegistry.markIncomplete('table-snapshot-unsafe-for-mount')
506-
} catch {
507-
resolvedSecretTraceRegistry.markIncomplete('table-snapshot-unsafe-for-mount')
508-
}
509-
510-
if (hasCloudStorage()) {
511-
// Mount by reference: the sandbox fetches the snapshot straight from storage via a
512-
// presigned URL, so the bytes never pass through the web process — the only ceiling is
513-
// sandbox disk (enforced at materialization by SNAPSHOT_MAX_BYTES).
514-
if (snapshot.size > SNAPSHOT_MAX_BYTES) {
515-
throw new Error(
516-
`Input table "${tableId}" is ${Math.round(snapshot.size / 1024 / 1024)}MB, over the ${SNAPSHOT_MAX_BYTES / 1024 / 1024}MB table mount limit.`
517-
)
518-
}
519-
const url = await generatePresignedDownloadUrl(
520-
snapshot.key,
521-
'execution',
522-
MOUNT_URL_TTL_SECONDS
523-
)
524-
sandboxFiles.push({ type: 'url', path: mountPath, url })
525-
continue
526-
}
527-
528-
// Local storage: a presigned URL is an app-internal serve path a remote sandbox can't
529-
// reach, so fall back to buffering the bytes through the web process (file-mount guards).
530-
if (snapshot.size > MAX_FILE_SIZE) {
531-
throw new Error(
532-
`Input table "${tableId}" is ${Math.round(snapshot.size / 1024 / 1024)}MB, over the ${MAX_FILE_SIZE / 1024 / 1024}MB per-file mount limit.`
533-
)
534-
}
535-
if (mounted.buffered + snapshot.size > MAX_TOTAL_SIZE) {
536-
throw new Error(
537-
`Mounting "${tableId}" would exceed the ${MAX_TOTAL_SIZE / 1024 / 1024}MB total mount limit. Mount fewer or smaller tables.`
538-
)
539-
}
540-
const buffer = await downloadFile({
541-
key: snapshot.key,
542-
context: 'execution',
543-
maxBytes: MAX_FILE_SIZE,
544-
})
545-
mounted.buffered += buffer.length
546-
sandboxFiles.push({ path: mountPath, content: buffer.toString('utf-8') })
547-
continue
548-
}
549-
550-
/** The table fits completely within the bounded inline query. */
551-
const rows = await queryRows(
552-
table,
553-
{ limit: TABLE_LIMITS.DEFAULT_QUERY_LIMIT },
554-
'copilot-fn-exec'
555-
)
481+
const snapshot = await getOrCreateTableSnapshot(table, 'copilot-fn-exec')
556482
if (!resolvedSecretTraceRegistry) {
557483
throw new Error(
558484
`Input table "${tableId}" cannot be mounted because its secret provenance is unavailable.`
559485
)
560486
}
561487
try {
562-
const provenance = await loadTableRowSecretProvenance(rows.rows, {
563-
userId: provenanceUserId ?? 'opaque-model-mount',
488+
const safeForModelMount = await isTableSnapshotSafeForModelMount({
489+
tableId: table.id,
564490
workspaceId,
491+
rowsVersion: snapshot.version,
565492
})
566-
if (
567-
!provenance.complete ||
568-
!(await resolvedSecretTraceRegistry.importProvenance(provenance, {
569-
trusted: true,
570-
origin: 'copilotFunctionExecute.result',
571-
}))
572-
) {
573-
resolvedSecretTraceRegistry.markIncomplete('source-provenance-incomplete', {
574-
origin: 'copilotFunctionExecute.result',
575-
})
576-
}
493+
if (!safeForModelMount)
494+
resolvedSecretTraceRegistry.markIncomplete('table-snapshot-unsafe-for-mount')
577495
} catch {
578-
resolvedSecretTraceRegistry.markIncomplete('source-provenance-incomplete', {
579-
origin: 'copilotFunctionExecute.result',
580-
})
496+
resolvedSecretTraceRegistry.markIncomplete('table-snapshot-unsafe-for-mount')
581497
}
582498

583-
const columns = table.schema.columns
584-
const csvLines = [toCsvRow(columns.map((column) => neutralizeCsvFormula(column.name)))]
585-
for (const row of rows.rows) {
586-
csvLines.push(
587-
toCsvRow(columns.map((column) => formatCsvCell(column, row.data[getColumnId(column)])))
499+
if (hasCloudStorage()) {
500+
// Mount by reference: the sandbox fetches the snapshot straight from storage via a
501+
// presigned URL, so the bytes never pass through the web process — the only ceiling is
502+
// sandbox disk (enforced at materialization by SNAPSHOT_MAX_BYTES).
503+
if (snapshot.size > SNAPSHOT_MAX_BYTES) {
504+
throw new Error(
505+
`Input table "${tableId}" is ${Math.round(snapshot.size / 1024 / 1024)}MB, over the ${SNAPSHOT_MAX_BYTES / 1024 / 1024}MB table mount limit.`
506+
)
507+
}
508+
const url = await generatePresignedDownloadUrl(
509+
snapshot.key,
510+
'execution',
511+
MOUNT_URL_TTL_SECONDS
512+
)
513+
sandboxFiles.push({ type: 'url', path: mountPath, url })
514+
continue
515+
}
516+
517+
// Local storage: a presigned URL is an app-internal serve path a remote sandbox can't
518+
// reach, so fall back to buffering the bytes through the web process (file-mount guards).
519+
if (snapshot.size > MAX_FILE_SIZE) {
520+
throw new Error(
521+
`Input table "${tableId}" is ${Math.round(snapshot.size / 1024 / 1024)}MB, over the ${MAX_FILE_SIZE / 1024 / 1024}MB per-file mount limit.`
588522
)
589523
}
590-
const csvContent = csvLines.join('\n')
591-
sandboxFiles.push({ path: mountPath, content: csvContent })
524+
if (mounted.buffered + snapshot.size > MAX_TOTAL_SIZE) {
525+
throw new Error(
526+
`Mounting "${tableId}" would exceed the ${MAX_TOTAL_SIZE / 1024 / 1024}MB total mount limit. Mount fewer or smaller tables.`
527+
)
528+
}
529+
const buffer = await downloadFile({
530+
key: snapshot.key,
531+
context: 'execution',
532+
maxBytes: MAX_FILE_SIZE,
533+
})
534+
mounted.buffered += buffer.length
535+
sandboxFiles.push({ path: mountPath, content: buffer.toString('utf-8') })
592536
}
593537
}
594538

@@ -727,7 +671,6 @@ export async function executeFunctionExecute(
727671
inputFiles,
728672
inputTables,
729673
inputDirectories,
730-
secretActorUserId ?? context.userId,
731674
mountedRegistry,
732675
inputFiles.length > 0 || inputDirectories.length > 0
733676
? resolveCopilotFilePrincipal(context)

0 commit comments

Comments
 (0)