glob: judge a ** by its own surroundings, not by the brace branch being matched

[robobun]

Problem

• new Bun.Glob("**/a**/{x,y}").match("ab/ay") returns true. The segment ay is neither x nor y; **/a*/{x,y} (which is what a** is documented to mean) correctly returns false, and so do picomatch, micromatch and bash. Same for **/a**/{x} vs ab/ax, **/a**/{x,y}/c vs ab/ay/c, **/{a,b}**/{x,y} vs q/bz/bx, and !**/a**/{x,y} gives the inverse wrong answer. Found by a Bun.Glob-vs-picomatch differential fuzzer.
• It takes all three ingredients: a real globstar earlier in the pattern, a later segment ending in a ** that is not a whole segment, and a brace group after it. a**/{x,y}, */a**/{x,y} and **/a**/y are all correct against ab/ay.
• Cause, src/glob/matcher.rs, b'*' arm of glob_match_impl: whether a ** starts a segment was checked with state.glob_index.saturating_sub(glob_start) < 3 || glob[glob_index - 3] == b'/', where glob_start is the index the current glob_match_impl call began at: 0 at top level, but the start of the branch when matching a brace alternative (so that {**/a,**/b} works). When a branch fails, it backtracks into the enclosing globstar, which re-runs the part of the pattern that precedes the group inside the branch's call. Every index there is below glob_start, the subtraction saturates to 0, and every ** re-matched on the way back qualifies as a globstar. **/a**/{x,y} is then matched against ab/ay as **/ = ab/, a = a, an empty **/, and y = y.
• The same check also ran only after skip_globstars had folded any /** segments following the ** into it, so x**/** was matched as one trailing globstar and matched x and xy (glob: stop a non-segment ** from swallowing the ** segments after it in match() #38996 fixes this shape on its own; see the note below).

Fix

• is_segment_start() decides from the **'s own neighbourhood whether it begins a segment: it is at index 0 of the pattern, follows a /, or is the branch_idx of a frame on brace_stack. That result gates both skip_globstars and the globstar path; a ** that fails it goes through the * arm twice, which is exactly what a* does. Only the "followed by / or end of pattern" half of the rule is still checked after the collapse.
• Why this is the right rule: the matcher's own definition of a globstar is a ** that is a whole segment, and which position a ** sits at does not depend on which brace branch is being tried, so the answer must not depend on glob_start. The brace stack holds exactly the branches we are currently inside (match_brace_branch pushes before matching the branch plus the rest of the pattern and pops after), so branch_idx identifies a branch-leading ** wherever we arrive at it from, including by backtracking, and a literal , or { (a,**/b) does not qualify. For a ** that does begin a segment nothing changes: a pattern-start ** is index 0 (the ! prefix is sliced off up front, which is what let the glob_start parameter go away), a branch-leading ** is on the stack, and every ** reached by the collapse is preceded by /.
• Test: test/js/bun/glob/match.test.ts, the new describe("a \` that is not a whole segment behaves like `*`"). 11 of its 61 assertions fail on the unfixed binary (7 brace-backtracking shapes, 4 x/ shapes); the other 50 are controls that pass before and after: the same patterns against paths they should match, real globstars across the same backtrack (/a//{x,y}, /{/x,y}, /a/{b/**/x,y}`), and each of the three ingredients removed.
• match.test.ts (31 tests, 1582 assertions, including the ported bash/micromatch star, globstar and brace suites) and the ported node fs.glob suite (448 tests) pass with the debug build; all 61 new assertions agree with brace-expansion + picomatch.
• Differential check, fixed debug build against the current release: 4000 generated patterns (tokens mixing real globstars, brace groups with and without leading **, and a**/**b/{a,b}**/a,** shapes) x 40 paths. For every pattern P, fixed(P) equals unfixed(P with each non-segment ** spelled *), and the 1558 patterns with no non-segment ** are unchanged; 21 results across 12 patterns changed, all spurious matches of a non-segment **.
• scan() is unaffected: it matches one /-separated component at a time, so a component never backtracks into another one.
• Overlap with glob: stop a non-segment ** from swallowing the ** segments after it in match() #38996: that PR moves the start-of-segment check ahead of the collapse but keeps it expressed as glob_index <= glob_start, which is what this bug needs replaced. Whichever lands second rebases to a small delta; on top of glob: stop a non-segment ** from swallowing the ** segments after it in match() #38996 this PR is the is_segment_start rule plus the brace-backtracking tests.
• Not changed here: a ** whose segment boundary is brace syntax rather than / (a/{**,b}, {foo/**,bar}/baz) is still matched as *. That is a pre-existing false negative and is tracked separately.

Background

• The matcher (src/glob/matcher.rs, ported from the glob-match crate) is a backtracking matcher over the raw pattern bytes; braces are not expanded up front. Every * records a wildcard resume point; a qualifying ** also records a globstar resume point, and a later failure resumes there to let the globstar eat one more segment and re-match everything after it.
• A brace group is matched by match_brace, which calls match_brace_branch per alternative; that pushes a Brace { open_brace_idx, branch_idx, close_brace_idx } frame and recursively runs glob_match_impl starting at the branch, over the rest of the whole pattern. The recursive call inherits the resume points, so a failure inside the branch can resume at a globstar that precedes the group and walk back through the pattern prefix while the branch's frame is still on the stack.
• "Globstar" means a ** that is a whole path segment; any other ** (a**, **b) is documented to behave like * (the a**c, foo**bar, foo** cases in match.test.ts). skip_globstars is the optimization that folds **/**/** into one globstar so the matcher does not backtrack through equivalent segments.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 13 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 429bcdcd-feb0-4c7f-b60e-c72e63d57fd5

📥 Commits

Reviewing files that changed from the base of the PR and between 732491c and 305458f.

📒 Files selected for processing (2)

• src/glob/matcher.rs
• test/js/bun/glob/match.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status: fix pushed, waiting for CI.

Reproduced on the current release (USE_SYSTEM_BUN=1): new Bun.Glob("**/a**/{x,y}").match("ab/ay") returns true; with this branch (bun bd) it returns false. 11 of the 61 new assertions in test/js/bun/glob/match.test.ts fail on the unfixed binary and all pass with the fix; the rest of match.test.ts and the ported node fs.glob suite are unchanged.

Related: #38996 fixes the x**/** shape independently; this PR replaces the glob_start-relative check that #38996 keeps, so the two overlap on a few lines and whichever lands second needs a small rebase.

[claude]

I reviewed this PR and didn't find any bugs. The is_segment_start rule and the !-prefix slicing check out, and the test coverage is thorough. Given it's a semantic change to the core glob backtracking matcher (used across the bundler, install, shell, and fs.glob) and the noted overlap with #38996, a human look would still be worthwhile.

What was reviewed:

• Traced the b'*' arm before/after: gating skip_globstars on is_segment_start fixes both the brace-backtrack case and x**/**, and the removed glob_index - 3 check is fully subsumed.
• Checked is_segment_start for false hits from stale/sequential brace frames — branch_idx is a unique glob offset inside the group, so a ** outside it can't collide; glob[glob_index-1] can't underflow.
• Verified re-slicing glob past ! keeps all indices consistent (state starts at 0) and preserves !**/… behavior.
• Confirmed **b and top-level a** take the same effective path as before; scan() is untouched.

Extended reasoning...

Overview

This PR fixes a correctness bug in src/glob/matcher.rs where a non-segment ** (e.g. a**) was incorrectly promoted to a globstar when re-scanned via backtracking from inside a brace branch, causing patterns like **/a**/{x,y} to spuriously match ab/ay. The fix replaces the glob_start-relative check with is_segment_start(), which decides from the **'s own position (index 0, preceded by /, or a live branch_idx on the brace stack), and moves that check ahead of skip_globstars so x**/** no longer collapses into a trailing globstar. The ! prefix is now sliced off the pattern up front so index 0 is the true pattern start, which lets the glob_start parameter be removed entirely. 61 new assertions in match.test.ts cover the failing shapes, positive controls, real-globstar controls, and each-ingredient-removed controls.

Security risks

None. This is pure in-memory pattern matching over caller-supplied byte slices; no I/O, no allocation changes, no new panics. The only index arithmetic added is glob[glob_index as usize - 1], guarded by glob_index != 0.

Level of scrutiny

Medium-high. The diff is small and the reasoning in the PR description is unusually rigorous (mechanism traced to the saturating subtraction, differential fuzz against picomatch over 4000×40 inputs, existing 1582-assertion suite plus the ported node fs.glob suite passing). But glob_match_impl is a hand-written backtracking matcher whose brace/globstar interaction is subtle, and it sits under Bun.Glob, the bundler, bun install, the shell, and fs.glob. A behavior change here fans out widely.

Other factors

• I traced the new rule against the concern in skip_branch's doc comment that the brace stack "also holds already-exited sequential groups": those frames' branch_idx values point inside their own group, so a ** after the group cannot collide with them — no false positives.
• The **b case (segment-start but not segment-end) still enters the is_globstar block and fails the trailing-/ check exactly as before; the top-level a** case now takes the single-* arm twice instead of the += 2 short path but ends at the same glob_index with equivalent wildcard state.
• The PR explicitly flags overlap with #38996 and a pre-existing brace-boundary false negative left for a follow-up; a maintainer should coordinate the merge order.
• No prior human review on the timeline; only a rate-limited CodeRabbit placeholder.

[robobun]

Nothing actionable came out of the automated review above (no line comments). On its one open point, merge order with #38996: both PRs now link to each other, and the tests in each stay valid regardless of which lands first; the second one only needs a rebase of the few shared lines in the b'*' arm. I will rebase this one if #38996 goes in first.

[robobun]

Closing #38996 in favour of this PR: it contains the same matcher change for the x**/** shape (deciding segment start on the first ** before skip_globstars folds the run) and its second test block covers that shape, so landing both would only mean a rebase for whichever came second.

Two things from #38996 that may be worth folding in here, since they are not in the current test block:

1. A few more spellings of the swallowed-run shape, each of which matched on the release build and now does not: ?**/** vs b, [a]**/** vs a, {x,a}**/** vs ab (the ** follows a }), a**/**/ vs a, a**/**/*.js vs ab.js, and !a**/** vs ab (returns false on the release build, should be true). The compact way to pin the whole class is to assert that each inline spelling matches exactly the same paths as its * spelling, e.g. a**/** vs a*/**, a**/**/b vs a*/**/b, **/a**/** vs **/a*/**, over a list of paths with and without a /.
2. **/{a,b} vs x/y/b as a control: the failing a branch backtracks to the ** at index 0 and re-examines it from inside the branch, which is the glob_index == 0 arm of is_segment_start (and the case that would underflow if that arm were ever dropped).

For the reviewer's benefit, the differential run from #38996 applies to the ordering change this PR shares: 8612 patterns over an 8-token grammar x 128 paths (1,102,336 results) against the release build; the only results that changed were for patterns with a non-segment ** directly followed by ** segments, and after the change the only remaining disagreements with picomatch 4 in that class were !pattern against "", where picomatch rejects empty input regardless of the pattern.