fix: refuse home dir when base path has a trailing separator - #890
dmtrKovalenko merged 1 commit into
Conversation
The home-directory guard compared raw OsStr bytes against dirs::home_dir(), so "/home/u/" slipped past a check written for "/home/u". fff-mcp hits this whenever $HOME is itself a git repository: git root discovery returns the root with a trailing slash, the guard never fires, and the whole home directory is indexed without --enable-home-scan. Compare as Path instead, which is component-wise and ignores the trailing separator. Adds a regression test that fails on the old comparison. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe home-directory guard now compares ChangesHome path guard
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The home-directory guard now handles the reported trailing-separator path. No actionable merge risk remains. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change strengthens the default refusal of home-directory scans for paths with trailing separators. No new exposure was identified, but behavior for filesystem aliases remains unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Root cause
The home-directory guard in
FilePicker::new(crates/fff-core/src/file_picker.rs:897-898) compares rawOsStrbytes:"/home/u/"and"/home/u"are different byte strings, so a base path with a trailing separator passes the guard.fff-mcpproduces exactly that path whenever$HOMEis itself a git repository. Git root discovery returns the root with a trailing slash, so starting the server with cwd =$HOMEand no--enable-home-scanindexes the whole home directory:(fff-mcp 0.11.0 / 95fd777, Linux x86_64 musl.)
Fix
Compare as
Pathinstead.Pathequality is component-wise and ignores the trailing separator:The watcher's own guard (
watcher/background_watcher.rs:68) already comparesPathBufs, so it was not affected.Test
refuses_home_dir_with_trailing_separatorbuilds aFilePicker(no walk, no watcher) at$HOMEand at$HOME/, and assertsErr(Error::FilesystemRoot(_))for both.FAILED ... home dir accepted as /home/mikes/ok;cargo test -p fff-search --lib→ 194 passed, 0 failedcargo fmt --checkis clean, and clippy reports no new warnings (28 before and after, none infile_picker.rs)🤖 Generated with Claude Code
Summary by CodeRabbit