Skip to content

Commit 04ac686

Browse files
committed
fix(files): accept files/ VFS paths in v2 file describe, rename and delete
1 parent b3f9298 commit 04ac686

9 files changed

Lines changed: 271 additions & 25 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ sim files describe <fileId> [options]
102102

103103
| Argument | Required | Description |
104104
| --- | --- | --- |
105-
| `fileId` | Yes | File identifier. |
105+
| `fileId` | Yes | File identifier, or the file’s VFS path: `files/<folder>/<name>`. |
106106

107107
</CommandTable>
108108

@@ -303,7 +303,7 @@ sim files rename <fileId> [options]
303303

304304
| Argument | Required | Description |
305305
| --- | --- | --- |
306-
| `fileId` | Yes | File identifier. |
306+
| `fileId` | Yes | File identifier, or the file’s VFS path: `files/<folder>/<name>`. |
307307

308308
</CommandTable>
309309

@@ -613,7 +613,7 @@ sim files delete <fileId> [options]
613613

614614
| Argument | Required | Description |
615615
| --- | --- | --- |
616-
| `fileId` | Yes | File identifier. |
616+
| `fileId` | Yes | File identifier, or the file’s VFS path: `files/<folder>/<name>`. |
617617

618618
</CommandTable>
619619

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,7 +1171,7 @@ sim files delete <fileId> [options]
11711171

11721172
| Argument | Required | Description |
11731173
| --- | --- | --- |
1174-
| `fileId` | Yes | File identifier. |
1174+
| `fileId` | Yes | File identifier, or the file’s VFS path: `files/<folder>/<name>`. |
11751175

11761176
</CommandTable>
11771177

@@ -1416,7 +1416,7 @@ sim files describe <fileId> [options]
14161416

14171417
| Argument | Required | Description |
14181418
| --- | --- | --- |
1419-
| `fileId` | Yes | File identifier. |
1419+
| `fileId` | Yes | File identifier, or the file’s VFS path: `files/<folder>/<name>`. |
14201420

14211421
</CommandTable>
14221422

@@ -1581,7 +1581,7 @@ sim files rename <fileId> [options]
15811581

15821582
| Argument | Required | Description |
15831583
| --- | --- | --- |
1584-
| `fileId` | Yes | File identifier. |
1584+
| `fileId` | Yes | File identifier, or the file’s VFS path: `files/<folder>/<name>`. |
15851585

