fix(glob): count escape backslashes at the right offset in detect_glob_syntax

[robobun]

Adopts #34275 by @sgammon (fix and Rust unit tests are theirs); this PR adds an end-to-end bun install test that exercises the only caller.

Problem

detect_glob_syntax in src/glob/lib.rs searches each special token (*, {, [, ?) in a slice that advances past escaped occurrences, but counts the preceding backslashes at the slice-relative index into the full pattern:

if let Some(idx) = slice.iter().position(|&b| b == token) {
    let mut i = idx;                                   // relative to `slice`
    while i > 0 && potential_pattern[i - 1] == b'\\' { // indexes the full pattern

The first hit for each token is correct (slice == potential_pattern), so the bug only fires after at least one escaped occurrence has been skipped. Walking \*x*: the first * is correctly seen as escaped and skipped; the second * at slice index 1 then checks potential_pattern[0] (the backslash at the front of the string, not the x that actually precedes it) and is wrongly treated as escaped too, so the function returns false.

The only caller is workspace expansion in bun install (src/install/lockfile/Package/WorkspaceMap.rs), so a workspaces entry with an escaped token followed by an unescaped one of the same kind is treated as a literal path instead of being glob-expanded:

{ "name": "root", "workspaces": ["\\*x*"] }

error: Workspace not found "\*x*"

Fix

Rebase the match index onto the full pattern before counting:

let mut i = potential_pattern.len() - slice.len() + idx;

Verification

• cargo test -p bun_glob: 3 new unit tests pass (unescaped tokens, escaped tokens, escaped-then-unescaped regression).
• test/cli/install/bad-workspace.test.ts: new test creates a *xpkg/ workspace and a root workspaces: ["\\*x*"]; bun install now expands the glob and links the workspace. Fails on main with Workspace not found "\*x*", passes with this change. Skipped on Windows (* is not a valid filename character there).

---

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/bad-workspace.test.ts

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 20 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: 73434052-7393-4105-a344-a243dc9fcc7a

📥 Commits

Reviewing files that changed from the base of the PR and between 468dac3 and 00ae8b9.

📒 Files selected for processing (2)

• src/glob/lib.rs
• test/cli/install/bad-workspace.test.ts

---

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

[github-actions]

This PR may be a duplicate of:

1. fix(glob): count escape backslashes at the right offset in detect_glob_syntax #34275 - Fixes the same detect_glob_syntax backslash offset bug in src/glob/lib.rs with identical code changes; this PR explicitly adopts that fix

🤖 Generated with Claude Code

[claude]

LGTM — one-line offset fix with clear mechanism and both unit + e2e coverage.

What was reviewed:

• Verified the rebase arithmetic: slice is always a suffix of potential_pattern, so len - slice.len() + idx is the correct absolute index with no under/overflow.
• Confirmed WorkspaceMap.rs is the only caller; blast radius is limited to workspace glob detection.
• Integration test uses tempDir/bunEnv, drains pipes concurrently, asserts positive lockfile contents (not just error absence), and skips Windows with a stated reason.

Extended reasoning...

Overview

Single-line logic fix in src/glob/lib.rs::detect_glob_syntax: the loop that counts preceding backslashes was indexing potential_pattern with a slice-relative index. Since slice shrinks after each escaped match, the second and later hits per token counted backslashes at the wrong offset. The fix rebases idx onto the full pattern via potential_pattern.len() - slice.len() + idx. Three new Rust unit tests cover unescaped, escaped, and escaped-then-unescaped cases; a new bun install test in bad-workspace.test.ts exercises the only production caller end-to-end.

Security risks

None. This is a pure detection heuristic over a workspace pattern string with no allocation, syscalls, or untrusted-length arithmetic. The change strictly makes detection more accurate (previously-missed globs now detected); it cannot cause a pattern that was correctly rejected to be accepted.

Level of scrutiny

Low. The change is one arithmetic expression whose correctness is verifiable by inspection: slice is always &potential_pattern[offset..], so potential_pattern.len() - slice.len() recovers offset, and adding idx (which is < slice.len()) yields a valid index into potential_pattern. Grep confirms the only caller is workspace expansion in src/install/lockfile/Package/WorkspaceMap.rs, matching the PR's claim.

Other factors

The added tests follow repo conventions (tempDir with using, Promise.all on stdout/stderr/exited, bunEnv/bunExe, exit-code assertion last, Windows skip with a comment naming why). The integration test asserts the lockfile actually contains the linked workspace, so it fails for the right reason rather than merely checking the error string is absent. No prior reviews or outstanding comments on the timeline.

[claude]

LGTM — one-line index rebase in detect_glob_syntax is correct; slice is always a suffix of potential_pattern, so len - slice.len() + idx is the absolute offset and can't overflow or go out of bounds.

What was reviewed:

• Traced the loop: slice only ever re-slices forward from potential_pattern, so the offset math holds on every iteration.
• Confirmed WorkspaceMap.rs is the only caller; no other sibling site to fix.
• New e2e test drains pipes concurrently, uses using/tempDir, and the Windows skip is justified (* is illegal in NTFS filenames).
• Unit tests cover all four tokens plus the negative case (\\*x\\* → false).

Extended reasoning...

Overview

One-line fix in src/glob/lib.rs::detect_glob_syntax: the backslash-counting loop was indexing potential_pattern with a slice-relative idx, so after the first escaped token was skipped the escape count was read from the wrong bytes. The fix rebases idx to an absolute offset via potential_pattern.len() - slice.len() + idx. Since slice is only ever assigned as a forward suffix of potential_pattern (slice = &slice[idx + 1..]), slice.len() <= potential_pattern.len() and idx < slice.len(), so the expression is always in [0, potential_pattern.len()) — no panic or wrap risk. The rest of the loop already indexed potential_pattern[i - 1], so this is purely making i mean what the existing code assumed it meant.

Three new #[cfg(test)] unit tests cover unescaped tokens, escaped tokens, even-backslash-count, and the regression case (escaped-then-unescaped) for each of * { [ ?, plus a negative assertion. A new e2e test in bad-workspace.test.ts exercises the sole caller (workspace glob expansion) end-to-end and asserts on the lockfile contents.

Security risks

None. Pure byte-level string scanning over a workspace-config value; no allocation, no I/O, no user code invoked. The only behavioral change is that some patterns previously mis-classified as literals are now correctly treated as globs — which is what the user wrote.

Level of scrutiny

Low. The function is 30 lines, self-contained, and has exactly one caller (src/install/lockfile/Package/WorkspaceMap.rs:213). The bug and fix are both mechanically verifiable by tracing two iterations of the loop on \\*x*. The affected input shape (an escaped glob metachar followed by an unescaped one of the same kind in a workspaces array entry) is exotic enough that blast radius is minimal.

Other factors

The comment-cop feedback about over-commenting was addressed in 00ae8b9 (thread resolved); the file now carries only the original two-line comment. Test follows harness conventions: tempDir + using, Bun.spawn with await using, concurrent Promise.all drain, exit-code asserted last, skipIf(isWindows) with a reason. The PR description states the e2e test fails on main with Workspace not found and passes with the fix, and the unit tests independently pin the invariant.

[robobun]

^{Updated 11:22 PM PT - Jul 30th, 2026}

✅ @robobun, your commit 00ae8b9af42ff89acb8b2fbfac55321a6f25433c passed in Build #86045! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 36504

That installs a local version of the PR into your bun-36504 executable, so you can run:

bun-36504 --bun

[robobun]

Superseded. main fixed this in 2c5c312 (#37052): the backslash scan in detect_glob_syntax now indexes into slice rather than potential_pattern, which is equivalent to the rebase done here. Verified against a current main build with the workspaces: ["\\*x*"] scenario from this PR's test (the *xpkg workspace is linked). Closing along with #34275.