Repository navigation
Conversation
PR #31220 changed Bun Shell so that glob metacharacters arriving via ${...} interpolation, $var expansion, command substitution, or quoted text are matched literally instead of as pattern syntax. PR #34324 made a redirect target that expands to more than one word an 'ambiguous redirect' error instead of silently writing to a concatenated junk filename. docs/runtime/shell.mdx mentioned globs only in a one-line Features bullet, so neither the literal-interpolation rule nor the migration path for scripts that build patterns at runtime was documented. This adds a Glob expansion section covering: - which tokens are pattern syntax (*, **, {a,b,...}) and that ?, [...] and leading ! are literal - the zsh-style 'no matches found' failure - that interpolated values are never treated as glob syntax (with a note about the pre-1.4 behavior) - the two escape hatches for dynamic patterns: { raw: pattern } and expanding with Bun.Glob then passing an array It also adds an 'Ambiguous redirect targets' subsection under Redirection and cross-links the $.escape and Security sections to the new material.
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughChangesShell documentation
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
There was a problem hiding this comment.
LGTM — docs-only addition documenting existing shell glob/redirect behavior.
What was reviewed:
- Error message strings (
no matches found,ambiguous redirect: at) matchsrc/runtime/shell/states/Expansion.rsandCmd.rsexactly. - Claim that only
*/**/{...}are pattern syntax (not?/[...]) matches the token set inshell_body.rsandExpansion.rs. - Internal links (
/runtime/glob,#dynamic-glob-patterns,#interpolated-values-are-not-glob-patterns) resolve;import { Glob } from "bun"matches usage indocs/runtime/glob.mdx.
Extended reasoning...
Overview
This PR adds a "Glob expansion" section and an "Ambiguous redirect targets" subsection to docs/runtime/shell.mdx, plus two cross-links from the existing $.escape and Security sections. It documents behavior that shipped in earlier PRs (#31220, #34324) but was never surfaced in the shell docs: interpolated values are matched literally rather than as glob syntax, zero-match globs fail zsh-style with no matches found, multi-word redirect targets fail with ambiguous redirect, and the two migration paths for dynamic patterns ({ raw: ... } and Bun.Glob + array interpolation).
Security risks
None. Docs-only; no source, test, or config files touched. The new content actually improves the security posture of the docs by explicitly warning that { raw: ... } bypasses escaping and reintroduces command injection when fed untrusted input.
Level of scrutiny
Low. This is prose in a .mdx file with no executable impact. I verified factual accuracy by grepping the shell source: the exact error strings appear in src/runtime/shell/states/Expansion.rs:683 and src/runtime/shell/states/Cmd.rs:799 / Builtin.rs:546, and the tokenizer only special-cases Asterisk/DoubleAsterisk/BraceBegin (confirming ? and [...] are literal). The <Note> component and import { Glob } from "bun" pattern are already used elsewhere in the docs tree, and the /runtime/glob link target exists.
Other factors
No CODEOWNERS entry covers docs/. No prior human review comments to address. The MDX structure is well-formed (balanced code fences, matched <Note> tags), and the anchor slugs for the new cross-links match the heading text under standard GitHub/MDX slugification. The PR description states every example was verified against a current build, and the outputs shown are consistent with the source I checked.
- Use echo instead of ls in the interpolation and {raw} examples so the
output comments match Bun's builtin (echo space-joins on one line;
ls prints one entry per line in unsorted order).
- Show .nothrow() on the ambiguous-redirect example and note that the
no-matches example throws, so no example presents an unreachable
follow-on await.
- Present Bun.Glob + array interpolation before {raw} in the
dynamic-patterns section, matching the safe-first ordering used in
the $.escape and Security sections, and add a 'trusted input only'
comment to the {raw} snippet.
- Add an empty-array guard to the Bun.Glob example and explain why:
interpolating [] contributes zero arguments rather than triggering
'no matches found'.
- Replace 'always a single literal argument' with wording that is
consistent with arrays becoming one argument per element.
|
Updated 7:44 AM PT - Jul 30th, 2026
✅ @robobun, your commit d39980f7f50501220a3078fc6ae054e96bc50a24 passed in 🧪 To try this PR locally: bunx bun-pr 36462That installs a local version of the PR into your bun-36462 --bun |
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 `@docs/runtime/shell.mdx`:
- Around line 622-623: Update the dynamic glob guidance near the raw example to
direct runtime or user-controlled patterns to Bun.Glob instead of recommending
raw. Clearly limit raw to trusted patterns so readers do not treat arbitrary
dynamic input as shell source.
🪄 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: 666dbed0-811e-4aea-bcd6-e7956f13fa03
📒 Files selected for processing (1)
docs/runtime/shell.mdx
The one-line cross-link under the {raw} example read as an unqualified
recommendation to use {raw} for dynamic glob patterns. Reword it to
restrict {raw} to trusted strings and point runtime/user-derived
patterns at Bun.Glob, matching the ordering in the Dynamic glob
patterns section.
There was a problem hiding this comment.
LGTM — docs-only addition documenting already-shipped shell glob/redirect behavior.
What was reviewed:
- Verified error strings against source:
no matches found(Expansion.rs:683) andbun: ambiguous redirect: at \...`` (Cmd.rs:799, Builtin.rs:546) match the doc examples. - Confirmed only
*/**/braces are pattern tokens (glob_hint()in shell_parser/parse.rs:706) — the claim that?,[...], and!are literal is accurate. - Checked link targets exist (
docs/runtime/glob.mdx, in-page anchors) andGlobis exported frombun. - CodeRabbit's
{ raw }safety concern was addressed in d39980f and the thread is resolved.
Extended reasoning...
Overview
This PR adds a Glob expansion section and an Ambiguous redirect targets subsection to docs/runtime/shell.mdx, plus two cross-links from the existing $.escape and Security sections. It documents behavior that shipped in PRs #31220 (interpolated glob metacharacters are literal) and #34324 (multi-word redirect target → ambiguous redirect). No source or test files are touched.
Security risks
None from the change itself — it's prose. The content discusses a security-relevant escape hatch ({ raw: ... }), and the guidance is safe-first: Bun.Glob is presented as the default for runtime patterns, { raw } is explicitly scoped to "a fixed string you control (not derived from user input)" with a closing warning that it "reintroduces command injection". The one CodeRabbit concern on this point was addressed in commit d39980f and confirmed resolved.
Level of scrutiny
Low. Docs-only, ~100 lines of MDX. The bar per .claude/docs/landing-prs.md is factual accuracy and voice consistency. I spot-checked every load-bearing factual claim against src/:
glob_hint()insrc/shell_parser/parse.rs:706-708matches onlyAsterisk | DoubleAsterisk, confirming?/[...]/!are literal.- Error message text in
src/runtime/shell/states/Expansion.rs:683andsrc/runtime/shell/states/Cmd.rs:799/Builtin.rs:546matches the doc examples verbatim. docs/runtime/glob.mdxexists;Globis exported frombun(bun.d.ts:8241); the<Note>component is already used elsewhere in this same file.- Anchor slugs (
#dynamic-glob-patterns,#interpolated-values-are-not-glob-patterns) match their heading text under standard MDX slugification.
Other factors
CI is green (Build #85819). The PR description states every example was run against a current build. The only reviewer feedback (CodeRabbit, on { raw } phrasing) is resolved. No prior review from me on this PR. The prose matches the surrounding doc's voice and structure (H2/H3 headings, fenced js blocks with import { $ } from "bun").
|
Heads up: #39706 changes one statement in this section. With that PR, a |
|
Note for whoever lands this: #43746 changes what this section describes. With that PR a pattern that matches nothing is passed to the command as written (POSIX 2.13.3, same as bash), and the If #43746 merges first, these parts need an update:
|
What changed
PR #31220 changed Bun Shell so that glob metacharacters arriving via
${...}interpolation,$varexpansion, command substitution, or quoted text are matched literally rather than as pattern syntax. PR #34324 made a redirect target that expands to more than one word anambiguous redirecterror instead of silently writing to a concatenated junk filename.docs/runtime/shell.mdxpreviously mentioned globs only in a one-line Features bullet, so neither the literal-interpolation rule nor the migration path for scripts that build glob patterns at runtime was discoverable from the docs.Why
Scripts that relied on the old behavior (building a pattern in a JS variable and interpolating it) silently stop globbing, and the only observable symptom is
ls: *.ts: No such file or directoryfrom the target command. The two working escape hatches ({ raw: pattern }andBun.Glob+ array interpolation) already exist but were only documented in the context of$.escape, not glob expansion.Additions
*,**,{a,b,...}) and that?,[...], and leading!are literalno matches foundfailure on zero matches{ raw: pattern }(trusted input) ornew Glob(pattern).scan()then pass the array$.escapeand Security sectionsVerification
Every example in the new sections was run against a current build:
Docs-only change; no source or test files touched.
no test proof · iteration 0 · docs-only change; test-proof not applicable