fix(engine): pre-expand brace patterns with path separators in ctx.glob()#422
Conversation
…ob()
Bun.Glob.scan() silently returns empty results for brace patterns whose
alternatives contain path separators (e.g. `svc/{src/env.ts,env.ts}`),
while Bun.Glob.match() handles them correctly. This caused ctx.glob()
and ctx.grepFiles() in .rules.ts files to silently find nothing, making
rule authoring errors very hard to diagnose.
Add expandBracePattern() that detects {alt1,alt2} groups where at least
one alternative contains `/` and expands them into separate patterns
before scanning. Simple braces without `/` pass through unchanged since
Bun.Glob handles those natively.
Applied in ctx.glob(), ctx.grepFiles(), and resolveScopedFiles().
Upstream: oven-sh/bun#32596
Closes #421
Claude-Session: https://claude.ai/code/session_01C3VSm9YHmfhZ9kLw2fkLE4
Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
Deploying archgate-cli with
|
| Latest commit: |
66ce3cd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9a5be120.archgate-cli.pages.dev |
| Branch Preview URL: | https://fix-glob-brace-expansion-421.archgate-cli.pages.dev |
Code Coverage
Full HTML report available in workflow artifacts. Per-directory breakdown
|
📝 WalkthroughWalkthroughThe PR fixes a silent 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/engine/runner.ts`:
- Around line 87-98: The expandBracePattern function returns the pattern
unchanged when the first brace group lacks a forward slash, but this skips
checking for subsequent brace groups that may contain slashes and need
expansion. Instead of the early return when `!alternatives.includes("/")`,
recursively process the suffix by calling expandBracePattern on the suffix
parameter and combine each result with the prefix to properly handle multi-brace
patterns like `a/{b,c}/{d/e,f}` where later brace groups contain slashes even
though earlier ones do not.
In `@tests/engine/expand-brace-pattern.test.ts`:
- Around line 14-46: The test suite for expandBracePattern is missing coverage
for patterns with multiple brace groups at the same nesting level. Add a new
test case within the describe("expandBracePattern", ...) block that tests a
pattern like "a/{b,c}/{d/e,f}" to document the current behavior and serve as a
regression check. This test should verify how the function handles multiple
independent brace groups in a single pattern and will establish baseline
behavior for this scenario.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ebfdba52-ef57-45a7-99ee-31a6b198f799
📒 Files selected for processing (5)
.claude/agent-memory/archgate-developer/MEMORY.mdsrc/engine/git-files.tssrc/engine/runner.tstests/engine/expand-brace-pattern.test.tstests/engine/git-files.test.ts
When the first brace group has no path separators but a later one does
(e.g. `a/{b,c}/{d/e,f}`), the function was returning the pattern
unchanged, silently reintroducing the Bun.Glob.scan() bug. Now
recursively checks the suffix for additional brace groups that need
expansion.
Claude-Session: https://claude.ai/code/session_01C3VSm9YHmfhZ9kLw2fkLE4
Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
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/engine/runner.ts (1)
159-163:⚠️ Potential issue | 🔴 CriticalAdd re-validation of expanded patterns to prevent absolute-path bypass.
safeGlob()runs only on the raw input pattern, butexpandBracePattern()can produce absolute patterns that were never validated. For example, a pattern like{/absolute/path/*,relative/*}bypasses the initial check becauseisAbsolute()on the raw string returns false (it starts with{), yet the expansion yields an absolute pattern that Bun.Glob may not sandbox to the project root.Suggested fix
@@ const patterns = expandBracePattern(pattern); const seen = new Set<string>(); for (const p of patterns) { + safeGlob(p); const g = new Bun.Glob(p); @@ const globs = expandBracePattern(fileGlob); @@ const seen = new Set<string>(); for (const p of globs) { + safeGlob(p); const g = new Bun.Glob(p);Apply at both locations: lines 161 and 207.
🤖 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/engine/runner.ts` around lines 159 - 163, The expandBracePattern() function can produce absolute paths that bypass the initial safeGlob() validation since the raw input pattern may not be absolute (e.g., starting with a brace character). Add validation of each expanded pattern in the patterns array after expandBracePattern() is called but before they are used with Bun.Glob. Check each pattern returned by expandBracePattern() with isAbsolute() to ensure no absolute paths are processed, and apply this same validation at both occurrences where expanded patterns are used (around line 161 in the loop iteration and at line 207).
🤖 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/engine/runner.ts`:
- Around line 159-163: The expandBracePattern() function can produce absolute
paths that bypass the initial safeGlob() validation since the raw input pattern
may not be absolute (e.g., starting with a brace character). Add validation of
each expanded pattern in the patterns array after expandBracePattern() is called
but before they are used with Bun.Glob. Check each pattern returned by
expandBracePattern() with isAbsolute() to ensure no absolute paths are
processed, and apply this same validation at both occurrences where expanded
patterns are used (around line 161 in the loop iteration and at line 207).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c8502793-518b-4711-86c7-7fe8949f4916
📒 Files selected for processing (2)
src/engine/runner.tstests/engine/expand-brace-pattern.test.ts
## Summary
- `expandBracePattern()` can produce absolute paths that bypass the
initial `safeGlob()` check (e.g. `{/etc/passwd,foo}` starts with `{` so
`isAbsolute()` returns false, but expands to `/etc/passwd`)
- Add `safeGlob()` validation on each expanded pattern before passing to
`Bun.Glob` in both `ctx.glob()` and `ctx.grepFiles()`
- Add security test verifying that absolute paths produced by brace
expansion are blocked
Follow-up to #422, addressing [CodeRabbit review
feedback](#422 (review)).
## Test plan
- [x] New test: `"blocks absolute paths produced by brace expansion"` in
`runner-security.test.ts`
- [x] All existing tests pass (1305 pass, 0 fail)
- [x] `bun run validate` passes (lint, typecheck, format, test, 39/39
ADR rules, knip, build)
https://claude.ai/code/session_01C3VSm9YHmfhZ9kLw2fkLE4
---------
Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
# archgate ## [0.45.6](v0.45.5...v0.45.6) (2026-06-22) ### Bug Fixes * **engine:** pre-expand brace patterns with path separators in ctx.glob() ([#422](#422)) ([6eade7a](6eade7a)), closes [#421](#421) * **engine:** re-validate expanded brace patterns against safeGlob ([#424](#424)) ([09593e7](09593e7)), closes [#422](#422) --- This PR was generated with [simple-release](https://github.com/TrigenSoftware/simple-release). <details> <summary>📄 Cheatsheet</summary> <br> You can configure the bot's behavior through a pull request comment using the `!simple-release/set-options` command. ### Command Format ````md !simple-release/set-options ```json { "bump": {}, "publish": {} } ``` ```` ### Useful Parameters #### Bump | Parameter | Type | Description | |-----------|------|-------------| | `version` | `string` | Force set specific version | | `as` | `'major' \| 'minor' \| 'patch' \| 'prerelease'` | Release type | | `prerelease` | `string` | Pre-release identifier (e.g., "alpha", "beta") | | `firstRelease` | `boolean` | Whether this is the first release | | `skip` | `boolean` | Skip version bump | | `byProject` | `Record<string, object>` | Per-project bump options for monorepos | #### Publish | Parameter | Type | Description | |-----------|------|-------------| | `skip` | `boolean` | Skip publishing | | `access` | `'public' \| 'restricted'` | Package access level | | `tag` | `string` | Tag for npm publication | ### Usage Examples #### Force specific version ````md !simple-release/set-options ```json { "bump": { "version": "2.0.0" } } ``` ```` #### Force major bump ````md !simple-release/set-options ```json { "bump": { "as": "major" } } ``` ```` #### Create alpha pre-release ````md !simple-release/set-options ```json { "bump": { "prerelease": "alpha" } } ``` ```` #### Publish with specific access and tag ````md !simple-release/set-options ```json { "bump": { "prerelease": "beta" }, "publish": { "access": "public", "tag": "beta" } } ``` ```` ### Access Restrictions The command can only be used by users with permissions: - repository owner - organization member - collaborator ### Notes - The last comment with `!simple-release/set-options` command takes priority - JSON must be valid, otherwise the command will be ignored - Parameters apply only to the current release execution - The command can be updated by editing the comment or adding a new one </details> <!-- Please do not edit this comment. simple-release-pull-request: true simple-release-branch-from: release simple-release-branch-to: main --> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Summary
Bun.Glob.scan()silently returns empty results for brace patterns whose alternatives contain path separators (e.g.svc/{src/env.ts,env.ts}), whilematch()handles them correctlyexpandBracePattern()that detects{alt1,alt2}groups containing/and expands them into separate patterns before scanning — simple braces without/pass through unchangedctx.glob(),ctx.grepFiles(), andresolveScopedFiles()so rule authors and ADRfiles:globs work correctly with brace expansionCloses #421
Test plan
expandBracePattern()— no braces, simple braces (no/), braces with/, multiple alternatives, suffix handling, no-prefix patternsctx.glob()andctx.grepFiles()with brace patterns containing path separatorsresolveScopedFiles()with brace patterns containing path separators/) still work natively via Bun.Globbun run validatepasses (lint, typecheck, format, 1303 tests, 39/39 ADR rules, knip, build)https://claude.ai/code/session_01C3VSm9YHmfhZ9kLw2fkLE4