Skip to content

Commit 0c2f3f8

Browse files
shazronclaude
andauthored
feat: add script to detect removable npm overrides (#824)
* feat: add script to detect removable npm overrides Adds bin/check-overrides.js which temporarily removes each override from package.json, re-resolves the lockfile, and runs npm audit to determine whether the override is still needed. Exits 1 if any override can be safely removed (useful for CI). Runs automatically via prepare (local installs) and prepack (before publish). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: remove stale rimraf override Upstream packages have updated their own rimraf constraints, so forcing ^5.0.7 via overrides is no longer needed to pass npm audit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove check-overrides from prepare hook prepare runs on every npm install, making it slow and surprising for developers and CI. prepack (publish time) is the right place to catch stale overrides before they ship. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: propagate npm audit errors instead of silently returning 0 A failed audit (network error, malformed output, missing fields) was indistinguishable from a clean one, risking false 'safe to remove' results. Now throws with a descriptive message. Baseline failure exits with code 2; per-override audit failure is recorded as an error in the report rather than a false clean result. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: run all npm operations in temp dirs to avoid mutating repo files Previously the baseline audit ran in ROOT, which caused npm to update package-lock.json when it detected a drift between package.json and the lockfile. Now every npm invocation (baseline and per-override checks) works in an isolated temp dir that is cleaned up in finally blocks, so the script never touches any file in the working tree. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: guard Math.max on empty results and register signal handlers for cleanup - Math.max(...[]) throws RangeError when no entries were processed (e.g. all errored before push); guard with a results.length check - SIGINT/SIGTERM handlers now call cleanupTmpDirs() before exiting so a mid-run kill does not leave temp directories behind; active dirs are tracked in a Set and removed from it in the finally block so the handler only touches dirs that haven't been cleaned up yet Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address four edge cases in check-overrides 1. Null stderr: spawnSync returns null stderr when a process is killed by a signal; use (stderr ?? '').trim() instead of stderr?.trim() to make the null handling explicit. 2. Negative newVulns: removing an override can reduce the vuln count if it was masking a vulnerability elsewhere. Report this explicitly in both plain and markdown output rather than silently showing as clean. 3. Network-restricted environments: add --prefer-offline flag (pass via CLI) to use only locally cached advisory data; document the network assumption in the script header comment. 4. Prepack behavior: document in the script header that check-overrides runs as part of prepack and will block publish when stale overrides are found (package.json cannot carry comments as it is plain JSON). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent fb23124 commit 0c2f3f8

2 files changed

Lines changed: 238 additions & 3 deletions

File tree

bin/check-overrides.js

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
#!/usr/bin/env node
2+
3+
/**
4+
* Checks each npm override in package.json and reports whether it can be safely removed.
5+
*
6+
* For each override: temporarily removes it, re-resolves the lockfile (no actual install),
7+
* runs npm audit, then cleans up. Original files are never modified.
8+
*
9+
* Exits 0 if all overrides are still needed, 1 if any can be removed.
10+
* Runs as part of prepack to prevent publishing with stale overrides.
11+
*
12+
* Note: npm audit makes network requests to the registry. In network-restricted
13+
* environments pass --prefer-offline to use only locally cached advisory data.
14+
*
15+
* Usage: node bin/check-overrides.js [--markdown] [--prefer-offline]
16+
*/
17+
18+
const { spawnSync } = require('child_process')
19+
const fs = require('fs')
20+
const os = require('os')
21+
const path = require('path')
22+
23+
const ROOT = process.cwd()
24+
const PKG_PATH = path.join(ROOT, 'package.json')
25+
const LOCK_PATH = path.join(ROOT, 'package-lock.json')
26+
const MARKDOWN = process.argv.includes('--markdown')
27+
const PREFER_OFFLINE = process.argv.includes('--prefer-offline')
28+
29+
// ── helpers ──────────────────────────────────────────────────────────────────
30+
31+
function npm(cwd, ...args) {
32+
return spawnSync('npm', args, { cwd, encoding: 'utf8' })
33+
}
34+
35+
function auditVulnCount(cwd) {
36+
const args = ['audit', '--json']
37+
if (PREFER_OFFLINE) args.push('--prefer-offline')
38+
const { stdout, stderr, status } = npm(cwd, ...args)
39+
let parsed
40+
try {
41+
parsed = JSON.parse(stdout)
42+
} catch {
43+
throw new Error(`npm audit returned non-JSON output (exit ${status}):\n${stderr || stdout || '(no output)'}`)
44+
}
45+
if (!parsed?.metadata?.vulnerabilities) {
46+
throw new Error(`npm audit JSON missing expected metadata.vulnerabilities field:\n${stdout}`)
47+
}
48+
const v = parsed.metadata.vulnerabilities
49+
return (v.critical || 0) + (v.high || 0) + (v.moderate || 0) + (v.low || 0)
50+
}
51+
52+
/**
53+
* Flattens nested overrides into dot-path entries, e.g.:
54+
* { "foo": "^1", "bar": { "baz": "^2" } }
55+
* becomes:
56+
* [ { dotPath: "foo", label: "foo" },
57+
* { dotPath: "bar.baz", label: "bar > baz" } ]
58+
*/
59+
function flattenOverrides(overrides, prefix = '') {
60+
const entries = []
61+
for (const [key, val] of Object.entries(overrides)) {
62+
const dotPath = prefix ? `${prefix}.${key}` : key
63+
const label = prefix ? `${prefix} > ${key}` : key
64+
if (val !== null && typeof val === 'object') {
65+
entries.push(...flattenOverrides(val, dotPath))
66+
} else {
67+
entries.push({ dotPath, label, val })
68+
}
69+
}
70+
return entries
71+
}
72+
73+
function deleteAtDotPath(obj, dotPath) {
74+
const parts = dotPath.split('.')
75+
let cur = obj
76+
for (let i = 0; i < parts.length - 1; i++) {
77+
cur = cur[parts[i]]
78+
if (cur == null) return
79+
}
80+
delete cur[parts.at(-1)]
81+
// prune empty parent objects
82+
if (parts.length > 1) {
83+
const parent = parts.slice(0, -1).reduce((o, k) => o[k], obj)
84+
if (parent && Object.keys(parent).length === 0) {
85+
deleteAtDotPath(obj, parts.slice(0, -1).join('.'))
86+
}
87+
}
88+
}
89+
90+
// ── main ─────────────────────────────────────────────────────────────────────
91+
92+
const originalPkg = fs.readFileSync(PKG_PATH, 'utf8')
93+
const pkg = JSON.parse(originalPkg)
94+
const overrides = pkg.overrides || {}
95+
96+
if (Object.keys(overrides).length === 0) {
97+
console.log('No overrides found in package.json.')
98+
process.exit(0)
99+
}
100+
101+
const entries = flattenOverrides(overrides)
102+
if (entries.length === 0) {
103+
console.log('No scalar overrides found.')
104+
process.exit(0)
105+
}
106+
107+
process.stderr.write('Checking baseline audit… ')
108+
let baselineVulns
109+
const baselineDir = fs.mkdtempSync(path.join(os.tmpdir(), 'check-overrides-baseline-'))
110+
try {
111+
fs.writeFileSync(path.join(baselineDir, 'package.json'), originalPkg)
112+
fs.copyFileSync(LOCK_PATH, path.join(baselineDir, 'package-lock.json'))
113+
baselineVulns = auditVulnCount(baselineDir)
114+
} catch (e) {
115+
process.stderr.write('failed\n')
116+
console.error(`Error: could not establish baseline — ${e.message}`)
117+
process.exit(2)
118+
} finally {
119+
fs.rmSync(baselineDir, { recursive: true, force: true })
120+
}
121+
process.stderr.write(`${baselineVulns} vulnerabilities\n\n`)
122+
123+
const results = []
124+
125+
// ── signal handlers ──────────────────────────────────────────────────────────
126+
127+
const activeTmpDirs = new Set()
128+
129+
function cleanupTmpDirs() {
130+
for (const dir of activeTmpDirs) {
131+
try { fs.rmSync(dir, { recursive: true, force: true }) } catch {}
132+
}
133+
}
134+
135+
process.on('SIGINT', () => { cleanupTmpDirs(); process.exit(130) })
136+
process.on('SIGTERM', () => { cleanupTmpDirs(); process.exit(143) })
137+
138+
// ─────────────────────────────────────────────────────────────────────────────
139+
140+
for (const entry of entries) {
141+
process.stderr.write(` Checking "${entry.label}"… `)
142+
143+
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'check-overrides-'))
144+
activeTmpDirs.add(tmpDir)
145+
try {
146+
const testPkg = JSON.parse(originalPkg)
147+
deleteAtDotPath(testPkg.overrides, entry.dotPath)
148+
fs.writeFileSync(path.join(tmpDir, 'package.json'), JSON.stringify(testPkg, null, 2))
149+
fs.copyFileSync(LOCK_PATH, path.join(tmpDir, 'package-lock.json'))
150+
151+
const { status, stderr } = npm(
152+
tmpDir, 'install', '--package-lock-only', '--ignore-scripts', '--no-audit', '--silent'
153+
)
154+
if (status !== 0) {
155+
// stderr may be null if the process was killed by a signal
156+
results.push({ ...entry, canRemove: false, error: (stderr ?? '').trim() || 'npm install failed' })
157+
process.stderr.write('install failed\n')
158+
continue
159+
}
160+
161+
let vulns
162+
try {
163+
vulns = auditVulnCount(tmpDir)
164+
} catch (e) {
165+
results.push({ ...entry, canRemove: false, error: `audit failed: ${e.message}` })
166+
process.stderr.write('audit failed\n')
167+
continue
168+
}
169+
const newVulns = vulns - baselineVulns
170+
const canRemove = newVulns <= 0
171+
172+
results.push({ ...entry, canRemove, newVulns })
173+
if (canRemove) {
174+
const note = newVulns < 0 ? ` (removing reduces vulns by ${-newVulns})` : ''
175+
process.stderr.write(`safe to remove${note}\n`)
176+
} else {
177+
process.stderr.write(`still needed (+${newVulns} vuln${newVulns !== 1 ? 's' : ''})\n`)
178+
}
179+
} finally {
180+
fs.rmSync(tmpDir, { recursive: true, force: true })
181+
activeTmpDirs.delete(tmpDir)
182+
}
183+
}
184+
185+
// ── report ───────────────────────────────────────────────────────────────────
186+
187+
const removable = results.filter(r => r.canRemove)
188+
const needed = results.filter(r => !r.canRemove)
189+
const exitCode = removable.length > 0 ? 1 : 0
190+
191+
if (MARKDOWN) {
192+
console.log('# Override Removal Report\n')
193+
console.log(`Baseline: **${baselineVulns}** audit vulnerabilities with all overrides in place.\n`)
194+
195+
if (removable.length) {
196+
console.log('## Safe to Remove\n')
197+
console.log('These overrides no longer affect the audit result and can be deleted from `package.json`:\n')
198+
for (const r of removable) {
199+
const note = r.newVulns < 0 ? ` _(removing this actually reduces vulns by ${-r.newVulns})_` : ''
200+
console.log(`- \`${r.label}\` → \`${r.val}\`${note}`)
201+
}
202+
console.log()
203+
}
204+
205+
if (needed.length) {
206+
console.log('## Still Needed\n')
207+
console.log('Removing these overrides would introduce new vulnerabilities:\n')
208+
for (const r of needed) {
209+
if (r.error) {
210+
console.log(`- \`${r.label}\` — ⚠️ error during check: ${r.error}`)
211+
} else {
212+
console.log(`- \`${r.label}\` → \`${r.val}\` — removing adds **+${r.newVulns}** vuln${r.newVulns !== 1 ? 's' : ''}`)
213+
}
214+
}
215+
}
216+
} else {
217+
const w = (results.length ? Math.max(...results.map(r => r.label.length)) : 0) + 2
218+
console.log('\nOverride Removal Report')
219+
console.log('='.repeat(60))
220+
for (const r of results) {
221+
const label = r.label.padEnd(w)
222+
if (r.error) {
223+
console.log(` ERROR ${label}${r.error}`)
224+
} else if (r.canRemove) {
225+
const note = r.newVulns < 0 ? ` (removing reduces vulns by ${-r.newVulns})` : 'no longer needed'
226+
console.log(` REMOVE ${label}${note}`)
227+
} else {
228+
console.log(` KEEP ${label}removing adds +${r.newVulns} vuln${r.newVulns !== 1 ? 's' : ''}`)
229+
}
230+
}
231+
console.log()
232+
}
233+
234+
process.exit(exitCode)

package.json

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@
126126
]
127127
},
128128
"overrides": {
129-
"rimraf": "^5.0.7",
130129
"tar": "^7.4.3",
131130
"@octokit/rest": "^20.0.2",
132131
"@tootallnate/once": "3.0.1",
@@ -144,8 +143,10 @@
144143
"posttest": "npm run lint",
145144
"lint": "eslint src test e2e",
146145
"gen-health": "node bin/gen-health-table.js",
147-
"prepack": "oclif manifest && oclif readme",
146+
"prepack": "npm run check-overrides && oclif manifest && oclif readme",
148147
"version": "oclif readme && git add README.md",
149-
"e2e": "jest --collectCoverage=false --testRegex './e2e/e2e.js'"
148+
"e2e": "jest --collectCoverage=false --testRegex './e2e/e2e.js'",
149+
"check-overrides": "node bin/check-overrides.js",
150+
"check-overrides:md": "node bin/check-overrides.js --markdown"
150151
}
151152
}

0 commit comments

Comments
 (0)