fix(ng-dev/release): validate .nvmrc and resolved node path in invokeNvmInstall#3753
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several security enhancements, including preventing script injection in GitHub Actions by using environment variables, securing artifact metadata injection against symbolic link and TOCTOU attacks, and validating .nvmrc contents and resolved Node.js paths in ExternalCommands to prevent command injection and path traversal. Feedback on the changes highlights a vulnerability in the path validation logic where using startsWith could allow partial path traversal (e.g., /home/user matching /home/user_attacker). The reviewer suggests using path.relative and path.isAbsolute to perform robust, path-segment-aware validation.
josephperrott
force-pushed
the
fix/sec-nvm-path-hijack-436c969b
branch
from
db84f8a to
588ec54
Compare
…NvmInstall Validates the version string read from .nvmrc to ensure it matches a valid version pattern and does not contain path traversal characters. Also resolves the absolute path of the node binary returned from 'nvm which' and asserts that it resides under the home directory (e.g., ~/.nvm) rather than inside the project checkout or an untrusted path. This prevents RCE via PATH hijacking. Added unit tests in ng-dev/release/publish/test/external-commands.spec.ts to verify validation behavior with valid and malicious inputs. Vulnerability: 436c969b
josephperrott
force-pushed
the
fix/sec-nvm-path-hijack-436c969b
branch
from
588ec54 to
6ef3fc8
Compare
|
This PR was merged into the repository. The changes were merged into the following branches:
|
2 participants
This PR resolves a PATH hijacking vulnerability in the release publishing tool.
Changes
.nvmrcversion content matches a valid version pattern.nvm whichpoints under the home directory (~/.nvm).ng-dev/release/publish/test/external-commands.spec.ts.bazel test //ng-dev/release/publish/test:test.Vulnerability: 436c969b