You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(deps): override glob and minimatch to clear npm audit - #667
dependency-audit / npm audit fails on main (run 41727967767), so the Audit gate that combsmith/.github#1470 makes required would block every merge here. This PR clears both root advisories with npm overrides.
babel-plugin-istanbul → test-exclude@6, which uses the glob 7 / minimatch 3 default-export APIs
"test-exclude": "^7.0.2", which uses the named { glob } / { minimatch } exports that glob 14 / minimatch 11 provide
Why overrides: no parent has released a fix. The latest jest (30.5.2) still requires glob ^13.0.6, and the latest eslint (10.14.0) and typescript-eslint (8.72.1) still require minimatch ^10. Drop these overrides once upstream catches up.
Why plain top-level overrides: CI runs npm 10.8.2, which rejected the first version's ranged keys ("glob@>=9.0.0 <14.0.2") in npm ci. Parent-scoped overrides behave differently in npm 10 and npm 11, because npm 11 also applies them to test-exclude's glob. The lockfile was regenerated with npm 10.8.2 from main's lockfile, re-resolving only the glob, minimatch and test-exclude entries.
Verification (local)
npm ci, npm ls glob minimatch test-exclude and npm audit pass under both npm 10.8.2 and npm 11.11.0. Audit reports 0 vulnerabilities (17 moderate on main).
Node 20.20.2: jest --coverage passes 22 suites and 1090 tests, and scripts/check-coverage.js passes.
eslint src: 0 errors and 22 warnings, the same as main's CI. tsc --noEmit, prettier --check . and check:pii pass.
Playwright was not run locally (no browser installed); CI covers it.
Risk
Low. The change is dev-only: package.json overrides plus the lockfile, with no runtime or workflow changes. The main risk is a behaviour change from forcing glob 14 / minimatch 11 on jest, eslint and test-exclude. Two checks cover it: eslint still lints all 28 tracked src files with the same 22 warnings, and coverage, which depends on test-exclude, still reports the same totals. The new packages (glob 14.0.3, minimatch 11.0.4, path-scurry 3.0.0, lru-cache 11.5.3, @isaacs/nonrecursive-glob-match 1.0.3) are all published by isaacs, the glob/minimatch maintainer, and npm audit signatures verifies all 453 packages. minimatch 11 and lru-cache 11 require Node 20 || >=22. Every workflow pins Node 20, and the repo declares no engines field.
Rollback
Revert this PR. That restores the previous overrides and lockfile, and the npm audit advisories come back.
Monitoring
Watch for jest file discovery, coverage collection or eslint file matching changing on later PRs. A drop in the number of tests or linted files would be the signal. Track removing the overrides in #668.
Updated package version resolution. No user-facing changes are included in this update, and the app’s visible features and behavior remain unchanged. These maintenance updates affect how package versions are selected during project setup and do not change the experience available in the product.
npm audit on main fails with two moderate advisories:
- GHSA-3g3m-2fjr-6pw4 (glob >=9.0.0 <=14.0.1), via jest's
@jest/reporters, jest-config and jest-runtime
- GHSA-g3hv-75wg-m249 (minimatch >=7.0.0 <11.0.1), via glob, eslint,
@eslint/config-array and @typescript-eslint/typescript-estree
No upstream release fixes either yet: jest 30.5.2 still depends on
glob ^13.0.6 and eslint 10.14.0 / typescript-eslint 8.72.1 still depend
on minimatch ^10. Add ranged overrides so only the vulnerable versions
are lifted (glob -> ^14.0.2, minimatch -> ^11.0.1); the glob 7 /
minimatch 3 pair under test-exclude is outside both ranges and stays.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
combsmith/google-app-scripts#666: Changes the same dependency overrides for glob and minimatch, with different version constraints.
Suggested reviewers:donpetry-bot
Merge Risk: Minimal · up to 2a4d8
No actionable merge-blocking defect is established; the dependency overrides appear compatible with the repository’s Node 20 workflows.
Pre-merge checks: 4
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title names the main change: dependency overrides for glob and minimatch to clear npm audit findings. The raw summary confirms package.json adds overrides for glob, minimatch, and test-exclude. Th…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title names the main change: dependency overrides for glob and minimatch to clear npm audit findings. The raw summary confirms package.json adds overrides for glob, minimatch, and test-exclude. The objectives describe the same goal. The title is conventional and specific, so it passes. It does not mention test-exclude, but that is a supporting override, not the main point.
Finishing Touches
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Reviewing files that changed from the base of the PR and between 52682ae and 2a4d833.
Files ignored due to path filters: 1
package-lock.json is excluded by !**/package-lock.json
Files selected for processing: 1
package.json
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
Superseded by automated re-review at 2a4d833103aab9b93abdeaff414fbce4617e9c54 — 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:52682aef9e1a64a2de3c93b0cba6f7a2693c7527 Cascade: triage → audit (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
The change only touches dev dependencies (two scoped npm overrides and the lockfile). It clears both advisories, and I found no security problem in it: no workflow, code or secret changes, and every new package comes from registry.npmjs.org with an sha512 integrity hash. It still cannot be approved. The required checks build-and-test, coverage and SonarCloud all failed on this head SHA within about 10 to 12 seconds. That timing points to an install or lockfile failure that this PR may have caused, and I could not read the job logs to confirm the cause.
Findings
major (ci): Required checks build-and-test (CI), coverage (Coverage) and SonarCloud failed on 52682ae. Each ran for only 10 to 12 seconds, and so did the non-required Node.js Tests and Playwright UI Tests. Five workflows failing that fast suggests one shared early cause, most likely npm ci rejecting the lockfile. Unconfirmed possibilities: (a) the lockfile came from npm 11 on Node 24, as the PR description says, while CI runs Node 20 / npm 10, and the two npm versions may treat range-keyed overrides (glob@>=9.0.0 <14.0.2) differently when checking that package.json and the lockfile are in sync; (b) glob 14.0.3 declares minipass ^7.1.3, but the diff leaves node_modules/minipass unchanged. If that entry is still 7.1.2 the lockfile is invalid. I could not read the job logs or the full lockfile, so I could not confirm either. The dependency-audit job passing does not rule this out, because npm audit reads the lockfile without installing it. Before merge, read the logs and either fix the cause or show that the same failure happens on main.
minor (supply-chain, package-lock.json:1299): The new transitive package @isaacs/nonrecursive-glob-match@1.0.3 is declared as a dependency in the minimatch 11.0.4 lock entry. It resolves from the canonical scoped-tarball URL on registry.npmjs.org and carries an sha512 integrity hash. Its scope, @isaacs, matches the maintainer of glob and minimatch (both funding URLs point to github.com/sponsors/isaacs). An npm scope belongs to a single owner, so typosquatting is unlikely. I could not check its publish date, provenance attestation or advisory history without network access. A quick npm view @isaacs/nonrecursive-glob-match / npm audit signatures check is advisable. The same goes for path-scurry 3.0.0 and lru-cache 11.5.3.
minor (compatibility, package.json:22): The overrides force major versions the parent packages don't declare. jest 30's @jest/reporters, jest-config and jest-runtime get glob 14.0.3 instead of ^10/^13. eslint, @eslint/config-array and @typescript-eslint/typescript-estree get minimatch 11.0.4 instead of ^9/^10. The PR author reports local Node 24 runs where 1090 tests pass, lint finds the same 22 warnings across all 28 files, and glob.sync is still exported. I could not verify any of this, and CI has not confirmed it yet. A behaviour change in minimatch 11 could make eslint silently lint fewer files.
minor (compatibility, package-lock.json:5462): Node 18 support is dropped. The previous top-level minimatch 10.2.6 allowed Node 18. minimatch 11.0.4 and lru-cache 11.5.3 declare engines 20 || >=22. Node 18 is past end of life, and the PR says CI uses Node 20. I did not see package.json engines or the other workflows' node-version settings, so I could not confirm nothing still runs on Node 18.
minor (description): The PR description has no risk, rollback or monitoring sections (DESCRIPTION_MISSING: 3), and no issue is linked. Rollback is a plain revert of the two overrides plus the lockfile. Add the three short sections, and add a tracking issue for removing the overrides once jest and eslint/typescript-eslint ship patched ranges.
info (dependency-risk, package.json:23): The override keys are limited to the vulnerable ranges, which is good practice: glob 7 and minimatch 3 under test-exclude stay as they are. The targets (^14.0.2, ^11.0.1) are caret ranges, but the lockfile pins the resolved versions, so the risk is low.
info (security): I found no security regression. Only package.json and package-lock.json change, and all packages are dev-only (dev: true) with registry.npmjs.org sources and sha512 integrity. There are no git or tarball URL sources, no install scripts in the added entries, no workflow or secret changes, and no runtime code changes. gitleaks, CodeQL (actions, javascript-typescript, python) and AgentShield all passed. The two advisories are cleared: the dependency-audit npm audit and Audit gate checks pass.
info (process): The safety checks flag STANDARDS_SYNC_PR: true, but this PR only changes dependency manifests and is not a standards sync. The heuristic looks like a misfire and should not change how this PR is routed.
info (bot-feedback): When this audit ran, the Codex and CodeRabbit reviews were still in progress, and CodeRabbit excludes package-lock.json. No advisory bot findings were available to weigh.
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
Resolve all unresolved review thread comments from other reviewers
Ensure all CI checks pass after your changes
Rebase on the target branch if behind
Do NOT modify files unrelated to the findings above
The review cascade will automatically re-review after new commits are pushed.
CI (Node 20, npm 10.8.2) rejected the lockfile in `npm ci` because
npm 10 mis-handles ranged override keys ("glob@>=9.0.0 <14.0.2"): once
the override lifts a copy out of the range it re-flags it as missing.
Parent-scoped overrides don't work either, since npm 11 applies an
`@jest/reporters` override to its whole subtree, which includes
test-exclude's glob 7, and npm 10 doesn't.
Replace them with plain top-level overrides:
- glob ^14.0.2 (GHSA-3g3m-2fjr-6pw4)
- minimatch ^11.0.1 (GHSA-g3hv-75wg-m249)
- test-exclude ^7.0.2, so babel-plugin-istanbul's test-exclude uses
the named-export glob/minimatch APIs instead of glob 7 / minimatch 3
The lockfile was regenerated with npm 10.8.2 from main's lockfile,
re-resolving only the glob, minimatch and test-exclude entries.
`npm ci`, `npm ls` and `npm audit` are clean under npm 10.8.2 and
npm 11.11.0.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixed in 2a4d833. The npm ci failure came from npm 10.8.2, which CI uses, rejecting the ranged override keys. The overrides are now plain top-level glob/minimatch/test-exclude entries, with the lockfile regenerated using npm 10.8.2. CI is green, including build-and-test, coverage, SonarCloud, Playwright and Audit gate. On the minor findings: npm audit signatures verifies all 453 packages, and every new package is published by isaacs. Tests, coverage and lint were re-run on Node 20 with the same results as main. All workflows pin Node 20. Risk, Rollback and Monitoring sections are added, and #668 tracks removing the overrides.
This PR changes only dev dependencies: package.json gains three plain top-level overrides (glob ^14.0.2, minimatch ^11.0.1, test-exclude ^7.0.2), and package-lock.json is regenerated to match. Together these clear GHSA-3g3m-2fjr-6pw4 (glob) and GHSA-g3hv-75wg-m249 (minimatch). The new commit (2a4d833) replaces the ranged override keys that npm 10.8.2 rejected in npm ci. That rejection was the root cause of the CI failures that blocked the previous review (52682ae). All required checks now pass on this head. I agree with the triage assessment: the change is low risk, dev-only, and has no runtime, workflow or secret impact.
Linked issue analysis
No closing issue is linked, which is appropriate: this is a CI-unblocking fix, and the PR description explains the motivation (dependency-audit / npm audit fails on main, and combsmith/.github#1470 makes the Audit gate required). The follow-up to remove the overrides once jest, eslint and typescript-eslint ship patched ranges is tracked in #668, which is referenced but not auto-closed. That answers the prior review's request for a tracking issue.
major (ci): fixed. build-and-test, coverage, SonarCloud, Node.js Tests and Playwright UI Tests now all pass and run for a normal length of time (about 13 s to 1.5 min, not the earlier 10–12 s install failures). This is consistent with the author's diagnosis that npm 10.8.2 rejected the ranged override keys. npm ci succeeding in CI also confirms that the lockfile is internally consistent (for example glob 14's minipass ^7.1.3 and minimatch 11's brace-expansion ^5.0.12 requirements are satisfied). Several now-orphaned glob 10 transitive packages are pruned (foreground-child, jackspeak, @isaacs/cliui, inflight, once, etc.).
minor (supply-chain): accepted. The new packages are @isaacs/nonrecursive-glob-match@1.0.3, path-scurry@3.0.0 and lru-cache@11.5.3. Each resolves from registry.npmjs.org with an sha512 integrity hash and is marked dev: true. The author reports that npm audit signatures verifies all 453 packages. I cannot run that check myself, so that part remains unconfirmed by this review.
minor (compatibility): substantially fixed. jest, eslint and test-exclude are forced to glob 14 / minimatch 11. CI exercises these code paths: jest discovery and coverage via coverage and build-and-test, and lint via build-and-test. All of them pass. The author also reports unchanged test counts (1090), coverage totals and lint results (22 warnings across 28 files). I could not compare the job logs to confirm those counts, so the monitoring note in the PR body (watch for drops in test or linted-file counts) is the right safeguard.
minor (Node 18): accepted. minimatch 11.0.4 and lru-cache 11.5.3 declare engines: 20 || >=22. Node 18 is past end of life, and the author states that every workflow pins Node 20 and the repo declares no engines field. I could not read this repo's workflows to confirm that.
minor (description): fixed. Risk, Rollback and Monitoring sections are present (DESCRIPTION_MISSING: 0), and #668 tracks removing the overrides.
New in this commit:
info (compatibility, package.json:25): The test-exclude ^7.0.2 override goes outside babel-plugin-istanbul's declared ^6 range, and test-exclude 7.0.2 itself declares glob ^10.4.1 / minimatch ^10.2.2 but gets 14 / 11. It therefore depends on the named-export { glob } / { minimatch } APIs staying compatible. The PR body states this tradeoff explicitly, and the green coverage job exercises it end to end. This is acceptable as a temporary measure until upstream catches up (#668).
info (dependency-risk, package.json:23-25): The safety checks flag three unpinned caret ranges. The lockfile pins the exact resolved versions (14.0.3, 11.0.4, 7.0.2) and CI installs with npm ci, so the practical risk is low.
info (process):STANDARDS_SYNC_PR: true is a heuristic misfire. This is a dependency-manifest change, not a standards sync, and the flag did not affect how this PR was assessed.
info (security): No security regression. Only package.json and package-lock.json change, there are no git or tarball sources and no install scripts in the added entries, and no workflow, secret or runtime code is touched. gitleaks, CodeQL (actions, javascript-typescript, python), AgentShield, SonarCloud (quality gate passed, 0 new issues, 0 hotspots), dependency-audit npm audit and the Audit gate all pass. The MCP run_secret_scanning tool was not available in this run, so that scan was skipped; gitleaks passed.
info (bot-feedback): Codex completed its review of 2a4d833 with no findings. CodeRabbit's check is still IN_PROGRESS on this head. Its last finished review (52682ae) produced no actionable comments, and it excludes package-lock.json. I treat it as advisory and non-blocking.
CI status
All completed checks on 2a4d833 pass: build-and-test, Secret scan (gitleaks), autofix, coverage, Node.js Tests, Playwright UI Tests, SonarCloud / SonarCloud Code Analysis, CodeQL (actions, javascript-typescript, python), AgentShield, dependency-audit (Detect ecosystems, npm audit, Audit gate), and Graphite AI Reviews. Ecosystem audits that don't apply here (pnpm, cargo, pip, govulncheck), and the dependabot and dev-lead jobs, were skipped as expected. The only incomplete check is the advisory CodeRabbit bot, which is still in progress. There are no submitted reviews, unresolved threads or unanswered human questions. The merge state is BLOCKED only because a review is required.
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.
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
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.
donpetry-bot
left a comment
Summary
dependency-audit / npm auditfails onmain(run 41727967767), so theAudit gatethat combsmith/.github#1470 makes required would block every merge here. This PR clears both root advisories with npmoverrides.>=9.0.0 <=14.0.1@jest/reporters,jest-config,jest-runtime(glob 10.5.0)"glob": "^14.0.2"→ 14.0.3>=7.0.0 <11.0.1eslint,@eslint/config-array,@typescript-eslint/typescript-estree"minimatch": "^11.0.1"→ 11.0.4babel-plugin-istanbul→test-exclude@6, which uses the glob 7 / minimatch 3 default-export APIs"test-exclude": "^7.0.2", which uses the named{ glob }/{ minimatch }exports that glob 14 / minimatch 11 provideWhy overrides: no parent has released a fix. The latest jest (30.5.2) still requires
glob ^13.0.6, and the latest eslint (10.14.0) and typescript-eslint (8.72.1) still requireminimatch ^10. Drop these overrides once upstream catches up.Why plain top-level overrides: CI runs npm 10.8.2, which rejected the first version's ranged keys (
"glob@>=9.0.0 <14.0.2") innpm ci. Parent-scoped overrides behave differently in npm 10 and npm 11, because npm 11 also applies them totest-exclude's glob. The lockfile was regenerated with npm 10.8.2 from main's lockfile, re-resolving only the glob, minimatch and test-exclude entries.Verification (local)
npm ci,npm ls glob minimatch test-excludeandnpm auditpass under both npm 10.8.2 and npm 11.11.0. Audit reports 0 vulnerabilities (17 moderate on main).jest --coveragepasses 22 suites and 1090 tests, andscripts/check-coverage.jspasses.eslint src: 0 errors and 22 warnings, the same as main's CI.tsc --noEmit,prettier --check .andcheck:piipass.Risk
Low. The change is dev-only: package.json
overridesplus the lockfile, with no runtime or workflow changes. The main risk is a behaviour change from forcing glob 14 / minimatch 11 on jest, eslint and test-exclude. Two checks cover it: eslint still lints all 28 trackedsrcfiles with the same 22 warnings, and coverage, which depends on test-exclude, still reports the same totals. The new packages (glob 14.0.3, minimatch 11.0.4, path-scurry 3.0.0, lru-cache 11.5.3,@isaacs/nonrecursive-glob-match1.0.3) are all published by isaacs, the glob/minimatch maintainer, andnpm audit signaturesverifies all 453 packages. minimatch 11 and lru-cache 11 require Node20 || >=22. Every workflow pins Node 20, and the repo declares noenginesfield.Rollback
Revert this PR. That restores the previous overrides and lockfile, and the npm audit advisories come back.
Monitoring
Watch for jest file discovery, coverage collection or eslint file matching changing on later PRs. A drop in the number of tests or linted files would be the signal. Track removing the overrides in #668.
Follow-up: #668. Related: combsmith/.github#1406, combsmith/.github#1470
🤖 Generated with Claude Code
Summary by CodeRabbit