Skip to content

fix(engine): re-validate expanded brace patterns against safeGlob#424

Merged
rhuanbarreto merged 2 commits into
mainfrom
fix/glob-brace-expansion-safeglob
Jun 22, 2026
Merged

fix(engine): re-validate expanded brace patterns against safeGlob#424
rhuanbarreto merged 2 commits into
mainfrom
fix/glob-brace-expansion-safeglob

Conversation

@rhuanbarreto

Copy link
Copy Markdown
Contributor

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.

Test plan

  • New test: "blocks absolute paths produced by brace expansion" in runner-security.test.ts
  • All existing tests pass (1305 pass, 0 fail)
  • bun run validate passes (lint, typecheck, format, test, 39/39 ADR rules, knip, build)

https://claude.ai/code/session_01C3VSm9YHmfhZ9kLw2fkLE4

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().

Follow-up to #422, addressing CodeRabbit review feedback.

Claude-Session: https://claude.ai/code/session_01C3VSm9YHmfhZ9kLw2fkLE4
Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 81a78761-3c06-447a-95c2-9a590fc7ff84

📥 Commits

Reviewing files that changed from the base of the PR and between ac2f9a1 and 076d519.

📒 Files selected for processing (1)
  • tests/engine/runner-security.test.ts

📝 Walkthrough

Walkthrough

safeGlob() is now called on each per-alternative pattern produced by expandBracePattern() inside both the glob() and grepFiles() methods in src/engine/runner.ts. This ensures brace-expanded alternatives that resolve to absolute paths or contain .. are rejected even when the original user-supplied pattern passed initial validation. Two new regression tests in tests/engine/runner-security.test.ts exercise this path by passing {/etc/passwd,src/a.ts} to ctx.glob and ctx.grepFiles() respectively, asserting both result errors contain "access denied".

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main security fix: re-validating expanded brace patterns against safeGlob to block absolute paths that bypass initial checks.
Description check ✅ Passed The description clearly explains the vulnerability, the solution, and testing approach, providing sufficient context about why the fix is necessary and what was tested.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/glob-brace-expansion-safeglob

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 22, 2026

Copy link
Copy Markdown

Deploying archgate-cli with  Cloudflare Pages  Cloudflare Pages

Latest commit: 076d519
Status: ✅  Deploy successful!
Preview URL: https://3e8c3157.archgate-cli.pages.dev
Branch Preview URL: https://fix-glob-brace-expansion-saf.archgate-cli.pages.dev

View logs

@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Code Coverage

Metric Value
Lines 90.4% (6890 / 7618)
Threshold 90% minimum — met
Platforms Linux + Windows

Full HTML report available in workflow artifacts.

Per-directory breakdown
Directory Coverage Lines
src/commands/ 88.1% 1874 / 2128
src/engine/ 93.3% 1420 / 1522
src/formats/ 100.0% 142 / 142
src/helpers/ 90.3% 3454 / 3826

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@tests/engine/runner-security.test.ts`:
- Around line 183-200: Add a new regression test following the same pattern as
the existing "blocks absolute paths produced by brace expansion" test, but for
the grepFiles() method instead of glob(). The test should call ctx.grepFiles
with a regex pattern (like /./), and the same brace expansion string with
absolute path "{/etc/passwd,src/a.ts}". Verify that the result contains the
"access denied" error message, ensuring both the glob() and grepFiles() security
patches are protected against regression.
🪄 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: 5acc22b9-45d6-43ec-943c-dff96b269800

📥 Commits

Reviewing files that changed from the base of the PR and between 6eade7a and ac2f9a1.

📒 Files selected for processing (2)
  • src/engine/runner.ts
  • tests/engine/runner-security.test.ts

Comment thread tests/engine/runner-security.test.ts
Covers the grepFiles() path in addition to the existing glob() test,
ensuring both entry points re-validate expanded patterns via safeGlob().

Claude-Session: https://claude.ai/code/session_01C3VSm9YHmfhZ9kLw2fkLE4
Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
@rhuanbarreto rhuanbarreto merged commit 09593e7 into main Jun 22, 2026
19 checks passed
@rhuanbarreto rhuanbarreto deleted the fix/glob-brace-expansion-safeglob branch June 22, 2026 18:06
@archgatebot archgatebot Bot mentioned this pull request Jun 22, 2026
rhuanbarreto pushed a commit that referenced this pull request Jun 22, 2026
# 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant