fix(relay): refuse recursive fs.watch on home/filesystem roots#7948
fix(relay): refuse recursive fs.watch on home/filesystem roots#7948alpiyev99 wants to merge 1 commit into
Conversation
A watch rooted at the account home makes @parcel/watcher brute-force crawl the entire tree (container storage, ML model dirs — millions of entries). Observed live: one libuv worker pinned for 90+ min, 1.4 GB RSS, every fs request starved, all workspaces on the connection frozen. This surfaced only after native deps began installing successfully (the old relay logged "File watcher not available" and never loaded parcel). Refuse home, filesystem roots, and ancestors of home; callers already tolerate a missing watcher (same as the no-parcel path). Log every accepted watch root so a broad-root requester is traceable in relay.log. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a helper 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/relay/fs-handler.ts (1)
44-60: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winResolve
rootPathbefore the broad-root check.
expandTilde()leaves..components untouched, so inputs like/home/user/../../can bypass this string comparison. Normalize both sides withpath.resolve()before checking.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2aedf987-e072-4f0d-994c-fc1eacdfb094
📒 Files selected for processing (2)
src/relay/fs-handler.test.tssrc/relay/fs-handler.ts
| it('fs.watch refuses broad roots (home, /, ancestors of home) without subscribing', async () => { | ||
| // Why: a home-rooted recursive watch makes @parcel/watcher crawl the whole | ||
| // account tree (container storage, model dirs) and starves the relay. | ||
| await expect( | ||
| dispatcher.callRequest('fs.watch', { rootPath: homedir() }) | ||
| ).resolves.toBeUndefined() | ||
| await expect(dispatcher.callRequest('fs.watch', { rootPath: '~' })).resolves.toBeUndefined() | ||
| await expect(dispatcher.callRequest('fs.watch', { rootPath: '/' })).resolves.toBeUndefined() | ||
| // No watch state registered — unwatch for those roots is a no-op, not a crash. | ||
| dispatcher._notificationHandlers.get('fs.unwatch')?.( | ||
| { rootPath: homedir() }, | ||
| { | ||
| clientId: 0, | ||
| isStale: () => false | ||
| } | ||
| ) | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Test doesn't verify the refusal actually prevented subscription.
watch returns undefined in both the broad-root refusal path and the normal path, so toBeUndefined() passes even if isBroadWatchRoot is broken and @parcel/watcher is subscribed. The test should assert mockSubscribe was not called to actually verify the refusal behavior. Additionally, the ancestor-of-home branch of isBroadWatchRoot (home.startsWith(${norm}/)) is not exercised — adding a case for path.dirname(homedir()) would cover it.
🧪 Proposed fix: assert mockSubscribe not called + add ancestor test
it('fs.watch refuses broad roots (home, /, ancestors of home) without subscribing', async () => {
// Why: a home-rooted recursive watch makes `@parcel/watcher` crawl the whole
// account tree (container storage, model dirs) and starves the relay.
await expect(
dispatcher.callRequest('fs.watch', { rootPath: homedir() })
).resolves.toBeUndefined()
await expect(dispatcher.callRequest('fs.watch', { rootPath: '~' })).resolves.toBeUndefined()
await expect(dispatcher.callRequest('fs.watch', { rootPath: '/' })).resolves.toBeUndefined()
+ // Ancestor of home (e.g. /home when home is /home/user) is just as broad.
+ await expect(
+ dispatcher.callRequest('fs.watch', { rootPath: path.dirname(homedir()) })
+ ).resolves.toBeUndefined()
+ // Verify no subscription was actually created for any refused root.
+ expect(mockSubscribe).not.toHaveBeenCalled()
// No watch state registered — unwatch for those roots is a no-op, not a crash.
dispatcher._notificationHandlers.get('fs.unwatch')?.(
{ rootPath: homedir() },
{
clientId: 0,
isStale: () => false
}
)
})Verify that mockSubscribe is the correct mock for @parcel/watcher's subscribe:
#!/bin/bash
# Description: Verify mockSubscribe mocks `@parcel/watcher` subscribe
rg -n 'mockSubscribe|parcel.*watcher|vi\.mock' src/relay/fs-handler.test.ts -B 2 -A 5
2 participants
Summary
On a remote SSH host, adding/opening projects could freeze every workspace on the connection: the file explorer and Source Control hang, remote terminals stall, and it never recovers on its own. Root cause: the relay ends up recursively watching the account home directory, and
@parcel/watcherbrute-force crawls the entire tree.This guards the relay against recursive watches on
$HOME, filesystem roots, and ancestors of$HOME.Root cause (traced live on a real host)
The worktree base-directory watcher (
buildWorktreeBaseDirectoryWatchTargets→maybeAddBaseTarget) watches each repo'sworkspaceRootso it can notice worktrees created/removed outside Orca. When a user's remote repos all live directly under their home (e.g.~/ERP,~/Local-ai,~/HyDE, …),workspaceRootresolves to$HOMEitself.The relay then calls
@parcel/watcher.subscribe($HOME, …). parcel is recursive and its ignore list (WATCHER_IGNORE_DIRS:node_modules,.git,dist,build,.next,.cache,target,.venv,__pycache__) does not include~/.local. On a real dev box$HOMEcontains rootless container storage (~/.local/share/docker/containerd/**,~/.local/share/containers/**) and ML model dirs — millions of files. parcel's initial crawl:so
fs.readDir/fs.statfor all workspaces on that connection exceed the 30s request timeout and the UI freezes. It only surfaced once native deps installed successfully — before that the relay loggedFile watcher not availableand never loaded parcel.Captured directly from
relay.logwith this change deployed:With the guard in place the relay settled at ~227 MB and idle CPU; without it, the same action pinned a core and grew to 1.4 GB.
Change
src/relay/fs-handler.ts:isBroadWatchRoot(rootPath)— true for/,$HOME, and any ancestor of$HOME.watch()refuses those roots before importing/subscribing parcel and returns (callers already tolerate a missing watcher — same behavior as the no-parcel path). A recursive watch on the whole home directory has no legitimate use; only the base dir's immediate entries matter, which the existing base-directory poller covers.watch start: <root>) so a broad-root requester stays traceable.src/relay/fs-handler.test.ts:fs.watchonhomedir(),~, and/resolves without subscribing, and a follow-upfs.unwatchis a no-op (not a crash).Notes / follow-up
This is a relay-side backstop that catches any caller. The cleaner root fix is main-side:
worktree-base-directoryshould not request a recursive watch whenworkspaceRootis the home dir / a filesystem root — it should rely onworktree-base-directory-poller(a shallow one-level readdir) instead. Happy to follow up with that if preferred. One caveat of the relay-side guard alone: the main process sees the watch as "succeeded", so it won't itself switch to polling for that root — acceptable (Orca still picks up worktree changes on the next refresh), but the main-side fix would preserve live base-dir detection.Tested:
pnpm run typecheckclean; relay tests pass (43); verified live on a Linux SSH host as above.