Skip to content

Commit ca7acbc

Browse files
authored
fix(files): accept files/ VFS paths in v2 file download and sim files get (#9637)
1 parent 781c896 commit ca7acbc

11 files changed

Lines changed: 155 additions & 17 deletions

File tree

‎apps/docs/content/docs/cli/files.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ sim files get <fileId> [options]
183183

184184
| Argument | Required | Description |
185185
| --- | --- | --- |
186-
| `fileId` | Yes | File whose content to read |
186+
| `fileId` | Yes | File whose content to read: its id, or its `files/<folder>/<name>` VFS path |
187187

188188
</CommandTable>
189189

‎apps/docs/content/docs/cli/reference.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1761,7 +1761,7 @@ sim files get <fileId> [options]
17611761

17621762
| Argument | Required | Description |
17631763
| --- | --- | --- |
1764-
| `fileId` | Yes | File whose content to read |
1764+
| `fileId` | Yes | File whose content to read: its id, or its `files/<folder>/<name>` VFS path |
17651765

17661766
</CommandTable>
17671767

‎apps/docs/openapi-v2-files-audit.json‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2048,13 +2048,12 @@
20482048
"name": "fileId",
20492049
"in": "path",
20502050
"required": true,
2051-
"description": "File identifier.",
2051+
"description": "File identifier, or the VFS path of a live file: `files/<folder>/<name>`. Pass the id when two files in a folder have names that display alike.",
20522052
"schema": {
20532053
"type": "string",
20542054
"minLength": 1,
2055-
"maxLength": 128,
2056-
"pattern": "^[A-Za-z0-9_-]+$",
2057-
"description": "File identifier."
2055+
"maxLength": 4096,
2056+
"description": "File identifier, or the VFS path of a live file: `files/<folder>/<name>`. Pass the id when two files in a folder have names that display alike."
20582057
}
20592058
},
20602059
{

‎apps/sim/app/api/v2/files/[fileId]/route.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,23 @@ describe('v2 single-file routes', () => {
187187
expect(response.headers.get('X-Content-Type-Options')).toBe('nosniff')
188188
})
189189

190+
it('downloads a file named by its files/ VFS path', async () => {
191+
const vfsPath = 'files/Docs/data.csv'
192+
const response = await GET(
193+
new NextRequest(
194+
`http://localhost:3000/api/v2/files/${encodeURIComponent(vfsPath)}?workspaceId=${WORKSPACE_ID}`
195+
),
196+
{ params: Promise.resolve({ fileId: vfsPath }) }
197+
)
198+
199+
expect(response.status).toBe(200)
200+
expect(mocks.download).toHaveBeenCalledWith(
201+
expect.objectContaining({
202+
input: { fileId: vfsPath, acceptVfsPath: true, assertedWorkspaceId: WORKSPACE_ID },
203+
})
204+
)
205+
})
206+
190207
it('conceals cross-workspace download authorization', async () => {
191208
mocks.download.mockRejectedValue(new NoWorkspaceAccessError())
192209

‎apps/sim/app/api/v2/files/[fileId]/route.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ export const revalidate = 0
2626
* The response carries no JSON envelope, so rate-limit state is surfaced via
2727
* `X-RateLimit-*` headers. Errors still render the canonical v2 JSON error body.
2828
* Lookups are workspace-scoped (IDOR-safe): a file in another workspace 404s.
29+
* `[fileId]` also takes the `files/<folder>/<name>` VFS path listings print.
2930
*
3031
* A generated doc whose artifact is still compiling renders `CONFLICT`; retry.
3132
*
@@ -41,6 +42,7 @@ export const GET = defineV2BinaryRoute({
4142
errorPolicy: v2FileErrorPolicies.concealResourceAuthorization,
4243
mapInput: ({ params, query }) => ({
4344
fileId: params.fileId,
45+
acceptVfsPath: true,
4446
assertedWorkspaceId: query.workspaceId,
4547
}),
4648
useCase: downloadWorkspaceFileStream,

‎apps/sim/lib/api/contracts/v2/files.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -849,7 +849,7 @@ export const v2CompleteFileUploadContract = defineRouteContract({
849849
export const v2DownloadFileContract = defineRouteContract({
850850
method: 'GET',
851851
path: '/api/v2/files/[fileId]',
852-
params: v2FileParamsSchema,
852+
params: v2FileIdOrPathParamsSchema,
853853
query: v2FileWorkspaceQuerySchema,
854854
response: {
855855
mode: 'binary',

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ export function createFileReadTransport(context: {
109109
principal,
110110
input: {
111111
fileId: parsed.data.params.fileId,
112+
acceptVfsPath: true,
112113
assertedWorkspaceId: parsed.data.query.workspaceId,
113114
},
114115
})

‎apps/sim/lib/workspace-files/application/download-workspace-file.ts‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,16 @@ import {
1414
requireAdmittedWorkspaceFile,
1515
} from '@/lib/workspace-files/application/admit-file-content'
1616
import { defineAuthorizedWorkspaceFileUseCase } from '@/lib/workspace-files/application/authorized-workspace-file-use-case'
17-
import { resolveActiveWorkspaceFileContext } from '@/lib/workspace-files/application/file-context'
17+
import {
18+
resolveActiveWorkspaceFileContext,
19+
type WorkspaceFileTargetInput,
20+
} from '@/lib/workspace-files/application/file-context'
1821
import { finishFileDelivery } from '@/lib/workspace-files/application/finish-file-delivery'
1922
import { fileOperations } from '@/lib/workspace-files/application/operations'
2023
import { resolveRenderedWorkspaceArtifact } from '@/lib/workspace-files/application/resolve-rendered-workspace-artifact'
2124
import { createFileReadReceipt, type FileReadReceipt } from '@/lib/workspace-files/read-receipt'
2225

23-
export interface DownloadWorkspaceFileInput {
24-
fileId: string
25-
assertedWorkspaceId?: string
26-
}
26+
export type DownloadWorkspaceFileInput = WorkspaceFileTargetInput
2727

2828
export interface DownloadWorkspaceFileResult {
2929
file: NonNullable<Awaited<ReturnType<typeof getWorkspaceFile>>>
@@ -107,7 +107,12 @@ async function executeDownloadWorkspaceFileStream({
107107
const result = await streamWorkspaceFileRecord(file, principal)
108108
const provenance = await finishFileDelivery({
109109
principal,
110-
authorize: () => downloadWorkspaceFileStream.authorize({ principal, input }),
110+
// Recheck the file the reference resolved to, not whatever its path names by now.
111+
authorize: () =>
112+
downloadWorkspaceFileStream.authorize({
113+
principal,
114+
input: { fileId: context.fileId, assertedWorkspaceId: input.assertedWorkspaceId },
115+
}),
111116
receipt:
112117
result.receipt ??
113118
createFileReadReceipt({ entityType: 'workspace', entityId: context.workspaceId }, [

‎apps/sim/lib/workspace-files/delivery.integration.ts‎

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
import { setUploadDirServer, uploadsSetupMock } from '@sim/testing/mocks/uploads-setup.mock'
2121
import { getErrorMessage } from '@sim/utils/errors'
2222
import { generateId } from '@sim/utils/id'
23-
import { and, eq, inArray } from 'drizzle-orm'
23+
import { and, eq, inArray, sql } from 'drizzle-orm'
2424
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
2525

2626
vi.mock('@/lib/uploads/core/setup.server', () => uploadsSetupMock)
@@ -30,6 +30,7 @@ import {
3030
observeContentDelivery,
3131
runWorkflowOutsideContentDelivery,
3232
} from '@/lib/core/application'
33+
import { createFileReadTransport } from '@/lib/mothership/agent-cli/file-read-transport'
3334
import {
3435
createOwnedWorkspaceFile,
3536
uploadWorkspaceFile,
@@ -38,6 +39,7 @@ import { markWorkspaceFileSecretProvenanceUnknown } from '@/lib/uploads/contexts
3839
import * as storage from '@/lib/uploads/core/storage-service'
3940
import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types'
4041
import { hydrateUserFilesWithBase64 } from '@/lib/uploads/utils/user-file-base64.server'
42+
import { v2FileErrorPolicies } from '@/lib/workspace-files/api'
4143
import { downloadWorkspaceFileStream } from '@/lib/workspace-files/application/download-workspace-file'
4244
import { readWorkspaceInlineFile } from '@/lib/workspace-files/application/read-workspace-inline-file'
4345
import { createFileReadReceipt, recheckFileReadReceipt } from '@/lib/workspace-files/read-receipt'
@@ -302,6 +304,110 @@ check('same-revision provenance downgrade reaches the content admission gate', a
302304
expect(observed.at(-1)).toEqual({ status: 'unknown' })
303305
})
304306

307+
describe('Downloads addressed by a files/ VFS path', () => {
308+
async function pathFixture() {
309+
const f = await fixture()
310+
const docsFolderId = generateId()
311+
await db.execute(sql`
312+
INSERT INTO folder (id, name, user_id, workspace_id, resource_type, parent_id)
313+
VALUES (${docsFolderId}, 'Docs', ${f.editorId}, ${f.workspaceId}, 'file', NULL)`)
314+
const upload = (content: string, folderId: string | null) =>
315+
uploadWorkspaceFile(
316+
f.workspaceId,
317+
f.editorId,
318+
Buffer.from(content),
319+
'report.csv',
320+
'text/csv',
321+
{ folderId, notifyWorkspaceChange: false }
322+
)
323+
const docs = await upload('docs,report\n', docsFolderId)
324+
const root = await upload('root,report\n', null)
325+
return { ...f, docs, root }
326+
}
327+
328+
const refusal = (promise: Promise<unknown>) =>
329+
promise.then(
330+
() => null,
331+
(error: { code?: string; message?: string }) => ({ code: error.code, message: error.message })
332+
)
333+
334+
check('Chat downloads the one file a path names, exactly as it downloads by id', async () => {
335+
const f = await pathFixture()
336+
const transport = createFileReadTransport({
337+
endpoint: 'https://sim.test',
338+
transport: async () => {
339+
throw new Error('A single-file download never leaves the process')
340+
},
341+
invocation: { userId: f.editorId, workspaceId: f.workspaceId },
342+
})
343+
const download = async (reference: string) => {
344+
const response = await transport(
345+
`https://sim.test/api/v2/files/${encodeURIComponent(reference)}?workspaceId=${f.workspaceId}`
346+
)
347+
return {
348+
status: response.status,
349+
contentType: response.headers.get('content-type'),
350+
contentLength: response.headers.get('content-length'),
351+
body: await response.text(),
352+
}
353+
}
354+
355+
const byPath = await download('files/Docs/report.csv')
356+
expect(byPath).toMatchObject({ status: 200, body: 'docs,report\n' })
357+
expect(await download(f.docs.id)).toEqual(byPath)
358+
expect((await download('files/report.csv')).body).toBe('root,report\n')
359+
expect((await download('files/Missing/report.csv')).status).toBe(404)
360+
})
361+
362+
check('a path answers a non-member exactly as a missing path and a missing id', async () => {
363+
const f = await pathFixture()
364+
const outsider = await fixture()
365+
const download = (fileId: string) =>
366+
downloadWorkspaceFileStream.execute({
367+
principal: createSessionPrincipal({ userId: outsider.editorId }),
368+
input: { fileId, acceptVfsPath: true, assertedWorkspaceId: f.workspaceId },
369+
})
370+
const existing = await refusal(download('files/Docs/report.csv'))
371+
expect(existing).toMatchObject({ code: 'forbidden' })
372+
expect(await refusal(download('files/Docs/missing.csv'))).toEqual(existing)
373+
374+
const rendered = async (fileId: string) => {
375+
const response = await download(fileId).then(
376+
() => null,
377+
(error: unknown) => v2FileErrorPolicies.concealResourceAuthorization.render(error)
378+
)
379+
return response && { status: response.status, body: await response.text() }
380+
}
381+
const answer = await rendered('files/Docs/report.csv')
382+
expect(answer?.status).toBe(404)
383+
expect(await rendered('files/Docs/missing.csv')).toEqual(answer)
384+
expect(await rendered(`wf_${generateId()}`)).toEqual(answer)
385+
})
386+
387+
check('the delivery recheck keys to the resolved file, not to its path', async () => {
388+
const f = await pathFixture()
389+
const open = storage.downloadFileStream
390+
vi.spyOn(storage, 'downloadFileStream').mockImplementationOnce(async (options) => {
391+
const source = await open(options)
392+
await db
393+
.update(workspaceFiles)
394+
.set({ originalName: 'renamed.csv' })
395+
.where(eq(workspaceFiles.id, f.docs.id))
396+
return source
397+
})
398+
const result = await downloadWorkspaceFileStream.execute({
399+
principal: f.principal,
400+
input: {
401+
fileId: 'files/Docs/report.csv',
402+
acceptVfsPath: true,
403+
assertedWorkspaceId: f.workspaceId,
404+
},
405+
})
406+
expect(result.file.id).toBe(f.docs.id)
407+
expect(await new Response(result.stream).text()).toBe('docs,report\n')
408+
})
409+
})
410+
305411
describe('A workflow run Chat started reads workspace files as any run does', () => {
306412
async function chatStartedRunRead(provenance: 'recorded' | 'unknown') {
307413
const f = await fixture()

‎packages/sim-cli/src/commands/protocol/files-get.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,10 @@ async function downloadToOutput<Operation extends DownloadOperation>(
352352
}
353353

354354
const savedTarget = await saveToFile(response.body, target, Boolean(options.force))
355+
// A VFS path names the file but is not its id, and the download response carries none.
356+
const fileId = pathParams.fileId?.startsWith('files/') ? undefined : pathParams.fileId
355357
printProtocolResult(profile.output, {
356-
...(pathParams.fileId ? { id: pathParams.fileId } : {}),
358+
...(fileId ? { id: fileId } : {}),
357359
path: savedTarget,
358360
status: 'saved',
359361
})
@@ -429,7 +431,10 @@ export function attachProjectFileSnapshotExport(files: Command): void {
429431
export function attachFileGet(files: Command): void {
430432
const [command, connect] = apiCommand(files, 'get', ['downloadFile'])
431433
command
432-
.argument('<fileId>', 'File whose content to read')
434+
.argument(
435+
'<fileId>',
436+
'File whose content to read: its id, or its `files/<folder>/<name>` VFS path'
437+
)
433438
.allowExcessArguments(false)
434439
.description('Download a file’s content to stdout or a local file')
435440
.option('-o, --output-file <path>', 'Write content to a file instead of stdout')

0 commit comments

Comments
 (0)