diff --git a/apps/sim/lib/function-execution/overwrite-export.integration.ts b/apps/sim/lib/function-execution/overwrite-export.integration.ts new file mode 100644 index 00000000000..ce37ef99892 --- /dev/null +++ b/apps/sim/lib/function-execution/overwrite-export.integration.ts @@ -0,0 +1,182 @@ +/** Real multi-file Function exports admitting overwrite targets exactly as the writer upserts them. */ +import { mkdtempSync } from 'node:fs' +import { rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { db } from '@sim/db' +import { auditLog, organization, user, workspace, workspaceFiles } from '@sim/db/schema' +import { deleteWorkspaceFixture } from '@sim/db/testing/workspace-fixtures' +import { remoteSandboxMock, remoteSandboxMockFns } from '@sim/testing/mocks/remote-sandbox.mock' +import { generateId } from '@sim/utils/id' +import { and, eq, inArray, isNull } from 'drizzle-orm' +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' + +const fixtureStorage = vi.hoisted(() => ({ root: '' })) +const sandboxEnvironment = vi.hoisted(() => { + const fixture = { + SANDBOX_PROVIDER: 'e2b', + E2B_ENABLED: 'true', + E2B_API_KEY: 'integration-provider-fixture-not-a-real-key', + E2B_FUNCTION_TEMPLATE_ID: 'fixture:11111111-1111-4111-8111-111111111111', + E2B_FUNCTION_TEMPLATE_GENERATION: '1785792000000', + } + const previous = Object.entries(fixture).map(([key]) => [key, process.env[key]] as const) + Object.assign(process.env, fixture) + return previous +}) +vi.mock('@/lib/execution/remote-sandbox', () => remoteSandboxMock) +vi.mock('@/lib/uploads/core/setup.server', () => ({ + get UPLOAD_DIR_SERVER() { + return fixtureStorage.root + }, +})) + +import { functionExecuteBodySchema } from '@/lib/api/contracts' +import { executeFunctionRequest } from '@/lib/function-execution/execute-request' +import { + createKnowledgeAclFixtureIds, + seedKnowledgeAclFixture, +} from '@/lib/knowledge/__integration__/seed-source-access-fixture' +import { createWorkspaceFileFolder } from '@/lib/uploads/contexts/workspace/file-folder-manager' +import { uploadWorkspaceFile } from '@/lib/uploads/contexts/workspace/file-manager' +import { createWorkspaceFileDelegatedPrincipal } from '@/lib/workspace-files/application/delegated-principal' +import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' + +const fixtures: ReturnType[] = [] + +beforeAll(() => { + fixtureStorage.root = mkdtempSync(path.join(tmpdir(), 'sim-function-overwrite-export-')) +}) + +afterAll(async () => { + try { + for (const ids of fixtures) { + await db.delete(auditLog).where(eq(auditLog.workspaceId, ids.workspaceId)) + await deleteWorkspaceFixture(db, eq(workspace.id, ids.workspaceId)) + await db.delete(organization).where(eq(organization.id, ids.organizationId)) + await db.delete(user).where(inArray(user.id, [ids.aliceId, ids.bobId])) + } + await rm(fixtureStorage.root, { recursive: true, force: true }) + } finally { + for (const [key, value] of sandboxEnvironment) { + if (value === undefined) Reflect.deleteProperty(process.env, key) + else process.env[key] = value + } + } +}) + +/** A workspace whose admin owns an empty `Reports` folder. */ +async function seed() { + const ids = createKnowledgeAclFixtureIds() + fixtures.push(ids) + await seedKnowledgeAclFixture(ids) + const folder = await createWorkspaceFileFolder({ + workspaceId: ids.workspaceId, + userId: ids.aliceId, + name: 'Reports', + }) + return { ...ids, folderId: folder.id, workflowId: generateId(), executionId: generateId() } +} + +type Fixture = Awaited> + +async function exportTwoFiles(ids: Fixture, overwritePath: string) { + remoteSandboxMockFns.mockExecuteInSandbox.mockResolvedValueOnce({ + result: 'done', + stdout: '', + sandboxId: 'fixture', + exportedFiles: { + '/home/user/summary.csv': 'name,total\nnew,2\n', + '/home/user/notes.txt': 'fresh notes', + }, + }) + return executeFunctionRequest( + { headers: new Headers(), signal: AbortSignal.timeout(15_000) }, + functionExecuteBodySchema.parse({ + workspaceId: ids.workspaceId, + workflowId: ids.workflowId, + executionId: ids.executionId, + language: 'python', + code: 'print("done")', + outputs: { + files: [ + { path: overwritePath, mode: 'overwrite', sandboxPath: '/home/user/summary.csv' }, + { path: 'files/Reports/notes.txt', sandboxPath: '/home/user/notes.txt' }, + ], + }, + }), + { + attributedUserId: ids.aliceId, + fileAccessUserId: ids.aliceId, + principal: createWorkspaceFileDelegatedPrincipal({ + serviceId: 'executor', + subjectUserId: ids.aliceId, + workspaceId: ids.workspaceId, + delegationId: generateId(), + executionId: ids.executionId, + }), + resolvedSecretTraceRegistry: new ResolvedSecretTraceRegistry([], { + userId: ids.aliceId, + workspaceId: ids.workspaceId, + }), + } + ) +} + +function liveFilesInFolder(ids: Fixture) { + return db + .select({ + id: workspaceFiles.id, + name: workspaceFiles.originalName, + size: workspaceFiles.sizeBytes, + }) + .from(workspaceFiles) + .where( + and( + eq(workspaceFiles.workspaceId, ids.workspaceId), + eq(workspaceFiles.folderId, ids.folderId), + isNull(workspaceFiles.deletedAt) + ) + ) +} + +describe('multi-file Function export with overwrite targets', () => { + it('creates a missing overwrite target instead of failing after the code ran', async () => { + const ids = await seed() + + const response = await exportTwoFiles(ids, 'files/Reports/summary.csv') + const body = await response.json() + + expect(response.status, JSON.stringify(body)).toBe(200) + expect(body.success).toBe(true) + const files = await liveFilesInFolder(ids) + expect(files.map((file) => file.name).sort()).toEqual(['notes.txt', 'summary.csv']) + expect(files.find((file) => file.name === 'summary.csv')?.size).toBe( + Buffer.byteLength('name,total\nnew,2\n') + ) + }) + + it('still replaces an existing overwrite target in place', async () => { + const ids = await seed() + const existing = await uploadWorkspaceFile( + ids.workspaceId, + ids.aliceId, + Buffer.from('name,total\nold,1\nolder,0\n'), + 'summary.csv', + 'text/csv', + { folderId: ids.folderId, exactName: true, notifyWorkspaceChange: false } + ) + + const response = await exportTwoFiles(ids, 'files/Reports/summary.csv') + const body = await response.json() + + expect(response.status, JSON.stringify(body)).toBe(200) + const files = await liveFilesInFolder(ids) + expect(files.map((file) => file.name).sort()).toEqual(['notes.txt', 'summary.csv']) + expect(files.find((file) => file.name === 'summary.csv')).toEqual({ + id: existing.id, + name: 'summary.csv', + size: Buffer.byteLength('name,total\nnew,2\n'), + }) + }) +}) diff --git a/apps/sim/lib/mothership/agent-cli/engines/file-read.ts b/apps/sim/lib/mothership/agent-cli/engines/file-read.ts index be1f7ade79a..3121a8b4d45 100644 --- a/apps/sim/lib/mothership/agent-cli/engines/file-read.ts +++ b/apps/sim/lib/mothership/agent-cli/engines/file-read.ts @@ -6,7 +6,10 @@ import { isDefaultVisualMediaType, observePrivateFile, } from '@/lib/mothership/agent-cli/engines/observe-private-file' -import { readScratchFile } from '@/lib/mothership/agent-cli/engines/scratch-file-read' +import { + isScratchReference, + readScratchFile, +} from '@/lib/mothership/agent-cli/engines/scratch-file-read' import { type AgentCliEngine, agentCliFail, agentCliOk } from '@/lib/mothership/agent-cli/types' import { readChatAttachment } from '@/lib/mothership/chat/application/read-attachment' import { workspaceFileVfsPath } from '@/lib/uploads/contexts/workspace/file-manager' @@ -86,7 +89,7 @@ export const fileReadCommand: AgentCliEngine = { runtime.signal ) } - if (reference.startsWith('/tmp/') || reference.startsWith('/home/user/')) + if (isScratchReference(reference)) return await readScratchFile(reference, runtime, flags, query.data) if ( runtime.chatOrganizationId && diff --git a/apps/sim/lib/mothership/agent-cli/engines/file-view.test.ts b/apps/sim/lib/mothership/agent-cli/engines/file-view.test.ts index c3afe8fa087..9b9bfac84ca 100644 --- a/apps/sim/lib/mothership/agent-cli/engines/file-view.test.ts +++ b/apps/sim/lib/mothership/agent-cli/engines/file-view.test.ts @@ -3,10 +3,13 @@ import { PDFDocument } from 'pdf-lib' import sharp from 'sharp' import { describe, expect, it, vi } from 'vitest' -const { read } = vi.hoisted(() => ({ read: vi.fn() })) +const { read, scratch } = vi.hoisted(() => ({ read: vi.fn(), scratch: vi.fn() })) vi.mock('@/lib/workspace-files/application/read-workspace-file-artifact', () => ({ readWorkspaceFileArtifact: { execute: read }, })) +vi.mock('@/lib/mothership/chat/application/read-sandbox-file', () => ({ + readChatSandboxFile: { execute: scratch }, +})) import { runEngine } from '@/lib/mothership/agent-cli/engines' import type { AgentCliRuntime } from '@/lib/mothership/agent-cli/types' @@ -105,4 +108,31 @@ describe('registered files view boundary', () => { expect(result.stderr).not.toContain('database host') expect(result.observations).toBeUndefined() }) + it('reads an absolute sandbox path from the chat scratch machine, never the workspace', async () => { + const bytes = await sharp({ create: { width: 2, height: 2, channels: 3, background: 'red' } }) + .png() + .toBuffer() + scratch.mockResolvedValue({ buffer: bytes, name: 'x.png', path: '/home/user/x.png' }) + const result = await runEngine( + 'files view', + ['/home/user/x.png'], + { ...runtime, chatId: 'trusted-chat' }, + {} + ) + expect(result.exitCode).toBe(0) + expect(JSON.parse(result.stdout)).toMatchObject({ + source: 'sandbox', + representation: 'visual', + path: '/home/user/x.png', + }) + expect(ArtifactObservations.parse(result.observations)).toHaveLength(1) + expect(result.resources).toBeUndefined() + expect(scratch).toHaveBeenCalledWith( + expect.objectContaining({ + principal, + input: expect.objectContaining({ chatId: 'trusted-chat', path: '/home/user/x.png' }), + }) + ) + expect(read).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/lib/mothership/agent-cli/engines/file-view.ts b/apps/sim/lib/mothership/agent-cli/engines/file-view.ts index 34d201efb33..aa56e700145 100644 --- a/apps/sim/lib/mothership/agent-cli/engines/file-view.ts +++ b/apps/sim/lib/mothership/agent-cli/engines/file-view.ts @@ -3,6 +3,10 @@ import { getErrorMessage } from '@sim/utils/errors' import { SimApiError } from 'sim/embed' import { OrchestrationError } from '@/lib/core/orchestration/types' import { observePrivateFile } from '@/lib/mothership/agent-cli/engines/observe-private-file' +import { + isScratchReference, + readScratchFile, +} from '@/lib/mothership/agent-cli/engines/scratch-file-read' import { type AgentCliEngine, type AgentCliFlags, @@ -106,6 +110,8 @@ export const fileViewCommand: AgentCliEngine = { const reference = positionals[0] if (!reference) return agentCliFail('files view requires a workspace file reference.') try { + if (isScratchReference(reference)) + return await readScratchFile(reference, runtime, { ...flags, render: true }, {}) if ( runtime.chatOrganizationId && runtime.chatId && diff --git a/apps/sim/lib/mothership/agent-cli/engines/scratch-file-read.ts b/apps/sim/lib/mothership/agent-cli/engines/scratch-file-read.ts index accf2cd0553..169a807ab4f 100644 --- a/apps/sim/lib/mothership/agent-cli/engines/scratch-file-read.ts +++ b/apps/sim/lib/mothership/agent-cli/engines/scratch-file-read.ts @@ -3,6 +3,11 @@ import { observePrivateFile } from '@/lib/mothership/agent-cli/engines/observe-p import type { AgentCliFlags, AgentCliRuntime } from '@/lib/mothership/agent-cli/types' import { readChatSandboxFile } from '@/lib/mothership/chat/application/read-sandbox-file' +/** Absolute sandbox paths name the chat's scratch filesystem, never a workspace file. */ +export function isScratchReference(reference: string): boolean { + return reference.startsWith('/tmp/') || reference.startsWith('/home/user/') +} + /** The authorized scratch snapshot never becomes a workspace file or resource-panel entry. */ export async function readScratchFile( reference: string, diff --git a/apps/sim/lib/mothership/vfs/resource-writer.test.ts b/apps/sim/lib/mothership/vfs/resource-writer.test.ts index 7659a9f70e9..c28c5e6cc4b 100644 --- a/apps/sim/lib/mothership/vfs/resource-writer.test.ts +++ b/apps/sim/lib/mothership/vfs/resource-writer.test.ts @@ -320,6 +320,40 @@ describe('resource writer', () => { }) }) + it('validates an overwrite of a missing target as the create the writer will perform', async () => { + const { OrchestrationError } = await import('@/lib/core/orchestration/types') + mocks.resolveWorkspaceFileReference.mockRejectedValue( + new OrchestrationError('not_found', 'File not found') + ) + mocks.findWorkspaceFileFolderIdByPath.mockResolvedValue('folder-reports') + mocks.getWorkspaceFileByName.mockResolvedValue(null) + const target = { path: 'files/Reports/summary.csv', mode: 'overwrite' } as const + + await expect( + validateWorkspaceFileWriteTarget({ + workspaceId: 'workspace-1', + principal: createSessionPrincipal(), + target, + }) + ).resolves.toEqual({ + mode: 'create', + vfsPath: 'files/Reports/summary.csv', + fileName: 'summary.csv', + folderId: 'folder-reports', + }) + + mocks.admitCreateWorkspaceFile.mockRejectedValue( + new OrchestrationError('forbidden', 'Insufficient permissions') + ) + await expect( + validateWorkspaceFileWriteTarget({ + workspaceId: 'workspace-1', + principal: createSessionPrincipal(), + target, + }) + ).rejects.toThrow('Insufficient permissions') + }) + it('upserts: an overwrite of a missing target falls through to create', async () => { const { OrchestrationError } = await import('@/lib/core/orchestration/types') mocks.resolveWorkspaceFileReference.mockRejectedValue( diff --git a/apps/sim/lib/mothership/vfs/resource-writer.ts b/apps/sim/lib/mothership/vfs/resource-writer.ts index b9e9fb1fc2d..d516f94da10 100644 --- a/apps/sim/lib/mothership/vfs/resource-writer.ts +++ b/apps/sim/lib/mothership/vfs/resource-writer.ts @@ -170,16 +170,21 @@ export async function validateWorkspaceFileWriteTarget(args: { target: WorkspaceFileWriteTarget }): Promise { if (args.target.mode === 'overwrite') { - const existing = await resolveWorkspaceFileReference({ - principal: args.principal, - operation: fileOperations.updateContent, - workspaceId: args.workspaceId, - reference: args.target.path, - }) - return { - mode: 'overwrite', - vfsPath: vfsPathForRecord(existing), - existingFileId: existing.id, + try { + const existing = await resolveWorkspaceFileReference({ + principal: args.principal, + operation: fileOperations.updateContent, + workspaceId: args.workspaceId, + reference: args.target.path, + }) + return { + mode: 'overwrite', + vfsPath: vfsPathForRecord(existing), + existingFileId: existing.id, + } + } catch (error) { + // Mirrors writeWorkspaceFileByPath: a missing overwrite target is written as a create. + if (asOrchestrationError(error)?.code !== 'not_found') throw error } }