glob: match explicitly-named dotfiles and resolve literal paths through symlinks - #32853
Conversation
…gh symlinks Bun.Glob.scan() silently returned no matches for two classes of pattern that every reference implementation (bash, picomatch, minimatch, fast-glob) handles: 1. A pattern segment that spells out a leading dot, like '.dotdir/inner.txt' or '.env', was filtered out by the dot:false default even though the user explicitly named the dotfile. The dot option is meant to govern wildcards, not literal names. 2. A literal (non-wildcard) path segment that happens to be a symlink to a directory, like 'linkdir/file.txt', was blocked by followSymlinks:false even though the user explicitly wrote that path segment. followSymlinks is meant to govern wildcard traversal. Both produced empty results with no error, which is dangerous for build/test/deploy file selection: a project behind a symlinked mount or a config naming a dotfile silently processes nothing. Fix match_pattern_dir/match_pattern_impl to let a segment through the dot filter when the pattern component itself starts with a literal '.', and extend the symlink entry handling to descend when the matching pattern component is a non-wildcard literal, restricting the propagated active set to those literal components so wildcard traversal still respects followSymlinks:false.
|
Warning Review limit reached
More reviews will be available in 58 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Walkthrough
ChangesDotfile and symlink traversal fixes
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
|
Updated 1:40 PM PT - Jun 27th, 2026
❌ @robobun, your commit 3d20655 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 32853That installs a local version of the PR into your bun-32853 --bun |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/bun/glob/scan.test.ts`:
- Around line 994-1014: The symlink setup in makeTree and the related tests are
using bare early returns when fs.symlinkSync fails with EPERM/EACCES, which
makes unsupported Windows runs look like passing tests instead of skipped ones.
Update the symlink-dependent cases around makeTree and the cycle test to use the
test harness’s skip mechanism with an explicit reason, so the suite is reported
as skipped when symlinks aren’t available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5a97bd41-de3c-4bdf-a29c-e5de844c7cb1
📒 Files selected for processing (2)
src/glob/GlobWalker.rstest/js/bun/glob/scan.test.ts
Probe symlink creation once at suite scope and describe.skipIf on it, so a Windows run without symlink privileges reports the suite as skipped instead of passing vacuously. Also simplifies the per-test setup to plain 'using' now that makeTree always succeeds.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
When `**/.dotdir/...` advances past a hidden `.dotdir` entry via the explicit-dot next segment (bump=2), eval_dir was unconditionally re-adding the `**` index to the child active set, letting wildcard recursion continue inside the hidden directory. That over-matches: `.dotdir/foo/.dotdir/inner.txt` would satisfy `**/.dotdir/inner.txt` even though the only decomposition needs `**` to consume the hidden `.dotdir/foo` prefix, which bash/picomatch/minimatch/fast-glob all reject with dot:false. Gate the keep-`**`-alive line on the entry not being hidden, and add `.dotdir/foo/.dotdir/inner.txt` to the `**/.dotdir/inner.txt` test fixture to cover it.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/glob/GlobWalker.rs (1)
1058-1072: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate the symlink work-item push shared by both symlink branches.
Lines 1058-1072 (
FileKind::SymLink) and 1151-1163 (reclassifiedUnknown→SymLink) build and push the symlinkWorkItemwith byte-identical logic (join →work_item_logical_path→entry_start→push(WorkItem::new_symlink(..))). These are parallel paths for the same effect; keeping two copies risks divergence if one is later fixed and the other missed. Extract a helper on the walker and call it from both sites oncefollow_activeis computed.♻️ Proposed helper + call sites
New method on
impl GlobWalker:fn push_symlink_work( &mut self, dir_dir_path: &[u8], entry_name: &[u8], follow_active: ComponentSet, ) -> Result<(), AllocError> { let subdir_entry_name = self.join(&[dir_dir_path, entry_name])?; let joined = work_item_logical_path(&subdir_entry_name); let entry_start: u32 = u32::try_from(joined.len() - strings::basename(joined).len()).unwrap(); self.workbuf .push(WorkItem::new_symlink(subdir_entry_name, follow_active, entry_start)); Ok(()) }
FileKind::SymLinksite:if let Some(follow_active) = follow_active { - let subdir_parts: &[&[u8]] = &[dir_dir_path, entry_name]; - let subdir_entry_name = self.walker.join(subdir_parts)?; - let joined = work_item_logical_path(&subdir_entry_name); - let entry_start: u32 = - u32::try_from(joined.len() - strings::basename(joined).len()) - .unwrap(); - - self.walker.workbuf.push(WorkItem::new_symlink( - subdir_entry_name, - follow_active, - entry_start, - )); + self.walker + .push_symlink_work(dir_dir_path, entry_name, follow_active)?; continue; }Apply the same replacement at lines 1151-1163.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/glob/GlobWalker.rs` around lines 1058 - 1072, The symlink work-item creation logic is duplicated in both the FileKind::SymLink branch and the reclassified Unknown→SymLink branch, so extract the shared join/work_item_logical_path/entry_start/WorkItem::new_symlink sequence into a helper on GlobWalker. Add a method such as push_symlink_work that takes the directory path, entry name, and follow_active value, performs the shared work, and returns the same Result type used today. Then replace both call sites with that helper once follow_active is known so the two branches stay behaviorally identical and cannot drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/glob/GlobWalker.rs`:
- Around line 1058-1072: The symlink work-item creation logic is duplicated in
both the FileKind::SymLink branch and the reclassified Unknown→SymLink branch,
so extract the shared
join/work_item_logical_path/entry_start/WorkItem::new_symlink sequence into a
helper on GlobWalker. Add a method such as push_symlink_work that takes the
directory path, entry name, and follow_active value, performs the shared work,
and returns the same Result type used today. Then replace both call sites with
that helper once follow_active is known so the two branches stay behaviorally
identical and cannot drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 98fd541b-8f8f-4668-998d-44403d09dc88
📒 Files selected for processing (2)
src/glob/GlobWalker.rstest/js/bun/glob/scan.test.ts
The SymLink and Unknown->SymLink arms in Iterator::next both build and enqueue a symlink WorkItem with identical logic. Extract it into a single helper so the two paths cannot drift.
…al_dir
The `**/.dotdir/...` peek-ahead added to match_pattern_dir only runs
when eval_dir is reached. The SymLink (follow_symlinks:true) and
Unknown readdir arms pre-filter entries through eval_impl first, and
eval_impl had no knowledge of the next-segment peek. With active={**}
and a hidden entry, match_pattern_impl returns false and the entry was
dropped before eval_dir ever ran, so a symlinked .dotdir (or a real
.dotdir reported as DT_UNKNOWN on NFS/overlayfs/FUSE) silently missed
the fix that works for DT_DIR.
Teach eval_impl the same peek so it stays at least as permissive as
eval_dir. eval_impl is purely a pre-filter; eval_dir/eval_file remain
the authoritative checks downstream, so widening it cannot over-match.
|
CI status: the glob changes are green on every lane that ran them. The remaining red across builds #65518 / #65534 / #65566 is unrelated infrastructure and known-flaky tests:
None of those touch |
|
Independently reproduced the dot-segment half of this and landed on the same shape; pushed a minimal variant to One difference worth a look: that branch gates the dot filter on bash is stricter there (bracket expressions don't match a leading dot), so either reading is defensible. Leaving this here in case fast-glob parity is the tiebreaker; not opening a separate PR. |
2 participants
What
Fixes two cases where
Bun.Glob.scan()silently returns an empty result for patterns that look obviously correct and work in every reference implementation (bash, picomatch, minimatch, fast-glob):Both are silent empty results, not errors. Glob is how people select files for builds, tests, deploys and uploads, so "no matches" quietly excludes files. A project that lives behind a symlink (pnpm layouts, mounted volumes,
/tmpon macOS) or a config that names a dotfile explicitly would process nothing.Why
Explicit dotfiles.
match_pattern_dirandmatch_pattern_implinGlobWalker.rsrejected any entry whose name starts with.wheneverdot: false, without looking at the pattern segment. The ecosystem convention is that thedotoption only governs whether wildcards match dotfiles; a segment whose pattern text itself starts with.is an explicit request for that name and matches regardless:.dotdir/inner.txt.dotdir/inner.txt.*/inner.txt.dotdir/inner.txt*/inner.txt.dotdir/inner.txt**/inner.txt.dotdir/inner.txtLiteral path through a symlink. The
SymLinkarm of the directory iterator only descended whenfollow_symlinkswas set. ButfollowSymlinksis documented (and implemented everywhere else) as a wildcard-traversal option: whether*/**should walk through symlinked directories. A segment that names the symlink literally is an explicit path the user wrote; fast-glob resolves it regardless offollowSymbolicLinks:followSymbolicLinks:falsefollowSymlinks:falsebeforelinkdir/file.txt["linkdir/file.txt"][]["linkdir/file.txt"]linkdir/*.txt["linkdir/file.txt"][]["linkdir/file.txt"]*/file.txt["realdir/file.txt"]["realdir/file.txt"]["realdir/file.txt"]**/file.txt["realdir/file.txt"]["realdir/file.txt"]["realdir/file.txt"]How
match_pattern_impl: bypass the dot filter when the pattern component's own text starts with..match_pattern_dir: move the dot check after the**-advances-to-next-segment check so**/.dotdir/...can advance;**on its own still never descends through a hidden entry.SymLinkentry handling: whenfollow_symlinksis off, compute the subset of active components that areSyntaxHint::Literaland match the entry name; if non-empty, push the symlink work item with only that subset. Wildcard components stay out of the propagated set, so*/**still respectfollowSymlinks:falseand cycles reached via wildcards cannot loop.Tests
Added to
test/js/bun/glob/scan.test.ts: positive cases for each fix plus negative cases proving wildcards still hide dotfiles, wildcards still respectfollowSymlinks:false, and aloop -> .symlink cycle reached via**under a literally-named parent does not recurse.Not in this PR
Four other
Bun.Globdivergences from the ecosystem surfaced alongside these, all in the pattern matcher rather than the walker: single-item{a}braces expanding instead of being literal, unterminated{and[not falling back to literal, andscan()not honouring the same backslash escaping asmatch(). Those live insrc/glob/matcher.rs/ component classification and deserve their own change.Fixes #28021 (the
Bun.Globscanner-level dot suppression described there; thefs.globsymptom was separately sidestepped by the minimatch port in #31830, butBun.Globitself still had the bug).