From e566c75dc3c9ca2889f2c08f52ddac1ea0d55750 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 3 Nov 2026 12:32:31 -0800 Subject: [PATCH] fix(files): accept files/ VFS paths in v2 file download and sim files get --- apps/docs/content/docs/cli/files.mdx | 2 +- apps/docs/content/docs/cli/reference.mdx | 2 +- apps/docs/openapi-v2-files-audit.json | 7 +- .../app/api/v2/files/[fileId]/route.test.ts | 17 +++ apps/sim/app/api/v2/files/[fileId]/route.ts | 2 + apps/sim/lib/api/contracts/v2/files.ts | 2 +- .../agent-cli/file-read-transport.ts | 1 + .../application/download-workspace-file.ts | 17 ++- .../workspace-files/delivery.integration.ts | 108 +++++++++++++++++- .../src/commands/protocol/files-get.ts | 9 +- packages/sim-cli/src/generated/v2-api.ts | 5 +- 11 files changed, 155 insertions(+), 17 deletions(-) diff --git a/apps/docs/content/docs/cli/files.mdx b/apps/docs/content/docs/cli/files.mdx index 959f5809299..655dc78ab90 100644 --- a/apps/docs/content/docs/cli/files.mdx +++ b/apps/docs/content/docs/cli/files.mdx @@ -183,7 +183,7 @@ sim files get [options] | Argument | Required | Description | | --- | --- | --- | -| `fileId` | Yes | File whose content to read | +| `fileId` | Yes | File whose content to read: its id, or its `files//` VFS path | diff --git a/apps/docs/content/docs/cli/reference.mdx b/apps/docs/content/docs/cli/reference.mdx index b6f24256769..6d9396c5ba1 100644 --- a/apps/docs/content/docs/cli/reference.mdx +++ b/apps/docs/content/docs/cli/reference.mdx @@ -1761,7 +1761,7 @@ sim files get [options] | Argument | Required | Description | | --- | --- | --- | -| `fileId` | Yes | File whose content to read | +| `fileId` | Yes | File whose content to read: its id, or its `files//` VFS path | diff --git a/apps/docs/openapi-v2-files-audit.json b/apps/docs/openapi-v2-files-audit.json index 0b741d9516c..2fcff52693b 100644 --- a/apps/docs/openapi-v2-files-audit.json +++ b/apps/docs/openapi-v2-files-audit.json @@ -2048,13 +2048,12 @@ "name": "fileId", "in": "path", "required": true, - "description": "File identifier.", + "description": "File identifier, or the VFS path of a live file: `files//`. Pass the id when two files in a folder have names that display alike.", "schema": { "type": "string", "minLength": 1, - "maxLength": 128, - "pattern": "^[A-Za-z0-9_-]+$", - "description": "File identifier." + "maxLength": 4096, + "description": "File identifier, or the VFS path of a live file: `files//`. Pass the id when two files in a folder have names that display alike." } }, { diff --git a/apps/sim/app/api/v2/files/[fileId]/route.test.ts b/apps/sim/app/api/v2/files/[fileId]/route.test.ts index 16a98c257ea..57e377c8593 100644 --- a/apps/sim/app/api/v2/files/[fileId]/route.test.ts +++ b/apps/sim/app/api/v2/files/[fileId]/route.test.ts @@ -187,6 +187,23 @@ describe('v2 single-file routes', () => { expect(response.headers.get('X-Content-Type-Options')).toBe('nosniff') }) + it('downloads a file named by its files/ VFS path', async () => { + const vfsPath = 'files/Docs/data.csv' + const response = await GET( + new NextRequest( + `http://localhost:3000/api/v2/files/${encodeURIComponent(vfsPath)}?workspaceId=${WORKSPACE_ID}` + ), + { params: Promise.resolve({ fileId: vfsPath }) } + ) + + expect(response.status).toBe(200) + expect(mocks.download).toHaveBeenCalledWith( + expect.objectContaining({ + input: { fileId: vfsPath, acceptVfsPath: true, assertedWorkspaceId: WORKSPACE_ID }, + }) + ) + }) + it('conceals cross-workspace download authorization', async () => { mocks.download.mockRejectedValue(new NoWorkspaceAccessError()) diff --git a/apps/sim/app/api/v2/files/[fileId]/route.ts b/apps/sim/app/api/v2/files/[fileId]/route.ts index 8cec04f28f5..abdf31b1c2f 100644 --- a/apps/sim/app/api/v2/files/[fileId]/route.ts +++ b/apps/sim/app/api/v2/files/[fileId]/route.ts @@ -26,6 +26,7 @@ export const revalidate = 0 * The response carries no JSON envelope, so rate-limit state is surfaced via * `X-RateLimit-*` headers. Errors still render the canonical v2 JSON error body. * Lookups are workspace-scoped (IDOR-safe): a file in another workspace 404s. + * `[fileId]` also takes the `files//` VFS path listings print. * * A generated doc whose artifact is still compiling renders `CONFLICT`; retry. * @@ -41,6 +42,7 @@ export const GET = defineV2BinaryRoute({ errorPolicy: v2FileErrorPolicies.concealResourceAuthorization, mapInput: ({ params, query }) => ({ fileId: params.fileId, + acceptVfsPath: true, assertedWorkspaceId: query.workspaceId, }), useCase: downloadWorkspaceFileStream, diff --git a/apps/sim/lib/api/contracts/v2/files.ts b/apps/sim/lib/api/contracts/v2/files.ts index 4bf57c0f621..c04595e20a5 100644 --- a/apps/sim/lib/api/contracts/v2/files.ts +++ b/apps/sim/lib/api/contracts/v2/files.ts @@ -849,7 +849,7 @@ export const v2CompleteFileUploadContract = defineRouteContract({ export const v2DownloadFileContract = defineRouteContract({ method: 'GET', path: '/api/v2/files/[fileId]', - params: v2FileParamsSchema, + params: v2FileIdOrPathParamsSchema, query: v2FileWorkspaceQuerySchema, response: { mode: 'binary', diff --git a/apps/sim/lib/mothership/agent-cli/file-read-transport.ts b/apps/sim/lib/mothership/agent-cli/file-read-transport.ts index cb38de7bf8e..066a54a9abb 100644 --- a/apps/sim/lib/mothership/agent-cli/file-read-transport.ts +++ b/apps/sim/lib/mothership/agent-cli/file-read-transport.ts @@ -109,6 +109,7 @@ export function createFileReadTransport(context: { principal, input: { fileId: parsed.data.params.fileId, + acceptVfsPath: true, assertedWorkspaceId: parsed.data.query.workspaceId, }, }) diff --git a/apps/sim/lib/workspace-files/application/download-workspace-file.ts b/apps/sim/lib/workspace-files/application/download-workspace-file.ts index 47d47fa8e17..1842e82f168 100644 --- a/apps/sim/lib/workspace-files/application/download-workspace-file.ts +++ b/apps/sim/lib/workspace-files/application/download-workspace-file.ts @@ -14,16 +14,16 @@ import { requireAdmittedWorkspaceFile, } from '@/lib/workspace-files/application/admit-file-content' import { defineAuthorizedWorkspaceFileUseCase } from '@/lib/workspace-files/application/authorized-workspace-file-use-case' -import { resolveActiveWorkspaceFileContext } from '@/lib/workspace-files/application/file-context' +import { + resolveActiveWorkspaceFileContext, + type WorkspaceFileTargetInput, +} from '@/lib/workspace-files/application/file-context' import { finishFileDelivery } from '@/lib/workspace-files/application/finish-file-delivery' import { fileOperations } from '@/lib/workspace-files/application/operations' import { resolveRenderedWorkspaceArtifact } from '@/lib/workspace-files/application/resolve-rendered-workspace-artifact' import { createFileReadReceipt, type FileReadReceipt } from '@/lib/workspace-files/read-receipt' -export interface DownloadWorkspaceFileInput { - fileId: string - assertedWorkspaceId?: string -} +export type DownloadWorkspaceFileInput = WorkspaceFileTargetInput export interface DownloadWorkspaceFileResult { file: NonNullable>> @@ -107,7 +107,12 @@ async function executeDownloadWorkspaceFileStream({ const result = await streamWorkspaceFileRecord(file, principal) const provenance = await finishFileDelivery({ principal, - authorize: () => downloadWorkspaceFileStream.authorize({ principal, input }), + // Recheck the file the reference resolved to, not whatever its path names by now. + authorize: () => + downloadWorkspaceFileStream.authorize({ + principal, + input: { fileId: context.fileId, assertedWorkspaceId: input.assertedWorkspaceId }, + }), receipt: result.receipt ?? createFileReadReceipt({ entityType: 'workspace', entityId: context.workspaceId }, [ diff --git a/apps/sim/lib/workspace-files/delivery.integration.ts b/apps/sim/lib/workspace-files/delivery.integration.ts index 4a94dd5e3db..d7f59ea4bd1 100644 --- a/apps/sim/lib/workspace-files/delivery.integration.ts +++ b/apps/sim/lib/workspace-files/delivery.integration.ts @@ -20,7 +20,7 @@ import { import { setUploadDirServer, uploadsSetupMock } from '@sim/testing/mocks/uploads-setup.mock' import { getErrorMessage } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' -import { and, eq, inArray } from 'drizzle-orm' +import { and, eq, inArray, sql } from 'drizzle-orm' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('@/lib/uploads/core/setup.server', () => uploadsSetupMock) @@ -30,6 +30,7 @@ import { observeContentDelivery, runWorkflowOutsideContentDelivery, } from '@/lib/core/application' +import { createFileReadTransport } from '@/lib/mothership/agent-cli/file-read-transport' import { createOwnedWorkspaceFile, uploadWorkspaceFile, @@ -38,6 +39,7 @@ import { markWorkspaceFileSecretProvenanceUnknown } from '@/lib/uploads/contexts import * as storage from '@/lib/uploads/core/storage-service' import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types' import { hydrateUserFilesWithBase64 } from '@/lib/uploads/utils/user-file-base64.server' +import { v2FileErrorPolicies } from '@/lib/workspace-files/api' import { downloadWorkspaceFileStream } from '@/lib/workspace-files/application/download-workspace-file' import { readWorkspaceInlineFile } from '@/lib/workspace-files/application/read-workspace-inline-file' import { createFileReadReceipt, recheckFileReadReceipt } from '@/lib/workspace-files/read-receipt' @@ -302,6 +304,110 @@ check('same-revision provenance downgrade reaches the content admission gate', a expect(observed.at(-1)).toEqual({ status: 'unknown' }) }) +describe('Downloads addressed by a files/ VFS path', () => { + async function pathFixture() { + const f = await fixture() + const docsFolderId = generateId() + await db.execute(sql` + INSERT INTO folder (id, name, user_id, workspace_id, resource_type, parent_id) + VALUES (${docsFolderId}, 'Docs', ${f.editorId}, ${f.workspaceId}, 'file', NULL)`) + const upload = (content: string, folderId: string | null) => + uploadWorkspaceFile( + f.workspaceId, + f.editorId, + Buffer.from(content), + 'report.csv', + 'text/csv', + { folderId, notifyWorkspaceChange: false } + ) + const docs = await upload('docs,report\n', docsFolderId) + const root = await upload('root,report\n', null) + return { ...f, docs, root } + } + + const refusal = (promise: Promise) => + promise.then( + () => null, + (error: { code?: string; message?: string }) => ({ code: error.code, message: error.message }) + ) + + check('Chat downloads the one file a path names, exactly as it downloads by id', async () => { + const f = await pathFixture() + const transport = createFileReadTransport({ + endpoint: 'https://sim.test', + transport: async () => { + throw new Error('A single-file download never leaves the process') + }, + invocation: { userId: f.editorId, workspaceId: f.workspaceId }, + }) + const download = async (reference: string) => { + const response = await transport( + `https://sim.test/api/v2/files/${encodeURIComponent(reference)}?workspaceId=${f.workspaceId}` + ) + return { + status: response.status, + contentType: response.headers.get('content-type'), + contentLength: response.headers.get('content-length'), + body: await response.text(), + } + } + + const byPath = await download('files/Docs/report.csv') + expect(byPath).toMatchObject({ status: 200, body: 'docs,report\n' }) + expect(await download(f.docs.id)).toEqual(byPath) + expect((await download('files/report.csv')).body).toBe('root,report\n') + expect((await download('files/Missing/report.csv')).status).toBe(404) + }) + + check('a path answers a non-member exactly as a missing path and a missing id', async () => { + const f = await pathFixture() + const outsider = await fixture() + const download = (fileId: string) => + downloadWorkspaceFileStream.execute({ + principal: createSessionPrincipal({ userId: outsider.editorId }), + input: { fileId, acceptVfsPath: true, assertedWorkspaceId: f.workspaceId }, + }) + const existing = await refusal(download('files/Docs/report.csv')) + expect(existing).toMatchObject({ code: 'forbidden' }) + expect(await refusal(download('files/Docs/missing.csv'))).toEqual(existing) + + const rendered = async (fileId: string) => { + const response = await download(fileId).then( + () => null, + (error: unknown) => v2FileErrorPolicies.concealResourceAuthorization.render(error) + ) + return response && { status: response.status, body: await response.text() } + } + const answer = await rendered('files/Docs/report.csv') + expect(answer?.status).toBe(404) + expect(await rendered('files/Docs/missing.csv')).toEqual(answer) + expect(await rendered(`wf_${generateId()}`)).toEqual(answer) + }) + + check('the delivery recheck keys to the resolved file, not to its path', async () => { + const f = await pathFixture() + const open = storage.downloadFileStream + vi.spyOn(storage, 'downloadFileStream').mockImplementationOnce(async (options) => { + const source = await open(options) + await db + .update(workspaceFiles) + .set({ originalName: 'renamed.csv' }) + .where(eq(workspaceFiles.id, f.docs.id)) + return source + }) + const result = await downloadWorkspaceFileStream.execute({ + principal: f.principal, + input: { + fileId: 'files/Docs/report.csv', + acceptVfsPath: true, + assertedWorkspaceId: f.workspaceId, + }, + }) + expect(result.file.id).toBe(f.docs.id) + expect(await new Response(result.stream).text()).toBe('docs,report\n') + }) +}) + describe('A workflow run Chat started reads workspace files as any run does', () => { async function chatStartedRunRead(provenance: 'recorded' | 'unknown') { const f = await fixture() diff --git a/packages/sim-cli/src/commands/protocol/files-get.ts b/packages/sim-cli/src/commands/protocol/files-get.ts index ba1662d5345..c6b35a34ae9 100644 --- a/packages/sim-cli/src/commands/protocol/files-get.ts +++ b/packages/sim-cli/src/commands/protocol/files-get.ts @@ -352,8 +352,10 @@ async function downloadToOutput( } const savedTarget = await saveToFile(response.body, target, Boolean(options.force)) + // A VFS path names the file but is not its id, and the download response carries none. + const fileId = pathParams.fileId?.startsWith('files/') ? undefined : pathParams.fileId printProtocolResult(profile.output, { - ...(pathParams.fileId ? { id: pathParams.fileId } : {}), + ...(fileId ? { id: fileId } : {}), path: savedTarget, status: 'saved', }) @@ -429,7 +431,10 @@ export function attachProjectFileSnapshotExport(files: Command): void { export function attachFileGet(files: Command): void { const [command, connect] = apiCommand(files, 'get', ['downloadFile']) command - .argument('', 'File whose content to read') + .argument( + '', + 'File whose content to read: its id, or its `files//` VFS path' + ) .allowExcessArguments(false) .description('Download a file’s content to stdout or a local file') .option('-o, --output-file ', 'Write content to a file instead of stdout') diff --git a/packages/sim-cli/src/generated/v2-api.ts b/packages/sim-cli/src/generated/v2-api.ts index 64b146e3987..b5e898068a5 100644 --- a/packages/sim-cli/src/generated/v2-api.ts +++ b/packages/sim-cli/src/generated/v2-api.ts @@ -18319,7 +18319,10 @@ export const V2_OPERATIONS = { method: 'GET', path: '/api/v2/files/[fileId]', pathParams: ['fileId'] as const, - pathParamDocs: { fileId: 'File identifier.' }, + pathParamDocs: { + fileId: + 'File identifier, or the VFS path of a live file: `files//`. Pass the id when two files in a folder have names that display alike.', + }, responseMode: 'binary', summary: 'Download file', query: {