fix(file_tools): resolve relative paths against TERMINAL_CWD for worktree isolation#12695
fix(file_tools): resolve relative paths against TERMINAL_CWD for worktree isolation#12695aniruddhaadak80 wants to merge 1 commit into
Conversation
…te file operations within the worktree as described in NousResearch#12689. Prevents operations from leaking back to the repository root directory.
There was a problem hiding this comment.
Pull request overview
This PR aims to fix worktree isolation in CLI -w mode by ensuring file tool path resolution uses TERMINAL_CWD (worktree base) rather than the host process CWD, preventing file operations from targeting the main repo unintentionally.
Changes:
- Added a
_resolve_path()helper to resolve relative paths againstTERMINAL_CWD. - Updated sensitive-path checks and read staleness tracking to use
_resolve_path(). - Updated
read_file_tool()’s internal resolved-path handling to use_resolve_path().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _resolve_path(filepath: str) -> Path: | ||
| """Resolve a path relative to TERMINAL_CWD (the worktree base directory) | ||
| instead of the main repository root. | ||
| """ | ||
| p = Path(filepath).expanduser() | ||
| if not p.is_absolute(): | ||
| base = os.environ.get("TERMINAL_CWD", os.getcwd()) | ||
| p = Path(base) / p | ||
| return p.resolve() |
There was a problem hiding this comment.
_resolve_path() resolves relative paths against TERMINAL_CWD, but the actual read/write/patch operations execute relative to the live terminal environment cwd (which can change after cd). This can desync safety checks/dedup/staleness from the file actually being touched (e.g., a relative write after cd /etc could bypass _check_sensitive_path; dedup can return a stale stub for the wrong file). Consider resolving relative paths against the per-task terminal env cwd (or the same effective cwd used by ShellFileOperations) rather than the static TERMINAL_CWD env var, and plumb task_id through as needed.
| def _resolve_path(filepath: str) -> Path: | ||
| """Resolve a path relative to TERMINAL_CWD (the worktree base directory) | ||
| instead of the main repository root. | ||
| """ | ||
| p = Path(filepath).expanduser() | ||
| if not p.is_absolute(): | ||
| base = os.environ.get("TERMINAL_CWD", os.getcwd()) | ||
| p = Path(base) / p | ||
| return p.resolve() |
There was a problem hiding this comment.
New TERMINAL_CWD-based path resolution is core to worktree isolation, but there are no tests asserting that read_file/write_file/patch resolve relative paths against TERMINAL_CWD (and not os.getcwd()). Adding a focused unit test would prevent regressions, especially around relative paths with .. segments.
Resolves #12689
What Changed
When launching Hermes with the worktree flag, file tools were resolving relative paths using
os.getcwd()orPath(path).resolve(), which points to the main repository root. This allowed operations to leak out of worktree isolation.This PR introduces a local
_resolve_pathhelper insidetools/file_tools.pyto ensure operations correctly resolve againstTERMINAL_CWDwhen it is set, preventing modification of files outside the worktree.