Skip to content

Commit 768141a

Browse files
authored
fix(copilot): read sandbox paths in files view and admit missing overwrite targets in multi-file exports (#9200)
1 parent 109bd82 commit 768141a

7 files changed

Lines changed: 278 additions & 13 deletions

File tree

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
/** Real multi-file Function exports admitting overwrite targets exactly as the writer upserts them. */
2+
import { mkdtempSync } from 'node:fs'
3+
import { rm } from 'node:fs/promises'
4+
import { tmpdir } from 'node:os'
5+
import path from 'node:path'
6+
import { db } from '@sim/db'
7+
import { auditLog, organization, user, workspace, workspaceFiles } from '@sim/db/schema'
8+
import { deleteWorkspaceFixture } from '@sim/db/testing/workspace-fixtures'
9+
import { remoteSandboxMock, remoteSandboxMockFns } from '@sim/testing/mocks/remote-sandbox.mock'
10+
import { generateId } from '@sim/utils/id'
11+
import { and, eq, inArray, isNull } from 'drizzle-orm'
12+
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
13+
14+
const fixtureStorage = vi.hoisted(() => ({ root: '' }))
15+
const sandboxEnvironment = vi.hoisted(() => {
16+
const fixture = {
17+
SANDBOX_PROVIDER: 'e2b',
18+
E2B_ENABLED: 'true',
19+
E2B_API_KEY: 'integration-provider-fixture-not-a-real-key',
20+
E2B_FUNCTION_TEMPLATE_ID: 'fixture:11111111-1111-4111-8111-111111111111',
21+
E2B_FUNCTION_TEMPLATE_GENERATION: '1785792000000',
22+
}
23+
const previous = Object.entries(fixture).map(([key]) => [key, process.env[key]] as const)
24+
Object.assign(process.env, fixture)
25+
return previous
26+
})
27+
vi.mock('@/lib/execution/remote-sandbox', () => remoteSandboxMock)
28+
vi.mock('@/lib/uploads/core/setup.server', () => ({
29+
get UPLOAD_DIR_SERVER() {
30+
return fixtureStorage.root
31+
},
32+
}))
33+
34+
import { functionExecuteBodySchema } from '@/lib/api/contracts'
35+
import { executeFunctionRequest } from '@/lib/function-execution/execute-request'
36+
import {
37+
createKnowledgeAclFixtureIds,
38+
seedKnowledgeAclFixture,
39+
} from '@/lib/knowledge/__integration__/seed-source-access-fixture'
40+
import { createWorkspaceFileFolder } from '@/lib/uploads/contexts/workspace/file-folder-manager'
41+
import { uploadWorkspaceFile } from '@/lib/uploads/contexts/workspace/file-manager'
42+
import { createWorkspaceFileDelegatedPrincipal } from '@/lib/workspace-files/application/delegated-principal'
43+
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
44+
45+
const fixtures: ReturnType<typeof createKnowledgeAclFixtureIds>[] = []
46+
47+
beforeAll(() => {
48+
fixtureStorage.root = mkdtempSync(path.join(tmpdir(), 'sim-function-overwrite-export-'))
49+
})
50+
51+
afterAll(async () => {
52+
try {
53+
for (const ids of fixtures) {
54+
await db.delete(auditLog).where(eq(auditLog.workspaceId, ids.workspaceId))
55+
await deleteWorkspaceFixture(db, eq(workspace.id, ids.workspaceId))
56+
await db.delete(organization).where(eq(organization.id, ids.organizationId))
57+
await db.delete(user).where(inArray(user.id, [ids.aliceId, ids.bobId]))
58+
}
59+
await rm(fixtureStorage.root, { recursive: true, force: true })
60+
} finally {
61+
for (const [key, value] of sandboxEnvironment) {
62+
if (value === undefined) Reflect.deleteProperty(process.env, key)
63+
else process.env[key] = value
64+
}
65+
}
66+
})
67+
68+
/** A workspace whose admin owns an empty `Reports` folder. */
69+
async function seed() {
70+
const ids = createKnowledgeAclFixtureIds()
71+
fixtures.push(ids)
72+
await seedKnowledgeAclFixture(ids)
73+
const folder = await createWorkspaceFileFolder({
74+
workspaceId: ids.workspaceId,
75+
userId: ids.aliceId,
76+
name: 'Reports',
77+
})
78+
return { ...ids, folderId: folder.id, workflowId: generateId(), executionId: generateId() }
79+
}
80+
81+
type Fixture = Awaited<ReturnType<typeof seed>>
82+
83+
async function exportTwoFiles(ids: Fixture, overwritePath: string) {
84+
remoteSandboxMockFns.mockExecuteInSandbox.mockResolvedValueOnce({
85+
result: 'done',
86+
stdout: '',
87+
sandboxId: 'fixture',
88+
exportedFiles: {
89+
'/home/user/summary.csv': 'name,total\nnew,2\n',
90+
'/home/user/notes.txt': 'fresh notes',
91+
},
92+
})
93+
return executeFunctionRequest(
94+
{ headers: new Headers(), signal: AbortSignal.timeout(15_000) },
95+
functionExecuteBodySchema.parse({
96+
workspaceId: ids.workspaceId,
97+
workflowId: ids.workflowId,
98+
executionId: ids.executionId,
99+
language: 'python',
100+
code: 'print("done")',
101+
outputs: {
102+
files: [
103+
{ path: overwritePath, mode: 'overwrite', sandboxPath: '/home/user/summary.csv' },
104+
{ path: 'files/Reports/notes.txt', sandboxPath: '/home/user/notes.txt' },
105+
],
106+
},
107+
}),
108+
{
109+
attributedUserId: ids.aliceId,
110+
fileAccessUserId: ids.aliceId,
111+
principal: createWorkspaceFileDelegatedPrincipal({
112+
serviceId: 'executor',
113+
subjectUserId: ids.aliceId,
114+
workspaceId: ids.workspaceId,
115+
delegationId: generateId(),
116+
executionId: ids.executionId,
117+
}),
118+
resolvedSecretTraceRegistry: new ResolvedSecretTraceRegistry([], {
119+
userId: ids.aliceId,
120+
workspaceId: ids.workspaceId,
121+
}),
122+
}
123+
)
124+
}
125+
126+
function liveFilesInFolder(ids: Fixture) {
127+
return db
128+
.select({
129+
id: workspaceFiles.id,
130+
name: workspaceFiles.originalName,
131+
size: workspaceFiles.sizeBytes,
132+
})
133+
.from(workspaceFiles)
134+
.where(
135+
and(
136+
eq(workspaceFiles.workspaceId, ids.workspaceId),
137+
eq(workspaceFiles.folderId, ids.folderId),
138+
isNull(workspaceFiles.deletedAt)
139+
)
140+
)
141+
}
142+
143+
describe('multi-file Function export with overwrite targets', () => {
144+
it('creates a missing overwrite target instead of failing after the code ran', async () => {
145+
const ids = await seed()
146+
147+
const response = await exportTwoFiles(ids, 'files/Reports/summary.csv')
148+
const body = await response.json()
149+
150+
expect(response.status, JSON.stringify(body)).toBe(200)
151+
expect(body.success).toBe(true)
152+
const files = await liveFilesInFolder(ids)
153+
expect(files.map((file) => file.name).sort()).toEqual(['notes.txt', 'summary.csv'])
154+
expect(files.find((file) => file.name === 'summary.csv')?.size).toBe(
155+
Buffer.byteLength('name,total\nnew,2\n')
156+
)
157+
})
158+
159+
it('still replaces an existing overwrite target in place', async () => {
160+
const ids = await seed()
161+
const existing = await uploadWorkspaceFile(
162+
ids.workspaceId,
163+
ids.aliceId,
164+
Buffer.from('name,total\nold,1\nolder,0\n'),
165+
'summary.csv',
166+
'text/csv',
167+
{ folderId: ids.folderId, exactName: true, notifyWorkspaceChange: false }
168+
)
169+
170+
const response = await exportTwoFiles(ids, 'files/Reports/summary.csv')
171+
const body = await response.json()
172+
173+
expect(response.status, JSON.stringify(body)).toBe(200)
174+
const files = await liveFilesInFolder(ids)
175+
expect(files.map((file) => file.name).sort()).toEqual(['notes.txt', 'summary.csv'])
176+
expect(files.find((file) => file.name === 'summary.csv')).toEqual({
177+
id: existing.id,
178+
name: 'summary.csv',
179+
size: Buffer.byteLength('name,total\nnew,2\n'),
180+
})
181+
})
182+
})

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import {
66
isDefaultVisualMediaType,
77
observePrivateFile,
88
} from '@/lib/mothership/agent-cli/engines/observe-private-file'
9-
import { readScratchFile } from '@/lib/mothership/agent-cli/engines/scratch-file-read'
9+
import {
10+
isScratchReference,
11+
readScratchFile,
12+
} from '@/lib/mothership/agent-cli/engines/scratch-file-read'
1013
import { type AgentCliEngine, agentCliFail, agentCliOk } from '@/lib/mothership/agent-cli/types'
1114
import { readChatAttachment } from '@/lib/mothership/chat/application/read-attachment'
1215
import { workspaceFileVfsPath } from '@/lib/uploads/contexts/workspace/file-manager'
@@ -86,7 +89,7 @@ export const fileReadCommand: AgentCliEngine = {
8689
runtime.signal
8790
)
8891
}
89-
if (reference.startsWith('/tmp/') || reference.startsWith('/home/user/'))
92+
if (isScratchReference(reference))
9093
return await readScratchFile(reference, runtime, flags, query.data)
9194
if (
9295
runtime.chatOrganizationId &&

‎apps/sim/lib/mothership/agent-cli/engines/file-view.test.ts‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@ import { PDFDocument } from 'pdf-lib'
33
import sharp from 'sharp'
44
import { describe, expect, it, vi } from 'vitest'
55

6-
const { read } = vi.hoisted(() => ({ read: vi.fn() }))
6+
const { read, scratch } = vi.hoisted(() => ({ read: vi.fn(), scratch: vi.fn() }))
77
vi.mock('@/lib/workspace-files/application/read-workspace-file-artifact', () => ({
88
readWorkspaceFileArtifact: { execute: read },
99
}))
10+
vi.mock('@/lib/mothership/chat/application/read-sandbox-file', () => ({
11+
readChatSandboxFile: { execute: scratch },
12+
}))
1013

1114
import { runEngine } from '@/lib/mothership/agent-cli/engines'
1215
import type { AgentCliRuntime } from '@/lib/mothership/agent-cli/types'
@@ -105,4 +108,31 @@ describe('registered files view boundary', () => {
105108
expect(result.stderr).not.toContain('database host')
106109
expect(result.observations).toBeUndefined()
107110
})
111+
it('reads an absolute sandbox path from the chat scratch machine, never the workspace', async () => {
112+
const bytes = await sharp({ create: { width: 2, height: 2, channels: 3, background: 'red' } })
113+
.png()
114+
.toBuffer()
115+
scratch.mockResolvedValue({ buffer: bytes, name: 'x.png', path: '/home/user/x.png' })
116+
const result = await runEngine(
117+
'files view',
118+
['/home/user/x.png'],
119+
{ ...runtime, chatId: 'trusted-chat' },
120+
{}
121+
)
122+
expect(result.exitCode).toBe(0)
123+
expect(JSON.parse(result.stdout)).toMatchObject({
124+
source: 'sandbox',
125+
representation: 'visual',
126+
path: '/home/user/x.png',
127+
})
128+
expect(ArtifactObservations.parse(result.observations)).toHaveLength(1)
129+
expect(result.resources).toBeUndefined()
130+
expect(scratch).toHaveBeenCalledWith(
131+
expect.objectContaining({
132+
principal,
133+
input: expect.objectContaining({ chatId: 'trusted-chat', path: '/home/user/x.png' }),
134+
})
135+
)
136+
expect(read).not.toHaveBeenCalled()
137+
})
108138
})

