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

* feat: implement secure path validation for downloadManyFiles

- Adds protection against path traversal (../) using normalized path
resolution.
- Prevents Windows-style drive letter injection while allowing GCS
timestamps.
- Implements directory jail logic to ensure absolute-style paths are
relative to destination.
- Preserves backward compatibility by returning an augmented
DownloadResponse array.
- Automates recursive directory creation for validated nested files.
- Adds comprehensive 13-scenario test suite for edge-case parity.

* fix double-assignment

* feat(storage): secure path resolution and preserve result parity

- Implemented "jail" logic using path.resolve to prevent traversal.
- Neutralized leading slashes in GCS object names.
- Pre-allocated results array to maintain 1:1 input/output index parity.
- Added automatic recursive directory creation for nested local paths.
- Fixed prioritization of destination options in downloadManyFiles.

* feat(storage): prioritize drive check for Windows compatibility

- Reordered security checks to catch illegal drive letters before path
resolution.
- Fixed SkipReason mismatch (Illegal Character vs Path Traversal) on
Windows.
- Ensured absolute Windows paths are neutralized before traversal
validation.

* fix(storage): storage downloadManyFiles scoping and path traversal safety

Isolated async loop variables to fix path leakage, decoupled prefix
from destination logic, and added cross-platform traversal checks
for both forward and backslashes.

* fix(storage): ensure unique path resolution and cross-platform safety

Updated Parity Check tests with platform-conditional logic to handle
OS-specific backslash resolution.

* fix(storage): address logical escapes in stripPrefix

* fix: clarify downloadManyFiles docs

* build(storage): suppress punycode deprecation errors in tests

Node.js 22+ treats the deprecation of the built-in 'punycode' module
as a fatal error during the test load phase. Since this originates
deep within nested dependencies (node-fetch > whatwg-url), this
change adds --no-deprecation to NODE_OPTIONS in the test script
to allow the suite to run on modern Node versions.

* build(storage): fix CI test detection and Node 22 compatibility

- Added 'cross-env' to devDependencies for Windows compatibility.
- Used 'cross-env' to pass '--no-deprecation' to the test runner.
- This prevents 'punycode' deprecation warnings from crashing the suite.

---------

Co-authored-by: Gabe Pearhill <86282859+pearigee@users.noreply.github.com>

Author: thiyaguk09

handwritten/storage/package.json

@@ -73,7 +73,7 @@
     "samples-test": "npm link && cd samples/ && npm link ../ && npm test && cd ../",
     "system-test:esm": "mocha build/esm/system-test --timeout 600000 --exit",
     "system-test": "mocha build/cjs/system-test --timeout 600000 --exit",
-    "test": "c8 mocha build/cjs/test"
+    "test": "cross-env NODE_OPTIONS='--no-deprecation' c8 mocha build/cjs/test"
   },
   "dependencies": {
     "@google-cloud/paginator": "^5.0.0",
@@ -130,7 +130,8 @@
     "path-to-regexp": "6.3.0",
     "tmp": "^0.2.0",
     "typescript": "^5.1.6",
-    "yargs": "^17.3.1"
+    "yargs": "^17.3.1",
+    "cross-env": "^7.0.3"
   },
   "homepage": "https://github.com/googleapis/google-cloud-node/tree/main/handwritten/storage"
 }

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