15861586
</CommandTable>
15871587

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

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2151,13 +2151,12 @@
21512151
"name": "fileId",
21522152
"in": "path",
21532153
"required": true,
2154-
"description": "File identifier.",
2154+
"description": "File identifier, or the file’s VFS path: `files/<folder>/<name>`.",
21552155
"schema": {
21562156
"type": "string",
21572157
"minLength": 1,
2158-
"maxLength": 128,
2159-
"pattern": "^[A-Za-z0-9_-]+$",
2160-
"description": "File identifier."
2158+
"maxLength": 4096,
2159+
"description": "File identifier, or the file’s VFS path: `files/<folder>/<name>`."
21612160
}
21622161
},
21632162
{
@@ -2238,13 +2237,12 @@
22382237
"name": "fileId",
22392238
"in": "path",
22402239
"required": true,
2241-
"description": "File identifier.",
2240+
"description": "File identifier, or the file’s VFS path: `files/<folder>/<name>`.",
22422241
"schema": {
22432242
"type": "string",
22442243
"minLength": 1,
2245-
"maxLength": 128,
2246-
"pattern": "^[A-Za-z0-9_-]+$",
2247-
"description": "File identifier."
2244+
"maxLength": 4096,
2245+
"description": "File identifier, or the file’s VFS path: `files/<folder>/<name>`."
22482246
}
22492247
}
22502248
],
@@ -2489,13 +2487,12 @@
24892487
"name": "fileId",
24902488
"in": "path",
24912489
"required": true,
2492-
"description": "File identifier.",
2490+
"description": "File identifier, or the file’s VFS path: `files/<folder>/<name>`.",
24932491
"schema": {
24942492
"type": "string",
24952493
"minLength": 1,
2496-
"maxLength": 128,
2497-
"pattern": "^[A-Za-z0-9_-]+$",
2498-
"description": "File identifier."
2494+
"maxLength": 4096,
2495+
"description": "File identifier, or the file’s VFS path: `files/<folder>/<name>`."
24992496
}
25002497
},
25012498
{

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import {
4545
import { MAX_FOLDER_PATH_BYTES, MAX_FOLDER_PATH_SEGMENTS } from '@/lib/folders/paths'
4646
import { MAX_WORKSPACE_FILE_SIZE } from '@/lib/uploads/shared/types'
4747
import { MAX_TEXT_EXTRACTION_BYTES } from '@/lib/uploads/utils/file'
48+
import { isWorkspaceFileVfsPath } from '@/lib/vfs/path'
4849
import { MAX_ZIP_DOWNLOAD_FILES } from '@/lib/workspace-files/limits'
4950
import {
5051
FILE_SEARCH_DEFAULT_MAX_RESULTS,
@@ -302,6 +303,17 @@ export const v2FileReferenceParamsSchema = z.object({
302303

303304
export type V2FileReferenceParams = z.output<typeof v2FileReferenceParamsSchema>
304305

306+
/** Also takes the `files/…` path `glob` and listings print, so no listing round-trip finds the id. */
307+
const v2FileIdOrPathParamsSchema = z.object({
308+
fileId: requiredFieldSchema('File reference is required')
309+
.max(MAX_FOLDER_PATH_BYTES, 'File reference is too long')
310+
.refine(
311+
(value) => isWorkspaceFileVfsPath(value) || workspaceFileIdSchema.safeParse(value).success,
312+
'Invalid file id or path: pass a file id or its `files/<folder>/<name>` VFS path'
313+
)
314+
.describe('File identifier, or the file’s VFS path: `files/<folder>/<name>`.'),
315+
})
316+
305317
export const v2CreateFileBodySchema = z
306318
.object({
307319
workspaceId: workspaceIdSchema.describe('Workspace in which to create the file.'),
@@ -943,7 +955,7 @@ export const v2ReadFileTextContract = defineRouteContract({
943955
export const v2GetFileContract = defineRouteContract({
944956
method: 'GET',
945957
path: '/api/v2/files/[fileId]/metadata',
946-
params: v2FileParamsSchema,
958+
params: v2FileIdOrPathParamsSchema,
947959
query: v2GetFileMetadataQuerySchema,
948960
response: {
949961
mode: 'json',
@@ -955,7 +967,7 @@ export const v2RenameFileContract = defineRouteContract({
955967
method: 'PATCH',
956968
path: '/api/v2/files/[fileId]',
957969
query: noInputSchema,
958-
params: v2FileParamsSchema,
970+
params: v2FileIdOrPathParamsSchema,
959971
body: v2RenameFileBodySchema,
960972
response: {
961973
mode: 'json',
@@ -966,7 +978,7 @@ export const v2RenameFileContract = defineRouteContract({
966978
export const v2DeleteFileContract = defineRouteContract({
967979
method: 'DELETE',
968980
path: '/api/v2/files/[fileId]',
969-
params: v2FileParamsSchema,
981+
params: v2FileIdOrPathParamsSchema,
970982
query: v2FileWorkspaceQuerySchema,
971983
response: {
972984
mode: 'json',

‎apps/sim/lib/uploads/contexts/workspace/file-manager.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2342,6 +2342,21 @@ async function getWorkspaceFileByExactReference(
23422342
return folderId ? getWorkspaceFileByName(workspaceId, segments.at(-1) ?? '', { folderId }) : null
23432343
}
23442344

2345+
/**
2346+
* The live workspace file at an exact `files/…` VFS path, or null. Unlike
2347+
* {@link resolveWorkspaceFileReference} there is no name or listing fallback, so a mutation
2348+
* addressed by path lands only on the one file it names. Throws `VfsPathError` if malformed.
2349+
*/
2350+
export async function getWorkspaceFileByVfsPath(
2351+
workspaceId: string,
2352+
vfsPath: string
2353+
): Promise<WorkspaceFileRecord | null> {
2354+
return getWorkspaceFileByExactReference(
2355+
workspaceId,
2356+
normalizeWorkspaceFileReferenceSegments(vfsPath)
2357+
)
2358+
}
2359+
23452360
/**
23462361
* Resolve a workspace file record from either its id or a VFS/name reference.
23472362
*

‎apps/sim/lib/vfs/path.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ export class VfsPathError extends Error {
1010
}
1111
}
1212

13+
/** A `files/…` workspace-file VFS path. File ids are drawn from `[A-Za-z0-9_-]`, so no id is one. */
14+
export function isWorkspaceFileVfsPath(reference: string): boolean {
15+
return reference.startsWith('files/')
16+
}
17+
1318
/**
1419
* The name a VFS path segment displays: NFC-composed, trimmed, control characters removed,
1520
* whitespace runs collapsed. Its SQL twin is `displaySegmentKey` in `@sim/db/schema`, which an
Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
/**
2+
* Real PostgreSQL behavior of single-file operations addressed by a `files/…` VFS path instead
3+
* of an id: the path names exactly one live file in the asserted workspace, never a same-named
4+
* file in another folder or workspace, a chat upload or an archived file, and authorization is
5+
* unchanged.
6+
*/
7+
import { db } from '@sim/db'
8+
import { permissions, user, workspace, workspaceFiles } from '@sim/db/schema'
9+
import { assertDisposableTestDatabaseUrl } from '@sim/db/testing/test-infrastructure'
10+
import { deleteWorkspaceFixture, insertWorkspaceFixture } from '@sim/db/testing/workspace-fixtures'
11+
import { generateId } from '@sim/utils/id'
12+
import { eq, inArray, sql } from 'drizzle-orm'
13+
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
14+
import { deleteWorkspaceFileOperation } from '@/lib/workspace-files/application/delete-workspace-file'
15+
import { readWorkspaceFileMetadataWithVersion } from '@/lib/workspace-files/application/read-workspace-file-metadata'
16+
import { renameWorkspaceFile } from '@/lib/workspace-files/application/rename-workspace-file'
17+
18+
const alice = generateId()
19+
const bob = generateId()
20+
const carol = generateId()
21+
const workspaceId = generateId()
22+
const otherWorkspaceId = generateId()
23+
const folders = { docs: generateId(), media: generateId(), foreign: generateId() }
24+
const session = (userId: string) => ({ kind: 'session' as const, userId, sessionId: generateId() })
25+
26+
async function insertFile(input: {
27+
name: string
28+
folderId?: string | null
29+
workspaceId?: string
30+
context?: string
31+
deleted?: boolean
32+
}): Promise<string> {
33+
const id = `wf_${generateId()}`
34+
const owner = input.workspaceId ?? workspaceId
35+
await db.execute(sql`
36+
INSERT INTO ${workspaceFiles} (id, key, user_id, workspace_id, folder_id, context, original_name,
37+
content_type, size_bytes, uploaded_at, updated_at, deleted_at)
38+
VALUES (${id}, ${`workspace/${owner}/${generateId()}-${input.name}`}, ${alice}, ${owner},
39+
${input.folderId ?? null}, ${input.context ?? 'workspace'}, ${input.name}, 'text/csv', 10,
40+
now(), now(), ${input.deleted ? sql`now()` : sql`NULL`})`)
41+
return id
42+
}
43+
44+
async function row(id: string) {
45+
const [file] = await db
46+
.select({ name: workspaceFiles.originalName, deletedAt: workspaceFiles.deletedAt })
47+
.from(workspaceFiles)
48+
.where(eq(workspaceFiles.id, id))
49+
return file
50+
}
51+
52+
const describeFile = (fileId: string, userId = alice, assertedWorkspaceId = workspaceId) =>
53+
readWorkspaceFileMetadataWithVersion.execute({
54+
principal: session(userId),
55+
input: { fileId, assertedWorkspaceId },
56+
})
57+
58+
const deleteFile = (fileId: string, userId = alice) =>
59+
deleteWorkspaceFileOperation.execute({
60+
principal: session(userId),
61+
input: { fileId, assertedWorkspaceId: workspaceId },
62+
})
63+
64+
describe('single-file operations addressed by VFS path', () => {
65+
const ids = {
66+
rootReport: '',
67+
docsReport: '',
68+
mediaReport: '',
69+
foreignReport: '',
70+
chatUpload: '',
71+
archived: '',
72+
}
73+
74+
beforeAll(async () => {
75+
assertDisposableTestDatabaseUrl(process.env.DATABASE_URL ?? '')
76+
await db.insert(user).values(
77+
[alice, bob, carol].map((id) => ({
78+
id,
79+
name: `Fixture ${id}`,
80+
email: `${id}@fixture.test`,
81+
emailVerified: true,
82+
createdAt: new Date(),
83+
updatedAt: new Date(),
84+
}))
85+
)
86+
await insertWorkspaceFixture(db, [
87+
{ id: workspaceId, name: 'Path fixture', ownerId: alice, billedAccountUserId: alice },
88+
{ id: otherWorkspaceId, name: 'Other fixture', ownerId: alice, billedAccountUserId: alice },
89+
])
90+
await db.insert(permissions).values([
91+
{
92+
id: generateId(),
93+
userId: alice,
94+
entityType: 'workspace',
95+
entityId: workspaceId,
96+
permissionType: 'admin',
97+
},
98+
{
99+
id: generateId(),
100+
userId: bob,
101+
entityType: 'workspace',
102+
entityId: workspaceId,
103+
permissionType: 'read',
104+
},
105+
{
106+
id: generateId(),
107+
userId: alice,
108+
entityType: 'workspace',
109+
entityId: otherWorkspaceId,
110+
permissionType: 'admin',
111+
},
112+
])
113+
await db.execute(sql`
114+
INSERT INTO folder (id, name, user_id, workspace_id, resource_type, parent_id) VALUES
115+
(${folders.docs}, 'Docs', ${alice}, ${workspaceId}, 'file', NULL),
116+
(${folders.media}, 'Media', ${alice}, ${workspaceId}, 'file', NULL),
117+
(${folders.foreign}, 'Docs', ${alice}, ${otherWorkspaceId}, 'file', NULL)`)
118+
ids.rootReport = await insertFile({ name: 'report.csv' })
119+
ids.docsReport = await insertFile({ name: 'report.csv', folderId: folders.docs })
120+
ids.mediaReport = await insertFile({ name: 'report.csv', folderId: folders.media })
121+
ids.foreignReport = await insertFile({
122+
name: 'report.csv',
123+
folderId: folders.foreign,
124+
workspaceId: otherWorkspaceId,
125+
})
126+
ids.chatUpload = await insertFile({ name: 'notes.txt', context: 'mothership' })
127+
ids.archived = await insertFile({ name: 'old.csv', deleted: true })
128+
})
129+
130+
afterAll(async () => {
131+
await deleteWorkspaceFixture(db, inArray(workspace.id, [workspaceId, otherWorkspaceId]))
132+
await db.delete(user).where(inArray(user.id, [alice, bob, carol]))
133+
})
134+
135+
it('describes the one file at the exact path, not a same-named file elsewhere', async () => {
136+
expect((await describeFile('files/Docs/report.csv')).file.id).toBe(ids.docsReport)
137+
expect((await describeFile('files/report.csv')).file.id).toBe(ids.rootReport)
138+
await expect(describeFile('files/Archive/report.csv')).rejects.toMatchObject({
139+
code: 'not_found',
140+
})
141+
await expect(describeFile('files/Docs')).rejects.toMatchObject({ code: 'not_found' })
142+
})
143+
144+
it('renames and deletes only the file the path names', async () => {
145+
const renamed = await renameWorkspaceFile.execute({
146+
principal: session(alice),
147+
input: { fileId: 'files/Docs/report.csv', assertedWorkspaceId: workspaceId, name: 'q3.csv' },
148+
})
149+
expect(renamed.file.id).toBe(ids.docsReport)
150+
expect((await row(ids.docsReport))?.name).toBe('q3.csv')
151+
152+
expect(await deleteFile('files/Media/report.csv')).toMatchObject({
153+
id: ids.mediaReport,
154+
deleted: true,
155+
})
156+
expect((await row(ids.mediaReport))?.deletedAt).not.toBeNull()
157+
158+
for (const untouched of [ids.rootReport, ids.foreignReport]) {
159+
expect(await row(untouched)).toMatchObject({ name: 'report.csv', deletedAt: null })
160+
}
161+
})
162+
163+
it('resolves a path only inside the asserted workspace', async () => {
164+
await expect(
165+
describeFile('files/Docs/report.csv', alice, otherWorkspaceId)
166+
).resolves.toMatchObject({ file: { id: ids.foreignReport } })
167+
await expect(
168+
readWorkspaceFileMetadataWithVersion.execute({
169+
principal: session(alice),
170+
input: { fileId: 'files/report.csv' },
171+
})
172+
).rejects.toMatchObject({ code: 'not_found' })
173+
})
174+
175+
it('never reaches a chat upload or an archived file through a path', async () => {
176+
await expect(deleteFile('files/notes.txt')).rejects.toMatchObject({ code: 'not_found' })
177+
expect((await row(ids.chatUpload))?.deletedAt).toBeNull()
178+
await expect(describeFile('files/old.csv')).rejects.toMatchObject({ code: 'not_found' })
179+
})
180+
181+
it('keeps authorization: a read member cannot delete and a non-member cannot read', async () => {
182+
await expect(deleteFile('files/report.csv', bob)).rejects.toMatchObject({ code: 'forbidden' })
183+
await expect(describeFile('files/report.csv', carol)).rejects.toMatchObject({
184+
code: 'forbidden',
185+
})
186+
expect((await row(ids.rootReport))?.deletedAt).toBeNull()
187+
})
188+
189+
it('rejects a dot-segment path instead of walking out of its folder', async () => {
190+
await expect(deleteFile('files/Docs/../report.csv')).rejects.toMatchObject({
191+
code: 'validation',
192+
})
193+
expect((await row(ids.rootReport))?.deletedAt).toBeNull()
194+
})
195+
})

0 commit comments

Comments
 (0)