fix(storage): resolve Node compatibility crashes, security vulnerability, and stream hangs (#8622)

* fix(deps): update yargs to v17.7.2 for Node 18 and 26 compatibility

* fix: relocate yargs shim generation to local directory and handle stream error callbacks in File.ts

* fix: update safeFs reference in tests and add yargs shim generation with cleanup on process exit

* fix: update fs mocking to use property descriptors for safer member access

* refactor: simplify yargs resolution by targeting the build index directly instead of using shim files

* refactor: generate unique local shim files for yargs resolution to improve security and prevent path conflicts

Author: thiyaguk09

handwritten/storage/package.json

@@ -71,7 +71,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": "cross-env NODE_OPTIONS='--no-deprecation' c8 mocha build/cjs/test"
+    "test": "cross-env NODE_OPTIONS=\"--require ./scripts/preload-yargs.cjs --no-deprecation\" c8 mocha build/cjs/test"
   },
   "dependencies": {
     "@google-cloud/paginator": "^5.0.0",
@@ -106,7 +106,7 @@
     "@types/request": "^2.48.4",
     "@types/sinon": "^17.0.0",
     "@types/tmp": "0.2.6",
-    "@types/yargs": "^17.0.10",
+    "@types/yargs": "^17.0.35",
     "c8": "^9.0.0",
     "form-data": "^4.0.4",
     "gapic-tools": "^0.4.0",
@@ -125,7 +125,7 @@
     "path-to-regexp": "6.3.0",
     "tmp": "^0.2.0",
     "typescript": "^5.1.6",
-    "yargs": "^17.3.1",
+    "yargs": "^17.7.2",
     "cross-env": "^7.0.3"
   },
   "homepage": "https://github.com/googleapis/google-cloud-node/tree/main/handwritten/storage"

handwritten/storage/scripts/preload-yargs.cjs

@@ -0,0 +1,39 @@
+const Module = require('module');
+const fs = require('fs');
+const path = require('path');
+
+const originalResolveFilename = Module._resolveFilename;
+
+Module._resolveFilename = function(request, parent, isMain, options) {
+  if (request === 'yargs/yargs') {
+    const resolved = originalResolveFilename.apply(this, arguments);
+    if (resolved.endsWith('.mjs')) {
+      return resolved;
+    }
+    // Create a unique shim file in the local scripts directory to avoid shared temp directory vulnerabilities
+    const safeHash = Buffer.from(resolved).toString('base64').replace(/[^a-zA-Z0-9]/g, '');
+    const shimPath = path.join(__dirname, `yargs-shim-${safeHash}.cjs`);
+
+    if (!fs.existsSync(shimPath)) {
+      const content = fs.readFileSync(resolved, 'utf8');
+      // Replace `./build/index.cjs` with the absolute path
+      const buildIndexPath = path.join(path.dirname(resolved), 'build', 'index.cjs');
+      // We must replace ALL relative requires. Luckily yargs/yargs only requires `./build/index.cjs`
+      const newContent = content.replace(/require\(['"]\.\/build\/index\.cjs['"]\)/g, `require(${JSON.stringify(buildIndexPath)})`);
+      fs.writeFileSync(shimPath, newContent);
+    }
+
+    global.__yargsShimCleanups = global.__yargsShimCleanups || new Set();
+    if (!global.__yargsShimCleanups.has(shimPath)) {
+      global.__yargsShimCleanups.add(shimPath);
+      process.on('exit', () => {
+        try {
+          fs.unlinkSync(shimPath);
+        } catch (e) {}
+      });
+    }
+
+    return shimPath;
+  }
+  return originalResolveFilename.apply(this, arguments);
+};

handwritten/storage/src/file.ts

@@ -2187,6 +2187,29 @@ class File extends ServiceObject<File, FileMetadata> {
       // remove temporary noop listener as we now create a pipeline that handles the errors
       emitStream.removeListener('error', noop);
 
+      if (fileWriteStream.destroyed) {
+        let callbackCalled = false;
+        const onError = (err: Error) => {
+          if (!callbackCalled) {
+            callbackCalled = true;
+            pipelineCallback(err);
+          }
+        };
+        fileWriteStream.once('error', onError);
+        emitStream.destroy();
+
+        process.nextTick(() => {
+          fileWriteStream.removeListener('error', onError);
+          if (!callbackCalled) {
+            callbackCalled = true;
+            // eslint-disable-next-line @typescript-eslint/no-explicit-any
+            const err = (fileWriteStream as any).errored || new Error('Write stream destroyed');
+            pipelineCallback(err);
+          }
+        });
+        return;
+      }
+
       pipeline(
         emitStream,
         ...(transformStreams as [Transform]),

handwritten/storage/test/file.ts

@@ -118,7 +118,15 @@ const fakePromisify = {
 };
 
 const fsCached = fs;
-const fakeFs = {...fsCached};
+const safeFs: any = {};
+const descriptors = Object.getOwnPropertyDescriptors(fsCached);
+for (const key of Object.keys(descriptors)) {
+  const desc = descriptors[key];
+  if (desc && !desc.get) {
+    Object.defineProperty(safeFs, key, desc);
+  }
+}
+const fakeFs = {...safeFs} as typeof fs;
 
 const zlibCached = zlib;
 let createGunzipOverride: Function | null;
@@ -223,7 +231,7 @@ describe('File', () => {
   });
 
   beforeEach(() => {
-    Object.assign(fakeFs, fsCached);
+    Object.assign(fakeFs, safeFs);
     Object.assign(fakeOs, osCached);
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     FakeServiceObject.prototype.request = util.noop as any;