fix(validation): accept absolute paths in validate_path_relative_to when working_dir is set - #1345
Conversation
…hen working_dir is set Signed-off-by: Hugues Clouatre <hugues@linux.com>
There was a problem hiding this comment.
Aptu Review
✅ Approve — This PR updates validate_path_relative_to to allow absolute paths when a working_dir is provided, resolving an issue where absolute paths were incorrectly rejected. It achieves this by bypassing validate_parent_in_root for new files and instead canonicalizing the parent directory to ensure it exists and is a valid directory.
Posted by aptu
…tory errors Signed-off-by: Hugues Clouatre <hugues@linux.com>
There was a problem hiding this comment.
Aptu Review
✅ Approve — This PR updates validate_path_relative_to to correctly handle absolute paths when an explicit working_dir is provided. It replaces the overly restrictive validate_parent_in_root check with a direct resolution strategy that respects the operator-defined scope while maintaining security against path traversal via canonicalize.
Posted by aptu
…lution Signed-off-by: Hugues Clouatre <hugues@linux.com>
There was a problem hiding this comment.
Aptu Review
✅ Approve — This PR updates validate_path_relative_to to correctly support absolute paths when an explicit working_dir is provided. It removes the restrictive validate_parent_in_root check for the require_exists=false case, shifting the responsibility of containment to the caller as per the MCP specification, while maintaining robust path resolution and error reporting.
Posted by aptu
fix(validation): accept absolute paths in
validate_path_relative_towhenworking_diris setProblem
edit_overwriterejected absolute paths even when the caller supplied an explicitworking_dir. For example, writingAGENTS.mdto/Users/hugues/git/career/while the MCP server CWD is the aptu-coder repo failed with "path is outside the working directory".Root cause
validate_path_relative_todelegated tovalidate_parent_in_rootfor therequire_exists=falsebranch. That function enforcescanonical_parent.starts_with(root)-- correct when called fromvalidate_pathto keep paths inside the server CWD, but wrong when an explicitworking_diris provided. The containment boundary forworking_dircallers is the orchestrator's responsibility, not the server's; the server's job is path resolution. The delegated call rejected any parent that did not sit inside the server CWD, ruling out all absolute paths.Fix
Replace the
validate_parent_in_rootdelegation invalidate_path_relative_to'srequire_exists=falsebranch with minimal path resolution:.././ trailing-slash paths (nofile_namecomponent).canonical_working_dir(Path::joinhandles absolute right-hand sides by discarding the left side, so absolute paths resolve correctly).canonicalizethe parent -- resolves symlinks and requires the directory to exist.canonical_parent.join(file_name).No containment check.
validate_path_relative_tois a path resolver; access control is the orchestrator's job.validate_parent_in_rootis unchanged and continues to enforce CWD containment forvalidate_path(noworking_dir).Tests
test_validate_path_in_dir_traversal_to_sibling_accepted_with_working_dir: asserts that../sibling/fileis accepted when the caller provides an explicitworking_dir; containment enforcement is the orchestrator's responsibility.test_validate_path_in_dir_rejects_sibling_prefix_validate_path: confirmsvalidate_path(CWD-scoped, noworking_dir) still rejects paths outside the server CWD.test_validate_path_relative_to_accepts_absolute_path_with_working_dir: new happy-path -- absolute path withworking_diroutside the server CWD resolves correctly.All 152 tests pass.
cargo clippy -- -D warningsandcargo fmt --checkclean.