Update vulnerable dependencies#2692
Conversation
There was a problem hiding this comment.
Pull request overview
This PR attempts to update vulnerable dependencies by using npm overrides to pin specific versions of glob, form-data, and tar packages across the dependency tree.
Changes:
- Added npm
overridessection to package.json pinning glob (7.2.3), form-data (2.5.4), and tar (^7.5.3) - Updated npm-shrinkwrap.json to reflect the dependency resolution with overrides
- Added corresponding entries to common-versions.json for Rush monorepo consistency
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| package.json | Added overrides section to force specific versions of vulnerable dependencies across the dependency tree |
| common/config/rush/npm-shrinkwrap.json | Updated lockfile reflecting the overridden dependency versions and their transitive dependencies |
| common/config/rush/common-versions.json | Added version constraints for glob, form-data, and tar to maintain Rush monorepo consistency |
Files not reviewed (1)
- common/config/rush/npm-shrinkwrap.json: Language not supported
package.json
Outdated
| "zip-stream": { | ||
| "glob": "7.2.3" | ||
| }, | ||
| "form-data": "2.5.4", |
There was a problem hiding this comment.
The form-data version 2.5.4 is deprecated according to npm with the message "This version has an incorrect dependency; please use v2.5.5". Using a deprecated version with known issues defeats the purpose of updating vulnerable dependencies.
Update to version 2.5.5 to avoid the dependency issue in 2.5.4.
| "form-data": "2.5.4", | |
| "form-data": "2.5.5", |
| "glob": "7.2.3" | ||
| }, | ||
| "form-data": "2.5.4", | ||
| "tar": "^7.5.3" |
There was a problem hiding this comment.
The version specification is inconsistent within the overrides section. Most packages use exact versions (e.g., "7.2.3"), but tar uses a caret range ("^7.5.3"). For security-focused dependency updates, exact version pinning is typically preferred to ensure the specific patched version is used.
Consider using "7.5.3" (or the latest secure version) without the caret for consistency and security predictability.
| "tar": "^7.5.3" | |
| "tar": "7.5.3" |
| */ | ||
| // "some-library": "1.2.3" | ||
| "glob": "7.2.3", | ||
| "form-data": "2.5.4", |
There was a problem hiding this comment.
The form-data version 2.5.4 is deprecated according to npm with the message "This version has an incorrect dependency; please use v2.5.5". This should be updated to 2.5.5 to match the recommended version and avoid the dependency issue.
| "form-data": "2.5.4", | |
| "form-data": "2.5.5", |
| // "some-library": "1.2.3" | ||
| "glob": "7.2.3", | ||
| "form-data": "2.5.4", | ||
| "tar": "^7.5.3" |
There was a problem hiding this comment.
The version specification is inconsistent. glob and form-data use exact versions while tar uses a caret range ("^7.5.3"). For security-focused dependency updates, consider using exact version pinning consistently across all dependencies.
Consider using "7.5.3" (or the latest secure version) without the caret for consistency.
| "tar": "^7.5.3" | |
| "tar": "7.5.3" |
| "selenium-server-standalone-jar": "^3.141.5", | ||
| "serve-static": "^1.13.2", | ||
| "sinon": "^7.3.1", | ||
| "tar": "^7.5.3", |
There was a problem hiding this comment.
The tar package version 7.5.6 (satisfying ^7.5.3) requires Node >=18, but the CI workflow tests against Node 16, 18, and 20 (see .github/workflows/ci.yml:21). This will cause installation or runtime failures on Node 16.
Either:
- Drop Node 16 support and update the CI matrix to only test Node 18 and 20
- Use an older version of tar that supports Node 16 (if compatible with security requirements)
This should be addressed before merging to avoid breaking the build for Node 16 users.
| "overrides": { | ||
| "glob": "7.2.3", | ||
| "@rollup/plugin-commonjs": { | ||
| "glob": "^8.0.3" |
There was a problem hiding this comment.
The overrides configuration has an inconsistency: glob is pinned to exact version "7.2.3" for most packages, but @rollup/plugin-commonjs uses "^8.0.3" (a different major version with caret). This could lead to unexpected behavior.
Consider standardizing to either:
- Pin all glob versions to "7.2.3" if that's the secure version needed
- Use consistent semver ranges if different major versions are intentionally required
Based on the shrinkwrap changes showing @rollup/plugin-commonjs getting glob 8.1.0, it appears this might be intentional, but the reasoning should be documented or reconsidered for consistency.
| "glob": "^8.0.3" | |
| "glob": "7.2.3" |
| "whatwg-fetch": "^3.6.2", | ||
| "@types/node": "18.19.121" | ||
| }, | ||
| "overrides": { |
There was a problem hiding this comment.
this is awful and it's going to become unmanageable, there has to be another way to manage these... Rather than list then for every dependency
There was a problem hiding this comment.
The copilot instance decided to it that way for some random reason, having a single override should work as well, we should remove it once all dependencies are updated, we may need to switch certain dependencies to other more active and secure alternatives in the future
2 participants
No description provided.