Repository navigation
fix(permissions): resolve relative worktree edit paths - #1930
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 17 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
0xghost42
left a comment
There was a problem hiding this comment.
This looks correct and the root cause is well diagnosed. Traced it through: expandPath(tool.getPath(input)) with no explicit baseDir defaults to getCwd(), which resolves to the session cwd state (getCwdOverride() → getCwdState() → getOriginalCwd()), not the process cwd — so relative tool paths from a scoped/bridged session now resolve against the session's nested worktree exactly as intended, and the same absolute path is used for the symlink-variant gathering and the deny-rule checks. Both entry points that take raw tool input (checkReadPermissionForTool and checkWritePermissionForTool) are updated consistently, and the JSDoc invariant note is updated to match. The nested-worktree regression test using a real git worktree add is a nice touch — it exercises the actual failure path rather than a mock.
Two things worth confirming, neither blocking:
-
Are read/write the only permission entry points that receive a raw, possibly-relative tool input? If any other tool routes its path through a different permission check (or straight into
getPathsForPermissionCheckwith an un-expanded value), it would still hit the original bug — the doc explicitly warns that a path derived differently from the internal re-derivation "would silently check deny rules for the wrong path." A quick grep to confirm these two are the only raw-input sites would close that off. -
expandPathon an already-absolute path — worth a one-line confirmation (or an assertion in the test) that it's idempotent, so an absolutefile_pathisn't re-based against the session cwd. It should be, givenpath.resolvesemantics, but it's the kind of thing a future refactor could regress.
Nice fix.
Summary
Root cause
checkWritePermissionForToolpassed a relative path to the symlink-permission helper. That helper resolves its final path from the shared process CWD, which can differ from a scoped session's worktree CWD. The write was then classified outside the working directory and prompted despiteacceptEdits.Validation
bun install --frozen-lockfilebun test src/utils/permissions/filesystem.test.ts src/utils/permissions/permissions.test.ts tests/sdk/permissions.test.tsbun run typecheckbun run buildbun run security:pr-scan -- --base upstream/maingit diff --checkFinal independently reviewed SHA:
d036d01f3d0fa01fb79442ad2392eb7fde5fb0ebFixes #1910