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,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

bewcloud.config.sample.ts

@@ -18,7 +18,7 @@ const config: PartialDeep<Config> = {
   // 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

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}
         />
 

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}
     />
   );

lib/config.ts

@@ -180,7 +180,7 @@ export class AppConfig {
     return this.config.files.allowPublicSharing;
   }
 
-  static async isDirectoryDownloadsAllowed(): Promise<boolean> {
+  static async areDirectoryDownloadsAllowed(): Promise<boolean> {
     await this.loadConfig();
 
     return this.config.files.allowDirectoryDownloads;

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');

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<Data, FreshContextState> = {
     }
 
     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 });
     }
   },

routes/file-share/[fileShareId].tsx

@@ -126,7 +126,7 @@ export default function FilesPage({ data }: PageProps<Data, FreshContextState>)
         initialPath={data.currentPath}
         baseUrl={data.baseUrl}
         isFileSharingAllowed
-        isDirectoryDownloadsAllowed={false}
+        areDirectoryDownloadsAllowed={false}
         fileShareId={data.fileShareId}
       />
     </main>

routes/files.tsx

@@ -11,7 +11,7 @@ interface Data {
   currentPath: string;
   baseUrl: string;
   isFileSharingAllowed: boolean;
-  isDirectoryDownloadsAllowed: boolean;
+  areDirectoryDownloadsAllowed: boolean;
 }
 
 export const handler: Handlers<Data, FreshContextState> = {
@@ -41,7 +41,7 @@ export const handler: Handlers<Data, FreshContextState> = {
     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<Data, FreshContextState> = {
       currentPath,
       baseUrl,
       isFileSharingAllowed: isPublicFileSharingAllowed,
-      isDirectoryDownloadsAllowed,
+      areDirectoryDownloadsAllowed,
     });
   },
 };
@@ -63,7 +63,7 @@ export default function FilesPage({ data }: PageProps<Data, FreshContextState>)
         initialPath={data.currentPath}
         baseUrl={data.baseUrl}
         isFileSharingAllowed={data.isFileSharingAllowed}
-        isDirectoryDownloadsAllowed={data.isDirectoryDownloadsAllowed}
+        areDirectoryDownloadsAllowed={data.areDirectoryDownloadsAllowed}
       />
     </main>
   );

static/images/download.svg

@@ -1,13 +1,14 @@
 <svg
   xmlns="http://www.w3.org/2000/svg"
-  viewBox="0 0 24 24"
   fill="none"
+  viewBox="0 0 24 24"
+  stroke-width="1.5"
   stroke="currentColor"
-  stroke-width="2"
-  stroke-linecap="round"
-  stroke-linejoin="round"
+  class="size-6"
 >
-  <path d="M21 15v4a2 2 0 0 1-2 2H5a2 2 0 0 1-2-2v-4" />
-  <polyline points="7,10 12,15 17,10" />
-  <line x1="12" y1="15" x2="12" y2="3" />
+  <path
+    stroke-linecap="round"
+    stroke-linejoin="round"
+    d="M3 16.5v2.25A2.25 2.25 0 0 0 5.25 21h13.5A2.25 2.25 0 0 0 21 18.75V16.5M16.5 12 12 16.5m0 0L7.5 12m4.5 4.5V3"
+  />
 </svg>