Conversation
All filesystem tools previously validated user-supplied paths through ad-hoc inline checks (is_absolute, trim_start_matches, etc.) with no protection against symlink escape attacks. This commit replaces those scattered checks with a shared `check_workspace_path` pipeline in `fs/utils.rs` and two public entry points: - `resolve_workspace_path` — canonicalizes through symlinks on existing ancestors, guaranteeing the result lies inside the workspace root. Used by write tools (`create_file`, `delete_file`, `modify_file`, `move_file`, `read_file`). - `clean_workspace_path` — lexically normalizes without following symlinks, while still verifying the canonical ancestor stays in-root. Used by read/search tools (`grep_files`, `list_files`) where the returned path should match the user's input shape. The validator rejects absolute paths, `..`-escape attempts that survive normalization, symlinks whose canonical target falls outside the root, path components exceeding 100 bytes, and paths with more than 20 components. `fs_move_file` was also expanded to handle directories in addition to files: directories are moved atomically with `fs::rename`, require the target to not already exist, and get a broader dirty check via `count_dirty_paths_impl` (any non-empty `git status --porcelain` line counts). A same-path guard prevents no-op moves, and the success message now distinguishes between file and directory moves. The tool description in `.jp/mcp/tools/fs/move_file.toml` is updated to reflect the new capability. Signed-off-by: Jean Mertz <git@jeanmertz.com>
Signed-off-by: Jean Mertz <git@jeanmertz.com>
Signed-off-by: Jean Mertz <git@jeanmertz.com>
Signed-off-by: Jean Mertz <git@jeanmertz.com>
Signed-off-by: Jean Mertz <git@jeanmertz.com>
JeanMertz
added a commit
that referenced
this pull request
May 19, 2026
…658) Signed-off-by: Jean Mertz <git@jeanmertz.com>
JeanMertz
added a commit
that referenced
this pull request
May 19, 2026
…658) Signed-off-by: Jean Mertz <git@jeanmertz.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All filesystem tools previously validated user-supplied paths through ad-hoc inline checks (is_absolute, trim_start_matches, etc.) with no protection against symlink escape attacks. This commit replaces those scattered checks with a shared
check_workspace_pathpipeline infs/utils.rsand two public entry points:resolve_workspace_path— canonicalizes through symlinks on existing ancestors, guaranteeing the result lies inside the workspace root. Used by write tools (create_file,delete_file,modify_file,move_file,read_file).clean_workspace_path— lexically normalizes without following symlinks, while still verifying the canonical ancestor stays in-root. Used by read/search tools (grep_files,list_files) where the returned path should match the user's input shape.The validator rejects absolute paths,
..-escape attempts that survive normalization, symlinks whose canonical target falls outside the root, path components exceeding 100 bytes, and paths with more than 20 components.fs_move_filewas also expanded to handle directories in addition to files: directories are moved atomically withfs::rename, require the target to not already exist, and get a broader dirty check viacount_dirty_paths_impl(any non-emptygit status --porcelainline counts). A same-path guard prevents no-op moves, and the success message now distinguishes between file and directory moves. The tool description in.jp/mcp/tools/fs/move_file.tomlis updated to reflect the new capability.