‎apps/sim/lib/mothership/agent-cli/engines/file-view.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ import { getErrorMessage } from '@sim/utils/errors'
33
import { SimApiError } from 'sim/embed'
44
import { OrchestrationError } from '@/lib/core/orchestration/types'
55
import { observePrivateFile } from '@/lib/mothership/agent-cli/engines/observe-private-file'
6+
import {
7+
isScratchReference,
8+
readScratchFile,
9+
} from '@/lib/mothership/agent-cli/engines/scratch-file-read'
610
import {
711
type AgentCliEngine,
812
type AgentCliFlags,
@@ -106,6 +110,8 @@ export const fileViewCommand: AgentCliEngine = {
106110
const reference = positionals[0]
107111
if (!reference) return agentCliFail('files view requires a workspace file reference.')
108112
try {
113+
if (isScratchReference(reference))
114+
return await readScratchFile(reference, runtime, { ...flags, render: true }, {})
109115
if (
110116
runtime.chatOrganizationId &&
111117
runtime.chatId &&

‎apps/sim/lib/mothership/agent-cli/engines/scratch-file-read.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ import { observePrivateFile } from '@/lib/mothership/agent-cli/engines/observe-p
33
import type { AgentCliFlags, AgentCliRuntime } from '@/lib/mothership/agent-cli/types'
44
import { readChatSandboxFile } from '@/lib/mothership/chat/application/read-sandbox-file'
55

6+
/** Absolute sandbox paths name the chat's scratch filesystem, never a workspace file. */
7+
export function isScratchReference(reference: string): boolean {
8+
return reference.startsWith('/tmp/') || reference.startsWith('/home/user/')
9+
}
10+
611
/** The authorized scratch snapshot never becomes a workspace file or resource-panel entry. */
712
export async function readScratchFile(
813
reference: string,

‎apps/sim/lib/mothership/vfs/resource-writer.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,40 @@ describe('resource writer', () => {
320320
})
321321
})
322322

323+
it('validates an overwrite of a missing target as the create the writer will perform', async () => {
324+
const { OrchestrationError } = await import('@/lib/core/orchestration/types')
325+
mocks.resolveWorkspaceFileReference.mockRejectedValue(
326+
new OrchestrationError('not_found', 'File not found')
327+
)
328+
mocks.findWorkspaceFileFolderIdByPath.mockResolvedValue('folder-reports')
329+
mocks.getWorkspaceFileByName.mockResolvedValue(null)
330+
const target = { path: 'files/Reports/summary.csv', mode: 'overwrite' } as const
331+
332+
await expect(
333+
validateWorkspaceFileWriteTarget({
334+
workspaceId: 'workspace-1',
335+
principal: createSessionPrincipal(),
336+
target,
337+
})
338+
).resolves.toEqual({
339+
mode: 'create',
340+
vfsPath: 'files/Reports/summary.csv',
341+
fileName: 'summary.csv',
342+
folderId: 'folder-reports',
343+
})
344+
345+
mocks.admitCreateWorkspaceFile.mockRejectedValue(
346+
new OrchestrationError('forbidden', 'Insufficient permissions')
347+
)
348+
await expect(
349+
validateWorkspaceFileWriteTarget({
350+
workspaceId: 'workspace-1',
351+
principal: createSessionPrincipal(),
352+
target,
353+
})
354+
).rejects.toThrow('Insufficient permissions')
355+
})
356+
323357
it('upserts: an overwrite of a missing target falls through to create', async () => {
324358
const { OrchestrationError } = await import('@/lib/core/orchestration/types')
325359
mocks.resolveWorkspaceFileReference.mockRejectedValue(

‎apps/sim/lib/mothership/vfs/resource-writer.ts‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -170,16 +170,21 @@ export async function validateWorkspaceFileWriteTarget(args: {
170170
target: WorkspaceFileWriteTarget
171171
}): Promise<WorkspaceFileWriteValidation> {
172172
if (args.target.mode === 'overwrite') {
173-
const existing = await resolveWorkspaceFileReference({
174-
principal: args.principal,
175-
operation: fileOperations.updateContent,
176-
workspaceId: args.workspaceId,
177-
reference: args.target.path,
178-
})
179-
return {
180-
mode: 'overwrite',
181-
vfsPath: vfsPathForRecord(existing),
182-
existingFileId: existing.id,
173+
try {
174+
const existing = await resolveWorkspaceFileReference({
175+
principal: args.principal,
176+
operation: fileOperations.updateContent,
177+
workspaceId: args.workspaceId,
178+
reference: args.target.path,
179+
})
180+
return {
181+
mode: 'overwrite',
182+
vfsPath: vfsPathForRecord(existing),
183+
existingFileId: existing.id,
184+
}
185+
} catch (error) {
186+
// Mirrors writeWorkspaceFileByPath: a missing overwrite target is written as a create.
187+
if (asOrchestrationError(error)?.code !== 'not_found') throw error
183188
}
184189
}
185190

0 commit comments

Comments
 (0)