Fix vulnerability by updating bn.js versions to address CVE-2026-2739#1567
Conversation
📝 WalkthroughWalkthroughAdds version normalization logic for the bn.js package in the pnpm configuration file. The readPackage hook is updated to rewrite bn.js versions to 5.2.3 when matching ^5, and 4.12.3 otherwise, across both dependencies and devDependencies. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
gigara
left a comment
There was a problem hiding this comment.
We're handling overrides in common/config/rush/.pnpmfile.cjs. Better to keep all in one place to maintain consistency.
Updated it with d7c2e91 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
common/config/rush/.pnpmfile.cjs (2)
137-145: Dead-codereadPackagefunction is never invokedThe standalone
function readPackageat Line 137 is never called or exported; the active hook is the one registered at Line 17 insidemodule.exports.hooks. This appears to be a leftover template stub and can be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/config/rush/.pnpmfile.cjs` around lines 137 - 145, Remove the dead standalone function readPackage defined at the bottom (function readPackage(packageJson, context)) since it is never invoked or exported; keep the active hook implementation under module.exports.hooks (the hook registered at Line 17) as the canonical entrypoint, and ensure there are no other references to readPackage before deleting it to avoid breaking anything.
37-63: Pre-existing duplicate overrides fordiff,eslint, andlodash
diff,eslint, andlodasheach appear twice in both thedependenciesblock (lines 37/52, 40/55, 46/61) and thedevDependenciesblock. The duplicate assignments are harmless today (the second write is identical to the first), but they create a maintenance trap — future edits to one copy that miss the other will silently leave them inconsistent.Consider removing the duplicate entries while you already have this file open for changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/config/rush/.pnpmfile.cjs` around lines 37 - 63, The diff shows duplicate override assignments for pkg.dependencies['diff'], pkg.dependencies['eslint'], and pkg.dependencies['lodash'] (and the same duplicates in the devDependencies block); remove the redundant repeated assignments so each package key is set exactly once per block (keep one occurrence of pkg.dependencies['diff'] = '^8.0.3', pkg.dependencies['eslint'] = '^9.27.0', pkg.dependencies['lodash'] = '4.17.23' and the corresponding single assignments in devDependencies), ensuring no behavioral change but eliminating the maintenance trap.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/config/rush/.pnpmfile.cjs`:
- Around line 64-68: The current check uses
pkg.dependencies['bn.js'].startsWith('^5') which misses tilde, exact, or range
specs and can wrongly pin v5 consumers to 4.x; update the detection in the bn.js
handling to test the version string with a semver-aware regex that matches any
expression resolving to major version 5 (e.g. /(^|[>=~^* ,])5(\\.|$)/ or
equivalent that detects ranges, tildes, carets, exacts and inequalities) and use
that result to select '5.2.3' vs '4.12.3'; apply the same robust detection
change to the symmetric devDependencies block that handles
pkg.devDependencies['bn.js'] so both dependency and devDependency cases behave
the same.
---
Nitpick comments:
In `@common/config/rush/.pnpmfile.cjs`:
- Around line 137-145: Remove the dead standalone function readPackage defined
at the bottom (function readPackage(packageJson, context)) since it is never
invoked or exported; keep the active hook implementation under
module.exports.hooks (the hook registered at Line 17) as the canonical
entrypoint, and ensure there are no other references to readPackage before
deleting it to avoid breaking anything.
- Around line 37-63: The diff shows duplicate override assignments for
pkg.dependencies['diff'], pkg.dependencies['eslint'], and
pkg.dependencies['lodash'] (and the same duplicates in the devDependencies
block); remove the redundant repeated assignments so each package key is set
exactly once per block (keep one occurrence of pkg.dependencies['diff'] =
'^8.0.3', pkg.dependencies['eslint'] = '^9.27.0', pkg.dependencies['lodash'] =
'4.17.23' and the corresponding single assignments in devDependencies), ensuring
no behavioral change but eliminating the maintenance trap.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
common/config/rush/.pnpmfile.cjs
| if (pkg.dependencies['bn.js']) { | ||
| pkg.dependencies['bn.js'] = pkg.dependencies['bn.js'].startsWith('^5') | ||
| ? '5.2.3' | ||
| : '4.12.3'; | ||
| } |
There was a problem hiding this comment.
Version-range detection via startsWith('^5') will silently cross-major-downgrade non-caret v5 consumers
The condition startsWith('^5') only matches the exact prefix ^5. Any upstream package that declares bn.js with a tilde range (~5.x.x), a bare exact version (5.2.2), or an inequality range (>=5.0.0) will fall through to the else branch and be silently pinned to 4.12.3 — a cross-major downgrade that could cause hard-to-debug runtime breakage.
Replace the heuristic with a regex that detects any semver expression resolving to major version 5:
🛡️ Proposed fix — robust major-version detection
- if (pkg.dependencies['bn.js']) {
- pkg.dependencies['bn.js'] = pkg.dependencies['bn.js'].startsWith('^5')
- ? '5.2.3'
- : '4.12.3';
- }
+ if (pkg.dependencies['bn.js']) {
+ pkg.dependencies['bn.js'] = /(?:^|\s)[~^]?5\./.test(pkg.dependencies['bn.js'])
+ ? '5.2.3'
+ : '4.12.3';
+ }Apply the same change symmetrically to the devDependencies block at lines 117-121.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@common/config/rush/.pnpmfile.cjs` around lines 64 - 68, The current check
uses pkg.dependencies['bn.js'].startsWith('^5') which misses tilde, exact, or
range specs and can wrongly pin v5 consumers to 4.x; update the detection in the
bn.js handling to test the version string with a semver-aware regex that matches
any expression resolving to major version 5 (e.g. /(^|[>=~^* ,])5(\\.|$)/ or
equivalent that detects ranges, tildes, carets, exacts and inequalities) and use
that result to select '5.2.3' vs '4.12.3'; apply the same robust detection
change to the symmetric devDependencies block that handles
pkg.devDependencies['bn.js'] so both dependency and devDependency cases behave
the same.
PR Description
Security Fix
Resolves CVE-2026-2739: Denial of Service vulnerability in
bn.jsvia callingmaskn(0)Changes
Added global package overrides in pnpm-config.json to pin
bn.jsto patched versions across all dependencies:bn.js@^4.0.0→4.12.3— Fixes upstream consumers:asn1.js,create-ecdh,diffie-hellman,elliptic,miller-rabin,public-encryptbn.js@^5.0.0→5.2.3— Fixes upstream consumers:browserify-rsaTechnical Details
The vulnerability affects cryptographic operations that utilize
bn.jsfor big number arithmetic. The affected packages are transitive dependencies included in the build chain for various VS Code extensions.Vulnerability Details:
bn.js< 4.12.3 and < 5.2.3Verification
Updated dependencies via
rush update, confirming:bn.js@4.12.3now resolves for all^4.0.0constraintsbn.js@5.2.3now resolves for all^5.0.0constraintsSummary by CodeRabbit