Skip to content

fix(deps): clear npm audit advisories for minimatch and glob - #572

Merged
donpetry-bot merged 1 commit into
mainfrom
fix/npm-audit-glob-minimatch
Nov 5, 2026
Merged

donpetry-bot merged 1 commit into
mainfrom
fix/npm-audit-glob-minimatch

Conversation

@don-petry

@don-petry don-petry commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

dependency-audit / npm audit fails on main (run 41744902323). This blocks rolling out combsmith/.github#1470, which makes dependency-audit / Audit gate a required check. All 24 findings come from two root advisories in dev dependencies only:

Advisory Vulnerable Fixed Reached via
GHSA-g3hv-75wg-m249 minimatch globparts DoS >=7.0.0 <11.0.1 11.0.1 @electron-forge/* → @electron/packager, @electron/universal → @electron/asar; @typescript-eslint/typescript-estree; eslint-plugin-import-x; @stryker-mutator/core
GHSA-3g3m-2fjr-6pw4 glob AbortSignal bypass >=9.0.0 <=14.0.1 14.0.2 @electron-forge/* → @electron/asar 4

No parent has a release on the fixed majors yet: @electron/asar 4.3.1 still wants glob ^13 and minimatch ^10. The same holds for typescript-eslint 8.72.1, stryker 10.1.1 and import-x 4.17.1. No open Dependabot PR covers these.

Probably also covers #568 (OpenSSF Scorecard: "2 existing vulnerabilities"). Unconfirmed until the next Scorecard scan, so this PR does not auto-close it.

Changes

  • Range-scoped overrides in package.json: "minimatch@>=7.0.0 <11.0.1": "^11.0.1" and "glob@>=9.0.0 <14.0.2": "^14.0.2". They replace only the vulnerable copies. ESLint core keeps minimatch 3, and @electron/asar 3 keeps glob 7. Those versions are outside both advisories and would break on the new majors.
  • eslint-plugin-import-x → eslint-plugin-import@^2.32.0. import-x 4.17.1 does import { makeRe } from 'minimatch', and minimatch 11 removed makeRe, so ESLint failed to load the config. The upstream fix is fix: support minimatch v9/v10/v11 and drop makeRe un-ts/eslint-plugin-import-x#570, still unreleased. eslint-plugin-import is the plugin import-x forked from. It uses minimatch 3, which is not affected. The config only uses order and no-duplicates, and their options are unchanged. Only the import-x/ prefix becomes import/.

Testing

Local, clean npm ci --legacy-peer-deps:

  • npm audit --audit-level=low: 0 vulnerabilities (was 24 moderate)
  • npm run typecheck, npm run lint, npm run format:check: pass
  • npm run test:coverage: 80 files, 819 tests pass
  • npm run package and npm run make (darwin): pass
  • npx stryker run --dryRunOnly: passes. In CI, the full mutation job (stryker, which uses minimatch 11) passes on this head.
  • Same lint behaviour as import-x, with no resolver configured: both plugins ran on a main checkout and on this branch. In every src file (100 files) the import statements were reversed, and a probe added mixed builtin, external, @shared/* alias, relative and import type imports. Both plugins report the same 303 order/no-duplicates errors on the same files and lines, with identical text. The current tree is clean under both.
  • @electron/asar: the forge config doesn't enable asar, so make doesn't run the overridden asar code. I smoke-tested @electron/asar 4.3.1 with glob 14.0.3 and minimatch 11.0.4 directly: createPackageWithOptions with unpack: '*.node' and unpackDir packs and unpacks correctly.
  • CI on this head is green on ubuntu, macOS and Windows, including dependency-audit / Audit gate, mutation and e2e.

Risk

Low. Nothing shipped in the app changes: every touched package is a dev or build-time dependency, and dependencies is untouched. The residual risks:

  • Major-version overrides (minimatch 10→11, glob 13→14, path-scurry 2→3) for tools that declare the older ranges. minimatch 11 needs Node 20 or later, and CI and local builds use Node 24 or LTS. If packaging ever enables asar, the asar glob/minimatch path is covered only by the smoke test above.
  • New transitive package @isaacs/nonrecursive-glob-match (minimatch 11's matcher, same maintainer). It is pinned with an integrity hash in the lockfile.
  • Plugin swap: rule behaviour was checked as equal on this codebase (see Testing). Removing import-x also drops unrs-resolver and its native binding packages, which had an install script.

Rollback

Revert this PR. That restores package.json, package-lock.json and eslint.config.mjs, and no data or runtime state is involved. Reverting brings back the npm audit failure, so the Audit gate would block merges again.

Monitoring

  • dependency-audit / Audit gate and npm audit on main after merge. A new advisory in the overridden ranges would show up there.
  • quality, mutation and e2e on later PRs. A minimatch 11 or glob 14 incompatibility in eslint, stryker or forge would fail those jobs.
  • Removing the overrides: once @electron/asar, typescript-eslint and stryker accept minimatch ≥11.0.1 / glob ≥14.0.2, Dependabot bumps plus npm ls minimatch glob will show the overrides are no longer needed. Delete them then. Switching back to import-x after fix: support minimatch v9/v10/v11 and drop makeRe un-ts/eslint-plugin-import-x#570 ships is optional.

🤖 Generated with Claude Code

npm audit on main fails on two root advisories, reached through
@electron-forge/* (via @electron/packager and @electron/universal ->
@electron/asar), @typescript-eslint/*, eslint-plugin-import-x and
@stryker-mutator/core:

- GHSA-g3hv-75wg-m249 minimatch >=7.0.0 <11.0.1 (fixed in 11.0.1)
- GHSA-3g3m-2fjr-6pw4 glob >=9.0.0 <=14.0.1 (fixed in 14.0.2)

No parent has released a version that takes the fixed majors, so add
range-scoped overrides. They only replace the vulnerable 7.x-10.x
minimatch and 9.x-13.x glob, and leave the minimatch 3 / glob 7 that
eslint and @electron/asar 3 use alone.

eslint-plugin-import-x imports makeRe from minimatch, which minimatch 11
removed, so ESLint failed to load the config. Its fix
(un-ts/eslint-plugin-import-x#570) is unreleased, so switch to
eslint-plugin-import 2.32, the plugin import-x forked from. The two
rules the config uses (order, no-duplicates) keep the same options.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@don-petry
don-petry requested a review from a team as a code owner November 4, 2026 21:26
@don-petry don-petry added the dev-lead:hands-off Opt an item out of the dev-lead persona automation entirely label Nov 4, 2026
@don-petry

Copy link
Copy Markdown
Contributor Author

@donpetry-bot please review

@chatgpt-codex-connector

This comment has been minimized.

@coderabbitai

This comment has been minimized.

@don-petry

Copy link
Copy Markdown
Contributor Author

dev-lead is withholding action on this item.

It is labeled dev-lead:hands-off (deliberately kept off-limits to dev-lead (e.g. changes to the dev-lead automation itself)), so dev-lead will not pick it up while that label is present. This notice is posted once so the withhold is visible rather than looking like a stalled run.

To re-enable automated pickup: remove the dev-lead:hands-off label.

@donpetry-bot

Copy link
Copy Markdown
Contributor

@don-petry I'm on it — starting a fresh review now. Results will appear in a few minutes.

@sonarqubecloud

This comment has been minimized.

@donpetry-bot

donpetry-bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at 3647bdc3409094540b97b63cfba5c039a4543d63 — click to expand prior review.

Review — fix requested (cycle 1/3)

The automated review identified the following issues. Please address each one:

Findings to fix

Automated review — NEEDS HUMAN REVIEW

Risk: MEDIUM
Reviewed commit: 3647bdc3409094540b97b63cfba5c039a4543d63
Cascade: triage → deep+duck (triage: haiku 4.5 [sonnet 5.5, sonnet 5] → deep: opus 5.5 [opus 4.8, sonnet 5.5] + duck: sonnet 5.5 → audit: opus 5.5 [opus 4.8, opus 4.7])

Summary

Both reviewers rate the risk MEDIUM. The PR only changes dev dependencies, using range-scoped minimatch and glob overrides plus an eslint-plugin-import-x to eslint-plugin-import swap, and the audit and security gates pass. They differ on the decision: the deep reviewer escalates because the description lacks risk, rollback and monitoring sections and the mutation job was still running, while the rubber duck approves. Both flagged the in-progress mutation check, which is the strongest signal. Only the rubber duck pointed out that no eslint-plugin-import resolver is configured, so import/order may group aliased or TS imports differently, and that the overrides will need a tracking issue for removal once upstream parents release.

Cross-engine agreement

partial

Findings

  • major: The safety checks flag 3 of 5 required description sections as missing: risk, rollback and monitoring. Rollback is simple (revert the commit and lockfile; only dev dependencies and lint config are touched). The author should add these sections.
  • major: The mutation check was IN_PROGRESS at head 3647bdc. Stryker uses minimatch, which the override changes, so wait for it to finish green before merging. The rubber duck could not confirm whether it is a required check. The PR description says only stryker run --dryRunOnly was exercised locally.
  • minor: The override pulls in a new transitive package, @isaacs/nonrecursive-glob-match@1.0.3, as a minimatch 11.0.4 dependency. The scope matches the minimatch maintainer, and the lockfile entry has a registry URL and a sha512 integrity hash. Unconfirmed: no registry access to check provenance or that the hash matches the published tarball. (package-lock.json:2492)
  • minor: The lockfile pulls in minimatch 11, glob 14 and path-scurry 3, all dev-only with pinned integrity hashes. minimatch 11 declares node 20 || >=22, while its glob 14.0.3 dependents declare 18 || 20 || >=22. This only matters on Node 18. CI passed on all three OSes. (package-lock.json:1269)
  • minor: The overrides force major-version jumps on dev-tool consumers: minimatch 10 to 11, glob 13 to 14 and path-scurry 2 to 3. minimatch 11 drops Node 18. The forge config does not enable asar, so CI never runs the overridden @electron/asar code path, and that path rests only on the author's manual smoke test. These are dev dependencies only. (package.json:88)
  • minor: The overrides apply to every transitive consumer. The ranges (minimatch@>=7.0.0 <11.0.1, glob@>=9.0.0 <14.0.2) will need removal once upstream parents release, so a tracking issue would help. If packaging ever enables asar, the glob 14 / minimatch 11 path would be untested in CI. (package.json:87)
  • minor: eslint-plugin-import 2.32 does not use import-x's bundled resolver, and eslint.config.mjs sets no settings['import/resolver']. import/order may group .ts/.tsx and path-aliased imports differently from before and could pass imports it used to flag. The PR's probe covered only out-of-order and duplicate imports. Unconfirmed, since lint was not run. Consider a probe with mixed aliased, relative and external imports, or an explicit resolver setting. (eslint.config.mjs:64)
  • info: The import-x/ prefix is now import/, and the order and no-duplicates options are unchanged. Lint passes, and the author's probe confirms both rules still fire. This is not a security issue. (eslint.config.mjs:29)
  • info: Net reduction in install-time attack surface: removing eslint-plugin-import-x also drops unrs-resolver (which had an install script) and its 19 optional native binding packages. The new eslint-plugin-import dependencies are at patched versions.
  • info: The 4 'unpinned' ranges flagged by the safety checks are caret ranges pinned by the committed lockfile (minimatch 11.0.4, glob 14.0.3, eslint-plugin-import 2.32.0). The postcss-nested entry already existed; only a trailing comma changed. Not blocking. (package.json:66)
  • info: The run_secret_scanning MCP tool was unavailable in this read-only environment, so it was not run. The CI gitleaks scan passed and the diff contains no credential-like content.
  • info: No issue is linked. CodeRabbit suggests Scorecard: Vulnerabilities (8/10) #568 is related. It could not be read, so whether this PR covers it is unconfirmed.

Reviewed by the PR-review cascade (triage: haiku 4.5 [sonnet 5.5, sonnet 5] → deep: opus 5.5 [opus 4.8, sonnet 5.5] + duck: sonnet 5.5 → audit: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.

Additional tasks

  1. Resolve all unresolved review thread comments from other reviewers
  2. Ensure all CI checks pass after your changes
  3. Rebase on the target branch if behind
  4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed, or after a change to the PR body, labels, or linked issues.

@donpetry-bot
donpetry-bot merged commit d494bac into main Nov 5, 2026
42 checks passed
@donpetry-bot
donpetry-bot deleted the fix/npm-audit-glob-minimatch branch November 5, 2026 22:54
@don-petry

Copy link
Copy Markdown
Contributor Author

Response to the cycle 1/3 review at 3647bdc. No code change was needed. The PR description has been updated.

Finding Disposition
major: description missing risk, rollback and monitoring sections Fixed. Added ## Risk, ## Rollback and ## Monitoring sections, and renamed Verification to ## Testing.
major: mutation was in progress at 3647bdc Resolved. mutation finished SUCCESS on 3647bdc, as did the whole CI rollup, including dependency-audit / Audit gate, e2e and quality on all three OSes.
minor: no import/resolver set, so import/order might group differently Checked: no difference. I ran import-x on a main checkout and eslint-plugin-import on this branch. In all 100 src files the import statements were reversed, and a probe added builtin, external, @shared/* alias, relative and import type imports. Both plugins report the same 303 order/no-duplicates errors on the same files and lines, with identical text. Bare specifiers fall back to external-looking-name classification, and this codebase has no @shared/* imports, so no resolver is needed.
minor: new transitive package @isaacs/nonrecursive-glob-match Accepted. It is minimatch 11's own matcher, from the same maintainer, and the lockfile pins it with a sha512 integrity hash. Noted under Risk.
minor: minimatch 11 needs Node ≥20 Accepted. CI uses Node LTS (24) and there is no Node 18 target. Noted under Risk.
minor: major-version overrides; asar path not exercised by CI Accepted. asar is off in the forge config, and a direct @electron/asar 4.3.1 smoke test with glob 14 and minimatch 11 passed. Noted under Risk.
minor: overrides need removing later Added the removal trigger and check under ## Monitoring.
info: #568 Referenced as probably covered (Scorecard "2 existing vulnerabilities" matches the two root advisories). It is not closed automatically until a Scorecard re-scan confirms.

@donpetry-bot please review

@donpetry-bot

Copy link
Copy Markdown
Contributor

@don-petry I'm on it — starting a fresh review now. Results will appear in a few minutes.

@donpetry-bot donpetry-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: 3647bdc3409094540b97b63cfba5c039a4543d63
Review mode: triage-approved (single reviewer)

Summary

This PR clears the two root npm audit advisories, GHSA-g3hv-75wg-m249 (minimatch) and GHSA-3g3m-2fjr-6pw4 (glob). It does this with range-scoped overrides (minimatch@>=7.0.0 <11.0.1 → ^11.0.1, glob@>=9.0.0 <14.0.2 → ^14.0.2). It also replaces eslint-plugin-import-x with eslint-plugin-import@^2.32.0, because import-x 4.x imports makeRe, which minimatch 11 removed. Every touched package is a dev or build-time dependency. dependencies and app code are unchanged.

The head commit is the same one reviewed in cycle 1 (3647bdc). This re-review was triggered by the author's description update and their reply to each finding. I rate the risk MEDIUM rather than the triage's LOW, because the PR swaps a dev dependency and forces major-version overrides. These are all dev-only and covered by green CI, so they do not block approval.

Cycle-1 findings:

  • major: description missing Risk, Rollback and Monitoring sections. Resolved. The PR body now has ## Risk, ## Rollback and ## Monitoring sections, and the safety checks report DESCRIPTION_MISSING: 0.
  • major: mutation still running at the head. Resolved. mutation finished with SUCCESS on 3647bdc (completed 2026-11-04T21:43:41Z).
  • minor: no import/resolver setting, so import/order might group imports differently. The author answered this. They ran both plugins against reversed imports in all 100 src files plus a probe with builtin, external, @shared/* alias, relative and import type imports. Both plugins reported the same 303 errors with identical text. I could not run lint myself to check this, so it rests on the author's report. CI quality (lint included) is green on all three OSes.
  • minor: overrides need removing later; asar path not exercised in CI; minimatch 11 needs Node 20+; new @isaacs/nonrecursive-glob-match package. The author accepted these, and they are now documented under Risk and Monitoring, including when and how to remove the overrides. None of them blocks.

Linked issue analysis

No closing issue is linked. The PR mentions #568 (OpenSSF Scorecard "2 existing vulnerabilities") as probably covered, without a closing keyword. That is the right call, because it stays open until a Scorecard re-scan confirms. The description states the motivation fully: dependency-audit / npm audit fails on main and blocks combsmith/.github#1470. The change addresses it, and dependency-audit / npm audit and Audit gate both pass on this head. I could not read #568 or #1470 directly.

Findings

  • info: The override keys are range-scoped, so minimatch 3 (ESLint core, eslint-plugin-import) and glob 7 (@electron/asar 3) keep their versions. The lockfile matches this. Every rewritten minimatch copy (@electron/packager, @electron/universal, @stryker-mutator/core, @typescript-eslint/typescript-estree) moves from 10.x to 11.0.4, and the glob copies move from 13.0.6 to 14.0.3. Entries have sha512 integrity hashes and registry.npmjs.org URLs. (package.json:88)
  • info: The eslint.config.mjs change is only a rename (import-x/ → import/). The order and no-duplicates options are unchanged. (eslint.config.mjs:64)
  • info: The new transitive dependencies of eslint-plugin-import are at patched versions where it matters: json5@1.0.2 (CVE-2022-46175 fix) and semver@6.3.2. Removing unrs-resolver drops a package with an install script and its 19 optional native bindings, which reduces install-time attack surface.
  • info: The safety checks flag 4 unpinned ranges. These are caret ranges locked by the committed lockfile. The postcss-nested entry already existed and only gained a trailing comma. Not blocking. (package.json:66)
  • info: The run_secret_scanning MCP tool is not available in this read-only environment, so it was not run. The CI gitleaks scan passed, and the diff has no credential-like content.
  • info: Review-thread resolution state is not in the pre-fed metadata. All bot summary comments are minimized as resolved, the only formal review is an APPROVED, and no human-reviewer questions are open.

CI status

All checks completed with SUCCESS or SKIPPED, none failed or pending. Results:

  • quality passes on ubuntu, macOS and Windows.
  • mutation, e2e, Secret scan (gitleaks) and Workflow regression guards pass.
  • dependency-audit / npm audit and dependency-audit / Audit gate pass.
  • CodeQL (actions, javascript-typescript, python), SonarCloud (Quality Gate passed, 0 new issues, 0 hotspots), CodeRabbit (no actionable comments) and AgentShield pass.

Skipped jobs are the ecosystem audits that don't apply here (pnpm, cargo, govulncheck, pip-audit) and automation jobs held by the dev-lead:hands-off label.


Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.

@donpetry-bot

Copy link
Copy Markdown
Contributor

pr-review approved on PARTIAL advisory evidence: 2/5 required advisory bots reported before the gate's head-age-timeout fallback proceeded. Recorded for the miss-rate metric (#1596).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev-lead:hands-off Opt an item out of the dev-lead persona automation entirely

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants