Skip to content

Commit 1fcf99d

Browse files
waleedlatif1waleedlatif1
committed
fix(files): resolve v2 file paths only on the v2 surface, through the canonical resolver
1 parent 04b66d4 commit 1fcf99d

15 files changed

Lines changed: 157 additions & 101 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, or the file’s VFS path: `files/<folder>/<name>`. |
105+
| `fileId` | Yes | 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. |
106106

107107
</CommandTable>
108108

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

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

308308
</CommandTable>
309309

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

614614
| Argument | Required | Description |
615615
| --- | --- | --- |
616-
| `fileId` | Yes | File identifier, or the file’s VFS path: `files/<folder>/<name>`. |
616+
| `fileId` | Yes | 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. |
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
@@ -1181,7 +1181,7 @@ sim files delete <fileId> [options]
11811181

11821182
| Argument | Required | Description |
11831183
| --- | --- | --- |
1184-
| `fileId` | Yes | File identifier, or the file’s VFS path: `files/<folder>/<name>`. |
1184+
| `fileId` | Yes | 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. |
11851185

11861186
</CommandTable>
11871187

@@ -1426,7 +1426,7 @@ sim files describe <fileId> [options]
14261426

14271427
| Argument | Required | Description |
14281428
| --- | --- | --- |
1429-
| `fileId` | Yes | File identifier, or the file’s VFS path: `files/<folder>/<name>`. |
1429+
| `fileId` | Yes | 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. |
14301430

14311431
</CommandTable>
14321432

@@ -1591,7 +1591,7 @@ sim files rename <fileId> [options]
15911591

15921592
| Argument | Required | Description |
15931593
| --- | --- | --- |
1594-
| `fileId` | Yes | File identifier, or the file’s VFS path: `files/<folder>/<name>`. |
1594+
| `fileId` | Yes | 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. |
15951595

