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';
@@ -540,6 +542,34 @@ export class TransferManager {
    * instead of being returned as a buffer.
    * @returns {Promise<DownloadResponse[]>}
    *
+   * @behavior
+   * **Return shape change (breaking/observable behavior):**
+   * - Previously, the returned array only contained entries for files that were successfully downloaded.
+   * - This meant the response length could be smaller than the number of requested input files.
+   * - Now, the returned array always has the same length and ordering as the input file list.
+   * - Each index in the response corresponds directly to the same index in the input.
+   * - Files that are skipped or fail will still have an entry in the result with:
+   *   - `skipped = true`
+   *   - `reason` populated with a {@link SkipReason}
+   *
+   * **New guarantees:**
+   * - Response length === number of requested files
+   * - Stable positional mapping between input and output
+   * - All outcomes (success, skipped, error) are explicitly represented
+   *
+   * @security
+   * **Path traversal protection (new):**
+   * - File paths are resolved relative to the configured destination directory.
+   * - Any file whose resolved path escapes the base destination directory is rejected.
+   * - This prevents directory traversal attacks (e.g. `../../etc/passwd`).
+   * - Such files are not downloaded and instead return:
+   *   - `skipped = true`
+   *   - `reason = SkipReason.PATH_TRAVERSAL`
+   *
+   * **Additional validation:**
+   * - File names containing illegal drive prefixes (e.g. `C:\`) are skipped
+   * to prevent unintended writes on host systems.
+   *
    * @example
    * ```
    * const {Storage} = require('@google-cloud/storage');
@@ -554,12 +584,15 @@ export class TransferManager {
    * // The following files have been downloaded:
    * // - "file1.txt" (with the contents from my-bucket.file1.txt)
    * // - "file2.txt" (with the contents from my-bucket.file2.txt)
+   * // response.length === 2 (always matches input length)
+   * // Each entry corresponds to the respective input file
    * const response = await transferManager.downloadManyFiles([bucket.File('file1.txt'), bucket.File('file2.txt')]);
    * // The following files have been downloaded:
    * // - "file1.txt" (with the contents from my-bucket.file1.txt)
    * // - "file2.txt" (with the contents from my-bucket.file2.txt)
    * const response = await transferManager.downloadManyFiles('test-folder');
-   * // All files with GCS prefix of 'test-folder' have been downloaded.
+   * // All files with GCS prefix of 'test-folder' have been processed.
+   * // Skipped or failed files are still included in the response.
    * ```
    *
    */
@@ -570,9 +603,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 || '.'
+    );
+
     if (!Array.isArray(filesOrFolder)) {
       const directoryFiles = await this.bucket.getFiles({
         prefix: filesOrFolder,
@@ -592,45 +629,102 @@ 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 fileName = options.stripPrefix
+        ? file.name.replace(regex, '')
+        : file.name;
+
+      const dest = fileName.replace(/^[\\/]+/, '');
+
+      const resolvedPath = path.resolve(baseDestination, dest);
+      const relativeFromBase = path.relative(baseDestination, resolvedPath);
+
+      const isOutside =
+        path.isAbsolute(relativeFromBase) ||
+        relativeFromBase.split(/[\\/]/).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)) {
+        const skippedResult = [Buffer.alloc(0)] as DownloadResponseWithStatus;
+        skippedResult.skipped = true;
+        skippedResult.reason = SkipReason.ALREADY_EXISTS;
+        skippedResult.fileName = file.name;
+        skippedResult.localPath = resolvedPath;
+        finalResults[i] = skippedResult;
         continue;
       }
 
       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>;
+          const passThroughOptionsCopy = {
+            ...options.passthroughOptions,
+            destination: resolvedPath,
+            [GCCL_GCS_CMD_KEY]: GCCL_GCS_CMD_FEATURE.DOWNLOAD_MANY,
+          };
+
+          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;
+
+            finalResults[i] = {
+              ...resp,
+              skipped: false,
+              fileName: file.name,
+              localPath: destination,
+            } as DownloadResponse;
+          } 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;
   }
 
   /**