fix: check-overrides.js compares severity buckets, not just a flat total - #847
Merged
AnujVishwakarma-src merged 1 commit intoSep 21, 2026
Conversation
The old comparison summed critical+high+moderate+low into one number, so a shift from moderate to high severity (equal counts) looked like zero net change and got reported as 'safe to remove'. Found on the tar override: removing it keeps the total at 54 either way, but tar sprawls from 2 consolidated copies to 6 per-consumer ones (cacache/node-gyp/pacote/npm/ yeoman-generator), reintroducing 6 older high-severity hardlink/symlink path-traversal CVEs that the pinned version was protecting against. Now compares each severity bucket independently — an override is only reported removable if no bucket gets worse, regardless of what the flat total does.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
bin/check-overrides.jsdecides whether an override is removable by comparing a single flat sum (critical + high + moderate + low) before and after removal. This misses severity regressions where the total happens to stay flat.Fix
Compare each severity bucket (critical/high/moderate/low) independently. An override is only reported as removable if no bucket gets worse, regardless of what the flat total does.
Validated
tarnow correctly reportsKEEP ... removing changes: +8 high, -8 moderateinstead of the previous falseREMOVE.@octokit/restand@yeoman/conflicter > diffstill correctly reportKEEP(unchanged behavior for genuine single-direction regressions).npm test: 53/53 pass, lint clean (this script lives inbin/, outside the enforcedsrc/**lint/coverage scope, same as before).--markdownoutput modes checked.Example:
"overrides": {
"tar": "^7.4.3",
"@octokit/rest": "^20.0.2",
"@yeoman/conflicter": {
"diff": "^8.0.4"
}
},
Override Removal Report
check-overrides.js:219============================================================
check-overrides.js:228 KEEP tar removing adds +21 vulns
check-overrides.js:228 KEEP @octokit/rest removing adds +20 vulns
check-overrides.js:228 KEEP @yeoman/conflicter > diff removing adds +15 vulns
with override : 53 vulnerabilities (7 low, 25 moderate, 20 high, 1 critical)
without override: 53 vulnerabilities (7 low, 17 moderate, 28 high, 1 critical)
Ideally overrride should be there!! ::-->