Skip to content

fix: close case-insensitive blocklist bypass and dotfile filter gap#115

Open
sadegh wants to merge 2 commits into
bitbonsai:mainfrom
sadegh:fix/security-pathfilter
Open

fix: close case-insensitive blocklist bypass and dotfile filter gap#115
sadegh wants to merge 2 commits into
bitbonsai:mainfrom
sadegh:fix/security-pathfilter

Conversation

@sadegh
Copy link
Copy Markdown

@sadegh sadegh commented Apr 28, 2026

Summary

Two security fixes to PathFilter, both in src/pathfilter.ts:

1. Case-insensitive blocklist bypass (HIGH)

The blocklist regex was case-sensitive, so .Obsidian/app.json and .GIT/hooks/post-merge bypassed the .obsidian/** and .git/** patterns on macOS/Windows (case-insensitive filesystems). Fixed by adding the i flag to the compiled regex in simpleGlobMatch.

2. Dotfile filter gap (HIGH)

isFile() returns false for dotfiles (last component starts with .) because they have a dot at position 0. This caused dotfiles like .bashrc and notes/.env to skip the extension allowlist entirely. Fixed by explicitly denying dotfiles that don't end with an allowed extension in the else branch of isAllowed.

Extensionless non-dotfile paths (notes, 1. Project, attachments/folder) are unaffected — they're treated as directory names and pass through as before.

Chained impact: Combining both gaps enabled writing to .Git/hooks/post-merge on macOS (capital G bypasses .git/**, no extension bypasses the allowlist), which executes on the next git pull in a vault that is also a git repo.

Changes

  • src/pathfilter.ts — two targeted changes, no new methods or helpers
  • src/pathfilter.test.ts — 6 new regression tests across two describe blocks
  • src/filesystem.ts — comment added to moveFile documenting why isAllowedForListing is intentional for binary file moves (no logic change)
  • src/filesystem.test.ts — one test documenting the moveFile dotfile-destination behavior

All 187 tests pass (180 existing + 7 new).

Test coverage

New tests cover:

  • Mixed-case .obsidian, .OBSIDIAN, .ObSiDiAn paths
  • Mixed-case .Git, .GIT paths
  • Node_Modules bypass
  • .bashrc, .zshrc, notes/.env dotfile writes
  • .hidden.md (hidden file with allowed extension — should pass)
  • Extensionless non-dotfile paths still allowed (notes, Makefile, attachments/folder)
  • Documented moveFile dotfile-destination behavior

sadegh added 2 commits April 28, 2026 03:15
…n PathFilter

On macOS/Windows (case-insensitive filesystems), the blocklist regex was
case-sensitive, allowing paths like .Obsidian/app.json or .GIT/hooks/post-merge
to bypass protection against .obsidian and .git. Combined with the fact that
dotfiles and extensionless paths (where isFile() returns false) skipped the
extension allowlist entirely, this enabled an RCE chain: write a git hook via
.Git/hooks/post-merge and execute arbitrary code on the next git pull.

Fixes:
- simpleGlobMatch: add `i` flag to regex so blocklist matches case-insensitively
- isAllowed: deny dotfiles (.bashrc, .zshrc, notes/.env) that don't end with
  an allowed extension; extensionless non-dotfile paths (directories, Makefile)
  are still allowed since they can't escape the vault boundary
- moveFile: restore isAllowedForListing (intentional for binary file moves) with
  an explanatory comment; previously changed to isAllowed which broke .png moves

Adds regression tests covering all three fixes.
- Reduce pathfilter.ts block comment to one line (why, not how)
- Change .md test to .hidden.md — more realistic hidden markdown file
- Add comment in moveFile noting that newPath uses isAllowedForListing
  so dotfile destinations are not blocked (by design for binary moves)
- Add test documenting current move_file dotfile-destination behavior
ErycM added a commit to ErycM/mcpvault that referenced this pull request May 15, 2026
Anyone cloning this fork (including future-self) should immediately
understand (a) why it exists, (b) what's different from upstream, (c)
how the cherry-picks were security-audited before pulling them in, (d)
how to keep the fork in sync as upstream evolves, and (e) the plan to
eventually retire the fork once PRs bitbonsai#113/bitbonsai#115 + our own --allowed-extensions
all land upstream.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.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