fs,path: fix glob globstar section bounds by gurgunday · Pull Request #66738 · nodejs/node

gurgunday · 2026-10-14T18:54:41Z

The last start index of each globstar-delimited body section was computed from the non-globstar prefix counts in reverse order. When sections have different lengths, the bound is too tight, so paths such as a/b/c/d/e failed to match **/a/b/c/**/d/**. This affected path.matchesGlob() and exclude patterns in fs.glob()

Compute the bound from the number of non-globstar parts in the section and every section after it

FYI minimatch has the bug too

nodejs-github-bot · 2026-10-14T18:54:47Z

Review requested:

@nodejs/path

avivkeller · 2026-10-14T18:56:37Z

FYI minimatch has the bug too

Is this a breaking change, since the previous implementation suffered the same issue? Or just a bug fix

gurgunday · 2026-10-14T19:02:22Z

I don't have a strong opinion, but this can't be backported anyway right? I think no active release has the C++ port? And previous releases (24, 26) have the bug from minimatch directly

avivkeller

LGTM

codecov · 2026-10-14T20:42:34Z

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.50%. Comparing base (531d28f) to head (6a5e851).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/glob/glob_matcher.cc	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #66738   +/-   ##
=======================================
  Coverage   90.49%   90.50%           
=======================================
  Files         797      797           
  Lines      278221   278219    -2     
  Branches    53462    53458    -4     
=======================================
+ Hits       251786   251804   +18     
+ Misses      16809    16800    -9     
+ Partials     9626     9615   -11

Files with missing lines	Coverage Δ
src/glob/glob_matcher.cc	`66.53% <50.00%> (-0.14%)`	⬇️

... and 24 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-10-14T22:52:02Z

CI: https://ci.nodejs.org/job/node-test-pull-request/78399/

nodejs-github-bot · 2026-10-15T10:22:30Z

CI: https://ci.nodejs.org/job/node-test-pull-request/78402/

vedchaudhari · 2026-10-15T11:40:54Z

Thanks for spotting this! Reported upstream in isaacs/minimatch#329, it's a regression since 10.2.3.

gurgunday · 2026-10-16T21:34:13Z

Should I add do not land labels for these? Since the C++ wasn't back-ported either?

aduh95 · 2026-10-19T08:22:37Z

Should I add do not land labels for these? Since the C++ wasn't back-ported either?

If it can't or shouldn't land, yes you should add the dont-land labels

nodejs-github-bot · 2026-10-19T15:23:27Z

Commit Queue failed

   ✖  This PR needs to wait 52 more hours to land (or 0 minutes if there is one more approval)
   ⚠  This PR has conflicts that must be resolved

The pull request was removed from the Commit Queue and labeled commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. . After resolving the failure, remove that label and add commit-queue PRs queued for automated landing through the Commit Queue. to retry.

Full Commit Queue output

�[36m⠋�[39m Loading data for nodejs/node/pull/66738
�[36m⠋�[39m Loading data for nodejs/node/pull/66738
�[36m⠋�[39m Getting collaborator contacts from README of nodejs/node
�[36m⠋�[39m Getting PR from nodejs/node/pull/66738
�[36m⠋�[39m Getting reviews from nodejs/node/pull/66738
�[36m⠋�[39m Getting comments from nodejs/node/pull/66738
�[36m⠋�[39m Getting commits from nodejs/node/pull/66738
✔  Done loading data for nodejs/node/pull/66738
----------------------------------- PR info ------------------------------------
Title      fs,path: fix glob globstar section bounds (#66738)
Author     Gürgün Dayıoğlu <hey@gurgun.day> (@gurgunday)
Branch     gurgunday:fix-glob-globstar-bound -> nodejs:main
Labels     c++, author ready, needs-ci, commit-queue, lacks-second-approval
Commits    1
 - fs,path: fix glob globstar section bounds
Committers 1
 - Gürgün Dayıoğlu <hey@gurgun.day>
PR-URL: https://github.com/nodejs/node/pull/66738
Reviewed-By: Aviv Keller <me@aviv.sh>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/66738
Reviewed-By: Aviv Keller <me@aviv.sh>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 14 Oct 2026 18:54:41 GMT
   ✔  Approvals: 1
   ✔  - Aviv Keller (@avivkeller): https://github.com/nodejs/node/pull/66738#pullrequestreview-5523202911
   ✖  This PR needs to wait 52 more hours to land (or 0 minutes if there is one more approval)
   ⚠  This PR has conflicts that must be resolved
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-10-15T10:22:30Z: https://ci.nodejs.org/job/node-test-pull-request/78402/
�[36m⠙�[39m Querying data for job/node-test-pull-request/78402/
�[36m⠙�[39m Querying data for job/node-test-pull-request/78402/
�[36m⠙�[39m Querying API for job/node-test-pull-request/78402/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
�[36m⠹�[39m Querying API for job/node-test-pull-request/78402/
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

View workflow run

github-actions · 2026-10-20T03:28:27Z

This pull request has conflicts with its base branch, removing the author ready label.
Please rebase your branch onto the latest base branch, resolve the conflicts locally, and force-push.
Afterwards the pull request needs a fresh collaborator approval, and a collaborator will add the label back once it is author ready again.

The last start index of each globstar-delimited body section was computed from the non-globstar prefix counts in reverse order. When sections have different lengths, the bound is too tight, so paths such as `a/b/c/d/e` failed to match `**/a/b/c/**/d/**`. This affected `path.matchesGlob()` and `exclude` patterns in `fs.glob()`. Compute the bound from the number of non-globstar parts in the section and every section after it. Signed-off-by: Gürgün Dayıoğlu <hey@gurgun.day>

gurgunday requested a review from avivkeller October 14, 2026 18:54

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Oct 14, 2026

avivkeller approved these changes Oct 14, 2026

View reviewed changes

gurgunday added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 14, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 14, 2026

gurgunday added author ready PRs with CI started, the required approvals, and no outstanding review comments. resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. labels Oct 15, 2026

github-actions Bot removed the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Oct 15, 2026

vedchaudhari mentioned this pull request Oct 15, 2026

Regression: multiple non-adjacent ** globstars fail to match some valid paths isaacs/minimatch#329

Open

MFA-G mentioned this pull request Oct 16, 2026

fix: globstar body section bounds for sections of different lengths isaacs/minimatch#330

Open

gurgunday added the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 16, 2026

nodejs-github-bot added the lacks-second-approval Commit Queue PRs awaiting a second collaborator approval or completion of the required wait. label Oct 16, 2026

github-actions Bot removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Oct 20, 2026

gurgunday force-pushed the fix-glob-globstar-bound branch from 0a97eb6 to 6a5e851 Compare October 20, 2026 10:24

gurgunday requested review from aduh95 and avivkeller October 20, 2026 17:43

Navigation Menu

Repository navigation

fs,path: fix glob globstar section bounds - #66738

fs,path: fix glob globstar section bounds#66738
gurgunday wants to merge 1 commit into
nodejs:mainfrom
gurgunday:fix-glob-globstar-bound

gurgunday commented Oct 14, 2026

Uh oh!

nodejs-github-bot commented Oct 14, 2026

Uh oh!

avivkeller commented Oct 14, 2026

Uh oh!

gurgunday commented Oct 14, 2026 •

edited

Loading

Uh oh!

avivkeller left a comment

Uh oh!

codecov Bot commented Oct 14, 2026 •

edited

Loading

Uh oh!

nodejs-github-bot commented Oct 14, 2026

Uh oh!

nodejs-github-bot commented Oct 15, 2026

Uh oh!

vedchaudhari commented Oct 15, 2026

Uh oh!

gurgunday commented Oct 16, 2026

Uh oh!

aduh95 commented Oct 19, 2026

Uh oh!

nodejs-github-bot commented Oct 19, 2026

Uh oh!

github-actions Bot commented Oct 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

gurgunday commented Oct 14, 2026

Uh oh!

nodejs-github-bot commented Oct 14, 2026

Uh oh!

avivkeller commented Oct 14, 2026

Uh oh!

gurgunday commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

avivkeller left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nodejs-github-bot commented Oct 14, 2026

Uh oh!

nodejs-github-bot commented Oct 15, 2026

Uh oh!

vedchaudhari commented Oct 15, 2026

Uh oh!

gurgunday commented Oct 16, 2026

Uh oh!

aduh95 commented Oct 19, 2026

Uh oh!

nodejs-github-bot commented Oct 19, 2026

Commit Queue failed

Uh oh!

github-actions Bot commented Oct 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gurgunday commented Oct 14, 2026 •

edited

Loading

codecov Bot commented Oct 14, 2026 •

edited

Loading