From 3b7447f6cea873f5919c24282270f7fe727a8026 Mon Sep 17 00:00:00 2001 From: Tilman Date: Sun, 5 Oct 2025 22:22:20 +0200 Subject: [PATCH] Address feedback - `isDirectoryDownloadsAllowed` -> `areDirectoryDownloadsAllowed` - send `parentPath` & `name` to API instead of resolving `fullPath` on client - call `ensureUserPathIsValidAndSecurelyAccessible` before zipping - set config `allowDirectoryDownloads` default to `false` - add `zip` to Dockerfile and replace in-house zip algorithm - replace `download.svg` with heroicon's `arrow-down-tray` - `replace` with glob -> `replaceAll` with string --- Dockerfile | 2 +- bewcloud.config.sample.ts | 2 +- components/files/MainFiles.tsx | 11 ++--- islands/files/FilesWrapper.tsx | 6 +-- lib/config.ts | 2 +- lib/models/files.ts | 4 +- routes/api/files/download-directory.tsx | 60 ++++++++++++++----------- routes/file-share/[fileShareId].tsx | 2 +- routes/files.tsx | 8 ++-- static/images/download.svg | 15 ++++--- 10 files changed, 60 insertions(+), 52 deletions(-) diff --git a/Dockerfile b/Dockerfile index 40f2e8b..a7e1517 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,6 @@ FROM denoland/deno:ubuntu-2.5.2 EXPOSE 8000 -RUN apt-get update && apt-get install -y make WORKDIR /app @@ -21,3 +20,4 @@ USER deno RUN deno cache --reload main.ts CMD ["run", "--allow-all", "main.ts"] +RUN apt-get update && apt-get install -y make zip diff --git a/bewcloud.config.sample.ts b/bewcloud.config.sample.ts index dfbe56b..a22412e 100644 --- a/bewcloud.config.sample.ts +++ b/bewcloud.config.sample.ts @@ -18,7 +18,7 @@ const config: PartialDeep = { // files: { // rootPath: 'data-files', // allowPublicSharing: false, // If true, public file sharing will be allowed (still requires a user to enable sharing for a given file or directory) - // allowDirectoryDownloads: true, // If true, directories can be downloaded as zip files + // allowDirectoryDownloads: false, // If true, directories can be downloaded as zip files // }, // core: { // enabledApps: ['news', 'notes', 'photos', 'expenses', 'contacts', 'calendar'], // dashboard and files cannot be disabled diff --git a/components/files/MainFiles.tsx b/components/files/MainFiles.tsx index ed304d0..0907b6e 100644 --- a/components/files/MainFiles.tsx +++ b/components/files/MainFiles.tsx @@ -48,7 +48,7 @@ interface MainFilesProps { initialPath: string; baseUrl: string; isFileSharingAllowed: boolean; - isDirectoryDownloadsAllowed: boolean; + areDirectoryDownloadsAllowed: boolean; fileShareId?: string; } @@ -59,7 +59,7 @@ export default function MainFiles( initialPath, baseUrl, isFileSharingAllowed, - isDirectoryDownloadsAllowed, + areDirectoryDownloadsAllowed, fileShareId, }: MainFilesProps, ) { @@ -415,8 +415,9 @@ export default function MainFiles( function onClickDownloadDirectory(parentPath: string, name: string) { // Create download URL with proper path encoding - const fullPath = parentPath + name + '/'; - const downloadUrl = `/api/files/download-directory?path=${encodeURIComponent(fullPath)}`; + const downloadUrl = `/api/files/download-directory?parentPath=${encodeURIComponent(parentPath)}&name=${ + encodeURIComponent(name) + }`; // Create a temporary anchor element to trigger download const link = document.createElement('a'); @@ -855,7 +856,7 @@ export default function MainFiles( onClickDeleteFile={onClickDeleteFile} onClickCreateShare={isFileSharingAllowed ? onClickCreateShare : undefined} onClickOpenManageShare={isFileSharingAllowed ? onClickOpenManageShare : undefined} - onClickDownloadDirectory={isDirectoryDownloadsAllowed ? onClickDownloadDirectory : undefined} + onClickDownloadDirectory={areDirectoryDownloadsAllowed ? onClickDownloadDirectory : undefined} fileShareId={fileShareId} /> diff --git a/islands/files/FilesWrapper.tsx b/islands/files/FilesWrapper.tsx index 4e08207..84630fe 100644 --- a/islands/files/FilesWrapper.tsx +++ b/islands/files/FilesWrapper.tsx @@ -7,7 +7,7 @@ interface FilesWrapperProps { initialPath: string; baseUrl: string; isFileSharingAllowed: boolean; - isDirectoryDownloadsAllowed: boolean; + areDirectoryDownloadsAllowed: boolean; fileShareId?: string; } @@ -19,7 +19,7 @@ export default function FilesWrapper( initialPath, baseUrl, isFileSharingAllowed, - isDirectoryDownloadsAllowed, + areDirectoryDownloadsAllowed, fileShareId, }: FilesWrapperProps, ) { @@ -30,7 +30,7 @@ export default function FilesWrapper( initialPath={initialPath} baseUrl={baseUrl} isFileSharingAllowed={isFileSharingAllowed} - isDirectoryDownloadsAllowed={isDirectoryDownloadsAllowed} + areDirectoryDownloadsAllowed={areDirectoryDownloadsAllowed} fileShareId={fileShareId} /> ); diff --git a/lib/config.ts b/lib/config.ts index b1267ba..4b66c54 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -180,7 +180,7 @@ export class AppConfig { return this.config.files.allowPublicSharing; } - static async isDirectoryDownloadsAllowed(): Promise { + static async areDirectoryDownloadsAllowed(): Promise { await this.loadConfig(); return this.config.files.allowDirectoryDownloads; diff --git a/lib/models/files.ts b/lib/models/files.ts index b068f6c..8d17830 100644 --- a/lib/models/files.ts +++ b/lib/models/files.ts @@ -599,8 +599,8 @@ export async function ensureUserPathIsValidAndSecurelyAccessible(userId: string, const resolvedFullPath = `${resolve(fullPath)}/`; // Normalize path separators for consistent comparison on Windows - const normalizedUserRootPath = userRootPath.replace(/\\/g, '/'); - const normalizedResolvedFullPath = resolvedFullPath.replace(/\\/g, '/'); + const normalizedUserRootPath = userRootPath.replaceAll('\\', '/'); + const normalizedResolvedFullPath = resolvedFullPath.replaceAll('\\', '/'); if (!normalizedResolvedFullPath.startsWith(normalizedUserRootPath)) { throw new Error('Invalid file path'); diff --git a/routes/api/files/download-directory.tsx b/routes/api/files/download-directory.tsx index 2e0f5cf..cceebbf 100644 --- a/routes/api/files/download-directory.tsx +++ b/routes/api/files/download-directory.tsx @@ -1,9 +1,9 @@ import { Handlers } from 'fresh/server.ts'; -import { join, resolve } from '@std/path'; +import { join } from '@std/path'; import { FreshContextState } from '/lib/types.ts'; import { AppConfig } from '/lib/config.ts'; -import { DirectoryModel, FileModel } from '/lib/models/files.ts'; +import { ensureUserPathIsValidAndSecurelyAccessible } from '/lib/models/files.ts'; interface Data {} @@ -21,47 +21,53 @@ export const handler: Handlers = { } const searchParams = new URL(request.url).searchParams; - let directoryPath = searchParams.get('path') || '/'; + const parentPath = searchParams.get('parentPath') || '/'; + const name = searchParams.get('name'); - // Send invalid paths back to root - if (!directoryPath.startsWith('/') || directoryPath.includes('../')) { - return new Response('Invalid path', { status: 400 }); + if (!name) { + return new Response('Directory name is required', { status: 400 }); } - // Always append a trailing slash - if (!directoryPath.endsWith('/')) { - directoryPath = `${directoryPath}/`; - } + // Construct the full directory path + const directoryPath = join(parentPath, name) + '/'; try { - // Get all files and subdirectories recursively - const filesAndDirectories = await getDirectoryContentsRecursively( - context.state.user.id, - directoryPath, - ); + await ensureUserPathIsValidAndSecurelyAccessible(context.state.user.id, directoryPath); - if (filesAndDirectories.length === 0) { - return new Response('Directory not found or empty', { status: 404 }); + // Get the actual filesystem path + const filesRootPath = config.files?.rootPath || 'data-files'; + const userRootPath = join(filesRootPath, context.state.user.id); + const fullDirectoryPath = join(userRootPath, directoryPath); + + // Use the zip command to create the archive + const zipProcess = new Deno.Command('zip', { + args: ['-r', '-', '.'], + cwd: fullDirectoryPath, + stdout: 'piped', + stderr: 'piped', + }); + + const { code, stdout, stderr } = await zipProcess.output(); + + if (code !== 0) { + const errorText = new TextDecoder().decode(stderr); + console.error('Zip command failed:', errorText); + return new Response('Error creating zip archive', { status: 500 }); } - // Create zip archive - const zipData = await createZipArchive(filesAndDirectories, directoryPath); - - // Get directory name for filename - const directoryName = directoryPath === '/' - ? 'root' - : directoryPath.split('/').filter(Boolean).pop() || 'directory'; - - return new Response(zipData as BodyInit, { + return new Response(stdout, { status: 200, headers: { 'content-type': 'application/zip', - 'content-disposition': `attachment; filename="${directoryName}.zip"`, + 'content-disposition': `attachment; filename="${name}.zip"`, 'cache-control': 'no-cache, no-store, must-revalidate', }, }); } catch (error) { console.error('Error creating directory zip:', error); + if (error.message === 'Invalid file path') { + return new Response('Invalid directory path', { status: 400 }); + } return new Response('Error creating zip archive', { status: 500 }); } }, diff --git a/routes/file-share/[fileShareId].tsx b/routes/file-share/[fileShareId].tsx index 8196904..0709597 100644 --- a/routes/file-share/[fileShareId].tsx +++ b/routes/file-share/[fileShareId].tsx @@ -126,7 +126,7 @@ export default function FilesPage({ data }: PageProps) initialPath={data.currentPath} baseUrl={data.baseUrl} isFileSharingAllowed - isDirectoryDownloadsAllowed={false} + areDirectoryDownloadsAllowed={false} fileShareId={data.fileShareId} /> diff --git a/routes/files.tsx b/routes/files.tsx index 34a7f22..abf4980 100644 --- a/routes/files.tsx +++ b/routes/files.tsx @@ -11,7 +11,7 @@ interface Data { currentPath: string; baseUrl: string; isFileSharingAllowed: boolean; - isDirectoryDownloadsAllowed: boolean; + areDirectoryDownloadsAllowed: boolean; } export const handler: Handlers = { @@ -41,7 +41,7 @@ export const handler: Handlers = { const userFiles = await FileModel.list(context.state.user.id, currentPath); const isPublicFileSharingAllowed = await AppConfig.isPublicFileSharingAllowed(); - const isDirectoryDownloadsAllowed = await AppConfig.isDirectoryDownloadsAllowed(); + const areDirectoryDownloadsAllowed = await AppConfig.areDirectoryDownloadsAllowed(); return await context.render({ userDirectories, @@ -49,7 +49,7 @@ export const handler: Handlers = { currentPath, baseUrl, isFileSharingAllowed: isPublicFileSharingAllowed, - isDirectoryDownloadsAllowed, + areDirectoryDownloadsAllowed, }); }, }; @@ -63,7 +63,7 @@ export default function FilesPage({ data }: PageProps) initialPath={data.currentPath} baseUrl={data.baseUrl} isFileSharingAllowed={data.isFileSharingAllowed} - isDirectoryDownloadsAllowed={data.isDirectoryDownloadsAllowed} + areDirectoryDownloadsAllowed={data.areDirectoryDownloadsAllowed} /> ); diff --git a/static/images/download.svg b/static/images/download.svg index 937ef5b..19839fc 100644 --- a/static/images/download.svg +++ b/static/images/download.svg @@ -1,13 +1,14 @@ - - - +