Skip to content

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

Open
thiyaguk09 wants to merge 8 commits into
googleapis:mainfrom
thiyaguk09:fix/yargs-esm-error
Open

fix(storage): resolve Node compatibility crashes, security vulnerability, and stream hangs#8622
thiyaguk09 wants to merge 8 commits into
googleapis:mainfrom
thiyaguk09:fix/yargs-esm-error

Conversation

@thiyaguk09

Copy link
Copy Markdown
Contributor

This PR resolves Node.js version compatibility crashes, fixes a security vulnerability in our build scripts, and prevents stream cleanup tasks from hanging.

Dependency & Node 26 Fixes: Upgraded yargs to ^17.7.2 to resolve a require is not defined crash on Node 26, while preserving support for Node 18.

Secure Yargs Preload Shim: * Fixed a CWE-377 security vulnerability by moving the generated shim file from the global temp directory to the local project folder. Added auto-cleanup on exit and skipped shimming for .mjs files to prevent crashes on Node 22+.

Stream Reliability (src/file.ts): Added a fallback to ensure pipelineCallback always executes if a write stream is destroyed early, preventing hanging promises.

Test Warning Cleanup (test/file.ts): Updated fakeFs to clone the fs module safely without triggering deprecated getter warnings.

@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Jun 12, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a preload script to safely resolve yargs/yargs dependencies, updates yargs packages, handles destroyed write streams in file.ts, and refactors the test suite to safely copy fs properties. The review feedback suggests two key improvements: first, simplifying the yargs preload script by directly resolving to the CommonJS build file to eliminate disk I/O and potential cleanup issues; second, using Object.getOwnPropertyDescriptors when copying fs properties in tests to completely avoid triggering deprecated getters and printing warnings.

Comment thread handwritten/storage/scripts/preload-yargs.cjs
Comment thread handwritten/storage/test/file.ts
@thiyaguk09 thiyaguk09 marked this pull request as ready for review June 12, 2026 13:37
@thiyaguk09 thiyaguk09 requested a review from a team as a code owner June 12, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant