Skip to content

Commit 65e0eeb

Browse files
committed
fix(files): harden folder uploads and sharing boundaries
1 parent 8a1b0ce commit 65e0eeb

32 files changed

Lines changed: 725 additions & 154 deletions

File tree

‎.github/workflows/checks.yml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,14 @@ jobs:
201201
working-directory: packages/db
202202
run: bun run db:migrate
203203

204+
- name: Cache Files browser download
205+
if: matrix.group == 'files'
206+
uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v5
207+
with:
208+
path: ~/.cache/ms-playwright
209+
key: files-browser-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('bun.lock') }}
210+
restore-keys: files-browser-${{ runner.os }}-${{ runner.arch }}-
211+
204212
- name: Install Files browser from the pinned Playwright dependency
205213
if: matrix.group == 'files'
206214
run: node node_modules/@playwright/test/cli.js install --with-deps chromium

‎apps/sim/app/api/files/public/[token]/content/route.test.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
import { publicShare } from '@sim/db/schema'
12
import { createRouteContext } from '@sim/testing/helpers/http'
3+
import { queueTableRows } from '@sim/testing/mocks/database.mock'
24
import { publicSharesMock, publicSharesMockFns } from '@sim/testing/mocks/public-shares.mock'
35
import { createMockRequest } from '@sim/testing/mocks/request.mock'
46
import { storageServiceMock, storageServiceMockFns } from '@sim/testing/mocks/storage-service.mock'
@@ -60,6 +62,7 @@ describe('GET /api/files/public/[token]/content', () => {
6062
mockResolveActiveShareByToken.mockResolvedValue(passwordShare)
6163
mockDownloadFile.mockResolvedValue(Buffer.from('data'))
6264
mockResolveServableDoc.mockResolvedValue({ kind: 'passthrough' })
65+
queueTableRows(publicShare, [{ id: 'sh_1' }])
6366
})
6467

6568
it('returns 401 and never reads storage when a password share is unauthorized', async () => {

‎apps/sim/app/api/files/public/[token]/inline/route.test.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
import { publicShare } from '@sim/db/schema'
12
import { createRouteContext } from '@sim/testing/helpers/http'
3+
import { queueTableRows } from '@sim/testing/mocks/database.mock'
24
import { publicSharesMock, publicSharesMockFns } from '@sim/testing/mocks/public-shares.mock'
35
import { createMockRequest } from '@sim/testing/mocks/request.mock'
46
import { storageServiceMock, storageServiceMockFns } from '@sim/testing/mocks/storage-service.mock'
@@ -61,6 +63,7 @@ describe('GET /api/files/public/[token]/inline', () => {
6163
filename: 'photo.png',
6264
})
6365
mockDownloadFile.mockImplementation(downloadByKey())
66+
queueTableRows(publicShare, [{ id: 'sh_1' }])
6467
})
6568

