From bbf4d1cd906ab419545d298f210d2790f0943815 Mon Sep 17 00:00:00 2001 From: Shazron Abdullah <36107+shazron@users.noreply.github.com> Date: Fri, 8 May 2026 20:21:30 +0800 Subject: [PATCH 1/7] 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 --- bin/check-overrides.js | 176 +++++++++++++++++++++++++++++++++++++++++ package.json | 7 +- 2 files changed, 181 insertions(+), 2 deletions(-) create mode 100755 bin/check-overrides.js diff --git a/bin/check-overrides.js b/bin/check-overrides.js new file mode 100755 index 0000000..8d14bd9 --- /dev/null +++ b/bin/check-overrides.js @@ -0,0 +1,176 @@ +#!/usr/bin/env node + +/** + * Checks each npm override in package.json and reports whether it can be safely removed. + * + * For each override: temporarily removes it, re-resolves the lockfile (no actual install), + * runs npm audit, then restores the original files. + * + * Usage: node bin/check-overrides.js [--markdown] + */ + +const { spawnSync } = require('child_process') +const fs = require('fs') +const path = require('path') + +const ROOT = process.cwd() +const PKG_PATH = path.join(ROOT, 'package.json') +const LOCK_PATH = path.join(ROOT, 'package-lock.json') +const MARKDOWN = process.argv.includes('--markdown') + +// ── helpers ────────────────────────────────────────────────────────────────── + +function npm(...args) { + return spawnSync('npm', args, { cwd: ROOT, encoding: 'utf8' }) +} + +function auditVulnCount() { + const { stdout } = npm('audit', '--json') + try { + const { metadata: { vulnerabilities: v } } = JSON.parse(stdout) + return (v.critical || 0) + (v.high || 0) + (v.moderate || 0) + (v.low || 0) + } catch { + return 0 + } +} + +/** + * Flattens nested overrides into dot-path entries, e.g.: + * { "foo": "^1", "bar": { "baz": "^2" } } + * becomes: + * [ { dotPath: "foo", label: "foo" }, + * { dotPath: "bar.baz", label: "bar > baz" } ] + */ +function flattenOverrides(overrides, prefix = '') { + const entries = [] + for (const [key, val] of Object.entries(overrides)) { + const dotPath = prefix ? `${prefix}.${key}` : key + const label = prefix ? `${prefix} > ${key}` : key + if (val !== null && typeof val === 'object') { + entries.push(...flattenOverrides(val, dotPath)) + } else { + entries.push({ dotPath, label, val }) + } + } + return entries +} + +function deleteAtDotPath(obj, dotPath) { + const parts = dotPath.split('.') + let cur = obj + for (let i = 0; i < parts.length - 1; i++) { + cur = cur[parts[i]] + if (cur == null) return + } + delete cur[parts.at(-1)] + // prune empty parent objects + if (parts.length > 1) { + const parent = parts.slice(0, -1).reduce((o, k) => o[k], obj) + if (parent && Object.keys(parent).length === 0) { + deleteAtDotPath(obj, parts.slice(0, -1).join('.')) + } + } +} + +// ── main ───────────────────────────────────────────────────────────────────── + +const originalPkg = fs.readFileSync(PKG_PATH, 'utf8') +const originalLock = fs.readFileSync(LOCK_PATH, 'utf8') +const pkg = JSON.parse(originalPkg) +const overrides = pkg.overrides || {} + +if (Object.keys(overrides).length === 0) { + console.log('No overrides found in package.json.') + process.exit(0) +} + +const entries = flattenOverrides(overrides) +if (entries.length === 0) { + console.log('No scalar overrides found.') + process.exit(0) +} + +process.stderr.write('Checking baseline audit… ') +const baselineVulns = auditVulnCount() +process.stderr.write(`${baselineVulns} vulnerabilities\n\n`) + +const results = [] + +for (const entry of entries) { + process.stderr.write(` Checking "${entry.label}"… `) + + const testPkg = JSON.parse(originalPkg) + deleteAtDotPath(testPkg.overrides, entry.dotPath) + + fs.writeFileSync(PKG_PATH, JSON.stringify(testPkg, null, 2)) + + try { + const { status, stderr } = npm( + 'install', '--package-lock-only', '--ignore-scripts', '--no-audit', '--silent' + ) + if (status !== 0) { + results.push({ ...entry, canRemove: false, error: stderr?.trim() || 'npm install failed' }) + process.stderr.write('install failed\n') + continue + } + + const vulns = auditVulnCount() + const newVulns = vulns - baselineVulns + const canRemove = newVulns <= 0 + + results.push({ ...entry, canRemove, newVulns }) + process.stderr.write(canRemove ? 'safe to remove\n' : `still needed (+${newVulns} vuln${newVulns !== 1 ? 's' : ''})\n`) + } finally { + fs.writeFileSync(PKG_PATH, originalPkg) + fs.writeFileSync(LOCK_PATH, originalLock) + } +} + +// ── report ─────────────────────────────────────────────────────────────────── + +const removable = results.filter(r => r.canRemove) +const needed = results.filter(r => !r.canRemove) +const exitCode = removable.length > 0 ? 1 : 0 + +if (MARKDOWN) { + console.log('# Override Removal Report\n') + console.log(`Baseline: **${baselineVulns}** audit vulnerabilities with all overrides in place.\n`) + + if (removable.length) { + console.log('## Safe to Remove\n') + console.log('These overrides no longer affect the audit result and can be deleted from `package.json`:\n') + for (const r of removable) { + console.log(`- \`${r.label}\` → \`${r.val}\``) + } + console.log() + } + + if (needed.length) { + console.log('## Still Needed\n') + console.log('Removing these overrides would introduce new vulnerabilities:\n') + for (const r of needed) { + if (r.error) { + console.log(`- \`${r.label}\` — ⚠️ error during check: ${r.error}`) + } else { + console.log(`- \`${r.label}\` → \`${r.val}\` — removing adds **+${r.newVulns}** vuln${r.newVulns !== 1 ? 's' : ''}`) + } + } + } +} else { + const w = Math.max(...results.map(r => r.label.length)) + 2 + console.log('\nOverride Removal Report') + console.log('='.repeat(60)) + for (const r of results) { + const label = r.label.padEnd(w) + if (r.error) { + console.log(` ERROR ${label}${r.error}`) + } else if (r.canRemove) { + console.log(` REMOVE ${label}no longer needed`) + } else { + console.log(` KEEP ${label}removing adds +${r.newVulns} vuln${r.newVulns !== 1 ? 's' : ''}`) + } + } + console.log() +} + +process.exit(exitCode) diff --git a/package.json b/package.json index 5a19376..01f03c8 100644 --- a/package.json +++ b/package.json @@ -144,8 +144,11 @@ "posttest": "npm run lint", "lint": "eslint src test e2e", "gen-health": "node bin/gen-health-table.js", - "prepack": "oclif manifest && oclif readme", + "prepare": "npm run check-overrides", + "prepack": "npm run check-overrides && oclif manifest && oclif readme", "version": "oclif readme && git add README.md", - "e2e": "jest --collectCoverage=false --testRegex './e2e/e2e.js'" + "e2e": "jest --collectCoverage=false --testRegex './e2e/e2e.js'", + "check-overrides": "node bin/check-overrides.js", + "check-overrides:md": "node bin/check-overrides.js --markdown" } } From 39c34b430f30694fe3f0db37d3a954500fc86bc4 Mon Sep 17 00:00:00 2001 From: Shazron Abdullah <36107+shazron@users.noreply.github.com> Date: Fri, 8 May 2026 20:22:22 +0800 Subject: [PATCH 2/7] 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 --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 01f03c8..8396d8d 100644 --- a/package.json +++ b/package.json @@ -126,7 +126,6 @@ ] }, "overrides": { - "rimraf": "^5.0.7", "tar": "^7.4.3", "@octokit/rest": "^20.0.2", "@tootallnate/once": "3.0.1", From 07013935fb2d0755acd90bc293bc7036288e06af Mon Sep 17 00:00:00 2001 From: Shazron Abdullah <36107+shazron@users.noreply.github.com> Date: Fri, 8 May 2026 20:26:28 +0800 Subject: [PATCH 3/7] 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 --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 8396d8d..317c4e8 100644 --- a/package.json +++ b/package.json @@ -143,7 +143,6 @@ "posttest": "npm run lint", "lint": "eslint src test e2e", "gen-health": "node bin/gen-health-table.js", - "prepare": "npm run check-overrides", "prepack": "npm run check-overrides && oclif manifest && oclif readme", "version": "oclif readme && git add README.md", "e2e": "jest --collectCoverage=false --testRegex './e2e/e2e.js'", From 6841e1b6826ca48c915eb0388ffd3d6f090ab548 Mon Sep 17 00:00:00 2001 From: Shazron Abdullah <36107+shazron@users.noreply.github.com> Date: Fri, 8 May 2026 20:28:04 +0800 Subject: [PATCH 4/7] 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 --- bin/check-overrides.js | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/bin/check-overrides.js b/bin/check-overrides.js index 8d14bd9..e0241ba 100755 --- a/bin/check-overrides.js +++ b/bin/check-overrides.js @@ -25,13 +25,18 @@ function npm(...args) { } function auditVulnCount() { - const { stdout } = npm('audit', '--json') + const { stdout, stderr, status } = npm('audit', '--json') + let parsed try { - const { metadata: { vulnerabilities: v } } = JSON.parse(stdout) - return (v.critical || 0) + (v.high || 0) + (v.moderate || 0) + (v.low || 0) + parsed = JSON.parse(stdout) } catch { - return 0 + throw new Error(`npm audit returned non-JSON output (exit ${status}):\n${stderr || stdout || '(no output)'}`) } + if (!parsed?.metadata?.vulnerabilities) { + throw new Error(`npm audit JSON missing expected metadata.vulnerabilities field:\n${stdout}`) + } + const v = parsed.metadata.vulnerabilities + return (v.critical || 0) + (v.high || 0) + (v.moderate || 0) + (v.low || 0) } /** @@ -91,7 +96,14 @@ if (entries.length === 0) { } process.stderr.write('Checking baseline audit… ') -const baselineVulns = auditVulnCount() +let baselineVulns +try { + baselineVulns = auditVulnCount() +} catch (e) { + process.stderr.write('failed\n') + console.error(`Error: could not establish baseline — ${e.message}`) + process.exit(2) +} process.stderr.write(`${baselineVulns} vulnerabilities\n\n`) const results = [] @@ -114,7 +126,14 @@ for (const entry of entries) { continue } - const vulns = auditVulnCount() + let vulns + try { + vulns = auditVulnCount() + } catch (e) { + results.push({ ...entry, canRemove: false, error: `audit failed: ${e.message}` }) + process.stderr.write('audit failed\n') + continue + } const newVulns = vulns - baselineVulns const canRemove = newVulns <= 0 From a58f3dec63c39ff1deb561de24885bf749d3dbb2 Mon Sep 17 00:00:00 2001 From: Shazron Abdullah <36107+shazron@users.noreply.github.com> Date: Fri, 8 May 2026 20:30:56 +0800 Subject: [PATCH 5/7] 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 --- bin/check-overrides.js | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/bin/check-overrides.js b/bin/check-overrides.js index e0241ba..f562799 100755 --- a/bin/check-overrides.js +++ b/bin/check-overrides.js @@ -11,6 +11,7 @@ const { spawnSync } = require('child_process') const fs = require('fs') +const os = require('os') const path = require('path') const ROOT = process.cwd() @@ -20,12 +21,12 @@ const MARKDOWN = process.argv.includes('--markdown') // ── helpers ────────────────────────────────────────────────────────────────── -function npm(...args) { - return spawnSync('npm', args, { cwd: ROOT, encoding: 'utf8' }) +function npm(cwd, ...args) { + return spawnSync('npm', args, { cwd, encoding: 'utf8' }) } -function auditVulnCount() { - const { stdout, stderr, status } = npm('audit', '--json') +function auditVulnCount(cwd) { + const { stdout, stderr, status } = npm(cwd, 'audit', '--json') let parsed try { parsed = JSON.parse(stdout) @@ -80,7 +81,6 @@ function deleteAtDotPath(obj, dotPath) { // ── main ───────────────────────────────────────────────────────────────────── const originalPkg = fs.readFileSync(PKG_PATH, 'utf8') -const originalLock = fs.readFileSync(LOCK_PATH, 'utf8') const pkg = JSON.parse(originalPkg) const overrides = pkg.overrides || {} @@ -97,12 +97,17 @@ if (entries.length === 0) { process.stderr.write('Checking baseline audit… ') let baselineVulns +const baselineDir = fs.mkdtempSync(path.join(os.tmpdir(), 'check-overrides-baseline-')) try { - baselineVulns = auditVulnCount() + fs.writeFileSync(path.join(baselineDir, 'package.json'), originalPkg) + fs.copyFileSync(LOCK_PATH, path.join(baselineDir, 'package-lock.json')) + baselineVulns = auditVulnCount(baselineDir) } catch (e) { process.stderr.write('failed\n') console.error(`Error: could not establish baseline — ${e.message}`) process.exit(2) +} finally { + fs.rmSync(baselineDir, { recursive: true, force: true }) } process.stderr.write(`${baselineVulns} vulnerabilities\n\n`) @@ -111,14 +116,15 @@ const results = [] for (const entry of entries) { process.stderr.write(` Checking "${entry.label}"… `) - const testPkg = JSON.parse(originalPkg) - deleteAtDotPath(testPkg.overrides, entry.dotPath) - - fs.writeFileSync(PKG_PATH, JSON.stringify(testPkg, null, 2)) - + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'check-overrides-')) try { + const testPkg = JSON.parse(originalPkg) + deleteAtDotPath(testPkg.overrides, entry.dotPath) + fs.writeFileSync(path.join(tmpDir, 'package.json'), JSON.stringify(testPkg, null, 2)) + fs.copyFileSync(LOCK_PATH, path.join(tmpDir, 'package-lock.json')) + const { status, stderr } = npm( - 'install', '--package-lock-only', '--ignore-scripts', '--no-audit', '--silent' + tmpDir, 'install', '--package-lock-only', '--ignore-scripts', '--no-audit', '--silent' ) if (status !== 0) { results.push({ ...entry, canRemove: false, error: stderr?.trim() || 'npm install failed' }) @@ -128,7 +134,7 @@ for (const entry of entries) { let vulns try { - vulns = auditVulnCount() + vulns = auditVulnCount(tmpDir) } catch (e) { results.push({ ...entry, canRemove: false, error: `audit failed: ${e.message}` }) process.stderr.write('audit failed\n') @@ -140,8 +146,7 @@ for (const entry of entries) { results.push({ ...entry, canRemove, newVulns }) process.stderr.write(canRemove ? 'safe to remove\n' : `still needed (+${newVulns} vuln${newVulns !== 1 ? 's' : ''})\n`) } finally { - fs.writeFileSync(PKG_PATH, originalPkg) - fs.writeFileSync(LOCK_PATH, originalLock) + fs.rmSync(tmpDir, { recursive: true, force: true }) } } From 7d10d6e7a909dad61105722e3b4992394372f7f2 Mon Sep 17 00:00:00 2001 From: Shazron Abdullah <36107+shazron@users.noreply.github.com> Date: Fri, 8 May 2026 20:33:35 +0800 Subject: [PATCH 6/7] 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 --- bin/check-overrides.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/bin/check-overrides.js b/bin/check-overrides.js index f562799..fa5dcf2 100755 --- a/bin/check-overrides.js +++ b/bin/check-overrides.js @@ -113,10 +113,26 @@ process.stderr.write(`${baselineVulns} vulnerabilities\n\n`) const results = [] +// ── signal handlers ────────────────────────────────────────────────────────── + +const activeTmpDirs = new Set() + +function cleanupTmpDirs() { + for (const dir of activeTmpDirs) { + try { fs.rmSync(dir, { recursive: true, force: true }) } catch {} + } +} + +process.on('SIGINT', () => { cleanupTmpDirs(); process.exit(130) }) +process.on('SIGTERM', () => { cleanupTmpDirs(); process.exit(143) }) + +// ───────────────────────────────────────────────────────────────────────────── + for (const entry of entries) { process.stderr.write(` Checking "${entry.label}"… `) const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'check-overrides-')) + activeTmpDirs.add(tmpDir) try { const testPkg = JSON.parse(originalPkg) deleteAtDotPath(testPkg.overrides, entry.dotPath) @@ -147,6 +163,7 @@ for (const entry of entries) { process.stderr.write(canRemove ? 'safe to remove\n' : `still needed (+${newVulns} vuln${newVulns !== 1 ? 's' : ''})\n`) } finally { fs.rmSync(tmpDir, { recursive: true, force: true }) + activeTmpDirs.delete(tmpDir) } } @@ -181,7 +198,7 @@ if (MARKDOWN) { } } } else { - const w = Math.max(...results.map(r => r.label.length)) + 2 + const w = (results.length ? Math.max(...results.map(r => r.label.length)) : 0) + 2 console.log('\nOverride Removal Report') console.log('='.repeat(60)) for (const r of results) { From d10f461aacc4e48ec0bd61d488d7fdc14e36e5ed Mon Sep 17 00:00:00 2001 From: Shazron Abdullah <36107+shazron@users.noreply.github.com> Date: Fri, 8 May 2026 20:38:07 +0800 Subject: [PATCH 7/7] 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 --- bin/check-overrides.js | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/bin/check-overrides.js b/bin/check-overrides.js index fa5dcf2..57eefcf 100755 --- a/bin/check-overrides.js +++ b/bin/check-overrides.js @@ -4,9 +4,15 @@ * Checks each npm override in package.json and reports whether it can be safely removed. * * For each override: temporarily removes it, re-resolves the lockfile (no actual install), - * runs npm audit, then restores the original files. + * runs npm audit, then cleans up. Original files are never modified. * - * Usage: node bin/check-overrides.js [--markdown] + * Exits 0 if all overrides are still needed, 1 if any can be removed. + * Runs as part of prepack to prevent publishing with stale overrides. + * + * Note: npm audit makes network requests to the registry. In network-restricted + * environments pass --prefer-offline to use only locally cached advisory data. + * + * Usage: node bin/check-overrides.js [--markdown] [--prefer-offline] */ const { spawnSync } = require('child_process') @@ -18,6 +24,7 @@ const ROOT = process.cwd() const PKG_PATH = path.join(ROOT, 'package.json') const LOCK_PATH = path.join(ROOT, 'package-lock.json') const MARKDOWN = process.argv.includes('--markdown') +const PREFER_OFFLINE = process.argv.includes('--prefer-offline') // ── helpers ────────────────────────────────────────────────────────────────── @@ -26,7 +33,9 @@ function npm(cwd, ...args) { } function auditVulnCount(cwd) { - const { stdout, stderr, status } = npm(cwd, 'audit', '--json') + const args = ['audit', '--json'] + if (PREFER_OFFLINE) args.push('--prefer-offline') + const { stdout, stderr, status } = npm(cwd, ...args) let parsed try { parsed = JSON.parse(stdout) @@ -143,7 +152,8 @@ for (const entry of entries) { tmpDir, 'install', '--package-lock-only', '--ignore-scripts', '--no-audit', '--silent' ) if (status !== 0) { - results.push({ ...entry, canRemove: false, error: stderr?.trim() || 'npm install failed' }) + // stderr may be null if the process was killed by a signal + results.push({ ...entry, canRemove: false, error: (stderr ?? '').trim() || 'npm install failed' }) process.stderr.write('install failed\n') continue } @@ -160,7 +170,12 @@ for (const entry of entries) { const canRemove = newVulns <= 0 results.push({ ...entry, canRemove, newVulns }) - process.stderr.write(canRemove ? 'safe to remove\n' : `still needed (+${newVulns} vuln${newVulns !== 1 ? 's' : ''})\n`) + if (canRemove) { + const note = newVulns < 0 ? ` (removing reduces vulns by ${-newVulns})` : '' + process.stderr.write(`safe to remove${note}\n`) + } else { + process.stderr.write(`still needed (+${newVulns} vuln${newVulns !== 1 ? 's' : ''})\n`) + } } finally { fs.rmSync(tmpDir, { recursive: true, force: true }) activeTmpDirs.delete(tmpDir) @@ -181,7 +196,8 @@ if (MARKDOWN) { console.log('## Safe to Remove\n') console.log('These overrides no longer affect the audit result and can be deleted from `package.json`:\n') for (const r of removable) { - console.log(`- \`${r.label}\` → \`${r.val}\``) + const note = r.newVulns < 0 ? ` _(removing this actually reduces vulns by ${-r.newVulns})_` : '' + console.log(`- \`${r.label}\` → \`${r.val}\`${note}`) } console.log() } @@ -206,7 +222,8 @@ if (MARKDOWN) { if (r.error) { console.log(` ERROR ${label}${r.error}`) } else if (r.canRemove) { - console.log(` REMOVE ${label}no longer needed`) + const note = r.newVulns < 0 ? ` (removing reduces vulns by ${-r.newVulns})` : 'no longer needed' + console.log(` REMOVE ${label}${note}`) } else { console.log(` KEEP ${label}removing adds +${r.newVulns} vuln${r.newVulns !== 1 ? 's' : ''}`) }