Harden read/copy/move path operations and enforce no-overwrite semantics#1
Conversation
Agent-Logs-Url: https://github.com/GhaziAlibi/softpath/sessions/7db4165a-56ca-493e-8fd4-f053a21fd429 Co-authored-by: GhaziAlibi <123127137+GhaziAlibi@users.noreply.github.com>
Agent-Logs-Url: https://github.com/GhaziAlibi/softpath/sessions/cf3e7309-c5c7-40a5-85af-529908a69b40 Co-authored-by: GhaziAlibi <123127137+GhaziAlibi@users.noreply.github.com>
Agent-Logs-Url: https://github.com/GhaziAlibi/softpath/sessions/46c4f7f5-2b5a-41bc-a85c-a588dbc1a029 Co-authored-by: GhaziAlibi <123127137+GhaziAlibi@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c3eaa78b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let dest_path = dest.as_ref(); | ||
| crate::utils::check_path_traversal(dest_path)?; | ||
| crate::utils::check_symlink_cycles(dest_path)?; | ||
| if dest_path.exists() { |
There was a problem hiding this comment.
move_to now relies on dest_path.exists() to enforce no-overwrite semantics, but Path::exists() returns false for dangling symlinks (and some metadata errors). In that case this guard is bypassed and fs::rename can still replace the destination path entry, so an existing symlink at dest is overwritten even though this change intends to reject all pre-existing destinations. This affects any call where dest is a broken symlink; use a symlink-aware existence check (for example via symlink_metadata/try_exists handling) before rename.
Useful? React with 👍 / 👎.
cargo test --verbose)