6669
it('rejects exhausted image budgets before share lookup, authentication, or storage reads', async () => {

‎apps/sim/app/api/files/public/[token]/route.test.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { publicShare as publicShareTable } from '@sim/db/schema'
2+
import { queueTableRows } from '@sim/testing/mocks/database.mock'
13
import { publicSharesMock, publicSharesMockFns } from '@sim/testing/mocks/public-shares.mock'
24
import { NextRequest } from 'next/server'
35
import { beforeEach, describe, expect, it, vi } from 'vitest'
@@ -60,6 +62,7 @@ describe('GET /api/files/public/[token]', () => {
6062
beforeEach(() => {
6163
mockEnforceRateLimit.mockResolvedValue(null) // allow by default
6264
mockValidateDeploymentAuth.mockResolvedValue({ authorized: true }) // public by default
65+
queueTableRows(publicShareTable, [{ id: 'sh_1' }])
6366
})
6467

6568
it('returns 429 when the per-IP rate limit is exceeded', async () => {

‎apps/sim/app/f/[token]/public-folder-view.tsx‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,19 @@ export function PublicFolderView({ token, name, workspaceName, ownerName }: Publ
4343
fileId ? null : cursor,
4444
!fileId || file.isSuccess
4545
)
46-
const page = listing.data
4746
const brand = useBrandConfig()
4847
const provenance = buildProvenance(workspaceName, ownerName)
4948
const error = fileId ? (file.error ?? listing.error) : listing.error
50-
const loading = fileId ? file.isPending || listing.isPending : listing.isPending
49+
const loading =
50+
listing.isPending ||
51+
listing.isFetching ||
52+
Boolean(fileId && (file.isPending || file.isFetching))
53+
const page = !loading && !error ? listing.data : undefined
54+
const currentFile = !loading && !error ? file.data : undefined
5155
const entries = page?.entries ?? []
5256
const breadcrumbs = page?.breadcrumbs ?? []
5357
const scrollRef = useRef<HTMLDivElement>(null)
54-
const edges = useScrollEdges(scrollRef, { enabled: !fileId && listing.isSuccess })
58+
const edges = useScrollEdges(scrollRef, { enabled: !fileId && Boolean(page) })
5559

5660
return (
5761
<div className='light desktop-title-bar-page flex h-screen flex-col overflow-hidden bg-[var(--bg)]'>
@@ -98,10 +102,10 @@ export function PublicFolderView({ token, name, workspaceName, ownerName }: Publ
98102
) : (
99103
<OverflowText label={name} />
100104
)}
101-
{file.data && (
105+
{currentFile && (
102106
<>
103107
<ChevronRight className='size-[14px] shrink-0 text-[var(--text-muted)]' />
104-
<OverflowText label={file.data.name} className='text-[var(--text-body)]' />
108+
<OverflowText label={currentFile.name} className='text-[var(--text-body)]' />
105109
</>
106110
)}
107111
</nav>
@@ -110,14 +114,14 @@ export function PublicFolderView({ token, name, workspaceName, ownerName }: Publ
110114
)}
111115
</div>
112116
</div>
113-
{fileId && file.data && !error && (
117+
{fileId && currentFile && (
114118
<Chip
115119
variant='primary'
116120
leftIcon={Download}
117121
onClick={() => {
118122
const anchor = document.createElement('a')
119123
anchor.href = `/api/files/public/${token}/content?fileId=${encodeURIComponent(fileId)}`
120-
anchor.download = file.data.name
124+
anchor.download = currentFile.name
121125
anchor.click()
122126
}}
123127
>
@@ -145,14 +149,14 @@ export function PublicFolderView({ token, name, workspaceName, ownerName }: Publ
145149
>
146150
Loading…
147151
</div>
148-
) : fileId && file.data ? (
152+
) : fileId && currentFile ? (
149153
<PublicFilePreview
150154
token={token}
151155
fileId={fileId}
152-
name={file.data.name}
153-
type={file.data.type}
154-
size={file.data.size}
155-
version={file.data.version ?? 0}
156+
name={currentFile.name}
157+
type={currentFile.type}
158+
size={currentFile.size}
159+
version={currentFile.version ?? 0}
156160
/>
157161
) : (
158162
<div

‎apps/sim/app/workspace/[workspaceId]/components/folders/use-folder-row-drag-drop.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,7 @@ export function useFolderRowDragDrop({
451451
* one files the drag there directly.
452452
*/
453453
breadcrumb: {
454+
isDragging: draggedRowIds.size > 0,
454455
activeIndex: activeDropTarget?.kind === 'crumb' ? activeDropTarget.index : null,
455456
onDragOver: (e: DragEvent<HTMLElement>, folderId: string | null, index: number) => {
456457
if (optionsRef.current.externalDrop?.matches(e.dataTransfer)) return

‎apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-header/resource-header.tsx‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@ export interface ResourceAction {
117117
* on one files the drag there — the counterpart to spring-loading, which only ever goes deeper.
118118
*/
119119
export interface BreadcrumbDropConfig {
120+
/** Expands collapsed ancestors while rows are being moved. */
121+
isDragging?: boolean
120122
/** Index of the crumb currently under the drag, or `null`. Indexed because `null` is a folder. */
121123
activeIndex: number | null
122124
onDragOver: (e: DragEvent<HTMLElement>, folderId: string | null, index: number) => void
@@ -149,19 +151,15 @@ export const ResourceHeader = memo(function ResourceHeader({
149151
aside,
150152
breadcrumbDrop,
151153
}: ResourceHeaderProps) {
152-
/**
153-
* Breadcrumb mode is reserved for nested pages (length > 1). A single-crumb
154-
* "breadcrumb" is just the current page, so it falls through to the static
155-
* title below — keeping the top-left non-interactive and hover-free,
156-
* identical to a title-only page (e.g. the Files root matches the Tables root).
157-
*/
154+
/** Single-crumb roots retain their static title and may offer child-folder navigation. */
158155
const hasBreadcrumbs = breadcrumbs != null && breadcrumbs.length > 1
159156
const rootCrumb = breadcrumbs?.length === 1 ? breadcrumbs[0] : undefined
160157
const TitleIcon = Icon ?? rootCrumb?.icon
161158
const titleLabel = title ?? rootCrumb?.label
162159
const collapsePath =
163160
breadcrumbs != null &&
164161
breadcrumbs.length > 4 &&
162+
!breadcrumbDrop?.isDragging &&
165163
breadcrumbs.some((item) => item.navigationItems !== undefined)
166164
const terminalBreadcrumbIndex =
167165
hasBreadcrumbs && breadcrumbs[breadcrumbs.length - 1].terminal ? breadcrumbs.length - 1 : -1
@@ -287,14 +285,23 @@ export const ResourceHeader = memo(function ResourceHeader({
287285
* layout (the JS-driven `--sidebar-width` settling) and the title
288286
* CSS-truncates to "T…" while the `shrink-0` actions hold width.
289287
*/
290-
<span className={cn(chipGeometryClass, 'inline-flex shrink-0 cursor-default')}>
291-
{TitleIcon && <TitleIcon className={chipContentIconClass} />}
292-
{titleLabel && (
293-
<span className='block whitespace-nowrap text-[var(--text-body)] text-sm'>
294-
{titleLabel}
295-
</span>
288+
<>
289+
<span className={cn(chipGeometryClass, 'inline-flex shrink-0 cursor-default')}>
290+
{TitleIcon && <TitleIcon className={chipContentIconClass} />}
291+
{titleLabel && (
292+
<span className='block whitespace-nowrap text-[var(--text-body)] text-sm'>
293+
{titleLabel}
294+
</span>
295+
)}
296+
</span>
297+
{rootCrumb?.navigationItems !== undefined && (
298+
<BreadcrumbNavigation
299+
label={rootCrumb.label}
300+
items={rootCrumb.navigationItems}
301+
actions={rootCrumb.dropdownItems}
302+
/>
296303
)}
297-
</span>
304+
</>
298305
)}
299306
</div>
300307
{(aside || (actions && actions.length > 0)) && (

‎apps/sim/app/workspace/[workspaceId]/files/components/upload-menu.tsx‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ interface UploadMenuProps {
1919
onCancel: () => void
2020
}
2121

22+
/** Offers file and folder uploads, or cancellation while an upload is running. */
2223
export function UploadMenu({
2324
label,
2425
disabled,
@@ -30,12 +31,7 @@ export function UploadMenu({
3031
return (
3132
<DropdownMenu modal={false}>
3233
<DropdownMenuTrigger asChild>
33-
<Chip
34-
leftIcon={Upload}
35-
rightAdornment={<ChipChevronDown />}
36-
disabled={disabled}
37-
aria-label='Upload'
38-
>
34+
<Chip leftIcon={Upload} rightAdornment={<ChipChevronDown />} disabled={disabled}>
3935
{label}
4036
</Chip>
4137
</DropdownMenuTrigger>

‎apps/sim/app/workspace/[workspaceId]/files/files.tsx‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,7 +1004,7 @@ function FilesContent() {
10041004
matches: hasExternalFiles,
10051005
onDropIntoFolder: (dataTransfer, targetFolderId) => {
10061006
dismissUploadOverlay()
1007-
void upload(selectionFromDrop(dataTransfer), targetFolderId)
1007+
void upload((signal) => selectionFromDrop(dataTransfer, signal), targetFolderId)
10081008
},
10091009
},
10101010
})
@@ -1014,10 +1014,7 @@ function FilesContent() {
10141014
if (!list || list.length === 0) return
10151015
const selected = Array.from(list)
10161016
e.currentTarget.value = ''
1017-
await upload(
1018-
Promise.resolve().then(() => selectionFromFiles(selected)),
1019-
currentFolderId
1020-
)
1017+
await upload(() => selectionFromFiles(selected), currentFolderId)
10211018
}
10221019

10231020
const handleDragEnter = (e: React.DragEvent) => {
@@ -1049,7 +1046,7 @@ function FilesContent() {
10491046
*/
10501047
rowDragDropConfig.externalDropHandled()
10511048
dismissUploadOverlay()
1052-
await upload(selectionFromDrop(e.dataTransfer), currentFolderId)
1049+
await upload((signal) => selectionFromDrop(e.dataTransfer, signal), currentFolderId)
10531050
}
10541051

10551052
const handleDownload = useCallback(

‎apps/sim/app/workspace/[workspaceId]/files/hooks/use-file-uploads.ts‎

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,8 @@ interface UseFileUploadsOptions {
2727
/** Serializes captured upload batches and reconciles the browser once each batch settles. */
2828
export function useFileUploads({ workspaceId, canEdit, onStart }: UseFileUploadsOptions) {
2929
const queue = useRef(Promise.resolve())
30-
const controller = useRef<AbortController | null>(null)
31-
const pendingBatches = useRef(0)
32-
const generation = useRef(0)
30+
const batchesRef = useRef<Set<AbortController> | null>(null)
31+
const batches = (batchesRef.current ??= new Set<AbortController>())
3332
const mounted = useRef(true)
3433
const [progress, setProgress] = useState(IDLE_PROGRESS)
3534
const queryClient = useQueryClient()
@@ -41,35 +40,40 @@ export function useFileUploads({ workspaceId, canEdit, onStart }: UseFileUploads
4140
mounted.current = true
4241
return () => {
4342
mounted.current = false
44-
generation.current += 1
45-
controller.current?.abort()
43+
for (const batch of batches) batch.abort()
4644
}
47-
}, [])
45+
}, [workspaceId, canEdit, batches])
4846

4947
const upload = useCallback(
50-
(selection: UploadSelection | Promise<UploadSelection>, targetFolderId: string | null) => {
51-
const selected = Promise.resolve(selection)
52-
// Attach rejection handling immediately while a preceding batch is still uploading.
53-
const captured = selected.then(
54-
(value) => ({ success: true as const, value }),
55-
(error: unknown) => ({ success: false as const, error })
56-
)
48+
(
49+
captureSelection: (signal: AbortSignal) => UploadSelection | Promise<UploadSelection>,
50+
targetFolderId: string | null
51+
) => {
5752
if (!workspaceId || !canEdit || !mounted.current) return Promise.resolve()
58-
if (pendingBatches.current >= MAX_PENDING_UPLOAD_BATCHES) {
53+
if (batches.size >= MAX_PENDING_UPLOAD_BATCHES) {
5954
toast.error('The upload queue is full', {
6055
description: 'Wait for an upload to finish before adding more files or folders.',
6156
})
6257
return Promise.resolve()
6358
}
64-
pendingBatches.current += 1
65-
const batchGeneration = generation.current
59+
const abort = new AbortController()
60+
batches.add(abort)
61+
let selected: Promise<UploadSelection>
62+
try {
63+
selected = Promise.resolve(captureSelection(abort.signal))
64+
} catch (error) {
65+
selected = Promise.reject(error)
66+
}
67+
// Native drag entries must be captured before the drop event releases its data store.
68+
const captured = selected.then(
69+
(value) => ({ success: true as const, value }),
70+
(error: unknown) => ({ success: false as const, error })
71+
)
6672
const run = async () => {
67-
if (!mounted.current || batchGeneration !== generation.current) {
68-
pendingBatches.current -= 1
73+
if (!mounted.current || abort.signal.aborted) {
74+
batches.delete(abort)
6975
return
7076
}
71-
const abort = new AbortController()
72-
controller.current = abort
7377
let attempted = false
7478
try {
7579
onStart()
@@ -95,6 +99,7 @@ export function useFileUploads({ workspaceId, canEdit, onStart }: UseFileUploads
9599
paths: directories,
96100
signal: abort.signal,
97101
})
102+
abort.signal.throwIfAborted()
98103
for (const folder of prepared.folders)
99104
folderIds.set(JSON.stringify(folder.path), folder.id)
100105
} catch (error) {
@@ -125,7 +130,7 @@ export function useFileUploads({ workspaceId, canEdit, onStart }: UseFileUploads
125130
skipToast: true,
126131
skipInvalidation: true,
127132
onProgress: ({ percent }) => {
128-
if (mounted.current)
133+
if (mounted.current && !abort.signal.aborted)
129134
setProgress({
130135
completed: index,
131136
total: allowed.length,
@@ -147,7 +152,7 @@ export function useFileUploads({ workspaceId, canEdit, onStart }: UseFileUploads
147152
})
148153
}
149154
}
150-
if (mounted.current)
155+
if (mounted.current && !abort.signal.aborted)
151156
setProgress({
152157
completed: index + 1,
153158
total: allowed.length,
@@ -172,22 +177,20 @@ export function useFileUploads({ workspaceId, canEdit, onStart }: UseFileUploads
172177
toast.error(getErrorMessage(error, 'Could not upload the selection'))
173178
}
174179
} finally {
175-
pendingBatches.current -= 1
180+
batches.delete(abort)
176181
if (attempted) invalidateWorkspaceFileBrowsers(queryClient, workspaceId)
177-
controller.current = null
178182
if (mounted.current) setProgress(IDLE_PROGRESS)
179183
}
180184
}
181185
queue.current = queue.current.then(run, run)
182186
return queue.current
183187
},
184-
[workspaceId, canEdit, onStart, prepareFolders, uploadFile, notifyLimit, queryClient]
188+
[workspaceId, canEdit, onStart, prepareFolders, uploadFile, notifyLimit, queryClient, batches]
185189
)
186190

187191
const cancel = useCallback(() => {
188-
generation.current += 1
189-
controller.current?.abort()
190-
}, [])
192+
for (const batch of batches) batch.abort()
193+
}, [batches])
191194

192195
return { upload, cancel, progress, uploading: progress.preparing || progress.total > 0 }
193196
}

0 commit comments

Comments
 (0)