Skip to content

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

Open
thiyaguk09 wants to merge 3 commits into
mainfrom
fix/yargs-esm-error
Open

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

Conversation

@thiyaguk09

@thiyaguk09 thiyaguk09 commented Jun 9, 2026

Copy link
Copy Markdown
Owner

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.

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

Copy link
Copy Markdown

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 for yargs to handle relative path resolution during testing, updates yargs dependencies, safely copies fs properties in tests to avoid deprecated getters, and adds a check for destroyed write streams in file.ts. Feedback on these changes highlights a security vulnerability in the preload script where writing to os.tmpdir() could allow insecure temporary file injection; it is recommended to write the shim locally and remove the unused os import. Additionally, in file.ts, checking for a destroyed stream without a fallback could result in a hung promise if no error is emitted, so a robust callback mechanism using process.nextTick should be implemented.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread handwritten/storage/scripts/preload-yargs.cjs Outdated
Comment thread handwritten/storage/src/file.ts
Comment thread handwritten/storage/scripts/preload-yargs.cjs Outdated
@thiyaguk09

Copy link
Copy Markdown
Owner Author

/Gemini review

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

Copy link
Copy Markdown

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 updates dependencies (specifically upgrading yargs), introduces a preload script to shim yargs module resolution, adds safety checks for destroyed write streams in file.ts, and refactors the test setup to safely copy the fs module while ignoring deprecated getters. The review feedback suggests cleaning up the generated shim files on process exit to avoid untracked files, and simplifying the test setup by copying keys directly from the cached fs reference instead of re-requiring the module.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread handwritten/storage/scripts/preload-yargs.cjs
Comment thread handwritten/storage/test/file.ts
@thiyaguk09 thiyaguk09 changed the title fix(deps): update yargs to v17.7.2 for Node 18 and 26 compatibility fix(storage): resolve Node compatibility crashes, security vulnerability, and stream hangs Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant