chore(deps): update npm dependencies#129
Conversation
Updated packages: - js-yaml: 3.14.2 -> 4.2.0 - argparse: 1.0.10 -> 2.0.1 Resolved Dependabot alert 43 / GHSA-h67p-54hq-rp68 by using an npm override for the transitive development dependency pulled in by Jest coverage tooling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
⚠️ Not ready to approve
The override forces a semver-incompatible major js-yaml upgrade for at least one transitive consumer and introduces a license change (argparse), both of which should be addressed/confirmed before approval.
Pull request overview
Updates the JS dependency graph for crates/string-offsets/js by adding an npm overrides entry and regenerating the lockfile to move js-yaml to a patched version intended to resolve Dependabot alert 43.
Changes:
- Added an npm
overridesrule to forcejs-yamlto^4.2.0. - Regenerated
package-lock.jsonto reflectjs-yaml@4.2.0and its updated transitive deps (argparse@2.0.1), removing now-unused packages (esprima,sprintf-js).
File summaries
| File | Description |
|---|---|
| crates/string-offsets/js/package.json | Adds an npm override to force js-yaml to a patched major version. |
| crates/string-offsets/js/package-lock.json | Updates resolved dependency tree to js-yaml@4.2.0 and prunes no-longer-needed transitive packages. |
Copilot's findings
Files not reviewed (1)
- crates/string-offsets/js/package-lock.json: Generated file
- Files reviewed: 1/2 changed files
- Comments generated: 2
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "node_modules/js-yaml": { | ||
| "version": "3.14.2", | ||
| "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.14.2.tgz", | ||
| "integrity": "sha512-PMSmkqxr106Xa156c2M265Z+FTrPl+oxd/rgOQy2tijQeK5TxQ43psO1ZCwhVOSdnn+RzkzlRz/eY4BgJBYVpg==", | ||
| "version": "4.2.0", | ||
| "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.2.0.tgz", | ||
| "integrity": "sha512-ePWsvanv0DWuDRsW8dnt+R4jQ31SCRCQ7hhNcPXZPsoBZiemuZNYGf7adZdqX2D86j6rvKp3RpCxVTSb8WQlOw==", | ||
| "dev": true, | ||
| "funding": [ | ||
| { | ||
| "type": "github", | ||
| "url": "https://github.com/sponsors/puzrin" | ||
| }, | ||
| { | ||
| "type": "github", | ||
| "url": "https://github.com/sponsors/nodeca" | ||
| } | ||
| ], | ||
| "license": "MIT", | ||
| "dependencies": { | ||
| "argparse": "^1.0.7", | ||
| "esprima": "^4.0.0" | ||
| "argparse": "^2.0.1" | ||
| }, |
There was a problem hiding this comment.
Addressed by removing the Jest/Istanbul dependency chain rather than overriding js-yaml. The JS sanity tests now use Node built-ins (node:test and node:assert/strict), so @istanbuljs/load-nyc-config and its js-yaml@^3.13.1 dependency are no longer present in the lockfile.
| "node_modules/argparse": { | ||
| "version": "1.0.10", | ||
| "resolved": "https://registry.npmjs.org/argparse/-/argparse-1.0.10.tgz", | ||
| "integrity": "sha512-o5Roy6tNG4SL/FOkCAN6RzjiakZS25RLYFrcMttJqbdd8BWrnA+fGz57iN5Pb06pvBGvl5gQ0B48dJlslXvoTg==", | ||
| "version": "2.0.1", | ||
| "resolved": "https://registry.npmjs.org/argparse/-/argparse-2.0.1.tgz", | ||
| "integrity": "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==", | ||
| "dev": true, | ||
| "license": "MIT", | ||
| "dependencies": { | ||
| "sprintf-js": "~1.0.2" | ||
| } | ||
| "license": "Python-2.0" | ||
| }, |
There was a problem hiding this comment.
Addressed by removing the Jest/Istanbul dependency chain. argparse is no longer present in the regenerated lockfile, so the Python-2.0 license change is avoided.
Replace Jest with Node's built-in test runner for the string-offsets JS sanity tests. This removes the transitive Istanbul dependency chain that pulled in vulnerable js-yaml, avoiding a semver-incompatible override. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Removes the vulnerable npm dependency chain in
crates/string-offsets/jsthat pulled injs-yamlthrough Jest/Istanbul coverage tooling. Instead of using an npm override across a semver-incompatible major version, the JS sanity tests now use Node's built-innode:testrunner andnode:assert/strict.Changed packages:
jest,@types/jestbabel-plugin-istanbul->@istanbuljs/load-nyc-config->js-yamloverridesentrySecurity alerts resolved
Reviewer notes
There is no current stable native upgrade path in the Jest/Istanbul dependency chain: the latest
@istanbuljs/load-nyc-configstill declaresjs-yaml: ^3.13.1, and the latest stable Jest transform chain still pulls that package throughbabel-plugin-istanbul. Removing Jest for these two small sanity tests avoids both the semver-incompatible override and theargparselicense change noted in review.Validation
npm testincrates/string-offsets/jsmake lintmake testmake buildmake build-jsGenerated by the update-deps skill.