feat(storage): Implement robust path validation and structured skip reporting

handwritten/storage/src/file.ts

@@ -396,6 +396,22 @@ export interface CopyCallback {
 
 export type DownloadResponse = [Buffer];
 
+export type DownloadResponseWithStatus = [Buffer] & {
+  skipped?: boolean;
+  reason?: SkipReason;
+  fileName?: string;
+  localPath?: string;
+  message?: string;
+  error?: Error;
+};
+
+export enum SkipReason {
+  PATH_TRAVERSAL = 'PATH_TRAVERSAL',
+  ILLEGAL_CHARACTER = 'ILLEGAL_CHARACTER',
+  ALREADY_EXISTS = 'ALREADY_EXISTS',
+  DOWNLOAD_ERROR = 'DOWNLOAD_ERROR',
+}
+
 export type DownloadCallback = (
   err: RequestError | null,
   contents: Buffer,

handwritten/storage/src/transfer-manager.ts

@@ -18,9 +18,11 @@ import {Bucket, UploadOptions, UploadResponse} from './bucket.js';
 import {
   DownloadOptions,
   DownloadResponse,
+  DownloadResponseWithStatus,
   File,
   FileExceptionMessages,
   RequestError,
+  SkipReason,
 } from './file.js';
 import pLimit from 'p-limit';
 import * as path from 'path';
@@ -570,9 +572,13 @@ export class TransferManager {
     const limit = pLimit(
       options.concurrencyLimit || DEFAULT_PARALLEL_DOWNLOAD_LIMIT,
     );
-    const promises: Promise<DownloadResponse>[] = [];
+    const promises: Promise<void>[] = [];
     let files: File[] = [];
 
+    const baseDestination = path.resolve(
+      options.passthroughOptions?.destination || options.prefix || '.'
+    );
+
     if (!Array.isArray(filesOrFolder)) {
       const directoryFiles = await this.bucket.getFiles({
         prefix: filesOrFolder,
@@ -592,45 +598,95 @@ export class TransferManager {
       : EMPTY_REGEX;
     const regex = new RegExp(stripRegexString, 'g');
 
-    for (const file of files) {
-      const passThroughOptionsCopy = {
-        ...options.passthroughOptions,
-        [GCCL_GCS_CMD_KEY]: GCCL_GCS_CMD_FEATURE.DOWNLOAD_MANY,
-      };
+    const finalResults: DownloadResponseWithStatus[] = new Array(files.length);
 
-      if (options.prefix || passThroughOptionsCopy.destination) {
-        passThroughOptionsCopy.destination = path.join(
-          options.prefix || '',
-          passThroughOptionsCopy.destination || '',
-          file.name,
-        );
+    for (let i = 0; i < files.length; i++) {
+      const file = files[i];
+
+      const hasIllegalDrive = /^[a-zA-Z]:/.test(file.name);
+      if (hasIllegalDrive) {
+        const skippedResult = [Buffer.alloc(0)] as DownloadResponseWithStatus;
+        skippedResult.skipped = true;
+        skippedResult.reason = SkipReason.ILLEGAL_CHARACTER;
+        skippedResult.fileName = file.name;
+        finalResults[i] = skippedResult;
+        continue;
       }
-      if (options.stripPrefix) {
-        passThroughOptionsCopy.destination = file.name.replace(regex, '');
+
+      const normalizedGcsName = file.name
+        .replace(/\\/g, '/')
+        .replace(/^\/+/, '');
+
+      const dest = options.stripPrefix
+        ? normalizedGcsName.replace(regex, '')
+        : normalizedGcsName;
+
+      const resolvedPath = path.resolve(baseDestination, dest);
+      const relativeFromBase = path.relative(baseDestination, resolvedPath);
+
+      const isOutside =
+        path.isAbsolute(relativeFromBase) ||
+        relativeFromBase.split(path.sep).includes('..');
+
+      if (isOutside) {
+        const skippedResult = [Buffer.alloc(0)] as DownloadResponseWithStatus;
+        skippedResult.skipped = true;
+        skippedResult.reason = SkipReason.PATH_TRAVERSAL;
+        skippedResult.fileName = file.name;
+        skippedResult.localPath = resolvedPath;
+        finalResults[i] = skippedResult;
+        continue;
       }
-      if (
-        options.skipIfExists &&
-        existsSync(passThroughOptionsCopy.destination || '')
-      ) {
+
+      if (options.skipIfExists && existsSync(resolvedPath)) {
         continue;
       }
 
+      const passThroughOptionsCopy = {
+        ...options.passthroughOptions,
+        destination: resolvedPath,
+        [GCCL_GCS_CMD_KEY]: GCCL_GCS_CMD_FEATURE.DOWNLOAD_MANY,
+      };
+
       promises.push(
         limit(async () => {
-          const destination = passThroughOptionsCopy.destination;
-          if (destination && destination.endsWith(path.sep)) {
-            await fsp.mkdir(destination, {recursive: true});
-            return Promise.resolve([
-              Buffer.alloc(0),
-            ]) as Promise<DownloadResponse>;
+          try {
+            const destination = passThroughOptionsCopy.destination!;
+
+            if (destination.endsWith(path.sep) || destination.endsWith('/')) {
+              await fsp.mkdir(destination, {recursive: true});
+              const dirResp = [Buffer.alloc(0)] as DownloadResponseWithStatus;
+              dirResp.skipped = false;
+              dirResp.fileName = file.name;
+              dirResp.localPath = destination;
+              finalResults[i] = dirResp;
+              return;
+            }
+
+            await fsp.mkdir(path.dirname(destination), {recursive: true});
+
+            const resp = (await file.download(
+              passThroughOptionsCopy
+            )) as DownloadResponseWithStatus;
+            resp.skipped = false;
+            resp.fileName = file.name;
+            resp.localPath = destination;
+            finalResults[i] = resp;
+          } catch (err) {
+            const errorResp = [Buffer.alloc(0)] as DownloadResponseWithStatus;
+            errorResp.skipped = true;
+            errorResp.reason = SkipReason.DOWNLOAD_ERROR;
+            errorResp.fileName = file.name;
+            errorResp.localPath = resolvedPath;
+            errorResp.error = err as Error;
+            finalResults[i] = errorResp;
           }
-
-          return file.download(passThroughOptionsCopy);
-        }),
+        })
       );
     }
 
-    return Promise.all(promises);
+    await Promise.all(promises);
+    return finalResults;
   }
 
   /**