15961596
</CommandTable>
15971597

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2151,12 +2151,12 @@
21512151
"name": "fileId",
21522152
"in": "path",
21532153
"required": true,
2154-
"description": "File identifier, or the file’s VFS path: `files/<folder>/<name>`.",
2154+
"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.",
21552155
"schema": {
21562156
"type": "string",
21572157
"minLength": 1,
21582158
"maxLength": 4096,
2159-
"description": "File identifier, or the file’s VFS path: `files/<folder>/<name>`."
2159+
"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."
21602160
}
21612161
},
21622162
{
@@ -2237,12 +2237,12 @@
22372237
"name": "fileId",
22382238
"in": "path",
22392239
"required": true,
2240-
"description": "File identifier, or the file’s VFS path: `files/<folder>/<name>`.",
2240+
"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.",
22412241
"schema": {
22422242
"type": "string",
22432243
"minLength": 1,
22442244
"maxLength": 4096,
2245-
"description": "File identifier, or the file’s VFS path: `files/<folder>/<name>`."
2245+
"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."
22462246
}
22472247
}
22482248
],
@@ -2487,12 +2487,12 @@
24872487
"name": "fileId",
24882488
"in": "path",
24892489
"required": true,
2490-
"description": "File identifier, or the file’s VFS path: `files/<folder>/<name>`.",
2490+
"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.",
24912491
"schema": {
24922492
"type": "string",
24932493
"minLength": 1,
24942494
"maxLength": 4096,
2495-
"description": "File identifier, or the file’s VFS path: `files/<folder>/<name>`."
2495+
"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."
24962496
}
24972497
},
24982498
{

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,12 @@ describe('GET /api/v2/files/[fileId]/metadata', () => {
113113
expect((await response.json()).error.code).toBe('NOT_FOUND')
114114
expect(readMetadata).toHaveBeenCalledWith(
115115
expect.objectContaining({
116-
input: { fileId: FILE_ID, assertedWorkspaceId: WORKSPACE_ID, includeDeleted: false },
116+
input: {
117+
fileId: FILE_ID,
118+
acceptVfsPath: true,
119+
assertedWorkspaceId: WORKSPACE_ID,
120+
includeDeleted: false,
121+
},
117122
})
118123
)
119124
})
@@ -127,7 +132,12 @@ describe('GET /api/v2/files/[fileId]/metadata', () => {
127132
expect((await response.json()).error.code).toBe('NOT_FOUND')
128133
expect(readMetadata).toHaveBeenCalledWith(
129134
expect.objectContaining({
130-
input: { fileId: FILE_ID, assertedWorkspaceId: WORKSPACE_ID, includeDeleted: false },
135+
input: {
136+
fileId: FILE_ID,
137+
acceptVfsPath: true,
138+
assertedWorkspaceId: WORKSPACE_ID,
139+
includeDeleted: false,
140+
},
131141
})
132142
)
133143
})
@@ -159,7 +169,12 @@ describe('GET /api/v2/files/[fileId]/metadata', () => {
159169
expect(readMetadata).toHaveBeenCalledWith(
160170
expect.objectContaining({
161171
principal: auth.principal,
162-
input: { fileId: FILE_ID, assertedWorkspaceId: WORKSPACE_ID, includeDeleted: true },
172+
input: {
173+
fileId: FILE_ID,
174+
acceptVfsPath: true,
175+
assertedWorkspaceId: WORKSPACE_ID,
176+
includeDeleted: true,
177+
},
163178
})
164179
)
165180
})

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ export const GET = defineV2JsonRoute({
2626
errorPolicy: v2FileErrorPolicies.concealResourceAuthorization,
2727
mapInput: ({ params, query }) => ({
2828
fileId: params.fileId,
29+
acceptVfsPath: true,
2930
assertedWorkspaceId: query.workspaceId,
3031
includeDeleted: query.scope === 'archived',
3132
}),

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ export const PATCH = defineV2JsonRoute({
6363
errorPolicy: v2FileErrorPolicies.concealResourceAuthorization,
6464
mapInput: ({ params, body }) => ({
6565
fileId: params.fileId,
66+
acceptVfsPath: true,
6667
assertedWorkspaceId: body.workspaceId,
6768
name: body.name,
6869
}),
@@ -85,6 +86,7 @@ export const DELETE = defineV2JsonRoute({
8586
errorPolicy: v2FileErrorPolicies.concealResourceAuthorization,
8687
mapInput: ({ params, query }) => ({
8788
fileId: params.fileId,
89+
acceptVfsPath: true,
8890
assertedWorkspaceId: query.workspaceId,
8991
}),
9092
useCase: deleteWorkspaceFileOperation,

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,9 @@ const v2FileIdOrPathParamsSchema = z.object({
311311
(value) => isWorkspaceFileVfsPath(value) || workspaceFileIdSchema.safeParse(value).success,
312312
'Invalid file id or path: pass a file id or its `files/<folder>/<name>` VFS path'
313313
)
314-
.describe('File identifier, or the file’s VFS path: `files/<folder>/<name>`.'),
314+
.describe(
315+
'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.'
316+
),
315317
})
316318

317319
export const v2CreateFileBodySchema = z

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

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

2345-
/**
2346-
* The live workspace file whose canonical `files/…` path — as listings print it — is `vfsPath`,
2347-
* or null. Unlike {@link findWorkspaceFileRecord} there is no virtual `/content` or `/meta.json`
2348-
* suffix and no leaf-name fallback, so a mutation addressed by path lands only on the file it
2349-
* names. Two names that display alike are ambiguous and refused. Throws `VfsPathError` if
2350-
* malformed.
2351-
*/
2352-
export async function getWorkspaceFileByVfsPath(
2353-
workspaceId: string,
2354-
vfsPath: string
2355-
): Promise<WorkspaceFileRecord | null> {
2356-
const segments = decodeVfsReferenceSegments(vfsPath.slice('files/'.length))
2357-
const leaf = segments.at(-1)
2358-
if (leaf === undefined) return null
2359-
const pathKey = segments.map(normalizeVfsSegment).join('/')
2360-
const candidates = await listWorkspaceFileCandidates(workspaceId, {
2361-
ids: [],
2362-
leafNames: [normalizeDisplaySegment(leaf)],
2363-
})
2364-
const matches = candidates.filter(
2365-
(file) =>
2366-
canonicalWorkspaceFilePath({ folderPath: file.folderPath, name: file.name }).slice(
2367-
'files/'.length
2368-
) === pathKey
2369-
)
2370-
if (matches.length > 1) {
2371-
throw new OrchestrationError(
2372-
'validation',
2373-
`"${vfsPath}" names more than one file whose names display alike; pass the file id instead`
2374-
)
2375-
}
2376-
return matches[0] ?? null
2377-
}
2378-
23792345
/**
23802346
* Resolve a workspace file record from either its id or a VFS/name reference.
23812347
*

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,12 @@ 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. */
13+
/**
14+
* A `files/…` workspace-file VFS path naming something under the prefix. File ids are drawn from
15+
* `[A-Za-z0-9_-]`, so no id is one.
16+
*/
1417
export function isWorkspaceFileVfsPath(reference: string): boolean {
15-
return reference.startsWith('files/')
18+
return reference.startsWith('files/') && reference.length > 'files/'.length
1619
}
1720

1821
/**

‎apps/sim/lib/workspace-files/__integration__/file-path-references.integration.ts‎

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,13 @@ async function row(id: string) {
5353
const describeFile = (fileId: string, userId = alice, assertedWorkspaceId = workspaceId) =>
5454
readWorkspaceFileMetadataWithVersion.execute({
5555
principal: session(userId),
56-
input: { fileId, assertedWorkspaceId },
56+
input: { fileId, acceptVfsPath: true, assertedWorkspaceId },
5757
})
5858

5959
const deleteFile = (fileId: string, userId = alice) =>
6060
deleteWorkspaceFileOperation.execute({
6161
principal: session(userId),
62-
input: { fileId, assertedWorkspaceId: workspaceId },
62+
input: { fileId, acceptVfsPath: true, assertedWorkspaceId: workspaceId },
6363
})
6464

6565
/** The `{ code, message }` each path is refused with, or null where it resolved. */
@@ -70,7 +70,10 @@ const refusalsFor = (
7070
Promise.all(
7171
paths.map((fileId) =>
7272
readWorkspaceFileMetadataWithVersion
73-
.execute({ principal, input: { fileId, assertedWorkspaceId: workspaceId } })
73+
.execute({
74+
principal,
75+
input: { fileId, acceptVfsPath: true, assertedWorkspaceId: workspaceId },
76+
})
7477
.then(
7578
() => null,
7679
(error: { code?: string; message?: string }) => ({
@@ -171,6 +174,7 @@ describe('single-file operations addressed by VFS path', () => {
171174
code: 'not_found',
172175
})
173176
await expect(describeFile('files/Docs')).rejects.toMatchObject({ code: 'not_found' })
177+
await expect(describeFile('files/')).rejects.toMatchObject({ code: 'not_found' })
174178
await expect(describeFile('files/only-here.csv')).rejects.toMatchObject({
175179
code: 'not_found',
176180
})
@@ -203,7 +207,12 @@ describe('single-file operations addressed by VFS path', () => {
203207
it('renames and deletes only the file the path names', async () => {
204208
const renamed = await renameWorkspaceFile.execute({
205209
principal: session(alice),
206-
input: { fileId: 'files/Docs/report.csv', assertedWorkspaceId: workspaceId, name: 'q3.csv' },
210+
input: {
211+
fileId: 'files/Docs/report.csv',
212+
acceptVfsPath: true,
213+
assertedWorkspaceId: workspaceId,
214+
name: 'q3.csv',
215+
},
207216
})
208217
expect(renamed.file.id).toBe(ids.docsReport)
209218
expect((await row(ids.docsReport))?.name).toBe('q3.csv')
@@ -226,7 +235,7 @@ describe('single-file operations addressed by VFS path', () => {
226235
await expect(
227236
readWorkspaceFileMetadataWithVersion.execute({
228237
principal: session(alice),
229-
input: { fileId: 'files/report.csv' },
238+
input: { fileId: 'files/report.csv', acceptVfsPath: true },
230239
})
231240
).rejects.toMatchObject({ code: 'not_found' })
232241
})
@@ -274,7 +283,11 @@ describe('single-file operations addressed by VFS path', () => {
274283
await expect(
275284
readWorkspaceFileMetadataWithVersion.execute({
276285
principal: delegated,
277-
input: { fileId: 'files/report.csv', assertedWorkspaceId: workspaceId },
286+
input: {
287+
fileId: 'files/report.csv',
288+
acceptVfsPath: true,
289+
assertedWorkspaceId: workspaceId,
290+
},
278291
})
279292
).resolves.toMatchObject({ file: { id: ids.rootReport } })
280293
const outcomes = await refusalsFor(delegated, [
@@ -287,6 +300,37 @@ describe('single-file operations addressed by VFS path', () => {
287300
expect(outcomes[2]).toEqual(outcomes[0])
288301
})
289302

303+
it('answers a non-member personal key the same for every path where the workspace disables keys', async () => {
304+
await db
305+
.update(workspace)
306+
.set({ allowPersonalApiKeys: false })
307+
.where(eq(workspace.id, workspaceId))
308+
try {
309+
const outcomes = await refusalsFor(
310+
{ kind: 'personal_api_key', userId: carol, keyId: generateId() },
311+
[await ambiguousPath(), 'files/missing.csv', 'files/report.csv']
312+
)
313+
expect(outcomes[0]).not.toBeNull()
314+
expect(outcomes[1]).toEqual(outcomes[0])
315+
expect(outcomes[2]).toEqual(outcomes[0])
316+
} finally {
317+
await db
318+
.update(workspace)
319+
.set({ allowPersonalApiKeys: true })
320+
.where(eq(workspace.id, workspaceId))
321+
}
322+
})
323+
324+
it('resolves no path for a caller that did not opt in', async () => {
325+
await expect(
326+
deleteWorkspaceFileOperation.execute({
327+
principal: session(alice),
328+
input: { fileId: 'files/report.csv', assertedWorkspaceId: workspaceId },
329+
})
330+
).rejects.toMatchObject({ code: 'not_found' })
331+
expect((await row(ids.rootReport))?.deletedAt).toBeNull()
332+
})
333+
290334
it('rejects a dot-segment path instead of walking out of its folder', async () => {
291335
await expect(deleteFile('files/Docs/../report.csv')).rejects.toMatchObject({
292336
code: 'validation',

0 commit comments

Comments
 (0)